diff --git a/docs/testing.md b/docs/testing.md index 6a0b7984a9..334024cc1a 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -92,7 +92,7 @@ without serial scheduling. Pure domain/service, composition, conformance, evaluation, CLI boundary, and MCP boundary projects share module graphs. Their tests avoid process-global state and keep resource cleanup test-scoped. Each MCP session still owns its resources, and CLI cases retain fresh command -instances or subprocesses. CLI module mocks belong in `tests/process-global/` +instances or subprocesses. CLI and MCP module mocks belong in `tests/process-global/` so they keep per-file isolation. Product catalog verification stays with the isolated filesystem boundary tests because it loads both source and compiled module graphs. Adapter, other boundary, acceptance, and process-global projects diff --git a/src/domain/flutter/flutterBuildAnalysis.ts b/src/domain/flutter/flutterBuildAnalysis.ts index 37c3987b8f..d3a888919b 100644 --- a/src/domain/flutter/flutterBuildAnalysis.ts +++ b/src/domain/flutter/flutterBuildAnalysis.ts @@ -15,19 +15,25 @@ const ABI_SELECTOR_MAX_CHARACTERS = /** * ZIP labels are literal names, including backslashes and controls. They are - * never used as extraction paths. A Unicode regex gives runtime validation - * and JSON Schema the same character unit; Zod max() counts UTF-16 units. - * The final lookahead requires the actual end, including a trailing newline. + * never used as extraction paths. Count Unicode code points without copying + * the label; Zod max() counts UTF-16 units. Advertise the same code-point + * budget as JSON Schema maxLength, including every trailing line terminator. */ +const ABI_SELECTOR_MESSAGE = `ABI must be the exact lib/ directory label: 1–${String(ABI_SELECTOR_MAX_CHARACTERS)} Unicode characters without '/'.`; const abiSelectorSchema = z .string() - .regex( - new RegExp( - `^[^/]{1,${String(ABI_SELECTOR_MAX_CHARACTERS)}}(?![\\s\\S])`, - "u", - ), - `ABI must be the exact lib/ directory label: 1–${String(ABI_SELECTOR_MAX_CHARACTERS)} Unicode characters without '/'.`, - ); + .min(1, ABI_SELECTOR_MESSAGE) + .refine( + (label) => { + const characters = label[Symbol.iterator](); + for (let count = 0; count <= ABI_SELECTOR_MAX_CHARACTERS; count += 1) + if (characters.next().done) return true; + return false; + }, + { message: ABI_SELECTOR_MESSAGE, abort: true }, + ) + .regex(/^[^\/]*$/u, ABI_SELECTOR_MESSAGE) + .meta({ maxLength: ABI_SELECTOR_MAX_CHARACTERS }); /** * Caller intent for Flutter build identification. This family is pure diff --git a/src/process/capture/ProcessCaptureLifecycle.ts b/src/process/capture/ProcessCaptureLifecycle.ts index 155a8dfd8d..dd5c393b30 100644 --- a/src/process/capture/ProcessCaptureLifecycle.ts +++ b/src/process/capture/ProcessCaptureLifecycle.ts @@ -4,6 +4,7 @@ import { open, mkdtemp, rm, stat } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { setImmediate, setTimeout as delay } from "node:timers/promises"; +import { readFileHandleChunks } from "../../filesystem/readFileHandleChunks.js"; import type { IPty } from "@lydell/node-pty"; import { AnalysisCleanupLease, @@ -44,6 +45,7 @@ import { import { describeProcessCaptureExecutionFailure, normalizeCaptureFailure, + processCaptureCancelled, ProcessCaptureError, } from "./ProcessCaptureError.js"; import { assertNotCancelled } from "./ProcessScenarioRuntimeValidation.js"; @@ -563,13 +565,14 @@ export const observeSelectedExecutable = async ( "Selected executable path changed while it was being opened.", }; - // IO above can settle after cancellation. Do not construct a stream with an - // already-aborted signal: Node can emit a second, unhandled AbortError. + // IO above can settle after cancellation. Native FileHandle streams can + // invalidate the descriptor on abort even with autoClose:false; borrowed + // positional reads leave its close exclusively with the capture owner. assertNotCancelled(signal); const hash = createHash("sha256"); - for await (const chunk of handle.createReadStream({ - autoClose: false, + for await (const chunk of readFileHandleChunks(handle, { signal, + cancellationFailure: processCaptureCancelled, })) { assertNotCancelled(signal); hash.update(chunk); @@ -596,7 +599,7 @@ export const observeSelectedExecutable = async ( if ( cause instanceof ProcessCaptureReadFailure && !cause.cleanupIncomplete && - cause.userCategory !== "cancelled" && + cause.cancellation === undefined && cause.outcome.kind === "failed" ) { const reason = filesystemFailure(cause.outcome.cause); diff --git a/src/process/capture/ProcessCaptureRead.ts b/src/process/capture/ProcessCaptureRead.ts index 829b0ba925..9fbe1908e8 100644 --- a/src/process/capture/ProcessCaptureRead.ts +++ b/src/process/capture/ProcessCaptureRead.ts @@ -36,6 +36,7 @@ export class ProcessCaptureReadFailure< readonly cleanupLeases: readonly AnalysisCleanupLease[]; readonly cleanupCause: unknown; readonly retention: CloseRetention; + /** An observed abort is independent of whether cancellation caused the read failure. */ readonly cancellation: Cancellation | undefined; constructor(options: { @@ -49,13 +50,18 @@ export class ProcessCaptureReadFailure< }) { const cleanupLeases = options.cleanupLease === undefined ? [] : [options.cleanupLease]; - const cancellation = + const primaryCancellation = options.outcome.kind === "failed" ? operationCancellation(options.outcome.cause, options.signal) : options.completionCancellation; + const cancellation = + primaryCancellation ?? + (options.signal?.aborted === true + ? { reason: options.signal.reason } + : undefined); const primary = - cancellation !== undefined - ? cancellation.reason + primaryCancellation !== undefined + ? primaryCancellation.reason : options.outcome.kind === "failed" ? options.outcome.cause : options.cleanupCause; @@ -78,7 +84,7 @@ export class ProcessCaptureReadFailure< `Process capture read failed for ${options.path}${reasons.length === 0 ? "" : `: ${reasons.join("; ")}`}`, { cause: primary, - ...(cancellation === undefined + ...(primaryCancellation === undefined ? {} : { userCategory: "cancelled" as const, diff --git a/tests/boundary/filesystem/directAnalysisSnapshotReplay.test.ts b/tests/boundary/filesystem/directAnalysisSnapshotReplay.test.ts index c856c757f8..19bb92eb30 100644 --- a/tests/boundary/filesystem/directAnalysisSnapshotReplay.test.ts +++ b/tests/boundary/filesystem/directAnalysisSnapshotReplay.test.ts @@ -118,7 +118,7 @@ const expectWorkflowQuestionsRetained = ( expect.arrayContaining( result.residual_unknowns.map((question) => expect.objectContaining({ - question, + question: `${question} (Evidence ${evidence.evidence_id})`, supporting_evidence_ids: [evidence.evidence_id], }), ), diff --git a/tests/boundary/filesystem/zipDetection.test.ts b/tests/boundary/filesystem/zipDetection.test.ts index b403c59755..f7c5d6f1bd 100644 --- a/tests/boundary/filesystem/zipDetection.test.ts +++ b/tests/boundary/filesystem/zipDetection.test.ts @@ -16,6 +16,7 @@ import { import { parseBinaryTarget } from "../../../src/application/BinaryTargetResolver.js"; import { artifactOccurrenceAt } from "../../fixtures/artifactEntryOrder.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { asarHeaderBytes } from "../../fixtures/asarHeader.js"; describe("ZIP signature detection", () => { it.each([ @@ -65,30 +66,8 @@ describe("ZIP signature detection", () => { const textPkg = join(root, "notes.pkg"); const dmg = join(root, "image.dmg"); const textDmg = join(root, "notes.dmg"); - const headerSize = 20; - await writeFile( - asar, - Buffer.from([ - 4, - 0, - 0, - 0, - headerSize, - 0, - 0, - 0, - headerSize - 4, - 0, - 0, - 0, - 2, - 0, - 0, - 0, - 0x7b, - 0x7d, - ]), - ); + const asarBytes = asarHeaderBytes(JSON.stringify({ files: {} })); + await writeFile(asar, asarBytes); await writeFile(textAsar, Buffer.from("not an asar")); await writeFile(pkg, Buffer.from("xar!")); await writeFile(textPkg, Buffer.from("not a pkg")); @@ -103,6 +82,11 @@ describe("ZIP signature detection", () => { (await classifyAndHashRoot(textAsar, false, await lstat(textAsar))) .format, ).toBe("file"); + // A valid prefix cannot admit extents outside the observed container. + await writeFile(asar, asarBytes.subarray(0, -1)); + expect( + (await classifyAndHashRoot(asar, false, await lstat(asar))).format, + ).toBe("file"); expect( (await classifyAndHashRoot(pkg, false, await lstat(pkg))).format, ).toBe("pkg"); diff --git a/tests/boundary/mcp/flutterMcp.test.ts b/tests/boundary/mcp/flutterMcp.test.ts index 03e879f0e9..f4e50531b7 100644 --- a/tests/boundary/mcp/flutterMcp.test.ts +++ b/tests/boundary/mcp/flutterMcp.test.ts @@ -219,7 +219,14 @@ it("advertises and enforces the same Unicode selector grammar through MCP", asyn if (tool === undefined) throw new Error("Missing AOT tool"); const ajv = new Ajv2020({ strict: false, validateFormats: false }); expect(ajv.validateSchema(tool.inputSchema)).toBe(true); - for (const abi of ["😀".repeat(1010), "l".repeat(1010)]) { + for (const abi of [ + "😀".repeat(1010), + "l".repeat(1010), + "😀".repeat(1009) + "\n", + ...["\n", "\r", "\u2028", "\u2029"].map( + (terminator) => "l".repeat(1009) + terminator, + ), + ]) { const arguments_ = { path: "/fixture.apk", abi }; expect(ajv.validate(tool.inputSchema, arguments_)).toBe(true); expect( @@ -235,7 +242,9 @@ it("advertises and enforces the same Unicode selector grammar through MCP", asyn "", "lib/x86_64", "😀".repeat(1011), - "l".repeat(1010) + "\n", + ...["\n", "\r", "\u2028", "\u2029"].map( + (terminator) => "l".repeat(1010) + terminator, + ), ]) { const arguments_ = { path: "/fixture.apk", abi }; expect(ajv.validate(tool.inputSchema, arguments_)).toBe(false); diff --git a/tests/boundary/process/processCaptureLifecycle.test.ts b/tests/boundary/process/processCaptureLifecycle.test.ts index 29f39e7468..b865b7225c 100644 --- a/tests/boundary/process/processCaptureLifecycle.test.ts +++ b/tests/boundary/process/processCaptureLifecycle.test.ts @@ -176,14 +176,11 @@ itWithCaptureCapability( itWithCaptureCapability( "keeps the process deadline bounded when the wall clock moves backward", async () => { - const wallStart = Date.now(); - let wallReads = 0; - const now = vi.spyOn(Date, "now").mockImplementation(() => { - wallReads += 1; - return wallReads === 1 ? wallStart : wallStart - 60_000 + wallReads; - }); + let wallNow = Date.now(); + const now = vi.spyOn(Date, "now").mockImplementation(() => wallNow); const controller = new AbortController(); - const fallback = setTimeout(() => controller.abort(), 1_500); + let fallback: ReturnType | undefined; + let reachedRunning = false; try { const result = await captureProcessScenario( parseProcessScenario({ @@ -194,10 +191,27 @@ itWithCaptureCapability( idle_timeout_ms: 5_000, }), controller.signal, + process.platform, + process.env, + undefined, + undefined, + { + async report(update) { + if (update.phase === "running" && !reachedRunning) { + reachedRunning = true; + wallNow -= 60_000; + fallback = setTimeout(() => controller.abort(), 1_500); + } else if ( + update.phase === "settling" || + update.phase === "cleanup" + ) + clearTimeout(fallback); + }, + }, ); - expect(result.ok).toBe(true); - if (!result.ok) throw result.error; - expect(result.value.exit.reason).toBe("timeout"); + expect(reachedRunning).toBe(true); + const { capture } = captureObservations(result); + expect(capture.exit.reason).toBe("timeout"); } finally { clearTimeout(fallback); now.mockRestore(); @@ -584,7 +598,7 @@ itWithCaptureCapability( } const controller = new AbortController(); - setTimeout(() => controller.abort(), 50); + let reachedRunning = false; const cancelled = await captureProcessScenario( parseProcessScenario({ executable: process.execPath, @@ -594,9 +608,43 @@ itWithCaptureCapability( idle_timeout_ms: 5_000, }), controller.signal, + process.platform, + process.env, + undefined, + undefined, + { + async report(update) { + if (update.phase === "running") { + reachedRunning = true; + controller.abort(); + } + }, + }, ); if (cancelled.ok) throw new Error("expected cancellation"); - expect(cancelled.error.message).toContain("cancelled"); + expect(reachedRunning).toBe(true); + if (!(cancelled.error instanceof ProcessCaptureError)) + throw cancelled.error; + if (cancelled.error.cleanupIncomplete) + expectUnverifiedHostCleanup(cancelled.error); + else + expect(cancelled.error.cleanupReport).toMatchObject({ + owned_process_group: { state: "cleaned" }, + terminal_renderer: { state: "cleaned" }, + temporary_root: { state: "cleaned" }, + }); + const cancellation = cancelled.error.cleanupIncomplete + ? cancelled.error.cause + : cancelled.error; + expect(cancellation, cancelled.error.message).toMatchObject({ + reason: "cancelled", + userCategory: "cancelled", + }); + expect(cancelled.error.partialObservation).toMatchObject({ + observations: { + exit: { state: "available", value: { reason: "cancelled" } }, + }, + }); }, ); diff --git a/tests/boundary/process/processCaptureProgress.test.ts b/tests/boundary/process/processCaptureProgress.test.ts index c30d3cce60..ce50a9f6de 100644 --- a/tests/boundary/process/processCaptureProgress.test.ts +++ b/tests/boundary/process/processCaptureProgress.test.ts @@ -206,6 +206,7 @@ const connectProgressClient = async ( ): Promise<{ readonly client: Client; readonly notifications: string[]; + readonly cleanupProgress: Promise; }> => { const session = createTestBinarySession(() => { throw new Error("Process capture must not launch a binary provider"); @@ -219,9 +220,14 @@ const connectProgressClient = async ( version: "1", }); const notifications: string[] = []; + let sawCleanup: (() => void) | undefined; + const cleanupProgress = new Promise((resolve) => { + sawCleanup = resolve; + }); if (recordNotifications) { client.setNotificationHandler("notifications/progress", (notification) => { notifications.push(JSON.stringify(notification)); + if (notification.params.message?.startsWith("cleanup:")) sawCleanup?.(); }); } const [clientTransport, serverTransport] = @@ -231,7 +237,7 @@ const connectProgressClient = async ( onTestFinished(async () => { await Promise.allSettled([client.close(), server.close(), session.close()]); }); - return { client, notifications }; + return { client, notifications, cleanupProgress }; }; itWithCaptureCapability( @@ -283,7 +289,7 @@ itWithCaptureCapability( ); itWithCaptureCapability( - "reports unknown resource cleanup when temporary-root preparation fails", + "reports no acquired resources when temporary-root creation fails", async () => { const scenario = await writeScenario( "rea-process-progress-prepare-failed-", @@ -311,9 +317,9 @@ itWithCaptureCapability( terminal: true, details: { disposition: "failed", - owned_process_group: "unverified", - terminal_renderer: "unverified", - temporary_root: "unverified", + owned_process_group: "not_required", + terminal_renderer: "not_required", + temporary_root: "not_required", }, }); }, @@ -590,9 +596,13 @@ itWithCaptureCapability( controller.abort(); await completion; } - await expect - .poll(() => cancelling.notifications.join("\n")) - .toContain("disposition=cancelled"); + // Client cancellation ends its wait before the server has released its + // process tree. Wait for the actual terminal notification under the test's + // existing deadline instead of giving cleanup a separate polling deadline. + await cancelling.cleanupProgress; + expect(cancelling.notifications.join("\n")).toContain( + "disposition=cancelled", + ); expect(cancelling.notifications.join("\n")).toContain( "owned_process_group=cleaned", ); diff --git a/tests/boundary/process/processSnapshotHandleOwnership.test.ts b/tests/boundary/process/processSnapshotHandleOwnership.test.ts index fb5b493688..69571b7d1b 100644 --- a/tests/boundary/process/processSnapshotHandleOwnership.test.ts +++ b/tests/boundary/process/processSnapshotHandleOwnership.test.ts @@ -704,6 +704,9 @@ it("preserves an IO failure that arrives with cancellation before descriptor cle cleanupIncomplete: false, }); expect((failure as ProcessCaptureReadFailure).userCategory).toBeUndefined(); + expect((failure as ProcessCaptureReadFailure).cancellation?.reason).toBe( + reason, + ); expect(injected.fileHandle?.fd).toBe(-1); }); diff --git a/tests/boundary/process/selectedExecutableCleanup.test.ts b/tests/boundary/process/selectedExecutableCleanup.test.ts index 080237832a..6bc6d173bc 100644 --- a/tests/boundary/process/selectedExecutableCleanup.test.ts +++ b/tests/boundary/process/selectedExecutableCleanup.test.ts @@ -96,6 +96,67 @@ afterEach(async () => { await Promise.all( roots.splice(0).map((root) => rm(root, { recursive: true, force: true })), ); + vi.restoreAllMocks(); +}); + +it("cancels during executable bytes without transferring descriptor cleanup or blocking the scope", async () => { + const bytes = "executable bytes\n".repeat(16_384); + const path = await createExecutable(bytes); + const controller = new AbortController(); + const reason = new Error("cancel after the first physical executable read"); + injected.path = path; + const scope = new ProcessCaptureResourceScope(); + const fileSystem = { + open: async (selected: string, flags: number) => { + const handle = await open(selected, flags); + const read = handle.read.bind(handle); + const readAndCancel = async ( + buffer: Buffer, + offset: number, + length: number, + position: number | bigint | null, + ) => { + const result = await read(buffer, offset, length, position); + controller.abort(reason); + return result; + }; + vi.spyOn(handle, "read").mockImplementation( + readAndCancel as typeof handle.read, + ); + // Exercise the same physical-read interleaving in the old native-stream + // implementation: abort after bytes arrive, while its iterator owns data. + const stream = handle.createReadStream.bind(handle); + vi.spyOn(handle, "createReadStream").mockImplementation((options) => { + const source = stream(options); + source.once("data", () => controller.abort(reason)); + return source; + }); + return handle; + }, + stat: (selected: string) => stat(selected, { bigint: true }), + }; + + const failure = await caughtFailure(() => + scope.run(() => + observeSelectedExecutable(path, controller.signal, fileSystem, scope), + ), + ); + expect(controller.signal.aborted).toBe(true); + expect(failure.cause).toBe(reason); + expect(failure.userCategory).toBe("cancelled"); + expect(failure.cleanupIncomplete).toBe(false); + expect(failure.cleanupLeases).toEqual([]); + expect(injected.closed).toEqual([path]); + expect(injected.handles[0]?.isOpen()).toBe(false); + + await expect( + scope.run(() => + observeSelectedExecutable(path, undefined, undefined, scope), + ), + ).resolves.toMatchObject({ + sha256: createHash("sha256").update(bytes).digest("hex"), + }); + await scope.close(); }); it("retains the completed executable observation and descriptor owner after close failure", async () => { @@ -177,6 +238,80 @@ it("preserves selected executable IO failure when descriptor close also fails", await scope.close(); }); +it.each([ + [false, false], + [false, true], + [true, false], + [true, true], +])( + "retains pending read failure and cancellation (system error: %s, close denied: %s)", + async (systemError, closeDenied) => { + const path = await createExecutable("selected executable read failure"); + const primary = new Error("physical executable read rejected"); + if (systemError) Object.assign(primary, { code: "EIO", syscall: "read" }); + const reason = { request: "cancel while the descriptor read is pending" }; + const controller = new AbortController(); + let enter!: () => void; + let release!: () => void; + const entered = new Promise((resolve) => { + enter = resolve; + }); + const pending = new Promise((resolve) => { + release = resolve; + }); + injected.path = path; + injected.closeFailure = closeDenied + ? new Error("descriptor close denied after failed read") + : undefined; + const scope = new ProcessCaptureResourceScope(); + const fileSystem = { + open: async (selected: string, flags: number) => { + const handle = await open(selected, flags); + handle.read = async () => { + enter(); + await pending; + throw primary; + }; + return handle; + }, + stat: (selected: string) => stat(selected, { bigint: true }), + }; + + const reading = caughtFailure(() => + scope.run(() => + observeSelectedExecutable(path, controller.signal, fileSystem, scope), + ), + ); + await entered; + controller.abort(reason); + release(); + const failure = await reading; + expect(failure.cause).toBe(primary); + expect(failure.outcome).toMatchObject({ kind: "failed", cause: primary }); + expect(failure.cancellation?.reason).toBe(reason); + expect(failure.userCategory).toBeUndefined(); + expect(failure.cleanupIncomplete).toBe(closeDenied); + expect(failure.cleanupCause).toBe(injected.closeFailure); + expect(failure.cleanupLeases).toHaveLength(closeDenied ? 1 : 0); + expect(injected.closed).toEqual([path]); + + injected.closeFailure = undefined; + await scope.retryCleanup(); + expect(failure.cleanupIncomplete).toBe(false); + expect(injected.handles[0]?.isOpen()).toBe(false); + await expect( + scope.run(() => + observeSelectedExecutable(path, undefined, undefined, scope), + ), + ).resolves.toMatchObject({ + sha256: createHash("sha256") + .update("selected executable read failure") + .digest("hex"), + }); + await scope.close(); + }, +); + it("keeps the exact cancellation reason when cancellation arrives during descriptor close", async () => { const path = await createExecutable( "selected executable cancelled during close", diff --git a/tests/process-global/filesystemSnapshotFallbackCancellation.test.ts b/tests/process-global/filesystemSnapshotFallbackCancellation.test.ts index 49e30b90a2..8f13b7af2a 100644 --- a/tests/process-global/filesystemSnapshotFallbackCancellation.test.ts +++ b/tests/process-global/filesystemSnapshotFallbackCancellation.test.ts @@ -58,7 +58,9 @@ import { parseProcessScenario } from "../../src/domain/process/processScenario.j import { hashFile, snapshotRoots, + ProcessCaptureSnapshotFailure, } from "../../src/process/capture/FilesystemSnapshot.js"; +import { ProcessCaptureReadFailure } from "../../src/process/capture/ProcessCaptureRead.js"; import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; afterEach(() => { @@ -99,7 +101,29 @@ it.each(["hash", "snapshot"] as const)( }), controller.signal, ); - await expect(result).rejects.toBe(reason); + const failure = await result.catch((cause: unknown) => cause); + if (operation === "hash") { + expect(failure).toBeInstanceOf(ProcessCaptureReadFailure); + const read = failure as ProcessCaptureReadFailure; + expect(read.cause).toBe(reason); + expect(read.userCategory).toBe("cancelled"); + expect(read.outcome).toMatchObject({ kind: "failed", cause: reason }); + expect(read.cleanupLeases).toEqual([]); + expect(read.cleanupIncomplete).toBe(false); + } else { + expect(failure).toBeInstanceOf(ProcessCaptureSnapshotFailure); + const snapshot = failure as ProcessCaptureSnapshotFailure; + expect(snapshot.cause).toBe(reason); + expect(snapshot.userCategory).toBe("cancelled"); + expect(snapshot.snapshot).toMatchObject({ + files: [], + completeRoots: [], + truncated: true, + coverage: { enumeration_truncated: true }, + }); + expect(snapshot.cleanupLeases).toEqual([]); + expect(snapshot.cleanupIncomplete).toBe(false); + } expect(controller.signal.aborted).toBe(true); expect(observation.handles).toHaveLength(1); await expect(observation.handles[0]?.stat()).rejects.toMatchObject({ diff --git a/tests/process-global/filesystemSnapshotRaces.test.ts b/tests/process-global/filesystemSnapshotRaces.test.ts index 7c3df89247..b684cf371d 100644 --- a/tests/process-global/filesystemSnapshotRaces.test.ts +++ b/tests/process-global/filesystemSnapshotRaces.test.ts @@ -17,7 +17,6 @@ const race = vi.hoisted(() => ({ | "cancel_directory_close" | "", target: "", - followup: false, opened: false, closeCompleted: false, controller: undefined as AbortController | undefined, @@ -61,14 +60,12 @@ vi.mock("node:fs/promises", async (importOriginal) => { const target = await original.readlink(...args); if (path === race.target) { if (race.mode === "lstat_missing") { - race.followup = true; await original.rm(path); } else if (race.mode === "lstat_identity_changed") { const replacement = `${path}.replacement`; await original.symlink("replacement-target", replacement); await original.rm(path); await original.rename(replacement, path); - race.followup = true; } else if (race.mode === "cancel_readlink") { race.controller?.abort(race.reason); } @@ -77,15 +74,6 @@ vi.mock("node:fs/promises", async (importOriginal) => { }, lstat: async (...args: Parameters) => { const path = args[0]; - if ( - path === race.target && - (race.mode === "lstat_missing" || - race.mode === "lstat_identity_changed") && - race.followup - ) { - race.followup = false; - return original.lstat(...args); - } if ( path === race.target && race.mode === "directory_missing" && @@ -103,44 +91,11 @@ vi.mock("node:fs/promises", async (importOriginal) => { if (args[0] === race.target && race.mode === "directory_missing") race.opened = true; if (args[0] === race.target && race.mode === "cancel_directory_close") { - const iterator = directory[Symbol.asyncIterator](); - let closed = false; - const closeBeforeCancel = async () => { - if (closed) return; - closed = true; - try { - await directory.close(); - } catch (cause: unknown) { - if ( - !(cause instanceof Error) || - !("code" in cause) || - cause.code !== "ERR_DIR_CLOSED" - ) - throw cause; - } + const close = directory.close.bind(directory); + vi.spyOn(directory, "close").mockImplementation(async () => { + await close(); race.closeCompleted = true; race.controller?.abort(race.reason); - }; - const wrappedIterator = { - [Symbol.asyncIterator]() { - return this; - }, - async next() { - const next = await iterator.next(); - if (next.done) await closeBeforeCancel(); - return next; - }, - async return() { - const result = await iterator.return?.(); - await closeBeforeCancel(); - return result ?? { done: true, value: undefined }; - }, - }; - return new Proxy(directory, { - get(target, property, receiver) { - if (property === Symbol.asyncIterator) return () => wrappedIterator; - return Reflect.get(target, property, receiver); - }, }); } // The selected directory's next lstat is snapshotRoots' post-enumeration @@ -150,22 +105,54 @@ vi.mock("node:fs/promises", async (importOriginal) => { }; }); -import { parseProcessScenario } from "../../src/domain/process/processScenario.js"; +import { + parseProcessScenario, + type ProcessScenario, +} from "../../src/domain/process/processScenario.js"; import { classifyFilesystemEffects } from "../../src/process/capture/ProcessFilesystemEffects.js"; -import { snapshotRoots } from "../../src/process/capture/FilesystemSnapshot.js"; +import { + snapshotRoots, + ProcessCaptureSnapshotFailure, +} from "../../src/process/capture/FilesystemSnapshot.js"; import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; const resetRace = () => { race.mode = ""; race.target = ""; - race.followup = false; race.opened = false; race.closeCompleted = false; race.controller = undefined; race.reason = undefined; }; -afterEach(resetRace); +afterEach(() => { + resetRace(); + vi.restoreAllMocks(); +}); + +const expectCancelledSnapshot = async ( + scenario: ProcessScenario, + controller: AbortController, +): Promise => { + const cause = await snapshotRoots(scenario, controller.signal).catch( + (failure: unknown) => failure, + ); + expect(controller.signal.aborted).toBe(true); + expect(cause).toBeInstanceOf(ProcessCaptureSnapshotFailure); + const failure = cause as ProcessCaptureSnapshotFailure; + expect(failure.cause).toBe(controller.signal.reason); + expect(failure.userCategory).toBe("cancelled"); + expect(failure.snapshot).toMatchObject({ + truncated: true, + completeRoots: [], + coverage: { enumeration_truncated: true }, + }); + expect(failure.snapshot?.files).toContainEqual( + expect.objectContaining({ path: "root_0:.", type: "directory" }), + ); + expect(failure.cleanupLeases).toEqual([]); + expect(failure.cleanupIncomplete).toBe(false); +}; const withTemporaryRoot = async ( prefix: string, @@ -335,9 +322,7 @@ it.skipIf(process.platform === "win32")( race.target = link; race.controller = controller; race.reason = reason; - await expect(snapshotRoots(scenario, controller.signal)).rejects.toBe( - reason, - ); + await expectCancelledSnapshot(scenario, controller); }), ); @@ -358,13 +343,11 @@ it.skipIf(process.platform === "win32")( race.target = link; race.controller = controller; race.reason = reason; - await expect(snapshotRoots(scenario, controller.signal)).rejects.toBe( - reason, - ); + await expectCancelledSnapshot(scenario, controller); }), ); -it("observes cancellation after the selected directory iterator closes", async () => +it("observes cancellation after the selected directory handle closes", async () => withTemporaryRoot("rea-snapshot-directory-close-cancel-", async (root) => { const directory = join(root, "selected"); await mkdir(directory); @@ -375,14 +358,12 @@ it("observes cancellation after the selected directory iterator closes", async ( filesystem_observation_paths: [directory], }); const controller = new AbortController(); - const reason = new Error("cancel after directory iterator close"); + const reason = new Error("cancel after directory handle close"); race.mode = "cancel_directory_close"; race.target = directory; race.controller = controller; race.reason = reason; - await expect(snapshotRoots(scenario, controller.signal)).rejects.toBe( - reason, - ); + await expectCancelledSnapshot(scenario, controller); expect(race.closeCompleted).toBe(true); })); @@ -426,8 +407,17 @@ it.skipIf(process.platform === "win32")( }); race.mode = "readlink_eio"; race.target = link; - await expect(snapshotRoots(scenario)).rejects.toMatchObject({ - code: "EIO", + const cause = await snapshotRoots(scenario).catch( + (failure: unknown) => failure, + ); + expect(cause).toBeInstanceOf(ProcessCaptureSnapshotFailure); + const failure = cause as ProcessCaptureSnapshotFailure; + expect(failure.cause).toMatchObject({ code: "EIO" }); + expect(failure.userCategory).toBeUndefined(); + expect(failure.snapshot).toMatchObject({ + truncated: true, + completeRoots: [], + coverage: { enumeration_omissions: [] }, }); }), ); diff --git a/tests/boundary/cli/javascriptRuntimeObservationCli.test.ts b/tests/process-global/javascriptRuntimeObservationCli.test.ts similarity index 64% rename from tests/boundary/cli/javascriptRuntimeObservationCli.test.ts rename to tests/process-global/javascriptRuntimeObservationCli.test.ts index ae2c29c8cb..531d6e1c90 100644 --- a/tests/boundary/cli/javascriptRuntimeObservationCli.test.ts +++ b/tests/process-global/javascriptRuntimeObservationCli.test.ts @@ -1,28 +1,27 @@ -import { execFile } from "node:child_process"; -import { rm, writeFile } from "node:fs/promises"; +import { writeFile } from "node:fs/promises"; import { join } from "node:path"; import { pathToFileURL } from "node:url"; -import { promisify } from "node:util"; import { Cli } from "incur"; import { afterEach, describe, expect, test, vi } from "vitest"; -import { observeJavaScriptRuntime } from "../../../src/application/javascript/JavaScriptRuntimeObservationService.js"; -import type { JavaScriptRuntimeObservationPort } from "../../../src/application/javascript/JavaScriptRuntimeObservationPort.js"; -import { V8InspectorProvider } from "../../../src/inspector/V8InspectorProvider.js"; -import { observeJavaScriptRuntimeInputSchema } from "../../../src/domain/javascript/javascriptRuntimeObservation.js"; -import { startFakeV8Inspector } from "../../fixtures/inspector/fakeV8Inspector.js"; -import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; -import { registerJavaScriptRuntimeObservationCommands } from "../../../src/cliJavaScriptRuntimeCommands.js"; -import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js"; -import { ok } from "../../../src/domain/result.js"; -import { silentLogger } from "../../../src/logger.js"; +import { observeJavaScriptRuntime } from "../../src/application/javascript/JavaScriptRuntimeObservationService.js"; +import type { JavaScriptRuntimeObservationPort } from "../../src/application/javascript/JavaScriptRuntimeObservationPort.js"; +import { V8InspectorProvider } from "../../src/inspector/V8InspectorProvider.js"; +import { observeJavaScriptRuntimeInputSchema } from "../../src/domain/javascript/javascriptRuntimeObservation.js"; +import { startFakeV8Inspector } from "../fixtures/inspector/fakeV8Inspector.js"; +import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; +import { registerJavaScriptRuntimeObservationCommands } from "../../src/cliJavaScriptRuntimeCommands.js"; +import { ProviderCleanupError } from "../../src/domain/providerCleanupError.js"; +import { ok } from "../../src/domain/result.js"; +import { silentLogger } from "../../src/logger.js"; +import { cliTest } from "../support/cli/cliFixture.js"; const runtimeFactory = vi.hoisted(() => ({ provider: undefined as JavaScriptRuntimeObservationPort | undefined, })); -vi.mock("../../../src/composition/javascriptRuntimeObservation.js", () => ({ +vi.mock("../../src/composition/javascriptRuntimeObservation.js", () => ({ createJavaScriptRuntimeObservationProvider: () => { if (runtimeFactory.provider === undefined) throw new Error("No runtime observation provider was configured"); @@ -30,28 +29,20 @@ vi.mock("../../../src/composition/javascriptRuntimeObservation.js", () => ({ }, })); -const execute = promisify(execFile); const TIMEOUT_MS = 20_000; describe("JavaScript runtime observation CLI parity", () => { const resources: Array<{ close(): Promise }> = []; - const temporary: string[] = []; afterEach(async () => { runtimeFactory.provider = undefined; await Promise.all(resources.splice(0).map((item) => item.close())); - await Promise.all( - temporary - .splice(0) - .map((path) => rm(path, { recursive: true, force: true })), - ); }); - test( + cliTest( "returns the same passive Inspector Evidence contracts", - async () => { + async ({ cli }) => { const root = await createTestTempDirectory("rea-v8-cli-"); - temporary.push(root); const entry = join(root, "entry.js"); await writeFile(entry, "export const value = 1;\n"); const inspector = await startFakeV8Inspector({ @@ -60,11 +51,16 @@ describe("JavaScript runtime observation CLI parity", () => { resources.push(inspector); const environment = { ...process.env }; - const listed = await runCli( - ["list-javascript-runtime-targets", inspector.endpoint, "--json"], + const listed = await cli.run({ + arguments: [ + "list-javascript-runtime-targets", + inspector.endpoint, + "--json", + ], environment, - ); - expect(listed).toMatchObject({ + }); + expect(listed.exitCode, listed.stderr).toBe(0); + expect(listed.json).toMatchObject({ operation: "list_javascript_runtime_targets", provider: { id: "rea-v8-inspector" }, normalized_result: { @@ -72,8 +68,8 @@ describe("JavaScript runtime observation CLI parity", () => { }, }); - const observed = await runCli( - [ + const observed = await cli.run({ + arguments: [ "observe-javascript-runtime", inspector.endpoint, inspector.targetId, @@ -84,8 +80,9 @@ describe("JavaScript runtime observation CLI parity", () => { "--json", ], environment, - ); - expect(observed).toMatchObject({ + }); + expect(observed.exitCode, observed.stderr).toBe(0); + expect(observed.json).toMatchObject({ operation: "observe_javascript_runtime", provider: { id: "rea-v8-inspector" }, normalized_result: { @@ -99,18 +96,23 @@ describe("JavaScript runtime observation CLI parity", () => { }, }, }); - const direct = await observeJavaScriptRuntime( - new V8InspectorProvider(), - observeJavaScriptRuntimeInputSchema.parse({ - inspector_endpoint: inspector.endpoint, - target_id: inspector.targetId, - runtime_kind: "node", - observation_ms: 10, - }), - ); - expect(direct.ok).toBe(true); - if (direct.ok) - expect(evidenceIdFrom(observed)).toBe(direct.value.evidence_id); + const provider = new V8InspectorProvider(); + try { + const direct = await observeJavaScriptRuntime( + provider, + observeJavaScriptRuntimeInputSchema.parse({ + inspector_endpoint: inspector.endpoint, + target_id: inspector.targetId, + runtime_kind: "node", + observation_ms: 10, + }), + ); + expect(direct.ok).toBe(true); + if (direct.ok) + expect(evidenceIdFrom(observed.json)).toBe(direct.value.evidence_id); + } finally { + await provider.close(); + } }, TIMEOUT_MS, ); @@ -172,22 +174,6 @@ describe("JavaScript runtime observation CLI parity", () => { }); }); -const runCli = async ( - arguments_: readonly string[], - environment: NodeJS.ProcessEnv, -): Promise => { - const { stdout } = await execute( - process.execPath, - ["scripts/rea.mjs", ...arguments_], - { - cwd: process.cwd(), - env: environment, - maxBuffer: 16 * 1_024 * 1_024, - }, - ); - return JSON.parse(stdout); -}; - const evidenceIdFrom = (value: unknown): string => { if (typeof value !== "object" || value === null) throw new TypeError("Missing CLI Evidence"); diff --git a/tests/process-global/processPrelaunchCancellation.test.ts b/tests/process-global/processPrelaunchCancellation.test.ts index 453a88cb4a..ede237e948 100644 --- a/tests/process-global/processPrelaunchCancellation.test.ts +++ b/tests/process-global/processPrelaunchCancellation.test.ts @@ -49,12 +49,17 @@ vi.mock("node:fs/promises", async (load) => { control.handle = handle; const nativeStat = handle.stat.bind(handle); const nativeClose = handle.close.bind(handle); - const nativeStream = handle.createReadStream.bind(handle); - handle.createReadStream = (options) => { - const stream = nativeStream(options); - stream.once("end", () => (control.sampled = true)); - return stream; - }; + const nativeRead = handle.read.bind(handle); + handle.read = (async ( + buffer: Buffer, + offset: number, + length: number, + position: number | bigint | null, + ) => { + const result = await nativeRead(buffer, offset, length, position); + if (result.bytesRead === 0) control.sampled = true; + return result; + }) as typeof handle.read; handle.stat = (async (...parameters: Parameters) => { const metadata = await nativeStat(...parameters); if (control.sampled && control.stage === "final-stat") @@ -125,7 +130,6 @@ it.each(["none", "before", "open", "final-stat", "close"] as const)( }, ); expect(control.launches).toBe(stage === "none" ? 1 : 0); - expect(result.ok).toBe(false); if (result.ok) throw new Error("Expected sentinel failure or cancellation"); if (stage === "none") expect(String(result.error.cause)).toContain("PTY launch sentinel"); diff --git a/tests/boundary/mcp/processSnapshotCleanupMcp.test.ts b/tests/process-global/processSnapshotCleanupMcp.test.ts similarity index 89% rename from tests/boundary/mcp/processSnapshotCleanupMcp.test.ts rename to tests/process-global/processSnapshotCleanupMcp.test.ts index 31959327d6..c277bdb381 100644 --- a/tests/boundary/mcp/processSnapshotCleanupMcp.test.ts +++ b/tests/process-global/processSnapshotCleanupMcp.test.ts @@ -10,7 +10,6 @@ const injected = vi.hoisted(() => ({ failClose: false, opens: 0, handle: undefined as Awaited> | undefined, - nativeClose: undefined as (() => Promise) | undefined, })); vi.mock("node:fs/promises", async (importOriginal) => { @@ -23,7 +22,6 @@ vi.mock("node:fs/promises", async (importOriginal) => { injected.opens += 1; injected.handle = handle; const nativeClose = handle.close.bind(handle); - injected.nativeClose = nativeClose; handle.close = async () => { if (injected.failClose) throw new Error("controlled file close failure"); @@ -34,14 +32,14 @@ vi.mock("node:fs/promises", async (importOriginal) => { }; }); -import { createTestBinarySession } from "../../fixtures/binarySession.js"; -import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; -import type { AnalysisClient } from "../../../src/application/AnalysisProvider.js"; -import { observed } from "../../fixtures/analysisExecution.js"; -import { createServer } from "../../../src/server/createServer.js"; -import { silentLogger } from "../../../src/logger.js"; -import { ToolResultDelivery } from "../../../src/server/toolResult.js"; -import { ok } from "../../../src/domain/result.js"; +import { createTestBinarySession } from "../fixtures/binarySession.js"; +import { parseMcpToolError } from "../fixtures/mcpToolError.js"; +import type { AnalysisClient } from "../../src/application/AnalysisProvider.js"; +import { observed } from "../fixtures/analysisExecution.js"; +import { createServer } from "../../src/server/createServer.js"; +import { silentLogger } from "../../src/logger.js"; +import { ToolResultDelivery } from "../../src/server/toolResult.js"; +import { ok } from "../../src/domain/result.js"; const connectSession = async () => { const session = createTestBinarySession( @@ -172,6 +170,5 @@ it("retains and retries the exact snapshot file handle through public MCP captur await rm(root, { recursive: true, force: true }); injected.path = ""; injected.handle = undefined; - injected.nativeClose = undefined; } }); diff --git a/tests/process-global/updateMaintenance.test.ts b/tests/process-global/updateMaintenance.test.ts index 11babb5273..c24eab96a0 100644 --- a/tests/process-global/updateMaintenance.test.ts +++ b/tests/process-global/updateMaintenance.test.ts @@ -16,6 +16,7 @@ import { systemUpdateHost, } from "../../src/application/UpdateRuntime.js"; import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; +import { cliTest } from "../support/cli/cliFixture.js"; const entryPoint = resolve("scripts/rea.mjs"); let home: string; @@ -381,21 +382,20 @@ describe("updated executable maintenance planning", () => { }); describe("update subprocess boundaries", () => { - it("exposes update as the sole CLI command", async () => { - const help = await runUpdateCommand( - [process.execPath, entryPoint, "--help"], - process.env, - ); - expect(help.ok).toBe(true); - if (help.ok) { - expect(help.value).toMatch(/^\s{2}update\s/mu); - expect(help.value).not.toMatch(/^\s{2}upgrade\s/mu); - } - const removed = await runUpdateCommand( - [process.execPath, entryPoint, "upgrade", "--json"], - process.env, - ); - expect(removed.ok).toBe(false); + cliTest("exposes update as the sole CLI command", async ({ cli }) => { + const help = await cli.run({ + arguments: ["--help"], + environment: process.env, + }); + expect(help.exitCode, help.stderr).toBe(0); + expect(help.stdout).toMatch(/^\s{2}update\s/mu); + expect(help.stdout).not.toMatch(/^\s{2}upgrade\s/mu); + const removed = await cli.run({ + arguments: ["upgrade", "--json"], + environment: process.env, + }); + expect(removed.exitCode, removed.stderr).toBe(1); + expect(removed.json).toMatchObject({ code: "COMMAND_NOT_FOUND" }); }); it("captures output through stream closure and keeps arguments literal", async () => { const literal = 'space and $(echo bad); & | "quotes"';