test(harness): pin crew-harness realignment safety for secondmates and crewmates - #1921
Open
sbracewell64 wants to merge 2 commits into
Open
sbracewell64 wants to merge 2 commits into
sbracewell64 wants to merge 2 commits into
Conversation
…d crewmates `config/crew-harness` answers two questions with a single value: it is the crewmate fallback and the second link of the secondmate resolution chain (`config/secondmate-harness` -> `config/crew-harness` -> own). Correcting it for one of those roles can silently re-answer the other, and nothing enforced that an explicit `config/secondmate-harness` insulates a secondmate launch from such an edit, or that an active `config/crew-dispatch.json` keeps the edit away from crewmate launches. Add three regressions to the existing split suite: - D1 pins the resolution-level property: with `config/secondmate-harness` set, editing `config/crew-harness` leaves `fm-harness.sh secondmate` unchanged. - D2 pins the same property at the spawn chokepoint, which re-resolves on every launch, so the pin survives a respawn after the edit. - D3 pins that `config/crew-harness` cannot reach a crewmate launch while dispatch profiles are active, for each matching rule and the default: a resolved profile produces an identical launch command and recorded profile across crew-harness values, and an unresolved spawn refuses rather than reading the file. Each case carries a negative control, because all three would also hold if `config/crew-harness` had simply stopped working: the unpinned secondmate must move with the edit, and with no dispatch file the crewmate launch must diverge by crew-harness value. Both controls were confirmed to fail when the pin resolution and the dispatch backstop were mutated in turn. No behavior change; resolution logic is unmodified.
sbracewell64
force-pushed
the
fm/cfvc-14-e05-expiry
branch
from
August 7, 2026 22:46
7878b76 to
0fce11e
Compare
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
Split config/crew-harness's two jobs so the expiring temporary Claude crew-routing authorization (E-05, expires 2026-08-08) expires into a CORRECT default rather than a stale one. config/crew-harness has read 'claude' since 2026-07-22 and answers two different questions at once: it is a DEAD crewmate fallback (config/crew-dispatch.json is always active and its default is harness pi, and fm-spawn refuses an unresolved crewmate spawn rather than reading the file) and the LIVE second link of the secondmate launch chain (bin/fm-harness.sh secondmate resolves config/secondmate-harness -> config/crew-harness -> own). An exception that expires into a stale default has not expired.
Ordering is load-bearing and was specified up front: config/secondmate-harness must be set explicitly FIRST, then config/crew-harness aligned with the dispatch default. The reverse order changes secondmate launch behaviour as a side effect of a crewmate correction. No secondmate exists today, so this is the cheap moment.
DELIBERATE SCOPE DECISION, which is why this diff is tests-only: config/ and data/captain.md are LOCAL, gitignored files in the operator's private home. They cannot be changed from a worktree and are not tracked material, so the tracked deliverable is the resolution logic plus tests, and the exact local config writes were reported separately to the operator. The two local writes are: config/secondmate-harness = claude (already applied by the operator after the captain answered the one engineer question - secondmates launch on Claude, with Pi kept available as a deliberate later option), and config/crew-harness claude -> pi, to be applied on or after 2026-08-08 together with deleting the expiring captain-preferences section.
SECOND DELIBERATE DECISION: bin/fm-harness.sh was NOT modified. Its fallback chain already resolves an explicit config/secondmate-harness ahead of the crew fallback, verified directly against the live config bytes on a scratch copy. What was missing was ENFORCEMENT, not logic - nothing pinned the ordering property that makes the crew-harness realignment safe. So the change adds three regressions to the existing split suite tests/fm-secondmate-harness.test.sh (new section D), extending that file rather than adding a new runner:
Each case deliberately carries a NEGATIVE CONTROL, because all three assertions would also hold if config/crew-harness had simply stopped working everywhere: the unpinned secondmate must move with the edit, and with no dispatch file the crewmate launch must diverge by crew-harness value. The launch-command comparisons normalize the task id out of the embedded brief path, because otherwise the divergence control would pass vacuously on the differing ids alone. Both controls were confirmed non-vacuous by mutation testing: disabling the pin resolution fails D1/D2, and removing the dispatch backstop fails D3.
This touches worker-launch harness resolution and was treated with R1-RUNTIME rigor per the plan's certification note. bin/fm-lint.sh is clean, the full split suite and tests/fm-spawn-dispatch-profile.test.sh pass.
KNOWN PRE-EXISTING FAILURE, not caused by this change and deliberately not fixed here: tests/fm-secondmate-harness.test.sh's test_spawned_secondmate_uses_its_harness_supervision_model fails whenever the checkout sits on a named feature branch, because the worktree-tangle guard fires on it and its output reaches that assertion. Confirmed identical at base commit ed376cf with this change absent.
What Changed
tests/fm-secondmate-harness.test.sh: withconfig/secondmate-harnesspinned, editingconfig/crew-harnessleavesfm-harness.sh secondmateresolution unchanged (D1) and the pin survives a respawn through the spawn chokepoint, which re-resolves on every launch (D2); while dispatch profiles are active,config/crew-harnesscannot reach a crewmate launch — launch command and recorded profile are identical across crew-harness values for each matching rule and the default, and an unresolved crewmate spawn refuses instead of reading the file (D3).bin/or config changes:bin/fm-harness.shalready resolves an explicitconfig/secondmate-harnessahead of the crew fallback, and these tests enforce that ordering so the expiring E-05 crew-harness realignment (claude → pi on 2026-08-08) lands safely. The remaining files in the nominal range vs base 33a4287 are fork-trunk commits already landed through PRs feat(bin): guard watcher liveness across supervision scripts #8–Crash/reboot resilience + atomic O_EXCL lock (fix WSL2 mkdir non-atomicity) #48 that origin/main does not yet carry.Risk Assessment
✅ Low: A tests-only additive commit whose new assertions I verified statically against the unmodified production logic in bin/fm-harness.sh and bin/fm-spawn.sh (resolution chain, per-launch re-resolution, dispatch backstop message and exit code), each carrying non-vacuity negative controls, satisfying every required intent constraint and touching no production code.
Testing
Ran the full fm-secondmate-harness suite (all 50 tests pass, D1–D3 included) plus the adjacent dispatch-profile suite, proved all three new regressions non-vacuous by mutating the pin resolution and the dispatch backstop in turn (each mutation is caught by its intended test), and captured a CLI transcript of the E-05 expiry scenario showing the pinned secondmate harness survives the crew-harness claude→pi realignment while the unpinned control moves; no visual evidence applies since this is a shell-CLI test harness with no rendered surface, and the worktree was left clean.
Evidence: E-05 expiry scenario CLI transcript (pinned secondmate survives crew-harness claude→pi; unpinned control moves)
$ # STEP 1 - the 2026-08-08 realignment: crew-harness claude -> pi $ echo pi > config/crew-harness $ bin/fm-harness.sh secondmate # MUST stay claude: the explicit pin insulates secondmate launches claude $ bin/fm-harness.sh crew # crewmate fallback now matches the dispatch default (pi) pi $ # CONTROL - the same realignment with NO pin $ echo pi > config/crew-harness && bin/fm-harness.sh secondmate # unpinned: the edit moves secondmate launches too piEvidence: New D-section regressions passing (baseline, unmodified bin/)
ok - D1 a config/crew-harness realignment leaves a PINNED secondmate harness alone; an unpinned one moves with it (the pin-first ordering is load-bearing) ok - D2 spawn: a pinned config/secondmate-harness keeps launching its own harness across a crew-harness realignment; an unpinned home takes the edit ok - D3 spawn: config/crew-harness cannot reach a crewmate launch while dispatch profiles are active, for every matching rule and the default; with no dispatch file it is live againEvidence: Mutation A: pin resolution disabled in fm-harness.sh — D1 and D2 each fail (tests are non-vacuous)
--- D1 alone --- not ok - pinned: secondmate resolved 'claude' before the edit, expected the pinned pi exit=1 --- D2 alone --- not ok - pinned spawn: pre-edit secondmate launched on 'claude', expected the pinned pi exit=1Evidence: Mutation B: dispatch backstop removed from fm-spawn.sh — D3 fails (backstop assertion is live)
--- D3 alone --- not ok - an unresolved crew spawn should refuse while dispatch profiles are active (crew-harness='claude'): expected exit 1, got 0 exit=1Evidence: Full fm-secondmate-harness suite run (50 tests, exit 0)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
⏭️ **Rebase** - skipped
.agents/skills/afk/SKILL.md- branch carries 46 commit(s) that exist on your local main branch but were never pushed to origin/main; rebasing would bundle this unrelated work (201 file(s)) into the PR:Push main to origin, or rebase your branch onto origin/main, before gating.
tests/fm-secondmate-harness.test.sh:2590- crew_ship_spawn does not pin CLAUDE_CONFIG_DIR, unlike the mirrored run_spawn helper in tests/fm-spawn-dispatch-profile.test.sh which pins it explicitly (with a rationale comment) because fm-spawn.sh prefixes 'CLAUDE_CONFIG_DIR=...' onto claude launch commands. No current D3 assertion can be affected — the dispatch-profile rows use grok/pi/codex (never claude) and the negative control only does a substring check — but any future tightening of D3 to exact-launch comparisons involving claude would inherit a dependency on the developer's ambient environment. Purely a hardening/consistency note.tests/fm-secondmate-harness.test.sh:1- Scope note: the supplied base 33a4287 is the upstream merge-base, so the nominal range spans ~45 already-landed fork-trunk commits (each merged via its own PR, ending at ed376cf / PR Crash/reboot resilience + atomic O_EXCL lock (fix WSL2 mkdir non-atomicity) #48). The branch's actual change is the single tests-only tip commit ef1e200 (+216 lines in tests/fm-secondmate-harness.test.sh), which is what this review assessed in depth and which matches the intent's 'tests-only' claim exactly — no non-test files are touched by the tip commit.✅ **Test** - passed
✅ No issues found.
bash tests/fm-secondmate-harness.test.sh— full changed suite, all 50 tests pass (exit 0), including new D1/D2/D3; the known branch-dependent supervision test passes on this detached-HEAD checkout, consistent with the intent's claimTrimmed D-only runner over the suite:test_pinned_secondmate_survives_a_crew_harness_realignment(D1),test_pinned_secondmate_spawn_survives_a_crew_harness_realignment(D2),test_crew_harness_is_inert_for_dispatch_resolved_crewmates(D3) — all pass on unmodified bin/bash tests/fm-spawn-dispatch-profile.test.sh— adjacent suite covering the dispatch backstop D3 leans on, all passMutation A (non-vacuity): made resolve_secondmate() in bin/fm-harness.sh ignore config/secondmate-harness — D1 and D2 each fail independently; revertedMutation B (non-vacuity): removed the crew-dispatch.json consultation backstop from bin/fm-spawn.sh's single-spawn path — D3 fails; revertedManual end-to-end E-05 expiry scenario viaFM_CONFIG_OVERRIDEscratch config: pinned secondmate-harness=claude holdsfm-harness.sh secondmate=claude across crew-harness claude→pi whilecrewmoves to pi; unpinned control moves to pi with the editDiff audit of ef1e200 vs its parent: single-file, tests-only (+216 lines), bin/ and config untouched — matches both deliberate scope decisions in the intentPost-test cleanup: mutations reverted via git checkout, temp runners deleted,git statusclean, no leaked /tmp fixtures✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.