From f2ee0acef66e03d4e721f8389ed07614cac13454 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 22 Oct 2026 08:32:23 +0200 Subject: [PATCH 1/3] fix(gate): serialize heavy gates behind the host validation lock Co-Authored-By: Claude Sonnet 5.5 --- AGENTS.md | 4 +- docs/agents/pull-requests.md | 2 +- docs/agents/testing.md | 6 +- scripts/check-affected/checks.ts | 28 +++-- scripts/gate/host-locked.test.ts | 177 +++++++++++++++++++++++++++++++ scripts/gate/host-locked.ts | 89 ++++++++++++++++ scripts/gate/run.ts | 11 +- 7 files changed, 295 insertions(+), 22 deletions(-) create mode 100644 scripts/gate/host-locked.test.ts create mode 100644 scripts/gate/host-locked.ts diff --git a/AGENTS.md b/AGENTS.md index c8749ee632..ca5fb030e9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -48,11 +48,11 @@ The versioned CLI help owns command semantics. To drive an app, start with the s - In a new worktree, run `pnpm install --frozen-lockfile && pnpm build` before you test. - Do not change other worktrees, devices, sessions, dirty files, or experiments. Do not use `git stash`; all worktrees share it. - Do not copy `.env` into a worktree. For host credentials, source the main checkout's file by its absolute path. -- Give each parallel worker different files and a different device. `pnpm check:affected --run` and `pnpm check` run one at a time automatically. +- Give each parallel worker different files and a different device. `pnpm check:affected --run`, `pnpm check`, and heavy `pnpm gate` ids run one at a time automatically. - The layering scan reads only tracked files. Stage new modules before you trust its result. - When you rename a file, move its Fallow baseline entry. Do not regenerate unrelated baselines. -The validation lock automatically reclaims provably dead owners. An unknown owner continues to block. +The validation lock reclaims provably dead owners. An unknown owner blocks. If the wait times out, report the printed owner and the lock state. Do not remove the lock manually. ## Agent docs diff --git a/docs/agents/pull-requests.md b/docs/agents/pull-requests.md index 7111e4568c..a4ac925d1c 100644 --- a/docs/agents/pull-requests.md +++ b/docs/agents/pull-requests.md @@ -14,7 +14,7 @@ Apply authorization already given. Merging and releasing need separate authoriza Follow [validation](testing.md). Record the `check:affected --run --head` receipt in the PR body. A serialized gate stage can validate the pushed head instead. For unresolved failures, follow the blocked-validation table. -An authorized incomplete draft can record a blocker. A blocker is not a pass. +A blocker is not a pass. Push once per review round. Each push restarts CI. Drafts skip device lanes. For device-facing changes, follow [device verification](device-verification.md). diff --git a/docs/agents/testing.md b/docs/agents/testing.md index 9bad09365a..49887b1ab4 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -10,8 +10,10 @@ Serialized validation requires macOS or Linux. On Windows, use plan mode or focused checks. Both broad commands acquire the same host lock. +So do `pnpm gate` ids `unit`, `unit-ci`, `provider-integration`, `fuzz-parsers`, `mutation`, `mutation-affected`, and `concurrency-torture`. +A heavy gate in a locked validation inherits the lock. GitHub runners skip it. Wait for the owner to finish, or cancel your waiter with Ctrl-C. -Do not add lock wrappers. Do not delete lock files. +Do not 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. @@ -34,7 +36,6 @@ Execution stops at the first failure. Unexecuted checks remain `not-run`. Only a passed receipt with `exactCommit: true` proves validation at the named commit. 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. During validation, stop all edits. ## Interpret selection @@ -63,7 +64,6 @@ Do not add tests that assert prose. ## A gate failed Keep failure attribution separate from validation status. -Never call a failing gate passed. | Evidence | Next action | Outcome | | --- | --- | --- | diff --git a/scripts/check-affected/checks.ts b/scripts/check-affected/checks.ts index 60b87b1ecc..d783ec7210 100644 --- a/scripts/check-affected/checks.ts +++ b/scripts/check-affected/checks.ts @@ -24,10 +24,17 @@ export type CheckSpec = { // torture), and the instrumented coverage run stay authoritative on // GitHub CI. readonly localRunnable: boolean; + // Whether `pnpm gate` takes the host validation lock before it runs the check. Set it for a check that + // runs a whole-repo suite or spawns many workers. + readonly hostLocked: boolean; }; function gate(id: CheckId, label: string, script: string, localRunnable = true): CheckSpec { - return { id, label, kind: { type: 'script', script }, localRunnable }; + return { id, label, kind: { type: 'script', script }, localRunnable, hostLocked: false }; +} + +function heavyGate(id: CheckId, label: string, script: string, localRunnable = true): CheckSpec { + return { ...gate(id, label, script, localRunnable), hostLocked: true }; } export const CHECK_CATALOG: readonly CheckSpec[] = [ @@ -62,12 +69,17 @@ export const CHECK_CATALOG: readonly CheckSpec[] = [ label: 'Tests related by Vitest module graph', kind: { type: 'vitest-related' }, localRunnable: true, + hostLocked: false, }, - gate('unit', 'Unit + smoke suite', 'check:unit'), + heavyGate('unit', 'Unit + smoke suite', 'check:unit'), // CI owns the default coverage run; the scripts remain available for explicit local diagnosis. gate('coverage', 'Changed-line coverage', 'check:coverage-changed', false), gate('coverage-crap', 'Coverage-based CRAP verdict', 'check:coverage-crap', false), - gate('provider-integration', 'Provider-backed integration suite', 'test:integration:provider'), + heavyGate( + 'provider-integration', + 'Provider-backed integration suite', + 'test:integration:provider', + ), gate( 'integration-progress', 'Integration architecture-progress gate', @@ -97,7 +109,7 @@ export const CHECK_CATALOG: readonly CheckSpec[] = [ // // `unit-ci` is the CI form of the unit suite under coverage. Local affected checks use Vitest's // related graph without instrumentation; CI owns the full coverage run and changed-line verdict. - gate('unit-ci', 'CI unit suite under coverage', 'test:coverage:ci', false), + heavyGate('unit-ci', 'CI unit suite under coverage', 'test:coverage:ci', false), gate('affected-selector', 'Affected-check selector model', 'check:affected:test'), gate('gate-manifest', 'Gate manifest — every gate owned and wired', 'check:gate-manifest'), gate('gate-manifest-model', 'Gate manifest model', 'check:gate-manifest:test'), @@ -153,12 +165,12 @@ export const CHECK_CATALOG: readonly CheckSpec[] = [ 'maestro:conformance:regenerate', false, ), - gate('fuzz-parsers', 'Parser fuzz invariants', 'fuzz:parsers', false), - gate('mutation', 'Mutation sweep', 'mutation:run', false), - gate('mutation-affected', 'Affected mutation shard selection', 'mutation:affected', false), + heavyGate('fuzz-parsers', 'Parser fuzz invariants', 'fuzz:parsers', false), + heavyGate('mutation', 'Mutation sweep', 'mutation:run', false), + heavyGate('mutation-affected', 'Affected mutation shard selection', 'mutation:affected', false), gate('mutation-check', 'Mutation score for an existing report', 'mutation:check', false), gate('mutation-model', 'Mutation harness self-test', 'mutation:test'), - gate( + heavyGate( 'concurrency-torture', 'Session/lease/lock torture sweep', 'test:concurrency-torture', diff --git a/scripts/gate/host-locked.test.ts b/scripts/gate/host-locked.test.ts new file mode 100644 index 0000000000..90f370f322 --- /dev/null +++ b/scripts/gate/host-locked.test.ts @@ -0,0 +1,177 @@ +// `pnpm gate` serializes heavy gates behind the host validation lock and leaves cheap ones alone. + +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 { CHECK_CATALOG } from '../check-affected/checks.ts'; +import { acquireValidationLock, VALIDATION_PARENT_ENV } from '../check-affected/host-lock.ts'; +import { executeGate, requiresHostLock } from './host-locked.ts'; + +const skip = process.platform === 'win32'; +const cleanEnv = (extra: NodeJS.ProcessEnv = {}): NodeJS.ProcessEnv => { + const env = { ...process.env, ...extra }; + delete env[VALIDATION_PARENT_ENV]; + delete env.GITHUB_ACTIONS; + return { ...env, ...extra }; +}; + +function fixture() { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'gate-host-lock-')); + const lockDirPath = path.join(directory, 'gate.lock'); + const marker = path.join(directory, 'marker'); + const writeMarker = [ + process.execPath, + '-e', + `require('node:fs').writeFileSync(${JSON.stringify(marker)}, process.env.${VALIDATION_PARENT_ENV} ?? '')`, + ]; + return { + lockDirPath, + marker, + writeMarker, + cleanup: () => fs.rmSync(directory, { recursive: true, force: true }), + }; +} + +test('the locked gates are the whole-repo suites and worker-spawning sweeps', () => { + assert.deepEqual( + CHECK_CATALOG.filter((spec) => spec.hostLocked).map((spec) => spec.id), + [ + 'unit', + 'provider-integration', + 'unit-ci', + 'fuzz-parsers', + 'mutation', + 'mutation-affected', + 'concurrency-torture', + ], + ); +}); + +test( + 'unit-ci waits for the host lock owner, then runs under it and releases it', + { skip }, + async () => { + const { lockDirPath, marker, writeMarker, cleanup } = fixture(); + try { + const owner = await acquireValidationLock( + new AbortController().signal, + lockDirPath, + () => {}, + ); + let waitReport = ''; + const waiting = executeGate({ + id: 'unit-ci', + command: writeMarker, + cwd: process.cwd(), + env: cleanEnv(), + lockDirPath, + report: (message) => { + waitReport = message; + }, + }); + await new Promise((resolve) => { + const poll = setInterval(() => { + if (waitReport) { + clearInterval(poll); + resolve(); + } + }, 20); + }); + assert.match(waitReport, /\[wait\] host validation/); + assert.equal( + fs.existsSync(marker), + false, + 'gate must not run while another owner holds the lock', + ); + await owner.release(); + assert.equal(await waiting, 0); + assert.equal(fs.readFileSync(marker, 'utf8'), 'gate:unit-ci'); + assert.equal(fs.existsSync(lockDirPath), false, 'gate must release the lock'); + assert.equal(fs.existsSync(`${lockDirPath}.active.json`), false); + } finally { + cleanup(); + } + }, +); + +test( + 'a locked gate returns the command exit code and still releases the lock', + { skip }, + async () => { + const { lockDirPath, cleanup } = fixture(); + try { + const exitCode = await executeGate({ + id: 'unit-ci', + command: [process.execPath, '-e', 'process.exit(7)'], + cwd: process.cwd(), + env: cleanEnv(), + lockDirPath, + }); + assert.equal(exitCode, 7); + assert.equal(fs.existsSync(lockDirPath), false); + } finally { + cleanup(); + } + }, +); + +test( + 'a gate inside a locked validation inherits the owner instead of deadlocking', + { skip }, + async () => { + const { lockDirPath, marker, writeMarker, cleanup } = fixture(); + try { + const owner = await acquireValidationLock( + new AbortController().signal, + lockDirPath, + () => {}, + ); + const env = cleanEnv({ [VALIDATION_PARENT_ENV]: '/parent/receipt.json' }); + assert.equal(requiresHostLock('unit-ci', env), false); + const exitCode = await executeGate({ + id: 'unit-ci', + command: writeMarker, + cwd: process.cwd(), + env, + lockDirPath, + report: () => assert.fail('nested gate must not wait'), + }); + assert.equal(exitCode, 0); + assert.equal(fs.readFileSync(marker, 'utf8'), '/parent/receipt.json'); + assert.ok(fs.existsSync(lockDirPath), 'nested gate must not release the parent lock'); + await owner.release(); + } finally { + cleanup(); + } + }, +); + +test('cheap gates and GitHub runners do not wait for the lock', { skip }, async () => { + const { lockDirPath, marker, writeMarker, cleanup } = fixture(); + try { + const owner = await acquireValidationLock(new AbortController().signal, lockDirPath, () => {}); + for (const [id, env] of [ + ['lint', cleanEnv()], + ['unit-ci', cleanEnv({ GITHUB_ACTIONS: 'true' })], + ] as const) { + fs.rmSync(marker, { force: true }); + assert.equal(requiresHostLock(id, env), false, id); + const exitCode = await executeGate({ + id, + command: writeMarker, + cwd: process.cwd(), + env, + lockDirPath, + report: () => assert.fail(`${id} must not wait`), + }); + assert.equal(exitCode, 0); + assert.equal(fs.readFileSync(marker, 'utf8'), ''); + } + assert.ok(fs.existsSync(lockDirPath), 'unlocked gates leave the owner in place'); + await owner.release(); + } finally { + cleanup(); + } +}); diff --git a/scripts/gate/host-locked.ts b/scripts/gate/host-locked.ts new file mode 100644 index 0000000000..dcca4fc478 --- /dev/null +++ b/scripts/gate/host-locked.ts @@ -0,0 +1,89 @@ +// Heavy gates take the host validation lock that `pnpm check:affected --run` and `pnpm check` hold, +// so concurrent worktrees queue instead of oversubscribing the host. +// +// A gate that already runs under that lock sees the inherited owner receipt and runs directly. +// GitHub runners are single-tenant and run the gate directly. + +import path from 'node:path'; +import { runCmdStreaming } from '@agent-device/host-kit/command'; +import { acquireValidationLock, VALIDATION_PARENT_ENV } from '../check-affected/host-lock.ts'; +import { getCheckSpec } from '../check-affected/checks.ts'; + +const COMMAND_RUNNER = path.join(import.meta.dirname, '..', 'check-affected', 'command.ts'); +const SIGNAL_EXIT_CODES = { SIGHUP: 129, SIGINT: 130, SIGTERM: 143 } as const; + +export type GateExecution = { + id: string; + command: readonly string[]; + cwd: string; + env: NodeJS.ProcessEnv; + lockDirPath?: string; + report?: (message: string) => void; +}; + +export function requiresHostLock(id: string, env: NodeJS.ProcessEnv): boolean { + return ( + getCheckSpec(id).hostLocked && env.GITHUB_ACTIONS !== 'true' && !env[VALIDATION_PARENT_ENV] + ); +} + +export async function executeGate(execution: GateExecution): Promise { + const { id, command, cwd, env } = execution; + const [executable, ...args] = command as [string, ...string[]]; + const stream = { + allowFailure: true, + onStdoutChunk: (chunk: string) => void process.stdout.write(chunk), + onStderrChunk: (chunk: string) => void process.stderr.write(chunk), + }; + if (!requiresHostLock(id, env)) { + return (await runCmdStreaming(executable, args, { cwd, env, ...stream })).exitCode; + } + + const controller = new AbortController(); + let cancelled = 0; + const cancel = (signal: keyof typeof SIGNAL_EXIT_CODES) => () => { + cancelled = SIGNAL_EXIT_CODES[signal]; + controller.abort(); + }; + const handlers = (Object.keys(SIGNAL_EXIT_CODES) as (keyof typeof SIGNAL_EXIT_CODES)[]).map( + (signal) => [signal, cancel(signal)] as const, + ); + for (const [signal, handler] of handlers) process.on(signal, handler); + + const receipt = `gate:${id}`; + let lock: Awaited> | undefined; + let commandPid: number | undefined; + try { + lock = await acquireValidationLock(controller.signal, execution.lockDirPath, execution.report); + lock.recordCommand(receipt, null); + const result = await runCmdStreaming( + process.execPath, + ['--experimental-strip-types', COMMAND_RUNNER, executable, ...args], + { + ...stream, + cwd, + env: { ...env, [VALIDATION_PARENT_ENV]: receipt }, + captureOutput: false, + detached: true, + kill: { signal: 'SIGTERM', graceMs: 5000 }, + signal: controller.signal, + stdin: 'run\n', + onSpawn: (child) => { + commandPid = child.pid; + if (commandPid) lock?.recordCommand(receipt, commandPid); + }, + }, + ); + return cancelled || result.exitCode; + } catch (error) { + if (cancelled) return cancelled; + throw error; + } finally { + try { + await lock?.finishCommand(commandPid); + } finally { + await lock?.release(); + for (const [signal, handler] of handlers) process.off(signal, handler); + } + } +} diff --git a/scripts/gate/run.ts b/scripts/gate/run.ts index b7122b4418..4a773ce263 100644 --- a/scripts/gate/run.ts +++ b/scripts/gate/run.ts @@ -10,9 +10,10 @@ import { pathToFileURL } from 'node:url'; import fs from 'node:fs'; import path from 'node:path'; -import { runCmdStreaming, runCmdSync } from '@agent-device/host-kit/command'; +import { runCmdSync } from '@agent-device/host-kit/command'; import { runEntrypoint } from '../lib/cli-entrypoint.ts'; import { CHECK_CATALOG, resolveCommand } from '../check-affected/checks.ts'; +import { executeGate } from './host-locked.ts'; const USAGE = 'Usage: pnpm gate [args…]\n'; @@ -49,13 +50,7 @@ async function main(argv: readonly string[]): Promise { }; const command = resolveGate(id, pkg.scripts, rest); process.stdout.write(`[gate] ${id}: ${command.join(' ')}\n`); - const result = await runCmdStreaming(command[0] as string, command.slice(1), { - cwd: repoRoot, - allowFailure: true, - onStdoutChunk: (chunk) => void process.stdout.write(chunk), - onStderrChunk: (chunk) => void process.stderr.write(chunk), - }); - return result.exitCode; + return executeGate({ id, command, cwd: repoRoot, env: process.env }); } if (import.meta.url === pathToFileURL(process.argv[1] ?? '').href) { From cdc78ec9c374620dcc589ea5cc9041a7701b575a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 22 Oct 2026 08:44:50 +0200 Subject: [PATCH 2/3] fix(gate): scope lock decisions to the gate environment Pass the environment to acquireValidationLock so nested-owner detection and the gate runner read the same variables, skip the lock on unsupported hosts, cover signal cancellation, and tighten the lock documentation. Co-Authored-By: Claude Sonnet 5.5 --- docs/agents/testing.md | 8 ++--- scripts/check-affected/host-lock.ts | 5 +-- scripts/gate/host-locked.test.ts | 51 +++++++++++++++++++++++++---- scripts/gate/host-locked.ts | 12 +++++-- 4 files changed, 61 insertions(+), 15 deletions(-) diff --git a/docs/agents/testing.md b/docs/agents/testing.md index 49887b1ab4..d6052eef06 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -10,11 +10,10 @@ Serialized validation requires macOS or Linux. On Windows, use plan mode or focused checks. Both broad commands acquire the same host lock. -So do `pnpm gate` ids `unit`, `unit-ci`, `provider-integration`, `fuzz-parsers`, `mutation`, `mutation-affected`, and `concurrency-torture`. -A heavy gate in a locked validation inherits the lock. GitHub runners skip it. +These `pnpm gate` ids acquire the same lock: `unit`, `unit-ci`, `provider-integration`, `fuzz-parsers`, `mutation`, `mutation-affected`, and `concurrency-torture`. +A gate inside a locked validation reuses that lock. A GitHub runner takes no lock. Wait for the owner to finish, or cancel your waiter with Ctrl-C. -Do not delete lock files. -A wait ends after two hours. Nested validation fails immediately. +A wait ends after two hours. A nested `pnpm check` or `pnpm check:affected --run` 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 contains: @@ -46,7 +45,6 @@ A full selection for these changes is intentional. 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`. GitHub owns: diff --git a/scripts/check-affected/host-lock.ts b/scripts/check-affected/host-lock.ts index 9163841d5b..5ca8da9f1e 100644 --- a/scripts/check-affected/host-lock.ts +++ b/scripts/check-affected/host-lock.ts @@ -29,15 +29,16 @@ export async function acquireValidationLock( signal: AbortSignal, lockDirPath = HOST_LOCK_PATH, report: (message: string) => void = (message) => process.stderr.write(`${message}\n`), + env: NodeJS.ProcessEnv = process.env, ): 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]) { + if (env[VALIDATION_PARENT_ENV]) { throw new Error( - `Nested validation is not allowed. Parent receipt: ${process.env[VALIDATION_PARENT_ENV]}`, + `Nested validation is not allowed. Parent receipt: ${env[VALIDATION_PARENT_ENV]}`, ); } const owner = { ...readCurrentOwnerIdentity(), acquiredAtMs: Date.now() }; diff --git a/scripts/gate/host-locked.test.ts b/scripts/gate/host-locked.test.ts index 90f370f322..489da8767c 100644 --- a/scripts/gate/host-locked.test.ts +++ b/scripts/gate/host-locked.test.ts @@ -36,15 +36,17 @@ function fixture() { test('the locked gates are the whole-repo suites and worker-spawning sweeps', () => { assert.deepEqual( - CHECK_CATALOG.filter((spec) => spec.hostLocked).map((spec) => spec.id), + CHECK_CATALOG.filter((spec) => spec.hostLocked) + .map((spec) => spec.id) + .sort(), [ - 'unit', - 'provider-integration', - 'unit-ci', + 'concurrency-torture', 'fuzz-parsers', 'mutation', 'mutation-affected', - 'concurrency-torture', + 'provider-integration', + 'unit', + 'unit-ci', ], ); }); @@ -59,6 +61,7 @@ test( new AbortController().signal, lockDirPath, () => {}, + cleanEnv(), ); let waitReport = ''; const waiting = executeGate({ @@ -127,6 +130,7 @@ test( new AbortController().signal, lockDirPath, () => {}, + cleanEnv(), ); const env = cleanEnv({ [VALIDATION_PARENT_ENV]: '/parent/receipt.json' }); assert.equal(requiresHostLock('unit-ci', env), false); @@ -151,7 +155,12 @@ test( test('cheap gates and GitHub runners do not wait for the lock', { skip }, async () => { const { lockDirPath, marker, writeMarker, cleanup } = fixture(); try { - const owner = await acquireValidationLock(new AbortController().signal, lockDirPath, () => {}); + const owner = await acquireValidationLock( + new AbortController().signal, + lockDirPath, + () => {}, + cleanEnv(), + ); for (const [id, env] of [ ['lint', cleanEnv()], ['unit-ci', cleanEnv({ GITHUB_ACTIONS: 'true' })], @@ -175,3 +184,33 @@ test('cheap gates and GitHub runners do not wait for the lock', { skip }, async cleanup(); } }); + +test( + 'a signal while waiting cancels the gate and leaves the owner in place', + { skip }, + async () => { + const { lockDirPath, marker, writeMarker, cleanup } = fixture(); + try { + const owner = await acquireValidationLock( + new AbortController().signal, + lockDirPath, + () => {}, + cleanEnv(), + ); + const waiting = executeGate({ + id: 'unit-ci', + command: writeMarker, + cwd: process.cwd(), + env: cleanEnv(), + lockDirPath, + report: () => void process.emit('SIGTERM'), + }); + assert.equal(await waiting, 143); + assert.equal(fs.existsSync(marker), false); + assert.ok(fs.existsSync(lockDirPath), 'cancelled waiter must not release the owner'); + await owner.release(); + } finally { + cleanup(); + } + }, +); diff --git a/scripts/gate/host-locked.ts b/scripts/gate/host-locked.ts index dcca4fc478..777eef16ef 100644 --- a/scripts/gate/host-locked.ts +++ b/scripts/gate/host-locked.ts @@ -23,7 +23,10 @@ export type GateExecution = { export function requiresHostLock(id: string, env: NodeJS.ProcessEnv): boolean { return ( - getCheckSpec(id).hostLocked && env.GITHUB_ACTIONS !== 'true' && !env[VALIDATION_PARENT_ENV] + getCheckSpec(id).hostLocked && + (process.platform === 'darwin' || process.platform === 'linux') && + env.GITHUB_ACTIONS !== 'true' && + !env[VALIDATION_PARENT_ENV] ); } @@ -54,7 +57,12 @@ export async function executeGate(execution: GateExecution): Promise { let lock: Awaited> | undefined; let commandPid: number | undefined; try { - lock = await acquireValidationLock(controller.signal, execution.lockDirPath, execution.report); + lock = await acquireValidationLock( + controller.signal, + execution.lockDirPath, + execution.report, + env, + ); lock.recordCommand(receipt, null); const result = await runCmdStreaming( process.execPath, From 1628711576bba9bfa757f3037a51a8690b9b6660 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 22 Oct 2026 10:21:42 +0200 Subject: [PATCH 3/3] refactor(gate): run every host-locked command through runLockedCommand Move the publish, activate, finish protocol and the signal-to-exit-code mapping into scripts/check-affected so validation runs and heavy gates share one owner. Restore the testing and pull-request rules that the previous revision dropped, and cover a signal that arrives while the locked command is running. Co-Authored-By: Claude Sonnet 5.5 --- AGENTS.md | 4 +- docs/agents/pull-requests.md | 2 +- docs/agents/testing.md | 10 ++-- scripts/check-affected/command-execution.ts | 53 ++++++----------- scripts/check-affected/execution.ts | 24 +------- scripts/check-affected/host-lock.test.ts | 48 +++++++++++++++- scripts/check-affected/host-lock.ts | 45 +++++++++++++++ scripts/check-affected/termination.ts | 26 +++++++++ scripts/gate/host-locked.test.ts | 42 +++++++++++++- scripts/gate/host-locked.ts | 64 +++++++-------------- 10 files changed, 207 insertions(+), 111 deletions(-) create mode 100644 scripts/check-affected/termination.ts diff --git a/AGENTS.md b/AGENTS.md index ca5fb030e9..ce4ea08e31 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -48,11 +48,11 @@ The versioned CLI help owns command semantics. To drive an app, start with the s - In a new worktree, run `pnpm install --frozen-lockfile && pnpm build` before you test. - Do not change other worktrees, devices, sessions, dirty files, or experiments. Do not use `git stash`; all worktrees share it. - Do not copy `.env` into a worktree. For host credentials, source the main checkout's file by its absolute path. -- Give each parallel worker different files and a different device. `pnpm check:affected --run`, `pnpm check`, and heavy `pnpm gate` ids run one at a time automatically. +- Give each parallel worker different files and a different device. `pnpm check:affected --run`, `pnpm check`, and heavy `pnpm gate` ids share one lock. - The layering scan reads only tracked files. Stage new modules before you trust its result. - When you rename a file, move its Fallow baseline entry. Do not regenerate unrelated baselines. -The validation lock reclaims provably dead owners. An unknown owner blocks. +The validation lock automatically reclaims provably dead owners. An unknown owner continues to block. If the wait times out, report the printed owner and the lock state. Do not remove the lock manually. ## Agent docs diff --git a/docs/agents/pull-requests.md b/docs/agents/pull-requests.md index a4ac925d1c..7111e4568c 100644 --- a/docs/agents/pull-requests.md +++ b/docs/agents/pull-requests.md @@ -14,7 +14,7 @@ Apply authorization already given. Merging and releasing need separate authoriza Follow [validation](testing.md). Record the `check:affected --run --head` receipt in the PR body. A serialized gate stage can validate the pushed head instead. For unresolved failures, follow the blocked-validation table. -A blocker is not a pass. +An authorized incomplete draft can record a blocker. A blocker is not a pass. Push once per review round. Each push restarts CI. Drafts skip device lanes. For device-facing changes, follow [device verification](device-verification.md). diff --git a/docs/agents/testing.md b/docs/agents/testing.md index d6052eef06..a9392aba42 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -9,11 +9,10 @@ Serialized validation requires macOS or Linux. On Windows, use plan mode or focused checks. -Both broad commands acquire the same host lock. -These `pnpm gate` ids acquire the same lock: `unit`, `unit-ci`, `provider-integration`, `fuzz-parsers`, `mutation`, `mutation-affected`, and `concurrency-torture`. -A gate inside a locked validation reuses that lock. A GitHub runner takes no lock. +Both broad commands and heavy `pnpm gate` ids acquire the same host lock. Wait for the owner to finish, or cancel your waiter with Ctrl-C. -A wait ends after two hours. A nested `pnpm check` or `pnpm check:affected --run` fails immediately. +Do not add lock wrappers. Do not 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 contains: @@ -35,6 +34,7 @@ Execution stops at the first failure. Unexecuted checks remain `not-run`. Only a passed receipt with `exactCommit: true` proves validation at the named commit. 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. During validation, stop all edits. ## Interpret selection @@ -45,6 +45,7 @@ A full selection for these changes is intentional. 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`. GitHub owns: @@ -62,6 +63,7 @@ Do not add tests that assert prose. ## A gate failed Keep failure attribution separate from validation status. +Never call a failing gate passed. | Evidence | Next action | Outcome | | --- | --- | --- | diff --git a/scripts/check-affected/command-execution.ts b/scripts/check-affected/command-execution.ts index 7d7d92d154..c4ef25167f 100644 --- a/scripts/check-affected/command-execution.ts +++ b/scripts/check-affected/command-execution.ts @@ -1,7 +1,6 @@ import fs from 'node:fs'; import path from 'node:path'; -import { runCmdStreaming } from '@agent-device/host-kit/command'; -import { VALIDATION_PARENT_ENV, type ValidationLock } from './host-lock.ts'; +import { runLockedCommand, type ValidationLock } from './host-lock.ts'; import type { CheckResult, ValidationRun } from './execution.ts'; import type { TestReport } from './test-report.ts'; @@ -25,42 +24,23 @@ export async function executeValidationCommand( () => run.report(`[running] ${result.id} ${Math.round((Date.now() - started) / 1000)}s`), 15_000, ); - let commandPid: number | undefined; try { - 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: 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); - }, - 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); - }, + const exitCode = await runLockedCommand(lock, run.receiptPath, command, { + cwd, + env: { ...process.env, AGENT_DEVICE_VALIDATION_REPORT: reportPrefix }, + signal: run.controller.signal, + onStdoutChunk: (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; + onStderrChunk: (chunk) => { + fs.writeSync(descriptor, chunk); + }, + }); + result.exitCode = exitCode; + result.status = exitCode === 0 ? 'passed' : 'failed'; + run.report(`[${result.status}] ${result.id}; exit=${exitCode}`); + if (exitCode !== 0) reportFailureTail(logPath, run.report); + return exitCode; } catch (error) { result.status = run.controller.signal.aborted ? 'cancelled' : 'failed'; result.reason = error instanceof Error ? error.message : String(error); @@ -71,7 +51,6 @@ export async function executeValidationCommand( result.durationMs = Date.now() - started; collectTestReports(result, run.directory, reportPrefix); run.save(); - await lock.finishCommand(commandPid); } } diff --git a/scripts/check-affected/execution.ts b/scripts/check-affected/execution.ts index 72e5745205..9b782b94cd 100644 --- a/scripts/check-affected/execution.ts +++ b/scripts/check-affected/execution.ts @@ -2,6 +2,7 @@ import fs from 'node:fs'; import path from 'node:path'; import { executeValidationCommand } from './command-execution.ts'; import { acquireValidationLock } from './host-lock.ts'; +import { abortOnTerminationSignal } from './termination.ts'; import { gitValue, readCheckoutState, type CheckoutState } from './checkout-state.ts'; import type { TestReport } from './test-report.ts'; import type { CheckId, CheckPlan } from './model.ts'; @@ -118,22 +119,7 @@ function openValidationRun(cwd: string, args: ValidationArgs) { 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); + const { cancellation, dispose } = abortOnTerminationSignal(controller); return { receipt, directory, @@ -142,11 +128,7 @@ function openValidationRun(cwd: string, args: ValidationArgs) { cancellation, save, report, - dispose: () => { - process.off('SIGHUP', hangup); - process.off('SIGINT', interrupt); - process.off('SIGTERM', terminate); - }, + dispose, }; } diff --git a/scripts/check-affected/host-lock.test.ts b/scripts/check-affected/host-lock.test.ts index 9c0cf237b0..e78ef64b35 100644 --- a/scripts/check-affected/host-lock.test.ts +++ b/scripts/check-affected/host-lock.test.ts @@ -3,7 +3,7 @@ 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'; +import { acquireValidationLock, runLockedCommand, VALIDATION_PARENT_ENV } from './host-lock.ts'; test( 'host lock excludes a waiter, supports cancellation, then admits it after release', @@ -72,3 +72,49 @@ test('unsupported hosts reject validation before writing lock state', async () = fs.rmSync(directory, { recursive: true, force: true }); } }); + +test( + 'a locked command publishes ownership while it runs and clears it when it finishes', + { skip: process.platform === 'win32' }, + async () => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'validation-run-')); + const lock = path.join(directory, 'gate.lock'); + const activePath = `${lock}.active.json`; + const env = { ...process.env }; + delete env[VALIDATION_PARENT_ENV]; + try { + const owner = await acquireValidationLock(new AbortController().signal, lock, () => {}, env); + const observed: string[] = []; + const exitCode = await runLockedCommand( + owner, + '/fixture/receipt.json', + [ + process.execPath, + '-e', + `process.stdout.write(process.env.${VALIDATION_PARENT_ENV}); process.exit(3)`, + ], + { + cwd: directory, + env, + signal: new AbortController().signal, + onStdoutChunk: (chunk) => { + const active = JSON.parse(fs.readFileSync(activePath, 'utf8')) as { + receipt: string; + pid: number; + activation: string; + }; + observed.push(chunk, active.receipt, active.activation); + assert.ok(active.pid > 0); + }, + onStderrChunk: () => {}, + }, + ); + assert.equal(exitCode, 3); + assert.deepEqual(observed, ['/fixture/receipt.json', '/fixture/receipt.json', 'stdin']); + assert.equal(fs.existsSync(activePath), false); + await owner.release(); + } finally { + fs.rmSync(directory, { recursive: true, force: true }); + } + }, +); diff --git a/scripts/check-affected/host-lock.ts b/scripts/check-affected/host-lock.ts index 5ca8da9f1e..e052556813 100644 --- a/scripts/check-affected/host-lock.ts +++ b/scripts/check-affected/host-lock.ts @@ -2,6 +2,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { setTimeout } from 'node:timers/promises'; +import { runCmdStreaming } from '@agent-device/host-kit/command'; import { publishFileSync, inspectProcessLock, @@ -17,6 +18,7 @@ import { } from '@agent-device/host-kit/process'; export const VALIDATION_PARENT_ENV = 'AGENT_DEVICE_VALIDATION_PARENT'; +const COMMAND_RUNNER = path.join(import.meta.dirname, 'command.ts'); const HOST_LOCK_PATH = path.join(os.homedir(), '.cache', 'agent-device', 'validation.lock'); export type ValidationLock = { @@ -71,6 +73,49 @@ export async function acquireValidationLock( } } +// Owns the order orphan reclaim depends on: publish ownership, activate the command, then finish. +export async function runLockedCommand( + lock: ValidationLock, + receipt: string, + command: readonly string[], + options: { + cwd: string; + env: NodeJS.ProcessEnv; + signal: AbortSignal; + onStdoutChunk: (chunk: string) => void; + onStderrChunk: (chunk: string) => void; + }, +): Promise { + const [executable, ...args] = command as [string, ...string[]]; + let commandPid: number | undefined; + try { + lock.recordCommand(receipt, null); + const output = await runCmdStreaming( + process.execPath, + ['--experimental-strip-types', COMMAND_RUNNER, executable, ...args], + { + cwd: options.cwd, + allowFailure: true, + captureOutput: false, + detached: true, + kill: { signal: 'SIGTERM', graceMs: 5000 }, + signal: options.signal, + stdin: 'run\n', + onSpawn: (child) => { + commandPid = child.pid; + if (commandPid) lock.recordCommand(receipt, commandPid); + }, + env: { ...options.env, [VALIDATION_PARENT_ENV]: receipt }, + onStdoutChunk: options.onStdoutChunk, + onStderrChunk: options.onStderrChunk, + }, + ); + return output.exitCode; + } finally { + await lock.finishCommand(commandPid); + } +} + function recordValidationCommand(lockDirPath: string, receipt: string, pid: number | null): void { publishFileSync({ destination: `${lockDirPath}.active.json`, diff --git a/scripts/check-affected/termination.ts b/scripts/check-affected/termination.ts new file mode 100644 index 0000000000..51aeeede12 --- /dev/null +++ b/scripts/check-affected/termination.ts @@ -0,0 +1,26 @@ +const SIGNAL_EXIT_CODES = { SIGHUP: 129, SIGINT: 130, SIGTERM: 143 } as const; + +// Aborts the controller on a termination signal and records the conventional shell exit code. +export function abortOnTerminationSignal(controller: AbortController): { + cancellation: { code: number }; + dispose: () => void; +} { + const cancellation = { code: 0 }; + const handlers = (Object.keys(SIGNAL_EXIT_CODES) as (keyof typeof SIGNAL_EXIT_CODES)[]).map( + (signal) => + [ + signal, + () => { + cancellation.code = SIGNAL_EXIT_CODES[signal]; + controller.abort(); + }, + ] as const, + ); + for (const [signal, handler] of handlers) process.on(signal, handler); + return { + cancellation, + dispose: () => { + for (const [signal, handler] of handlers) process.off(signal, handler); + }, + }; +} diff --git a/scripts/gate/host-locked.test.ts b/scripts/gate/host-locked.test.ts index 489da8767c..621ba8eaac 100644 --- a/scripts/gate/host-locked.test.ts +++ b/scripts/gate/host-locked.test.ts @@ -11,7 +11,7 @@ import { executeGate, requiresHostLock } from './host-locked.ts'; const skip = process.platform === 'win32'; const cleanEnv = (extra: NodeJS.ProcessEnv = {}): NodeJS.ProcessEnv => { - const env = { ...process.env, ...extra }; + const env = { ...process.env }; delete env[VALIDATION_PARENT_ENV]; delete env.GITHUB_ACTIONS; return { ...env, ...extra }; @@ -214,3 +214,43 @@ test( } }, ); + +test( + 'a signal while the locked command runs stops it and still releases the lock', + { skip }, + async () => { + const { lockDirPath, marker, cleanup } = fixture(); + try { + const running = executeGate({ + id: 'unit-ci', + command: [ + process.execPath, + '-e', + `require('node:fs').writeFileSync(${JSON.stringify(marker)}, String(process.pid)); setInterval(() => {}, 1000)`, + ], + cwd: process.cwd(), + env: cleanEnv(), + lockDirPath, + }); + await new Promise((resolve) => { + const poll = setInterval(() => { + if (fs.existsSync(marker) && fs.readFileSync(marker, 'utf8')) { + clearInterval(poll); + resolve(); + } + }, 20); + }); + assert.ok( + fs.existsSync(`${lockDirPath}.active.json`), + 'ownership is published while running', + ); + process.emit('SIGTERM'); + assert.equal(await running, 143); + assert.throws(() => process.kill(Number(fs.readFileSync(marker, 'utf8')), 0), /ESRCH/); + assert.equal(fs.existsSync(lockDirPath), false, 'gate must release the lock'); + assert.equal(fs.existsSync(`${lockDirPath}.active.json`), false); + } finally { + cleanup(); + } + }, +); diff --git a/scripts/gate/host-locked.ts b/scripts/gate/host-locked.ts index 777eef16ef..645dcd7456 100644 --- a/scripts/gate/host-locked.ts +++ b/scripts/gate/host-locked.ts @@ -4,13 +4,14 @@ // A gate that already runs under that lock sees the inherited owner receipt and runs directly. // GitHub runners are single-tenant and run the gate directly. -import path from 'node:path'; import { runCmdStreaming } from '@agent-device/host-kit/command'; -import { acquireValidationLock, VALIDATION_PARENT_ENV } from '../check-affected/host-lock.ts'; +import { + acquireValidationLock, + runLockedCommand, + VALIDATION_PARENT_ENV, +} from '../check-affected/host-lock.ts'; import { getCheckSpec } from '../check-affected/checks.ts'; - -const COMMAND_RUNNER = path.join(import.meta.dirname, '..', 'check-affected', 'command.ts'); -const SIGNAL_EXIT_CODES = { SIGHUP: 129, SIGINT: 130, SIGTERM: 143 } as const; +import { abortOnTerminationSignal } from '../check-affected/termination.ts'; export type GateExecution = { id: string; @@ -32,30 +33,19 @@ export function requiresHostLock(id: string, env: NodeJS.ProcessEnv): boolean { export async function executeGate(execution: GateExecution): Promise { const { id, command, cwd, env } = execution; - const [executable, ...args] = command as [string, ...string[]]; const stream = { - allowFailure: true, onStdoutChunk: (chunk: string) => void process.stdout.write(chunk), onStderrChunk: (chunk: string) => void process.stderr.write(chunk), }; if (!requiresHostLock(id, env)) { - return (await runCmdStreaming(executable, args, { cwd, env, ...stream })).exitCode; + const [executable, ...args] = command as [string, ...string[]]; + return (await runCmdStreaming(executable, args, { cwd, env, allowFailure: true, ...stream })) + .exitCode; } const controller = new AbortController(); - let cancelled = 0; - const cancel = (signal: keyof typeof SIGNAL_EXIT_CODES) => () => { - cancelled = SIGNAL_EXIT_CODES[signal]; - controller.abort(); - }; - const handlers = (Object.keys(SIGNAL_EXIT_CODES) as (keyof typeof SIGNAL_EXIT_CODES)[]).map( - (signal) => [signal, cancel(signal)] as const, - ); - for (const [signal, handler] of handlers) process.on(signal, handler); - - const receipt = `gate:${id}`; + const { cancellation, dispose } = abortOnTerminationSignal(controller); let lock: Awaited> | undefined; - let commandPid: number | undefined; try { lock = await acquireValidationLock( controller.signal, @@ -63,35 +53,21 @@ export async function executeGate(execution: GateExecution): Promise { execution.report, env, ); - lock.recordCommand(receipt, null); - const result = await runCmdStreaming( - process.execPath, - ['--experimental-strip-types', COMMAND_RUNNER, executable, ...args], - { - ...stream, - cwd, - env: { ...env, [VALIDATION_PARENT_ENV]: receipt }, - captureOutput: false, - detached: true, - kill: { signal: 'SIGTERM', graceMs: 5000 }, - signal: controller.signal, - stdin: 'run\n', - onSpawn: (child) => { - commandPid = child.pid; - if (commandPid) lock?.recordCommand(receipt, commandPid); - }, - }, - ); - return cancelled || result.exitCode; + const exitCode = await runLockedCommand(lock, `gate:${id}`, command, { + cwd, + env, + signal: controller.signal, + ...stream, + }); + return cancellation.code || exitCode; } catch (error) { - if (cancelled) return cancelled; + if (cancellation.code) return cancellation.code; throw error; } finally { try { - await lock?.finishCommand(commandPid); - } finally { await lock?.release(); - for (const [signal, handler] of handlers) process.off(signal, handler); + } finally { + dispose(); } } }