Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
126 changes: 126 additions & 0 deletions scripts/check-fixture-clock-races.ts
Original file line number Diff line number Diff line change
@@ -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();
8 changes: 7 additions & 1 deletion test/unit/burden-forecast.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> = {}): IssueRecord {
Expand Down
108 changes: 108 additions & 0 deletions test/unit/check-fixture-clock-races-script.test.ts
Original file line number Diff line number Diff line change
@@ -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<void> {
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);
});
});
8 changes: 7 additions & 1 deletion test/unit/check-stuck-required-checks-script.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> };

// #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 };
Expand Down
8 changes: 7 additions & 1 deletion test/unit/contributor-open-pr-monitor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<PullRequestRecord> & Pick<PullRequestRecord, "number">): PullRequestRecord {
Expand Down
Loading
Loading