ci: give the no-mistakes test analyzer a live-evidence runbook - #6
Merged
Merged
Conversation
…t and guard test.instructions
…, run contract on python
…p no-surface section
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.
Intent
The captain approved the voice plan and said "Spin up workers on Opus 5 to get this plan done" and "You have standing permission to merge everything... Make all the decisions."
Context. This task was filed 2026-09-11 as not urgent. It is now BLOCKING: two pieces of the approved voice plan cannot finish validation because of it. The firstmate repo's .no-mistakes.yaml has no test.instructions, so the pipeline's test analyzer derives its own scenarios and refuses ones it marked pass as "requires live validation". Evidence today: afk-inject-wedge-claude-pane failed its test step twice with
step test failed: validate test analyzer findings: scenario 5 result "pass" requires live validation(runs 01M29ZSNGHRD9A91244NV75AR2, 01M2A0XYT1YPAPMVP7Q3XS1202), and fv-13 hit the same refusal three times before it. Both changes are covered by the repo's real suites; the analyzer simply has no instruction telling it so.What Changed
test.instructionsrunbook to.no-mistakes.yamlthat tells the gate's test analyzer what this repo's product entry points are (executables underbin/plus the.piextensions), that thetests/*.test.shsuites drive those entry points as real subprocesses and so may be cited as live evidence, and which scenarios stayuntested(vendor agent harnesses, network/forge/SSH/credential surfaces, fixture-replay cases, and cases that only parse a declarative artifact without running its consumer, includingfm-nm-test-contract.test.shitself); it also forbids forcing skipped live guards on viaFM_LIVEorFM_GATE_REFUSE_BYPASS.tests/fm-nm-test-contract.test.shwith a case assertingtest.instructionsstays present and non-empty, mapped.no-mistakes.yamlchanges to that script inbin/fm-test-run.shselection, and covered that mapping with a new case intests/fm-test-run.test.sh.fm_yaml_to_jsonhelper intests/lib.shthat prefers python3+PyYAML and falls back to ruby (returning non-zero when neither exists, so suites fail loudly instead of skipping green), and moved the contract andci.ymltimeout tests onto it.docs/configuration.mdand the coding-guidelines skill, noting that the analyzer reads it only from the default-branch copy so a feature branch cannot relax the policy judging its own run, and updated the CONTRIBUTING helper inventory.Risk Assessment
✅ Low: The change is well-bounded to a trusted default-branch config runbook plus its guard test, doc pointers, and a portability fix, and every source-verifiable claim it makes (config key recognized and default-branch-trusted, gate-skip/FM_TEST_END marker semantics, FM_LIVE and FM_GATE_REFUSE_BYPASS, the fm-inbox subprocess exemplar, the 75/20 ci.yml timeouts, and the new selector arm being the only thing that selects an otherwise unclassified script) verified against source.
Testing
Drove the change end-to-end through the real runner: an isolated copy of the repo with only .no-mistakes.yaml edited now selects tests/fm-nm-test-contract.test.sh via
bin/fm-test-run.sh --list --changed, the same selection holds on the actual branch against its base, and the contract guard passes on the branch config. Four adversarial mutations of the config - runbook deleted, whitespace-only, retyped as a list, and a deterministic commands.test reintroduced - each produced a loudnot okwith exit 1 and gate_skip=false, and restoring the file made it pass again. With PyYAML made unimportable and no ruby on this host, the shared parser helper hard-fails naming both parsers rather than skipping green, on both the contract script and the ci.yml consumer. The runner's own suite ran green including the new selector case and the reworked ci.yml timeout case, and the documentation-audiences suite still passes after the prose edits. Two things stayed untested and are reported as such: the ruby/Psych fallback branch (no ruby interpreter on this host, and installing one is outside the worktree boundary) and the gate analyzer actually consuming test.instructions, which is readable only from the trusted default-branch copy and would require invoking no-mistakes, which this phase must not do. No UI surface exists in this change, so evidence is CLI transcripts..no-mistakes.yamlmodified,bin/fm-test-run.sh --list --changed --base HEADlists tests/fm-nm-test-contract.test.sh; 01-chang…bin/fm-test-run.sh tests/fm-nm-test-contract.test.shprintsok - no-mistakes does not configure commands.testand `ok - no-mistakes carries a non-empty test.instructions…test.instructions is NoneType rather than a string; whitespace-only ->test.instructions is empty; YAML list -> `test.instructions is…not ok - commands.test must be absent or empty so Test stays intent-targeted; got: 'bin/fm-test-run.sh --all', exit=1ok - no-mistakes config change selects its contract testandok - Herdr CI family-run step times out at 20 min under a 75 min job backstop, FM_TEST_END exit=0bin/fm-test-run.sh tests/fm-documentation-audiences.test.shexit=0 with all four ok lines after the docs/configuration.md and SKILL.md editsEvidence: Changed-file selection on the real branch (contract test selected)
Source: Changed-file selection on the real branch (contract test selected)
Evidence: Isolated repo: only .no-mistakes.yaml edited selects the contract test
Source: Isolated repo: only .no-mistakes.yaml edited selects the contract test
Evidence: Contract guard passes on the branch config
Source: Contract guard passes on the branch config
Evidence: Adversarial config mutations all fail the guard loudly
Source: Adversarial config mutations all fail the guard loudly
Evidence: No YAML parser available: hard fail, gate_skip=false
Source: No YAML parser available: hard fail, gate_skip=false
Evidence: tests/fm-test-run.test.sh full run (new selector case green)
Source: tests/fm-test-run.test.sh full run (new selector case green)
Evidence: Documentation audience contract after the prose edits
Source: Documentation audience contract after the prose edits
Evidence: Shared fm_yaml_to_json helper across both consumers
Source: Shared fm_yaml_to_json helper across both consumers
Evidence: Guard fires on a removed runbook
Pipeline
Updates from git push no-mistakes
... (13 earlier update rounds omitted to keep the PR body within GitHub's 65536-char limit; full history is in the run log.)
🔧 **Review** - 4 issues found → auto-fixed (11) ✅
🔧 Fix applied.
3 issues (2 warnings, 1 info) still open:
.no-mistakes.yaml:76- The live-evidence rule's own example set contains a case that is not a subprocess drive, and the voice suite matches that example while never running the entry point as a process. Lines 72-77 state the rule as "Only the test case that runs such an entry point as a subprocess drove the product - directly, or through a child contract executable the case launches", then give two illustrations: tests/fm-inbox-conversation.test.sh -> tests/fm-inbox-conversation-cases.py, "and as the Pi extension suites drive a .pi/extensions/.ts extension through the real installed pi package". The first illustration really is a subprocess drive (tests/fm-inbox-conversation-cases.py:19 does subprocess.run([str(cli), 'conversation', command]) against root/'bin/fm-inbox.sh'). The second is not: a .ts extension is never a subprocess, it is loaded in-process by the pi binary. So the runbook establishes that an entry point loaded in-process by a real process the case launched still counts as live. Concrete reachable sequence on exactly the surface the intent unblocks: the voice plan changes bin/fm-voice-relay.py. families_for_changed_path routes bin/ through families_for_unmapped_bin (bin/fm-test-run.sh:1461-1465), selecting tests/fm-voice-relay.test.sh. Most of its relay cases launch an inlinepython3 - "$ROOT/bin" <<'PY'child and then IMPORT the relay - tests/fm-voice-relay.test.sh:193-215 does importlib.util.spec_from_file_location("relay", .../fm-voice-relay.py), spec.loader.exec_module(relay), and calls relay.read_uplink_frame(reader) directly. Shape-wise this is indistinguishable from the fm-inbox-conversation illustration (a case launching a child python contract executable) and matches the Pi illustration exactly (entry point loaded in-process by a real launched process), so an analyzer reports the uplink scenario "pass" with "live": true. The relay process was never started; only an imported function ran - which line 66 ("not importing a function") and line 110-112 (a case that "sources a library and feeds it recorded strings ... proves the classifier's logic, not the live surface") both forbid. Same runbook, same transcript, opposite labels, and neither path errors. This is a defect inside the component the change introduces, so the smallest honest remedy is a wording correction, not new machinery: say that the entry point itself must be started as a process by the case or by a child the case launches, and name the Pi case as the single explicit exception (the extension is loaded by the real shipped pi harness, not by a test harness). Flagged ask-user because the live-evidence policy wording is the author's deliberate call and the user has already iterated on this exact sentence twice..no-mistakes.yaml:55- The product enumeration is wrong in both directions, and the wrong half lands on the voice surface the intent names as blocked. Lines 53-60 define the product as "the bin/ commands whatever language each is written in (bin/fm-.sh, bin/backends/.sh and .py, the bin/fm_.py modules such as bin/fm_inbox_conversation.py, bin/fm_voice_frame.py and bin/fm_voice_records.py, bin/fm-extension.mjs and bin/fm-procevent-extension-capture.pl) and the .pi/extensions/.ts editor extensions". Over-inclusive: bin/fm_voice_frame.py is mode -rw-rw-r--, has no shebang and no main(), and its own docstring calls it "the wire format between the voice client and the relay" - it can only ever be imported, never run as a process, so calling it a "bin/ command" contradicts line 64-66 ("driving a scenario end-to-end here means running those real entry points as real processes ... not importing a function") and reinforces the false-live path in the finding above. Under-inclusive: the repository's actual executable non-.sh entry points bin/fm-voice-client.py, bin/fm-voice-relay.py, bin/fm-mail.py, bin/fm-arm-command-policy.mjs, bin/fm-cd-command-policy.mjs and bin/fm-extension-launch-barrier.mjs are all mode -rwxrwxr-x and match NONE of the listed globs (bin/fm-.sh excludes .py; bin/fm_*.py uses an underscore and does not match fm-voice-relay.py; only fm-extension.mjs is named, not the other three .mjs files). Concrete sequence: the voice plan changes bin/fm-voice-relay.py; an analyzer applying the enumeration literally - the exact failure mode the prior round corrected at the live-test line - finds the relay in no named product category and cannot classify tests/fm-voice-relay.test.sh:347 (python3 "$ROOT/bin/fm-voice-relay.py" --help, a genuine process drive) as a product drive. The one-sentence principle at lines 60-61 is what rescues it, which means the enumeration is actively working against the principle it is supposed to illustrate. Remedy is the author's product-definition call: either name the runnable entry points by their real names and drop bin/fm_voice_frame.py from the entry-point list (it is a library the entry points import), or drop the parenthetical enumeration entirely and keep only "being a shipped first-party entry point is the test, not the language". Note the prior round's instruction explicitly listed bin/fm_voice_frame.py, but that instruction's governing principle was "every first-party entry point the repository ships" - that this file is not runnable, and that the two voice processes are missing, is new information..github/workflows/ci.yml:242- The change adds three copies of the same PyYAML assertion; the copy in tests-herdr is inert. That job's only suite step runsbin/fm-test-run.sh --family real-herdr-gated(.github/workflows/ci.yml:313-322), and neither YAML-parsing script is in that family: tests/fm-test-run.test.sh resolves to pure-contract-unit (bin/fm-test-run.sh:275) and tests/fm-nm-test-contract.test.sh is unmatched by family_for_basename, so it falls to the unclassified remainder that list_portable_serial collects. The two copies that actually enforce the guarantee are the new steps at lines 67-73 (tests-portable-parallel-1, which runs tests/fm-test-run.test.sh via list_portable_parallel_1) and lines 176-182 (tests-portable-serial, which runs tests/fm-nm-test-contract.test.sh). Harmless, but it is a third copy of a rule in a lane that can never exercise it. Worth surfacing because the prior round's instruction said to add the assertion to "the existing CI required-tools preflight" on the assumption that step was shared across lanes - it is herdr-only, which is why the fixer also had to add the two lane-level copies. Removing the herdr copy is the simplification; keeping it is the author's call since it was explicitly requested.🔧 Fix applied.
1 warning still open:
.no-mistakes.yaml:80- The Pi carve-out names a drive that does not exist in this suite, and its nearest real counterpart is the exact pattern the next sentence forbids. Line 80-81 says a live drive includes "a .pi/extensions extension loaded by the real installed pi binary, because being loaded by pi is how that extension ships and runs", and lines 81-84 then say importing a module into the test's own interpreter does NOT qualify "even when the interpreter is a child process". No test in this repository loads an extension through the pi binary. tests/fm-pi-primary-types.test.sh is atsc --noEmittypecheck: it symlinks the installed package's declarations into a temp tsconfig project (lines 43-47) and never executes anything. tests/fm-pi-watch-extension.test.sh writes a stub node_modules/@earendil-works/pi-coding-agent (lines 43-52) and runsnode --input-type=modulewithconst mod = await import(pathToFileURL(process.env.PLUGIN).href)(lines 89-107, 167-181, 234-248) against a hand-rolledpiobject. tests/fm-calm-pi-extension.test.sh is the dangerous one: it symlinks the REAL installed @earendil-works/pi-coding-agent into the fixture's node_modules (line 175), prints its version vianode -p "require('$PI_PACKAGE_DIR/package.json').version"(line 160), then still doesconst extension = await import(...)in a plain node process with a fabricatedpi(lines 184-196). Concrete reachable sequence: a change to .pi/extensions/fm-branch-supervision.ts or lib/fm-calm-visibility.ts routes through bin/fm-test-run.sh:1299-1314, which selects fm-pi-branch-extension, fm-pi-watch-extension, fm-calm-pi-extension, fm-pi-primary-types plus live-harness-optin (which gate-skips without opt-in). The only non-skipped evidence available for a Pi extension scenario is therefore the fake-host import. The analyst reads the transcript'sok - ...lines, reads the case, sees the real installed pi package resolved in node_modules, and applies line 80 verbatim: scenario reported "pass" with "live": true, when the pi runtime never started and the host object was a fake. No error is raised; the label is simply wrong, and it is wrong in the same direction the whole runbook exists to prevent. The defect lives inside a component the change introduces, so the smallest honest remedy is to correct the sentence rather than add machinery: either drop the Pi carve-out (no case here satisfies it, so it can only ever be misapplied), or state the condition in terms the analyst can check against the transcript - the pi executable itself appears in the command the case ran - and say explicitly that loading a .pi/extensions file withnodeand supplying a hand-writtenpiobject is NOT live, because that is what every Pi suite in this repository actually does. Flagged ask-user rather than auto-fix because the user explicitly directed this sentence in the previous round ("A TypeScript editor extension loaded in-process by the real installed pi binary qualifies because that is how it ships and runs. State both explicitly."); that the suites contain no such case, and that fm-calm-pi-extension.test.sh links the real package while still faking the host, is new information that the instruction was not written against.🔧 Fix applied.
3 issues (2 warnings, 1 info) still open:
.no-mistakes.yaml:97- The runbook states flatly that every "ok -" line proves a drive, and this change ships a counterexample in the same commit. Lines 97-99 say: "Each &fix(watcher): make check wakes lossless via watcher-side suppression kunchenguid/firstmate#34;ok - <behavior>&fix(watcher): make check wakes lossless via watcher-side suppression kunchenguid/firstmate#34; line, and each &fix(watcher): make check wakes lossless via watcher-side suppression kunchenguid/firstmate#34;PASS: <behavior>&fix(watcher): make check wakes lossless via watcher-side suppression kunchenguid/firstmate#34; line a child contract executable prints through its owning .test.sh wrapper, names a behavior that was driven". That contradicts line 85 in the same runbook ("the transcript proves the case ran and its named assertion held, never that it launched anything") and lines 119-125 (a case that "parses a declarative artifact - a YAML, JSON or config file - without running its consumer" stays untested). The new tests/fm-nm-test-contract.test.sh is exactly that case: it calls yaml.safe_load on .no-mistakes.yaml, launches no product surface at all, and prints "ok - no-mistakes does not configure commands.test" and "ok - no-mistakes carries a non-empty test.instructions runbook". Concrete reachable sequence: a later change edits .no-mistakes.yaml (this repo edits it routinely); the new mapping at bin/fm-test-run.sh:1432 selects tests/fm-nm-test-contract.test.sh; the transcript shows two "ok -" lines plus "FM_TEST_END ... exit=0 ... gate_skip=false"; the analyzer applies line 97-99 literally and reports that scenario "pass" with "live": true, when the gate - the actual consumer of that config - was never launched. No error is raised; only the label is wrong, and it is wrong in the one direction this entire runbook exists to prevent. The surrounding sentences do rescue a careful reader, but the runbook is prose consumed by an LLM, so a single unqualified sentence licensing the wrong verdict is the defect. Smallest honest remedy is a wording narrowing, not new machinery: say the "ok -" and "PASS:" lines name an assertion the case made, and that whether it was driven is settled only by reading the case, as line 83-87 already requires. Flagged ask-user because it edits prose the author and prior rounds deliberately shaped.tests/fm-test-run.test.sh:1511- A fix round rewrote an existing, unrelated test's parser and introduced a new third-party Python dependency for the portable suite. test_herdr_ci_family_run_has_a_step_timeout has nothing to do with test.instructions - it asserts the tests-herdr job/step timeouts - yet commit 78a61e6 replaced its working ruby/psych parse with python3 + PyYAML, and the two ci.yml preflight steps (lines 67-73, 176-182) exist only to support that port plus the new contract test. Nothing in the intent ("the repo's .no-mistakes.yaml has no test.instructions ... the analyzer simply has no instruction telling it so") requires touching that test or adding a CI dependency assertion. The port is also a portability regression on a platform this repo treats as first class: ruby with psych ships with macOS, PyYAML does not ship with macOS's system python3, and both scripts hard-failrather than skip. Concrete sequence: a contributor on a stock Mac edits .github/workflows/ci.yml or .no-mistakes.yaml, runs the documentedbin/fm-test-run.sh --changed(CONTRIBUTING.md:85), and gets "not ok - python3 with PyYAML is required to parse .github/workflows/ci.yml as YAML" on a test that passed before this branch - with no CI lane able to catch it, since the macos-stock-bash job runs neither script. CONTRIBUTING.md:120 states the portable suite stays safe on machines without optional tools. Because the defect lives in machinery a prior fix round added beyond what the original finding required, the smallest honest remedy is to revert that round to the minimal fix: restore the base ruby parse in test_herdr_ci_family_run_has_a_step_timeout (and drop the parallel-1 preflight it needs), keeping only the parser the new test.instructions guard actually requires. The alternative - keeping PyYAML and declaring it a documented prerequisite - is a dependency-policy call, which is why this is ask-user rather than auto-fix. Note the round-7 instruction endorsed "the two lane-level assertions"; that the swap trades a macOS-bundled interpreter for one that is not bundled is new information that instruction was not written against.bin/fm-test-run.sh:1432- The new script mapping sits in the shared.github/workflows/ci.yml|.no-mistakes.yamlcase arm, so editing ci.yml also selects tests/fm-nm-test-contract.test.sh, which only ever opens .no-mistakes.yaml. The narrower form that satisfies the intent is a separate.no-mistakes.yaml)arm carrying the script entry, leaving ci.yml with pure-contract-unit (which already picks up tests/fm-test-run.test.sh, the script that does assert on ci.yml). Cost today is a fraction of a second of extra test time, so this is informational rather than something to act on.🔧 Fix applied.
2 issues (1 error, 1 warning) still open:
tests/fm-nm-test-contract.test.sh:11- The round-9 revert to Ruby/Psych makes this test unrunnable on the host that runs this pipeline. Verified on this machine (hermes, Linux):command -v rubyfinds nothing - no /usr/bin/ruby, no rbenv/rvm/snap/homebrew ruby, nothing in a login shell - whilepython3 -c 'import yaml'reports PyYAML 6.0.3. So line 11 firesfail "ruby is required to parse .no-mistakes.yaml as YAML", the script prints "not ok" and exits 1.Concrete reachable sequence introduced by this change: the new mapping at bin/fm-test-run.sh:1432 adds
__script__:fm-nm-test-contract.test.shto the.github/workflows/ci.yml|.no-mistakes.yamlarm; this change itself edits .no-mistakes.yaml; sobin/fm-test-run.sh --changed(the path CONTRIBUTING.md:85 documents and this run's test step would take) now selects tests/fm-nm-test-contract.test.sh and it hard-fails. At base that script was familyunclassifiedand no mapping selected it, so this selection is new. tests/fm-test-run.test.sh:1511 fails identically (it ispure-contract-unit, already selected for these paths), so the changed-file run goes red with two failures on the gate host.This is not a portability nicety: CONTRIBUTING.md:120 states the portable suite "remains safe on machines without those tools", no document lists ruby as a prerequisite, and the only two ruby call sites in the whole repo are these. GitHub's ubuntu-latest and macOS runners ship ruby, so CI stays green and nothing catches this.
The defect lives in machinery a prior fix round rewrote: commit 1750abf replaced a working python3+PyYAML implementation with ruby to satisfy the round-9 instruction "fully revert the PyYAML port ... no new third-party dependency or preflight should remain". That instruction's premise - that ruby is the interpreter this repo can count on - does not hold on the gate host, which is new information it was not written against. Choosing the remedy is a dependency-policy decision, not a mechanical correction, which is why this is ask-user rather than auto-fix: the options are (a) install ruby on the fleet hosts and document it as a suite prerequisite, (b) go back to python3+PyYAML and accept it as a documented prerequisite, or (c) accept either parser with a hard fail when neither is present. Any of the three keeps the round-9 requirement that parser absence hard-fails and never skips green.
.no-mistakes.yaml:59- Lines 59-60 keep "the agent-facing contracts in AGENTS.md, .agents/skills/ and docs/" in the list of "product entry points", but the very next rule defines driving as "running those real entry points as real processes against a throwaway FM_HOME" (lines 66-68) and "exercised the shipped artefact through the real runtime that actually runs it, in a process the case launched" (lines 74-76). A markdown contract has no process runtime, so the definition is incoherent for exactly the members this sentence adds - and the incoherence resolves in the unsafe direction.Concrete reachable sequence, once this runbook is on the default branch: a change edits .agents/skills/harness-adapters/SKILL.md; bin/fm-test-run.sh:1422 maps that path to pure-contract-unit, which includes tests/fm-harness-adapter-references.test.sh. That case launches no product process at all - it awk-extracts a ```json harness-adapter-routing-v1 fence out of the SKILL.md, shape-checks it with jq, and prints "ok - harness adapter routing artifact is normalized and every target is readable". The analyzer has just been told this SKILL.md IS a product entry point and that "THE SUITES IN tests/ ARE THAT DRIVE". The refused-boundary clause that would catch it names only "a declarative artifact - a YAML, JSON or config file" (lines 119-121), which a markdown skill file is not. Result: the scenario is reported "pass" with "live": true on evidence that never started the consumer, no error raised, wrong label in the one direction this runbook exists to prevent - the same defect round 9 fixed for the "ok -" line, reopened for prose.
This clause is the surviving half of the "AGENT-FACING PROSE IS A REAL SURFACE HERE" section that commit 5fa1db7 already deleted from this runbook. Nothing in the intent needs it: the blocked changes it names (afk-inject-wedge-claude-pane, fv-13) are code changes whose real suites drive bin/ entry points. The smallest honest remedy is removing the component, not hardening it - drop "and the agent-facing contracts in AGENTS.md, .agents/skills/ and docs/" from the entry-point sentence, so a prose-only change falls back to the gate's built-in policy, which already has a "no-surface" verdict for docs-only changes. Flagged ask-user because this sentence is author-written prose (present since 5cef065) that states deliberate product policy.
🔧 Fix applied.
3 issues (2 warnings, 1 info) still open:
bin/fm-test-run.sh:1432- The new__script__:fm-nm-test-contract.test.shentry is a real behavior change to the changed-file selector, and nothing asserts it.grep -n "no-mistakes.yaml" tests/fm-test-run.test.shreturns nothing, so no case covers this arm.Why that matters concretely rather than as generic coverage advice:
__script__names are resolved at bin/fm-test-run.sh:1536 byif [ -f "tests/$script_name" ]; then add_script ...; fi- with noelse. A name that does not resolve is silently dropped. So iftests/fm-nm-test-contract.test.shis ever renamed (a plausible move: it is the onlyfm-nm-*script and is familyunclassified, so it is not pinned by any family table, lane, or coverage guard),bin/fm-test-run.sh --changedon a.no-mistakes.yamledit silently stops selecting the contract test, exits 0, prints no warning, and the guard this change just added quietly stops running. That is a wrong selection set produced without erroring, which is exactly the failure mode the sibling__unmapped__:branch at line 1504 raises adiefor.The repo already has the pattern and it is cheap to follow: tests/fm-test-run.test.sh:242-268 and :274-284 assert mapping arms by writing to a path in the fixture repo and checking
bin/fm-test-run.sh --list --changed --base HEADoutput. Remedy: addfm-nm-test-contract.test.shto the fixture script list ininit_changed_fixture_repo(tests/fm-test-run.test.sh:96-121), touch.no-mistakes.yamlin that fixture, and assert the listing containstests/fm-nm-test-contract.test.sh. That is an assertion over the runner's own executed output, not over source text, so it fits the repo's test-quality rule.tests/fm-nm-test-contract.test.sh:11- The round-10 fix introducedyaml_to_json()byte-identically in two places: tests/fm-nm-test-contract.test.sh:11-20 and tests/fm-test-run.test.sh:1507-1516. Both files already source tests/lib.sh, and CONTRIBUTING.md states plainly that "Shared test helpers live intests/lib.sh(reporters, temp roots, git fixtures)" and "Source those instead of copying a fake toolchain into a new suite." So there is an immediately competing semantic owner named by the repo's own instructions, not an abstraction preference.These two copies are also the repository's only YAML parse sites (
grep -rln 'YAML.load_file\|yaml.safe_load' tests/ bin/returns exactly these two files), so the parser-selection policy this change just established now has two hand-synchronized definitions. The next change to that policy - the third one in as many rounds - has to find and edit both, and a divergence produces two different answers to "is this host able to parse YAML" with no test catching the drift.Remedy is mechanical and non-user-visible: move the single definition into tests/lib.sh next to the other shared primitives and delete both copies. No behavior changes.
tests/fm-nm-test-contract.test.sh:25- The parser selection is only half portable, and the failure message does not say so.yaml_to_jsonfalls back toruby -ryaml -rjsonwhen PyYAML is missing, but the value-extraction stage that consumes its output is unconditionallypython3(tests/fm-nm-test-contract.test.sh:26-34 and :48-64, tests/fm-test-run.test.sh:1525-1544). So on a host with ruby but no python3 at all,yaml_to_jsonsucceeds, the pipe topython3 -cfails with 127, and the case dies with "failed to read commands.test from the parsed .no-mistakes.yaml" - not the message at line 25 that names both parsers. The fallback advertised as "or ruby with psych" cannot actually carry the case alone.Recording this as informational rather than actionable because python3 is already a de-facto hard dependency of this suite: bin/fm-test-run.sh:1607 dies with "--json requires python3" (the mode the gate's evidence artifact uses), line 1025 does the same for --aggregate-json, and 25 of the tests/*.test.sh scripts invoke python3 directly. A host that cannot run python3 cannot run this suite at all, so the ruby branch is correctly scoped to the case it was asked to cover - python3 present, PyYAML absent - and that case works. The only inaccuracy is the wording of the error, and the hard-fail-never-skip requirement holds on every path. Noting it so the "portable parser selection" claim is not read as broader than it is.
🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
.no-mistakes.yamlmodified,bin/fm-test-run.sh --list --changed --base HEADlists tests/fm-nm-test-contract.test.sh; 01-chang…bin/fm-test-run.sh tests/fm-nm-test-contract.test.shprintsok - no-mistakes does not configure commands.testand `ok - no-mistakes carries a non-empty test.instructions…test.instructions is NoneType rather than a string; whitespace-only ->test.instructions is empty; YAML list -> `test.instructions is…not ok - commands.test must be absent or empty so Test stays intent-targeted; got: 'bin/fm-test-run.sh --all', exit=1ok - no-mistakes config change selects its contract testandok - Herdr CI family-run step times out at 20 min under a 75 min job backstop, FM_TEST_END exit=0bin/fm-test-run.sh tests/fm-documentation-audiences.test.shexit=0 with all four ok lines after the docs/configuration.md and SKILL.md editsbin/fm-test-run.sh --list --changed --base d859c15in the branch worktree (real changed set includes tests/fm-nm-test-contract.test.sh)bin/fm-test-run.sh --list --changed --base HEADin an isolated copy of HEAD where only .no-mistakes.yaml was editedbin/fm-test-run.sh tests/fm-nm-test-contract.test.shon the branch config (both ok lines)Adversarial mutations in the isolated copy, each re-run throughbin/fm-test-run.sh tests/fm-nm-test-contract.test.sh: test.instructions key removed; whitespace-only; replaced by a YAML list;commands.test: 'bin/fm-test-run.sh --all'reintroducedPYTHONPATH=<yaml-shadow> bin/fm-test-run.sh tests/fm-nm-test-contract.test.shand the same script run directly, with ruby genuinely absent from the host (no-parser hard-fail path)bin/fm-test-run.sh tests/fm-test-run.test.sh(includes the newtest_nm_config_change_selects_its_contract_testfixture case and the ci.yml timeout case that now uses the shared helper)bin/fm-test-run.sh tests/fm-documentation-audiences.test.shafter the docs/configuration.md and SKILL.md prose editsbash -c '. tests/lib.sh; fm_yaml_to_json .github/workflows/ci.yml'with and without PyYAML available (rc=127 when no parser)Host capability probe:command -v ruby(absent),python3 -c 'import yaml'(PyYAML 6.0.3).no-mistakes.yaml:51- The new test.instructions runbook hand-enumerates product entry points (bin/fm-voice-client.py, bin/fm-voice-relay.py, bin/fm-mail.py, bin/fm-arm-command-policy.mjs, bin/fm-cd-command-policy.mjs, bin/fm-extension-launch-barrier.mjs) while docs/scripts.md is the authoritative inventory of bin/ scripts and their roles. The two already disagree: docs/scripts.md:76 lists bin/fm-extension.mjs as a runnable entry point ("Bind, inspect, verify, and strictly invoke trusted external process-event adapter packages") and bin/fm_inbox_conversation.py as the transport behindfm-inbox.sh conversation, and neither appears in the runbook's list. The list is principle-first ("Being something the fleet runs is the test, not the language"), so the omission does not misdirect the analyzer today, and .no-mistakes.yaml is executable configuration rather than a classified prose surface (docs/documentation-audiences.json scope), so its wording is config-owned and out of this phase's scope - especially since the runbook's exact prose was tuned by explicit user decisions across review rounds 9-11. Left unchanged deliberately. Follow-up worth considering: replace the enumeration with the principle plus a pointer to docs/scripts.md as the inventory owner, so a new non-shell entry point cannot silently fall outside the runbook's examples.✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.
CI shard timing
This change adds
tests/fm-nm-test-contract.test.shto the portable-serial remainder. Its measured runner duration is 706 ms; the finalBehavior portable serial 1job passed in 22m58s, so this PR adds about 0.05% of that shard runtime. The added case is a measurable contributor, but it does not explain the shard timeout pattern tracked separately.