Skip to content

fix(ship): a failing git status refuses instead of reading as a clean tree (CC-629 b) - #673

Merged
screenleon merged 3 commits into
mainfrom
fix/CC-629b-ship-dirty-and-dispatch-hashes
Oct 4, 2026
Merged

screenleon merged 3 commits into
mainfrom
fix/CC-629b-ship-dirty-and-dispatch-hashes

Conversation

@screenleon

Copy link
Copy Markdown
Owner

What

CC-629 group (b), the same bug class as CC-627 (#671) and CC-629 (a) (#672): a failed git status read as a clean tree in pmctl ship.

runtime/lib/pmctl-ship.sh tested [[ -n "$(git status ...)" ]] (or assigned the output without looking at git's status) in these places, so a git that failed (a killed git, a damaged index, a safe.directory rejection, an unreadable gitdir) was an empty, clean tree:

  • the three publication guards of ship finish (after the gate, after the full suite, immediately before the irreversible push/PR): the guard let the push through;
  • the two status reads of a dispatched lane's auto-commit (before and after patching .gitignore): the lane went on to the gate with nothing staged;
  • the dirty-tree precondition of ship prepare: it created its feature branch.

The change

  • New _pmctl_ship_require_clean_tree <work_dir> <dirty-message> replaces the three copy-pasted guard blocks: 0 only when git status could be read and is empty; a dirty tree is refused with the same messages as before; a failed status is refused with "unable to read the worktree status (git status failed: <git's first message>) -- refusing ... Fix the repository state, or re-run if the failure was transient." The auto-commit reads and prepare got explicit checks with the same report (_pmctl_ship_report_status_failure; it runs the status once more to get git's message, empty if it then works).
  • pr-gate.sh: the six working-tree fingerprints of the injection check (before dispatch, after the reviewer sessions, after synthesis) print "unable to fingerprint the working tree (git diff HEAD or the hash step failed)" before exit 1.

Correction to the CC-629 ticket text: it called the pr-gate.sh fingerprints fail-open ("both sides failing gives equal hashes"). They are not: all six statements sit at top level of the script body under set -euo pipefail (set +e appears once, in a QA helper that does not enclose them), so a failing git already stopped the gate with exit 1, silently. I verified the construct (X=$(false | sha256sum) aborts), the critic and the security review confirmed no call context suppresses errexit, and artifact_filter_porcelain ends in return 0 so it cannot mask a failing git status. This part only adds the message; BACKLOG is corrected in this PR.

No change for a working git: the dirty messages are byte-identical to main's, the status pathspec and exit codes are untouched. A plain revert is safe (nothing is persisted).

Review (critic, qa-tester, security, risk, architecture): nothing blocked; fixed here

  1. Tests that did not pin where the gate stops (qa-tester): removing the exit 1 of a pr-gate handler survived, because only the Nth call failed, the gate ran on and exited 1 later on a hash mismatch. The wrapper now fails every call from the Nth on, and the cases assert that exactly N numbered calls happened and that no "modified working tree" line appeared. Mutants killed (handler without exit 1 at the pre-dispatch diff and the post-synthesis status).
  2. Coverage of all six fingerprints (qa-tester, critic, architecture): added the before-dispatch status leg and a new after-synthesis case; the finish test no longer runs useless legs (it runs as many as the control's git status call count).
  3. Diagnostics (security, risk, critic, architecture): git's own message is in the refusal; the dispatched lane says nothing was committed or pushed and that re-running is safe (.gitignore may already carry the bookkeeping patterns); the injection-check message no longer claims git was the failing stage (under pipefail it may be the filter or the hash tool).

Not in this PR

  • gate-git.sh (shared helper for CC-627/629a) and a shared tests/lib/git-stub.sh test fixture (the wrapper technique now exists in five suites): own PRs, see CC-629 / CC-631 (migrating suites inside a bugfix PR is risky on a host where many suites cannot run cleanly).
  • Findings that predate this change and are advisory (CC-632): the finish-marker exclusion :(exclude).pm-dispatch-ship-finish.json is a prefix pathspec (a directory of that name hides its untracked files from status; the fingerprint still hashes them and trips the last guard); the fingerprint reads do not pin --untracked-files=all --ignore-submodules=none -c core.fsmonitor=false; pr-gate.sh _worktree_is_dirty and gate-result-verify.sh ~1767 read ls-files through $(...) in a test.

Evidence

  • Windows (Git Bash): the three ship cases (ship finish clean GO with every status call failed in turn, the dispatched lane's two reads, prepare) and the three pr-gate cases (injection-check-git-failure-before/after-dispatch/after-synthesis, about 3 / 2 / 2 minutes on this host) pass; real Linux (WSL): the three ship cases pass.

  • Mutants killed: main's pmctl-ship.sh; the guard helper ignoring the failure; each of the three guard sites reverted to the old form; pre_ensure_status, dirty_status and prepare ignoring the failure; their messages dropped; a pr-gate handler without exit 1.

  • lint-shellcheck, lint-test-docstrings, lint-test-suite-registry, lint-script-domain-inventory pass. The whole test-pmctl-ship-finish.sh and test-pr-gate.sh were not run (a few finish cases fail on this host with and without the change: gh unavailable, publish assessment).

  • Permanent test admissions: six new cases: ship finish (case_finish_unreadable_worktree_status_refuses_push, case_finish_dispatched_lane_unreadable_status_refuses) in tests/shell/test-pmctl-ship-finish.sh, ship prepare (case_prepare_unreadable_worktree_status_refuses) in tests/shell/test-pmctl-ship.sh, and the injection check (test_injection_check_git_failure_before_dispatch_is_reported, test_injection_check_git_failure_after_dispatch_is_reported, test_injection_check_git_failure_after_synthesis_is_reported) in tests/shell/test-pr-gate.sh

🤖 Generated with Claude Code

https://claude.ai/code/session_011c6rVDk6zyfgLPEfbVrZi9

screenleon and others added 3 commits October 4, 2026 12:32
…the injection check names the git failure (CC-629 b)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011c6rVDk6zyfgLPEfbVrZi9
…on-check tests pin where the gate stops

- the unable-to-read refusals carry git's first message and a remedy; the dispatched lane says
  nothing was committed and re-running is safe
- the injection-check message says "git diff HEAD or the hash step failed" (under pipefail the
  failing stage may be the filter or the hash tool)
- tests: the wrapper fails every call from the Nth on, the cases assert the gate stopped at that call
  (a handler without exit 1 is caught), all six fingerprints are covered (new after-synthesis case,
  before-dispatch status leg), the finish test runs only as many legs as the control has status calls

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011c6rVDk6zyfgLPEfbVrZi9
…open CC-631, CC-632

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011c6rVDk6zyfgLPEfbVrZi9
@screenleon
screenleon merged commit 3da6610 into main Oct 4, 2026
81 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant