Skip to content

fix(bin): require positive landing proof before teardown - #1

Merged
jokim1 merged 6 commits into
lila-mainfrom
fm/fm-teardown-landed-false-positive-adopt
Aug 16, 2026
Merged

fix(bin): require positive landing proof before teardown#1
jokim1 merged 6 commits into
lila-mainfrom
fm/fm-teardown-landed-false-positive-adopt

Conversation

@jokim1

@jokim1 jokim1 commented Aug 15, 2026

Copy link
Copy Markdown
Owner

What Changed

  • Require positive landing proof from the live default branch or a merged PR before tearing down ordinary ship worktrees, including fail-closed handling for missing worktrees, open PRs, and remote default-branch drift.
  • Re-run dirty-worktree, landing, and PR checks after worker quiescence and immediately before Treehouse, Orca, and Playbot workspace removal attempts.
  • Update the Playbot teardown contract, GitLab guidance, and regression coverage for the stricter safety checks.

Risk Assessment

✅ Low: The updated implementation now revalidates Playbot worktrees after endpoint quiescence and immediately before deletion, while preserving the prior force, scout, and secondmate carveouts.

Testing

Inspected the target change and intent, added an end-to-end Playbot cleanup regression, passed the focused adapter and complete cleanup-command suites, manually captured the operator-facing late-write refusal and persisted state, and verified only the intentional test change remains; no lint or static analysis was run per instruction.

Evidence: End-to-end late-write refusal transcript

$ bin/fm-teardown.sh task-x1 exit_code=1 REFUSED: worktree ... has uncommitted changes. error: playbot teardown refused ... preserving every durable record Lifecycle calls: stop, archive - no delete Preserved: late-worker-write.txt, task metadata, route, and outbox

$ bin/fm-teardown.sh task-x1
exit_code=1

