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(); + }, +);