fix(teardown): require captain-authorization token for --force#137
Open
e-jung wants to merge 4 commits into
Open
fix(teardown): require captain-authorization token for --force#137e-jung wants to merge 4 commits into
e-jung wants to merge 4 commits into
Conversation
--force bypasses teardown's work-not-landed, dirty, scout-report, and secondmate-child safety checks, so it can discard unreviewed work. Per prime directive kunchenguid#3 firstmate must never self-authorize it, but the script had no structural barrier: firstmate could run --force on its own judgment. Add a two-step guard that separates decision from authorization: 1. The captain explicitly OKs discarding THIS task's work. 2. firstmate records that as state/<id>.force-granted. 3. firstmate runs fm-teardown.sh <id> --force. --force takes effect only when the token exists; without it, --force is INERT (FORCE is cleared, normal safety checks run). The token is consumed on every --force invocation (success or refuse), so each force-teardown needs a fresh captain OK and a stale token can never carry over. Every --force invocation is logged to state/.force-audit.log with timestamp, task id, caller pid, and the authorization verdict. By reassigning FORCE based on authorization, all existing --force comparisons behave correctly with no changes to the rest of the teardown logic. Update the force-teardown tests to grant the token (they model captain-authorized discards), and add coverage for the inert-without-token, token-consumed, and audit-log behaviors. Document the two-step model in AGENTS.md (prime directive kunchenguid#3, secondmate teardown, state layout).
The --force guard only runs (and consumes the captain-authorization token) when --force is passed. If firstmate recorded a token at state/<id>.force-granted but then ran a NORMAL teardown without --force, the token survived teardown and could linger as stale authorization. Add the token to the final per-task state cleanup so it is removed alongside the task's other volatile state regardless of how teardown ran. On an authorized --force teardown the token is already consumed up front, so this is a harmless no-op there; it only matters for the stale-token case. Addresses the review finding from the gate run.
… secondmate skill
4 tasks
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
The developer wanted to fix a prime-directive violation where firstmate could self-authorize bin/fm-teardown.sh --force without confirming the captain actually approved it. The goal was a deterministic structural guard making --force inert unless a captain-authorization token (e.g. state/.force-granted) exists, which firstmate may only create after the captain explicitly says to force-teardown that task. Requirements: the token must be consumed on use (deleted after teardown completes or refuses); every --force invocation, authorized or not, must be logged to state/.force-audit.log with timestamp, task id, caller PID, and authorization status; a documentation comment explaining the two-step model must be added; existing tests must pass and a new test for the guard must be added; the fix must stay minimal without refactoring unrelated teardown logic. Constraints: ship only through the no-mistakes gate via git push no-mistakes (never fork/origin/gh pr create), never merge the PR, and confine all changes to the worktree.
What Changed
@myfirstmatementions with an acknowledge/act/follow-up lifecycle, dismisses skipped mentions at the relay, and supports dry-run preview (bin/fm-x-*.sh,fmx-respondskill).fm-home-seed.sh,fm-backlog-handoff.sh, secondmate-provisioning skill)./afk) daemon, a deterministic crew-state helper, provably-working wake absorption, and a captain-authorization token guard that makesteardown --forceinert without explicit approval.Risk Assessment
✅ Low: Focused, well-bounded security hardening that adds a fail-closed token guard to --force with thorough test coverage (4 new tests plus 6 updated secondmate tests), effective control of all downstream FORCE checks, no affected internal callers, and correct token lifecycle (consumed on every invocation, stale tokens cleaned on normal teardown).
Testing
Validated the --force captain-authorization guard end-to-end. All 4 new force-guard unit tests plus the modified force-override test pass, and the full secondmate-safety suite (49 tests) passes with the token granted on each --force path. A manual run of the real bin/fm-teardown.sh across 6 scenarios confirmed operator-facing behavior: --force is inert without the token (refuses unlanded work, preserves the worktree), effective with it, fail-closed on consumption (token consumed even when teardown later refuses), stale-token cleanup on normal teardown, full audit logging (timestamp/task/pids/verdict), and the in-script doc comment. The one test failure (content-landed) is pre-existing and orthogonal: I confirmed it reproduces identically on the base commit, and the new guard block is unreachable for that test because it never passes --force. Worktree left clean.
Evidence: Force-guard end-to-end demo (real bin/fm-teardown.sh, 6 scenarios)
SCENARIO 1 (no token): exit 1, 'WARNING: --force on task-x1 is not captain-authorized', 'REFUSED: ...work not yet merged', worktree PRESERVED. SCENARIO 2 (captain token): exit 0, no REFUSED, token CONSUMED. SCENARIO 3 (fail-closed): token granted then meta removed -> teardown refuses AFTER guard; audit authorized=yes; token CONSUMED. SCENARIO 4 (stale token, normal teardown): exit 0, token REMOVED. Audit log: 2026-06-29T16:54:22Z task=task-x1 caller_pid=550223 pid=550274 authorized=no / ...authorized=yes / ...authorized=yes.Evidence: fm-secondmate-safety.test.sh full output (49 ok, 0 fail)
Evidence: Force-guard unit test results (tests/fm-teardown.test.sh subset)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
tests/fm-teardown.test.sh:445- test_content_in_default_fallback_allows (case h) fails with exit 1, but this is PRE-EXISTING and UNRELATED to the force-guard change. Verified by checking out base commit 81c94db in a throwaway worktree and running the same suite: the identical 'content-landed' failure reproduces there. The change cannot affect it because that test runsfm-teardown.shWITHOUT --force, so the new guard block (gated onif [ "$FORCE" = "--force" ]) is never entered; the only other diff is one extra file in the finalrm -fcleanup list, which is a no-op for this test. Not actionable for this PR.bash tests/fm-teardown.test.sh (force-guard tests: test_force_without_token_is_inert, test_force_consumes_token, test_force_audit_log, test_normal_teardown_cleans_stale_force_token, and modified test_local_only_force_overrides_unpushed all pass)bash tests/fm-secondmate-safety.test.sh (full suite, 49 ok / 0 fail; the 6 --force invocations now grant the token via grant_domain_force)Manual end-to-end against the real bin/fm-teardown.sh: SCENARIO 1 inert-without-token (exit 1, WARNING + REFUSED, worktree preserved)Manual: SCENARIO 2 authorized --force takes effect (exit 0, no REFUSED, token consumed)Manual: SCENARIO 3 fail-closed token consumption (token granted then meta removed so teardown refuses AFTER the guard; audit shows authorized=yes; token still consumed)Manual: SCENARIO 4 stale token removed by normal teardown (exit 0, token gone)Manual: SCENARIO 5 audit log inspection (every --force line carries ts, task=, caller_pid=, pid=, authorized=)Manual: SCENARIO 6 doc comment lines 35-42 presentVerification that content-landed failure is pre-existing: git worktree of base 81c94db, bash tests/fm-teardown.test.sh reproduces the same failuregit status --porcelain (worktree clean; no transient artifacts committed)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.