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
14 changes: 14 additions & 0 deletions .fallowrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
"src/utils/png-worker.ts",
"scripts/patch-xcuitest-runner-icon.ts",
"scripts/runner-request-count/run.ts",
"scripts/lib/contention-retry-run.ts",
"scripts/vitest-runner-timeout-setup.ts",
"test/contention-retry-fixtures/vitest.fixture.config.ts",
"test/contention-retry-fixtures/timeout-provenance.fixture.ts",
"src/utils/update-check-entry.ts",
"examples/sdk/client-session.ts",
"examples/sdk/metro-runtime.ts",
Expand Down Expand Up @@ -77,6 +81,16 @@
"GatedKeysAreResolverKeys"
]
},
{
"comment": "Contention retry lane (#1419): the reporter default is loaded by Vitest from a `--reporter=<path>` string, and SUBPROCESS_STUB_TESTS is consumed by vitest.config.ts \u2014 a tool config outside --production analysis.",
"file": "scripts/lib/{contention-retry.ts,contention-retry-reporter.ts}",
"exports": ["default", "SUBPROCESS_STUB_TESTS"]
},
{
"comment": "Contention retry gate fixtures (#1419): the config default is loaded by the child `vitest run --config` the gate test spawns.",
"file": "test/contention-retry-fixtures/vitest.fixture.config.ts",
"exports": ["default"]
},
{
"comment": "Mutation-lane seam: readTestScope is consumed by vitest.mutation.config.ts, a tool config outside --production analysis.",
"file": "scripts/mutation/test-scope.ts",
Expand Down
18 changes: 17 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,26 @@ jobs:
- name: Test changed-line coverage gate
run: pnpm check:coverage-changed:test

# The retry list is an enumerated set of owned waivers, so an expired entry
# must fail before the suite runs rather than quietly keeping its retry.
- name: Check contention retry policy
run: pnpm check:contention-retry

# Wrapped in the single-retry policy (#1419): a timeout-shaped failure in
# an enumerated contention-flaky file reruns that file once and reports it
# in the job summary. Assertion failures fail here on the first run.
- name: Run coverage
env:
OUTPUT_ECONOMY_BASE: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }}
run: pnpm test:coverage
run: pnpm test:coverage:ci

- name: Upload contention-retry envelope
if: always()
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
with:
name: contention-retry-envelope
if-no-files-found: ignore
path: .tmp/contention-retry/lane-envelope.json

