Second, distinct flake from #342. That one is a timing substrate in src/orchestrator/factory.test.ts; this is a wall-clock assertion in src/node/tailscale-preview.test.ts. Filing separately so neither gets used to wave the other through.
Observation
main @ 8d6b89a2 (the #349 merge), run 32668344048:
FAIL src/node/tailscale-preview.test.ts > TailscalePreviewManager >
bounds a slow readiness probe by one wall-clock deadline
AssertionError: expected 492 to be less than 250
4 of 5 jobs green; only package failed, on that one assertion.
Why this is a flake and not the merge
I checked this specifically because I merged #322 earlier today on a stale base and it did break main, so a red immediately after my merge deserved suspicion rather than a shrug. This one is not that.
Why it is worth fixing rather than tolerating
A wall-clock bound on a busy runner is a coin toss that lands on whoever merges next. We already have #342 doing the same thing, and I have now watched two separate authors spend real time proving an inherited red was not theirs. That is how a repo trains people to ignore failing checks — and the cost lands the day a red check is real.
Deliverable
Make the deadline assertion insensitive to runner load. Options in preference order:
- Fake timers. The test is about whether a deadline bounds the probe, not about how many real milliseconds elapse. Control the clock and assert the bound fired, not that wall time was under 250ms.
- Assert ordering/causality — the probe was aborted by the deadline — rather than duration.
- Only if neither is possible: widen the bound and say in a comment what load assumption it encodes.
Option 3 alone is the weakest: it moves the coin toss rather than removing it.
A must-fire/must-not-fire pair would settle it: with the deadline present the probe aborts, with it removed it does not — and neither assertion should depend on how fast the machine is.
Second, distinct flake from #342. That one is a timing substrate in
src/orchestrator/factory.test.ts; this is a wall-clock assertion insrc/node/tailscale-preview.test.ts. Filing separately so neither gets used to wave the other through.Observation
main@8d6b89a2(the #349 merge), run32668344048:4 of 5 jobs green; only
packagefailed, on that one assertion.Why this is a flake and not the merge
aadd633epassed all 5 jobs. The squash ontomainchanged no source.src/orchestrator/factory.ts,factory.test.ts, andtypes.ts. Nothing undersrc/node/.492 < 250is a scheduler/load measurement, not a logic outcome. A test that asserts elapsed real time fails whenever the runner is busy, which is exactly what a shared CI box is.I checked this specifically because I merged #322 earlier today on a stale base and it did break
main, so a red immediately after my merge deserved suspicion rather than a shrug. This one is not that.Why it is worth fixing rather than tolerating
A wall-clock bound on a busy runner is a coin toss that lands on whoever merges next. We already have #342 doing the same thing, and I have now watched two separate authors spend real time proving an inherited red was not theirs. That is how a repo trains people to ignore failing checks — and the cost lands the day a red check is real.
Deliverable
Make the deadline assertion insensitive to runner load. Options in preference order:
Option 3 alone is the weakest: it moves the coin toss rather than removing it.
A must-fire/must-not-fire pair would settle it: with the deadline present the probe aborts, with it removed it does not — and neither assertion should depend on how fast the machine is.