test(cli): make the #4873 exit-code pin deterministic — literal duration, no wall clock (#6266) - #6481
Merged
Conversation
…ion, no wall clock (#6266) The case read the duration off a live `createTimer()`: const durationMs = timer.elapsed() + 531; expect(durationMs & 0xff).toBe(19); which holds only while `elapsed()` returns exactly 0 — and an `await emitJson(...)` sits between the two statements. One millisecond turns 531 into 532 and `& 0xff` from 19 into 20, the exact number CI reported. The clock supplied a failure mode and no coverage: neither the type rejection nor Node's 8-bit truncation is a function of how long anything took. The duration is now the literal 531. The guard keeps bearing load — both `@ts-expect-error` directives are untouched, and the runtime half still asserts that the duration reaches the exit-code slot verbatim; the truncation line is joined by an explicit statement of why 19 is a defect (it is not one of the two codes `CliExitCode` defines). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017uFVNMmTxLpmfQYiuKM1Yx
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckNo hand-written docs reference the 0 changed package(s). ✅ |
os-project-manager
marked this pull request as ready for review
August 8, 2026 01:49
This was referenced Aug 8, 2026
This was referenced Aug 8, 2026
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.
Fixes #6266
What was wrong
packages/cli/src/utils/format.exit-code.test.tsread its duration off a live wall clock and then asserted a hard-coded truncation of it:531 & 0xffis 19,532 & 0xffis 20,533 & 0xffis 21. The assertion therefore held only whiletimer.elapsed()returned exactly 0 — and anawait emitJson(...)plus module work sits between the two statements. One millisecond ticking on a busy runner turned the case red, which is why it kicked three unrelated PRs in 24h (#6248, #6375, #6429 — the last from a merge-queue generation) through CI's merge ref.Mechanism chosen, and why
The issue thread carried two candidate shapes. I chose (a) the literal, and folded in the useful half of (b):
const durationMs = 531;. Zero clock reads, so the truncation assertion is deterministic by construction rather than by luck.process.exitCode === durationMscompares against the same variable whatever the clock says), but it leaves aDate.now()read in a case where nothing under test is a function of elapsed time. Neither the type rejection nor Node's 8-bit truncation depends on how long anything took, so the clock supplied a failure mode and no coverage. Keeping it invites the next edit to assert something about that variable's value and re-open exactly this bug.So: (a)'s determinism, plus (b)'s framing — the property assertion and the truncation illustration are both kept, I only removed the wall clock that fed them.
The #4873 guard still bears load
Nothing was weakened to silence the flake. All three load-bearing parts are intact, and I verified each by breaking it:
@ts-expect-errordirectives (the lasting pin)CliExitCodetonumberinformat.tserror TS2578: Unused '@ts-expect-error' directive.at both lines 114 and 119emitTexttoprocess.exitCode = 1AssertionError: expected 1 to be 531AssertionError: expected 21 to be 19at the& 0xffline — the same signature CI reported asexpected 20 to be 19That third row is worth reading closely: with the millisecond forced, only the
& 0xffline went red;expect(process.exitCode).toBe(durationMs)stayed green. The clock-dependent assertion was the one line, and it is the one line this PR changes.I also added one assertion naming why 19 is a defect rather than a curiosity — it is not one of the two codes
CliExitCodedefines, so a scripted caller read a successful run as a failure it could not name. In fairness both& 0xfflines are now arithmetic on a literal: they state the truncation rather than measure it. That is deliberate and is what the previous version was too, minus the coin flip.Determinism evidence
@objectstack/clisuite: 91 files, 928 tests, 928 passed. (The CI run that reported this flake was 927/928 with this case as the sole failure.)Scope
Test-only, one file. No changeset, per the ruling on the issue — an empty changeset stalls the release pipeline (#4898), so the
skip-changesetlabel carries this instead.Generated by Claude Code