Skip to content

feat(bin): give the crew-state reader typed verdicts and freshness (land of upstream #1829) - #59

Open
sbracewell64 wants to merge 7 commits into
mainfrom
fm/land-cfvc-05-onto-fork
Open

feat(bin): give the crew-state reader typed verdicts and freshness (land of upstream #1829)#59
sbracewell64 wants to merge 7 commits into
mainfrom
fm/land-cfvc-05-onto-fork

Conversation

@sbracewell64

Copy link
Copy Markdown
Owner

Lands the structured crew-state work (CFVC-05) onto the fork trunk.

The contribution exists upstream as kunchenguid#1829 (pipeline head 07599714). This branch was cut fresh from the fork trunk and carries only that contribution's own four commits; the upstream PR branch was left untouched.

What changes

  • bin/fm-crew-state.sh: a structured --json mode emitting the reader's derivation as typed fields (state, source, detail, precedence_applied, busy_signal, run_step, terminal_error, evidence_age_secs) instead of only the human prose line. A cancelled run now reports aborted and a pipeline that broke without judging the work reports interrupted - both re-run conditions, previously flattened into failed, which told firstmate the change had been rejected and sent it to fix code nothing had criticised.
  • bin/fm-classify-lib.sh: FM_CREW_STATE_VOCABULARY becomes the single owner of the verdict set, with crew_state_clears_open_decision and crew_state_absorb_class enumerating every member explicitly. Both return a distinct code for a verdict they were never taught, so a verdict added later cannot fall into the permissive branch by omission - which is how interrupted, stale, and idle silently cleared open decisions under the negative-condition chain this replaces.
  • bin/fm-busy-lib.sh: a BUSY turn record past FM_BUSY_MAX_BUSY_AGE_SECS (default 3600) classifies stale record-expired rather than being reported as a live turn forever. stale and unknown stay distinct: evidence that aged out is a different answer from no evidence.
  • bin/fm-fleet-snapshot.sh: consumes --json and projects the typed fields instead of re-deriving state/source/detail by splitting the prose line on its separators. A non-object read is rejected explicitly rather than being projected into an all-null object.
  • Test suites for all of the above, plus tests/wake-helpers.sh serving both reader modes from one canned verdict.

Landing verification

Three-way merge screen against the fork trunk:

$ git merge-tree --write-tree fork/main HEAD; echo "exit: $?"
8c731e9384aceab8b323a67ca1e156a37b832f20
exit: 0
$ git rev-parse HEAD^{tree}
8c731e9384aceab8b323a67ca1e156a37b832f20

Merge result equals this branch's own tree, so nothing on the trunk is lost. The branch touches the same 13 files as the contribution's own diff.

Five conflicts were resolved against the current trunk. Three were adjacency-only (both sides adding neighbouring functions); the remaining two were real and resolved so the trunk wins wherever the contribution did not deliberately change it:

  • crew_absorb_class (bin/fm-classify-lib.sh) - the trunk has evolved this past the version the contribution rewrote, gaining a settled class and an advancing-descendant-CPU fallback (fork fix(bin): detect child-process work during supervision (land of upstream #1676) #44). The contribution's deliberate change is the typed read and the vocabulary enumeration, not the removal of those sources. Resolved by keeping the trunk's structure and rewiring its shared read onto --json and crew_state_absorb_class, so both the trunk's extra evidence sources and the contribution's coverage gate survive.
  • checks-passed handling (bin/fm-crew-state.sh, AGENTS.md) - the trunk corroborates that claim against the run's own CI log before repeating it (fork fix(bin): refuse merges without verified green checks (land of upstream #1614) #48). The contribution only changed the failed/cancelled arms, so the corroboration is kept intact alongside the new aborted/interrupted verdicts.

The contribution's upstream base carries Muse-adapter and Relay-rename work that the fork trunk does not have. Those lines came in through conflict context only, and were dropped rather than landed - this branch introduces no reference to either.

Test evidence

bin/fm-lint.sh - clean (ShellCheck 0.11.0, pinned 0.11.0), exit 0.

bin/fm-test-run.sh --changed --base fork/main - 84 scripts, 1567 passing checks, 9 failing, 2 gate skips:

FM_TEST_SUMMARY total=84 failed=10 skipped_gate=2 duration_ms=1408209

That summary reads 10 because it was taken before the last commit on this branch; the tenth was this work's own flaky row, fixed in 32ed351 and re-run 5/5 green (see below). The nine remaining failures are unchanged.

This work's own suites pass:

  • tests/fm-crew-state.test.sh exit=0, 69 checks
  • tests/fm-busy-state.test.sh exit=0, 22 checks
  • tests/fm-fleet-snapshot-view.test.sh exit=0
  • tests/fm-watch-triage.test.sh exit=0 and tests/fm-daemon.test.sh exit=0 - the trunk's own consumers of the rewired absorb path

Flaky row found and fixed. tests/fm-crew-state.test.sh passed on one full run and failed on the next with no code change between them:

not ok - conformance row failed -> expired busy record is stale:
  prose detail [... (7200s ago)], structured detail [... (7201s ago)]

The row compares the prose and structured renderings of one verdict but reads them through two separate reader invocations, and an expired-record detail embeds the age measured at read time - so a second boundary between the two calls renders the same correct answer one tick apart. 32ed351 normalizes the measured age on both sides; any other divergence in the detail still fails the row, and the age value keeps its own assertion against a single read. Five consecutive runs after the fix: run1..run5 exit=0, zero not ok.

Nine failures disclosed, all proven pre-existing on the base. A clean clone of ed376cf (this PR's base, none of these changes applied) fails all nine identically:

$ git clone <repo> /tmp/cfvc05-base && git -C /tmp/cfvc05-base checkout ed376cf
$ bin/fm-test-run.sh <the nine scripts>
FM_TEST_SUMMARY total=9 failed=9 skipped_gate=0 duration_ms=169050
suite base ed376cf this branch
fm-calm-pi-extension exit=1 exit=1
fm-busy-adapter-wiring exit=1 exit=1
fm-pi-watch-extension exit=1 exit=1
fm-turnend-guard exit=1 exit=1
fm-watcher-lock exit=1 exit=1
fm-backend-tmux-smoke exit=1 exit=1
fm-launch exit=1 exit=1
fm-tmux-agent-liveness exit=1 exit=1
fm-session-start exit=1 exit=1

Gate skips (fm-pi-primary-types, fm-claude-stop-autoarm-live-e2e) are environment gates: Pi and a live Claude endpoint are not installed here.

CI state

The fork trunk's own CI is currently failing on main itself - the last five pushes to main all concluded failure. Checks on this PR are expected to be red for that reason rather than because of this change, and are not chased here. The local evidence above is what this delivery stands on.

CFVC-05. The fleet's authoritative current-state reader answered every
question with a confident-looking prose sentence and had no way to say it
did not know, so each unmodelled condition was coerced into some other
condition's verdict. This adds the structured mode the increment asks for,
and closes the four measured defects that shared that single cause.

WHAT WAS MEASURED (2026-08-06, no-mistakes v1.40.3)

1. busy-record-never-expires. bin/fm-busy-lib.sh parsed the record's ts=
   for format and never compared it to the clock, so a worker killed
   mid-turn classified `busy` forever. The watcher's BUSY_TURN_MAX_SECS
   cannot cover this: it ages a pane already believed busy, and a dead
   worker's pane renders an idle footer, so that backstop never arms for
   exactly these workers.
2. crew-state-unknown-after-resolved-line. A `resolved:` line - the closure
   verb every brief instructs crews to write - reported
   `unknown / none / no current-state source available`, claiming no source
   existed while the endpoint was readable and idle. Checking the whole verb
   set against bin/fm-classify-lib.sh rather than the two reported cases
   showed `captain-held:` hit the same fallthrough, confirming it was the
   verb CLASS and not two instances.
3. crew-state-treats-infrastructure-kill-as-rejection. `error: daemon
   crashed during execution` appeared on five terminal runs in one
   afternoon's history, each reported as `failed`. The pipeline broke; the
   work was never judged. Firstmate read it as the change being rejected.
4. crew-state-reports-aborted-run-as-failed. A deliberate `cancelled`
   outcome, which supersession produces on purpose, also reported `failed`.

WHAT CHANGED

Verdict vocabulary, one condition per verdict, none borrowing another:
`failed` (a step judged the work and rejected it), `aborted` (deliberately
cancelled), `interrupted` (the pipeline broke without judging), `idle`
(alive, nothing running, nothing declared), `stale` (evidence aged out),
`unknown` (genuinely no usable evidence).

The failed/interrupted split reads the pipeline's own structural
attribution marker - it prefixes step-attributed errors with
"step <name> failed:" - and never reads the error prose to guess whether a
step failure was "really" the code's fault. The full text travels to the
caller as terminal_error so the semantic residue stays visible instead of
being encoded as message-matching. `interrupted` requires POSITIVE evidence:
a terminal failure with no error field keeps the plain `failed` verdict,
because absence of evidence must not manufacture a claim in either
direction.

Freshness travels with the verdict. A busy record expires
(FM_BUSY_MAX_BUSY_AGE_SECS, 3600s); an idle record never does, because age
cannot make a finished turn unfinished. Expiry is terminal like malformed
and gen-mismatch, so no weaker source can re-answer for a worker the task's
own record just proved had stopped. Each answer reports evidence_age_secs
and precedence_applied.

RETIREMENT

The ad-hoc per-consumer source combination is gone, not left alongside the
new path. bin/fm-classify-lib.sh's crew_absorb_class and
bin/fm-fleet-snapshot.sh's crew_state_json both reconstructed structure by
slicing the prose line on its middle-dot separators; both now read typed
fields. No consumer in bin/ parses this reader's prose. crew_absorb_class
enumerates every verdict explicitly rather than silently defaulting, which
is how a correct reader still produced a wrong supervision outcome.

PREVENTION

tests/fm-crew-state.test.sh carries a conformance table of evidence-inputs
to expected-verdict covering every source combination and every sanctioned
status verb. Two coverage gates fail when a verdict or verb exists with no
row, so the next one cannot be added silently - that, not the four fixes, is
the point. FM_CREW_STATE_VOCABULARY in bin/fm-classify-lib.sh is the single
owner consumers and gates read.

CERTIFICATION - RED-CAPABLE

Run against the pre-change reader, the table reds on exactly the defect rows
and passes the other 18, proving it is precise rather than blanket-red:
  deliberate cancel is aborted        want=aborted      got=failed
  pipeline crash is interrupted       want=interrupted  got=failed
  expired busy record is stale        want=stale        got=working
  verb resolved closes a decision     want=idle         got=unknown
  verb captain-held closes a decision want=idle         got=unknown
  idle endpoint with no log is idle   want=idle         got=unknown
Against the change, 24/24 pass. test_conformance_checker_is_red_capable
proves in-band on every run that the checker rejects a wrong state, source,
and precedence, so it cannot silently become an always-green verifier. The
busy-expiry test includes a control that raises the bound and watches the
verdict return to `busy`, proving the new check is what decides.

TWO COMPLETION CRITERIA SUBSTITUTED, PER THE COMMISSION'S STALE-CRITERION CLAUSE

- `pane_hash` is not emitted. The reader computes no pane hash, and adding
  one would mean capturing and hashing rendered output on every heartbeat -
  the signal the semantic busy-state redesign deliberately abandoned because
  rendered output is not turn state. Replaced with `busy_seq`, the busy
  record's strictly-increasing counter, which answers the same question
  (did the worker's state actually move between two reads) from the
  architecture's own advancing-evidence source.
- The empty-check-set negative control is written as prose/structured
  equivalence rather than pinning `green`. bin/fm-crew-state.sh's
  nm_ci_checks_state is the single owner of that rule and upstream PR 1614
  is the open change that tightens it; duplicating its fix here would leave
  two owners of one invariant. The equivalence assertion is stronger in one
  respect: it guarantees the structured mode can never be the path that
  reports a green the prose mode would not, whichever way 1614 resolves.

EXCLUDED: RUN SELECTION - AND THE EXPOSURE THAT LEAVES OPEN

Resolving the CURRENT run for a task, rather than whatever the repo-scoped
status returns, is NOT in this change. Upstream PR 1816 is an open change to
this same file implementing exactly that, and the increment's own sequencing
note forbids opening another concurrent editor of it. Firstmate ruled that
1816 owns it: duplicating an open PR is the defect CFVC-02 exists to
prevent, and rebasing onto an upstream open PR would carry another author's
unlanded work into ours.

Verified rather than taken on trust, against the installed no-mistakes
v1.40.3:
- Bare `no-mistakes axi` DOES emit `runs[N]{id,branch,status,head,pr}:`. The
  comment still in this file saying that table never appears was verified
  against v1.32.2 and is now STALE. 1816's core mechanism is therefore real
  code, not the dead path that comment describes. Those exact lines are what
  1816 rewrites, so they are deliberately left untouched here rather than
  edited into a conflict.
- 1816 resolves the current run by filtering that table to the task's branch,
  then to a matching code head, preferring pending/running over
  completed/failed/cancelled regardless of row order, then re-querying
  `axi status --run <id>` and verifying id, branch, and head before accepting
  it. That is genuine current-run resolution, not a repo-scoped answer.
- The defect it fixes is live in the run history right now: branch
  fm/wedge-aging-ignores-provably-working carries one running run plus two
  older failed runs, distinguishable only by head - exactly the shape that
  produces a false failed verdict.
- One caveat: 1816's `active_run:` block parser matches that key exactly, but
  v1.40.3 emits `other_branch_active_run:` (confirmed in live output and in
  the binary's strings; no bare `active_run:` key found). That first helper
  appears inert on this version, so the runs-table fallback is what actually
  carries the fix. It is not a defect - a cross-branch block MUST NOT be
  attributed to this task - but the "authoritative when present" comment
  overstates what runs today.
- 1816 is delivered, not landed: open, unmerged, zero reviews, and no CI
  checks configured on it.

EXPOSURE. Until 1816 lands AND this fork resyncs, the false-failed-verdict
defect stays open: crew-state can report `failed` for a lane the pipeline
authority shows as running. Measured three times on 2026-08-06. Nothing in
this change detects or mitigates it; the typed verdicts make the wrong answer
machine-readable, not correct.

REVISIT TRIGGER. If 1816 has not landed by the time this fork next resyncs,
firstmate re-evaluates rather than letting the gap drift.
The fork trunk's crew_state_is_settled explained its `failed` exclusion by
noting that failed also reconciled a cancelled run. This contribution splits
that: a cancelled run now reports `aborted` and a broken pipeline reports
`interrupted`. The case arms already return the right answer for all three -
only the stated reason had gone stale, along with two comments describing the
reader's prose line that is now a typed verdict.
The typed read replaced a prose parse whose non-matching line yielded
`unreadable`, which the watcher's process-liveness source is still allowed to
answer for. A present-but-empty state field carries no answer either, so it
takes the same path rather than falling through to a decided-looking class.
The conformance row compares the prose and structured renderings of the same
verdict, but reads them through two separate invocations of the reader. An
expired-record detail embeds the age measured at read time, so a second boundary
falling between the two invocations rendered 7200s in one and 7201s in the
other - the same correct answer one tick apart - and failed the row. Observed on
a loaded machine: the row passed on one full run and failed on the next with no
code change between them.

The measured age is now normalized on both sides, so the row tests the rendering
it is there to test. Any other divergence in the detail still fails it, and the
age value keeps its own assertion against a single read, which is the only place
the two can be compared without a race.
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