Conversation
Owner
|
Automated reminder: thanks for the PR! This branch currently has a merge conflict with the base branch. When you get a chance, please rebase onto (or merge) the latest base branch, resolve the conflict, and push. After that, checks will re-run and the PR will get looked at again. Noted for firstmate#1307 at |
…locally Once the no-mistakes pipeline commits its own gate fixes, the run head exists only in the bare gate repo (the worktree's no-mistakes remote) and is not an object in the task worktree. The worktree-only head lookup then rejected every live run past its first gate fix, and attribution fell through to a stale prior run whose head still equalled the worktree HEAD, reporting a live validation as failed (reproduced 2026-07-25 and 2026-07-29). Resolve the head in the gate repo when the worktree lacks the object and apply the same ancestry test there, so diverged or rewritten heads and locally advanced tips stay rejected.
The scheduler test's slot-refill margin was 0.45s, while now_ms spawned python3 twice around every fixture; a python3 that resolves through a version-manager shim (pyenv) costs ~200ms per call, so on such hosts the refill deterministically missed the window and the test failed. Prefer core-perl Time::HiRes (~10ms) for now_ms and widen the slow fixture to 2s so the test pins the refill-before-oldest-finishes property instead of host process-spawn speed.
The missing-adapter-export subtest imports the .ts adapters directly, which needs node's native TypeScript type stripping (22.18+); unlike its siblings it is not gated on the installed pi package, so an older default node failed it with ERR_UNKNOWN_FILE_EXTENSION instead of skipping. Probe the import capability itself and skip when absent; verified the subtest still runs and passes under node 24.
The two gate-repo tests that assert the status-log fallback predate the semantic busy verdict this branch rebased onto: crew_busy_verdict now reports `unknown pane` when no lifecycle record exists, instead of falling through to the status log. Both tests exercise the rejection path (a diverged gate-only head, and local work advanced past the run head), so they need the same idle record and harness= meta their already-converted sibling test_local_advanced_past_run_head_invalidates uses. The rejection assertions themselves are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Kallas95
force-pushed
the
fm/fm-crewstate-gate-head
branch
from
August 6, 2026 13:29
f48dce8 to
bd668f8
Compare
|
Closed as superseded — this work already landed on main via #3681. — Kun's Firstmate |
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
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.
What Changed
bin/fm-crew-state.sh: collapsed the duplicatednm_run_head_matches_worktree/nm_coarse_head_matches_worktreepair into a singlenm_head_matches_worktree <run-head-rev>(theaxi statusvariant now just reads the TOONheadfield and delegates). When the task worktree cannot resolve the run head, resolution falls back to theno-mistakesremote's gate repo — accepting both a bare path and the same path infile://form — and applies the equality/ancestry test there. A live run whose head is a pipeline gate-fix commit now binds instead of falling through to a stale prior run on the same branch; diverged heads and locally-advanced worktrees still fail attribution.bin/fm-test-run.sh:now_msnow prefersperl -MTime::HiRes(~10ms) over apython3that may resolve through a version-manager shim (~200ms), and each interpreter only wins if it actually prints a value — a perl withoutTime::HiResor a stubpython3falls through to the next source and finally to second-precisiondate, rather than aborting the whole runner underset -eu.fm-crew-statecases build the real topology (throwaway worktree + real bare gate repo + gate-only commit) to cover run-step binding, the coarse runs-list fallback, afile://remote, and the two non-attribution controls;fm-test-runaddstest_timing_survives_broken_interpretersand lengthens the jobs-scheduler slow fixture from 0.5s to 2s for slot-refill headroom on loaded hosts; thefm-calm-pimissing-adapter-export subtest now probes node's native TypeScript type stripping and skips when unavailable instead of failing withERR_UNKNOWN_FILE_EXTENSION.Risk Assessment
✅ Low: The final round is a four-line test-diagnostic ordering fix that adopts the file's existing evidence-before-cleanup idiom verbatim, and every prior finding across the branch is now resolved with the two source changes verified behavior-correct on all paths and covered by new regression tests.
Testing
Ran the three suites touched by the range (fm-crew-state,fm-test-run,fm-calm-pi-extension) plus the project's conservative changed-file lane of 25 scripts, then produced product-level CLI evidence beyond pass/fail: an end-to-end repro that builds a real crew worktree and bare gate repo with a gate-only fix commit and runs the crew-state helper from both the base and target commits (failed -> working), including negative controls, a file:// remote variant, and the watcher absorb decision; a before/after transcript of the jobs-scheduler timing test under a slow python3 shim (deterministic pre-fix failure, 3/3 passes shipped) and of the timing test's failure diagnostics; and both directions of the calm-pi node capability gate. All targeted suites pass. The changed lane showed one unrelated red, fm-kimi-harness, caused by the host default python3 3.9.6 lacking tomllib; the Kimi files are untouched by this range and that suite passes once a tomllib-capable python3 is on PATH. No UI surface is involved, so evidence is CLI transcripts rather than screenshots. I also added a regression test for the fix's previously uncovered file:// gate-remote path; the working tree otherwise carries no transient artifacts.Evidence: Gate-only run head: live run read as failed before, working now (full CLI transcript incl. negative controls, file:// remote, watcher decision)
=== firstmate reads the crew state === $ bin/fm-crew-state.sh incident # BASE a53ffc1 (before the fix) state: failed · source: run-step · run failed $ bin/fm-crew-state.sh incident # THIS BRANCH (gate-repo head resolution) state: working · source: run-step · validating (running) === Downstream: watcher absorb decision for the live gate-fix crew === # BASE a53ffc1 absorb class : none provably working: no -> the wake SURFACES this crew as no longer working # THIS BRANCH absorb class : working provably working: yes -> a stale/no-verb wake is ABSORBED (crew left alone)Evidence: E2E harness that produced the gate-head transcript (real worktree + bare gate repo + gate-only fix commit)
Evidence: Scheduler timing flake reproduced pre-fix and closed by the shipped pair, plus failure-diagnostics comparison
per-call clock cost on this host: perl -MTime::HiRes : 24ms real python3: 73ms python3 via shim: 630ms old slot-refill margin: 450ms (0.5s slow fixture) BASE test file + BASE runner -> not ok - scheduler waited for oldest worker FAIL BASE test file + THIS BRANCH runner -> PASS / FAIL / PASS (0.5s margin still marginal) THIS test file (2s) + BASE runner -> PASS SHIPPED pair, repeats 1-3/3 -> PASS, PASS, PASS BEFORE cleanup fix: not ok - a broken perl and python3 must not abort the runner: (message empty, temp already removed) SHIPPED: fm-test-run.sh: line 123: * 1000: syntax error: operand expected (runner evidence printed)Evidence: Runner-timing evidence harness and the single-subtest driver
Evidence: calm-pi node capability gate, both directions (skips when node cannot import .ts, runs and passes when it can)
--- default node v23.3.0 --- TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".ts" skip: node cannot import TypeScript modules for Pi calm missing-adapter-export test --- with type stripping enabled --- ok - missing Pi presentation class exports reach the independent adapter degradation pathPipeline
Updates from git push no-mistakes
⏭️ **intent** - skipped
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 3 issues found → auto-fixed (2) ✅
bin/fm-test-run.sh:112- The new preferrednow_msbranch is guarded only bycommand -v perl, which does not guarantee theTime::HiResmodule is installed (it ships with full perl distributions but can be absent from minimal/stripped perl packages). When the module is missing,perl -MTime::HiRes=time -e ...aborts at compile time with exit 2 and empty stdout. Becausebin/fm-test-run.shruns underset -eu(line 65), the very first callerRUN_STARTED_MS=$(now_ms)(line 1154) propagates that non-zero status and terminates the entire test runner, instead of falling through to the python3 ordatebranches that are already written directly below. The previous code could not fail this way:python3 -c 'import time; ...'only ran when python3 existed, andtimeis unconditionally available there. Fix by making the perl branch fall through on failure (e.g. capture into a variable,if ms=$(perl ... 2>/dev/null) && [ -n "$ms" ]) or by probingperl -MTime::HiRes -e1once at script init and caching the result.bin/fm-crew-state.sh:440-nm_head_matches_worktreerepeats the same equality-then-ancestry test in both the worktree branch (lines 441-445) and the gate-repo branch (lines 450-454). The two only differ in which repo the git commands target, so the function can be flattened by resolving the repo first and running a single test: setrepo=$WT, and ifrev-parse --verifyfails there, reassignrepoto the gate path and re-resolve; then a single[ "$run_full" = "$local_full" ] && return 0followed bygit -C "$repo" merge-base --is-ancestor "$local_full" "$run_full" 2>/dev/nullas the trailing command. This is behavior-identical (the equality check is unreachable in the gate branch, since a head equal to worktree HEAD always resolves locally, and--is-ancestoris reflexive anyway) and removes one of the two duplicated branches plus about eight lines.tests/fm-calm-pi-extension.test.sh:307- The new TypeScript-import capability probe writesprobe.tsinto a directory with nopackage.json, while the fixture it gates creates{"type":"module"}at line 320 before importing its.tsadapters. The probe therefore exercises node's ambiguous-format detection for.tsrather than the explicit-ESM path the real subtest uses. If that detection does not apply (or is disabled), a node that fully supports type stripping would fail the probe with a SyntaxError and skip the missing-adapter-export regression test entirely, silently losing coverage on exactly the versions where it should run. Verified on this host (node v23.3.0) that the probe correctly fails with ERR_UNKNOWN_FILE_EXTENSION, so the gate works for the older-node case; the gap is only on capable nodes. Writing{"type":"module"}into the probe dir makes the probe match what it gates and removes the ambiguity.🔧 Fix: harden now_ms fallbacks, flatten head match, fix ts probe
1 warning still open:
tests/fm-test-run.test.sh:275- All four failure paths of the newtest_timing_survives_broken_interpretersdelete the temp dir before reading from it. At lines 275-276 and 283-284rm -rf "$tmp"runs as its own statement ahead offail "...: $(cat "$tmp/err.txt")", and at lines 279 and 287 the group{ rm -rf "$tmp"; fail "...: $(cat "$tmp/out.txt")"; }does the same. Group members execute in order andfail's arguments are expanded only whenfailis invoked, i.e. after therm, so the command substitution always reads a deleted path: the diagnostic body is empty andcatadditionally writes "No such file or directory" to stderr. The embedded runner stderr/stdout - precisely the evidence needed if the now_ms fallback chain ever regresses - is therefore never shown. Fix by capturing into a local before removing (err=$(cat "$tmp/err.txt"); rm -rf "$tmp"; fail "...: $err"), or follow the idiom already used at line 494 in this same file:{ cat "$tmp/out" "$tmp/err"; rm -rf "$tmp"; fail "<static message>"; }. Pass/fail detection itself is unaffected, so this is a debuggability defect rather than a wrong-result one.🔧 Fix: print runner evidence before temp cleanup in timing test
✅ Re-checked - no issues remain.
tests/fm-kimi-harness.test.sh- tests/fm-kimi-harness.test.sh fails on this host inside the changed-file lane with "fm-kimi-turnend-hook: refused: python3 with tomllib is required to validate config.toml" - the default python3 is 3.9.6 (tomllib landed in 3.11). Both the test and bin/fm-kimi-turnend-hook.sh are untouched by this range, and the suite passes with python3.13 on PATH, so it is a host-environment red unrelated to this change; no repo change is needed.bin/fm-test-run.sh tests/fm-crew-state.test.sh- all cases pass, including the 4 new gate-repo head-resolution cases and the file:// case I addedbin/fm-test-run.sh tests/fm-test-run.test.sh- passes, including the newtest_timing_survives_broken_interpretersbin/fm-test-run.sh tests/fm-calm-pi-extension.test.sh- passes; the .ts-import subtest skips on this host's node 23.3.0NODE_OPTIONS="--experimental-strip-types --no-warnings" bin/fm-test-run.sh tests/fm-calm-pi-extension.test.sh- the same subtest runs and passes when node can import .tsbin/fm-test-run.sh --changed --base a53ffc17fe5b4c6c2ba0de80e82adbbf16ec9253- 25 scripts, 1 unrelated failure (fm-kimi-harness, host python3 lacks tomllib)bin/fm-test-run.sh tests/fm-kimi-harness.test.shwith python3.13 on PATH - passes, confirming the red is environmentalManual E2E:gate-head-e2e.shbuilds a real worktree + bare gate repo + gate-only fix commit and runsbin/fm-crew-state.shfrom base a53ffc1 vs target for the incident, a diverged gate-only head, a locally advanced tip, and a file:// gate remoteManual E2E:crew_absorb_class/crew_is_provably_working(bin/fm-classify-lib.sh, via FM_CREW_STATE_BIN) over the same incident topology with both helper versionsManual:runner-timing-evidence.shrunstest_jobs_parallel_scheduler_and_failure_propagationfor base/target test-file x base/target runner under a ~630ms python3 shim, 3 repeats of the shipped pair, plus the failure-diagnostics comparison oftest_timing_survives_broken_interpretersFixture-integrity checks:git -C <worktree> cat-file -e <fix-head>^{commit}proves the run head is not an object in the crew worktree, andgit merge-baseproves the diverged control has no shared ancestordocs/fm-test-portable-shards.md:8- Follow-up, deliberately out of scope here: the 2026-07-29 isolation-proof durations in docs/fm-test-isolation-proof.md / .json feed the duration-balanced lane partition documented at docs/fm-test-portable-shards.md:8-48, and this change lengthened tests/fm-test-run.test.sh (jobs-scheduler fixture 0.5s -> 2s plus a new clock-fallthrough test) and tests/fm-crew-state.test.sh (new gate-repo cases). The recorded tables are dated evidence of a specific proof run, so I did not overwrite them with local numbers from different hardware; the measured imbalance (318 ms across ~162 s lanes) stays immaterial. Refresh both records frombin/fm-test-isolation-proof.sh --jobs 4 --json ...the next time the lane memberships in bin/fm-test-run.sh are re-derived, since that step edits executable code.✅ **Push** - passed
✅ No issues found.