From 7ea519d5355cf2de5a951b3bed3a4384fadb882a Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Wed, 26 Aug 2026 17:27:05 -0700 Subject: [PATCH] fix(cli): block recovery after lifecycle probe failures --- .../sandbox/gateway-state-drift.test.ts | 17 +++++ src/lib/actions/sandbox/gateway-state.ts | 3 + .../openshell/sandbox-observer-cli.ts | 10 +-- src/lib/gateway-runtime-action.test.ts | 40 ++++++++++++ src/lib/gateway-runtime-action.ts | 63 ++++++++++++++----- 5 files changed, 112 insertions(+), 21 deletions(-) diff --git a/src/lib/actions/sandbox/gateway-state-drift.test.ts b/src/lib/actions/sandbox/gateway-state-drift.test.ts index 0a4b8105016..bccd3e23431 100644 --- a/src/lib/actions/sandbox/gateway-state-drift.test.ts +++ b/src/lib/actions/sandbox/gateway-state-drift.test.ts @@ -263,6 +263,23 @@ describe("sandbox gateway state drift guard", () => { expect(removeSandboxSpy).not.toHaveBeenCalled(); }); + it("does not select a gateway when its lifecycle probe blocks recovery (#10421)", async () => { + getNamedGatewayLifecycleStateSpy.mockReturnValue({ + state: "connected_other", + activeGateway: "openshell", + status: "", + recoveryBlocked: true, + }); + const missing = { state: "missing", output: "NotFound" }; + + await expect( + gatewayState.reconcileMissingAgainstNamedGateway("alpha", missing), + ).resolves.toEqual(missing); + + expect(runOpenshellSpy).not.toHaveBeenCalled(); + expect(removeSandboxSpy).not.toHaveBeenCalled(); + }); + it("routes gateway-error recovery to the sandbox persisted gateway", async () => { detectPreflightIssueSpy.mockReturnValue(null); getSandboxSpy.mockReturnValue({ diff --git a/src/lib/actions/sandbox/gateway-state.ts b/src/lib/actions/sandbox/gateway-state.ts index 6e6a200cebb..ed39a120e7e 100644 --- a/src/lib/actions/sandbox/gateway-state.ts +++ b/src/lib/actions/sandbox/gateway-state.ts @@ -442,6 +442,9 @@ export async function reconcileMissingAgainstNamedGateway( return tryRecoverDockerDriverSandbox(sandboxName, missingLookup, pinnedGatewayName); } const lifecycle = getNamedGatewayLifecycleState(targetGatewayName); + if (lifecycle.recoveryBlocked) { + return missingLookup; + } if (lifecycle.state === "connected_other") { runOpenshell(["gateway", "select", targetGatewayName], { ignoreError: true, diff --git a/src/lib/adapters/openshell/sandbox-observer-cli.ts b/src/lib/adapters/openshell/sandbox-observer-cli.ts index 8ab1b40e1ce..6a95929d078 100644 --- a/src/lib/adapters/openshell/sandbox-observer-cli.ts +++ b/src/lib/adapters/openshell/sandbox-observer-cli.ts @@ -143,7 +143,9 @@ function successfulCommandOutput(result: CapturedSandboxCommandResult): string { return stripOpenShellCliAnsi(result.stdout ?? result.output); } -function commandError(result: CapturedSandboxCommandResult): OpenShellSandboxError | null { +export function classifyCliOpenShellCommandError( + result: CapturedSandboxCommandResult, +): OpenShellSandboxError | null { const output = stripOpenShellCliAnsi(commandOutput(result)); const errorCode = (result.error as NodeJS.ErrnoException | undefined)?.code; if (errorCode === "ETIMEDOUT") { @@ -156,7 +158,7 @@ function commandError(result: CapturedSandboxCommandResult): OpenShellSandboxErr }; } if ( - /\b(?:authentication failed|unauthorized|forbidden|permission denied|missing gateway auth token|device identity required|invalid token|expired token)\b/iu.test( + /\b(?:authentication failed|unauthorized|forbidden|permission denied|requires admin privileges|missing gateway auth token|device identity required|invalid token|expired token)\b/iu.test( output, ) ) { @@ -223,7 +225,7 @@ export function createCliOpenShellSandboxLookup( timeout: request.timeoutMs ?? deps.defaultTimeoutMs ?? OPENSHELL_PROBE_TIMEOUT_MS, }); const output = commandOutput(result); - const error = commandError(result); + const error = classifyCliOpenShellCommandError(result); if (error && error.kind !== "command") { return { result: failure(error), displayOutput: "" }; } @@ -256,7 +258,7 @@ export function createCliOpenShellSandboxObserver( includeStreams: true, timeout: request.timeoutMs ?? deps.defaultTimeoutMs ?? OPENSHELL_PROBE_TIMEOUT_MS, }); - const error = commandError(result); + const error = classifyCliOpenShellCommandError(result); if (error) return failure(error); return success(parseCliOpenShellSandboxInventory(successfulCommandOutput(result))); }; diff --git a/src/lib/gateway-runtime-action.test.ts b/src/lib/gateway-runtime-action.test.ts index 7b57fe83b08..262cfffb9c5 100644 --- a/src/lib/gateway-runtime-action.test.ts +++ b/src/lib/gateway-runtime-action.test.ts @@ -155,6 +155,46 @@ describe("gateway-runtime-action per-sandbox gateway routing", () => { }); describe("recoverNamedGatewayRuntime", () => { + it.each([ + { label: "authentication", status: 1, output: "gateway info requires admin privileges" }, + { label: "schema validation", status: 1, output: "protobuf decode error: invalid wire type" }, + { label: "gateway identity validation", status: 1, output: "handshake verification failed" }, + { label: "request validation", status: 2, output: "unknown option: --json" }, + ] as const)( + "does not select or start a gateway when $label lifecycle probe fails (#10421)", + async ({ status, output }) => { + captureSpy.mockReturnValue({ status, output }); + runSpy.mockReturnValue({ status: 0 } as never); + + const result = await gatewayRuntime.recoverNamedGatewayRuntime({ + gatewayName: "nemoclaw-8090", + }); + + expect(result).toMatchObject({ recovered: false, attempted: false }); + expect(runSpy).not.toHaveBeenCalled(); + expect(startGatewaySpy).not.toHaveBeenCalled(); + }, + ); + + it("does not start a gateway when the post-selection lifecycle probe fails (#10421)", async () => { + captureSpy + .mockReturnValueOnce({ + status: 0, + output: "Status: Connected\nGateway: nemoclaw\n", + }) + .mockReturnValueOnce({ status: 0, output: "Gateway: nemoclaw\n" }) + .mockReturnValue({ status: 1, output: "gateway info requires admin privileges" }); + runSpy.mockReturnValue({ status: 0 } as never); + + const result = await gatewayRuntime.recoverNamedGatewayRuntime({ + gatewayName: "nemoclaw-8090", + }); + + expect(result).toMatchObject({ recovered: false, attempted: true }); + expect(runSpy).toHaveBeenCalledOnce(); + expect(startGatewaySpy).not.toHaveBeenCalled(); + }); + it("selects the supplied gateway name on the recovery path", async () => { captureSpy .mockReturnValueOnce({ status: 0, output: "Status: Disconnected\nGateway: nemoclaw\n" }) diff --git a/src/lib/gateway-runtime-action.ts b/src/lib/gateway-runtime-action.ts index fdb6822049f..e5af1807d11 100644 --- a/src/lib/gateway-runtime-action.ts +++ b/src/lib/gateway-runtime-action.ts @@ -3,6 +3,10 @@ import { stripAnsi } from "./adapters/openshell/client"; import * as openshellRuntime from "./adapters/openshell/runtime"; +import { + classifyCliOpenShellCommandError, + type CapturedSandboxCommandResult, +} from "./adapters/openshell/sandbox-observer-cli"; import { OPENSHELL_OPERATION_TIMEOUT_MS, OPENSHELL_PROBE_TIMEOUT_MS, @@ -54,6 +58,29 @@ function getActiveGatewayName(output = ""): string | null { return match ? match[1].trim() : null; } +function blocksNamedGatewayRecovery(result: CapturedSandboxCommandResult): boolean { + const error = classifyCliOpenShellCommandError(result); + return ( + error?.kind === "authentication" || + error?.kind === "schema" || + (error?.kind === "transport" && error.reason === "identity_mismatch") || + (error?.kind === "command" && error.reason === "invalid_request") + ); +} + +export type NamedGatewayLifecycleState = { + state: + | "healthy_named" + | "named_unreachable" + | "named_unhealthy" + | "connected_other" + | "missing_named"; + status: string; + gatewayInfo: string; + activeGateway: string | null; + recoveryBlocked?: boolean; +}; + /** * Classify the lifecycle state of the named gateway (healthy_named, * named_unreachable, named_unhealthy, connected_other, or missing_named) from @@ -64,7 +91,7 @@ function getActiveGatewayName(output = ""): string | null { export function getNamedGatewayLifecycleState( gatewayName: string = resolveGatewayName(GATEWAY_PORT), opts: { ignoreProbeErrors?: boolean } = {}, -) { +): NamedGatewayLifecycleState { // #5714: callers that must stay non-fatal (e.g. plain `nemoclaw list` // recovery) opt into `ignoreProbeErrors` so a hung/timed-out `openshell // status` returns a not-healthy classification instead of `process.exit`ing @@ -97,6 +124,12 @@ export function getNamedGatewayLifecycleState( const refusing = /Connection refused|client error \(Connect\)|tcp connect error/i.test( cleanStatus, ); + const lifecycle = { + status: status.output, + gatewayInfo: gatewayInfo.output, + activeGateway, + recoveryBlocked: blocksNamedGatewayRecovery(status) || blocksNamedGatewayRecovery(gatewayInfo), + }; // OpenShell 0.0.72 can serve status and sandbox RPCs but does not implement // GetGatewayInfo. The pre-upgrade backup deliberately uses the current CLI // against that existing gateway before replacing it. Accept only the exact @@ -105,44 +138,34 @@ export function getNamedGatewayLifecycleState( if (connected && activeGateway === gatewayName && (named || gatewayInfoUnsupported)) { return { state: "healthy_named", - status: status.output, - gatewayInfo: gatewayInfo.output, - activeGateway, + ...lifecycle, }; } if (activeGateway === gatewayName && named && refusing) { return { state: "named_unreachable", - status: status.output, - gatewayInfo: gatewayInfo.output, - activeGateway, + ...lifecycle, }; } if (activeGateway === gatewayName && named) { return { state: "named_unhealthy", - status: status.output, - gatewayInfo: gatewayInfo.output, - activeGateway, + ...lifecycle, }; } if (connected) { return { state: "connected_other", - status: status.output, - gatewayInfo: gatewayInfo.output, - activeGateway, + ...lifecycle, }; } return { state: "missing_named", - status: status.output, - gatewayInfo: gatewayInfo.output, - activeGateway, + ...lifecycle, }; } -type NamedGatewayLifecycleStateName = ReturnType["state"]; +type NamedGatewayLifecycleStateName = NamedGatewayLifecycleState["state"]; export type RecoverNamedGatewayRuntimeOptions = { recoverableStates?: readonly NamedGatewayLifecycleStateName[]; @@ -161,6 +184,9 @@ export async function recoverNamedGatewayRuntime(options: RecoverNamedGatewayRun ], ); const before = getNamedGatewayLifecycleState(gatewayName); + if (before.recoveryBlocked) { + return { recovered: false, before, after: before, attempted: false }; + } if (before.state === "healthy_named") { return { recovered: true, before, after: before, attempted: false }; } @@ -174,6 +200,9 @@ export async function recoverNamedGatewayRuntime(options: RecoverNamedGatewayRun timeout: OPENSHELL_OPERATION_TIMEOUT_MS, }); let after = getNamedGatewayLifecycleState(gatewayName); + if (after.recoveryBlocked) { + return { recovered: false, before, after, attempted: true }; + } if (after.state === "healthy_named") { process.env.OPENSHELL_GATEWAY = gatewayName; return { recovered: true, before, after, attempted: true, via: "select" };