diff --git a/src/application/process/ProcessCli.test.ts b/src/application/process/ProcessCli.test.ts new file mode 100644 index 000000000..88ab756ad --- /dev/null +++ b/src/application/process/ProcessCli.test.ts @@ -0,0 +1,90 @@ +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { + captureProcessScenarioFile, + projectProcessCliError, +} from "./ProcessCli.js"; + +const roots: string[] = []; +afterEach(async () => { + await Promise.all( + roots.splice(0).map((root) => rm(root, { recursive: true, force: true })), + ); +}); +const fixture = async (): Promise => { + const root = await mkdtemp(join(tmpdir(), "rea-process-cli-diagnostics-")); + roots.push(root); + return root; +}; + +describe("process CLI input diagnostics", () => { + it("preserves local diagnostics for unexpected process failures", () => { + const projected = projectProcessCliError( + new Error("SECRET internal failure /private/path"), + ); + expect(projected).toEqual({ + error: "Process command failed", + category: "execution_failure", + message: + "Process command could not complete: SECRET internal failure /private/path", + }); + expect(JSON.stringify(projected)).not.toContain("stack"); + }); + + it("projects missing, malformed JSON, and invalid scenario files", async () => { + const root = await fixture(); + const malformed = join(root, "malformed.json"); + const invalid = join(root, "invalid.json"); + const invalidUtf8 = join(root, "invalid-utf8.json"); + await writeFile(malformed, "not-json"); + await writeFile(invalid, "{}"); + await writeFile( + invalidUtf8, + Buffer.concat([ + Buffer.from('{"executable":"'), + Buffer.from([0xff]), + Buffer.from('"}'), + ]), + ); + + expect( + await captureProcessScenarioFile(join(root, "missing.json")), + ).toEqual({ + error: "Process command failed", + category: "invalid_input", + message: expect.stringContaining( + `Process input file could not be read: ${join(root, "missing.json")} (ENOENT`, + ), + }); + expect(await captureProcessScenarioFile(malformed)).toEqual({ + error: "Process command failed", + category: "invalid_input", + message: expect.stringContaining( + `Process input file is not valid UTF-8 JSON: ${malformed}`, + ), + }); + expect(await captureProcessScenarioFile(invalidUtf8)).toEqual({ + error: "Process command failed", + category: "invalid_input", + message: expect.stringContaining( + `Process input file is not valid UTF-8 JSON: ${invalidUtf8}`, + ), + }); + expect(await captureProcessScenarioFile(invalid)).toEqual({ + error: "Process command failed", + code: "invalid_request", + category: "invalid_input", + message: "Analysis input is invalid. Check the arguments and try again.", + retryable: true, + remediation: { action: "Correct the listed arguments and retry." }, + details: expect.objectContaining({ + operation: "capture_process_scenario", + issues: expect.arrayContaining([ + expect.objectContaining({ path: ["executable"] }), + ]), + }), + }); + }); +}); diff --git a/src/application/process/ProcessCli.ts b/src/application/process/ProcessCli.ts index 45b16b953..eb0fc1651 100644 --- a/src/application/process/ProcessCli.ts +++ b/src/application/process/ProcessCli.ts @@ -175,10 +175,9 @@ const readJson = async (path: string): Promise => { try { bytes = await readFile(path); } catch (cause: unknown) { - void cause; throw new ProcessCliFailure( "invalid_input", - "Process input file could not be read. Check that the path exists and is readable.", + `Process input file could not be read: ${path} (${describeValidationFailure(cause)}). Check the reported filesystem constraint and retry.`, ); } try { @@ -189,10 +188,9 @@ const readJson = async (path: string): Promise => { const parsed: unknown = JSON.parse(text); return parsed; } catch (cause: unknown) { - void cause; throw new ProcessCliFailure( "invalid_input", - "Process input file is not valid JSON. Repair the file, then try again.", + `Process input file is not valid UTF-8 JSON: ${path} (${describeValidationFailure(cause)}). Repair the file, then try again.`, ); } }; @@ -211,7 +209,7 @@ const cliAnalysisError = (error: AnalysisError): ProcessCliErrorOutput => ({ ...projectAnalysisError(error), }); -/** Project any process CLI failure without exposing its cause. */ +/** Preserve typed failures and local diagnostics at the process CLI boundary. */ export const projectProcessCliError = ( cause: unknown, ): ProcessCliErrorOutput => { @@ -225,7 +223,6 @@ export const projectProcessCliError = ( return { error: "Process command failed", category: "execution_failure", - message: - "Process command could not complete. Check the input files and run `rea doctor`, then try again.", + message: `Process command could not complete: ${describeValidationFailure(cause)}`, }; }; diff --git a/src/domain/process/processCapture.ts b/src/domain/process/processCapture.ts index 24ef7f862..bbfbdfa0c 100644 --- a/src/domain/process/processCapture.ts +++ b/src/domain/process/processCapture.ts @@ -182,11 +182,6 @@ export interface UnverifiedProcessCapture { readonly selected_executable_sha256: string | null; /** Digest associated with the launch only when path metadata stayed stable across spawn. */ readonly executable_sha256: string | null; - /** - * Digest recorded by the original v3 format before selected and launch - * executable digests were distinguished. Kept only on migrated captures. - */ - readonly legacy_executable_sha256?: string | undefined; readonly executable_identity: { readonly state: "path_metadata_unchanged" | "unknown"; readonly reason: string | null; @@ -205,12 +200,8 @@ export interface UnverifiedProcessCapture { readonly settlement: VerifiedProcessSettlement; readonly process_samples: readonly ProcessSample[]; readonly filesystem_checkpoints: readonly FilesystemCheckpoint[]; - /** - * Global observation order across independently recorded collections. - * - * Absence is accepted for captures written before this journal existed. - */ - readonly event_journal?: readonly ProcessCaptureEventJournalEntry[]; + /** Global observation order across independently recorded collections. */ + readonly event_journal: readonly ProcessCaptureEventJournalEntry[]; readonly files_before: readonly FileState[]; readonly files_after: readonly FileState[]; readonly filesystem_effects: readonly FileEffect[]; @@ -401,10 +392,6 @@ const processCaptureShapeSchema = z.strictObject({ .string() .regex(/^[a-f0-9]{64}$/u) .nullable(), - legacy_executable_sha256: z - .string() - .regex(/^[a-f0-9]{64}$/u) - .optional(), executable_identity: z.strictObject({ state: z.enum(["path_metadata_unchanged", "unknown"]), reason: z.string().nullable(), @@ -479,15 +466,13 @@ const processCaptureShapeSchema = z.strictObject({ truncated: z.boolean(), }), ), - event_journal: z - .array( - z.object({ - capture_order: z.number().int().nonnegative(), - collection: z.enum(PROCESS_CAPTURE_EVENT_COLLECTIONS), - index: z.number().int().nonnegative(), - }), - ) - .default([]), + event_journal: z.array( + z.object({ + capture_order: z.number().int().nonnegative(), + collection: z.enum(PROCESS_CAPTURE_EVENT_COLLECTIONS), + index: z.number().int().nonnegative(), + }), + ), files_before: z.array(fileStateSchema), files_after: z.array(fileStateSchema), filesystem_effects: z.array(fileEffectSchema), @@ -681,7 +666,7 @@ export const processCaptureSchema = processCaptureShapeSchema }); }) .describe( - "The capture must preserve its canonical scenario, comparison, and normalization SHA-256 commitments; ordered capture timestamps and contiguous sequence numbers; before and final filesystem snapshots with truncation propagated; and exit-code consistency with deadline termination. When parsing older input without an event journal, REA supplies an empty journal; empty journals are valid. A non-empty journal must reference every captured observation exactly once with unique in-range references. These cross-field invariants are checked by REA after capture.", + "The capture must preserve its canonical scenario, comparison, and normalization SHA-256 commitments; ordered capture timestamps and contiguous sequence numbers; before and final filesystem snapshots with truncation propagated; and exit-code consistency with deadline termination. The event journal is required; empty journals are valid. A non-empty journal must reference every captured observation exactly once with unique in-range references. These cross-field invariants are checked by REA after capture.", ); export { parseProcessCapture } from "./processCaptureParsing.js"; diff --git a/src/domain/process/processCapture.validation.test.ts b/src/domain/process/processCapture.validation.test.ts index 558d4743c..c2482bb45 100644 --- a/src/domain/process/processCapture.validation.test.ts +++ b/src/domain/process/processCapture.validation.test.ts @@ -3,7 +3,6 @@ import { expect, it } from "vitest"; import { digestProcessCommitment, parseProcessCapture, - type UnverifiedProcessCapture, } from "./processCapture.js"; import { compareUnverifiedProcessCaptures as compareProcessCaptures, @@ -28,6 +27,7 @@ it("never considers truncated captures equivalent", () => { exit: { code: 0, signal: null, reason: "exited" as const }, process_samples: [], filesystem_checkpoints: emptyCapture().filesystem_checkpoints, + event_journal: [], files_before: [], files_after: [], filesystem_effects: [], @@ -92,10 +92,10 @@ it("rejects settlement and cleanup combinations that cannot occur", () => { ).toThrow("cleanup_outcome"); }); -it("accepts old captures without a journal and validates complete journals", () => { +it("requires an explicit journal and validates complete journals", () => { const capture = emptyCapture(); const { event_journal: _eventJournal, ...oldCapture } = capture; - expect(parseProcessCapture(oldCapture).event_journal).toEqual([]); + expect(() => parseProcessCapture(oldCapture)).toThrow("event_journal"); const eventJournal = [ { capture_order: 0, collection: "filesystem_checkpoints", index: 0 }, @@ -142,90 +142,25 @@ it("accepts old captures without a journal and validates complete journals", () } }); -it("migrates persisted v3 executable digests without claiming a selected digest", () => { +it("rejects old executable identity representations without migration", () => { const capture = emptyCapture(); const { - selected_executable_sha256: _selectedExecutableSha256, - executable_identity: _executableIdentity, - executable_sha256, - ...legacyManifest + selected_executable_sha256: _selected, + executable_identity: _identity, + ...oldManifest } = capture.manifest; - const legacyCapture = { - ...capture, - manifest: { ...legacyManifest, executable_sha256 }, - }; - - const migrated = parseProcessCapture(legacyCapture); - - expect(migrated.manifest).toMatchObject({ - selected_executable_sha256: null, - executable_sha256: null, - executable_identity: { - state: "unknown", - reason: expect.stringContaining("did not distinguish"), - }, - legacy_executable_sha256: executable_sha256, - scenario: capture.manifest.scenario, - full_scenario_sha256: capture.manifest.full_scenario_sha256, - }); - expect(compareProcessCaptures(migrated, migrated)).toMatchObject({ - status: "unchanged", - }); - const exported = JSON.stringify(migrated); - expect(parseProcessCapture(JSON.parse(exported))).toEqual(migrated); - expect(() => - parseProcessCapture({ - ...legacyCapture, - manifest: { - ...legacyCapture.manifest, - executable_sha256: "f".repeat(64), - }, - }), - ).toThrow("legacy_executable_sha256"); - - const nullDigestScenario = { - ...legacyCapture.manifest.scenario, - executable_sha256: null, - }; - const malformedLegacyBase = { - ...legacyCapture, - manifest: { - ...legacyCapture.manifest, - executable_sha256: null, - scenario: nullDigestScenario, - full_scenario_sha256: digestProcessCommitment(nullDigestScenario), - }, - }; - expect(() => parseProcessCapture(malformedLegacyBase)).toThrow(); - expect(() => - parseProcessCapture({ - ...malformedLegacyBase, - manifest: { - ...malformedLegacyBase.manifest, - executable_sha256: "not-a-digest", - }, - }), + parseProcessCapture({ ...capture, manifest: oldManifest }), ).toThrow(); - const { - executable_sha256: _missingLegacyDigest, - ...manifestWithoutLegacyDigest - } = malformedLegacyBase.manifest; - expect(() => - parseProcessCapture({ - ...malformedLegacyBase, - manifest: manifestWithoutLegacyDigest, - }), - ).toThrow(); - - const { executable_identity: _missingIdentity, ...incompleteModernManifest } = - capture.manifest; expect(() => parseProcessCapture({ ...capture, - manifest: incompleteModernManifest, + manifest: { + ...capture.manifest, + legacy_executable_sha256: capture.manifest.executable_sha256, + }, }), - ).toThrow(); + ).toThrow("legacy_executable_sha256"); }); it("requires compatible contracts and enforces capture age through a clock seam", () => { diff --git a/src/domain/process/processCaptureExample.ts b/src/domain/process/processCaptureExample.ts index d920dabdc..22df573ea 100644 --- a/src/domain/process/processCaptureExample.ts +++ b/src/domain/process/processCaptureExample.ts @@ -54,6 +54,7 @@ export const EMPTY_PROCESS_CAPTURE_EXAMPLE = { truncated: false, }, ], + event_journal: [], files_before: [], files_after: [], filesystem_effects: [], diff --git a/src/domain/process/processCaptureParsing.ts b/src/domain/process/processCaptureParsing.ts index 018abf966..dda070e59 100644 --- a/src/domain/process/processCaptureParsing.ts +++ b/src/domain/process/processCaptureParsing.ts @@ -2,7 +2,6 @@ import { processCaptureSchema, type UnverifiedProcessCapture, } from "./processCapture.js"; -import { z } from "zod"; declare class ProcessCaptureProof { private readonly verified: never; @@ -23,47 +22,9 @@ const attachProof = (capture: UnverifiedProcessCapture): ProcessCapture => { throw new TypeError("Process Capture proof ownership failed"); }; -const recordSchema = z.record(z.string(), z.unknown()); -const LEGACY_EXECUTABLE_IDENTITY_REASON = - "Legacy v3 capture did not distinguish selected and launch executable digests"; - -const migrateLegacyV3Capture = (input: unknown): unknown => { - const capture = recordSchema.safeParse(input); - if (!capture.success) return input; - const manifest = recordSchema.safeParse(capture.data.manifest); - if (!manifest.success) return input; - if ( - manifest.data.provider_version !== "3" || - "selected_executable_sha256" in manifest.data || - "executable_identity" in manifest.data || - "legacy_executable_sha256" in manifest.data - ) - return input; - - const { executable_sha256 } = manifest.data; - if ( - typeof executable_sha256 !== "string" || - !/^[a-f0-9]{64}$/u.test(executable_sha256) - ) - return input; - return { - ...capture.data, - manifest: { - ...manifest.data, - selected_executable_sha256: null, - executable_sha256: null, - executable_identity: { - state: "unknown", - reason: LEGACY_EXECUTABLE_IDENTITY_REASON, - }, - legacy_executable_sha256: executable_sha256, - }, - }; -}; - /** Parse unknown input and reject invalid commitments or semantics. */ export const parseProcessCapture = (input: unknown): ProcessCapture => { - const parsed = processCaptureSchema.safeParse(migrateLegacyV3Capture(input)); + const parsed = processCaptureSchema.safeParse(input); if (parsed.success) return attachProof(parsed.data); const issues = parsed.error.issues.flatMap((issue) => issue.code === "custom" diff --git a/src/domain/process/processCaptureValidation.ts b/src/domain/process/processCaptureValidation.ts index 041c4c122..9ab0f749c 100644 --- a/src/domain/process/processCaptureValidation.ts +++ b/src/domain/process/processCaptureValidation.ts @@ -26,17 +26,8 @@ const validateCommitments = ( require: RequireInvariant, ): void => { const { manifest } = capture; - if (manifest.legacy_executable_sha256 !== undefined) { - require(manifest.selected_executable_sha256 === null && - manifest.executable_sha256 === null && - manifest.executable_identity.state === "unknown" && - manifest.executable_identity.reason !== null && - manifest.scenario.executable_sha256 === - manifest.legacy_executable_sha256, "manifest.legacy_executable_sha256", "legacy executable digest must remain distinct from unknown selected and launch digests"); - } else { - require(manifest.scenario.executable_sha256 === - manifest.selected_executable_sha256, "manifest.selected_executable_sha256", "selected executable commitment does not match the scenario projection"); - } + require(manifest.scenario.executable_sha256 === + manifest.selected_executable_sha256, "manifest.selected_executable_sha256", "selected executable commitment does not match the scenario projection"); require(manifest.executable_identity.state === "path_metadata_unchanged" ? manifest.executable_sha256 === manifest.selected_executable_sha256 && manifest.executable_identity.reason === null @@ -82,7 +73,7 @@ const validateEventJournal = ( capture: UnverifiedProcessCapture, require: RequireInvariant, ): void => { - const journal = capture.event_journal ?? []; + const journal = capture.event_journal; if (journal.length === 0) return; const sizes = { frames: capture.frames.length, diff --git a/src/domain/process/processTraceComparison.test.ts b/src/domain/process/processTraceComparison.test.ts index 83dc4b7db..5e251935f 100644 --- a/src/domain/process/processTraceComparison.test.ts +++ b/src/domain/process/processTraceComparison.test.ts @@ -241,7 +241,7 @@ describe("process trace constraints and evidence sufficiency", () => { expect(() => parseProcessCapture({ ...complete, - event_journal: (complete.event_journal ?? []).slice(1), + event_journal: complete.event_journal.slice(1), }), ).toThrow("event_journal"); }); diff --git a/src/domain/process/processTraceEvaluation.ts b/src/domain/process/processTraceEvaluation.ts index 44c0c05cc..87cd2aae3 100644 --- a/src/domain/process/processTraceEvaluation.ts +++ b/src/domain/process/processTraceEvaluation.ts @@ -160,7 +160,7 @@ const collectRecords = ( sources: ReadonlySet, ): readonly TraceRecord[] => { const records: TraceRecord[] = []; - for (const location of capture.event_journal ?? []) { + for (const location of capture.event_journal) { const record = projectProcessObservation(capture, location); if (record !== null && sources.has(record.source)) records.push({ @@ -416,7 +416,7 @@ export const evaluateProcessTraceSide = ( capture.residual_unknowns.some(({ scope }) => relevantScopes.has(scope)) ) return { result: unknownSide(), diagnostic: null }; - if ((capture.event_journal ?? []).length === 0) + if (capture.event_journal.length === 0) return { result: unknownSide(), diagnostic: { diff --git a/src/domain/reconstructionVerification.test.ts b/src/domain/reconstructionVerification.test.ts index 2de3e7717..3122868fc 100644 --- a/src/domain/reconstructionVerification.test.ts +++ b/src/domain/reconstructionVerification.test.ts @@ -288,40 +288,20 @@ describe("reconstruction verification", () => { }); }); -describe("saved v3 reconstruction capture compatibility", () => { - it("verifies saved v3 process captures and rejects a malformed legacy digest", () => { - const legacy = legacyV3Capture(); - const left = source("1", "controlled-replay", "observed", legacy); - const right = source("2", "controlled-replay"); - const comparison = processComparison(left, right, processResult()); - const bundle = createEvidenceBundle([comparison, left, right]); - - expect( - verifyReconstruction(behavioralSpec(comparison), bundle), - ).toMatchObject({ - status: "pass", - summary: { total: 1, passed: 1, failed: 0, unknown: 0 }, - }); - - const malformedLegacy = { - ...legacy, - manifest: { ...legacy.manifest, executable_sha256: null }, - }; - const malformedLeft = source( - "3", +describe("reconstruction process capture input", () => { + it("rejects saved captures without explicit executable identity", () => { + const left = source( + "1", "controlled-replay", "observed", - malformedLegacy, - ); - const malformedComparison = processComparison( - malformedLeft, - right, - processResult(), + legacyV3Capture(), ); + const right = source("2", "controlled-replay"); + const comparison = processComparison(left, right, processResult()); expect(() => verifyReconstruction( - behavioralSpec(malformedComparison), - createEvidenceBundle([malformedComparison, malformedLeft, right]), + behavioralSpec(comparison), + createEvidenceBundle([comparison, left, right]), ), ).toThrow(); }); diff --git a/src/process/ProcessOwnership.ts b/src/process/ProcessOwnership.ts index ff883c46a..e0330b916 100644 --- a/src/process/ProcessOwnership.ts +++ b/src/process/ProcessOwnership.ts @@ -189,13 +189,9 @@ export const cleanupWindowsProcessTree = async ( const result = await host.terminateTree(rootPid); return { cleaned: true, signaled: result === "terminated" }; } catch (cause: unknown) { - // The reason string is a pinned validation contract; keep it stable and - // do not interpolate the cause into caller-visible diagnostics here. - void cause; return { cleaned: false, - reason: - "Windows P0 process-tree termination failed; Job Object ownership is unavailable", + reason: `Windows process-tree termination failed for PID ${String(rootPid)}: ${errorMessage(cause)}`, }; } }; diff --git a/src/process/ProcessOwnership.validation.test.ts b/src/process/ProcessOwnership.validation.test.ts index 763c6182d..29abdf876 100644 --- a/src/process/ProcessOwnership.validation.test.ts +++ b/src/process/ProcessOwnership.validation.test.ts @@ -306,7 +306,7 @@ describe("Windows P0 process-tree cleanup", () => { await expect(cleanupWindowsProcessTree(42, failing)).resolves.toEqual({ cleaned: false, reason: - "Windows P0 process-tree termination failed; Job Object ownership is unavailable", + "Windows process-tree termination failed for PID 42: taskkill failed", }); }); }); diff --git a/src/process/capture/ProcessCaptureError.ts b/src/process/capture/ProcessCaptureError.ts index 1eea782a5..f618b8969 100644 --- a/src/process/capture/ProcessCaptureError.ts +++ b/src/process/capture/ProcessCaptureError.ts @@ -1,3 +1,4 @@ +import { DarwinProcessOwnershipInspectionError } from "../DarwinProcessRunTokenReader.js"; import { AnalysisError } from "../../domain/analysisErrorBase.js"; import type { PartialProcessCaptureObservation, @@ -61,3 +62,20 @@ export const processCaptureCancelled = (): ProcessCaptureError => reason: "cancelled", userMessage: "Process capture was cancelled. Start it again when ready.", }); + +/** Preserve typed process-inspection preflight failures at the capture boundary. */ +export const normalizeCaptureFailure = ( + cause: unknown, + signal: AbortSignal | undefined, +): unknown => { + if (cause instanceof ProcessCaptureError) return cause; + if (cause instanceof DarwinProcessOwnershipInspectionError) + return new ProcessCaptureError(cause.message, { cause }); + if ( + signal?.aborted === true && + (cause === signal.reason || + (cause instanceof Error && cause.name === "AbortError")) + ) + return processCaptureCancelled(); + return cause; +}; diff --git a/src/process/capture/ProcessCaptureLifecycle.ts b/src/process/capture/ProcessCaptureLifecycle.ts index 7d8325384..86cd88d0c 100644 --- a/src/process/capture/ProcessCaptureLifecycle.ts +++ b/src/process/capture/ProcessCaptureLifecycle.ts @@ -37,6 +37,7 @@ import { classifyFilesystemEffects } from "./ProcessFilesystemEffects.js"; import { cleanupOwnedProcessGroup, verifyNoTokenOwnedProcesses, + type OwnedProcessGroup, type ProcessCleanupResult, type ProcessOwnershipBaseline, } from "../ProcessOwnership.js"; @@ -47,6 +48,7 @@ import { } from "../ProcessOwnershipObservation.js"; import { describeProcessCaptureExecutionFailure, + normalizeCaptureFailure, ProcessCaptureError, } from "./ProcessCaptureError.js"; import { assertNotCancelled } from "./ProcessScenarioRuntimeValidation.js"; @@ -685,6 +687,66 @@ const verifyTokenOwnedProcessesUntilSettled = async ( return result; }; +const releaseCapturedProcessGroup = async ( + ownership: OwnedProcessGroup, + host: ProcessCaptureCleanupHost, +): Promise => { + if (host.platform === "win32") + return { + state: "unverified", + reason: + "owned process cleanup is unverifiable on Windows without process-job authority", + }; + let outcome: ProcessCaptureCleanupReport["owned_process_group"] = { + state: "cleaned", + reason: null, + }; + const unverified: Array<{ + readonly pid: number; + readonly diagnostic: string; + }> = []; + try { + const cleaned = await host.cleanupProcessGroup(ownership); + unverified.push(...(cleaned.unverified ?? [])); + if (!cleaned.cleaned) { + outcome = { state: "unverified", reason: cleaned.reason }; + } else { + const verified = await verifyTokenOwnedProcessesUntilSettled(() => + host.verifyTokenOwnedProcesses( + ownership.runId, + undefined, + ownership.captureBaseline, + ownership, + ), + ); + unverified.push(...(verified.unverified ?? [])); + if (!verified.cleaned) + outcome = { state: "unverified", reason: verified.reason }; + } + } catch (cause: unknown) { + outcome = { + state: "failed", + reason: + cause instanceof Error + ? cause.message || cause.name || "owned process cleanup failed" + : "owned process cleanup failed", + }; + } + return unverified.length === 0 + ? outcome + : { + ...outcome, + unverified_processes: [ + ...new Map( + unverified.map(({ pid, diagnostic }) => [ + `${String(pid)}:${diagnostic}`, + { pid, reason: diagnostic }, + ]), + ).values(), + ], + }; +}; + export const releaseProcessResources = async (options: { readonly timers: ReadonlySet; readonly terminal: Pick | undefined; @@ -712,68 +774,24 @@ export const releaseProcessResources = async (options: { : "renderer dispose failed", }; } - let ownedProcessGroup: ProcessCaptureCleanupReport["owned_process_group"] = { - state: options.terminal === undefined ? "not_required" : "cleaned", - reason: null, - }; - if (options.terminal !== undefined) { - if (host.platform === "win32") { - ownedProcessGroup = { - state: "unverified", - reason: - "owned process cleanup is unverifiable on Windows without process-job authority", - }; - } else { - const terminalPid = options.terminal.pid; - const relation = { - leaderPid: terminalPid, - processGroupId: terminalPid, - ...(options.sampledProcessGroupIds === undefined - ? {} - : { sampledProcessGroupIds: options.sampledProcessGroupIds }), - }; - const cleaned = await host.cleanupProcessGroup({ - runId: options.runId, - leaderPid: terminalPid, - processGroupId: terminalPid, - sweepTokenOwnedProcesses: true, - ...(options.sampledProcessGroupIds === undefined - ? {} - : { sampledProcessGroupIds: options.sampledProcessGroupIds }), - ...(options.captureBaseline === undefined - ? {} - : { captureBaseline: options.captureBaseline }), - }); - const unverified = [...(cleaned.unverified ?? [])]; - if (!cleaned.cleaned) { - ownedProcessGroup = { state: "unverified", reason: cleaned.reason }; - } else { - const verified = await verifyTokenOwnedProcessesUntilSettled(() => - host.verifyTokenOwnedProcesses( - options.runId, - undefined, - options.captureBaseline, - relation, - ), + const ownedProcessGroup = + options.terminal === undefined + ? { state: "not_required" as const, reason: null } + : await releaseCapturedProcessGroup( + { + runId: options.runId, + leaderPid: options.terminal.pid, + processGroupId: options.terminal.pid, + sweepTokenOwnedProcesses: true, + ...(options.sampledProcessGroupIds === undefined + ? {} + : { sampledProcessGroupIds: options.sampledProcessGroupIds }), + ...(options.captureBaseline === undefined + ? {} + : { captureBaseline: options.captureBaseline }), + }, + host, ); - unverified.push(...(verified.unverified ?? [])); - if (!verified.cleaned) - ownedProcessGroup = { state: "unverified", reason: verified.reason }; - } - if (unverified.length > 0) - ownedProcessGroup = { - ...ownedProcessGroup, - unverified_processes: [ - ...new Map( - unverified.map(({ pid, diagnostic }) => [ - `${String(pid)}:${diagnostic}`, - { pid, reason: diagnostic }, - ]), - ).values(), - ], - }; - } - } let temporaryRoot: ProcessCaptureCleanupReport["temporary_root"] = { state: "cleaned", reason: null, @@ -1018,16 +1036,36 @@ export const prepareProcessCapture = async ( await host.prepareOwnershipInspector?.(signal); assertNotCancelled(signal); const before = await captureSnapshot(scenario, signal); + const runId = randomUUID(); const temporaryRoot = await host.createTemporaryRoot(); try { assertNotCancelled(signal); - const runId = randomUUID(); const ownershipBaseline = (await host.captureOwnershipBaseline?.(signal)) ?? []; assertNotCancelled(signal); return { temporaryRoot, runId, ownershipBaseline, before }; } catch (cause: unknown) { - await host.cleanup(temporaryRoot); + const cleanup = await releaseProcessResources({ + timers: new Set(), + terminal: undefined, + renderer: undefined, + runId, + temporaryRoot, + host: { ...processCaptureCleanupHost, removeTemporaryRoot: host.cleanup }, + }); + resolveProcessResult( + undefined, + normalizeCaptureFailure(cause, signal), + cleanup, + createProcessCaptureObservationBuffer({ + frames: [], + interactions: [], + samples: [], + eventJournal: [], + before, + }), + { scenario }, + ); throw cause; } }; diff --git a/src/process/capture/ProcessHarness.test.ts b/src/process/capture/ProcessHarness.test.ts index 83a16ffeb..bb74c82c6 100644 --- a/src/process/capture/ProcessHarness.test.ts +++ b/src/process/capture/ProcessHarness.test.ts @@ -3,7 +3,6 @@ import { processScenarioSchema } from "../../domain/process/processScenario.js"; import { AnalysisCapabilityUnavailableError } from "../../domain/analysisErrorCore.js"; import { captureProcessScenario, - normalizeCaptureFailure, ProcessCaptureError, } from "./ProcessHarness.js"; import { settleProcessCaptureJournal } from "./ProcessCaptureLifecycle.js"; @@ -16,14 +15,105 @@ import { EMPTY_PROCESS_CAPTURE_EXAMPLE } from "../../domain/process/processCaptu import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; import { emptyProcessCapture } from "../../domain/process/processCapture.fixture.js"; import { analysisErrorProjectionSchema } from "../../contracts/errorSchemas.js"; -import { processCaptureCancelled } from "./ProcessCaptureError.js"; +import { + normalizeCaptureFailure, + processCaptureCancelled, +} from "./ProcessCaptureError.js"; import { releaseProcessResources, + prepareProcessCapture, resolveProcessResult, type ProcessCaptureCleanupHost, } from "./ProcessCaptureLifecycle.js"; import { DarwinProcessOwnershipInspectionError } from "../DarwinProcessRunTokenReader.js"; +it("retains cancellation and cleanup facts when ownership baseline inspection aborts", async () => { + const controller = new AbortController(); + const scenario = processScenarioSchema.parse({ + executable: process.execPath, + }); + await expect( + prepareProcessCapture( + scenario, + controller.signal, + async () => ({ files: [], truncated: false, completeRoots: [] }), + { + createTemporaryRoot: async () => "/fixture/allocated-root", + captureOwnershipBaseline: async () => { + controller.abort(); + controller.signal.throwIfAborted(); + return []; + }, + cleanup: async () => undefined, + }, + ), + ).rejects.toMatchObject({ + reason: "cancelled", + message: "process capture was cancelled", + cleanupReport: { + temporary_root: { state: "cleaned", reason: null }, + }, + partialObservation: { + observations: { + filesystem_snapshots: { + before: { + state: "available", + value: { files: [], truncated: false }, + }, + }, + }, + }, + }); +}); + +it("keeps prelaunch snapshot evidence and the ownership failure when root cleanup throws", async () => { + const ownershipFailure = new Error("process identity baseline denied"); + const scenario = processScenarioSchema.parse({ + executable: process.execPath, + }); + try { + await prepareProcessCapture( + scenario, + undefined, + async () => ({ files: [], truncated: true, completeRoots: [] }), + { + createTemporaryRoot: async () => "/fixture/allocated-root", + captureOwnershipBaseline: async () => { + throw ownershipFailure; + }, + cleanup: async () => { + throw new Error("root removal denied"); + }, + }, + ); + throw new Error("expected preparation failure"); + } catch (cause: unknown) { + if (!(cause instanceof ProcessCaptureError)) throw cause; + expect(cause.cause).toBe(ownershipFailure); + expect( + analysisErrorProjectionSchema.parse(projectAnalysisError(cause)), + ).toMatchObject({ + code: "cleanup_incomplete", + details: { + execution_failure: ownershipFailure.message, + cleanup_report: { + temporary_root: { state: "failed", reason: "root removal denied" }, + }, + partial_observation: { + observations: { + filesystem_snapshots: { + before: { + state: "available", + value: { files: [], truncated: true }, + }, + }, + }, + }, + }, + }); + } +}); + it("rejects legacy replay output instead of silently discarding it", () => { expect( processCaptureSchema.safeParse({ @@ -100,6 +190,74 @@ it("keeps empty cleanup exception messages actionable in the report", async () = expect(report.temporary_root).toEqual({ state: "failed", reason: "Error" }); }); +it.each(["termination", "verification"] as const)( + "retains observations and releases the temporary root when process %s throws", + async (stage) => { + const cleanupFailure = new Error(`${stage} inspection failed`); + const removeTemporaryRoot = vi.fn(async () => undefined); + const host: ProcessCaptureCleanupHost = { + platform: "linux", + cleanupProcessGroup: async () => { + if (stage === "termination") throw cleanupFailure; + return { + cleaned: true, + signaled: true, + unverified: [{ pid: 900, diagnostic: "environment unavailable" }], + }; + }, + verifyTokenOwnedProcesses: async () => { + throw cleanupFailure; + }, + removeTemporaryRoot, + }; + const cleanup = await releaseProcessResources({ + timers: new Set(), + terminal: { pid: 321 }, + renderer: undefined, + runId: "fixture-run", + temporaryRoot: "/fixture/root", + host, + }); + + expect(removeTemporaryRoot).toHaveBeenCalledWith("/fixture/root"); + expect(cleanup.temporary_root).toEqual({ state: "cleaned", reason: null }); + expect(cleanup.owned_process_group).toMatchObject({ + state: "failed", + reason: cleanupFailure.message, + }); + if (stage === "verification") + expect(cleanup.owned_process_group.unverified_processes).toEqual([ + { pid: 900, reason: "environment unavailable" }, + ]); + + const executionFailure = new Error("capture snapshot failed"); + const capture = parseProcessCapture({ + ...emptyProcessCapture(), + frames: [{ sequence: 0, at_ms: 0, data: "collected before failure" }], + event_journal: [], + }); + try { + resolveProcessResult(capture, executionFailure, cleanup); + throw new Error("expected cleanup failure"); + } catch (cause: unknown) { + if (!(cause instanceof ProcessCaptureError)) throw cause; + expect(cause.cause).toBe(executionFailure); + expect( + analysisErrorProjectionSchema.parse(projectAnalysisError(cause)), + ).toMatchObject({ + code: "cleanup_incomplete", + details: { + cleanup_report: cleanup, + execution_failure: "capture snapshot failed", + partial_observation: { + capture: { frames: [{ data: "collected before failure" }] }, + }, + }, + }); + } + }, +); + it("waits for token-owned processes to exit after one cleanup signal", async () => { vi.useFakeTimers(); try { diff --git a/src/process/capture/ProcessHarness.ts b/src/process/capture/ProcessHarness.ts index d13712499..672e5209b 100644 --- a/src/process/capture/ProcessHarness.ts +++ b/src/process/capture/ProcessHarness.ts @@ -16,6 +16,7 @@ import { type AnalysisError } from "../../domain/analysisErrorBase.js"; import { describeProcessCaptureExecutionFailure, ProcessCaptureError, + normalizeCaptureFailure, processCaptureCancelled, } from "./ProcessCaptureError.js"; export { ProcessCaptureError } from "./ProcessCaptureError.js"; @@ -55,7 +56,6 @@ import { makeProcessCaptureEnvironment } from "./ProcessCaptureEnvironment.js"; import { classifyFilesystemEffects } from "./ProcessFilesystemEffects.js"; import { processCaptureOwnershipUnavailableReason } from "./ProcessCaptureCapability.js"; import type { ProcessOwnershipBaseline } from "../ProcessOwnership.js"; -import { DarwinProcessOwnershipInspectionError } from "../DarwinProcessRunTokenReader.js"; export { probeProcessCaptureCapability } from "./ProcessCaptureCapability.js"; interface StartedCaptureRuntime { @@ -89,7 +89,6 @@ const cleanupFailedStartup = async (options: { options.observations.process_samples.value, ).filter((groupId) => groupId !== options.terminal?.pid) : undefined; - options.terminal?.kill("SIGKILL"); if (options.observations !== undefined && options.renderer !== undefined) { try { options.observations.rendered_frames = { @@ -322,23 +321,6 @@ const finishProcessRun = async (options: { ); }; -/** Preserve typed process-inspection preflight failures at the capture boundary. */ -export const normalizeCaptureFailure = ( - cause: unknown, - signal: AbortSignal | undefined, -): unknown => { - if (cause instanceof ProcessCaptureError) return cause; - if (cause instanceof DarwinProcessOwnershipInspectionError) - return new ProcessCaptureError(cause.message, { cause }); - if ( - signal?.aborted === true && - (cause === signal.reason || - (cause instanceof Error && cause.name === "AbortError")) - ) - return processCaptureCancelled(); - return cause; -}; - const completeCapture = async (options: { readonly scenario: ProcessScenario; readonly hostPlatform: NodeJS.Platform; @@ -572,7 +554,6 @@ const runProcessScenario = async ( ...(signal === undefined ? {} : { signal }), }); } catch (cause: unknown) { - runtime?.terminal.kill("SIGKILL"); executionFailure = normalizeCaptureFailure(cause, signal); if ( runtime !== undefined && diff --git a/tests/boundary/process/darwinProcessRunTokenReader.test.ts b/tests/boundary/process/darwinProcessRunTokenReader.test.ts index a0dfb3967..6e438f6c7 100644 --- a/tests/boundary/process/darwinProcessRunTokenReader.test.ts +++ b/tests/boundary/process/darwinProcessRunTokenReader.test.ts @@ -24,7 +24,7 @@ import { } from "../../../src/process/ProcessOwnershipObservation.js"; import { createDarwinProcessRunTokenReader } from "../../../src/process/DarwinProcessRunTokenReader.js"; import { execFileOutput } from "../../../src/process/ExecFileOutput.js"; -import { normalizeCaptureFailure } from "../../../src/process/capture/ProcessHarness.js"; +import { normalizeCaptureFailure } from "../../../src/process/capture/ProcessCaptureError.js"; import { PlaywrightElectronActiveProvider } from "../../../src/browser/PlaywrightElectronActiveProvider.js"; import { electronActiveObservationInputSchema } from "../../../src/domain/javascript/electronActiveObservation.js"; @@ -538,7 +538,7 @@ it.skipIf(process.platform === "win32")( executable_path: "/missing/electron", application_path: "/missing/application.js", }); - captureOutcome = new PlaywrightElectronActiveProvider() + captureOutcome = new PlaywrightElectronActiveProvider({}) .capture(input, { signal: controller.signal }) .then( (value) => ({ state: "result" as const, value }), @@ -650,7 +650,7 @@ it.skipIf(process.platform === "win32")( application_path: fileURLToPath(import.meta.url), application_root: process.cwd(), }); - captureOutcome = new PlaywrightElectronActiveProvider() + captureOutcome = new PlaywrightElectronActiveProvider({}) .capture(input, { signal: controller.signal }) .then( (value) => ({ state: "result" as const, value }), diff --git a/tests/boundary/process/processCli.test.ts b/tests/boundary/process/processCli.test.ts index d5c9de51c..0d47850d6 100644 --- a/tests/boundary/process/processCli.test.ts +++ b/tests/boundary/process/processCli.test.ts @@ -105,71 +105,6 @@ describe("process CLI errors", () => { CLI_INTEGRATION_TIMEOUT_MS, ); - it("projects unexpected causes without their message or stack", () => { - const projected = projectProcessCliError( - new Error("SECRET internal failure /private/path"), - ); - expect(projected).toEqual({ - error: "Process command failed", - category: "execution_failure", - message: - "Process command could not complete. Check the input files and run `rea doctor`, then try again.", - }); - expect(JSON.stringify(projected)).not.toContain("SECRET"); - }); - - it("projects missing, malformed JSON, and invalid scenario files", async () => { - const root = await fixture(); - const malformed = join(root, "malformed.json"); - const invalid = join(root, "invalid.json"); - const invalidUtf8 = join(root, "invalid-utf8.json"); - await writeFile(malformed, "not-json"); - await writeFile(invalid, "{}"); - await writeFile( - invalidUtf8, - Buffer.concat([ - Buffer.from('{"executable":"'), - Buffer.from([0xff]), - Buffer.from('"}'), - ]), - ); - - expect( - await captureProcessScenarioFile(join(root, "missing.json")), - ).toEqual({ - error: "Process command failed", - category: "invalid_input", - message: - "Process input file could not be read. Check that the path exists and is readable.", - }); - expect(await captureProcessScenarioFile(malformed)).toEqual({ - error: "Process command failed", - category: "invalid_input", - message: - "Process input file is not valid JSON. Repair the file, then try again.", - }); - expect(await captureProcessScenarioFile(invalidUtf8)).toEqual({ - error: "Process command failed", - category: "invalid_input", - message: - "Process input file is not valid JSON. Repair the file, then try again.", - }); - expect(await captureProcessScenarioFile(invalid)).toEqual({ - error: "Process command failed", - code: "invalid_request", - category: "invalid_input", - message: "Analysis input is invalid. Check the arguments and try again.", - retryable: true, - remediation: { action: "Correct the listed arguments and retry." }, - details: expect.objectContaining({ - operation: "capture_process_scenario", - issues: expect.arrayContaining([ - expect.objectContaining({ path: ["executable"] }), - ]), - }), - }); - }); - it("rejects NUL arguments before process launch", async () => { const root = await fixture(); const scenario = join(root, "nul-argument.json");