--- operator output ---
●━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
●  WATCHER DOWN - SUPERVISION IS OFF
●  1 task(s) in flight, but no live watcher process holds this home lock (last beat: 3s ago).
●  Trust the emitted supervision protocol for this harness; do not use shell & for watcher repair.
●  This is a supervision warning only; the guarded operation WILL still run.
●  repair missing watcher supervision according to the session-start block for this harness; do not use shell &.
●━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
REFUSED: worktree /var/folders/vx/kq2w6_xj1sq82jh1gtsjl0_m0000gn/T//fm-teardown-tests.qu4Op2/playbot-archive-mutation-refusal/wt has uncommitted changes.
uncommitted changes present
Commit them (or get the captain's explicit OK to discard, then --force).
error: playbot teardown refused or failed for task-x1 (refuse:worktree-safety-recheck-failed); preserving every durable record
--- Playbot lifecycle calls ---
stop
archive
--- preserved late write ---
created while Playbot archived the thread
--- preserved durable records ---
present task-x1.meta
present task-x1.playbot-route.json
present task-x1.playbot-outbox.json
--- worktree status ---
?? late-worker-write.txt
Evidence: Playbot adapter regression transcript
FM_TEST_BEGIN 2026-08-16T07:30:56Z tests/fm-playbot-backend.test.sh family=unclassified expected_gate_skip=none
ok - target parsing accepts only exact playbot:<thread-id>
ok - runtime check refuses spawn intake until evidence and runtime health pass
ok - send_key rejects every named key, including Enter, Escape, and C-c
ok - send_text_submit matches the fm-send empty-success contract: nonempty failure, nonzero exit, nothing sent
ok - composer_state maps exact pending-queue evidence to empty/pending/unknown
ok - busy/target-exists/agent-state honor the recovery-grade vocabulary with no invented dead
ok - capture returns the bounded exact-thread rollout as data
ok - interrupt reports success only after exact idle-state proof
ok - lifecycle mutations refuse fail-closed before Phase 1 evidence; pre-seam create stays split
ok - binding_resolve returns the exact bound triple and refuses unbound projects
ok - endpoint validation enforces the exact playbot:<thread-id> window and the route_write-bound route
ok - endpoint_confirmed_gone proves gone only from an authoritative inventory
ok - abort cleanup confirmation requires thread, workspace, and worktree absence
ok - teardown refuses live/mismatched endpoints and reports retained/retired for a confirmed-gone thread
ok - teardown rechecks worktree safety after Playbot quiescence and before workspace deletion
ok - confirmed-gone Playbot deletion also requires a passing worktree safety recheck
fm-playbot-backend: all tests passed
FM_TEST_END 2026-08-16T07:31:17Z tests/fm-playbot-backend.test.sh exit=0 duration_ms=21334 gate_skip=false
FM_TEST_SUMMARY total=1 failed=0 skipped_gate=0 duration_ms=21947
FM_TEST_SUMMARY_FAMILY family=unclassified count=1 duration_ms=21334 failed=0
FM_TEST_SLOWEST rank=1 script=tests/fm-playbot-backend.test.sh duration_ms=21334
Evidence: Full cleanup regression transcript
FM_TEST_BEGIN 2026-08-16T07:31:31Z tests/fm-teardown.test.sh family=pr-forge expected_gate_skip=none
ok - local-only worktree with HEAD on a fork remote is torn down (fix holds)
●━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
●  WATCHER DOWN - SUPERVISION IS OFF
●  1 task(s) in flight, but no live watcher process holds this home lock (last beat: 2s ago).
●  Trust the emitted supervision protocol for this harness; do not use shell & for watcher repair.
●  This is a supervision warning only; the guarded operation WILL still run.
●  repair missing watcher supervision according to the session-start block for this harness; do not use shell &.
●━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
ok - teardown prompts tasks-axi backlog refresh when compatible
●━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
●  WATCHER DOWN - SUPERVISION IS OFF
●  1 task(s) in flight, but no live watcher process holds this home lock (last beat: 2s ago).
●  Trust the emitted supervision protocol for this harness; do not use shell & for watcher repair.
●  This is a supervision warning only; the guarded operation WILL still run.
●  repair missing watcher supervision according to the session-start block for this harness; do not use shell &.
●━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
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 - no-mistakes worktree with HEAD on origin is torn down (no regression)
ok - no-mistakes worktree with genuinely unlanded work is refused (safety preserved)
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 - successful teardown enqueues one refill wake that drain clears
ok - herdr teardown removes pane-owned escalation dedupe state
ok - herdr flat teardown refuses before returning the isolated copy under lock contention and the retry completes cleanly
ok - herdr flat teardown never erases records when pane presence is unparseable
ok - herdr flat teardown preflight refuses before every destructive change
ok - forced secondmate teardown preflights every Herdr child before cleanup mutation
ok - forced secondmate teardown holds every descendant lifecycle and metadata lock
ok - forced secondmate teardown retains Herdr child identity until exact pane disappearance
ok - forced teardown retains a nested secondmate home and its grandchild's Herdr identity when the grandchild close is unconfirmed
ok - herdr projection teardown retires its journal only after confirming the exact recorded pane is gone
ok - herdr projection teardown retains every record when post-close presence is unknown
ok - herdr projection teardown surfaces failed focus restoration without turning confirmed cleanup into a hard failure
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
●━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
●  WATCHER DOWN - SUPERVISION IS OFF
●  1 task(s) in flight, but no live watcher process holds this home lock (last beat: 2s ago).
●  Trust the emitted supervision protocol for this harness; do not use shell & for watcher repair.
●  This is a supervision warning only; the guarded operation WILL still run.
●  repair missing watcher supervision according to the session-start block for this harness; do not use shell &.
●━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
WARNING: watcher still down (same stale episode; last beat: 8s ago, grace 900s) - full banner already printed this episode.
ok - fm-pr-check does not refresh PR head after HEAD moves
●━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
●  WATCHER DOWN - SUPERVISION IS OFF
●  1 task(s) in flight, but no live watcher process holds this home lock (last beat: 2s ago).
●  Trust the emitted supervision protocol for this harness; do not use shell & for watcher repair.
●  This is a supervision warning only; the guarded operation WILL still run.
●  repair missing watcher supervision according to the session-start block for this harness; do not use shell &.
●━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
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 - open PR whose content is in default via a sibling is refused, not reaped
ok - task whose own PR is merged is torn down even when a sibling shares its content
ok - pushed open-PR ship refuses teardown and keeps meta+poll (R1-742)
ok - missing worktree refuses even with pr_head and MERGED (F4)
ok - pushed unique work without pr= refuses without positive land proof (R1-D)
ok - CLOSED unmerged pushed work refuses (R1-A)
ok - GitLab MERGED squash with tree on live D allows (FR1)
ok - GitLab OPEN refuses even with sibling content in default
ok - unconfirmed forge + sibling tree refuses (R2-R1)
ok - same-branch default OID force-push after fetch refuses (V4-F1)
ok - a remote default symref disappearing after fetch refuses teardown
ok - a failed post-fetch remote default lookup refuses teardown
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
ok - a task's own parked no-mistakes run is aborted, not orphaned, before the worker is removed
ok - teardown refuses before reap or removal when a task-owned run remains parked
ok - a different run cannot confirm the targeted abort
ok - empty post-abort status is not accepted as confirmation
ok - the CLI's exact run-not-found signal confirms completion
ok - a parked run on another branch is never aborted by this task's teardown (ownership is precise)
ok - a task-owned autonomous running step is left alone rather than aborted
ok - a leaked descendant process rooted under the task's worktree is reaped by teardown, not left surviving
ok - a leaked descendant process rooted under the task's per-task tasktmp is reaped by teardown too
ok - missing lsof falls back to reaping the tmux pane process group
ok - an erroring lsof scan refuses teardown and preserves the task
ok - a reused pid with a different start time is never force-killed
ok - an exec change preserves birth identity and the process is reaped
ok - a process spawned during grace is reaped on a later pass
ok - persistent leaked processes refuse teardown after bounded retries
ok - a process exiting during identity lookup does not block teardown
ok - the run abort and the leaked-process reap both complete before the destructive worktree return
ok - post-reap worktree mutations are revalidated before destructive return
ok - Playbot archive-time mutations refuse workspace deletion and preserve task records
FM_TEST_END 2026-08-16T07:58:42Z tests/fm-teardown.test.sh exit=0 duration_ms=1631612 gate_skip=false
FM_TEST_SUMMARY total=1 failed=0 skipped_gate=0 duration_ms=1632401
FM_TEST_SUMMARY_FAMILY family=pr-forge count=1 duration_ms=1631612 failed=0
FM_TEST_SLOWEST rank=1 script=tests/fm-teardown.test.sh duration_ms=1631612

Pipeline

Updates from git push no-mistakes

⏭️ **intent** - skipped

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 2 issues found → auto-fixed (2) ✅
  • 🚨 bin/fm-teardown.sh:2732 - Safety validation runs only before the pipeline is aborted and worktree processes are reaped. The callback passed to teardown_treehouse_return runs only after stale-lock cleanup, not before normal returns or retries, and Orca/Playbot removal has no post-quiescence check. A terminating process can therefore leave new changes, or landing state can change, after approval but before destructive removal. Re-run the full dirty/landing/PR validation after quiescence and immediately before every destructive return/removal attempt.
  • ⚠️ bin/fm-teardown.sh:976 - The post-fetch default-name check silently accepts an empty second ls-remote --symref result. If remote HEAD becomes detached or its symref cannot be read while the previously named branch retains the same OID, teardown proceeds without proving that the live default branch name still matches. Preserve whether the initial lookup returned a symref and require the second lookup to return the same non-empty name; also propagate ls-remote failure rather than relying on the pipeline's awk status.

🔧 Fix: Close teardown revalidation and remote symref races
1 error still open:

  • 🚨 bin/fm-teardown.sh:2843 - The Playbot recheck still occurs before playbot_teardown_endpoint, whose adapter stops/archives the live worker and only then deletes the workspace. That worker can write to the worktree after this validation while stop/archive completes, leaving the destructive delete unchecked. Run the full safety callback inside the Playbot teardown flow after confirmed thread quiescence and immediately before each workspace-removal call.

🔧 Fix: Recheck Playbot worktrees after endpoint quiescence
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • Inspected commit 824ef7a65977a1bf3c77a7b5ccf3fe1675db2ab9 against base 5b9e8629923a0b7623c82d5e151490e8185f59f7 and repository guidance to extract intent
  • git diff --check -- tests/fm-teardown.test.sh
  • bin/fm-test-run.sh tests/fm-playbot-backend.test.sh
  • bin/fm-test-run.sh tests/fm-teardown.test.sh
  • Manual evidence run of bin/fm-teardown.sh task-x1 through the Playbot archive-time late-write fixture, capturing refusal output, lifecycle calls, preserved file, records, and Git status
  • Final git diff --check and git status --short cleanup verification
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

@cursor

cursor Bot commented Aug 15, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

jokim1 added a commit that referenced this pull request Aug 15, 2026
Codex adversarial blockers on PR #1:
- B1: classify recorded PR/MR by forge (gh for GitHub, glab for GitLab);
  restore landed GitLab MR cleanup via content-in-default after MERGED.
- B2: CLOSED is not landing evidence; require content in default (or refuse).
- B3: MERGED requires current HEAD in PR head (or unpushed patch match /
  content-in-default) even when the branch is fully pushed.
- W1: strict one-line STATE\\tHEAD parse; MERGED needs a valid SHA; unknown
  and multi-line responses refuse.
- W2: remote reachability alone is not landing for ship tasks without
  content or merged-PR proof (covers no-pr= unique pushed work).
- W4: refusal text names provider and distinguishes lookup vs malformed
  vs unknown.

Regression tests: GitLab merged/open, CLOSED unique, MERGED head mismatch,
no-pr unique pushed, malformed multi-line response.
Empty-unpushed remote reachability is not landing. Require live default tip
proof (ancestry, no-pr tree equality, or MERGED containment/tree) against a
default tip whose remote OID still matches after fetch. Always refuse OPEN.
Unconfirmed never accepts tree equality. Missing worktree refuses. Recheck
dirty+land+full PR classify after quiescence before destructive return.
@jokim1
jokim1 force-pushed the fm/fm-teardown-landed-false-positive-adopt branch from bdd6425 to 0e9bd25 Compare August 16, 2026 06:42
@jokim1 jokim1 changed the title fix(bin): block teardown for open recorded PRs fix(bin): require positive landing proof before teardown Aug 16, 2026
@jokim1
jokim1 merged commit 6c6a22f into lila-main Aug 16, 2026
23 of 25 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