From 1629266e3c08c90fed6bba5383528af7986a12b7 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 21 Oct 2026 02:03:17 +0800 Subject: [PATCH] fix(errors): preserve canonical failures and correction details Retain process input and unexpected failure diagnostics through the shared error projector. Preserve filesystem causes and firmware validation issues, and mark unchanged invalid requests nonretryable. Use typed stat settlement and parse unknown command failures without losing null or undefined rejection causes. Refs #5397, #5401, #5402, #5642 --- .../firmware/FirmwareAnalysisService.ts | 9 +- src/application/process/ProcessCli.ts | 84 ++++--- .../reverse/ReverseEngineeringService.ts | 83 +++++-- src/domain/analysisErrorProjection.test.ts | 28 +++ src/domain/analysisErrorProjection.ts | 1 - .../applicationWorkflowCli.test.ts | 2 +- tests/boundary/cli/adbCli.test.ts | 2 +- .../cli/browserInputDiagnostics.test.ts | 2 +- .../cli/filteredFailureOutput.test.ts | 4 +- .../cli/firmwareIncompleteRange.test.ts | 56 +++++ .../reconstructionObligationLedgerCli.test.ts | 2 +- .../cli/reverseEngineeringTargetInput.test.ts | 52 +++++ .../filesystem/processCliFileInput.test.ts | 177 ++++++++++++--- .../filesystem/targetDiagnostics.test.ts | 3 +- .../firmwareValidationProjection.test.ts | 60 +++++ .../mcp/applicationEvidenceMcp.test.ts | 2 +- ...browserScenarioInputDiagnosticsMcp.test.ts | 4 +- .../mcp/cleanupPrimaryProjection.test.ts | 2 +- tests/boundary/process/processCli.test.ts | 82 ++++++- .../reverseEngineeringCommandPort.test.ts | 205 ++++++++++++++++++ .../reverseEngineeringUnknownFailures.test.ts | 65 ++++++ 21 files changed, 832 insertions(+), 93 deletions(-) create mode 100644 tests/boundary/cli/firmwareIncompleteRange.test.ts create mode 100644 tests/boundary/cli/reverseEngineeringTargetInput.test.ts create mode 100644 tests/boundary/firmware/firmwareValidationProjection.test.ts create mode 100644 tests/composition/reverseEngineeringUnknownFailures.test.ts diff --git a/src/application/firmware/FirmwareAnalysisService.ts b/src/application/firmware/FirmwareAnalysisService.ts index 6b553c6c4a..765a9da13d 100644 --- a/src/application/firmware/FirmwareAnalysisService.ts +++ b/src/application/firmware/FirmwareAnalysisService.ts @@ -12,6 +12,7 @@ import { import type { AnalysisError } from "../../domain/analysisErrorBase.js"; import { createEvidence, type Evidence } from "../../domain/evidence.js"; import { jsonObjectSchema } from "../../domain/jsonValue.js"; +import { projectInputIssues } from "../../domain/inputIssueProjection.js"; import { err, type Result } from "../../domain/result.js"; import { completeProviderEvidence } from "../ProviderOperationExecution.js"; @@ -35,7 +36,13 @@ export class FirmwareAnalysisService { ); const parsed = firmwareRequestSchema.safeParse({ operation, input }); if (!parsed.success) - return err(new AnalysisInputError(operation, { cause: parsed.error })); + return err( + new AnalysisInputError( + operation, + { cause: parsed.error }, + projectInputIssues(parsed.error.issues, { operation, input }), + ), + ); const executed = await this.provider.execute(parsed.data, options); if (!executed.ok) return executed; const execution = executed.value; diff --git a/src/application/process/ProcessCli.ts b/src/application/process/ProcessCli.ts index f04936b12a..6849847a2a 100644 --- a/src/application/process/ProcessCli.ts +++ b/src/application/process/ProcessCli.ts @@ -7,6 +7,8 @@ import { AnalysisInputError, } from "../../domain/analysisErrorCore.js"; import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; +import type { AnalysisErrorProjection } from "../../domain/analysisErrorProjection.js"; +import { ProviderAdapterError } from "../../domain/providerAdapterError.js"; import { createEvidence, parseEvidence } from "../../domain/evidence.js"; import { describeValidationFailure } from "../../domain/evidenceBundle.js"; import { jsonValueSchema } from "../../domain/jsonValue.js"; @@ -31,10 +33,8 @@ import { readJsonFile } from "../JsonFiles.js"; import { RegularFileCleanupFailure } from "../RegularFileRead.js"; /** Safe process-command failure returned to the CLI adapter. */ -export interface ProcessCliErrorOutput { +export interface ProcessCliErrorOutput extends AnalysisErrorProjection { readonly error: "Process command failed"; - readonly category: string; - readonly message: string; } /** Identify the process workflow's typed diagnostic result for CLI exit status. */ @@ -48,7 +48,16 @@ export const isProcessCliFailure = ( "category" in value && typeof value.category === "string" && "message" in value && - typeof value.message === "string"; + typeof value.message === "string" && + "code" in value && + typeof value.code === "string" && + "retryable" in value && + typeof value.retryable === "boolean" && + "remediation" in value && + typeof value.remediation === "object" && + value.remediation !== null && + "action" in value.remediation && + typeof value.remediation.action === "string"; /** Capture one JSON scenario through the shared process harness and Evidence contract. */ export const captureProcessScenarioFile = async ( @@ -102,7 +111,7 @@ export const captureProcessScenarioFile = async ( return createProcessCaptureEvidence(parsed.data, captured.value); } catch (cause: unknown) { if (cause instanceof RegularFileCleanupFailure) throw cause; - return projectProcessCliError(cause); + return projectProcessCliError(cause, "capture_process_scenario"); } }; @@ -155,7 +164,7 @@ export const compareProcessEvidenceFiles = async ( }); } catch (cause: unknown) { if (cause instanceof RegularFileCleanupFailure) throw cause; - return projectProcessCliError(cause); + return projectProcessCliError(cause, "compare_process_captures"); } }; @@ -170,15 +179,17 @@ const parseCaptureEvidence = (input: unknown, side: "left" | "right") => { { cause }, projectInputIssues(cause.issues, input), ); - throw invalidCaptureEvidence(cause); + throw invalidCaptureEvidence(cause, side); } if ( evidence.operation !== "capture_process_scenario" || evidence.predicate_type !== "rea.process-capture" ) { - throw new ProcessCliFailure( - "invalid_input", - "Capture evidence is not from the current process-capture workflow. Create new capture evidence, then try again.", + throw processInputFailure( + "compare_process_captures", + `Capture evidence is not from the current process-capture workflow. Create new capture evidence, then try again.`, + [side], + "invalid_value", ); } if (!isProcessEvidenceProvider(evidence.provider)) @@ -196,7 +207,7 @@ const parseCaptureEvidence = (input: unknown, side: "left" | "right") => { locations: evidence.locations, }; } catch (cause: unknown) { - throw invalidCaptureEvidence(cause); + throw invalidCaptureEvidence(cause, side); } }; @@ -214,10 +225,12 @@ const parseTraceSpecification = (input: unknown) => { ); }; -const invalidCaptureEvidence = (cause: unknown): ProcessCliFailure => - new ProcessCliFailure( - "invalid_input", +const invalidCaptureEvidence = (cause: unknown, side: "left" | "right") => + processInputFailure( + "compare_process_captures", `Capture evidence is malformed (${describeValidationFailure(cause)}). Create new capture evidence, then try again.`, + [side], + "invalid_value", ); const readJson = async ( @@ -237,23 +250,27 @@ const readJson = async ( if (signal?.aborted) throw new AnalysisCancelledError(operation); if (loaded.ok) return loaded.value; if (loaded.error._tag !== "EvidenceFileError") throw loaded.error; + if (loaded.error.reason === "io") throw loaded.error; const detail = describeValidationFailure(loaded.error.cause); - throw new ProcessCliFailure( - "invalid_input", + throw processInputFailure( + operation, loaded.error.reason === "invalid-json" ? `Process input file is not valid UTF-8 JSON: ${path} (${detail}). Repair the file, then try again.` : `Process input file could not be read: ${path} (${detail}). Check the reported filesystem constraint and retry.`, + ["input_file"], + "invalid_value", ); }; -class ProcessCliFailure extends Error { - constructor( - readonly category: string, - readonly userMessage: string, - ) { - super(userMessage); - } -} +const processInputFailure = ( + operation: string, + userMessage: string, + path: readonly (string | number)[], + reason: "invalid_value" | "missing_argument", +): AnalysisInputError => + new AnalysisInputError(operation, { userMessage }, [ + { path, reason, message: userMessage }, + ]); const cliAnalysisError = (error: AnalysisError): ProcessCliErrorOutput => ({ error: "Process command failed", @@ -263,17 +280,14 @@ const cliAnalysisError = (error: AnalysisError): ProcessCliErrorOutput => ({ /** Preserve typed failures and local diagnostics at the process CLI boundary. */ export const projectProcessCliError = ( cause: unknown, + operation: "capture_process_scenario" | "compare_process_captures", ): ProcessCliErrorOutput => { - if (cause instanceof ProcessCliFailure) - return { - error: "Process command failed", - category: cause.category, - message: cause.userMessage, - }; if (cause instanceof AnalysisError) return cliAnalysisError(cause); - return { - error: "Process command failed", - category: "execution_failure", - message: `Process command could not complete: ${describeValidationFailure(cause)}`, - }; + return cliAnalysisError( + new ProviderAdapterError(PROCESS_PROVIDER.id, operation, { + cause, + diagnostics: { phase: "process_cli" }, + userMessage: `Process command could not complete: ${describeValidationFailure(cause)}`, + }), + ); }; diff --git a/src/application/reverse/ReverseEngineeringService.ts b/src/application/reverse/ReverseEngineeringService.ts index b28db77b21..0979313365 100644 --- a/src/application/reverse/ReverseEngineeringService.ts +++ b/src/application/reverse/ReverseEngineeringService.ts @@ -16,11 +16,12 @@ import { AnalysisResourceConstraintError, AnalysisTimeoutError, } from "../../domain/analysisErrorCore.js"; -import type { - AnalysisCleanupObservation, +import { AnalysisError, + type AnalysisCleanupObservation, } from "../../domain/analysisErrorBase.js"; import { AnalysisOutputError } from "../../domain/analysisErrorCore.js"; +import { ProviderAdapterError } from "../../domain/providerAdapterError.js"; import { createEvidence, type Evidence } from "../../domain/evidence.js"; import type { JsonValue } from "../../domain/jsonValue.js"; import { err, ok, type Result } from "../../domain/result.js"; @@ -320,7 +321,7 @@ export class ReverseEngineeringService { if (absolutePath !== undefined) digest = await this.#digest(absolutePath, signal, this.#stat); } catch (cause) { - const code = (cause as NodeJS.ErrnoException).code; + const code = commandFailureCode(cause); if (signal?.aborted) return err(new AnalysisCancelledError(operation, { cause })); if (code === "EACCES" || code === "EPERM") { @@ -335,7 +336,48 @@ export class ReverseEngineeringService { return err( new AnalysisTimeoutError(operation, DEFAULT_TIMEOUT_MS, { cause }), ); - return err(new AnalysisInputError(operation, { cause })); + if ( + code !== "ENOENT" && + code !== "ENOTDIR" && + code !== "EINVAL" && + code !== "ELOOP" && + code !== "ENAMETOOLONG" + ) { + if (cause instanceof AnalysisError) return err(cause); + return err( + new ProviderAdapterError(profile.identity.id, operation, { + cause, + diagnostics: { + phase: "target_preflight", + ...(code === undefined ? {} : { system_code: code }), + }, + }), + ); + } + const targetReason = + code === "EINVAL" + ? "The selected target is not a regular file." + : code === "ELOOP" + ? "The selected target path contains a symbolic-link loop." + : code === "ENAMETOOLONG" + ? "The selected target path is too long." + : "The selected target was not found."; + return err( + new AnalysisInputError( + operation, + { + cause, + userMessage: `${targetReason} Selected path: ${absolutePath}. Check the path and choose a readable regular file before retrying.`, + }, + [ + { + path: ["path"], + reason: "invalid_value", + message: `${targetReason}${code === undefined ? "" : ` (host error ${code})`}`, + }, + ], + ), + ); } let stdout = ""; let stderr = ""; @@ -401,8 +443,7 @@ export class ReverseEngineeringService { }, ); const capture = execFileOutputFailure(cause); - const code = - capture?.code ?? (cause as NodeJS.ErrnoException).code ?? undefined; + const code = capture?.code ?? commandFailureCode(cause); if (earlyFailure === undefined && code === "E2BIG") earlyFailure = new AnalysisResourceConstraintError( operation, @@ -931,6 +972,14 @@ const digestFile = async ( }); }; +const commandFailureCode = (cause: unknown): string | number | undefined => { + if (typeof cause !== "object" || cause === null) return undefined; + const code: unknown = Reflect.get(cause, "code"); + return typeof code === "string" || typeof code === "number" + ? code + : undefined; +}; + const statWithDeadline = ( path: string, signal: AbortSignal | undefined, @@ -940,22 +989,24 @@ const statWithDeadline = ( return Promise.reject(new AnalysisCancelledError("artifact_digest")); return new Promise((resolveStat, rejectStat) => { let settled = false; - const finish = (cause?: unknown, value?: Stats): void => { + const finish = (outcome: Result): void => { if (settled) return; settled = true; clearTimeout(timer); signal?.removeEventListener("abort", abort); - if (cause !== undefined) rejectStat(cause); - else resolveStat(value!); + if (outcome.ok) resolveStat(outcome.value); + else rejectStat(outcome.error); }; const abort = (): void => - finish(new AnalysisCancelledError("artifact_digest")); + finish(err(new AnalysisCancelledError("artifact_digest"))); const timer = setTimeout( () => finish( - Object.assign(new Error("Artifact stat timed out"), { - code: "ETIMEDOUT", - }), + err( + Object.assign(new Error("Artifact stat timed out"), { + code: "ETIMEDOUT", + }), + ), ), DEFAULT_TIMEOUT_MS, ); @@ -968,11 +1019,11 @@ const statWithDeadline = ( // stat rejection is observed and cannot become an unhandled rejection. try { void stat(path).then( - (info) => finish(undefined, info), - (cause: unknown) => finish(cause), + (info) => finish(ok(info)), + (cause: unknown) => finish(err(cause)), ); } catch (cause) { - finish(cause); + finish(err(cause)); } }); }; diff --git a/src/domain/analysisErrorProjection.test.ts b/src/domain/analysisErrorProjection.test.ts index c91888b64f..086db40b02 100644 --- a/src/domain/analysisErrorProjection.test.ts +++ b/src/domain/analysisErrorProjection.test.ts @@ -26,6 +26,34 @@ import { analysisErrorProjectionSchema } from "../contracts/errorSchemas.js"; import { nativeCallPartialObservationSchema } from "./native/nativeCallPartialObservation.js"; describe("agent recovery metadata", () => { + it("does not mark deterministic input validation as retryable", () => { + const projected = projectAnalysisError( + new AnalysisInputError("extract_firmware", undefined, [ + { path: ["range", "length"], reason: "missing_argument" }, + ]), + ); + + expect(projected).toMatchObject({ + code: "invalid_request", + retryable: false, + remediation: { + action: expect.stringContaining("Do not repeat the unchanged call."), + }, + }); + expect(analysisErrorProjectionSchema.parse(projected)).toEqual(projected); + }); + + it("keeps retryability for transient conflicts and provider timeouts", () => { + expect( + projectAnalysisError(new UnknownRegistryError("revision-conflict")), + ).toMatchObject({ code: "revision_conflict", retryable: true }); + expect( + projectAnalysisError( + new AnalysisTimeoutError("inspect_binary_layout", 2500), + ), + ).toMatchObject({ code: "provider_timeout", retryable: true }); + }); + it("names the input contract for malformed analysis arguments", () => { const projected = projectAnalysisError( new AnalysisInputError("inspect_analysis_view", undefined, [ diff --git a/src/domain/analysisErrorProjection.ts b/src/domain/analysisErrorProjection.ts index e523a9ff65..05bb2aec9a 100644 --- a/src/domain/analysisErrorProjection.ts +++ b/src/domain/analysisErrorProjection.ts @@ -592,7 +592,6 @@ const lifecycleErrorDetails = ( const RETRYABLE_CODES: ReadonlySet = new Set([ "artifact_changed", - "invalid_request", "provider_timeout", "cancelled", "revision_conflict", diff --git a/tests/acceptance/applications/applicationWorkflowCli.test.ts b/tests/acceptance/applications/applicationWorkflowCli.test.ts index 482e6238f8..7a96b08fca 100644 --- a/tests/acceptance/applications/applicationWorkflowCli.test.ts +++ b/tests/acceptance/applications/applicationWorkflowCli.test.ts @@ -1680,7 +1680,7 @@ describe("application workflow CLI validation", () => { ]); expect(malformed).toMatchObject({ code: "invalid_request", - retryable: true, + retryable: false, details: { issues: [{ path: [], reason: "invalid_format", expected: "JSON" }], }, diff --git a/tests/boundary/cli/adbCli.test.ts b/tests/boundary/cli/adbCli.test.ts index 5d67eed043..d26123b9e8 100644 --- a/tests/boundary/cli/adbCli.test.ts +++ b/tests/boundary/cli/adbCli.test.ts @@ -135,7 +135,7 @@ it.each(["string:key", ":", "string"])( expect(error).toMatchObject({ code: "invalid_request", category: "invalid_input", - retryable: true, + retryable: false, remediation: { action: expect.any(String) }, }); expect(error.remediation.action).not.toBe(""); diff --git a/tests/boundary/cli/browserInputDiagnostics.test.ts b/tests/boundary/cli/browserInputDiagnostics.test.ts index b502e950ba..16b1182fd2 100644 --- a/tests/boundary/cli/browserInputDiagnostics.test.ts +++ b/tests/boundary/cli/browserInputDiagnostics.test.ts @@ -131,7 +131,7 @@ describe("browser and Inspector CLI input diagnostics", () => { category: "invalid_input", message: "Analysis input is invalid. Check the arguments and try again.", - retryable: true, + retryable: false, remediation: { action: `Read the declared input schema or CLI usage for ${input.operation}; if using MCP and the catalog may be stale, call tools/list. Correct the listed fields, then retry. Do not repeat the unchanged call.`, }, diff --git a/tests/boundary/cli/filteredFailureOutput.test.ts b/tests/boundary/cli/filteredFailureOutput.test.ts index 684b7b518d..35988e676a 100644 --- a/tests/boundary/cli/filteredFailureOutput.test.ts +++ b/tests/boundary/cli/filteredFailureOutput.test.ts @@ -96,7 +96,7 @@ it.each(["json", "toon", "yaml", "md"] as const)( expect(document).toMatchObject({ code: "invalid_request", category: "invalid_input", - retryable: true, + retryable: false, remediation: { action: expect.stringContaining("tools/list"), }, @@ -141,7 +141,7 @@ it.each(["json", "toon", "yaml", "md"] as const)( error: { code: "invalid_request", category: "invalid_input", - retryable: true, + retryable: false, remediation: { action: expect.stringContaining("tools/list"), }, diff --git a/tests/boundary/cli/firmwareIncompleteRange.test.ts b/tests/boundary/cli/firmwareIncompleteRange.test.ts new file mode 100644 index 0000000000..d648844080 --- /dev/null +++ b/tests/boundary/cli/firmwareIncompleteRange.test.ts @@ -0,0 +1,56 @@ +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { expect } from "vitest"; + +import { analysisErrorProjectionSchema } from "../../../src/contracts/errorSchemas.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; + +cliTest( + "identifies the missing companion option for firmware range input", + async ({ cli }) => { + const root = await createTestTempDirectory( + "rea-firmware-incomplete-range-", + ); + const firmware = join(root, "firmware.bin"); + await writeFile(firmware, new Uint8Array()); + const output = join(root, "extracted"); + const cases = [ + { option: "--offset", missing: "length" }, + { option: "--length", missing: "offset" }, + ] as const; + + for (const { option, missing } of cases) { + const result = await cli.run({ + arguments: [ + "extract-firmware", + firmware, + output, + option, + "5", + "--json", + ], + cwd: root, + }); + expect(result.exitCode).toBe(1); + expect(analysisErrorProjectionSchema.safeParse(result.json).success).toBe( + true, + ); + expect(result.json).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + retryable: false, + details: { + operation: "extract_firmware", + issues: [ + { + path: ["input", "range", missing], + reason: "missing_argument", + }, + ], + }, + }); + } + }, +); diff --git a/tests/boundary/cli/reconstructionObligationLedgerCli.test.ts b/tests/boundary/cli/reconstructionObligationLedgerCli.test.ts index eee6b3df1a..91b101364c 100644 --- a/tests/boundary/cli/reconstructionObligationLedgerCli.test.ts +++ b/tests/boundary/cli/reconstructionObligationLedgerCli.test.ts @@ -69,7 +69,7 @@ describe("reconstruction obligation ledger CLI parity", () => { expect(JSON.parse(stdout)).toMatchObject({ code: "invalid_request", category: "invalid_input", - retryable: true, + retryable: false, details: { operation: "build_reconstruction_obligation_ledger", issues: [ diff --git a/tests/boundary/cli/reverseEngineeringTargetInput.test.ts b/tests/boundary/cli/reverseEngineeringTargetInput.test.ts new file mode 100644 index 0000000000..f7ca9169a6 --- /dev/null +++ b/tests/boundary/cli/reverseEngineeringTargetInput.test.ts @@ -0,0 +1,52 @@ +import { join } from "node:path"; + +import { expect } from "vitest"; + +import { analysisErrorProjectionSchema } from "../../../src/contracts/errorSchemas.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; + +cliTest( + "reports missing and directory objdump inputs with structured reasons", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-objdump-invalid-target-"); + const missing = join(root, "missing.bin"); + const directory = root; + + for (const [path, expectedReason] of [ + [missing, "not found"], + [directory, "not a regular file"], + ] as const) { + const result = await cli.run({ + arguments: [ + "inspect-with-objdump", + path, + "symbols", + "--format", + "json", + ], + cwd: root, + }); + expect(result.exitCode).toBe(1); + expect(analysisErrorProjectionSchema.safeParse(result.json).success).toBe( + true, + ); + expect(result.json).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + retryable: false, + message: expect.stringContaining(path), + details: { + operation: "inspect_with_objdump", + issues: [ + { + path: ["path"], + reason: "invalid_value", + message: expect.stringContaining(expectedReason), + }, + ], + }, + }); + } + }, +); diff --git a/tests/boundary/filesystem/processCliFileInput.test.ts b/tests/boundary/filesystem/processCliFileInput.test.ts index 23f3f9631b..07295b15c5 100644 --- a/tests/boundary/filesystem/processCliFileInput.test.ts +++ b/tests/boundary/filesystem/processCliFileInput.test.ts @@ -4,7 +4,6 @@ import { describe, expect, it, onTestFinished, vi } from "vitest"; import { captureProcessScenarioFile, compareProcessEvidenceFiles, - projectProcessCliError, } from "../../../src/application/process/ProcessCli.js"; import { JSON_BYTE_ORDER_MARK_MESSAGE } from "../../../src/application/Utf8JsonInput.js"; import { createEvidence } from "../../../src/domain/evidence.js"; @@ -12,23 +11,58 @@ import { PROCESS_PROVIDER } from "../../../src/domain/process/processEvidencePro import { EMPTY_PROCESS_CAPTURE_EXAMPLE } from "../../../src/domain/process/processCaptureExample.js"; import { OwnedFileHandle } from "../../../src/filesystem/OwnedFileHandle.js"; import { RegularFileCleanupFailure } from "../../../src/application/RegularFileRead.js"; +import { analysisCliErrorEnvelopeSchema } from "../../../src/contracts/errorSchemas.js"; +import { EvidenceFileError } from "../../../src/domain/evidenceErrors.js"; +import { err } from "../../../src/domain/result.js"; +import * as jsonFiles from "../../../src/application/JsonFiles.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { holdFirstFileRead } from "../../support/holdFirstFileRead.js"; describe("process CLI error projection", () => { - 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.each([ + { + operation: "capture_process_scenario", + execute: (path: string) => captureProcessScenarioFile(path), + }, + { + operation: "compare_process_captures", + execute: (path: string) => compareProcessEvidenceFiles(path, path), + }, + ] as const)( + "projects an unexpected $operation failure canonically", + async ({ operation, execute }) => { + const failure = new Error("SECRET internal failure /private/path"); + const read = vi + .spyOn(jsonFiles, "readJsonFile") + .mockImplementation(async () => { + throw failure; + }); + + try { + const projected = await execute("/selected/input.json"); + expect( + analysisCliErrorEnvelopeSchema.safeParse(projected).success, + ).toBe(true); + expect(projected).toMatchObject({ + error: "Process command failed", + code: "execution_failure", + category: "execution_failure", + retryable: false, + message: + "Process command could not complete: SECRET internal failure /private/path", + details: { + operation, + provider_id: PROCESS_PROVIDER.id, + diagnostics: { phase: "process_cli" }, + }, + }); + expect(JSON.stringify(projected)).not.toContain("stack"); + } finally { + read.mockRestore(); + } + }, + ); }); describe("process CLI file cancellation", () => { @@ -139,6 +173,33 @@ describe("process CLI file cancellation", () => { }); describe("process CLI input diagnostics", () => { + it("keeps physical file I/O failures in the execution-error projection", async () => { + const path = "/selected/scenario.json"; + const ioFailure = new EvidenceFileError("read", "io", { + cause: Object.assign(new Error("disk I/O failed"), { code: "EIO" }), + path, + }); + const read = vi + .spyOn(jsonFiles, "readJsonFile") + .mockResolvedValue(err(ioFailure)); + + try { + const result = await captureProcessScenarioFile(path); + expect(result).toMatchObject({ + error: "Process command failed", + code: "execution_failure", + category: "execution_failure", + retryable: false, + details: { operation: "read", reason: "io", path }, + }); + expect(analysisCliErrorEnvelopeSchema.safeParse(result).success).toBe( + true, + ); + } finally { + read.mockRestore(); + } + }); + it("projects missing, malformed JSON, and invalid scenario files", async () => { const root = await createTestTempDirectory("rea-process-cli-diagnostics-"); const malformed = join(root, "malformed.json"); @@ -155,38 +216,94 @@ describe("process CLI input diagnostics", () => { ]), ); - expect( - await captureProcessScenarioFile(join(root, "missing.json")), - ).toEqual({ + const missing = await captureProcessScenarioFile( + join(root, "missing.json"), + ); + expect(missing).toMatchObject({ error: "Process command failed", + code: "invalid_request", category: "invalid_input", - message: expect.stringContaining( - `Process input file could not be read: ${join(root, "missing.json")} (ENOENT`, - ), + message: expect.stringContaining("Process input file could not be read:"), + retryable: false, + remediation: { + action: expect.stringContaining("Correct the listed fields"), + }, + details: { + issues: [ + { + path: ["input_file"], + reason: "invalid_value", + message: expect.stringContaining( + `Process input file could not be read: ${join(root, "missing.json")} (ENOENT`, + ), + }, + ], + }, }); - expect(await captureProcessScenarioFile(malformed)).toEqual({ + expect(analysisCliErrorEnvelopeSchema.safeParse(missing).success).toBe( + true, + ); + + const invalidJson = await captureProcessScenarioFile(malformed); + expect(invalidJson).toMatchObject({ error: "Process command failed", + code: "invalid_request", category: "invalid_input", message: expect.stringContaining( `Process input file is not valid UTF-8 JSON: ${malformed}`, ), + retryable: false, + remediation: { + action: expect.stringContaining("Correct the listed fields"), + }, + details: { + issues: [{ path: ["input_file"], reason: "invalid_value" }], + }, }); - expect(await captureProcessScenarioFile(invalidUtf8)).toEqual({ - error: "Process command failed", + expect(analysisCliErrorEnvelopeSchema.safeParse(invalidJson).success).toBe( + true, + ); + + const invalidUtf8Json = await captureProcessScenarioFile(invalidUtf8); + expect(invalidUtf8Json).toMatchObject({ + code: "invalid_request", category: "invalid_input", + retryable: false, message: expect.stringContaining( `Process input file is not valid UTF-8 JSON: ${invalidUtf8}`, ), + details: { issues: [{ path: ["input_file"], reason: "invalid_value" }] }, }); - expect(await captureProcessScenarioFile(invalid)).toEqual({ + expect( + analysisCliErrorEnvelopeSchema.safeParse(invalidUtf8Json).success, + ).toBe(true); + + const compareMissing = await compareProcessEvidenceFiles( + join(root, "missing-left.json"), + join(root, "missing-right.json"), + ); + expect(compareMissing).toMatchObject({ + error: "Process command failed", + code: "invalid_request", + category: "invalid_input", + retryable: false, + details: { + operation: "compare_process_captures", + issues: [{ path: ["input_file"], reason: "invalid_value" }], + }, + }); + expect( + analysisCliErrorEnvelopeSchema.safeParse(compareMissing).success, + ).toBe(true); + + expect(await captureProcessScenarioFile(invalid)).toMatchObject({ error: "Process command failed", code: "invalid_request", category: "invalid_input", message: "Analysis input is invalid. Check the arguments and try again.", - retryable: true, + retryable: false, remediation: { - action: - "Read the declared input schema or CLI usage for capture_process_scenario; if using MCP and the catalog may be stale, call tools/list. Correct the listed fields, then retry. Do not repeat the unchanged call.", + action: expect.stringContaining("Correct the listed fields"), }, recovery: { kind: "inspect_input_contract", @@ -200,7 +317,9 @@ describe("process CLI input diagnostics", () => { }), }); }); +}); +describe("process CLI file cleanup", () => { it.each([false, true])( "preserves the open-file owner when close fails (cancelled: %s)", async (cancelDuringClose) => { @@ -238,17 +357,21 @@ describe("process CLI input diagnostics", () => { } }, ); +}); +describe("process CLI UTF-8 input diagnostics", () => { it("names a leading byte-order mark instead of quoting it as a token", async () => { const path = join( await createTestTempDirectory("rea-process-cli-diagnostics-"), "scenario.json", ); await writeFile(path, '\uFEFF{"executable":"/bin/echo"}'); - expect(await captureProcessScenarioFile(path)).toEqual({ + expect(await captureProcessScenarioFile(path)).toMatchObject({ error: "Process command failed", + code: "invalid_request", category: "invalid_input", message: `Process input file is not valid UTF-8 JSON: ${path} (${JSON_BYTE_ORDER_MARK_MESSAGE}). Repair the file, then try again.`, + details: { issues: [{ path: ["input_file"], reason: "invalid_value" }] }, }); }); }); diff --git a/tests/boundary/filesystem/targetDiagnostics.test.ts b/tests/boundary/filesystem/targetDiagnostics.test.ts index 998657082a..5cf02b2452 100644 --- a/tests/boundary/filesystem/targetDiagnostics.test.ts +++ b/tests/boundary/filesystem/targetDiagnostics.test.ts @@ -249,8 +249,7 @@ describe("raw-binary load rejection diagnostics", () => { expect(projected).toMatchObject({ code: "invalid_request", category: "invalid_input", - // invalid_request is retryable: correcting the load argument succeeds. - retryable: true, + retryable: false, message: "Cannot open artifact: raw-binary arm entry_point 0x1 must be 4-byte aligned.", details: { diff --git a/tests/boundary/firmware/firmwareValidationProjection.test.ts b/tests/boundary/firmware/firmwareValidationProjection.test.ts new file mode 100644 index 0000000000..e06396dc85 --- /dev/null +++ b/tests/boundary/firmware/firmwareValidationProjection.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it, vi } from "vitest"; + +import { FirmwareAnalysisService } from "../../../src/application/firmware/FirmwareAnalysisService.js"; +import { analysisErrorProjectionSchema } from "../../../src/contracts/errorSchemas.js"; +import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; + +describe("firmware request validation projection", () => { + it.each([ + { + input: { + path: "/tmp/firmware.bin", + output_directory: "/tmp/out", + range: { offset: 5 }, + }, + missing: "length", + }, + { + input: { + path: "/tmp/firmware.bin", + output_directory: "/tmp/out", + range: { length: 5 }, + }, + missing: "offset", + }, + ] as const)( + "identifies missing range.$missing before provider admission", + async ({ input, missing }) => { + const execute = vi.fn(async () => { + throw new Error("Provider must not execute for invalid input"); + }); + const service = new FirmwareAnalysisService({ execute }); + const result = await service.execute("extract_firmware", input); + + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected request validation to fail"); + const projected = projectAnalysisError(result.error); + expect(analysisErrorProjectionSchema.safeParse(projected).success).toBe( + true, + ); + expect(projected).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + retryable: false, + details: { + operation: "extract_firmware", + issues: [ + { + path: ["input", "range", missing], + reason: "missing_argument", + }, + ], + }, + remediation: { + action: expect.stringContaining("Correct the listed fields"), + }, + }); + expect(execute).not.toHaveBeenCalled(); + }, + ); +}); diff --git a/tests/boundary/mcp/applicationEvidenceMcp.test.ts b/tests/boundary/mcp/applicationEvidenceMcp.test.ts index ba71b79213..9e49735b40 100644 --- a/tests/boundary/mcp/applicationEvidenceMcp.test.ts +++ b/tests/boundary/mcp/applicationEvidenceMcp.test.ts @@ -316,7 +316,7 @@ async function assertRejectedEvidenceReferences( error: { code: "invalid_request", category: "invalid_input", - retryable: true, + retryable: false, }, }); } diff --git a/tests/boundary/mcp/browserScenarioInputDiagnosticsMcp.test.ts b/tests/boundary/mcp/browserScenarioInputDiagnosticsMcp.test.ts index e39bbf7a61..59a2274eff 100644 --- a/tests/boundary/mcp/browserScenarioInputDiagnosticsMcp.test.ts +++ b/tests/boundary/mcp/browserScenarioInputDiagnosticsMcp.test.ts @@ -144,7 +144,7 @@ cliTest.for(["full", "compact"] as const)( expect(cliDiagnostic).toMatchObject({ code: "invalid_request", category: "invalid_input", - retryable: true, + retryable: false, remediation: { action: "Read the declared input schema or CLI usage for capture_browser_scenario; if using MCP and the catalog may be stale, call tools/list. Correct the listed fields, then retry. Do not repeat the unchanged call.", @@ -171,7 +171,7 @@ cliTest.for(["full", "compact"] as const)( expect(mcpError).toMatchObject({ code: "invalid_request", category: "invalid_input", - retryable: true, + retryable: false, recovery: { kind: "inspect_input_contract", operation: tool.name }, details: { operation: tool.name, diff --git a/tests/boundary/mcp/cleanupPrimaryProjection.test.ts b/tests/boundary/mcp/cleanupPrimaryProjection.test.ts index a6113bdc09..e108e7e4ee 100644 --- a/tests/boundary/mcp/cleanupPrimaryProjection.test.ts +++ b/tests/boundary/mcp/cleanupPrimaryProjection.test.ts @@ -67,7 +67,7 @@ it("restores the primary error after exact cleanup retry through the same MCP co expect(await call()).toMatchObject({ code: "invalid_request", category: "invalid_input", - retryable: true, + retryable: false, details: { issues: [{ path: ["serial"], reason: "invalid_value" }], cleanup_history: { diff --git a/tests/boundary/process/processCli.test.ts b/tests/boundary/process/processCli.test.ts index 4d4987436c..16a4468c56 100644 --- a/tests/boundary/process/processCli.test.ts +++ b/tests/boundary/process/processCli.test.ts @@ -19,6 +19,10 @@ import { createEvidence } from "../../../src/domain/evidence.js"; import { ProcessCaptureError } from "../../../src/process/capture/ProcessCaptureError.js"; import { PROCESS_PROVIDER } from "../../../src/domain/process/processEvidenceProvider.js"; import { INVESTIGATION_EXAMPLES } from "../../../src/contracts/investigationExamples.js"; +import { + analysisCliErrorEnvelopeSchema, + analysisErrorProjectionSchema, +} from "../../../src/contracts/errorSchemas.js"; const execFileAsync = promisify(execFile); const CLI_INTEGRATION_TIMEOUT_MS = 60_000; @@ -75,6 +79,71 @@ describe("documented process CLI workflow", () => { }); describe("process CLI errors", () => { + cliTest( + "returns canonical JSON errors for missing and malformed process files", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-process-cli-errors-"); + const malformed = join(root, "malformed.json"); + await writeFile(malformed, "not-json"); + const cases = [ + ["capture-process", join(root, "missing.json"), "--json"], + ["capture-process", malformed, "--json"], + [ + "compare-process-captures", + join(root, "missing-left.json"), + join(root, "missing-right.json"), + "--json", + ], + ] as const; + + for (const arguments_ of cases) { + const result = await cli.run({ + arguments: arguments_, + cwd: root, + }); + expect(result.exitCode).toBe(1); + expect( + analysisCliErrorEnvelopeSchema.safeParse(result.json).success, + ).toBe(true); + expect(result.json).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + retryable: false, + details: { + operation: expect.stringMatching( + /^(capture_process_scenario|compare_process_captures)$/u, + ), + issues: [ + expect.objectContaining({ + path: ["input_file"], + reason: "invalid_value", + }), + ], + }, + }); + } + + const full = await cli.run({ + arguments: [ + "capture-process", + join(root, "missing.json"), + "--json", + "--full-output", + ], + cwd: root, + }); + expect(full.exitCode).toBe(1); + expect(full.json).toMatchObject({ ok: false }); + if (typeof full.json !== "object" || full.json === null) + throw new Error("Expected a full-output CLI error document"); + const fullError = Reflect.get(full.json, "error"); + expect(analysisErrorProjectionSchema.safeParse(fullError).success).toBe( + true, + ); + }, + CLI_INTEGRATION_TIMEOUT_MS, + ); + it( "exits unsuccessfully without writing failure-shaped evidence", async () => { @@ -165,6 +234,7 @@ describe("process CLI evidence validation", () => { temporary_root: { state: "cleaned", reason: null }, }, }), + "compare_process_captures", ); expect(projected).toMatchObject({ code: "cleanup_incomplete", @@ -227,11 +297,21 @@ describe("process CLI evidence validation", () => { error: "Process command failed", category: "invalid_input", }); - expect(await compareProcessEvidenceFiles(unrelated, unrelated)).toEqual({ + const failure = await compareProcessEvidenceFiles(unrelated, unrelated); + expect(analysisCliErrorEnvelopeSchema.safeParse(failure).success).toBe( + true, + ); + expect(failure).toMatchObject({ error: "Process command failed", + code: "invalid_request", + retryable: false, category: "invalid_input", message: "Capture evidence is not from the current process-capture workflow. Create new capture evidence, then try again.", + details: { + operation: "compare_process_captures", + issues: [{ path: ["left"], reason: "invalid_value" }], + }, }); }); diff --git a/tests/composition/reverseEngineeringCommandPort.test.ts b/tests/composition/reverseEngineeringCommandPort.test.ts index e5c5a31421..95c4b47524 100644 --- a/tests/composition/reverseEngineeringCommandPort.test.ts +++ b/tests/composition/reverseEngineeringCommandPort.test.ts @@ -5,6 +5,8 @@ import { join } from "node:path"; import { expect, it, vi } from "vitest"; import { ReverseEngineeringService } from "../../src/application/reverse/ReverseEngineeringService.js"; +import { analysisErrorProjectionSchema } from "../../src/contracts/errorSchemas.js"; +import { projectAnalysisError } from "../../src/domain/analysisErrorProjection.js"; import { REVERSE_ENGINEERING_TOOL_CONTRACTS } from "../../src/contracts/reverseEngineeringToolContracts.js"; import { execFileOutput } from "../../src/process/ExecFileOutput.js"; @@ -112,3 +114,206 @@ it("does not reintroduce ambient variables absent from the captured provider env await rm(directory, { recursive: true, force: true }); } }); + +it("projects missing and non-file objdump targets with actionable path reasons", async () => { + const directory = await mkdtemp( + join(tmpdir(), "rea-reverse-invalid-target-"), + ); + const missing = join(directory, "missing.bin"); + const run = vi.fn(async () => ({ stdout: "", stderr: "" })); + const service = new ReverseEngineeringService({ + commands: { + objdump: () => ({ + executable: "unused-objdump", + arguments: [missing], + identity: { + id: "test.objdump", + name: "Test objdump", + version: null, + }, + }), + rizin: ({ path }) => ({ + executable: "unused-rizin", + arguments: [path], + identity: { id: "test.rizin", name: "Test Rizin", version: null }, + }), + }, + run, + }); + + try { + for (const [path, reason] of [ + [missing, "not found"], + [directory, "not a regular file"], + ] as const) { + const result = await service.inspectWithObjdump({ + path, + operation: "symbols", + }); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected target validation failure"); + const projection = projectAnalysisError(result.error); + expect(analysisErrorProjectionSchema.safeParse(projection).success).toBe( + true, + ); + expect(projection).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + retryable: false, + message: expect.stringContaining(path), + remediation: { + action: expect.stringContaining("Correct the listed fields"), + }, + details: { + operation: "inspect_with_objdump", + issues: [ + { + path: ["path"], + reason: "invalid_value", + message: expect.stringContaining(reason), + }, + ], + }, + }); + } + expect(run).not.toHaveBeenCalled(); + } finally { + await rm(directory, { recursive: true, force: true }); + } +}); + +it("keeps target preflight I/O failures out of invalid-input classification", async () => { + const directory = await mkdtemp( + join(tmpdir(), "rea-reverse-target-io-failure-"), + ); + const path = join(directory, "fixture.bin"); + await writeFile(path, Buffer.from([0x7f, 0x45, 0x4c, 0x46])); + const ioFailure = Object.assign(new Error("device I/O failure"), { + code: "EIO", + }); + const stat = vi.fn(async () => { + throw ioFailure; + }); + const run = vi.fn(async () => ({ stdout: "", stderr: "" })); + const service = new ReverseEngineeringService({ + stat, + commands: { + objdump: () => ({ + executable: "unused-objdump", + arguments: [path], + identity: { + id: "test.objdump", + name: "Test objdump", + version: null, + }, + }), + rizin: () => ({ + executable: "unused-rizin", + arguments: [path], + identity: { id: "test.rizin", name: "Test Rizin", version: null }, + }), + }, + run, + }); + + try { + const result = await service.inspectWithObjdump({ + path, + operation: "symbols", + }); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected target preflight I/O failure"); + expect(result.error).toMatchObject({ + _tag: "ProviderAdapterError", + cause: ioFailure, + diagnostics: { phase: "target_preflight", system_code: "EIO" }, + }); + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "execution_failure", + category: "execution_failure", + retryable: false, + details: { + provider_id: "test.objdump", + operation: "inspect_with_objdump", + diagnostics: { phase: "target_preflight", system_code: "EIO" }, + }, + }); + expect(run).not.toHaveBeenCalled(); + expect(stat).toHaveBeenCalledOnce(); + } finally { + await rm(directory, { recursive: true, force: true }); + } +}); + +it.each([ + { code: "ELOOP", reason: "symbolic-link loop" }, + { code: "ENAMETOOLONG", reason: "path is too long" }, +])( + "classifies selected target $code as correctable input", + async ({ code, reason }) => { + const directory = await mkdtemp( + join(tmpdir(), "rea-reverse-target-path-error-"), + ); + const path = join(directory, "fixture.bin"); + const pathFailure = Object.assign(new Error(`stat failed: ${code}`), { + code, + }); + const stat = vi.fn(async () => { + throw pathFailure; + }); + const run = vi.fn(async () => ({ stdout: "", stderr: "" })); + const service = new ReverseEngineeringService({ + stat, + commands: { + objdump: () => ({ + executable: "unused-objdump", + arguments: [path], + identity: { + id: "test.objdump", + name: "Test objdump", + version: null, + }, + }), + rizin: () => ({ + executable: "unused-rizin", + arguments: [path], + identity: { id: "test.rizin", name: "Test Rizin", version: null }, + }), + }, + run, + }); + + try { + const result = await service.inspectWithObjdump({ + path, + operation: "symbols", + }); + expect(result.ok).toBe(false); + if (result.ok) throw new Error(`Expected ${code} target path failure`); + expect(result.error).toMatchObject({ + _tag: "AnalysisInputError", + cause: pathFailure, + }); + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + retryable: false, + message: expect.stringContaining(path), + details: { + operation: "inspect_with_objdump", + issues: [ + { + path: ["path"], + reason: "invalid_value", + message: expect.stringContaining(reason), + }, + ], + }, + }); + expect(run).not.toHaveBeenCalled(); + expect(stat).toHaveBeenCalledOnce(); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }, +); diff --git a/tests/composition/reverseEngineeringUnknownFailures.test.ts b/tests/composition/reverseEngineeringUnknownFailures.test.ts new file mode 100644 index 0000000000..8d47a4e774 --- /dev/null +++ b/tests/composition/reverseEngineeringUnknownFailures.test.ts @@ -0,0 +1,65 @@ +import { stat, writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { expect, it, vi } from "vitest"; + +import { ReverseEngineeringService } from "../../src/application/reverse/ReverseEngineeringService.js"; +import { analysisErrorProjectionSchema } from "../../src/contracts/errorSchemas.js"; +import { projectAnalysisError } from "../../src/domain/analysisErrorProjection.js"; +import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; + +it.each([ + { stage: "stat", cause: null }, + { stage: "stat", cause: undefined }, + { stage: "command", cause: null }, + { stage: "command", cause: undefined }, +] as const)( + "preserves a $cause rejection from $stage without throwing while classifying it", + async ({ stage, cause }) => { + const root = await createTestTempDirectory("rea-reverse-unknown-failure-"); + const path = join(root, "input.bin"); + await writeFile(path, new Uint8Array([0x7f, 0x45, 0x4c, 0x46])); + const run = vi.fn(async () => { + throw cause; + }); + const service = new ReverseEngineeringService({ + stat: + stage === "stat" + ? async () => { + throw cause; + } + : stat, + run, + commands: { + objdump: () => ({ + executable: "unused-objdump", + arguments: [path], + identity: { id: "test.objdump", name: "Test objdump", version: null }, + }), + rizin: () => ({ + executable: "unused-rizin", + arguments: [path], + identity: { id: "test.rizin", name: "Test Rizin", version: null }, + }), + }, + }); + + const result = await service.inspectWithObjdump({ + path, + operation: "symbols", + }); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected injected boundary rejection"); + expect(result.error.cause).toBe(cause); + const projected = projectAnalysisError(result.error); + expect(analysisErrorProjectionSchema.safeParse(projected).success).toBe( + true, + ); + expect(projected.code).toBe( + stage === "stat" ? "execution_failure" : "capability_unavailable", + ); + if (stage === "stat") expect(run).not.toHaveBeenCalled(); + else expect(run).toHaveBeenCalledOnce(); + await service.close(); + }, +);