diff --git a/docs/cli.md b/docs/cli.md index 7136e00a51..e1575faa0e 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -561,6 +561,14 @@ be verified. See [MCP contracts](mcp-contracts.md) for session lifecycle and [Ghidra first-query deadlines](mcp-contracts.md#ghidra-first-query-deadlines-and-recovery) for import, client timeouts and recovery. +A one-shot command releases its provider session and REA runtime before it +prints a failure, so it omits the same-session reuse and +`update_analysis_settings` guidance an MCP connection receives. Its +`details.limit` reports `scope: "rea-invocation"` and +`applies_at: "next_invocation"`; set the named `environment` (or the per-call +`parameter`) for a new invocation and rerun. For example, a Ghidra decompile +deadline names `REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS`. + If one-shot analysis fails and closing its session also fails, the original error details remain inline and `details.cleanup_error` describes the later close failure. Each diagnostic retains its own reported resources; the later diff --git a/docs/design/limits-and-recovery.md b/docs/design/limits-and-recovery.md index 4ccf0c4da2..9eccda38fb 100644 --- a/docs/design/limits-and-recovery.md +++ b/docs/design/limits-and-recovery.md @@ -222,6 +222,25 @@ Error copying, attribution, redaction, and delivery must preserve these facts. Human-readable remediation explains the same failure; clients should not need to parse that sentence to identify an editable setting. +`scope` and `applies_at` describe a continuing caller, such as an MCP +connection. A one-shot invocation, such as a CLI command, has released its +provider session and exited its runtime before its caller reads the failure, +so its adapter projects with an explicit `"invocation"` caller lifetime. +`details.limit` then keeps every other field and reports +`scope: "rea-invocation"`, `applies_at: "next_invocation"` and the launch +`environment` seed when one exists. `agent_editable: true` still holds, because +the caller chooses that environment value or per-call `parameter` for the new +invocation; `rea_restart_required: false` means no running REA process needs a +restart. A producer keeps guidance that holds only while its caller retains the +session, such as "the Ghidra session remains usable" or a same-session runtime +update, in `continuingCallerGuidance`; the shared presentation adds it only for +a continuing caller. The producer's own message and remediation must stay true +after the session is released. A composed error records each embedded child +failure through `childFailureDiagnostics`, which snapshots it at composition +for both lifetimes when they differ, so cleanup history keeps its state and +the final projection selects the caller's form. Failures retained inside +Evidence keep the continuing form, so CLI and MCP Evidence stay identical. + Cache policy is part of enforcement. Successful reusable analysis can survive a budget update. Transient Ghidra decompile failures remain uncached so a raised deadline can recover on the same Program. Admission checks still apply to reused @@ -286,7 +305,9 @@ whether live repair is possible. ## What agents do Use `details.limit` for a direct runtime update when it names an editable -setting. Follow `remediation.action` for per-call, fixed and external controls. +setting with `scope: "rea-runtime"`. For `scope: "rea-invocation"`, set its +`environment` or `parameter` for a new invocation instead. +Follow `remediation.action` for per-call, fixed and external controls. Inspect settings only when bounds or application details are missing. Restart only when it says the value is read at startup. A sentence that a control does not raise the limit is not a request to change it. diff --git a/docs/installation.md b/docs/installation.md index 84720b4271..1378665340 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -1321,7 +1321,8 @@ fails with `provider_timeout` and that message; other Ghidra tools stay available. If the caller also cancels while the timeout settles, the session still closes and the message says so. Session reuse concerns an active MCP or library session; a standalone CLI command still closes its own session when it -finishes. In MCP, `ghidra.decompile_timeout_seconds` can override the launch +finishes, so its timeout names `REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS` for a new +invocation instead. In MCP, `ghidra.decompile_timeout_seconds` can override the launch value for newly accepted operations on the existing Program; explicit `null` removes the deadline. A transient timeout remains uncached so the same function can be retried after an update. See the diff --git a/docs/user-visible-errors.csv b/docs/user-visible-errors.csv index 2b81769b2a..b3b874a1df 100644 --- a/docs/user-visible-errors.csv +++ b/docs/user-visible-errors.csv @@ -119,7 +119,7 @@ Then rerun this installer.",implemented,verified by shell branch inspection 90,install.sh:version mismatch,Installed rea reports a different version,installed version ACTUAL does not match EXPECTED.,Low risk; gives exact integrity mismatch,Keep current copy,unchanged,verified by exact installer test 90.5,@modelcontextprotocol/server:tool input validation,Caller sends arguments that violate a declared tool schema,"Canonical invalid_request with the SDK diagnostic plus recovery.kind=inspect_input_contract and a bounded instruction to read the advertised schema or call tools/list",Keeps field/type correction while telling the agent how to recover; diagnostics over 4 KiB stay on the existing retained-error path,Normalize ordinary validation failures and retain oversized diagnostics,implemented,verified by real in-memory and stdio MCP client/server calls with wrong argument types 90.6,incur:unknown CLI command,Caller enters an unknown REA command,"'COMMAND' is not a command for 'rea'. Suggested command: rea --help",Low risk; identifies input and exact discovery action,Keep framework-generated copy,unchanged,verified by real packaged dispatcher invocation -90.7,src/ghidra/GhidraProviderClient.ts:projectPrimarySessionError,One procedure's decompile reaches REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS,"Ghidra decompilation of {entry} exceeded the {N}-second limit set by REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS. Only this procedure's decompilation was stopped; the Ghidra session remains usable.",Low risk; names the setting and keeps the session,Keep current copy,implemented,provider projection test and real-provider lane +90.7,src/ghidra/GhidraProviderClient.ts:projectPrimarySessionError,One procedure's decompile reaches REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS,"Ghidra decompilation of {entry} exceeded the {N}-second limit set by REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS. Only this procedure's decompilation was stopped. (MCP adds: The Ghidra session remains usable.)",Low risk; names the setting; only a continuing MCP caller is told the session remains usable,Keep current copy,implemented,"provider projection test, CLI/MCP fixture-bridge boundary test and real-provider lane" 90.8,src/ghidra/GhidraFileOffsetFailure.ts:ghidraFileOffsetUnavailable,"address_to_file_offset receives a well-formed address that is unmapped, uninitialized, without an authoritative target-file source, ambiguous or beyond the exact JSON integer range",{bridge reason}. REA establishes no single exact original-file offset for this well-formed address and does not derive or choose one.,Previous copy called the input invalid and directed schema repair for a valid address,"Keep the bridge reason as the headline, point to inspect_native_load_image, omit recovery.kind=inspect_input_contract and record details.issues[0].observed.file_mapping",implemented,provider projection tests; MCP and CLI analysis path through the real Ghidra client with a simulated bridge; real Ghidra not run 91,src/application/binaryDiagnostics/ByteEntropyService.ts (issues); src/domain/analysisErrorPresentation.ts (recovery),Invalid file or window,Expected a regular file.; Selected file could not be found.; Selected byte window starts beyond the end of the file.; Selected byte window extends beyond the end of the file.; File size exceeds exact numeric offset representation.,Invalid selection cannot produce complete entropy Evidence,"Read the declared input schema or CLI usage for inspect_byte_entropy; if using MCP and the catalog may be stale, call tools/list. Correct the listed fields, then retry. Do not repeat the unchanged call.",unchanged,Executed CLI/stdio MCP: path/offset/length refusals and tiny + system-binary controls; shared canonical recovery; service/MCP tests check classification and fields; remaining listed errors source-inspected 92,src/domain/analysisView/byteEntropyView.ts,Non-summary entropy view,Byte entropy Evidence supports the summary view; retrieve parent Evidence for the complete block series.,Unsupported view selection,Use the summary view or retrieve complete parent Evidence,implemented,typed input rejection diff --git a/src/application/BinaryTargetResolver.ts b/src/application/BinaryTargetResolver.ts index a8a4472327..3ce2e473f3 100644 --- a/src/application/BinaryTargetResolver.ts +++ b/src/application/BinaryTargetResolver.ts @@ -23,7 +23,7 @@ import { import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; import { jsonValueSchema } from "../domain/jsonValue.js"; -import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; +import { childFailureDiagnostics } from "../domain/analysisErrorProjection.js"; import { constants, type Stats } from "node:fs"; import { createHash } from "node:crypto"; import { access, realpath, stat, type FileHandle } from "node:fs/promises"; @@ -607,14 +607,12 @@ const targetFileCleanupError = ( : String(failure.cleanupCause), resources: [failure.path], }, - diagnostics: { - ...(primary === undefined + ...childFailureDiagnostics( + primary === undefined ? {} : { primary_error: { error: primary } }, + completedTarget === undefined ? {} - : { primary_error: projectAnalysisError(primary) }), - ...(completedTarget === undefined - ? {} - : { completed_target: completedTarget }), - }, + : { completed_target: completedTarget }, + ), }, ); bindRegularFileCleanup(failure, error); diff --git a/src/application/DirectAnalysis.ts b/src/application/DirectAnalysis.ts index 978790c4d4..b09b0f332a 100644 --- a/src/application/DirectAnalysis.ts +++ b/src/application/DirectAnalysis.ts @@ -35,7 +35,6 @@ import { AnalysisSnapshotMismatchError, type PublishedEvidenceFile, } from "../domain/evidenceErrors.js"; -import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import { access, stat } from "node:fs/promises"; import { absoluteCallerPath } from "../filesystem/callerPath.js"; @@ -74,6 +73,7 @@ import type { BinaryTargetError } from "../domain/configurationErrors.js"; import { directAnalysisCleanupFailure, projectDirectAnalysisFailure, + projectInvocationFailure, managedAnalysisCleanupFailure, withSessionCleanup, } from "./DirectAnalysisCleanup.js"; @@ -119,7 +119,10 @@ const ARTIFACT_ANALYSIS_OPERATION_SET: ReadonlySet = new Set( /** * Open one binary, execute one tool, and always release provider resources. * Unlike MCP mode, every CLI invocation is intentionally isolated and does not - * retain a target or provider client for a subsequent command. + * retain a target, provider session or runtime setting for a subsequent + * command. Failures are projected for that invocation lifetime, so they never + * offer same-session reuse or runtime updates; a new invocation's launch + * environment or arguments carry any changed limit. */ export const runDirectAnalysis = async ( dependencies: DirectAnalysisDependencies, @@ -855,6 +858,6 @@ const cliError = ( ): JsonValue => ({ error: "Analysis failed", ...(callerInputPath === undefined - ? projectAnalysisError(error) + ? projectInvocationFailure(error) : projectDirectAnalysisFailure(error, callerInputPath)), }); diff --git a/src/application/DirectAnalysisCleanup.ts b/src/application/DirectAnalysisCleanup.ts index f8d901e5fd..003b09e86e 100644 --- a/src/application/DirectAnalysisCleanup.ts +++ b/src/application/DirectAnalysisCleanup.ts @@ -1,5 +1,12 @@ import type { AnalysisError } from "../domain/analysisErrorBase.js"; -import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; +import { + childFailureDiagnostics, + projectAnalysisError, +} from "../domain/analysisErrorProjection.js"; +import { + analysisErrorUserMessage, + composedCallerGuidance, +} from "../domain/analysisErrorPresentation.js"; import { jsonValueSchema, type JsonValue } from "../domain/jsonValue.js"; import { ProviderAdapterError } from "../domain/providerAdapterError.js"; import { err, type Result } from "../domain/result.js"; @@ -11,12 +18,21 @@ import { withEvidenceCallerInputPath, } from "../domain/evidence.js"; +/** + * Project a one-shot failure for its invocation, which has released its + * provider session and runtime settings by the time the caller reads it. + */ +export const projectInvocationFailure = ( + error: AnalysisError, +): ReturnType => + projectAnalysisError(error, "invocation"); + /** Add the actual outer request path to authenticated partial Evidence. */ export const projectDirectAnalysisFailure = ( error: AnalysisError, callerInputPath: string, ): ReturnType => { - const projection = projectAnalysisError(error); + const projection = projectInvocationFailure(error); const observation = tryParseEvidence(projection.details?.partial_observation); return observation === undefined ? projection @@ -63,7 +79,7 @@ export const directAnalysisCleanupFailure = ( ): JsonValue => { const project = (failure: AnalysisError) => callerInputPath === undefined - ? projectAnalysisError(failure) + ? projectInvocationFailure(failure) : projectDirectAnalysisFailure(failure, callerInputPath); const cleanup = project(error); if (!result.ok) { @@ -114,18 +130,20 @@ export const managedAnalysisCleanupFailure = ( : failure, ); } - const cleanup = projectAnalysisError(error); const failure = new ProviderAdapterError( error instanceof ProviderAdapterError ? error.providerId : "rea", "close_binary", { cause: error, - remediationAction: cleanup.remediation.action, - cleanup: { reason: cleanup.message, resources: error.cleanupResources }, - diagnostics: { - cleanup_error: cleanup, - partial_observation: jsonValueSchema.parse(result.value), + ...composedCallerGuidance({ remediation: error }), + cleanup: { + reason: analysisErrorUserMessage(error, "invocation"), + resources: error.cleanupResources, }, + ...childFailureDiagnostics( + { cleanup_error: { error } }, + { partial_observation: jsonValueSchema.parse(result.value) }, + ), }, ); return err( diff --git a/src/application/DirectAnalysisStatus.ts b/src/application/DirectAnalysisStatus.ts index 31d624644f..8214f5b203 100644 --- a/src/application/DirectAnalysisStatus.ts +++ b/src/application/DirectAnalysisStatus.ts @@ -1,4 +1,3 @@ -import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { jsonObjectSchema, type JsonValue } from "../domain/jsonValue.js"; import { createServerIdentity } from "../serverIdentity.js"; import { silentLogger } from "../logger.js"; @@ -12,6 +11,7 @@ import { catalogIdentityForToolFamilies } from "../catalogIdentity.js"; import { compactPresentationForMcpInputSchemaProfile } from "../config/mcpInputSchemaProfile.js"; import { directAnalysisCleanupFailure, + projectInvocationFailure, withSessionCleanup, } from "./DirectAnalysisCleanup.js"; import { ok } from "../domain/result.js"; @@ -25,7 +25,7 @@ export const runSessionStatus = async ( logger: Logger = silentLogger, ): Promise => { const config = dependencies.readConfiguration(); - if (!config.ok) return { error: projectAnalysisError(config.error) }; + if (!config.ok) return { error: projectInvocationFailure(config.error) }; const session = await dependencies.createBinarySession(config.value, logger); return withSessionCleanup( session, diff --git a/src/application/JsonFiles.ts b/src/application/JsonFiles.ts index 1218671e41..93c104f902 100644 --- a/src/application/JsonFiles.ts +++ b/src/application/JsonFiles.ts @@ -20,7 +20,7 @@ import { AnalysisResourceConstraintError, } from "../domain/analysisErrorCore.js"; import { AnalysisError } from "../domain/analysisErrorBase.js"; -import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; +import { childFailureDiagnostics } from "../domain/analysisErrorProjection.js"; import { err, ok, type Result } from "../domain/result.js"; import { JsonInputPhysicalReadFailure, @@ -170,7 +170,7 @@ const readJsonFileInput = async ( cleanupPrimary: primary, ...(primary === undefined ? {} - : { diagnostics: { primary_error: projectAnalysisError(primary) } }), + : childFailureDiagnostics({ primary_error: { error: primary } })), }); bindRegularFileCleanup(cause, error); return err(error); @@ -272,7 +272,7 @@ const jsonFileCleanupError = ( ), cleanupPrimary: primary, cleanup: cleanup.cleanup, - diagnostics: { primary_error: projectAnalysisError(primary) }, + ...childFailureDiagnostics({ primary_error: { error: primary } }), }).bindCleanupDependencies([primary, cleanup]); }; diff --git a/src/cli/applicationCommands.ts b/src/cli/applicationCommands.ts index e7a64f5048..6a88982ae4 100644 --- a/src/cli/applicationCommands.ts +++ b/src/cli/applicationCommands.ts @@ -218,7 +218,7 @@ const registerJsonCommand = ({ ? result.value : { error: "Application workflow failed", - ...projectAnalysisError(result.error), + ...projectAnalysisError(result.error, "invocation"), }; }), }); diff --git a/src/cli/finishAnalysisCommand.ts b/src/cli/finishAnalysisCommand.ts index 74ca59058f..ba4b58943b 100644 --- a/src/cli/finishAnalysisCommand.ts +++ b/src/cli/finishAnalysisCommand.ts @@ -34,8 +34,10 @@ export const finishAnalysisCommand = async < ) => Value | undefined; } = {}, ): Promise>> => { + // The command releases its provider resources and runtime before the caller + // reads this result, so guidance needing a continuing session is omitted. const projectFailure = (error: AnalysisError) => - projectAnalysisError(options.redactFailure?.(error) ?? error); + projectAnalysisError(options.redactFailure?.(error) ?? error, "invocation"); let result: Result; let cleanup: AnalysisError | undefined; try { @@ -61,6 +63,7 @@ export const finishAnalysisCommand = async < result.error.capturedOutput !== undefined ? projectAnalysisErrorWithoutCapturedOutput( options.redactFailure?.(result.error) ?? result.error, + "invocation", ) : initialFailure; try { diff --git a/src/domain/analysisErrorBase.ts b/src/domain/analysisErrorBase.ts index d17099e6d0..cb0b73ba09 100644 --- a/src/domain/analysisErrorBase.ts +++ b/src/domain/analysisErrorBase.ts @@ -54,6 +54,27 @@ export interface AnalysisCleanupObservation { readonly resources: readonly string[]; } +/** + * How long the adapter delivering a failure keeps REA's runtime and provider + * session for its caller. An MCP connection continues: later calls reuse the + * session and can update runtime settings. A one-shot invocation, such as a + * CLI command, releases its provider session and exits before its caller can + * act; only a new invocation's launch settings or arguments apply. + */ +export type AnalysisCallerLifetime = "continuing" | "invocation"; + +/** + * Producer guidance that holds only for a continuing caller, kept apart from + * the request's outcome. The error's own message and remediation must remain + * true after an invocation has released the provider session. + */ +export interface AnalysisContinuingCallerGuidance { + /** Appended to the outcome, such as that the provider session remains usable. */ + readonly userMessage?: string; + /** Replaces the remediation, such as a same-session runtime setting update. */ + readonly remediationAction?: string; +} + /** Optional provider-neutral context that must survive typed error projection. */ export interface AnalysisErrorOptions extends ErrorOptions { /** Typed primary retained when a later lifecycle failure composes this error. */ @@ -61,6 +82,16 @@ export interface AnalysisErrorOptions extends ErrorOptions { /** Producer-owned, caller-safe facts retained through every typed projection. */ readonly diagnostics?: Readonly> | undefined; readonly limit?: AnalysisSettingLimit | undefined; + readonly continuingCallerGuidance?: + | AnalysisContinuingCallerGuidance + | undefined; + /** + * The same diagnostics recorded for an invocation caller, present only when + * an embedded child failure projects differently for that lifetime. + */ + readonly invocationDiagnostics?: + | Readonly> + | undefined; readonly capturedOutput?: AnalysisCapturedOutput | undefined; readonly cleanup?: AnalysisCleanupObservation | undefined; readonly partialObservation?: AnalysisPartialObservation | undefined; @@ -87,6 +118,12 @@ export abstract class AnalysisError extends Error { | undefined; #cleanupDependents: WeakRef[] = []; readonly diagnostics: Readonly> | undefined; + readonly continuingCallerGuidance: + | Readonly + | undefined; + readonly invocationDiagnostics: + | Readonly> + | undefined; readonly capturedOutput: AnalysisCapturedOutput | undefined; readonly cleanup: AnalysisCleanupObservation | undefined; cleanupIncomplete: boolean; @@ -103,6 +140,14 @@ export abstract class AnalysisError extends Error { options?.diagnostics === undefined ? undefined : structuredClone(options.diagnostics); + this.continuingCallerGuidance = + options?.continuingCallerGuidance === undefined + ? undefined + : Object.freeze({ ...options.continuingCallerGuidance }); + this.invocationDiagnostics = + options?.invocationDiagnostics === undefined + ? undefined + : structuredClone(options.invocationDiagnostics); this.capturedOutput = options?.capturedOutput === undefined ? undefined @@ -186,10 +231,27 @@ export abstract class AnalysisError extends Error { retainDiagnostics(diagnostics: Readonly>): this { Object.assign(this, { diagnostics: structuredClone({ ...this.diagnostics, ...diagnostics }), + ...(this.invocationDiagnostics === undefined + ? {} + : { + invocationDiagnostics: structuredClone({ + ...this.invocationDiagnostics, + ...diagnostics, + }), + }), }); return this; } + /** Diagnostics recorded for a caller with this lifetime. */ + diagnosticsFor( + callerLifetime: AnalysisCallerLifetime, + ): Readonly> | undefined { + return callerLifetime === "invocation" + ? (this.invocationDiagnostics ?? this.diagnostics) + : this.diagnostics; + } + retainSecondaryFailure( failure: AnalysisError, diagnostics?: Readonly>, diff --git a/src/domain/analysisErrorCleanup.ts b/src/domain/analysisErrorCleanup.ts index 0f1f74fb7d..81aa84dcdc 100644 --- a/src/domain/analysisErrorCleanup.ts +++ b/src/domain/analysisErrorCleanup.ts @@ -1,19 +1,18 @@ import { ProviderAdapterError } from "./providerAdapterError.js"; -import { - projectAnalysisError, - projectAnalysisErrorWithoutCapturedOutput, -} from "./analysisErrorProjection.js"; +import { childFailureDiagnostics } from "./analysisErrorProjection.js"; import type { AnalysisError } from "./analysisErrorBase.js"; -import { analysisErrorRemediationAction } from "./analysisErrorPresentation.js"; +import { + analysisErrorUserMessage, + composedCallerGuidance, +} from "./analysisErrorPresentation.js"; /** Preserve the original lifecycle failure alongside incomplete cleanup. */ export const analysisErrorWithCleanupFailure = ( primary: AnalysisError, cleanup: AnalysisError, operation = "close_binary", -): AnalysisError => { - const projected = projectAnalysisError(cleanup); - return new ProviderAdapterError( +): AnalysisError => + new ProviderAdapterError( cleanup instanceof ProviderAdapterError ? cleanup.providerId : "rea", operation, { @@ -23,9 +22,8 @@ export const analysisErrorWithCleanupFailure = ( "Analysis failed and cleanup remains incomplete.", { cause: primary }, ), - userMessage: projectAnalysisError(primary).message, // A retry offer belongs to the owner of the composed resources, if any. - remediationAction: analysisErrorRemediationAction(cleanup), + ...composedCallerGuidance({ message: primary, remediation: cleanup }), ...(primary.capturedOutput === undefined ? {} : { capturedOutput: primary.capturedOutput }), @@ -33,7 +31,9 @@ export const analysisErrorWithCleanupFailure = ( ? {} : { partialObservation: primary.partialObservation }), cleanup: { - reason: cleanup.cleanup?.reason ?? projected.message, + reason: + cleanup.cleanup?.reason ?? + analysisErrorUserMessage(cleanup, "invocation"), resources: [ ...new Set([ ...(primary.cleanupIncomplete ? primary.cleanupResources : []), @@ -41,13 +41,14 @@ export const analysisErrorWithCleanupFailure = ( ]), ], }, - diagnostics: { - primary_error: - primary.capturedOutput === undefined - ? projectAnalysisError(primary) - : projectAnalysisErrorWithoutCapturedOutput(primary), - cleanup_error: projected, - }, + ...childFailureDiagnostics({ + primary_error: { + error: primary, + ...(primary.capturedOutput === undefined + ? {} + : { capturedOutput: "omit" as const }), + }, + cleanup_error: { error: cleanup }, + }), }, ); -}; diff --git a/src/domain/analysisErrorPresentation.ts b/src/domain/analysisErrorPresentation.ts index 0959da3c64..76a4e0ff40 100644 --- a/src/domain/analysisErrorPresentation.ts +++ b/src/domain/analysisErrorPresentation.ts @@ -25,6 +25,7 @@ import { ResourceSnapshotMismatchError, } from "./evidenceErrors.js"; import { + HOPPER_STARTUP_TIMEOUT_REMEDIATION, HopperRemoteError, HopperProcessError, HopperStartError, @@ -38,6 +39,8 @@ import { fixedAllowanceRemediation } from "./fixedAllowanceRemediation.js"; import { primaryAnalysisError, resolvedAnalysisError, + type AnalysisCallerLifetime, + type AnalysisContinuingCallerGuidance, type AnalysisError, type AnalysisErrorTag, } from "./analysisErrorBase.js"; @@ -53,10 +56,17 @@ export const MCP_TRANSPORT_EVIDENCE_DISCOVERY = `Discover retained records with /** Next step when one MCP frame cannot carry a selected page of complete records. */ export const MCP_TRANSPORT_EVIDENCE_RECORDS = `Request fewer records with get_evidence_bundle detail: records, a smaller limit or an exact evidence_id filter; inspect compatible records in pages through inspect_analysis_view. A single record larger than one response needs export. ${MCP_TRANSPORT_EVIDENCE_EXPORT}`; +/** Name the next step that is true for how long the caller keeps REA's runtime and session. */ export const analysisErrorRemediationAction = ( error: AnalysisError, + callerLifetime: AnalysisCallerLifetime = "continuing", ): string => { error = resolvedAnalysisError(error); + const continuing = + callerLifetime === "continuing" + ? error.continuingCallerGuidance?.remediationAction + : undefined; + if (continuing !== undefined) return continuing; if ( (error instanceof ProviderAdapterError || error instanceof AnalysisCapabilityUnavailableError) && @@ -127,7 +137,7 @@ export const analysisErrorRemediationAction = ( return "Use an artifact within the capacity reported in details.detail. Changing output detail or retrying the unchanged input does not remove this boundary."; if (error instanceof HopperTimeoutError) return error.operation === undefined - ? "Review details.launcher and inspect Hopper for ongoing initial analysis or a pending loader, demo, or license dialog. For a large target, use update_analysis_settings with setting hopper.startup_timeout_ms and action set for the next startup attempt without restarting REA, or increase REA_HOPPER_STARTUP_TIMEOUT_MS for future REA processes. Allow at least that long in the MCP client. Resolve retained cleanup before reopening." + ? HOPPER_STARTUP_TIMEOUT_REMEDIATION : error.providerState === "busy" ? "Check binary_session.analysis_activity, wait for the active Hopper request to finish, then retry." : "Check binary_session for Hopper health, then retry the operation."; @@ -364,8 +374,68 @@ const STATIC_ERROR_CATEGORIES: Readonly< BinaryTargetError: "unavailable", }; -export const analysisErrorUserMessage = (error: AnalysisError): string => { +/** Describe the request's outcome, adding continuing-caller facts only for that caller. */ +export const analysisErrorUserMessage = ( + error: AnalysisError, + callerLifetime: AnalysisCallerLifetime = "continuing", +): string => { + const { outcome, continuing } = analysisErrorUserMessageParts(error); + return callerLifetime === "invocation" || continuing === undefined + ? outcome + : `${outcome} ${continuing}`; +}; + +/** The outcome every caller receives and the addition only a continuing caller does. */ +const analysisErrorUserMessageParts = ( + error: AnalysisError, +): { readonly outcome: string; readonly continuing?: string } => { error = resolvedAnalysisError(error); + const outcome = outcomeUserMessage(error); + const continuing = error.continuingCallerGuidance?.userMessage; + return continuing === undefined ? { outcome } : { outcome, continuing }; +}; + +/** + * Re-attribute one failure's message and another's remediation to a composed + * error, keeping the form each caller lifetime receives. + */ +export const composedCallerGuidance = (sources: { + readonly message?: AnalysisError; + readonly remediation: AnalysisError; +}): { + readonly userMessage?: string; + readonly remediationAction: string; + readonly continuingCallerGuidance?: AnalysisContinuingCallerGuidance; +} => { + const message = + sources.message === undefined + ? undefined + : analysisErrorUserMessageParts(sources.message); + const remediationAction = analysisErrorRemediationAction( + sources.remediation, + "invocation", + ); + const continuingRemediation = analysisErrorRemediationAction( + sources.remediation, + ); + const continuingCallerGuidance = { + ...(message?.continuing === undefined + ? {} + : { userMessage: message.continuing }), + ...(continuingRemediation === remediationAction + ? {} + : { remediationAction: continuingRemediation }), + }; + return { + ...(message === undefined ? {} : { userMessage: message.outcome }), + remediationAction, + ...(Object.keys(continuingCallerGuidance).length === 0 + ? {} + : { continuingCallerGuidance }), + }; +}; + +const outcomeUserMessage = (error: AnalysisError): string => { if (error instanceof AnalysisSnapshotMismatchError) return error.message; if (error instanceof AnalysisUnsupportedTargetError) return error.message; if (error instanceof AnalysisResourceConstraintError) return error.reason; diff --git a/src/domain/analysisErrorProjection.test.ts b/src/domain/analysisErrorProjection.test.ts index a88e632aae..921845a980 100644 --- a/src/domain/analysisErrorProjection.test.ts +++ b/src/domain/analysisErrorProjection.test.ts @@ -16,6 +16,8 @@ import { import { BrowserObservationError } from "./browserObservationError.js"; import { EvidenceFileError, EvidenceIntegrityError } from "./evidenceErrors.js"; import { + HOPPER_STARTUP_TIMEOUT_CONTINUING_REMEDIATION, + HOPPER_STARTUP_TIMEOUT_REMEDIATION, HopperProcessError, HopperRemoteError, HopperStartError, @@ -39,6 +41,12 @@ import { import { ProviderSelectionError } from "./providerSelectionError.js"; import { analysisErrorProjectionSchema } from "../contracts/errorSchemas.js"; import { nativeCallPartialObservationSchema } from "./native/nativeCallPartialObservation.js"; +import { analysisSettingLimit } from "./analysisSettingDefinitions.js"; +import { redactExplicitFailure } from "./explicitSensitiveFailure.js"; +import { redactExplicitText } from "./explicitSensitiveValues.js"; +import { analysisErrorWithCleanupFailure } from "./analysisErrorCleanup.js"; +import type { AnalysisCallerLifetime } from "./analysisErrorBase.js"; +import { ProviderCleanupError } from "./providerCleanupError.js"; describe("agent recovery metadata", () => { it("does not mark deterministic input validation as retryable", () => { @@ -1331,3 +1339,232 @@ it("preserves ordinary configuration recovery and projects a producer-owned sess }); expect(repairing.code).toBe("configuration_invalid"); }); + +const continuingOnly = { + userMessage: "The fixture session remains usable.", + remediationAction: "Update the fixture runtime setting on this session.", +}; + +describe("analysis error projection: caller lifetime", () => { + it.each([ + new AnalysisTimeoutError("procedure_pseudo_code", 5000, { + userMessage: "Only this request stopped.", + remediationAction: "Rerun with a longer launch deadline.", + continuingCallerGuidance: continuingOnly, + }), + new ProviderAdapterError("fixture", "procedure_pseudo_code", { + userMessage: "Only this request stopped.", + remediationAction: "Rerun with a longer launch deadline.", + continuingCallerGuidance: continuingOnly, + }), + new AnalysisResourceConstraintError( + "search_strings", + "cpu", + "Only this request stopped.", + null, + { + remediationAction: "Rerun with a longer launch deadline.", + continuingCallerGuidance: continuingOnly, + }, + ), + ])( + "states continuing-session guidance only to a continuing caller ($_tag)", + (error) => { + expect(projectAnalysisError(error)).toMatchObject({ + message: + "Only this request stopped. The fixture session remains usable.", + remediation: { + action: "Update the fixture runtime setting on this session.", + }, + }); + const invocation = projectAnalysisError(error, "invocation"); + expect(invocation).toMatchObject({ + code: projectAnalysisError(error).code, + message: "Only this request stopped.", + remediation: { action: "Rerun with a longer launch deadline." }, + }); + expect(analysisErrorUserMessage(error, "invocation")).toBe( + "Only this request stopped.", + ); + expect(analysisErrorRemediationAction(error, "invocation")).toBe( + "Rerun with a longer launch deadline.", + ); + }, + ); + + it("re-scopes a limit for a finished invocation and keeps its enforced facts", () => { + const limit = analysisSettingLimit( + "regex.matching_deadline_ms", + 250, + "setting", + ); + const error = new AnalysisResourceConstraintError( + "search_strings", + "cpu", + "Regex matching exceeded its deadline.", + null, + { limit }, + ); + expect(projectAnalysisError(error).details?.limit).toEqual(limit); + expect(projectAnalysisError(error, "invocation").details?.limit).toEqual({ + setting: "regex.matching_deadline_ms", + effective_value: 250, + source: "setting", + unit: "ms", + minimum: limit.minimum, + maximum: limit.maximum, + agent_editable: true, + rea_restart_required: false, + applies_at: "next_invocation", + scope: "rea-invocation", + environment: "REA_REGEX_MATCHING_DEADLINE_MS", + }); + // The error keeps its runtime facts for other projections. + expect(error.limit).toEqual(limit); + }); + + it("names a per-call parameter, not an environment, when a setting has no launch seed", () => { + const error = new AnalysisTimeoutError("inspect_wasm_artifact", 100, { + limit: analysisSettingLimit("wasm.command_timeout_ms", 100, "request"), + }); + expect(projectAnalysisError(error, "invocation").details?.limit).toEqual( + expect.objectContaining({ + setting: "wasm.command_timeout_ms", + source: "request", + parameter: "command_timeout_ms", + applies_at: "next_invocation", + scope: "rea-invocation", + }), + ); + expect( + projectAnalysisError(error, "invocation").details?.limit, + ).not.toHaveProperty("environment"); + }); + + it("delivers each lifetime's Hopper startup recovery", () => { + const error = new HopperTimeoutError( + 30_000, + undefined, + undefined, + "not_started", + "launch", + undefined, + { + limit: analysisSettingLimit( + "hopper.startup_timeout_ms", + 30_000, + undefined, + ), + }, + ); + expect(projectAnalysisError(error).remediation.action).toBe( + HOPPER_STARTUP_TIMEOUT_CONTINUING_REMEDIATION, + ); + const invocation = projectAnalysisError(error, "invocation"); + expect(invocation.remediation.action).toBe( + HOPPER_STARTUP_TIMEOUT_REMEDIATION, + ); + expect(invocation.details?.limit).toMatchObject({ + setting: "hopper.startup_timeout_ms", + effective_value: 30_000, + scope: "rea-invocation", + applies_at: "next_invocation", + environment: "REA_HOPPER_STARTUP_TIMEOUT_MS", + }); + // A busy request is not a startup: it carries no lifetime-specific text. + const busy = new HopperTimeoutError(30_000, "trace_feature", 42, "busy"); + expect(projectAnalysisError(busy, "invocation").remediation).toEqual( + projectAnalysisError(busy).remediation, + ); + }); +}); + +describe("analysis error projection: composed failures for a caller lifetime", () => { + it("projects a composed cleanup failure's children for the caller and keeps their history", () => { + const limit = analysisSettingLimit( + "regex.matching_deadline_ms", + 250, + "setting", + ); + const primary = new AnalysisTimeoutError("search_strings", 250, { + limit, + userMessage: "Only this request stopped.", + remediationAction: "Rerun with a longer launch deadline.", + continuingCallerGuidance: continuingOnly, + }); + const cleanup = new ProviderCleanupError( + "fixture", + ["fixture-process"], + { reason: "exit unconfirmed" }, + { remediationAction: "Close the retained fixture process." }, + ); + const composed = analysisErrorWithCleanupFailure( + primary, + cleanup, + ).bindCleanupDependencies([primary, cleanup]); + const child = (lifetime: AnalysisCallerLifetime) => + projectAnalysisError(composed, lifetime).details; + expect(child("invocation")).toMatchObject({ + limit: { scope: "rea-invocation" }, + diagnostics: { + primary_error: projectAnalysisError(primary, "invocation"), + cleanup_error: { code: "cleanup_incomplete" }, + }, + }); + expect(child("continuing")).toMatchObject({ + limit: { scope: "rea-runtime" }, + diagnostics: { primary_error: projectAnalysisError(primary) }, + }); + expect(projectAnalysisError(composed, "invocation").message).toBe( + "Only this request stopped.", + ); + expect(projectAnalysisError(composed).message).toBe( + `Only this request stopped. ${continuingOnly.userMessage}`, + ); + // After cleanup resolves, history keeps the composition-time snapshot. + cleanup.resolveCleanup(); + for (const lifetime of ["continuing", "invocation"] as const) + expect(child(lifetime)?.cleanup_history).toMatchObject({ + primary_error: projectAnalysisError(primary, lifetime), + cleanup_error: { code: "cleanup_incomplete" }, + }); + }); + + it("records one lifetime form when no child depends on the caller", () => { + const composed = analysisErrorWithCleanupFailure( + new AnalysisInputError("read_bytes", undefined, [ + { path: ["address"], reason: "invalid_format" }, + ]), + new ProviderCleanupError("fixture", ["fixture-process"], {}), + ); + expect(composed.invocationDiagnostics).toBeUndefined(); + expect(projectAnalysisError(composed, "invocation").details).toEqual( + projectAnalysisError(composed).details, + ); + }); + + it("keeps continuing guidance through redaction and redacts its text", () => { + const guidance = { + userMessage: "The session for secret-token remains usable.", + remediationAction: "Retry secret-token on this session.", + }; + const error = new AnalysisTimeoutError("procedure_pseudo_code", 5000, { + userMessage: "Only this request stopped.", + remediationAction: "Rerun with a longer launch deadline.", + continuingCallerGuidance: guidance, + }); + const redact = (text: string) => redactExplicitText(text, ["secret-token"]); + const redacted = redactExplicitFailure(error, ["secret-token"]); + expect(projectAnalysisError(redacted)).toMatchObject({ + message: `Only this request stopped. ${redact(guidance.userMessage)}`, + remediation: { action: redact(guidance.remediationAction) }, + }); + expect(JSON.stringify(projectAnalysisError(redacted))).not.toContain( + "secret-token", + ); + expect(projectAnalysisError(redacted, "invocation")).toMatchObject({ + message: "Only this request stopped.", + remediation: { action: "Rerun with a longer launch deadline." }, + }); + }); +}); diff --git a/src/domain/analysisErrorProjection.ts b/src/domain/analysisErrorProjection.ts index 1ed227078d..9211830bdf 100644 --- a/src/domain/analysisErrorProjection.ts +++ b/src/domain/analysisErrorProjection.ts @@ -50,23 +50,94 @@ import { providerRetryAction } from "./providerOperationHealth.js"; import { primaryAnalysisError, resolvedAnalysisError, + type AnalysisCallerLifetime, type AnalysisError, + type AnalysisErrorOptions, type AnalysisErrorTag, } from "./analysisErrorBase.js"; +import { invocationSettingLimit } from "./analysisSettingDefinitions.js"; -/** Project expected failures into exhaustive, secret-safe caller fields. */ +/** + * Project expected failures into exhaustive, secret-safe caller fields. The + * default describes a continuing caller, such as an MCP connection, and is + * also the adapter-independent form retained inside Evidence. An adapter that + * releases its provider session and runtime after this request, such as a + * one-shot CLI command, passes `"invocation"`. + */ export const projectAnalysisError = ( error: AnalysisError, -): AnalysisErrorProjection => projectError(error, true); + callerLifetime: AnalysisCallerLifetime = "continuing", +): AnalysisErrorProjection => projectError(error, true, callerLifetime); /** Cleanup history references output retained by its enclosing error owner. */ export const projectAnalysisErrorWithoutCapturedOutput = ( error: AnalysisError, -): AnalysisErrorProjection => projectError(error, false); + callerLifetime: AnalysisCallerLifetime = "continuing", +): AnalysisErrorProjection => projectError(error, false, callerLifetime); + +/** A child failure embedded in a composed error's diagnostics. */ +export interface AnalysisChildFailure { + readonly error: AnalysisError; + /** Omit output that the composing error already retains. */ + readonly capturedOutput?: "omit"; +} + +/** + * Record child failures as composition-time snapshots, so cleanup history + * keeps the state it had. The invocation form is recorded only when a child's + * projection depends on the caller lifetime; the final projection selects it. + */ +export const childFailureDiagnostics = ( + children: Readonly>, + diagnostics: Readonly> = {}, +): Pick => { + const entries = Object.entries(children); + const project = ( + { error, capturedOutput }: AnalysisChildFailure, + callerLifetime: AnalysisCallerLifetime, + ) => projectError(error, capturedOutput !== "omit", callerLifetime); + const continuing = { + ...diagnostics, + ...Object.fromEntries( + entries.map(([key, child]) => [key, project(child, "continuing")]), + ), + }; + const varying = entries.filter(([, { error }]) => + projectionDependsOnCallerLifetime(error), + ); + return varying.length === 0 + ? { diagnostics: continuing } + : { + diagnostics: continuing, + invocationDiagnostics: { + ...continuing, + ...Object.fromEntries( + varying.map(([key, child]) => [key, project(child, "invocation")]), + ), + }, + }; +}; + +/** Every lifetime-dependent input the projection reads lies on this chain. */ +const projectionDependsOnCallerLifetime = (error: AnalysisError): boolean => { + for ( + let node: AnalysisError | undefined = error; + node !== undefined; + node = node.cleanupPrimary + ) + if ( + node.limit !== undefined || + node.continuingCallerGuidance !== undefined || + node.invocationDiagnostics !== undefined + ) + return true; + return false; +}; const projectError = ( error: AnalysisError, includeCapturedOutput: boolean, + callerLifetime: AnalysisCallerLifetime, ): AnalysisErrorProjection => { const composed = error; error = resolvedAnalysisError(error); @@ -76,7 +147,9 @@ const projectError = ( diagnosticSource.cleanupPrimary !== undefined ) diagnosticSource = diagnosticSource.cleanupPrimary; - const diagnostics = diagnosticSource.diagnostics; + const diagnostics = diagnosticSource.diagnosticsFor(callerLifetime); + const cleanupHistory = + composed === error ? undefined : composed.diagnosticsFor(callerLifetime); const capturedOutput = composed.capturedOutput ?? error.capturedOutput; const partialObservation = composed.partialObservation ?? error.partialObservation; @@ -97,7 +170,14 @@ const projectError = ( const primaryDetails = errorDetails(primary); const details = { ...primaryDetails, - ...(primary.limit === undefined ? {} : { limit: { ...primary.limit } }), + ...(primary.limit === undefined + ? {} + : { + limit: + callerLifetime === "invocation" + ? { ...invocationSettingLimit(primary.limit) } + : { ...primary.limit }, + }), ...(cleanupReport === undefined ? {} : { cleanup_report: jsonValueSchema.parse(cleanupReport) }), @@ -109,9 +189,9 @@ const projectError = ( ? {} : { provider_diagnostics: primaryDetails.diagnostics }), }), - ...(composed === error || composed.diagnostics === undefined + ...(cleanupHistory === undefined ? {} - : { cleanup_history: composed.diagnostics }), + : { cleanup_history: cleanupHistory }), ...(error.cleanupIncomplete ? { cleanup: "incomplete", @@ -144,11 +224,11 @@ const projectError = ( cleanupRetry === undefined ? errorRecovery(error) : cleanupRetryRecovery(cleanupRetry); - const remediation = analysisErrorRemediationAction(error); + const remediation = analysisErrorRemediationAction(error, callerLifetime); return { code, category: analysisErrorCategory(error), - message: analysisErrorUserMessage(error), + message: analysisErrorUserMessage(error, callerLifetime), retryable: RETRYABLE_CODES.has(code) || (error instanceof AnalysisCapabilityUnavailableError && diff --git a/src/domain/analysisSettingDefinitions.ts b/src/domain/analysisSettingDefinitions.ts index 95712f7cbe..1dd1feef6c 100644 --- a/src/domain/analysisSettingDefinitions.ts +++ b/src/domain/analysisSettingDefinitions.ts @@ -1,5 +1,9 @@ import { z } from "zod"; -import type { AnalysisSettingId } from "./analysisSettings.js"; +import type { + AnalysisInvocationSettingLimit, + AnalysisSettingId, + AnalysisSettingLimit, +} from "./analysisSettings.js"; import { DEFAULT_ASAR_MAX_HEADER_BYTES, MAXIMUM_ASAR_MAX_HEADER_BYTES, @@ -225,8 +229,8 @@ export const analysisSettingValueSchema = (id: AnalysisSettingId) => { export const analysisSettingLimit = ( setting: AnalysisSettingId, effective_value: number | null, - source: import("./analysisSettings.js").AnalysisSettingLimit["source"], -): import("./analysisSettings.js").AnalysisSettingLimit => { + source: AnalysisSettingLimit["source"], +): AnalysisSettingLimit => { const definition = analysisSettingDefinition(setting); return { setting, @@ -244,3 +248,18 @@ export const analysisSettingLimit = ( : { parameter: definition.parameter }), }; }; + +/** Re-scope an enforced limit for a caller whose runtime ended with this invocation. */ +export const invocationSettingLimit = ({ + applies_at: _appliesAt, + scope: _scope, + ...facts +}: AnalysisSettingLimit): AnalysisInvocationSettingLimit => { + const environment = analysisSettingDefinition(facts.setting).environment; + return { + ...facts, + applies_at: "next_invocation", + scope: "rea-invocation", + ...(environment === undefined ? {} : { environment }), + }; +}; diff --git a/src/domain/analysisSettings.ts b/src/domain/analysisSettings.ts index 0b5e438c02..03476d1b46 100644 --- a/src/domain/analysisSettings.ts +++ b/src/domain/analysisSettings.ts @@ -36,3 +36,19 @@ export interface AnalysisSettingLimit { readonly required_value?: number; readonly parameter?: string; } + +/** + * The same limit for a caller whose REA runtime ended with one invocation. No + * runtime override can reach it; a new invocation reads the launch + * environment seed or the per-call argument. The caller can still choose that + * value, and no running REA process needs a restart. + */ +export interface AnalysisInvocationSettingLimit extends Omit< + AnalysisSettingLimit, + "applies_at" | "scope" +> { + readonly applies_at: "next_invocation"; + readonly scope: "rea-invocation"; + /** Launch-environment seed that a new invocation reads, when one exists. */ + readonly environment?: string; +} diff --git a/src/domain/explicitSensitiveFailure.ts b/src/domain/explicitSensitiveFailure.ts index 3237aa96f8..e55be448a7 100644 --- a/src/domain/explicitSensitiveFailure.ts +++ b/src/domain/explicitSensitiveFailure.ts @@ -145,9 +145,22 @@ const redactFailure = ( ? {} : { observed: visit(issue.observed) }), }); + const guidance = error.continuingCallerGuidance; const context = { cause: error, ...(error.limit === undefined ? {} : { limit: error.limit }), + ...(guidance === undefined + ? {} + : { + continuingCallerGuidance: { + ...(guidance.userMessage === undefined + ? {} + : { userMessage: text(guidance.userMessage) }), + ...(guidance.remediationAction === undefined + ? {} + : { remediationAction: text(guidance.remediationAction) }), + }, + }), ...(error.cleanupPrimary === undefined ? {} : { @@ -156,6 +169,9 @@ const redactFailure = ( ...(error.diagnostics === undefined ? {} : { diagnostics: diagnostics(error.diagnostics) }), + ...(error.invocationDiagnostics === undefined + ? {} + : { invocationDiagnostics: diagnostics(error.invocationDiagnostics) }), ...(error.capturedOutput === undefined ? {} : { diff --git a/src/domain/hopperErrors.ts b/src/domain/hopperErrors.ts index 88a577da9f..fd00aaedce 100644 --- a/src/domain/hopperErrors.ts +++ b/src/domain/hopperErrors.ts @@ -36,6 +36,14 @@ export type HopperLauncherOutcome = { readonly diagnostic_truncated: boolean; }; +/** Startup-timeout recovery that holds after the calling invocation has ended. */ +export const HOPPER_STARTUP_TIMEOUT_REMEDIATION = + "Review details.launcher and inspect Hopper for ongoing initial analysis or a pending loader, demo, or license dialog. For a large target, set REA_HOPPER_STARTUP_TIMEOUT_MS higher in the environment of a new REA invocation, then rerun the command; that invocation starts Hopper again. Resolve retained cleanup before rerunning."; + +/** Startup-timeout recovery for a caller whose REA runtime keeps running. */ +export const HOPPER_STARTUP_TIMEOUT_CONTINUING_REMEDIATION = + "Review details.launcher and inspect Hopper for ongoing initial analysis or a pending loader, demo, or license dialog. For a large target, use update_analysis_settings with setting hopper.startup_timeout_ms and action set for the next startup attempt without restarting REA, or increase REA_HOPPER_STARTUP_TIMEOUT_MS for future REA processes. Allow at least that long in the MCP client. Resolve retained cleanup before reopening."; + /** Hopper did not respond within the configured operation deadline. */ export class HopperTimeoutError extends HopperError { readonly _tag = "HopperTimeoutError"; @@ -53,7 +61,14 @@ export class HopperTimeoutError extends HopperError { ) { super( `Hopper ${operation === undefined ? "startup" : operation} timed out after ${String(timeoutMs)}ms`, - options, + operation === undefined + ? { + ...options, + continuingCallerGuidance: { + remediationAction: HOPPER_STARTUP_TIMEOUT_CONTINUING_REMEDIATION, + }, + } + : options, ); } } diff --git a/src/ghidra/GhidraProvider.behavior.test.ts b/src/ghidra/GhidraProvider.behavior.test.ts index cb92f8d6f3..6961b178cc 100644 --- a/src/ghidra/GhidraProvider.behavior.test.ts +++ b/src/ghidra/GhidraProvider.behavior.test.ts @@ -10,7 +10,10 @@ import { analysisProviderCandidate } from "../contracts/toolOutputSchemaPrimitiv import { ProcessOutputTokenScanner } from "../process/ProcessOutputTokenScanner.js"; import { GhidraNativeAotSnapshotOwner } from "./GhidraNativeAotSnapshotOwner.js"; import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; -import { resolvedAnalysisError } from "../domain/analysisErrorBase.js"; +import { + resolvedAnalysisError, + type AnalysisCallerLifetime, +} from "../domain/analysisErrorBase.js"; import { createDeferred } from "../../tests/fixtures/binarySession.js"; import { ProviderInspectionScope } from "../application/ProviderInspectionScope.js"; import { ok as javaProbeOk, err, ok } from "../domain/result.js"; @@ -49,7 +52,11 @@ import { elf, thinMach } from "../domain/binaryTarget.fixture.js"; import { GhidraProvider } from "./GhidraProvider.js"; import type { GhidraTargetSnapshot } from "./GhidraClient.js"; import { + GHIDRA_ANNOTATIONS_RETAINED, + GHIDRA_SESSION_RETAINED, + ghidraDecompileTimeoutRecovery, ghidraMissingProgramGuidance, + ghidraStartupTimeoutGuidance, type GhidraProviderClientFactory, } from "./GhidraProviderClient.js"; import type { GhidraHeadlessLauncher } from "./GhidraLauncher.js"; @@ -1491,7 +1498,7 @@ describe("Ghidra decompiler failure projection", () => { expect(projected).toMatchObject({ code: "execution_failure", retryable: false, - message: `${message}. Only the decompiler failed for this procedure; the Ghidra session remains usable.`, + message: `${message}. Only the decompiler failed for this procedure. ${GHIDRA_SESSION_RETAINED}`, details: { diagnostics: { remote_code: code } }, }); expect(projected.message).not.toContain("rea doctor"); @@ -1500,6 +1507,16 @@ describe("Ghidra decompiler failure projection", () => { "facet_availability.pseudocode", ); expect(projected.remediation.action).not.toContain("rea doctor"); + // A one-shot invocation has released the session it would reuse. + const invocation = projectAnalysisError(result.error, "invocation"); + expect(invocation).toMatchObject({ + code: "execution_failure", + remediation: projected.remediation, + }); + expect(projected.message).toBe( + `${invocation.message} ${GHIDRA_SESSION_RETAINED}`, + ); + expect(invocation.message).not.toContain(GHIDRA_SESSION_RETAINED); } }); @@ -1541,12 +1558,23 @@ describe("Ghidra decompiler failure projection", () => { expect(projected).toMatchObject({ code: "provider_timeout", retryable: true, - message: `${message}. Only this procedure's decompilation was stopped; the Ghidra session remains usable.`, + message: `${message}. Only this procedure's decompilation was stopped. ${GHIDRA_SESSION_RETAINED}`, recovery: { kind: "inspect_timeout_state", timeout_ms: 5000 }, - details: { diagnostics: { remote_code: code } }, + details: { + diagnostics: { remote_code: code }, + limit: { + setting: "ghidra.decompile_timeout_seconds", + effective_value: 5, + applies_at: "next_operation", + agent_editable: true, + scope: "rea-runtime", + rea_restart_required: false, + }, + }, }); const action = projected.remediation.action; expect(action).toContain("analyze_function"); + expect(action).toContain("update_analysis_settings"); expect(action).toContain("REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS"); expect(action).toContain("tool-call deadline"); expect(action).toContain("new Codex/Kimi registrations"); @@ -1557,6 +1585,135 @@ describe("Ghidra decompiler failure projection", () => { }); }); +describe("Ghidra decompile deadline for a finished invocation", () => { + const decompileDeadline = async ( + seconds: number, + existingProject: boolean, + cancelled = false, + ) => { + const environment: Record = { + REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS: String(seconds), + }; + if (existingProject) { + const root = await mkdtemp(join(tmpdir(), "rea-deadline-project-")); + onTestFinished(async () => { + await rm(root, { recursive: true, force: true }); + }); + await mkdir(join(root, "saved.rep")); + await writeFile(join(root, "saved.gpr"), "marker"); + environment.REA_GHIDRA_PROJECT_PATH = join(root, "saved.gpr"); + } + const code = "decompile_timed_out"; + const message = `Ghidra decompilation of 0x401000 exceeded the ${String(seconds)}-second configured decompilation limit`; + const observed = new GhidraSessionError( + "remote", + message, + { remote_code: code }, + { remoteCode: code }, + ); + const ghidra = provider( + installationHost(), + () => ({ + start: () => Promise.resolve(ok(sessionInfo())), + callTool: () => + Promise.resolve( + err( + cancelled + ? observed.withConcurrentCancellation({ reason: "deadline" }) + : observed, + ), + ), + close: () => Promise.resolve(ok(null)), + }), + environment, + ); + const client = await createElfClient(ghidra); + const result = await client.execute("procedure_pseudo_code", { + procedure: "main", + }); + if (result.ok) throw new Error("Expected the decompile deadline"); + return { + message, + continuing: projectAnalysisError(result.error), + invocation: projectAnalysisError(result.error, "invocation"), + }; + }; + + it.each([ + { seconds: 5, existingProject: false }, + { seconds: 2_147_483, existingProject: false }, + { seconds: 5, existingProject: true }, + ])( + "delivers each lifetime's recovery for a $seconds-second deadline (existing project: $existingProject)", + async ({ seconds, existingProject }) => { + const { message, continuing, invocation } = await decompileDeadline( + seconds, + existingProject, + ); + const recovery = ghidraDecompileTimeoutRecovery(seconds, existingProject); + for (const projected of [continuing, invocation]) + expect(projected).toMatchObject({ + code: "provider_timeout", + retryable: true, + recovery: { + kind: "inspect_timeout_state", + timeout_ms: seconds * 1000, + }, + details: { diagnostics: { remote_code: "decompile_timed_out" } }, + }); + expect(invocation.remediation.action).toBe(recovery.remediationAction); + expect(continuing.remediation.action).toBe( + recovery.continuingRemediationAction, + ); + // Only a saved project's rerun reopens it instead of importing again. + expect( + invocation.remediation.action.includes("REA_GHIDRA_PROJECT_PATH"), + ).toBe(existingProject); + expect(invocation.message.startsWith(message)).toBe(true); + expect(continuing.message).toBe( + `${invocation.message} ${GHIDRA_SESSION_RETAINED}`, + ); + const enforced = { + setting: "ghidra.decompile_timeout_seconds", + effective_value: seconds, + unit: "seconds", + minimum: 1, + maximum: 2_147_483, + agent_editable: true, + rea_restart_required: false, + }; + expect(invocation.details?.limit).toEqual({ + ...enforced, + applies_at: "next_invocation", + scope: "rea-invocation", + environment: "REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS", + }); + expect(continuing.details?.limit).toEqual({ + ...enforced, + applies_at: "next_operation", + scope: "rea-runtime", + }); + }, + ); + + it.each([false, true])( + "names what a session closed by cancellation repeats (existing project: %s)", + async (existingProject) => { + const { continuing, invocation } = await decompileDeadline( + 5, + existingProject, + true, + ); + // Cancellation closed the session, so no caller is told it remains. + expect(invocation.message).toBe(continuing.message); + expect(continuing.message).not.toContain(GHIDRA_SESSION_RETAINED); + expect(continuing.message.includes("REA_GHIDRA_PROJECT_PATH")).toBe( + existingProject, + ); + }, + ); +}); + describe("Ghidra failures during cancellation cleanup", () => { it.each( [ @@ -1628,6 +1785,17 @@ describe("Ghidra failures during cancellation cleanup", () => { cleanupFailed ? "cleanup_incomplete" : projectedCode, ); expect(projected.message).toContain(message); + // A one-shot invocation never inherits a session it has released, + // including inside a composed cleanup failure's child diagnostics. + const invocation = JSON.stringify( + projectAnalysisError(result.error, "invocation"), + ); + expect(invocation).toContain(message); + for (const retained of [ + GHIDRA_SESSION_RETAINED, + GHIDRA_ANNOTATIONS_RETAINED, + ]) + expect(invocation).not.toContain(retained); if (!cancelled) { expect(projected.message).toContain( remoteCode === "regex_stack_exhausted" @@ -3302,44 +3470,45 @@ it("projects Ghidra startup failure and its incomplete cleanup together", async }); }); -describe("Ghidra deadline guidance", () => { - const timedOut = async ( - phase: "start" | "request", - message: string, - environment: Readonly> = {}, - ) => { - const failure = new GhidraSessionError( - "timeout", - message, - { failure_kind: "timeout" }, - { timeoutMs: 250 }, - ); - const ghidra = provider( - installationHost(), - () => ({ - start: async () => - phase === "start" ? err(failure) : ok(sessionInfo()), - callTool: async () => err(failure), - close: async () => ok(null), - }), - environment, - ); - onTestFinished(async () => { - await ghidra.closeOwnedResources(); - }); - const target = executableTarget("elf", "x86_64"); - const profile = await ghidra.resolveAnalysisProfile(target); - if (!profile.ok || profile.value.profile === null) - throw new Error("Expected Ghidra analysis profile"); - const result = await ghidra - .createClient(target, profile.value.profile) - .execute("list_documents", {}); - if (result.ok) throw new Error("Expected a Ghidra timeout"); - return projectAnalysisError(result.error); - }; +/** Project one Ghidra deadline failure for the selected caller lifetime. */ +const ghidraTimedOut = async ( + phase: "start" | "request", + message: string, + environment: Readonly> = {}, + callerLifetime: AnalysisCallerLifetime = "continuing", +) => { + const failure = new GhidraSessionError( + "timeout", + message, + { failure_kind: "timeout" }, + { timeoutMs: 250 }, + ); + const ghidra = provider( + installationHost(), + () => ({ + start: async () => (phase === "start" ? err(failure) : ok(sessionInfo())), + callTool: async () => err(failure), + close: async () => ok(null), + }), + environment, + ); + onTestFinished(async () => { + await ghidra.closeOwnedResources(); + }); + const target = executableTarget("elf", "x86_64"); + const profile = await ghidra.resolveAnalysisProfile(target); + if (!profile.ok || profile.value.profile === null) + throw new Error("Expected Ghidra analysis profile"); + const result = await ghidra + .createClient(target, profile.value.profile) + .execute("list_documents", {}); + if (result.ok) throw new Error("Expected a Ghidra timeout"); + return projectAnalysisError(result.error, callerLifetime); +}; +describe("Ghidra deadline guidance", () => { it("names the startup deadline setting and its value when startup times out", async () => { - const projected = await timedOut( + const projected = await ghidraTimedOut( "start", "Ghidra startup deadline elapsed", ); @@ -3376,7 +3545,7 @@ describe("Ghidra deadline guidance", () => { }); await mkdir(join(root, "large.rep")); await writeFile(join(root, "large.gpr"), "marker"); - const projected = await timedOut( + const projected = await ghidraTimedOut( "start", "Ghidra startup deadline elapsed", { REA_GHIDRA_PROJECT_PATH: join(root, "large.gpr") }, @@ -3405,7 +3574,7 @@ describe("Ghidra deadline guidance", () => { }); it("keeps generic guidance for a bridge request deadline", async () => { - const projected = await timedOut( + const projected = await ghidraTimedOut( "request", "Ghidra bridge request timed out", ); @@ -3417,6 +3586,115 @@ describe("Ghidra deadline guidance", () => { }); }); +describe("Ghidra startup deadline for a finished invocation", () => { + const startupLimit = { + setting: "ghidra.startup_timeout_ms", + effective_value: 250, + agent_editable: true, + rea_restart_required: false, + }; + const invocationLimit = { + ...startupLimit, + applies_at: "next_invocation", + scope: "rea-invocation", + environment: "REA_GHIDRA_STARTUP_TIMEOUT_MS", + }; + + it.each([false, true])( + "delivers each lifetime's startup recovery (existing project: %s)", + async (existingProject) => { + const environment: Record = {}; + if (existingProject) { + const root = await mkdtemp(join(tmpdir(), "rea-timeout-project-")); + onTestFinished(async () => { + await rm(root, { recursive: true, force: true }); + }); + await mkdir(join(root, "large.rep")); + await writeFile(join(root, "large.gpr"), "marker"); + environment.REA_GHIDRA_PROJECT_PATH = join(root, "large.gpr"); + } + const elapsed = "Ghidra startup deadline elapsed"; + const invocation = await ghidraTimedOut( + "start", + elapsed, + environment, + "invocation", + ); + const continuing = await ghidraTimedOut("start", elapsed, environment); + const guidance = ghidraStartupTimeoutGuidance(250, existingProject); + expect(invocation).toMatchObject({ + code: "provider_timeout", + remediation: { action: guidance.remediationAction }, + details: { timeout_ms: 250, limit: invocationLimit }, + }); + expect(continuing).toMatchObject({ + code: "provider_timeout", + remediation: { + action: guidance.continuingCallerGuidance.remediationAction, + }, + details: { + timeout_ms: 250, + limit: { + ...startupLimit, + applies_at: "next_startup", + scope: "rea-runtime", + }, + }, + }); + // Existing-project mode runs no auto-analysis to disable. + expect( + invocation.remediation.action.includes("REA_GHIDRA_DISABLED_ANALYZERS"), + ).toBe(!existingProject); + }, + ); + + it("projects a composed cleanup failure's child startup timeout for the caller", async () => { + const cleanup = new ProviderCleanupError( + "ghidra", + ["owned-ghidra-process"], + { reason: "process cleanup unconfirmed" }, + ); + const ghidra = provider(installationHost(), () => ({ + start: async () => + err( + new GhidraSessionError( + "timeout", + "Ghidra startup deadline elapsed", + { failure_kind: "timeout" }, + { timeoutMs: 250, cleanupFailure: cleanup }, + ), + ), + callTool: async () => ok(null), + close: async () => err(cleanup), + })); + const result = await (await createElfClient(ghidra)).execute("health", {}); + if (result.ok) throw new Error("Expected the composed startup failure"); + const guidance = ghidraStartupTimeoutGuidance(250, false); + const child = (lifetime: AnalysisCallerLifetime) => + projectAnalysisError(result.error, lifetime).details?.diagnostics; + expect(projectAnalysisError(result.error, "invocation")).toMatchObject({ + code: "cleanup_incomplete", + details: { limit: invocationLimit }, + }); + expect(child("invocation")).toMatchObject({ + primary_error: { + code: "provider_timeout", + remediation: { action: guidance.remediationAction }, + details: { limit: invocationLimit }, + }, + cleanup_error: { code: "cleanup_incomplete" }, + }); + expect(child("continuing")).toMatchObject({ + primary_error: { + remediation: { + action: guidance.continuingCallerGuidance.remediationAction, + }, + details: { limit: { scope: "rea-runtime" } }, + }, + }); + }); +}); + describe("Ghidra missing project program", () => { const startFailure = async ( stdout: string, diff --git a/src/ghidra/GhidraProviderClient.ts b/src/ghidra/GhidraProviderClient.ts index dc8f3e5876..d884caf87d 100644 --- a/src/ghidra/GhidraProviderClient.ts +++ b/src/ghidra/GhidraProviderClient.ts @@ -3,6 +3,7 @@ import { ghidraProjectHistoryEvidence } from "./GhidraProjectHistory.js"; import type { Evidence, EvidenceLocation } from "../domain/evidence.js"; import { ghidraProjectCommitmentSchema } from "./GhidraProjectSnapshot.js"; import { analysisSettingLimit } from "../domain/analysisSettingDefinitions.js"; +import { MAX_GHIDRA_DECOMPILE_TIMEOUT_SECONDS } from "../domain/analysisResourceLimits.js"; import { z } from "zod"; import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { artifactFilePrimaryFacts } from "../artifacts/ArtifactFileOperation.js"; @@ -1145,21 +1146,33 @@ const projectPrimarySessionError = ( if (failure.kind === "remote" && failure.remoteCode === "decompile_failed") return new ProviderAdapterError("ghidra", operation, { ...context, - userMessage: `${failure.message.trim().replace(/\.$/u, "")}. ${ghidraSessionContinuationMessage( + ...ghidraRequestOutcome( failure, - "Only the decompiler failed for this procedure; the Ghidra session remains usable.", - )}`, + "Only the decompiler failed for this procedure.", + GHIDRA_SESSION_RETAINED, + limits.ghidraExistingProject !== undefined, + ), remediationAction: "Call analyze_function (CLI: `rea function`) for the same procedure. It returns assembly, callers, callees, references and basic blocks from the Ghidra listing and records this decompiler failure under facet_availability.pseudocode. Repeating the unchanged decompile request is not expected to succeed.", }); // A request-level deadline, not a provider failure: other operations stay - // available. Only promise session reuse when no caller cancellation closed - // the session while the timeout settled. + // available. Only a continuing caller can reuse the session, and only when + // no caller cancellation closed it while the timeout settled. if ( failure.kind === "remote" && failure.remoteCode === "decompile_timed_out" && limits.ghidraDecompileTimeoutSeconds !== undefined - ) + ) { + const outcome = ghidraRequestOutcome( + failure, + "Only this procedure's decompilation was stopped.", + GHIDRA_SESSION_RETAINED, + limits.ghidraExistingProject !== undefined, + ); + const recovery = ghidraDecompileTimeoutRecovery( + limits.ghidraDecompileTimeoutSeconds, + limits.ghidraExistingProject !== undefined, + ); return new AnalysisTimeoutError( operation, limits.ghidraDecompileTimeoutSeconds * 1000, @@ -1170,14 +1183,15 @@ const projectPrimarySessionError = ( limits.ghidraDecompileTimeoutSeconds, undefined, ), - userMessage: `${failure.message.trim().replace(/\.$/u, "")}. ${ghidraSessionContinuationMessage( - failure, - "Only this procedure's decompilation was stopped; the Ghidra session remains usable.", - )}`, - remediationAction: - "Use analyze_function (CLI: `rea function`) for listing facts. Call update_analysis_settings with setting ghidra.decompile_timeout_seconds and action set to raise the per-function deadline for the next operation on this Program. For CLI or future REA processes, set REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS. Set the MCP tool-call deadline to cover startup and the full call; earlier cancellation closes the session. `rea setup` estimates deadlines for new Codex/Kimi registrations and preserves existing positive values; edit existing deadlines yourself.", + userMessage: outcome.userMessage, + remediationAction: recovery.remediationAction, + continuingCallerGuidance: { + ...outcome.continuingCallerGuidance, + remediationAction: recovery.continuingRemediationAction, + }, }, ); + } if (failure.kind === "analysis_timeout") return new AnalysisTimeoutError( operation, @@ -1187,21 +1201,28 @@ const projectPrimarySessionError = ( if ( failure.kind === "remote" && failure.remoteCode === "regex_stack_exhausted" - ) + ) { + const outcome = ghidraRequestOutcome( + failure, + undefined, + GHIDRA_ANNOTATIONS_RETAINED, + limits.ghidraExistingProject !== undefined, + ); return new AnalysisResourceConstraintError( operation, "memory", - `${failure.message.trim().replace(/\.$/u, "")}. ${ghidraSessionContinuationMessage( - failure, - "The active analysis session and annotations remain available.", - )}`, + outcome.userMessage, null, { ...context, + ...(outcome.continuingCallerGuidance === undefined + ? {} + : { continuingCallerGuidance: outcome.continuingCallerGuidance }), remediationAction: "Retry this search in literal mode or simplify the regex.", }, ); + } if (failure.kind === "remote" && isGhidraInputFailureCode(failure.remoteCode)) return new AnalysisInputError(operation, context, [ { path: [], reason: "invalid_value", message: failure.message }, @@ -1438,15 +1459,68 @@ const projectObservedJvmMemoryFailure = ( ); }; -/** Interpret the queue's observed cancellation and cleanup outcome once. */ -const ghidraSessionContinuationMessage = ( +/** Outcome addition that only a caller keeping the Ghidra session receives. */ +export const GHIDRA_SESSION_RETAINED = "The Ghidra session remains usable."; + +/** The same addition when a failed search leaves session annotations intact. */ +export const GHIDRA_ANNOTATIONS_RETAINED = + "The active analysis session and annotations remain available."; + +/** What a new Ghidra session repeats in the configured startup mode. */ +const ghidraSessionRestart = (existingProject: boolean): string => + existingProject + ? "copies and opens the selected REA_GHIDRA_PROJECT_PATH project again, without import or auto-analysis" + : "repeats import and analysis"; + +/** + * Decompile-deadline recovery for each caller lifetime. A finished invocation + * can raise the deadline only for a new invocation; a continuing caller can + * update it for the next operation on the same Program. + */ +export const ghidraDecompileTimeoutRecovery = ( + seconds: number, + existingProject: boolean, +): { + readonly remediationAction: string; + readonly continuingRemediationAction: string; +} => ({ + remediationAction: `${ + seconds < MAX_GHIDRA_DECOMPILE_TIMEOUT_SECONDS + ? `Set REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS above ${String(seconds)} seconds (maximum ${String(MAX_GHIDRA_DECOMPILE_TIMEOUT_SECONDS)}), or unset it to leave decompilation unbounded,` + : `REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS is already at its maximum of ${String(seconds)} seconds; unset it to leave decompilation unbounded` + } in the environment of a new REA invocation, then rerun the command. That invocation starts a new Ghidra session and ${ghidraSessionRestart(existingProject)}. Use \`rea function\` for this procedure's listing facts without pseudocode.`, + continuingRemediationAction: + "Use analyze_function (CLI: `rea function`) for listing facts. Call update_analysis_settings with setting ghidra.decompile_timeout_seconds and action set to raise the per-function deadline for the next operation on this Program. For CLI or future REA processes, set REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS. Set the MCP tool-call deadline to cover startup and the full call; earlier cancellation closes the session. `rea setup` estimates deadlines for new Codex/Kimi registrations and preserves existing positive values; edit existing deadlines yourself.", +}); + +/** + * Interpret the queue's observed cancellation and cleanup outcome once. A + * session that survived this request stays usable only to a caller that keeps + * it, so that fact is continuing-caller guidance rather than the outcome. + */ +const ghidraRequestOutcome = ( failure: GhidraSessionError, - healthyMessage: string, -): string => { - if (failure.concurrentCancellation === undefined) return healthyMessage; - return failure.cleanupFailure === undefined - ? "The request was also cancelled before the failure settled, so REA closed the Ghidra session; the next request starts a new session and repeats import and analysis." - : "The request was also cancelled, and Ghidra session shutdown left cleanup unconfirmed. Close the target to retry retained cleanup before starting another Ghidra session."; + requestScope: string | undefined, + retainedSession: string, + existingProject: boolean, +): { + readonly userMessage: string; + readonly continuingCallerGuidance?: { readonly userMessage: string }; +} => { + const detail = `${failure.message.trim().replace(/\.$/u, "")}.`; + if (failure.concurrentCancellation === undefined) + return { + userMessage: + requestScope === undefined ? detail : `${detail} ${requestScope}`, + continuingCallerGuidance: { userMessage: retainedSession }, + }; + return { + userMessage: `${detail} ${ + failure.cleanupFailure === undefined + ? `The request was also cancelled before the failure settled, so REA closed the Ghidra session; the next request starts a new session and ${ghidraSessionRestart(existingProject)}.` + : "The request was also cancelled, and Ghidra session shutdown left cleanup unconfirmed. Close the target to retry retained cleanup before starting another Ghidra session." + }`, + }; }; const memoryCgroupSchema = z.object({ @@ -1522,15 +1596,25 @@ const projectSigkill = ( }; /** Name the settings that change Ghidra's startup deadline for the configured startup mode. */ -const ghidraStartupTimeoutGuidance = ( +export const ghidraStartupTimeoutGuidance = ( timeoutMs: number, existingProject: boolean, ) => { - const raise = `use update_analysis_settings with setting ghidra.startup_timeout_ms and action set above ${String(timeoutMs)}, or set REA_GHIDRA_STARTUP_TIMEOUT_MS in the launch environment for future REA processes`; - const retry = - "Export useful Evidence and resolve any retained cleanup before starting a new provider attempt. Runtime updates apply to the next startup attempt without restarting REA."; - const client = - "The MCP client's tool-call deadline must also allow startup and the query."; + const remediation = (continuing: boolean): string => { + const raise = continuing + ? `use update_analysis_settings with setting ghidra.startup_timeout_ms and action set above ${String(timeoutMs)}, or set REA_GHIDRA_STARTUP_TIMEOUT_MS in the launch environment for future REA processes` + : `set REA_GHIDRA_STARTUP_TIMEOUT_MS above ${String(timeoutMs)} in the environment of a new REA invocation, then rerun the command`; + const retry = continuing + ? "Export useful Evidence and resolve any retained cleanup before starting a new provider attempt. Runtime updates apply to the next startup attempt without restarting REA." + : "Resolve any retained cleanup before rerunning."; + const client = continuing + ? " The MCP client's tool-call deadline must also allow startup and the query." + : ""; + const reopen = continuing ? "Reopening" : "Each invocation"; + return existingProject + ? `Large projects can need longer: ${raise}. Existing-project mode does not run auto-analysis, so analyzer settings do not shorten this startup. ${retry}${client}` + : `Large binaries can need longer: ${raise}; REA_GHIDRA_DISABLED_ANALYZERS can skip selected expensive analyzers with reduced coverage. ${retry} ${reopen} repeats import from scratch; for repeated work on the same native ELF, PE or Mach-O binary on a POSIX host, analyze it once into a Ghidra project and set REA_GHIDRA_PROJECT_PATH to reuse that analysis.${client}`; + }; return { limit: analysisSettingLimit( "ghidra.startup_timeout_ms", @@ -1540,9 +1624,8 @@ const ghidraStartupTimeoutGuidance = ( userMessage: existingProject ? `Ghidra did not finish copying and opening the selected project within its configured ${String(timeoutMs)} ms startup deadline (REA_GHIDRA_STARTUP_TIMEOUT_MS).` : `Ghidra did not finish importing and analyzing the target within its configured ${String(timeoutMs)} ms startup deadline (REA_GHIDRA_STARTUP_TIMEOUT_MS).`, - remediationAction: existingProject - ? `Large projects can need longer: ${raise}. Existing-project mode does not run auto-analysis, so analyzer settings do not shorten this startup. ${retry} ${client}` - : `Large binaries can need longer: ${raise}; REA_GHIDRA_DISABLED_ANALYZERS can skip selected expensive analyzers with reduced coverage. ${retry} Reopening repeats import from scratch; for repeated work on the same native ELF, PE or Mach-O binary on a POSIX host, analyze it once into a Ghidra project and set REA_GHIDRA_PROJECT_PATH to reuse that analysis. ${client}`, + remediationAction: remediation(false), + continuingCallerGuidance: { remediationAction: remediation(true) }, }; }; diff --git a/src/kuna/KunaCommand.ts b/src/kuna/KunaCommand.ts index 26154e163a..463c505305 100644 --- a/src/kuna/KunaCommand.ts +++ b/src/kuna/KunaCommand.ts @@ -341,6 +341,28 @@ export const copyAndHash = async ( }; }; +/** + * The same run takes as long again, so only a longer deadline helps. Only a + * session that outlives this request can also read the later project export. + */ +export const kunaTimeoutRecovery = ( + setting: string, + subcommand: string, +): { + readonly remediationAction: string; + readonly continuingCallerGuidance?: { readonly remediationAction: string }; +} => { + const remediationAction = `Raise ${setting} in the environment that starts REA, then restart REA; it reads Kuna settings when it starts.`; + return subcommand === "decompile" + ? { + remediationAction, + continuingCallerGuidance: { + remediationAction: `${remediationAction} Within a session, pseudocode is also read from the project export once it finishes.`, + }, + } + : { remediationAction }; +}; + /** Map an owned-command failure to its typed error, keeping unproven cleanup owned. */ const commandFailure = ( cause: unknown, @@ -391,12 +413,7 @@ const commandFailure = ( primary = new AnalysisTimeoutError(operation, ms, { ...details, userMessage: `kuna ${subcommand} did not finish within ${setting} (${String(ms)} ms).`, - // The same run takes as long again, so only a longer deadline helps. - remediationAction: `Raise ${setting} in the environment that starts REA, then restart REA; it reads Kuna settings when it starts.${ - subcommand === "decompile" - ? " Within a session, pseudocode is also read from the project export once it finishes." - : "" - }`, + ...kunaTimeoutRecovery(setting, subcommand), }); } // The same input overflows the same fixed buffer; waiting cannot help. diff --git a/src/process/ArtifactAnalysisResources.ts b/src/process/ArtifactAnalysisResources.ts index 290c2bdff5..aebf66d041 100644 --- a/src/process/ArtifactAnalysisResources.ts +++ b/src/process/ArtifactAnalysisResources.ts @@ -366,7 +366,7 @@ export class ArtifactAnalysisResources { failures.flatMap((failure) => failure.cleanupResources), { reason: "One or more analysis runtime resources remain owned.", - failures: failures.map(projectAnalysisError), + failures: failures.map((failure) => projectAnalysisError(failure)), }, { operation: failures[0]?.operation ?? this.#artifacts.operation, diff --git a/src/process/RegexMatcher.ts b/src/process/RegexMatcher.ts index 43d3778ebf..028f2e1172 100644 --- a/src/process/RegexMatcher.ts +++ b/src/process/RegexMatcher.ts @@ -461,7 +461,7 @@ export const regexMatchingDeadlineError = ( new AnalysisResourceConstraintError( operation, "cpu", - `Regex matching exceeded its supervised deadline of ${String(deadline.deadlineMs)} ms (source ${deadline.source}). ${regexMatchingDeadlineRecovery(deadline)} The service remains available.`, + `Regex matching exceeded its supervised deadline of ${String(deadline.deadlineMs)} ms (source ${deadline.source}). ${regexMatchingDeadlineRecovery(deadline)}`, { matching_deadline_ms: deadline.deadlineMs, matching_deadline_source: deadline.source, @@ -472,7 +472,12 @@ export const regexMatchingDeadlineError = ( deadline.deadlineMs, deadline.source, ), - remediationAction: `${regexMatchingDeadlineRecovery(deadline)} Simplify the ECMAScript Unicode regex or use literal mode, then retry in the same session.`, + remediationAction: `${regexMatchingDeadlineRecovery(deadline)} Simplify the ECMAScript Unicode regex or use literal mode, then retry.`, + // A one-shot invocation has ended its session before the caller retries. + continuingCallerGuidance: { + userMessage: "The service remains available.", + remediationAction: `${regexMatchingDeadlineRecovery(deadline)} Simplify the ECMAScript Unicode regex or use literal mode, then retry in the same session.`, + }, }, ); diff --git a/src/providers/AnalysisClientFailure.test.ts b/src/providers/AnalysisClientFailure.test.ts index aebe5f4807..b03c27a764 100644 --- a/src/providers/AnalysisClientFailure.test.ts +++ b/src/providers/AnalysisClientFailure.test.ts @@ -2,6 +2,8 @@ import { expect, it } from "vitest"; import { ProviderCleanupError } from "../domain/providerCleanupError.js"; import type { FileOffsetPartialObservation } from "../domain/native/fileOffsetPartialObservation.js"; import { analysisClientFailureForOperation } from "./AnalysisClientFailure.js"; +import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; +import { regexMatchingDeadlineError } from "../process/RegexMatcher.js"; const observation: FileOffsetPartialObservation = { operation: "address_to_file_offset", @@ -62,3 +64,30 @@ it("does not promote ordinary partial evidence into a completed result", () => { expect(attributed.partialObservation).toBe(observation); expect(attributed.completedObservation).toBeUndefined(); }); + +it("keeps a regex deadline's continuing-session guidance when attributing it to a provider operation", () => { + const source = regexMatchingDeadlineError("search_procedures"); + const attributed = analysisClientFailureForOperation( + source, + "hopper", + "search_procedures", + ); + for (const lifetime of ["continuing", "invocation"] as const) { + const delivered = projectAnalysisError(attributed, lifetime); + const owned = projectAnalysisError(source, lifetime); + expect(delivered.message).toBe(owned.message); + expect(delivered.remediation).toEqual(owned.remediation); + expect(delivered.details?.limit).toEqual(owned.details?.limit); + } + // The two lifetimes receive different guidance from the same owner. + expect(projectAnalysisError(attributed).remediation).not.toEqual( + projectAnalysisError(attributed, "invocation").remediation, + ); + expect( + projectAnalysisError(attributed, "invocation").details?.limit, + ).toMatchObject({ + setting: "regex.matching_deadline_ms", + scope: "rea-invocation", + environment: "REA_REGEX_MATCHING_DEADLINE_MS", + }); +}); diff --git a/src/providers/AnalysisClientFailure.ts b/src/providers/AnalysisClientFailure.ts index 54cb303bc0..ef985844c2 100644 --- a/src/providers/AnalysisClientFailure.ts +++ b/src/providers/AnalysisClientFailure.ts @@ -48,18 +48,26 @@ export const analysisClientFailureContext = ( : {}), } : undefined; - const diagnostics = + const attribute = ( + recorded: Readonly> | undefined, + ): Readonly> | undefined => providerRequest === undefined - ? cause.diagnostics + ? recorded : { provider_request: providerRequest, - ...(cause.diagnostics === undefined - ? {} - : { provider_diagnostics: cause.diagnostics }), + ...(recorded === undefined ? {} : { provider_diagnostics: recorded }), }; + const diagnostics = attribute(cause.diagnostics); + const invocationDiagnostics = + cause.invocationDiagnostics === undefined + ? undefined + : attribute(cause.invocationDiagnostics); return { cause, ...(cause.limit === undefined ? {} : { limit: cause.limit }), + ...(cause.continuingCallerGuidance === undefined + ? {} + : { continuingCallerGuidance: cause.continuingCallerGuidance }), ...(cause.cleanupPrimary === undefined ? {} : { @@ -76,6 +84,7 @@ export const analysisClientFailureContext = ( ? {} : { capturedOutput: cause.capturedOutput }), ...(diagnostics === undefined ? {} : { diagnostics }), + ...(invocationDiagnostics === undefined ? {} : { invocationDiagnostics }), ...(cause.cleanupIncomplete ? { cleanup: { diff --git a/src/sqlite/SqliteDatabaseProvider.ts b/src/sqlite/SqliteDatabaseProvider.ts index f003f0a375..5a66ca4942 100644 --- a/src/sqlite/SqliteDatabaseProvider.ts +++ b/src/sqlite/SqliteDatabaseProvider.ts @@ -242,7 +242,11 @@ export class SqliteDatabaseProvider implements SqliteDatabasePort { new ProviderCleanupError( SQLITE_PROVIDER_IDENTITY.id, failures.flatMap((failure) => failure.cleanupResources), - { failures: failures.map(projectAnalysisError) }, + { + failures: failures.map((failure) => + projectAnalysisError(failure), + ), + }, { operation }, ).bindCleanupDependencies(failures), ); diff --git a/tests/boundary/filesystem/webArtifactReadOwnership.test.ts b/tests/boundary/filesystem/webArtifactReadOwnership.test.ts index b7cdbd20a4..eeb2a2a945 100644 --- a/tests/boundary/filesystem/webArtifactReadOwnership.test.ts +++ b/tests/boundary/filesystem/webArtifactReadOwnership.test.ts @@ -356,7 +356,9 @@ it("preserves both web cleanup owners' resources, diagnostics and recovery actio }, }, ); - const projected = [artifacts, resolver].map(projectAnalysisError); + const projected = [artifacts, resolver].map((failure) => + projectAnalysisError(failure), + ); await expect(service.close()).rejects.toMatchObject({ cleanupResources: ["artifact-root", "shared-resource", "resolver-root"], diagnostics: { failures: projected }, diff --git a/tests/boundary/mcp/ghidraDecompileDeadlineLifetime.test.ts b/tests/boundary/mcp/ghidraDecompileDeadlineLifetime.test.ts new file mode 100644 index 0000000000..78a23ca334 --- /dev/null +++ b/tests/boundary/mcp/ghidraDecompileDeadlineLifetime.test.ts @@ -0,0 +1,188 @@ +import { access } from "node:fs/promises"; + +import { expect, it } from "vitest"; +import { z } from "zod"; + +import { AnalysisProviderRegistry } from "../../../src/application/binary/AnalysisProviderRegistry.js"; +import { composeBinarySession } from "../../../src/application/binary/BinarySessionComposition.js"; +import { runDirectAnalysis } from "../../../src/application/DirectAnalysis.js"; +import type { DirectAnalysisDependencies } from "../../../src/application/DirectAnalysisDependencies.js"; +import { projectCliAnalysisResult } from "../../../src/cli/projectCliAnalysisResult.js"; +import { parseConfig } from "../../../src/config/parseConfig.js"; +import { analysisCliErrorEnvelopeSchema } from "../../../src/contracts/errorSchemas.js"; +import { parseEvidence } from "../../../src/domain/evidence.js"; +import { GhidraClient } from "../../../src/ghidra/GhidraClient.js"; +import { ghidraProfileInstallationHost } from "../../../src/ghidra/GhidraInstallation.fixture.js"; +import { GhidraProvider } from "../../../src/ghidra/GhidraProvider.js"; +import { + GHIDRA_SESSION_RETAINED, + ghidraDecompileTimeoutRecovery, +} from "../../../src/ghidra/GhidraProviderClient.js"; +import { silentLogger } from "../../../src/logger.js"; +import { createApplicationMcpHarness } from "../../fixtures/applicationMcpHarness.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; +import { GhidraFixtureLauncher } from "../providers/ghidra/ghidraClient.fixture.js"; + +// Only installation discovery and the Java bridge are simulated. The bridge +// fails one procedure_pseudo_code request with the production deadline code; +// requests cross the real client socket, provider projection and adapters. +const environment = { + GHIDRA_INSTALL_DIR: "/fixture/ghidra", + REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS: "1", +}; +const BRIDGE_MESSAGE = + "Ghidra decompilation of 0x1000 exceeded the 1-second configured decompilation limit (ghidra.decompile_timeout_seconds; launch seed REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS)"; + +const ghidraSession = (launcher: GhidraFixtureLauncher) => { + const config = parseConfig(environment); + if (!config.ok) throw config.error; + return composeBinarySession( + new AnalysisProviderRegistry([ + new GhidraProvider( + config.value, + silentLogger, + {}, + ghidraProfileInstallationHost, + (options) => + new GhidraClient({ + ...options, + platform: process.platform, + launcher, + }), + ), + ]), + ); +}; + +const expectReleased = async (launcher: GhidraFixtureLauncher) => { + expect(launcher.processes.length).toBeGreaterThan(0); + for (const root of launcher.runtimeRoots) + await expect(access(root)).rejects.toMatchObject({ code: "ENOENT" }); + for (const child of launcher.processes) + expect(child.exitCode !== null || child.signalCode !== null).toBe(true); +}; + +const cliFailure = z.object({ message: z.string() }); + +/** Facts about the enforced deadline, independent of the caller's lifetime. */ +const enforcedLimit = { + setting: "ghidra.decompile_timeout_seconds", + effective_value: 1, + unit: "seconds", + minimum: 1, + maximum: 2_147_483, + agent_editable: true, + rea_restart_required: false, +}; +/** A finished invocation can act only through a new invocation's launch value. */ +const invocationLimit = { + ...enforcedLimit, + applies_at: "next_invocation", + scope: "rea-invocation", + environment: "REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS", +}; + +const runId = z.object({ + result: z.object({ analysis_run: z.object({ run_id: z.string() }) }), +}); + +it("tells a finished CLI invocation to rerun with the launch setting, not to reuse its released session", async () => { + const launcher = new GhidraFixtureLauncher("decompile_timed_out"); + const create = () => ghidraSession(launcher); + const dependencies: DirectAnalysisDependencies = { + readConfiguration: () => parseConfig(environment), + createBinarySession: create, + createNativeDispatchBinarySession: create, + createArtifactBinarySession: create, + createNativeBinarySession: create, + createManagedBinarySession: create, + }; + const projected = analysisCliErrorEnvelopeSchema.parse( + projectCliAnalysisResult( + await runDirectAnalysis( + dependencies, + process.execPath, + "procedure_pseudo_code", + { procedure: "fixture_main" }, + { providerId: "ghidra" }, + ), + ), + ); + // The invocation has already released the Program the guidance describes. + await expectReleased(launcher); + const recovery = ghidraDecompileTimeoutRecovery(1, false); + expect(projected).toMatchObject({ + error: "Analysis failed", + code: "provider_timeout", + category: "timeout", + retryable: true, + recovery: { kind: "inspect_timeout_state", timeout_ms: 1000 }, + // The owner's invocation recovery for the effective 1-second deadline. + remediation: { action: recovery.remediationAction }, + details: { + diagnostics: { remote_code: "decompile_timed_out" }, + limit: invocationLimit, + }, + }); + const { message } = cliFailure.parse(projected); + expect(message.startsWith(BRIDGE_MESSAGE)).toBe(true); + expect(message).not.toContain(GHIDRA_SESSION_RETAINED); +}, 30_000); + +it("keeps same-session and runtime-update guidance for a continuing MCP connection", async () => { + const launcher = new GhidraFixtureLauncher("decompile_timed_out"); + const harness = await createApplicationMcpHarness({ + session: ghidraSession(launcher), + serverOptions: { logger: silentLogger }, + clientIdentity: { name: "ghidra-decompile-deadline", version: "1" }, + }); + const tool = (name: string, arguments_: Record = {}) => + harness.client.callTool({ name, arguments: arguments_ }); + try { + const opened = await tool("open_binary", { + path: process.execPath, + provider_id: "ghidra", + }); + expect(opened.isError).not.toBe(true); + const timedOut = parseMcpToolError( + await tool("procedure_pseudo_code", { procedure: "fixture_main" }), + ).error; + expect(timedOut).toMatchObject({ + code: "provider_timeout", + retryable: true, + recovery: { kind: "inspect_timeout_state", timeout_ms: 1000 }, + remediation: { + action: ghidraDecompileTimeoutRecovery(1, false) + .continuingRemediationAction, + }, + details: { + limit: { + ...enforcedLimit, + applies_at: "next_operation", + scope: "rea-runtime", + }, + }, + }); + expect(timedOut.details?.limit).not.toHaveProperty("environment"); + expect(timedOut.message.startsWith(BRIDGE_MESSAGE)).toBe(true); + expect(timedOut.message.endsWith(` ${GHIDRA_SESSION_RETAINED}`)).toBe(true); + // The claim is true here: the same provider run answers the next call. + const run = runId.parse((await tool("binary_session")).structuredContent) + .result.analysis_run.run_id; + const address = await tool("procedure_address", { + procedure: "fixture_main", + }); + expect(address.isError, JSON.stringify(address)).not.toBe(true); + expect(parseEvidence(address.structuredContent).normalized_result).toBe( + "0x1000", + ); + expect( + runId.parse((await tool("binary_session")).structuredContent).result + .analysis_run.run_id, + ).toBe(run); + expect(launcher.processes).toHaveLength(1); + } finally { + await harness.close(); + await expectReleased(launcher); + } +}, 30_000); diff --git a/tests/boundary/providers/ghidra/ghidraClient.fixture.ts b/tests/boundary/providers/ghidra/ghidraClient.fixture.ts index a0102a0954..045a9065db 100644 --- a/tests/boundary/providers/ghidra/ghidraClient.fixture.ts +++ b/tests/boundary/providers/ghidra/ghidraClient.fixture.ts @@ -27,6 +27,7 @@ type FixtureMode = | "future_id" | "analysis_timeout" | "remote_error" + | "decompile_timed_out" | "remote_startup_error" | "shutdown_error" | "invalid_shutdown_ack" diff --git a/tests/boundary/providers/kuna/kunaSingleFunctionRuns.test.ts b/tests/boundary/providers/kuna/kunaSingleFunctionRuns.test.ts index e7c22bec3f..468dd9329f 100644 --- a/tests/boundary/providers/kuna/kunaSingleFunctionRuns.test.ts +++ b/tests/boundary/providers/kuna/kunaSingleFunctionRuns.test.ts @@ -1,6 +1,7 @@ import { expect, it as test } from "vitest"; import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js"; +import { kunaTimeoutRecovery } from "../../../../src/kuna/KunaCommand.js"; import { fakeKunaCalls, @@ -311,6 +312,14 @@ it("names the setting that owns a single-function deadline", async () => { }, details: { timeout_ms: 300 }, }); + // A finished invocation cannot read a later export from this session. + const recovery = kunaTimeoutRecovery("REA_KUNA_TIMEOUT_MS", "decompile"); + expect( + projectAnalysisError(result.error, "invocation").remediation.action, + ).toBe(recovery.remediationAction); + expect(projectAnalysisError(result.error).remediation.action).toBe( + recovery.continuingCallerGuidance?.remediationAction, + ); } finally { expect(await client.close()).toEqual({ ok: true, value: null }); } diff --git a/tests/fixtures/fakeGhidra.mjs b/tests/fixtures/fakeGhidra.mjs index 99cd7310a8..67bcbfe77e 100644 --- a/tests/fixtures/fakeGhidra.mjs +++ b/tests/fixtures/fakeGhidra.mjs @@ -264,6 +264,24 @@ const handleRequest = (socket, server, request, state) => { return; } } + // The bridge's per-procedure deadline: only this request fails. + if ( + state.mode === "decompile_timed_out" && + request.method === "procedure_pseudo_code" + ) { + socket.write( + `${JSON.stringify({ + id: request.id, + ok: false, + error: { + code: "decompile_timed_out", + message: + "Ghidra decompilation of 0x1000 exceeded the 1-second configured decompilation limit (ghidra.decompile_timeout_seconds; launch seed REA_GHIDRA_DECOMPILE_TIMEOUT_SECONDS)", + }, + })}\n`, + ); + return; + } if (request.method === "address_to_file_offset") { socket.write( `${JSON.stringify({ id: request.id, ...fileOffsetReply(request.params.address) })}\n`,