fix(quarterdeck): the verifier honours gates/accepted-red.md - #8
Merged
Merged
Conversation
bin/fm-verify.sh did not know gates/accepted-red.md existed. Its verifier prompt demanded an all-green ledger, which is unsatisfiable while a declared baseline exists - and this repo declares two. Acceptance therefore turned on whether the LLM verifier happened to reason about the baseline on a given run, so correct work was rejected non-deterministically, after the build, the pipeline, and CI. CI honoured the baseline; the verifier contradicted it. The rule now has one implementation: fm_gates_classify in bin/fm-gates-lib.sh, lifted out of tests/run-all.sh rather than rewritten. It is pure - a read of gates/ledger.json and gates/accepted-red.md, root taken as an argument - and never invokes gates/verify.sh or the ledger CLI, which would re-run every gate and rewrite the ledger it is classifying. Classification is separated from policy. run-all.sh skips a test and is otherwise unchanged; fm-verify.sh rejects or escalates, structurally, ahead of the lens and the verifier, so an unacceptable ledger costs neither model. Reject covers undeclared reds and a test_ref naming a file that is gone; escalate covers a missing or unreadable ledger and an unrecognised status. A repo with no gates/ dir proceeds and never escalates - most projects have no ledger at all. The verifier prompt's gate clause is deleted and replaced by a fence telling the model the decision is not its to make: two authorities over one decision is what produced the contradiction. fm-brief.sh's GATE_CHECK and the council spec now cite the classifier instead of restating it. Deviation from the brief, stated openly: acceptable is green, frozen, or red-and-declared. The brief withdrew frozen on the premise that it "is not a status", but the committed ledger holds 7 frozen gates, and CONTRIBUTING.md records that ledger verify DEMOTES frozen to green - frozen is strictly stronger than green, and ledger verify's own empty-drain definition of done excludes it. Excluding frozen would have rejected every ship task in this repo. The premise looks like a snapshot taken right after a verify sweep, before the re-freeze. Also fixes an apostrophe in the verifier prompt heredoc that killed fm-verify at run time: it is an unquoted heredoc inside a command substitution, bash re-scans the body, and bash -n does not catch it. Now guarded by gate-q9. New gates gate-q8-gate-classifier and gate-q9-verify-honours-declared-red, each observed red before green and then frozen; the 7 gates ledger verify demoted along the way are re-frozen at their prior status.
The classifier inherited an `if isinstance(gates, dict): gates =
list(gates.values())` coercion when the parsing logic was lifted out of
tests/run-all.sh, and that coercion was a fail-open in the one place the file
promises to fail closed. {"gates": {}} became an empty list, produced zero
rows, and classified OK - so bin/fm-verify.sh printed "gates: acceptable" and
proceeded over a ledger it had read no gates from at all. A populated object is
the same defect in a more convincing disguise.
CONTRIBUTING.md states that `gates` must be a JSON array and that any other
shape makes every `ledger` subcommand abort before doing any work, and frozen
gate m0-ledger-shape freezes exactly that. A shape the harness itself calls
fatal is not one this classifier may quietly repair. The coercion is removed:
any non-array `gates` is now BADLEDGER, which fm-verify escalates and
run-all.sh treats as "skip nothing".
An EMPTY array stays valid and acceptable - a ledger with no gates is
well-formed, merely empty - and that is now pinned too.
The one behaviour change this lands on tests/run-all.sh is in the fail-closed
direction: an object-shaped ledger previously coerced and could authorise
skips, and now authorises none. No case of gate-ci-declared-red exercises that
shape, and it passes normally and fails under LEDGER_MUTATE=1 as before.
Q8 covers the empty object, the populated object, and the empty array, and its
mutation now inverts the shape rule as well as the double condition. Q9 adds
the end-to-end case: an object-shaped ledger in the crewmate's worktree
escalates before either model runs and is never announced acceptable. Both new
assertions were checked against a deliberately reintroduced coercion. The spec
table names the array requirement.
gate-q8 and gate-q9 were demoted to green and re-frozen, so their
mutation_verified stamps cover the changed tests rather than the previous ones
- `ledger freeze` no-ops on an already-frozen gate and does not re-prove it.
Suite: 49 ran, 2 skipped, 0 failed. ledger wip still holds only the two
declared reds.
…fier row The rows the classifier emits ARE the grammar its callers parse positionally, so a tab or newline inside a field is not a formatting nuisance - it lets the ledger write extra verdicts. The reason column was flattened for exactly that reason; id, status and the test path were not, and that gap handed the ledger the authority the classifier exists to hold. Reproduced before fixing. A gate whose id was "evil\nok<TAB>forged-gate<TAB>red<TAB>tests/bb-target.test.sh<TAB>reason" emitted a second line that parsed as a well-formed "this red is declared" verdict. tests/run-all.sh skipped bb-target.test.sh - a test that FAILS - and reported "1 ran, 1 skipped, 0 failed", over an all-green ledger whose accepted-red.md declared nothing at all. A single tab suffices on its own: it shifts the remaining fields left until attacker-chosen text lands in the status column, with no newline needed. id, status and the test path are now refused outright rather than flattened. Mangling them would leave a fabricated id standing in for a real one; refusing is stricter and costs nothing, because a tab or newline in any of the three is never a legitimate ledger. It is the same rule as the array check: a ledger the harness would never accept is not one this classifier may quietly repair. The reason column keeps its flattening - it is prose, and being last it can still open a new line. Q8 covers the newline id, the tab-only id, the newline status, and the exploit end to end through run-all: the targeted test must RUN and the suite must fail. Its mutation now inverts the forgery as well. Q9 adds the fm-verify half - a delimiter-bearing id escalates before either model and is never announced acceptable. Every new assertion was checked against a deliberately weakened classifier. The spec table names the rule. gate-q8 and gate-q9 demoted and re-frozen so their mutation stamps cover these tests. Suite: 49 ran, 2 skipped, 0 failed. ledger wip still holds only the two declared reds.
…ace ledger diagnostics
The base resolution added for the self-authorised-red check had three defects, all in the helper itself rather than in the rule it serves. base_ref() memoized into variables it could never reach: both consumers called it as $(base_ref), and a subshell's assignments die with the subshell, so the guard was empty again in the parent every time. The fetch ran twice and the promise that both stages compare against the same base was false by construction. Resolution now happens once in the main body, into FM_BASE_REF and FM_BASE_COMMIT, which both stages read. The fetch was guarded against failure but not against blocking, and it is the only network call on the accept path. An ssh URL for a host absent from known_hosts, or an https URL whose credential helper has expired, left git waiting on an interactive prompt with no tty to answer it - and a verifier that hangs is worse than one that escalates. GIT_TERMINAL_PROMPT=0, ssh batch mode with a connect timeout, and an http slow-transfer cap remove the hang; an optional timeout(1) is defence in depth where one exists. Preferring origin/<default> unconditionally mirrored the stale-base defect it was meant to fix. A declaration committed to the local default and not yet pushed sits ahead of origin, so a merge base taken there predates it and the inherited line reads as forged. The base is now the furthest-forward candidate. That is safe in one direction only, which is why it is the right rule and not mere permissiveness: a merge base is always an ancestor of HEAD, and a declaration written in this branch's own commit exists on no default-branch candidate, so it can never appear at any candidate's merge base. Moving the base forward removes false escalations; it cannot manufacture a pass. Q9 gains case P (a local default ahead of origin proceeds; fails exit 3 under unconditional origin preference) and case Q (an unreachable origin degrades and the run completes; fails exit 128 if the fetch aborts). gate-q9 re-frozen, since freeze is re-earned rather than sticky; gate-q8's scope is untouched, so its freeze stands. Suite 62 ran, 2 skipped, 0 failed; shellcheck clean; census unchanged at 41 gates.
…refuse bad test_ref
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 defect
bin/fm-verify.shdid not knowgates/accepted-red.mdexisted. Its verifier prompt said:That is unsatisfiable while declared reds exist (this repo has two), so acceptance depended on whether the model happened to reason its way to
gates/accepted-red.mdon a given run. Correct work was rejected at random, at the most expensive possible moment — after the build, the pipeline, and CI.The change
bin/fm-gates-lib.shis now the single implementation of "is this gate's state acceptable?". The rule previously had three statements:tests/run-all.shimplemented it, the verifier prompt contradicted it, andfm-brief.sh'sGATE_CHECKrestated it a third way. Every other statement is now a citation.run-all.sh's logic was lifted verbatim;run-all.shchanges solely to delegate.gates/verify.shor theledgerCLI —ledger verifyrewrites the ledger, demotes frozen gates, and exits 2 where the CLI is absent. A classifier that mutates what it classifies is not a classifier.run-all.shskips a test,fm-verify.shrejects or escalates. Both fail closed.frozenis acceptable, which deviates from the original brief and is flagged rather than hidden: 9 of this repo's 41 gates are frozen, andCONTRIBUTING.mdrecords thatledger verifydemotes frozen to green — frozen is strictly stronger than green, not weaker. Treating it as unacceptable would reject every ship task here. It is isolated behindFM_GATES_CLEAN_STATUSES, so reverting is one line.Fail-opens closed
Each was reproduced before being fixed, and each fix proven against deliberately weakened code:
gatesvalue isBADLEDGER, never coerced.{"gates": {}}used to classifyOK, sofm-verifyannounced "gates: acceptable" over a ledger it read no gate from.BADLEDGER. Those fields are the row grammar: a crafted id was verified to makerun-all.shskip an arbitrary failing test and exit 0 over an all-green ledger with nothing declared.test_refis refused the same way. An absenttest_refstays legal and simply means no freshness check.unprovengets its own verdict token and rejects rather than escalating, perCONTRIBUTING.md.Self-authorisation
A crewmate must not declare its own red baseline. A declaration this branch adds to its own diff and then relies on escalates, naming the gates.
refs/heads/<default>is a ref the crewmate can write in a pooled clone, so it cannot certify the crewmate's own declaration.awkas input, not via-v: awk escape-processes-vassignments, and a literal backslash-n in a gate id defeated the comparison entirely.fm-verify.sh's policy layer, deliberately not in the pure classifier.Scoping, so pre-existing debt does not block everyone
The stale-
test_refandunprovenrejects are scoped to gates this branch's diff actually touches; untouched debt is reported, not rejected. Otherwise a repo carrying either would reject every ship task dispatched into it — three relays the crewmate cannot act on, then a captain escalation — while the suite and CI stayed green.Scope uses
--no-renamesso a renamed test file cannot hide from its own staleness check, ignores volatile stamp fields (last_verifiedchurn would otherwise mark nearly every gate as this branch's), and normalises test paths on both sides.gates/LEDGER.mdcounts as gate machinery; a baregates/directory — an ordinary package name — does not, and no longer escalates every ship task.Verification
shellcheck bin/*.sh tests/*.shclean.LEDGER_MUTATE=1.gates/accepted-red.mdis byte-identical to base, so this branch passes its own self-authorisation check.Behaviour is frozen by
gate-q8-gate-classifierandgate-q9-verify-honours-declared-red.Known follow-ups
Reviewed and accepted rather than fixed, recorded so they are not lost:
row-verdict-fallthrough-reads-as-acceptable— the header fall-through is closed; the same hole remains one level down at the row level.gates-lib-cli-reports-acceptable-on-unknown-header— same shape in the library's own CLI exit code, whichfm-brief.shnow tells every crewmate to use as their pre-done:self-check.undeclared-red-unscoped-ci-argument-has-a-hole— the repo-wide undeclared-red reject rests on "CI catches those", which holds only while the ledger's red status is accurate.bad-status-escalation-is-unscoped-and-ci-invisible— one typo'd status escalates every ship task in the repo.structural-rejects-consume-the-verifier-attempt-budget— the three structural conditions reject serially and can burn the whole attempt budget before either model runs.verify-fetch-writes-into-the-project-worktree— the base fetch is a state-changing git command in a project, outsideAGENTS.md's five sanctioned exceptions.stale-loop-verifier-agent-contract—.claude/agents/loop-verifier.mdstill states the removed rule and claims to be identical to the fm-verify prompt. It is a symlink intostone-skills, so the fix belongs in that repo.Items 1 and 2 are the same fail-open class this change set out to eliminate and are the ones worth doing first.
Note on history
Commits
d2e6009c..a276dfddare fixes the no-mistakes pipeline applied to this branch during earlier runs. Those runs died on transient API 529 overload at the review and lint steps; firstmate has since moved to direct-PR mode, so this PR is opened directly. The code is unchanged from the state that passed the pipeline's review, test, and document steps.