Skip to content

feat(bin): add computed worker-state projection with provenance - #2866

Open
pramendra wants to merge 8 commits into
kunchenguid:mainfrom
pramendra:fm/fleet-ws3-single-state-projection
Open

feat(bin): add computed worker-state projection with provenance#2866
pramendra wants to merge 8 commits into
kunchenguid:mainfrom
pramendra:fm/fleet-ws3-single-state-projection

Conversation

@pramendra

Copy link
Copy Markdown

Intent

Workstream 3 of the fleet engineering plan (closes B): build one authoritative computed worker-state projection with explicit provenance (which source produced a state value, and when it was computed) that every surface reads instead of independently re-deriving worker state. Motivation: a pane-idleness heuristic and a single axi status snapshot read identically for 'slow but genuinely computing' and 'actually hung', and different surfaces (fm-crew-state.sh vs fm-peek.sh) had no structural guarantee they would agree about a worker's state at the same instant. Scope for this pass (deliberately narrowed): implement the projection and its provenance-carrying record shape in a new bin/fm-worker-state-lib.sh, and migrate the existing bin/fm-crew-state.sh and bin/fm-peek.sh read paths onto it, so both surfaces render the exact same computed function output and can never structurally disagree. fm-crew-state.sh becomes a thin renderer over the shared projection (its CLI output format is unchanged for backward compatibility with existing callers/tests). fm-peek.sh renders the same projection as a stderr annotation alongside its existing raw pane-tail capture on stdout (stdout stays byte-identical for every programmatic caller). fm-peek's call explicitly skips the projection's live pane/busy-probe tier because peek is about to make that exact live capture itself moments later - this avoids doubling a live backend round-trip and, for backends whose transport/mock is call-order-sensitive, avoids consuming or corrupting the response meant for peek's own raw capture (discovered and fixed via tests/fm-backend-orca.test.sh). Explicitly NOT built in this pass: a live dashboard or UI, and migration of the wake classifier (fm-classify-lib.sh) or fm-watch.sh/fm-supervise-daemon.sh's supervision hot loop onto the new library directly - fm-classify-lib.sh already benefits transitively since it shells out to fm-crew-state.sh, but the watcher/daemon's own direct fm_busy_classify calls (a separate, real disagreement source noted in the task brief) are left untouched as out of scope for this pass.

What Changed

  • Add bin/fm-worker-state-lib.sh, extracting fm-crew-state.sh's reconciliation logic (no-mistakes run-step → status-log/busy-state reconciliation → semantic busy-state contract → status-log fallback → unknown) into a single fm_worker_state_project function that emits a key=value record with state, source, detail, and computed_at provenance fields, plus fm_worker_state_field/fm_worker_state_render_line helpers.
  • Migrate bin/fm-crew-state.sh to a thin renderer over fm_worker_state_project, preserving its existing CLI output format; bin/fm-peek.sh now resolves the target's task id and prints the same projection as a stderr annotation before its raw pane capture, while stdout remains byte-identical for programmatic callers.
  • Add an optional skip-live-probe argument to fm_worker_state_project and thread it through fm_busy_classify (bin/fm-busy-lib.sh) so fm-peek.sh can skip live pane/busy round-trips (remote-endpoint state check, herdr-native busy check, Grok tail capture) it is about to perform itself, avoiding a doubled or response-corrupting live backend call.
  • Update docs/architecture.md and docs/scripts.md to document the new library, and add tests/fm-worker-state-lib.test.sh covering the projection's tiers and skip-live behavior; adjust tests/fm-crew-state.test.sh for the refactor.

Risk Assessment

✅ Low: All three previously-flagged skip-live double-round-trip gaps (herdr-native busy check, remote-secondmate state probe, grok fallback capture) are correctly fixed and each is now covered by a dedicated behavioral test with call-count assertions and a without-skip-live sanity check; the accidental .serena artifacts were removed and .serena/ added to .gitignore.

Testing

Ran the targeted new suite tests/fm-worker-state-lib.test.sh (8/8 pass) plus the existing fm-crew-state, fm-peek-remote, fm-backend-orca, fm-busy-state, and fm-busy-adapter-wiring suites (all pass, confirming no regression/behavior change to unmigrated CLI output). Manually exercised the real fm-crew-state.sh and fm-peek.sh binaries against a scratch worktree and fake tmux with an armed busy record, confirming byte-identical state/source lines and unchanged raw stdout on peek, directly demonstrating the workstream's core intent end-to-end.

Evidence: Manual E2E: fm-crew-state.sh and fm-peek.sh agree on the shared projection

Source: Manual E2E: fm-crew-state.sh and fm-peek.sh agree on the shared projection

state: working · source: pane · harness busy (claude-hook) -- byte-identical from both fm-crew-state.sh stdout and fm-peek.sh stderr; fm-peek.sh stdout remains the unchanged raw pane tail ('all quiet\n> \n').

