diff --git a/package.json b/package.json index 1296ea57e..b77c78a57 100644 --- a/package.json +++ b/package.json @@ -60,6 +60,7 @@ "db:migrations:check": "tsx scripts/check-migrations.ts", "db:migrations:immutable:check": "tsx scripts/check-released-migrations-immutable.ts", "turbo-inputs:check": "tsx scripts/check-turbo-typecheck-inputs.ts", + "fixture-clock-races:check": "tsx scripts/check-fixture-clock-races.ts", "workspace-dep-ranges:check": "tsx scripts/check-workspace-dep-ranges.ts", "db:schema-drift:check": "tsx scripts/check-schema-drift.ts", "actionlint": "node --experimental-strip-types scripts/actionlint.ts", @@ -143,7 +144,7 @@ "test:smoke:browser:install": "playwright install chromium", "test:smoke:browser": "node --experimental-strip-types scripts/smoke-ui-browser.ts", "pretest:ci": "npm run check-node-version", - "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:migrations:immutable:check && npm run turbo-inputs:check && npm run workspace-dep-ranges:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:contract-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run control-plane:contract:check && npm run control-plane:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run server-manifest:check && npm run dead-source-files:check && npm run dead-exports:check && npm run publishable-deps:check && npm run checkers-wired:check && npm run regate-sort-key:check && npm run maintainer-associations:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run release-linked-versions:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run mcp:client-config:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", + "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:migrations:immutable:check && npm run turbo-inputs:check && npm run workspace-dep-ranges:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:contract-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run control-plane:contract:check && npm run control-plane:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run server-manifest:check && npm run dead-source-files:check && npm run dead-exports:check && npm run publishable-deps:check && npm run fixture-clock-races:check && npm run checkers-wired:check && npm run regate-sort-key:check && npm run maintainer-associations:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run release-linked-versions:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run mcp:client-config:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", "test:release": "npm run test:ci && npm run changelog:check", "test:release:mcp": "npm run test:ci", "test:watch": "vitest", diff --git a/scripts/check-fixture-clock-races.ts b/scripts/check-fixture-clock-races.ts new file mode 100644 index 000000000..5347923dd --- /dev/null +++ b/scripts/check-fixture-clock-races.ts @@ -0,0 +1,126 @@ +#!/usr/bin/env node +// A test fixture must not re-read the clock per timestamp (#9955). +// +// THE BUG THIS CATCHES, in the exact shape that reached CI. `test/unit/queue-trends.test.ts` had: +// +// function atDaysAgo(daysAgo: number) { return new Date(Date.now() - daysAgo * 864e5).toISOString(); } +// +// Every call re-read the clock, so two timestamps in ONE fixture were mutually inconsistent by however long +// elapsed between the calls. The code under test anchors its window on the newest snapshot: +// +// targetMs = latestMs - windowDays * day +// baseline = newest snapshot with fetchedAt <= targetMs +// +// `atDaysAgo(0)` evaluated at T0 and `atDaysAgo(7)` a moment later at T1 put the "7 days ago" row at T1-7d -- +// NEWER than the target T0-7d. No baseline, every window "unavailable", assertion fails. It passed only when +// both calls landed in the same millisecond. Reproduced deterministically with a 2ms offset. +// +// WHY IT IS WORTH A CHECKER. Reviews here are one-shot for everyone but the maintainer. A false red on a +// contributor PR is not a re-run away from fine -- it auto-closes correct work, and the contributor cannot +// reopen. A fixture that fails on timing alone is therefore a gate-correctness problem, not CI noise. It +// surfaced on #9950, whose only changed file was a GitHub workflow. +// +// WHAT IS ALLOWED. Reading the clock live is correct when the passage of time IS the thing under test -- +// a polling `waitFor(predicate)`, or a lock-expiry helper comparing against `Date.now()`. Those take no +// offset parameter to project a fixture time from, which is exactly how this check tells them apart: it +// only reports helpers that take an OFFSET and derive a timestamp from a freshly-read clock. +// +// THE FIX is always the same: capture one instant per file and derive every fixture timestamp from it. +import { readFileSync, readdirSync } from "node:fs"; +import { join } from "node:path"; +import { fileURLToPath, URL } from "node:url"; + +export type FixtureClockRace = { file: string; helper: string; calls: number }; + +/** Helper declarations of the racy shape: takes an offset parameter AND computes from a fresh `Date.now()`. */ +const OFFSET_HELPER_RE = /(?:^|\n)\s*(?:export\s+)?(?:function\s+(\w+)\s*\(([^)]*)\)|const\s+(\w+)\s*=\s*\(([^)]*)\)\s*(?::[^=]+)?=>)/g; + +/** + * PURE: every offset-taking fixture helper in `source` that derives a timestamp from a freshly-read clock and + * is called more than once. One call cannot be inconsistent with itself, so a single-use helper is not a race + * -- it takes two timestamps from two clock reads for the bug to exist. + */ +export function findFixtureClockRaces(file: string, source: string): FixtureClockRace[] { + const races: FixtureClockRace[] = []; + for (const match of source.matchAll(OFFSET_HELPER_RE)) { + const name = match[1] ?? match[3]; + const params = match[2] ?? match[4]; + if (!name || !params || params.trim() === "") continue; + // The helper's body: from its declaration to the next blank line at column 0, which is where every + // top-level declaration in these files ends. Deliberately crude -- a false NEGATIVE here just means the + // check misses one, while a parser dependency for a lint rule would be worse. + const start = match.index ?? 0; + const body = source.slice(start, start + 400); + if (!/Date\.now\(\)/.test(body)) continue; + // The offset must come FROM THE PARAMETER. `Date.now() - days * 86_400_000` projects a fixture time and is + // the racy shape; `Date.now() + 60 * 60_000` (a token expiry an hour out) and `expiresAt <= Date.now()` + // (a liveness check) are not -- neither derives its offset from an argument the caller varies per call. + // Without this the check flags any helper that merely happens to mention the clock, and a checker that + // cries wolf gets muted, which is worse than not having one. + const parameterNames = params + .split(",") + .map((parameter) => /(\w+)\s*[:=)]?/.exec(parameter.trim())?.[1]) + .filter((parameter): parameter is string => Boolean(parameter)); + const projectsFromParameter = parameterNames.some((parameter) => + // The span between the operator and the parameter must stay inside ONE arithmetic expression: `,`, `)` + // and `}` end it. Allowing them let `Date.now() + 60 * 60_000, body` reach an unrelated `body` + // identifier two fields later and report a token-expiry helper as a fixture race. + new RegExp(`Date\\.now\\(\\)\\s*[-+][^;\\n,)}]*\\b${parameter}\\b`).test(body), + ); + if (!projectsFromParameter) continue; + // Count CALL SITES. `function name(` itself matches the call shape and must be subtracted; the arrow form + // `const name = (` does not, and subtracting there undercounted every arrow helper by one -- which silently + // exempted the exactly-two-call case, the smallest set that can actually race. + const declarationLooksLikeACall = Boolean(match[1]); + const calls = [...source.matchAll(new RegExp(`\\b${name}\\s*\\(`, "g"))].length - (declarationLooksLikeACall ? 1 : 0); + // One report per helper: the declaration regex can match a single helper twice (its `function` and + // arrow forms overlap on some shapes), and reporting the same name twice reads as two problems. + if (calls >= 2 && !races.some((race) => race.helper === name)) races.push({ file, helper: name, calls }); + } + return races; +} + +function walk(dir: string, out: string[]): void { + let entries: ReadonlyArray<{ name: string; isDirectory(): boolean }>; + try { + entries = readdirSync(dir, { withFileTypes: true }); + } catch { + return; + } + for (const entry of entries) { + const path = join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name !== "node_modules") walk(path, out); + // The checker-testing files (check-*-script.test.ts) embed the very pattern under test inside fixture + // STRING LITERALS, exactly as check-turbo-typecheck-inputs.ts's own fixtures do. Those are not real + // fixtures and flagging them would make this checker permanently red on its own test suite. + } else if (entry.name.endsWith(".test.ts") && !/^check-.*-script\.test\.ts$/.test(entry.name)) out.push(path); + } +} + +function main(): void { + const root = join(fileURLToPath(new URL(".", import.meta.url)), ".."); + const files: string[] = []; + walk(join(root, "test"), files); + + const races = files.flatMap((file) => findFixtureClockRaces(file.slice(root.length + 1), readFileSync(file, "utf8"))); + if (races.length > 0) { + console.error("Fixture helpers re-read the clock per timestamp, which is a race:\n"); + for (const race of races) console.error(` ${race.file} ${race.helper}() (${race.calls} calls)`); + console.error( + "\n Two timestamps built from two Date.now() reads are mutually inconsistent by the time elapsed between\n" + + " them. Where the code under test compares them against a boundary derived from one of them, a single\n" + + " millisecond flips the result -- a false-positive red CI, which under one-shot review auto-closes\n" + + " correct contributor work.\n\n" + + " Fix: capture one instant per file and derive every fixture timestamp from it:\n" + + " const FIXTURE_NOW_MS = Date.now();\n" + + " const daysAgo = (d: number) => new Date(FIXTURE_NOW_MS - d * 86_400_000).toISOString();\n\n" + + " Reading the clock live is still correct where the passage of time IS under test (a polling waitFor,\n" + + " a lock-expiry check) -- those take no offset to project from, and are not reported.", + ); + process.exit(1); + } + console.log(`fixture-clock-races: OK — ${files.length} test files, no offset helper re-reads the clock.`); +} + +if (process.argv[1]?.endsWith("check-fixture-clock-races.ts")) main(); diff --git a/test/unit/burden-forecast.test.ts b/test/unit/burden-forecast.test.ts index 4f7b7ffc3..2c65a4719 100644 --- a/test/unit/burden-forecast.test.ts +++ b/test/unit/burden-forecast.test.ts @@ -228,8 +228,14 @@ function repoFixture(fullName: string): RepositoryRecord { } as RepositoryRecord; } +// #9955: anchored to ONE instant for the whole file. Re-reading Date.now() per call makes timestamps +// within a single fixture mutually inconsistent by however long elapsed between the calls, which is a +// real race wherever the code under test compares them against a boundary derived from one of them -- +// it cost a false-positive red CI on #9950, on a PR that changed nothing but a workflow file. +const FIXTURE_NOW_MS = Date.now(); + function daysAgo(days: number): string { - return new Date(Date.now() - days * 86_400_000).toISOString(); + return new Date(FIXTURE_NOW_MS - days * 86_400_000).toISOString(); } function issue(repoFullName: string, number: number, title: string, overrides: Partial = {}): IssueRecord { diff --git a/test/unit/check-fixture-clock-races-script.test.ts b/test/unit/check-fixture-clock-races-script.test.ts new file mode 100644 index 000000000..cdcce8f79 --- /dev/null +++ b/test/unit/check-fixture-clock-races-script.test.ts @@ -0,0 +1,108 @@ +// The fixture-clock-race checker must catch the real shape and stay quiet on the legitimate ones (#9955). +// +// A checker that cries wolf gets muted, and a muted checker is worse than no checker -- so the negative cases +// here matter as much as the positive one. Reading the clock live is CORRECT wherever the passage of time is +// itself under test; the distinguishing property is whether the helper projects a fixture timestamp from an +// offset its caller varies. +import { describe, expect, it } from "vitest"; + +import { findFixtureClockRaces } from "../../scripts/check-fixture-clock-races"; + +describe("findFixtureClockRaces (#9955)", () => { + it("REGRESSION: catches the exact queue-trends shape that reached CI", () => { + // Verbatim the helper that produced "expected 'unavailable' to be 'ready'" on #9950 -- a PR whose only + // changed file was a GitHub workflow. + const source = ` +function atDaysAgo(daysAgo: number): string { + return new Date(Date.now() - daysAgo * 24 * 60 * 60 * 1000).toISOString(); +} +const a = atDaysAgo(0); +const b = atDaysAgo(7); +`; + expect(findFixtureClockRaces("test/unit/x.test.ts", source)).toEqual([{ file: "test/unit/x.test.ts", helper: "atDaysAgo", calls: 2 }]); + }); + + it("catches the arrow form too, which is how most of these are written", () => { + const source = ` +const daysAgo = (days: number) => new Date(Date.now() - days * 86_400_000).toISOString(); +seed(daysAgo(1)); +seed(daysAgo(30)); +`; + expect(findFixtureClockRaces("f.test.ts", source).map((race) => race.helper)).toEqual(["daysAgo"]); + }); + + it("stays quiet once the file anchors on a single captured instant -- the fix must actually clear it", () => { + const source = ` +const FIXTURE_NOW_MS = Date.now(); +const daysAgo = (days: number) => new Date(FIXTURE_NOW_MS - days * 86_400_000).toISOString(); +seed(daysAgo(1)); +seed(daysAgo(30)); +`; + expect(findFixtureClockRaces("f.test.ts", source)).toEqual([]); + }); + + it("does NOT flag a polling helper -- the passage of time is the thing it tests", () => { + const source = ` +function waitFor(predicate: () => boolean, ms = 3000): Promise { + const deadline = Date.now() + ms; + while (Date.now() < deadline) { if (predicate()) return; } +} +await waitFor(() => a); +await waitFor(() => b); +`; + expect(findFixtureClockRaces("f.test.ts", source)).toEqual([]); + }); + + it("does NOT flag a helper whose parameter has nothing to do with the clock", () => { + // seedSelfHealingToken(body) mentions Date.now() for a token expiry an hour out. The offset is a + // constant, not the caller's argument, so nothing varies between calls and there is no inconsistency. + const source = ` +function seedSelfHealingToken(body: unknown) { + return { token: "t", expiresAtMs: Date.now() + 60 * 60_000, body }; +} +seedSelfHealingToken({}); +seedSelfHealingToken({ a: 1 }); +`; + expect(findFixtureClockRaces("f.test.ts", source)).toEqual([]); + }); + + it("does NOT flag a liveness comparison against the clock", () => { + const source = ` +const alive = (key: string) => { + const entry = store.get(key); + if (entry.expiresAtMs <= Date.now()) return null; + return entry; +}; +alive("a"); +alive("b"); +`; + expect(findFixtureClockRaces("f.test.ts", source)).toEqual([]); + }); + + it("does NOT flag a single-use helper -- one call cannot disagree with itself", () => { + const source = ` +const daysAgo = (days: number) => new Date(Date.now() - days * 86_400_000).toISOString(); +seed(daysAgo(1)); +`; + expect(findFixtureClockRaces("f.test.ts", source)).toEqual([]); + }); + + it("does NOT flag a zero-argument now() helper -- there is no offset to project from", () => { + const source = ` +const now = () => new Date(Date.now()).toISOString(); +seed(now()); +seed(now()); +`; + expect(findFixtureClockRaces("f.test.ts", source)).toEqual([]); + }); + + it("reports a helper ONCE even when its declaration matches more than one form", () => { + const source = ` +const dayAgo = (n: number) => new Date(Date.now() - n * 86_400_000).toISOString(); +seed(dayAgo(1)); +seed(dayAgo(2)); +seed(dayAgo(3)); +`; + expect(findFixtureClockRaces("f.test.ts", source)).toHaveLength(1); + }); +}); diff --git a/test/unit/check-stuck-required-checks-script.test.ts b/test/unit/check-stuck-required-checks-script.test.ts index 2cf574e78..a8e111e3d 100644 --- a/test/unit/check-stuck-required-checks-script.test.ts +++ b/test/unit/check-stuck-required-checks-script.test.ts @@ -19,8 +19,14 @@ import { type CheckRun = { name: string; status: string; started_at?: string; html_url?: string }; type ApiOptions = { method?: string; body?: string; headers?: Record }; +// #9955: anchored to ONE instant for the whole file. Re-reading Date.now() per call makes timestamps +// within a single fixture mutually inconsistent by however long elapsed between the calls, which is a +// real race wherever the code under test compares them against a boundary derived from one of them -- +// it cost a false-positive red CI on #9950, on a PR that changed nothing but a workflow file. +const FIXTURE_NOW_MS = Date.now(); + function minutesAgoIso(minutes: number): string { - return new Date(Date.now() - minutes * 60_000).toISOString(); + return new Date(FIXTURE_NOW_MS - minutes * 60_000).toISOString(); } const scope = { owner: "acme", repoName: "widget", thresholdMinutes: 20 }; diff --git a/test/unit/contributor-open-pr-monitor.test.ts b/test/unit/contributor-open-pr-monitor.test.ts index ef936252e..ec1b52149 100644 --- a/test/unit/contributor-open-pr-monitor.test.ts +++ b/test/unit/contributor-open-pr-monitor.test.ts @@ -34,8 +34,14 @@ const maintainerRole: RoleContext = { guidance: "maintainer", }; +// #9955: anchored to ONE instant for the whole file. Re-reading Date.now() per call makes timestamps +// within a single fixture mutually inconsistent by however long elapsed between the calls, which is a +// real race wherever the code under test compares them against a boundary derived from one of them -- +// it cost a false-positive red CI on #9950, on a PR that changed nothing but a workflow file. +const FIXTURE_NOW_MS = Date.now(); + function daysAgo(days: number): string { - return new Date(Date.now() - days * 86_400_000).toISOString(); + return new Date(FIXTURE_NOW_MS - days * 86_400_000).toISOString(); } function pr(overrides: Partial & Pick): PullRequestRecord { diff --git a/test/unit/decision-record.test.ts b/test/unit/decision-record.test.ts index bdd6be1d2..5672d3b56 100644 --- a/test/unit/decision-record.test.ts +++ b/test/unit/decision-record.test.ts @@ -568,7 +568,11 @@ describe("verifier vs absence (#9474 pruned records, #9489 grace + interior orph vi.useRealTimers(); } }; - const daysAgo = (days: number) => new Date(Date.now() - days * 24 * 60 * 60 * 1000); + // #9955: anchored to ONE instant for the whole file -- re-reading Date.now() per call makes two fixture +// timestamps mutually inconsistent by the time elapsed between them, which flips any comparison the code +// under test derives from one of them. Enforced by scripts/check-fixture-clock-races.ts. +const FIXTURE_NOW_MS = Date.now(); +const daysAgo = (days: number) => new Date(FIXTURE_NOW_MS - days * 24 * 60 * 60 * 1000); it("REGRESSION (#9474): a record pruned by the 180-day retention window verifies clean, counted in prunedRecords -- not reported as tampering", async () => { const env = createTestEnv(); diff --git a/test/unit/issue-quality-report-package.test.ts b/test/unit/issue-quality-report-package.test.ts index cc8a5210a..becfca19f 100644 --- a/test/unit/issue-quality-report-package.test.ts +++ b/test/unit/issue-quality-report-package.test.ts @@ -14,8 +14,14 @@ function now(): string { return new Date().toISOString(); } +// #9955: anchored to ONE instant for the whole file. Re-reading Date.now() per call makes timestamps +// within a single fixture mutually inconsistent by however long elapsed between the calls, which is a +// real race wherever the code under test compares them against a boundary derived from one of them -- +// it cost a false-positive red CI on #9950, on a PR that changed nothing but a workflow file. +const FIXTURE_NOW_MS = Date.now(); + function daysAgoIso(days: number): string { - return new Date(Date.now() - days * 86_400_000).toISOString(); + return new Date(FIXTURE_NOW_MS - days * 86_400_000).toISOString(); } function registryConfig(overrides: Partial = {}): RegistryRepoConfig { diff --git a/test/unit/orb-analytics.test.ts b/test/unit/orb-analytics.test.ts index d5913b6fa..ed602ef6e 100644 --- a/test/unit/orb-analytics.test.ts +++ b/test/unit/orb-analytics.test.ts @@ -2,6 +2,11 @@ import { describe, expect, it } from "vitest"; import { computeFleetAnalytics, getFleetHealthSummary, HEALTH_STALE_HOURS, wilsonInterval } from "../../src/orb/analytics"; import { createTestEnv, } from "../helpers/d1"; +// #9955: anchored to ONE instant for the whole file -- re-reading Date.now() per call makes two fixture +// timestamps mutually inconsistent by the time elapsed between them, which flips any comparison the code +// under test derives from one of them. Enforced by scripts/check-fixture-clock-races.ts. +const FIXTURE_NOW_MS = Date.now(); + let seq = 0; /** Insert N orb_signals rows for one instance with a fixed verdict/outcome/reversal/cycle. */ async function signals( @@ -613,7 +618,7 @@ describe("fleetFramingEligible (#9168)", () => { // #9783: orb_signals prunes at 90 days into orb_signal_rollups, so a window that reaches past the prune has // to read both halves or the headline silently under-counts as history ages out. describe("computeFleetAnalytics() over folded history (#9783)", () => { - const dayAgo = (n: number) => new Date(Date.now() - n * 86_400_000).toISOString(); +const dayAgo = (n: number) => new Date(FIXTURE_NOW_MS - n * 86_400_000).toISOString(); const foldedCell = async (env: Env, day: string, over: { verdict?: string; outcome?: string; n: number }) => env.DB @@ -676,7 +681,8 @@ describe("computeFleetAnalytics() over folded history (#9783)", () => { // rollup. So a window reaching past the retention horizon must not quietly report percentiles computed from // only the surviving rows. describe("cycle-time observability across the retention horizon (#9783)", () => { - const dayAgo = (n: number) => new Date(Date.now() - n * 86_400_000).toISOString(); + // #9955: same anchor as the module-level helper this one shadows -- both must be deterministic. + const dayAgo = (n: number) => new Date(FIXTURE_NOW_MS - n * 86_400_000).toISOString(); const seedCycle = async (env: Env) => { await env.DB.prepare("INSERT INTO orb_instances (instance_id, registered) VALUES ('inst', 1)").run(); diff --git a/test/unit/pending-pr-scenarios.test.ts b/test/unit/pending-pr-scenarios.test.ts index fcc879986..8beb4c7a9 100644 --- a/test/unit/pending-pr-scenarios.test.ts +++ b/test/unit/pending-pr-scenarios.test.ts @@ -34,8 +34,14 @@ const maintainerRole: RoleContext = { guidance: "maintainer", }; +// #9955: anchored to ONE instant for the whole file. Re-reading Date.now() per call makes timestamps +// within a single fixture mutually inconsistent by however long elapsed between the calls, which is a +// real race wherever the code under test compares them against a boundary derived from one of them -- +// it cost a false-positive red CI on #9950, on a PR that changed nothing but a workflow file. +const FIXTURE_NOW_MS = Date.now(); + function daysAgo(days: number): string { - return new Date(Date.now() - days * 86_400_000).toISOString(); + return new Date(FIXTURE_NOW_MS - days * 86_400_000).toISOString(); } function pr(overrides: Partial & Pick): PullRequestRecord { diff --git a/test/unit/queue-trends.test.ts b/test/unit/queue-trends.test.ts index ff98221f5..974b4c3ac 100644 --- a/test/unit/queue-trends.test.ts +++ b/test/unit/queue-trends.test.ts @@ -289,6 +289,21 @@ function queueHealthSnapshot(id: string, daysAgo: number, values: { openPrs: num }; } +// #9955: ONE instant for the whole file. Calling Date.now() per invocation made timestamps within a single +// test mutually inconsistent by however long elapsed between the calls -- and buildWindow anchors on the +// LATEST snapshot, not on wall-clock: +// +// targetMs = latestMs - windowDays * day +// baseline = newest snapshot with fetchedAt <= targetMs +// +// So when `atDaysAgo(0)` was evaluated at T0 and `atDaysAgo(7)` a moment later at T1, the "7 days ago" +// snapshot landed at T1 - 7d, which is NEWER than the target T0 - 7d. No baseline is found, every window +// reports "unavailable", and the assertion fails. It passed only when both calls hit the same millisecond. +// Reproduced deterministically with a 2ms offset; observed for real on #9950, a PR that changed nothing but +// a workflow file. Under one-shot review a false red is not a re-run away from fine -- it auto-closes correct +// contributor work -- so a timing-dependent fixture is a gate correctness problem, not just CI noise. +const FIXTURE_NOW_MS = Date.now(); + function atDaysAgo(daysAgo: number): string { - return new Date(Date.now() - daysAgo * 24 * 60 * 60 * 1000).toISOString(); + return new Date(FIXTURE_NOW_MS - daysAgo * 24 * 60 * 60 * 1000).toISOString(); } diff --git a/test/unit/selfhost-docker-prune-script.test.ts b/test/unit/selfhost-docker-prune-script.test.ts index d009ab6c7..cf7870279 100644 --- a/test/unit/selfhost-docker-prune-script.test.ts +++ b/test/unit/selfhost-docker-prune-script.test.ts @@ -104,8 +104,14 @@ function stubDate(binDir: string): void { chmodSync(join(binDir, "date"), 0o755); } +// #9955: anchored to ONE instant for the whole file. Re-reading Date.now() per call makes timestamps +// within a single fixture mutually inconsistent by however long elapsed between the calls, which is a +// real race wherever the code under test compares them against a boundary derived from one of them -- +// it cost a false-positive red CI on #9950, on a PR that changed nothing but a workflow file. +const FIXTURE_NOW_MS = Date.now(); + function isoHoursAgo(hours: number): string { - return new Date(Date.now() - hours * 3600_000).toISOString(); + return new Date(FIXTURE_NOW_MS - hours * 3600_000).toISOString(); } function runPruneScript( diff --git a/test/unit/signals-v2.test.ts b/test/unit/signals-v2.test.ts index 71e76aed3..e706ac38d 100644 --- a/test/unit/signals-v2.test.ts +++ b/test/unit/signals-v2.test.ts @@ -42,7 +42,11 @@ import type { ContributorRepoStatRecord, IssueRecord, PullRequestRecord, RecentM // Shared-fixture timestamps are relative to "now" so age-bucket / reviewability windows (e.g. the // `< 30 days` likely-reviewable cutoff) never drift past their boundary as real time advances (no time-bomb). -const isoDaysAgo = (days: number): string => new Date(Date.now() - days * 24 * 60 * 60 * 1000).toISOString(); +// #9955: anchored to ONE instant for the whole file -- re-reading Date.now() per call makes two fixture +// timestamps mutually inconsistent by the time elapsed between them, which flips any comparison the code +// under test derives from one of them. Enforced by scripts/check-fixture-clock-races.ts. +const FIXTURE_NOW_MS = Date.now(); +const isoDaysAgo = (days: number): string => new Date(FIXTURE_NOW_MS - days * 24 * 60 * 60 * 1000).toISOString(); const repo: RepositoryRecord = { fullName: "JSONbored/loopover",