Skip to content

feat(bin): make a terminal done claim mean something a machine checked - #3683

Open
ICGNU3 wants to merge 21 commits into
kunchenguid:mainfrom
ICGNU3:fm/fm-done-means-verified-v1
Open

ICGNU3 wants to merge 21 commits into
kunchenguid:mainfrom
ICGNU3:fm/fm-done-means-verified-v1

Conversation

@ICGNU3

@ICGNU3 ICGNU3 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

What this changes

A terminal done: claim was free prose. bin/fm-dod-lib.sh told every no-mistakes worker to append done: PR {url} checks green; bin/fm-classify-lib.sh matched the token done and the fleet treated the claim as fact. There was no step between "worker asserts" and "firstmate believes", and nothing in a claim identified the commit being claimed, so nothing could compare what a pipeline validated with what a PR shipped.

Three failures in this fleet came from that gap: a claim that a post-validation commit was a "behavioral no-op" when the regression test proving the fix had changed by 67/31 and had never been executed by any run; a terminal claim on a commit the pipeline never saw, made by force-pushing over the validated head; and a done: PR ... checks green for a PR later closed without merging, which still read "done" three weeks later because the merge poll only ever watched for merged.

Now:

  1. Terminal claims carry machine-checkable identity. bin/fm-done-claim-lib.sh is the single owner of the grammar, its parser, and the durable verdict; bin/fm-dod-lib.sh and bin/fm-brief.sh render the worker instruction from that owner, so what a worker is told to write and what firstmate parses cannot drift.
    • no-mistakes / direct-PR: done: pr=<url> head=<full-sha> - <one line>
    • local-only: done: branch=fm/<id> head=<full-sha> - <one line>
    • scout: done: report=<path> - <one line>
  2. A verifier decides, not the worker. bin/fm-verify-done.sh <task-id> establishes the claim from the forge, from git, and from the validation run's own record - never from the claim's prose. It answers verified (0), unverified (3, could not establish) or contradicted (4, established and false), and writes state/<id>.done-verdict bound to the exact claim it judged, so a later claim supersedes rather than inherits a verdict.
  3. Nothing treats an unverified claim as done. bin/fm-crew-state.sh reports done-unverified with the reason; bin/fm-teardown.sh refuses unverified and contradicted alongside its existing landed-work gates; the fleet snapshot/view carry the token and the session-start fleet digest prints the durable verdict per task (a local read - the digest still makes no network call).
  4. A closed-unmerged PR contradicts its own done record. bin/fm-pr-poll.sh now reports closed-unmerged as well as merged, through the existing poll; bin/fm-merge-outcome-lib.sh publishes it as a contradiction, and the notification marker gained a v2 line recording which outcome was delivered, so a close is reported once and a later reopen-and-merge of the same PR still reports its own outcome. Only merged retires the poll.
  5. Non-conforming status lines are visible. A line matching no known verb (Migration syntax: OK) is surfaced in the drain marked UNRECOGNISED (matches no status verb): ... instead of being absorbed. Nothing is rejected - losing a worker's words is worse than showing an unclassifiable line - but unlike note: lines these are bounded per drain, with a count of any held back, so one chatty worker cannot bury the rest of the drain.

Two deliberate boundaries: the crew-state downgrade fires only when a claim exists (a run-step passed on a task that never claimed anything is already machine-established and stays done), and done-unverified is a current-state token from that reader only - never a status-log verb, so the classifier's vocabulary is untouched.

Proved by failure, in a disposable fixture

Each of these was planted and watched be caught before any of it was trusted. Validated commit b5df0aa, shipped commit 3aed810, a fake forge and a fake validation run:

1. a done: naming a PR that does not exist
unverified: the forge could not be reached for https://github.com/o/r/pull/999, so the shipped commit is unknown
  -> exit 3

2. a done: whose head= differs from the PR's actual head
contradicted: https://github.com/o/r/pull/7 is at b5df0aa5fb8da235490d2ebfd2db84eeb06e096e, not the claimed 3aed810c636c698de90d5b167ff544cf68cc1151
  -> exit 4

3. a done: whose head= differs from the validated commit (the real failure)
contradicted: validation ran against b5df0aa5fb8da235490d2ebfd2db84eeb06e096e, but the claim ships 3aed810c636c698de90d5b167ff544cf68cc1151
  -> exit 4

4. a done: on a PR that is closed and unmerged
contradicted: https://github.com/o/r/pull/7 is closed without merging, so this task is not done
  -> exit 4

5. a legacy done: with no head= at all
unverified: legacy claim, no commit identity
  -> exit 3

control: an honest claim
verified: https://github.com/o/r/pull/7 is MERGED at the claimed 3aed810c636c698de90d5b167ff544cf68cc1151, validated at 3aed810c636c698de90d5b167ff544cf68cc1151 (run passed); checks: SUCCESS
  -> exit 0

Case 1 is caught as unverified rather than contradicted on purpose: gh pr view failing cannot distinguish "no such PR" from "network down" without depending on its stderr wording, and a vendor string is not something a guard this important should rest on. Not knowing is never a pass.

Two things the mocked proof above would not have caught, so they were checked for real:

  • The checks state is pulled out of a rollup that mixes check runs (.conclusion, null while a run is still going, then .status) with commit statuses (.state). A fallback applied across the whole list rather than per entry silently drops every entry of the other shape and records SUCCESS for a pull request with an in-progress check. tests/fm-done-verified.test.sh now feeds a real payload through the script's own query with real jq (skipping itself where jq is absent) and asserts all three shapes survive.
  • The claim's summary is arbitrary worker prose, so splitting it into fields disables globbing first; otherwise an unquoted *.md in a summary would expand against whatever directory the reader is in. Pinned, including that the caller's own setting is restored.

All five failures, plus the honest control, are pinned in tests/fm-done-verified.test.sh (16 cases, including the grammar round-trip, the verdict-to-claim binding, the poll's two terminal outcomes, the marker's outcome dedup, and the drain flag). The teardown gate is pinned in tests/fm-teardown.test.sh (5 cases: no claim, legacy, contradicted, established, --force), and the crew-state downgrade in tests/fm-crew-state.test.sh (4 existing assertions tightened from a vacuous state: done substring match, plus one new case proving an established claim still reports done).

This change is the first thing held to its own contract

The delivery of this PR reports in the grammar it introduces:

done: pr=<this PR's url> head=<the commit this PR carries> - <one line>

That is not decoration. The first draft of this work was reported in the exact free-prose shape the change abolishes, which is the change failing its own contract on first use. It is reported the new way now, and bin/fm-verify-done.sh is run against that claim before it is made.

The four test failures in this run pre-date this change

Named, and shown A/B rather than asserted. Baseline is 3d2a08b (this branch's parent), branch is 3b7baee; each suite was run from a pristine git archive of the baseline and from the branch, on the same machine, back to back:

Suite Failing assertion Baseline 3d2a08b Branch 3b7baee
fm-wake-queue a subshell reclaimed its parent's live hold (rc=13) rc=1, fails rc=1, fails identically
fm-muse-harness fm-harness.sh under process 'muse-bin-0.1.0-R708.1' reported '', expected muse rc=1, fails rc=1, fails identically
fm-composer-lib a half-block rule row must count as a structural edge rc=1, fails rc=1, fails identically
fm-teardown herdr-preflight-missing-adapter: teardown continued without its required preflight rc=1, fails rc=1, fails identically

Same assertion, same exit code, on both sides of the change. The fm-teardown case is the one worth noting explicitly, because this PR does add a gate to that script: its five new claim-gate assertions all pass, and the suite's only failure is the herdr-preflight case, which fails the same way with none of this change present.

Out of 88 suites and 2135 assertions in the changed-test selection, those four are the only failures.

What this costs, and what it will refuse

  • Every in-flight task's claim is legacy. A claim written before this contract carries no commit identity and can never be established, so its teardown refuses until a conforming claim is appended. The refusal says exactly that. This is the intended direction - acceptance only gets stricter - but it is a real migration step for work already running.
  • Armed merge polls need one re-arm. The poll is byte-identity-validated, so changing bin/fm-pr-poll.sh invalidates every existing registration; those polls surface as rejected unauthenticated state checks until bin/fm-pr-check.sh <id> <url> re-arms them. That is the validation working, not breaking.
  • An unreachable forge blocks teardown. unverified refuses, as specified. This matches how teardown already refuses when it cannot establish landedness.
  • A GitLab merge request can never be established, so its cleanup needs --force. glab exposes a merge request's head commit only inside JSON, which would need a JSON processor firstmate does not require - the same reason bin/fm-pr-check.sh already records no pr_head for GitLab. The verdict says so explicitly and says re-running will not change it. I did not write a glab --output json head read because there is no glab here to prove it against, and firstmate-coding-guidelines is explicit that a vendor-output-dependent check must be proven end to end against the real tool rather than assumed. This is the one place worth a decision before merge: accept --force for GitLab cleanup, or land a glab head read separately, proven on a machine that has glab.

Overlap with open work

No open PR does any of this, so nothing here duplicates one. There is ordinary file overlap to rebase through: bin/fm-classify-lib.sh, bin/fm-teardown.sh and bin/fm-wake-drain.sh with #3604; bin/fm-watch.sh with #3605 and #3594; bin/fm-pr-lib.sh with #3615 (different functions - that PR changes the metadata identity whitelist, this one the notification marker); AGENTS.md and docs/scripts.md with several.

Two corrections to the record of how this was built

This change is about durable records telling the truth, so its own record should.

A wrong instruction, and where it came from. The local-only guard was first hardened only on the retired-branch path. Fixing that, firstmate directed the invariant be expressed as the claimed head must differ from the merge-base of fm/<id> and the default branch — an option this worker had proposed, and which firstmate chose. Neither of us checked it against the shape of landed work, and it was wrong. For local-only work that has landed, merge-base(fm/<id>, default) equals the branch tip; that is the ordinary shape of landed work, so the rule would have reported every honest landed claim as contradicted — the exact false-contradiction class the same instruction had just been written to eliminate. The adversarial review caught it, not us. The shipped invariant instead reads the branch's own creation point from its reflog: a branch still sitting where git created it introduced nothing, and unreadable history reads unverified, never contradicted, because absence of evidence is never evidence of falsity.

Re-review did not cover the last round. The pipeline's review agent went silent and hit its 30-minute timeout three times over the life of this branch. That is infrastructure, not this change: every fix round applied its work and preserved it, and lint plus the full 53-case suite passed on each recovered head. The honest scope, stated narrowly because a limitation named with its blast radius is useful and a vague warning is noise: five re-review passes completed (the initial review and four fix rounds), and the final applied round was not adversarially re-reviewed. That round changed exactly five things, so a reviewer knows where to look rather than being told to distrust everything:

  1. the keyed-supersession authority fix — only a line with no correlation key may withdraw a task's claim, so a child's keyed failure cannot retract a parent's;
  2. the poll-retirement proxy fix — retire whenever the outcome is on the record, not only when a verdict was written;
  3. the .. report verdict — refusing to resolve a path is unverified, not contradicted;
  4. the FM_DRAIN_UNRECOGNISED_MAX=0 clamp, which previously wedged a task's cursor;
  5. the AGENTS.md correction distinguishing contradicted (close) from stale (merge).

A standing instruction goes stale exactly like a done claim. The sentence originally drafted for this section said the change was never able to complete a re-review pass. That was true when it was written and false by the time it would have been published — five passes had completed in between. An instruction is a claim about the world, and it is answerable to the same test this change applies to a worker's claim: verified at the moment you act on it, not at the moment it was issued. Executing it faithfully would have put a false statement into the very document arguing that records must be true.

A third failure mode: measuring what is easy instead of what it means

Distinct from an untrue record, and distinct from a rule diffused across sites. The local-only guard's invariant was wrong three times running, and each version tested did something change rather than did this branch author work — merge-base, then "the branch moved since it was created". Three attempts, three different cheap proxies for a question none of them actually asked.

The tell was available every time: the reason line the guard emitted was false. It narrated "which it introduced after being created at B" about a branch that introduced nothing. A guard that has to narrate why it passed, and narrates something untrue, is telling you the invariant is wrong.

The same substitution turned out to be in three of the four arms — consistency where the invariant was authorship:

Arm What it tested What it should have tested
local-only the branch moved the branch recorded a commit
scout some file exists at the claimed path this task's report exists
direct-PR the claim is internally consistent with the forge this task produced that PR
no-mistakes — already bound: the validated commit must come from this worktree's own run —

The direct-PR case is the sharpest: it could be satisfied by naming any open PR whose head you state correctly. Not a weak binding — no binding. Each of these was found by asking the same question of the next arm rather than waiting for a review to ask it.

The change briefly did the thing it was built to stop

The honest headline of the last round. The authority fix made fm_done_claim_last stop reading through a [key=...], so a routed-phase close is no longer mistaken for the task's own claim. But five other readers still read keyed lines as terminal, and one of them is bin/fm-crew-state.sh — the reader this entire change centres on. With no claim left to downgrade, a routed-phase done [key=docs] made the whole task read plain done, where before this branch it read done-unverified.

A change built to stop a task looking more finished than it is briefly made a task look more finished than it is.

The mechanism is worth more than the embarrassment. Making one reader stricter while its siblings stayed loose did not merely leave a twin open — it created a disagreement between readers about the same line, which is worse than either behaviour applied consistently. Six readers giving two answers is duplicated authority with the contradiction written down and accepted. The repair is not five corrected copies of the test, which would only reset the clock; it is one shared predicate every reader must call, the same shape as making observation and recording inseparable at the merge funnel.

One boundary was deliberately not crossed. The same key-blindness exists in readers of paused, blocked and needs-decision — but there it is load-bearing rather than defective, because a routed phase that is blocked or paused genuinely is a supervision concern of the task, and the keyed-decision fold is built on keys meaning exactly that. Only for a terminal outcome does a keyed line state something untrue about the task. Widening the rule to those readers would have been the merge-base error again: a rule that is correct for the case that prompted it, applied where its premise does not hold.

Who found the twins, and how

One defect shape recurred through this whole change: a rule closed at the instance that prompted it and left open at its twin. It happened often enough to be worth recording how each one surfaced, because the means matter more than the count.

The twin Found by
report_to_parent guarded, its sibling report_child_ledger_locked left publishing a raw verb adversarial review
absent mode= turning a true local-only claim into contradicted adversarial review
the scout arm accepting some file rather than this task's report adversarial review
the local-only invariant as merge-base — correct unlanded, wrong landed the false reason line it emitted
the local-only invariant as "the branch moved" — defeated by the rebase the brief instructs adversarial review, via the same false reason line
retraction authority guarded, assertion authority left open adversarial review
the retirement proxy — "a verdict was written" standing in for "the outcome is on the record" adversarial review
.. rejection testing the spelling of a location rather than the location adversarial review
verdict and narration gated on different tests adversarial review
the pessimistic record that rots — a child reported unverified and never corrected adversarial review
the merged twin of the closed-unmerged verdict author, doing a mapping firstmate asked for
the direct-PR arm testing consistency rather than authorship author, sweeping the sibling arms on instruction
AGENTS.md §7 "always" contradicting §9's conditional author, checking a merge he had just driven

Three of thirteen were found by the author, and both of those came from being told to go and count something rather than from noticing unprompted. The false-narration tell caught two invariant errors and one wording error on its own. Everything else came from the adversarial layer.

That is the argument for keeping an adversarial review even when the author has internalised the lesson — by the end of this change the author could state the pattern precisely, describe its mechanism, and still miss its next instance in a file he had already edited. Knowing the shape of a defect is not the same capability as detecting it in your own work.

Half-stated rules: the symmetric case is the one that gets missed

Three rules in this change were stated for the case in front of us and not for the symmetric one, and each missing half was found later by review rather than by the person who wrote the rule:

Rule as stated The half that was missing
the local-only invariant, as merge-base correct for unlanded work, wrong for landed work, where merge-base is the tip
who may retract a claim who may assert one — the same question, left open inside the fix that closed the first half
the funnel records contradicted on a close only when the standing claim is about that PR, not whichever claim happens to stand

The pattern is not carelessness about detail; each rule was precise about the case it named. It is that a rule gets written against the instance that prompted it, and the instance never contains its own mirror image. Authority over a claim turned out to be one question — who may change what claim stands — and writing it as two rules, assertion and retraction, is exactly what let the second stay open while the first was being closed.

Where a rule has a direction, the discipline that would have caught all three is to ask what the same rule says pointed the other way, before implementing the direction you were handed.

A checklist of known points is a proxy for the set of actual consumers

The seventh proxy in this change, and the first whose subject is exhaustiveness itself.

Adding a verb to the fleet's vocabulary needs more than writing it down, so a five-point registration checklist was specified up front: captain-relevant set, known-verb set, not-terminal set, current-state mapping, wedge classification. All five were correct. Two more existed — the free-text captain regex, where an unanchored token made worker prose containing ready: silently absorbed, and the summary buckets, where the new state landed in none of them while the snapshot still reported valid.

A checklist of known registration points is a proxy for the set of actual consumers, and the difference between them is where the next defect lives. The checklist was written because the author knew a new verb needs registering — knowing the hazard produced a list of remembered places rather than a sweep of real ones, and a remembered list feels like coverage in a way an unfinished sweep does not.

The repair for the specific instance is to register the state everywhere. The repair for the class is to stop relying on the enumeration being complete: a snapshot that reports valid while a live item belongs to no bucket is broken independently of which state caused it, so an unmatched state now surfaces as a named invalidity rather than vanishing. A future state added without touching the buckets fails loudly instead of disappearing quietly.

Defensive additions are surface, not safety

The sixth proxy in this change, and the first whose source was defensiveness rather than convenience — which is why it is worth naming separately.

Of five findings in one round, three came from constraints added defensively rather than derived from the invariant: two volunteered as safety (gating poll retirement on the verdict write; narrowing supersession to one verb), one demanded as belt and braces (rejecting any .. component outright). Each became a new proxy, and each grew its own false narration:

  • gating retirement on the verdict write used "a verdict was written" as a proxy for "the outcome is on the record" — so a close with no claim to write a verdict for, which is the documented normal case, left the poll asking the forge forever. The safety addition reintroduced the exact standing cost the change had just removed.
  • narrowing supersession ruled what may withdraw a claim and never who may speak for the task — so a keyed child failure, published by code into a parent's status, retracted a mate task's own claim. An authority hole created by a narrowing meant to close one.
  • rejecting .. outright refused to resolve a path, then reported that refusal as contradicted with the reason "walks out of its own directory" — false for a path that resolves inside it.

A safety addition that is not derived from the invariant is not extra safety, it is extra surface. Each of these felt like tightening. Each added a new thing that could be wrong, in the same shape as everything else this change catalogues: a cheap stand-in for the question actually being asked.

One word doing two jobs

The brief told a no-mistakes worker to append done: {summary} before the pipeline had run. Under the contract this change introduces, that line is the task's standing terminal claim — carrying no identity — so from the moment a worker followed its own brief, every no-mistakes task read done-unverified. Including the run-step path whose exemption comment says a task that never claimed anything stays done: the brief guaranteed a claim always existed, so the exemption was unreachable, in a comment asserting coverage the code could not have.

The reason string was untrue in a specific way worth naming: it said "legacy claim" and teardown said "a claim written before this contract" — assertions about when the line was written, about a line the current brief had produced minutes earlier. A guard asserting the provenance of evidence it never observed, inside a change about not asserting what you have not established.

The repair is not to teach the readers to ignore a pre-validation done:; that would be a seventh proxy. It is that done: was doing two jobs — I have finished implementing, now validate and the task is complete — and every consequence followed from the conflation rather than from any reader being wrong. The pre-validation handoff gets its own verb, so done: is terminal by construction.

That turned out to need more than a word swap: the handoff must stay captain-relevant, since that is how firstmate learns to start the pipeline, and working: and paused: are both deliberately excluded from captain relevance. A new verb has to be registered — in the captain-relevant set, in the known-verb set (or the unrecognised-line surface this same change added would flag it), kept out of the terminal set, given a truthful current-state mapping, and checked against wedge classification. The fleet simply had no verb for "implementation complete, awaiting the go-ahead to validate", and the missing vocabulary is what made a completion word get borrowed for a phase marker.

And the temporal claim was dropped rather than special-cased: a claim with no commit identity is now described as exactly that, whenever it was written, which is truthful for the old population and the new one at once.

A false premise is more dangerous than a false conclusion

Every earlier instance in this change was a false reason line describing what had happened. The last one was different: the guard did not narrate an untrue conclusion — it rested on an untrue premise, and stated that premise in its own comment as the argument for why the rule was sound.

The comment asserted that "a correlation token or a [key=...] only ever follows the verb word". This fleet's classifier recognises two stated positions for that key, before the colon and at the head of the note, so the sentence was false — and it was load-bearing, because it was the justification the single-speaker rule rested on. A sub-event could assert or retract a task's claim through the spelling the premise had ruled out.

A wrong premise offered as justification is more dangerous than a wrong conclusion, because a reader checking the reasoning finds an argument that looks complete and stops there. A false conclusion invites the next question. A false premise closes it.

When a fact has an owner, ask the owner

Three defects in this one change reduce to the same sentence, and it is worth stating as a rule rather than leaving as three fixes that happen to share a shape:

  • five readers each deciding privately whether a line was the task's own terminal outcome — and disagreeing once one of them was tightened;
  • the site that observed a PR's terminal outcome never calling the function that records it;
  • the authority predicate re-deriving "is this line keyed" instead of asking the classifier that already owns both spellings of the key.

When a fact has an owner, ask the owner; re-deriving it is how the copies drift apart. That is the general form of the duplicated-authority defect this repo already names in its own contributor rules — and it recurred three times inside a single change written by someone who had just read those rules.

The signal that kept working: a false reason line

Three times a guard here narrated why it passed, and the narration was untrue. "which it introduced after being created at B" — about a branch that introduced nothing. "the local copy is at the claimed X, which it committed at Y" — where Y was a different commit. In this change alone that signal caught two invariant errors and one wording error, which is the argument for a rule: a guard should say what it observed, never what it concluded. An observation can be checked against the world. A conclusion can only be checked against the reasoning that produced it, which is exactly what was wrong.

Making the way out honesty rather than force

Refusing cleanup on a false claim created a trap: a task whose PR was abandoned could never be cleaned up, and --force — tied to explicit discard authority — was the only exit. The fix was not to weaken the gate. It was noticing that the trap was never about the verdict at all: fm_done_claim_last treated a done: line as standing forever, so a task that honestly appended failed: was still held to an assertion it had withdrawn.

A later failed: line now supersedes the claim. A task that still claims done still cannot be cleaned up on a false claim, and unlanded work stays protected by teardown's own landed-work gates. What changes is that the exit from a false claim is withdrawing it rather than overriding the guard. Make the way out honesty and people take it; make it force and they learn to reach for force.

And the poll retires once the contradiction is durably recorded — a poll exists to observe a terminal outcome, and polling after recording it is waste.

What this change actually taught, in two sentences

The funnel is the single place where observing and recording become inseparable. A rule that lives in prose gets re-violated at every new site; a rule that lives in the one function every observer must call cannot be. fm_merge_outcome_report has exactly two callers and fm_done_verdict_write had exactly one, and that gap — the site that observes never touching the function that records — was the defect, not any individual site's logic.

Terminal evidence has three shapes, not two. Conflating the third with the first is what made a correct rule look like a collision:

Shape Example What it may do to a standing verdict
Absence of evidence — we could not look forge unreachable, gh off PATH, reflog pruned, run aged out Nothing. The world has not changed; we simply failed to check. Never downgrades.
Positive evidence of falsity — we looked, the claim is false PR closed without merging Records contradicted, and may overwrite.
The world changed — what was established is about a world that no longer exists PR merged, possibly at a head nobody verified Marks it stale. Not wrong, not still valid.

The anti-downgrade rule protecting a verdict from a transient outage was right, and it was never meant to preserve a verdict about a superseded world. Without the third state, the merged arm of the funnel would have let validated-is-not-shipped — the original failure this task was commissioned for — become reachable again through the merge path, one arm away from where it had just been closed on the close path.

Known gaps, not fixed here

Named rather than silently carried, and deliberately left out of scope so this change could land proven rather than growing while unlanded.

  • ready in the holds bucket reads as externally_held. ready was placed with holds because both mean "someone must act", but the summary's state ladder falls to externally_held once a hold exists and no active work does. So a home whose only live child is a handoff report summarises as blocked on someone else, when ready is precisely the state saying firstmate is the one who must act, and fm-bearings-snapshot.sh renders it under that heading. The reason text is still shown, so nothing is lost; the label is wrong. Reporting it honestly needs either a bucket of its own or precedence above holds — both new structure, which is why it is a follow-up rather than a commit here.
  • An abandoned PR's poll has no cheap retirement. A PR closed and never reopened records its contradiction and retires its poll, but a task left in that state is refused by the claim gate until its claim is withdrawn with a failed: line. That is the intended path, and it is a manual one.
  • done: and ready: are anchored in the captain-relevant default; the legacy free-text tokens are not. PR ready, checks green, ready in branch and merged still match anywhere in a line, because they describe prose rather than a leading verb. That is deliberate, and it means prose containing those phrases is still absorbed rather than flagged.
  • A test fixture's ten-second wall-clock bound produces false reds under load. run_watcher_bounded in tests/fm-pr-check-security.test.sh wraps a real watcher process in a perl alarm 10 and, on expiry, kills with TERM and exits 124. On a machine running the full 88-script selection that timer is routinely starved, so the suite fails with first merged watcher cycle failed and empty stderr while passing standalone at the same commit. A genuine test defect, not this change, and left as a follow-up with the mechanism recorded so nobody has to re-derive it.
  • docs/verification/secondmate-parent-channel.md records a transcript this change makes stale. Under the new contract a child's legacy done: PR ... checks green publishes as blocked [key=child-outcome-...] ... claim=unverified — a different fingerprint key, since the verdict is now part of the event identity — and fm-pr-check.sh prints an advisory claim: line before armed:. The recorded output predates both. It was not hand-edited to match, because editing a transcript of an observed run would fabricate evidence; a dated currency note names exactly which lines are no longer current instead. Re-running the fixture needs two real tmux homes and two live watchers, which is not safe to do while other work is live in this home, so it is a follow-up.
  • GitLab merge requests cannot be established. glab exposes no head commit through its CLI without a JSON processor firstmate does not require, so those claims stay unverified and their cleanup needs explicit discard authority. Documented in the verifier's header, and unchanged by this work.

The proxy ledger

One shape recurred more than any other: a cheap stand-in used for the question actually being asked. Eight of them, and the ledger is more useful than any single fix, because the stand-in is always easier to compute and nearly always right, which is exactly why it survives review.

# The proxy The question it stood in for
1 merge-base(fm/<id>, default) != claim did this branch author the work? — true for unlanded work, false for landed
2 "the branch moved since it was created" did this branch author the work? — a rebase the brief instructs moves it
3 some file exists at the claimed path is this this task's report?
4 the claim and the forge agree about a head did this task produce that PR? (direct-PR had no binding at all)
5 the spelling of a resolved path the location of a resolved path — .. walks straight through a prefix test
6 "a verdict was written" is the outcome on the record? — a close with no claim writes no verdict
7 a five-point registration checklist the set of actual consumers of a vocabulary
8 "not in any bucket" unaccounted for — a deliberate exclusion is neither
9 "accounted for" (in any state) accounted for in this state — the exclusion's stated reason covered only one
10 flat CPU on a daemon's first agent child is the pipeline stuck? — see below

Three of these were introduced defensively — added as extra safety rather than derived from the invariant — which is its own entry in the lesson: a safety addition not derived from the invariant is not extra safety, it is extra surface.

Absence as evidence, applied to a test result

The same distinction this change is built on, landing somewhere it was not designed for.

The suite's one unexplained failure printed first merged watcher cycle failed: and then nothing. The empty stderr was the finding, not a gap in it: a failing assertion prints its message, so silence is positive evidence of a signal kill rather than an assertion failure — which pointed straight at the fixture's alarm 10 around a real process, exiting 124 with no output. Standalone at the same commit, the suite passes.

Reading absence as unexplained would have meant either treating a healthy change as broken or waving through a red step on a hunch. Reading absence as what it positively rules out separated a starved timer from a real break, with evidence either way. unverified versus contradicted is the same move: what you did not observe is not nothing, but it is also not falsity, and knowing which is which is the whole discipline.

Prediction from having run it beats sampling from outside

The stuck-detector said the run was hung. The prediction said the test step would take ~45 minutes, because that selection had been run by hand earlier the same day. It finished in 2685917ms — 45 minutes.

Worth recording as more than a coincidence: the supervisor sampling a process from outside had a plausible general rule and got it wrong, while the party who had actually executed the thing had a specific expectation and got it right. When a general diagnostic and a direct measurement disagree, the direct measurement is not merely another opinion.

The tenth proxy was in the supervisor, in a tool built the same day

Recorded because it is the strongest single argument in this change for why a claim must be checkable rather than trusted, including a claim from the person directing the work.

The supervisor built a stuck-detector, wrote it down as fleet knowledge, and four hours later used it to order a healthy run stopped — minutes before that run produced the deliverable being asked for. The diagnostic sampled CPU on a daemon's first agent child and read flat CPU as the pipeline is stuck. It fails in exactly two ways, both present at once:

  • a finished agent reads flat because it is a corpse, not a hang — the review step had completed and the run had advanced;
  • a shell driver reads flat because its work happens in forked children — the test step was executing bin/fm-test-run.sh with live children consuming CPU and no agent pid at all.

What made it visible was not a better detector. It was checking the premise instead of executing the instruction: review,completed in the status, fm-test-run.sh alive as pid 9741, a test script executing under it, and a prior run of the same selection taking ~45 minutes. The order was withdrawn once the evidence was verified independently.

The general form is the one this change exists to install. A durable record can stop being true; so can an instruction, and so can a diagnostic rule — including one written by the person who is right about everything else in the room. An instruction is a claim about the world, answerable to the same test as a worker's done:. Both times an instruction was refused here, the refusal was correct — once for a sentence that had gone stale between being written and being published, once for a diagnosis applied to the wrong process.

The fix kept catching itself, and that is the argument for it

The most important finding in this change, and it is not about any one defect.

This change reproduced its own target defect inside its own fixes four separate times:

  1. a guard narrating "which it introduced after being created at B" about a branch that introduced nothing — an untrue record, emitted by the thing built to stop untrue records;
  2. registering a new verb re-opened silent absorption of worker prose — the exact behaviour item 5 was written to end, reintroduced by a fix for a different part of the same change;
  3. a five-point registration checklist standing in for the set of actual consumers — a proxy, written because the author knew proxies were the enemy;
  4. a guard against invisible state making a loudly visible false claim, and one false claim discarding an entire home's ledger.

None of these was carelessness, and reading them as sloppiness misses what they show. Each fix was written by someone who had just articulated the exact rule it went on to break, could state the mechanism precisely, and was actively looking for it. The defect class is structural to systems that describe themselves. Any component that reports on state adds state to report on; any rule about records is itself a record that can go stale; any guard that must explain itself can explain itself untruthfully. A written rule was never going to be sufficient, because the writing is inside the system the rule is about.

That is the argument for this change rather than against it. A guard that catches its own author, repeatedly, while its author is paying full attention, is measuring something a more careful author would not have avoided. The eight proxies, the six twins, the four self-inflicted instances — every one was found by a mechanism that outlived the moment of insight that produced it: an adversarial reviewer, a false reason line, a planted failure, a test that fails when two lists disagree. The insight did not generalise. The mechanisms did.

If this had shipped clean on the first pass, the honest conclusion would not have been that the change was good. It would have been that nothing was checking.

Closeout: the larger truth

The status log claims an outcome while the forge holds it. That is one instance of a general shape in this repo: a durable local record asserts a fact that an external system is the real authority for, and nothing re-reads the authority after the record is written. The record is not wrong when it is written; it becomes wrong later, silently, and the only thing that would notice is a re-read nobody scheduled.

Every other place that shape appears here:

Record External authority Can it drift today?
data/backlog.md Done rows carrying pr_url the forge: did that PR actually merge Yes. This change makes an armed poll report a close, but teardown retires the poll, so a Done row whose PR is closed afterwards is never re-checked. This is the residual half of failure 3.
state/<id>.meta pr_head= the forge: the PR's current head Yes. Written once at registration and never refreshed; any later push, including the pipeline's own fix commits, leaves it stale. Consumers already treat it as advisory (teardown reads the head live, fm-pr-merge.sh treats a disagreeing value as stale), so the drift is contained by convention rather than prevented.
data/projects.md registry entries the forge and the filesystem: does the repo still exist, is it archived or renamed, is the clone still there Yes. Rebuilt only when a session start finds it absent or an operator asks.
docs/verification/*.md the installed tools: harness versions, backend behaviour, CLI surfaces Yes. A vendor upgrade silently invalidates a dated record; the guidelines say to refresh via the live guard, but nothing forces it.
state/public-followup/ registrations the relay platform: does that thread still exist Yes. A registration for a deleted thread stays owed until retired by hand.
data/learnings.md, data/captain.md prose whatever system the fact describes Yes. AGENTS.md section 10 already says to verify volatile details before acting, which is an instruction, not a check.
data/secondmates.md routes and remote_host= the remote machine: is that home reachable Bounded. The startup liveness sweep reconciles dead/missing and deliberately preserves ambiguous ones, so a permanently-gone route persists but is not mistaken for healthy.
state/<id>.meta window= / backend target the multiplexer: does the endpoint exist Bounded. Read live at every session start and by fm-crew-state.sh.
state/<id>.pr-poll + .pr-poll-registration the forge: the PR identity No. Every component is revalidated on every poll and the URL is reconstructed from them.
state/<id>.pr-poll-merge-notified none - it records this home's own delivery No.

Not fixed here.

A `done:` claim was free prose. `bin/fm-dod-lib.sh` told every no-mistakes
worker to append `done: PR {url} checks green`, `bin/fm-classify-lib.sh`
matched the token `done`, and the fleet treated the claim as fact. A claim
identified no commit, so nothing could compare what a pipeline validated with
what a PR shipped, and the merge poll only ever watched for `merged` - so a
"done" record for a PR later closed without merging aged quietly.

Terminal claims now carry machine-checkable identity, and a verifier decides:

- bin/fm-done-claim-lib.sh (new) is the one owner of the claim grammar, its
  parser, and the durable verdict record. bin/fm-dod-lib.sh and bin/fm-brief.sh
  render the worker instruction from that owner, so the shape a worker is told
  to write and the shape firstmate parses cannot drift.
- bin/fm-verify-done.sh (new) establishes a claim from the forge, from git, and
  from the validation run's own record, never from the claim's prose. It answers
  verified / unverified / contradicted and binds its verdict to the exact claim
  it judged, so a later claim supersedes rather than inherits it. Comparing the
  validated commit with the shipped commit is the check that catches a fix
  landing after validation, or a force-push over a validated head.
- Nothing treats an unverified claim as done: bin/fm-crew-state.sh reports
  `done-unverified` with the reason, bin/fm-teardown.sh refuses unverified and
  contradicted alongside its existing landed-work gates, and both the fleet
  snapshot and the session-start digest surface the verdict.
- bin/fm-pr-poll.sh reports a PR closed WITHOUT merging as well as a merge, and
  bin/fm-merge-outcome-lib.sh publishes it as a contradiction. The notification
  marker gained a v2 line recording which outcome was delivered, so a close is
  reported once and a later reopen-and-merge still reports its own outcome.
  Only a merge retires the poll.
- A status line matching no known verb is flagged in the drain rather than
  absorbed, bounded per drain with a count of any held back.

Legacy claims degrade rather than explode: they carry no commit identity, so
they read as unverified and are never silently upgraded.

Claude-Session: https://claude.ai/code/session_01N6zvmegmTrHiWnrfXBxuPU
Two findings in the cases added by the review fix round:
`local dir=$1 data=${2:-$dir/data}` reads `dir` inside the same `local`
(SC2318), and `FM_CAPTAIN_RE` is consumed by the sourced classifier rather
than by this file (SC2034).

Claude-Session: https://claude.ai/code/session_01N6zvmegmTrHiWnrfXBxuPU
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-04T02:20:07.341453Z 393d4be PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@greptile-apps

greptile-apps Bot commented Sep 4, 2026

Copy link
Copy Markdown

Confidence Score: 3/5

The PR should not merge until verification rejects failed or unfinished validation runs and cached verdicts cannot survive an open-PR head change.

The new completion authority can currently establish work that did not pass validation, and teardown can rely on stale forge evidence after the claimed PR changes.

Files Needing Attention: bin/fm-verify-done.sh, bin/fm-teardown.sh, tests/fm-done-verified.test.sh

Reviews (1): Last reviewed commit: "no-mistakes(document): refresh docs stal..." | Re-trigger Greptile

Comment thread bin/fm-verify-done.sh
Comment on lines +639 to +641
RUN_OUTCOME=$(fm_nm_strip_quotes "$(fm_nm_field "$RUN_OUT" outcome)")
RUN_STATUS=$(fm_nm_strip_quotes "$(fm_nm_field "$RUN_OUT" status)")
verdict_is verified "$PR_URL is $PR_STATE at the claimed $HEAD_CLAIM, validated at $RUN_HEAD (run ${RUN_OUTCOME:-$RUN_STATUS}); checks: $CHECKS"

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 Failed runs become verified

When a no-mistakes run is still running or has completed with a failed outcome, the verifier uses RUN_OUTCOME and RUN_STATUS only in its explanation before recording verified, causing crew state to report done and allowing teardown of work that validation has not accepted.

Knowledge Base Used: Session transitions and teardown

Comment thread bin/fm-teardown.sh Outdated
Comment on lines +2655 to +2667
if [ -n "$TEARDOWN_CLAIM" ] && fm_done_claim_status "$STATE" "$ID" \
&& [ "$FM_DONE_CLAIM_STATE" = verified ]; then
# A durable record already establishes THIS EXACT claim (fm_done_claim_status
# matches on the claim's hash, so a newer `done:` line does not inherit it).
# Re-running the verifier could only weaken that: `unverified` is the absence
# of evidence, and the sources it reads go away for ordinary reasons long
# after a claim was established - the forge unreachable, `gh` or `no-mistakes`
# off PATH, the validation run aged out of `axi status`. Deferring to the
# record here is what makes fm_done_verdict_write's anti-downgrade rule mean
# something at the only gate that blocks, and it leaves `--force` (explicit
# discard authority) for work that was never established, not for work that
# was. Every refusal below still stands whenever no such record does.
TEARDOWN_CLAIM=

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 Cached verdict outlives PR head

When an open PR is force-pushed or receives another commit after its claim was verified, teardown trusts the cached verdict without rerunning the live head comparison, causing it to delete local evidence even though the PR now ships an unclaimed and unvalidated commit.

Knowledge Base Used:

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 393d4be4d6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread bin/fm-verify-done.sh
Comment on lines +639 to +641
RUN_OUTCOME=$(fm_nm_strip_quotes "$(fm_nm_field "$RUN_OUT" outcome)")
RUN_STATUS=$(fm_nm_strip_quotes "$(fm_nm_field "$RUN_OUT" status)")
verdict_is verified "$PR_URL is $PR_STATE at the claimed $HEAD_CLAIM, validated at $RUN_HEAD (run ${RUN_OUTCOME:-$RUN_STATUS}); checks: $CHECKS"

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 Badge Reject failed validation outcomes

Captain, when axi status returns outcome: failed or cancelled for the same branch and head—for example, a cancelled validation whose PR remains open—these lines read the outcome but always persist verified. This lets fm-crew-state and teardown trust a terminal claim even though the authoritative run classifies it as failed; only passed or checks-passed should produce a verified verdict.

AGENTS.md reference: AGENTS.md:L369-L370

Useful? React with 👍 / 👎.

Comment thread bin/fm-done-claim-lib.sh
Comment on lines +373 to +377
standing=$(_fm_done_verdict_standing "$state" "$id" "$hash")
case "$verdict" in
unverified)
case "$standing" in ''|unverified) ;; *) return 0 ;; esac
;;

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 Badge Prevent stale verification from overwriting contradictions

When claim verification overlaps a watcher-observed PR closure, the verifier can query OPEN, the watcher can then write contradicted, and the verifier can subsequently replace that record with verified based on its older observation. The standing-record read and replacement are not serialized, and verified has no guard against an intervening contradiction, so the closed PR can again appear established; make the observation/write atomic or use a compare-and-swap that preserves a newer terminal contradiction.

AGENTS.md reference: AGENTS.md:L379-L380

Useful? React with 👍 / 👎.

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate: this is waiting on the author.

First look on HEAD 393d4be4d67fc0791e824d2d4e0d58a9e9a450a3 vs main 31cba0db93f08f5d7ce87442d716fcb97fc00d5d. MERGEABLE=CONFLICTING / DIRTY. ICGNU3 is not a blocked author. No .github/** / workflow edits in the file list — security surface looks clean for a fork PR, but there is nothing to approve yet.

Attestation: MISSING. Body has no no-mistakes-pipeline-attestation:v1 binding this tip. Tip is workflow-zero (only Greptile; no CI / Require no-mistakes suites on this SHA). Rebase onto current main, re-raise via git push no-mistakes so the body SHA matches the tip, and push so tip CI/NM re-arm — we will approve first-time fork runs after a fresh diff review once action_required appears.

Contract-class: new-default. Unconfigured firstmate would always require machine-checkable done: identity, run fm-verify-done, refuse teardown on unverified/contradicted claims, downgrade crew-state to done-unverified, and treat closed-unmerged as contradiction. Main's free-prose done: path does not promise that contract. A motive about three fleet failures does not turn a new always-on completion authority into restore.

VISION.md per-rule

  • One captain, one interface — aligns. Verdicts stay below deck; captain-facing outcomes become more honest.
  • Authority is explicit and never inferred — does not align as restore. New always-on claim grammar + teardown refusal assumes consent for a stricter completion gate.
  • Scripts own the mechanics, agents own the judgment — aligns. Verification is deterministic scripts; workers only emit claims.
  • A restart is a non-event — aligns. Durable done-verdict bound to the exact claim.
  • Delegation with a spine — aligns. Independent verification between worker confidence and "done".
  • The fleet outlives any vendor — aligns / note GitLab head gap documented (unverified + --force).
  • Scope — aligns as command-layer completion authority; Greptile 3/5 flags real author work on failed/unfinished validation acceptance and stale forge evidence after open-PR head change (fm-verify-done.sh / teardown).

Do not merge (new-default + MISSING attestation + CONFLICTING + workflow-zero + Greptile findings). Do not escalate to Firstmate until MATCH + green CI/NM + mergeable — then flag only for the default-behavior decision. Not a captain-decision hold yet.

Written under a compaction warning. Holds the two hard rules, the two P1
findings with their sites, what is already changed and what remains, the
identified CI failure, the A/B-proven pre-existing test failures, and a
verbatim captain instruction about a file that must be left untouched.

Claude-Session: https://claude.ai/code/session_01N6zvmegmTrHiWnrfXBxuPU
… head

Two ways a terminal claim could still be believed on evidence that did not
support it.

The run's result gated nothing. fm-verify-done.sh read the validation run's
outcome and status and then used them only inside the string it printed before
recording `verified`, so a run still in flight, or one that ran and rejected the
work, still produced an established claim: crew state reported done and cleanup
was free to proceed. Matching heads only ever said validation LOOKED at this
commit. The outcome now gates the verdict itself - terminal, successful, and
accepted, all three - and a bare `completed` with no accepted outcome does not
qualify. Below that bar the split is the one every other source in the script
uses: a run that rejected the work is evidence of falsity, while a cancelled or
unrecognised one established nothing either way.

A verdict outlived the world it was about. A `verified` record was trusted at
the teardown gate without re-reading the live head, so an open PR that was
force-pushed or gained another commit afterwards left the record right about a
commit the PR no longer carried - and cleanup deleted the local copy while the
PR shipped a commit nothing claimed and nothing validated. The verdict now
carries the exact head it evaluated, and a mismatch against the LIVE head reads
as `stale` when a verdict already stands for this claim at the head the claim
names. Staleness is not established, so it blocks: the gate always re-runs the
verifier, and a standing record now rescues only absence (3), never falsity (4)
and never staleness (5). With no verdict standing, the same mismatch stays
`contradicted` - a claim naming a commit the PR never carried is falsity, not
the world having moved.

Both are planted and both were watched to fail. Removing the outcome gate
verifies a claim whose run reports `running`, and removing only its accepted-
outcome arm verifies one whose run reports `failed`, each reproducing the defect
verbatim in the narration beside a `verified` verdict. Removing teardown's stale
arm carries a moved-head claim through cleanup. Each keeps a positive control so
it cannot pass vacuously.

Claude-Session: https://claude.ai/code/session_01T2APQvC3ewFivzVZZFs8pm
@tiago-peixoto

Copy link
Copy Markdown
Contributor

We rechecked this concern against current upstream main on September 4. bin/fm-dod-lib.sh:177 still distinguishes pushed PR delivery, a committed local-only branch, and an intermediate no-mistakes done: before validation. A terminal word alone therefore cannot establish remote preservation; it also must not be treated as permission to discard a local-only branch. This corroborates the exact-artifact concern without treating the proposed always-on verifier as an existing default promise.

We can contribute a bounded regression/evidence case for that distinction within this proposal if useful. We accept the maintainer's new-default classification and author-first requirements; we are not opening a competing completion mechanism or claiming this PR is reviewed or ready.

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.

3 participants