Manual end-to-end verification: bin/fm-crew-state.sh and bin/fm-peek.sh reading the
shared bin/fm-worker-state-lib.sh projection for the same worker at the same instant.

Setup: a scratch git worktree + firstmate state dir with a fake tmux (pane idle text)
and a busy semantic record armed/applied via bin/fm-busy-event.sh (source claude-hook).

=== fm-crew-state.sh stdout (thin renderer over shared projection) ===
state: working · source: pane · harness busy (claude-hook)

=== fm-peek.sh: stdout (raw pane tail, unchanged) vs stderr (projection annotation) ===
--- peek stdout ---
all quiet
>

--- peek stderr (state annotation line) ---
state: working · source: pane · harness busy (claude-hook)

=== Byte-identical check ===
MATCH: crew-state and peek agree exactly -> 'state: working · source: pane · harness busy (claude-hook)'

Observations:
- fm-peek.sh's stdout is the raw pane tail only ("all quiet\n> \n") - unchanged shape,
  no projection text mixed in, confirming stdout stays byte-identical for programmatic callers.
- fm-peek.sh's stderr carries the same "state: ... source: ... detail" line that
  fm-crew-state.sh prints on stdout, produced by the same fm_worker_state_project /
  fm_worker_state_render_line functions in bin/fm-worker-state-lib.sh - the two
  surfaces cannot structurally disagree because they render one shared computation.

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 2 issues found → auto-fixed (3) ✅
  • ⚠️ bin/fm-worker-state-lib.sh:261 - fm-peek's skip-live-probe flag (_fm_ws_skip_live=1) only gates pane_readable() and the grok-specific tail40 pre-capture inside crew_busy_verdict. It does not reach the herdr-native busy-state check inside fm_busy_classify (bin/fm-busy-lib.sh:897-903), which unconditionally calls fm_backend_busy_state("herdr", target) — a live agent get round trip — whenever no semantic busy record exists for the task. Concretely: peeking a herdr-backed ship/scout task that has no fm-busy-event.sh record and no matching no-mistakes run makes this live herdr call from the projection annotation, immediately followed by fm-peek.sh's own separate live capture call to the same backend/target moments later — exactly the double-live-round-trip hazard the skip-live flag was introduced to eliminate (per this file's own header and fm-peek.sh's header comment), just for herdr instead of orca. The library's own comment above the pane_readable gate ("this tier still answers for free whenever a semantic record exists") implicitly concedes this: the caveat only holds when a record exists, but doesn't get enforced when it doesn't. No test in this change (or the existing herdr/orca suites) exercises this path — the herdr test that calls fm-peek.sh uses a session:window raw target with a colon, which fm_backend_task_id_for_selector rejects outright, so the projection call is skipped entirely and this gap is never hit.
  • ⚠️ bin/fm-worker-state-lib.sh:211 - The remote-secondmate tier of fm_worker_state_project (lines 211-237) is also not gated by _fm_ws_skip_live: it unconditionally calls fm-on.sh ... fm-remote-secondmate-control.sh state <id> — a live remote round trip — before returning. Since fm-peek.sh now calls the projection (with skip_live=1) unconditionally whenever a task id resolves, and separately makes its own fm-remote-secondmate-control.sh capture <id> <n> call moments later for the same remote secondmate, every fm-peek.sh invocation against a remote secondmate now costs two live round trips to the remote host instead of one, contrary to the stated goal of the skip-live tier.

🔧 Fix: {"summary": "Gate herdr-native and remote-secondmate live probes on skip-live flag"}
1 warning still open:

  • ⚠️ bin/fm-busy-lib.sh:926 - The fix round threaded skip_live into fm_busy_classify but only gated the herdr-native check (line 902); it left the grok arm (lines 926-937) unconditional. fm_worker_state_project's crew_busy_verdict (bin/fm-worker-state-lib.sh:277-285) skips its own grok tail40 pre-capture precisely when skip_live=1, which guarantees tail40 arrives empty at fm_busy_classify in that case. The grok arm then sees tail40 empty and makes its OWN live fm_backend_capture call (line 929) - exactly the doubled live round trip / call-order-sensitive-mock corruption the skip-live flag exists to prevent, and the specific scenario the header comments and intent cite as the original motivating discovery (via tests/fm-backend-orca.test.sh's queued fake responses). Concrete path: a grok-harnessed ship/scout task with no semantic busy record (fm_busy_record_read fails), no matching no-mistakes run, no remote_host - fm-peek.sh calls fm_worker_state_project(id, 1), which reaches crew_busy_verdict with tail40='', calls fm_busy_classify(...,tail40='',skip_live=1), which - since backend/harness don't hit the earlier semantic branches - falls into the grok arm and issues a fresh live capture, immediately followed by fm-peek.sh's own separate live capture at bin/fm-peek.sh:70 to the same backend/target. For a call-order-sensitive transport/mock (queued responses), the second capture can consume the wrong queued response or run out, corrupting fm-peek's stdout. No test in this change exercises harness=grok through fm-peek's skip_live=1 path (the existing orca fm-peek test uses harness=claude, which never reaches the grok arm), so this gap goes unexercised.

