Skip to content

feat(bin): allow evidenced pre-merge task teardown - #1401

Open
sbracewell64 wants to merge 4 commits into
kunchenguid:mainfrom
sbracewell64:fm/teardown-policy-code
Open

feat(bin): allow evidenced pre-merge task teardown#1401
sbracewell64 wants to merge 4 commits into
kunchenguid:mainfrom
sbracewell64:fm/teardown-policy-code

Conversation

@sbracewell64

Copy link
Copy Markdown

Intent

Implement the captain's 2026-07-31 cleanup ruling in bin/fm-teardown.sh. Permit cleanup of a completed PR-based task before merge only when the latest completion exactly names the canonically recorded GitHub PR, live GitHub evidence shows that same open non-draft PR has an all-passing completed check suite with zero failed, pending, or skipped/cancelled/expected buckets and is mergeable, the live head exactly matches the recorded PR head, and repository evidence proves local HEAD is contained in that PR head; preserve the local task branch on this path and print its branch/PR identity on successful cleanup, without adding retention metadata or a reaper. Treat absent, duplicated, stale, contradictory, or unavailable completion, check, mergeability, identity, or head evidence as a refusal. Continue refusing red, pending, skipped, or conflicted PRs, uncommitted work, genuinely unpublished commits, and every existing unlanded-work safety condition; merely pushed remote reachability is intentionally insufficient and GitHub outages refuse. Replace the obsolete generic 'land its PR' diagnosis with evidence-specific refusal. Reuse the authenticated configured-remote PR-head fetch path, pin GitHub test mocks to exact canonical arguments, centralize branch finalization, and ensure the preserve notice survives detach failure without broadening retention behavior. Add regression coverage for the allowed path and every refusal boundary, retain existing merged/content/local-only/lock/backend behavior, and run required lint and focused teardown suites.

What Changed

  • Allow completed PR-based tasks to tear down before merge only when canonical completion, live GitHub checks, mergeability, PR-head identity, and local commit ancestry all validate.
  • Preserve the task branch and report its PR identity on successful pre-merge cleanup, while returning evidence-specific refusals for missing, ambiguous, stale, failing, pending, skipped, conflicted, or unpublished states.
  • Centralize branch finalization, update teardown policy documentation, and expand regression coverage across allowed and refusal paths while retaining existing teardown behavior.

Risk Assessment

✅ Low: Captain, the fix commit resolves all prior findings and the full branch diff now conforms to the source-verifiable teardown safety criteria without introducing another material issue.

Testing

After session bootstrap and diff/coverage inspection, the focused teardown suite passed across the permitted pre-merge path and its refusal boundaries, and a separate CLI-level scenario demonstrated successful cleanup, exact branch/PR preservation notice, and an unchanged preserved ref; no UI artifact applies because this is a shell CLI policy change, and no source or transient working-tree artifacts remained.

Evidence: Focused teardown test suite
ok - local-only worktree with HEAD on a fork remote is torn down (fix holds)
ok - teardown prompts tasks-axi backlog refresh when compatible
ok - teardown honors config/backlog-backend=manual even when tasks-axi is compatible
ok - local-only worktree with truly unpushed work is refused (safety preserved)
ok - local-only worktree with work merged into local main is torn down (no regression)
ok - completed green mergeable PR is cleaned before merge with its local branch preserved
ok - preserved branch identity survives detach failure
ok - green PR whose suite reports skipped checks refuses cleanup
ok - unusable check suites refuse pre-merge cleanup with evidence-specific reasons
ok - no-mistakes worktree with genuinely unlanded work is refused (safety preserved)
ok - red PR refuses cleanup and preserves all task recovery state
ok - pending PR checks refuse cleanup
ok - conflicted PR refuses cleanup and preserves all task recovery state
ok - green PR does not authorize cleanup of a genuinely unpublished later commit
ok - patch-equivalent divergent local HEAD refuses pre-merge cleanup
ok - absent or ambiguous completion, check, mergeability, identity, and head evidence all refuse cleanup
ok - local-only worktree with unpushed work is torn down under --force (escape hatch)
ok - teardown completes when an exact busy-state sidecar is already absent
ok - herdr teardown removes pane-owned escalation dedupe state
ok - herdr projection teardown retires its journal only after confirming the exact recorded pane is gone
ok - herdr projection teardown retains the stale journal and attempts no workspace cleanup when exact-pane close is unconfirmed
ok - squash-merged + deleted-branch worktree (PR merged) is torn down (the fix)
ok - squash-merged PR accepts a local HEAD that is an ancestor of the final PR head
ok - teardown discovers a merged PR by branch name and tears down when no pr= was ever recorded
ok - squash-merged PR accepts replayed unpushed local patches contained in the PR head
ok - merged PR does not allow teardown after a later local commit
ok - fm-pr-check does not refresh PR head after HEAD moves
ok - fm-pr-check records the remote PR head when the local worktree lags
ok - worktree whose content already landed in the default branch is torn down (content fallback)
ok - content fallback refreshes origin default before comparing trees
ok - dirty worktree is refused even when its committed work has landed (dirty always wins)
ok - gh lookup error with content not in default refuses (fail-safe)
ok - provably-stale worktree index.lock (old, no live holder) is cleared and teardown succeeds
ok - live-held worktree index.lock is never removed and teardown refuses
ok - lsof errors leave worktree index.lock in place and refuse teardown
ok - stale lock cleanup rechecks and refuses dirty worktree before return
ok - normal repo index.lock is resolved from the worktree and cleared when stale
ok - lock mtime read failures leave worktree index.lock in place and refuse teardown
ok - transient index.lock cleared after first failed return is retried successfully without force-remove
ok - persistent index.lock exhausts retries and refuses without force-removing the lock
ok - empty retry wait overrides use the default without aborting teardown
ok - fractional legacy retry wait remains supported without arithmetic
Evidence: Successful green, mergeable pre-merge teardown transcript
LIVE PR FIXTURE:
state: open
draft: false
head:
  sha: dd7ffd60a98d8593e2f09a267d2169875dbb5e8c
