john/firstmate
Watch
1
0
Fork
You've already forked firstmate
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(teardown): name branch and push-PR remedy in committed-but-never-pushed refusal #1

Merged
john merged 1 commit from fm/fm-delivery-gate into main 2026-08-07 15:26:14 +00:00
Conversation 0 Commits 1 Files changed 4 +252 -8
john commented 2026-08-06 20:58:32 +00:00
Owner
Copy link

What this changes

On 2026-08-06 a ship worker appended done: having only committed locally, six times across four runtimes (Claude, Codex, Pi/GLM, Pi/qwopus): the branch was never pushed and no pull request existed. Two were caught only because firstmate independently checked whether the PR URL resolved.

The enforcement condition already existed. bin/fm-teardown.sh's validate_worktree_teardown_safety already refused a branch whose commits exist on no remote when work_is_landed could not prove a merged PR or content already in the default branch - test_no_mistakes_truly_unpushed_refuses and matrix row (e) already asserted REFUSE at the base commit. This change does NOT introduce that refusal; it improves a guard that is now demonstrably load-bearing. It is the last line that would have caught those six premature done reports.

The most useful fact for whoever reads this next: the guard was already there, and still six workers reported done early. The gap was not a missing refusal - it was that the refusal did not make the remedy obvious.

What was actually missing, and what this fixes

  • The refusal fired but did not NAME the branch or state that the remedy is to push and open the PR, so an operator hitting it had to reconstruct why. It now names the branch, says the work is committed but never pushed, and gives the push-and-open-PR remedy (not a --force nudge).
  • Ambiguous delivery mode was not documented as deliberate. Any non-local-only mode - including one that cannot be determined from recorded metadata - now fails closed under the same refusal rather than silently allowing cleanup.
  • Coverage was thinner than the failure rate justified. Seven new tests in tests/fm-teardown.test.sh cover: no-mistakes and direct-PR unpushed refusal naming the branch and remedy, pushed-branch not refused, unrecognized-mode fail-closed, local-only not caught, scout unaffected, and the --force escape hatch.
  • Nothing on the prevention side told a worker that committed is not done. bin/fm-brief.sh's direct-PR definition-of-done now states plainly that committing is not done - done is the pull request.

