Repository navigation
fix: enforce acceptance and check guards before merging - #212
Merged
Merged
Conversation
fm-pr-merge.sh and fm-merge-local.sh now refuse a ship task with any accepted-blocked acceptance criterion, and fm-pr-merge.sh refuses a PR whose reported checks are failing, pending, or unreadable. Each refusal names the reason and --captain-instruction, the only override; its verbatim words are recorded in data/<id>/captain-merge-instructions.jsonl before the merge runs. The green, no-accepted-blocked path is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Captain 2026-10-06 "go all of them" (follow-ups of the receipt-simplification review, after PR #211 merged as 1608b2c): add mechanical merge guards. Two rules lived only as instructions in AGENTS.md section 7: never auto-merge a task with any accepted-blocked acceptance criterion, and never merge a red PR. Make bin/fm-pr-merge.sh and bin/fm-merge-local.sh enforce them.
Constraints (from the captain/firstmate brief):
Acceptance criteria:
Implementation decisions: shared bin/fm-merge-guard-lib.sh owns the accepted-blocked guard, flag validation (one non-blank line), and the durable override record (fm-merge-override.v1 JSONL appended to data//captain-merge-instructions.jsonl, which survives teardown unlike state/.meta) written before the merge only when the flag actually overrides a reason. Guard applies only to kind=ship tasks (mirrors fm-pr-check's evidence gate); unreadable evidence or unreadable checks refuse (overridable). Checks are read with gh pr checks --json name,bucket (pass/skipping green, pending pending, everything else failing), falling back to gh-axi pr checks when gh is absent or fails, mirroring the script's existing gh-primary/gh-axi-fallback outcome read; a PR with no reported checks ("no checks reported") is not red, so repos without CI still merge. Guards run before PR metadata recording so a refused merge records nothing. In fm-merge-local the guard runs after the fast-forward safety checks, just before the merge; task ids are now validated.
What Changed
--captain-instructionto both merge commands, validating one non-blank line and durably recording the verbatim instruction and overridden reasons before merging.Risk Assessment
✅ Low: Captain, the guards are bounded, conform to the accepted intent and subsequent decisions, and introduce no substantiated material defects.
Testing
Both targeted Bash test files passed. Real local merges demonstrated the guards and malformed-receipt fix; fake-forge tests covered PR checks and existing merge behavior. CLI and persisted-state evidence was captured, disposable data was removed, and no source changes were made. Live GitHub validation remains unavailable without isolated credentials.
Evidence: Live local merge outputs, branch state, and durable override records
Evidence: PR public-interface outputs with a fake forge; not live GitHub evidence
Evidence: Isolated GitHub configuration has no credentials
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 3 issues found → auto-fixed ✅
bin/fm-pr-merge.sh:534- With gh absent or its checks read failing, gh-axi reports a skipped job asskip. This classifier omits that value, so an otherwise green PR is refused as failing and unnecessarily requires a captain override. Recognizeskipas green, preserving the same invariant as bin/fm-pr-merge.sh:506, and exercise that fallback result through the public interface.bin/fm-pr-merge.sh:109- Simplification: the added--captain-instruction=...alias is unnecessary for the required--captain-instruction "<exact words>"interface. Remove this alternate matching path, including bin/fm-pr-merge.sh:115 and bin/fm-merge-local.sh:39; use the documented spelling in tests/fm-pr-merge.test.sh:1826.bin/fm-merge-guard-lib.sh:30- Simplification: this introduces a second task-ID policy alongside fm_pr_task_id_valid in bin/fm-pr-lib.sh, with different treatment of leading underscores and hyphens. Validation is required, but a parallel definition is not. Remove this helper and reuse the existing owner at bin/fm-merge-local.sh:45; PR merges already use that owner at bin/fm-pr-merge.sh:98.🔧 Fix applied.
✅ Re-checked - no issues remain.
bin/fm-merge-guard-lib.sh:55- The shared guard permits invalid receipt evidence: fm-receipt-check exits 2 with status=invalid and accepted_blocked=[], but the guard only rejects exit codes greater than 2. A real local merge with malformed receipt JSON exited 0 and advanced main without an override. Providing an instruction also landed without recording it. Reject invalid receipt accounting before trusting its empty accepted_blocked list, and add a public-interface regression.TMPDIR="$PWD/.test-phase-tmp" bashsourcingtests/fm-merge-local.test.sh, with product transcripts captured before cleanupTMPDIR="$PWD/.test-phase-tmp" bashsourcingtests/fm-pr-merge.test.sh, with fake-forge transcripts and durable records capturedTMPDIR="$PWD/.test-phase-tmp" bash tests/fm-main-ci-watch.test.shpython3 .test-phase-tmp/adversarial.py: real local Git merges, receipt-check output, branch hashes, instruction validation, record failure, and divergenceTMPDIR="$PWD/.test-phase-tmp" bash .test-phase-tmp/pending-override.sh🔧 Fix applied.
1 warning still open:
TMPDIR="$PWD/.test-phase-tmp" bash tests/fm-merge-local.test.shTMPDIR="$PWD/.test-phase-tmp" bash tests/fm-pr-merge.test.shPython disposable-repository driver executed realbash bin/fm-receipt-check.shandbash bin/fm-merge-local.sh, checking exit codes, branch SHAs, refusal messages, and persisted override JSON.Public local interface checks for unsafe task IDs, invalid instructions, unwritable override records, and divergent branches.gh auth statuswith an empty worktree-local GH_CONFIG_DIR and token environment variables removed.Removed disposable repositories and test configuration; verifiedgit status --shortwas clean.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.