🔧 Fix: {"summary": "Gate grok's fallback live capture on skip-live flag too"}
2 issues (1 warning, 1 info) still open:

  • ⚠️ .serena/project.yml:1 - Fix-round commit 6313454 added .serena/.gitignore and .serena/project.yml (171 lines) to the tree alongside the intended fm-busy-lib.sh/fm-worker-state-lib.sh changes. These are local Serena MCP tool config/cache-ignore files unrelated to gating herdr-native and remote-secondmate live probes, are not referenced by any code or test in this change, and are not covered by any existing .gitignore entry in this repo. They appear to be an accidental side effect of the fixer agent's tool usage during that round rather than an intended part of the fix, and now permanently pollute this branch's history and diff.
  • ℹ️ tests/fm-worker-state-lib.test.sh:130 - The three live-probe double-round-trip bugs fixed across the two auto-fix rounds (herdr-native fm_backend_busy_state, remote-secondmate state call, grok fallback capture) are all only reachable via harness=herdr, harness=grok, or a remote_host meta value, but tests/fm-worker-state-lib.test.sh's only skip-live coverage (test_peek_skips_the_live_probe_crew_state_still_makes) uses harness=claude with a local pane target, so none of the three fixed paths — nor a regression reintroducing them — would be caught by the test suite.

🔧 Fix: {"summary": "Untrack stray .serena files, add skip-live regression tests"}
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • bash tests/fm-worker-state-lib.test.sh
  • bash tests/fm-crew-state.test.sh
  • bash tests/fm-peek-remote.test.sh
  • bash tests/fm-backend-orca.test.sh
  • bash tests/fm-busy-state.test.sh
  • bash tests/fm-busy-adapter-wiring.test.sh
  • Manual: ran bin/fm-crew-state.sh and bin/fm-peek.sh directly against a scratch worktree + fake tmux + armed busy record, compared crew-state stdout to peek's stderr annotation, and inspected peek's stdout for unchanged raw pane-tail shape
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Your Name added 5 commits August 23, 2026 21:10
…and peek

fm-crew-state.sh's authoritative reconciliation (run-step, else the
semantic busy-state contract, else the status log) used to live entirely
inside that one script. Extract it into fm-worker-state-lib.sh as a
reusable projection carrying explicit provenance (source and a computed_at
instant) that fm-crew-state.sh now just renders, and have fm-peek.sh render
the same projection as a stderr annotation so a reader is never left to
judge busy-vs-hung from raw pane text alone. Peek's call skips the
projection's live pane/busy probe since peek is about to make that exact
capture itself, avoiding a doubled backend round-trip.
@greptile-apps

greptile-apps Bot commented Aug 23, 2026

Copy link
Copy Markdown

Confidence Score: 4/5

The PR is not yet safe to merge because short Grok peek captures can still make its state annotation disagree with crew-state.

fm-peek passes the caller-selected line count directly into Grok classification, while crew-state classifies a 40-line capture, so a busy signature omitted from a short peek remains visible to crew-state and produces conflicting current states.

Files Needing Attention: bin/fm-peek.sh

Reviews (4): Last reviewed commit: "no-mistakes: apply CI fixes" | Re-trigger Greptile

Comment thread bin/fm-busy-lib.sh
Comment on lines +930 to +933
if [ "$skip_live" = 1 ]; then
printf 'unknown live-probe-skipped'
return 0
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Skipped Grok probe blocks fallback

When fm-peek.sh projects a Grok worker with no semantic busy record, no matching run, and a valid status-log state, this branch returns unknown live-probe-skipped; the projection immediately emits that unknown pane result instead of reaching its file-only status-log fallback, causing peek and crew-state to report different states for the same worker.

Comment thread bin/fm-worker-state-lib.sh Outdated
Comment thread bin/fm-worker-state-lib.sh
Comment thread bin/fm-peek.sh
# as the direct unwrapped call used to.
RAW=$(ec=0; fm_backend_capture "$BACKEND" "$T" "$N" "$EXPECTED_LABEL" || ec=$?; printf x; exit "$ec")
RAW=${RAW%x}
WORKER_STATE_RECORD=$(fm_worker_state_project "$TASK_ID" 1 "$RAW") && fm_worker_state_render_line "$WORKER_STATE_RECORD" >&2

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Short capture misclassifies Grok state

When a caller requests fewer than 12 lines and a recordless Grok worker's busy signature is inside the canonical projection window but outside that shortened capture, fm-peek.sh passes the incomplete tail to the projection and reports an idle or status-log-derived state while fm-crew-state.sh reports working.

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