fix(tests): stop three suites deciding on host properties instead of behavior - #1907
Open
Ndidi wants to merge 1 commit into
Open
fix(tests): stop three suites deciding on host properties instead of behavior#1907Ndidi wants to merge 1 commit into
Ndidi wants to merge 1 commit into
Conversation
…behavior fm-bootstrap, fm-pi-watch-extension and fm-remote-backlog-handoff each failed on an untouched checkout. All three failed for the same reason: the case proved a property of the code by borrowing an unpinned property of the machine, so its verdict reported how the host was provisioned rather than what the code did. - fm-bootstrap asserted `MISSING: orca` while relying on the host having no binary of that name. /usr/bin/orca is the GNOME screen reader, unrelated to the Orca runtime backend, so bootstrap correctly found an orca and reported nothing. This failed for every desktop Linux user with the accessibility tool installed and passed for everyone else. - fm-pi-watch-extension gave the OpenCode plugin a fixed 120ms to refuse a foreign session lock. That check walks the process ancestry with one `ps` per hop, so its cost scales with how deeply the process is nested: 172ms at eight hops here, ~40ms on a shallow container. The sleep expired mid-check, and the arming phase then inherited the unfinished verdict and never armed. - fm-remote-backlog-handoff symlinked the host's tasks-axi into the fixture's bin directory. A packaged JS CLI resolves its own package relative to $0, so pnpm's shim looked for its package beside the symlink, failed to load, and the receive step reported tasks-axi incompatible. Two shared helpers now own the patterns that were being open-coded or missed. fm_fake_absent_env writes a BASH_ENV shim that makes a tool genuinely absent, which no fakebin entry can express, and replaces the hand-rolled copies the git and jq cases already carried. fm_fake_passthrough exposes a real host tool under a fixture path without breaking a $0-relative launcher. The same shape is swept out of the rest of fm-bootstrap: the herdr/zellij/cmux gate cases and the bundled-cmux case assumed those names were absent from PATH, and the no-tmux premise would have gone silently vacuous on any host shipping tmux rather than failing. Two further pre-existing failures in the same suites are fixed alongside them. The hung-successor cases gave a successor 250ms to become ready, but both plugins spawn the arm child through `bash -lc` and a login shell alone costs 200-320ms sourcing profiles, so the child was killed before it could record that it ran; the budget now clears that startup while staying under the restoration cases' own 5s wait. The remote handoff's three marker waits allowed 5s for steps that fork bash, node and tasks-axi, which only decides how long a genuine hang takes to report and now sits well above real startup.
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.
Three of firstmate's own suites failed on an untouched checkout at
70aeba8, each at the same assertion every run.All three failed for one reason: the case proved a property of the code by borrowing an unpinned property of the machine, so its verdict reported how the host was provisioned rather than what the code did.
The three causes
fm-bootstrap-backend=orca should require only the Orca-specific missing tool.The case asserted
MISSING: orcawhile relying on the host having no binary of that name./usr/bin/orcais the GNOME screen reader, entirely unrelated to the Orca runtime backend, so bootstrap correctly found anorca, correctly reported nothing missing, and the assertion read empty output.This failed for every desktop Linux user with the accessibility tool installed and passed for everyone else.
fm-pi-watch-extension-OpenCode watch plugin must arm only when this session owns the fleet lock.Not a product defect.
The case gave the plugin a fixed 120ms to refuse a foreign session lock.
That check walks the process ancestry with one
psper hop, so its cost scales with how deeply the process is nested: measured at 172ms across eight hops to pid 1 here, and roughly 40ms on a shallow container where node sits two or three hops down.The sleep expired mid-check, and because the plugin coalesces a launch already in flight, the arming phase then inherited the unfinished
read-onlyverdict and never armed.Awaiting the settled verdict instead, the plugin refuses and then arms correctly.
fm-remote-backlog-handoff-remote atomic receipt did not deliver ios-a before the dropped acknowledgement.The fixture symlinked the host's
tasks-axiinto its own bin directory.A packaged JS CLI resolves its own package relative to
$0- pnpm's global shim loads$(dirname "$0")/global/...- so the symlink sent it looking for its package beside the symlink.It failed to load, the receive step reported
tasks-axiincompatible, and the handoff assertion failed for a reason unrelated to the handoff.What changed
Two shared helpers in
tests/lib.shnow own the patterns that were being open-coded or missed:fm_fake_absent_envwrites aBASH_ENVshim that makes a tool genuinely absent.No fakebin entry can express absence: adding a stub makes
command -vsucceed, and omitting the name only uncovers whatever the host installed.This replaces the hand-rolled copies the
gitandjqcases already carried, so there is one owner rather than three.fm_fake_passthroughexposes a real host tool under a fixture path via anexecwrapper, which keeps$0pointing at the resolved command so a launcher's own path arithmetic still works.The same shape is swept out of the rest of
fm-bootstrap.The herdr/zellij/cmux gate cases and the bundled-cmux case all assumed those names were absent from PATH, and the no-tmux premise would have gone silently vacuous on any host shipping tmux rather than failing - the buggy
TOOLSlist those cases exist to catch would simply have stopped being detected.fm-remote-backlog-handoffalso now gates on the sametasks-axicompatibility verdict the receive step applies, so an unsupported host skips honestly instead of reporting a handoff failure that is really a version floor.Two further pre-existing failures fixed alongside
Both were confirmed present on the pristine baseline before any change, and both are the same family.
Pi must deliver the actionable wake after bounded hung-successor recoveryfailed 3/3 runs on untouched70aeba8.Both watcher plugins spawn the arm child through
bash -lc, and a login shell sources the system and user profiles before the arm script's first line runs - 200-320ms measured on this host.The 250ms readiness budget was therefore racing shell startup, and the child was killed before it could append the row proving it ran, which reads as the plugin having skipped a retry.
The budget now clears that startup while staying under the restoration cases' own 5s wait for the wake prompt; those two bounds are coupled, and a value above roughly 1500 makes the case sample the arm log mid-restoration and see one row too few.
That coupling is now stated at the constant.
first serialized handoff never reached receiptwas masked: the suite previously died at the atomic-receipt assertion above and never reached it.Its three marker waits allowed 5s for background steps that fork bash, node and
tasks-axi.These bounds only decide how long a genuine hang takes to report, so they cost nothing on the happy path and are now set well above real startup.
Verification
Two full rounds of all three suites, both entirely green:
Each cause was also confirmed by direct measurement rather than inference:
/usr/bin/orcaidentified as the screen reader; the ancestry walk timed at 172ms over eight hops with the plugin arming correctly once its verdict is awaited; the pnpm shim reproduced failing to load through a relocating symlink; andbash -lc 'true'timed at 0.20-0.32s.shellcheck0.11.0 (the pinned version, overbin/fm-lint.sh's exact file set) is clean on all four changed shell files, andbin/fm-doc-audience-check.shpasses (65 surfaces, 212 local links).Known and deliberately not addressed
bin/fm-test-run.sh --check-coverageis still broken on this baseline, comparingLC_ALL=Csorted lists under ambient collation.That is PR #1885, which has not merged yet -
origin/mainis still at70aeba8- so it is left alone.fm-pi-watch-extensionretains residual load sensitivity beyond the case fixed here.On a box running at load 60-100 on 16 cores, assertions untouched by this PR intermittently fail (
Pi session transitions must rearm through an explicit generation owner,OpenCode watch plugin must not treat external healthy output as an owned arm); the suite passes cleanly once the machine settles.That suite carries fifteen hand-tuned wall-clock bounds, several coupled to each other, and making it genuinely load-independent means retuning them as one coherent change rather than adjusting them individually - doing that piecemeal is exactly how a single constant change here broke a different assertion mid-review.
Flagging it as separate work rather than half-tuning it.