Repository navigation
Tests: fix the budget-health second-boundary flake; weekly optional shifted-clock run; lint on clock-reading helpers (#687) [PROTECTED: owner merges] - #774
Conversation
…time once, on a fixed clock The test set a credential's window to reset at at(600) (now + 600 s, in whole seconds) and later expected at(600) again. When a second ticked between the two reads (the budget's takes and the view run in between), the expected reset was one second later than the stored one and the test failed (#687). The reset time is now read once and reused, and the test runs on a fixed clock (node:test's mock Date) set 1 ms before a second boundary, which the setup crosses, so the case that flaked runs every time. Test (fails without): test/github-budget-health.test.ts "per credential" with the second at(600) read put back fails on the fixed clock every run: resetAt '...:22.000Z' stored, '...:23.000Z' expected. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
…days ahead A test whose result depends on today's date passes the day it is written and fails on main weeks later (#687). The new clock-shift workflow runs the whole unit suite with TEST_CLOCK_SHIFT=+45d, then +400d, once a week (and on a PR that changes the shift or the workflow, and by hand). It is required by nothing, and no ci.yml check changes. test/clock-shift.ts moves Date's "now" and the file times fs reports and takes by the same amount. The file times move too because the engine compares the clock with mtimes: with only Date moved, the intake purged a file written a second earlier as 14 days old, which is what failed the five intake tests #687 first named. The job loads it through NODE_OPTIONS, so the Node processes a test starts run at the same shift: test/github-budget.test.ts and test/real-chrome-lock.test.ts start Node children without the test preload, and hung when only the parent moved. test/isolate.ts is unchanged: the required shards never load the shift. Test (fails without): test/clock-shift.test.ts "a test that only passes before a date fails on the shifted clock": a fixture test with a 30-day window passes on the real clock and fails at +45d. The other tests check that Date and the file times move together, that a Node child inherits the shift, and that nothing moves without it. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
The flake #687 found came from a helper, `const at = (s) => Math.floor(Date.now() / 1000) + s`: two calls read two clocks, and a test that set a value with one and expected it with the other failed whenever a second ticked in between. test/no-clock-helpers.test.ts counts such helpers (declared on one line, `const f = (...) => ...` or `function f(...) { ... }`, whose body reads Date.now() or new Date()) in every file under test/. The 13 already there are allowed by file, each count exact and only ever lowered, as test/no-wall-clock-durations.test.ts does (#582); a file with none may never get one. Narrow on purpose: every wall-clock read in test/ is some 370 reads in over 100 files, and many are right (a stamp some while ago compared with a file's real mtime, which a pinned Date cannot move). Test (fails without): the lint's fixtures, source text fed to its function (no repo file planted): the helper that flaked and six of its kin are each found; a clock read once and reused, a fixed date, the clock passed by reference, another Date's now() and comments are not. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Evaluator verdict: CHANGES NEEDED (round 1/2)Reviewed head Acceptance criteria (restated #687)
Fail-without (measured)
Head checks (measured, in my own worktree, temporary HOME and state)
The weekly job hangs (measured)
Protection
Scope
Hygiene (measured)
CI (measured,
|
…t's Date.now mock works and can't recurse The weekly clock-shift job hung for its whole 2 h on #774 at +45d. test/dashboard-auth.test.ts mocks the clock with `const wallReal = Date.now.bind(Date); t.mock.method(Date, 'now', () => wallReal() + offset)`. The Proxy's now called RealDate.now() on every read, and the mock had replaced RealDate.now, so the mock called the shifted clock, which called the mock, until "Maximum call stack size exceeded". A plain mock didn't take effect either: Date.now() returned the mocked value plus the shift. The real clock is now captured once at load, and Date.now becomes an ordinary property holding the shifted clock. A test can replace it, the replacement takes effect, and anything it calls reaches the captured real clock. Test (fails without): test/clock-shift.test.ts "shifted: a test that mocks Date.now sees its mock, and a mock calling the clock it found there finishes": on the old helper, a fixed mock read 3888012345 instead of 12345, and the wallReal pattern failed with "Maximum call stack size exceeded". dashboard-auth.test.js at +45d with the job's env now gives 22/22. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
…h shifts On #774 one hung file held the +45d step until the job's 2 h timeout. The log was silent, and the +400d step never ran. Each step now runs with --test-timeout=600000, so a test that hangs is cancelled after 10 minutes and named with its file in the summary, and with --test-force-exit, so a file whose tests are done exits even with a handle left open. Each step also has its own 75-minute limit, so +400d runs whatever +45d does, and the job's limit is 170 minutes. Test (fails without): a scratch fixture test that never finishes and keeps an interval open. Plain `node --test` never ends: `timeout 60` exits 124. With the job's two flags (timeout 5 s), it exits 1 in about 5 s and names `hangs.test.mjs` "test timed out after 5000ms". Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Round 1 fixes (head
|
Evaluator verdict: PASSRound 2 of 2, head All local runs used my own worktrees with a temporary HOME, temporary state and held-out dirs, and a fake Round-1 required changes
Acceptance criteria (#687 as restated by the owner)
1. The
|
| Test | Kind | Cause (from the code) | What I recommend to the owner |
|---|---|---|---|
pr-hygiene-coordinator :20 "a stale PR…" and :60 "an abandoned PR…" (both shifts) |
Test-harness artefact. It isn't a product date bug and it isn't flaky. It fails every time when the shift is larger than the thresholds. | The builder's cause is right. src/coordinator/pr-hygiene.ts:72 and :87 read the commit time with git log --format=%ct, and the test harness's commits come from git (test/repro-state-graph/harness.ts:18), which stamps them with the real clock. The shifted hygieneNow therefore sees a base that is already 45 days old: merged_in 1 !== 0 on the "fresh" pass, and closed: 1 where skipped: 1 is expected. Measured: in a scratch worktree I gave the harness's git helper GIT_COMMITTER_DATE/GIT_AUTHOR_DATE set from Node's Date.now(). The file then passed 6/6 at +45d, at +400d and on the real clock. |
Don't mark it. The stale and abandon logic is exactly the date-dependent code this job exists to watch. Best fix: a follow-up issue to date the harness's commits from Node's clock (test-only, about 2 lines, measured above). libfaketime for git in the weekly job would also work (inferred), but it adds a system dependency to the job. Until one of those lands, keep the job red as a report. |
claude-account-script :138/:171 "a year" (+400d only) |
Test-harness artefact. It isn't a product date bug and it isn't flaky. It fails every time once the shift is over 65 days (365 < shift + 300). | The builder's cause is wrong. expiresAt isn't written by the shell. It's written by a Node child, node --input-type=module -e at scripts/setup/lib.sh:625 (Date.now() + 365 days). That child runs on the real clock because the test starts the script with env: { PATH, HOME } only (test/claude-account-script.test.ts:136), which drops NODE_OPTIONS and TEST_CLOCK_SHIFT. Measured: in a scratch worktree, passing those two variables through that env gave tests 22 / pass 22 at +400d. Without them it reproduces locally: not ok 1 … 'wl-alpha-agent: a year'. |
Don't mark it, and libfaketime won't fix it. An LD_PRELOAD/FAKETIME set in the environment would be stripped by the same env: {…} (inferred). Best fix: a follow-up issue to pass TEST_CLOCK_SHIFT and NODE_OPTIONS through in that test's spawn env when they're set (test-only, measured above). Until then, keep it red as a report. |
dashboard-switch-speed :328 "gated, 120 ms, 10 Mbit/s…" (+400d only) |
Flaky, already known. It isn't the date. | The same assertion, busy's Board after its fresh answer, is open issue #639. That issue records failures on unshifted ci.yml runs (2 in about 42 after 2026-10-18). This run's new diagnostic line shows board-instance-error: "busy: can't load its board: instance busy's dashboard isn't answering on 127.0.0.1:44639 (read ECONNRESET)" (measured: the job log). That fits the mechanism #639 infers: one failed fresh read drops the board already shown. It passed at +45d in the same job. The builder's "runner load" label (guess) is consistent. Inferred: not date-related, since the browser isn't shifted and the failure is a dropped loopback connection. |
Keep it red as a report, with nothing marked for the clock-shift job. Attach this log line to #639 as evidence: it's the first failure there that says which check failed and why. |
None of these is skipped or excluded in the PR (measured: the diff; no skip or todo was added. Each step's skipped 2 are the two existing real-agent-user tests that need WORKLANE_TEST_AGENT_USER). That complies with rule 10.
4. PR body
- Every claim is labelled, and the full-run numbers are cited from run 39204725103 (measured: read the body).
- The workflow is named as self-modification ("the CI workflows",
src/review.ts:40) and asengine(measured: both files read). - Two inaccuracies:
- (a) The
claude-account-scriptcause is wrong, as above. It matters, because it points the owner at libfaketime, which wouldn't fix that test. - (b) "(
test/clock-shift.ts, 82 lines)": the file has 90 (measured:wc -l). That's trivial.
- (a) The
--test-force-exithiding leaked handles is disclosed in the workflow comment but not in the body. I'd add one line saying so.
Protection (measured)
.github/workflows/clock-shift.ymlisengine(.worklane/review.yaml:7) and self-modification (src/review.ts, "the CI workflows").- The PR is marked protected:
[PROTECTED: owner merges]in the title,**Risk class:** protected, theneeds-ownerlabel, and real## What this can never doand## Compromised componentsections. - None of the four
test/files is inengine-guardsorSELF_MODIFICATION_PARTS. - "Can never do" holds:
- No required check loads the helper.
- No
src/orscripts/file changes. - No ci.yml job changes.
- The workflow has
permissions: contents: readand no secrets. - No regression for existing installs: not applicable, since nothing touches setup, firewall, sandbox, locks or visibility (inferred from the file list).
- Owner's real state (rule A): the new tests write only under
tmpdir()and run under the isolation preload (inferred from reading them, and all my runs used temporary dirs).
5. Overlap with #790 and merging with main (measured)
git merge-treeofd781239cwith5b8618e0is clean, and so is the merge with currentorigin/maince8a5c50(egress-firewall: drop each covered user's subordinate uids too, not only its own uid (#740) [PROTECTED: owner merges] #778 landed since).- Test: budget health reads the limited credential's reset time once #790 (
e599df00) makes the sameresetLimitedchange totest/github-budget-health.test.ts, and its hunks are identical to this PR's. Merging the two heads is clean, and the merged file equals this PR's version, so whichever lands second is a no-op on that file. - On
ce8a5c50merged with this head (built in a scratch worktree, not committed), the real clock gives 92/92 forclock-shift,no-clock-helpers,github-budget-health,isolate.test(its import changed on main),no-wall-clock-durations,dashboard-auth(changed on main, andwallRealis still at :344),update-checks,self-hosted-guardandci-batch. At +45d,clock-shift,github-budget-healthanddashboard-authgive 36/36.
6. Usual checks (measured)
- Fail-without: see section 1. The flake fail-without was measured in round 1, and the hang fixture in section 2.
- At the head:
npm run typecheckis clean, andnpm run check:denylistreports1277 files clean.clock-shift,no-clock-helpers,github-budget-health,isolate.test,no-wall-clock-durationsanddashboard-authpass 65/65. - All 5 commits have
Signed-off-by: Jayden Vasquez. There are no Co-Authored-By or vendor lines in the commits or the body, no new dependencies, and nothing under.claude/orsrc/.Closes #687is present. - CI (
gh pr checks 774): every required check passes:agentshield,dco,denylist,new-deps,push-gate,scan,test (ubuntu-latest, 22)andtest (ubuntu-latest, 24). All shards and every ci-slow job pass too. Only the optionalunit tests on a shifted clockfails, for the findings above. That's this PR's job reporting, not a defect in the PR.
Scope
Nothing outside #687. The pull_request path trigger is how the job gets checked, which is reasonable (inferred).
Before merge (body only, not blocking)
- Fix the
claude-account-scriptrow. The cause is a Node child (scripts/setup/lib.sh:625) on the real clock, because the test's spawnenvdropsNODE_OPTIONSandTEST_CLOCK_SHIFT. libfaketime wouldn't reach it. - Change "82 lines" to 90, and add one line saying
--test-force-exithides leaked handles in this job only, with the required shards still catching them.
Proposed follow-ups (owner's choice; none is in this PR)
- Date the repro harness's git commits from Node's clock (
pr-hygiene-coordinator). - Pass the shift env through in
claude-account-script's spawn. - Add this run's ECONNRESET line to dashboard-switch-speed: say which check failed; keep a shown Board when one fresh read fails (rare flake) #639.
|
Round-2 verdict (6260924401) body fixes, made by the orchestrator: claude-account-script's cause corrected (a Node child on the real clock because the test passes only PATH and HOME), clock-shift.ts is 90 lines, and a line that --test-force-exit hides leaked handles in the weekly job only. Body only; no code change. |
) Round 1 dropped --test-force-exit and relied on --test-timeout to end a file whose process doesn't exit. That holds on Node 22 only: on Node 24, which the job uses, a file with a handle left open is never ended, so the step would wait for its 75-minute limit with no summary (#774 again; the new open-handle test failed on the Node 24 shard). scripts/ci/clock-shift-run.mjs runs every test file in a `node --test` of its own, without force-exit, as many at once as node --test does. A file passes only when it exits 0 with a summary; one still running after --file-timeout (25 minutes in the job) is killed with its process group and named, and the run fails. The run ends with totals and the failed files. The tests read both steps from the workflow and require their exact command lines (no `|| true`, no continue-on-error on any step or the job), then run each step on fixtures: a test registered after an await runs; a file with an open handle, a failing test and a hanging test each fail the step, named; no test file fails it. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Intent: build #687 as the owner restated it on 2026-10-18. Three things: (1) fix the one real flaky test the investigation found (GitHub budget health "per credential", off by one second); (2) add a small clock-shift helper that one optional weekly CI job uses to run the unit suite 45 and 400 days ahead; (3) add a lint, narrow on purpose, against test helpers that read the wall clock on every call. There is no deadline.
Risk class: protected. It adds
.github/workflows/clock-shift.yml..github/**isenginein .worklane/review.yaml, and a CI workflow is also a self-modification path: "the CI workflows",src/review.ts:40. Everything else is undertest/, and none of it is in engine-guards or the self-modification parts.test/isolate.tsis unchanged.Closes #687
The issue's premise was wrong
The issue said five intake tests would start failing on main around 2026-11-02 and had to be fixed before 2026-11-01. They are not date bombs.
TEST_CLOCK_SHIFTplusNODE_OPTIONS=--import=…/dist/test/clock-shift.js):intake-coordinator,intake-githubandintake-repliergavetests 40 / pass 40 / fail 0at both+45dand+400d.What changed
test/github-budget-health.test.ts). The test set a credential's reset time withat(600)and later expectedat(600)again.atreadsDate.now()in whole seconds each time it's called, so a second ticking between the two calls broke the comparison. The fix reads the reset time once and reuses it. The test now also runs on a fixed clock (node:test'smock.timers, Date only) set 1 ms before a second boundary. The setup moves that clock 2 ms, across the boundary, so the case that used to flake now runs on every run.test/clock-shift.ts, 90 lines). It accepts only relative shifts (+45d,-2h,+1500ms; the instant form is gone). It moves Date's "now" and the file times fs reports and takes (stat/lstat/fstat and utimes/lutimes/futimes, in sync, callback and promise forms). It captures the realDate.nowonce, at load, and makesDate.nowan ordinary property holding the shifted clock. A test that mocksDate.nowtherefore sees its own mock, and a mock that calls the clock it found there can't recurse (round 1:test/dashboard-auth.test.ts'swallReal = Date.now.bind(Date)mock recursed without end and hung the first run). Only the new workflow loads it for the suite, throughNODE_OPTIONS. Because of that, the Node children a test starts run at the same shift. Those children were what madegithub-budgetandreal-chrome-lockhang when only the parent moved.test/isolate.tsis not edited, and the required shards never load the helper..github/workflows/clock-shift.yml, workflow "clock shift", job "unit tests on a shifted clock"). It runs every Monday at 05:17 UTC, can be started by hand, and also runs on a PR that changestest/clock-shift.tsor the workflow itself, which is why it runs on this PR. It runs the whole unit suite at+45dand then at+400d. Each run uses--test-timeout=600000 --test-force-exit, and each step has a 75-minute limit, so a hang is named with its file and the next step still runs. Nothing requires it: not the ruleset, not .worklane/config.yaml, not the updater's checks. Noci.ymljob or check name changes. I made it a separate workflow rather than adding aschedule:trigger to ci.yml, because that trigger would rerun every ci.yml job under the required names.--test-force-exitalso hides leaked handles in this job only; the required shards run without it, so a leak still hangs them there (measured by the round-2 review).test/no-clock-helpers.test.ts). It counts helpers declared on one line (const f = (…) => …orfunction f(…) { … }) whose body readsDate.now()ornew Date(). That is the shape of the helper that flaked. The 13 helpers already in test/ are allowed per file, each with an exact count that can only go down, the same ratchet test/no-wall-clock-durations.test.ts uses (Clocks: dashboard listing caches use a monotonic injectable clock, so a wall-clock step cannot keep or cut a cache window; lint new Date.now() durations in tests (v0.5.3) #582). A file with none can never gain one.Why the lint is this narrow (the full version was measured and rejected)
clock-stats.mjsover test/): the full version, "a test reads the wall clock without an injectable clock", would flag 372 reads in 109 of 429 test files. A narrower "mixes a fixed ISO date with a wall-clock read and doesn't pin Date" rule still flags 130 reads in 29 files.How it was verified
at(600)read back into the new test and kept the fixed clock. It failed:tests 10, pass 9, fail 1, withactual resetAt: '2026-10-19T03:06:22.000Z'andexpected resetAt: '2026-10-19T03:06:23.000Z'. With the fix:tests 10, pass 10, fail 0. Command:node --import ./dist/test/isolate.js --test dist/test/github-budget-health.test.js, with a temporary HOME, state and held-out dirs and a fakeclaudefirst on PATH.+45dwith "past the date". The other three tests in that file pass: Date and the file times move together, a Node child inherits the shift, and nothing moves without the preload or with an empty shift.athelper that flaked and six similar helpers are each found. A clock read once and reused, a fixed date, the clock passed by reference,RealDate.now()and comments are not.NODE_OPTIONS, at+45d,github-budget.test.jsgave 26/26 together withgithub-budget-health.test.jsandclock-shift.test.js, andreal-chrome-lock.test.jsgave 7/7.Date.nowmock (measured): I ran the new clock-shift.test.ts case on the old helper. It failed: a fixed mock read3888012345instead of12345, and thewallRealpattern failed withMaximum call stack size exceeded. With the fix, the file passes 5/5, anddashboard-auth.test.jsat+45dwith the job's env passes 22/22 and exits.node --testnever ends:timeout 60exits 124. With--test-timeout=5000 --test-force-exit, it exits 1 after about 5 s and nameshangs.test.mjs"test timed out after 5000ms".clock-shift,no-clock-helpers,no-wall-clock-durations,isolate(the "every way the suite runs preloads the isolation" check reads the new workflow line), and the workflow-reading testsci-slow-fixture-sha,ci-preview-done,ci-slow-trigger,ci-batch,update-checks,web-ui-required,testrun-tmpdir,self-hosted-guard,ci-new-deps,push-gate,release-workflowandstarter-ci. That first run had one failure: the lint countedRealDate.now()inside the helper itself. I narrowed the lint's pattern so a name that merely ends inDatedoesn't match, added a fixture for that case, and the rerun of the lint tests passed 8/8.npm run typecheck: clean (measured).npm run check:denylist:1277 files clean(measured).npm run check:dco -- origin/main HEAD:all commits signed off(measured).npm run check:push:3 commit(s) and 1 tip(s) clean(measured).pull_requesttrigger ran the workflow (run 39204725103, headd781239c).workflow_dispatchcan't start it yet, because GitHub only dispatches a workflow file that exists on the default branch. The job finished in 26m29s, and both steps ran to the end:+45d:tests 3543, pass 3539, fail 2, cancelled 0;+400d:tests 3543, pass 3537, fail 4, cancelled 0.gh pr checks 774). This job is red because of the findings below.Findings from the full shifted run (for the owner; no file is excluded or skipped)
test/pr-hygiene-coordinator.test.ts"a stale PR of the harness gets main merged in…" (line 20) and "an abandoned PR … is closed…" (line 60)test/claude-account-script.test.ts"switch pauses every instance, installs the token…" (a year, line 183)expiresAtis written by a Node child of the script (scripts/setup/lib.sh:625), which runs on the real clock because the test starts the script withenv: { PATH, HOME }only (test/claude-account-script.test.ts:136), droppingNODE_OPTIONSandTEST_CLOCK_SHIFT; the test compares it with Node's shiftedDate.now() + 300 days(measured by the round-2 review: passing both variables through gives 22/22 at +400d). Faking time for the shell alone would not fix it.test/dashboard-switch-speed.test.ts"gated, 120 ms, 10 Mbit/s: … under 300 ms (median of 5)" (line 328)card count: 0on switch 5 of 5. It passed at +45d in the same job, so it looks like runner load rather than the date (guess).All three come from the helper's known limit, which is that only Node processes move, plus one timing test. None of them is a test whose result depends on today's date on a real machine (inferred). Options, for the owner to choose: give those tests' git and shell steps a fake time (for example libfaketime in the weekly job only), mark the stale-PR tests as "needs the whole machine's clock", or keep the job red as a report. This PR picks none of them (rule 10: nothing is skipped to get green).
New dependencies
None.
What this can never do
clock-shift.ymlsetsTEST_CLOCK_SHIFTandNODE_OPTIONSfor the suite. In the required shards,test/clock-shift.test.tssets them only for the probe and fixture child processes it starts, never for itself or another test file. The helper does nothing whenTEST_CLOCK_SHIFTis unset or empty, andtest/isolate.tsdoesn't import it (inferred from the diff; the "not shifted" test measures that a preloaded process withTEST_CLOCK_SHIFTset but without the job'sNODE_OPTIONSreads the real clock).src/orscripts/file changes, and the helper compiles todist/test/, which package.jsonfilesdoesn't publish (dist/src,dist/web, …) (measured: read package.jsonfiles).Compromised component
permissions: contents: read, uses no secrets, and its PR trigger is limited to two paths. A PR that changes the helper runs the suite with it loaded, asci.ymlalready runs any PR's test code.NODE_OPTIONSitself (for example, push-gate-hook's frozen-time import): in the weekly job, that test replaces the job's value for its own child, so that child runs on the real clock while the parent is shifted. That could produce a false red in the optional job only, never in a required check (inferred). On this PR's full run,push-gate-hookpassed at both shifts (measured: it isn't among the failures).clock-shift.ymlchanges what this CI runs, so it staysengineand self-modification, and the owner merges it. It holds no secrets and can't push.