From 2a7746998d47446c4db6d4f524e0f183d6803f3f Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:58:31 +0800 Subject: [PATCH] fix(process)!: require current captures and preserve cleanup failures Remove v3 capture conversion and require explicit executable identity and event journals. Preserve cancellation, execution diagnostics, snapshots, and cleanup outcomes from preparation through CLI delivery. --- src/application/process/ProcessCli.test.ts | 90 ++++++++++ src/application/process/ProcessCli.ts | 11 +- src/domain/process/processCapture.ts | 35 ++-- .../process/processCapture.validation.test.ts | 91 ++-------- src/domain/process/processCaptureExample.ts | 1 + src/domain/process/processCaptureParsing.ts | 41 +---- .../process/processCaptureValidation.ts | 15 +- .../process/processTraceComparison.test.ts | 2 +- src/domain/process/processTraceEvaluation.ts | 4 +- src/domain/reconstructionVerification.test.ts | 38 +--- src/process/ProcessOwnership.ts | 6 +- .../ProcessOwnership.validation.test.ts | 2 +- src/process/capture/ProcessCaptureError.ts | 18 ++ .../capture/ProcessCaptureLifecycle.ts | 164 +++++++++++------- src/process/capture/ProcessHarness.test.ts | 162 ++++++++++++++++- src/process/capture/ProcessHarness.ts | 21 +-- .../darwinProcessRunTokenReader.test.ts | 6 +- tests/boundary/process/processCli.test.ts | 65 ------- 18 files changed, 419 insertions(+), 353 deletions(-) create mode 100644 src/application/process/ProcessCli.test.ts 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");