# Reuses the lcov the coverage step just wrote (never runs coverage twice)
# and fails when changed-line coverage < the threshold in
Expand Down
4 changes: 3 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,9 @@ connect errors, retry policy, or command typing, start in
- Contention flakes: `request-handler-catalog` ("specialized daemon routes...") and the doctor
provider scenario time out under host load. Before believing a regression, rerun in isolation AND
reproduce on plain `origin/main` under the same load. A changing failure set that passes in
isolation is contention, not your change.
isolation is contention, not your change. The files CI may rerun once for a timeout-shaped failure
are enumerated in `scripts/lib/contention-retry.ts` (docs/agents/testing.md, "Contention retry
policy").

## Docs & skills

Expand Down
48 changes: 48 additions & 0 deletions docs/agents/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,54 @@ Use `AGENT_DEVICE_IOS_E2E_TIER=full` for the nightly subset. Step history, cover
screenshots, recordings, traces, and failure context are written below
`test/artifacts/ios-simulator/` and uploaded by the existing shared artifact action. The six
Settings replays remain additive OS-chrome coverage and are not modified by this suite.
## Contention retry policy (enumerated, timeouts only)

Some test files stub a real binary and then spawn or wait on it, so under host load they fail for a
reason that has nothing to do with the diff. The set of such files is **enumerated** in
`scripts/lib/contention-retry.ts` (`CONTENTION_RETRY_FILES`) — never a glob, which would silently
enroll every future file under a directory. That one constant also derives `SUBPROCESS_STUB_TESTS`,
the serialized `subprocess-stub` Vitest project in `vitest.config.ts`, so the execution contract and
the retry policy cannot drift apart.

The CI Coverage job runs the suite through `pnpm test:coverage:ci`
(`scripts/lib/contention-retry-run.ts`), which applies one rule:

- **Timeouts only, proven by the runner.** A rerun happens only when *every* failure in the run is a
test the runner itself aborted, in a listed file. Eligibility comes from a mark written inside the
runner (`scripts/vitest-runner-timeout-setup.ts`, a setup file on every project): the runner owns
the controller behind `context.signal` and aborts it with the timeout error it raises, so a test
that merely *throws* the exact timeout message — immediately, or after blocking the event loop past
its budget — never carries the mark. `task.meta` is writable by test code, so the mark is not a
flag but the run's secret: the lane mints it per run, and the setup file takes it out of the
environment as it loads — before any test module is imported — so a test writing the marker itself
has no value to write. Error text is never consulted for eligibility;
`test/contention-retry-fixtures/` drives those forgeries through a real Vitest run in the gate. One
assertion failure — in a listed file or not — fails the job on the first run, so a real regression
can never be papered over.
- **Anything a rerun cannot re-check blocks the retry.** Unhandled errors, module load/setup errors,
a coverage-threshold miss, or a nonzero exit no failed test explains are recorded as blockers
(`scripts/lib/contention-retry-blockers.ts`) and fail the job, so a green retry can never erase a
second, unrelated failure from the same run.
- **Gates that fail a run without failing a test publish structurally.** A reporter-level verdict
(the slow-test ratchet setting `process.exitCode = 1`) is invisible in test results, so it is
recorded on the shared blocker channel `scripts/lib/run-blocker-bus.ts`; the retry lane's failure
sink drains it and refuses the rerun. **Any new gate reporter must call `recordRunBlocker`**, and
must be ordered before the sink in `reporters()`.
- **One retry, of the failed files only.** Not the suite, and never twice. Two timed-out tests in one
file are one retry, and count as one. The rerun keeps the first run's execution modes — the same
`--project` selection and the same V8 instrumentation (`scripts/lib/contention-retry-args.ts`), so
a coverage-job failure is never accepted by a run that could not reproduce it. Its coverage lands
in `coverage/contention-retry/`, leaving the first run's report as the changed-line gate's evidence.
- **Retries stay visible.** Every retried file is named in the job summary with its tracking issue
and review date, and the run writes the shared scheduled-lane envelope
(`scripts/lib/lane-envelope.ts`, #1430) with the retry count, so a permanently flaky file shows up
as lane health rather than as a green check.

Adding a file to the list is a reviewed waiver in the ADR 0011 sense: a `reason` naming the concrete
spawn/wait that makes it contention-flaky, a `trackingIssue` for removing that wait, and a `reviewBy`
date. `pnpm check:contention-retry` (its own CI step, and part of `pnpm check:unit`) fails on an
expired entry, a missing file, or a glob, so an entry is renewed or removed rather than inherited.

## Speed rules (experiment-backed, 2026-07-04)

Measured on the full unit suite (340 files, 3,210 tests, 48s wall at ~7x parallelism):
Expand Down
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@
"check:mcp-metadata": "node scripts/sync-mcp-metadata.mjs --check",
"version": "node scripts/sync-mcp-metadata.mjs && git add server.json",
"check:tooling": "pnpm lint && pnpm typecheck && pnpm check:layering && pnpm depgraph:test && pnpm check:production-exports && pnpm check:mcp-metadata && pnpm build && pnpm check:bundle-owner-files",
"check:unit": "pnpm test:unit && pnpm test:smoke",
"check:unit": "pnpm check:contention-retry && pnpm test:unit && pnpm test:smoke",
"check": "pnpm check:tooling && pnpm check:fallow && pnpm check:unit",
"prepack": "pnpm check:mcp-metadata && pnpm build:all && pnpm package:apple-runner:npm && pnpm package:android-snapshot-helper:npm && pnpm package:android-ime-helper:npm",
"typecheck": "tsc -p tsconfig.json && tsc -p examples/sdk/tsconfig.json",
Expand All @@ -159,6 +159,8 @@
"test": "vitest run --project unit-core --project subprocess-stub",
"test:unit": "vitest run --project unit-core --project subprocess-stub",
"test:coverage": "vitest run --coverage",
"test:coverage:ci": "node --experimental-strip-types scripts/lib/contention-retry-run.ts --coverage",
"check:contention-retry": "node --experimental-strip-types --test scripts/lib/contention-retry-policy.test.ts",
"test:integration:provider": "vitest run --project provider-integration",
"test:integration:progress": "node --experimental-strip-types scripts/integration-progress.ts",
"test:integration:progress:check": "node --experimental-strip-types scripts/integration-progress.ts --check",
Expand Down
31 changes: 31 additions & 0 deletions scripts/lib/contention-retry-args.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Vitest argv for the two runs of the contention retry lane (#1419). The retry
// must execute the failed files the same way the first run did — same projects,
// same V8 instrumentation — or a failure the Coverage job produced could be
// accepted by a run that could not reproduce it.

export type RunModes = { projects: readonly string[]; coverage: boolean };

/** Where the retry's own coverage lands, so the first run's report stays the gate's evidence. */
export const RETRY_COVERAGE_DIR = 'coverage/contention-retry';

function projectArgs(modes: RunModes): string[] {
return modes.projects.flatMap((name) => ['--project', name]);
}

export function firstRunArgs(modes: RunModes): string[] {
return [...projectArgs(modes), ...(modes.coverage ? ['--coverage'] : [])];
}

export function retryRunArgs(modes: RunModes, files: readonly string[]): string[] {
if (!modes.coverage) return [...projectArgs(modes), ...files];
// Global thresholds describe the whole suite; a handful of files can never meet
// them. A first-run threshold failure is a blocker, so the retry never runs.
return [
...projectArgs(modes),
'--coverage',
`--coverage.reportsDirectory=${RETRY_COVERAGE_DIR}`,
'--coverage.thresholds.statements=0',
'--coverage.thresholds.lines=0',
...files,
];
}
35 changes: 35 additions & 0 deletions scripts/lib/contention-retry-blockers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Blockers a reporter cannot see, read from the run's process outcome (#1419):
// the coverage verdict, and any nonzero exit no failed test explains.

import type { RunBlocker } from './contention-retry.ts';

/** Vitest's own coverage verdict lines (`checkCoverages`, vitest 4). */
const COVERAGE_FAILURE = [
/ERROR: Coverage for [a-z]+ \([\d.]+%\) does not meet (?:global )?threshold/i,
/ERROR: Coverage for [a-z]+ \([\d.]+%\) does not meet .*minimum threshold/i,
];

export type ProcessOutcome = {
ok: boolean;
/** Combined stdout+stderr of the run. */
output: string;
failureCount: number;
};

export function processBlockers(outcome: ProcessOutcome): RunBlocker[] {
if (outcome.ok) return [];
const blockers: RunBlocker[] = [];
const coverage = outcome.output
.split('\n')
.filter((line) => COVERAGE_FAILURE.some((pattern) => pattern.test(line)));
for (const line of coverage) {
blockers.push({ kind: 'coverage threshold', detail: line.trim() });
}
if (outcome.failureCount === 0 && blockers.length === 0) {
blockers.push({
kind: 'unexplained failure',
detail: 'the run exited nonzero with no failed test recorded — see the job log',
});
}
return blockers;
}
149 changes: 149 additions & 0 deletions scripts/lib/contention-retry-lane.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
// Orchestration for the single-retry policy (#1419): run the suite, and when it
// fails only with timeout-shaped failures in enumerated files, rerun exactly
// those files once. Kept free of process spawning and file writing so both
// acceptance cases — an injected assertion failure fails on the first run, an
// injected timeout passes on retry — are driven directly in the gate test.

import { laneEnvelope, type LaneEnvelope } from './lane-envelope.ts';
import {
CONTENTION_RETRY_FILES,
expiredRetryEntries,
formatRetrySummary,
normalizeTestFile,
planContentionRetry,
type RetryOutcome,
type RetryPlan,
type RunBlocker,
type TestFailure,
} from './contention-retry.ts';

export type TestRun = {
ok: boolean;
failures: readonly TestFailure[];
/** Non-test failures; any of them forbids a retry. */
blockers?: readonly RunBlocker[];
};

/** Telemetry payload written to the shared lane envelope (scripts/lib/lane-envelope.ts). */
export type ContentionRetryTelemetry = {
/** Files rerun in this job (one entry per file, not per failed test). */
retried: ReadonlyArray<{ file: string; testNames: readonly string[]; trackingIssue: string }>;
retryCount: number;
retryOutcome: RetryOutcome | null;
listSize: number;
};

export type ContentionRetryResult = {
ok: boolean;
summary: string;
envelope: LaneEnvelope<ContentionRetryTelemetry>;
};

export type ContentionRetryOptions = {
/** Runs the whole suite. */
runAll: () => Promise<TestRun>;
/** Reruns exactly the given files. */
runFiles: (files: readonly string[]) => Promise<TestRun>;
commit: string;
configHash: string;
vitestVersion: string;
startedAtMs: number;
now?: () => number;
/** Clock for the waiver expiry gate. */
today?: Date;
};

const LANE_ID = 'unit-contention-retry';

export async function runWithContentionRetry(
options: ContentionRetryOptions,
): Promise<ContentionRetryResult> {
const today = options.today ?? new Date();
const expired = expiredRetryEntries(today);
if (expired.length > 0) {
const lines = expired.map(
(entry) => `- \`${entry.file}\` (review by ${entry.reviewBy}, ${entry.trackingIssue})`,
);
return finish(options, {
ok: false,
summary: [
'## Contention retry (#1419)',
'',
'Retry list expired — renew the review date or remove the entry:',
'',
...lines,
'',
].join('\n'),
plan: undefined,
outcome: null,
});
}

const first = await options.runAll();
if (first.ok) {
return finish(options, { ok: true, summary: '', plan: undefined, outcome: null });
}

const plan = planContentionRetry(first.failures, first.blockers ?? []);
if (!plan.retry) {
return finish(options, {
ok: false,
summary: formatRetrySummary({ plan }),
plan,
outcome: null,
});
}

const retried = await options.runFiles(plan.files);
const outcome: RetryOutcome = retried.ok ? 'passed' : 'failed';
return finish(options, {
ok: retried.ok,
summary: formatRetrySummary({ plan, outcome }),
plan,
outcome,
});
}

function finish(
options: ContentionRetryOptions,
state: {
ok: boolean;
summary: string;
plan: RetryPlan | undefined;
outcome: RetryOutcome | null;
},
): ContentionRetryResult {
const plan = state.plan;
// Keyed by file, matching what actually reran.
const retried = plan?.retry
? plan.files.map((file) => ({
file,
testNames: plan.failures
.filter((failure) => normalizeTestFile(failure.file) === file)
.map((failure) => failure.testName),
trackingIssue:
CONTENTION_RETRY_FILES.find((entry) => entry.file === file)?.trackingIssue ?? '',
}))
: [];
return {
ok: state.ok,
summary: state.summary,
envelope: laneEnvelope<ContentionRetryTelemetry>({
lane: LANE_ID,
commit: options.commit,
tool: { vitest: options.vitestVersion },
configHash: options.configHash,
startedAtMs: options.startedAtMs,
now: options.now?.(),
// The retry set is enumerated, not randomized.
seed: null,
result: state.ok ? 'pass' : 'fail',
data: {
retried,
retryCount: retried.length,
retryOutcome: state.outcome,
listSize: CONTENTION_RETRY_FILES.length,
},
}),
};
}
Loading
Loading