Scope and invariants

  • work_is_landed is unchanged, so the squash-merge-then-delete-branch flow and the content-in-default fallback still allow genuinely landed teardowns.
  • local-only (delivers a local-main merge) and scout (delivers a report) stay excluded exactly as before.
  • The existing --force explicit-discard override still works; no new bypass is added.
  • The existing uncommitted-work and landed-work refusals behave exactly as before.
  • bin/fm-lint.sh passes and bin/*.sh stays shellcheck-clean. A pre-existing SC2086 in tests/fm-spawn-dispatch-profile.test.sh (a word-split flag string, present at the base commit) was also cleared so the lint gate is green.

Pre-existing, not caused by this change (flagged for transparency)

  • bin/fm-doc-audience-check.sh fails on 7 unclassified imported skill pages (before-building, handoff, next-decision, pi-custom-model, to-spec, to-tickets, wayfinder) present at the base commit a5ec819. Classifying them is a separate change against that skills import; this change touches no documentation surface and did not cause it.

Test plan

  • bash tests/fm-teardown.test.sh - 46 ok, 0 not ok (39 existing + 7 new).
  • bash tests/fm-brief.test.sh - 20 ok, 0 not ok.
  • bin/fm-lint.sh - PASS.
## What this changes On 2026-08-06 a ship worker appended `done:` having only committed locally, six times across four runtimes (Claude, Codex, Pi/GLM, Pi/qwopus): the branch was never pushed and no pull request existed. Two were caught only because firstmate independently checked whether the PR URL resolved. **The enforcement condition already existed.** `bin/fm-teardown.sh`'s `validate_worktree_teardown_safety` already refused a branch whose commits exist on no remote when `work_is_landed` could not prove a merged PR or content already in the default branch - `test_no_mistakes_truly_unpushed_refuses` and matrix row (e) already asserted REFUSE at the base commit. This change does NOT introduce that refusal; it improves a guard that is now demonstrably load-bearing. It is the last line that would have caught those six premature `done` reports. The most useful fact for whoever reads this next: the guard was already there, and still six workers reported done early. The gap was not a missing refusal - it was that the refusal did not make the remedy obvious. ## What was actually missing, and what this fixes - The refusal fired but did not NAME the branch or state that the remedy is to push and open the PR, so an operator hitting it had to reconstruct why. It now names the branch, says the work is committed but never pushed, and gives the push-and-open-PR remedy (not a `--force` nudge). - Ambiguous delivery mode was not documented as deliberate. Any non-local-only mode - including one that cannot be determined from recorded metadata - now fails closed under the same refusal rather than silently allowing cleanup. - Coverage was thinner than the failure rate justified. Seven new tests in `tests/fm-teardown.test.sh` cover: no-mistakes and direct-PR unpushed refusal naming the branch and remedy, pushed-branch not refused, unrecognized-mode fail-closed, local-only not caught, scout unaffected, and the `--force` escape hatch. - Nothing on the prevention side told a worker that committed is not done. `bin/fm-brief.sh`'s direct-PR definition-of-done now states plainly that committing is not done - done is the pull request. ## Scope and invariants - `work_is_landed` is unchanged, so the squash-merge-then-delete-branch flow and the content-in-default fallback still allow genuinely landed teardowns. - local-only (delivers a local-main merge) and scout (delivers a report) stay excluded exactly as before. - The existing `--force` explicit-discard override still works; no new bypass is added. - The existing uncommitted-work and landed-work refusals behave exactly as before. - `bin/fm-lint.sh` passes and `bin/*.sh` stays shellcheck-clean. A pre-existing SC2086 in `tests/fm-spawn-dispatch-profile.test.sh` (a word-split flag string, present at the base commit) was also cleared so the lint gate is green. ## Pre-existing, not caused by this change (flagged for transparency) - `bin/fm-doc-audience-check.sh` fails on 7 unclassified imported skill pages (`before-building`, `handoff`, `next-decision`, `pi-custom-model`, `to-spec`, `to-tickets`, `wayfinder`) present at the base commit `a5ec819`. Classifying them is a separate change against that skills import; this change touches no documentation surface and did not cause it. ## Test plan - `bash tests/fm-teardown.test.sh` - 46 ok, 0 not ok (39 existing + 7 new). - `bash tests/fm-brief.test.sh` - 20 ok, 0 not ok. - `bin/fm-lint.sh` - PASS.
fix(teardown): refuse cleanup of committed-but-never-pushed PR-based work
Some checks failed
CI / Lint shell scripts (pull_request) Has been cancelled
Details
CI / Test coverage guard (pull_request) Has been cancelled
Details
CI / Behavior portable parallel 1 (pull_request) Has been cancelled
Details
CI / Behavior portable parallel 2 (pull_request) Has been cancelled
Details
CI / Behavior portable serial 1 (pull_request) Has been cancelled
Details
CI / Behavior portable serial 2 (pull_request) Has been cancelled
Details
CI / Behavior portable serial 3 (pull_request) Has been cancelled
Details
CI / Behavior portable serial 4 (pull_request) Has been cancelled
Details
CI / Behavior tests (Herdr) (pull_request) Has been cancelled
Details
CI / Behavior timing aggregate (pull_request) Has been cancelled
Details
CI / Stock macOS Bash snapshot compatibility (pull_request) Has been cancelled
Details
CI / Repo invariants (pull_request) Has been cancelled
Details
Require no-mistakes / PR must be raised via no-mistakes (pull_request) Has been cancelled
Details
97359ee068
 
Six times on 2026-08-06 a ship worker appended done: having only committed
locally; its branch was never pushed and no PR existed. fm-teardown.sh
already refused uncommitted work and unpushed-unlanded work, but its refusal
message for the PR-based modes did not name the branch or distinguish
committed-but-undelivered from uncommitted, leaving --force as a co-equal
remedy instead of pushing and opening the PR.

Extend the existing unlanded-work refusal in validate_worktree_teardown_safety
in its established shape (no parallel check): for a ship task whose recorded
delivery mode is no-mistakes or direct-PR, when its branch carries commits on
no remote and work_is_landed cannot prove a merged PR or content already in the
default branch, refuse by branch name stating the work is committed but never
pushed, with the remedy of pushing and opening (or landing) its pull request.
work_is_landed is unchanged, so the squash-merge-then-delete-branch flow and
the content-in-default fallback still allow genuinely landed teardowns.

local-only (delivers a local-main merge) and scout (delivers a report) stay
excluded exactly as before, and an undeterminable delivery mode fails closed
under the same refusal rather than silently allowing cleanup. --force remains
the only explicit-discard override; no new bypass is added.

Secondarily, sharpen the direct-PR definition-of-done in bin/fm-brief.sh so
committing is unmistakably not done: done is the pull request.

Also clear a pre-existing SC2086 in tests/fm-spawn-dispatch-profile.test.sh
(quoting a word-split flag string into an array) so bin/fm-lint.sh is green.

Tests extend tests/fm-teardown.test.sh with matrix coverage for the
committed-but-never-delivered refusal through the executable interface.
john merged commit 8576b4fe42 into main 2026-08-07 15:26:14 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
john/firstmate!1
No description provided.