diff --git a/src/lib/actions/sandbox/connect-flow.test.ts b/src/lib/actions/sandbox/connect-flow.test.ts index a23f94a5f53..1690cae865c 100644 --- a/src/lib/actions/sandbox/connect-flow.test.ts +++ b/src/lib/actions/sandbox/connect-flow.test.ts @@ -907,4 +907,83 @@ describe("connectSandbox flow", () => { expect(logOutput).not.toContain("Probe complete"); expect(exitSpy).toHaveBeenCalledWith(1); }); + + it("does not suggest a manual forward when gateway recovery fails before forward start", async () => { + const harness = createConnectHarness({ + processCheck: { + checked: true, + wasRunning: false, + recovered: false, + forwardRecovered: false, + recoveryFailureDetail: + "the replacement container identity changed during the final managed supervisor health check", + }, + }); + + await expect(harness.connectSandbox("alpha", { probeOnly: true })).rejects.toThrow( + "process.exit(1)", + ); + + const errorOutput = harness.errorSpy.mock.calls.map((call) => String(call[0] ?? "")).join("\n"); + expect(errorOutput).toContain("NemoClaw could not recover the OpenClaw gateway in 'alpha'"); + expect(errorOutput).toContain( + "the replacement container identity changed during the final managed supervisor health check", + ); + expect(errorOutput).not.toContain("gateway is running"); + expect(errorOutput).not.toContain("openshell forward start"); + expect(harness.runAutoPairSpy).not.toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + }); + + it("redacts untrusted gateway recovery details before reporting them", async () => { + const opaqueToken = "opaque-gateway-recovery-token"; + const harness = createConnectHarness({ + processCheck: { + checked: true, + wasRunning: false, + recovered: false, + forwardRecovered: false, + recoveryFailureDetail: `OpenShell failed\nAuthorization: Bearer ${opaqueToken}\u001b[31m`, + }, + }); + + await expect(harness.connectSandbox("alpha", { probeOnly: true })).rejects.toThrow( + "process.exit(1)", + ); + + const errorOutput = harness.errorSpy.mock.calls.map((call) => String(call[0] ?? "")).join("\n"); + expect(errorOutput).toContain("Recovery detail:"); + expect(errorOutput).not.toContain(opaqueToken); + expect(errorOutput).not.toContain("\u001b"); + expect(errorOutput).toMatch(/Recovery detail: .*\.$/mu); + expect(exitSpy).toHaveBeenCalledWith(1); + }); + + it("keeps a direct recovery failure detail separate from an earlier callback layer", () => { + const harness = createConnectHarness(); + harness.checkAndRecoverSpy.mockImplementation( + ( + _sandboxName: string, + options?: { + onRecoveryFailureLayer?: (layer: string, detail?: string) => void; + }, + ) => { + options?.onRecoveryFailureLayer?.("supervisor not running", "SUPERVISOR_NOT_RUNNING"); + return { + checked: true, + wasRunning: false, + recovered: false, + forwardRecovered: false, + recoveryFailureDetail: + "the managed supervisor health check for the recreated sandbox did not pass", + }; + }, + ); + + expect(harness.restoreSandboxStartupState("alpha")).toMatchObject({ + recoveryFailureDetail: + "the managed supervisor health check for the recreated sandbox did not pass", + recoveryFailureLayer: null, + }); + }); }); diff --git a/src/lib/actions/sandbox/connect.ts b/src/lib/actions/sandbox/connect.ts index 415c9882347..2b35e463fc7 100644 --- a/src/lib/actions/sandbox/connect.ts +++ b/src/lib/actions/sandbox/connect.ts @@ -260,6 +260,21 @@ function exitOnForwardRecoveryFailure( process.exit(1); } +function exitOnGatewayRecoveryFailure( + sandboxName: string, + agentName: string, + detail: string, +): 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}'.`, + ); + console.error(` Recovery detail: ${safeDetail}${terminalPunctuation}`); + process.exit(1); +} + async function settlePortablePairingOrExit(sandboxName: string): Promise { const result = await settlePortableOpenClawPairing(sandboxName); if (result.kind === "incomplete") { @@ -318,6 +333,13 @@ async function runSandboxConnectProbe(sandboxName: string): Promise { detail, ); } + if ("recoveryFailureDetail" in processCheck && processCheck.recoveryFailureDetail) { + exitOnGatewayRecoveryFailure( + sandboxName, + agentName, + String(processCheck.recoveryFailureDetail), + ); + } if (processCheck.wasRunning) { await ensureSandboxInferenceRouteOrExit(sandboxName, agent); // Defense-in-depth scope-upgrade approval on the probe-only / `recover` @@ -958,16 +980,22 @@ function maybeEnsureHermesToolGatewayBroker(sb: SandboxEntry | null): void { } export function restoreSandboxStartupState(sandboxName: string): SandboxStartupRecoveryResult { - let recoveryFailureDetail: string | null = null; - let recoveryFailureLayer: GatewayRestartFailureLayer | null = null; + let reportedRecoveryFailureDetail: string | null = null; + let reportedRecoveryFailureLayer: GatewayRestartFailureLayer | null = null; const processCheck = checkAndRecoverSandboxProcesses(sandboxName, { quiet: true, onRecoveryFailureLayer: (layer, detail) => { - recoveryFailureLayer = layer; - recoveryFailureDetail = detail ?? null; + reportedRecoveryFailureLayer = layer; + reportedRecoveryFailureDetail = detail ?? null; }, }); - return { ...processCheck, recoveryFailureDetail, recoveryFailureLayer }; + const directRecoveryFailureDetail = + "recoveryFailureDetail" in processCheck ? processCheck.recoveryFailureDetail : null; + const recoveryFailureDetail = directRecoveryFailureDetail ?? reportedRecoveryFailureDetail; + const recoveryFailureLayer = directRecoveryFailureDetail + ? null + : reportedRecoveryFailureLayer; + return Object.assign(processCheck, { recoveryFailureDetail, recoveryFailureLayer }); } function restoreInteractiveTerminal(): void { diff --git a/src/lib/actions/sandbox/process-recovery.ts b/src/lib/actions/sandbox/process-recovery.ts index f0b63008d45..36a65eddf54 100644 --- a/src/lib/actions/sandbox/process-recovery.ts +++ b/src/lib/actions/sandbox/process-recovery.ts @@ -441,6 +441,144 @@ export function waitForManagedGatewaySupervisor( return false; } +type FinalRelaunchManagedSupervisorReadiness = { ready: true } | { detail: string; ready: false }; + +function waitForFinalRelaunchManagedSupervisor( + sandboxName: string, + requestGatewaySupervisorAction: typeof executeGatewaySupervisorAction, +): FinalRelaunchManagedSupervisorReadiness { + let probeResult: ManagedGatewaySupervisorActionResult | null = null; + try { + const ready = waitForManagedGatewaySupervisor(sandboxName, { + intervalSeconds: readNonNegativeNumberEnv( + "NEMOCLAW_GATEWAY_RECOVERY_POLL_INTERVAL_SECONDS", + 3, + ), + requestGatewaySupervisorActionImpl: (name, action, timeout) => { + probeResult = requestGatewaySupervisorAction(name, action, timeout); + return probeResult; + }, + }); + if (ready) return { ready: true }; + } catch { + return { + detail: + "the replacement container identity changed during the final managed supervisor health check", + ready: false, + }; + } + const failure = classifyGatewayRestartFailure(probeResult); + return { detail: `${failure.layer}: ${failure.detail}`, ready: false }; +} + +function finalRelaunchContainerFailureDetail( + completion: ReturnType, +): string | null { + if (completion.replacementStoppedForCommit === false) { + return "Docker could not stop the replacement container for the final recovery handoff. NemoClaw did not start the primary dashboard/API host forward"; + } + if (completion.replacementRestarted === false) { + return "Docker could not start the replacement container to complete the final recovery handoff. NemoClaw did not start the primary dashboard/API host forward"; + } + return null; +} + +type FinalRelaunchRecoveryFailure = { + checked: true; + forwardRecovered: false; + forwardRecoveryFailed?: undefined; + forwardRecoveryFailureDetail?: undefined; + recovered: false; + recoveryFailureDetail?: string; + wasRunning: false; +}; + +function finalRelaunchRecoveryFailure( + recoveryFailureDetail?: string, +): FinalRelaunchRecoveryFailure { + const failure = { + checked: true, + forwardRecovered: false, + recovered: false, + wasRunning: false, + } as const; + return recoveryFailureDetail ? { ...failure, recoveryFailureDetail } : failure; +} + +function finalizeRelaunchedRecovery( + sandboxName: string, + relaunch: ManagedSupervisorRelaunch, + { + printRecoveryHints, + quiet, + requestManagedProbe, + waitForRecoveryReadiness, + }: { + printRecoveryHints: () => void; + quiet: boolean; + requestManagedProbe: typeof executeGatewaySupervisorAction; + waitForRecoveryReadiness: () => string | null; + }, +): FinalRelaunchRecoveryFailure | null { + let completion: ReturnType; + try { + completion = relaunch.finalize(true); + if (completion.stateRestored === false || completion.rolledBack) { + const recoveryFailureDetail = completion.rolledBack + ? "Sandbox recovery did not complete; the previous container was restored" + : "Sandbox recovery failed and the previous container could not be restored automatically"; + if (!quiet) { + console.error(` ${recoveryFailureDetail}.`); + if (completion.rolledBack && completion.stateBackupRemoved === false) { + console.error(" Warning: the temporary sandbox state backup could not be removed."); + } + if (!completion.rolledBack) printRecoveryHints(); + } + return finalRelaunchRecoveryFailure(recoveryFailureDetail); + } + } catch { + if (!quiet) { + console.error( + " NemoClaw could not confirm the final replacement container handoff. It did not start the primary dashboard/API host forward.", + ); + } + return finalRelaunchRecoveryFailure( + "NemoClaw could not confirm the final replacement container handoff. It did not start the primary dashboard/API host forward", + ); + } + + const containerFailureDetail = finalRelaunchContainerFailureDetail(completion); + if (containerFailureDetail) return finalRelaunchRecoveryFailure(containerFailureDetail); + + if (completion.replacementRestarted === true) { + const managedSupervisor = waitForFinalRelaunchManagedSupervisor( + sandboxName, + requestManagedProbe, + ); + if (!managedSupervisor.ready) { + 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}`, + ); + } + const finalReadinessFailureDetail = waitForRecoveryReadiness(); + if (finalReadinessFailureDetail) { + return finalRelaunchRecoveryFailure(finalReadinessFailureDetail); + } + } + + if (!completion.backupRemoved && !quiet) { + console.error( + " Warning: the recovered sandbox is healthy, but its previous container backup could not be removed.", + ); + } + if (completion.stateBackupRemoved === false && !quiet) { + console.error( + " Warning: the recovered sandbox is healthy, but its temporary state backup could not be removed.", + ); + } + return null; +} + export function confirmRecoveredSandboxGatewayManaged( sandboxName: string, options: { @@ -778,15 +916,15 @@ function recreatedSandboxOpenShellReadinessFailureDetail( const detail = (() => { switch (failure) { case "managed-health-definitive-failure": - return "the recreated sandbox failed the managed health guard, so the primary dashboard/API host forward was not started"; + return "the managed supervisor health check for the pinned replacement container did not pass. NemoClaw did not start the primary dashboard/API host forward"; case "managed-health-inconclusive-timeout": - return "the recreated sandbox managed health guard stayed inconclusive within the readiness deadline, so the primary dashboard/API host forward was not started"; + return "the managed supervisor health check for the pinned replacement container stayed inconclusive within the OpenShell readiness deadline. NemoClaw did not start the primary dashboard/API host forward"; case "openshell-readiness-failure": - return "the recreated sandbox did not become ready in OpenShell, so the primary dashboard/API host forward was not started"; + return "the recreated sandbox did not become ready in OpenShell. NemoClaw did not start the primary dashboard/API host forward"; } })(); const managedHealthResult = managedHealthFailureDetail - ? ` Managed health result: ${managedHealthFailureDetail}` + ? ` Managed supervisor health check result: ${managedHealthFailureDetail}` : ""; const openshellResult = openshellError ? ` Last OpenShell readiness error: ${openshellError}` @@ -795,7 +933,7 @@ function recreatedSandboxOpenShellReadinessFailureDetail( } // Default seconds to wait for OpenShell to re-register a recreated sandbox as -// Ready before giving up and surfacing the manual-recover hint. Aligned with +// Ready before returning a classified recovery failure. Aligned with // `connect`'s readiness budget (`waitForSandboxReadyOrExit` defaults to 120s): // both prove the same post-recreate sandbox readiness, but this path used to // give up 4x sooner (30s), so a cold-start `phase: Error` settling window that @@ -1145,11 +1283,12 @@ function isHermesAgent( * whose OpenClaw processes are not running. Also re-establishes the * host-side dashboard port-forward when it has gone dead independently * of the gateway. Returns an object describing the outcome: - * `{ checked, wasRunning, recovered, forwardRecovered, forwardRecoveryFailed?, secretBoundaryRefused?, secretBoundaryReason? }`. + * `{ checked, wasRunning, recovered, forwardRecovered, forwardRecoveryFailed?, recoveryFailureDetail?, secretBoundaryRefused?, secretBoundaryReason? }`. * `onRecoveryFailureLayer` reports the classified managed-restart failure so a * quiet caller (`recover`, `connect --probe-only`) can still explain why * recovery is not retryable instead of printing a generic "check the gateway - * log". The result shape is unchanged so existing callers keep their contract. + * log". Failures before forward recovery use `recoveryFailureDetail`; actual + * forward failures retain `forwardRecoveryFailed` and their forward detail. */ function checkAndRecoverSandboxProcessesWithoutHostLock( sandboxName: string, @@ -1367,7 +1506,7 @@ function checkAndRecoverSandboxProcessesWithoutHostLock( ? (name: string, action: "restart" | "recover" | "probe", timeout = 210000) => requestPinnedGatewaySupervisorAction(name, action, timeout, relaunch.containerId) : requestGatewaySupervisorAction; - let relaunchedIdentityRejected = false; + let relaunchedIdentityChanged = false; let relaunchedManagedHealthFailureDetail: string | null = null; const confirmRelaunchedManagedHealth = relaunch ? (timeout = OPENSHELL_PROBE_TIMEOUT_MS) => { @@ -1380,7 +1519,6 @@ function checkAndRecoverSandboxProcessesWithoutHostLock( }, }); if (confirmed === false) { - relaunchedIdentityRejected = true; const failure = classifyGatewayRestartFailure(probeResult); relaunchedManagedHealthFailureDetail = `${failure.layer}: ${failure.detail}`; } else if (confirmed === true) { @@ -1388,7 +1526,7 @@ function checkAndRecoverSandboxProcessesWithoutHostLock( } return confirmed; } catch { - relaunchedIdentityRejected = true; + relaunchedIdentityChanged = true; relaunchedManagedHealthFailureDetail = "the pinned replacement sandbox identity changed during the managed probe"; return false; @@ -1457,8 +1595,7 @@ function checkAndRecoverSandboxProcessesWithoutHostLock( wasRunning: false, recovered: false, forwardRecovered: false, - forwardRecoveryFailed: true, - forwardRecoveryFailureDetail: `the recreated sandbox failed the managed health guard while waiting for its gateway. Managed health result: ${relaunchedManagedHealthFailureDetail}`, + 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 }; @@ -1467,30 +1604,29 @@ function checkAndRecoverSandboxProcessesWithoutHostLock( // recovery has already passed its authenticated control and health gates; // a replacement also rechecks its pinned identity before readiness. const recoveryRequiresReadiness = recovery.kind === "managed" || relaunch; - const readinessFailureDetail = recoveryRequiresReadiness - ? (() => { - const readinessOptions: RecreatedSandboxOpenShellReadyOptions = { - beforeProbe: relaunch - ? (timeoutMs) => confirmRelaunchedManagedHealth?.(timeoutMs) ?? null + const waitForRecoveryReadiness = () => { + const readinessOptions: RecreatedSandboxOpenShellReadyOptions = { + beforeProbe: relaunch + ? (timeoutMs) => confirmRelaunchedManagedHealth?.(timeoutMs) ?? null + : undefined, + }; + const readiness = + waitForRecreatedSandboxOpenShellReadyImpl === waitForRecreatedSandboxOpenShellReady + ? waitForRecreatedSandboxOpenShellReadyResult(sandboxName, readinessOptions) + : waitForRecreatedSandboxOpenShellReadyImpl(sandboxName, readinessOptions) + ? ({ ready: true } as const) + : ({ failure: "openshell-readiness-failure", ready: false } as const); + return readiness.ready + ? null + : recreatedSandboxOpenShellReadinessFailureDetail( + readiness.failure, + "openshellError" in readiness ? readiness.openshellError : undefined, + readiness.failure === "managed-health-definitive-failure" + ? (relaunchedManagedHealthFailureDetail ?? undefined) : undefined, - }; - const readiness = - waitForRecreatedSandboxOpenShellReadyImpl === waitForRecreatedSandboxOpenShellReady - ? waitForRecreatedSandboxOpenShellReadyResult(sandboxName, readinessOptions) - : waitForRecreatedSandboxOpenShellReadyImpl(sandboxName, readinessOptions) - ? ({ ready: true } as const) - : ({ failure: "openshell-readiness-failure", ready: false } as const); - return readiness.ready - ? null - : recreatedSandboxOpenShellReadinessFailureDetail( - readiness.failure, - "openshellError" in readiness ? readiness.openshellError : undefined, - readiness.failure === "managed-health-definitive-failure" - ? (relaunchedManagedHealthFailureDetail ?? undefined) - : undefined, - ); - })() - : null; + ); + }; + const readinessFailureDetail = recoveryRequiresReadiness ? waitForRecoveryReadiness() : null; if (readinessFailureDetail) { try { relaunch?.finalize(false); @@ -1503,55 +1639,22 @@ function checkAndRecoverSandboxProcessesWithoutHostLock( wasRunning: false, recovered: false, forwardRecovered: false, - forwardRecoveryFailed: true, - forwardRecoveryFailureDetail: readinessFailureDetail, + recoveryFailureDetail: readinessFailureDetail, }; } if (relaunch) { - try { - const completion = relaunch.finalize(true); - if (completion.stateRestored === false || completion.rolledBack) { - if (!quiet) { - console.error( - completion.rolledBack - ? " Sandbox recovery did not complete; the previous container was restored." - : " Sandbox recovery failed and the previous container could not be restored automatically.", - ); - if (completion.rolledBack && completion.stateBackupRemoved === false) { - console.error(" Warning: the temporary sandbox state backup could not be removed."); - } - if (!completion.rolledBack) { - printHostManagedGatewayRecoveryHints( - sandboxName, - recoveryAgent, - managedRecoveryFailureLayer, - ); - } - } - return { - checked: true, - wasRunning: false, - recovered: false, - forwardRecovered: false, - }; - } - if (!completion.backupRemoved && !quiet) { - console.error( - " Warning: the recovered sandbox is healthy, but its previous container backup could not be removed.", - ); - } - if (completion.stateBackupRemoved === false && !quiet) { - console.error( - " Warning: the recovered sandbox is healthy, but its temporary state backup could not be removed.", - ); - } - } catch { - if (!quiet) { - console.error( - " Warning: the recovered sandbox is healthy, but container transaction cleanup could not be confirmed.", - ); - } - } + const finalizationFailure = finalizeRelaunchedRecovery(sandboxName, relaunch, { + printRecoveryHints: () => + printHostManagedGatewayRecoveryHints( + sandboxName, + recoveryAgent, + managedRecoveryFailureLayer, + ), + quiet, + requestManagedProbe, + waitForRecoveryReadiness, + }); + if (finalizationFailure) return finalizationFailure; } const mcpRefusal = processRecoveryMcpReconciliationRefusal(sandboxName, false); if (mcpRefusal) return mcpRefusal; @@ -1560,16 +1663,16 @@ function checkAndRecoverSandboxProcessesWithoutHostLock( beforeStart: confirmRelaunchedManagedHealthForForward ?? undefined, isWsl: isWslOverride, }); - if (!forwardRecovered && relaunchedIdentityRejected) { - return withManagedControlCompletion({ + if (!forwardRecovered && relaunchedManagedHealthFailureDetail) { + return { checked: true, wasRunning: false, - recovered: true, + recovered: false, forwardRecovered: false, - forwardRecoveryFailed: true, - forwardRecoveryFailureDetail: - "the primary dashboard/API host forward could not be re-established", - }); + 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}`, + }; } const dashboardForwardRecovered = ensureHermesDashboardPortForwardIfEnabled(sandboxName); const messagingForwardRecovered = recoverMessagingHostForward(sandboxName, { quiet }); diff --git a/test/process-recovery-supervisor-relaunch.test.ts b/test/process-recovery-supervisor-relaunch.test.ts index 7c8f8d44072..9e9db633817 100644 --- a/test/process-recovery-supervisor-relaunch.test.ts +++ b/test/process-recovery-supervisor-relaunch.test.ts @@ -10,11 +10,22 @@ import { 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"; +import { finalizeDockerGpuPatchBackup } from "../src/lib/onboard/docker-gpu-patch-finalize.ts"; import * as registry from "../src/lib/state/registry.ts"; const OPENSHELL_RELAY_CHANNEL_DROPPED_STDERR = `Error: × status: Unavailable, message: "relay │ channel dropped", details: [], metadata: MetadataMap { headers: {} } `; +const ACCEPTED_MANAGED_PROBE = { + status: 0, + stdout: "GATEWAY_PID=4242\n", + stderr: "", +} as const; +const MISSING_MANAGED_SUPERVISOR = { + status: 1, + stdout: "", + stderr: "SUPERVISOR_NOT_RUNNING", +} as const; afterEach(() => { vi.restoreAllMocks(); @@ -47,13 +58,17 @@ function setImmediateRecoveryPolling() { vi.stubEnv("NEMOCLAW_FORWARD_RECOVERY_WAIT_MS", "0"); } -function composedRelaunchTransaction(order: string[]) { - const finalizeTransaction = vi.fn(({ supervisorReady }: { supervisorReady: boolean }) => { - order.push(supervisorReady ? "commit-container" : "rollback-container"); - return supervisorReady - ? { backupRemoved: true, rolledBack: false } - : { backupRemoved: false, rolledBack: true }; - }); +function composedRelaunchTransaction( + order: string[], + finalizeTransaction: typeof finalizeDockerGpuPatchBackup = vi.fn( + ({ supervisorReady }: { supervisorReady: boolean }) => { + order.push(supervisorReady ? "commit-container" : "rollback-container"); + return supervisorReady + ? { backupRemoved: true, rolledBack: false } + : { backupRemoved: false, rolledBack: true }; + }, + ), +) { const resolveContainer = vi .fn() .mockReturnValueOnce("old-container-id") @@ -614,11 +629,11 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { wasRunning: false, recovered: false, forwardRecovered: false, - forwardRecoveryFailed: true, - forwardRecoveryFailureDetail: expect.stringContaining( + recoveryFailureDetail: expect.stringContaining( "unsafe config path: GATEWAY_UNSAFE_CONFIG_PATH", ), }); + expect(result).not.toHaveProperty("forwardRecoveryFailed"); expect(requestPinnedGatewaySupervisorAction).toHaveBeenCalledWith( "wait-failed-box", "probe", @@ -686,6 +701,258 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { ); }); + it("restores the primary dashboard/API host forward after the final legacy container restart (#9364)", () => { + mockOpenClawSandbox("legacy-handoff-box"); + setImmediateRecoveryPolling(); + const order: string[] = []; + let forwardStarted = false; + const dockerStop = vi.fn(() => ({ status: 0 })); + const dockerRm = vi.fn(() => ({ status: 0 })); + const dockerStart = vi.fn(() => ({ status: 0 })); + const finalizeTransaction = vi.fn( + (options: Parameters[0]) => + finalizeDockerGpuPatchBackup(options, { dockerStop, dockerRm, dockerStart }), + ); + const { relaunchManagedSupervisorSessionImpl } = composedRelaunchTransaction( + order, + finalizeTransaction, + ); + const requestGatewaySupervisorAction = vi.fn((_name: string, action: string) => + action === "recover" ? { status: 1, stdout: "", stderr: "SUPERVISOR_NOT_RUNNING" } : null, + ); + const restartedGateway = { + status: 0, + stdout: `v1 ${"c".repeat(64)} complete ok 4242 4343\nGATEWAY_PID=4343`, + stderr: "", + }; + const requestPinnedGatewaySupervisorAction = vi + .fn() + .mockReturnValueOnce(MISSING_MANAGED_SUPERVISOR) + .mockReturnValueOnce(ACCEPTED_MANAGED_PROBE) + .mockReturnValueOnce(ACCEPTED_MANAGED_PROBE) + .mockReturnValueOnce(restartedGateway) + .mockReturnValueOnce(MISSING_MANAGED_SUPERVISOR) + .mockReturnValue(ACCEPTED_MANAGED_PROBE); + const waitForRecreatedSandboxOpenShellReadyImpl = vi.fn( + (_name: string, options?: { beforeProbe?: (timeoutMs: number) => boolean | null }) => + options?.beforeProbe?.(1000) === true, + ); + vi.spyOn(forwardHealth, "isLocalForwardReachable").mockImplementation(() => forwardStarted); + vi.spyOn(openshellRuntime, "captureOpenshell").mockImplementation((args) => { + const responses = { + "forward list": () => ({ + status: 0, + output: forwardStarted + ? "SANDBOX BIND PORT PID STATUS\nlegacy-handoff-box 127.0.0.1 18789 12345 running" + : "SANDBOX BIND PORT PID STATUS", + }), + }; + return ( + responses[args.join(" ") as keyof typeof responses]?.() ?? { + status: 1, + output: "", + stdout: "", + stderr: "unexpected openshell command", + } + ); + }); + const runOpenshell = vi.spyOn(openshellRuntime, "runOpenshell").mockImplementation((args) => { + forwardStarted ||= args.join(" ") === "forward start --background 18789 legacy-handoff-box"; + return { status: 0 } as never; + }); + + const result = checkAndRecoverSandboxProcesses("legacy-handoff-box", { + quiet: true, + isSandboxGatewayRunningImpl: () => false, + requestGatewaySupervisorAction, + requestPinnedGatewaySupervisorAction, + relaunchManagedSupervisorSessionImpl, + waitForRecreatedSandboxOpenShellReadyImpl, + }); + + expect(result).toMatchObject({ + checked: true, + wasRunning: false, + recovered: true, + forwardRecovered: true, + }); + expect(dockerStop).toHaveBeenCalledWith( + "replacement-container-id", + expect.objectContaining({ ignoreError: true }), + ); + expect(dockerRm).toHaveBeenCalledWith( + "openshell-recovery-box-nemoclaw-backup", + expect.objectContaining({ ignoreError: true }), + ); + expect(dockerStart).toHaveBeenCalledWith( + "replacement-container-id", + expect.objectContaining({ ignoreError: true }), + ); + expect(waitForRecreatedSandboxOpenShellReadyImpl).toHaveBeenCalledTimes(2); + expect(dockerStart.mock.invocationCallOrder[0]).toBeLessThan( + waitForRecreatedSandboxOpenShellReadyImpl.mock.invocationCallOrder[1], + ); + expect(waitForRecreatedSandboxOpenShellReadyImpl.mock.invocationCallOrder[1]).toBeLessThan( + runOpenshell.mock.invocationCallOrder[0], + ); + expect(runOpenshell).toHaveBeenCalledWith( + ["forward", "start", "--background", "18789", "legacy-handoff-box"], + expect.objectContaining({ ignoreError: true }), + ); + }); + + it.each([ + { + condition: "Docker cannot stop the replacement container", + finalizeOutcome: () => ({ + backupRemoved: false, + replacementStoppedForCommit: false, + rolledBack: false, + stateRestored: true, + }), + expectedDetail: "Docker could not stop the replacement container", + expectedReadinessCalls: 1, + finalPinnedAction: () => ACCEPTED_MANAGED_PROBE, + finalReadinessReady: true, + }, + { + condition: "Docker cannot start the replacement container", + finalizeOutcome: () => ({ + backupRemoved: true, + replacementRestarted: false, + replacementStoppedForCommit: true, + rolledBack: false, + stateRestored: true, + }), + expectedDetail: "Docker could not start the replacement container", + expectedReadinessCalls: 1, + finalPinnedAction: () => ACCEPTED_MANAGED_PROBE, + finalReadinessReady: true, + }, + { + condition: "the final container handoff cannot be confirmed", + finalizeOutcome: () => { + throw new Error("final handoff unavailable"); + }, + expectedDetail: "could not confirm the final replacement container handoff", + expectedReadinessCalls: 1, + finalPinnedAction: () => ACCEPTED_MANAGED_PROBE, + finalReadinessReady: true, + }, + { + condition: "the final OpenShell readiness check fails", + finalizeOutcome: () => ({ + backupRemoved: true, + replacementRestarted: true, + replacementStoppedForCommit: true, + rolledBack: false, + stateRestored: true, + }), + expectedDetail: "did not become ready in OpenShell", + expectedReadinessCalls: 2, + finalPinnedAction: () => ACCEPTED_MANAGED_PROBE, + finalReadinessReady: false, + }, + { + condition: "the managed supervisor health check does not pass", + finalizeOutcome: () => ({ + backupRemoved: true, + replacementRestarted: true, + replacementStoppedForCommit: true, + rolledBack: false, + stateRestored: true, + }), + expectedDetail: "managed supervisor health check for the pinned replacement container", + expectedReadinessCalls: 1, + finalPinnedAction: () => MISSING_MANAGED_SUPERVISOR, + finalReadinessReady: true, + }, + { + condition: "the pinned container identity changes", + finalizeOutcome: () => ({ + backupRemoved: true, + replacementRestarted: true, + replacementStoppedForCommit: true, + rolledBack: false, + stateRestored: true, + }), + expectedDetail: "replacement container identity changed", + expectedReadinessCalls: 1, + finalPinnedAction: () => { + throw new Error("replacement identity changed"); + }, + finalReadinessReady: true, + }, + ])( + "does not start the primary dashboard/API host forward when $condition (#9364)", + ({ + expectedDetail, + expectedReadinessCalls, + finalPinnedAction, + finalReadinessReady, + finalizeOutcome, + }) => { + mockOpenClawSandbox("failed-handoff-box"); + setImmediateRecoveryPolling(); + const finalize = vi.fn((_supervisorReady: boolean) => finalizeOutcome()); + const relaunchManagedSupervisorSessionImpl = vi.fn(() => ({ + containerId: "replacement-container-id", + finalize, + })); + const requestGatewaySupervisorAction = vi.fn(() => ({ + status: 1, + stdout: "", + stderr: "SUPERVISOR_NOT_RUNNING", + })); + const requestPinnedGatewaySupervisorAction = vi + .fn() + .mockReturnValueOnce(ACCEPTED_MANAGED_PROBE) + .mockReturnValueOnce(ACCEPTED_MANAGED_PROBE) + .mockImplementation(finalPinnedAction); + const waitForRecreatedSandboxOpenShellReadyImpl = vi + .fn() + .mockImplementationOnce( + (_name: string, options?: { beforeProbe?: (timeoutMs: number) => boolean | null }) => + options?.beforeProbe?.(1000) === true, + ) + .mockImplementation( + (_name: string, options?: { beforeProbe?: (timeoutMs: number) => boolean | null }) => + options?.beforeProbe?.(1000) === true && finalReadinessReady, + ); + const captureOpenshell = vi + .spyOn(openshellRuntime, "captureOpenshell") + .mockReturnValue({ status: 0, output: "" }); + const runOpenshell = vi + .spyOn(openshellRuntime, "runOpenshell") + .mockReturnValue({ status: 0 } as never); + + const result = checkAndRecoverSandboxProcesses("failed-handoff-box", { + quiet: true, + isSandboxGatewayRunningImpl: () => false, + requestGatewaySupervisorAction, + requestPinnedGatewaySupervisorAction, + relaunchManagedSupervisorSessionImpl, + waitForRecreatedSandboxOpenShellReadyImpl, + }); + + expect(result).toMatchObject({ + checked: true, + wasRunning: false, + recovered: false, + forwardRecovered: false, + recoveryFailureDetail: expect.stringContaining(expectedDetail), + }); + expect(result).not.toHaveProperty("forwardRecoveryFailed"); + expect(finalize).toHaveBeenCalledOnce(); + expect(finalize).toHaveBeenCalledWith(true); + expect(waitForRecreatedSandboxOpenShellReadyImpl).toHaveBeenCalledTimes( + expectedReadinessCalls, + ); + expect(captureOpenshell).not.toHaveBeenCalled(); + expect(runOpenshell).not.toHaveBeenCalled(); + }, + ); + it("rolls back when post-restore restart does not report an exact ok disposition", () => { mockOpenClawSandbox("post-restore-fail"); setImmediateRecoveryPolling(); @@ -719,6 +986,7 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { wasRunning: false, recovered: false, forwardRecovered: false, + recoveryFailureDetail: "Sandbox recovery did not complete; the previous container was restored", }); expect(order).toEqual(["restore-state", "post-restore-restart", "rollback-container"]); expect(requestPinnedGatewaySupervisorAction).toHaveBeenCalledTimes(4); @@ -754,7 +1022,12 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { _options?: { beforeProbe?: (timeoutMs: number) => boolean | null; timeoutSeconds?: number }, ) => true, ); - const runOpenshell = vi.spyOn(openshellRuntime, "runOpenshell"); + const captureOpenshell = vi + .spyOn(openshellRuntime, "captureOpenshell") + .mockReturnValue({ status: 0, output: "" }); + const runOpenshell = vi + .spyOn(openshellRuntime, "runOpenshell") + .mockReturnValue({ status: 0 } as never); const result = checkAndRecoverSandboxProcesses("restore-failed-box", { quiet: true, @@ -770,6 +1043,8 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { wasRunning: false, recovered: false, forwardRecovered: false, + recoveryFailureDetail: + "Sandbox recovery did not complete; the previous container was restored", }); expect(finalize).toHaveBeenCalledOnce(); expect(finalize).toHaveBeenCalledWith(true); @@ -781,6 +1056,7 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { expect(waitForRecreatedSandboxOpenShellReadyImpl.mock.calls[0]?.[1]).not.toHaveProperty( "timeoutSeconds", ); + expect(captureOpenshell).not.toHaveBeenCalled(); expect(runOpenshell).not.toHaveBeenCalled(); }); @@ -806,6 +1082,12 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { })); vi.spyOn(console, "log").mockImplementation(() => undefined); const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + const captureOpenshell = vi + .spyOn(openshellRuntime, "captureOpenshell") + .mockReturnValue({ status: 0, output: "" }); + const runOpenshell = vi + .spyOn(openshellRuntime, "runOpenshell") + .mockReturnValue({ status: 0 } as never); const result = checkAndRecoverSandboxProcesses("restore-rollback", { quiet: false, @@ -821,7 +1103,12 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { wasRunning: false, recovered: false, forwardRecovered: false, + recoveryFailureDetail: + "Sandbox recovery failed and the previous container could not be restored automatically", }); + expect(result).not.toHaveProperty("forwardRecoveryFailed"); + expect(captureOpenshell).not.toHaveBeenCalled(); + expect(runOpenshell).not.toHaveBeenCalled(); const output = errorSpy.mock.calls.flat().join("\n"); expect(output).toContain( "Sandbox recovery failed and the previous container could not be restored automatically.", @@ -958,8 +1245,7 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { wasRunning: false, recovered: false, forwardRecovered: false, - forwardRecoveryFailed: true, - forwardRecoveryFailureDetail: expect.stringContaining("did not become ready in OpenShell"), + recoveryFailureDetail: expect.stringContaining("did not become ready in OpenShell"), }); expect(finalize).toHaveBeenCalledOnce(); expect(finalize).toHaveBeenCalledWith(false); @@ -1022,8 +1308,7 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { wasRunning: false, recovered: false, forwardRecovered: false, - forwardRecoveryFailed: true, - forwardRecoveryFailureDetail: expect.stringContaining( + recoveryFailureDetail: expect.stringContaining( 'Last OpenShell readiness error: Error: status: Unavailable, message: "relay channel dropped"', ), }); @@ -1073,17 +1358,35 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { wasRunning: false, recovered: false, forwardRecovered: false, - forwardRecoveryFailed: true, - forwardRecoveryFailureDetail: expect.stringContaining("failed the managed health guard"), + recoveryFailureDetail: expect.stringContaining( + "managed supervisor health check for the pinned replacement container did not pass", + ), }); - expect(result.forwardRecoveryFailureDetail).toContain( + expect("recoveryFailureDetail" in result ? result.recoveryFailureDetail : "").toContain( "unsafe config path: GATEWAY_UNSAFE_CONFIG_PATH", ); expect(finalize).toHaveBeenCalledWith(false); expect(captureOpenshell).not.toHaveBeenCalled(); }); - it("rejects a healthy forward when the replacement identity changes after readiness", () => { + it.each([ + { + condition: "the replacement identity changes after readiness", + expectedDetail: "replacement container identity changed", + finalProbe: () => { + throw new Error("replacement identity changed"); + }, + }, + { + condition: "the final managed supervisor health check is rejected", + expectedDetail: "unsafe config path: GATEWAY_UNSAFE_CONFIG_PATH", + finalProbe: () => ({ + status: 1, + stdout: "", + stderr: "GATEWAY_UNSAFE_CONFIG_PATH", + }), + }, + ])("rejects a healthy forward when $condition (#9364)", ({ expectedDetail, finalProbe }) => { mockOpenClawSandbox("drifted-box"); vi.mocked(agentRuntime.getSessionAgent).mockReturnValue({ name: "openclaw", @@ -1112,9 +1415,7 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { .fn() .mockReturnValueOnce(acceptedProbe) .mockReturnValueOnce(acceptedProbe) - .mockImplementationOnce(() => { - throw new Error("replacement identity changed"); - }) + .mockImplementationOnce(finalProbe) .mockReturnValue(acceptedProbe); const waitForRecreatedSandboxOpenShellReadyImpl = vi.fn( (_name, options) => options.beforeProbe?.(1000) === true, @@ -1140,10 +1441,11 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { expect(result).toMatchObject({ checked: true, wasRunning: false, - recovered: true, + recovered: false, forwardRecovered: false, - forwardRecoveryFailed: true, + recoveryFailureDetail: expect.stringContaining(expectedDetail), }); + expect(result).not.toHaveProperty("forwardRecoveryFailed"); expect(requestPinnedGatewaySupervisorAction).toHaveBeenCalledTimes(3); expect(requestPinnedGatewaySupervisorAction).toHaveBeenLastCalledWith( "drifted-box", diff --git a/test/support/connect-flow-test-harness.ts b/test/support/connect-flow-test-harness.ts index 372ad2959b9..3a213f01d82 100644 --- a/test/support/connect-flow-test-harness.ts +++ b/test/support/connect-flow-test-harness.ts @@ -12,6 +12,7 @@ import type { ConfigObject } from "../../src/lib/security/credential-filter"; import type { SandboxEntry } from "../../src/lib/state/registry"; type ConnectSandbox = (typeof import("../../src/lib/actions/sandbox/connect"))["connectSandbox"]; +type RestoreSandboxStartupState = (typeof import("../../src/lib/actions/sandbox/connect"))["restoreSandboxStartupState"]; type GatewayRouteMutationLock = (typeof import("../../src/lib/inference/gateway-route-mutation-lock"))["withGatewayRouteMutationLock"]; type LaunchReadinessPublicationResult = @@ -50,6 +51,7 @@ export type ConnectHarness = { recoverPortableDemoLifecycleSpy: MockInstance; registryEntries: SandboxEntry[]; resolveAgentConfigSpy: MockInstance; + restoreSandboxStartupState: RestoreSandboxStartupState; runAutoPairSpy: MockInstance; runOpenshellSpy: MockInstance; runSetupDnsProxySpy: MockInstance; @@ -79,6 +81,7 @@ export type ConnectHarnessOptions = { forwardRecovered?: boolean; forwardRecoveryFailed?: boolean; forwardRecoveryFailureDetail?: string; + recoveryFailureDetail?: string; secretBoundaryRefused?: boolean; secretBoundaryReason?: SecretBoundaryRefusalReason; mcpReconciliationRefused?: boolean; @@ -374,6 +377,7 @@ export function createConnectHarness(options: ConnectHarnessOptions = {}): Conne recoverPortableDemoLifecycleSpy, registryEntries, resolveAgentConfigSpy, + restoreSandboxStartupState: requireDist(connectModulePath).restoreSandboxStartupState, runAutoPairSpy, settlePortablePairingSpy, runOpenshellSpy,