feat(bin): sync fork with upstream delivery contract, supervision, and CI sharding - #2
Merged
Merged
Conversation
* perf(ci): shard the portable serial behavior lane across runners The Behavior portable serial job ran all 69 scripts of the serial remainder on one runner. The measured serial sum on run 30725985757 was 1143762 ms (19m04s) against a 20-minute timeout, so the job intermittently reached the cap and was cancelled with every step passing. Setup is only about 7s, so the cost is entirely test wall time. Split the lane into four separate-runner shards. Each shard is still strictly serial, and separate runners mean no two of these stateful scripts ever share a machine, so the split needs no concurrency isolation proof. Assignment is longest-processing-time bin packing over measured per-script duration hints, balancing every shard to 285941 ms (~4m46s) of expected work, and the timeout tightens from 20 to 15 minutes. bin/fm-test-run.sh owns the shard count and refuses a lane whose "ofN" disagrees with it, while ci.yml derives the same count from strategy.job-total rather than a literal, so changing it in either file alone fails the lane loudly instead of leaving part of the required suite unrun. --check-coverage additionally proves the shards are non-empty, disjoint, and exactly equal to the serial lane. No test is weakened, skipped, or removed. Also replace the wall-clock sleeps in the --jobs scheduler test fixture with an explicit signal handshake between the fixtures. The old 0.5s-versus-0.05s race failed on a loaded machine; the handshake passes under sustained CPU saturation. * no-mistakes(review): Correct portable serial shard balance evidence * no-mistakes(document): Document portable serial shard evidence accurately
…henguid#1545) * fix(bin): identify harness sessions by path and report delivered wakes Two supervision faults, both reported by a contributor and both open on the default branch. Fault 1: the Stop auto-arm never claims the home. fm_harness_ancestry_pid() matched only the basename of `ps -o comm=`, and Claude Code's native installer names the per-session executable by its version (.../share/claude/versions/ 2.1.220), so that basename identifies nothing. Three real failure shapes follow: a version-named session is missed entirely and the hook exits 0 with the epoch never written (unconditional on Linux, where procps reports the kernel exec name and ignores argv[0]); a claude-named daemon that directly parents sessions wins the outermost-contiguous-claude rule ahead of the session itself; and a session that is both version-named and daemon-parented has its live lock reclaimed as stale and rewritten to the shared daemon pid, corrupting the home's ownership record. Harness identity now also reads whole components of the executable path and of argv[0], which is what both platforms still carry. Matching whole components only keeps that widening safe: bin/fm-claude-stop-autoarm.sh and ~/.claude/hooks scripts have no "claude" component. Ownership is then decided against the session's whole contiguous harness ancestry rather than one chosen pid, which is the honest form of the question the library already documents ("does the current process descend from that same harness?"). That subsumes the outermost-pid rule for Claude's nested bg-spare worker chain instead of reverting it, and lets a daemon-parented session recognize its own lock. Lock acquisition still writes the outermost pid of the run, the only pid that lives as long as the session. Fault 2: an attached arm reports a delivered cycle as FAILED. The watcher prints its one reason line to its own stdout, so only the arm that forked it can read that line; an arm that attached observes nothing but a released lock and called a completely successful cycle "cycle ended without an actionable reason". No supervision event was lost - the durable queue held it - but every harness protocol reads that line as "supervision is down" and directs a manual re-arm. The arm now resolves an unobservable close against the durable wake queue, which records every wake before the watcher prints it and whose sequence counter never rewinds, not even across a drain. A cycle the queue proves delivered a wake reports that wake and exits 0; a cycle whose records a handling turn already drained reports the delivery without inventing a reason line; only a cycle that delivered nothing is still the typed nonzero failure. Fixing it in the arm covers codex, opencode, pi, grok and kimi, not just the Claude Stop path. Regressions: tests/fm-session-lock-ancestry.test.sh pins both platforms' ps semantics behind a deterministic process table and runs the real Stop auto-arm in version-named, daemon-parented, and combined real process trees, each orphaned so the walk cannot escape the fixture. tests/fm-watch-arm.test.sh drives a real watcher and a real attached arm through a real wake. Every fault case fails on the previous code. * no-mistakes(review): Bind watcher delivery records to process identity * no-mistakes(review): Return validated watcher identity atomically * no-mistakes(review): Track watcher successors by PID and identity * no-mistakes(document): Consolidate watcher arm-cycle documentation ownership
* fix(supervision): harden Claude auto-arm failure handling * no-mistakes(review): Guarantee automatic retry after Claude auto-arm failures * no-mistakes(review): Gate attended fail-open on verified supervision failure * no-mistakes(document): Document Claude auto-arm retry and guard scope * no-mistakes: apply CI fixes * fix(supervision): make Claude fail-open progression monotonic * no-mistakes(review): Preserve auto-arm failure episodes until verified watcher recovery * no-mistakes(review): Linearize auto-arm failure progression across existing locks * no-mistakes(review): Linearize positive recovery across shared failure episode lock * no-mistakes(review): Scope Claude recovery contention to Claude guard mode * no-mistakes(document): Align supervision auto-arm documentation * no-mistakes(review): Preserve actionable wakes despite healthy successors * no-mistakes(document): Refresh supervision auto-arm documentation
…d#1563) * feat(bin): require an explicit ship delivery mode in fm-brief A ship brief's definition of done was shaped by a silent per-project registry lookup, so an adjusted brief and the task's recorded delivery could disagree and no one had to decide anything per task. fm-brief now requires --mode on ship scaffolds, validates it against the closed set, refuses the conditional no-mistakes-prod-only registry policy as a task mode, and records the choice as a fixed machine-readable "Delivery contract: mode=<mode>" line that fm-spawn can check. --mode is refused on scout and secondmate scaffolds, and --yolo is refused outright because the worker never owns approval decisions. * feat(bin): require an explicit ship delivery contract at spawn and promotion fm-spawn resolved every ship and scout task's mode and yolo from the project registry, so the delivery posture was never a per-task decision and could contradict the brief the worker was about to follow. fm-spawn now requires --mode and --yolo on ship spawns, validates both against their closed sets, and reads the brief's recorded delivery contract line and refuses a mismatch before any endpoint exists; a brief scaffolded before that line existed warns once and launches on the flag. A batch carries one shared contract that each pair still checks against its own brief. Scout and secondmate spawns refuse the flags, and a scout now records no mode or yolo at all, which teardown and the snapshot already tolerate. When the explicit mode carries less rigor than the project's standing posture, a deviation notice is printed and the spawn continues, so the registry stays advisory rather than an enforced default. fm-promote requires the same two flags, because a scout carries no posture to inherit, and writes them into the task record with the kind flip. fm-project-mode keeps its one registry parser for the mechanical consumers that have no task in hand, accepts the conditional no-mistakes-prod-only annotation and maps it to its most rigorous leg for them, and grows --raw so the deviation notice can tell a conditional policy apart from a flat mode. * docs: record the explicit per-task delivery contract AGENTS.md section 7 now owns how each ship task's mode and yolo are resolved at intake, including the surface classification for a no-mistakes-prod-only project and the unregistered-project fallback, and the project-management skill defines that conditional policy as a registration-time posture with its defaults and initialization consequences. The registry blurb, script table, and architecture section follow: the registry records the captain's standing posture, and task delivery is decided per task and passed explicitly. * test: pass ship delivery flags per call site in the Herdr launcher e2e The shared spawn helper also launches a secondmate, which refuses the flags, so the contract belongs at each ship call site rather than inside the helper. * test: pass the ship delivery contract in the secondmate suites Both suites scaffold or spawn an ordinary ship task as the control case for a secondmate assertion, so each needs the explicit contract the ship path now requires.
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
2 participants
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
Synchronize the kaku-san/firstmate fork with the current kunchenguid/firstmate default branch after fetching and verifying origin/main=2bd6d0c1ba81a88baf279461ca06f61a33221ca6, upstream/main=4ee4a0a2790cfaa5e47b30fa462f16546f2ab5b6, and merge base=cd73e75e02a1c1e74811b00c5ee08ffae8a59e1e. Preserve the fork's existing parallel-first dispatch requirement and intent while integrating portable CI sharding, corrected session-lock and attached supervision, hardened Claude supervision recovery, and explicit per-task delivery contracts. Reconcile AGENTS.md deliberately so these requirements compose under one-owner and size discipline and retain every unique safety boundary; review semantic interactions beyond textual conflicts without unrelated customization or misplaced evidence prose. Do not force-push, rewrite history, drop upstream safety boundaries, push a default branch, merge the PR, touch the primary local copy, or touch its untracked bun-baseline.zip. Validate the complete integrated branch with the documented test runner, bin/fm-lint.sh, bin/fm-doc-audience-check.sh, and focused contract tests. Push only fm/firstmate-upstream-sync to kaku-san/firstmate and open a green PR against that fork's default branch; captain retains merge approval.
What Changed
bin/fm-spawn.sh,bin/fm-brief.sh, andbin/fm-promote.shnow require an explicit--mode/--yolodelivery contract on every ship task and refuse a spawn whose mode disagrees with the brief's recorded contract;bin/fm-session-lock-lib.sh,bin/fm-watch-arm.sh, andbin/fm-turnend-guard.shcorrect session-lock ancestry and attached-watcher supervision;bin/fm-claude-stop-autoarm.shgains failure-episode and attended-alarm records that harden auto-arm recovery. New coverage lands intests/fm-task-delivery.test.sh,tests/fm-session-lock-ancestry.test.sh, andtests/fm-watch-arm.test.sh..github/workflows/ci.ymlruns the portable serial lane as a 4-way matrix withfail-fast: false, derivingFM_SERIAL_LANEfromstrategy.job-totalso a resized matrix is refused rather than silently leaving a shard unrun;bin/fm-test-run.showns shard membership, rejects anofNthat disagrees with it, and the coverage guard now proves the shards partition the serial lane exactly. The lane timeout drops from 20 to 15 minutes.AGENTS.mdwas reconciled by hand to keep the fork's parallel-first dispatch clause intact alongside upstream's new intake rules for resolving delivery mode and yolo posture.README.md,docs/architecture.md, and the skill and contributor docs move from per-project "project modes" to per-task "delivery modes", anddocs/fm-test-portable-shards.mdrecords that the lane now holds 72 scripts against 69 weight hints, so the three newly added tests run on the conservative default weight until the hints are refreshed from a green CI run.Risk Assessment
✅ Low: The merge is upstream-verbatim outside AGENTS.md (git diff 4ee4a0a..7bc4c0c touches only that file), the fork's parallel-first clause and its strengthened serialization wording are preserved intact, upstream's delivery-contract paragraph composes with them under disjoint ownership, both PreToolUse seatbelts and every upstream safety boundary remain unchanged, and the only issues found are a stale balance table and a message-less fail-closed exit that self-heals on the next Stop.
Testing
I exercised the four integrated upstream behaviors and the preserved fork requirement against the complete merged branch with the documented runner, bin/fm-test-run.sh. Fifteen targeted scripts covering the delivery contract, session-lock ancestry, Claude auto-arm recovery, portable CI sharding, attached watcher supervision, turn-end guard, spawn dispatch, and AGENTS.md doc discipline ran green except one case in tests/fm-watcher-lock.test.sh, which failed once under concurrent load and then passed four consecutive isolated reruns; that file is byte-identical to upstream/main, so the flake is pre-existing upstream rather than merge-induced, and it is reported as a warning for the captain. Beyond the automated suite I captured product-level evidence a reviewer can read directly: a CLI transcript of fm-spawn.sh refusing all four malformed delivery contracts with no task metadata written, a shard transcript proving the four portable serial shards exactly partition the 72-script lane and that a mismatched shard count is refused, a merge-reconciliation transcript showing AGENTS.md is the only file differing from upstream/main with all four fork lines intact and line accounting exactly additive, and a rendered HTML page plus screenshot of the merged section 7 intake block colour-coded by provenance so the fork's parallel-first clause and upstream's delivery-contract block can be seen composing under one owner. The UI-facing surface here is agent-instruction prose rather than an app screen, so the rendered-HTML screenshot is the reviewer-visible artifact for it. Two validations named in the intent, bin/fm-lint.sh and bin/fm-doc-audience-check.sh, were deliberately not run: this phase is barred from linters and static analysis, and the Lint phase owns them. During cleanup I killed two orphaned watcher processes the flaky case left pointing at this worktree (they were holding the runner's stdout pipe and stalling it) and confirmed the worktree is clean with no stray files or processes.
/var/folders/hv/gd626zx51jb1_3rghgd97mp00000gn/T/no-mistakes-evidence/01KZ3ATDAK72A7T6ET2E48CYFM/agents-md-reconciled.png)Evidence: Same reconciliation view as rendered HTML
Evidence: Per-task delivery contract: fm-spawn.sh CLI refusals
$ fm-spawn.sh demo-1 <project> claude # contract never decided error: ship spawns require --mode <no-mistakes|direct-PR|local-only>; resolve it at intake from the captain's instruction and the project's registered posture in data/projects.md exit=1 $ fm-spawn.sh demo-1 <project> claude --mode fast-path --yolo off # mode outside the closed set error: --mode must be one of no-mistakes, direct-PR, local-only (got 'fast-path') exit=1 $ fm-spawn.sh demo-1 <project> claude --mode no-mistakes # yolo posture omitted error: ship spawns require --yolo <on|off>; it is this task's routine approval authority, not a project lookup exit=1 $ fm-spawn.sh demo-1 <project> claude --mode direct-PR --yolo off # brief records mode=no-mistakes error: delivery mismatch for demo-1: the brief says mode=no-mistakes but this spawn passed --mode direct-PR; correct the flag or re-scaffold the brief so the worker's instructions and the task record agree exit=1 # task metadata written by the refused spawns (must be none): state/: []Evidence: Portable CI sharding: 4 shards exactly partition the serial lane
$ bin/fm-test-run.sh --check-coverage FM_TEST_COVERAGE ok total=107 parallel=24 serial=72 serial_shards=4 herdr=11 shard 1of4 -> 17 scripts shard 2of4 -> 18 shard 3of4 -> 16 shard 4of4 -> 21 whole lane: 72 union of shards: 72 duplicates in union: 0 PARTITION EXACT: union == whole lane, no missing, no duplicates $ bin/fm-test-run.sh --list --lane portable-serial-1of3 fm-test-run: lane 'portable-serial-1of3' asks for 3 portable serial shards but this runner is configured for 4 (see --list-lanes) exit=2Evidence: Merge reconciliation proof: parents, upstream-boundary sweep, fork-line containment, line accounting
actual merge parents of HEAD: 2bd6d0c1ba81a88baf279461ca06f61a33221ca6 4ee4a0a2790cfaa5e47b30fa462f16546f2ab5b6 # 1. Of the 56 files upstream changed, only AGENTS.md differs from upstream in the merge: differs from upstream: AGENTS.md total files differing from upstream/main: 1 (bin/, tests/, docs/, .github/ all byte-identical to upstream) # 3. The fork-only commit 2bd6d0c is fully contained in the merge (no fork line dropped): PRESENT At every intake, and whenever long validation, infrastructure or platform work, an external wait... PRESENT The Selected delivery path and approval authority subsection exclusively owns standingyoloau... PRESENT Identification may be silent and may find no material path, and identifying a path is never auth... PRESENT Serialize only after naming a true semantic dependency, shared mutable external state, incompati... # 4. Line accounting - the merge is exactly additive over the merge base: merge-base (cd73e75): AGENTS.md 60449 bytes, 536 lines fork-main (2bd6d0c): AGENTS.md 61593 bytes, 539 lines upstream-main (4ee4a0a): AGENTS.md 61576 bytes, 542 lines merged (7bc4c0c): AGENTS.md 62720 bytes, 545 lines 536 (base) + 3 (fork-only net) + 6 (upstream-only net) = 545 merged linesEvidence: Targeted behavior-test run log (15 scripts)
/var/folders/hv/gd626zx51jb1_3rghgd97mp00000gn/T/no-mistakes-evidence/01KZ3ATDAK72A7T6ET2E48CYFM/targeted-timing.json)Evidence: Clean isolated rerun of fm-watcher-lock confirming the flake
Source: Clean isolated rerun of fm-watcher-lock confirming the flake (local file:
/var/folders/hv/gd626zx51jb1_3rghgd97mp00000gn/T/no-mistakes-evidence/01KZ3ATDAK72A7T6ET2E48CYFM/watcher-lock-rerun.log)Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
⏭️ **Review** - skipped
docs/fm-test-portable-shards.md:67- The portable-serial shard table and the portable_serial_weight_hints block in bin/fm-test-run.sh were measured for a 69-script serial lane in the oldest of the four merged upstream commits (f5ab708 / PR perf: shard portable serial tests across CI runners kunchenguid/firstmate#1544). The three later upstream commits add tests/fm-session-lock-ancestry.test.sh, tests/fm-watch-arm.test.sh, and tests/fm-task-delivery.test.sh; none appears in list_portable_parallel_1/2 or the real-herdr-gated family, so all three derive into portable-serial with no hint and take PORTABLE_SERIAL_DEFAULT_WEIGHT_MS (20000). tests/fm-claude-stop-autoarm.test.sh (hinted 60521 ms) also grew by 186 lines and tests/fm-turnend-guard.test.sh (hinted 5986 ms) by 461 lines in the same window. Coverage is unaffected because list_portable_serial is derived and run_coverage_guard proves the shards partition it exactly; only the published 15/18/17/19 partition, the ~285.9 s per-shard estimate, and the '3x margin' rationale for cutting .github/workflows/ci.yml tests-portable-serial from 20 to 15 timeout-minutes are no longer evidenced. Worst-case rebalance still estimates well under the 15-minute cap. Refreshing hints from a green CI run on this branch would restore the doc's accuracy.bin/fm-turnend-guard.sh:151- On the healthy-watcher path in Claude mode,fm_failure_episode_reset "$STATE" && exit 0falls through to a bareexit 2, blocking the Stop with no stderr, so the model receives 'Stop hook feedback' with no reason or instruction. fm_failure_episode_reset acquires .turnend-claude-blocks.lock through fm_lock_try_acquire, which is single-attempt and non-blocking (bin/fm-wake-lib.sh), so any concurrent holder makes it return 1 even though the watcher is verified healthy. Both .claude/settings.json Stop hooks fire on the same event and the previous Stop's asyncRewake auto-arm stays attached across the idle period, so bin/fm-claude-stop-autoarm.sh:209 can be taking that same lock on its HEALTHY branch while a new Stop's guard runs; the loser blocks (guard) or rewakes with no banner (auto-arm). The window is microseconds and the next Stop self-heals, and the fail-closed choice matches docs/turnend-guard.md ('positive watcher recovery clears the block budget ... before either hook reports ordinary recovery'), but a blocking exit with no message gives the operator and the model nothing to act on. A one-line stderr naming the lost budget lock would close the diagnostic gap without changing the fail-closed semantics.⏭️ **Test** - skipped
tests/fm-watcher-lock.test.sh:474- tests/fm-watcher-lock.test.sh casewatch restart attaches to a verified healthy peeris timing-flaky under concurrent machine load: it failed once during my run (not ok - restart did not attach to the verified healthy peer: watcher: started pid=78004 (beacon fresh)) while a Chrome screenshot render was running in parallel, then passed 4/4 in isolation. The case runsfm-watch-arm --restartwithFM_ARM_CONFIRM_TIMEOUT=1andFM_ARM_ATTACH_POLL=0.1, so under load the healthy-peer confirmation window can expire and the arm starts a fresh watcher instead of attaching. Two aggravating factors: (1) on failure thefailhelper exits the script but leaves the startedfm-watch.shchild alive, and that orphan holds fm-test-run.sh's stdout pipe, so the runner hung indefinitely (~10 min until I killed pid 78004) rather than moving to the next script; (2) the file is unchanged by this merge - it is byte-identical to upstream/main - so this is a pre-existing upstream flake, not merge-induced. Fixing it is outside a sync merge's scope, so the captain should decide whether to widen the confirmation budget / add child cleanup to the case now or file it upstream.bin/fm-test-run.sh --json <evidence>/targeted-timing.json tests/fm-task-delivery.test.sh tests/fm-session-lock-ancestry.test.sh tests/fm-claude-stop-autoarm.test.sh tests/fm-test-run.test.sh tests/fm-watch-arm.test.sh tests/fm-watcher-lock.test.sh tests/fm-turnend-guard.test.sh tests/fm-brief.test.sh tests/fm-spawn-batch.test.sh tests/fm-spawn-dispatch-profile.test.sh tests/fm-documentation-audiences.test.sh tests/fm-ensure-agents-md.test.sh tests/fm-supervision-instructions.test.sh tests/fm-guard-stale-banner.test.sh tests/fm-wake-queue.test.sh- 15 scripts, 14 green on first passbash tests/fm-watcher-lock.test.shx3 standalone plusbin/fm-test-run.sh tests/fm-watcher-lock.test.sh- 4/4 clean reruns (30 ok, 0 not ok each) isolating the healthy-peer flakebin/fm-test-run.sh --check-coverage- FM_TEST_COVERAGE ok total=107 parallel=24 serial=72 serial_shards=4 herdr=11bin/fm-test-run.sh --list-lanesand--list --lane portable-serial-{1,2,3,4}of4diffed against--list --lane portable-serial- union equals the whole lane, zero duplicates, zero missingbin/fm-test-run.sh --list --lane portable-serial-1of3- runner refuses a shard count that disagrees with it (exit 2)Manual CLI verification of the per-task delivery contract:bin/fm-spawn.sh <id> <project> claudewith (a) no delivery flags, (b)--mode fast-path --yolo off, (c)--mode no-mistakesand no yolo, (d)--mode direct-PR --yolo offagainst a brief recording mode=no-mistakes - all four refused with distinct explanations and left state/ emptygit rev-list --parents -n1 7bc4c0c- merge parents match the brief's origin/main and upstream/main exactlygit diff 4ee4a0a 7bc4c0cplus a per-filegit diff --quiet 4ee4a0a 7bc4c0c -- <f>sweep over all 56 upstream-changed files - AGENTS.md is the sole divergencegit show 2bd6d0c -- AGENTS.md | grep '^+'each line re-checked withgrep -Fqxagainst merged AGENTS.md - all 4 fork lines PRESENTgit show <sha>:AGENTS.md | wc -c/-lat cd73e75 / 2bd6d0c / 4ee4a0a / 7bc4c0c - line accounting 536+3+6=545Rendered the merged AGENTS.md section 7 intake block to HTML with per-line provenance and screenshotted it viachrome-devtools-axi open+screenshotat 1440x1620docs/fm-test-portable-shards.md:79- The portable-serial weight hints in bin/fm-test-run.sh cover 69 scripts while the lane now holds 72; the three tests this merge added (fm-session-lock-ancestry, fm-watch-arm, fm-task-delivery) run on the conservative PORTABLE_SERIAL_DEFAULT_WEIGHT_MS default. I documented the drift, but refreshing the hints needs per-shard timing artifacts from a green CI run and is a code change outside this docs phase. Follow-up: refresh the hints and the table from the next green run using the procedure already in docs/fm-test-portable-shards.md.AGENTS.md:118- This change adds state/.watch-deliveries.log (and its lock) as the watcher's terminal-delivery ledger, which is not listed in AGENTS.md section 2's state inventory. I deliberately did not add it: the pre-existing sibling ledger state/.watch-cycle-exits.log is also absent from that inventory, and docs/watcher-continuity.md already owns both, so adding a line would grow always-loaded guidance against the established precedent. Flagging as a judgment call in case the captain wants arm-layer ledgers enumerated there.✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.