mergeable: true
summary: "3 passed, 0 failed, 3 total"
TEARDOWN CLI OUTPUT:
Preserved local branch fm/task-x1: task-x1 was cleaned up before https://github.com/example/repo/pull/7 merged. Delete the branch once that PR lands.
/tmp/fm-teardown-tests.IFeOgf/reviewer-e2e/project: already current
teardown task-x1 complete (window firstmate:fm-task-x1, worktree /tmp/fm-teardown-tests.IFeOgf/reviewer-e2e/wt)
Backlog: task-x1 just finished. Run tasks-axi done task-x1 --pr https://github.com/example/repo/pull/7, then run tasks-axi ready for dependency-cleared candidates, check date gates, and dispatch only work whose blockers are gone and date is due.
EXIT STATUS: 0
PRESERVED REF: refs/heads/fm/task-x1 -> dd7ffd60a98d8593e2f09a267d2169875dbb5e8c

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 5 issues found → auto-fixed ✅
  • 🚨 bin/fm-teardown.sh:643 - Intent requires “zero failed, pending, or skipped/cancelled/expected buckets” and explicitly says to continue refusing skipped PRs. This condition instead accepts skipped checks whenever passed + skipped == total; require skipped == 0 and update the regression that currently expects skipped checks to pass.
  • 🚨 bin/fm-teardown.sh:458 - Intent requires repository proof that local HEAD is contained in the PR head and refusal of genuinely unpublished commits. The patch-ID fallback accepts a divergent local commit merely because its patch matches a PR commit, even though local HEAD is not an ancestor of the PR head; use strict ancestry for the pre-merge path.
  • 🚨 bin/fm-teardown.sh:562 - Intent says duplicated completion evidence must refuse. Only the final non-empty status line is inspected, so two identical completion events—or an earlier contradictory completion followed by a valid one—still authorize cleanup. Count and validate completion records rather than accepting solely from the last line.
  • 🚨 bin/fm-teardown.sh:1476 - Intent requires that the preserve notice survive detach failure. The notice is emitted only inside the successful checkout --detach branch, so a detach failure silently omits the sole surviving branch/PR identity even though cleanup continues. Emit the preservation notice independently of detach success.
  • ⚠️ bin/fm-teardown.sh:1491 - Intent requires branch finalization to be centralized, but the Orca and non-Orca paths duplicate branch discovery, detach, preserve announcement, and deletion logic. Extract one finalization helper so both backends cannot drift—particularly around detach-failure handling.

🔧 Fix: Enforce strict pre-merge teardown evidence
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • bin/fm-session-start.sh
  • Inspected git diff f7d0d0a703a717880656709d7906615505b3f2c0..052d4ce166b73f6c1f2edb4a59097abaa5230ccf and the teardown test matrix.
  • bash tests/fm-teardown.test.sh > /tmp/no-mistakes-evidence/01KYWDZ21C7BTCTPBSPAH5EF5Q/fm-teardown-focused-suite.log 2>&1
  • Ran a hermetic end-to-end bin/fm-teardown.sh task-x1 scenario using exact canonical GitHub mock arguments and captured live-PR fixtures, CLI output, exit status, and the surviving branch ref in green-mergeable-teardown-transcript.txt.
  • Checked git status --short and searched for transient build/cache directories after testing; the working tree remained clean.
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

@kunchenguid

kunchenguid commented Jul 31, 2026

Copy link
Copy Markdown
Owner

Automated reminder: thanks for the PR! This branch currently has a merge conflict with the base branch.

When you get a chance, please rebase onto (or merge) the latest base branch, resolve the conflict, and push. After that, checks will re-run and the PR will get looked at again.

Noted for firstmate#1401 at a0961dd3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants