fix(watch): stop false-wedging a finished crew that appends resolved: after done: - #10
Merged
Merged
Conversation
… after done:
A crew that opened a keyed decision or blocker, got unblocked, finished, then
durably CLOSED the key ends its status log:
done: PR https://.../pull/NN
resolved: <how the blocker cleared> [key=...]
The terminal-state check read only the LAST status line. `resolved:` is not a
captain-relevant terminal verb, so a genuinely-done idle pane was misread as
non-terminal: fm-crew-state.sh reported `unknown - no source`, and the stale
path (both the always-on watcher and the away-mode daemon) escalated it as a
`possible wedge`, repeatedly, each escalation burning a firstmate turn on a
non-event - and false-waking firstmate during afk.
Fix in the one shared owner (bin/fm-classify-lib.sh): add
`last_state_status_line`, which scans backward past `resolved:` bookkeeping to
the last REAL STATE verb, plus a `status_line_is_resolved` helper. `done:` then
`resolved:` recovers `done:` (terminal); `working:` then `resolved:` recovers
`working:` (NOT terminal), because a crew can close a blocker mid-task and keep
working - treating that as terminal would be strictly worse.
Route every terminal-state caller through it: stale_is_terminal (watcher),
classify_stale + mark_escalated_seen + the transient-stale dispatcher (daemon),
so both supervision modes agree. fm-crew-state.sh's no-run fallback reveals a
TERMINAL verb (done/failed) through a trailing resolved: but never resurrects a
blocked:/needs-decision: the resolve CLOSED - which key a resolve closes stays
owned by the keyed status_open_decisions fold.
Regression tests pin both directions in the colocated suites.
Marl0nL
added a commit
that referenced
this pull request
Aug 24, 2026
…nk (#47) * test(bootstrap): isolate the orca-backend gate case from an ambient orca binary Phase 5 salvage of fork fix 9a8ecfd. The orca-backend-selected case builds its PATH from $BASE_PATH (which includes /usr/bin), so on a host that has an unrelated `orca` program installed (GNOME's screen reader lives at /usr/bin/orca) bootstrap sees orca present and does not emit the expected "MISSING: orca" line, failing the assertion spuriously. Upstream never fixed this. Add make_orca_free_sysbin so the case observes orca as genuinely absent, proving the gate requires it regardless of the ambient system. Reproduced failing on the integration trunk before this change. * fix(brief): forbid agent commit co-author in ship, scout, and charter scaffolds Phase 5 salvage of fork fix ff2c958. Project and secondmate crews never load firstmate-coding-guidelines (a firstmate-repo-only trigger), so the fork's no-agent-co-author rule is invisible to them and every commit falls back to the harness default Co-Authored-By trailer. Upstream never pushed the rule into briefs. State it directly in the generated ship, scout, and secondmate charter scaffolds, where commit-capable crews actually read it. Note for review: this encodes the fork's standing policy (no agent co-author on ANY crew commit, AGENTS.md), which is broader than upstream's firstmate-repo-only ban. * fix(secondmates): truncate the registry charter-summary to a scannable one-liner Phase 5 salvage of fork fix 8b25fc8, re-homed onto upstream's bin/fm-secondmate-charter-lib.sh (the logic moved there from fm-home-seed.sh). data/secondmates.md prints in full at every session start in every home with a secondmate, so a multi-thousand-word charter inlined as the summary field burns context forever. Deterministically truncate the summary to a short one-liner (REGISTRY_SUMMARY_CAP=200, word-boundary cut, no model call); a caller can pass FM_SECONDMATE_SUMMARY directly. The full charter is still copied verbatim to the home's data/charter.md, so nothing is lost. The scope field, which the main firstmate uses for routing, is left untouched. The parenthesis/semicolon sanitizer stays, documented as protecting the registry field-split parser. Upstream never truncated the summary. New colocated lib test covers the cap, short passthrough, the explicit override, and that scope is never truncated. * fix(herdr-lab): scope lab calls with a leading --session so agent start cannot leak into default Phase 5 salvage of fork fix ff80250. bin/fm-herdr-lab.sh promises a named non-default isolated session for every Herdr lifecycle action (AGENTS.md section 11 treats it as a safety contract), but fm_herdr_lab_raw appended --session <lab> at the END of the argument list. For `agent start <name> ... -- <argv>` the flag therefore landed after the `--` separator, inside the agent's own argv, leaving the Herdr call itself unscoped and creating the agent in the ambient (live default) session. Compose every call as `herdr --session <lab> <args...>` with the flag leading, the position Herdr's own usage documents: it is consumed as a global option before subcommand dispatch, so no positional operand and nothing after a `--` separator can capture it. Also fail closed: fm_herdr_lab_raw re-validates the lab name and refuses any argument list carrying its own --session anywhere. Upstream's fm-herdr-lab.sh was byte-identical to the pre-fix fork state (still trailing). The adapter path bin/backends/herdr.sh keeps its trailing --session and is unaffected: it forwards no `--` command and never calls agent start. Regression coverage flips the fake-herdr to resolve --session as a global flag stopping at `--`, and adds the agent-start leak/target assertions. * fix(spawn): refuse a worktree another task of this home already owns Phase 5 salvage of fork fix fea2afe. Twice on 2026-07-29 a spawn was handed a worktree a LIVE task was still working in: in the main home a herdr-server restart left one task's meta pointing at another task's worktree (the confused crewmate wrote its uncommitted fixes there, where they were reverted as foreign edits and lost); in a secondmate home a spawn re-leased a live crewmate's worktree and `treehouse get` reset it to the default branch. The pool judges a slot free from live process cwds, so a crashed-but-owned slot (or an agent whose cwd is the pane's launch dir) looks free. This home's own state/*.meta is the record that knows the slot is taken. fm-spawn now refuses to launch into a worktree ANY other task of the current FM_HOME records as `worktree=`, naming the owning task. A respawn of the SAME task id into its own recorded worktree stays legitimate. The check runs inside the isolation assertion (both tmux and Orca ship/scout paths) and, on the secondmate path, before the pre-launch fast-forward and config push. It is deliberately home-scoped and never sweeps sibling homes. Upstream never added this guard (the closest, kunchenguid#765 two-stable-reads, settles same-task path races, not cross-task ownership). New colocated lease test; the dispatch-profile batch and path-spelling fixtures are corrected to give each live task its own slot, since two live tasks must never share one. * fix(paths): respect treehouse's path spelling, canonicalize our own Phase 5 salvage of fork fix 8107742. Two path-aliasing bugs with one root cause and opposite fixes. Where /home is a symlink to /var/home (the default on every ostree/atomic Fedora variant), one directory has two spellings, and whether to canonicalize depends on who owns the spelling. teardown: `treehouse return` string-matches its argument against the spelling treehouse recorded, so every teardown failed on such a box with "is not managed by treehouse". Firstmate never receives that spelling for a crew worktree (the worktree is found by polling the pane's cwd, which every backend reports OS-resolved), so ask treehouse's own inventory which spelling is its, matching on physical identity, at the handoff boundary - which also repairs metas written before this fix. Degrades to the caller's path when treehouse cannot be asked (fm-teardown.sh: treehouse_recorded_path). turn-end guard: fm_watcher_lock_matches_pid string-compared two of firstmate's OWN values, so a watcher armed from one spelling and a hook invoked through the other failed to match and the guard blocked turn ends at a live, beating watcher. Both sides are ours, so canonicalize both before comparing (fm-wake-lib.sh: fm_same_path; same fix in fm-watch-arm.sh). The matcher stays strict: a genuinely different home still fails. Upstream never handled the /home->/var/home aliasing (the closest, kunchenguid#765, settles same-task path-read races). Recorded in docs/treehouse-path-contract.md. Tests build the alias with ln -s so they hold on a box whose /home is not a symlink. * fix(backlog): name this home's backlog explicitly in hand-run tasks-axi guidance Phase 5 narrow salvage of fork fix 1b9fbf6. tasks-axi resolves both its .tasks.toml and the backlog path from the process working directory, so a tasks-axi call hand-run from a shell standing inside a project clone reads that clone's absent backlog as empty (a confident wrong answer) and can write a stray backlog.md into the clone - a forbidden project write. Upstream already pins every AUTOMATED bin/ call site to an explicit backlog file (fm-backlog-handoff.sh --file, the captain/decision-hold cd-into-home wrappers, session-start --file), so the fork's shared fm_backlog_file/fm_tasks_axi_run library rewrite is NOT re-applied - it would duplicate a solution upstream reached differently. Salvage only the residual instruction surface upstream lacks: the AGENTS.md section 10 rule that every hand-run tasks-axi call names `--file "$FM_HOME/data/backlog.md"`, and fm-teardown's post-teardown backlog reminder, whose printed done/ready commands a firstmate copy-runs, now naming the home backlog explicitly. * fix(supervision): stop false-wedging a finished crew that appends resolved: after done:, and stop churning stale wakes for a crew parked awaiting merge Phase 5 salvage of fork fixes c6b215c (#10) and c2c9168 (#12), bundled because #12 builds on #10's real-state reading. #10: AGENTS.md section 11 tells a crew that opened a keyed decision or blocker to durably close it with a `resolved:` line, so a crew that finished correctly ends its log `done: ...` then `resolved: ...`. The terminal-state scan read the raw last line, so it misread that finished log as non-terminal and false-wedged an idle-but-done pane. Add status_line_is_resolved + last_state_status_line to the one shared owner (fm-classify-lib.sh) and route stale_is_terminal, the away daemon's classify_stale / mark_escalated_seen / handle_wake, and crew-state's resolved-line promotion through it. `done:` then `resolved:` reads terminal; `working:` then `resolved:` (a mid-task blocker close) stays non-terminal. #12: a done crew with an armed merge-monitor (state/<id>.check.sh) idles until its PR merges; its redraw-jittered idle pane is not byte-stable, so the stale seam would re-surface it as a fresh possible-wedge on every distinct hash. Add crew_is_parked_awaiting_merge (reconciled done + armed check, re-read every evaluation, fail-closed) and consult it as the first absorb in both the always-on watcher's terminal-stale branch and the daemon's classify_stale, absorbing with no wedge timer. A re-activated crew (busy pane, or its verb moved off done) fails the predicate and full stale sensitivity resumes with no manual re-arm. Upstream's Stop-auto-arm continuity model did not close either gap (its classify_stale still falls a done:+resolved: log through to transient-stale). * fix(spawn): fail loudly when a spawn creates a pane but no agent starts Phase 5 salvage of fork fix 723ab78. fm-spawn printed `spawned` as soon as it typed the launch command; a launch send that never landed (observed live against a freshly restarted herdr server) left a bare no-agent shell yet still reported success and went In flight - a silent failure, especially dangerous in away mode. After sending the launch command, poll fm_backend_agent_alive until the agent is confirmed running before printing `spawned`. A confidently dead pane triggers one launch re-send (race mitigation) and, if still dead, a LOUD non-zero failure that names the pane. The check runs only for a backend+harness whose liveness probe can reach a confident verdict (new fm_backend_agent_probe_verifiable: herdr for any harness, tmux for non-pi harnesses); an inconclusive `unknown` proceeds with a warning (never a false failure), and unverifiable backends keep the pre-confirmation behaviour (a documented gap). Kimi already confirms its own ready+delivery, so it is skipped. Upstream added no general spawn confirmation (only a kimi-specific one). FM_SPAWN_CONFIRM=0 disables it; tests/lib.sh defaults it off so every fake-pane spawn test skips the poll. * test(spawn): release the pool-base lease before the idempotent-repeat spawn Fixture correction belonging with the worktree-lease guard (fdc9099): the idempotent-repeat case re-spawns a second task id into the same pooled base to prove the base refresh is idempotent, which the new lease guard correctly refuses (two live tasks must never record the same worktree). Model the first task's teardown by removing its meta before the repeat, exactly as the dispatch-profile path-spelling fixture does. Found by the full-suite run; the test passes on the base and failed only under the guard. * fix(fm-pr-merge): guard --hostname host-redirect and pin GH_HOST on the merge call Phase 5 salvage of the second half of fork fix b810a37 (#42). Upstream 5b6d0fb (kunchenguid#2779) independently closed the -R/-dR bundled-cluster half of #42 (already on the trunk), but the trunk's reject_repo_overrides lets --hostname pass straight through its --*) arm, and the github merge call sets no GH_HOST. --hostname is a gh-axi/gh global flag that sets GH_HOST for the child gh, so an extra --hostname arg merges the recorded PR against a different GitHub host while state still records the github.com URL - a merge-authority host redirect the trunk did not guard. Add --hostname|--hostname=* to reject_repo_overrides and pin GH_HOST=github.com on the github merge call. Correct the cluster-guard comment's rationale (gh-axi does not currently expand/forward such a cluster; the guard stays correct defensively either way), which #42's own review flagged. New tests: --hostname and --hostname=<value> refuse before recording or reading state, and the successful github merge is pinned to GH_HOST=github.com (both mutation-verified). The GitLab path already scopes glab by the URL host. * docs/style: one sentence per line, trim duplicated contracts, tidy a log string Review cleanups (non-blocking findings 4-6, 8): - docs/treehouse-path-contract.md: split the three multi-sentence audit bullets to one sentence per line (repo style). - One-owner: the treehouse path contract is now stated in full only in docs/treehouse-path-contract.md; the fm_same_path (fm-wake-lib.sh) and treehouse_recorded_path (fm-teardown.sh) headers keep the load-bearing "do NOT canonicalize a path an external tool owns - that is the bug" note plus a one-line why and a cross-ref, dropping the restated ostree-aliasing story. - One-owner: the "scope is never truncated because it is the routing input" rationale lives once in docs/configuration.md; fm-home-seed.sh's header states the fact and points there. - fm-supervise-daemon.sh: classify_stale's transient-stale log line now prints $term (the real-state line the decision is made on), not the raw $last, which could show a trailing resolved: bookkeeping line. No behaviour change. --------- Co-authored-by: Marlon <marlonleicester@gmail.com>
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
1 participant
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.
The bug (observed live many times on 2026-07-17)
A crew that finished correctly - PR open, pane idle, work pushed - was escalated as
stale: <pane> (idle Ns, possible wedge), repeatedly, each escalation burning a firstmate turn on a non-event (and false-waking firstmate during afk). It bit crews that did the right thing.Cause
AGENTS.md section 11 tells a crew that OPENED a keyed decision or blocker to durably CLOSE it with a
resolved:line. A crew that opens a blocker, gets unblocked, finishes, then closes the key ends its status log:The terminal-state check read only the LAST status line.
resolved:is not in the captain-relevant terminal verb set, so:bin/fm-crew-state.shreportedstate: unknown - no current-state source availablestale_is_terminalwas false, so a genuinely-done pane was treated as a possible wedge (both the always-on watcher and the away-mode daemon).Fix
Not a naive "add
resolved:to the terminal set" - aresolved:means a KEY closed, not that the TASK finished, and a crew can resolve a blocker MID-TASK and keep working.Fixed in the one shared owner,
bin/fm-classify-lib.sh(used by both the watcher and the away-mode daemon):last_state_status_linescans backward pastresolved:bookkeeping to the last REAL STATE verb, plus astatus_line_is_resolvedhelper.done:thenresolved:->done:(terminal);working:thenresolved:->working:(NOT terminal).Every terminal-state caller now routes through it:
stale_is_terminal(always-on watcher)classify_stale,mark_escalated_seen, and the transient-stale dispatcher (away-mode daemon), so both modes agreefm-crew-state.sh's no-run fallback reveals a TERMINAL verb (done/failed) through a trailingresolved:but never resurrects ablocked:/needs-decision:the resolve CLOSED - which key a resolve closes stays owned by the keyedstatus_open_decisionsfold.This directly improves away-mode reliability: the daemon uses this exact predicate, and a false-wedge escalation during afk woke firstmate for nothing.
Tests
Regression tests pin both directions in the colocated suites:
tests/fm-watch-triage.test.sh:stale_is_terminalfordone:+resolved:(terminal) andworking:+resolved:(not terminal), plus directlast_state_status_line/status_line_is_resolvedcoverage.tests/fm-crew-state.test.sh:done:+resolved:reportsdone(real state), whileblocked:/needs-decision:+resolved:still fall to idle.Verified green: fm-watch-triage, fm-crew-state, fm-daemon, fm-watcher-lock, fm-watch-checkpoint, fm-wake-daemon-lifecycle-e2e, fm-afk-return, fm-pi-watch-extension, fm-fleet-snapshot-view, fm-bearings-snapshot, fm-lint. No new /tmp fixture leaks.