fix: unblock the fork trunk's portable serial 2 check - #64
Merged
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.
…n teardown With the wait bound fixed, the same suite reached its final case and failed there: a remote secondmate retirement completed, reported success, and left the retired home behind as a stray tree containing data/.parent-route/wake-ledger.tsv. 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. The line was unreachable evidence there in any case: it lived only inside the deleted home, and the parent home never reaches its own ledger write for a remote retirement. The write now skips a destination inside a home this teardown just removed, by path and by the vanished directory, so neither spelling of the path resurrects it. Its position is unchanged, so a teardown that refuses after the removal still records no terminal line. Both directions are covered and were witnessed red: without the guard the retirement rebuilds the home, and with the guard applied too broadly a retirement whose ledger lives outside that home loses its terminal line.
The first guard asked whether the ledger belonged to the removed home partly by testing whether its directory had vanished. That over-reached: any secondmate retirement whose ledger directory did not exist yet took the skip path, so its terminal line was dropped - and silently, because the skip bypassed the "wake ledger terminal line not recorded" warning as well. Only the ledger that lives inside the removed home should ever be skipped. The question is now answered before the removal, while both paths still resolve, and the answer is carried as a verdict. Both paths resolve through their nearest existing ancestor, so a destination that does not exist yet still compares correctly and two spellings of the same home still compare equal. Every other destination keeps its previous behavior, including creating a directory that is not there yet and warning when the write fails. The new case pins exactly the regression: a retirement whose ledger lives outside the removed home in a directory that does not exist yet must still create it and record the line. It was witnessed red against the previous guard while the other two cases stayed green.
…mp root This suite's EXIT trap signalled the remote job worker and removed the temp root in the same breath. Signalling is not stopping: the worker can still be writing job state under that root while the removal walks it, and `rm -rf` then fails with "Directory not empty". Because that removal is the trap's last command, its status becomes the script's, so a run whose every assertion passed still reports failure - which is exactly how it presented, printing ALL TESTS PASSED and then exiting 1. The race is pre-existing on the trunk rather than new: the CI run for base ed376cf logged the identical "Directory not empty" failure twice for the sibling remote fixture, where it happened to be harmless because that suite's trap does not end on the removal. What changed is only exposure - this script moved between serial shards, and so onto a different runner. The trap now waits for the worker to actually die before removing, the same bounded wait tests/fm-remote-secondmate-lifecycle-e2e.test.sh already uses. Witnessed red through a control that models the real condition, a worker that keeps writing and does not die the instant it is signalled: removing immediately after the signal failed 5 of 5 times with the identical message, and waiting for the worker to die first failed 0 of 5.
sbracewell64
added a commit
that referenced
this pull request
Aug 9, 2026
* fix(tests): bound the blocked-worker waits by wall clock, not poll count 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. * fix(bin): stop a retired secondmate home from being rebuilt by its own teardown With the wait bound fixed, the same suite reached its final case and failed there: a remote secondmate retirement completed, reported success, and left the retired home behind as a stray tree containing data/.parent-route/wake-ledger.tsv. 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. The line was unreachable evidence there in any case: it lived only inside the deleted home, and the parent home never reaches its own ledger write for a remote retirement. The write now skips a destination inside a home this teardown just removed, by path and by the vanished directory, so neither spelling of the path resurrects it. Its position is unchanged, so a teardown that refuses after the removal still records no terminal line. Both directions are covered and were witnessed red: without the guard the retirement rebuilds the home, and with the guard applied too broadly a retirement whose ledger lives outside that home loses its terminal line. * fix(bin): decide ledger containment before the home is removed The first guard asked whether the ledger belonged to the removed home partly by testing whether its directory had vanished. That over-reached: any secondmate retirement whose ledger directory did not exist yet took the skip path, so its terminal line was dropped - and silently, because the skip bypassed the "wake ledger terminal line not recorded" warning as well. Only the ledger that lives inside the removed home should ever be skipped. The question is now answered before the removal, while both paths still resolve, and the answer is carried as a verdict. Both paths resolve through their nearest existing ancestor, so a destination that does not exist yet still compares correctly and two spellings of the same home still compare equal. Every other destination keeps its previous behavior, including creating a directory that is not there yet and warning when the write fails. The new case pins exactly the regression: a retirement whose ledger lives outside the removed home in a directory that does not exist yet must still create it and record the line. It was witnessed red against the previous guard while the other two cases stayed green. * fix(tests): let the handoff suite's worker die before removing its temp root This suite's EXIT trap signalled the remote job worker and removed the temp root in the same breath. Signalling is not stopping: the worker can still be writing job state under that root while the removal walks it, and `rm -rf` then fails with "Directory not empty". Because that removal is the trap's last command, its status becomes the script's, so a run whose every assertion passed still reports failure - which is exactly how it presented, printing ALL TESTS PASSED and then exiting 1. The race is pre-existing on the trunk rather than new: the CI run for base ed376cf logged the identical "Directory not empty" failure twice for the sibling remote fixture, where it happened to be harmless because that suite's trap does not end on the removal. What changed is only exposure - this script moved between serial shards, and so onto a different runner. The trap now waits for the worker to actually die before removing, the same bounded wait tests/fm-remote-secondmate-lifecycle-e2e.test.sh already uses. Witnessed red through a control that models the real condition, a worker that keeps writing and does not die the instant it is signalled: removing immediately after the signal failed 5 of 5 times with the identical message, and waiting for the worker to die first failed 0 of 5.
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.
What this fixes
The fork trunk's own
Behavior portable serial 2check is red ated376cf, so every open fork PR inherits a red required check. Two independent defects were involved; the first one hid the second.1. A blocked-worker wait bounded by a poll count instead of a duration
tests/fm-remote-secondmate-lifecycle-e2e.test.shwaits for a deliberately blocked worker to reach its barrier. The config-push inheritance barrier allowed 250 poll iterations ofsleep 0.02. 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.This is a dropped upstream fix rather than a code fault. Upstream raised exactly this bound to 1500 in upstream kunchenguid#1727 (
30b18b9, 2026-08-04); the fork reconciliationad53e97(2026-08-05) landed 250 at that one site while keeping 1500 at its sibling.Disconfirming evidence against a code-side cause: with an enlarged bound the transaction does reach its blocked write - measured at 297-341 iterations (19-26 seconds) across three loaded runs - and every serialization assertion after it passes.
Rather than restore the literal 1500, this converts the suite's blocked-worker waits to wall-clock bounds through a new shared helper,
fm_test_wait_fileintests/lib.sh. A poll count is not a duration: it shrinks precisely when the work it waits for is slowest, so a restored count would leave the same defect one loaded runner away. The helper bounds its wait the waybin/fm-remote-job-lib.shalready bounds its own polls, and it distinguishes a producer that died from a bound that expired. The suite uses 90 seconds for a remote transaction and 30 for a local marker - hang tripwires with margin over the measured 26-second worst case, not expected durations, since a healthy wait ends the moment its marker appears.2. A retirement that rebuilt the home it had just removed
Once the wait was fixed the suite reached its final case and failed there: a remote second-mate retirement completed, reported success, and left the retired home behind as a stray tree containing
data/.parent-route/wake-ledger.tsv.A remote second mate is retired by a host-local teardown whose
DATAis a private directory inside the home being removed (bin/fm-remote-secondmate-control.sh). The terminal wake-ledger line is written after that removal, andbin/fm-wake-ledger.shcreates its ledger's directory - so the telemetry write rebuilt the tree the retirement had just deleted. Confirmed by probing the ledger writer directly against a non-existent destination, and isolated as fork-specific by running the upstream trunk on the same machine, where the same suite passes (bin/fm-wake-ledger.shdoes not exist upstream).The write now skips only a destination that lies inside a home this teardown just removed. That question is answered before the removal, while both paths still resolve, and carried as a verdict; both paths resolve through their nearest existing ancestor, so a destination that does not exist yet still compares correctly and two spellings of the same home still compare equal. The write keeps its position in the script, so a teardown that refuses after the removal still records no terminal line and a retry cannot append a second one.
An earlier version of this guard inferred the same answer afterwards from a vanished directory. That over-reached - any second-mate retirement whose ledger directory did not exist yet lost its terminal line, silently, because the skip bypassed the
wake ledger terminal line not recordedwarning too. Review caught it; the third test case below pins it.Evidence
bin/fm-test-run.sh --lane portable-serial-2of4at this head: 25 tests, 0 failed. The previously red script ran for real (gate_skip=false) and passed.tests/fm-teardown.test.sh: 71 cases pass, including the three new retirement cases.bin/fm-lint.shclean,bin/fm-test-run.sh --check-coverageok,bin/fm-doc-audience-check.shok.Two hypotheses raised at intake are disconfirmed rather than assumed away: the pinned treehouse version cannot matter here, because 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 no longer near its timeout, being four sharded jobs of about 9.5 minutes against a 15-minute cap.
Disclosures
Check state. The validation pipeline reported
checks-passed, but no CI examined that head: zero check runs, and both upstream check suites sat ataction_requiredawaiting maintainer approval. A head nothing examined is unverified rather than green, so the evidence above is local. This fork PR is the first head that the fork's own workflows will actually judge - including the serial-2 fix it carries.Why this branch is separate. The pipeline run for this work pushed to
fm/fork-trunk-serial2-base-redand opened kunchenguid#2009 against upstream. At push time it rebased onto upstreammain, and becausebin/fm-wake-ledger.shdoes not exist upstream, the second-mate cleanup fix, its regression tests, and the review correction were silently dropped from the published head - verified: that head contains no wake-ledger lines inbin/fm-teardown.sh. That upstream PR and branch are deliberately left untouched; it stands as a partial contribution of the wall-clock change, which is the part that applies upstream. This branch is based on the fork trunked376cf, carries the complete fix, and bundles no upstream history.Local test environment. Two scripts in serial shard 1 fail on the machine used for this work -
tests/fm-tmux-agent-liveness.test.shandtests/fm-turnend-guard.test.sh. They are unrelated to this change and reproduce identically at the untouched baseed376cf; that lane passed in CI on the same base.Follow-ups, deliberately not in this change
tests/fm-remote-backlog-handoff.test.shat lines 139, 234, and 309, on the same kind of remote transactions, now thattests/lib.showns a wall-clock helper.docs/fm-test-portable-shards.mdhas drifted and needs refreshing from a green run's per-shard timing artifacts, which can only exist after this lands.