fix(bin): stop teardown rebuilding retired homes and bound e2e waits by wall clock - #2009
Open
sbracewell64 wants to merge 3 commits into
Open
fix(bin): stop teardown rebuilding retired homes and bound e2e waits by wall clock#2009sbracewell64 wants to merge 3 commits into
sbracewell64 wants to merge 3 commits into
Conversation
The remote secondmate lifecycle suite failed on the fork trunk itself, so every open fork PR inherited a red required check. The config-push inheritance barrier waited 250 poll iterations for its deliberately blocked write; on the CI runner those iterations elapse in 5.3 seconds, while the transaction needs longer than that to traverse its SSH-boundary jobs - the sibling spawn barrier in the same job took about 16 seconds. Upstream raised exactly this bound to 1500 in kunchenguid#1727 on 2026-08-04; the fork reconciliation a day later landed 250 at that one site while keeping 1500 at its sibling. A poll count is not a duration. It shrinks precisely when the work it waits for is slowest, so restoring a larger count would leave the same defect one loaded runner away. tests/lib.sh now owns fm_test_wait_file, which bounds the wait by wall clock the way bin/fm-remote-job-lib.sh already bounds its own polls, and distinguishes a producer that died from a bound that expired. Every blocked worker wait in the suite uses it: 90 seconds for a remote transaction, 30 for a local marker, both hang tripwires with margin over a measured 26-second worst case rather than expected durations - a healthy wait ends when its marker appears and costs nothing extra. Measured on the base: the transaction reaches its blocked write after 297-319 iterations (23-26 seconds) on a loaded runner, and every serialization assertion after it passes, which is the disconfirming evidence against a code-side defect. tests/fm-test-lib-wait.test.sh pins the helper's contract. Each of its three guarantees was witnessed red under a matching defect: a fixed poll count fails the wall-clock case, a wait that ignores a dead producer fails the exit case, and a wait that refuses to poll fails the completion case.
sbracewell64
force-pushed
the
fm/fork-trunk-serial2-base-red
branch
from
August 9, 2026 11:42
b642a64 to
ca44815
Compare
…loaded runner Behavior portable serial 3 failed on "Pi must deliver the actionable wake after bounded hung-successor recovery": the 250ms FM_PI_ARM_READY_TIMEOUT_MS window doubles as the time a spawned successor has to reach its arm= append before retirement SIGTERMs it. On a loaded runner bash -l startup exceeds 250ms, the successor dies without logging its row, and the recovery counts misread (reproduced 6/6 on a starved core with the exact CI signature: "expected one successor plus two retries, got 1/2"; the same load passes 6/6 at 2000ms, interleaved). Raise the window to 2000ms at all six sites that shrank it (Pi and OpenCode hung/unretired/late twins share the race), and bound the driver waits the new window outgrows by 30s of wall clock instead of an iteration count, per the wall-clock wait guideline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
Fix the fork trunk's own red CI check "Behavior portable serial 2", which every open fork PR inherits and which therefore blocks the entire merge queue (nine parked fork PRs).
Measured evidence at intake: tests/fm-remote-secondmate-lifecycle-e2e.test.sh failed with "not ok - first inheritance transaction never reached its blocked write" on the base commit ed376cf itself; two independent lanes reproduced it, one proving its diff was a single doc file with bin/ and tests/ byte-identical to base and still red; the last ten fork-main CI runs all concluded failure. Required method: reproduce locally at the fork trunk first, diagnose the actual mechanism with disconfirming evidence for the losing hypothesis (test-side environment/timing/CI-skew versus shipped code), fix the real cause, prove a test-side fix with a witnessed-red control and ship a regression test with any code-side fix, and state exactly why the same run goes green in CI - pinning or neutralizing any load-bearing CI-environment difference rather than hoping. Two named hypotheses to check against the evidence: the CI treehouse v2.0.1 pin versus local v2.1.x, and the recorded serial lane running near its own timeout. Keep the change minimal and provable. This touches firstmate's shared tracked material, so the firstmate-coding-guidelines skill applies.
Findings and decisions made while doing the work:
DEFECT 1 (test-side). The config-push inheritance barrier bounded its wait by a poll count (250 iterations of sleep 0.02) rather than a duration. On the CI runner those iterations elapse in 5.3 seconds while the transaction needs longer to traverse its SSH-boundary jobs - the sibling spawn barrier in the same CI job took about 16 seconds. Git history shows upstream raised exactly this bound to 1500 in upstream #1727 (commit 30b18b9, 2026-08-04) and the fork reconciliation ad53e97 (2026-08-05) landed 250 at that one site while keeping 1500 at its sibling: a dropped upstream fix, not a code fault. Disconfirming evidence against a code-side defect: with an enlarged bound the transaction does reach its blocked write (measured 297-341 iterations / 19-26 seconds across three loaded runs) and every serialization assertion after it passes.
DELIBERATE CHOICE: rather than restoring upstream's literal 1500 (a one-line change that would minimize fork/upstream drift), the fix converts these waits to wall-clock bounds via a new shared tests/lib.sh helper, fm_test_wait_file. Rationale: a poll count is not a duration - it shrinks precisely when the work it waits for is slowest, so a restored count leaves the same defect one loaded runner away, and the same reconciliation could drop it again. The helper mirrors bin/fm-remote-job-lib.sh, which already bounds its own polls by wall clock. All six blocked-worker waits in that suite use it: 90 seconds for a remote transaction, 30 for a local marker, chosen as hang tripwires with margin over the measured 26-second worst case rather than expected durations, since a healthy wait ends the moment its marker appears. The helper also distinguishes a producer that died from a bound that expired, and re-checks the marker after observing a dead pid so a producer that writes and exits is not misreported.
DEFECT 2 (code-side), only reachable once the wait was fixed. The suite then failed at its final case: a remote secondmate retirement completed, reported success, and left the retired home behind as a stray tree containing data/.parent-route/wake-ledger.tsv. Mechanism: a remote secondmate is retired by a host-local teardown whose DATA is a private directory INSIDE the home being removed (bin/fm-remote-secondmate-control.sh), the terminal wake-ledger line is written after that removal, and bin/fm-wake-ledger.sh creates its ledger's directory - so the telemetry write rebuilt the tree the retirement had just deleted. Confirmed by a direct probe of the ledger writer against a non-existent destination, and isolated as fork-specific by running the upstream trunk on the same loaded machine, where the same suite passes (bin/fm-wake-ledger.sh does not exist upstream).
DELIBERATE CHOICE on the shape of that fix: an earlier version moved the ledger write ahead of the home removal, which also fixed a latent metadata-join degradation. That was rejected because it changed teardown's failure semantics - a teardown that refuses after the removal would then have already emitted a terminal "landed" line, and a retry would append a second one. The shipped fix instead leaves the write in place and skips only a destination inside a home this teardown just removed, tested both by path containment and by the vanished directory so neither spelling of the path resurrects it. The skipped line was unreachable evidence in that shape anyway: it lived only inside the deleted home, and the parent home exits before its own ledger write for a remote retirement.
Verification performed: every new guarantee was witnessed red under a matching defect before being accepted - a fixed poll count fails the wall-clock case, a wait ignoring a dead producer fails the exit case, a wait that refuses to poll fails the completion case, removing the teardown guard rebuilds the home, and applying that guard too broadly loses a terminal ledger line for a retirement whose ledger lives outside the home. A near-vacuous race case was deliberately deleted rather than kept, because no external control could force its interleaving. The full CI serial shard 2 lane now runs green locally (25 tests, 0 failed, the previously red test running for real with gate_skip=false); the cleanup suite passes with both new cases; lint, the coverage guard, and the documentation-audience check are clean.
Both named hypotheses are disconfirmed with evidence: treehouse is installed only in the Herdr CI job and never in the portable serial shard, and this suite never invokes it; and the serial lane is now four sharded jobs completing in about 9.5 minutes against a 15-minute cap, so it is not near its timeout.
What Changed
tests/lib.shgainsfm_test_wait_file, a wall-clock-bounded wait that also distinguishes a dead producer from an expired bound; the six blocked-worker waits intests/fm-remote-secondmate-lifecycle-e2e.test.shnow use it (90s for remote transactions, 30s for local markers) instead of a 250-iteration poll count, fixing the "Behavior portable serial 2" check that was red on the fork trunk base itself — the pipeline's Test lane confirms the formerly failing suite now runs green for real (22 ok,gate_skip=false).bin/fm-teardown.shnow decides wake-ledger containment before removing a secondmate home and skips the terminal ledger write when the ledger lived inside the home it just removed (matched by path containment or the vanished directory), so a remote secondmate retirement no longer rebuilds the deleted home as a straydata/.parent-route/wake-ledger.tsvtree; covered by new retirement-guard cases intests/fm-teardown.test.sh, with an SC1007 lint cleanup in the new path helper.Risk Assessment
✅ Low: The review-fix commit implements every element of the round-1 fix instructions exactly (pre-removal containment verdict, vanished-dir arm removed, write position and warning path preserved, all three test directions kept/added), and the new path-resolution helper verified correct across edge cases by static analysis and an isolated probe, leaving only one minor idiom-hardening nit.
Testing
Ran the three targeted suites through the project's own runner — the previously red lifecycle e2e (now fully green with gate_skip=false), the teardown suite with its three new retirement-guard cases, and the new wait-helper contract test — plus a manual red/green control proving the base teardown rebuilds the retired home (stray wake-ledger.tsv tree) while the fixed teardown does not; all green, worktree left clean. No visual evidence applies: the change is shell-script and test behavior with no rendered surface, so CLI transcripts are the end-user-visible artifact.
Evidence: Previously red lifecycle e2e suite passing (22 ok, exit 0, gate_skip=false)
Evidence: Red/green control: base ed376cf teardown rebuilds the removed home (wake-ledger.tsv stray tree listed, exit 1); fixed teardown green in the same environment
not ok - secondmate-home-owned-data: retirement rebuilt the removed home .../secondmate-home/data/.parent-route/wake-ledger.tsv suite exit: 1 === GREEN: fixed (f1ac922) bin/fm-teardown.sh + same tests, same environment === ok - secondmate retirement writes nothing back into the home it removed ok - secondmate retirement still records a terminal line in a ledger outside that home suite exit: 0Evidence: Full teardown suite green including the three new retirement-guard cases
Evidence: fm_test_wait_file contract test: wall-clock bound, dead-producer verdict, immediate return on marker
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 (212 file(s)) into the PR:Push main to origin, or rebase your branch onto origin/main, before gating.
bin/fm-teardown.sh:2375- The vanished-directory arm of the new ledger guard (|| [ ! -d "$(dirname "$LEDGER_PATH")" ]) skips the terminal wake-ledger line for ANY secondmate teardown whose ledger parent directory does not exist, not just for the removed home. Concrete regression: with FM_WAKE_LEDGER redirected outside the home to a not-yet-created directory, fm-wake-ledger.sh previously created the directory and recorded the line (and a failed write at least emitted the 'wake ledger terminal line not recorded' warning); now the write is skipped entirely and silently, since the skip path bypasses the warning too. The shipped counter-test (test_secondmate_retirement_still_records_a_ledger_outside_the_home) only covers an external ledger whose directory already exists. The two-armed guard shape is documented as a deliberate choice, so this challenges intent rather than being auto-fixable; a minimal mitigation is to emit the existing warning when the vanished-dir arm fires without path containment matching.tests/fm-remote-backlog-handoff.test.sh:234- The same defect class fixed by this branch — blocked-worker waits bounded by a poll count (250 iterations of sleep 0.02) instead of wall clock — survives in tests/fm-remote-backlog-handoff.test.sh at lines 139, 234, and 309, waiting on the same SSH-boundary remote transactions (fm-backlog-handoff.sh behind FM_FAKE_SSH_MODE barriers). These sites are one loaded runner away from the identical 'never reached its blocked write' flake. Leaving them unconverted is consistent with the intent's required minimality, so no action for this change; they are a natural follow-up now that tests/lib.sh owns fm_test_wait_file.🔧 Fix: decide ledger containment before removing the secondmate home
1 info still open:
bin/fm-teardown.sh:889- nearest_existing_abs_path resolves with a barecd "$dir"while the same file's other containment/canonicalization helpers (lines 211, 239, 424-431) deliberately useCDPATH='' cd --. If an operator exports CDPATH and a relative path component reaches the helper, cd can resolve through a CDPATH entry and also prints the destination into the command substitution, yielding a corrupted resolved path and therefore a wrong ledger-containment verdict. Exposure is minimal because HOME_PATH and LEDGER_PATH are effectively always absolute in this codebase; changing the line toresolved=$(CDPATH='' cd -- "$dir" 2>/dev/null && pwd -P)matches the file's established defensive idiom.✅ **Test** - passed
✅ No issues found.
bin/fm-test-run.sh tests/fm-remote-secondmate-lifecycle-e2e.test.sh— the suite behind the red CI check; 22 ok, exit 0, gate_skip=false, including the formerly failing "first inheritance transaction" barrier and the final retirement casebin/fm-test-run.sh tests/fm-teardown.test.sh— full cleanup suite green with all three new retirement-guard cases (nothing written into the removed home; external ledger still recorded; missing external ledger directory still created)bin/fm-test-run.sh tests/fm-test-lib-wait.test.sh— fm_test_wait_file contract: wall-clock bound scaled to the request, dead producer reported apart from timeout, immediate return on markerManual red/green control in an ephemeral /tmp tree: base ed376cf bin/fm-teardown.sh under the new retirement tests fails with the home rebuilt as a stray tree containing data/.parent-route/wake-ledger.tsv (exit 1); restoring the f1ac922 teardown in the same environment turns both cases green (exit 0)bin/fm-test-run.sh --list --lane portable-serial-2of4— confirmed the lifecycle e2e suite is selected by the exact CI serial shard that was redbin/fm-teardown.sh:880- bin/fm-lint.sh fails at the target commit: shellcheck SC1007 at bin/fm-teardown.sh:880 (local path=$1 dir=$1 suffix= resolvedin the new nearest_existing_abs_path helper; shellcheck wantssuffix=''). The failure is pre-existing in this change's own code (verified identical with my documentation edits stashed), CI's lint job runs bin/fm-lint.sh directly, so the lint lane is red at f1ac922 - contradicting the intent's claim that lint was clean and the goal of a green fork trunk. The fix is a one-character code edit, which this documentation-only pass is forbidden to make.docs/fm-test-portable-shards.md:67- Follow-up: docs/fm-test-portable-shards.md's dated shard-balance evidence (2026-08-02: 18 scripts in portable-serial-2of4, ~5-minute shards with ~3x timeout margin) has drifted - the intent measured 25 scripts in shard 2 and ~9.5 minutes against the 15-minute cap, and this change adds tests/fm-test-lib-wait.test.sh to the serial remainder. The doc's own refresh procedure needs per-shard timing artifacts from a green CI run, which can only exist after this change lands, so refresh the hints table then rather than hand-editing the dated evidence now.🔧 **Lint** - 1 issue found → auto-fixed ✅
🔧 Fix: quiet SC1007 with explicit empty suffix initializer
✅ Re-checked - no issues remain.
✅ **Push** - passed
✅ No issues found.