Repository navigation
test: check the lanes' first look by structure, not by a zero-second clock - #121
Merged
Merged
Conversation
…clock Two lanes-observe assertions required lane_wait_s == 0 and failed once on a GitHub-hosted macOS runner with 1 (run 40331626282, passed on rerun). lane_wait_s is WAITED when the lanes first said yes, and WAITED starts before the hook resolves its owner and worker log, so a loaded runner can cross a second boundary before the first look. The held row is written once, at the end of the hook's first pass, so the checks now say what they meant without a clock: - held: lane_wait_s is a number, not null (recorded at the first look) - admitted: lane_wait_s equals the held row's value, and is less than waited_s (not re-taken when the single lane let the job in) Mutation-checked: making the hook overwrite ADMIT_LANE_WAIT on every pass fails both admitted-row checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019W86FcknjPBqAPtVcnH8Ja
4 of 7 tasks
CI's Shell tests stopped at line 972 with "syntax error near unexpected
token `('": macOS's /bin/bash 3.2 cannot parse a case statement inside a
command substitution, though bash 5 (and bash -n there) accept it. The case
now sets a variable that the ok line compares.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019W86FcknjPBqAPtVcnH8Ja
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.
Summary
Two assertions in
hooks/tests/run.sh's "lanes observe" block requiredlane_wait_sto be exactly0. They failed once on a GitHub-hosted macOS runner with1(run 40331626282 on #120, a docs-only PR) and passed on a rerun.Why it flakes:
lane_wait_sisWAITEDat the moment the lanes first said yes.WAITEDstarts counting before the hook runsadmit_resolve_wait_ownerandadmit_resolve_worker_log. On a loaded runner that setup can cross a whole-second boundary before the first look, so the first look records1.Why not just allow
≤ 1: that would weaken the second check. The bug it guards against islane_wait_sbeing re-taken at admission instead of at the first look, and then it would equalwaited_s, which can itself be1.What it checks instead. The hook writes the
heldrow once, at the end of its first pass, which gives checks that don't depend on timing:heldlane_wait_s == 0lane_wait_sis a number, notnull, so it was recorded at the first lookadmittedlane_wait_s == 0lane_wait_s, andlane_wait_s < waited_s(not re-taken at admission, which is always at least one poll later)This is the same idea as #76, which times two other "promptly" checks from the hook's own events. The two PRs touch different hunks of
run.sh.Mutation-checked: I temporarily made the hook overwrite
ADMIT_LANE_WAITon every pass (dropping the[ -n "$ADMIT_LANE_WAIT" ] ||guard). Both admitted-row checks fail (expected '0', got '2'andnot before). The hook was restored and is not part of this diff.Second commit (
91f53b7): the first push put acase … esacinside$(…). macOS's/bin/bash3.2 can't parse that, so CI stopped at line 972 with a syntax error. Bash 5 andbash -non Linux accept it. Thecasenow sets a variable that theokline compares; no other$(caseexists in the repo.Platform tested on
macos-latest, and that's what caught the bash 3.2 parse error above. I couldn't get a bash 3.2 binary into this environment (GNU's FTP site and GitHub mirrors are blocked by its proxy), so macOS CI is the bash 3.2 check.maintoo (148 pass / 2 fail on main, 149 / 2 with this change). So they're environmental here and unrelated.node -v→ v22.22.0 (not used by this test)Checklist
cd dashboard && npm test(no dashboard change)bash -nOK;shellcheck --severity=warning hooks/tests/run.shclean (same 5 info-level notes asmain)gh)🤖 Generated with Claude Code
https://claude.ai/code/session_019W86FcknjPBqAPtVcnH8Ja