From d01ee7b19ac9b7ff158fd149cf13d4cf0a5049cd Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 17:54:18 +0800 Subject: [PATCH] fix(verifiers): own setup terminal and snapshot lifetimes Join identity-verified PTY family cleanup and native exit before workspace removal, retaining failed cleanup capabilities for retry. Replace duplicate kill-and-reject loops with shared process ownership and deadlines. Capture configuration before setup using owned file handles and compare raw bytes. Preserve interactive cancellation, terminal restoration, redirected output, and selected inputs; remove the test-only forwarding export. Verified 122 focused cases, 9 README cases, build, static checks, docs, discovery, and duplication at 3.58%. --- scripts/lib/owned-verifier-terminal.d.mts | 8 + scripts/lib/owned-verifier-terminal.mjs | 13 + scripts/lib/verifier-file-snapshot-owner.mts | 81 ++++ scripts/lib/verifier-file-snapshots.d.mts | 5 + scripts/lib/verifier-file-snapshots.mjs | 22 + scripts/lib/verifier-terminal-owner.mts | 425 ++++++++++++++++ scripts/lib/verify-setup-terminal.d.mts | 19 + scripts/lib/verify-setup-terminal.mjs | 142 ++++++ scripts/verify-package-setup.d.mts | 8 +- scripts/verify-package-setup.mjs | 194 +------- .../filesystem/packageClientBackup.test.ts | 6 +- .../filesystem/verifierFileSnapshots.test.ts | 277 +++++++++++ .../packageSetupTerminalOwnership.test.ts | 321 ++++++++++++ .../process/verifierTerminalOwner.test.ts | 457 ++++++++++++++++++ 14 files changed, 1782 insertions(+), 196 deletions(-) create mode 100644 scripts/lib/owned-verifier-terminal.d.mts create mode 100644 scripts/lib/owned-verifier-terminal.mjs create mode 100644 scripts/lib/verifier-file-snapshot-owner.mts create mode 100644 scripts/lib/verifier-file-snapshots.d.mts create mode 100644 scripts/lib/verifier-file-snapshots.mjs create mode 100644 scripts/lib/verifier-terminal-owner.mts create mode 100644 scripts/lib/verify-setup-terminal.d.mts create mode 100644 scripts/lib/verify-setup-terminal.mjs create mode 100644 tests/boundary/filesystem/verifierFileSnapshots.test.ts create mode 100644 tests/boundary/process/packageSetupTerminalOwnership.test.ts create mode 100644 tests/boundary/process/verifierTerminalOwner.test.ts diff --git a/scripts/lib/owned-verifier-terminal.d.mts b/scripts/lib/owned-verifier-terminal.d.mts new file mode 100644 index 0000000000..6febc16766 --- /dev/null +++ b/scripts/lib/owned-verifier-terminal.d.mts @@ -0,0 +1,8 @@ +export { runOwnedVerifierTerminal } from "./verifier-terminal-owner.mjs"; +export type { + VerifierTerminalOptions, + VerifierTerminalDependencies, + VerifierTerminalResult, + VerifierTerminalFailure, + VerifierTerminalCleanupFailure, +} from "./verifier-terminal-owner.mjs"; diff --git a/scripts/lib/owned-verifier-terminal.mjs b/scripts/lib/owned-verifier-terminal.mjs new file mode 100644 index 0000000000..8388cb89db --- /dev/null +++ b/scripts/lib/owned-verifier-terminal.mjs @@ -0,0 +1,13 @@ +let implementation; + +/** Load the development terminal owner and canonical process helpers together. */ +export async function runOwnedVerifierTerminal(options, dependencies) { + implementation ??= import("tsx/esm/api").then(({ tsImport }) => + tsImport( + new URL("./verifier-terminal-owner.mts", import.meta.url).href, + import.meta.url, + ), + ); + const module = await implementation; + return module.runOwnedVerifierTerminal(options, dependencies); +} diff --git a/scripts/lib/verifier-file-snapshot-owner.mts b/scripts/lib/verifier-file-snapshot-owner.mts new file mode 100644 index 0000000000..0e321f6665 --- /dev/null +++ b/scripts/lib/verifier-file-snapshot-owner.mts @@ -0,0 +1,81 @@ +import { lstat, open } from "node:fs/promises"; + +import { ArtifactResourceScope } from "../../src/artifacts/ArtifactResourceScope.js"; +import { OwnedFileHandle } from "../../src/filesystem/OwnedFileHandle.js"; +import type { PackageVerifierCommands } from "./package-command-owner.mjs"; +import { withVerifierCleanup } from "./verifier-cleanup.mjs"; + +export type VerifierFileSnapshotEntry = + | { + readonly path: string; + readonly exists: true; + readonly content: Buffer; + } + | { + readonly path: string; + readonly exists: false; + readonly content: null; + }; + +/** Capture source values while the command owner retains every in-flight file. */ +export async function captureVerifierFileSnapshot( + commands: Pick, + paths: readonly string[], +): Promise { + const resources = new ArtifactResourceScope(); + commands.adopt(resources, "verifier file snapshots"); + return withVerifierCleanup( + () => + Promise.all( + paths.map((path) => + resources.run(async (): Promise => { + try { + await lstat(path); + } catch (cause: unknown) { + if ( + cause instanceof Error && + "code" in cause && + cause.code === "ENOENT" + ) + return { path, exists: false, content: null }; + throw cause; + } + const handle = await open(path, "r"); + const owner = new OwnedFileHandle(handle); + resources.retain({ + kind: "file-handle", + resource: path, + handle: owner, + }); + return { + path, + exists: true, + content: await handle.readFile(), + }; + }), + ), + ), + [() => resources.close()], + ); +} + +/** Compare the same selected paths against their completed baseline facts. */ +export async function assertVerifierFilesUnchanged( + commands: Pick, + snapshot: readonly VerifierFileSnapshotEntry[], + label: string, +): Promise { + const current = await captureVerifierFileSnapshot( + commands, + snapshot.map(({ path }) => path), + ); + for (const [index, entry] of snapshot.entries()) { + const observed = current[index]!; + if ( + entry.exists + ? !observed.exists || !entry.content.equals(observed.content) + : observed.exists + ) + throw new Error(`${label} changed ${entry.path}`); + } +} diff --git a/scripts/lib/verifier-file-snapshots.d.mts b/scripts/lib/verifier-file-snapshots.d.mts new file mode 100644 index 0000000000..7e6d84324c --- /dev/null +++ b/scripts/lib/verifier-file-snapshots.d.mts @@ -0,0 +1,5 @@ +export { + captureVerifierFileSnapshot, + assertVerifierFilesUnchanged, +} from "./verifier-file-snapshot-owner.mjs"; +export type { VerifierFileSnapshotEntry } from "./verifier-file-snapshot-owner.mjs"; diff --git a/scripts/lib/verifier-file-snapshots.mjs b/scripts/lib/verifier-file-snapshots.mjs new file mode 100644 index 0000000000..115efdafbf --- /dev/null +++ b/scripts/lib/verifier-file-snapshots.mjs @@ -0,0 +1,22 @@ +let implementation; + +/** Load the source file owner only when a verifier needs snapshot facts. */ +const loadImplementation = () => { + implementation ??= import("tsx/esm/api").then(({ tsImport }) => + tsImport( + new URL("./verifier-file-snapshot-owner.mts", import.meta.url).href, + import.meta.url, + ), + ); + return implementation; +}; + +export async function captureVerifierFileSnapshot(commands, paths) { + const module = await loadImplementation(); + return module.captureVerifierFileSnapshot(commands, paths); +} + +export async function assertVerifierFilesUnchanged(commands, snapshot, label) { + const module = await loadImplementation(); + return module.assertVerifierFilesUnchanged(commands, snapshot, label); +} diff --git a/scripts/lib/verifier-terminal-owner.mts b/scripts/lib/verifier-terminal-owner.mts new file mode 100644 index 0000000000..1c23ac41e9 --- /dev/null +++ b/scripts/lib/verifier-terminal-owner.mts @@ -0,0 +1,425 @@ +import { randomUUID } from "node:crypto"; +import type { IDisposable, IPty } from "@lydell/node-pty"; + +import type { RetainedResourceScope } from "../../src/application/AnalysisProvider.js"; +import { AnalysisCleanupLease } from "../../src/artifacts/AnalysisCleanupLease.js"; +import { ArtifactResourceScope } from "../../src/artifacts/ArtifactResourceScope.js"; +import { err, ok, type Result } from "../../src/domain/result.js"; +import { + cleanupOwnedProcessGroup, + type PersistentOwnedProcessGroup, +} from "../../src/process/ProcessOwnership.js"; +import { ProcessOwnershipEvidenceOwner } from "../../src/process/ProcessOwnershipEvidence.js"; +import { + prepareProcessOwnershipInspection, + systemProcessOwnershipHost, +} from "../../src/process/ProcessOwnershipObservation.js"; +import { PROCESS_CLEANUP_VERIFICATION_GRACE_MS } from "../../src/process/ProcessLifecycle.js"; +import { ProviderStartupDeadline } from "../../src/process/ProviderDeadline.js"; +import { processCaptureOwnershipUnavailableReason } from "../../src/process/capture/ProcessCaptureCapability.js"; +import { retainOwnedCommandCleanup } from "../../src/providers/OwnedCommandCleanup.js"; +import type { + PackageCleanupOwner, + PackageVerifierCommands, +} from "./package-command-owner.mjs"; +import { + closeVerifierResources, + withVerifierCleanup, +} from "./verifier-cleanup.mjs"; + +export interface VerifierTerminalOptions { + readonly commands: Pick; + readonly command: string; + readonly args: readonly string[]; + readonly cwd: string; + readonly environment: NodeJS.ProcessEnv; + readonly timeoutMs: number; + /** Capacity of the decoded text's UTF-8 representation, not raw PTY bytes. */ + readonly maxOutputBytes?: number; + readonly onData?: (output: string, write: (data: string) => void) => void; + readonly signal?: AbortSignal; +} + +export interface VerifierTerminalDependencies { + readonly loadPty?: () => Promise; + readonly cleanupProcessGroup?: typeof cleanupOwnedProcessGroup; +} + +export interface VerifierTerminalResult { + readonly output: string; + readonly exitCode: number; + readonly signal?: number; +} + +type TerminalExit = Omit; +type TerminalFailureReason = + | "timeout" + | "cancelled" + | "output_limit" + | "unsupported_platform"; + +/** Expected terminal failures retain the decoded output observed at failure. */ +export class VerifierTerminalFailure extends Error { + readonly _tag = "VerifierTerminalFailure"; + + constructor( + readonly reason: TerminalFailureReason, + readonly output: string, + message: string, + options?: ErrorOptions, + ) { + super(message, options); + this.name = "VerifierTerminalFailure"; + } +} + +/** The adopted owner remains the exact authority for retrying this release. */ +export class VerifierTerminalCleanupFailure extends Error { + readonly _tag = "VerifierTerminalCleanupFailure"; + readonly reason = "cleanup_incomplete"; + + constructor( + readonly cleanupOwner: PackageCleanupOwner, + readonly output: string, + message: string, + options?: ErrorOptions, + ) { + super(message, options); + this.name = "VerifierTerminalCleanupFailure"; + } +} + +/** Select a code-point prefix before allocating its retained representation. */ +const decodedPrefix = (text: string, capacity: number): string => { + let end = 0; + let bytes = 0; + for (const character of text) { + const size = Buffer.byteLength(character, "utf8"); + if (bytes + size > capacity) break; + bytes += size; + end += character.length; + } + return text.slice(0, end); +}; + +class VerifierTerminalOwner implements PackageCleanupOwner { + readonly #controller = new AbortController(); + readonly #runId = randomUUID(); + readonly #evidence = new ProcessOwnershipEvidenceOwner(); + readonly #preparationResources = new ArtifactResourceScope(); + readonly #resourceOwner: RetainedResourceScope = { + run: (work) => this.#preparationResources.run(work), + retain: (owner, resource) => + this.#preparationResources.retain({ + kind: "lease", + resource, + lease: AnalysisCleanupLease.forCloseOwner(owner, resource), + }), + }; + readonly #subscriptions = new Set(); + readonly #maxOutputBytes: number; + #output = ""; + #outputBytes = 0; + #outputLimited = false; + #terminal: IPty | undefined; + #ownership: PersistentOwnedProcessGroup | undefined; + #startup: Promise> | undefined; + #releasePromise: Promise | undefined; + #groupReleased = false; + #released = false; + #settled = false; + #cancelled: VerifierTerminalFailure | undefined; + #deadline: ProviderStartupDeadline | undefined; + #resolveCompletion!: (result: Result) => void; + readonly #completion = new Promise>( + (resolve) => { + this.#resolveCompletion = resolve; + }, + ); + #resolveExit!: () => void; + readonly #nativeExit = new Promise((resolve) => { + this.#resolveExit = resolve; + }); + readonly #onAbort = (): void => { + this.#cancel(this.options.signal?.reason); + }; + readonly #onDeadlineAbort = (): void => { + const deadline = this.#deadline; + if (deadline === undefined) return; + if (deadline.interruption === "timeout") { + this.#finish( + err( + new VerifierTerminalFailure( + "timeout", + this.#output, + `Verifier terminal timed out after ${String(this.options.timeoutMs)} ms`, + ), + ), + ); + return; + } + this.#cancel(deadline.signal.reason); + }; + + constructor( + private readonly options: VerifierTerminalOptions, + private readonly dependencies: VerifierTerminalDependencies, + ) { + this.#maxOutputBytes = options.maxOutputBytes ?? 1024 * 1024; + } + + start(): void { + // Adoption precedes even the first asynchronous preparation operation. + this.#startup = Promise.resolve() + .then(() => this.#acquire()) + .then(ok, (cause: unknown) => { + // Retain behind the already adopted terminal owner, even while its + // parent is closing. Startup cannot settle before transfer completes. + retainOwnedCommandCleanup( + cause, + this.#resourceOwner, + "package-verifier", + this.options.command, + ); + return err(cause); + }); + this.options.signal?.addEventListener("abort", this.#onAbort, { + once: true, + }); + if (this.options.signal?.aborted === true) this.#onAbort(); + } + + async result(): Promise> { + const startup = await this.#startup; + if (startup !== undefined && !startup.ok) return startup; + const exit = await this.#completion; + return exit.ok ? ok({ output: this.#output, ...exit.value }) : exit; + } + + throwIfCancelled(): void { + if (this.#cancelled !== undefined) throw this.#cancelled; + } + + close(): Promise { + if (!this.#released) + this.#cancel(new Error("Verifier terminal owner closed")); + return this.release(); + } + + /** Normal finalization releases resources without cancelling its own run. */ + release(): Promise { + if (this.#released) return Promise.resolve(); + if (this.#releasePromise !== undefined) return this.#releasePromise; + const attempt = Promise.resolve() + .then(() => this.#release()) + .finally(() => { + if (this.#releasePromise === attempt) this.#releasePromise = undefined; + }); + this.#releasePromise = attempt; + return attempt; + } + + async #acquire(): Promise { + const { signal } = this.#controller; + signal.throwIfAborted(); + await prepareProcessOwnershipInspection(signal); + signal.throwIfAborted(); + const captureBaseline = systemProcessOwnershipHost.captureBaseline; + if (captureBaseline === undefined) + throw new Error("Process ownership baseline is unavailable on this host"); + const baseline = await captureBaseline(signal); + signal.throwIfAborted(); + const { spawn } = await ( + this.dependencies.loadPty ?? (() => import("@lydell/node-pty")) + )(); + signal.throwIfAborted(); + const terminal = spawn(this.options.command, [...this.options.args], { + cwd: this.options.cwd, + env: { ...this.options.environment, REA_PROCESS_RUN_ID: this.#runId }, + name: "xterm-256color", + cols: 120, + rows: 40, + }); + this.#terminal = terminal; + this.#ownership = { + runId: this.#runId, + leaderPid: terminal.pid, + processGroupId: terminal.pid, + expectedParentPid: process.pid, + captureBaseline: baseline, + sweepTokenOwnedProcesses: true, + evidenceOwner: this.#evidence, + }; + this.#subscriptions.add( + terminal.onExit((exit) => { + this.#resolveExit(); + this.#finish(ok(exit)); + }), + ); + this.#subscriptions.add(terminal.onData((data) => this.#onData(data))); + if (!this.#settled) { + this.#deadline = new ProviderStartupDeadline( + this.options.timeoutMs, + this.#controller.signal, + ); + this.#deadline.signal.addEventListener("abort", this.#onDeadlineAbort, { + once: true, + }); + if (this.#deadline.signal.aborted) this.#onDeadlineAbort(); + } + } + + #onData(data: string): void { + if (this.#outputLimited) return; + const bytes = Buffer.byteLength(data, "utf8"); + const remaining = this.#maxOutputBytes - this.#outputBytes; + if (bytes > remaining) { + const prefix = decodedPrefix(data, remaining); + this.#output += prefix; + this.#outputBytes += Buffer.byteLength(prefix, "utf8"); + this.#outputLimited = true; + this.#finish( + err( + new VerifierTerminalFailure( + "output_limit", + this.#output, + `Verifier terminal decoded output exceeded ${String(this.#maxOutputBytes)} UTF-8 bytes`, + ), + ), + ); + return; + } + this.#output += data; + this.#outputBytes += bytes; + if (this.#settled) return; + try { + this.options.onData?.(this.#output, (input) => + this.#terminal?.write(input), + ); + } catch (cause: unknown) { + this.#finish(err(cause)); + } + } + + #finish(result: Result): void { + if (this.#settled) return; + this.#settled = true; + this.#disposeDeadline(); + this.#resolveCompletion(result); + } + + #disposeDeadline(): void { + this.#deadline?.signal.removeEventListener("abort", this.#onDeadlineAbort); + this.#deadline?.dispose(); + this.#deadline = undefined; + } + + #cancel(cause: unknown): void { + if (this.#released || this.#cancelled !== undefined) return; + this.#cancelled = new VerifierTerminalFailure( + "cancelled", + this.#output, + "Verifier terminal was cancelled", + { cause }, + ); + this.#controller.abort(this.#cancelled); + this.#finish(err(this.#cancelled)); + } + + async #release(): Promise { + // Join acquisition, not result(): close itself may be what ends execution. + await this.#startup; + this.#disposeDeadline(); + const terminal = await this.#releaseTerminal().then(ok, err); + const preparation = await this.#preparationResources.close().then(ok, err); + if (!terminal.ok && !preparation.ok) + throw new AggregateError( + [terminal.error, preparation.error], + "Verifier terminal and ownership preparation cleanup failed", + ); + if (!terminal.ok) throw terminal.error; + if (!preparation.ok) throw preparation.error; + this.options.signal?.removeEventListener("abort", this.#onAbort); + this.#released = true; + } + + async #releaseTerminal(): Promise { + if (this.#ownership !== undefined && !this.#groupReleased) { + const result = await ( + this.dependencies.cleanupProcessGroup ?? cleanupOwnedProcessGroup + )(this.#ownership); + if (!result.cleaned) + throw new VerifierTerminalCleanupFailure( + this, + this.#output, + result.reason, + { cause: result }, + ); + this.#groupReleased = true; + } + if (this.#terminal !== undefined && !(await this.#waitForNativeExit())) + throw new VerifierTerminalCleanupFailure( + this, + this.#output, + "Verified process cleanup did not produce the native terminal exit event", + ); + await closeVerifierResources( + [...this.#subscriptions].map((subscription) => () => { + subscription.dispose(); + this.#subscriptions.delete(subscription); + }), + ); + } + + async #waitForNativeExit(): Promise { + let timeout: ReturnType | undefined; + try { + return await Promise.race([ + this.#nativeExit.then(() => true), + new Promise((resolve) => { + timeout = setTimeout( + () => resolve(false), + PROCESS_CLEANUP_VERIFICATION_GRACE_MS, + ); + }), + ]); + } finally { + clearTimeout(timeout); + } + } +} + +/** Run one native POSIX PTY through observed exit and retained family cleanup. */ +export async function runOwnedVerifierTerminal( + options: VerifierTerminalOptions, + dependencies: VerifierTerminalDependencies = {}, +): Promise { + const unavailable = processCaptureOwnershipUnavailableReason( + process.platform, + ); + if (unavailable !== undefined) + throw new VerifierTerminalFailure("unsupported_platform", "", unavailable); + if (!Number.isFinite(options.timeoutMs) || options.timeoutMs <= 0) + throw new RangeError( + "Verifier terminal timeout must be positive and finite", + ); + const maxOutputBytes = options.maxOutputBytes ?? 1024 * 1024; + if (!Number.isSafeInteger(maxOutputBytes) || maxOutputBytes < 0) + throw new RangeError( + "Verifier terminal output capacity must be a non-negative safe integer", + ); + const owner = new VerifierTerminalOwner(options, dependencies); + options.commands.adopt(owner); + owner.start(); + let release: Promise | undefined; + const finish = (): Promise => (release ??= owner.release()); + return withVerifierCleanup(async () => { + const result = await owner.result(); + // Select late cancellation after finalization; composition reuses this exact + // attempt, including its rejection, instead of invoking cleanup twice. + await finish().then(ok, err); + if (!result.ok) throw result.error; + owner.throwIfCancelled(); + return result.value; + }, [finish]); +} diff --git a/scripts/lib/verify-setup-terminal.d.mts b/scripts/lib/verify-setup-terminal.d.mts new file mode 100644 index 0000000000..04a625a486 --- /dev/null +++ b/scripts/lib/verify-setup-terminal.d.mts @@ -0,0 +1,19 @@ +import type { PackageVerifierCommands } from "./verify-package-commands.mjs"; + +export interface SetupTerminalOptions { + readonly commands: Pick; + readonly command: string; + readonly environment: NodeJS.ProcessEnv; + readonly root: string; + readonly unchangedPaths: readonly string[]; +} + +/** Require positive canonical-input and echo observations. */ +export function terminalModesRestored(termios: unknown): boolean; + +export function verifyInteractiveSetup( + options: SetupTerminalOptions, +): Promise; +export function verifyRedirectedStdoutSetup( + options: SetupTerminalOptions, +): Promise; diff --git a/scripts/lib/verify-setup-terminal.mjs b/scripts/lib/verify-setup-terminal.mjs new file mode 100644 index 0000000000..2ce4db2902 --- /dev/null +++ b/scripts/lib/verify-setup-terminal.mjs @@ -0,0 +1,142 @@ +import { runOwnedVerifierTerminal } from "./owned-verifier-terminal.mjs"; +import { + captureVerifierFileSnapshot, + assertVerifierFilesUnchanged, +} from "./verifier-file-snapshots.mjs"; + +export const terminalModesRestored = (termios) => { + if (typeof termios !== "string") return false; + const modes = new Set(termios.trim().split(/\s+/u)); + return modes.has("icanon") && modes.has("echo"); +}; + +export async function verifyInteractiveSetup({ + commands, + command, + environment, + root, + unchangedPaths, +}) { + const unchanged = await captureVerifierFileSnapshot(commands, unchangedPaths); + let cancelled = false; + const wrapper = ` + const { spawnSync } = require("node:child_process"); + const child = spawnSync(process.argv[1], ["setup"], { stdio: "inherit", env: process.env, shell: process.platform === "win32" }); + const terminal = process.platform === "win32" + ? spawnSync("cmd.exe", ["/c", "mode", "CON"], { encoding: "utf8", stdio: ["inherit", "pipe", "inherit"] }) + : spawnSync("stty", ["-a"], { encoding: "utf8", stdio: ["inherit", "pipe", "inherit"] }); + process.stdout.write("\\n__REA_SETUP_EXIT__" + JSON.stringify({ status: child.status, termios: terminal.stdout, error: child.error?.message })); + process.exit(child.status ?? 1); + `; + let result; + try { + result = await runOwnedVerifierTerminal({ + commands, + command: process.execPath, + args: ["-e", wrapper, command], + cwd: root, + environment: { ...environment, NO_COLOR: "1", TERM: "xterm-256color" }, + timeoutMs: 20_000, + onData(output, write) { + if (!cancelled && output.includes("Which agents should use REA?")) { + cancelled = true; + write("\u0003"); + } + }, + }); + } catch (cause) { + if (cause?._tag === "VerifierTerminalFailure" && cause.reason === "timeout") + throw new Error( + `packaged setup wizard timed out after output: ${cause.output}`, + { cause }, + ); + throw cause; + } + const { output, exitCode } = result; + const marker = output.lastIndexOf("__REA_SETUP_EXIT__"); + let childResult; + try { + childResult = + marker < 0 ? undefined : JSON.parse(output.slice(marker + 18).trim()); + } catch { + childResult = undefined; + } + if ( + !cancelled || + exitCode !== 0 || + childResult?.status !== 0 || + !output.includes("\u001b[?25h") || + !terminalModesRestored(childResult?.termios) || + !output.includes("No changes were made.") + ) + throw new Error( + `packaged rea-agents setup did not cancel with a restored terminal and exit 0: ${output}`, + ); + await assertVerifierFilesUnchanged( + commands, + unchanged, + "Interactive cancellation", + ); +} + +export async function verifyRedirectedStdoutSetup({ + commands, + command, + environment, + root, + unchangedPaths, +}) { + const unchanged = await captureVerifierFileSnapshot(commands, unchangedPaths); + const wrapper = ` + const { spawnSync } = require("node:child_process"); + const child = spawnSync(process.argv[1], ["setup"], { encoding: "utf8", env: process.env, stdio: ["inherit", "pipe", "inherit"], shell: process.platform === "win32" }); + process.stdout.write("\\n__REA_REDIRECTED_SETUP__" + JSON.stringify({ status: child.status, stdout: child.stdout, error: child.error?.message, stdinTTY: process.stdin.isTTY, stderrTTY: process.stderr.isTTY, stdoutRedirected: true })); + process.exit(child.status ?? 1); + `; + let result; + try { + result = await runOwnedVerifierTerminal({ + commands, + command: process.execPath, + args: ["-e", wrapper, command], + cwd: root, + environment: { ...environment, NO_COLOR: "1", TERM: "xterm-256color" }, + timeoutMs: 15_000, + }); + } catch (cause) { + if (cause?._tag === "VerifierTerminalFailure" && cause.reason === "timeout") + throw new Error( + `packaged setup started an interactive prompt with redirected stdout: ${cause.output}`, + { cause }, + ); + throw cause; + } + const { output, exitCode } = result; + const marker = output.lastIndexOf("__REA_REDIRECTED_SETUP__"); + let childResult; + try { + childResult = + marker < 0 ? undefined : JSON.parse(output.slice(marker + 24).trim()); + } catch { + childResult = undefined; + } + const allOutput = `${output}\n${childResult?.stdout ?? ""}`; + if ( + (exitCode !== 0 && exitCode !== 1) || + childResult?.status !== exitCode || + childResult?.stdinTTY !== true || + childResult?.stderrTTY !== true || + childResult?.stdoutRedirected !== true || + childResult?.stdout?.includes("Which agents should use REA?") || + allOutput.includes("Which agents should use REA?") || + /\u001b\[[0-9;?]*[ -/]*[@-~]/u.test(allOutput) + ) + throw new Error( + `packaged setup did not remain plain and non-interactive with redirected stdout: ${output}`, + ); + await assertVerifierFilesUnchanged( + commands, + unchanged, + "Redirected-stdout setup", + ); +} diff --git a/scripts/verify-package-setup.d.mts b/scripts/verify-package-setup.d.mts index 31a2112866..e708c054b3 100644 --- a/scripts/verify-package-setup.d.mts +++ b/scripts/verify-package-setup.d.mts @@ -7,12 +7,12 @@ export function assertClientConfig( original: Uint8Array, ): Promise; -/** Return true only when stty output positively reports canonical input and echo. */ -export function terminalModesRestored(termios: unknown): boolean; - /** Verify setup using the parent's explicit command owner. */ export function verifyPackageSetup(input: { - readonly commands: Pick; + readonly commands: Pick< + PackageVerifierCommands, + "run" | "runWithStatus" | "adopt" + >; readonly cli: string; readonly packageRunnerCli: string; readonly environment: NodeJS.ProcessEnv; diff --git a/scripts/verify-package-setup.mjs b/scripts/verify-package-setup.mjs index db241d8f95..ddd84cc5d4 100644 --- a/scripts/verify-package-setup.mjs +++ b/scripts/verify-package-setup.mjs @@ -9,21 +9,17 @@ import { } from "node:fs/promises"; import { dirname, join } from "node:path"; -import { spawn } from "@lydell/node-pty"; - import { TOOL_CONTRACTS } from "../dist/contracts/toolContracts.js"; import { PRODUCT_IDENTITY } from "../dist/identity.js"; import { json, pathExists } from "./lib/verify-package-core.mjs"; import { parse as parseJsonc } from "jsonc-parser"; import { skillReferenceIssues } from "./lib/docs-facts.mjs"; +import { + verifyInteractiveSetup, + verifyRedirectedStdoutSetup, +} from "./lib/verify-setup-terminal.mjs"; import { setupPlanFailureMessage } from "./verify-package-setup-plan.mjs"; -export const terminalModesRestored = (termios) => { - if (typeof termios !== "string") return false; - const modes = new Set(termios.trim().split(/\s+/u)); - return modes.has("icanon") && modes.has("echo"); -}; - const OPENCODE_ORIGINAL = `{ // OpenCode preferences must survive setup. "model": "provider/model", @@ -49,186 +45,6 @@ const verifyMcpAdd = async ({ commands, cli, environment, npxLog }) => { throw new Error("Incur mcp add did not register the pinned npx command"); }; -const snapshotFiles = async (paths) => - Promise.all( - paths.map(async (path) => { - const exists = await pathExists(path); - return { - path, - exists, - content: exists ? await readFile(path, "utf8") : null, - }; - }), - ); - -const assertFilesUnchanged = async (snapshot, label) => { - for (const entry of snapshot) { - const exists = await pathExists(entry.path); - const content = exists ? await readFile(entry.path, "utf8") : null; - if (exists !== entry.exists || content !== entry.content) - throw new Error(`${label} changed ${entry.path}`); - } -}; - -const verifyInteractiveSetup = async ({ - command, - environment, - root, - unchangedPaths, -}) => - new Promise((resolvePromise, reject) => { - let output = ""; - let cancelled = false; - let settled = false; - const unchanged = snapshotFiles(unchangedPaths); - const wrapper = ` - const { spawnSync } = require("node:child_process"); - const child = spawnSync(process.argv[1], ["setup"], { stdio: "inherit", env: process.env, shell: process.platform === "win32" }); - const terminal = process.platform === "win32" - ? spawnSync("cmd.exe", ["/c", "mode", "CON"], { encoding: "utf8", stdio: ["inherit", "pipe", "inherit"] }) - : spawnSync("stty", ["-a"], { encoding: "utf8", stdio: ["inherit", "pipe", "inherit"] }); - process.stdout.write("\\n__REA_SETUP_EXIT__" + JSON.stringify({ status: child.status, termios: terminal.stdout, error: child.error?.message })); - process.exit(child.status ?? 1); - `; - const terminal = spawn(process.execPath, ["-e", wrapper, command], { - cwd: root, - env: { ...environment, NO_COLOR: "1", TERM: "xterm-256color" }, - name: "xterm-256color", - cols: 120, - rows: 40, - }); - const finish = (error) => { - if (settled) return; - settled = true; - clearTimeout(timeout); - if (error === undefined) resolvePromise(); - else reject(error); - }; - const timeout = setTimeout(() => { - terminal.kill(); - finish( - new Error(`packaged setup wizard timed out after output: ${output}`), - ); - }, 20_000); - - terminal.onData((data) => { - output += data; - if (!cancelled && output.includes("Which agents should use REA?")) { - cancelled = true; - terminal.write("\u0003"); - } - }); - terminal.onExit(async ({ exitCode }) => { - const marker = output.lastIndexOf("__REA_SETUP_EXIT__"); - const result = marker < 0 ? undefined : output.slice(marker + 18).trim(); - let childResult; - try { - childResult = result === undefined ? undefined : JSON.parse(result); - } catch { - childResult = undefined; - } - if ( - !cancelled || - exitCode !== 0 || - childResult?.status !== 0 || - !output.includes("\u001b[?25h") || - (process.platform !== "win32" && - !terminalModesRestored(childResult.termios)) || - !output.includes("No changes were made.") - ) { - finish( - new Error( - `packaged rea-agents setup did not cancel with a restored terminal and exit 0: ${output}`, - ), - ); - return; - } - try { - await assertFilesUnchanged(await unchanged, "Interactive cancellation"); - finish(); - } catch (error) { - finish(error); - } - }); - }); - -const verifyRedirectedStdoutSetup = async ({ - command, - environment, - root, - unchangedPaths, -}) => - new Promise((resolvePromise, reject) => { - let output = ""; - let settled = false; - const unchanged = snapshotFiles(unchangedPaths); - const wrapper = ` - const { spawnSync } = require("node:child_process"); - const child = spawnSync(process.argv[1], ["setup"], { encoding: "utf8", env: process.env, stdio: ["inherit", "pipe", "inherit"], shell: process.platform === "win32" }); - process.stdout.write("\\n__REA_REDIRECTED_SETUP__" + JSON.stringify({ status: child.status, stdout: child.stdout, error: child.error?.message, stdinTTY: process.stdin.isTTY, stderrTTY: process.stderr.isTTY, stdoutRedirected: true })); - process.exit(child.status ?? 1); - `; - const terminal = spawn(process.execPath, ["-e", wrapper, command], { - cwd: root, - env: { ...environment, NO_COLOR: "1", TERM: "xterm-256color" }, - name: "xterm-256color", - cols: 120, - rows: 40, - }); - const finish = (error) => { - if (settled) return; - settled = true; - clearTimeout(timeout); - if (error === undefined) resolvePromise(); - else reject(error); - }; - const timeout = setTimeout(() => { - terminal.kill(); - finish( - new Error( - `packaged setup started an interactive prompt with redirected stdout: ${output}`, - ), - ); - }, 15_000); - terminal.onData((data) => { - output += data; - }); - terminal.onExit(async ({ exitCode }) => { - const marker = output.lastIndexOf("__REA_REDIRECTED_SETUP__"); - let childResult; - try { - childResult = - marker < 0 ? undefined : JSON.parse(output.slice(marker + 24).trim()); - } catch { - childResult = undefined; - } - const allOutput = `${output}\n${childResult?.stdout ?? ""}`; - if ( - (exitCode !== 0 && exitCode !== 1) || - childResult?.status !== exitCode || - childResult.stdinTTY !== true || - childResult.stderrTTY !== true || - childResult.stdoutRedirected !== true || - childResult.stdout?.includes("Which agents should use REA?") || - allOutput.includes("Which agents should use REA?") || - /\u001b\[[0-9;?]*[ -/]*[@-~]/u.test(allOutput) - ) { - finish( - new Error( - `packaged setup did not remain plain and non-interactive with redirected stdout: ${output}`, - ), - ); - return; - } - try { - await assertFilesUnchanged(await unchanged, "Redirected-stdout setup"); - finish(); - } catch (error) { - finish(error); - } - }); - }); - const verifySetupPlan = async ({ commands, cli, @@ -771,12 +587,14 @@ export async function verifyPackageSetup({ ? await Promise.all([readFile(claudeConfig), readFile(cursorConfig)]) : []; await verifyInteractiveSetup({ + commands, command: packageRunnerCli, environment, root, unchangedPaths, }); await verifyRedirectedStdoutSetup({ + commands, command: packageRunnerCli, environment, root, diff --git a/tests/boundary/filesystem/packageClientBackup.test.ts b/tests/boundary/filesystem/packageClientBackup.test.ts index 8294c76c87..273e940017 100644 --- a/tests/boundary/filesystem/packageClientBackup.test.ts +++ b/tests/boundary/filesystem/packageClientBackup.test.ts @@ -2,10 +2,8 @@ import { readFile, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { expect, it } from "vitest"; -import { - assertClientConfig, - terminalModesRestored, -} from "../../../scripts/verify-package-setup.mjs"; +import { assertClientConfig } from "../../../scripts/verify-package-setup.mjs"; +import { terminalModesRestored } from "../../../scripts/lib/verify-setup-terminal.mjs"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; it("requires positive canonical-input and echo terminal mode tokens", () => { diff --git a/tests/boundary/filesystem/verifierFileSnapshots.test.ts b/tests/boundary/filesystem/verifierFileSnapshots.test.ts new file mode 100644 index 0000000000..8b4a0736a2 --- /dev/null +++ b/tests/boundary/filesystem/verifierFileSnapshots.test.ts @@ -0,0 +1,277 @@ +import { + access, + chmod, + readFile, + symlink, + unlink, + writeFile, +} from "node:fs/promises"; +import { join } from "node:path"; + +import { expect, it } from "vitest"; + +import { createPackageVerifierCommands } from "../../../scripts/lib/verify-package-commands.mjs"; +import { + assertVerifierFilesUnchanged, + captureVerifierFileSnapshot, +} from "../../../scripts/lib/verifier-file-snapshots.mjs"; +import { withVerifierCleanup } from "../../../scripts/lib/verifier-cleanup.mjs"; +import { + verifyInteractiveSetup, + verifyRedirectedStdoutSetup, +} from "../../../scripts/lib/verify-setup-terminal.mjs"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { processTest } from "../../support/process/processFixture.js"; + +const snapshotCommands = (root: string) => + createPackageVerifierCommands({ + workspace: { path: root, close: async () => {} }, + }); + +it("preserves selected path spelling, UTF-8 bytes and missing-file facts", async () => { + const root = await createTestTempDirectory("rea-verifier-snapshot-"); + const path = `${root}/./selected file.json`; + const missing = join(root, "missing file.json"); + const content = '\uFEFF{"label":"保留","value":"\\u0000"}\r\n'; + await writeFile(path, content); + const commands = await snapshotCommands(root); + const snapshot = await withVerifierCleanup( + () => captureVerifierFileSnapshot(commands, [missing, path]), + [() => commands.close()], + ); + + expect(snapshot).toEqual([ + { path: missing, exists: false, content: null }, + { path, exists: true, content: Buffer.from(content, "utf8") }, + ]); + expect(await readFile(path)).toEqual(Buffer.from(content, "utf8")); +}); + +it("detects distinct invalid UTF-8 bytes in verifier file readback", async () => { + const root = await createTestTempDirectory("rea-verifier-byte-readback-"); + const path = join(root, "selected.json"); + await writeFile(path, Buffer.from([0x80])); + const commands = await snapshotCommands(root); + const rejection = await withVerifierCleanup(async () => { + const snapshot = await captureVerifierFileSnapshot(commands, [path]); + await writeFile(path, Buffer.from([0x81])); + return assertVerifierFilesUnchanged(commands, snapshot, "Setup").then( + () => undefined, + (cause: unknown) => cause, + ); + }, [() => commands.close()]); + + expect(rejection).toBeInstanceOf(Error); + expect(rejection).toHaveProperty("message", `Setup changed ${path}`); +}); + +it.each(["content", "removed", "created"] as const)( + "detects a %s change after an unchanged readback", + async (change) => { + const root = await createTestTempDirectory("rea-verifier-readback-"); + const path = join(root, "selected.json"); + if (change !== "created") await writeFile(path, "original\n"); + const commands = await snapshotCommands(root); + const rejection = await withVerifierCleanup(async () => { + const snapshot = await captureVerifierFileSnapshot(commands, [path]); + await assertVerifierFilesUnchanged(commands, snapshot, "Setup"); + if (change === "removed") await unlink(path); + else await writeFile(path, "changed\n"); + return assertVerifierFilesUnchanged(commands, snapshot, "Setup").then( + () => undefined, + (cause: unknown) => cause, + ); + }, [() => commands.close()]); + + expect(rejection).toBeInstanceOf(Error); + expect(rejection).toHaveProperty("message", `Setup changed ${path}`); + }, +); + +it.skipIf(process.platform === "win32")( + "rejects a direct broken symlink instead of recording an absent file", + async () => { + const root = await createTestTempDirectory("rea-verifier-broken-link-"); + const path = join(root, "selected.json"); + await symlink(join(root, "missing-target.json"), path); + const commands = await snapshotCommands(root); + const rejection = await withVerifierCleanup( + () => + captureVerifierFileSnapshot(commands, [path]).then( + () => undefined, + (cause: unknown) => cause, + ), + [() => commands.close()], + ); + + expect(rejection).toHaveProperty("code", "ENOENT"); + }, +); + +it.skipIf(process.platform === "win32").each([ + ["interactive", verifyInteractiveSetup], + ["redirected stdout", verifyRedirectedStdoutSetup], +] as const)( + "finishes the %s baseline before launching setup", + async (_mode, verify) => { + const root = await createTestTempDirectory("rea-verifier-setup-baseline-"); + const path = join(root, "selected.json"); + const marker = join(root, "launched"); + const command = join(root, "setup-command"); + await symlink(join(root, "missing-target.json"), path); + await writeFile( + command, + '#!/bin/sh\nprintf launched > "$REA_SETUP_LAUNCH_MARKER"\n', + ); + await chmod(command, 0o700); + const commands = await snapshotCommands(root); + const rejection = await withVerifierCleanup( + () => + verify({ + commands, + command, + environment: { ...process.env, REA_SETUP_LAUNCH_MARKER: marker }, + root, + unchangedPaths: [path], + }).then( + () => undefined, + (cause: unknown) => cause, + ), + [() => commands.close()], + ); + const launched = await access(marker).then( + () => true, + (cause: unknown) => { + if ( + cause instanceof Error && + "code" in cause && + cause.code === "ENOENT" + ) + return false; + throw cause; + }, + ); + + expect(rejection).toHaveProperty("code", "ENOENT"); + expect(launched).toBe(false); + }, +); + +processTest( + "retains the exact failed file scope and directory until descriptor close is retried", + async ({ processes }) => { + const program = String.raw` + import assert from "node:assert/strict"; + import fs from "node:fs"; + import { access, writeFile } from "node:fs/promises"; + import { syncBuiltinESMExports } from "node:module"; + import { join } from "node:path"; + import { tsImport } from "tsx/esm/api"; + import { createPackageVerifierCommands } from ${JSON.stringify(new URL("../../../scripts/lib/verify-package-commands.mjs", import.meta.url).href)}; + import { captureVerifierFileSnapshot } from ${JSON.stringify(new URL("../../../scripts/lib/verifier-file-snapshots.mjs", import.meta.url).href)}; + import { withVerifierCleanup } from ${JSON.stringify(new URL("../../../scripts/lib/verifier-cleanup.mjs", import.meta.url).href)}; + const { createTestWorkspace, removeTestWorkspace } = await tsImport( + ${JSON.stringify(new URL("../../support/workspace/workspaceFixture.ts", import.meta.url).href)}, + ${JSON.stringify(import.meta.url)}, + ); + const workspace = await createTestWorkspace("rea-verifier-snapshot-owner-"); + const path = join(workspace.root, "input.json"); + const originalOpen = fs.promises.open; + let commands; + let handle; + let realClose; + let reads = 0; + let contents; + let attempts = 0; + const scopes = []; + const closeFailure = new Error("controlled real descriptor close failure"); + try { + await writeFile(path, "source bytes\n"); + fs.promises.open = async (...arguments_) => { + const opened = await originalOpen(...arguments_); + if (arguments_[0] === path) { + handle = opened; + const close = opened.close.bind(opened); + realClose = close; + const read = opened.readFile.bind(opened); + opened.readFile = async (...options) => { + reads++; + contents = await read(...options); + return contents; + }; + opened.close = async () => { + attempts++; + if (attempts === 1) throw closeFailure; + await close(); + }; + } + return opened; + }; + syncBuiltinESMExports(); + commands = await createPackageVerifierCommands({ + workspace: { path: workspace.root, close: () => removeTestWorkspace(workspace.root) }, + }); + const adopt = commands.adopt.bind(commands); + commands.adopt = (scope, resource) => { + scopes.push(scope); + adopt(scope, resource); + }; + const observed = await withVerifierCleanup(async () => { + const failure = await captureVerifierFileSnapshot(commands, [path]).then( + () => undefined, + error => error, + ); + await access(workspace.root); + return { + failure, + scope: scopes[0], + retained: scopes[0]?.hasPending, + descriptor: handle?.fd, + attempts, + reads, + contents, + }; + }, [() => commands.close()]); + const directoryExists = await access(workspace.root).then( + () => true, + error => { if (error.code === "ENOENT") return false; throw error; }, + ); + assert.ok(observed.failure instanceof AggregateError); + assert.equal(observed.failure.errors.length, 1); + const retainedFailure = observed.failure.errors[0]; + assert.equal(scopes.length, 1); + assert.equal(observed.retained, true); + assert.ok(observed.descriptor >= 0); + assert.equal(observed.attempts, 1); + assert.equal(observed.reads, 1); + assert.deepEqual(observed.contents, Buffer.from("source bytes\n")); + assert.equal(retainedFailure.failedOwners[0].handle.handle, handle); + assert.deepEqual(retainedFailure.cleanup.resources, [path]); + assert.deepEqual(retainedFailure.cause.errors, [closeFailure]); + assert.equal(observed.scope.hasPending, false); + assert.equal(attempts, 2); + assert.equal(handle.fd, -1); + assert.equal(directoryExists, false); + console.log(JSON.stringify({ exactHandleRetried: true, directoryReleasedAfterRetry: true })); + } finally { + fs.promises.open = originalOpen; + syncBuiltinESMExports(); + if (handle !== undefined) handle.close = realClose; + if (commands !== undefined) await commands.close(); + else await removeTestWorkspace(workspace.root); + } + `; + const result = await processes.run( + process.execPath, + ["--input-type=module", "--eval", program], + { timeoutMs: 20_000 }, + ); + + expect(result.exitCode, result.stderr).toBe(0); + expect(result.signal, result.stderr).toBeNull(); + expect(JSON.parse(result.stdout)).toEqual({ + exactHandleRetried: true, + directoryReleasedAfterRetry: true, + }); + }, +); diff --git a/tests/boundary/process/packageSetupTerminalOwnership.test.ts b/tests/boundary/process/packageSetupTerminalOwnership.test.ts new file mode 100644 index 0000000000..f04c2c4b2b --- /dev/null +++ b/tests/boundary/process/packageSetupTerminalOwnership.test.ts @@ -0,0 +1,321 @@ +import { chmod, mkdir, readFile, rm, writeFile } from "node:fs/promises"; +import { join, resolve } from "node:path"; +import { setTimeout as delay } from "node:timers/promises"; +import { pathToFileURL } from "node:url"; +import { expect } from "vitest"; +import { z } from "zod"; + +import { + createPackageVerifierCommands, + type PackageVerifierCommands, +} from "../../../scripts/lib/verify-package-commands.mjs"; +import { + verifyInteractiveSetup, + verifyRedirectedStdoutSetup, +} from "../../../scripts/lib/verify-setup-terminal.mjs"; +import { withVerifierCleanup } from "../../../scripts/lib/verifier-cleanup.mjs"; +import { systemProcessOwnershipHost } from "../../../src/process/ProcessOwnershipObservation.js"; +import { liveProcesses } from "../../../src/process/ProcessOwnershipProcessTree.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { processTest } from "../../support/process/processFixture.js"; + +const unchangedConfiguration = '{"callerSelected":"preserved"}\n'; +const timeoutMessage = + "packaged setup started an interactive prompt with redirected stdout:"; + +const descendantSource = ` +import { appendFileSync, existsSync } from "node:fs"; +const finish = (reason) => { + appendFileSync(process.env.TERMINAL_EVENTS, JSON.stringify({ kind: "descendant-exit", reason, rootExists: existsSync(process.env.HOME) }) + "\\n"); + process.exit(0); +}; +process.on("SIGHUP", () => undefined); +process.on("SIGTERM", () => finish("SIGTERM")); +process.send({ pid: process.pid }); +setInterval(() => { + if (existsSync(process.env.TERMINAL_RELEASE)) finish("fixture-release"); +}, 25); +setTimeout(() => finish("fixture-deadline"), 30000); +`; + +const setupStandinSource = ` +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; +import { appendFileSync, existsSync, writeFileSync } from "node:fs"; +assert.deepEqual(process.argv.slice(2), ["setup"]); +const tty = { stdin: Boolean(process.stdin.isTTY), stdout: Boolean(process.stdout.isTTY), stderr: Boolean(process.stderr.isTTY) }; +assert.deepEqual(tty, { stdin: true, stdout: false, stderr: true }); +const parent = process.ppid; +const finish = (reason) => { + appendFileSync(process.env.TERMINAL_EVENTS, JSON.stringify({ kind: "setup-exit", reason, rootExists: existsSync(process.env.HOME) }) + "\\n"); + process.exit(0); +}; +process.on("SIGHUP", () => finish("SIGHUP")); +process.on("SIGTERM", () => finish("SIGTERM")); +const child = spawn(process.execPath, [process.env.TERMINAL_DESCENDANT], { + stdio: ["ignore", "ignore", "ignore", "ipc"], +}); +const ready = await new Promise((resolve, reject) => { + child.once("message", resolve); + child.once("error", reject); +}); +writeFileSync(process.env.TERMINAL_READY, JSON.stringify({ pids: [process.pid, ready.pid], tty, arguments: process.argv.slice(2) })); +process.stderr.write("authored setup pending\\n"); +setInterval(() => { + if (existsSync(process.env.TERMINAL_RELEASE)) finish("fixture-release"); + if (process.ppid !== parent) finish("wrapper-exit"); +}, 25); +setTimeout(() => finish("fixture-deadline"), 30000); +`; + +const reportSchema = z.object({ + ready: z.object({ + pids: z.array(z.number().int().positive()).length(2), + tty: z.object({ + stdin: z.literal(true), + stdout: z.literal(false), + stderr: z.literal(true), + }), + arguments: z.tuple([z.literal("setup")]), + }), + liveAtRemoval: z.array(z.number().int().positive()), + configurationAtRemoval: z.string(), + rootRemoved: z.boolean(), + failure: z.object({ name: z.string(), message: z.string() }), +}); + +const releaseStandin = async (release: string, readyPath: string) => { + await writeFile(release, "release these authored setup processes"); + const ready = await readFile(readyPath, "utf8").then( + (text) => reportSchema.shape.ready.parse(JSON.parse(text)), + (cause: unknown) => { + if (cause instanceof Error && "code" in cause && cause.code === "ENOENT") + return undefined; + throw cause; + }, + ); + if (ready === undefined) return; + const deadline = Date.now() + 7_500; + while ( + liveProcesses(await systemProcessOwnershipHost.listProcesses()).some( + ({ pid }) => ready.pids.includes(pid), + ) + ) { + if (Date.now() >= deadline) + throw new Error( + "Authored setup processes did not consume their release marker", + ); + await delay(25); + } +}; + +// The executable only acknowledges terminal shape and owns a harmless child. +// The PTY, deadline, process census, command owner and workspace removal are real. +processTest.skipIf(process.platform === "win32")( + "joins a redirected-stdout setup descendant before removing its verifier workspace", + { timeout: 35_000 }, + async ({ processes }) => { + const root = await createTestTempDirectory("rea-package-setup-terminal-"); + const scratch = join(root, "scratch"); + const entry = join(root, "setup stand-in.mjs"); + const descendant = join(root, "descendant.mjs"); + const ready = join(root, "ready.json"); + const release = join(root, "release-setup"); + const events = join(root, "events.jsonl"); + const driver = join(root, "driver.mjs"); + await writeFile(entry, `#!${process.execPath}\n${setupStandinSource}`); + await chmod(entry, 0o755); + await writeFile(descendant, descendantSource); + const moduleUrl = (path: string) => pathToFileURL(resolve(path)).href; + await writeFile( + driver, + ` +import { existsSync } from "node:fs"; +import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; +import { createPackageVerifierCommands } from ${JSON.stringify(moduleUrl("scripts/lib/verify-package-commands.mjs"))}; +import { verifyRedirectedStdoutSetup } from ${JSON.stringify(moduleUrl("scripts/lib/verify-setup-terminal.mjs"))}; +import { withVerifierCleanup } from ${JSON.stringify(moduleUrl("scripts/lib/verifier-cleanup.mjs"))}; +import { systemProcessOwnershipHost } from ${JSON.stringify(moduleUrl("dist/process/ProcessOwnershipObservation.js"))}; +import { liveProcesses } from ${JSON.stringify(moduleUrl("dist/process/ProcessOwnershipProcessTree.js"))}; +const scratch = process.env.HOME; +await mkdir(scratch); +const config = ${JSON.stringify(join(scratch, "selected-config.json"))}; +await writeFile(config, ${JSON.stringify(unchangedConfiguration)}); +let commands, recoveryOwner, ready, liveAtRemoval, configurationAtRemoval, failure; +let removed = false; +const workspace = { + path: scratch, + async close() { + if (removed) return; + ready = JSON.parse(await readFile(process.env.TERMINAL_READY, "utf8")); + liveAtRemoval = liveProcesses(await systemProcessOwnershipHost.listProcesses()).filter(({ pid }) => ready.pids.includes(pid)).map(({ pid }) => pid); + configurationAtRemoval = await readFile(config, "utf8"); + await rm(scratch, { recursive: true, force: true }); + removed = true; + }, +}; +try { + await withVerifierCleanup(async () => { + commands = await createPackageVerifierCommands({ workspace }); + await verifyRedirectedStdoutSetup({ commands, command: ${JSON.stringify(entry)}, environment: process.env, root: ${JSON.stringify(process.cwd())}, unchangedPaths: [config] }); + }, [() => commands === undefined ? workspace.close() : commands.close()]); +} catch (cause) { + const failures = cause instanceof AggregateError ? cause.errors : [cause]; + // Capture exact returned cleanup authority before projecting or asserting facts. + recoveryOwner = failures.find(error => typeof error?.cleanupOwner?.close === "function")?.cleanupOwner ?? commands; + const primary = failures.find(error => error instanceof Error && error.message.startsWith(${JSON.stringify(timeoutMessage)})); + failure = primary === undefined ? undefined : { name: primary.name, message: primary.message }; +} finally { + if (recoveryOwner !== undefined) await recoveryOwner.close(); + else if (commands !== undefined) await commands.close(); + else await workspace.close(); +} +process.stdout.write(JSON.stringify({ ready, liveAtRemoval, configurationAtRemoval, rootRemoved: !existsSync(scratch), failure })); +`, + ); + try { + const result = await processes.run(process.execPath, [driver], { + timeoutMs: 25_000, + cwd: process.cwd(), + env: { + ...process.env, + HOME: scratch, + USERPROFILE: scratch, + TERMINAL_DESCENDANT: descendant, + TERMINAL_READY: ready, + TERMINAL_RELEASE: release, + TERMINAL_EVENTS: events, + }, + }); + expect(result.exitCode, result.stderr).toBe(0); + expect(result.signal, result.stderr).toBeNull(); + const report = reportSchema.parse(JSON.parse(result.stdout)); + expect(report.failure.name).toBe("Error"); + expect(report.failure.message).toContain(timeoutMessage); + expect(report.failure.message).toContain("authored setup pending"); + expect(report.configurationAtRemoval).toBe(unchangedConfiguration); + expect(report.liveAtRemoval).toEqual([]); + expect(report.rootRemoved).toBe(true); + } finally { + await releaseStandin(release, ready); + } + }, +); + +const interactiveStandinSource = ` +import assert from "node:assert/strict"; +import { writeFileSync } from "node:fs"; +assert.deepEqual(process.argv.slice(2), ["setup"]); +const tty = { stdin: Boolean(process.stdin.isTTY), stdout: Boolean(process.stdout.isTTY), stderr: Boolean(process.stderr.isTTY) }; +assert.deepEqual(tty, { stdin: true, stdout: true, stderr: true }); +process.stdin.setRawMode(true); +process.stdin.on("data", (data) => { + assert.deepEqual([...data], [3]); + process.stdin.setRawMode(false); + process.stdin.pause(); + writeFileSync(process.env.TERMINAL_RECEIPT, JSON.stringify({ mode: "interactive", pid: process.pid, tty, interrupt: [...data], rawRestored: process.stdin.isRaw === false, exitStatus: 0 })); + process.stdout.write("\\u001b[?25hNo changes were made.\\n", () => process.exit(0)); +}); +process.stdin.resume(); +process.stdout.write("Which agents should use REA?"); +`; + +const redirectedStandinSource = ` +import assert from "node:assert/strict"; +import { writeFileSync } from "node:fs"; +assert.deepEqual(process.argv.slice(2), ["setup"]); +const tty = { stdin: Boolean(process.stdin.isTTY), stdout: Boolean(process.stdout.isTTY), stderr: Boolean(process.stderr.isTTY) }; +assert.deepEqual(tty, { stdin: true, stdout: false, stderr: true }); +const plainOutput = "Authored plain setup output.\\n"; +writeFileSync(process.env.TERMINAL_RECEIPT, JSON.stringify({ mode: "redirected", pid: process.pid, tty, plainOutput, exitStatus: 1 })); +process.stdout.write(plainOutput); +process.exitCode = 1; +`; + +const positiveReceiptSchema = z.discriminatedUnion("mode", [ + z.strictObject({ + mode: z.literal("interactive"), + pid: z.number().int().positive(), + tty: z.strictObject({ + stdin: z.literal(true), + stdout: z.literal(true), + stderr: z.literal(true), + }), + interrupt: z.tuple([z.literal(3)]), + rawRestored: z.literal(true), + exitStatus: z.literal(0), + }), + z.strictObject({ + mode: z.literal("redirected"), + pid: z.number().int().positive(), + tty: z.strictObject({ + stdin: z.literal(true), + stdout: z.literal(false), + stderr: z.literal(true), + }), + plainOutput: z.literal("Authored plain setup output.\n"), + exitStatus: z.literal(1), + }), +]); + +processTest + .skipIf(process.platform === "win32") + .each(["interactive", "redirected"] as const)( + "preserves selected configuration and closes the real PTY for %s setup", + { timeout: 30_000 }, + async (mode) => { + const root = await createTestTempDirectory("rea-package-setup-success-"); + const scratch = join(root, "scratch"); + await mkdir(scratch); + const config = join(scratch, "selected-config.json"); + await writeFile(config, unchangedConfiguration); + const entry = join(root, "selected setup.mjs"); + const receiptPath = join(root, "terminal-receipt.json"); + await writeFile( + entry, + `#!${process.execPath}\n${mode === "interactive" ? interactiveStandinSource : redirectedStandinSource}`, + ); + await chmod(entry, 0o755); + let commands: PackageVerifierCommands | undefined; + let receipt: z.output | undefined; + let configurationAtRemoval: string | undefined; + let liveAtRemoval: readonly number[] | undefined; + const workspace = { + path: scratch, + async close() { + const pid = receipt?.pid; + liveAtRemoval = liveProcesses( + await systemProcessOwnershipHost.listProcesses(), + ) + .filter((entry) => entry.pid === pid) + .map(({ pid }) => pid); + configurationAtRemoval = await readFile(config, "utf8"); + await rm(scratch, { recursive: true, force: true }); + }, + }; + await withVerifierCleanup(async () => { + commands = await createPackageVerifierCommands({ workspace }); + const options = { + commands, + command: entry, + environment: { + ...process.env, + HOME: scratch, + TERMINAL_RECEIPT: receiptPath, + }, + root: process.cwd(), + unchangedPaths: [config], + }; + if (mode === "interactive") await verifyInteractiveSetup(options); + else await verifyRedirectedStdoutSetup(options); + receipt = positiveReceiptSchema.parse( + JSON.parse(await readFile(receiptPath, "utf8")), + ); + }, [() => (commands === undefined ? workspace.close() : commands.close())]); + + expect(receipt).toMatchObject({ mode }); + expect(configurationAtRemoval).toBe(unchangedConfiguration); + expect(liveAtRemoval).toEqual([]); + await expect(readFile(config)).rejects.toMatchObject({ code: "ENOENT" }); + }, +); diff --git a/tests/boundary/process/verifierTerminalOwner.test.ts b/tests/boundary/process/verifierTerminalOwner.test.ts new file mode 100644 index 0000000000..c031c902da --- /dev/null +++ b/tests/boundary/process/verifierTerminalOwner.test.ts @@ -0,0 +1,457 @@ +import { existsSync } from "node:fs"; +import { mkdir, rm } from "node:fs/promises"; +import { join } from "node:path"; + +import { expect, it, vi } from "vitest"; +import { z } from "zod"; + +import { + runOwnedVerifierTerminal, + type VerifierTerminalOptions, + type VerifierTerminalResult, +} from "../../../scripts/lib/owned-verifier-terminal.mjs"; +import { + createPackageVerifierCommands, + type PackageCleanupOwner, +} from "../../../scripts/lib/verify-package-commands.mjs"; +import { + cleanupOwnedProcessGroup, + type PersistentOwnedProcessGroup, + type ProcessCleanupFailure, +} from "../../../src/process/ProcessOwnership.js"; +import { systemProcessOwnershipHost } from "../../../src/process/ProcessOwnershipObservation.js"; +import { liveProcesses } from "../../../src/process/ProcessOwnershipProcessTree.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +type Outcome = + | { readonly ok: true; readonly value: Value } + | { readonly ok: false; readonly cause: unknown }; + +const observed = (promise: Promise): Promise> => + promise.then( + (value) => ({ ok: true, value }), + (cause: unknown) => ({ ok: false, cause }), + ); + +const deferred = () => { + let resolve!: () => void; + const promise = new Promise((finish) => { + resolve = finish; + }); + return { promise, resolve }; +}; + +const waitForBoundary = ( + boundary: Promise, + running: Promise>, +) => + Promise.race([ + boundary, + running.then((outcome) => { + throw new Error( + "Terminal settled before the authored lifecycle boundary", + { cause: outcome }, + ); + }), + ]); + +const terminalFixture = async () => { + const parent = await createTestTempDirectory("rea-verifier-terminal-"); + const path = join(parent, "workspace"); + await mkdir(path); + const pids = new Set(); + const censuses: number[][] = []; + const rootsAtRemoval: boolean[] = []; + let rootCloses = 0; + const commands = await createPackageVerifierCommands({ + workspace: { + path, + async close() { + rootCloses += 1; + rootsAtRemoval.push(existsSync(path)); + censuses.push( + liveProcesses(await systemProcessOwnershipHost.listProcesses()) + .filter(({ pid }) => pids.has(pid)) + .map(({ pid }) => pid), + ); + await rm(path, { recursive: true, force: true }); + }, + }, + }); + const adopted: PackageCleanupOwner[] = []; + const options: Omit = { + commands: { + adopt(owner, resource) { + adopted.push(owner); + commands.adopt(owner, resource); + }, + }, + command: process.execPath, + cwd: path, + environment: { + TERM: "xterm-256color", + REA_TERMINAL_SELECTED: "caller-selected", + }, + timeoutMs: 2_000, + }; + const cleanup: typeof cleanupOwnedProcessGroup = async (ownership, host) => { + pids.add(ownership.leaderPid); + return cleanupOwnedProcessGroup(ownership, host); + }; + return { + path, + parent, + commands, + adopted, + options, + cleanup, + pids, + censuses, + rootsAtRemoval, + rootCloses: () => rootCloses, + }; +}; + +const expectReleased = ( + fixture: Awaited>, +) => { + expect(existsSync(fixture.path)).toBe(false); + expect(fixture.rootCloses()).toBe(1); + expect(fixture.censuses).toEqual([[]]); + expect(fixture.rootsAtRemoval).toEqual([true]); +}; + +const observeClose = (owner: PackageCleanupOwner) => { + const entered = deferred(); + const original = owner.close; + const spy = vi.spyOn(owner, "close").mockImplementation(function ( + this: PackageCleanupOwner, + ) { + const closing = original.call(this); + entered.resolve(); + return closing; + }); + return { entered: entered.promise, restore: () => spy.mockRestore() }; +}; + +const posix = it.skipIf(process.platform === "win32"); + +posix( + "merges real PTY streams and preserves selected argv, environment and the write capability", + async () => { + const fixture = await terminalFixture(); + const ambient = "REA_TERMINAL_AMBIENT_SENTINEL"; + const previous = process.env[ambient]; + process.env[ambient] = "ambient-only"; + const args = ["value with spaces", "--literal=$HOME", "quote'and\"double"]; + const source = ` +process.stdin.setEncoding("utf8"); +process.stdin.once("data", input => process.stdout.write("__REPLY__" + JSON.stringify(input.trim()) + "\\n", () => process.exit(0))); +process.stdout.write("__FACTS__" + JSON.stringify({ args: process.argv.slice(1), selected: process.env.REA_TERMINAL_SELECTED, ambient: process.env.${ambient} ?? null }) + "\\n"); +process.stderr.write("__STDERR__\\n"); +process.stdout.write("__PROMPT__\\n"); +setTimeout(() => process.exit(91), 5000); +`; + let wrote = false; + let result: VerifierTerminalResult | undefined; + try { + result = await runOwnedVerifierTerminal( + { + ...fixture.options, + args: ["-e", source, "--", ...args], + onData(output, write) { + if (!wrote && output.includes("__PROMPT__")) { + wrote = true; + write("authored reply\n"); + } + }, + }, + { cleanupProcessGroup: fixture.cleanup }, + ); + } finally { + if (previous === undefined) delete process.env[ambient]; + else process.env[ambient] = previous; + await fixture.commands.close(); + } + const facts = result?.output + .split(/\r?\n/u) + .find((line) => line.startsWith("__FACTS__")); + + expect(result?.exitCode).toBe(0); + expect(wrote).toBe(true); + expect(JSON.parse(facts?.slice("__FACTS__".length) ?? "null")).toEqual({ + args, + selected: "caller-selected", + ambient: null, + }); + expect(result?.output).toContain("__STDERR__"); + expect(result?.output).toContain('__REPLY__"authored reply"'); + expect(fixture.adopted).toHaveLength(1); + expectReleased(fixture); + }, +); + +posix( + "cancels an acknowledged active terminal and joins it before workspace removal", + async () => { + const fixture = await terminalFixture(); + const ready = deferred(); + const source = + 'process.stdout.write("__PID__" + process.pid + "\\n"); setInterval(() => {}, 1000); setTimeout(() => process.exit(0), 10000);'; + const running = observed( + runOwnedVerifierTerminal( + { + ...fixture.options, + args: ["-e", source], + onData(output) { + const match = /__PID__(\d+)\r?\n/u.exec(output); + if (match !== null) { + fixture.pids.add(Number(match[1])); + ready.resolve(); + } + }, + }, + { cleanupProcessGroup: fixture.cleanup }, + ), + ); + let outcome: Outcome | undefined; + try { + await waitForBoundary(ready.promise, running); + await fixture.commands.close(); + outcome = await running; + } finally { + await fixture.commands.close(); + await running; + } + + expect(outcome).toEqual({ + ok: false, + cause: expect.objectContaining({ reason: "cancelled" }), + }); + expect(fixture.pids.size).toBe(1); + expect(fixture.adopted).toHaveLength(1); + expectReleased(fixture); + }, +); + +const heldSource = + 'process.stdout.write("callback-ready"); setInterval(() => {}, 1000); setTimeout(() => process.exit(0), 10000);'; + +posix.each(["timeout", "output_limit", "undefined_callback"] as const)( + "retains the exact terminal cleanup owner and primary %s failure through a denied release", + async (mode) => { + const fixture = await terminalFixture(); + let deny = true; + const ownerships: PersistentOwnedProcessGroup[] = []; + const denials: ProcessCleanupFailure[] = []; + const cleanup: typeof cleanupOwnedProcessGroup = async (ownership) => { + if (ownership.sweepTokenOwnedProcesses !== true) + throw new Error("Expected persistent token-swept terminal ownership"); + ownerships.push(ownership); + fixture.pids.add(ownership.leaderPid); + if (!deny) return fixture.cleanup(ownership); + const denial: ProcessCleanupFailure = { + cleaned: false, + reason: "controlled cleanup verification denial", + unverified: [ + { + pid: ownership.leaderPid, + diagnostic: "authored cleanup remains unverified", + }, + ], + }; + denials.push(denial); + return denial; + }; + let terminal: Outcome | undefined; + let rootClose: Outcome | undefined; + let retained; + try { + terminal = await observed( + runOwnedVerifierTerminal( + { + ...fixture.options, + timeoutMs: mode === "timeout" ? 250 : 2_000, + maxOutputBytes: mode === "output_limit" ? 5 : 1024, + args: [ + "-e", + mode === "output_limit" + ? 'process.stdout.write("é🙂Z"); setInterval(() => {}, 1000); setTimeout(() => process.exit(0), 10000);' + : heldSource, + ], + ...(mode === "undefined_callback" + ? { + onData() { + throw undefined; + }, + } + : {}), + }, + { cleanupProcessGroup: cleanup }, + ), + ); + rootClose = await observed(fixture.commands.close()); + retained = { + root: existsSync(fixture.path), + rootCloses: fixture.rootCloses(), + }; + } finally { + deny = false; + await fixture.commands.close(); + } + const failure = terminal?.ok === false ? terminal.cause : undefined; + const aggregate = z.object({ errors: z.array(z.unknown()) }).parse(failure); + const cleanupFailure = z + .object({ + cleanupOwner: z.unknown(), + cause: z.unknown(), + output: z.string(), + }) + .parse(aggregate.errors[1]); + const rootFailure = rootClose?.ok === false ? rootClose.cause : undefined; + + if (mode === "undefined_callback") { + expect(aggregate.errors[0]).toBeUndefined(); + expect(failure).toHaveProperty("cause", undefined); + } else { + expect(aggregate.errors[0]).toMatchObject({ reason: mode }); + if (mode === "output_limit") { + expect(aggregate.errors[0]).toMatchObject({ output: "é" }); + expect(cleanupFailure.output).toBe("é"); + } + } + expect(aggregate.errors).toHaveLength(2); + expect(fixture.adopted).toHaveLength(1); + expect(cleanupFailure.cleanupOwner).toBe(fixture.adopted[0]); + expect(cleanupFailure.cause).toBe(denials[0]); + expect(cleanupFailure.cause).toMatchObject({ + cleaned: false, + unverified: [{ diagnostic: "authored cleanup remains unverified" }], + }); + expect( + z.object({ cleanupOwner: z.unknown() }).parse(rootFailure).cleanupOwner, + ).toBe(fixture.commands); + expect(retained).toEqual({ root: true, rootCloses: 0 }); + expect(ownerships).toHaveLength(3); + for (const ownership of ownerships) expect(ownership).toBe(ownerships[0]); + expectReleased(fixture); + }, +); + +posix( + "selects cancellation while completed-child cleanup is pending and joins that same attempt", + async () => { + const fixture = await terminalFixture(); + const cleanupEntered = deferred(); + const allowCleanup = deferred(); + let attempts = 0; + const running = observed( + runOwnedVerifierTerminal( + { + ...fixture.options, + args: ["-e", 'process.stdout.write("completed child\\n");'], + }, + { + cleanupProcessGroup: async (ownership) => { + attempts += 1; + cleanupEntered.resolve(); + await allowCleanup.promise; + return fixture.cleanup(ownership); + }, + }, + ), + ); + let closeObservation: ReturnType | undefined; + let closing: Promise> | undefined; + let outcome: Outcome | undefined; + try { + await waitForBoundary(cleanupEntered.promise, running); + const owner = fixture.adopted[0]; + if (owner === undefined) throw new Error("Terminal was not adopted"); + closeObservation = observeClose(owner); + closing = observed(fixture.commands.close()); + await waitForBoundary(closeObservation.entered, closing); + allowCleanup.resolve(); + await closing; + outcome = await running; + } finally { + allowCleanup.resolve(); + await fixture.commands.close(); + await running; + closeObservation?.restore(); + } + + expect(outcome).toEqual({ + ok: false, + cause: expect.objectContaining({ reason: "cancelled" }), + }); + expect(attempts).toBe(1); + expect(fixture.adopted).toHaveLength(1); + expectReleased(fixture); + }, +); + +posix( + "joins a delayed native-module load without spawning after cancellation", + async () => { + const fixture = await terminalFixture(); + const loadEntered = deferred(); + const allowLoad = deferred(); + let spawns = 0; + let cleanups = 0; + const running = observed( + runOwnedVerifierTerminal( + { + ...fixture.options, + args: ["-e", 'process.stdout.write("must not spawn");'], + }, + { + loadPty: async () => { + const pty = await import("@lydell/node-pty"); + loadEntered.resolve(); + await allowLoad.promise; + return { + ...pty, + spawn(...args: Parameters) { + spawns += 1; + return pty.spawn(...args); + }, + }; + }, + cleanupProcessGroup: async (ownership) => { + cleanups += 1; + return fixture.cleanup(ownership); + }, + }, + ), + ); + let closeObservation: ReturnType | undefined; + let closing: Promise> | undefined; + let outcome: Outcome | undefined; + try { + await waitForBoundary(loadEntered.promise, running); + const owner = fixture.adopted[0]; + if (owner === undefined) throw new Error("Terminal was not adopted"); + closeObservation = observeClose(owner); + closing = observed(fixture.commands.close()); + await waitForBoundary(closeObservation.entered, closing); + allowLoad.resolve(); + await closing; + outcome = await running; + } finally { + allowLoad.resolve(); + await fixture.commands.close(); + await running; + closeObservation?.restore(); + } + + expect(outcome).toEqual({ + ok: false, + cause: expect.objectContaining({ reason: "cancelled" }), + }); + expect(spawns).toBe(0); + expect(cleanups).toBe(0); + expect(fixture.adopted).toHaveLength(1); + expectReleased(fixture); + }, +);