Skip to content

test: make the midpoint budget case event-driven and drop a duplicate suite - #6

Merged
elkaix merged 1 commit into
mainfrom
fix/ci-timing-flakes
Aug 12, 2026
Merged

test: make the midpoint budget case event-driven and drop a duplicate suite#6
elkaix merged 1 commit into
mainfrom
fix/ci-timing-flakes

Conversation

@elkaix

@elkaix elkaix commented Aug 12, 2026

Copy link
Copy Markdown
Member

Fixes the CI failure on main at 37b53fd (run 31552000770), where two suites went red.
Neither was a product defect — the identical content had passed as PR #5's check minutes
earlier.

liveness.sht14_midpoint_warning_continues_same_job

The case required the job to complete inside a window of (ceiling/2, ceiling), and then
spent seven serialized poll cycles crossing it — each spawning a node process — against
an 8-second ceiling. On a loaded runner the accumulated poll cost passed the ceiling, the
loop cancelled at its deadline exactly as designed, and the case reported rc=11 want 0.

This is intermittent, not new: the same case failed on c2f2ea9 nine days ago and passed on
b3adebc. Enlarging the numbers cannot fix it — the window stays proportionally
(ceiling/2, ceiling) under any values.

Fix: make completion causally follow the warning instead of racing it. The fixture
already supports MAESTRO_TEST_JOB_TERMINAL_FLAG (tests/job-lock.sh uses it), which
reports the job as running until a named file exists. The case now waits for the halfway
warning, then creates the file, so completion is one poll cycle after the warning rather
than seven. The ceiling is a single variable that the assertion pattern derives from, so
the two cannot drift. All four assertions are unchanged.

Counterexample, made deterministic with the fixture's MAESTRO_TEST_STATUS_HANG (delays
each status call, i.e. "a loaded runner" on demand):

Before After
MAESTRO_TEST_STATUS_HANG=1 rc=11 want 0 — the exact CI symptom passes

shared-git-dir.sh — duplicate lease suite

It re-ran the entire lease suite that tests/run.sh had already run and passed in the
same job, piping it through tail -1 so a failure inside it was undiagnosable — which is
why the original CI log could not say which case broke. No added coverage, ~55s of CI time,
and a doubled chance that any timing-sensitive lease case fails the build. Every other check
in the file stays.

42.87s → 2.07s.

Wait bounds (preventive, labelled as such)

Nine event waits go from 5s to 30s across liveness.sh, lease.sh, and
provenance-edge.sh. These fix nothing observed — they are hardening. Each loop breaks the
moment its event arrives, so a larger bound costs nothing on a healthy host and removes a
false negative on a loaded one; a genuinely absent event still fails, just later. Only loops
that break on success and report "never happened" were widened; no settling delay before a
negative assertion was found.

t19 — deliberately unchanged

t19 fails only in the Codex sandbox, with
MAESTRO_LOCK: process start identity unavailable for pid=.... There ps -o lstart= returns
nothing, so the contender fails closed instead of entering the wait and never prints the
string the case polls for. No bound can fix that. It passes on the GitHub runner
(PASS waiting contender TERM never cancels the active owner in the very run being fixed)
and on developer hosts.

Verification

Full suite under a fabricated empty HOME with /bin/bash 3.2 — what the runner actually
provides, and the environment in which bash tests/lease.sh alone would have given a false
green:

SUMMARY: 18 passed, 0 failed

… suite

CI failed on 37b53fd with two suites red, neither a product defect. The same
content had passed as PR #5's check minutes earlier.

t14_midpoint_warning_continues_same_job asked for a job to complete inside a
window of (ceiling/2, ceiling) and then spent seven serialized poll cycles
crossing it, each spawning a node process, against an 8-second ceiling. Every
unit of runner slowness ate the margin until the loop cancelled at its deadline
— behaving exactly as designed — and the case read that as failure. It had
already failed this way on c2f2ea9 nine days earlier.

Drive completion from the event instead of from the clock. The fixture's
MAESTRO_TEST_JOB_TERMINAL_FLAG, which tests/job-lock.sh already uses, reports
the job as running until a named file exists. The case now waits for the halfway
warning, then creates that file, so completion is causally after the warning and
one poll cycle away rather than seven. The ceiling lives in a single variable the
assertion pattern derives from, so the two can no longer drift apart. All four
assertions are unchanged.

tests/shared-git-dir.sh re-ran the entire lease suite that tests/run.sh had
already run and passed in the same job, piping it through `tail -1` so a failure
inside it could not be diagnosed at all. It added no coverage, cost ~55s, and
doubled the chance that any timing-sensitive lease case failed the build. Its
other checks stay; the re-run goes. 42.87s to 2.07s.

The remaining changes widen nine event waits from 5s to 30s across liveness,
lease, and provenance-edge. These are preventive, not fixes for anything
observed: each loop breaks as soon as its event arrives, so a larger bound costs
nothing when the host is healthy and removes a false negative when it is not. A
genuinely absent event still fails, only later.

t19 is deliberately unchanged. It fails in the Codex sandbox because ps reports
no process start identity there, so the contender fails closed instead of
waiting; it passes on the GitHub runner and on developer hosts.

Verified with the full suite under a fabricated empty HOME and /bin/bash 3.2,
which is what the runner actually provides: 18 passed, 0 failed.
@elkaix
elkaix merged commit 9f0d225 into main Aug 12, 2026
1 check passed
@elkaix
elkaix deleted the fix/ci-timing-flakes branch August 12, 2026 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant