diff --git a/scripts/verify-real-hopper.mjs b/scripts/verify-real-hopper.mjs index 1e4bc9fc3..8a47c4207 100644 --- a/scripts/verify-real-hopper.mjs +++ b/scripts/verify-real-hopper.mjs @@ -296,6 +296,7 @@ try { { name: "list_documents", arguments: {} }, options, ); + requireSuccessfulTool(documents, "list_documents"); requireBridgeProgress(progressUpdates); const rejectedProcedure = await client.callTool( { @@ -381,9 +382,13 @@ try { await client.callTool({ name: "list_documents", arguments: {} }, options), "list_documents after target switch", ); + const expectedDocumentsAfterTargetSwitch = + process.platform === "linux" + ? firstDocuments.length + : firstDocuments.length + 1; if ( !Array.isArray(documentsAfterTargetSwitch) || - documentsAfterTargetSwitch.length !== firstDocuments.length + 1 + documentsAfterTargetSwitch.length !== expectedDocumentsAfterTargetSwitch ) throw new Error("A distinct target did not receive one Hopper document"); const reopenedTarget = await client.callTool( diff --git a/src/application/AnalysisProvider.ts b/src/application/AnalysisProvider.ts index e995d0a64..81bf19591 100644 --- a/src/application/AnalysisProvider.ts +++ b/src/application/AnalysisProvider.ts @@ -14,6 +14,7 @@ import { jsonValueSchema } from "../domain/jsonValue.js"; import type { ProgressReporter } from "./ProgressReporter.js"; import type { ProviderRejectionCode } from "../contracts/providerSelection.js"; import type { ProcessLineageObservation } from "../process/ProcessOwnership.js"; +import type { ProviderOperationHealth } from "../domain/providerOperationHealth.js"; export interface ExecutionOptions { readonly signal?: AbortSignal; @@ -99,6 +100,8 @@ export interface AnalysisClient extends AnalysisOperationPort { runtimeLineageSnapshots?(): readonly ProviderRuntimeLineageSnapshot[]; /** Current provider work that may outlive a timed-out or cancelled caller. */ requestActivitySnapshots?(): readonly ProviderRequestActivitySnapshot[]; + /** Current operational health for the active provider bridge. */ + operationHealthSnapshot?(): ProviderOperationHealth | undefined; /** Close with a typed result when the provider can verify cleanup. */ closeWithOutcome?( options?: Pick & { diff --git a/src/application/binary/BinarySession.ts b/src/application/binary/BinarySession.ts index fa01de124..6526d58cd 100644 --- a/src/application/binary/BinarySession.ts +++ b/src/application/binary/BinarySession.ts @@ -339,6 +339,8 @@ export class BinarySession this.#active?.client.runtimeLineageSnapshots?.() ?? [], requestActivitySnapshots: this.#active?.client.requestActivitySnapshots?.() ?? [], + providerOperationHealth: + this.#active?.client.operationHealthSnapshot?.() ?? null, }); } diff --git a/src/application/binary/BinarySessionStatus.ts b/src/application/binary/BinarySessionStatus.ts index bbffb8311..db4aef94e 100644 --- a/src/application/binary/BinarySessionStatus.ts +++ b/src/application/binary/BinarySessionStatus.ts @@ -1,5 +1,6 @@ import type { BinaryTarget } from "../../domain/binaryTarget.js"; import type { JsonValue } from "../../domain/jsonValue.js"; +import type { ProviderOperationHealth } from "../../domain/providerOperationHealth.js"; import type { CapabilityDescriptor, ProviderRequestActivitySnapshot, @@ -21,6 +22,7 @@ interface BinarySessionStatusInput { readonly runId: string | undefined; readonly runtimeLineageSnapshots: readonly ProviderRuntimeLineageSnapshot[]; readonly requestActivitySnapshots: readonly ProviderRequestActivitySnapshot[]; + readonly providerOperationHealth?: ProviderOperationHealth | null; } /** Project internal provider routing state into the caller-visible session status. */ @@ -32,6 +34,7 @@ export const binarySessionStatus = ({ runId, runtimeLineageSnapshots, requestActivitySnapshots, + providerOperationHealth, }: BinarySessionStatusInput): JsonValue => { const configuredProvider = router.configuredIdentity(); const provider = providerSummary(configuredProvider); @@ -85,6 +88,9 @@ export const binarySessionStatus = ({ process_lineage: processLineageStatus(runtimeLineageSnapshots), }, analysis_activity: requestActivityStatus(requestActivitySnapshots), + provider_operation_health: providerOperationHealthStatus( + providerOperationHealth, + ), }; return target === undefined ? { open: false, ...common } @@ -203,3 +209,20 @@ const capabilityStatus = ( }, limitations: [...descriptor.limitations], }); + +const providerOperationHealthStatus = ( + health: ProviderOperationHealth | null | undefined, +): JsonValue => { + if (health === undefined || health === null) return null; + return { + state: health.state, + stage: health.stage, + retry_action: health.retryAction, + exit_code: health.exitCode, + requests: health.requests.map((request) => ({ + request_id: request.requestId, + operation: request.operation, + stage: request.stage, + })), + }; +}; diff --git a/src/application/binary/CompositeProvider.ts b/src/application/binary/CompositeProvider.ts index 4fd6e7923..95bc1affd 100644 --- a/src/application/binary/CompositeProvider.ts +++ b/src/application/binary/CompositeProvider.ts @@ -152,6 +152,18 @@ export class CompositeProvider implements AnalysisProvider { .sort((left, right) => left.provider.id.localeCompare(right.provider.id), ), + operationHealthSnapshot: () => { + for (const client of clients.values()) { + const snapshot = client.operationHealthSnapshot?.(); + if (snapshot !== undefined && snapshot.state !== "idle") + return snapshot; + } + for (const client of clients.values()) { + const snapshot = client.operationHealthSnapshot?.(); + if (snapshot !== undefined) return snapshot; + } + return undefined; + }, closeWithOutcome, close: async () => { await closeWithOutcome(); diff --git a/src/application/binary/LazyAnalysisProvider.ts b/src/application/binary/LazyAnalysisProvider.ts index f66d9694c..30f7d3df8 100644 --- a/src/application/binary/LazyAnalysisProvider.ts +++ b/src/application/binary/LazyAnalysisProvider.ts @@ -3,6 +3,7 @@ import type { BinaryTarget } from "../../domain/binaryTarget.js"; import { AnalysisCancelledError } from "../../domain/analysisErrorCore.js"; import { AnalysisError } from "../../domain/analysisErrorBase.js"; import { ProviderAdapterError } from "../../domain/providerAdapterError.js"; +import type { ProviderOperationHealth } from "../../domain/providerOperationHealth.js"; import { err } from "../../domain/result.js"; import { ABORTED, waitForAbortable } from "./AbortablePromise.js"; import type { @@ -115,6 +116,10 @@ class LazyAnalysisClient implements AnalysisClient { return this.#client?.requestActivitySnapshots?.() ?? []; } + operationHealthSnapshot(): ProviderOperationHealth | undefined { + return this.#client?.operationHealthSnapshot?.(); + } + async close(): Promise { this.#closed = true; if (this.#loading === undefined) return; diff --git a/src/contracts/toolOutputSchemaPrimitives.ts b/src/contracts/toolOutputSchemaPrimitives.ts index 9e064c2c3..d5d441fab 100644 --- a/src/contracts/toolOutputSchemaPrimitives.ts +++ b/src/contracts/toolOutputSchemaPrimitives.ts @@ -14,6 +14,11 @@ import { PROVIDER_REJECTION_CODES, type ProviderRejectionCode, } from "./providerSelection.js"; +import { + PROVIDER_FAILURE_STAGES, + PROVIDER_OPERATION_STATES, + PROVIDER_RETRY_ACTIONS, +} from "../domain/providerOperationHealth.js"; import { analysisErrorProjectionSchema } from "./errorSchemas.js"; import { prefixedDigestSchema } from "./../domain/digests.js"; @@ -169,6 +174,22 @@ export const analysisActivity = z.object({ ), }); +const providerOperationRequest = z.object({ + request_id: z.number().int().min(1), + operation: z.string().min(1), + stage: z.enum(PROVIDER_FAILURE_STAGES), +}); + +export const providerOperationHealth = z + .object({ + state: z.enum(PROVIDER_OPERATION_STATES), + stage: z.enum(PROVIDER_FAILURE_STAGES).nullable(), + retry_action: z.enum(PROVIDER_RETRY_ACTIONS).nullable(), + exit_code: z.number().int().nullable(), + requests: z.array(providerOperationRequest), + }) + .nullable(); + const providerRejectionCode: z.ZodType = z.enum( PROVIDER_REJECTION_CODES, ); @@ -315,6 +336,7 @@ export const sessionProvider = z capabilities: z.array(providerCapability), analysis_run: analysisRun, analysis_activity: analysisActivity, + provider_operation_health: providerOperationHealth, analysis_provider_binding: z .object({ provider: providerIdentity, diff --git a/src/domain/analysisErrorPresentation.ts b/src/domain/analysisErrorPresentation.ts index f8684605a..5b10a5fd8 100644 --- a/src/domain/analysisErrorPresentation.ts +++ b/src/domain/analysisErrorPresentation.ts @@ -25,6 +25,7 @@ import { } from "./hopperErrors.js"; import { ProviderSelectionError } from "./providerSelectionError.js"; import { UnknownRegistryError } from "./unknownRegistryError.js"; +import { providerRetryAction } from "./providerOperationHealth.js"; import { type AnalysisError, type AnalysisErrorTag, @@ -49,7 +50,7 @@ export const analysisErrorRemediationAction = ( ? "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."; if (error instanceof HopperProcessError) - return "Check binary_session provider health. Restart the owned Hopper process only when it has stopped."; + return hopperProcessRemediation(error); if (error instanceof HopperStartError) return error.ownerRunId === undefined ? "Check the Hopper launcher and target details, then retry opening the target." @@ -191,8 +192,7 @@ const hopperErrorUserMessage = (error: AnalysisError): string | undefined => { ? `Hopper timed out during ${request} while the provider remained busy. Check binary_session.analysis_activity, wait for the active request to finish, then retry.` : `Hopper timed out during ${request} before it started. Check binary_session for provider health, then retry.`; } - if (error instanceof HopperProcessError) - return `Hopper stopped during ${error.operation ?? "connection"}${error.exitCode === null ? "; its exit was not observed" : ` with exit code ${String(error.exitCode)}`}.${error.userMessage === undefined ? "" : ` ${error.userMessage}`} Check binary_session provider health before retrying.`; + if (error instanceof HopperProcessError) return hopperProcessMessage(error); if (error instanceof HopperStartError) return ( error.userMessage ?? @@ -203,6 +203,40 @@ const hopperErrorUserMessage = (error: AnalysisError): string | undefined => { return undefined; }; +const RESTART_OWNED_PROVIDER = + "Restart the owned provider by calling close_binary, then open the target again and retry."; + +const hopperProcessRemediation = (error: HopperProcessError): string => { + if (error.failureCode !== undefined && error.userMessage !== undefined) + return error.userMessage; + if (providerRetryAction(error.providerState) === "restart_provider") + return RESTART_OWNED_PROVIDER; + return "Read binary_session.provider_operation_health for this request before retrying or restarting the owned provider."; +}; + +const hopperProcessMessage = (error: HopperProcessError): string => { + const stage = error.stage; + const where = error.operation ?? stage; + const request = + error.requestId === undefined + ? "" + : ` Request id ${String(error.requestId)}.`; + const startup = + error.userMessage === undefined ? "" : ` ${error.userMessage}`; + if (error.providerState === "exited") { + const exit = + error.exitCode === null + ? "no exit code was observed" + : `exit code ${String(error.exitCode)}`; + const recovery = + error.failureCode === undefined ? ` ${RESTART_OWNED_PROVIDER}` : ""; + return `Hopper exited during ${where} (${exit}).${startup}${recovery}${request}`; + } + if (error.providerState === "unreachable") + return `Hopper became unreachable during ${where}; its process exit was not observed. ${RESTART_OWNED_PROVIDER}${startup}${request}`; + return `Hopper failed during ${where} and REA could not determine whether the provider is busy, exited, or unreachable.${startup} Read binary_session.provider_operation_health for this request before retrying or restarting.${request}`; +}; + const standardErrorMessage = (tag: AnalysisErrorTag): string | undefined => { if (UNREADABLE_OUTPUT_TAGS.has(tag)) return "Analysis returned an unreadable result. Retry once; if it continues, run `rea doctor`."; diff --git a/src/domain/analysisErrorProjection.test.ts b/src/domain/analysisErrorProjection.test.ts index 3bf294386..99c397018 100644 --- a/src/domain/analysisErrorProjection.test.ts +++ b/src/domain/analysisErrorProjection.test.ts @@ -390,3 +390,40 @@ describe("analysis error projection: caller contract", () => { }); }); }); + +describe("analysis error projection: operational diagnostics", () => { + it("distinguishes an observed Hopper exit from an undetermined provider failure", () => { + const exited = projectAnalysisError( + new HopperProcessError(7, undefined, "search_strings", 4), + ); + expect(exited).toMatchObject({ + code: "provider_unavailable", + retryable: true, + details: { + stage: "analysis", + provider_state: "exited", + retry_action: "restart_provider", + operation: "search_strings", + request_id: 4, + exit_code: 7, + }, + }); + expect(exited.message).not.toContain("Run `rea doctor`, then try again."); + expect(exited.remediation.action).toContain("Restart the owned provider"); + + const unknown = projectAnalysisError( + new HopperProcessError(null, undefined, "search_strings", 5), + ); + expect(unknown.details).toMatchObject({ + stage: "analysis", + provider_state: "unknown", + retry_action: "unknown", + operation: "search_strings", + request_id: 5, + exit_code: null, + }); + expect(unknown.message).toContain("could not determine"); + expect(unknown.message).not.toContain("stopped"); + expect(unknown.remediation.action).toContain("provider_operation_health"); + }); +}); diff --git a/src/domain/analysisErrorProjection.ts b/src/domain/analysisErrorProjection.ts index 3025dbb47..4b9ff4f45 100644 --- a/src/domain/analysisErrorProjection.ts +++ b/src/domain/analysisErrorProjection.ts @@ -25,11 +25,13 @@ import { HopperCancelledError, HopperProcessError, HopperRemoteError, + HopperStartError, HopperTimeoutError, } from "./hopperErrors.js"; import { ProviderAdapterError } from "./providerAdapterError.js"; import { ProviderSelectionError } from "./providerSelectionError.js"; import { UnknownRegistryError } from "./unknownRegistryError.js"; +import { providerRetryAction } from "./providerOperationHealth.js"; import { type AnalysisError, type AnalysisErrorTag, @@ -327,10 +329,16 @@ const providerErrorDetails = ( ...(error.operation === undefined ? {} : { operation: error.operation }), ...(error.requestId === undefined ? {} : { request_id: error.requestId }), }; - if (error instanceof HopperProcessError) + if (error instanceof HopperProcessError) { + const stage = error.stage; return { exit_code: error.exitCode, - stage: error.operation === undefined ? "connection" : "analysis", + stage, + provider_state: error.providerState, + retry_action: providerRetryAction( + error.providerState, + error.failureCode !== undefined, + ), ...(error.failureCode === undefined ? {} : { failure_code: error.failureCode }), @@ -340,6 +348,16 @@ const providerErrorDetails = ( ? {} : { diagnostics: { ...error.diagnostic } }), }; + } + if (error instanceof HopperStartError) + return { + stage: "launch", + provider_state: "unknown", + retry_action: error.ownerRunId === undefined ? "retry" : "unknown", + ...(error.ownerRunId === undefined + ? {} + : { owner_run_id: error.ownerRunId }), + }; return undefined; }; @@ -369,9 +387,10 @@ const lifecycleErrorDetails = ( return { operation: error.operation, timeout_ms: error.timeoutMs }; if (error instanceof HopperTimeoutError) return { - stage: error.operation === undefined ? "startup" : "analysis", + stage: error.operation === undefined ? "startup" : error.stage, timeout_ms: error.timeoutMs, provider_state: error.providerState, + retry_action: error.providerState === "busy" ? "wait" : "retry", ...(error.operation === undefined ? {} : { operation: error.operation }), ...(error.requestId === undefined ? {} : { request_id: error.requestId }), }; diff --git a/src/domain/hopperErrors.ts b/src/domain/hopperErrors.ts index a34f5a707..b582f6ac2 100644 --- a/src/domain/hopperErrors.ts +++ b/src/domain/hopperErrors.ts @@ -1,4 +1,8 @@ import { AnalysisError } from "./analysisErrorBase.js"; +import type { + HopperProcessProviderState, + ProviderFailureStage, +} from "./providerOperationHealth.js"; import { hopperStartupFailure, type HopperStartupDiagnostic, @@ -18,6 +22,9 @@ export class HopperTimeoutError extends HopperError { readonly operation?: string, readonly requestId?: number, readonly providerState: "busy" | "not_started" = "not_started", + readonly stage: ProviderFailureStage = operation === undefined + ? "launch" + : "analysis", ) { super( `Hopper ${operation === undefined ? "startup" : operation} timed out after ${String(timeoutMs)}ms`, @@ -76,17 +83,30 @@ export class HopperProcessError extends HopperError { readonly _tag = "HopperProcessError"; readonly failureCode: HopperStartupFailureCode | undefined; override readonly userMessage: string | undefined; + readonly providerState: HopperProcessProviderState; + readonly stage: ProviderFailureStage; constructor( readonly exitCode: number | null, readonly diagnostic?: HopperStartupFailureDiagnostic, readonly operation?: string, readonly requestId?: number, + providerState?: HopperProcessProviderState, + stage?: ProviderFailureStage, ) { super(`Hopper bridge stopped unexpectedly with code ${String(exitCode)}`); const failure = hopperStartupFailure(exitCode); + this.stage = + stage ?? + (failure === undefined + ? operation === undefined + ? "connection" + : "analysis" + : "launch"); this.failureCode = failure?.code; this.userMessage = failure?.message; + this.providerState = + providerState ?? (exitCode === null ? "unknown" : "exited"); } } diff --git a/src/domain/providerOperationHealth.ts b/src/domain/providerOperationHealth.ts new file mode 100644 index 000000000..2ccae72fe --- /dev/null +++ b/src/domain/providerOperationHealth.ts @@ -0,0 +1,88 @@ +/** Caller-visible operational state for one provider bridge. */ +export const PROVIDER_OPERATION_STATES = [ + "idle", + "busy", + "exited", + "unreachable", + "not_started", + "unknown", +] as const; + +/** Operational state REA can support with evidence, or unknown. */ +export type ProviderOperationState = (typeof PROVIDER_OPERATION_STATES)[number]; + +/** States a Hopper process failure can establish. */ +export type HopperProcessProviderState = "exited" | "unreachable" | "unknown"; + +/** What the caller should do next for this provider state. */ +export const PROVIDER_RETRY_ACTIONS = [ + "retry", + "wait", + "restart_provider", + "unknown", +] as const; + +/** Recovery choice for a provider operation failure. */ +export type ProviderRetryAction = (typeof PROVIDER_RETRY_ACTIONS)[number]; + +/** Stage that was in progress when the provider operation failed. */ +export const PROVIDER_FAILURE_STAGES = [ + "launch", + "connection", + "analysis", + "decompilation", + "unknown", +] as const; + +/** Known provider-operation stage, including an explicit unknown. */ +export type ProviderFailureStage = (typeof PROVIDER_FAILURE_STAGES)[number]; + +/** One provider request retained with a failure or an in-flight operation. */ +export interface ProviderOperationRequest { + readonly requestId: number; + readonly operation: string; + readonly stage: ProviderFailureStage; +} + +/** Sanitized provider health a session can show after the request returns. */ +export interface ProviderOperationHealth { + readonly state: ProviderOperationState; + readonly stage: ProviderFailureStage | null; + readonly retryAction: ProviderRetryAction | null; + readonly exitCode: number | null; + readonly requests: readonly ProviderOperationRequest[]; +} + +/** + * Choose a recovery action from the observed state. + * Startup failures keep their existing install or configuration guidance. + */ +export const providerRetryAction = ( + state: ProviderOperationState, + startupFailure = false, +): ProviderRetryAction | null => { + if (state === "idle") return null; + if (startupFailure && state === "exited") return "retry"; + switch (state) { + case "busy": + return "wait"; + case "exited": + case "unreachable": + return "restart_provider"; + case "not_started": + return "retry"; + case "unknown": + return "unknown"; + } +}; + +/** Use one stage when every retained request agrees, otherwise unknown. */ +export const sharedProviderFailureStage = ( + requests: readonly ProviderOperationRequest[], +): ProviderFailureStage | null => { + const first = requests[0]?.stage; + if (first === undefined) return null; + return requests.every((request) => request.stage === first) + ? first + : "unknown"; +}; diff --git a/src/hopper/HopperClient.ts b/src/hopper/HopperClient.ts index 05f951e20..62c82ff87 100644 --- a/src/hopper/HopperClient.ts +++ b/src/hopper/HopperClient.ts @@ -19,6 +19,12 @@ import { hopperStartupFailure, type HopperStartupFailureDiagnostic, } from "../domain/hopperStartupFailure.js"; +import { + providerRetryAction, + sharedProviderFailureStage, + type ProviderOperationHealth, + type ProviderOperationRequest, +} from "../domain/providerOperationHealth.js"; import { err, ok, type Result } from "../domain/result.js"; import type { JsonValue } from "../domain/jsonValue.js"; import type { ProgressReporter } from "../application/ProgressReporter.js"; @@ -31,6 +37,7 @@ import { ProviderProcessSupervisor, } from "../process/ProviderProcess.js"; import type { BridgeLaunch, BridgeLauncher } from "./BridgeLauncher.js"; +import { hopperOperationStage } from "./HopperOperationStage.js"; import type { HopperDiagnostic } from "./HopperDiagnostics.js"; import { cleanupHopperSession } from "./HopperCleanup.js"; import { @@ -94,6 +101,7 @@ export class HopperClient { #closing = false; #launcherExitCode: number | null | undefined; #launcherFailureDiagnostic: HopperStartupFailureDiagnostic | undefined; + #operationFailure: OperationFailureRecord | undefined; #startupController: AbortController | undefined; #startPromise: Promise> | undefined; #closePromise: Promise> | undefined; @@ -101,10 +109,46 @@ export class HopperClient { this.#responses.push(chunk); }; readonly #onSocketError = (): void => { - this.#failAll(new HopperProcessError(null)); + const error = new HopperProcessError( + null, + undefined, + undefined, + undefined, + "unreachable", + ); + this.#failAll(error); + this.#rememberProcessFailure(error, []); }; readonly #onSocketClose = (): void => { - if (!this.#closing) this.#failAll(new HopperProcessError(null)); + if (this.#closing) return; + const launch = this.#launch; + if (launch?.ownsProcessLifetime && this.#launcherExitCode !== undefined) + return; + if (launch?.ownsProcessLifetime) { + setImmediate(() => { + if (this.#closing || this.#launcherExitCode !== undefined) return; + const exitCode = launch.process.exitCode ?? null; + const error = new HopperProcessError( + exitCode, + this.#launcherFailureDiagnostic, + undefined, + undefined, + exitCode !== null ? "exited" : "unreachable", + ); + this.#failAll(error); + this.#rememberProcessFailure(error, []); + }); + return; + } + const error = new HopperProcessError( + null, + undefined, + undefined, + undefined, + "unreachable", + ); + this.#failAll(error); + this.#rememberProcessFailure(error, []); }; constructor(options: HopperClientOptions) { @@ -147,6 +191,59 @@ export class HopperClient { return this.#requests.activity(); } + /** + * Latest operational health for this owned bridge. + * A recorded exit remains visible after request activity returns to idle. + */ + operationHealth(): ProviderOperationHealth { + const failure = this.#operationFailure; + if (failure !== undefined) { + const requests: readonly ProviderOperationRequest[] = + failure.requests.map((request) => ({ + ...request, + stage: failure.startupFailure + ? "launch" + : hopperOperationStage(request.operation), + })); + return { + state: failure.state, + stage: + requests.length === 0 + ? failure.startupFailure + ? "launch" + : "connection" + : sharedProviderFailureStage(requests), + retryAction: providerRetryAction(failure.state, failure.startupFailure), + exitCode: failure.exitCode, + requests, + }; + } + const activity = this.requestActivity(); + if (activity !== null) { + const stage = hopperOperationStage(activity.operation); + return { + state: "busy", + stage, + retryAction: "wait", + exitCode: null, + requests: [ + { + requestId: activity.requestId, + operation: activity.operation, + stage, + }, + ], + }; + } + return { + state: "idle", + stage: null, + retryAction: null, + exitCode: null, + requests: [], + }; + } + /** Launch the bridge once and complete its authenticated health handshake. */ start(signal?: AbortSignal): Promise> { if (this.#closePromise !== undefined) { @@ -175,7 +272,25 @@ export class HopperClient { void started.then( (result) => { release(); - if (!result.ok) reset(); + if (!result.ok) { + if (result.error instanceof HopperProcessError) + this.#rememberProcessFailure(result.error, []); + else if ( + this.#operationFailure === undefined && + (result.error instanceof HopperTimeoutError || + result.error instanceof HopperStartError) + ) + this.#operationFailure = { + state: + result.error instanceof HopperTimeoutError + ? "not_started" + : "unknown", + exitCode: null, + startupFailure: true, + requests: [], + }; + reset(); + } }, (cause: unknown) => { this.#logger.debug( @@ -193,6 +308,7 @@ export class HopperClient { signal?: AbortSignal, ): Promise> { if (isAborted(signal)) return err(new HopperCancelledError()); + this.#operationFailure = undefined; if (this.#socket !== undefined || this.#runtimeRoot !== undefined) { return err(new HopperProtocolError("Hopper client is already started")); } @@ -393,6 +509,9 @@ export class HopperClient { new HopperProcessError( this.#launcherExitCode, this.#launcherFailureDiagnostic, + undefined, + undefined, + "exited", ), ); try { @@ -432,7 +551,16 @@ export class HopperClient { const socket = this.#socket; const token = this.#token; if (socket === undefined || socket.destroyed || token === undefined) { - return err(new HopperProcessError(null, undefined, method, id)); + return this.#unavailableRequest( + new HopperProcessError( + null, + undefined, + method, + id, + "unreachable", + hopperOperationStage(method), + ), + ); } if (options.signal?.aborted === true) return err(new HopperCancelledError()); @@ -441,6 +569,7 @@ export class HopperClient { ...(options.signal === undefined ? {} : { signal: options.signal }), ...(options.progress !== undefined ? { progress: options.progress } : {}), }); + this.#retainRequestHealth(result, id, method); this.#logger[result.ok ? "debug" : "warn"]( { method, @@ -525,10 +654,18 @@ export class HopperClient { type: "launcher-exit", code: event.code, }); - if (launch.ownsProcessLifetime && !this.#closing) + if (launch.ownsProcessLifetime && !this.#closing) { + this.#rememberExit(event.code); this.#failAll( - new HopperProcessError(event.code, this.#launcherFailureDiagnostic), + new HopperProcessError( + event.code, + this.#launcherFailureDiagnostic, + undefined, + undefined, + "exited", + ), ); + } } #abortProtocol(message: string, cause?: Error): void { @@ -539,8 +676,119 @@ export class HopperClient { #failAll(error: HopperError): void { this.#requests.failAll(error); } + + #unavailableRequest( + error: HopperProcessError, + ): Result { + this.#rememberProcessFailure( + error, + error.requestId === undefined || error.operation === undefined + ? [] + : [{ requestId: error.requestId, operation: error.operation }], + ); + return err(error); + } + + #retainRequestHealth( + result: Result, + requestId: number, + operation: string, + ): void { + if (this.#closing) return; + if (result.ok) return; + if (!(result.error instanceof HopperProcessError)) return; + this.#rememberProcessFailure(result.error, [ + { + requestId: result.error.requestId ?? requestId, + operation: result.error.operation ?? operation, + }, + ]); + } + + #rememberExit(code: number | null): void { + if (this.#closing) return; + const previous = this.#operationFailure; + this.#operationFailure = { + state: "exited", + exitCode: code ?? previous?.exitCode ?? null, + startupFailure: + hopperStartupFailure(code) !== undefined || + previous?.startupFailure === true, + requests: previous?.requests ?? [], + }; + } + + #rememberProcessFailure( + error: HopperProcessError, + requests: readonly FailedProviderRequest[], + ): void { + if (this.#closing) return; + const previous = this.#operationFailure; + const state = processFailureState(error); + const startupFailure = + error.failureCode !== undefined || previous?.startupFailure === true; + const merged = mergeFailedRequests(previous?.requests ?? [], requests); + if ( + previous !== undefined && + PROCESS_FAILURE_RANK[previous.state] > PROCESS_FAILURE_RANK[state] + ) { + this.#operationFailure = { + ...previous, + startupFailure, + requests: merged, + }; + return; + } + this.#operationFailure = { + state, + exitCode: + state === "exited" + ? (error.exitCode ?? previous?.exitCode ?? null) + : null, + startupFailure, + requests: merged, + }; + } } +interface FailedProviderRequest { + readonly requestId: number; + readonly operation: string; +} + +interface OperationFailureRecord { + readonly state: "exited" | "unreachable" | "unknown" | "not_started"; + readonly exitCode: number | null; + readonly startupFailure: boolean; + readonly requests: readonly FailedProviderRequest[]; +} + +const PROCESS_FAILURE_RANK = { + not_started: 0, + unknown: 0, + unreachable: 1, + exited: 2, +} as const; + +const processFailureState = ( + error: HopperProcessError, +): OperationFailureRecord["state"] => + error.providerState === "exited" || error.exitCode !== null + ? "exited" + : error.providerState; + +const mergeFailedRequests = ( + current: readonly FailedProviderRequest[], + added: readonly FailedProviderRequest[], +): readonly FailedProviderRequest[] => { + const byId = new Map(); + for (const request of [...current, ...added]) + byId.set(request.requestId, request); + return [...byId.values()].sort( + (left, right) => left.requestId - right.requestId, + ); +}; + const isAborted = (signal?: AbortSignal): boolean => signal?.aborted === true; const startupInterruption = ( diff --git a/src/hopper/HopperOperationStage.ts b/src/hopper/HopperOperationStage.ts new file mode 100644 index 000000000..517e6bd44 --- /dev/null +++ b/src/hopper/HopperOperationStage.ts @@ -0,0 +1,9 @@ +import type { ProviderFailureStage } from "../domain/providerOperationHealth.js"; + +/** Classify Hopper wire operations at their provider boundary. */ +export const hopperOperationStage = ( + operation: string, +): ProviderFailureStage => { + if (operation === "health") return "connection"; + return operation === "procedure_pseudo_code" ? "decompilation" : "analysis"; +}; diff --git a/src/hopper/HopperProvider.ts b/src/hopper/HopperProvider.ts index 96a71a345..a8de08e8a 100644 --- a/src/hopper/HopperProvider.ts +++ b/src/hopper/HopperProvider.ts @@ -235,6 +235,7 @@ export class HopperProvider implements AnalysisProviderCandidate { }, ]; }, + operationHealthSnapshot: () => client.operationHealth(), closeWithOutcome: (options) => client.closeWithOutcome(options), close: () => client.close(), }; diff --git a/src/hopper/HopperRequestQueue.ts b/src/hopper/HopperRequestQueue.ts index 97e9b7d2f..4cf2d7e00 100644 --- a/src/hopper/HopperRequestQueue.ts +++ b/src/hopper/HopperRequestQueue.ts @@ -2,6 +2,7 @@ import type { ProgressReporter, ProgressUpdate, } from "../application/ProgressReporter.js"; +import { hopperOperationStage } from "./HopperOperationStage.js"; import type { HopperBridgeEvent } from "./protocol.js"; import { HopperCancelledError, @@ -101,23 +102,12 @@ export class HopperRequestQueue { const entry = this.#active; if (entry === undefined || entry.id !== id) return false; this.#releaseWire(entry); - if (!entry.callerSettled) { - const contextual = - !result.ok && result.error instanceof HopperRemoteError - ? err( - new HopperRemoteError( - result.error.code, - result.error.safeMessage, - { - diagnosticType: result.error.diagnosticType, - operation: entry.method, - requestId: entry.id, - }, - ), - ) - : result; - this.#settleCaller(entry, contextual, "waiting"); - } + if (!entry.callerSettled) + this.#settleCaller( + entry, + result.ok ? result : err(withRequestContext(result.error, entry)), + "waiting", + ); this.#active = undefined; this.#drain(); return true; @@ -216,7 +206,20 @@ export class HopperRequestQueue { try { this.send( { id: entry.id, method: entry.method, params: entry.params }, - () => this.accept(entry.id, err(new HopperProcessError(null))), + () => + this.accept( + entry.id, + err( + new HopperProcessError( + null, + undefined, + entry.method, + entry.id, + "unreachable", + hopperOperationStage(entry.method), + ), + ), + ), ); } catch (cause: unknown) { this.accept( @@ -289,12 +292,23 @@ export class HopperRequestQueue { const withRequestContext = ( error: HopperError, entry: QueuedRequest, -): HopperError => - error instanceof HopperProcessError - ? new HopperProcessError( - error.exitCode, - error.diagnostic, - entry.method, - entry.id, - ) - : error; +): HopperError => { + if (error instanceof HopperProcessError) + return new HopperProcessError( + error.exitCode, + error.diagnostic, + error.operation ?? entry.method, + error.requestId ?? entry.id, + error.providerState, + error.operation === undefined && error.stage === "connection" + ? hopperOperationStage(entry.method) + : error.stage, + ); + if (error instanceof HopperRemoteError) + return new HopperRemoteError(error.code, error.safeMessage, { + diagnosticType: error.diagnosticType, + operation: error.operation ?? entry.method, + requestId: error.requestId ?? entry.id, + }); + return error; +}; diff --git a/tests/boundary/providers/hopper/hopperClient.health.test.ts b/tests/boundary/providers/hopper/hopperClient.health.test.ts new file mode 100644 index 000000000..7265dfba2 --- /dev/null +++ b/tests/boundary/providers/hopper/hopperClient.health.test.ts @@ -0,0 +1,178 @@ +import { EventEmitter } from "node:events"; + +import { describe, expect, it, onTestFinished, vi } from "vitest"; + +import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js"; +import { ok } from "../../../../src/domain/result.js"; +import type { + BridgeLauncher, + BridgeSession, +} from "../../../../src/hopper/BridgeLauncher.js"; +import { HopperClient } from "../../../../src/hopper/HopperClient.js"; +import { HopperFixtureLauncher } from "./hopperClient.fixture.js"; + +class ObservedProcess extends EventEmitter { + readonly stdout = null; + readonly stderr = null; + exitCode: number | null = null; + readonly signalCode = null; + + exit(code: number): void { + this.exitCode = code; + this.emit("exit", code, null); + this.emit("close", code, null); + } + + kill(): boolean { + this.exit(0); + return true; + } +} + +class LifecycleLauncher implements BridgeLauncher { + readonly fixture = new HopperFixtureLauncher(); + observed = new ObservedProcess(); + + constructor(readonly owned: boolean) {} + + async launch(session: BridgeSession) { + this.observed = new ObservedProcess(); + await this.fixture.launch(session); + return ok({ + process: this.observed, + ownsProcessLifetime: this.owned, + shutdownMode: "bridge-request" as const, + }); + } +} + +const start = async (owned: boolean) => { + const launcher = new LifecycleLauncher(owned); + const client = new HopperClient({ launcher, startupTimeoutMs: 10_000 }); + onTestFinished(async () => { + await client.close(); + for (const child of launcher.fixture.processes) child.kill(); + }); + expect((await client.start()).ok).toBe(true); + return { client, launcher }; +}; + +describe("Hopper provider lifecycle health", () => { + it("keeps a live bridge healthy after a non-owning launcher helper exits", async () => { + const { client, launcher } = await start(false); + launcher.observed.exit(0); + expect(client.operationHealth().state).toBe("idle"); + expect(await client.callTool("echo", { label: "GUI still alive" })).toEqual( + { + ok: true, + value: { label: "GUI still alive" }, + }, + ); + expect(client.operationHealth().state).toBe("idle"); + }); + + it("settles disconnected requests without attributing the helper exit to the GUI", async () => { + const { client, launcher } = await start(false); + launcher.observed.exit(0); + const pending = client.callTool("hang"); + await launcher.fixture.waitForRequest("hang"); + launcher.fixture.processes.at(-1)?.kill("SIGKILL"); + await vi.waitFor(() => + expect(client.operationHealth().state).toBe("unreachable"), + ); + expect(await pending).toMatchObject({ + ok: false, + error: { + providerState: "unreachable", + exitCode: null, + operation: "hang", + }, + }); + expect(client.operationHealth()).toMatchObject({ + state: "unreachable", + exitCode: null, + }); + expect( + await client.callTool("procedure_pseudo_code", { procedure: "main" }), + ).toMatchObject({ + ok: false, + error: { + providerState: "unreachable", + stage: "decompilation", + operation: "procedure_pseudo_code", + }, + }); + }); + + it("projects the exit code when the owned process exit is observed before disconnection", async () => { + const { client, launcher } = await start(true); + const pending = client.callTool("hang"); + const request = await launcher.fixture.waitForRequest("hang"); + launcher.observed.exit(7); + const result = await pending; + expect(result).toMatchObject({ + ok: false, + error: { + _tag: "HopperProcessError", + exitCode: 7, + providerState: "exited", + requestId: request.id, + }, + }); + if (!result.ok) + expect(projectAnalysisError(result.error)).toMatchObject({ + message: expect.stringContaining( + "Hopper exited during hang (exit code 7)", + ), + details: { + provider_state: "exited", + exit_code: 7, + stage: "analysis", + operation: "hang", + request_id: request.id, + }, + }); + }); + + it("retains an observed exit when an earlier successful reply resumes afterward", async () => { + const { client, launcher } = await start(true); + const first = client.callTool("echo", { gate: "before-exit" }); + const request = await launcher.fixture.waitForRequest( + "echo", + "before-exit", + ); + const controller = new AbortController(); + const second = client.callTool( + "hang", + {}, + { + signal: controller.signal, + progress: { + report: (update) => { + if (update.message === "hang started on Hopper's serial bridge") { + controller.abort(); + launcher.observed.exit(7); + } + return Promise.resolve(); + }, + }, + }, + ); + await vi.waitFor(() => + expect(client.requestActivity()?.queuedRequests).toBe(1), + ); + await launcher.fixture.release(request); + expect((await first).ok).toBe(true); + expect(await second).toMatchObject({ + ok: false, + error: { _tag: "HopperCancelledError" }, + }); + expect(client.operationHealth()).toMatchObject({ + state: "exited", + exitCode: 7, + }); + await client.close(); + expect((await client.start()).ok).toBe(true); + expect(client.operationHealth().state).toBe("idle"); + }); +}); diff --git a/tests/boundary/providers/hopper/hopperClient.protocol.test.ts b/tests/boundary/providers/hopper/hopperClient.protocol.test.ts index e263c6861..d419b6ac9 100644 --- a/tests/boundary/providers/hopper/hopperClient.protocol.test.ts +++ b/tests/boundary/providers/hopper/hopperClient.protocol.test.ts @@ -1,9 +1,13 @@ +import { once } from "node:events"; + import { describe, expect, it, onTestFinished, vi } from "vitest"; +import { HopperProcessError } from "../../../../src/domain/hopperErrors.js"; import { HOPPER_OPERATIONS } from "../../../../src/hopper/HopperProvider.js"; import { HopperClient } from "../../../../src/hopper/HopperClient.js"; import type { HopperDiagnostic } from "../../../../src/hopper/HopperDiagnostics.js"; import { providerCleanupFailure } from "../../../../src/hopper/HopperDiagnostics.js"; +import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js"; import { HopperFixtureLauncher as FixtureLauncher, @@ -192,3 +196,125 @@ describe("HopperClient response lifecycle", () => { }); }); }); + +describe("HopperClient operational diagnostics", () => { + it("records process exit code, stage, and restart guidance on unexpected exit", async () => { + const launcher = new FixtureLauncher(); + const client = await startClient(launcher); + const child = launcher.processes.at(-1); + if (child === undefined) throw new Error("Missing fixture process"); + const exited = once(child, "exit"); + const result = await client.callTool("exit"); + await exited; + expect(result.ok).toBe(false); + if (!result.ok) { + if (!(result.error instanceof HopperProcessError)) + throw new Error("Expected a provider process error"); + expect(["unreachable", "exited"]).toContain(result.error.providerState); + expect(result.error.exitCode).toBe( + result.error.providerState === "exited" ? 7 : null, + ); + expect(result.error).toMatchObject({ + _tag: "HopperProcessError", + operation: "exit", + }); + const projected = projectAnalysisError(result.error); + expect(projected).toMatchObject({ + code: "provider_unavailable", + retryable: true, + details: { + stage: "analysis", + provider_state: result.error.providerState, + retry_action: "restart_provider", + operation: "exit", + exit_code: result.error.exitCode, + }, + }); + expect(projected.remediation.action).toContain("close_binary"); + } + expect(client.operationHealth()).toMatchObject({ + state: "exited", + stage: "analysis", + retryAction: "restart_provider", + exitCode: 7, + requests: [ + expect.objectContaining({ + operation: "exit", + stage: "analysis", + }), + ], + }); + }); + + it("retains request correlation and health across concurrent requests when the bridge process terminates", async () => { + const launcher = new FixtureLauncher(); + const client = await startClient(launcher); + const hanging = client.callTool("hang"); + const inFlight = await launcher.waitForRequest("hang"); + const queued = client.callTool("echo", { label: "queued" }); + + const child = launcher.processes.at(-1); + if (child === undefined) throw new Error("Missing fixture process"); + const exited = once(child, "exit"); + child.kill("SIGKILL"); + await exited; + + const [hangingResult, queuedResult] = await Promise.all([hanging, queued]); + expect(hangingResult.ok).toBe(false); + expect(queuedResult.ok).toBe(false); + if (!hangingResult.ok && !queuedResult.ok) { + if ( + !(hangingResult.error instanceof HopperProcessError) || + !(queuedResult.error instanceof HopperProcessError) + ) + throw new Error("Expected correlated provider process errors"); + // Socket loss may be observed before the separate child-exit event. + // The immutable reply must preserve what was known when it settled. + expect(["unreachable", "exited"]).toContain( + hangingResult.error.providerState, + ); + expect(["unreachable", "exited"]).toContain( + queuedResult.error.providerState, + ); + expect(hangingResult.error).toMatchObject({ + _tag: "HopperProcessError", + operation: "hang", + requestId: inFlight.id, + }); + expect(queuedResult.error).toMatchObject({ + _tag: "HopperProcessError", + operation: "echo", + requestId: inFlight.id + 1, + }); + const hangingProjected = projectAnalysisError(hangingResult.error); + const queuedProjected = projectAnalysisError(queuedResult.error); + expect(hangingProjected.details).toMatchObject({ + stage: "analysis", + provider_state: hangingResult.error.providerState, + retry_action: "restart_provider", + operation: "hang", + request_id: inFlight.id, + }); + expect(queuedProjected.details).toMatchObject({ + stage: "analysis", + provider_state: queuedResult.error.providerState, + retry_action: "restart_provider", + operation: "echo", + request_id: inFlight.id + 1, + }); + expect(hangingProjected.remediation.action).toContain("close_binary"); + expect(queuedProjected.remediation.action).toContain("close_binary"); + } + expect(client.operationHealth()).toMatchObject({ + state: "exited", + retryAction: "restart_provider", + }); + expect(client.operationHealth().requests).toEqual([ + expect.objectContaining({ requestId: inFlight.id, operation: "hang" }), + expect.objectContaining({ + requestId: inFlight.id + 1, + operation: "echo", + }), + ]); + }); +}); diff --git a/tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts b/tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts index 23c05d4f0..40b2871f8 100644 --- a/tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts +++ b/tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts @@ -122,6 +122,40 @@ afterEach(async () => { await Promise.all(clients.splice(0).map((client) => client.close())); }); +describe("HopperClient startup deadline", () => { + it("applies one deadline to launcher, socket, and health startup phases", async () => { + const launcher = new LateSilentLauncher(); + const client = new HopperClient({ launcher, startupTimeoutMs: 500 }); + clients.push(client); + + const result = await client.start(); + + expect(result).toMatchObject({ + ok: false, + error: { _tag: "HopperTimeoutError", timeoutMs: 500 }, + }); + expect(client.operationHealth()).toMatchObject({ + state: "not_started", + stage: "launch", + retryAction: "retry", + exitCode: null, + }); + if (!result.ok) + expect(projectAnalysisError(result.error).details).toMatchObject({ + stage: "startup", + }); + expect(launcher.directories).toHaveLength(1); + await expect(access(launcher.directories[0] ?? "")).rejects.toMatchObject({ + code: "ENOENT", + }); + const process = launcher.processes[0]; + expect( + process !== undefined && + (process.exitCode !== null || process.signalCode !== null), + ).toBe(true); + }); +}); + describe("HopperClient startup failures", () => { it("cancels bridge startup without waiting for the startup timeout", async () => { const launcher = new CancelThenFixtureLauncher(); @@ -183,28 +217,6 @@ describe("HopperClient startup failures", () => { }); }); - it("applies one deadline to launcher, socket, and health startup phases", async () => { - const launcher = new LateSilentLauncher(); - const client = new HopperClient({ launcher, startupTimeoutMs: 500 }); - clients.push(client); - - const result = await client.start(); - - expect(result).toMatchObject({ - ok: false, - error: { _tag: "HopperTimeoutError", timeoutMs: 500 }, - }); - expect(launcher.directories).toHaveLength(1); - await expect(access(launcher.directories[0] ?? "")).rejects.toMatchObject({ - code: "ENOENT", - }); - const process = launcher.processes[0]; - expect( - process !== undefined && - (process.exitCode !== null || process.signalCode !== null), - ).toBe(true); - }); - it.each([70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80])( "reports Linux adapter exit %i during bridge startup", async (exitCode) => { diff --git a/tests/composition/analysis-sessions/binarySession.test.ts b/tests/composition/analysis-sessions/binarySession.test.ts index 205692e77..4f8f338b5 100644 --- a/tests/composition/analysis-sessions/binarySession.test.ts +++ b/tests/composition/analysis-sessions/binarySession.test.ts @@ -158,6 +158,76 @@ describe("detached provider and target metadata", () => { await session.close(); }); + + it("exposes provider operational health in session status", async () => { + const [first] = await createBinarySessionTargets(); + let healthState: "idle" | "busy" | "exited" = "busy"; + const provider: AnalysisProvider = { + identity: () => ({ id: "fixture", name: "Fixture", version: "1" }), + capabilities: () => [], + createClient: () => ({ + execute: () => Promise.resolve(ok("ok")), + operationHealthSnapshot: () => ({ + state: healthState, + stage: "analysis", + retryAction: healthState === "exited" ? "restart_provider" : "wait", + exitCode: healthState === "exited" ? 7 : null, + requests: [ + { + requestId: 1, + operation: "find_code_for_string", + stage: "analysis", + }, + ], + }), + close: () => Promise.resolve(), + }), + }; + const session = createTestBinarySession(provider); + expect(session.status()).toMatchObject({ + open: false, + provider_operation_health: null, + }); + expect((await session.open(first)).ok).toBe(true); + expect(session.status()).toMatchObject({ + open: true, + provider_operation_health: { + state: "busy", + stage: "analysis", + retry_action: "wait", + exit_code: null, + requests: [ + { + request_id: 1, + operation: "find_code_for_string", + stage: "analysis", + }, + ], + }, + }); + healthState = "exited"; + expect(session.status()).toMatchObject({ + open: true, + provider_operation_health: { + state: "exited", + stage: "analysis", + retry_action: "restart_provider", + exit_code: 7, + requests: [ + { + request_id: 1, + operation: "find_code_for_string", + stage: "analysis", + }, + ], + }, + }); + await session.close(); + expect(session.status()).toMatchObject({ + open: false, + provider_operation_health: null, + }); + }); }); describe("opening previewed targets", () => {