From f0d9186827e0ba09383ec3d4e67e902b3e747d55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 11:17:06 +0200 Subject: [PATCH 1/8] feat(tooling): serialize validation and emit compact execution receipts --- package.json | 4 +- scripts/check-affected/checkout-state.test.ts | 36 +++ scripts/check-affected/checkout-state.ts | 35 +++ .../check-affected/command-execution.test.ts | 114 +++++++++ scripts/check-affected/command-execution.ts | 106 +++++++++ scripts/check-affected/command.test.ts | 29 +++ scripts/check-affected/command.ts | 21 ++ scripts/check-affected/execution.fixtures.ts | 46 ++++ scripts/check-affected/execution.test.ts | 63 +++++ scripts/check-affected/execution.ts | 220 ++++++++++++++++++ scripts/check-affected/host-lock.test.ts | 45 ++++ scripts/check-affected/host-lock.ts | 127 ++++++++++ scripts/check-affected/run-cli.test.ts | 81 +++++++ scripts/check-affected/run.ts | 91 ++++++-- scripts/check-affected/test-report.test.ts | 67 ++++++ scripts/check-affected/test-report.ts | 61 +++++ vitest.config.ts | 9 +- 17 files changed, 1134 insertions(+), 21 deletions(-) create mode 100644 scripts/check-affected/checkout-state.test.ts create mode 100644 scripts/check-affected/checkout-state.ts create mode 100644 scripts/check-affected/command-execution.test.ts create mode 100644 scripts/check-affected/command-execution.ts create mode 100644 scripts/check-affected/command.test.ts create mode 100644 scripts/check-affected/command.ts create mode 100644 scripts/check-affected/execution.fixtures.ts create mode 100644 scripts/check-affected/execution.test.ts create mode 100644 scripts/check-affected/execution.ts create mode 100644 scripts/check-affected/host-lock.test.ts create mode 100644 scripts/check-affected/host-lock.ts create mode 100644 scripts/check-affected/run-cli.test.ts create mode 100644 scripts/check-affected/test-report.test.ts create mode 100644 scripts/check-affected/test-report.ts diff --git a/package.json b/package.json index c1d4ba95cc..7470b650c9 100644 --- a/package.json +++ b/package.json @@ -145,7 +145,7 @@ "fallow:baseline": "fallow health --report-only --save-baseline fallow-baselines/health.json --summary", "check:fallow": "fallow audit", "check:affected": "node --experimental-strip-types scripts/check-affected/run.ts", - "check:affected:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/check-affected/model.test.ts scripts/check-affected/platform-packages.test.ts scripts/check-affected/device-lanes.test.ts scripts/check-affected/lockfile-install-sync.test.ts scripts/check-affected/run.test.ts", + "check:affected:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/check-affected/*.test.ts", "gate": "node --experimental-strip-types scripts/gate/run.ts", "check:gate-manifest": "node --experimental-strip-types scripts/gate/check.ts", "check:gate-manifest:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/gate/*.test.ts", @@ -177,7 +177,7 @@ "check:mcp-metadata": "node scripts/sync-mcp-metadata.mjs --check", "check:tooling": "pnpm format:check && pnpm lint && pnpm typecheck && pnpm check:layering && pnpm depgraph:test && pnpm check:gate-manifest:test && pnpm check:gate-manifest && pnpm check:production-exports && pnpm check:tmpdir-leaks:test && pnpm check:xctest-selection && pnpm check:packaged-runner-swift && pnpm check:mcp-metadata && pnpm check:doc-prose && pnpm check:doc-links && pnpm build && pnpm check:bundle-owner-files && pnpm check:package", "check:unit": "pnpm test:unit && pnpm check:tmpdir-leaks && pnpm test:smoke", - "check": "pnpm check:tooling && pnpm check:fallow && pnpm check:unit", + "check": "node --experimental-strip-types scripts/check-affected/run.ts --run --full", "prepack": "npm_config_dry_run=false pnpm release:prepare", "typecheck": "tsc -b tsconfig.typecheck.json examples/sdk/tsconfig.json", "test-app:install": "pnpm install --dir examples/test-app", diff --git a/scripts/check-affected/checkout-state.test.ts b/scripts/check-affected/checkout-state.test.ts new file mode 100644 index 0000000000..6692893a32 --- /dev/null +++ b/scripts/check-affected/checkout-state.test.ts @@ -0,0 +1,36 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { test } from 'node:test'; +import { runCmdSync } from '@agent-device/host-kit/command'; +import { readCheckoutState } from './checkout-state.ts'; + +test('checkout fingerprint distinguishes edits with identical porcelain status and includes untracked contents', () => { + const cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'checkout-state-')); + const git = (...args: string[]) => runCmdSync('git', args, { cwd }); + try { + git('init', '-q'); + git('config', 'user.email', 'test@example.com'); + git('config', 'user.name', 'Test'); + fs.writeFileSync(path.join(cwd, 'source.ts'), 'base'); + git('add', '.'); + git('commit', '-qm', 'base'); + const clean = readCheckoutState(cwd); + assert.equal(clean.status, ''); + fs.writeFileSync(path.join(cwd, 'source.ts'), 'first'); + const first = readCheckoutState(cwd); + fs.writeFileSync(path.join(cwd, 'source.ts'), 'second'); + const second = readCheckoutState(cwd); + assert.equal(first.status, second.status); + assert.notEqual(first.fingerprint, second.fingerprint); + fs.writeFileSync(path.join(cwd, 'new.ts'), 'one'); + const untracked = readCheckoutState(cwd); + fs.writeFileSync(path.join(cwd, 'new.ts'), 'two'); + assert.notEqual(untracked.fingerprint, readCheckoutState(cwd).fingerprint); + git('add', '.'); + assert.notEqual(second.fingerprint, readCheckoutState(cwd).fingerprint); + } finally { + fs.rmSync(cwd, { recursive: true, force: true }); + } +}); diff --git a/scripts/check-affected/checkout-state.ts b/scripts/check-affected/checkout-state.ts new file mode 100644 index 0000000000..5b8d145d85 --- /dev/null +++ b/scripts/check-affected/checkout-state.ts @@ -0,0 +1,35 @@ +import crypto from 'node:crypto'; +import fs from 'node:fs'; +import path from 'node:path'; +import { runCmdSync } from '@agent-device/host-kit/command'; + +export type CheckoutState = { head: string; status: string; fingerprint: string }; + +export function gitValue(cwd: string, args: string[]): string { + return runCmdSync('git', args, { cwd }).stdout.trim(); +} + +export function readCheckoutState(cwd: string): CheckoutState { + const head = gitValue(cwd, ['rev-parse', 'HEAD']); + const status = gitValue(cwd, ['status', '--porcelain=v1', '--untracked-files=all']); + const hash = crypto.createHash('sha256').update(head).update(status); + for (const args of [ + ['diff', '--binary'], + ['diff', '--binary', '--cached'], + ]) { + hash.update(runCmdSync('git', args, { cwd, maxBuffer: 64 * 1024 * 1024 }).stdout); + } + const untracked = runCmdSync('git', ['ls-files', '--others', '--exclude-standard', '-z'], { + cwd, + }).stdout; + for (const file of untracked.split('\0').filter(Boolean).sort()) { + const absolute = path.join(cwd, file); + hash.update(file).update('\0'); + hash.update( + fs.lstatSync(absolute).isSymbolicLink() + ? fs.readlinkSync(absolute) + : fs.readFileSync(absolute), + ); + } + return { head, status, fingerprint: hash.digest('hex') }; +} diff --git a/scripts/check-affected/command-execution.test.ts b/scripts/check-affected/command-execution.test.ts new file mode 100644 index 0000000000..bdc9abee41 --- /dev/null +++ b/scripts/check-affected/command-execution.test.ts @@ -0,0 +1,114 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { test } from 'node:test'; +import { setTimeout } from 'node:timers/promises'; +import { runCmdBackground } from '@agent-device/host-kit/command'; +import { entry, fixture, run } from './execution.fixtures.ts'; +import type { ExecutionReceipt } from './execution.ts'; + +test('SIGTERM cancels the command group, records cancellation, and releases the host lock', async () => { + const { cwd } = fixture(); + const ready = path.join(cwd, '.tmp', 'ready.json'); + const source = ` + import { runValidation } from ${JSON.stringify(entry)}; + const code = await runValidation(${JSON.stringify(cwd)}, { base: 'HEAD', head: 'HEAD', json: true }, async ({execute}) => { + return execute([process.execPath, '-e', ${JSON.stringify(`require('node:fs').writeFileSync(${JSON.stringify(ready)}, JSON.stringify(process.pid)); setInterval(() => {}, 1000);`)}], ${JSON.stringify(cwd)}, 'sleep'); + }); process.exitCode = code; + `; + const { child, wait } = runCmdBackground( + process.execPath, + ['--experimental-strip-types', '--input-type=module', '-e', source], + { + cwd, + allowFailure: true, + env: { ...process.env, HOME: cwd, AGENT_DEVICE_VALIDATION_PARENT: '' }, + }, + ); + try { + const deadline = Date.now() + 5000; + while (!fs.existsSync(ready) && Date.now() < deadline) await setTimeout(20); + assert.ok(fs.existsSync(ready), 'command must actually start'); + const commandPid = JSON.parse(fs.readFileSync(ready, 'utf8')) as number; + child.kill('SIGTERM'); + const result = await wait; + assert.equal(result.exitCode, 143, result.stderr); + const receipt = JSON.parse(result.stdout) as ExecutionReceipt; + assert.equal(receipt.status, 'cancelled'); + assert.equal(receipt.results[0]?.status, 'cancelled'); + assert.throws(() => process.kill(commandPid, 0), { code: 'ESRCH' }); + assert.equal(run(cwd, 'HEAD', 'return 0;').exitCode, 0); + } finally { + child.kill('SIGKILL'); + await wait; + fs.rmSync(cwd, { recursive: true, force: true }); + } +}); + +test('a killed validation parent does not let a waiter overlap its surviving command group', async () => { + const { cwd } = fixture(); + const ready = path.join(cwd, '.tmp', 'orphan.json'); + const env = { ...process.env, HOME: cwd, AGENT_DEVICE_VALIDATION_PARENT: '' }; + const source = ` + import { runValidation } from ${JSON.stringify(entry)}; + const code = await runValidation(${JSON.stringify(cwd)}, { base: 'HEAD', head: 'HEAD', json: true }, async ({execute}) => execute([process.execPath, '-e', ${JSON.stringify(`require('node:fs').writeFileSync(${JSON.stringify(ready)}, JSON.stringify(process.pid)); setInterval(() => {}, 1000);`)}], ${JSON.stringify(cwd)}, 'orphan')); + process.exitCode = code; + `; + const parent = runCmdBackground( + process.execPath, + ['--experimental-strip-types', '--input-type=module', '-e', source], + { cwd, env, allowFailure: true }, + ); + let commandPid: number | undefined; + let waiter: ReturnType | undefined; + try { + const deadline = Date.now() + 5000; + while (!fs.existsSync(ready) && Date.now() < deadline) await setTimeout(20); + assert.ok(fs.existsSync(ready)); + commandPid = ( + JSON.parse( + fs.readFileSync( + path.join(cwd, '.cache', 'agent-device', 'validation.lock.active.json'), + 'utf8', + ), + ) as { pid: number } + ).pid; + parent.child.kill('SIGKILL'); + await parent.wait; + waiter = runCmdBackground( + process.execPath, + [ + '--experimental-strip-types', + '--input-type=module', + '-e', + ` + import { runValidation } from ${JSON.stringify(entry)}; + process.exitCode = await runValidation(${JSON.stringify(cwd)}, {base:'HEAD',head:'HEAD',json:true}, async () => 0); + `, + ], + { cwd, env, allowFailure: true }, + ); + let observedWait = false; + waiter.child.stderr?.on('data', (data) => { + if (String(data).includes('[wait] orphan')) observedWait = true; + }); + const waitDeadline = Date.now() + 5000; + while (!observedWait && Date.now() < waitDeadline) await setTimeout(20); + assert.equal(observedWait, true, 'waiter must observe surviving command before acquiring'); + process.kill(-commandPid, 'SIGTERM'); + const result = await waiter.wait; + assert.equal(result.exitCode, 0, result.stderr); + assert.equal(JSON.parse(result.stdout).status, 'passed'); + } finally { + parent.child.kill('SIGKILL'); + waiter?.child.kill('SIGTERM'); + if (commandPid) { + try { + process.kill(-commandPid, 'SIGKILL'); + } catch {} + } + await parent.wait; + await waiter?.wait; + fs.rmSync(cwd, { recursive: true, force: true }); + } +}); diff --git a/scripts/check-affected/command-execution.ts b/scripts/check-affected/command-execution.ts new file mode 100644 index 0000000000..eec398bb0b --- /dev/null +++ b/scripts/check-affected/command-execution.ts @@ -0,0 +1,106 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { runCmdStreaming } from '@agent-device/host-kit/command'; +import { + recordValidationCommand, + finishValidationCommand, + VALIDATION_PARENT_ENV, +} from './host-lock.ts'; +import type { CheckResult, ValidationRun } from './execution.ts'; +import type { TestReport } from './test-report.ts'; + +export async function executeValidationCommand( + run: ValidationRun, + result: CheckResult, + command: string[], + cwd: string, +): Promise { + run.controller.signal.throwIfAborted(); + const index = run.receipt.results.indexOf(result); + const logPath = path.join(run.directory, `${index}.log`); + const reportPrefix = path.join(run.directory, `${index}.tests`); + Object.assign(result, { command, status: 'running', logPath }); + run.save(); + run.report(`[run] ${result.id}; log=${logPath}`); + const descriptor = fs.openSync(logPath, 'w'); + const started = Date.now(); + const heartbeat = setInterval( + () => run.report(`[running] ${result.id} ${Math.round((Date.now() - started) / 1000)}s`), + 15_000, + ); + let commandPid: number | undefined; + try { + recordValidationCommand(run.receiptPath, null); + const output = await runCmdStreaming( + process.execPath, + ['--experimental-strip-types', path.join(import.meta.dirname, 'command.ts'), ...command], + { + cwd, + allowFailure: true, + detached: process.platform !== 'win32', + kill: { signal: 'SIGTERM', graceMs: 5000 }, + signal: run.controller.signal, + stdin: 'run\n', + onSpawn: (child) => { + commandPid = child.pid; + if (commandPid) recordValidationCommand(run.receiptPath, commandPid); + }, + env: { + ...process.env, + [VALIDATION_PARENT_ENV]: run.receiptPath, + AGENT_DEVICE_VALIDATION_REPORT: reportPrefix, + }, + onStdoutChunk: (chunk) => { + fs.writeSync(descriptor, chunk); + }, + onStderrChunk: (chunk) => { + fs.writeSync(descriptor, chunk); + }, + }, + ); + result.exitCode = output.exitCode; + result.status = output.exitCode === 0 ? 'passed' : 'failed'; + run.report(`[${result.status}] ${result.id}; exit=${output.exitCode}`); + if (output.exitCode !== 0) reportFailureTail(logPath, run.report); + return output.exitCode; + } catch (error) { + result.status = run.controller.signal.aborted ? 'cancelled' : 'failed'; + result.reason = error instanceof Error ? error.message : String(error); + throw error; + } finally { + clearInterval(heartbeat); + fs.closeSync(descriptor); + result.durationMs = Date.now() - started; + collectTestReports(result, run.directory, reportPrefix); + run.save(); + await finishValidationCommand(commandPid); + } +} + +function reportFailureTail(logPath: string, report: (line: string) => void): void { + report(fs.readFileSync(logPath, 'utf8').slice(-8000).split('\n').slice(-35).join('\n')); +} + +function collectTestReports(result: CheckResult, directory: string, reportPrefix: string): void { + try { + result.testReportPaths = fs + .readdirSync(directory) + .filter( + (file) => file.startsWith(path.basename(reportPrefix) + '.') && file.endsWith('.json'), + ) + .map((file) => path.join(directory, file)); + result.tests = result.testReportPaths.map((reportPath) => { + const report = JSON.parse(fs.readFileSync(reportPath, 'utf8')) as TestReport; + return { + ...report, + failures: report.failures.slice(0, 20).map((failure) => ({ + ...failure, + errors: failure.errors.map((error) => error.slice(0, 2000)), + })), + omittedFailures: Math.max(0, report.failures.length - 20), + }; + }); + } catch (error) { + result.testReportError = error instanceof Error ? error.message : String(error); + } +} diff --git a/scripts/check-affected/command.test.ts b/scripts/check-affected/command.test.ts new file mode 100644 index 0000000000..2f8d8d4e0b --- /dev/null +++ b/scripts/check-affected/command.test.ts @@ -0,0 +1,29 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { test } from 'node:test'; +import { runCmdSync } from '@agent-device/host-kit/command'; + +test('command cannot execute before activation and preserves the activated exit code', () => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'validation-activation-')); + const marker = path.join(directory, 'started'); + const args = [ + '--experimental-strip-types', + path.join(import.meta.dirname, 'command.ts'), + process.execPath, + '-e', + `require('node:fs').writeFileSync(${JSON.stringify(marker)}, 'yes'); process.exit(7);`, + ]; + try { + assert.equal(runCmdSync(process.execPath, args, { allowFailure: true }).exitCode, 1); + assert.equal(fs.existsSync(marker), false); + assert.equal( + runCmdSync(process.execPath, args, { allowFailure: true, stdin: 'run\n' }).exitCode, + 7, + ); + assert.equal(fs.readFileSync(marker, 'utf8'), 'yes'); + } finally { + fs.rmSync(directory, { recursive: true, force: true }); + } +}); diff --git a/scripts/check-affected/command.ts b/scripts/check-affected/command.ts new file mode 100644 index 0000000000..5971488deb --- /dev/null +++ b/scripts/check-affected/command.ts @@ -0,0 +1,21 @@ +import { runCmdStreaming } from '@agent-device/host-kit/command'; +import { runEntrypoint } from '../lib/cli-entrypoint.ts'; + +// The parent publishes group ownership before sending activation. EOF before activation cannot start a check. +runEntrypoint('validation command', async () => { + let activation = ''; + for await (const chunk of process.stdin) activation += String(chunk); + if (activation !== 'run\n') return 1; + const [command, ...args] = process.argv.slice(2); + if (!command) return 1; + const result = await runCmdStreaming(command, args, { + allowFailure: true, + onStdoutChunk: (chunk) => { + process.stdout.write(chunk); + }, + onStderrChunk: (chunk) => { + process.stderr.write(chunk); + }, + }); + return result.exitCode; +}); diff --git a/scripts/check-affected/execution.fixtures.ts b/scripts/check-affected/execution.fixtures.ts new file mode 100644 index 0000000000..9f3e9d42c6 --- /dev/null +++ b/scripts/check-affected/execution.fixtures.ts @@ -0,0 +1,46 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { pathToFileURL } from 'node:url'; +import { runCmdSync } from '@agent-device/host-kit/command'; +import type { ExecutionReceipt } from './execution.ts'; + +export const entry = pathToFileURL(path.join(import.meta.dirname, 'execution.ts')).href; + +export function fixture() { + const cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'validation-execution-')); + const git = (...args: string[]) => runCmdSync('git', args, { cwd }).stdout.trim(); + git('init', '-q'); + git('config', 'user.email', 'test@example.com'); + git('config', 'user.name', 'Test'); + fs.writeFileSync(path.join(cwd, '.gitignore'), '.tmp\n.cache\n'); + fs.writeFileSync(path.join(cwd, 'source.txt'), 'base'); + git('add', '.'); + git('commit', '-qm', 'base'); + const sha = git('rev-parse', 'HEAD'); + return { cwd, git, sha }; +} + +export function run(cwd: string, head: string, body: string) { + const result = runCmdSync( + process.execPath, + [ + '--experimental-strip-types', + '--input-type=module', + '-e', + ` + import { runValidation } from ${JSON.stringify(entry)}; + const code = await runValidation(${JSON.stringify(cwd)}, { base: 'HEAD', head: ${JSON.stringify(head)}, json: true }, async ({execute, receipt}) => { + ${body} + }); + process.exitCode = code; + `, + ], + { + cwd, + allowFailure: true, + env: { ...process.env, HOME: cwd, AGENT_DEVICE_VALIDATION_PARENT: '' }, + }, + ); + return { ...result, receipt: JSON.parse(result.stdout) as ExecutionReceipt }; +} diff --git a/scripts/check-affected/execution.test.ts b/scripts/check-affected/execution.test.ts new file mode 100644 index 0000000000..67bdefbf0d --- /dev/null +++ b/scripts/check-affected/execution.test.ts @@ -0,0 +1,63 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { test } from 'node:test'; +import { fixture, run } from './execution.fixtures.ts'; + +test('execution JSON contains exact checkout, command, failure exit, and bounded terminal output', () => { + const { cwd, sha } = fixture(); + try { + const result = run( + cwd, + sha, + `return execute([process.execPath, '-e', "console.log('x'.repeat(20000)); process.exit(7)"], ${JSON.stringify(cwd)}, 'fixture');`, + ); + assert.equal(result.exitCode, 7); + assert.equal(result.receipt.status, 'failed'); + assert.equal(result.receipt.exactCommit, true); + assert.equal(result.receipt.initial?.head, sha); + assert.equal(result.receipt.final?.head, sha); + assert.equal(result.receipt.results[0]?.exitCode, 7); + assert.ok(result.stderr.length < 10000); + assert.ok(fs.readFileSync(result.receipt.results[0]!.logPath!, 'utf8').length > 20000); + } finally { + fs.rmSync(cwd, { recursive: true, force: true }); + } +}); + +test('dirty local feedback cannot certify a commit; explicit head refuses dirty checkout', () => { + const { cwd, sha } = fixture(); + try { + fs.writeFileSync(path.join(cwd, 'source.txt'), 'dirty'); + const local = run(cwd, 'HEAD', 'return 0;'); + assert.equal(local.exitCode, 0); + assert.equal(local.receipt.exactCommit, false); + const exact = run(cwd, sha, 'throw new Error("must not run");'); + assert.equal(exact.exitCode, 1); + assert.match(exact.receipt.error!, /clean checkout/); + assert.deepEqual(exact.receipt.results, []); + } finally { + fs.rmSync(cwd, { recursive: true, force: true }); + } +}); + +test('mismatched head is rejected and checkout drift invalidates successful checks', () => { + const { cwd, sha, git } = fixture(); + try { + fs.writeFileSync(path.join(cwd, 'source.txt'), 'new commit'); + git('add', '.'); + git('commit', '-qm', 'head'); + const mismatch = run(cwd, sha, 'return 0;'); + assert.equal(mismatch.exitCode, 1); + const drift = run( + cwd, + 'HEAD', + `const fs = await import('node:fs'); fs.writeFileSync('source.txt', 'drift'); return 0;`, + ); + assert.equal(drift.exitCode, 1); + assert.equal(drift.receipt.checkoutChanged, true); + assert.equal(drift.receipt.exactCommit, false); + } finally { + fs.rmSync(cwd, { recursive: true, force: true }); + } +}); diff --git a/scripts/check-affected/execution.ts b/scripts/check-affected/execution.ts new file mode 100644 index 0000000000..0a2161c367 --- /dev/null +++ b/scripts/check-affected/execution.ts @@ -0,0 +1,220 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { executeValidationCommand } from './command-execution.ts'; +import { acquireValidationLock } from './host-lock.ts'; +import { gitValue, readCheckoutState, type CheckoutState } from './checkout-state.ts'; +import type { TestReport } from './test-report.ts'; +import type { CheckId, CheckPlan } from './model.ts'; + +export type CheckResult = { + id: string; + status: 'not-run' | 'running' | 'passed' | 'failed' | 'cancelled' | 'skipped' | 'deduplicated'; + command?: string[]; + exitCode?: number; + durationMs?: number; + logPath?: string; + reason?: string; + testReportPaths?: string[]; + testReportError?: string; + tests?: (Omit & { + failures: TestReport['failures']; + omittedFailures: number; + })[]; +}; + +export type ExecutionReceipt = { + version: 1; + cwd: string; + startedAt: string; + finishedAt?: string; + status: 'waiting' | 'running' | 'passed' | 'failed' | 'cancelled'; + exitCode: number; + refs: { + base: string; + head: string; + resolvedBase?: string; + resolvedHead?: string; + mergeBase?: string; + }; + initial?: CheckoutState; + final?: CheckoutState; + checkoutChanged: boolean; + exactCommit: boolean; + plan?: CheckPlan; + changedFiles?: string[]; + results: CheckResult[]; + error?: string; +}; + +export type ValidationContext = { + receipt: ExecutionReceipt; + execute: (command: string[], cwd: string, id: string) => Promise; + skip: (id: CheckId, reason: string, deduplicated: boolean) => void; +}; + +export async function runValidation( + cwd: string, + args: { base: string; head: string; json: boolean; full?: boolean }, + task: (context: ValidationContext) => Promise, +): Promise { + const run = openValidationRun(cwd, args); + const { receipt, controller, save, report, receiptPath, cancellation } = run; + let release: (() => Promise) | undefined; + save(); + report(`[receipt] ${receiptPath}`); + + try { + release = await acquireValidationLock(controller.signal); + prepareCheckout(cwd, args, receipt); + receipt.status = 'running'; + save(); + + receipt.exitCode = await task({ + receipt, + execute: (command, commandCwd, id) => + executeValidationCommand(run, checkResult(receipt, id), command, commandCwd), + skip: (id, reason, deduplicated) => { + Object.assign(checkResult(receipt, id), { + status: deduplicated ? 'deduplicated' : 'skipped', + reason, + }); + save(); + }, + }); + } catch (error) { + receipt.error = error instanceof Error ? error.message : String(error); + report(`[error] ${receipt.error}`); + receipt.exitCode = cancellation.code || 1; + } finally { + await closeValidationRun(run, cwd, release); + } + report( + `[${receipt.status}] exactCommit=${receipt.exactCommit}; receipt=${receiptPath}${receipt.error ? `; ${receipt.error}` : ''}`, + ); + if (args.json) process.stdout.write(`${JSON.stringify(receipt, null, 2)}\n`); + return receipt.exitCode; +} + +type ValidationArgs = { base: string; head: string; json: boolean; full?: boolean }; +export type ValidationRun = ReturnType; + +function openValidationRun(cwd: string, args: ValidationArgs) { + const root = path.join(cwd, '.tmp', 'validation'); + fs.mkdirSync(root, { recursive: true }); + const directory = fs.mkdtempSync(path.join(root, 'run-')); + const receiptPath = path.join(directory, 'receipt.json'); + const receipt: ExecutionReceipt = { + version: 1, + cwd, + startedAt: new Date().toISOString(), + status: 'waiting', + exitCode: 1, + refs: { base: args.base, head: args.head }, + checkoutChanged: false, + exactCommit: false, + results: [], + }; + const save = () => fs.writeFileSync(receiptPath, `${JSON.stringify(receipt, null, 2)}\n`); + const report = (line: string) => process.stderr.write(`${line}\n`); + const controller = new AbortController(); + const cancellation = { code: 0 }; + const interrupt = () => { + cancellation.code = 130; + controller.abort(); + }; + const terminate = () => { + cancellation.code = 143; + controller.abort(); + }; + const hangup = () => { + cancellation.code = 129; + controller.abort(); + }; + process.on('SIGHUP', hangup); + process.on('SIGINT', interrupt); + process.on('SIGTERM', terminate); + return { + receipt, + directory, + receiptPath, + controller, + cancellation, + save, + report, + dispose: () => { + process.off('SIGHUP', hangup); + process.off('SIGINT', interrupt); + process.off('SIGTERM', terminate); + }, + }; +} + +function prepareCheckout(cwd: string, args: ValidationArgs, receipt: ExecutionReceipt): void { + receipt.initial = readCheckoutState(cwd); + if (!args.full) receipt.refs.resolvedBase = gitValue(cwd, ['rev-parse', `${args.base}^{commit}`]); + receipt.refs.resolvedHead = gitValue(cwd, ['rev-parse', `${args.head}^{commit}`]); + if (receipt.refs.resolvedBase) + receipt.refs.mergeBase = gitValue(cwd, [ + 'merge-base', + receipt.refs.resolvedBase, + receipt.refs.resolvedHead, + ]); + if ( + receipt.refs.resolvedHead !== receipt.initial.head || + (args.head !== 'HEAD' && receipt.initial.status !== '') + ) { + throw new Error( + '--head must name the checked-out commit. An explicit ref also requires a clean checkout.', + ); + } +} + +function finishCheckout(cwd: string, receipt: ExecutionReceipt): void { + if (receipt.initial) { + receipt.final = readCheckoutState(cwd); + receipt.checkoutChanged = receipt.initial.fingerprint !== receipt.final.fingerprint; + receipt.exactCommit = + !receipt.initial.status && + !receipt.final.status && + !receipt.checkoutChanged && + receipt.initial.head === receipt.refs.resolvedHead; + if (receipt.checkoutChanged) { + receipt.exitCode = 1; + receipt.error = 'Checkout changed during validation. Run again after edits stop.'; + } + } +} + +function checkResult(receipt: ExecutionReceipt, id: string): CheckResult { + let result = receipt.results.find((value) => value.id === id); + if (!result) { + result = { id, status: 'not-run' }; + receipt.results.push(result); + } + return result; +} + +async function closeValidationRun( + run: ValidationRun, + cwd: string, + release: (() => Promise) | undefined, +): Promise { + const { receipt, cancellation, save } = run; + try { + finishCheckout(cwd, receipt); + } catch (error) { + receipt.exitCode = 1; + receipt.error = error instanceof Error ? error.message : String(error); + } + try { + await release?.(); + } catch (error) { + receipt.exitCode = 1; + receipt.error = error instanceof Error ? error.message : String(error); + } + run.dispose(); + receipt.exitCode = cancellation.code || receipt.exitCode; + receipt.status = cancellation.code ? 'cancelled' : receipt.exitCode === 0 ? 'passed' : 'failed'; + receipt.finishedAt = new Date().toISOString(); + save(); +} diff --git a/scripts/check-affected/host-lock.test.ts b/scripts/check-affected/host-lock.test.ts new file mode 100644 index 0000000000..8bd5784058 --- /dev/null +++ b/scripts/check-affected/host-lock.test.ts @@ -0,0 +1,45 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { test } from 'node:test'; +import { acquireValidationLock, VALIDATION_PARENT_ENV } from './host-lock.ts'; + +test('host lock excludes a waiter, supports cancellation, then admits it after release', async () => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'validation-lock-')); + const lock = path.join(directory, 'gate.lock'); + const parent = process.env[VALIDATION_PARENT_ENV]; + delete process.env[VALIDATION_PARENT_ENV]; + const controller = new AbortController(); + try { + const release = await acquireValidationLock(controller.signal, lock); + const waiting = acquireValidationLock(controller.signal, lock, (message) => { + assert.match(message, /wait.*host validation/); + controller.abort(); + }); + await assert.rejects(waiting, { name: 'AbortError' }); + assert.ok(fs.existsSync(lock), 'cancelled waiter must not release the owner'); + await release(); + const second = await acquireValidationLock(new AbortController().signal, lock); + await second(); + assert.equal(fs.existsSync(lock), false); + } finally { + if (parent === undefined) delete process.env[VALIDATION_PARENT_ENV]; + else process.env[VALIDATION_PARENT_ENV] = parent; + fs.rmSync(directory, { recursive: true, force: true }); + } +}); + +test('nested validation fails before acquisition', async () => { + const parent = process.env[VALIDATION_PARENT_ENV]; + process.env[VALIDATION_PARENT_ENV] = '/parent/receipt.json'; + try { + await assert.rejects( + acquireValidationLock(new AbortController().signal), + /Nested validation.*parent\/receipt/, + ); + } finally { + if (parent === undefined) delete process.env[VALIDATION_PARENT_ENV]; + else process.env[VALIDATION_PARENT_ENV] = parent; + } +}); diff --git a/scripts/check-affected/host-lock.ts b/scripts/check-affected/host-lock.ts new file mode 100644 index 0000000000..5595a2cb44 --- /dev/null +++ b/scripts/check-affected/host-lock.ts @@ -0,0 +1,127 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { setTimeout } from 'node:timers/promises'; +import { + publishFileSync, + inspectProcessLock, + tryAcquireProcessLock, + type ProcessLockRelease, +} from '@agent-device/host-kit/file'; +import { + isProcessGroupAlive, + readCurrentOwnerIdentity, + readProcessStartTime, + ownerIdentityDiffers, + signalProcessGroupBestEffort, +} from '@agent-device/host-kit/process'; + +export const VALIDATION_PARENT_ENV = 'AGENT_DEVICE_VALIDATION_PARENT'; +const HOST_LOCK_PATH = path.join(os.homedir(), '.cache', 'agent-device', 'validation.lock'); + +export async function acquireValidationLock( + signal: AbortSignal, + lockDirPath = HOST_LOCK_PATH, + report: (message: string) => void = (message) => process.stderr.write(`${message}\n`), +): Promise { + if (process.env[VALIDATION_PARENT_ENV]) { + throw new Error( + `Nested validation is not allowed. Parent receipt: ${process.env[VALIDATION_PARENT_ENV]}`, + ); + } + const owner = { ...readCurrentOwnerIdentity(), acquiredAtMs: Date.now() }; + const deadline = Date.now() + 2 * 60 * 60 * 1000; + let nextReport = 0; + while (true) { + signal.throwIfAborted(); + const activePath = `${lockDirPath}.active.json`; + const orphan = readOrphanCommand(lockDirPath); + const attempt = orphan + ? { status: 'busy' as const, inspection: { orphan } } + : tryAcquireProcessLock({ lockDirPath, owner, description: 'host validation' }); + if (attempt.status === 'acquired') { + fs.rmSync(activePath, { force: true }); + return attempt.acquisition.release; + } + if (Date.now() >= deadline) + throw new Error(`Host validation wait exceeded 2 hours: ${lockDirPath}`); + if (Date.now() >= nextReport) { + report( + `[wait] ${orphan ? 'orphan validation group' : 'host validation'}: ${JSON.stringify(attempt.inspection)}; Ctrl-C cancels this waiter`, + ); + nextReport = Date.now() + 15_000; + } + await setTimeout(500, undefined, { signal }); + } +} + +export function recordValidationCommand(receipt: string, pid: number | null): void { + publishFileSync({ + destination: `${HOST_LOCK_PATH}.active.json`, + contents: JSON.stringify({ + receipt, + pid, + startTime: pid === null ? null : readProcessStartTime(pid), + activation: 'stdin', + }), + }); +} + +export async function finishValidationCommand(pid: number | undefined): Promise { + if (pid !== undefined && process.platform !== 'win32') { + const deadline = Date.now() + 10_000; + while (isProcessGroupAlive(pid)) { + if (Date.now() >= deadline) + throw new Error( + `Validation command group ${pid} has not exited; host validation remains blocked.`, + ); + if (Date.now() >= deadline - 5000) signalProcessGroupBestEffort(pid, 'SIGKILL'); + await setTimeout(100); + } + } + fs.rmSync(`${HOST_LOCK_PATH}.active.json`, { force: true }); +} + +function readOrphanCommand(lockDirPath: string): { pid: number; receipt: string } | null { + const activePath = `${lockDirPath}.active.json`; + const inspection = inspectProcessLock(lockDirPath); + if ( + inspection.state === 'held' && + (inspection.liveness === 'live' || inspection.liveness === 'unknown') + ) + return null; + const active = readActiveCommand(activePath); + if (!active) return null; + const current = inspectProcessLock(lockDirPath); + if (current.state === 'held' && (current.liveness === 'live' || current.liveness === 'unknown')) + return null; + if (active.pid === null) return null; + const observed = { pid: active.pid, startTime: readProcessStartTime(active.pid) }; + if (ownerIdentityDiffers({ pid: active.pid, startTime: active.startTime }, observed)) return null; + return isProcessGroupAlive(active.pid) ? { pid: active.pid, receipt: active.receipt } : null; +} + +function readActiveCommand( + activePath: string, +): { pid: number | null; startTime: string | null; receipt: string } | null { + let contents: string; + try { + contents = fs.readFileSync(activePath, 'utf8'); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return null; + throw error; + } + const active = JSON.parse(contents) as { + pid: number | null; + startTime: string | null; + receipt: string; + activation: string; + }; + if ( + active.activation !== 'stdin' || + (active.pid !== null && (!Number.isInteger(active.pid) || active.pid <= 0)) + ) { + throw new Error(`Cannot verify validation command ownership: ${activePath}`); + } + return active; +} diff --git a/scripts/check-affected/run-cli.test.ts b/scripts/check-affected/run-cli.test.ts new file mode 100644 index 0000000000..27dd5f5676 --- /dev/null +++ b/scripts/check-affected/run-cli.test.ts @@ -0,0 +1,81 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { test } from 'node:test'; +import { runCmdSync } from '@agent-device/host-kit/command'; +import type { ExecutionReceipt } from './execution.ts'; + +const root = path.resolve(import.meta.dirname, '../..'); +const runner = path.join(import.meta.dirname, 'run.ts'); + +function fixture() { + const cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'validation-cli-')); + const git = (...args: string[]) => runCmdSync('git', args, { cwd }).stdout.trim(); + git('init', '-q'); + git('config', 'user.email', 'test@example.com'); + git('config', 'user.name', 'Test'); + const pkg = JSON.parse(fs.readFileSync(path.join(root, 'package.json'), 'utf8')); + pkg.scripts['check:agent-guidance'] = 'node -e "process.exit(7)"'; + for (const script of ['check:tooling', 'check:fallow', 'check:unit']) + pkg.scripts[script] = 'node -e "process.exit(0)"'; + fs.writeFileSync(path.join(cwd, 'package.json'), JSON.stringify(pkg)); + fs.writeFileSync(path.join(cwd, '.gitignore'), '.tmp\n.cache\n'); + fs.writeFileSync(path.join(cwd, 'AGENTS.md'), 'base'); + git('add', '.'); + git('commit', '-qm', 'base'); + const base = git('rev-parse', 'HEAD'); + fs.writeFileSync(path.join(cwd, 'AGENTS.md'), 'head'); + git('add', '.'); + git('commit', '-qm', 'head'); + return { cwd, base }; +} + +function invoke(cwd: string, args: string[]) { + const result = runCmdSync( + process.execPath, + ['--experimental-strip-types', runner, '--run', '--json', ...args], + { + cwd, + allowFailure: true, + env: { + ...process.env, + HOME: path.join(cwd, '.tmp', 'home'), + AGENT_DEVICE_VALIDATION_PARENT: '', + }, + }, + ); + return { ...result, receipt: JSON.parse(result.stdout) as ExecutionReceipt }; +} + +test('CLI JSON contains execution plan and preserves the failing command exit code', () => { + const { cwd, base } = fixture(); + try { + const result = invoke(cwd, ['--base', base]); + assert.equal(result.exitCode, 7, result.stderr); + assert.equal(result.receipt.status, 'failed'); + assert.equal(result.receipt.exactCommit, true); + assert.deepEqual(result.receipt.changedFiles, ['AGENTS.md']); + assert.deepEqual(result.receipt.plan?.checks, ['agent-guidance']); + assert.equal(result.receipt.results[0]?.exitCode, 7); + assert.equal(result.receipt.results[0]?.status, 'failed'); + } finally { + fs.rmSync(cwd, { recursive: true, force: true }); + } +}); + +test('full CLI works without origin/main and still blocks a missing install', () => { + const { cwd } = fixture(); + try { + const full = invoke(cwd, ['--full']); + assert.equal(full.exitCode, 0, full.stderr); + assert.equal(full.receipt.results[0]?.id, 'check:tooling'); + fs.writeFileSync(path.join(cwd, 'pnpm-lock.yaml'), 'lockfileVersion: 9'); + const stale = invoke(cwd, ['--full']); + assert.equal(stale.exitCode, 1); + assert.match(stale.stderr, /node_modules does not match/); + assert.deepEqual(stale.receipt.results, []); + } finally { + fs.rmSync(cwd, { recursive: true, force: true }); + } +}); diff --git a/scripts/check-affected/run.ts b/scripts/check-affected/run.ts index 6387d4535e..08ead90aaf 100644 --- a/scripts/check-affected/run.ts +++ b/scripts/check-affected/run.ts @@ -14,6 +14,7 @@ import { STALE_NODE_MODULES_MESSAGE, type LockfileInstallSyncResult, } from './lockfile-install-sync.ts'; +import { runValidation, type ValidationContext } from './execution.ts'; import { parseScriptArgs } from '../lib/cli-args.ts'; import { runEntrypoint } from '../lib/cli-entrypoint.ts'; import { @@ -34,11 +35,19 @@ function ciJobsByCheck(): Map { return owningLanes(loadModel(repoRoot, [])); } -type Args = { base: string; head: string; json: boolean; run: boolean }; +type Args = { + base: string; + head: string; + json: boolean; + run: boolean; + verbose?: boolean; + full?: boolean; +}; const repoRoot = runCmdSync('git', ['rev-parse', '--show-toplevel']).stdout.trim(); -const USAGE = 'Usage: pnpm check:affected [--base ] [--head ] [--json] [--run]\n'; +const USAGE = + 'Usage: pnpm check:affected [--base ] [--head ] [--json] [--run] [--verbose] [--full]\n'; function parseArgs(argv: readonly string[]): Args { const values = parseScriptArgs(argv, USAGE, { @@ -46,12 +55,16 @@ function parseArgs(argv: readonly string[]): Args { head: { type: 'string', default: 'HEAD' }, json: { type: 'boolean', default: false }, run: { type: 'boolean', default: false }, + verbose: { type: 'boolean', default: false }, + full: { type: 'boolean', default: false }, }); return { base: values.base ?? 'origin/main', head: values.head ?? 'HEAD', json: Boolean(values.json), run: Boolean(values.run), + verbose: Boolean(values.verbose), + full: Boolean(values.full), }; } @@ -173,7 +186,7 @@ function printPlanHuman(plan: CheckPlan, args: Args): void { // How a resolved command is executed. Injectable so the entrypoint's `--run` // propagation (order, skip of GitHub-authoritative checks, stop-on-failure) is // testable without spawning real processes. -export type CommandExecutor = (command: string[], cwd: string) => Promise; +export type CommandExecutor = (command: string[], cwd: string, id: string) => Promise; const streamingExecutor: CommandExecutor = async (command, cwd) => { const result = await runCmdStreaming(command[0]!, command.slice(1), { @@ -191,6 +204,7 @@ export async function runChecks( args: Args, options: { cwd?: string; + skip?: ValidationContext['skip']; execute?: CommandExecutor; changedFiles?: readonly string[]; checkLockfileSync?: (cwd: string) => LockfileInstallSyncResult; @@ -210,22 +224,24 @@ export async function runChecks( const relatedSelected = plan.checks.includes('vitest-related'); const ciJobs = skipped.length > 0 ? ciJobsByCheck() : new Map(); for (const spec of skipped) { - process.stdout.write(`\n[skip] ${spec.id} — ${describeOwner(spec.id, ciJobs)}\n`); + const reason = describeOwner(spec.id, ciJobs); + options.skip?.(spec.id, reason, false); + process.stderr.write(`[skip] ${spec.id} — ${reason}\n`); } for (const spec of runnable) { if (isCoveredByRelatedTests(spec, relatedSelected)) { - process.stdout.write(`\n[dedupe] ${spec.id} — covered by related tests or GitHub CI\n`); + options.skip?.(spec.id, 'covered by related tests', true); continue; } const command = resolveCommand(spec, pkg.scripts, args.base, options.changedFiles ?? []); - process.stdout.write(`\n[run] ${spec.id}: ${command.join(' ')}\n`); - const exitCode = await execute(command, cwd); + if (!options.execute) process.stderr.write(`[run] ${spec.id}: ${command.join(' ')}\n`); + const exitCode = await execute(command, cwd, spec.id); if (exitCode !== 0) { process.stderr.write(`\ncheck:affected: ${spec.id} failed.\n`); - return 1; + return exitCode; } } - process.stdout.write('\ncheck:affected: all runnable checks passed.\n'); + process.stderr.write('check:affected: all runnable checks passed.\n'); return 0; } @@ -255,20 +271,59 @@ function isCoveredByRelatedTests(spec: CheckSpec, relatedSelected: boolean): boo async function main(argv = process.argv.slice(2)): Promise { assertCatalogComplete(); const args = parseArgs(argv); + if (args.full && !args.run) throw new Error('--full requires --run'); const pkg = loadPackageJson(); // Validate every catalog command resolves before selecting, so a broken // catalog fails loudly rather than silently dropping a gate. for (const spec of CHECK_CATALOG) resolveCommand(spec, pkg.scripts, args.base); - const changedFiles = readChangedFiles(args.base, args.head); - const plan = selectChecks({ - changedFiles, - packageEntryFiles: packageEntryFiles(pkg), - }); - + const prepare = (base = args.base, currentPackage = pkg) => { + const changedFiles = readChangedFiles(base, args.head); + return { + changedFiles, + plan: selectChecks({ changedFiles, packageEntryFiles: packageEntryFiles(currentPackage) }), + }; + }; + if (args.run) + return await runValidation(repoRoot, args, async ({ receipt, execute, skip }) => { + const currentPackage = loadPackageJson(); + for (const spec of CHECK_CATALOG) resolveCommand(spec, currentPackage.scripts, args.base); + const install = checkLockfileInstallSync(repoRoot); + if (install.status === 'out-of-sync') { + reportStaleInstall(install.reason, repoRoot); + receipt.error = `${STALE_NODE_MODULES_MESSAGE}: ${install.reason}`; + return 1; + } + if (args.full) { + const scripts = ['check:tooling', 'check:fallow', 'check:unit']; + receipt.results = scripts.map((id) => ({ id, status: 'not-run' })); + for (const script of scripts) { + const code = await execute(['pnpm', 'run', script], repoRoot, script); + if (code !== 0) return code; + } + return 0; + } + const { changedFiles, plan } = prepare(receipt.refs.resolvedBase, currentPackage); + receipt.changedFiles = changedFiles; + receipt.plan = plan; + receipt.results = plan.checks.map((id) => ({ id, status: 'not-run' })); + process.stderr.write( + `Selected ${plan.checks.length} checks${plan.failOpen ? '; FAIL-OPEN: full set (see receipt.plan.failOpenReasons)' : ''}.\n`, + ); + return await runChecks( + plan, + currentPackage, + { ...args, base: receipt.refs.resolvedBase! }, + { changedFiles, execute, skip }, + ); + }); + const { plan } = prepare(); if (args.json) printPlanJson(plan, args); - else printPlanHuman(plan, args); - - if (args.run) return await runChecks(plan, pkg, args, { changedFiles }); + else if (args.verbose) printPlanHuman(plan, args); + else { + writeLine(`Selected ${plan.checks.length} checks: ${plan.checks.join(', ') || 'none'}`); + if (plan.failOpen) printFailOpen(plan); + writeLine('Use --verbose for selection reasons, --json for the plan, --run for execution.'); + } return 0; } diff --git a/scripts/check-affected/test-report.test.ts b/scripts/check-affected/test-report.test.ts new file mode 100644 index 0000000000..b696331cc8 --- /dev/null +++ b/scripts/check-affected/test-report.test.ts @@ -0,0 +1,67 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { pathToFileURL } from 'node:url'; +import { test } from 'node:test'; +import { runCmdSync } from '@agent-device/host-kit/command'; +import type { TestReport } from './test-report.ts'; + +test('real Vitest report preserves assertion, suite, and unhandled failures separately', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'validation-report-')); + const reportPath = path.join(root, 'report.json'); + const reporter = pathToFileURL(path.join(import.meta.dirname, 'test-report.ts')).href; + try { + fs.writeFileSync( + path.join(root, 'vitest.config.ts'), + ` + import { validationTestReporter } from ${JSON.stringify(reporter)}; + export default { test: { root: ${JSON.stringify(root)}, maxWorkers: 1, + reporters: ['dot', validationTestReporter(${JSON.stringify(reportPath)})] } }; + `, + ); + fs.writeFileSync( + path.join(root, 'cases.test.js'), + ` + import { test, expect, describe, beforeAll } from 'vitest'; + test('passes', () => expect(1).toBe(1)); + test('assertion', () => expect(1).toBe(2)); + test('unhandled', async () => { Promise.reject(new Error('fixture-unhandled')); await new Promise(r => setTimeout(r, 10)); }); + describe('suite', () => { beforeAll(() => { throw new Error('fixture-suite'); }); test('not reached', () => {}); }); + `, + ); + const result = runCmdSync( + 'pnpm', + ['exec', 'vitest', 'run', '--config', path.join(root, 'vitest.config.ts')], + { + cwd: path.resolve(import.meta.dirname, '../..'), + allowFailure: true, + env: { ...process.env, AGENT_DEVICE_VALIDATION_REPORT: '' }, + }, + ); + assert.equal(result.exitCode, 1); + assert.ok(fs.existsSync(reportPath), result.stderr + result.stdout); + const report = JSON.parse(fs.readFileSync(reportPath, 'utf8')) as TestReport; + assert.equal(report.passed, 2); + assert.ok(report.failed >= 1); + assert.equal(report.unhandledErrors, 1); + assert.ok( + report.failures.some( + (failure) => failure.kind === 'test' && failure.test?.includes('assertion'), + ), + ); + assert.ok( + report.failures.some( + (failure) => failure.kind === 'suite' && failure.errors.join().includes('fixture-suite'), + ), + ); + assert.ok( + report.failures.some( + (failure) => + failure.kind === 'unhandled' && failure.errors.join().includes('fixture-unhandled'), + ), + ); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +}); diff --git a/scripts/check-affected/test-report.ts b/scripts/check-affected/test-report.ts new file mode 100644 index 0000000000..d5e06c82cb --- /dev/null +++ b/scripts/check-affected/test-report.ts @@ -0,0 +1,61 @@ +import fs from 'node:fs'; +import type { Reporter } from 'vitest/node'; + +export type TestReport = { + passed: number; + failed: number; + skipped: number; + pending: number; + failedModules: number; + unhandledErrors: number; + failures: { + kind: 'test' | 'suite' | 'unhandled'; + file?: string; + test?: string; + errors: string[]; + }[]; +}; + +export function validationTestReporter(destination: string | undefined): Reporter { + return { + onTestRunEnd(modules, unhandledErrors) { + if (!destination) return; + const report: TestReport = { + passed: 0, + failed: 0, + skipped: 0, + pending: 0, + failedModules: modules.filter((module) => !module.ok()).length, + unhandledErrors: unhandledErrors.length, + failures: [], + }; + const messages = (errors: readonly { stack?: string; message?: string }[]) => + errors.map((error) => error.stack || error.message || String(error)); + for (const module of modules) { + for (const suite of [module, ...module.children.allSuites()]) { + const errors = suite.errors(); + if (errors.length) + report.failures.push({ + kind: 'suite', + file: module.moduleId, + errors: messages(errors), + }); + } + for (const test of module.children.allTests()) { + const result = test.result(); + report[result.state]++; + if (result.state === 'failed') + report.failures.push({ + kind: 'test', + file: module.moduleId, + test: test.fullName, + errors: messages(result.errors), + }); + } + } + for (const error of unhandledErrors) + report.failures.push({ kind: 'unhandled', errors: messages([error]) }); + fs.writeFileSync(destination, `${JSON.stringify(report, null, 2)}\n`); + }, + }; +} diff --git a/vitest.config.ts b/vitest.config.ts index 118acf1ca3..5d5d2a8851 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,8 +1,12 @@ import { defineConfig } from 'vitest/config'; +import { validationTestReporter } from './scripts/check-affected/test-report.ts'; import { resolveVitestMaxWorkers } from './scripts/lib/vitest-concurrency.ts'; import coverageRunStampReporter from './scripts/vitest-coverage-run-stamp-reporter.ts'; import slowTestGateReporter from './scripts/vitest-slow-test-reporter.ts'; +const validationReport = process.env.AGENT_DEVICE_VALIDATION_REPORT; +delete process.env.AGENT_DEVICE_VALIDATION_REPORT; + // A real per-case spawn is timeout noise under thousands of mutant reruns, so the // mutation lane excludes these tests even though the unit lane runs them normally. const MUTATION_EXCLUDED_SUBPROCESS_TESTS: readonly string[] = [ @@ -105,7 +109,10 @@ export default defineConfig({ // Gate reporters for every lane; a `--reporter` flag would replace them, so no lane passes one. // The stamp reporter only acts on a lane that ran coverage — see its module. reporters: [ - 'default', + 'dot', + validationTestReporter( + validationReport ? `${validationReport}.${process.pid}.json` : undefined, + ), slowTestGateReporter(), coverageRunStampReporter(COVERAGE_UNINSTRUMENTED_PROJECTS), ], From eb97c76ccd1715ddb4535a8d55536ea8d3227c27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 11:17:06 +0200 Subject: [PATCH 2/8] docs(agents): route tasks and clarify validation boundaries --- AGENTS.md | 163 ++++++---------------- CONTRIBUTING.md | 5 +- docs/agents/code-ownership.md | 42 ++++++ docs/agents/device-foldable.md | 24 ++++ docs/agents/device-verification.md | 128 ++++++----------- docs/agents/gates.md | 21 +++ docs/agents/pull-requests.md | 135 ++++++------------ docs/agents/review.md | 28 ++++ docs/agents/selector-capture.md | 10 ++ docs/agents/test-audit.md | 70 ++++------ docs/agents/test-authoring.md | 47 +++++++ docs/agents/test-experiments.md | 44 ++++++ docs/agents/testing.md | 213 ++++++----------------------- 13 files changed, 408 insertions(+), 522 deletions(-) create mode 100644 docs/agents/code-ownership.md create mode 100644 docs/agents/device-foldable.md create mode 100644 docs/agents/gates.md create mode 100644 docs/agents/review.md create mode 100644 docs/agents/test-authoring.md create mode 100644 docs/agents/test-experiments.md diff --git a/AGENTS.md b/AGENTS.md index f2ee773328..6e12cd3380 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,146 +1,61 @@ # AGENTS.md -`agent-device` is a CLI and daemon for automating Apple, Android, HarmonyOS, Vega, Linux, and web -targets. A long-lived daemon owns sessions; registry-derived commands route to platform runtimes. +`agent-device` is a CLI and daemon for automating devices and web targets. +The daemon owns sessions. Registry declarations route commands to platform runtimes. ## Task routing -Load only the procedures relevant to the task: +Read only the row for your current task. Follow further links only when they apply. | When the task involves | Read | | --- | --- | | Domain vocabulary | `CONTEXT.md`, `docs/agents/domain.md` | | Architecture decisions | `docs/adr/README.md` | -| Tests or gate selection | `docs/agents/testing.md` | -| Writing, auditing, or sweeping tests; dispatching test-audit subagents | `docs/agents/test-audit.md` | -| Selector capture, polling, or interaction fast paths | `docs/agents/selector-capture.md` | +| Source changes, package boundaries, routing, diagnostics | `docs/agents/code-ownership.md` | +| Running checks or diagnosing a failed gate | `docs/agents/testing.md` | +| Writing or changing tests | `docs/agents/test-authoring.md` | +| Auditing or deleting tests | `docs/agents/test-audit.md` | +| Mutation experiments or base comparisons | `docs/agents/test-experiments.md` | +| Changing gate selection, fuzzing, or mutation harnesses | `docs/agents/gates.md` | +| Selector capture, polling, interaction fast paths, replay | `docs/agents/selector-capture.md` | | Adding or changing a CLI flag | `docs/agents/cli-flags.md` | -| Opening or reviewing a PR | `docs/agents/pull-requests.md` | -| Apple runner changes or manual device verification | `docs/agents/device-verification.md` | +| Opening or updating a PR | `docs/agents/pull-requests.md` | +| Reviewing a change | `docs/agents/review.md` | +| Device-facing changes or manual device verification | `docs/agents/device-verification.md` | | Writing issues or PRDs, and triage labels | `docs/agents/issue-tracker.md`, `docs/agents/triage-labels.md` | | Web backend setup or diagnostics | `docs/agents/web-backend.md` | -Versioned CLI help owns command semantics. Read the relevant topic when behavior is unclear; -normal app-driving startup follows the skill's routing card. +Device-facing changes affect live device behavior: dispatch, capture, interaction, app lifecycle, or native execution. +A daemon edit that changes none of these needs no device run. -## Incident-derived principles +Versioned CLI help owns command semantics. For app-driving startup, use the skill's routing card. -- Classify interaction dispatch paths in `packages/contracts/src/interaction-guarantees.ts` and - verify each claim against the native implementation. -- Delegation on error proves no success-path parity. A fast path may succeed on a candidate the - shared rules would refuse. -- Prove both paths are reachable before measuring a fast path against its baseline. -- Repair recurring failures at the owning type, registry, or construction path; do not add guards - that reconstruct another source of truth. -- Keep control-flow narration and review history out of implementation comments. Encode invariants - in names, types, boundaries, and tests; reserve comments for public API docs, tool directives, - and non-obvious constraints that cannot be encoded. -- Key behavior on typed reasons and details, never error text; do not copy existing message sniffs. -- A claim about a platform family in code, help, or docs holds for every backend in it (Appium as - well as ADB on Android), or names the backends it covers. -- Keep metadata at response level when it applies to the whole snapshot. Append warnings through - the shared response builder; never replace prior warnings. -- Before preserving a compatibility shape, run `git tag --contains `. Unreleased API has no - external compatibility obligation. +## Working rules -## Declaration sites and enforcement +- Keep edits within the requested command family or module group. Inspect other platforms only when the task requires it. +- Fix recurring failures at the owning type, registry, or construction path. Do not reconstruct another owner's state. +- Use typed reasons and details for behavior. Never match error text. +- Add compatibility or fallback behavior only when explicitly requested or approved. Remove superseded paths during migrations. +- Encode invariants in names, types, and boundaries. Comments explain public APIs, tool directives, or constraints code cannot express. +- Resolve gate failures at the reported invariant. Do not suppress them or add an allowlist to get a pass. +- Use `@agent-device/selectors` for selector parsing and matching. See `docs/agents/code-ownership.md` for other shared owners. +- Keep skills as routing cards. Keep `CONTEXT.md` glossary-only. Update owning help and user docs when behavior changes. -Read the declaration rather than maintaining a prose copy: +## Worktree setup -- commands and their surface, runtime-use, batch, and timeout traits: - `packages/command-registry/src/registry.ts` -- daemon route ownership and request-policy traits: `packages/daemon/src/core/daemon-command-registry.ts` -- interaction paths and guarantees: `packages/contracts/src/interaction-guarantees.ts` -- canonical command names: `packages/command-registry/src/catalog.ts` -- device runtime-use declarations and fact admission: - `packages/command-registry/src/registry.ts`, `packages/daemon/src/core/runtime-admission.ts`, and - `src/platform-runtime-gateway.ts` -- common command input fields, and which surface may write an input key (model, operator, retired): - `src/commands/common-input-fields.ts` and `src/commands/input-audience.ts` +- Use `pnpm`. Use OXC for lint and format. Run `pnpm format` repository-wide. +- In a fresh worktree, run `pnpm install --frozen-lockfile && pnpm build` before testing. +- Preserve other worktrees, devices, sessions, dirty files, and experiments. +- Never copy `.env` into a worktree. For host credentials, source the main checkout's file by absolute path. +- Give parallel workers disjoint files and distinct devices. `pnpm check:affected --run` and `pnpm check` serialize automatically. +- The layering scan reads tracked files. Stage new modules before trusting its result. +- Move a path's Fallow baseline entry when renaming its file. Do not regenerate unrelated baselines. -Shared selector parsing and matching belongs in `@agent-device/selectors`; daemon-side request -cancellation and progress in `@agent-device/host-kit/request`, and the caller's per-call abort guard -beside the transports it serves in `src/daemon-client/daemon-client-transport.ts`; cross-layer -contracts in `packages/contracts/src`; CLI flags in `src/commands/cli-grammar`; cross-surface schema -composition in `src/commands/schema`. +The validation lock reclaims provably dead owners automatically. Unknown owners remain blocking. +If waiting times out, report the printed owner and lock state; do not remove it by hand. -Resolve registry completeness failures at the missing declaration. Diagnose other gate failures -at their reported invariant; do not suppress them or add an allowlist to get a pass. Build interaction -responses through `buildInteractionResponseData`; change cross-language rules through golden tables -under `contracts/fixtures/`. +## Agent docs -## Hard repository rules - -- Plain `.mjs` packaging fixtures that cannot import TypeScript execution helpers keep child-process - use local and prefer `execFile`. -- Apple target changes keep the kernel device model, runtime-fact admission, dispatch resolution, - Apple discovery, and xctestrun preparation in sync. -- iOS simulator-set scoping must never hide the host macOS desktop target. -- Skills stay minimal routing cards. -- Add compatibility or fallback behavior only when explicitly requested or approved. Otherwise, - complete the migration and remove superseded paths. -- Keep changes within one command family or module group unless the task explicitly crosses a - boundary. Platform-neutral work does not license inspecting every platform implementation. - -## Module and test topology - -- Name modules for the domain question they answer and colocate machine-readable claims with their - enforcement. Internal barrels are legacy; add barrels only at package boundaries. -- Files past 1,000 lines are architecture debt; split them before adding behavior, unless they - are generated or fixture data. -- Tests mirror source topology one-to-one. Split a source module and its test together; do not add to - the legacy `interaction.test.ts` or platform `index.test.ts` aggregations. Pure moves carry their - tests unchanged; rename-only hunks owe no new coverage. -- A test sits beside the module it is named for: `.test.ts` or `-.test.ts` - (R84). Cross-module package tests are `index-.test.ts` beside `index.ts`; repo-level guards on - Swift sources, manifests, docs or scripts live in `test/repo-guards/`. -- Add no package `exports` entry that points at test support; R83 only lets the legacy count fall. A - fixture one package uses stays inside it, imported relatively; helpers shared across packages live only in `@agent-device/test-kit`, a - `devDependency` production source never imports and that never depends on `platform-*`, - `provider-*`, the daemon or root. - -## Toolchain and worktree traps - -- Use `pnpm`; never add `package-lock.json`. OXC owns lint and format. Run `pnpm format` - repository-wide, not path-scoped. -- A fresh worktree requires `pnpm install --frozen-lockfile && pnpm build`. Until then package and - optional-peer resolution may point at another checkout and produce false failures. -- Worktrees never receive `.env`; no repo code reads it. When a command needs a host credential, - source the main checkout's copy by absolute path (`set -a && . /path/to/main/checkout/.env && set +a`) - instead of copying it in or writing it into the worktree. -- Parallel work needs disjoint edit ownership and distinct devices. Run one full gate per host; - concurrent subprocess-backed suites can produce contention timeouts. -- The layering scan reads tracked files only. Stage a new module before trusting its result. -- Fallow baselines are path-keyed. Move the matching baseline entry when renaming a file; never bulk - regenerate baselines to accept unrelated findings. - -## Runtime and diagnostics seams - -Diagnostics use `@agent-device/host-kit/diagnostics`. Request diagnostics belong in the session request log; -session artifact paths come from `packages/daemon-contracts/src/session-artifact-paths.ts`. App/device logs remain in `app.log`; -Apple runner and xcodebuild output remains in `runner.log`. - -Normalize failures with `normalizeError` and preserve `hint`, `diagnosticId`, `logPath`, and typed -`details`. An interaction taking five seconds or more is a daemon-log question: inspect runner -restart, stale-session recovery, accessibility failure, transport retry, and timeout evidence. - -Optional probes and caches are best-effort only when the feature contract says so. Their budgets -must be shorter than the required operation they precede. - -## Selector and replay invariants - -Element interactions support selector and `@ref`, then record a selector chain after resolution. -Replay failures may re-resolve that chain only to produce a ranked divergence suggestion; ADR 0012 -retired silent script rewriting. - -Selector keys stay centralized in `@agent-device/selectors`; `is` predicates use -`evaluateIsPredicate`. On macOS, snapshot rectangles are absolute in window space and point actions -translate through the interaction root frame. Prefer selector or ref tests over raw coordinates. - -## Documentation ownership - -Keep this file to repository traps and routing. Put task procedures in `docs/agents/`, decisions in -ADRs, and command semantics in versioned help. Link to executable registries instead of duplicating -their rules. `CONTEXT.md` is glossary-only: no implementation paths, decisions, or migration status. - -Behavior changes update owning help/metadata and user docs. Report docs or skill changes when relevant. +Write short instructions with one action per sentence. Put the condition before its action. +Use the same term for the same concept. Keep commands and domain terms exact. +Route specialized rules to their task document. Delete duplicate or enforced rules before adding reminders. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fbb525a122..f8cab5c4da 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -238,8 +238,9 @@ Useful direct entry points: - `pnpm fallow:all` — full-tree audit, including grandfathered baseline findings - `pnpm fallow:baseline` — intentionally regenerate the reviewed Fallow health baseline -See [`docs/agents/testing.md`](docs/agents/testing.md) for gate ownership, shared test utilities, -mutation/fuzz lanes, contention policy, and test-speed rules. For real devices, follow +See [`docs/agents/testing.md`](docs/agents/testing.md) for check selection and blocked validation; see +[`test authoring`](docs/agents/test-authoring.md) for shared test utilities, +and test-speed rules. See [gate maintenance](docs/agents/gates.md) for mutation and fuzz harnesses. For real devices, follow [`docs/agents/device-verification.md`](docs/agents/device-verification.md); a fixture-backed test does not prove that a native path was active. diff --git a/docs/agents/code-ownership.md b/docs/agents/code-ownership.md new file mode 100644 index 0000000000..b321116bec --- /dev/null +++ b/docs/agents/code-ownership.md @@ -0,0 +1,42 @@ +# Code ownership + +Read this for shared contracts, package boundaries, request routing, or diagnostics. +Before editing a high-fan-in module, run `pnpm depgraph affected ` to scope callers and guarantees. + +## Declarations + +Read the owner. Do not copy its policy into consumers. + +| Question | Owner | +| --- | --- | +| Command names | `packages/command-registry/src/catalog.ts` | +| Surface, runtime, batch, timeout traits | `packages/command-registry/src/registry.ts` | +| Daemon routes and request policy | `packages/daemon/src/core/daemon-command-registry.ts` | +| Interaction guarantees | `packages/contracts/src/interaction-guarantees.ts` | +| Runtime-fact admission | `packages/daemon/src/core/runtime-admission.ts`, `src/platform-runtime-gateway.ts` | +| Input fields and audiences | `src/commands/common-input-fields.ts`, `src/commands/input-audience.ts` | + +Shared selectors belong in `@agent-device/selectors`. Request cancellation and progress belong in +`@agent-device/host-kit/request`. Per-call abort guards belong beside transports in +`src/daemon-client/daemon-client-transport.ts`. Cross-layer contracts belong in `packages/contracts/src`. +CLI flags belong in `src/commands/cli-grammar`; schema composition belongs in `src/commands/schema`. +Build interaction data through `buildInteractionResponseData`. Change cross-language rules through `contracts/fixtures/` golden tables. + +## Modules and tests + +Name modules for the domain question they answer. Colocate claims with their enforcement. +Use barrels only at package boundaries. Split files over 1,000 lines before adding behavior, except generated or fixture data. +Split a source module and its tests together. Do not extend legacy interaction or platform index test aggregations. +Keep tests beside their named module. Put repository artifact guards in `test/repo-guards/`. +Keep a single-package fixture local. Shared helpers belong in `@agent-device/test-kit`, a devDependency. +Production code never imports test-kit. Test-kit never depends on platforms, providers, the daemon, or root. +Do not export test support from production packages. + +## Diagnostics + +Use `@agent-device/host-kit/diagnostics`. Keep request diagnostics in the session request log. +Get session artifact paths from `packages/daemon-contracts/src/session-artifact-paths.ts`. +Keep app/device output in `app.log`; keep Apple runner and xcodebuild output in `runner.log`. +Normalize errors with `normalizeError`. Preserve `hint`, `diagnosticId`, `logPath`, and typed `details`. +For interactions taking at least five seconds, inspect daemon and runner recovery, retries, accessibility failures, and timeouts. +Use best-effort probes or caches only when the feature contract permits them. Keep their budgets below the required operation's budget. diff --git a/docs/agents/device-foldable.md b/docs/agents/device-foldable.md new file mode 100644 index 0000000000..6e25add0cc --- /dev/null +++ b/docs/agents/device-foldable.md @@ -0,0 +1,24 @@ +# Foldable Apple verification + +Read ADR 0025 before changing capture behavior on a multi-panel device. The iOS 27.1 runtime ships +only with the Xcode that carries it, and `xcode-select` may point at an older one, so pin the +toolchain per command: +`DEVELOPER_DIR=/Contents/Developer xcrun devicectl device info displays --device `. + +- Panels: that command lists each integrated panel with `backlightState`. Only the lit panel is + capturable — a capture of the dark panel exits 0 and writes an all-black PNG. +- Input routing: verify a fresh control after each pose change. A successful synthesis acknowledgement + does not prove a hit. Inspect the runner and simulator `testmanagerd`/BackBoard logs for display + identity and delivery; the resolved app window owns gesture coordinates and its target screen. +- Pose: `agent-device fold closed|half-open|open` sends private HID inside the simulator and reads + the hinge back through `devicectl device motion hinge-angle`. Device Hub and host Accessibility + permission are not required. Re-snapshot afterwards: refs and coordinates do not survive folds. + Verify locally with Device Hub stopped and the simulator booted through `simctl boot`; test all + three poses, active panel capture, and an app interaction. Duo coverage remains local until GHA + supports the runtime. To inspect the angle independently: + `xcrun devicectl device motion hinge-angle --device --session-timeout 1 --timeout 5`. +- Touch overlays export at the captured track size. The compositor checks its output (size, + non-black) against the raw capture; on failure `record stop` keeps the raw file and reports + `overlayWarning`. `record start --hide-touches` is only for the fastest raw capture. +- iOS 27 SDK apps need UIScene lifecycle to launch on iOS 27.0/27.1. +- The 27.1 beta accepts only the iPhone Duo device type; use another runtime for a non-foldable control. diff --git a/docs/agents/device-verification.md b/docs/agents/device-verification.md index cd2781d5d5..96788096d4 100644 --- a/docs/agents/device-verification.md +++ b/docs/agents/device-verification.md @@ -1,99 +1,59 @@ -# Manual Device Verification +# Verify on a device -Read this for Apple runner changes or manual `agent-device` runs on simulators, emulators, or -physical devices. Live verification steps apply when exercising a device-facing path. +Use this for Apple runner changes or manual device runs. For foldables, also read [foldable verification](device-foldable.md). -## Build freshness +## Prepare -- After changing runtime code reached through `bin/agent-device.mjs` or the daemon: `pnpm build`, - then `pnpm clean:daemon` — the daemon does not self-reload. -- Before any Android verification from source: `pnpm build`, `pnpm build:android`, `pnpm clean:daemon`. - `build:android` refreshes and verifies both bundled Android helper artifacts for the current - package version. -- Graceful `shutdown` hands off an active healthy runner that already answered a command, on the - simulator and physical iOS lanes alike; a new daemon may adopt the old binary. A runner retained - idle after `close` is stopped before the daemon exits. After - Swift runner changes, run `pnpm build:xcuitest` before verification. Use the session cleanup - procedure below if ownership is stuck. The physical handoff's device steps are - `docs/evidence/ios-physical-runner-handoff-2026-09-19.md`; nothing in it is proven until someone - with a cabled device checks a box. +1. Build runtime changes with `pnpm build`, then `pnpm clean:daemon`. The daemon does not reload code. +2. For Android source verification, also run `pnpm build:android` before cleaning the daemon. +3. For Swift runner changes, run `pnpm build:xcuitest` before verification. + Graceful shutdown can hand off an active runner; a new daemon can adopt the old binary. +4. Use distinct devices for parallel worktrees. Inspect worktree-scoped state with `pnpm daemon:state-dir`. +5. For `Agent Device Tester`, follow `examples/test-app/README.md` for the build, Metro, and expected app surface. + An installed `com.callstack.agentdevicelab` alone is insufficient. +6. For Android apps using Metro, run `adb reverse tcp: tcp:` before opening the app or URL. -## Prove the path under test was actually active +## Prove the path -- Android: capture `snapshot -i --json` and require `androidSnapshot.backend` to be `android-helper` - with `helperVersion` equal to `package.json`'s version. A stock UIAutomator fallback is not valid - verification unless the fallback itself is the behavior under test. -- For repo-owned `Agent Device Tester` work, `examples/test-app/README.md` is the source of truth for - simulator, physical-device, Metro/dev-client, and app-surface steps. An already-installed - `com.callstack.agentdevicelab` is not sufficient — the README's Metro/dev-build and `snapshot -i` - checks must prove the expected app surface is running. -- For Android RN/Expo/dev-client apps that use local Metro, configure - `adb reverse tcp: tcp:` for the app's Metro port before opening the app or URL. +Run the exact changed route. Record commands, device, source SHA, and observable results. +For Android, require `snapshot -i --json` to show `androidSnapshot.backend: android-helper` +and `helperVersion` matching `package.json`. UIAutomator fallback proves only that fallback. -## Worktree ownership and runner diagnostics +For Apple connection failures, start at `packages/platform-apple/src/runner/runner-startup-transport.ts`. +Retry policy belongs in `runner-error-classification.ts`; command typing belongs in `runner-contract.ts`. +Keep transport below session/client behavior. Keep xctestrun preparation outside request execution. +After signing, warm Node with `node -e 0` before measuring Gatekeeper-sensitive startup. -- Source-checkout daemon state is worktree-scoped, but devices are not. Use `pnpm daemon:state-dir` - to inspect it and different devices for concurrent worktrees. -- The first Node process after a newly signed Apple runner launches may block during Gatekeeper - verification. Warm it with a throwaway `node -e 0` before measuring. -- `DEVICE_IN_USE` has two flavors. "already in use by session X" is this daemon — follow its - `close --session` hint. "owned by session X in workspace Y" is another worktree's device - claim — non-retriable; run the error's `device status`/`device release --stale` recovery, - never PID hunting. A live or uncertain claim blocks `open`. If the device rebooted since the - owner's last `open` (including a reboot you caused), `open` settles the owner's app, runner, and - accessibility resources, takes the claim, and reports the release in `warnings`. +## Ownership and cleanup -The OS-neutral Apple runner lives under `packages/platform-apple/src/runner/`. For connection errors, -start at `runner-startup-transport.ts`; for retry policy, at `runner-error-classification.ts`; for -command typing, at `runner-contract.ts`. Transport stays below session/client -behavior, and xctestrun build/cache logic stays outside request execution. +Use purpose-specific sessions. For extra isolation, use a separate `--state-dir` under `/private/tmp`. +Close every manually opened session, including failed attempts, with its original session, platform, UDID, and state directory. -## Session hygiene +For `DEVICE_IN_USE`: -- Close manually opened sessions, including failed verification attempts, using their original - `--session`, `--platform`, `--udid`, and `--state-dir` values. -- Use a purpose-specific session name for experiments, and an isolated `--state-dir` under - `/private/tmp` when you need cleanup isolation beyond the current worktree's default daemon. -- If `close` is blocked or ownership looks stuck, inspect it with - `agent-device device status --stale` (daemonless), stop the owning daemon with - `agent-device daemon stop --state-dir ` (add `--clean` to remove retained runners), and - release provably dead owners with `agent-device device release --stale`. Do not hunt PIDs with - `ps`/`kill`. -- If cleanup cannot be completed, report the remaining session name, state dir, and the - `device status --stale` output as a blocker. +- "already in use by session X": follow this daemon's `close --session` hint. +- "owned by session X in workspace Y": inspect `device status`; follow `device release --stale` only for provably dead owners. + A live or uncertain owner blocks `open`. Do not hunt PIDs or steal the claim. -## Foldable Apple devices +For stuck cleanup, run `agent-device device status --stale`. +Stop the owning daemon with `agent-device daemon stop --state-dir `; add `--clean` for retained runners. +Release dead owners with `agent-device device release --stale`. +If blocked, report the session, state directory, and status output. Never hide incomplete cleanup. +The physical runner handoff checklist lives in `docs/evidence/ios-physical-runner-handoff-2026-09-19.md`. -Read ADR 0025 before changing capture behavior on a multi-panel device. The iOS 27.1 runtime ships -only with the Xcode that carries it, and `xcode-select` may point at an older one, so pin the -toolchain per command: -`DEVELOPER_DIR=/Contents/Developer xcrun devicectl device info displays --device `. +## Platform policy -- Panels: that command lists each integrated panel with `backlightState`. Only the lit panel is - capturable — a capture of the dark panel exits 0 and writes an all-black PNG. -- Input routing: verify a fresh control after each pose change. A successful synthesis acknowledgement - does not prove a hit. Inspect the runner and simulator `testmanagerd`/BackBoard logs for display - identity and delivery; the resolved app window owns gesture coordinates and its target screen. -- Pose: `agent-device fold closed|half-open|open` sends private HID inside the simulator and reads - the hinge back through `devicectl device motion hinge-angle`. Device Hub and host Accessibility - permission are not required. Re-snapshot afterwards: refs and coordinates do not survive folds. - Verify locally with Device Hub stopped and the simulator booted through `simctl boot`; test all - three poses, active panel capture, and an app interaction. Duo coverage remains local until GHA - supports the runtime. To inspect the angle independently: - `xcrun devicectl device motion hinge-angle --device --session-timeout 1 --timeout 5`. -- Touch overlays export at the captured track size. The compositor checks its output (size, - non-black) against the raw capture; on failure `record stop` keeps the raw file and reports - `overlayWarning`. `record start --hide-touches` is only for the fastest raw capture. -- An app built with the iOS 27 SDK must adopt the UIScene lifecycle to launch on an iOS 27.0 or - 27.1 simulator at all: a legacy `UIApplicationDelegate` app traps at launch inside - `___UIApplicationEvaluateRuntimeIssueForNoSceneLifecycleAdoption`, which reads like a broken - device but is not one. -- The 27.1 runtime in this beta accepts only the `iPhone Duo` device type, so a second non-foldable - 27.1 simulator cannot be created as a control. +Apple runner changes run `pnpm check:xctest-selection` and build the affected target. +Swift `#if` guards own XCTest classification. Do not maintain a second test-name list. +Use macOS host lanes for pure decisions and simulator lanes for iOS/XCTest semantics. +If system policy rejects an unsigned host bundle, rebuild with `CODE_SIGN_IDENTITY="Apple Development"` +or select an identity from `security find-identity -v -p codesigning`. +First runs need XCUITest automation permission. -## Sandboxed environments +Keep Apple target changes synchronized across the kernel model, runtime-fact admission, dispatch, +discovery, and xctestrun preparation. Simulator-set scoping must not hide the host macOS target. +HarmonyOS has no provisioned CI device or HDC. Mocked tests do not replace local hardware evidence. +Read live smoke harnesses before running them; their environment contracts live in their entry files under `test/integration/`. -The daemon binds localhost. If the sandbox rejects the listener with `listen EPERM`, rerun with -host access when permitted. Generic `Failed to start daemon` or cleanup errors alone do not prove a -sandbox cause; inspect the underlying failure. Run other checks in the sandbox unless their tools -require host access. +If a sandbox rejects localhost with `listen EPERM`, use host access when permitted. +Inspect the underlying error first. A generic startup or cleanup failure does not prove a sandbox cause. diff --git a/docs/agents/gates.md b/docs/agents/gates.md new file mode 100644 index 0000000000..3a25eeb755 --- /dev/null +++ b/docs/agents/gates.md @@ -0,0 +1,21 @@ +# Maintain gates + +Use this when changing gate selection or harnesses. For execution, read [validation](testing.md). + +`CHECK_CATALOG` in `scripts/check-affected/checks.ts` owns checks and commands. +CI declares ownership through the shared `run-gate` action with a literal gate id. +Update the catalog and executable model for a new check. Run `pnpm check:gate-manifest` to verify ownership and reachability. +Plant a violation to prove a new gate or selector detects it. Use [isolated experiments](test-experiments.md). + +For parser and geometry changes, extend shared fast-check arbitraries and properties. +Use `PROPERTY_RUNS` budgets. Keep examples for past bugs or named decisions. +Parser fuzz targets live in `scripts/fuzz/targets.ts`. Generators carry planted invalid outcomes. +Promote discoveries with the harness command; do not hand-copy unshrunk inputs. + +Mutation scope lives in `scripts/mutation/modules.ts` and `scripts/mutation/run.ts`. +Scores are advisory; incomplete sweeps fail. Read `pnpm mutation:run --help` before a pilot. +Concurrency torture uses production lock decisions. Run `pnpm test:concurrency-torture`; replay with the printed `TORTURE_SEED`. + +Do not commit run- or commit-stamped output under `scripts/`. Generate it or fetch evidence artifacts. +Versioned corpora, schemas, fixtures, and golden tables remain source inputs. +Plain `.mjs` packaging fixtures keep child-process use local when TypeScript helpers cannot load; prefer `execFile`. diff --git a/docs/agents/pull-requests.md b/docs/agents/pull-requests.md index c44e7746e6..c7d3ea31a2 100644 --- a/docs/agents/pull-requests.md +++ b/docs/agents/pull-requests.md @@ -1,112 +1,55 @@ -# Pull Requests +# Open or update a PR -## Publication scope +A request to open or ship a PR authorizes branch creation, commits, push, and PR creation after validation. +Apply authorization already given. Merging and releasing need separate authorization. -A request to open or ship a PR includes branch creation, commits, push, and PR creation after -validation; merging and releasing require separate authorization. Apply authorization already given. -If repository or skill guidance still blocks an authorized step, cite the exact instruction and the -blocked action. +## Validate -## Readiness and validation +Follow [validation](testing.md). Before pushing, run `pnpm check:affected --run --head `. +Record its receipt in the PR body. A serialized gate stage can validate the pushed head instead. +Record that result before reporting publication. +For unresolved failures, follow the blocked-validation table. An authorized incomplete draft can record a blocker; it is not a pass. +Push once per review round. Each push restarts CI. Drafts skip device lanes. -Use `docs/agents/testing.md` to select checks and establish regression evidence. Before pushing, -run `pnpm check:affected --run` successfully on the commit being pushed. A serialized gate stage may -run it on the pushed head instead, but its result must be in the PR body before publication is -reported. If a gate fails, diagnose it and rerun after the cause is resolved; report unresolved -failures without claiming validation passed. Rerun a failed CI job once only when its log shows an -infrastructure fault (runner startup, network, device boot); the same failure twice is a finding. +For device-facing changes, follow [device verification](device-verification.md). +Prove the changed route ran live. Fixtures and neighbouring routes do not replace this evidence. +Report missing devices or commands and incomplete session cleanup. -Push once per review round, not per fix: each push restarts CI. Drafts skip device lanes. +- Published: branch pushed, PR opened, validation status recorded at a named commit. +- Merge-ready: required checks pass at the actual head, with live evidence for device-facing changes. -- **Published**: branch pushed, PR opened, and validation recorded at a named commit. -- **Merge-ready**: required checks pass on the actual head, with live evidence for device-facing - changes. Docs-only and pure-tooling changes do not need device runs. Reporting publication does - not require waiting for CI. +Do not wait for CI merely to report publication. Do not claim skipped integration or coverage passed. -A local unit run does not establish provider integration or coverage. When those checks are -selected, verify the **Integration Tests** and **Coverage** jobs on the PR head. +## Self-review -For device-facing changes, fixtures do not replace a live run whose output proves it reached the -route the change adds or modifies; a neighbouring route is not evidence. If blocked, record the -command and device needed and the remaining risk. Close manual sessions using -`docs/agents/device-verification.md` and report incomplete cleanup. +Give a fresh reviewer the worktree, head, base ref, and this prompt. Do not supply an implementation summary. -Gross diff budget: 1,000 lines by `git diff --stat origin/main...HEAD`; 2,500 for one coherent change -in self-contained commits. Stack only layers that land independently. Rename-only move PRs titled -`refactor(move)` are exempt when `git diff -M90% --stat origin/main...HEAD` proves no material -content change. +> You did not write this change. Assume it has bugs and find them. Read AGENTS.md and docs/agents/review.md. +> Read the full diff against the merge-base and untracked files. Open callers and siblings for each hunk. +> Keep this checkout read-only. Run experiments only in a separate disposable checkout. +> Report only findings with a triggering scenario: file:line, severity, what breaks, and the fix. If none, say so. -## Fresh-context self-review +Fix accepted findings and rerun affected checks. Record a reason for rejected findings. +If no independent reviewer is available, reread the full diff cold and report that limitation. +An independent reviewer is required at the growth thresholds in [review guidance](review.md). -Before opening a PR, give a reviewer with no implementation context (a subagent or new session) -only the prompt below, the worktree, head, and base ref (`origin/main`); not the diff or a summary. -Fix the findings you accept, rerun the affected checks, and keep a one-line reason for each -rejected finding. Without a way to spawn one, reread the full diff cold against the prompt and say -so. +## Scope and commits -> You did not write this change. Assume it has bugs and find them. Read `AGENTS.md`, then -> `git diff $(git merge-base HEAD)` and untracked files; open the callers and siblings -> of every hunk. Apply the Reviewing section of `docs/agents/pull-requests.md` and the -> Incident-derived principles. Look for leftovers: narrating comments, guards or try/catch the call -> site does not need, casts, dead code, a second way to do what the code already does one way. Would -> each new test fail if the change were reverted? Report only findings with a triggering scenario: -> `file:line`, severity, what breaks, and the fix. If there are none, say so. +Keep gross diffs below 1,000 lines, or 2,500 for one coherent change in self-contained commits. +Measure with `git diff --stat origin/main...HEAD`. Stack only independently landable layers. +Rename-only `refactor(move)` PRs are exempt when `git diff -M90% --stat` proves no material content change. +Use conventional commit prefixes, without agent tags. Put enforcement edits in a final `chore(gates): ` commit. -## Commits - -Use conventional commit prefixes; no `[codex]` tags. Implementation commits come first. Enforcement -edits — pins, baselines, ownership tables, exports maps, `.fallowrc`, gate manifests — go in one -final commit titled `chore(gates): `. - -## Rebasing onto a moving `main` - -Rebase for conflicts or when upstream changes affect a dependency or gate relevant to the PR -(`pnpm check:affected --base --head origin/main`); disjoint upstream changes -alone do not require one. After rebasing, follow the same validation lifecycle on the new head; -retain earlier evidence with its original commit attribution. +Rebase for conflicts or relevant upstream changes. Disjoint changes alone do not require rebase. +After rebase, validate the new head. Keep old evidence attributed to its original SHA. ## PR body -Ready-for-review by default; draft when requested or intentionally incomplete. Keep the body under -250 words using these sections: - -- `## Summary`: changed behavior and why. Include 1-3 CLI/Node/MCP examples for public API changes - and `Closes #123` when applicable. Note the touched-file count and any expansion of scope. -- `## Validation`: tested commit SHA, relevant results, CI status, and unresolved risks. Name exact - commands when needed to reproduce evidence or explain a limitation. Use remote-accessible evidence, - not local paths. For a fix, show `| base | head |` output of the same command; captured - pre-fix output counts. For docs-only changes, explain why runtime validation does not apply. - -## Reviewing - -- Read linked issue dependencies and relevant ADRs. Resolve prerequisite/base conflicts before - implementation details. An ADR conflict needs an explicit update or superseding decision. -- For routing or command-surface changes, trace the production path through daemon and backend; - helper tests that bypass the router do not prove that path. Check CLI, Node.js, daemon, MCP, - help, and docs where the surface is affected. -- A regression attributed to an earlier PR belongs at the seam that PR missed. For typed-error - changes, trace producers through normalization and transport and inspect sibling consumers; - message matching must not replace a lost reason code. -- Check interaction responses for compact defaults, bounded JSON arrays, and artifact paths for - large evidence; preserve existing warnings and typed error details. -- Use the CI Size workflow for size evidence; local comparisons are not required by default. - At roughly 700 net production lines (excluding tests, generated data, fixtures, and docs) or - more than 3 kB npm unpacked growth, an independent reviewer (never the cold-reread fallback) also - judges whether a smaller owning interface or deletion of superseded code would suffice. Weigh - gross churn for move-heavy changes. Ask the same of any new abstraction, seam, dependency, or - ownership rule. These thresholds trigger investigation, not rejection; explain justified growth - and why a smaller design was rejected. - -## Answering review findings - -- Fix the rule, not the listed sites. Find what enumerates the sites a finding names (the owning - helper, type, or registry) and satisfy the invariant there; patching only the named sites starts - the next round. -- A pattern caught twice is encoded in the first form that fits: a type or structure, a lint rule - or gate, a regression test, then an `AGENTS.md` principle. Drop the principle once a check - enforces it. -- A completeness claim carries its proof. Before writing "one reader", "no shim left", "pure move", - or "behavior unchanged elsewhere", run the grep or `git diff -M90%` that shows it and cite it. -- Test doubles fail the way production fails: fixtures carry the error production produces, and - stubs throw where the real call throws. Name the mutation that makes the new test fail. -- Push back with evidence when a finding is wrong; do not patch to end the thread. +Use ready-for-review by default. Use draft when requested or intentionally incomplete. +Keep the body below 250 words: + +- `## Summary`: behavior and reason; touched-file count; scope expansion; linked issue. + Add 1–3 CLI/Node/MCP examples for public API changes. +- `## Validation`: tested SHA, command results, CI obligations, and unresolved risks. + Use remote-accessible evidence. For fixes, show base/head SHAs and the same command's results. + For docs-only changes, explain why runtime tests do not apply. diff --git a/docs/agents/review.md b/docs/agents/review.md new file mode 100644 index 0000000000..3a7e8e4a59 --- /dev/null +++ b/docs/agents/review.md @@ -0,0 +1,28 @@ +# Review a change + +Keep reviewed source read-only. Run checks in an isolated checkout if they can modify it. Use [isolated experiments](test-experiments.md) for mutations or base comparisons. + +## Reviewing + +- Read linked dependencies and relevant ADRs. Resolve prerequisite conflicts first. Update or supersede conflicting decisions explicitly. +- Trace routing and surface changes through daemon and backend. Helper tests bypassing the router do not prove dispatch. + Check affected CLI, Node, daemon, MCP, help, and docs surfaces. +- Trace typed errors from producers through normalization, transport, and sibling consumers. Do not replace lost reason codes with text matching. +- Classify interaction paths in `packages/contracts/src/interaction-guarantees.ts`. Verify each claim against native code. + Error-path delegation does not prove success-path parity. Prove both paths reachable before comparing performance. +- A platform-family claim covers every backend, including Appium and ADB, or names the covered backends. +- Keep snapshot-wide metadata at response level. Append warnings through the shared builder; never replace prior warnings. + Check compact defaults, bounded arrays, and artifact paths for large evidence. +- Before retaining compatibility, run `git tag --contains `. Unreleased shapes have no external compatibility obligation. +- Require each new test to detect a credible production failure. Test doubles must fail at the same seam as production. +- Find dead code, redundant guards, casts, narrating comments, and competing owners. Fix the owning rule, not only named sites. + +Use CI Size evidence. At about 700 net production lines or 3 kB npm growth, require an independent reviewer to assess a smaller design. +Also assess any new abstraction, dependency, seam, or ownership rule. Weigh gross churn for moves. Explain justified growth. + +## Report findings + +Report only triggering scenarios: `file:line`, severity, what breaks, and the fix. +If none, say so. Reject incorrect findings with evidence. +Prove claims such as "one reader", "pure move", or "no shim left" with search or `git diff -M90%`. +Encode repeated findings as types, structure, checks, or tests. Remove prose reminders once enforced. diff --git a/docs/agents/selector-capture.md b/docs/agents/selector-capture.md index f357c3863f..c82a958f09 100644 --- a/docs/agents/selector-capture.md +++ b/docs/agents/selector-capture.md @@ -35,3 +35,13 @@ These are cross-route behavior requirements; their rationale and owning decision When changing one of these rules, update the relevant ADR, guarantee-matrix cell, contract scenario, and TypeScript/Swift parity table rather than describing a new exception here. + +## Replay and coordinates + +Element interactions support selector and `@ref`, then record a selector chain after resolution. +Replay failures may re-resolve that chain only to produce a ranked divergence suggestion; ADR 0012 +retired silent script rewriting. + +Selector keys stay centralized in `@agent-device/selectors`; `is` predicates use +`evaluateIsPredicate`. On macOS, snapshot rectangles are absolute in window space and point actions +translate through the interaction root frame. Prefer selector or ref tests over raw coordinates. diff --git a/docs/agents/test-audit.md b/docs/agents/test-audit.md index 533841567c..325268c8c5 100644 --- a/docs/agents/test-audit.md +++ b/docs/agents/test-audit.md @@ -1,57 +1,35 @@ -# Test Audit +# Audit tests -A test earns its cost by guarding behavior, credible regressions, or independent contracts. Optimize for confidence, not deletions. Gate selection stays in `docs/agents/testing.md`. +Use this to audit or delete tests. Optimize for confidence, not deletion counts. +Use [test authoring](test-authoring.md) for test quality and [validation](testing.md) for checks. -## Authoring gate +## Retain independent contracts -Answer all before adding a test: - -1. Which observable behavior, invariant, or independent contract does it guard? -2. Which credible regression makes it fail? Name the production edit. -3. Why doesn't existing coverage catch it? Each contract has one owner at its strongest boundary. -4. Does it need a production seam no production caller needs? Move the test to the real boundary. - -A bug regression must fail on pre-fix code for the intended reason; one that never failed visibly proves the mock. - -## Junk patterns - -Authoring rejects; audits hunt: - -- assertion-free probes; a lone `typeof x === 'function'` above the sibling calling `x`; -- self-comparison (`f(x)` vs `f(x)`, `map.get(k)` vs itself, fixture vs re-spelled fixture); -- expectation drawn from the helper under test; a `switch` in the test body compared to its arms; -- near-identical or renamed `test()` blocks; one golden replayed twice; -- type pins posing as runtime assertions (`Equal = true`, `X ? true : never`, `satisfies` after erasure, `toHaveLength(n)` over literals); if erasing leaves nothing executable, use `void [fixtures]`; -- mocks asserting their own inputs; negative controls passing for the wrong reason; -- names promising more than the input exercises; -- dead code called only by tests; tests preserving test-only exports; a production package `exports` entry only tests import. - -## Retention criteria - -Refuse to delete tests enforcing: registry, gate-manifest, layering, and DI-seam completeness; golden tables and cross-language source inspection; `@ts-expect-error` pins (an unused directive fails `tsc`); identity pins deep-equal can't subsume; exact-array facets a membership scan can't see duplicates in; vocabulary pins surviving a production+fixture co-update. Static-or-slow and implementation-similarity are not deletion-worthy: prove you can't break it. +Do not delete completeness guards for registries, gates, layering, or DI seams. +Retain golden tables, cross-language inspection, `@ts-expect-error` pins, identity checks, +exact-array checks that detect duplicates, and vocabulary checks surviving a production/fixture co-update. +A slow or implementation-like test is not redundant without measured evidence. ## Candidate evidence -Record before editing; a missing field blocks the edit: - -- location, test name, detectable failure (or why none); -- non-test callers of the covered seams; the stronger remaining owner proof (both assertion sets, `file:line`); -- skip guards in candidate and sibling (`skipIf`, `.skip`, env flags a workflow sets); -- the runner that executes the file: a Vitest project (`vitest.config.ts`), a `node --test` script (`test:smoke`, `check:*:test`), native XCTest, or a `CHECK_CATALOG` gate id. Only a file no runner reaches is a never-run finding; -- what deletion unlocks; risk. - -## Mutation discipline - -Plant the production edit representing the claimed bug: the pre-edit body must stay green while the post-edit body goes red. Report both counts (`8 passed` → `1 failed | 7 passed`). Revert and show `git status` clean before committing. For a deletion, prove the sibling catches the mutant. Never argue detection: measure it. - -## Focused validation +Before editing, record: -`pnpm exec vitest run --project unit-core `, or `--project apple-runner` for `packages/platform-apple/src/runner/**`; then `pnpm check:quick` and `pnpm check:affected --run` on the pushed commit. `scripts/**` tests are in no `tsconfig` include, so `pnpm typecheck` skips helpers there. +- Location, name, and detectable failure. +- Production callers and the stronger remaining test's assertions, with locations. +- Candidate and sibling skip guards, including workflow environment flags. +- Actual runner: Vitest project, Node script, XCTest lane, or gate id. Only unreachable tests are never-run findings. +- What deletion unlocks and its risk. -## Dispatching subagents +Use [isolated experiments](test-experiments.md) for mutations. +For a deletion, prove the retained test detects the mutant after removing the candidate. +Report baseline and mutant counts. Source inspection alone does not prove detection. -Run parallel read-only lanes with exclusive edit ownership: subsystem lanes (per platform package, `packages/daemon`, core packages, `scripts/**`), one cross-cutting junk-pattern sweep, one never-run inventory (Vitest includes, `node --test` globs, gate ids). Mandate in every prompt: READ-ONLY; read the production owner and name the bug; verify skip guards and runner/gate ownership; paste the retention list; false positives cost more than misses. +## Delegated audits -## Landing +Assign disjoint read scopes to read-only reviewers. Do not assign edit ownership to a reviewer. +Each brief names the reviewed SHA and requires production-owner inspection, skip/runner verification, +retention criteria, and concrete failure scenarios. Experiments use separate disposable checkouts. +Subsystem review, duplicate-pattern search, and runner inventory are separate scopes when needed. -One owner-boundary batch per PR; delete the test-only seam with the test; prefer net-negative production LOC. Report retained false positives, then rebase to `main` and rediscover. +Land one owner-boundary batch per PR. Remove unused test-only seams with the tests. +Report retained false positives. Rediscover after a relevant rebase. diff --git a/docs/agents/test-authoring.md b/docs/agents/test-authoring.md new file mode 100644 index 0000000000..ce6776c2b1 --- /dev/null +++ b/docs/agents/test-authoring.md @@ -0,0 +1,47 @@ +# Write tests + +Use this when adding or changing tests. Read [validation](testing.md) for checks and [test audits](test-audit.md) for deletions. + +## Choose the boundary + +Before adding a test, name: + +1. The observable behavior, invariant, or independent contract. +2. The production edit that would break it. +3. Why existing coverage does not catch that edit. + +Test through the strongest production boundary. Do not add test-only production exports or dependency injection. +A regression test must fail without the fix for the intended reason. Use [isolated experiments](test-experiments.md). +Pure moves retain their tests; verify discovery at the new path. +Pair classifier positives with the closest negative. Prove identical error text without the typed reason cannot trigger recovery. +For callback canaries, assert semantic success, not lifecycle completion. +Automatic replay coverage requires a PR or scheduled lane that runs the scenario at the actual head. + +Reject assertion-free probes, self-comparisons, expectations computed by the subject, duplicate cases, and mocks asserting their own inputs. +Reject tests whose names promise more than their inputs exercise. Type expressions erased at runtime do not prove runtime behavior. +Retain compile-time `@ts-expect-error` pins; an unused directive fails typechecking. + +## Fixtures and seams + +Look in `@agent-device/test-kit` and `packages/daemon/src/__tests__/support/` before adding helpers. +Keep single-package fixtures beside their module. Packages that test-kit depends on keep helpers local. +Do not import test-kit in a `vi.mock` factory for a module its entry loads. Spy on that seam in place. +Use `mkdtempForTest` or `mkdtempForTestSync`. Global setup removes the run directory after workers exit. +Mock the seam the subject consumes. Daemon handlers receive fake runtime facts and facets, not generic dispatch mocks. +When migrating an ADR 0019 command, move its tests to the runtime seam. + +Vitest workers signal only themselves and direct children. Mock process-signal helpers for fabricated PIDs. +Do not disable isolation, switch the fork pool, or add unit retries to hide failures. +Use existing clocks or tool seams instead of waiting production time. +Slow-test pins only shrink; a new pin needs measured justification. +Split oversized test families with their source. Do not extend test files over 1,000 lines. +Import the subject module, not a platform barrel. + +## Run a focused test + +Use `pnpm exec vitest run --project unit-core `. +For `packages/platform-apple/src/runner/**`, use `--project apple-runner`. +Local runs default to four workers. Set `AGENT_DEVICE_VITEST_MAX_WORKERS=` for a different host share; CI ignores it. +Scripts tests are outside root TypeScript includes. Check new helpers with their runner and review their types. + +Wire changes emitting `platform` or `appleOs` need provider integration and coverage evidence. diff --git a/docs/agents/test-experiments.md b/docs/agents/test-experiments.md new file mode 100644 index 0000000000..6fbbb62f1a --- /dev/null +++ b/docs/agents/test-experiments.md @@ -0,0 +1,44 @@ +# Isolated test experiments + +Use this for manual mutations, regression proof, or base comparisons. Scheduled mutation sweeps follow [gate maintenance](gates.md). +Keep the reviewed and implementation checkouts unchanged. Reviewers have read scopes, not edit ownership. + +## Prepare + +1. Record the reviewed SHA, `git status --porcelain=v1`, and any existing diff before the experiment. +2. Create a disposable checkout outside app-managed worktrees: + +```sh +reviewed_sha=$(git rev-parse HEAD) +experiment_dir=$(mktemp -d /private/tmp/agent-device-experiment.XXXXXX) +git worktree add --detach "$experiment_dir" "$reviewed_sha" +``` + +3. In that checkout, run `pnpm install --frozen-lockfile && pnpm build`. + Do not reuse another checkout's `node_modules` or copy `.env`. +4. For base comparison, create it at the named base SHA instead. Record that SHA before running. + Use the same command, environment, and worker limit. Run sequentially without competing test processes. +5. If the change is uncommitted, apply only its relevant patch and required untracked files to the disposable checkout. + Record the patch input. It is not exact-commit evidence. + +## Prove detection + +- Regression: run the new test against code without the fix. It must fail for the claimed bug. + Run it with the fix. It must pass. If it passes without the fix, improve the test. +- Mutation: run the unmodified focused suite. It must pass. Plant the production edit representing the claimed bug. + Run the same suite. It must fail for that bug. Record counts, for example `8 passed` then `1 failed, 7 passed`. +- Test deletion: perform the mutation after removing the candidate. Prove the retained test detects it. +- Structural gate or selection rule: plant a violation or failing selected test. Verify the intended gate reports it. +- Unchanged-base failure: do not copy head changes into the base checkout. Preserve both logs and SHAs. + +A setup failure is not evidence. Measure detection. +Use distinct checkouts and devices for parallel experiments. + +## Finish + +1. Remove only the disposable worktree you created: `git worktree remove --force "$experiment_dir"`. +2. Verify the reviewed SHA, status, and existing diff still match the recorded originals. +3. Report before/after counts, commands, SHAs or patch inputs, failure reason, and cleanup status. + +Stop and report if the reviewed checkout changes, setup fails, or detection remains unproven. +Never restore files in the reviewed checkout to hide an experiment's edits. diff --git a/docs/agents/testing.md b/docs/agents/testing.md index 17fa1c61b2..bf14535c45 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -1,184 +1,57 @@ -# Testing Notes +# Validate a change -Gate ownership lives in `scripts/check-affected/` and `scripts/gate/`. +Use this to select checks and interpret results. For test code, read [test authoring](test-authoring.md). +For mutations or base comparisons, read [isolated experiments](test-experiments.md). -## Which gates a change needs +## Run checks -Three tiers: +1. While editing, run the focused test or `pnpm check:quick`. +2. Before pushing, commit the change. Run `pnpm check:affected --run --head `. +3. For a broad refactor or an explicitly requested full gate, run `pnpm check`. -1. While editing: a focused test or `pnpm check:quick`. -2. On the exact pushed head (locally, or by a gate stage recording it on the PR): - `pnpm check:affected --run`. It derives the relevant local gates and lists checks CI or a - native toolchain owns. -3. For a broad refactor, or when the full deterministic gate is requested: `pnpm check`. +Both broad commands acquire the same host lock. Wait for the owner to finish, or cancel your waiter with Ctrl-C. +Do not add lock wrappers or delete lock files. A wait ends after two hours. Nested validation fails immediately. +Focused tests do not lock. Avoid subprocess-heavy tests beside a broad gate. -GitHub stays authoritative for provider integration, full coverage, native builds, device lanes, and -history-backed compatibility. To inspect the gate catalog or a plan: +The runner prints a receipt path and progress. Each command has a log; Vitest also writes JSON. +The receipt records refs, checkout state, commands, exit codes, skips, and test failures. +Execution stops at the first failure. Unexecuted checks remain `not-run`. -```sh -pnpm check:affected -pnpm check:affected --json -pnpm gate --help -``` +- `pnpm check:affected`: compact plan. +- `pnpm check:affected --verbose`: plan with selection reasons. +- `pnpm --silent check:affected --json`: JSON plan. +- `pnpm --silent check:affected --run --json`: JSON execution receipt on stdout; progress on stderr. +- `pnpm check:affected --run`: include staged, unstaged, and untracked changes for local feedback. -`check:affected --run` reports coverage obligations without instrumenting; when related tests are -selected, it runs one capped `vitest related` command. Run local coverage, then -`check:coverage-crap` gates touched functions on that run's stamp (CONTRIBUTING.md). +Only a passed receipt with `exactCommit: true` proves validation at the named commit. +A dirty-tree pass is local feedback. Checkout changes during execution invalidate the run. +Initial/final checks detect persistent changes; they do not sandbox the checkout. Stop all edits during validation. -Two selection traps recur: +## Interpret selection -- A response emitting `platform` or `appleOs` needs provider integration and coverage evidence; - only the provider project catches internal `apple` leaking onto the wire. -- A workspace package manifest or TypeScript config can rewire all consumers, so the affected - selector fails open to the full gate set on purpose. +A package manifest or TypeScript config can affect every consumer. A full selection for these changes is intentional. +`FAIL-OPEN` means the selector could not safely narrow the set. Read `plan.failOpenReasons` in the receipt. +Do not reduce selection to get a pass. -Docs-only changes need no runtime tests or tests asserting prose. After the required gates, repeat -or broaden only for changes, failures, or unresolved risks. +Read required-check status on the PR. The receipt lists skipped checks and owners in `results[].reason`. +GitHub owns full coverage, native builds, device lanes, provider integration, and history-backed compatibility. +Related tests do not replace those obligations. Check the Integration Tests and Coverage jobs at the actual PR head. +Docs-only changes need guidance checks, not runtime tests or tests asserting prose. +See `CONTRIBUTING.md` for local coverage and [gate maintenance](gates.md) for declarations. -## Platform and live-device policy +## A gate failed -HarmonyOS has no provisioned CI emulator, physical device, DevEco image, or HDC installation. Unit, -provider, and coverage tests mock the typed HDC seam. Real validation is local hardware evidence per -`docs/agents/device-verification.md`. Do not add a CI lane that assumes a developer host. +Keep failure attribution separate from validation status. Never call a failing gate passed. -Apple runner changes run `pnpm check:xctest-selection` and build the affected target. The source -`#if` guard is the XCTest lane classification — never maintain a second test-name list. Pure runner -decisions use the macOS host lane; iOS/XCTest semantics need a simulator lane. -The iOS PR lane derives platform-specific XCTests from Swift guards for runner changes; -nightly runs the full suite. +| Evidence | Next action | Outcome | +| --- | --- | --- | +| New failure caused by this change | Fix its owner. Rerun after the fix. | Validation failed until a new run passes. | +| Suspected host contention or infrastructure fault | Save the log. Run the same failing command on unchanged base in an isolated checkout, without competing tests. | Cause unproven until compared. Compare both runs. A solo pass alone does not prove contention. | +| Same failure reproduced on unchanged base | Record both SHAs, command, logs, and matching failure. Assign the blocker to a named person or linked issue. | Base-blocked. If publication is authorized, open/update an incomplete draft with the blocker and remaining checks. Do not mark merge-ready. | +| Base comparison unavailable or inconclusive | Record the missing tool/device/evidence and next action. | Blocked, cause unproven. An authorized draft can carry this limitation. | +| Failure source fixed, environment repaired, or new upstream commit contains the fix | Run the failing check, then the affected gate on the current head. | Replace blocked status only with new evidence. | -Local host-lane XCTest may need signing and automation permission: - -- System policy may refuse the unsigned bundle (`library load disallowed by system policy`, shown - as `Early unexpected exit … crashed with signal kill`). Rebuild signed: - `CODE_SIGN_IDENTITY="Apple Development"`, or pick an identity from - `security find-identity -v -p codesigning`. -- The first run needs XCUITest automation permission for the host app. - -Live smoke commands and their environment contracts live with their harnesses: - -- web: `test/integration/smoke-web-platform.test.ts` -- iOS: `test/integration/smoke-ios-simulator.test.ts` and - `test/integration/smoke-ios-simulator-coverage.test.ts` -- concurrency: `test/integration/nightly/concurrency-torture.test.ts` - -Read the entry file before running a lane. Do not copy its environment matrix here — it changes. - -## Shared test utilities - -Before creating fixtures, look in `@agent-device/test-kit` and -`packages/daemon/src/__tests__/support/`. Shared `DeviceInfo`, session, snapshot, store, -runtime-fact, and mocked-binary values belong in a sibling fixture module, not in repeated test -literals; a fixture one package uses is a `*.fixtures.ts` beside its module. The packages test-kit -builds on (kernel, contracts, capture-kit, host-kit, daemon-contracts) cannot -declare it; their tests keep helpers local. Never import test-kit in the `vi.mock` factory of a -module its entry loads (`host-kit/process`, `command`, `file`): spy it in place, as -`pinOwnProcessStartTime` does. Fixtures a package still exports (R83's legacy count) stay -importable until moved. - -Use `mkdtempForTest` or `mkdtempForTestSync`. Global setup redirects `TMPDIR` for the run and -removes it after every worker exits — skip per-test cleanup. An interrupted run may leave a -directory behind; the next run prunes it once its owner and every process using its `TMPDIR` are -gone. - -Mock the seam the subject consumes. A daemon handler that binds a runtime gets fake runtime facts -and facets, not a mock of generic dispatch. Generic dispatch mocks are migration debt — do not add -more. When an ADR 0019 command migrates, its tests move to the runtime seam in the same PR. - -Vitest workers may signal only themselves and their direct children; `hermetic-signal-setup.ts` -rejects other process-table writes. Tests with fabricated PIDs mock `signalPidsBestEffort`, -`signalProcessGroupBestEffort`, or the tool-provider seam. A real child the test spawned may be -signalled directly. - -## Regression evidence - -Observe a regression test fail without the fix, then pass with it. For new or changed structural -gates, plant a violation and verify the intended gate names it. Pure test moves retain their tests; -verify discovery at the new path. If selection rules change, plant a failure to prove selection. - -A callback-based canary must observe semantic success, not just lifecycle completion — e.g. React -Native Gesture Handler's -[`onFinalize`](https://docs.swmansion.com/react-native-gesture-handler/docs/fundamentals/callbacks-events/) -also fires on failed or interrupted recognition. Use an activation-dependent callback, or assert -success state before publishing a pass. - -A device replay counts as automatic regression coverage only when an automatic PR or scheduled lane -selects and runs it. Name the owning lane and confirm the scenario ran on the exact PR head. A -replay in a manual or unselected tier is test material, not automatic evidence. - -For structured classifiers, pair the positive case with the closest negative. When an error message -can be identical with and without a typed reason, the negative test must prove the message alone -cannot activate retry, fallback, or recovery. - -Test through public interfaces; never add production exports or test-only dependency injection — a -missing seam needs a real product seam. - -## Properties, fuzzing, and mutation - -Pure parser and geometry changes extend the shared fast-check arbitraries and properties — not just -another example. Keep examples for a real past bug or a named decision. Reuse `PROPERTY_RUNS` -budgets so property files stay inside the unit slow-test gate. - -Parser fuzz targets live in `scripts/fuzz/targets.ts`. Validation generators carry the invalid -outcome they planted, so silent acceptance and wrong error codes are failures. Promote a discovered case with the command the harness prints — never -hand-copy an unshrunk input. - -Mutation scope: `scripts/mutation/modules.ts#KERNEL_MODULES` (weekly), `scripts/mutation/run.ts#LANE_SOURCES` -(PR canary; dispatch adds `LANE_DEPENDENCIES`). Scores are advisory; incomplete sweeps fail. Pilot protocol: `pnpm mutation:run --help`. - -## Before editing a shared module - -Run `pnpm depgraph affected` before touching a high-fan-in module: - -```sh -pnpm depgraph affected packages/host-kit/src/command.ts -pnpm depgraph affected packages/daemon/src/core/ref-frame.ts --json --limit 25 -``` - -Use its dependent, command, and guarantee-cell report to scope inspection. `pnpm check:affected --run` -selects gates independently from the diff; it does not consume the depgraph report. - -## Gate ownership - -`CHECK_CATALOG` is the executable check registry. CI owns a check only through the shared `run-gate` -action with a literal gate id; `pnpm check:gate-manifest` verifies registration, workflow ownership, -path reachability, and routed device lanes. Raw shell text cannot declare ownership. - -New check: update the catalog and its executable model. - -## Concurrency torture lane - -The harness uses a deterministic scheduler for modeled lock grants and a real request-scope -serialization guard. A seed reproduces the scheduler trace and terminal invariant: - -```sh -pnpm test:concurrency-torture -TORTURE_SEED=1234 pnpm test:concurrency-torture -``` - -Lock plans come from the production request-lock decisions — never hand-author a parallel plan. The -modeled boundary is documented in the harness module; every failure prints its exact replay command. - -## Test evidence and versioned inputs - -Run- or commit-stamped benchmark output is never committed under `scripts/`: produce it at run -time or fetch it from the evidence branch. Versioned inputs (fuzz corpus, Maestro fixtures, -schemas, `contracts/fixtures/` tables) are unaffected. - -## Speed rules - -Changing timeout failures that pass alone may be host contention. Reproduce on `origin/main` under -the same load before classifying them as regressions. - -- Unit tests have no retry layer. Fix or remove flakes instead of hiding them behind retries. -- Unit tests do not wait production time. Prefer budget-derived cadence, assert the caller passes - the right timeout to its tool seam, or use an existing clock seam. -- The slow-test reporter enforces unit and integration budgets. Existing pins only shrink; a new - pin needs measured justification. -- Test files over 1,000 lines may be no longer than at the merge-base with `origin/main`, and no - new test file may cross that line. Split the family before adding tests; shrinking needs no - gate edit. -- Keep isolation enabled and the pool on forks; disabling isolation and changing pools did not - improve measured performance. Import the module under test, not a platform barrel. -- Local Vitest runs use a four-worker cap. Override it when a run needs a different host share: - `AGENT_DEVICE_VITEST_MAX_WORKERS=` (clamped to host CPUs, ignored in CI). +Match failures by check, location, diagnostic code, and failing assertion. Log comparison is evidence, not production error-text branching. +The implementing agent owns diagnosis until a named owner or issue accepts the blocker. +Do not loop on an unchanged failure. Do not widen timeouts, mask exit codes, or invoke CI authority as a local pass. +Rerun failed CI once only when its log shows an infrastructure fault. Treat the same failure twice as a finding. From ec9926f76c80031ff639bec4b7f2ed68d3de44e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 11:20:38 +0200 Subject: [PATCH 3/8] fix(tooling): fingerprint untracked nested Git repositories --- scripts/check-affected/checkout-state.test.ts | 29 +++++++++++++ scripts/check-affected/checkout-state.ts | 41 +++++++++++++++---- 2 files changed, 61 insertions(+), 9 deletions(-) diff --git a/scripts/check-affected/checkout-state.test.ts b/scripts/check-affected/checkout-state.test.ts index 6692893a32..d75627511d 100644 --- a/scripts/check-affected/checkout-state.test.ts +++ b/scripts/check-affected/checkout-state.test.ts @@ -34,3 +34,32 @@ test('checkout fingerprint distinguishes edits with identical porcelain status a fs.rmSync(cwd, { recursive: true, force: true }); } }); + +test('checkout fingerprint handles an untracked nested repository and detects its source edits', () => { + const cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'checkout-nested-')); + const git = (directory: string, ...args: string[]) => runCmdSync('git', args, { cwd: directory }); + try { + git(cwd, 'init', '-q'); + git(cwd, 'config', 'user.email', 'test@example.com'); + git(cwd, 'config', 'user.name', 'Test'); + fs.writeFileSync(path.join(cwd, 'source.ts'), 'base'); + git(cwd, 'add', '.'); + git(cwd, 'commit', '-qm', 'base'); + const nested = path.join(cwd, 'scratch'); + fs.mkdirSync(nested); + git(nested, 'init', '-q'); + fs.writeFileSync(path.join(nested, 'source.ts'), 'one'); + fs.writeFileSync(path.join(nested, '.gitignore'), 'ignored/\n'); + const first = readCheckoutState(cwd); + assert.match(first.status, /\?\? scratch\//); + fs.writeFileSync(path.join(nested, 'source.ts'), 'two'); + const second = readCheckoutState(cwd); + assert.equal(first.status, second.status); + assert.notEqual(first.fingerprint, second.fingerprint); + fs.mkdirSync(path.join(nested, 'ignored')); + fs.writeFileSync(path.join(nested, 'ignored', 'artifact'), 'ignored'); + assert.equal(second.fingerprint, readCheckoutState(cwd).fingerprint); + } finally { + fs.rmSync(cwd, { recursive: true, force: true }); + } +}); diff --git a/scripts/check-affected/checkout-state.ts b/scripts/check-affected/checkout-state.ts index 5b8d145d85..8594eb5cd4 100644 --- a/scripts/check-affected/checkout-state.ts +++ b/scripts/check-affected/checkout-state.ts @@ -22,14 +22,37 @@ export function readCheckoutState(cwd: string): CheckoutState { const untracked = runCmdSync('git', ['ls-files', '--others', '--exclude-standard', '-z'], { cwd, }).stdout; - for (const file of untracked.split('\0').filter(Boolean).sort()) { - const absolute = path.join(cwd, file); - hash.update(file).update('\0'); - hash.update( - fs.lstatSync(absolute).isSymbolicLink() - ? fs.readlinkSync(absolute) - : fs.readFileSync(absolute), - ); - } + for (const file of untracked.split('\0').filter(Boolean).sort()) + hashWorktreePath(hash, cwd, file); return { head, status, fingerprint: hash.digest('hex') }; } + +function hashWorktreePath(hash: crypto.Hash, cwd: string, file: string): void { + hash.update(file).update('\0'); + const absolute = path.join(cwd, file); + let stat: fs.Stats; + try { + stat = fs.lstatSync(absolute); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + hash.update('missing\0'); + return; + } + if (stat.isDirectory()) { + hash.update('directory\0'); + const files = runCmdSync( + 'git', + ['ls-files', '--cached', '--others', '--exclude-standard', '-z'], + { + cwd: absolute, + maxBuffer: 64 * 1024 * 1024, + }, + ).stdout; + for (const child of files.split('\0').filter(Boolean).sort()) + hashWorktreePath(hash, absolute, child); + return; + } + const contents = stat.isSymbolicLink() ? fs.readlinkSync(absolute) : fs.readFileSync(absolute); + hash.update(stat.isSymbolicLink() ? 'symlink\0' : 'file\0'); + hash.update(crypto.createHash('sha256').update(contents).digest()); +} From 3449a077a4ba5a0a46675dc41324ae4886f4dc89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 11:30:46 +0200 Subject: [PATCH 4/8] fix(tooling): bound individual failure excerpt lines --- scripts/check-affected/command-execution.ts | 10 +++++++++- scripts/check-affected/execution.test.ts | 4 ++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/scripts/check-affected/command-execution.ts b/scripts/check-affected/command-execution.ts index eec398bb0b..798ac23ba3 100644 --- a/scripts/check-affected/command-execution.ts +++ b/scripts/check-affected/command-execution.ts @@ -78,7 +78,15 @@ export async function executeValidationCommand( } function reportFailureTail(logPath: string, report: (line: string) => void): void { - report(fs.readFileSync(logPath, 'utf8').slice(-8000).split('\n').slice(-35).join('\n')); + report( + fs + .readFileSync(logPath, 'utf8') + .slice(-8000) + .split('\n') + .slice(-35) + .map((line) => line.slice(0, 400)) + .join('\n'), + ); } function collectTestReports(result: CheckResult, directory: string, reportPrefix: string): void { diff --git a/scripts/check-affected/execution.test.ts b/scripts/check-affected/execution.test.ts index 67bdefbf0d..abc53a5017 100644 --- a/scripts/check-affected/execution.test.ts +++ b/scripts/check-affected/execution.test.ts @@ -19,6 +19,10 @@ test('execution JSON contains exact checkout, command, failure exit, and bounded assert.equal(result.receipt.final?.head, sha); assert.equal(result.receipt.results[0]?.exitCode, 7); assert.ok(result.stderr.length < 10000); + assert.ok( + result.stderr.split('\n').every((line) => line.length <= 512), + 'failure excerpt lines stay bounded', + ); assert.ok(fs.readFileSync(result.receipt.results[0]!.logPath!, 'utf8').length > 20000); } finally { fs.rmSync(cwd, { recursive: true, force: true }); From e93d80a8a29728fa76d6e276327bee4ee00359e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 11:30:46 +0200 Subject: [PATCH 5/8] chore(gates): tighten agent context budgets and register validation entry --- .fallowrc.json | 1 + scripts/__tests__/agent-guidance-contract.test.ts | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.fallowrc.json b/.fallowrc.json index edfa17fe5a..6856a0344d 100644 --- a/.fallowrc.json +++ b/.fallowrc.json @@ -3,6 +3,7 @@ "entry": [ "tsdown.config.ts", "scripts/check-provider-plugin.mjs", + "scripts/check-affected/command.ts", "packages/provider-testmu/test/package-smoke.mjs", "packages/provider-testmu/tsdown.config.ts", "packages/provider-testmu/src/plugin.ts", diff --git a/scripts/__tests__/agent-guidance-contract.test.ts b/scripts/__tests__/agent-guidance-contract.test.ts index 9118ea6d1b..6e0df3cc32 100644 --- a/scripts/__tests__/agent-guidance-contract.test.ts +++ b/scripts/__tests__/agent-guidance-contract.test.ts @@ -6,10 +6,10 @@ import { test } from 'node:test'; const ROOT = path.resolve(import.meta.dirname, '../..'); const BYTE_BUDGETS = { - 'AGENTS.md': 10_000, - 'CONTEXT.md': 12_000, + 'AGENTS.md': 4_000, + 'CONTEXT.md': 11_000, } as const; -const FOCUSED_DOC_BUDGET = 10_000; +const FOCUSED_DOC_BUDGET = 5_000; const AGENT_DOCS_TOTAL_BUDGET = 40_000; type PackageManifest = { From d8d1080400f7fac1eaf6a07db5c9b3aa84ab8380 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 13:24:05 +0200 Subject: [PATCH 6/8] fix(tooling): address validation review findings --- CONTRIBUTING.md | 6 +- docs/agents/device-foldable.md | 2 +- docs/agents/device-verification.md | 17 ++-- docs/agents/gates.md | 4 +- docs/agents/pull-requests.md | 2 +- docs/agents/test-experiments.md | 2 +- docs/agents/testing.md | 22 +++-- packages/host-kit/src/internal/exec.test.ts | 22 +++++ packages/host-kit/src/internal/exec.ts | 9 +- scripts/check-affected/checkout-state.test.ts | 10 +++ scripts/check-affected/checkout-state.ts | 4 +- .../check-affected/command-execution.test.ts | 4 +- scripts/check-affected/command-execution.ts | 46 ++++++---- scripts/check-affected/command.test.ts | 25 ++++-- scripts/check-affected/command.ts | 9 ++ scripts/check-affected/execution.fixtures.ts | 2 +- scripts/check-affected/execution.ts | 5 +- scripts/check-affected/host-lock.test.ts | 84 ++++++++++++++++- scripts/check-affected/host-lock.ts | 90 +++++++++++++++---- scripts/check-affected/run-cli.test.ts | 8 +- scripts/check-affected/run.test.ts | 15 +--- scripts/check-affected/run.ts | 19 +--- scripts/check-affected/test-report.test.ts | 25 +++--- 23 files changed, 308 insertions(+), 124 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f8cab5c4da..5453163cd9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -217,7 +217,8 @@ For broad refactors or when explicitly requested, run the deterministic core agg pnpm check ``` -`pnpm check` covers formatting, lint, typechecking, layering, dependency-graph parity, production +`pnpm check` takes the same host lock as `check:affected --run`, prints a receipt path, and writes +logs under `.tmp/validation`. It covers formatting, lint, typechecking, layering, dependency-graph parity, production exports, MCP metadata, the distributable build, bundle ownership, Fallow, unit tests, and local smoke tests. It is intentionally not a simulation of every CI job: coverage, provider integration, history-backed compatibility, specialized toolchains, and live device/browser lanes remain separate. @@ -239,8 +240,7 @@ Useful direct entry points: - `pnpm fallow:baseline` — intentionally regenerate the reviewed Fallow health baseline See [`docs/agents/testing.md`](docs/agents/testing.md) for check selection and blocked validation; see -[`test authoring`](docs/agents/test-authoring.md) for shared test utilities, -and test-speed rules. See [gate maintenance](docs/agents/gates.md) for mutation and fuzz harnesses. For real devices, follow +[`test authoring`](docs/agents/test-authoring.md) for shared test utilities and test-speed rules. See [gate maintenance](docs/agents/gates.md) for mutation and fuzz harnesses. For real devices, follow [`docs/agents/device-verification.md`](docs/agents/device-verification.md); a fixture-backed test does not prove that a native path was active. diff --git a/docs/agents/device-foldable.md b/docs/agents/device-foldable.md index 6e25add0cc..5052023b28 100644 --- a/docs/agents/device-foldable.md +++ b/docs/agents/device-foldable.md @@ -16,7 +16,7 @@ toolchain per command: Verify locally with Device Hub stopped and the simulator booted through `simctl boot`; test all three poses, active panel capture, and an app interaction. Duo coverage remains local until GHA supports the runtime. To inspect the angle independently: - `xcrun devicectl device motion hinge-angle --device --session-timeout 1 --timeout 5`. + `DEVELOPER_DIR=/Contents/Developer xcrun devicectl device motion hinge-angle --device --session-timeout 1 --timeout 5`. - Touch overlays export at the captured track size. The compositor checks its output (size, non-black) against the raw capture; on failure `record stop` keeps the raw file and reports `overlayWarning`. `record start --hide-touches` is only for the fastest raw capture. diff --git a/docs/agents/device-verification.md b/docs/agents/device-verification.md index 96788096d4..75f6a4b96b 100644 --- a/docs/agents/device-verification.md +++ b/docs/agents/device-verification.md @@ -4,14 +4,15 @@ Use this for Apple runner changes or manual device runs. For foldables, also rea ## Prepare -1. Build runtime changes with `pnpm build`, then `pnpm clean:daemon`. The daemon does not reload code. -2. For Android source verification, also run `pnpm build:android` before cleaning the daemon. -3. For Swift runner changes, run `pnpm build:xcuitest` before verification. +1. Build runtime changes with `pnpm build`. +2. For Android source verification, run `pnpm build:android`. +3. For Swift runner changes, run `pnpm build:xcuitest`. Graceful shutdown can hand off an active runner; a new daemon can adopt the old binary. -4. Use distinct devices for parallel worktrees. Inspect worktree-scoped state with `pnpm daemon:state-dir`. -5. For `Agent Device Tester`, follow `examples/test-app/README.md` for the build, Metro, and expected app surface. +4. Run `pnpm clean:daemon` after the builds. The daemon does not reload code. +5. Use distinct devices for parallel worktrees. Inspect worktree-scoped state with `pnpm daemon:state-dir`. +6. For `Agent Device Tester`, follow `examples/test-app/README.md` for the build, Metro, and expected app surface. An installed `com.callstack.agentdevicelab` alone is insufficient. -6. For Android apps using Metro, run `adb reverse tcp: tcp:` before opening the app or URL. +7. For Android apps using Metro, run `adb reverse tcp: tcp:` before opening the app or URL. ## Prove the path @@ -33,7 +34,9 @@ For `DEVICE_IN_USE`: - "already in use by session X": follow this daemon's `close --session` hint. - "owned by session X in workspace Y": inspect `device status`; follow `device release --stale` only for provably dead owners. - A live or uncertain owner blocks `open`. Do not hunt PIDs or steal the claim. + A live or uncertain owner blocks `open` unless the device rebooted after the claim. + With reboot evidence, `open` settles old resources, takes the claim, and reports release in `warnings`. + Do not hunt PIDs or steal the claim. For stuck cleanup, run `agent-device device status --stale`. Stop the owning daemon with `agent-device daemon stop --state-dir `; add `--clean` for retained runners. diff --git a/docs/agents/gates.md b/docs/agents/gates.md index 3a25eeb755..801b4fb734 100644 --- a/docs/agents/gates.md +++ b/docs/agents/gates.md @@ -12,7 +12,9 @@ Use `PROPERTY_RUNS` budgets. Keep examples for past bugs or named decisions. Parser fuzz targets live in `scripts/fuzz/targets.ts`. Generators carry planted invalid outcomes. Promote discoveries with the harness command; do not hand-copy unshrunk inputs. -Mutation scope lives in `scripts/mutation/modules.ts` and `scripts/mutation/run.ts`. +Mutation declarations live in `scripts/mutation/modules.ts`; test ownership lives in `scripts/mutation/ownership.ts`. +`scripts/mutation/run.ts` uses `KERNEL_MODULES` weekly and `LANE_SOURCES` for PR canaries. +Dispatch with `--with-dependencies` adds `LANE_DEPENDENCIES`. Scores are advisory; incomplete sweeps fail. Read `pnpm mutation:run --help` before a pilot. Concurrency torture uses production lock decisions. Run `pnpm test:concurrency-torture`; replay with the printed `TORTURE_SEED`. diff --git a/docs/agents/pull-requests.md b/docs/agents/pull-requests.md index c7d3ea31a2..f63b7b609b 100644 --- a/docs/agents/pull-requests.md +++ b/docs/agents/pull-requests.md @@ -37,7 +37,7 @@ An independent reviewer is required at the growth thresholds in [review guidance Keep gross diffs below 1,000 lines, or 2,500 for one coherent change in self-contained commits. Measure with `git diff --stat origin/main...HEAD`. Stack only independently landable layers. -Rename-only `refactor(move)` PRs are exempt when `git diff -M90% --stat` proves no material content change. +Rename-only `refactor(move)` PRs are exempt when `git diff -M90% --stat origin/main...HEAD` proves no material content change. Use conventional commit prefixes, without agent tags. Put enforcement edits in a final `chore(gates): ` commit. Rebase for conflicts or relevant upstream changes. Disjoint changes alone do not require rebase. diff --git a/docs/agents/test-experiments.md b/docs/agents/test-experiments.md index 6fbbb62f1a..65e8f3df37 100644 --- a/docs/agents/test-experiments.md +++ b/docs/agents/test-experiments.md @@ -10,7 +10,7 @@ Keep the reviewed and implementation checkouts unchanged. Reviewers have read sc ```sh reviewed_sha=$(git rev-parse HEAD) -experiment_dir=$(mktemp -d /private/tmp/agent-device-experiment.XXXXXX) +experiment_dir=$(mktemp -d "${TMPDIR:-/tmp}/agent-device-experiment.XXXXXX") git worktree add --detach "$experiment_dir" "$reviewed_sha" ``` diff --git a/docs/agents/testing.md b/docs/agents/testing.md index bf14535c45..72976fbf58 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -1,7 +1,6 @@ # Validate a change -Use this to select checks and interpret results. For test code, read [test authoring](test-authoring.md). -For mutations or base comparisons, read [isolated experiments](test-experiments.md). +Use this to select checks and interpret results. Use [isolated experiments](test-experiments.md) for mutations or base comparisons. ## Run checks @@ -13,8 +12,8 @@ Both broad commands acquire the same host lock. Wait for the owner to finish, or Do not add lock wrappers or delete lock files. A wait ends after two hours. Nested validation fails immediately. Focused tests do not lock. Avoid subprocess-heavy tests beside a broad gate. -The runner prints a receipt path and progress. Each command has a log; Vitest also writes JSON. -The receipt records refs, checkout state, commands, exit codes, skips, and test failures. +The runner prints progress and a receipt path. The receipt links command logs and Vitest JSON. +It records refs, checkout state, commands, exit codes, skips, and failures. Execution stops at the first failure. Unexecuted checks remain `not-run`. - `pnpm check:affected`: compact plan. @@ -24,20 +23,19 @@ Execution stops at the first failure. Unexecuted checks remain `not-run`. - `pnpm check:affected --run`: include staged, unstaged, and untracked changes for local feedback. Only a passed receipt with `exactCommit: true` proves validation at the named commit. -A dirty-tree pass is local feedback. Checkout changes during execution invalidate the run. +A dirty-tree pass is local feedback. Detected checkout changes invalidate the run. Initial/final checks detect persistent changes; they do not sandbox the checkout. Stop all edits during validation. ## Interpret selection A package manifest or TypeScript config can affect every consumer. A full selection for these changes is intentional. -`FAIL-OPEN` means the selector could not safely narrow the set. Read `plan.failOpenReasons` in the receipt. +`FAIL-OPEN` means the selector could not safely narrow the set. Read `plan.failOpenReasons`. Do not reduce selection to get a pass. Read required-check status on the PR. The receipt lists skipped checks and owners in `results[].reason`. GitHub owns full coverage, native builds, device lanes, provider integration, and history-backed compatibility. Related tests do not replace those obligations. Check the Integration Tests and Coverage jobs at the actual PR head. -Docs-only changes need guidance checks, not runtime tests or tests asserting prose. -See `CONTRIBUTING.md` for local coverage and [gate maintenance](gates.md) for declarations. +For docs-only changes, run the selected documentation checks. Do not add tests asserting prose. ## A gate failed @@ -46,12 +44,12 @@ Keep failure attribution separate from validation status. Never call a failing g | Evidence | Next action | Outcome | | --- | --- | --- | | New failure caused by this change | Fix its owner. Rerun after the fix. | Validation failed until a new run passes. | -| Suspected host contention or infrastructure fault | Save the log. Run the same failing command on unchanged base in an isolated checkout, without competing tests. | Cause unproven until compared. Compare both runs. A solo pass alone does not prove contention. | -| Same failure reproduced on unchanged base | Record both SHAs, command, logs, and matching failure. Assign the blocker to a named person or linked issue. | Base-blocked. If publication is authorized, open/update an incomplete draft with the blocker and remaining checks. Do not mark merge-ready. | -| Base comparison unavailable or inconclusive | Record the missing tool/device/evidence and next action. | Blocked, cause unproven. An authorized draft can carry this limitation. | +| Suspected host contention or infrastructure fault | Save the log. Run the same failing command on unchanged base in an isolated checkout, without competing tests. | Cause unproven until compared. A solo pass does not prove contention. | +| Same failure reproduced on unchanged base | Record SHAs, command, logs, and matching failure. Assign the blocker to a named person or linked issue. | Base-blocked. If publication is authorized, publish a draft with the blocker and remaining checks. Do not mark merge-ready. | +| Base comparison unavailable or inconclusive | Record the missing tool/device/evidence and next action. | Blocked, cause unproven. An authorized draft can record this. | | Failure source fixed, environment repaired, or new upstream commit contains the fix | Run the failing check, then the affected gate on the current head. | Replace blocked status only with new evidence. | -Match failures by check, location, diagnostic code, and failing assertion. Log comparison is evidence, not production error-text branching. +Match failures by check, location, diagnostic code, and assertion. The implementing agent owns diagnosis until a named owner or issue accepts the blocker. Do not loop on an unchanged failure. Do not widen timeouts, mask exit codes, or invoke CI authority as a local pass. Rerun failed CI once only when its log shows an infrastructure fault. Treat the same failure twice as a finding. diff --git a/packages/host-kit/src/internal/exec.test.ts b/packages/host-kit/src/internal/exec.test.ts index 680639fa25..20bb597104 100644 --- a/packages/host-kit/src/internal/exec.test.ts +++ b/packages/host-kit/src/internal/exec.test.ts @@ -623,3 +623,25 @@ test('a non-xcrun failure keeps the caller hint', () => { }, ); }); + +test('runCmdStreaming can forward output without retaining it in the result', async () => { + let stdout = ''; + let stderr = ''; + const result = await runCmdStreaming( + process.execPath, + ['-e', "process.stdout.write('out'); process.stderr.write('err'); process.exitCode = 7;"], + { + allowFailure: true, + captureOutput: false, + onStdoutChunk: (chunk) => { + stdout += chunk; + }, + onStderrChunk: (chunk) => { + stderr += chunk; + }, + }, + ); + assert.equal(stdout, 'out'); + assert.equal(stderr, 'err'); + assert.deepEqual(result, { stdout: '', stderr: '', exitCode: 7, stdoutBuffer: undefined }); +}); diff --git a/packages/host-kit/src/internal/exec.ts b/packages/host-kit/src/internal/exec.ts index d7adefd874..0df5318cde 100644 --- a/packages/host-kit/src/internal/exec.ts +++ b/packages/host-kit/src/internal/exec.ts @@ -37,6 +37,8 @@ export type ExecOptions = { }; export type ExecStreamOptions = ExecOptions & { + /** Set false when chunk callbacks own output and only the exit code is needed. */ + captureOutput?: boolean; onStdoutChunk?: (chunk: string) => void; onStderrChunk?: (chunk: string) => void; onSpawn?: (child: ReturnType) => void; @@ -189,7 +191,8 @@ function runSpawnedCommand( options.onSpawn?.(child); let stdout = ''; - const stdoutChunks: Buffer[] | undefined = options.binaryStdout ? [] : undefined; + const stdoutChunks: Buffer[] | undefined = + options.binaryStdout && options.captureOutput !== false ? [] : undefined; let stderr = ''; let didTimeout = false; const timeoutMs = normalizeTimeoutMs(options.timeoutMs); @@ -266,13 +269,13 @@ function runSpawnedCommand( return; } const text = String(chunk); - stdout += text; + if (options.captureOutput !== false) stdout += text; options.onStdoutChunk?.(text); }); child.stderr.on('data', (chunk) => { const text = String(chunk); - stderr += text; + if (options.captureOutput !== false) stderr += text; options.onStderrChunk?.(text); }); diff --git a/scripts/check-affected/checkout-state.test.ts b/scripts/check-affected/checkout-state.test.ts index d75627511d..ef83a92e07 100644 --- a/scripts/check-affected/checkout-state.test.ts +++ b/scripts/check-affected/checkout-state.test.ts @@ -28,8 +28,18 @@ test('checkout fingerprint distinguishes edits with identical porcelain status a const untracked = readCheckoutState(cwd); fs.writeFileSync(path.join(cwd, 'new.ts'), 'two'); assert.notEqual(untracked.fingerprint, readCheckoutState(cwd).fingerprint); + const beforeChmod = readCheckoutState(cwd); + fs.chmodSync(path.join(cwd, 'new.ts'), 0o755); + if (process.platform !== 'win32') + assert.notEqual(beforeChmod.fingerprint, readCheckoutState(cwd).fingerprint); git('add', '.'); assert.notEqual(second.fingerprint, readCheckoutState(cwd).fingerprint); + // Both status and ls-files must tolerate more than spawnSync's default 1 MiB. + const wide = path.join(cwd, ...Array(3).fill('d'.repeat(220))); + fs.mkdirSync(wide, { recursive: true }); + for (let index = 0; index < 1200; index++) + fs.writeFileSync(path.join(wide, `${index}-${'x'.repeat(220)}`), ''); + assert.ok(readCheckoutState(cwd).status.length > 1024 * 1024); } finally { fs.rmSync(cwd, { recursive: true, force: true }); } diff --git a/scripts/check-affected/checkout-state.ts b/scripts/check-affected/checkout-state.ts index 8594eb5cd4..717759766c 100644 --- a/scripts/check-affected/checkout-state.ts +++ b/scripts/check-affected/checkout-state.ts @@ -6,7 +6,7 @@ import { runCmdSync } from '@agent-device/host-kit/command'; export type CheckoutState = { head: string; status: string; fingerprint: string }; export function gitValue(cwd: string, args: string[]): string { - return runCmdSync('git', args, { cwd }).stdout.trim(); + return runCmdSync('git', args, { cwd, maxBuffer: 64 * 1024 * 1024 }).stdout.trim(); } export function readCheckoutState(cwd: string): CheckoutState { @@ -21,6 +21,7 @@ export function readCheckoutState(cwd: string): CheckoutState { } const untracked = runCmdSync('git', ['ls-files', '--others', '--exclude-standard', '-z'], { cwd, + maxBuffer: 64 * 1024 * 1024, }).stdout; for (const file of untracked.split('\0').filter(Boolean).sort()) hashWorktreePath(hash, cwd, file); @@ -38,6 +39,7 @@ function hashWorktreePath(hash: crypto.Hash, cwd: string, file: string): void { hash.update('missing\0'); return; } + hash.update(String(stat.mode)).update('\0'); if (stat.isDirectory()) { hash.update('directory\0'); const files = runCmdSync( diff --git a/scripts/check-affected/command-execution.test.ts b/scripts/check-affected/command-execution.test.ts index bdc9abee41..5984c916c2 100644 --- a/scripts/check-affected/command-execution.test.ts +++ b/scripts/check-affected/command-execution.test.ts @@ -22,7 +22,7 @@ test('SIGTERM cancels the command group, records cancellation, and releases the { cwd, allowFailure: true, - env: { ...process.env, HOME: cwd, AGENT_DEVICE_VALIDATION_PARENT: '' }, + env: { ...process.env, HOME: cwd, USERPROFILE: cwd, AGENT_DEVICE_VALIDATION_PARENT: '' }, }, ); try { @@ -48,7 +48,7 @@ test('SIGTERM cancels the command group, records cancellation, and releases the test('a killed validation parent does not let a waiter overlap its surviving command group', async () => { const { cwd } = fixture(); const ready = path.join(cwd, '.tmp', 'orphan.json'); - const env = { ...process.env, HOME: cwd, AGENT_DEVICE_VALIDATION_PARENT: '' }; + const env = { ...process.env, HOME: cwd, USERPROFILE: cwd, AGENT_DEVICE_VALIDATION_PARENT: '' }; const source = ` import { runValidation } from ${JSON.stringify(entry)}; const code = await runValidation(${JSON.stringify(cwd)}, { base: 'HEAD', head: 'HEAD', json: true }, async ({execute}) => execute([process.execPath, '-e', ${JSON.stringify(`require('node:fs').writeFileSync(${JSON.stringify(ready)}, JSON.stringify(process.pid)); setInterval(() => {}, 1000);`)}], ${JSON.stringify(cwd)}, 'orphan')); diff --git a/scripts/check-affected/command-execution.ts b/scripts/check-affected/command-execution.ts index 798ac23ba3..9e65ee6d4e 100644 --- a/scripts/check-affected/command-execution.ts +++ b/scripts/check-affected/command-execution.ts @@ -1,16 +1,13 @@ import fs from 'node:fs'; import path from 'node:path'; import { runCmdStreaming } from '@agent-device/host-kit/command'; -import { - recordValidationCommand, - finishValidationCommand, - VALIDATION_PARENT_ENV, -} from './host-lock.ts'; +import { VALIDATION_PARENT_ENV, type ValidationLock } from './host-lock.ts'; import type { CheckResult, ValidationRun } from './execution.ts'; import type { TestReport } from './test-report.ts'; export async function executeValidationCommand( run: ValidationRun, + lock: ValidationLock, result: CheckResult, command: string[], cwd: string, @@ -29,21 +26,28 @@ export async function executeValidationCommand( 15_000, ); let commandPid: number | undefined; + const cancelTree = () => { + if (commandPid !== undefined) lock.cancelCommand(commandPid); + }; try { - recordValidationCommand(run.receiptPath, null); + lock.recordCommand(run.receiptPath, null); const output = await runCmdStreaming( process.execPath, ['--experimental-strip-types', path.join(import.meta.dirname, 'command.ts'), ...command], { cwd, allowFailure: true, + captureOutput: false, detached: process.platform !== 'win32', kill: { signal: 'SIGTERM', graceMs: 5000 }, signal: run.controller.signal, stdin: 'run\n', onSpawn: (child) => { commandPid = child.pid; - if (commandPid) recordValidationCommand(run.receiptPath, commandPid); + if (commandPid) lock.recordCommand(run.receiptPath, commandPid); + // Register before host-kit installs its abort handler, which can kill the wrapper. + if (process.platform === 'win32') + run.controller.signal.addEventListener('abort', cancelTree, { once: true }); }, env: { ...process.env, @@ -68,25 +72,33 @@ export async function executeValidationCommand( result.reason = error instanceof Error ? error.message : String(error); throw error; } finally { + run.controller.signal.removeEventListener('abort', cancelTree); clearInterval(heartbeat); fs.closeSync(descriptor); result.durationMs = Date.now() - started; collectTestReports(result, run.directory, reportPrefix); run.save(); - await finishValidationCommand(commandPid); + await lock.finishCommand(commandPid); } } function reportFailureTail(logPath: string, report: (line: string) => void): void { - report( - fs - .readFileSync(logPath, 'utf8') - .slice(-8000) - .split('\n') - .slice(-35) - .map((line) => line.slice(0, 400)) - .join('\n'), - ); + const descriptor = fs.openSync(logPath, 'r'); + try { + const size = fs.fstatSync(descriptor).size; + const tail = Buffer.alloc(Math.min(size, 8000)); + fs.readSync(descriptor, tail, 0, tail.length, size - tail.length); + report( + tail + .toString('utf8') + .split('\n') + .slice(-35) + .map((line) => line.slice(0, 400)) + .join('\n'), + ); + } finally { + fs.closeSync(descriptor); + } } function collectTestReports(result: CheckResult, directory: string, reportPrefix: string): void { diff --git a/scripts/check-affected/command.test.ts b/scripts/check-affected/command.test.ts index 2f8d8d4e0b..6bbb19b523 100644 --- a/scripts/check-affected/command.test.ts +++ b/scripts/check-affected/command.test.ts @@ -3,9 +3,9 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { test } from 'node:test'; -import { runCmdSync } from '@agent-device/host-kit/command'; +import { runCmdSync, runCmdStreaming } from '@agent-device/host-kit/command'; -test('command cannot execute before activation and preserves the activated exit code', () => { +test('command cannot execute before activation and preserves the activated exit code', async () => { const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'validation-activation-')); const marker = path.join(directory, 'started'); const args = [ @@ -13,15 +13,26 @@ test('command cannot execute before activation and preserves the activated exit path.join(import.meta.dirname, 'command.ts'), process.execPath, '-e', - `require('node:fs').writeFileSync(${JSON.stringify(marker)}, 'yes'); process.exit(7);`, + `require('node:fs').writeFileSync(${JSON.stringify(marker)}, 'yes'); process.stdout.write('o'.repeat(2000000)); process.stderr.write('e'.repeat(2000000)); process.exitCode = 7;`, ]; try { assert.equal(runCmdSync(process.execPath, args, { allowFailure: true }).exitCode, 1); assert.equal(fs.existsSync(marker), false); - assert.equal( - runCmdSync(process.execPath, args, { allowFailure: true, stdin: 'run\n' }).exitCode, - 7, - ); + const activated = await runCmdStreaming(process.execPath, args, { + allowFailure: true, + stdin: 'run\n', + onSpawn: (child) => { + child.stdout!.pause(); + child.stderr!.pause(); + setTimeout(() => { + child.stdout!.resume(); + child.stderr!.resume(); + }, 500); + }, + }); + assert.equal(activated.exitCode, 7); + assert.equal(activated.stdout, 'o'.repeat(2000000)); + assert.equal(activated.stderr, 'e'.repeat(2000000)); assert.equal(fs.readFileSync(marker, 'utf8'), 'yes'); } finally { fs.rmSync(directory, { recursive: true, force: true }); diff --git a/scripts/check-affected/command.ts b/scripts/check-affected/command.ts index 5971488deb..46ab92f5ad 100644 --- a/scripts/check-affected/command.ts +++ b/scripts/check-affected/command.ts @@ -10,6 +10,7 @@ runEntrypoint('validation command', async () => { if (!command) return 1; const result = await runCmdStreaming(command, args, { allowFailure: true, + captureOutput: false, onStdoutChunk: (chunk) => { process.stdout.write(chunk); }, @@ -17,5 +18,13 @@ runEntrypoint('validation command', async () => { process.stderr.write(chunk); }, }); + await Promise.all( + [process.stdout, process.stderr].map( + (stream) => + new Promise((resolve, reject) => + stream.write('', (error) => (error ? reject(error) : resolve())), + ), + ), + ); return result.exitCode; }); diff --git a/scripts/check-affected/execution.fixtures.ts b/scripts/check-affected/execution.fixtures.ts index 9f3e9d42c6..b65a8cb60e 100644 --- a/scripts/check-affected/execution.fixtures.ts +++ b/scripts/check-affected/execution.fixtures.ts @@ -39,7 +39,7 @@ export function run(cwd: string, head: string, body: string) { { cwd, allowFailure: true, - env: { ...process.env, HOME: cwd, AGENT_DEVICE_VALIDATION_PARENT: '' }, + env: { ...process.env, HOME: cwd, USERPROFILE: cwd, AGENT_DEVICE_VALIDATION_PARENT: '' }, }, ); return { ...result, receipt: JSON.parse(result.stdout) as ExecutionReceipt }; diff --git a/scripts/check-affected/execution.ts b/scripts/check-affected/execution.ts index 0a2161c367..72e5745205 100644 --- a/scripts/check-affected/execution.ts +++ b/scripts/check-affected/execution.ts @@ -64,7 +64,8 @@ export async function runValidation( report(`[receipt] ${receiptPath}`); try { - release = await acquireValidationLock(controller.signal); + const lock = await acquireValidationLock(controller.signal); + release = lock.release; prepareCheckout(cwd, args, receipt); receipt.status = 'running'; save(); @@ -72,7 +73,7 @@ export async function runValidation( receipt.exitCode = await task({ receipt, execute: (command, commandCwd, id) => - executeValidationCommand(run, checkResult(receipt, id), command, commandCwd), + executeValidationCommand(run, lock, checkResult(receipt, id), command, commandCwd), skip: (id, reason, deduplicated) => { Object.assign(checkResult(receipt, id), { status: deduplicated ? 'deduplicated' : 'skipped', diff --git a/scripts/check-affected/host-lock.test.ts b/scripts/check-affected/host-lock.test.ts index 8bd5784058..0f97b1044b 100644 --- a/scripts/check-affected/host-lock.test.ts +++ b/scripts/check-affected/host-lock.test.ts @@ -1,8 +1,10 @@ import assert from 'node:assert/strict'; +import childProcess from 'node:child_process'; +import { syncBuiltinESMExports } from 'node:module'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { test } from 'node:test'; +import { mock, test } from 'node:test'; import { acquireValidationLock, VALIDATION_PARENT_ENV } from './host-lock.ts'; test('host lock excludes a waiter, supports cancellation, then admits it after release', async () => { @@ -12,17 +14,51 @@ test('host lock excludes a waiter, supports cancellation, then admits it after r delete process.env[VALIDATION_PARENT_ENV]; const controller = new AbortController(); try { - const release = await acquireValidationLock(controller.signal, lock); + const owner = await acquireValidationLock(controller.signal, lock); const waiting = acquireValidationLock(controller.signal, lock, (message) => { assert.match(message, /wait.*host validation/); controller.abort(); }); await assert.rejects(waiting, { name: 'AbortError' }); assert.ok(fs.existsSync(lock), 'cancelled waiter must not release the owner'); - await release(); + owner.recordCommand('/fixture/receipt.json', null); + assert.ok(fs.existsSync(`${lock}.active.json`)); + await owner.finishCommand(undefined); + assert.equal(fs.existsSync(`${lock}.active.json`), false); + await owner.release(); const second = await acquireValidationLock(new AbortController().signal, lock); - await second(); + await second.release(); assert.equal(fs.existsSync(lock), false); + // A Windows orphan has a live PID, but no POSIX process group. + fs.writeFileSync( + `${lock}.active.json`, + JSON.stringify({ + pid: process.pid, + startTime: null, + activation: 'stdin', + receipt: '/orphan/receipt.json', + }), + ); + const platform = Object.getOwnPropertyDescriptor(process, 'platform')!; + const kill = process.kill.bind(process); + const probe = mock.method(process, 'kill', (pid: number, signal?: number | NodeJS.Signals) => { + if (pid < 0) throw Object.assign(new Error('no process groups'), { code: 'ESRCH' }); + return kill(pid, signal); + }); + try { + Object.defineProperty(process, 'platform', { value: 'win32' }); + const orphanWaiter = new AbortController(); + await assert.rejects( + acquireValidationLock(orphanWaiter.signal, lock, (message) => { + assert.match(message, /orphan validation group/); + orphanWaiter.abort(); + }), + { name: 'AbortError' }, + ); + } finally { + Object.defineProperty(process, 'platform', platform); + probe.mock.restore(); + } } finally { if (parent === undefined) delete process.env[VALIDATION_PARENT_ENV]; else process.env[VALIDATION_PARENT_ENV] = parent; @@ -43,3 +79,43 @@ test('nested validation fails before acquisition', async () => { else process.env[VALIDATION_PARENT_ENV] = parent; } }); + +test('Windows cancellation confirms the tree or preserves a blocking marker', async () => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'validation-windows-')); + const lock = path.join(directory, 'gate.lock'); + const parent = process.env[VALIDATION_PARENT_ENV]; + delete process.env[VALIDATION_PARENT_ENV]; + const owner = await acquireValidationLock(new AbortController().signal, lock); + owner.recordCommand('/fixture/receipt.json', process.pid); + const platform = Object.getOwnPropertyDescriptor(process, 'platform')!; + let exitCode = 1; + const spawn = mock.method(childProcess, 'spawnSync', (command: string, args: string[]) => { + assert.equal(command, 'taskkill.exe'); + assert.deepEqual(args, ['/PID', String(process.pid), '/T', '/F']); + return { status: exitCode, stdout: '', stderr: '' }; + }); + syncBuiltinESMExports(); + try { + Object.defineProperty(process, 'platform', { value: 'win32' }); + owner.cancelCommand(process.pid); + await assert.rejects(owner.finishCommand(undefined), /Cannot verify.*termination/); + await owner.release(); + await assert.rejects( + acquireValidationLock(new AbortController().signal, lock), + /Cannot verify.*termination/, + ); + assert.ok(fs.existsSync(`${lock}.active.json`)); + exitCode = 0; + owner.cancelCommand(process.pid); + await owner.finishCommand(undefined); + assert.equal(fs.existsSync(`${lock}.active.json`), false); + } finally { + Object.defineProperty(process, 'platform', platform); + spawn.mock.restore(); + syncBuiltinESMExports(); + await owner.release(); + if (parent === undefined) delete process.env[VALIDATION_PARENT_ENV]; + else process.env[VALIDATION_PARENT_ENV] = parent; + fs.rmSync(directory, { recursive: true, force: true }); + } +}); diff --git a/scripts/check-affected/host-lock.ts b/scripts/check-affected/host-lock.ts index 5595a2cb44..3c035b5a09 100644 --- a/scripts/check-affected/host-lock.ts +++ b/scripts/check-affected/host-lock.ts @@ -1,6 +1,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; +import { runCmdSync } from '@agent-device/host-kit/command'; import { setTimeout } from 'node:timers/promises'; import { publishFileSync, @@ -10,6 +11,7 @@ import { } from '@agent-device/host-kit/file'; import { isProcessGroupAlive, + isProcessAlive, readCurrentOwnerIdentity, readProcessStartTime, ownerIdentityDiffers, @@ -19,11 +21,18 @@ import { export const VALIDATION_PARENT_ENV = 'AGENT_DEVICE_VALIDATION_PARENT'; const HOST_LOCK_PATH = path.join(os.homedir(), '.cache', 'agent-device', 'validation.lock'); +export type ValidationLock = { + release: ProcessLockRelease; + recordCommand: (receipt: string, pid: number | null) => void; + finishCommand: (pid: number | undefined) => Promise; + cancelCommand: (pid: number) => void; +}; + export async function acquireValidationLock( signal: AbortSignal, lockDirPath = HOST_LOCK_PATH, report: (message: string) => void = (message) => process.stderr.write(`${message}\n`), -): Promise { +): Promise { if (process.env[VALIDATION_PARENT_ENV]) { throw new Error( `Nested validation is not allowed. Parent receipt: ${process.env[VALIDATION_PARENT_ENV]}`, @@ -41,7 +50,12 @@ export async function acquireValidationLock( : tryAcquireProcessLock({ lockDirPath, owner, description: 'host validation' }); if (attempt.status === 'acquired') { fs.rmSync(activePath, { force: true }); - return attempt.acquisition.release; + return { + release: attempt.acquisition.release, + recordCommand: (receipt, pid) => recordValidationCommand(lockDirPath, receipt, pid), + finishCommand: (pid) => finishValidationCommand(lockDirPath, pid), + cancelCommand: (pid) => cancelValidationCommand(lockDirPath, pid), + }; } if (Date.now() >= deadline) throw new Error(`Host validation wait exceeded 2 hours: ${lockDirPath}`); @@ -55,9 +69,9 @@ export async function acquireValidationLock( } } -export function recordValidationCommand(receipt: string, pid: number | null): void { +function recordValidationCommand(lockDirPath: string, receipt: string, pid: number | null): void { publishFileSync({ - destination: `${HOST_LOCK_PATH}.active.json`, + destination: `${lockDirPath}.active.json`, contents: JSON.stringify({ receipt, pid, @@ -67,19 +81,53 @@ export function recordValidationCommand(receipt: string, pid: number | null): vo }); } -export async function finishValidationCommand(pid: number | undefined): Promise { - if (pid !== undefined && process.platform !== 'win32') { - const deadline = Date.now() + 10_000; - while (isProcessGroupAlive(pid)) { - if (Date.now() >= deadline) - throw new Error( - `Validation command group ${pid} has not exited; host validation remains blocked.`, - ); - if (Date.now() >= deadline - 5000) signalProcessGroupBestEffort(pid, 'SIGKILL'); - await setTimeout(100); +// Mark uncertainty before taskkill: a partial tree termination must never permit another gate. +function cancelValidationCommand(lockDirPath: string, pid: number): void { + if (process.platform !== 'win32') return; + const destination = `${lockDirPath}.active.json`; + const active = JSON.parse(fs.readFileSync(destination, 'utf8')); + publishFileSync({ + destination, + contents: JSON.stringify({ ...active, terminationUnverified: true }), + }); + try { + const result = runCmdSync('taskkill.exe', ['/PID', String(pid), '/T', '/F'], { + allowFailure: true, + timeoutMs: 5000, + }); + if (result.exitCode === 0) + publishFileSync({ + destination, + contents: JSON.stringify({ ...active, terminationUnverified: false }), + }); + } catch { + // Preserve the blocking marker when Windows cannot confirm tree termination. + } +} + +async function finishValidationCommand( + lockDirPath: string, + pid: number | undefined, +): Promise { + readActiveCommand(`${lockDirPath}.active.json`); + if (pid !== undefined) await waitForCommandExit(lockDirPath, pid); + readActiveCommand(`${lockDirPath}.active.json`); + fs.rmSync(`${lockDirPath}.active.json`, { force: true }); +} + +async function waitForCommandExit(lockDirPath: string, pid: number): Promise { + const deadline = Date.now() + 10_000; + while (process.platform === 'win32' ? isProcessAlive(pid) : isProcessGroupAlive(pid)) { + if (Date.now() >= deadline) + throw new Error( + `Validation command group ${pid} has not exited; host validation remains blocked.`, + ); + if (Date.now() >= deadline - 5000) { + if (process.platform === 'win32') cancelValidationCommand(lockDirPath, pid); + else signalProcessGroupBestEffort(pid, 'SIGKILL'); } + await setTimeout(100); } - fs.rmSync(`${HOST_LOCK_PATH}.active.json`, { force: true }); } function readOrphanCommand(lockDirPath: string): { pid: number; receipt: string } | null { @@ -98,7 +146,11 @@ function readOrphanCommand(lockDirPath: string): { pid: number; receipt: string if (active.pid === null) return null; const observed = { pid: active.pid, startTime: readProcessStartTime(active.pid) }; if (ownerIdentityDiffers({ pid: active.pid, startTime: active.startTime }, observed)) return null; - return isProcessGroupAlive(active.pid) ? { pid: active.pid, receipt: active.receipt } : null; + return ( + process.platform === 'win32' ? isProcessAlive(active.pid) : isProcessGroupAlive(active.pid) + ) + ? { pid: active.pid, receipt: active.receipt } + : null; } function readActiveCommand( @@ -116,7 +168,13 @@ function readActiveCommand( startTime: string | null; receipt: string; activation: string; + terminationUnverified?: boolean; }; + if (active.terminationUnverified) { + throw new Error( + `Cannot verify validation command tree termination: ${activePath}; receipt=${active.receipt}`, + ); + } if ( active.activation !== 'stdin' || (active.pid !== null && (!Number.isInteger(active.pid) || active.pid <= 0)) diff --git a/scripts/check-affected/run-cli.test.ts b/scripts/check-affected/run-cli.test.ts index 27dd5f5676..5ffeee9dc1 100644 --- a/scripts/check-affected/run-cli.test.ts +++ b/scripts/check-affected/run-cli.test.ts @@ -15,11 +15,16 @@ function fixture() { git('init', '-q'); git('config', 'user.email', 'test@example.com'); git('config', 'user.name', 'Test'); - const pkg = JSON.parse(fs.readFileSync(path.join(root, 'package.json'), 'utf8')); + const { scripts } = JSON.parse(fs.readFileSync(path.join(root, 'package.json'), 'utf8')); + const pkg = { name: 'validation-fixture', private: true, scripts }; pkg.scripts['check:agent-guidance'] = 'node -e "process.exit(7)"'; for (const script of ['check:tooling', 'check:fallow', 'check:unit']) pkg.scripts[script] = 'node -e "process.exit(0)"'; fs.writeFileSync(path.join(cwd, 'package.json'), JSON.stringify(pkg)); + fs.writeFileSync( + path.join(cwd, 'pnpm-workspace.yaml'), + 'packages: []\npmOnFail: warn\nverifyDepsBeforeRun: warn\n', + ); fs.writeFileSync(path.join(cwd, '.gitignore'), '.tmp\n.cache\n'); fs.writeFileSync(path.join(cwd, 'AGENTS.md'), 'base'); git('add', '.'); @@ -41,6 +46,7 @@ function invoke(cwd: string, args: string[]) { env: { ...process.env, HOME: path.join(cwd, '.tmp', 'home'), + USERPROFILE: path.join(cwd, '.tmp', 'home'), AGENT_DEVICE_VALIDATION_PARENT: '', }, }, diff --git a/scripts/check-affected/run.test.ts b/scripts/check-affected/run.test.ts index fbf347d682..8cc6dc80fd 100644 --- a/scripts/check-affected/run.test.ts +++ b/scripts/check-affected/run.test.ts @@ -165,7 +165,7 @@ test('runChecks passes the selector change set to Vitest related', async () => { ); }); -test('runChecks skips GitHub-authoritative checks and passes when locals succeed', async () => { +test('runChecks skips CI checks, deduplicates related tests, and preserves local ordering', async () => { const executed: string[][] = []; const execute: CommandExecutor = async (command) => { executed.push(command); @@ -194,19 +194,6 @@ test('runChecks skips GitHub-authoritative checks and passes when locals succeed `${skipped} is GitHub-authoritative and must not run locally`, ); } -}); - -test('runChecks leaves coverage to CI and runs related tests once through Vitest config', async () => { - const executed: string[][] = []; - const execute: CommandExecutor = async (command) => { - executed.push(command); - return 0; - }; - const plan = selectChecks({ changedFiles: ['unknown/path.xyz'], packageEntryFiles: [] }); - - const code = await runChecks(plan, { scripts: ALL_SCRIPTS }, ARGS, { execute, cwd: '.' }); - - assert.equal(code, 0); const related = executed.filter((command) => command.includes('related')); assert.equal(related.length, 1); assert.ok( diff --git a/scripts/check-affected/run.ts b/scripts/check-affected/run.ts index 08ead90aaf..eb3c9c5bcb 100644 --- a/scripts/check-affected/run.ts +++ b/scripts/check-affected/run.ts @@ -8,7 +8,7 @@ import fs from 'node:fs'; import path from 'node:path'; import { pathToFileURL } from 'node:url'; -import { runCmdStreaming, runCmdSync } from '@agent-device/host-kit/command'; +import { runCmdSync } from '@agent-device/host-kit/command'; import { checkLockfileInstallSync, STALE_NODE_MODULES_MESSAGE, @@ -188,16 +188,6 @@ function printPlanHuman(plan: CheckPlan, args: Args): void { // testable without spawning real processes. export type CommandExecutor = (command: string[], cwd: string, id: string) => Promise; -const streamingExecutor: CommandExecutor = async (command, cwd) => { - const result = await runCmdStreaming(command[0]!, command.slice(1), { - cwd, - allowFailure: true, - onStdoutChunk: (chunk) => void process.stdout.write(chunk), - onStderrChunk: (chunk) => void process.stderr.write(chunk), - }); - return result.exitCode; -}; - export async function runChecks( plan: CheckPlan, pkg: PackageJson, @@ -205,10 +195,10 @@ export async function runChecks( options: { cwd?: string; skip?: ValidationContext['skip']; - execute?: CommandExecutor; + execute: CommandExecutor; changedFiles?: readonly string[]; checkLockfileSync?: (cwd: string) => LockfileInstallSyncResult; - } = {}, + }, ): Promise { const cwd = options.cwd ?? repoRoot; const checkLockfileSync = options.checkLockfileSync ?? checkLockfileInstallSync; @@ -218,7 +208,7 @@ export async function runChecks( reportStaleInstall(lockfileSync.reason, cwd); return 1; } - const execute = options.execute ?? streamingExecutor; + const execute = options.execute; const runnable = plan.checks.map(getCheckSpec).filter((spec: CheckSpec) => spec.localRunnable); const skipped = plan.checks.map(getCheckSpec).filter((spec: CheckSpec) => !spec.localRunnable); const relatedSelected = plan.checks.includes('vitest-related'); @@ -234,7 +224,6 @@ export async function runChecks( continue; } const command = resolveCommand(spec, pkg.scripts, args.base, options.changedFiles ?? []); - if (!options.execute) process.stderr.write(`[run] ${spec.id}: ${command.join(' ')}\n`); const exitCode = await execute(command, cwd, spec.id); if (exitCode !== 0) { process.stderr.write(`\ncheck:affected: ${spec.id} failed.\n`); diff --git a/scripts/check-affected/test-report.test.ts b/scripts/check-affected/test-report.test.ts index b696331cc8..74c3b4101b 100644 --- a/scripts/check-affected/test-report.test.ts +++ b/scripts/check-affected/test-report.test.ts @@ -1,15 +1,13 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; -import os from 'node:os'; import path from 'node:path'; import { pathToFileURL } from 'node:url'; import { test } from 'node:test'; -import { runCmdSync } from '@agent-device/host-kit/command'; +import { fixture, run } from './execution.fixtures.ts'; import type { TestReport } from './test-report.ts'; test('real Vitest report preserves assertion, suite, and unhandled failures separately', () => { - const root = fs.mkdtempSync(path.join(os.tmpdir(), 'validation-report-')); - const reportPath = path.join(root, 'report.json'); + const { cwd: root } = fixture(); const reporter = pathToFileURL(path.join(import.meta.dirname, 'test-report.ts')).href; try { fs.writeFileSync( @@ -17,7 +15,7 @@ test('real Vitest report preserves assertion, suite, and unhandled failures sepa ` import { validationTestReporter } from ${JSON.stringify(reporter)}; export default { test: { root: ${JSON.stringify(root)}, maxWorkers: 1, - reporters: ['dot', validationTestReporter(${JSON.stringify(reportPath)})] } }; + reporters: ['dot', validationTestReporter(process.env.AGENT_DEVICE_VALIDATION_REPORT + '.' + process.pid + '.json')] } }; `, ); fs.writeFileSync( @@ -30,18 +28,15 @@ test('real Vitest report preserves assertion, suite, and unhandled failures sepa describe('suite', () => { beforeAll(() => { throw new Error('fixture-suite'); }); test('not reached', () => {}); }); `, ); - const result = runCmdSync( - 'pnpm', - ['exec', 'vitest', 'run', '--config', path.join(root, 'vitest.config.ts')], - { - cwd: path.resolve(import.meta.dirname, '../..'), - allowFailure: true, - env: { ...process.env, AGENT_DEVICE_VALIDATION_REPORT: '' }, - }, + const result = run( + root, + 'HEAD', + `return execute(['pnpm', 'exec', 'vitest', 'run', '--config', ${JSON.stringify(path.join(root, 'vitest.config.ts'))}], ${JSON.stringify(path.resolve(import.meta.dirname, '../..'))}, 'report');`, ); assert.equal(result.exitCode, 1); - assert.ok(fs.existsSync(reportPath), result.stderr + result.stdout); - const report = JSON.parse(fs.readFileSync(reportPath, 'utf8')) as TestReport; + const check = result.receipt.results[0]!; + assert.equal(check.testReportPaths?.length, 1); + const report = check.tests?.[0] as TestReport; assert.equal(report.passed, 2); assert.ok(report.failed >= 1); assert.equal(report.unhandledErrors, 1); From be7a8aef93587f0f4ed5ef238261c0afc6b5a437 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 13:37:56 +0200 Subject: [PATCH 7/8] test(android): verify the final fill sample without a capture quota --- .../src/__tests__/fill-verification.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/platform-android/src/__tests__/fill-verification.test.ts b/packages/platform-android/src/__tests__/fill-verification.test.ts index f9890ac4b3..69725ead74 100644 --- a/packages/platform-android/src/__tests__/fill-verification.test.ts +++ b/packages/platform-android/src/__tests__/fill-verification.test.ts @@ -118,7 +118,9 @@ test('a value the app renders late still verifies, as long as it lands before th }); test('a value that never lands is a mismatch read from the last sample', async () => { - const session = createFillHelperSession({ textForCapture: () => 'Key echo' }); + const session = createFillHelperSession({ + textForCapture: (captureIndex) => `Key echo ${captureIndex}`, + }); let elapsedMs = 0; const verification = await underFakeTimers(async () => { @@ -136,12 +138,11 @@ test('a value that never lands is a mismatch read from the last sample', async ( assert.equal(verification.ok, false); assert.equal(verification.reason, 'text_mismatch'); - assert.equal(verification.actual, 'Key echo'); + assert.equal(verification.actual, `Key echo ${session.captureCount()}`); assert.ok( elapsedMs >= 1_500 && elapsedMs < 1_650, `sampled until the deadline, took ${elapsedMs} ms`, ); - assert.ok(session.captureCount() >= 9, 'kept sampling on its interval'); }); /** From 799b14c5f2260b221698f494374cc52280f4a4af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 16 Oct 2026 14:15:51 +0200 Subject: [PATCH 8/8] fix(tooling): limit serialized validation to supported hosts --- CONTRIBUTING.md | 3 + docs/agents/testing.md | 8 +- scripts/check-affected/checkout-state.test.ts | 1 + .../check-affected/command-execution.test.ts | 188 +++++++++--------- scripts/check-affected/command-execution.ts | 9 +- scripts/check-affected/execution.test.ts | 130 ++++++------ scripts/check-affected/host-lock.test.ts | 147 +++++--------- scripts/check-affected/host-lock.ts | 52 +---- scripts/check-affected/run-cli.test.ts | 68 ++++--- scripts/check-affected/test-report.test.ts | 100 +++++----- 10 files changed, 326 insertions(+), 380 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5453163cd9..0fd0eacb6f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -217,6 +217,9 @@ For broad refactors or when explicitly requested, run the deterministic core agg pnpm check ``` +Serialized validation (`pnpm check` and `check:affected --run`) requires macOS or Linux. +On Windows, use the affected plan and direct focused checks. + `pnpm check` takes the same host lock as `check:affected --run`, prints a receipt path, and writes logs under `.tmp/validation`. It covers formatting, lint, typechecking, layering, dependency-graph parity, production exports, MCP metadata, the distributable build, bundle ownership, Fallow, unit tests, and local smoke diff --git a/docs/agents/testing.md b/docs/agents/testing.md index 72976fbf58..6f5ae58238 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -1,6 +1,6 @@ # Validate a change -Use this to select checks and interpret results. Use [isolated experiments](test-experiments.md) for mutations or base comparisons. +For mutations or base comparisons, use [isolated experiments](test-experiments.md). ## Run checks @@ -8,12 +8,12 @@ Use this to select checks and interpret results. Use [isolated experiments](test 2. Before pushing, commit the change. Run `pnpm check:affected --run --head `. 3. For a broad refactor or an explicitly requested full gate, run `pnpm check`. +Serialized validation requires macOS or Linux. On Windows, use plan mode or focused checks. Both broad commands acquire the same host lock. Wait for the owner to finish, or cancel your waiter with Ctrl-C. Do not add lock wrappers or delete lock files. A wait ends after two hours. Nested validation fails immediately. Focused tests do not lock. Avoid subprocess-heavy tests beside a broad gate. -The runner prints progress and a receipt path. The receipt links command logs and Vitest JSON. -It records refs, checkout state, commands, exit codes, skips, and failures. +The runner prints progress and a receipt path with refs, checkout state, command logs, exits, skips, and Vitest results. Execution stops at the first failure. Unexecuted checks remain `not-run`. - `pnpm check:affected`: compact plan. @@ -29,7 +29,7 @@ Initial/final checks detect persistent changes; they do not sandbox the checkout ## Interpret selection A package manifest or TypeScript config can affect every consumer. A full selection for these changes is intentional. -`FAIL-OPEN` means the selector could not safely narrow the set. Read `plan.failOpenReasons`. +`FAIL-OPEN` means the selector could not safely narrow the set. Read `failOpenReasons` in plan JSON or `plan.failOpenReasons` in a receipt. Do not reduce selection to get a pass. Read required-check status on the PR. The receipt lists skipped checks and owners in `results[].reason`. diff --git a/scripts/check-affected/checkout-state.test.ts b/scripts/check-affected/checkout-state.test.ts index ef83a92e07..eee8bef01c 100644 --- a/scripts/check-affected/checkout-state.test.ts +++ b/scripts/check-affected/checkout-state.test.ts @@ -35,6 +35,7 @@ test('checkout fingerprint distinguishes edits with identical porcelain status a git('add', '.'); assert.notEqual(second.fingerprint, readCheckoutState(cwd).fingerprint); // Both status and ls-files must tolerate more than spawnSync's default 1 MiB. + git('config', 'core.longpaths', 'true'); const wide = path.join(cwd, ...Array(3).fill('d'.repeat(220))); fs.mkdirSync(wide, { recursive: true }); for (let index = 0; index < 1200; index++) diff --git a/scripts/check-affected/command-execution.test.ts b/scripts/check-affected/command-execution.test.ts index 5984c916c2..11d4c68513 100644 --- a/scripts/check-affected/command-execution.test.ts +++ b/scripts/check-affected/command-execution.test.ts @@ -7,108 +7,116 @@ import { runCmdBackground } from '@agent-device/host-kit/command'; import { entry, fixture, run } from './execution.fixtures.ts'; import type { ExecutionReceipt } from './execution.ts'; -test('SIGTERM cancels the command group, records cancellation, and releases the host lock', async () => { - const { cwd } = fixture(); - const ready = path.join(cwd, '.tmp', 'ready.json'); - const source = ` +test( + 'SIGTERM cancels the command group, records cancellation, and releases the host lock', + { skip: process.platform === 'win32' }, + async () => { + const { cwd } = fixture(); + const ready = path.join(cwd, '.tmp', 'ready.json'); + const source = ` import { runValidation } from ${JSON.stringify(entry)}; const code = await runValidation(${JSON.stringify(cwd)}, { base: 'HEAD', head: 'HEAD', json: true }, async ({execute}) => { return execute([process.execPath, '-e', ${JSON.stringify(`require('node:fs').writeFileSync(${JSON.stringify(ready)}, JSON.stringify(process.pid)); setInterval(() => {}, 1000);`)}], ${JSON.stringify(cwd)}, 'sleep'); }); process.exitCode = code; `; - const { child, wait } = runCmdBackground( - process.execPath, - ['--experimental-strip-types', '--input-type=module', '-e', source], - { - cwd, - allowFailure: true, - env: { ...process.env, HOME: cwd, USERPROFILE: cwd, AGENT_DEVICE_VALIDATION_PARENT: '' }, - }, - ); - try { - const deadline = Date.now() + 5000; - while (!fs.existsSync(ready) && Date.now() < deadline) await setTimeout(20); - assert.ok(fs.existsSync(ready), 'command must actually start'); - const commandPid = JSON.parse(fs.readFileSync(ready, 'utf8')) as number; - child.kill('SIGTERM'); - const result = await wait; - assert.equal(result.exitCode, 143, result.stderr); - const receipt = JSON.parse(result.stdout) as ExecutionReceipt; - assert.equal(receipt.status, 'cancelled'); - assert.equal(receipt.results[0]?.status, 'cancelled'); - assert.throws(() => process.kill(commandPid, 0), { code: 'ESRCH' }); - assert.equal(run(cwd, 'HEAD', 'return 0;').exitCode, 0); - } finally { - child.kill('SIGKILL'); - await wait; - fs.rmSync(cwd, { recursive: true, force: true }); - } -}); + const { child, wait } = runCmdBackground( + process.execPath, + ['--experimental-strip-types', '--input-type=module', '-e', source], + { + cwd, + allowFailure: true, + env: { ...process.env, HOME: cwd, USERPROFILE: cwd, AGENT_DEVICE_VALIDATION_PARENT: '' }, + }, + ); + try { + const deadline = Date.now() + 5000; + while (!fs.existsSync(ready) && Date.now() < deadline) await setTimeout(20); + assert.ok(fs.existsSync(ready), 'command must actually start'); + const commandPid = JSON.parse(fs.readFileSync(ready, 'utf8')) as number; + child.kill('SIGTERM'); + const result = await wait; + assert.equal(result.exitCode, 143, result.stderr); + const receipt = JSON.parse(result.stdout) as ExecutionReceipt; + assert.equal(receipt.status, 'cancelled'); + assert.equal(receipt.results[0]?.status, 'cancelled'); + assert.throws(() => process.kill(commandPid, 0), { code: 'ESRCH' }); + assert.equal(run(cwd, 'HEAD', 'return 0;').exitCode, 0); + } finally { + child.kill('SIGKILL'); + await wait; + fs.rmSync(cwd, { recursive: true, force: true }); + } + }, +); -test('a killed validation parent does not let a waiter overlap its surviving command group', async () => { - const { cwd } = fixture(); - const ready = path.join(cwd, '.tmp', 'orphan.json'); - const env = { ...process.env, HOME: cwd, USERPROFILE: cwd, AGENT_DEVICE_VALIDATION_PARENT: '' }; - const source = ` +test( + 'a killed validation parent does not let a waiter overlap its surviving command group', + { skip: process.platform === 'win32' }, + async () => { + const { cwd } = fixture(); + const ready = path.join(cwd, '.tmp', 'orphan.json'); + const env = { ...process.env, HOME: cwd, USERPROFILE: cwd, AGENT_DEVICE_VALIDATION_PARENT: '' }; + const source = ` import { runValidation } from ${JSON.stringify(entry)}; const code = await runValidation(${JSON.stringify(cwd)}, { base: 'HEAD', head: 'HEAD', json: true }, async ({execute}) => execute([process.execPath, '-e', ${JSON.stringify(`require('node:fs').writeFileSync(${JSON.stringify(ready)}, JSON.stringify(process.pid)); setInterval(() => {}, 1000);`)}], ${JSON.stringify(cwd)}, 'orphan')); process.exitCode = code; `; - const parent = runCmdBackground( - process.execPath, - ['--experimental-strip-types', '--input-type=module', '-e', source], - { cwd, env, allowFailure: true }, - ); - let commandPid: number | undefined; - let waiter: ReturnType | undefined; - try { - const deadline = Date.now() + 5000; - while (!fs.existsSync(ready) && Date.now() < deadline) await setTimeout(20); - assert.ok(fs.existsSync(ready)); - commandPid = ( - JSON.parse( - fs.readFileSync( - path.join(cwd, '.cache', 'agent-device', 'validation.lock.active.json'), - 'utf8', - ), - ) as { pid: number } - ).pid; - parent.child.kill('SIGKILL'); - await parent.wait; - waiter = runCmdBackground( + const parent = runCmdBackground( process.execPath, - [ - '--experimental-strip-types', - '--input-type=module', - '-e', - ` + ['--experimental-strip-types', '--input-type=module', '-e', source], + { cwd, env, allowFailure: true }, + ); + let commandPid: number | undefined; + let waiter: ReturnType | undefined; + try { + const deadline = Date.now() + 5000; + while (!fs.existsSync(ready) && Date.now() < deadline) await setTimeout(20); + assert.ok(fs.existsSync(ready)); + commandPid = ( + JSON.parse( + fs.readFileSync( + path.join(cwd, '.cache', 'agent-device', 'validation.lock.active.json'), + 'utf8', + ), + ) as { pid: number } + ).pid; + parent.child.kill('SIGKILL'); + await parent.wait; + waiter = runCmdBackground( + process.execPath, + [ + '--experimental-strip-types', + '--input-type=module', + '-e', + ` import { runValidation } from ${JSON.stringify(entry)}; process.exitCode = await runValidation(${JSON.stringify(cwd)}, {base:'HEAD',head:'HEAD',json:true}, async () => 0); `, - ], - { cwd, env, allowFailure: true }, - ); - let observedWait = false; - waiter.child.stderr?.on('data', (data) => { - if (String(data).includes('[wait] orphan')) observedWait = true; - }); - const waitDeadline = Date.now() + 5000; - while (!observedWait && Date.now() < waitDeadline) await setTimeout(20); - assert.equal(observedWait, true, 'waiter must observe surviving command before acquiring'); - process.kill(-commandPid, 'SIGTERM'); - const result = await waiter.wait; - assert.equal(result.exitCode, 0, result.stderr); - assert.equal(JSON.parse(result.stdout).status, 'passed'); - } finally { - parent.child.kill('SIGKILL'); - waiter?.child.kill('SIGTERM'); - if (commandPid) { - try { - process.kill(-commandPid, 'SIGKILL'); - } catch {} + ], + { cwd, env, allowFailure: true }, + ); + let observedWait = false; + waiter.child.stderr?.on('data', (data) => { + if (String(data).includes('[wait] orphan')) observedWait = true; + }); + const waitDeadline = Date.now() + 5000; + while (!observedWait && Date.now() < waitDeadline) await setTimeout(20); + assert.equal(observedWait, true, 'waiter must observe surviving command before acquiring'); + process.kill(-commandPid, 'SIGTERM'); + const result = await waiter.wait; + assert.equal(result.exitCode, 0, result.stderr); + assert.equal(JSON.parse(result.stdout).status, 'passed'); + } finally { + parent.child.kill('SIGKILL'); + waiter?.child.kill('SIGTERM'); + if (commandPid) { + try { + process.kill(-commandPid, 'SIGKILL'); + } catch {} + } + await parent.wait; + await waiter?.wait; + fs.rmSync(cwd, { recursive: true, force: true }); } - await parent.wait; - await waiter?.wait; - fs.rmSync(cwd, { recursive: true, force: true }); - } -}); + }, +); diff --git a/scripts/check-affected/command-execution.ts b/scripts/check-affected/command-execution.ts index 9e65ee6d4e..7d7d92d154 100644 --- a/scripts/check-affected/command-execution.ts +++ b/scripts/check-affected/command-execution.ts @@ -26,9 +26,6 @@ export async function executeValidationCommand( 15_000, ); let commandPid: number | undefined; - const cancelTree = () => { - if (commandPid !== undefined) lock.cancelCommand(commandPid); - }; try { lock.recordCommand(run.receiptPath, null); const output = await runCmdStreaming( @@ -38,16 +35,13 @@ export async function executeValidationCommand( cwd, allowFailure: true, captureOutput: false, - detached: process.platform !== 'win32', + detached: true, kill: { signal: 'SIGTERM', graceMs: 5000 }, signal: run.controller.signal, stdin: 'run\n', onSpawn: (child) => { commandPid = child.pid; if (commandPid) lock.recordCommand(run.receiptPath, commandPid); - // Register before host-kit installs its abort handler, which can kill the wrapper. - if (process.platform === 'win32') - run.controller.signal.addEventListener('abort', cancelTree, { once: true }); }, env: { ...process.env, @@ -72,7 +66,6 @@ export async function executeValidationCommand( result.reason = error instanceof Error ? error.message : String(error); throw error; } finally { - run.controller.signal.removeEventListener('abort', cancelTree); clearInterval(heartbeat); fs.closeSync(descriptor); result.durationMs = Date.now() - started; diff --git a/scripts/check-affected/execution.test.ts b/scripts/check-affected/execution.test.ts index abc53a5017..e5c5232877 100644 --- a/scripts/check-affected/execution.test.ts +++ b/scripts/check-affected/execution.test.ts @@ -4,64 +4,76 @@ import path from 'node:path'; import { test } from 'node:test'; import { fixture, run } from './execution.fixtures.ts'; -test('execution JSON contains exact checkout, command, failure exit, and bounded terminal output', () => { - const { cwd, sha } = fixture(); - try { - const result = run( - cwd, - sha, - `return execute([process.execPath, '-e', "console.log('x'.repeat(20000)); process.exit(7)"], ${JSON.stringify(cwd)}, 'fixture');`, - ); - assert.equal(result.exitCode, 7); - assert.equal(result.receipt.status, 'failed'); - assert.equal(result.receipt.exactCommit, true); - assert.equal(result.receipt.initial?.head, sha); - assert.equal(result.receipt.final?.head, sha); - assert.equal(result.receipt.results[0]?.exitCode, 7); - assert.ok(result.stderr.length < 10000); - assert.ok( - result.stderr.split('\n').every((line) => line.length <= 512), - 'failure excerpt lines stay bounded', - ); - assert.ok(fs.readFileSync(result.receipt.results[0]!.logPath!, 'utf8').length > 20000); - } finally { - fs.rmSync(cwd, { recursive: true, force: true }); - } -}); +test( + 'execution JSON contains exact checkout, command, failure exit, and bounded terminal output', + { skip: process.platform === 'win32' }, + () => { + const { cwd, sha } = fixture(); + try { + const result = run( + cwd, + sha, + `return execute([process.execPath, '-e', "console.log('x'.repeat(20000)); process.exit(7)"], ${JSON.stringify(cwd)}, 'fixture');`, + ); + assert.equal(result.exitCode, 7); + assert.equal(result.receipt.status, 'failed'); + assert.equal(result.receipt.exactCommit, true); + assert.equal(result.receipt.initial?.head, sha); + assert.equal(result.receipt.final?.head, sha); + assert.equal(result.receipt.results[0]?.exitCode, 7); + assert.ok(result.stderr.length < 10000); + assert.ok( + result.stderr.split('\n').every((line) => line.length <= 512), + 'failure excerpt lines stay bounded', + ); + assert.ok(fs.readFileSync(result.receipt.results[0]!.logPath!, 'utf8').length > 20000); + } finally { + fs.rmSync(cwd, { recursive: true, force: true }); + } + }, +); -test('dirty local feedback cannot certify a commit; explicit head refuses dirty checkout', () => { - const { cwd, sha } = fixture(); - try { - fs.writeFileSync(path.join(cwd, 'source.txt'), 'dirty'); - const local = run(cwd, 'HEAD', 'return 0;'); - assert.equal(local.exitCode, 0); - assert.equal(local.receipt.exactCommit, false); - const exact = run(cwd, sha, 'throw new Error("must not run");'); - assert.equal(exact.exitCode, 1); - assert.match(exact.receipt.error!, /clean checkout/); - assert.deepEqual(exact.receipt.results, []); - } finally { - fs.rmSync(cwd, { recursive: true, force: true }); - } -}); +test( + 'dirty local feedback cannot certify a commit; explicit head refuses dirty checkout', + { skip: process.platform === 'win32' }, + () => { + const { cwd, sha } = fixture(); + try { + fs.writeFileSync(path.join(cwd, 'source.txt'), 'dirty'); + const local = run(cwd, 'HEAD', 'return 0;'); + assert.equal(local.exitCode, 0); + assert.equal(local.receipt.exactCommit, false); + const exact = run(cwd, sha, 'throw new Error("must not run");'); + assert.equal(exact.exitCode, 1); + assert.match(exact.receipt.error!, /clean checkout/); + assert.deepEqual(exact.receipt.results, []); + } finally { + fs.rmSync(cwd, { recursive: true, force: true }); + } + }, +); -test('mismatched head is rejected and checkout drift invalidates successful checks', () => { - const { cwd, sha, git } = fixture(); - try { - fs.writeFileSync(path.join(cwd, 'source.txt'), 'new commit'); - git('add', '.'); - git('commit', '-qm', 'head'); - const mismatch = run(cwd, sha, 'return 0;'); - assert.equal(mismatch.exitCode, 1); - const drift = run( - cwd, - 'HEAD', - `const fs = await import('node:fs'); fs.writeFileSync('source.txt', 'drift'); return 0;`, - ); - assert.equal(drift.exitCode, 1); - assert.equal(drift.receipt.checkoutChanged, true); - assert.equal(drift.receipt.exactCommit, false); - } finally { - fs.rmSync(cwd, { recursive: true, force: true }); - } -}); +test( + 'mismatched head is rejected and checkout drift invalidates successful checks', + { skip: process.platform === 'win32' }, + () => { + const { cwd, sha, git } = fixture(); + try { + fs.writeFileSync(path.join(cwd, 'source.txt'), 'new commit'); + git('add', '.'); + git('commit', '-qm', 'head'); + const mismatch = run(cwd, sha, 'return 0;'); + assert.equal(mismatch.exitCode, 1); + const drift = run( + cwd, + 'HEAD', + `const fs = await import('node:fs'); fs.writeFileSync('source.txt', 'drift'); return 0;`, + ); + assert.equal(drift.exitCode, 1); + assert.equal(drift.receipt.checkoutChanged, true); + assert.equal(drift.receipt.exactCommit, false); + } finally { + fs.rmSync(cwd, { recursive: true, force: true }); + } + }, +); diff --git a/scripts/check-affected/host-lock.test.ts b/scripts/check-affected/host-lock.test.ts index 0f97b1044b..9c0cf237b0 100644 --- a/scripts/check-affected/host-lock.test.ts +++ b/scripts/check-affected/host-lock.test.ts @@ -1,121 +1,74 @@ import assert from 'node:assert/strict'; -import childProcess from 'node:child_process'; -import { syncBuiltinESMExports } from 'node:module'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { mock, test } from 'node:test'; +import { test } from 'node:test'; import { acquireValidationLock, VALIDATION_PARENT_ENV } from './host-lock.ts'; -test('host lock excludes a waiter, supports cancellation, then admits it after release', async () => { - const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'validation-lock-')); - const lock = path.join(directory, 'gate.lock'); - const parent = process.env[VALIDATION_PARENT_ENV]; - delete process.env[VALIDATION_PARENT_ENV]; - const controller = new AbortController(); - try { - const owner = await acquireValidationLock(controller.signal, lock); - const waiting = acquireValidationLock(controller.signal, lock, (message) => { - assert.match(message, /wait.*host validation/); - controller.abort(); - }); - await assert.rejects(waiting, { name: 'AbortError' }); - assert.ok(fs.existsSync(lock), 'cancelled waiter must not release the owner'); - owner.recordCommand('/fixture/receipt.json', null); - assert.ok(fs.existsSync(`${lock}.active.json`)); - await owner.finishCommand(undefined); - assert.equal(fs.existsSync(`${lock}.active.json`), false); - await owner.release(); - const second = await acquireValidationLock(new AbortController().signal, lock); - await second.release(); - assert.equal(fs.existsSync(lock), false); - // A Windows orphan has a live PID, but no POSIX process group. - fs.writeFileSync( - `${lock}.active.json`, - JSON.stringify({ - pid: process.pid, - startTime: null, - activation: 'stdin', - receipt: '/orphan/receipt.json', - }), - ); - const platform = Object.getOwnPropertyDescriptor(process, 'platform')!; - const kill = process.kill.bind(process); - const probe = mock.method(process, 'kill', (pid: number, signal?: number | NodeJS.Signals) => { - if (pid < 0) throw Object.assign(new Error('no process groups'), { code: 'ESRCH' }); - return kill(pid, signal); - }); +test( + 'host lock excludes a waiter, supports cancellation, then admits it after release', + { skip: process.platform === 'win32' }, + async () => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'validation-lock-')); + const lock = path.join(directory, 'gate.lock'); + const parent = process.env[VALIDATION_PARENT_ENV]; + delete process.env[VALIDATION_PARENT_ENV]; + const controller = new AbortController(); + try { + const owner = await acquireValidationLock(controller.signal, lock); + const waiting = acquireValidationLock(controller.signal, lock, (message) => { + assert.match(message, /wait.*host validation/); + controller.abort(); + }); + await assert.rejects(waiting, { name: 'AbortError' }); + assert.ok(fs.existsSync(lock), 'cancelled waiter must not release the owner'); + owner.recordCommand('/fixture/receipt.json', null); + assert.ok(fs.existsSync(`${lock}.active.json`)); + await owner.finishCommand(undefined); + assert.equal(fs.existsSync(`${lock}.active.json`), false); + await owner.release(); + const second = await acquireValidationLock(new AbortController().signal, lock); + await second.release(); + assert.equal(fs.existsSync(lock), false); + } finally { + if (parent === undefined) delete process.env[VALIDATION_PARENT_ENV]; + else process.env[VALIDATION_PARENT_ENV] = parent; + fs.rmSync(directory, { recursive: true, force: true }); + } + }, +); + +test( + 'nested validation fails before acquisition', + { skip: process.platform === 'win32' }, + async () => { + const parent = process.env[VALIDATION_PARENT_ENV]; + process.env[VALIDATION_PARENT_ENV] = '/parent/receipt.json'; try { - Object.defineProperty(process, 'platform', { value: 'win32' }); - const orphanWaiter = new AbortController(); await assert.rejects( - acquireValidationLock(orphanWaiter.signal, lock, (message) => { - assert.match(message, /orphan validation group/); - orphanWaiter.abort(); - }), - { name: 'AbortError' }, + acquireValidationLock(new AbortController().signal), + /Nested validation.*parent\/receipt/, ); } finally { - Object.defineProperty(process, 'platform', platform); - probe.mock.restore(); + if (parent === undefined) delete process.env[VALIDATION_PARENT_ENV]; + else process.env[VALIDATION_PARENT_ENV] = parent; } - } finally { - if (parent === undefined) delete process.env[VALIDATION_PARENT_ENV]; - else process.env[VALIDATION_PARENT_ENV] = parent; - fs.rmSync(directory, { recursive: true, force: true }); - } -}); - -test('nested validation fails before acquisition', async () => { - const parent = process.env[VALIDATION_PARENT_ENV]; - process.env[VALIDATION_PARENT_ENV] = '/parent/receipt.json'; - try { - await assert.rejects( - acquireValidationLock(new AbortController().signal), - /Nested validation.*parent\/receipt/, - ); - } finally { - if (parent === undefined) delete process.env[VALIDATION_PARENT_ENV]; - else process.env[VALIDATION_PARENT_ENV] = parent; - } -}); + }, +); -test('Windows cancellation confirms the tree or preserves a blocking marker', async () => { - const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'validation-windows-')); +test('unsupported hosts reject validation before writing lock state', async () => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'validation-unsupported-')); const lock = path.join(directory, 'gate.lock'); - const parent = process.env[VALIDATION_PARENT_ENV]; - delete process.env[VALIDATION_PARENT_ENV]; - const owner = await acquireValidationLock(new AbortController().signal, lock); - owner.recordCommand('/fixture/receipt.json', process.pid); const platform = Object.getOwnPropertyDescriptor(process, 'platform')!; - let exitCode = 1; - const spawn = mock.method(childProcess, 'spawnSync', (command: string, args: string[]) => { - assert.equal(command, 'taskkill.exe'); - assert.deepEqual(args, ['/PID', String(process.pid), '/T', '/F']); - return { status: exitCode, stdout: '', stderr: '' }; - }); - syncBuiltinESMExports(); try { Object.defineProperty(process, 'platform', { value: 'win32' }); - owner.cancelCommand(process.pid); - await assert.rejects(owner.finishCommand(undefined), /Cannot verify.*termination/); - await owner.release(); await assert.rejects( acquireValidationLock(new AbortController().signal, lock), - /Cannot verify.*termination/, + /requires macOS or Linux/, ); - assert.ok(fs.existsSync(`${lock}.active.json`)); - exitCode = 0; - owner.cancelCommand(process.pid); - await owner.finishCommand(undefined); - assert.equal(fs.existsSync(`${lock}.active.json`), false); + assert.deepEqual(fs.readdirSync(directory), []); } finally { Object.defineProperty(process, 'platform', platform); - spawn.mock.restore(); - syncBuiltinESMExports(); - await owner.release(); - if (parent === undefined) delete process.env[VALIDATION_PARENT_ENV]; - else process.env[VALIDATION_PARENT_ENV] = parent; fs.rmSync(directory, { recursive: true, force: true }); } }); diff --git a/scripts/check-affected/host-lock.ts b/scripts/check-affected/host-lock.ts index 3c035b5a09..9163841d5b 100644 --- a/scripts/check-affected/host-lock.ts +++ b/scripts/check-affected/host-lock.ts @@ -1,7 +1,6 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { runCmdSync } from '@agent-device/host-kit/command'; import { setTimeout } from 'node:timers/promises'; import { publishFileSync, @@ -11,7 +10,6 @@ import { } from '@agent-device/host-kit/file'; import { isProcessGroupAlive, - isProcessAlive, readCurrentOwnerIdentity, readProcessStartTime, ownerIdentityDiffers, @@ -25,7 +23,6 @@ export type ValidationLock = { release: ProcessLockRelease; recordCommand: (receipt: string, pid: number | null) => void; finishCommand: (pid: number | undefined) => Promise; - cancelCommand: (pid: number) => void; }; export async function acquireValidationLock( @@ -33,6 +30,11 @@ export async function acquireValidationLock( lockDirPath = HOST_LOCK_PATH, report: (message: string) => void = (message) => process.stderr.write(`${message}\n`), ): Promise { + if (process.platform !== 'darwin' && process.platform !== 'linux') { + throw new Error( + 'Serialized validation requires macOS or Linux. Use plan mode or focused checks on this host.', + ); + } if (process.env[VALIDATION_PARENT_ENV]) { throw new Error( `Nested validation is not allowed. Parent receipt: ${process.env[VALIDATION_PARENT_ENV]}`, @@ -54,7 +56,6 @@ export async function acquireValidationLock( release: attempt.acquisition.release, recordCommand: (receipt, pid) => recordValidationCommand(lockDirPath, receipt, pid), finishCommand: (pid) => finishValidationCommand(lockDirPath, pid), - cancelCommand: (pid) => cancelValidationCommand(lockDirPath, pid), }; } if (Date.now() >= deadline) @@ -81,30 +82,6 @@ function recordValidationCommand(lockDirPath: string, receipt: string, pid: numb }); } -// Mark uncertainty before taskkill: a partial tree termination must never permit another gate. -function cancelValidationCommand(lockDirPath: string, pid: number): void { - if (process.platform !== 'win32') return; - const destination = `${lockDirPath}.active.json`; - const active = JSON.parse(fs.readFileSync(destination, 'utf8')); - publishFileSync({ - destination, - contents: JSON.stringify({ ...active, terminationUnverified: true }), - }); - try { - const result = runCmdSync('taskkill.exe', ['/PID', String(pid), '/T', '/F'], { - allowFailure: true, - timeoutMs: 5000, - }); - if (result.exitCode === 0) - publishFileSync({ - destination, - contents: JSON.stringify({ ...active, terminationUnverified: false }), - }); - } catch { - // Preserve the blocking marker when Windows cannot confirm tree termination. - } -} - async function finishValidationCommand( lockDirPath: string, pid: number | undefined, @@ -117,15 +94,12 @@ async function finishValidationCommand( async function waitForCommandExit(lockDirPath: string, pid: number): Promise { const deadline = Date.now() + 10_000; - while (process.platform === 'win32' ? isProcessAlive(pid) : isProcessGroupAlive(pid)) { + while (isProcessGroupAlive(pid)) { if (Date.now() >= deadline) throw new Error( `Validation command group ${pid} has not exited; host validation remains blocked.`, ); - if (Date.now() >= deadline - 5000) { - if (process.platform === 'win32') cancelValidationCommand(lockDirPath, pid); - else signalProcessGroupBestEffort(pid, 'SIGKILL'); - } + if (Date.now() >= deadline - 5000) signalProcessGroupBestEffort(pid, 'SIGKILL'); await setTimeout(100); } } @@ -146,11 +120,7 @@ function readOrphanCommand(lockDirPath: string): { pid: number; receipt: string if (active.pid === null) return null; const observed = { pid: active.pid, startTime: readProcessStartTime(active.pid) }; if (ownerIdentityDiffers({ pid: active.pid, startTime: active.startTime }, observed)) return null; - return ( - process.platform === 'win32' ? isProcessAlive(active.pid) : isProcessGroupAlive(active.pid) - ) - ? { pid: active.pid, receipt: active.receipt } - : null; + return isProcessGroupAlive(active.pid) ? { pid: active.pid, receipt: active.receipt } : null; } function readActiveCommand( @@ -168,13 +138,7 @@ function readActiveCommand( startTime: string | null; receipt: string; activation: string; - terminationUnverified?: boolean; }; - if (active.terminationUnverified) { - throw new Error( - `Cannot verify validation command tree termination: ${activePath}; receipt=${active.receipt}`, - ); - } if ( active.activation !== 'stdin' || (active.pid !== null && (!Number.isInteger(active.pid) || active.pid <= 0)) diff --git a/scripts/check-affected/run-cli.test.ts b/scripts/check-affected/run-cli.test.ts index 5ffeee9dc1..b142a190d5 100644 --- a/scripts/check-affected/run-cli.test.ts +++ b/scripts/check-affected/run-cli.test.ts @@ -54,34 +54,42 @@ function invoke(cwd: string, args: string[]) { return { ...result, receipt: JSON.parse(result.stdout) as ExecutionReceipt }; } -test('CLI JSON contains execution plan and preserves the failing command exit code', () => { - const { cwd, base } = fixture(); - try { - const result = invoke(cwd, ['--base', base]); - assert.equal(result.exitCode, 7, result.stderr); - assert.equal(result.receipt.status, 'failed'); - assert.equal(result.receipt.exactCommit, true); - assert.deepEqual(result.receipt.changedFiles, ['AGENTS.md']); - assert.deepEqual(result.receipt.plan?.checks, ['agent-guidance']); - assert.equal(result.receipt.results[0]?.exitCode, 7); - assert.equal(result.receipt.results[0]?.status, 'failed'); - } finally { - fs.rmSync(cwd, { recursive: true, force: true }); - } -}); +test( + 'CLI JSON contains execution plan and preserves the failing command exit code', + { skip: process.platform === 'win32' }, + () => { + const { cwd, base } = fixture(); + try { + const result = invoke(cwd, ['--base', base]); + assert.equal(result.exitCode, 7, result.stderr); + assert.equal(result.receipt.status, 'failed'); + assert.equal(result.receipt.exactCommit, true); + assert.deepEqual(result.receipt.changedFiles, ['AGENTS.md']); + assert.deepEqual(result.receipt.plan?.checks, ['agent-guidance']); + assert.equal(result.receipt.results[0]?.exitCode, 7); + assert.equal(result.receipt.results[0]?.status, 'failed'); + } finally { + fs.rmSync(cwd, { recursive: true, force: true }); + } + }, +); -test('full CLI works without origin/main and still blocks a missing install', () => { - const { cwd } = fixture(); - try { - const full = invoke(cwd, ['--full']); - assert.equal(full.exitCode, 0, full.stderr); - assert.equal(full.receipt.results[0]?.id, 'check:tooling'); - fs.writeFileSync(path.join(cwd, 'pnpm-lock.yaml'), 'lockfileVersion: 9'); - const stale = invoke(cwd, ['--full']); - assert.equal(stale.exitCode, 1); - assert.match(stale.stderr, /node_modules does not match/); - assert.deepEqual(stale.receipt.results, []); - } finally { - fs.rmSync(cwd, { recursive: true, force: true }); - } -}); +test( + 'full CLI works without origin/main and still blocks a missing install', + { skip: process.platform === 'win32' }, + () => { + const { cwd } = fixture(); + try { + const full = invoke(cwd, ['--full']); + assert.equal(full.exitCode, 0, full.stderr); + assert.equal(full.receipt.results[0]?.id, 'check:tooling'); + fs.writeFileSync(path.join(cwd, 'pnpm-lock.yaml'), 'lockfileVersion: 9'); + const stale = invoke(cwd, ['--full']); + assert.equal(stale.exitCode, 1); + assert.match(stale.stderr, /node_modules does not match/); + assert.deepEqual(stale.receipt.results, []); + } finally { + fs.rmSync(cwd, { recursive: true, force: true }); + } + }, +); diff --git a/scripts/check-affected/test-report.test.ts b/scripts/check-affected/test-report.test.ts index 74c3b4101b..8c8444fe37 100644 --- a/scripts/check-affected/test-report.test.ts +++ b/scripts/check-affected/test-report.test.ts @@ -6,57 +6,61 @@ import { test } from 'node:test'; import { fixture, run } from './execution.fixtures.ts'; import type { TestReport } from './test-report.ts'; -test('real Vitest report preserves assertion, suite, and unhandled failures separately', () => { - const { cwd: root } = fixture(); - const reporter = pathToFileURL(path.join(import.meta.dirname, 'test-report.ts')).href; - try { - fs.writeFileSync( - path.join(root, 'vitest.config.ts'), - ` +test( + 'real Vitest report preserves assertion, suite, and unhandled failures separately', + { skip: process.platform === 'win32' }, + () => { + const { cwd: root } = fixture(); + const reporter = pathToFileURL(path.join(import.meta.dirname, 'test-report.ts')).href; + try { + fs.writeFileSync( + path.join(root, 'vitest.config.ts'), + ` import { validationTestReporter } from ${JSON.stringify(reporter)}; export default { test: { root: ${JSON.stringify(root)}, maxWorkers: 1, reporters: ['dot', validationTestReporter(process.env.AGENT_DEVICE_VALIDATION_REPORT + '.' + process.pid + '.json')] } }; `, - ); - fs.writeFileSync( - path.join(root, 'cases.test.js'), - ` + ); + fs.writeFileSync( + path.join(root, 'cases.test.js'), + ` import { test, expect, describe, beforeAll } from 'vitest'; - test('passes', () => expect(1).toBe(1)); - test('assertion', () => expect(1).toBe(2)); - test('unhandled', async () => { Promise.reject(new Error('fixture-unhandled')); await new Promise(r => setTimeout(r, 10)); }); - describe('suite', () => { beforeAll(() => { throw new Error('fixture-suite'); }); test('not reached', () => {}); }); + test('passes', { skip: process.platform === 'win32' }, () => expect(1).toBe(1)); + test('assertion', { skip: process.platform === 'win32' }, () => expect(1).toBe(2)); + test('unhandled', { skip: process.platform === 'win32' }, async () => { Promise.reject(new Error('fixture-unhandled')); await new Promise(r => setTimeout(r, 10)); }); + describe('suite', () => { beforeAll(() => { throw new Error('fixture-suite'); }); test('not reached', { skip: process.platform === 'win32' }, () => {}); }); `, - ); - const result = run( - root, - 'HEAD', - `return execute(['pnpm', 'exec', 'vitest', 'run', '--config', ${JSON.stringify(path.join(root, 'vitest.config.ts'))}], ${JSON.stringify(path.resolve(import.meta.dirname, '../..'))}, 'report');`, - ); - assert.equal(result.exitCode, 1); - const check = result.receipt.results[0]!; - assert.equal(check.testReportPaths?.length, 1); - const report = check.tests?.[0] as TestReport; - assert.equal(report.passed, 2); - assert.ok(report.failed >= 1); - assert.equal(report.unhandledErrors, 1); - assert.ok( - report.failures.some( - (failure) => failure.kind === 'test' && failure.test?.includes('assertion'), - ), - ); - assert.ok( - report.failures.some( - (failure) => failure.kind === 'suite' && failure.errors.join().includes('fixture-suite'), - ), - ); - assert.ok( - report.failures.some( - (failure) => - failure.kind === 'unhandled' && failure.errors.join().includes('fixture-unhandled'), - ), - ); - } finally { - fs.rmSync(root, { recursive: true, force: true }); - } -}); + ); + const result = run( + root, + 'HEAD', + `return execute(['pnpm', 'exec', 'vitest', 'run', '--config', ${JSON.stringify(path.join(root, 'vitest.config.ts'))}], ${JSON.stringify(path.resolve(import.meta.dirname, '../..'))}, 'report');`, + ); + assert.equal(result.exitCode, 1); + const check = result.receipt.results[0]!; + assert.equal(check.testReportPaths?.length, 1); + const report = check.tests?.[0] as TestReport; + assert.equal(report.passed, 2); + assert.ok(report.failed >= 1); + assert.equal(report.unhandledErrors, 1); + assert.ok( + report.failures.some( + (failure) => failure.kind === 'test' && failure.test?.includes('assertion'), + ), + ); + assert.ok( + report.failures.some( + (failure) => failure.kind === 'suite' && failure.errors.join().includes('fixture-suite'), + ), + ); + assert.ok( + report.failures.some( + (failure) => + failure.kind === 'unhandled' && failure.errors.join().includes('fixture-unhandled'), + ), + ); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }, +);