mirror of
https://github.com/morluto/rea
synced 2026-10-26 06:16:17 -04:00
fix(hopper): preserve observed process failures across requests (#1927)
* fix(hopper): preserve observed process failures across requests * test(hopper): consolidate retained cleanup and recover failed helper fixtures * docs(hopper): clarify first-use demo prompt and startup recovery
This commit is contained in:
@@ -245,7 +245,9 @@ Ghidra and IDA use your existing installations. See [provider setup](docs/instal
|
||||
<summary><strong>Do I need to start Hopper first?</strong></summary>
|
||||
|
||||
REA starts Hopper when an operation needs it. On macOS, a first-run dialog may
|
||||
ask you to choose demo mode or activate your license. See
|
||||
ask you to choose demo mode or activate your license. Complete that prompt in the
|
||||
same user session before your first CLI/MCP operation or unattended job. Open
|
||||
Hopper manually if needed. See
|
||||
[Hopper startup and troubleshooting](docs/installation.md#launcher-paths-and-troubleshooting).
|
||||
|
||||
</details>
|
||||
|
||||
+12
-2
@@ -567,8 +567,13 @@ need Xvfb, Python 3, X11 and XTEST; approved setup installs those dependencies
|
||||
on supported distributions. Add the curl installer's reported executable
|
||||
directory, commonly `~/.local/bin`, to your shell `PATH` if needed.
|
||||
|
||||
REA starts Hopper when needed. On macOS, its launcher can bring a window or
|
||||
first-run dialog forward; choose demo mode or activate your existing license.
|
||||
REA starts Hopper when needed. On macOS, complete its demo or license prompt in
|
||||
the same user session before the first CLI/MCP operation or unattended job. Open
|
||||
Hopper manually if needed; its launcher can also bring the dialog forward.
|
||||
If startup reports `Cannot launch Hopper! Connection is invalid.` or waits for
|
||||
bridge readiness, check Hopper for a pending demo, license, or loader dialog.
|
||||
Complete the dialog and retry. If REA reports `cleanup_incomplete`, follow the
|
||||
retained-resource guidance below before reopening the target.
|
||||
Hopper serializes analysis requests. Cancelling a wait can leave provider work
|
||||
running, which the session reports. Successful decompilation is cached until
|
||||
a relevant rename or comment changes it.
|
||||
@@ -597,6 +602,11 @@ if document closure is unconfirmed, REA retains the backing image and reports
|
||||
MCP-exit behavior.
|
||||
Startup deadlines report missing bridge readiness and preserve the launcher
|
||||
outcome; a successful helper exit does not prove that a loader dialog completed.
|
||||
If an owned provider process exits, request failures preserve its observed exit
|
||||
code or signal and bounded output in `details.launcher`. An exit can precede
|
||||
output-stream closure; `output_closed: false` keeps that uncertainty explicit.
|
||||
Later requests retain the known exit instead of reducing it to socket loss.
|
||||
An external GUI's helper exit does not establish that the GUI itself exited.
|
||||
Cursor navigation returns the observed object start when Hopper snaps an interior
|
||||
address; adjacent-object navigation rejects unmapped inputs and document ends.
|
||||
Native API text rejects NUL characters and unpaired Unicode surrogates before
|
||||
|
||||
@@ -51,9 +51,9 @@ export interface AnalysisCleanupObservation {
|
||||
|
||||
/** Optional provider-neutral context that must survive typed error projection. */
|
||||
export interface AnalysisErrorOptions extends ErrorOptions {
|
||||
readonly capturedOutput?: AnalysisCapturedOutput;
|
||||
readonly cleanup?: AnalysisCleanupObservation;
|
||||
readonly partialObservation?: AnalysisPartialObservation;
|
||||
readonly capturedOutput?: AnalysisCapturedOutput | undefined;
|
||||
readonly cleanup?: AnalysisCleanupObservation | undefined;
|
||||
readonly partialObservation?: AnalysisPartialObservation | undefined;
|
||||
}
|
||||
|
||||
/** Base class for expected analysis, provider, and session failures. */
|
||||
|
||||
@@ -265,9 +265,12 @@ const hopperProcessMessage = (error: HopperProcessError): string => {
|
||||
const startup =
|
||||
error.userMessage === undefined ? "" : ` ${error.userMessage}`;
|
||||
if (error.providerState === "exited") {
|
||||
const signal = error.launcherFailure?.signal;
|
||||
const exit =
|
||||
error.exitCode === null
|
||||
? "no exit code was observed"
|
||||
? signal === undefined || signal === null
|
||||
? "no exit code was observed"
|
||||
: `signal ${signal}`
|
||||
: `exit code ${String(error.exitCode)}`;
|
||||
const recovery =
|
||||
error.failureCode === undefined && stage !== "launch"
|
||||
|
||||
@@ -572,7 +572,7 @@ 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),
|
||||
new HopperProcessError(7, { operation: "search_strings", requestId: 4 }),
|
||||
);
|
||||
expect(exited).toMatchObject({
|
||||
code: "provider_unavailable",
|
||||
@@ -590,7 +590,10 @@ describe("analysis error projection: operational diagnostics", () => {
|
||||
expect(exited.remediation.action).toContain("Restart the owned provider");
|
||||
|
||||
const unknown = projectAnalysisError(
|
||||
new HopperProcessError(null, undefined, "search_strings", 5),
|
||||
new HopperProcessError(null, {
|
||||
operation: "search_strings",
|
||||
requestId: 5,
|
||||
}),
|
||||
);
|
||||
expect(unknown.details).toMatchObject({
|
||||
stage: "analysis",
|
||||
|
||||
+34
-16
@@ -1,4 +1,7 @@
|
||||
import { AnalysisError } from "./analysisErrorBase.js";
|
||||
import {
|
||||
AnalysisError,
|
||||
type AnalysisErrorOptions,
|
||||
} from "./analysisErrorBase.js";
|
||||
import type {
|
||||
HopperProcessProviderState,
|
||||
ProviderFailureStage,
|
||||
@@ -73,10 +76,10 @@ export type HopperDiagnosticType =
|
||||
| "bridge_exception";
|
||||
|
||||
/** Hopper's JSON-RPC endpoint returned an expected remote error response. */
|
||||
export interface HopperRemoteErrorContext {
|
||||
readonly diagnosticType?: HopperDiagnosticType;
|
||||
readonly operation?: string;
|
||||
readonly requestId?: number;
|
||||
export interface HopperRemoteErrorContext extends AnalysisErrorOptions {
|
||||
readonly diagnosticType?: HopperDiagnosticType | undefined;
|
||||
readonly operation?: string | undefined;
|
||||
readonly requestId?: number | undefined;
|
||||
}
|
||||
|
||||
export class HopperRemoteError extends HopperError {
|
||||
@@ -90,7 +93,7 @@ export class HopperRemoteError extends HopperError {
|
||||
readonly safeMessage: string,
|
||||
context: HopperRemoteErrorContext = {},
|
||||
) {
|
||||
super(`Hopper request failed (${String(code)}): ${safeMessage}`);
|
||||
super(`Hopper request failed (${String(code)}): ${safeMessage}`, context);
|
||||
this.diagnosticType = context.diagnosticType ?? "remote";
|
||||
this.operation = context.operation;
|
||||
this.requestId = context.requestId;
|
||||
@@ -98,35 +101,50 @@ export class HopperRemoteError extends HopperError {
|
||||
}
|
||||
|
||||
/** The owned Hopper bridge stopped before its client was closed. */
|
||||
export interface HopperProcessErrorContext extends AnalysisErrorOptions {
|
||||
readonly diagnostic?: HopperStartupFailureDiagnostic | undefined;
|
||||
readonly operation?: string | undefined;
|
||||
readonly requestId?: number | undefined;
|
||||
readonly providerState?: HopperProcessProviderState | undefined;
|
||||
readonly stage?: ProviderFailureStage | undefined;
|
||||
readonly launcherFailure?: HopperLauncherOutcome | undefined;
|
||||
}
|
||||
|
||||
export class HopperProcessError extends HopperError {
|
||||
readonly _tag = "HopperProcessError";
|
||||
readonly failureCode: HopperStartupFailureCode | undefined;
|
||||
override readonly userMessage: string | undefined;
|
||||
readonly providerState: HopperProcessProviderState;
|
||||
readonly stage: ProviderFailureStage;
|
||||
readonly diagnostic: HopperStartupFailureDiagnostic | undefined;
|
||||
readonly operation: string | undefined;
|
||||
readonly requestId: number | undefined;
|
||||
readonly launcherFailure: HopperLauncherOutcome | undefined;
|
||||
|
||||
constructor(
|
||||
readonly exitCode: number | null,
|
||||
readonly diagnostic?: HopperStartupFailureDiagnostic,
|
||||
readonly operation?: string,
|
||||
readonly requestId?: number,
|
||||
providerState?: HopperProcessProviderState,
|
||||
stage?: ProviderFailureStage,
|
||||
readonly launcherFailure?: HopperLauncherOutcome,
|
||||
context: HopperProcessErrorContext = {},
|
||||
) {
|
||||
super(`Hopper bridge stopped unexpectedly with code ${String(exitCode)}`);
|
||||
super(
|
||||
`Hopper bridge stopped unexpectedly with code ${String(exitCode)}`,
|
||||
context,
|
||||
);
|
||||
this.diagnostic = context.diagnostic;
|
||||
this.operation = context.operation;
|
||||
this.requestId = context.requestId;
|
||||
this.launcherFailure = context.launcherFailure;
|
||||
const failure = hopperStartupFailure(exitCode);
|
||||
this.stage =
|
||||
stage ??
|
||||
context.stage ??
|
||||
(failure === undefined
|
||||
? operation === undefined
|
||||
? context.operation === undefined
|
||||
? "connection"
|
||||
: "analysis"
|
||||
: "launch");
|
||||
this.failureCode = failure?.code;
|
||||
this.userMessage = failure?.message;
|
||||
this.providerState =
|
||||
providerState ?? (exitCode === null ? "unknown" : "exited");
|
||||
context.providerState ?? (exitCode === null ? "unknown" : "exited");
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -362,10 +362,9 @@ export class HopperApplicationLauncher implements BridgeLauncher {
|
||||
? { ok: true, strategy: selection.strategy }
|
||||
: {
|
||||
ok: false,
|
||||
error: new HopperProcessError(
|
||||
selection.exitCode,
|
||||
selection.diagnostic,
|
||||
),
|
||||
error: new HopperProcessError(selection.exitCode, {
|
||||
diagnostic: selection.diagnostic,
|
||||
}),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -202,9 +202,7 @@ const shutdownRequestFailure = (
|
||||
operation: "shutdown" | "shutdown_document",
|
||||
cause: unknown,
|
||||
): HopperProcessError => {
|
||||
const failure = new HopperProcessError(null, undefined, operation);
|
||||
failure.cause = cause;
|
||||
return failure;
|
||||
return new HopperProcessError(null, { operation, cause });
|
||||
};
|
||||
|
||||
const shutdownFailureDetails = (
|
||||
|
||||
+44
-44
@@ -129,13 +129,9 @@ export class HopperClient {
|
||||
this.#responses.push(chunk);
|
||||
};
|
||||
readonly #onSocketError = (): void => {
|
||||
const error = new HopperProcessError(
|
||||
null,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
"unreachable",
|
||||
);
|
||||
const error = new HopperProcessError(null, {
|
||||
providerState: "unreachable",
|
||||
});
|
||||
this.#failAll(error);
|
||||
this.#rememberProcessFailure(error, []);
|
||||
};
|
||||
@@ -151,25 +147,18 @@ export class HopperClient {
|
||||
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",
|
||||
);
|
||||
const error = new HopperProcessError(exitCode, {
|
||||
diagnostic: this.#launcherFailureDiagnostic,
|
||||
providerState: exitCode !== null ? "exited" : "unreachable",
|
||||
});
|
||||
this.#failAll(error);
|
||||
this.#rememberProcessFailure(error, []);
|
||||
});
|
||||
return;
|
||||
}
|
||||
const error = new HopperProcessError(
|
||||
null,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
"unreachable",
|
||||
);
|
||||
const error = new HopperProcessError(null, {
|
||||
providerState: "unreachable",
|
||||
});
|
||||
this.#failAll(error);
|
||||
this.#rememberProcessFailure(error, []);
|
||||
};
|
||||
@@ -729,15 +718,12 @@ export class HopperClient {
|
||||
)) ?? false;
|
||||
const launcherFailure = this.#launcherOutcome(outputClosed);
|
||||
if (ownsProviderLifetime)
|
||||
return new HopperProcessError(
|
||||
exitCode,
|
||||
this.#launcherFailureDiagnostic,
|
||||
undefined,
|
||||
undefined,
|
||||
"exited",
|
||||
"launch",
|
||||
return new HopperProcessError(exitCode, {
|
||||
diagnostic: this.#launcherFailureDiagnostic,
|
||||
providerState: "exited",
|
||||
stage: "launch",
|
||||
launcherFailure,
|
||||
);
|
||||
});
|
||||
if (
|
||||
this.#resources.launch?.authorizationDenialBeforeHandoff === true &&
|
||||
nativeHopperLaunchWasDenied(launcherFailure)
|
||||
@@ -766,18 +752,34 @@ export class HopperClient {
|
||||
},
|
||||
): Promise<Result<JsonValue, HopperError>> {
|
||||
const id = this.#nextId++;
|
||||
if (
|
||||
this.#resources.launch?.providerLifetime === "launcher-process" &&
|
||||
this.#launcherExitCode !== undefined
|
||||
) {
|
||||
return this.#unavailableRequest(
|
||||
new HopperProcessError(this.#launcherExitCode, {
|
||||
diagnostic: this.#launcherFailureDiagnostic,
|
||||
operation: method,
|
||||
requestId: id,
|
||||
providerState: "exited",
|
||||
stage: hopperOperationStage(method),
|
||||
launcherFailure: this.#launcherOutcome(
|
||||
(await this.#resources.processSupervisor?.waitForOutputClose(0)) ??
|
||||
false,
|
||||
),
|
||||
}),
|
||||
);
|
||||
}
|
||||
const socket = this.#socket;
|
||||
const token = this.#token;
|
||||
if (socket === undefined || socket.destroyed || token === undefined) {
|
||||
return this.#unavailableRequest(
|
||||
new HopperProcessError(
|
||||
null,
|
||||
undefined,
|
||||
method,
|
||||
id,
|
||||
"unreachable",
|
||||
hopperOperationStage(method),
|
||||
),
|
||||
new HopperProcessError(null, {
|
||||
operation: method,
|
||||
requestId: id,
|
||||
providerState: "unreachable",
|
||||
stage: hopperOperationStage(method),
|
||||
}),
|
||||
);
|
||||
}
|
||||
if (options.signal?.aborted === true)
|
||||
@@ -888,13 +890,11 @@ export class HopperClient {
|
||||
if (launch.providerLifetime === "launcher-process" && !this.#closing) {
|
||||
this.#rememberExit(event.code);
|
||||
this.#failAll(
|
||||
new HopperProcessError(
|
||||
event.code,
|
||||
this.#launcherFailureDiagnostic,
|
||||
undefined,
|
||||
undefined,
|
||||
"exited",
|
||||
),
|
||||
new HopperProcessError(event.code, {
|
||||
diagnostic: this.#launcherFailureDiagnostic,
|
||||
providerState: "exited",
|
||||
launcherFailure: this.#launcherOutcome(false),
|
||||
}),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -273,14 +273,12 @@ export class HopperRequestQueue {
|
||||
this.accept(
|
||||
entry.id,
|
||||
err(
|
||||
new HopperProcessError(
|
||||
null,
|
||||
undefined,
|
||||
entry.method,
|
||||
entry.id,
|
||||
"unreachable",
|
||||
hopperOperationStage(entry.method),
|
||||
),
|
||||
new HopperProcessError(null, {
|
||||
operation: entry.method,
|
||||
requestId: entry.id,
|
||||
providerState: "unreachable",
|
||||
stage: hopperOperationStage(entry.method),
|
||||
}),
|
||||
),
|
||||
),
|
||||
);
|
||||
@@ -386,19 +384,20 @@ const withRequestContext = (
|
||||
entry: QueuedRequest,
|
||||
): 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,
|
||||
);
|
||||
return new HopperProcessError(error.exitCode, {
|
||||
...error,
|
||||
cause: error.cause,
|
||||
operation: error.operation ?? entry.method,
|
||||
requestId: error.requestId ?? entry.id,
|
||||
stage:
|
||||
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,
|
||||
...error,
|
||||
cause: error.cause,
|
||||
operation: error.operation ?? entry.method,
|
||||
requestId: error.requestId ?? entry.id,
|
||||
});
|
||||
|
||||
@@ -338,20 +338,22 @@ describe("tool result projection", () => {
|
||||
const result = delivery.toCallToolResult(
|
||||
err(
|
||||
new HopperProcessError(80, {
|
||||
component: "hopper_private_display",
|
||||
operation: "launch",
|
||||
status: "error",
|
||||
failure_code: "x11_socket_directory_unusable",
|
||||
reason: "socket_directory_read_only",
|
||||
socket_directory: "/tmp/.X11-unix",
|
||||
socket_directory_mode: "0777",
|
||||
mount_read_only: true,
|
||||
effective_socket_directory_mode: "1777",
|
||||
effective_mount_read_only: false,
|
||||
wsl: true,
|
||||
strategy: "user-mount-namespace",
|
||||
fallback_reason: null,
|
||||
xvfb_stderr_bytes: 512,
|
||||
diagnostic: {
|
||||
component: "hopper_private_display",
|
||||
operation: "launch",
|
||||
status: "error",
|
||||
failure_code: "x11_socket_directory_unusable",
|
||||
reason: "socket_directory_read_only",
|
||||
socket_directory: "/tmp/.X11-unix",
|
||||
socket_directory_mode: "0777",
|
||||
mount_read_only: true,
|
||||
effective_socket_directory_mode: "1777",
|
||||
effective_mount_read_only: false,
|
||||
wsl: true,
|
||||
strategy: "user-mount-namespace",
|
||||
fallback_reason: null,
|
||||
xvfb_stderr_bytes: 512,
|
||||
},
|
||||
}),
|
||||
),
|
||||
contract,
|
||||
|
||||
@@ -3,7 +3,6 @@ import { access } from "node:fs/promises";
|
||||
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
|
||||
import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js";
|
||||
import { ok } from "../../../../src/domain/result.js";
|
||||
import type {
|
||||
BridgeLauncher,
|
||||
@@ -53,31 +52,6 @@ class OwnedFixtureLauncher implements BridgeLauncher {
|
||||
}
|
||||
}
|
||||
|
||||
class UnconfirmedFixtureLauncher implements BridgeLauncher {
|
||||
constructor(readonly ownsProcessLifetime = false) {}
|
||||
|
||||
launch(session: BridgeSession) {
|
||||
return Promise.resolve(
|
||||
ok({
|
||||
process: spawn(
|
||||
process.execPath,
|
||||
[
|
||||
fixturePath,
|
||||
session.socketPath,
|
||||
session.token,
|
||||
session.runId,
|
||||
"unconfirmed",
|
||||
],
|
||||
{ stdio: ["ignore", "ignore", "pipe"] },
|
||||
),
|
||||
ownsProcessLifetime: this.ownsProcessLifetime,
|
||||
providerLifetime: "external-application" as const,
|
||||
shutdownMode: "bridge-request" as const,
|
||||
}),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const clients: HopperClient[] = [];
|
||||
|
||||
afterEach(async () => {
|
||||
@@ -137,42 +111,6 @@ describe("HopperClient cleanup", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
["unowned", false],
|
||||
["owned helper", true],
|
||||
] as const)(
|
||||
"reports unconfirmed external document cleanup for an %s process",
|
||||
async (_description, ownsProcessLifetime) => {
|
||||
const client = new HopperClient({
|
||||
launcher: new UnconfirmedFixtureLauncher(ownsProcessLifetime),
|
||||
startupTimeoutMs: 1_000,
|
||||
});
|
||||
clients.push(client);
|
||||
expect((await client.start()).ok).toBe(true);
|
||||
|
||||
const closed = await client.close();
|
||||
|
||||
expect(closed).toMatchObject({
|
||||
ok: false,
|
||||
error: {
|
||||
_tag: "ProviderAdapterError",
|
||||
cleanupIncomplete: true,
|
||||
cleanupResources: expect.arrayContaining(["hopper-document"]),
|
||||
},
|
||||
});
|
||||
if (!closed.ok)
|
||||
expect(projectAnalysisError(closed.error)).toMatchObject({
|
||||
code: "cleanup_incomplete",
|
||||
details: {
|
||||
provider_id: "hopper",
|
||||
operation: "close_binary",
|
||||
cleanup: "incomplete",
|
||||
resources: expect.arrayContaining(["hopper-document"]),
|
||||
},
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("kills only its owned Hopper group and emits sanitized shutdown coordinates", async () => {
|
||||
const unrelated = spawn(
|
||||
process.execPath,
|
||||
|
||||
@@ -160,50 +160,71 @@ it("retains failed startup cleanup until an owned process and its backing image
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps an unconfirmed external document visible across sequential closes", async () => {
|
||||
const launcher = new RetryCleanupLauncher(true);
|
||||
const runtimeRoot = await PrivateRuntimeRoot.create();
|
||||
onTestFinished(() => runtimeRoot.close());
|
||||
const launched = await launcher.launch({
|
||||
directory: runtimeRoot.path,
|
||||
socketPath: join(runtimeRoot.path, "bridge.sock"),
|
||||
token: "fixture-token",
|
||||
runId: "fixture-run",
|
||||
});
|
||||
if (!launched.ok) throw launched.error;
|
||||
const child = launched.value.process;
|
||||
const supervisor = new ProviderProcessSupervisor(launched.value);
|
||||
onTestFinished(() => supervisor.dispose());
|
||||
const resources: HopperOwnedResources = {
|
||||
launch: launched.value,
|
||||
processSupervisor: supervisor,
|
||||
runtimeRoot,
|
||||
shutdownConfirmed: false,
|
||||
};
|
||||
for (let attempt = 0; attempt < 2; attempt += 1) {
|
||||
await expect(
|
||||
cleanupHopperSession({
|
||||
socket: undefined,
|
||||
resources,
|
||||
activeRequest: null,
|
||||
retainDocument: false,
|
||||
progress: undefined,
|
||||
logger: silentLogger,
|
||||
onDiagnostic: undefined,
|
||||
request: () => Promise.resolve(ok(null)),
|
||||
releaseTransport: (socket) => socket?.destroy(),
|
||||
}),
|
||||
).resolves.toMatchObject({
|
||||
ok: false,
|
||||
error: { cleanupResources: ["hopper-document", runtimeRoot.path] },
|
||||
it.each([false, true])(
|
||||
"keeps an unconfirmed external document across closes when helper ownership is %s",
|
||||
async (ownsHelper) => {
|
||||
const launcher = new RetryCleanupLauncher(true);
|
||||
const runtimeRoot = await PrivateRuntimeRoot.create();
|
||||
onTestFinished(() => runtimeRoot.close());
|
||||
const launched = await launcher.launch({
|
||||
directory: runtimeRoot.path,
|
||||
socketPath: join(runtimeRoot.path, "bridge.sock"),
|
||||
token: "fixture-token",
|
||||
runId: "fixture-run",
|
||||
});
|
||||
}
|
||||
expect(child.exitCode).toBeNull();
|
||||
expect(child.signalCode).toBeNull();
|
||||
await expect(
|
||||
access(join(runtimeRoot.path, "image.macho")),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
if (!launched.ok) throw launched.error;
|
||||
const child = launched.value.process;
|
||||
const launch: BridgeLaunch = ownsHelper
|
||||
? { ...launched.value, ownsProcessLifetime: true }
|
||||
: launched.value;
|
||||
const supervisor = new ProviderProcessSupervisor(launch);
|
||||
onTestFinished(() => supervisor.dispose());
|
||||
const resources: HopperOwnedResources = {
|
||||
launch,
|
||||
processSupervisor: supervisor,
|
||||
runtimeRoot,
|
||||
shutdownConfirmed: false,
|
||||
};
|
||||
const socket = new Socket();
|
||||
onTestFinished(() => {
|
||||
socket.destroy();
|
||||
});
|
||||
for (let attempt = 0; attempt < 2; attempt += 1) {
|
||||
await expect(
|
||||
cleanupHopperSession({
|
||||
socket: attempt === 0 ? socket : undefined,
|
||||
resources,
|
||||
activeRequest: null,
|
||||
retainDocument: false,
|
||||
progress: undefined,
|
||||
logger: silentLogger,
|
||||
onDiagnostic: undefined,
|
||||
request: () =>
|
||||
Promise.resolve(
|
||||
ok({
|
||||
shutdown: true,
|
||||
analysis_stopped: true,
|
||||
document_closed: false,
|
||||
}),
|
||||
),
|
||||
releaseTransport: (socket) => socket?.destroy(),
|
||||
}),
|
||||
).resolves.toMatchObject({
|
||||
ok: false,
|
||||
error: { cleanupResources: ["hopper-document", runtimeRoot.path] },
|
||||
});
|
||||
}
|
||||
if (ownsHelper)
|
||||
expect(child.exitCode !== null || child.signalCode !== null).toBe(true);
|
||||
else {
|
||||
expect(child.exitCode).toBeNull();
|
||||
expect(child.signalCode).toBeNull();
|
||||
}
|
||||
await expect(
|
||||
access(join(runtimeRoot.path, "image.macho")),
|
||||
).resolves.toBeUndefined();
|
||||
},
|
||||
);
|
||||
|
||||
it("blocks a fresh launch while cleanup remains unverified", async () => {
|
||||
const launcher = new RetryCleanupLauncher();
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { EventEmitter } from "node:events";
|
||||
import { PassThrough } from "node:stream";
|
||||
|
||||
import { describe, expect, it, onTestFinished, vi } from "vitest";
|
||||
|
||||
@@ -12,15 +13,18 @@ import { HopperClient } from "../../../../src/hopper/HopperClient.js";
|
||||
import { HopperFixtureLauncher } from "./hopperClient.fixture.js";
|
||||
|
||||
class ObservedProcess extends EventEmitter {
|
||||
readonly stdout = null;
|
||||
readonly stderr = null;
|
||||
readonly stdout = new PassThrough();
|
||||
readonly stderr = new PassThrough();
|
||||
exitCode: number | null = null;
|
||||
readonly signalCode = null;
|
||||
signalCode: NodeJS.Signals | null = null;
|
||||
|
||||
exit(code: number): void {
|
||||
exit(code: number | null, signal: NodeJS.Signals | null = null): void {
|
||||
this.exitCode = code;
|
||||
this.emit("exit", code, null);
|
||||
this.emit("close", code, null);
|
||||
this.signalCode = signal;
|
||||
this.emit("exit", code, signal);
|
||||
this.stdout.end();
|
||||
this.stderr.end();
|
||||
this.emit("close", code, signal);
|
||||
}
|
||||
|
||||
kill(): boolean {
|
||||
@@ -66,7 +70,7 @@ describe("Hopper provider lifecycle health", () => {
|
||||
launcher.observed.exit(0);
|
||||
const pending = client.callTool("hang");
|
||||
await launcher.fixture.waitForRequest("hang");
|
||||
launcher.fixture.processes.at(-1)?.kill("SIGKILL");
|
||||
launcher.fixture.processes.at(-1)?.send({ disconnect: true });
|
||||
await vi.waitFor(() =>
|
||||
expect(client.operationHealth().state).toBe("unreachable"),
|
||||
);
|
||||
@@ -92,6 +96,7 @@ describe("Hopper provider lifecycle health", () => {
|
||||
operation: "procedure_pseudo_code",
|
||||
},
|
||||
});
|
||||
expect((await client.close()).ok).toBe(true);
|
||||
});
|
||||
|
||||
it("projects the exit code when the owned process exit is observed before disconnection", async () => {
|
||||
@@ -120,6 +125,7 @@ describe("Hopper provider lifecycle health", () => {
|
||||
stage: "analysis",
|
||||
operation: "hang",
|
||||
request_id: request.id,
|
||||
launcher: { exit_code: 7, signal: null, output_closed: false },
|
||||
},
|
||||
});
|
||||
});
|
||||
@@ -166,3 +172,52 @@ describe("Hopper provider lifecycle health", () => {
|
||||
expect(client.operationHealth().state).toBe("idle");
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves an owned signal exit and diagnostics for active, queued and later callers", async () => {
|
||||
const { client, launcher } = await start(true);
|
||||
const active = client.callTool("hang");
|
||||
const request = await launcher.fixture.waitForRequest("hang");
|
||||
const queued = client.callTool("echo");
|
||||
await vi.waitFor(() =>
|
||||
expect(client.requestActivity()?.queuedRequests).toBe(1),
|
||||
);
|
||||
const token = launcher.fixture.tokens[0];
|
||||
launcher.observed.stderr.write(`API_KEY=local-evidence-kept ${token}\n`);
|
||||
launcher.observed.exit(null, "SIGTERM");
|
||||
const results = await Promise.all([active, queued]);
|
||||
for (const [index, result] of results.entries()) {
|
||||
if (result.ok) throw new Error("Expected an observed provider exit");
|
||||
const projected = projectAnalysisError(result.error);
|
||||
expect(projected).toMatchObject({
|
||||
code: "provider_unavailable",
|
||||
message: expect.stringContaining("signal SIGTERM"),
|
||||
details: {
|
||||
provider_state: "exited",
|
||||
operation: index === 0 ? "hang" : "echo",
|
||||
request_id: request.id + index,
|
||||
launcher: {
|
||||
exit_code: null,
|
||||
signal: "SIGTERM",
|
||||
output_closed: false,
|
||||
stderr: {
|
||||
text: "API_KEY=local-evidence-kept [redacted transport credential]\n",
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(JSON.stringify(projected)).not.toContain(token);
|
||||
}
|
||||
const later = await client.callTool("procedure_pseudo_code", {
|
||||
procedure: "main",
|
||||
});
|
||||
if (later.ok) throw new Error("An exited provider must reject later queries");
|
||||
expect(projectAnalysisError(later.error)).toMatchObject({
|
||||
code: "provider_unavailable",
|
||||
details: {
|
||||
provider_state: "exited",
|
||||
stage: "decompilation",
|
||||
operation: "procedure_pseudo_code",
|
||||
launcher: { signal: "SIGTERM", output_closed: true },
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,13 +1,68 @@
|
||||
import { spawn } from "node:child_process";
|
||||
import { access } from "node:fs/promises";
|
||||
|
||||
import { expect, it, onTestFinished } from "vitest";
|
||||
|
||||
import { AnalysisError } from "../../../../src/domain/analysisErrorBase.js";
|
||||
import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js";
|
||||
import { ok } from "../../../../src/domain/result.js";
|
||||
import type { BridgeLauncher } from "../../../../src/hopper/BridgeLauncher.js";
|
||||
import type {
|
||||
BridgeLauncher,
|
||||
BridgeSession,
|
||||
} from "../../../../src/hopper/BridgeLauncher.js";
|
||||
import { HopperApplicationLauncher } from "../../../../src/hopper/BridgeLauncher.js";
|
||||
import { HopperClient } from "../../../../src/hopper/HopperClient.js";
|
||||
import { HopperFixtureLauncher } from "./hopperClient.fixture.js";
|
||||
|
||||
// Failure assertions keep the external document unknown. Teardown then publishes
|
||||
// this run's authenticated bridge so normal cleanup can release its runtime/lease.
|
||||
const trackFailedHelperClient = (
|
||||
launcher: BridgeLauncher,
|
||||
startupTimeoutMs = 10_000,
|
||||
): HopperClient => {
|
||||
let session: BridgeSession | undefined;
|
||||
let external = false;
|
||||
const client = new HopperClient({
|
||||
launcher: {
|
||||
async launch(input, options) {
|
||||
session = input;
|
||||
const launched = await launcher.launch(input, options);
|
||||
external =
|
||||
launched.ok &&
|
||||
launched.value.providerLifetime === "external-application";
|
||||
return launched;
|
||||
},
|
||||
},
|
||||
startupTimeoutMs,
|
||||
});
|
||||
onTestFinished(async () => {
|
||||
const fixture = new HopperFixtureLauncher();
|
||||
const published =
|
||||
external && session !== undefined
|
||||
? await fixture.launch(session)
|
||||
: undefined;
|
||||
try {
|
||||
const closed = await client.close();
|
||||
if (!closed.ok) throw closed.error;
|
||||
if (published !== undefined) {
|
||||
expect(fixture.requests.map((request) => request.method)).toEqual([
|
||||
"health",
|
||||
"shutdown_document",
|
||||
]);
|
||||
}
|
||||
if (session !== undefined)
|
||||
await expect(access(session.directory)).rejects.toMatchObject({
|
||||
code: "ENOENT",
|
||||
});
|
||||
} finally {
|
||||
if (published?.ok) {
|
||||
const cleaned = await published.value.cleanup();
|
||||
expect(cleaned.cleaned).toBe(true);
|
||||
}
|
||||
}
|
||||
});
|
||||
return client;
|
||||
};
|
||||
|
||||
const projectPrimaryStartupFailure = (error: AnalysisError) => {
|
||||
if (!error.cleanupIncomplete) return projectAnalysisError(error);
|
||||
@@ -32,10 +87,7 @@ it("does not equate an owned native helper exit with an external GUI exit", asyn
|
||||
bridgeScriptPath: "/rea/hopper_bridge.py",
|
||||
launchMode: "native",
|
||||
});
|
||||
const client = new HopperClient({ launcher, startupTimeoutMs: 10_000 });
|
||||
onTestFinished(async () => {
|
||||
await client.close();
|
||||
}, 45_000);
|
||||
const client = trackFailedHelperClient(launcher);
|
||||
const result = await client.start();
|
||||
expect(result).toMatchObject({
|
||||
ok: false,
|
||||
@@ -84,10 +136,7 @@ it.each(["exit", "signal", "exit75"] as const)(
|
||||
);
|
||||
},
|
||||
};
|
||||
const client = new HopperClient({ launcher, startupTimeoutMs: 10_000 });
|
||||
onTestFinished(async () => {
|
||||
await client.close();
|
||||
}, 45_000);
|
||||
const client = trackFailedHelperClient(launcher);
|
||||
const result = await client.start();
|
||||
expect(result).toMatchObject({
|
||||
ok: false,
|
||||
@@ -136,10 +185,7 @@ it.each([0, 1])(
|
||||
);
|
||||
},
|
||||
};
|
||||
const client = new HopperClient({ launcher, startupTimeoutMs: 10_000 });
|
||||
onTestFinished(async () => {
|
||||
await client.close();
|
||||
}, 45_000);
|
||||
const client = trackFailedHelperClient(launcher);
|
||||
const result = await client.start();
|
||||
expect(result).toMatchObject({
|
||||
ok: false,
|
||||
@@ -174,10 +220,7 @@ it("preserves a successful helper's diagnostics when bridge readiness is never o
|
||||
);
|
||||
},
|
||||
};
|
||||
const client = new HopperClient({ launcher, startupTimeoutMs: 1500 });
|
||||
onTestFinished(async () => {
|
||||
await client.close();
|
||||
}, 45_000);
|
||||
const client = trackFailedHelperClient(launcher, 1500);
|
||||
const result = await client.start();
|
||||
if (result.ok)
|
||||
throw new Error("Expected missing bridge readiness to time out");
|
||||
|
||||
@@ -53,10 +53,7 @@ class CancelThenFixtureLauncher implements BridgeLauncher {
|
||||
}
|
||||
|
||||
class ExitingLauncher implements BridgeLauncher {
|
||||
constructor(
|
||||
readonly code: number,
|
||||
readonly ownsProcessLifetime = true,
|
||||
) {}
|
||||
constructor(readonly code: number) {}
|
||||
|
||||
launch() {
|
||||
return Promise.resolve(
|
||||
@@ -64,10 +61,8 @@ class ExitingLauncher implements BridgeLauncher {
|
||||
process: spawn(process.execPath, ["-e", `process.exit(${this.code})`], {
|
||||
stdio: ["ignore", "ignore", "pipe"],
|
||||
}),
|
||||
ownsProcessLifetime: this.ownsProcessLifetime,
|
||||
providerLifetime: this.ownsProcessLifetime
|
||||
? ("launcher-process" as const)
|
||||
: ("external-application" as const),
|
||||
ownsProcessLifetime: true as const,
|
||||
providerLifetime: "launcher-process" as const,
|
||||
shutdownMode: "bridge-request" as const,
|
||||
}),
|
||||
);
|
||||
@@ -286,21 +281,4 @@ describe("HopperClient startup failures", () => {
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("allows a short-lived launcher to hand off bridge startup", async () => {
|
||||
const client = new HopperClient({
|
||||
launcher: new ExitingLauncher(0, false),
|
||||
startupTimeoutMs: 100,
|
||||
});
|
||||
clients.push(client);
|
||||
const result = await client.start();
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) {
|
||||
expect(result.error).toMatchObject({
|
||||
cleanupIncomplete: true,
|
||||
cause: { _tag: "HopperTimeoutError" },
|
||||
diagnostics: { primary_error: { code: "provider_timeout" } },
|
||||
});
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Vendored
+7
@@ -10,8 +10,13 @@ const [
|
||||
] = process.argv.slice(2);
|
||||
let diagnosticOutputEmitted = false;
|
||||
const heldReplies = new Map();
|
||||
const sockets = new Set();
|
||||
if (process.send !== undefined)
|
||||
process.on("message", (message) => {
|
||||
if (message?.disconnect === true) {
|
||||
for (const socket of sockets) socket.destroy();
|
||||
return;
|
||||
}
|
||||
const reply = heldReplies.get(message?.release);
|
||||
if (reply === undefined) return;
|
||||
heldReplies.delete(message.release);
|
||||
@@ -173,6 +178,8 @@ const sendEchoWithProgress = (send, request) => {
|
||||
};
|
||||
|
||||
const server = createServer((socket) => {
|
||||
sockets.add(socket);
|
||||
socket.once("close", () => sockets.delete(socket));
|
||||
socket.on("error", () => undefined);
|
||||
socket.setEncoding("utf8");
|
||||
let buffer = "";
|
||||
|
||||
Reference in New Issue
Block a user