diff --git a/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx b/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx index 365f77e5573..b7fe51136fe 100644 --- a/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx +++ b/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx @@ -27,7 +27,7 @@ If Docker no longer has the container, follow the printed `rebuild --yes` guidan The `start` command returns success only after it authenticates the recovered agent runtime, OpenShell reports the sandbox ready, and host-side port forwards pass their checks. -If any check fails, the command keeps the existing container, exits nonzero, identifies the failure, and tells you to run `recover` before retrying `start`. +If a check fails, the command exits nonzero, identifies the failure, and prints recovery guidance before you retry `start`. @@ -135,6 +135,7 @@ After a transactional recreation, NemoClaw waits 120 seconds for OpenShell to re Set `NEMOCLAW_GATEWAY_RECOVERY_WAIT_SECONDS` before the recovery command to change this budget. A definitive managed-health failure still stops immediately. If re-registration, state restoration, or a later gateway check fails, NemoClaw attempts to roll back the replacement and leaves the primary dashboard or API host forward stopped. +If NemoClaw cannot confirm rollback to the previous container, inspect Docker state before you retry recovery. For the controller topology, trust boundary, and fail-closed conditions, refer to [Understand Gateway Lifecycle Control](../configure-sandboxes/understand-gateway-lifecycle-control). If recovery cannot repair a sandbox that needs credentials or a current controller contract, rebuild it. diff --git a/src/lib/actions/sandbox/connect-flow.test.ts b/src/lib/actions/sandbox/connect-flow.test.ts index 121c238d438..a2fcad6ce22 100644 --- a/src/lib/actions/sandbox/connect-flow.test.ts +++ b/src/lib/actions/sandbox/connect-flow.test.ts @@ -972,6 +972,77 @@ describe("connectSandbox flow", () => { expect(exitSpy).toHaveBeenCalledWith(1); }); + it.each([ + { + condition: "a Docker final-handoff failure", + expectedDetail: + "Docker could not start the replacement container to complete the final recovery handoff", + recoveryFailureDetail: + "Docker could not start the replacement container to complete the final recovery handoff", + }, + { + condition: "a pinned replacement identity failure", + expectedDetail: + "the replacement container identity changed during the final managed supervisor health check", + recoveryFailureDetail: + "the replacement container identity changed during the final managed supervisor health check", + }, + { + condition: "an OpenShell readiness failure", + expectedDetail: "the replacement container did not become ready in OpenShell", + recoveryFailureDetail: + "the replacement container did not become ready in OpenShell\nAuthorization: Bearer opaque-connect-recovery-token\u001b[31m", + }, + { + condition: "an unconfirmed rollback after a gateway wait failure", + expectedDetail: "NemoClaw could not confirm rollback to the previous sandbox container", + recoveryFailureDetail: + "NemoClaw could not confirm rollback to the previous sandbox container. Inspect Docker state before retrying. Recovery failure before rollback: the recovered gateway did not become responsive before the recovery timeout", + }, + { + condition: "a detail-free recovery failure", + expectedDetail: "the gateway recovery attempt did not complete", + recoveryFailureDetail: undefined, + }, + ])( + "stops non-probe connect before route repair, pairing, or SSH after $condition (#9364)", + async ({ expectedDetail, recoveryFailureDetail }) => { + const harness = createConnectHarness({ + registryEntry: { model: "qwen3-vl:4b", provider: "ollama-local" }, + processCheck: { + checked: true, + wasRunning: false, + recovered: false, + forwardRecovered: false, + recoveryFailureDetail, + }, + }); + + await expect(harness.connectSandbox("alpha")).rejects.toThrow("process.exit(1)"); + + const errorOutput = harness.errorSpy.mock.calls + .map((call) => String(call[0] ?? "")) + .join("\n"); + expect(errorOutput).toContain( + "Recovery failed: NemoClaw could not recover the OpenClaw gateway in 'alpha'", + ); + expect(errorOutput).toContain(expectedDetail); + expect(errorOutput).not.toContain("opaque-connect-recovery-token"); + expect(errorOutput).not.toContain("\u001b"); + expect(harness.ensureOllamaAuthProxySpy).not.toHaveBeenCalled(); + expect(harness.findReachableOllamaHostSpy).not.toHaveBeenCalled(); + expect(harness.withGatewayRouteMutationLockSpy).not.toHaveBeenCalled(); + expect(harness.settlePortablePairingSpy).not.toHaveBeenCalled(); + expect(harness.runAutoPairSpy).not.toHaveBeenCalled(); + expect(harness.spawnSyncSpy).not.toHaveBeenCalledWith( + "openshell", + ["sandbox", "connect", "alpha"], + expect.any(Object), + ); + expect(exitSpy).toHaveBeenCalledWith(1); + }, + ); + it("redacts untrusted gateway recovery details before reporting them", async () => { const opaqueToken = "opaque-gateway-recovery-token"; const harness = createConnectHarness({ diff --git a/src/lib/actions/sandbox/connect.ts b/src/lib/actions/sandbox/connect.ts index 0e2702e0969..878c2b7e643 100644 --- a/src/lib/actions/sandbox/connect.ts +++ b/src/lib/actions/sandbox/connect.ts @@ -267,14 +267,20 @@ function exitOnGatewayRecoveryFailure( sandboxName: string, agentName: string, detail: string, + operation: "Probe" | "Recovery" = "Probe", + showWedgeDiagnostics = false, ): never { const safeDetail = sanitizeSandboxStartupRecoveryDetail(detail); const terminalPunctuation = /[.!?]$/u.test(safeDetail) ? "" : "."; console.error(""); console.error( - ` Probe failed: NemoClaw could not recover the ${agentName} gateway in '${sandboxName}'.`, + ` ${operation} failed: NemoClaw could not recover the ${agentName} gateway in '${sandboxName}'.`, ); console.error(` Recovery detail: ${safeDetail}${terminalPunctuation}`); + if (showWedgeDiagnostics) { + printGatewayWedgeDiagnostics(sandboxName, executeSandboxExecCommand); + console.error(" Check /tmp/gateway.log inside the sandbox for details."); + } process.exit(1); } @@ -341,6 +347,8 @@ async function runSandboxConnectProbe(sandboxName: string): Promise { sandboxName, agentName, String(processCheck.recoveryFailureDetail), + "Probe", + true, ); } if (processCheck.wasRunning) { @@ -1295,6 +1303,16 @@ export async function prepareInteractiveSession( const agentName = agentRuntime.getAgentDisplayName(agentRuntime.getSessionAgent(sandboxName)); exitOnMcpReconciliationRefusal(sandboxName, agentName, processCheck, "Connect"); } + const recoveryFailureDetail = + "recoveryFailureDetail" in processCheck && processCheck.recoveryFailureDetail + ? String(processCheck.recoveryFailureDetail) + : processCheck.checked && processCheck.wasRunning === false && processCheck.recovered === false + ? "the gateway recovery attempt did not complete" + : null; + if (recoveryFailureDetail) { + const agentName = agentRuntime.getAgentDisplayName(agentRuntime.getSessionAgent(sandboxName)); + exitOnGatewayRecoveryFailure(sandboxName, agentName, recoveryFailureDetail, "Recovery"); + } // Ensure Ollama auth proxy is running (recovers from host reboots) ensureOllamaAuthProxy(); diff --git a/src/lib/actions/sandbox/gateway-restart.test.ts b/src/lib/actions/sandbox/gateway-restart.test.ts index 7cf49e12672..8e4285639e7 100644 --- a/src/lib/actions/sandbox/gateway-restart.test.ts +++ b/src/lib/actions/sandbox/gateway-restart.test.ts @@ -16,6 +16,7 @@ const supervisorFailureMarkers: Array< [string, ReturnType["layer"]] > = [ ["PRIVILEGED_CONTROL_UNAVAILABLE", "privileged control unavailable"], + ["MANAGED_CONTROL_IDENTITY_CHANGED", "container identity changed"], ["SUPERVISOR_UNAVAILABLE", "privileged control unavailable"], ["SUPERVISOR_UNAVAILABLE\nNEMOCLAW_CONTROL_STAGE=await-replacement", "supervisor unavailable"], ["SUPERVISOR_NOT_RUNNING", "supervisor not running"], @@ -90,6 +91,25 @@ describe("gateway restart failure classification precedence", () => { layer: "supervisor not running", }); }); + + it("does not classify an embedded identity marker as a protocol marker", () => { + expect(classify("failure mentions MANAGED_CONTROL_IDENTITY_CHANGED inline")).toMatchObject({ + layer: "launch failure", + }); + }); + + it("removes every complete identity marker line from the failure detail", () => { + const output = [ + " MANAGED_CONTROL_IDENTITY_CHANGED ", + "container changed once", + "MANAGED_CONTROL_IDENTITY_CHANGED", + "container changed again", + ].join("\n"); + expect(classify(output)).toEqual({ + layer: "container identity changed", + detail: "container changed once\ncontainer changed again", + }); + }); }); describe("restartSandboxGateway — host-mediated gateway restart", () => { diff --git a/src/lib/actions/sandbox/gateway-restart.ts b/src/lib/actions/sandbox/gateway-restart.ts index 22814d3ae84..ae4ec480355 100644 --- a/src/lib/actions/sandbox/gateway-restart.ts +++ b/src/lib/actions/sandbox/gateway-restart.ts @@ -15,6 +15,8 @@ export type GatewayRestartCommandResult = { stderr: string; }; +export const MANAGED_CONTROL_IDENTITY_CHANGED_MARKER = "MANAGED_CONTROL_IDENTITY_CHANGED"; + export type ManagedGatewayControlCompletion = { disposition: "ok" | "already-running"; oldPid: number; @@ -49,6 +51,7 @@ export type GatewayRestartFailureLayer = | "privileged control unavailable" | "supervisor not running" | "supervisor unavailable" + | "container identity changed" | "secret-boundary refusal" | "unsafe config path" | "config hash mismatch" @@ -185,6 +188,10 @@ export function classifyGatewayRestartFailure(result: GatewayRestartCommandResul } const output = gatewayRestartOutput(result); + const outputLines = output.split(/\r?\n/); + const isIdentityChangedMarkerLine = (line: string) => + line.trim() === MANAGED_CONTROL_IDENTITY_CHANGED_MARKER; + const hasIdentityChangedMarker = outputLines.some(isIdentityChangedMarkerLine); const detail = sanitizeGatewayRestartFailureDetail(output.trim()); if (output.includes("SUPERVISOR_NOT_RUNNING")) { return { @@ -198,6 +205,15 @@ export function classifyGatewayRestartFailure(result: GatewayRestartCommandResul detail: detail || "the managed gateway supervisor became unavailable", }; } + if (hasIdentityChangedMarker) { + return { + layer: "container identity changed", + detail: + sanitizeGatewayRestartFailureDetail( + outputLines.filter((line) => !isIdentityChangedMarkerLine(line)).join("\n").trim(), + ) || "the selected container identity changed", + }; + } if ( output.includes(MARKERS.ROOT_EXEC_UNAVAILABLE) || output.includes("PRIVILEGED_CONTROL_UNAVAILABLE") || diff --git a/src/lib/actions/sandbox/launch.test.ts b/src/lib/actions/sandbox/launch.test.ts index 3fcd572dc1a..787462eff1d 100644 --- a/src/lib/actions/sandbox/launch.test.ts +++ b/src/lib/actions/sandbox/launch.test.ts @@ -461,6 +461,27 @@ describe("launchSandbox", () => { expect(mocks.publishLaunchReadiness).toHaveBeenCalledBefore(mocks.execSandbox); }); + it("stops before readiness publication and agent execution when recovery rejects launch (#9364)", async () => { + mocks.inspectLaunchReadiness.mockResolvedValue({ + kind: "fallback", + category: "expired", + fence: { epochId: "a".repeat(64) }, + gatewayName: "nemoclaw", + gatewayPort: 8080, + fenceFailed: false, + recoveryBlocked: false, + }); + const recoveryFailure = new Error("process.exit(1)"); + mocks.prepareInteractiveSession.mockRejectedValueOnce(recoveryFailure); + + await expect(launchSandbox("alpha")).rejects.toBe(recoveryFailure); + + expect(mocks.prepareInteractiveSession).toHaveBeenCalledOnce(); + expect(mocks.publishLaunchReadiness).not.toHaveBeenCalled(); + expect(mocks.prepareHermesLightTerminalSkin).not.toHaveBeenCalled(); + expect(mocks.execSandbox).not.toHaveBeenCalled(); + }); + it("keeps ordinary launch available when evidence observation, hashing, or storage fails (#8942)", async () => { mocks.inspectLaunchReadiness.mockResolvedValue({ kind: "fallback", diff --git a/src/lib/actions/sandbox/process-recovery.ts b/src/lib/actions/sandbox/process-recovery.ts index 48281999b6b..0e3832d488d 100644 --- a/src/lib/actions/sandbox/process-recovery.ts +++ b/src/lib/actions/sandbox/process-recovery.ts @@ -26,6 +26,7 @@ import { sleepSeconds, waitUntil } from "../../core/wait"; import { ROOT, shellQuote } from "../../runner"; import { isDirectSandboxFallbackUnavailableError, + isPinnedSandboxContainerIdentityChangedError, privilegedSandboxExecArgv, withPrivilegedSandboxExecutionLease, } from "../../sandbox/privileged-exec"; @@ -48,6 +49,7 @@ import { type GatewayRestartResult, gatewayIntegrityRepairLines, isGatewayIntegrityRepairLayer, + MANAGED_CONTROL_IDENTITY_CHANGED_MARKER, type ManagedGatewayControlCompletion, parseManagedGatewayControlCompletion, printGatewayRestartFailure, @@ -253,7 +255,9 @@ function executeGatewaySupervisorActionPinned( return { status: 1, stdout: "", - stderr: `PRIVILEGED_CONTROL_UNAVAILABLE: ${detail}`, + stderr: isPinnedSandboxContainerIdentityChangedError(error) + ? `${MANAGED_CONTROL_IDENTITY_CHANGED_MARKER}\n${detail}` + : `PRIVILEGED_CONTROL_UNAVAILABLE: ${detail}`, }; } } @@ -449,7 +453,9 @@ export function waitForManagedGatewaySupervisor( return false; } -type FinalRelaunchManagedSupervisorReadiness = { ready: true } | { detail: string; ready: false }; +type FinalRelaunchManagedSupervisorReadiness = + | { ready: true } + | { failure: ReturnType; ready: false }; function waitForFinalRelaunchManagedSupervisor( sandboxName: string, @@ -470,13 +476,15 @@ function waitForFinalRelaunchManagedSupervisor( if (ready) return { ready: true }; } catch { return { - detail: - "the replacement container identity changed during the final managed supervisor health check", + failure: { + layer: "privileged control unavailable", + detail: + "the pinned managed supervisor probe could not be completed during the final replacement container health check", + }, ready: false, }; } - const failure = classifyGatewayRestartFailure(probeResult); - return { detail: `${failure.layer}: ${failure.detail}`, ready: false }; + return { failure: classifyGatewayRestartFailure(probeResult), ready: false }; } function finalRelaunchContainerFailureDetail( @@ -564,8 +572,13 @@ function finalizeRelaunchedRecovery( requestManagedProbe, ); if (!managedSupervisor.ready) { + if (managedSupervisor.failure.layer === "container identity changed") { + return finalRelaunchRecoveryFailure( + "the replacement container identity changed during the final managed supervisor health check. NemoClaw did not start the primary dashboard/API host forward", + ); + } return finalRelaunchRecoveryFailure( - `the managed supervisor health check for the pinned replacement container did not pass after the final replacement container restart. NemoClaw did not start the primary dashboard/API host forward. Managed supervisor health check result: ${managedSupervisor.detail}`, + `the managed supervisor health check for the pinned replacement container did not pass after the final replacement container restart. NemoClaw did not start the primary dashboard/API host forward. Managed supervisor health check result: ${managedSupervisor.failure.layer}: ${managedSupervisor.failure.detail}`, ); } const finalReadinessFailureDetail = waitForRecoveryReadiness(); @@ -587,6 +600,19 @@ function finalizeRelaunchedRecovery( return null; } +function recoveryDetailAfterRelaunchRollback( + relaunch: ManagedSupervisorRelaunch, + recoveryFailureDetail: string, +): string { + try { + if (relaunch.finalize(false).rolledBack) return recoveryFailureDetail; + } catch { + // Report only the fixed rollback classification below. Finalizer errors can + // contain Docker paths, container IDs, or other untrusted runtime detail. + } + return `NemoClaw could not confirm rollback to the previous sandbox container. Inspect Docker state before retrying. Recovery failure before rollback: ${recoveryFailureDetail}`; +} + export function confirmRecoveredSandboxGatewayManaged( sandboxName: string, options: { @@ -1520,11 +1546,13 @@ function checkAndRecoverSandboxProcessesWithoutHostLock( ? (name: string, action: "restart" | "recover" | "probe", timeout = 210000) => requestPinnedGatewaySupervisorAction(name, action, timeout, relaunch.containerId) : requestGatewaySupervisorAction; - let relaunchedIdentityChanged = false; - let relaunchedManagedHealthFailureDetail: string | null = null; + const relaunchedManagedHealth = { + failure: null as ReturnType | null, + }; const confirmRelaunchedManagedHealth = relaunch ? (timeout = OPENSHELL_PROBE_TIMEOUT_MS) => { - let probeResult: SandboxCommandResult | null = null; + relaunchedManagedHealth.failure = null; + let probeResult: ManagedGatewaySupervisorActionResult | null = null; try { const confirmed = confirmRecoveredSandboxGatewayManaged(sandboxName, { requestGatewaySupervisorActionImpl: (name, action) => { @@ -1533,16 +1561,14 @@ function checkAndRecoverSandboxProcessesWithoutHostLock( }, }); if (confirmed === false) { - const failure = classifyGatewayRestartFailure(probeResult); - relaunchedManagedHealthFailureDetail = `${failure.layer}: ${failure.detail}`; - } else if (confirmed === true) { - relaunchedManagedHealthFailureDetail = null; + relaunchedManagedHealth.failure = classifyGatewayRestartFailure(probeResult); } return confirmed; } catch { - relaunchedIdentityChanged = true; - relaunchedManagedHealthFailureDetail = - "the pinned replacement sandbox identity changed during the managed probe"; + relaunchedManagedHealth.failure = { + layer: "privileged control unavailable", + detail: "the pinned managed supervisor probe could not be completed", + }; return false; } } @@ -1576,19 +1602,18 @@ function checkAndRecoverSandboxProcessesWithoutHostLock( throw error; } if (!gatewayReady) { - let rolledBack = true; - if (relaunch) { - try { - rolledBack = relaunch.finalize(false).rolledBack; - } catch { - rolledBack = false; - } - } + const gatewayWaitFailureDetail = relaunchedManagedHealth.failure + ? `the managed supervisor health check for the recreated sandbox did not pass while NemoClaw waited for its gateway. Managed supervisor health check result: ${relaunchedManagedHealth.failure.layer}: ${relaunchedManagedHealth.failure.detail}` + : "the recovered gateway did not become responsive before the recovery timeout"; + const recoveryFailureDetail = relaunch + ? recoveryDetailAfterRelaunchRollback(relaunch, gatewayWaitFailureDetail) + : gatewayWaitFailureDetail; + const rollbackUnconfirmed = recoveryFailureDetail !== gatewayWaitFailureDetail; if (!quiet) { console.error(" Gateway process started but is not responding."); printGatewayWedgeDiagnostics(sandboxName, executeSandboxExecCommand); console.error(" Check /tmp/gateway.log inside the sandbox for details."); - if (!rolledBack) { + if (rollbackUnconfirmed) { console.error( " Automatic rollback of the previous sandbox container failed; inspect Docker state before retrying.", ); @@ -1603,16 +1628,13 @@ function checkAndRecoverSandboxProcessesWithoutHostLock( managedRecoveryFailureLayer, managedRecoveryFailureDetail ?? undefined, ); - if (relaunchedManagedHealthFailureDetail) { - return { - checked: true, - wasRunning: false, - recovered: false, - forwardRecovered: false, - recoveryFailureDetail: `the managed supervisor health check for the recreated sandbox did not pass while NemoClaw waited for its gateway. Managed supervisor health check result: ${relaunchedManagedHealthFailureDetail}`, - }; - } - return { checked: true, wasRunning: false, recovered: false, forwardRecovered: false }; + return { + checked: true, + wasRunning: false, + recovered: false, + forwardRecovered: false, + recoveryFailureDetail, + }; } // Host-forward recovery requires an OpenShell-ready sandbox. Managed // recovery has already passed its authenticated control and health gates; @@ -1636,24 +1658,23 @@ function checkAndRecoverSandboxProcessesWithoutHostLock( readiness.failure, "openshellError" in readiness ? readiness.openshellError : undefined, readiness.failure === "managed-health-definitive-failure" - ? (relaunchedManagedHealthFailureDetail ?? undefined) + ? relaunchedManagedHealth.failure + ? `${relaunchedManagedHealth.failure.layer}: ${relaunchedManagedHealth.failure.detail}` + : undefined : undefined, ); }; const readinessFailureDetail = recoveryRequiresReadiness ? waitForRecoveryReadiness() : null; if (readinessFailureDetail) { - try { - relaunch?.finalize(false); - } catch { - // The readiness error remains authoritative. The detail below directs - // the operator to the failed replacement without trusting it. - } + const recoveryFailureDetail = relaunch + ? recoveryDetailAfterRelaunchRollback(relaunch, readinessFailureDetail) + : readinessFailureDetail; return { checked: true, wasRunning: false, recovered: false, forwardRecovered: false, - recoveryFailureDetail: readinessFailureDetail, + recoveryFailureDetail, }; } if (relaunch) { @@ -1677,15 +1698,16 @@ function checkAndRecoverSandboxProcessesWithoutHostLock( beforeStart: confirmRelaunchedManagedHealthForForward ?? undefined, isWsl: isWslOverride, }); - if (!forwardRecovered && relaunchedManagedHealthFailureDetail) { + if (!forwardRecovered && relaunchedManagedHealth.failure) { return { checked: true, wasRunning: false, recovered: false, forwardRecovered: false, - recoveryFailureDetail: relaunchedIdentityChanged - ? "the replacement container identity changed during the primary dashboard/API host forward check" - : `the managed supervisor health check for the pinned replacement container did not pass during the primary dashboard/API host forward check. Managed supervisor health check result: ${relaunchedManagedHealthFailureDetail}`, + recoveryFailureDetail: + relaunchedManagedHealth.failure.layer === "container identity changed" + ? "the replacement container identity changed during the primary dashboard/API host forward check" + : `the managed supervisor health check for the pinned replacement container did not pass during the primary dashboard/API host forward check. Managed supervisor health check result: ${relaunchedManagedHealth.failure.layer}: ${relaunchedManagedHealth.failure.detail}`, }; } const dashboardForwardRecovered = ensureHermesDashboardPortForwardIfEnabled(sandboxName); diff --git a/src/lib/actions/sandbox/start.test.ts b/src/lib/actions/sandbox/start.test.ts index 421ff672328..d6162ab9a04 100644 --- a/src/lib/actions/sandbox/start.test.ts +++ b/src/lib/actions/sandbox/start.test.ts @@ -409,6 +409,22 @@ describe("startSandbox", () => { expect(h.verifyGateway).not.toHaveBeenCalled(); }); + it("does not claim preservation when startup recovery reports a failed rollback (#9364)", async () => { + const h = harness(); + h.restoreStartupState.mockReturnValue({ + ...FAILED_RECOVERY, + recoveryFailureDetail: + "NemoClaw could not confirm rollback to the previous sandbox container. Inspect Docker state before retrying. Recovery failure before rollback: the sandbox did not become ready in OpenShell", + }); + + const failure = await startSandbox("my-sandbox", h.deps).catch((error) => String(error)); + + expect(failure).toContain("could not confirm rollback"); + expect(failure).toContain("Inspect the current sandbox state before retrying"); + expect(failure).not.toContain("The existing sandbox was preserved"); + expect(h.verifyGateway).not.toHaveBeenCalled(); + }); + it("reports the started container by name (#6026)", async () => { const h = harness(); diff --git a/src/lib/actions/sandbox/start.ts b/src/lib/actions/sandbox/start.ts index 2aedc1e77f5..69e9cb2f356 100644 --- a/src/lib/actions/sandbox/start.ts +++ b/src/lib/actions/sandbox/start.ts @@ -107,14 +107,14 @@ function startupRecoveryFailure(check: SandboxStartupRecoveryResult): string | n : `${layer}: the agent gateway did not recover`; } -function preservedSandboxRecoveryError(sandboxName: string, detail: unknown): Error { +function startupRecoveryError(sandboxName: string, detail: unknown): Error { const { sanitizeSandboxStartupRecoveryDetail } = require("./connect") as typeof import("./connect"); const rawDetail = detail instanceof Error && detail.message ? detail.message : String(detail); const safeDetail = sanitizeSandboxStartupRecoveryDetail(rawDetail); return new Error( `Sandbox '${sandboxName}' started, but startup recovery failed: ${safeDetail}. ` + - `The existing sandbox was preserved. Run \`${cliName()} ${sandboxName} recover\`, then retry \`${cliName()} ${sandboxName} start\`.`, + `Inspect the current sandbox state before retrying. Run \`${cliName()} ${sandboxName} recover\`, then retry \`${cliName()} ${sandboxName} start\`.`, ); } @@ -190,7 +190,7 @@ export async function startSandbox( try { recovery = restoreStartupState(name); } catch (error) { - throw preservedSandboxRecoveryError(name, error); + throw startupRecoveryError(name, error); } let failure = startupRecoveryFailure(recovery); if (failure && isMissingManagedSupervisorStartupFailure(recovery, failure)) { @@ -208,12 +208,12 @@ export async function startSandbox( try { recovery = restoreStartupState(name); } catch (error) { - throw preservedSandboxRecoveryError(name, error); + throw startupRecoveryError(name, error); } failure = startupRecoveryFailure(recovery); } } - if (failure) throw preservedSandboxRecoveryError(name, failure); + if (failure) throw startupRecoveryError(name, failure); log(" Checking gateway health and host forwards…"); await (deps.verifyGateway ?? verifyGateway)(name); readiness.inference = checkStartedSandboxInference(name, resolved.sandbox, deps, log); diff --git a/src/lib/sandbox/privileged-exec.test.ts b/src/lib/sandbox/privileged-exec.test.ts index e9364587582..d35f6f07ed8 100644 --- a/src/lib/sandbox/privileged-exec.test.ts +++ b/src/lib/sandbox/privileged-exec.test.ts @@ -432,6 +432,30 @@ describe("privileged sandbox exec routing", () => { ); }); + it("types a portable-target pinned container identity change", () => { + withPrivilegedExecMocks( + { + getSandbox: () => ({ name: "alpha", openshellDriver: "docker" }), + listSandboxes: () => ({ sandboxes: [{ name: "alpha" }], defaultSandbox: "alpha" }), + dockerCapture: vi.fn(), + resolvePortableDemoPrivilegedExecTarget: () => ({ + assertRuntimeAuthority: vi.fn(), + containerId: "current-container-id", + dockerHost: "unix:///run/user/1001/podman/podman.sock", + }), + }, + ({ isPinnedSandboxContainerIdentityChangedError, privilegedSandboxExecArgv }) => { + let refusal: unknown; + try { + privilegedSandboxExecArgv("alpha", ["id"], false, true, "previous-container-id"); + } catch (error) { + refusal = error; + } + expect(isPinnedSandboxContainerIdentityChangedError(refusal)).toBe(true); + }, + ); + }); + it("rejects a non-direct driver before consulting a stale portable receipt (#8584)", () => { const resolvePortableDemoPrivilegedExecTarget = vi.fn(); @@ -529,16 +553,22 @@ describe("privileged sandbox exec routing", () => { listSandboxes: () => ({ sandboxes: [{ name: "alpha" }], defaultSandbox: "alpha" }), dockerCapture: () => "current-container-id\topenshell-alpha\n", }, - ({ privilegedSandboxExecArgv }) => { - expect(() => + ({ isPinnedSandboxContainerIdentityChangedError, privilegedSandboxExecArgv }) => { + let refusal: unknown; + try { privilegedSandboxExecArgv( "alpha", ["/trusted/control"], false, true, "previous-container-id", - ), - ).toThrow(/container identity changed.*refusing privileged execution/i); + ); + } catch (error) { + refusal = error; + } + expect(refusal).toBeInstanceOf(Error); + expect(String(refusal)).toMatch(/container identity changed.*refusing privileged execution/i); + expect(isPinnedSandboxContainerIdentityChangedError(refusal)).toBe(true); }, ); }); diff --git a/src/lib/sandbox/privileged-exec.ts b/src/lib/sandbox/privileged-exec.ts index 675851f27ba..dc47ccf4ec8 100644 --- a/src/lib/sandbox/privileged-exec.ts +++ b/src/lib/sandbox/privileged-exec.ts @@ -55,6 +55,16 @@ class DirectSandboxFallbackUnavailableError extends Error { } } +class PinnedSandboxContainerIdentityChangedError extends Error { + constructor(sandboxName: string) { + super( + `OpenShell container identity changed for sandbox '${sandboxName}'; ` + + "refusing privileged execution against a different container.", + ); + this.name = "PinnedSandboxContainerIdentityChangedError"; + } +} + function normalizeDriver(driver: unknown): string | null { return typeof driver === "string" && driver.trim() ? driver.trim().toLowerCase() : null; } @@ -192,6 +202,12 @@ function isDirectSandboxFallbackUnavailableError( return error instanceof DirectSandboxFallbackUnavailableError; } +function isPinnedSandboxContainerIdentityChangedError( + error: unknown, +): error is PinnedSandboxContainerIdentityChangedError { + return error instanceof PinnedSandboxContainerIdentityChangedError; +} + function missingRegistryEntryError(sandboxName: string): Error { return new Error( `No NemoClaw registry entry found for '${sandboxName}'; ` + @@ -276,10 +292,7 @@ function privilegedSandboxExecArgv( : null; if (portableTarget) { if (expectedContainerId !== undefined && portableTarget.containerId !== expectedContainerId) { - throw new Error( - `OpenShell container identity changed for sandbox '${sandboxName}'; ` + - "refusing privileged execution against a different container.", - ); + throw new PinnedSandboxContainerIdentityChangedError(sandboxName); } const sanitizedEnvArgs = sanitizeEnvironment ? SANITIZED_PRIVILEGED_ENV.flatMap((value) => ["--env", value]) @@ -303,10 +316,7 @@ function privilegedSandboxExecArgv( const container = findDirectSandboxContainer(sandboxName); if (container) { if (expectedContainerId !== undefined && container !== expectedContainerId) { - throw new Error( - `OpenShell container identity changed for sandbox '${sandboxName}'; ` + - "refusing privileged execution against a different container.", - ); + throw new PinnedSandboxContainerIdentityChangedError(sandboxName); } const sanitizedEnvArgs = sanitizeEnvironment ? SANITIZED_PRIVILEGED_ENV.flatMap((value) => ["--env", value]) @@ -328,6 +338,7 @@ function privilegedSandboxExecArgv( export { containerNameMatchesSandbox, isDirectSandboxFallbackUnavailableError, + isPinnedSandboxContainerIdentityChangedError, privilegedSandboxExecArgv, resolveDirectSandboxContainer, selectDirectSandboxContainer, diff --git a/test/cli/connect-recovery-settle.test.ts b/test/cli/connect-recovery-settle.test.ts index 1d91421c2a4..332b896bffc 100644 --- a/test/cli/connect-recovery-settle.test.ts +++ b/test/cli/connect-recovery-settle.test.ts @@ -135,7 +135,10 @@ describe("CLI dispatch", () => { expect(r.code).toBe(1); expect(r.out).toContain( - `Probe failed: OpenClaw gateway is not running in '${sandboxName}' and automatic recovery failed.`, + `Probe failed: NemoClaw could not recover the OpenClaw gateway in '${sandboxName}'.`, + ); + expect(r.out).toContain( + "Recovery detail: the recovered gateway did not become responsive before the recovery timeout.", ); expect(r.out).toContain("#4710 wedge signature"); expect(r.out).toContain("config change requires gateway restart (plugins.installs)"); diff --git a/test/process-recovery-managed-controller.test.ts b/test/process-recovery-managed-controller.test.ts index 84fbc2e2e28..614e46e66f1 100644 --- a/test/process-recovery-managed-controller.test.ts +++ b/test/process-recovery-managed-controller.test.ts @@ -61,6 +61,10 @@ describe("managed gateway recovery controller", () => { recovered: false, forwardRecovered: false, }; + const timedOutGateway = { + ...unrecoveredGateway, + recoveryFailureDetail: "the recovered gateway did not become responsive before the recovery timeout", + }; const controllerNonce = "a".repeat(64); const restartingContainerId = "b".repeat(64); const successfulControl = { status: 0, stdout: "GATEWAY_PID=123\n", stderr: "" }; @@ -165,7 +169,7 @@ describe("managed gateway recovery controller", () => { label: "persistent post-settle controller contention", recoverResults: [successfulControl], managedProbeResults: [{ status: 1, stdout: "", stderr: "SUPERVISOR_BUSY" }], - expectedResult: unrecoveredGateway, + expectedResult: timedOutGateway, expectedActions: ["recover", "probe", "probe"], settleSeconds: "1", }, @@ -176,7 +180,7 @@ describe("managed gateway recovery controller", () => { { status: 1, stdout: "", stderr: "SUPERVISOR_BUSY" }, { status: 1, stdout: "", stderr: "GATEWAY_HEALTH_TIMEOUT" }, ], - expectedResult: unrecoveredGateway, + expectedResult: timedOutGateway, expectedActions: ["recover", "probe", "probe"], settleSeconds: "1", }, @@ -293,7 +297,7 @@ describe("managed gateway recovery controller", () => { label: "OpenShell managed controller wedge", recoverResults: [successfulControl], managedProbeResult: { status: 1, stdout: "", stderr: "GATEWAY_HEALTH_TIMEOUT" }, - expectedResult: unrecoveredGateway, + expectedResult: timedOutGateway, expectedActions: ["recover", "probe"], settleSeconds: "1", }, diff --git a/test/process-recovery-primitives.test.ts b/test/process-recovery-primitives.test.ts index ed4faeeebbd..1405e0f59f4 100644 --- a/test/process-recovery-primitives.test.ts +++ b/test/process-recovery-primitives.test.ts @@ -17,6 +17,7 @@ const { executeSandboxCommand, executeSandboxExecCommand, resolveSandboxDashboardPort, + waitForManagedGatewaySupervisor, } = requireSource( "../src/lib/actions/sandbox/process-recovery.ts", ) as typeof import("../src/lib/actions/sandbox/process-recovery.js"); @@ -25,6 +26,267 @@ afterEach(() => { vi.restoreAllMocks(); }); +describe("waitForManagedGatewaySupervisor", () => { + const restartingContainerId = "a".repeat(64); + const restartingContainer = { + status: 1, + stdout: "", + stderr: `Error response from daemon: Container ${restartingContainerId} is restarting, wait until the container is running`, + managedControlRestartingContainerId: restartingContainerId, + } as const; + + it("retries a controller probe after status 137 with no output (#8726)", () => { + const sleepImpl = vi.fn(); + const requestGatewaySupervisorActionImpl = vi + .fn() + .mockReturnValueOnce({ status: 137, stdout: "", stderr: "" }) + .mockReturnValueOnce({ + status: 0, + stdout: "GATEWAY_PID=4242", + stderr: "", + }); + + expect( + waitForManagedGatewaySupervisor("new-clone", { + intervalSeconds: 3, + maxAttempts: 2, + requestGatewaySupervisorActionImpl, + sleepImpl, + }), + ).toBe(true); + expect(sleepImpl).toHaveBeenCalledOnce(); + expect(sleepImpl).toHaveBeenCalledWith(3); + }); + + it("stops after two status 137 controller probes with no output (#8726)", () => { + const sleepImpl = vi.fn(); + const requestGatewaySupervisorActionImpl = vi.fn(() => ({ + status: 137, + stdout: "", + stderr: "", + })); + + expect( + waitForManagedGatewaySupervisor("new-clone", { + intervalSeconds: 3, + maxAttempts: 2, + requestGatewaySupervisorActionImpl, + sleepImpl, + }), + ).toBe(false); + expect(requestGatewaySupervisorActionImpl).toHaveBeenCalledTimes(2); + expect(sleepImpl).toHaveBeenCalledOnce(); + expect(sleepImpl).toHaveBeenCalledWith(3); + }); + + it("does not retry a status 137 controller probe with diagnostic output (#8726)", () => { + const sleepImpl = vi.fn(); + + expect( + waitForManagedGatewaySupervisor("new-clone", { + maxAttempts: 2, + requestGatewaySupervisorActionImpl: vi.fn(() => ({ + status: 137, + stdout: "", + stderr: "container stopped", + })), + sleepImpl, + }), + ).toBe(false); + expect(sleepImpl).not.toHaveBeenCalled(); + }); + + it("waits through an exact managed-container restart transition (#8726)", () => { + const sleepImpl = vi.fn(); + const requestGatewaySupervisorActionImpl = vi + .fn() + .mockReturnValueOnce(restartingContainer) + .mockReturnValueOnce({ + status: 0, + stdout: "GATEWAY_PID=4242", + stderr: "", + }); + + expect( + waitForManagedGatewaySupervisor("new-clone", { + intervalSeconds: 3, + maxAttempts: 2, + requestGatewaySupervisorActionImpl, + sleepImpl, + }), + ).toBe(true); + expect(requestGatewaySupervisorActionImpl).toHaveBeenCalledTimes(2); + expect(sleepImpl).toHaveBeenCalledOnce(); + expect(sleepImpl).toHaveBeenCalledWith(3); + }); + + it("stops after two managed-container restart transitions (#8726)", () => { + const sleepImpl = vi.fn(); + const requestGatewaySupervisorActionImpl = vi.fn(() => restartingContainer); + + expect( + waitForManagedGatewaySupervisor("new-clone", { + intervalSeconds: 3, + maxAttempts: 2, + requestGatewaySupervisorActionImpl, + sleepImpl, + }), + ).toBe(false); + expect(requestGatewaySupervisorActionImpl).toHaveBeenCalledTimes(2); + expect(sleepImpl).toHaveBeenCalledOnce(); + expect(sleepImpl).toHaveBeenCalledWith(3); + }); + + it("does not wait through an unbound Docker restart diagnostic (#8726)", () => { + const sleepImpl = vi.fn(); + + expect( + waitForManagedGatewaySupervisor("new-clone", { + maxAttempts: 2, + requestGatewaySupervisorActionImpl: vi.fn(() => ({ + status: 1, + stdout: "", + stderr: restartingContainer.stderr, + })), + sleepImpl, + }), + ).toBe(false); + expect(sleepImpl).not.toHaveBeenCalled(); + }); + + it("waits through an exact missing-supervisor startup race", () => { + const sleepImpl = vi.fn(); + const requestGatewaySupervisorActionImpl = vi + .fn() + .mockReturnValueOnce({ + status: 1, + stdout: "", + stderr: "SUPERVISOR_NOT_RUNNING", + }) + .mockReturnValueOnce({ + status: 0, + stdout: "GATEWAY_PID=4242", + stderr: "", + }); + + expect( + waitForManagedGatewaySupervisor("new-clone", { + intervalSeconds: 3, + maxAttempts: 2, + requestGatewaySupervisorActionImpl, + sleepImpl, + }), + ).toBe(true); + expect(sleepImpl).toHaveBeenCalledOnce(); + expect(sleepImpl).toHaveBeenCalledWith(3); + }); + + it("waits through exact pending direct control while a clone container appears", () => { + const sleepImpl = vi.fn(); + const requestGatewaySupervisorActionImpl = vi + .fn() + .mockReturnValueOnce({ + status: 1, + stdout: "", + stderr: "PRIVILEGED_CONTROL_UNAVAILABLE", + }) + .mockReturnValueOnce({ + status: 0, + stdout: "GATEWAY_PID=4242", + stderr: "", + }); + + expect( + waitForManagedGatewaySupervisor("new-clone", { + intervalSeconds: 3, + maxAttempts: 2, + requestGatewaySupervisorActionImpl, + sleepImpl, + }), + ).toBe(true); + expect(sleepImpl).toHaveBeenCalledOnce(); + expect(sleepImpl).toHaveBeenCalledWith(3); + }); + + it("waits while a new clone gateway is not healthy yet (#7818)", () => { + const sleepImpl = vi.fn(); + const requestGatewaySupervisorActionImpl = vi + .fn() + .mockReturnValueOnce({ + status: 1, + stdout: "", + stderr: "GATEWAY_HEALTH_TIMEOUT", + }) + .mockReturnValueOnce({ + status: 0, + stdout: "GATEWAY_PID=4242", + stderr: "", + }); + + expect( + waitForManagedGatewaySupervisor("new-clone", { + intervalSeconds: 3, + maxAttempts: 2, + requestGatewaySupervisorActionImpl, + sleepImpl, + }), + ).toBe(true); + expect(sleepImpl).toHaveBeenCalledOnce(); + expect(sleepImpl).toHaveBeenCalledWith(3); + }); + + it("does not wait when a health marker includes unclassified output (#7818)", () => { + const sleepImpl = vi.fn(); + + expect( + waitForManagedGatewaySupervisor("new-clone", { + maxAttempts: 2, + requestGatewaySupervisorActionImpl: vi.fn(() => ({ + status: 1, + stdout: "", + stderr: "GATEWAY_HEALTH_TIMEOUT\nunexpected detail", + })), + sleepImpl, + }), + ).toBe(false); + expect(sleepImpl).not.toHaveBeenCalled(); + }); + + it("does not wait through an unclassified supervisor refusal", () => { + const sleepImpl = vi.fn(); + + expect( + waitForManagedGatewaySupervisor("new-clone", { + maxAttempts: 2, + requestGatewaySupervisorActionImpl: vi.fn(() => ({ + status: 1, + stdout: "", + stderr: "prefix SUPERVISOR_NOT_RUNNING suffix", + })), + sleepImpl, + }), + ).toBe(false); + expect(sleepImpl).not.toHaveBeenCalled(); + }); + + it("does not wait through a detailed privileged-control refusal", () => { + const sleepImpl = vi.fn(); + + expect( + waitForManagedGatewaySupervisor("new-clone", { + maxAttempts: 2, + requestGatewaySupervisorActionImpl: vi.fn(() => ({ + status: 1, + stdout: "", + stderr: "PRIVILEGED_CONTROL_UNAVAILABLE: container identity changed", + })), + sleepImpl, + }), + ).toBe(false); + expect(sleepImpl).not.toHaveBeenCalled(); + }); +}); + describe("executeGatewaySupervisorAction", () => { const controlPath = "/usr/local/bin/nemoclaw-gateway-control"; const targetContainerId = "a".repeat(64); @@ -46,14 +308,35 @@ describe("executeGatewaySupervisorAction", () => { it("keeps other privileged-control refusals terminal and classified", () => { const privilegedExec = requireSource("../src/lib/sandbox/privileged-exec.ts"); vi.spyOn(privilegedExec, "privilegedSandboxExecArgv").mockImplementation(() => { - throw new Error("container identity changed"); + throw new Error( + "OpenShell container identity changed for sandbox 'new-clone'; refusing privileged execution against a different container.", + ); + }); + vi.spyOn(privilegedExec, "isDirectSandboxFallbackUnavailableError").mockReturnValue(false); + + expect(executeGatewaySupervisorAction("new-clone", "probe", 100)).toEqual({ + status: 1, + stdout: "", + stderr: + "PRIVILEGED_CONTROL_UNAVAILABLE: OpenShell container identity changed for sandbox 'new-clone'; refusing privileged execution against a different container.", + }); + }); + + it("emits the managed-control identity marker for a pinned container refusal (#9364)", () => { + const privilegedExec = requireSource("../src/lib/sandbox/privileged-exec.ts"); + vi.spyOn(privilegedExec, "privilegedSandboxExecArgv").mockImplementation(() => { + throw new Error( + "OpenShell container identity changed for sandbox 'new-clone'; refusing privileged execution against a different container.", + ); }); vi.spyOn(privilegedExec, "isDirectSandboxFallbackUnavailableError").mockReturnValue(false); + vi.spyOn(privilegedExec, "isPinnedSandboxContainerIdentityChangedError").mockReturnValue(true); expect(executeGatewaySupervisorAction("new-clone", "probe", 100)).toEqual({ status: 1, stdout: "", - stderr: "PRIVILEGED_CONTROL_UNAVAILABLE: container identity changed", + stderr: + "MANAGED_CONTROL_IDENTITY_CHANGED\nOpenShell container identity changed for sandbox 'new-clone'; refusing privileged execution against a different container.", }); }); diff --git a/test/process-recovery-supervisor-relaunch.test.ts b/test/process-recovery-supervisor-relaunch.test.ts index b5895bd644b..19d13b19741 100644 --- a/test/process-recovery-supervisor-relaunch.test.ts +++ b/test/process-recovery-supervisor-relaunch.test.ts @@ -3,10 +3,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import * as forwardHealth from "../src/lib/actions/sandbox/forward-health.ts"; -import { - checkAndRecoverSandboxProcesses, - waitForManagedGatewaySupervisor, -} from "../src/lib/actions/sandbox/process-recovery.ts"; +import { checkAndRecoverSandboxProcesses } from "../src/lib/actions/sandbox/process-recovery.ts"; import { relaunchManagedSupervisorSession } from "../src/lib/actions/sandbox/supervisor-relaunch.ts"; import * as openshellRuntime from "../src/lib/adapters/openshell/runtime.ts"; import * as agentRuntime from "../src/lib/agent/runtime.ts"; @@ -26,6 +23,13 @@ const MISSING_MANAGED_SUPERVISOR = { stdout: "", stderr: "SUPERVISOR_NOT_RUNNING", } as const; +function pinnedIdentityRefusal(sandboxName: string) { + return { + status: 1, + stdout: "", + stderr: `MANAGED_CONTROL_IDENTITY_CHANGED\nOpenShell container identity changed for sandbox '${sandboxName}'; refusing privileged execution against a different container.`, + } as const; +} afterEach(() => { vi.restoreAllMocks(); @@ -161,267 +165,6 @@ function scriptedPinnedGatewayRecovery( ); } -describe("waitForManagedGatewaySupervisor", () => { - const restartingContainerId = "a".repeat(64); - const restartingContainer = { - status: 1, - stdout: "", - stderr: `Error response from daemon: Container ${restartingContainerId} is restarting, wait until the container is running`, - managedControlRestartingContainerId: restartingContainerId, - } as const; - - it("retries a controller probe after status 137 with no output (#8726)", () => { - const sleepImpl = vi.fn(); - const requestGatewaySupervisorActionImpl = vi - .fn() - .mockReturnValueOnce({ status: 137, stdout: "", stderr: "" }) - .mockReturnValueOnce({ - status: 0, - stdout: "GATEWAY_PID=4242", - stderr: "", - }); - - expect( - waitForManagedGatewaySupervisor("new-clone", { - intervalSeconds: 3, - maxAttempts: 2, - requestGatewaySupervisorActionImpl, - sleepImpl, - }), - ).toBe(true); - expect(sleepImpl).toHaveBeenCalledOnce(); - expect(sleepImpl).toHaveBeenCalledWith(3); - }); - - it("stops after two status 137 controller probes with no output (#8726)", () => { - const sleepImpl = vi.fn(); - const requestGatewaySupervisorActionImpl = vi.fn(() => ({ - status: 137, - stdout: "", - stderr: "", - })); - - expect( - waitForManagedGatewaySupervisor("new-clone", { - intervalSeconds: 3, - maxAttempts: 2, - requestGatewaySupervisorActionImpl, - sleepImpl, - }), - ).toBe(false); - expect(requestGatewaySupervisorActionImpl).toHaveBeenCalledTimes(2); - expect(sleepImpl).toHaveBeenCalledOnce(); - expect(sleepImpl).toHaveBeenCalledWith(3); - }); - - it("does not retry a status 137 controller probe with diagnostic output (#8726)", () => { - const sleepImpl = vi.fn(); - - expect( - waitForManagedGatewaySupervisor("new-clone", { - maxAttempts: 2, - requestGatewaySupervisorActionImpl: vi.fn(() => ({ - status: 137, - stdout: "", - stderr: "container stopped", - })), - sleepImpl, - }), - ).toBe(false); - expect(sleepImpl).not.toHaveBeenCalled(); - }); - - it("waits through an exact managed-container restart transition (#8726)", () => { - const sleepImpl = vi.fn(); - const requestGatewaySupervisorActionImpl = vi - .fn() - .mockReturnValueOnce(restartingContainer) - .mockReturnValueOnce({ - status: 0, - stdout: "GATEWAY_PID=4242", - stderr: "", - }); - - expect( - waitForManagedGatewaySupervisor("new-clone", { - intervalSeconds: 3, - maxAttempts: 2, - requestGatewaySupervisorActionImpl, - sleepImpl, - }), - ).toBe(true); - expect(requestGatewaySupervisorActionImpl).toHaveBeenCalledTimes(2); - expect(sleepImpl).toHaveBeenCalledOnce(); - expect(sleepImpl).toHaveBeenCalledWith(3); - }); - - it("stops after two managed-container restart transitions (#8726)", () => { - const sleepImpl = vi.fn(); - const requestGatewaySupervisorActionImpl = vi.fn(() => restartingContainer); - - expect( - waitForManagedGatewaySupervisor("new-clone", { - intervalSeconds: 3, - maxAttempts: 2, - requestGatewaySupervisorActionImpl, - sleepImpl, - }), - ).toBe(false); - expect(requestGatewaySupervisorActionImpl).toHaveBeenCalledTimes(2); - expect(sleepImpl).toHaveBeenCalledOnce(); - expect(sleepImpl).toHaveBeenCalledWith(3); - }); - - it("does not wait through an unbound Docker restart diagnostic (#8726)", () => { - const sleepImpl = vi.fn(); - - expect( - waitForManagedGatewaySupervisor("new-clone", { - maxAttempts: 2, - requestGatewaySupervisorActionImpl: vi.fn(() => ({ - status: 1, - stdout: "", - stderr: restartingContainer.stderr, - })), - sleepImpl, - }), - ).toBe(false); - expect(sleepImpl).not.toHaveBeenCalled(); - }); - - it("waits through an exact missing-supervisor startup race", () => { - const sleepImpl = vi.fn(); - const requestGatewaySupervisorActionImpl = vi - .fn() - .mockReturnValueOnce({ - status: 1, - stdout: "", - stderr: "SUPERVISOR_NOT_RUNNING", - }) - .mockReturnValueOnce({ - status: 0, - stdout: "GATEWAY_PID=4242", - stderr: "", - }); - - expect( - waitForManagedGatewaySupervisor("new-clone", { - intervalSeconds: 3, - maxAttempts: 2, - requestGatewaySupervisorActionImpl, - sleepImpl, - }), - ).toBe(true); - expect(sleepImpl).toHaveBeenCalledOnce(); - expect(sleepImpl).toHaveBeenCalledWith(3); - }); - - it("waits through exact pending direct control while a clone container appears", () => { - const sleepImpl = vi.fn(); - const requestGatewaySupervisorActionImpl = vi - .fn() - .mockReturnValueOnce({ - status: 1, - stdout: "", - stderr: "PRIVILEGED_CONTROL_UNAVAILABLE", - }) - .mockReturnValueOnce({ - status: 0, - stdout: "GATEWAY_PID=4242", - stderr: "", - }); - - expect( - waitForManagedGatewaySupervisor("new-clone", { - intervalSeconds: 3, - maxAttempts: 2, - requestGatewaySupervisorActionImpl, - sleepImpl, - }), - ).toBe(true); - expect(sleepImpl).toHaveBeenCalledOnce(); - expect(sleepImpl).toHaveBeenCalledWith(3); - }); - - it("waits while a new clone gateway is not healthy yet (#7818)", () => { - const sleepImpl = vi.fn(); - const requestGatewaySupervisorActionImpl = vi - .fn() - .mockReturnValueOnce({ - status: 1, - stdout: "", - stderr: "GATEWAY_HEALTH_TIMEOUT", - }) - .mockReturnValueOnce({ - status: 0, - stdout: "GATEWAY_PID=4242", - stderr: "", - }); - - expect( - waitForManagedGatewaySupervisor("new-clone", { - intervalSeconds: 3, - maxAttempts: 2, - requestGatewaySupervisorActionImpl, - sleepImpl, - }), - ).toBe(true); - expect(sleepImpl).toHaveBeenCalledOnce(); - expect(sleepImpl).toHaveBeenCalledWith(3); - }); - - it("does not wait when a health marker includes unclassified output (#7818)", () => { - const sleepImpl = vi.fn(); - - expect( - waitForManagedGatewaySupervisor("new-clone", { - maxAttempts: 2, - requestGatewaySupervisorActionImpl: vi.fn(() => ({ - status: 1, - stdout: "", - stderr: "GATEWAY_HEALTH_TIMEOUT\nunexpected detail", - })), - sleepImpl, - }), - ).toBe(false); - expect(sleepImpl).not.toHaveBeenCalled(); - }); - - it("does not wait through an unclassified supervisor refusal", () => { - const sleepImpl = vi.fn(); - - expect( - waitForManagedGatewaySupervisor("new-clone", { - maxAttempts: 2, - requestGatewaySupervisorActionImpl: vi.fn(() => ({ - status: 1, - stdout: "", - stderr: "prefix SUPERVISOR_NOT_RUNNING suffix", - })), - sleepImpl, - }), - ).toBe(false); - expect(sleepImpl).not.toHaveBeenCalled(); - }); - - it("does not wait through a detailed privileged-control refusal", () => { - const sleepImpl = vi.fn(); - - expect( - waitForManagedGatewaySupervisor("new-clone", { - maxAttempts: 2, - requestGatewaySupervisorActionImpl: vi.fn(() => ({ - status: 1, - stdout: "", - stderr: "PRIVILEGED_CONTROL_UNAVAILABLE: container identity changed", - })), - sleepImpl, - }), - ).toBe(false); - expect(sleepImpl).not.toHaveBeenCalled(); - }); -}); - describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { it("checks managed recovery and OpenShell readiness before starting host forwards (#8662)", () => { mockOpenClawSandbox("stopped-box"); @@ -565,10 +308,10 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { ); }); - it("rolls back when recreation starts but managed control never accepts it", () => { + it("reports an unconfirmed rollback when recreated gateway health never resolves", () => { mockOpenClawSandbox("rejected-box"); setImmediateRecoveryPolling(); - const finalize = vi.fn(() => ({ backupRemoved: false, rolledBack: true })); + const finalize = vi.fn(() => ({ backupRemoved: false, rolledBack: false })); const relaunchManagedSupervisorSessionImpl = vi.fn(() => ({ containerId: "replacement-container-id", finalize, @@ -586,7 +329,18 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { relaunchManagedSupervisorSessionImpl, }); - expect(result).toMatchObject({ checked: true, wasRunning: false, recovered: false }); + expect(result).toMatchObject({ + checked: true, + wasRunning: false, + recovered: false, + forwardRecovered: false, + recoveryFailureDetail: expect.stringContaining( + "NemoClaw could not confirm rollback to the previous sandbox container", + ), + }); + expect("recoveryFailureDetail" in result ? result.recoveryFailureDetail : "").toContain( + "the recovered gateway did not become responsive before the recovery timeout", + ); expect(requestPinnedGatewaySupervisorAction).toHaveBeenCalledWith( "rejected-box", "probe", @@ -878,8 +632,22 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { }), expectedDetail: "replacement container identity changed", expectedReadinessCalls: 1, + finalPinnedAction: () => pinnedIdentityRefusal("failed-handoff-box"), + finalReadinessReady: true, + }, + { + condition: "the final pinned managed probe throws", + finalizeOutcome: () => ({ + backupRemoved: true, + replacementRestarted: true, + replacementStoppedForCommit: true, + rolledBack: false, + stateRestored: true, + }), + expectedDetail: "pinned managed supervisor probe could not be completed", + expectedReadinessCalls: 1, finalPinnedAction: () => { - throw new Error("replacement identity changed"); + throw new Error("opaque-pinned-probe-sentinel"); }, finalReadinessReady: true, }, @@ -942,6 +710,9 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { forwardRecovered: false, recoveryFailureDetail: expect.stringContaining(expectedDetail), }); + expect("recoveryFailureDetail" in result ? result.recoveryFailureDetail : "").not.toContain( + "opaque-", + ); expect(result).not.toHaveProperty("forwardRecoveryFailed"); expect(finalize).toHaveBeenCalledOnce(); expect(finalize).toHaveBeenCalledWith(true); @@ -1208,7 +979,11 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { it("uses the shared recreate-readiness budget after a longer gateway health wait", () => { mockOpenClawSandbox("unready-box", 600); setImmediateRecoveryPolling(); - const finalize = vi.fn(() => ({ backupRemoved: true, rolledBack: false })); + const finalize = vi.fn(() => ({ + backupRemoved: false, + rolledBack: true, + stateRestored: false, + })); const relaunchManagedSupervisorSessionImpl = vi.fn(() => ({ containerId: "replacement-container-id", finalize, @@ -1259,10 +1034,79 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { expect(runOpenshell).not.toHaveBeenCalled(); }); + it.each([ + { + condition: "the rollback reports failure", + finalizeOutcome: () => ({ + backupRemoved: false, + rolledBack: false, + stateRestored: false, + }), + }, + { + condition: "the rollback throws", + finalizeOutcome: () => { + throw new Error("opaque-finalizer-sentinel"); + }, + }, + ])("reports the readiness failure and unconfirmed rollback when $condition (#9364)", ({ + finalizeOutcome, + }) => { + mockOpenClawSandbox("rollback-box"); + setImmediateRecoveryPolling(); + const finalize = vi.fn((_supervisorReady: boolean) => finalizeOutcome()); + const relaunchManagedSupervisorSessionImpl = vi.fn(() => ({ + containerId: "replacement-container-id", + finalize, + })); + const requestGatewaySupervisorAction = vi.fn(() => MISSING_MANAGED_SUPERVISOR); + const requestPinnedGatewaySupervisorAction = vi.fn(() => ACCEPTED_MANAGED_PROBE); + const waitForRecreatedSandboxOpenShellReadyImpl = vi.fn(() => false); + const captureOpenshell = vi + .spyOn(openshellRuntime, "captureOpenshell") + .mockReturnValue({ status: 0, output: "" }); + const runOpenshell = vi + .spyOn(openshellRuntime, "runOpenshell") + .mockReturnValue({ status: 0 } as never); + + const result = checkAndRecoverSandboxProcesses("rollback-box", { + quiet: true, + isSandboxGatewayRunningImpl: () => false, + requestGatewaySupervisorAction, + requestPinnedGatewaySupervisorAction, + relaunchManagedSupervisorSessionImpl, + waitForRecreatedSandboxOpenShellReadyImpl, + }); + + expect(result).toMatchObject({ + checked: true, + wasRunning: false, + recovered: false, + forwardRecovered: false, + recoveryFailureDetail: expect.stringContaining( + "NemoClaw could not confirm rollback to the previous sandbox container", + ), + }); + expect("recoveryFailureDetail" in result ? result.recoveryFailureDetail : "").toContain( + "did not become ready in OpenShell", + ); + expect("recoveryFailureDetail" in result ? result.recoveryFailureDetail : "").not.toContain( + "opaque-finalizer-sentinel", + ); + expect(finalize).toHaveBeenCalledOnce(); + expect(finalize).toHaveBeenCalledWith(false); + expect(captureOpenshell).not.toHaveBeenCalled(); + expect(runOpenshell).not.toHaveBeenCalled(); + }); + it("reports the last structured OpenShell error when readiness times out", () => { mockOpenClawSandbox("relay-dropped-box"); setImmediateRecoveryPolling(); - const finalize = vi.fn(() => ({ backupRemoved: true, rolledBack: false })); + const finalize = vi.fn(() => ({ + backupRemoved: false, + rolledBack: true, + stateRestored: false, + })); const relaunchManagedSupervisorSessionImpl = vi.fn(() => ({ containerId: "replacement-container-id", finalize, @@ -1320,7 +1164,11 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { it("reports a definitive managed health failure separately from OpenShell readiness", () => { mockOpenClawSandbox("managed-failed-box"); setImmediateRecoveryPolling(); - const finalize = vi.fn(() => ({ backupRemoved: true, rolledBack: false })); + const finalize = vi.fn(() => ({ + backupRemoved: false, + rolledBack: true, + stateRestored: false, + })); const relaunchManagedSupervisorSessionImpl = vi.fn(() => ({ containerId: "replacement-container-id", finalize, @@ -1373,9 +1221,7 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { { condition: "the replacement identity changes after readiness", expectedDetail: "replacement container identity changed", - finalProbe: () => { - throw new Error("replacement identity changed"); - }, + finalProbe: () => pinnedIdentityRefusal("drifted-box"), }, { condition: "the final managed supervisor health check is rejected", @@ -1386,6 +1232,13 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { stderr: "GATEWAY_UNSAFE_CONFIG_PATH", }), }, + { + condition: "the final pinned managed probe throws", + expectedDetail: "pinned managed supervisor probe could not be completed", + finalProbe: () => { + throw new Error("opaque-forward-probe-sentinel"); + }, + }, ])("rejects a healthy forward when $condition (#9364)", ({ expectedDetail, finalProbe }) => { mockOpenClawSandbox("drifted-box"); vi.mocked(agentRuntime.getSessionAgent).mockReturnValue({ @@ -1445,6 +1298,9 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { forwardRecovered: false, recoveryFailureDetail: expect.stringContaining(expectedDetail), }); + expect("recoveryFailureDetail" in result ? result.recoveryFailureDetail : "").not.toContain( + "opaque-", + ); expect(result).not.toHaveProperty("forwardRecoveryFailed"); expect(requestPinnedGatewaySupervisorAction).toHaveBeenCalledTimes(3); expect(requestPinnedGatewaySupervisorAction).toHaveBeenLastCalledWith( @@ -1460,4 +1316,64 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { stdio: "ignore", }); }); + + it("reports GATEWAY_UNSAFE_CONFIG_PATH after a transient identity refusal clears (#9364)", () => { + mockOpenClawSandbox("current-probe-box"); + setImmediateRecoveryPolling(); + vi.stubEnv("NEMOCLAW_GATEWAY_RECOVERY_WAIT_SECONDS", "1"); + const finalize = vi.fn(() => ({ backupRemoved: true, rolledBack: false })); + const relaunchManagedSupervisorSessionImpl = vi.fn(() => ({ + containerId: "replacement-container-id", + finalize, + })); + const requestGatewaySupervisorAction = vi.fn(() => MISSING_MANAGED_SUPERVISOR); + const unsafeConfigProbe = { + status: 1, + stdout: "", + stderr: "GATEWAY_UNSAFE_CONFIG_PATH", + } as const; + const requestPinnedGatewaySupervisorAction = vi + .fn() + .mockReturnValueOnce(pinnedIdentityRefusal("current-probe-box")) + .mockReturnValueOnce(ACCEPTED_MANAGED_PROBE) + .mockReturnValueOnce(ACCEPTED_MANAGED_PROBE) + .mockReturnValue(unsafeConfigProbe); + const waitForRecreatedSandboxOpenShellReadyImpl = vi.fn( + (_name, options) => options.beforeProbe?.(1000) === true, + ); + vi.spyOn(forwardHealth, "isLocalForwardReachable").mockReturnValue(true); + vi.spyOn(openshellRuntime, "captureOpenshell").mockReturnValue({ + status: 0, + output: + "SANDBOX BIND PORT PID STATUS\ncurrent-probe-box 127.0.0.1 18789 12345 running", + }); + const runOpenshell = vi + .spyOn(openshellRuntime, "runOpenshell") + .mockReturnValue({ status: 0 } as never); + + const result = checkAndRecoverSandboxProcesses("current-probe-box", { + quiet: true, + isSandboxGatewayRunningImpl: () => false, + requestGatewaySupervisorAction, + requestPinnedGatewaySupervisorAction, + relaunchManagedSupervisorSessionImpl, + waitForRecreatedSandboxOpenShellReadyImpl, + }); + + expect(result).toMatchObject({ + checked: true, + wasRunning: false, + recovered: false, + forwardRecovered: false, + recoveryFailureDetail: expect.stringContaining( + "unsafe config path: GATEWAY_UNSAFE_CONFIG_PATH", + ), + }); + expect("recoveryFailureDetail" in result ? result.recoveryFailureDetail : "").not.toContain( + "identity changed", + ); + expect(requestPinnedGatewaySupervisorAction).toHaveBeenCalledTimes(4); + expect(finalize).toHaveBeenCalledWith(true); + expect(runOpenshell).toHaveBeenCalledOnce(); + }); });