feat(bin): add captain-visible no-mistakes observers - #8
Merged
Merged
Conversation
added 11 commits
July 22, 2026 17:26
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
Implement Firstmate's default captain-visible no-mistakes observer experience: after the worker starts validation and an authoritative branch-matched run ID exists, open at most one separate non-focused observer terminal running 'no-mistakes attach --run ' without transferring any axi run/respond, cancellation, CI, ask-user, or merge ownership away from the worker. Support tmux and Herdr with trusted task/run/backend/worker/observer identity, idempotent retries, durable q/exit detach tombstones, explicit reopen only, exact observer-only cleanup integrated into task teardown, and bounded failure that preserves validation while printing the exact manual attach command. Keep zellij, Orca, and cmux on safe manual fallback until separately proven. Keep owning instructions in the lifecycle script with only concise trigger/safety pointers in shared docs. Cover harness invocation, authoritative discovery, separation, mismatch/malformed/unreadable state, interrupted create reconciliation, detach/reopen, fallback, and cleanup deterministically, plus an isolated tmux smoke test. Per captain decision, do not add live Herdr proof; document a concise captain-run manual verification checklist and keep the approved scope minimal.
What Changed
Risk Assessment
✅ Low: The change now consistently enforces token-bound identity, idempotent observer creation and detach handling, exact cleanup, safe fallback, and narrowly reconciled interrupted states without a remaining substantiated defect.
Testing
The supplied full baseline was green; focused lifecycle, live isolated-tmux, and teardown tests also passed, and the captured transcript demonstrates one non-focused sibling observer, durable q-detach with exact manual fallback, explicit-only reopen, and observer-only cleanup preserving the worker. Herdr behavior remains deterministic fake-CLI coverage with the intentionally documented captain-run checklist; no live Herdr proof was added per scope. No screenshot was captured because the smoke runs on a detached isolated tmux socket without a rendered GUI surface, so a direct terminal lifecycle transcript was used.
Evidence: Captain-visible tmux observer lifecycle transcript
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed (8) ✅
bin/fm-no-mistakes-observer.sh:804- The required “idempotent retries” behavior is contradicted here:startadopts an existing branch run only when its top-level status is exactlyrunning. Other active states already recognized elsewhere in this repository, such asawaiting_approvalandfix_review, fall through and send the validation invocation again, potentially starting duplicate validation. Treat every authoritative nonterminal branch run as existing before invoking the worker.bin/fm-no-mistakes-observer.sh:648- The required “interrupted create reconciliation” and “idempotent retries” are incomplete for Herdr. The record becomesreadybefore the separatesend-textand Enter operations; interruption after text is sent leaves the command in the composer, and retry sends it again before Enter, potentially executing a concatenated invalid command instead of attaching. Persist an intermediate launch phase or safely verify/clear the composer before retrying.🔧 Fix: Harden observer retry idempotence
1 error still open:
bin/fm-no-mistakes-observer.sh:664- The required “idempotent retries” behavior still has a Herdr race. After Enter succeeds, this resets the record toreadywhile the_sessionprocess cannot mark itattacheduntil the caller releases the lifecycle lock. A retry that acquires the lock first seesreadyand sends the full attach command again into the observer pane. Preserve a distinct submitted/launching state that_sessioncan claim but ordinary retries refuse, so successful submission cannot reopen the text-sending path.🔧 Fix: Prevent Herdr observer resubmission race
1 error still open:
bin/fm-no-mistakes-observer.sh:905- The required “durable q/exit detach tombstones” behavior is still incomplete. If_sessioncannot claim thesubmittedrecord within its bounded retry loop—for example after the launcher dies while leaving its lock directory—it exits here without changing the record. Ordinary retries and explicitreopenthen preservesubmittedforever as “awaiting attachment,” although the observer process has exited. On claim timeout, safely tombstone the same task/run/token record as detached or failed without resending text or Enter.🔧 Fix: Tombstone unclaimed Herdr observer sessions
2 errors still open:
bin/fm-no-mistakes-observer.sh:909- The required bounded exit reconciliation is still incomplete. After the fixed claim wait,_sessiontombstones only if it can immediately acquire the lifecycle lock, and then only forsubmitted. A launcher or concurrent status operation can legitimately hold the lock past this point, leaving Herdr permanentlysubmitted; tmux can similarly remainreadyafter its wrapper exits. Subsequent retries can therefore preserve or report an observer that has no attach process. Reconcile the same-token launch state after lock release within a bounded path for both backends.bin/fm-no-mistakes-observer.sh:883- The required trusted identity and idempotent lifecycle are contradicted because claiming an observer is not atomic with the lifecycle lock._sessionchecks that the lock directory is absent, then loads and rewrites the record without acquiring it; cleanup can acquire the lock and remove the record between those operations, after which_sessionresurrects stale state asattachedand runs the old attach command. Acquire and hold the task lock while validating and transitioning the same task/run/token record, then release it before the foreground attach.🔧 Fix: Lock observer session lifecycle transitions
1 error still open:
bin/fm-no-mistakes-observer.sh:933- The required durable exit tombstone and bounded-failure behavior can still be lost to normal lock contention. Claim-timeout and attach-exit reconciliation each make one bounded transition attempt and silently ignore exhaustion, while another lifecycle operation may hold the same lock across several independently bounded status/terminal calls. The observer can therefore exit onqwhile remainingattached(or die whileready/submitted), blocking explicit reopen and printing no manual attach fallback. Make the exit/timed-out transition durable beyond transient lock ownership without resurrecting removed records.🔧 Fix: Persist observer detach across lock contention
3 errors still open:
bin/fm-no-mistakes-observer.sh:324- The required trusted token identity and durable exit handling can be lost because every generation writes the same task-wide pending path outside the lifecycle lock. A delayed old observer can overwrite a newer token’s pending detach after explicit reopen; consumption then discards the stale token mismatch and loses the newer exit evidence, potentially leaving its dead recordattached. Use token-specific or otherwise non-clobbering pending transitions.bin/fm-no-mistakes-observer.sh:944- The required exact observer cleanup can race with late pending publication. Cleanup removes the record and current sidecar under the lock, but a contended_sessionmay publish.observer.pendingafter cleanup releases it. Teardown only invokes observer cleanup when.observerexists and does not otherwise remove this late sidecar, leaving durable task/run/token state after teardown. Add a cleanup-safe publication handshake or unconditional race-safe pending cleanup.bin/fm-no-mistakes-observer.sh:782- The required explicit reopen behavior takes two commands when anattachedobserver endpoint has already exited. This branch tombstones it asdetachedand returns manual fallback even when the current action isreopen; only a secondreopencreates the replacement. Whenallow_reopen=1, continue safely from the newly established detach tombstone in the same invocation.🔧 Fix: Scope observer handoffs to generation tokens
1 error still open:
bin/fm-no-mistakes-observer.sh:514- The required interrupted-create reconciliation and exact teardown cleanup still have a publication gap. Initialization creates the token generation directory before atomically writing the authoritative observer record; interruption between those operations leaves an unreferenced.<task>.observer-<token>directory. Later starts use another token, and teardown skips observer cleanup when.observeris absent, so the orphan survives task removal. Publish both transactionally or safely reconcile recordless task-owned generations.🔧 Fix: Publish observer records before generation state
1 error still open:
bin/fm-no-mistakes-observer.sh:526- The required interrupted-create reconciliation and teardown cleanup remain incomplete after the record-first change. Interruption after this record write but before generation creation leaves a validcreatingrecord with no backend session or endpoint. Direct cleanup then treats the missing provisional identity as ambiguous and refuses teardown, although no observer could have been created. Recognize this same-token pristine pre-endpoint state as safely unlaunched and remove its record/generation without targeting a terminal.🔧 Fix: Clean pristine unlaunched observer records safely
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"Baseline supplied by the gate:command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"(reported successful)bash tests/fm-no-mistakes-observer.test.shbash tests/fm-no-mistakes-observer-tmux-smoke.test.shbash tests/fm-teardown.test.shInstrumented and ran the isolated tmux smoke flow to record actual window focus/identity, trusted observer state, q-detach tombstone, manual attach fallback, explicit reopen, exact cleanup, and surviving worker terminal intmux-observer-lifecycle.txt✅ **Document** - passed
✅ No issues found.
🔧 **Lint** - 1 issue found → auto-fixed ✅
🔧 Fix: Fix observer pending reset assignments, captain
✅ Re-checked - no issues remain.
✅ **Push** - passed
✅ No issues found.