diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index f76b34a6717..4dbd5d4ea49 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1070,7 +1070,8 @@ On Jetson/Tegra hosts, the compatibility path uses the NVIDIA runtime and adds e These include selected `/dev/nvmap`, `/dev/nvhost-*`, and `/dev/nvgpu/igpu0/*` nodes plus real `/dev/dri/renderD*` character devices. After compatibility recreation starts, onboarding keeps the pre-patch container as a rollback backup until the replacement passes the Ready, GPU, and applicable local-inference checks. If one of those checks fails before backup removal, onboarding prints failure diagnostics and attempts to restore the pre-patch container. -To commit the replacement, NemoClaw stops it, removes the rollback backup, starts the replacement as the final container lifecycle event, and verifies OpenShell supervisor readiness again. +To commit the replacement, NemoClaw stops it, removes the rollback backup, and waits until a successful OpenShell sandbox list has no row with that sandbox name. +NemoClaw then starts the replacement as the final container lifecycle event and verifies OpenShell supervisor readiness again within the same handoff deadline. If that final handoff cannot be confirmed, onboarding exits with the container diagnostics and cleanup guidance instead of reporting success. If rollback fails, onboarding reports that the pre-patch container was not restored and prints container-cleanup guidance. GPU-proof diagnostics are captured before rollback and can print that guidance before the final container state is known, so inspect the sandbox and its labeled Docker containers before running a deletion command. diff --git a/src/lib/actions/sandbox/process-recovery.ts b/src/lib/actions/sandbox/process-recovery.ts index bb9986ccc50..6656674bbe4 100644 --- a/src/lib/actions/sandbox/process-recovery.ts +++ b/src/lib/actions/sandbox/process-recovery.ts @@ -10,6 +10,7 @@ import { captureSandboxSshConfig, getOpenshellBinary, isCommandTimeout, + runOpenshell, } from "../../adapters/openshell/runtime"; import { OPENSHELL_PROBE_TIMEOUT_MS } from "../../adapters/openshell/timeouts"; import { @@ -498,6 +499,9 @@ function finalRelaunchContainerFailureDetail( 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.lifecycleReleaseObserved === false) { + return "OpenShell did not release the sandbox name before the final recovery handoff. NemoClaw did not restart the replacement container or 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"; } @@ -748,6 +752,7 @@ function recoverSandboxProcesses( const relaunch = relaunchManagedSupervisorSessionImpl(sandboxName, { quiet, deps: { + runOpenshell, confirmMissingSupervisor: (containerId) => isExactlyMissingManagedSupervisor( requestPinnedGatewaySupervisorAction(sandboxName, "probe", 210000, containerId), diff --git a/src/lib/actions/sandbox/supervisor-relaunch.test.ts b/src/lib/actions/sandbox/supervisor-relaunch.test.ts index de8fff68946..08ea2622049 100644 --- a/src/lib/actions/sandbox/supervisor-relaunch.test.ts +++ b/src/lib/actions/sandbox/supervisor-relaunch.test.ts @@ -74,6 +74,7 @@ function baseDeps(overrides: ManagedSupervisorRelaunchDeps = {}) { failedFiles: [], })), removeBackup: vi.fn(() => true), + runOpenshell: vi.fn(() => ({ status: 0, stdout: "No sandboxes found.\n" })), recreate: vi.fn(() => patchResult()), finalize: vi.fn(({ supervisorReady }) => supervisorReady @@ -153,10 +154,15 @@ describe("relaunchManagedSupervisorSession", () => { }); expect(deps.restoreState).toHaveBeenCalledWith("alpha", "/tmp/rebuild-backups/alpha/recovery"); expect(deps.removeBackup).toHaveBeenCalledWith("alpha", "/tmp/rebuild-backups/alpha/recovery"); - expect(deps.finalize).toHaveBeenCalledWith({ - result: expect.objectContaining({ newContainerId: "new-container-id" }), - supervisorReady: true, - }); + expect(deps.finalize).toHaveBeenCalledWith( + { + lifecycleReleaseTimeoutSecs: 900, + result: expect.objectContaining({ newContainerId: "new-container-id" }), + sandboxName: "alpha", + supervisorReady: true, + }, + { runOpenshell: deps.runOpenshell }, + ); }); it("retries only transport-level state backup failures after a container restart", () => { @@ -381,9 +387,26 @@ describe("relaunchManagedSupervisorSession", () => { }); expect(order).toEqual(["restore-state", "restart-restored-gateway", "commit-container"]); expect(deps.restartRestoredManagedGateway).toHaveBeenCalledWith("new-container-id"); - expect(deps.finalize).toHaveBeenCalledWith({ - result: expect.objectContaining({ newContainerId: "new-container-id" }), - supervisorReady: true, + expect(deps.finalize).toHaveBeenCalledWith( + { + lifecycleReleaseTimeoutSecs: 900, + result: expect.objectContaining({ newContainerId: "new-container-id" }), + sandboxName: "alpha", + supervisorReady: true, + }, + { runOpenshell: deps.runOpenshell }, + ); + }); + + it("uses only an injected host sleep for lifecycle polling after recreation (#9531)", () => { + const sleep = vi.fn(); + const deps = baseDeps({ sleep }); + const relaunch = relaunchManagedSupervisorSession("alpha", { quiet: true, deps }); + + expect(relaunch?.finalize(true)).toMatchObject({ backupRemoved: true, rolledBack: false }); + expect(deps.finalize).toHaveBeenCalledWith(expect.objectContaining({ supervisorReady: true }), { + runOpenshell: deps.runOpenshell, + sleep, }); }); diff --git a/src/lib/actions/sandbox/supervisor-relaunch.ts b/src/lib/actions/sandbox/supervisor-relaunch.ts index ada035db3e6..cf71bf3bf68 100644 --- a/src/lib/actions/sandbox/supervisor-relaunch.ts +++ b/src/lib/actions/sandbox/supervisor-relaunch.ts @@ -13,6 +13,7 @@ import { type DockerGpuPatchFinalizeOutcome, finalizeDockerGpuPatchBackup, } from "../../onboard/docker-gpu-patch-finalize"; +import { getDockerGpuSupervisorReconnectTimeoutSecs } from "../../onboard/docker-gpu-supervisor-reconnect"; import { recreateOpenShellDockerSandboxWithStartupCommand } from "../../onboard/docker-startup-command-patch"; import { buildSandboxRuntimeEnvArgs } from "../../onboard/sandbox-create-launch"; import { resolveDirectSandboxContainer } from "../../sandbox/privileged-exec"; @@ -57,6 +58,7 @@ export type ManagedSupervisorRelaunchDeps = { removeBackup?: typeof sandboxState.removeSandboxStateBackup; recreate?: typeof recreateOpenShellDockerSandboxWithStartupCommand; finalize?: typeof finalizeDockerGpuPatchBackup; + runOpenshell?: NonNullable[1]>["runOpenshell"]; }; function inspectContainer(containerId: string): DockerContainerInspect { @@ -284,8 +286,22 @@ export function relaunchManagedSupervisorSession( // both succeed. return finalizeFailure(); } + const runLifecycleProbe = deps.runOpenshell; + if (!runLifecycleProbe) return finalizeFailure(); + const lifecycleDeps = { + runOpenshell: runLifecycleProbe, + ...(deps.sleep ? { sleep: deps.sleep } : {}), + }; const outcome = { - ...finalize({ result, supervisorReady: true }), + ...finalize( + { + result, + supervisorReady: true, + sandboxName, + lifecycleReleaseTimeoutSecs: getDockerGpuSupervisorReconnectTimeoutSecs(1), + }, + lifecycleDeps, + ), stateRestored: true, stateBackupRemoved: removeSettledStateBackup(), }; diff --git a/src/lib/onboard/docker-gpu-patch-finalize.test.ts b/src/lib/onboard/docker-gpu-patch-finalize.test.ts index 428611aaf38..5a9d1b78d51 100644 --- a/src/lib/onboard/docker-gpu-patch-finalize.test.ts +++ b/src/lib/onboard/docker-gpu-patch-finalize.test.ts @@ -68,14 +68,25 @@ describe("finalizeDockerGpuPatchBackup", () => { const dockerRm = vi.fn((_name: string) => ({ status: 0 })); const dockerStart = vi.fn(() => ({ status: 0 })); const outcome = finalizeDockerGpuPatchBackup( - { result: deferredCreateResult(), supervisorReady: true }, - { dockerStop, dockerRm, dockerStart }, + { + result: deferredCreateResult(), + supervisorReady: true, + sandboxName: "alpha", + lifecycleReleaseTimeoutSecs: 60, + }, + { + dockerStop, + dockerRm, + dockerStart, + runOpenshell: vi.fn(() => ({ status: 0, stdout: "No sandboxes found.\n" })), + }, ); expect(outcome).toEqual({ backupRemoved: true, rolledBack: false, replacementStoppedForCommit: true, replacementRestarted: true, + lifecycleReleaseObserved: true, }); expect(dockerStop).toHaveBeenCalledWith( "new-container-id", @@ -97,6 +108,124 @@ describe("finalizeDockerGpuPatchBackup", () => { ); }); + it("waits for the sandbox name to disappear before restarting the replacement (#9531)", () => { + const events: string[] = []; + const dockerStop = vi.fn(() => { + events.push("stop replacement"); + return { status: 0 }; + }); + const dockerRm = vi.fn(() => { + events.push("remove backup"); + return { status: 0 }; + }); + const dockerStart = vi.fn(() => { + events.push("start replacement"); + return { status: 0 }; + }); + const runOpenshell = vi + .fn() + .mockImplementationOnce(() => { + events.push("observe deleting"); + return { status: 0, stdout: "alpha 2026-08-21 05:53:16 Deleting\n" }; + }) + .mockImplementationOnce(() => { + events.push("observe error"); + return { status: 0, stdout: "alpha 2026-08-21 05:53:18 Error\n" }; + }) + .mockImplementationOnce(() => { + events.push("observe name absence"); + return { status: 0, stdout: "beta 2026-08-21 05:53:20 Ready\n" }; + }); + + const outcome = finalizeDockerGpuPatchBackup( + { + result: deferredCreateResult(), + supervisorReady: true, + sandboxName: "alpha", + lifecycleReleaseTimeoutSecs: 60, + }, + { dockerStop, dockerRm, dockerStart, runOpenshell, sleep: vi.fn() }, + ); + + expect(outcome).toMatchObject({ + backupRemoved: true, + lifecycleReleaseObserved: true, + replacementRestarted: true, + }); + expect(events).toEqual([ + "stop replacement", + "remove backup", + "observe deleting", + "observe error", + "observe name absence", + "start replacement", + ]); + }); + + it("does not treat failed lifecycle probes as a release receipt (#9531)", () => { + const runOpenshell = vi + .fn() + .mockReturnValueOnce({ status: 0, stdout: "Error: gateway unavailable" }) + .mockReturnValueOnce({ status: 1, stderr: "gateway unavailable" }); + const dockerStart = vi.fn(() => ({ status: 0 })); + + const outcome = finalizeDockerGpuPatchBackup( + { + result: deferredCreateResult(), + supervisorReady: true, + sandboxName: "alpha", + lifecycleReleaseTimeoutSecs: 1, + }, + { + dockerStop: vi.fn(() => ({ status: 0 })), + dockerRm: vi.fn(() => ({ status: 0 })), + dockerStart, + runOpenshell, + sleep: vi.fn(), + }, + ); + + expect(outcome).toMatchObject({ + backupRemoved: true, + lifecycleReleaseObserved: false, + replacementRestarted: false, + }); + expect(runOpenshell).toHaveBeenCalledTimes(2); + expect(runOpenshell.mock.calls[0]?.[1]?.timeout).toBeGreaterThan(0); + expect(runOpenshell.mock.calls[0]?.[1]?.timeout).toBeLessThanOrEqual(1000); + expect(runOpenshell.mock.calls[1]?.[1]?.timeout).toBeGreaterThan(0); + expect(runOpenshell.mock.calls[1]?.[1]?.timeout).toBeLessThanOrEqual(1000); + expect(dockerStart).not.toHaveBeenCalled(); + }); + + it("does not treat an unrelated terminal lifecycle phase as the stopped replacement (#9531)", () => { + const runOpenshell = vi.fn(() => ({ + status: 0, + stdout: "alpha 2026-08-21 05:53:18 Failed\n", + })); + + const dockerStart = vi.fn(() => ({ status: 0 })); + const outcome = finalizeDockerGpuPatchBackup( + { + result: deferredCreateResult(), + supervisorReady: true, + sandboxName: "alpha", + lifecycleReleaseTimeoutSecs: 1, + }, + { + dockerStop: vi.fn(() => ({ status: 0 })), + dockerRm: vi.fn(() => ({ status: 0 })), + dockerStart, + runOpenshell, + sleep: vi.fn(), + }, + ); + + expect(outcome.lifecycleReleaseObserved).toBe(false); + expect(runOpenshell).toHaveBeenCalledTimes(2); + expect(dockerStart).not.toHaveBeenCalled(); + }); + it("rolls back to the backup container when supervisor reconnect failed", () => { const dockerStop = vi.fn(() => ({ status: 0 })); const dockerRm = vi.fn((_name: string) => ({ status: 0 })); @@ -186,7 +315,15 @@ describe("finalizeDockerGpuPatchBackup", () => { it("is a no-op when the backup was already removed by the patch helper", () => { const dockerRm = vi.fn((_name: string) => ({ status: 0 })); const result = { ...deferredCreateResult(), backupRemoved: true }; - const outcome = finalizeDockerGpuPatchBackup({ result, supervisorReady: true }, { dockerRm }); + const outcome = finalizeDockerGpuPatchBackup( + { + result, + supervisorReady: true, + sandboxName: "alpha", + lifecycleReleaseTimeoutSecs: 60, + }, + { dockerRm }, + ); expect(outcome).toEqual({ backupRemoved: true, rolledBack: false }); expect(dockerRm).not.toHaveBeenCalled(); }); @@ -199,19 +336,26 @@ describe("finalizeDockerGpuPatchBackup", () => { })); const dockerStart = vi.fn(() => ({ status: 0 })); const outcome = finalizeDockerGpuPatchBackup( - { result: deferredCreateResult(), supervisorReady: true }, + { + result: deferredCreateResult(), + supervisorReady: true, + sandboxName: "alpha", + lifecycleReleaseTimeoutSecs: 60, + }, { dockerStop, dockerRm, dockerStart }, ); expect(outcome).toEqual({ backupRemoved: false, rolledBack: false, replacementStoppedForCommit: true, - replacementRestarted: true, + replacementRestarted: false, + lifecycleReleaseObserved: false, }); expect(dockerRm).toHaveBeenCalledWith( "openshell-alpha-nemoclaw-gpu-backup-1780491860342", expect.objectContaining({ ignoreError: true }), ); + expect(dockerStart).not.toHaveBeenCalled(); }); it("fails closed when backup removal has no exit status", () => { @@ -219,15 +363,22 @@ describe("finalizeDockerGpuPatchBackup", () => { const dockerRm = vi.fn((_name: string) => ({ status: null, stderr: "timed out" })); const dockerStart = vi.fn(() => ({ status: 0 })); const outcome = finalizeDockerGpuPatchBackup( - { result: deferredCreateResult(), supervisorReady: true }, + { + result: deferredCreateResult(), + supervisorReady: true, + sandboxName: "alpha", + lifecycleReleaseTimeoutSecs: 60, + }, { dockerStop, dockerRm, dockerStart }, ); expect(outcome).toEqual({ backupRemoved: false, rolledBack: false, replacementStoppedForCommit: true, - replacementRestarted: true, + replacementRestarted: false, + lifecycleReleaseObserved: false, }); + expect(dockerStart).not.toHaveBeenCalled(); }); it("retains the backup when the replacement cannot be stopped for the final handoff", () => { @@ -236,7 +387,12 @@ describe("finalizeDockerGpuPatchBackup", () => { const dockerStart = vi.fn(() => ({ status: 0 })); const outcome = finalizeDockerGpuPatchBackup( - { result: deferredCreateResult(), supervisorReady: true }, + { + result: deferredCreateResult(), + supervisorReady: true, + sandboxName: "alpha", + lifecycleReleaseTimeoutSecs: 60, + }, { dockerStop, dockerRm, dockerStart }, ); @@ -251,11 +407,17 @@ describe("finalizeDockerGpuPatchBackup", () => { it("reports a failed replacement restart after the backup is removed", () => { const outcome = finalizeDockerGpuPatchBackup( - { result: deferredCreateResult(), supervisorReady: true }, + { + result: deferredCreateResult(), + supervisorReady: true, + sandboxName: "alpha", + lifecycleReleaseTimeoutSecs: 60, + }, { dockerStop: vi.fn(() => ({ status: 0 })), dockerRm: vi.fn(() => ({ status: 0 })), dockerStart: vi.fn(() => ({ status: 1 })), + runOpenshell: vi.fn(() => ({ status: 0, stdout: "No sandboxes found.\n" })), }, ); @@ -264,6 +426,7 @@ describe("finalizeDockerGpuPatchBackup", () => { rolledBack: false, replacementStoppedForCommit: true, replacementRestarted: false, + lifecycleReleaseObserved: true, }); }); diff --git a/src/lib/onboard/docker-gpu-patch-finalize.ts b/src/lib/onboard/docker-gpu-patch-finalize.ts index 26b8b7c0ea1..75c9d90d43a 100644 --- a/src/lib/onboard/docker-gpu-patch-finalize.ts +++ b/src/lib/onboard/docker-gpu-patch-finalize.ts @@ -30,22 +30,31 @@ import { rollbackToBackupContainer, } from "./docker-gpu-patch-rollback"; import type { DockerGpuPatchDeps, DockerGpuPatchResult } from "./docker-gpu-patch-types"; +import { waitForOpenShellSandboxLifecycleRelease } from "./docker-gpu-supervisor-reconnect"; export { restoreDockerGpuPatchBackupAfterRecreateFailure as rollbackDockerGpuPatchOnRecreateFailure, rollbackToBackupContainer, } from "./docker-gpu-patch-rollback"; -export type DockerGpuPatchFinalizeOptions = { - result: DockerGpuPatchResult; - supervisorReady: boolean; -}; +export type DockerGpuPatchFinalizeOptions = + | { + result: DockerGpuPatchResult; + supervisorReady: false; + } + | { + result: DockerGpuPatchResult; + supervisorReady: true; + sandboxName: string; + lifecycleReleaseTimeoutSecs: number; + }; export type DockerGpuPatchFinalizeOutcome = { backupRemoved: boolean; rolledBack: boolean; replacementStoppedForCommit?: boolean; replacementRestarted?: boolean; + lifecycleReleaseObserved?: boolean; replacementStopConfirmed?: boolean; replacementRemovalConfirmed?: boolean; replacementPresence?: "absent" | "present" | "unknown"; @@ -81,12 +90,37 @@ export function finalizeDockerGpuPatchBackup( } const rmResult = resolved.dockerRm(options.result.backupContainerName, containerOpts); const backupRemoved = hasZeroDockerExitStatus(rmResult); + const sandboxName = options.sandboxName; + const lifecycleReleaseTimeoutSecs = options.lifecycleReleaseTimeoutSecs; + const hasLifecycleContext = + sandboxName.length > 0 && + Number.isFinite(lifecycleReleaseTimeoutSecs) && + lifecycleReleaseTimeoutSecs > 0; + if (backupRemoved && hasLifecycleContext) { + console.log( + ` Waiting for OpenShell to retire the previous lifecycle record before restarting the replacement (up to ${lifecycleReleaseTimeoutSecs}s)...`, + ); + } + const lifecycleReleaseObserved = + backupRemoved && hasLifecycleContext + ? waitForOpenShellSandboxLifecycleRelease(sandboxName, lifecycleReleaseTimeoutSecs, deps) + : false; + if (!lifecycleReleaseObserved) { + return { + backupRemoved, + rolledBack: false, + replacementStoppedForCommit: true, + replacementRestarted: false, + lifecycleReleaseObserved: false, + }; + } const startResult = resolved.dockerStart(options.result.newContainerId, containerOpts); return { backupRemoved, rolledBack: false, replacementStoppedForCommit: true, replacementRestarted: hasZeroDockerExitStatus(startResult), + lifecycleReleaseObserved: true, }; } const rollback = rollbackToBackupContainer( diff --git a/src/lib/onboard/docker-gpu-sandbox-create-lifecycle.test.ts b/src/lib/onboard/docker-gpu-sandbox-create-lifecycle.test.ts index 1f1bcd465d5..36bc94980ca 100644 --- a/src/lib/onboard/docker-gpu-sandbox-create-lifecycle.test.ts +++ b/src/lib/onboard/docker-gpu-sandbox-create-lifecycle.test.ts @@ -50,6 +50,7 @@ describe("createDockerGpuSandboxCreatePatch composed flow", () => { const finalizeBackup = vi.fn(() => ({ backupRemoved: true, rolledBack: false, + lifecycleReleaseObserved: true, replacementRestarted: true, })); const capturePreRollbackDiagnostics = vi.fn(() => null); @@ -91,7 +92,15 @@ describe("createDockerGpuSandboxCreatePatch composed flow", () => { await patch.commitAfterReady(); expect(finalizeBackup).toHaveBeenCalledTimes(1); - expect(finalizeBackup).toHaveBeenCalledWith({ result, supervisorReady: true }, deps); + expect(finalizeBackup).toHaveBeenCalledWith( + { + result, + supervisorReady: true, + sandboxName: "alpha", + lifecycleReleaseTimeoutSecs: 900, + }, + deps, + ); expect(waitForSupervisor).toHaveBeenCalledTimes(2); expect(capturePreRollbackDiagnostics).not.toHaveBeenCalled(); expect(onPatchFailureExit).not.toHaveBeenCalled(); @@ -124,7 +133,15 @@ describe("createDockerGpuSandboxCreatePatch composed flow", () => { patch.waitForSupervisorReconnectIfNeeded(); await expect(patch.commitAfterReady()).resolves.toBeUndefined(); - expect(finalizeBackup).toHaveBeenCalledWith({ result, supervisorReady: true }, deps); + expect(finalizeBackup).toHaveBeenCalledWith( + { + result, + supervisorReady: true, + sandboxName: "alpha", + lifecycleReleaseTimeoutSecs: 900, + }, + deps, + ); expect(waitForSupervisor).toHaveBeenCalledTimes(1); expect(onPatchFailureExit).not.toHaveBeenCalled(); }); @@ -157,6 +174,38 @@ describe("createDockerGpuSandboxCreatePatch composed flow", () => { expect(onPatchFailureExit).toHaveBeenCalledOnce(); }); + it("rejects final handoff when OpenShell never releases the deleting lifecycle record (#9531)", async () => { + const deps = makeDeps(); + const result = deferredCreateResult(); + const waitForSupervisor = vi.fn(() => true); + const onPatchFailureExit = vi.fn(); + const patch = createDockerGpuSandboxCreatePatch({ + route: "compatibility", + sandboxName: "alpha", + timeoutSecs: 60, + deps, + overrides: { + findContainerIds: vi.fn(() => ["existing-container"]), + recreatePatch: vi.fn(() => result), + waitForSupervisor, + finalizeBackup: vi.fn(() => ({ + backupRemoved: true, + rolledBack: false, + lifecycleReleaseObserved: false, + replacementRestarted: true, + })), + onPatchFailureExit, + }, + }); + + patch.maybeApplyDuringCreate(); + patch.waitForSupervisorReconnectIfNeeded(); + await expect(patch.commitAfterReady()).rejects.toThrow("final runtime handoff"); + + expect(waitForSupervisor).toHaveBeenCalledOnce(); + expect(onPatchFailureExit).toHaveBeenCalledOnce(); + }); + it("reports a failed post-Ready rollback instead of treating it as restored", async () => { const deps = makeDeps(); const result = deferredCreateResult(); diff --git a/src/lib/onboard/docker-gpu-sandbox-create.ts b/src/lib/onboard/docker-gpu-sandbox-create.ts index 14d333e1269..cc4983bb01a 100644 --- a/src/lib/onboard/docker-gpu-sandbox-create.ts +++ b/src/lib/onboard/docker-gpu-sandbox-create.ts @@ -468,23 +468,41 @@ export function createDockerGpuSandboxCreatePatch( throw failure; } } + const supervisorReconnectTimeoutSecs = getDockerGpuSupervisorReconnectTimeoutSecs( + options.timeoutSecs, + ); + const finalHandoffDeadlineMs = Date.now() + supervisorReconnectTimeoutSecs * 1000; const finalizeOutcome = result - ? finalizeBackup({ result, supervisorReady: true }, options.deps) + ? finalizeBackup( + { + result, + supervisorReady: true, + sandboxName: options.sandboxName, + lifecycleReleaseTimeoutSecs: supervisorReconnectTimeoutSecs, + }, + options.deps, + ) : null; cutoverFinalized = true; if (!finalizeOutcome) return; if (finalizeOutcome.backupRemoved && finalizeOutcome.replacementRestarted === undefined) { return; } - if (finalizeOutcome.backupRemoved && finalizeOutcome.replacementRestarted) { - const supervisorReconnectTimeoutSecs = getDockerGpuSupervisorReconnectTimeoutSecs( - options.timeoutSecs, + if ( + finalizeOutcome.backupRemoved && + finalizeOutcome.replacementRestarted && + finalizeOutcome.lifecycleReleaseObserved === true + ) { + const remainingReconnectTimeoutSecs = Math.max( + 0, + Math.ceil((finalHandoffDeadlineMs - Date.now()) / 1000), ); console.log( - ` Waiting for OpenShell supervisor to confirm the final container handoff (up to ${supervisorReconnectTimeoutSecs}s)...`, + ` Waiting for OpenShell supervisor to confirm the final container handoff (up to ${remainingReconnectTimeoutSecs}s)...`, ); if ( - waitForSupervisor(options.sandboxName, supervisorReconnectTimeoutSecs, { + remainingReconnectTimeoutSecs > 0 && + waitForSupervisor(options.sandboxName, remainingReconnectTimeoutSecs, { runOpenshell: options.deps.runOpenshell, runCaptureOpenshell: options.deps.runCaptureOpenshell, sleep: options.deps.sleep, diff --git a/src/lib/onboard/docker-gpu-supervisor-reconnect.test.ts b/src/lib/onboard/docker-gpu-supervisor-reconnect.test.ts index 7f77a5de8fb..b8c1282c261 100644 --- a/src/lib/onboard/docker-gpu-supervisor-reconnect.test.ts +++ b/src/lib/onboard/docker-gpu-supervisor-reconnect.test.ts @@ -6,9 +6,64 @@ import { describe, expect, it, vi } from "vitest"; import { getDockerGpuSupervisorReconnectErrorDebouncePolls, getDockerGpuSupervisorReconnectTimeoutSecs, + waitForOpenShellSandboxLifecycleRelease, waitForOpenShellSupervisorReconnect, } from "./docker-gpu-supervisor-reconnect"; +describe("Docker GPU final lifecycle release", () => { + it.each([ + ["an explicit empty list", "No sandboxes found.\n"], + ["another phase-bearing sandbox", "beta 2026-08-21 05:53:18 Ready\n"], + ])("accepts %s as a release receipt (#9531)", (_receipt, stdout) => { + const runOpenshell = vi.fn(() => ({ status: 0, stdout })); + + expect( + waitForOpenShellSandboxLifecycleRelease("alpha", 1, { + runOpenshell, + sleep: vi.fn(), + }), + ).toBe(true); + expect(runOpenshell).toHaveBeenCalledOnce(); + }); + + it.each([ + ["a header", "NAME CREATED PHASE\n"], + ["a gateway error", "Error: gateway unavailable\n"], + ["a phase-free row", "beta 2026-08-21 05:53:18\n"], + ["an unrecognized phase", "beta 2026-08-21 05:53:18 Retiring\n"], + ["the selected sandbox in Deleting", "alpha 2026-08-21 05:53:18 Deleting\n"], + ["the selected sandbox in Ready", "alpha 2026-08-21 05:53:18 Ready\n"], + ["the selected sandbox in Provisioning", "alpha 2026-08-21 05:53:18 Provisioning\n"], + ["the selected sandbox in Error", "alpha 2026-08-21 05:53:18 Error\n"], + ["the selected sandbox in Failed", "alpha 2026-08-21 05:53:18 Failed\n"], + ])("rejects %s as a release receipt (#9531)", (_case, stdout) => { + const runOpenshell = vi.fn(() => ({ status: 0, stdout })); + + expect( + waitForOpenShellSandboxLifecycleRelease("alpha", 1, { + runOpenshell, + sleep: vi.fn(), + }), + ).toBe(false); + expect(runOpenshell).toHaveBeenCalledTimes(2); + }); + + it.each([ + ["a failed probe", { status: 1, stderr: "gateway unavailable" }], + ["a probe without an exit status", { status: null, stderr: "timed out" }], + ])("rejects %s as a release receipt (#9531)", (_case, result) => { + const runOpenshell = vi.fn(() => result); + + expect( + waitForOpenShellSandboxLifecycleRelease("alpha", 1, { + runOpenshell, + sleep: vi.fn(), + }), + ).toBe(false); + expect(runOpenshell).toHaveBeenCalledTimes(2); + }); +}); + // The Docker GPU patch supervisor-reconnect wait must absorb a transient // Error phase reported while OpenShell's sandbox-list cache catches up to // the newly-recreated GPU container. The old-container teardown briefly diff --git a/src/lib/onboard/docker-gpu-supervisor-reconnect.ts b/src/lib/onboard/docker-gpu-supervisor-reconnect.ts index 9db684642c1..911d352d245 100644 --- a/src/lib/onboard/docker-gpu-supervisor-reconnect.ts +++ b/src/lib/onboard/docker-gpu-supervisor-reconnect.ts @@ -23,6 +23,7 @@ * recovers to Ready is the runtime evidence required. */ +import { parseLiveSandboxEntries } from "../runtime-recovery"; import { hasZeroDockerExitStatus } from "./docker-command-result"; import { DOCKER_GPU_PATCH_TIMEOUT_MS } from "./docker-gpu-patch-constants"; import { envInt } from "./env"; @@ -72,6 +73,63 @@ export type DockerGpuSupervisorReconnectDeps = { errorPhaseDebouncePolls?: number; }; +/** + * Workaround contract for the OpenShell lifecycle race in #9531: + * + * - Removing the rollback backup can strand the exact sandbox in `Deleting` + * while its replacement container is healthy. + * - `openshell sandbox list` owns lifecycle authority. Docker health cannot + * prove that OpenShell retired the previous record. + * - This layer waits after backup removal and before replacement restart so + * OpenShell processes the stale deletion before the new registration. + * - The caller enters this wait only after the replacement reached Ready and + * was deliberately stopped. A successful list must omit the sandbox name; + * a name-and-phase row cannot identify which container owns that lifecycle. + * - `waits for the sandbox name to disappear before restarting the + * replacement (#9531)` protects the event order. `rejects final handoff when + * OpenShell never releases the deleting lifecycle record (#9531)` protects + * the composed failure path. + * + * Remove this wait only when OpenShell binds deletion to the removed container + * identity or provides an identity-bound lifecycle-release receipt. + */ +export function waitForOpenShellSandboxLifecycleRelease( + sandboxName: string, + timeoutSecs: number, + deps: Pick, +): boolean { + if (!deps.runOpenshell) return false; + const sleep = deps.sleep ?? defaultSleep; + const boundedTimeoutSecs = Math.max(1, Math.round(timeoutSecs)); + const deadline = Date.now() + boundedTimeoutSecs * 1000; + const maxAttempts = Math.max(1, Math.ceil(boundedTimeoutSecs / 2) + 1); + + for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { + const remainingBeforeProbeMs = deadline - Date.now(); + if (remainingBeforeProbeMs <= 0) break; + const result = deps.runOpenshell(["sandbox", "list"], { + ignoreError: true, + suppressOutput: true, + timeout: Math.min(DOCKER_GPU_PATCH_TIMEOUT_MS, remainingBeforeProbeMs), + }); + if (hasZeroDockerExitStatus(result)) { + const output = String(result.stdout ?? "").trim(); + const entries = parseLiveSandboxEntries(output); + const sandboxPresent = entries.some((entry) => entry.name === sandboxName); + const hasPhaseBearingEntry = entries.some((entry) => entry.phase !== null); + const explicitEmptyList = output === "No sandboxes found" || output === "No sandboxes found."; + if (explicitEmptyList || (hasPhaseBearingEntry && !sandboxPresent)) { + return true; + } + } + const remainingBeforeSleepMs = deadline - Date.now(); + if (attempt < maxAttempts && remainingBeforeSleepMs > 0) { + sleep(Math.min(2, remainingBeforeSleepMs / 1000)); + } + } + return false; +} + function defaultSleep(seconds: number): void { Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, Math.max(0, seconds) * 1000); } diff --git a/test/onboard-messaging.test.ts b/test/onboard-messaging.test.ts index 042340d91c6..f5fa6e03269 100644 --- a/test/onboard-messaging.test.ts +++ b/test/onboard-messaging.test.ts @@ -87,7 +87,7 @@ const { EventEmitter } = require("node:events"); const fs = require("node:fs"); const commands = []; runner.run = (command, opts = {}) => { - commands.push({ command: _n(command), env: opts.env || null }); + commands.push({ command: _n(command), env: opts.env || null }); if (_n(command).includes("sandbox list")) return { status: 0, stdout: "No sandboxes found." }; // provider-get returns not-found so messaging providers are created fresh if (_n(command).includes("provider get")) return { status: 1 }; return _n(command).includes("sandbox get") && _n(command).includes("my-assistant") ? { status: 0, stdout: Buffer.from("Name: my-assistant\nId: sbx-4f2a91c0d7\n"), stderr: Buffer.alloc(0) } : { status: 0 }; @@ -366,7 +366,7 @@ const commands = []; let registeredSandbox = null; runner.run = (command, opts = {}) => { const normalized = _n(command); - commands.push({ command: normalized, env: opts.env || null }); + commands.push({ command: normalized, env: opts.env || null }); if (normalized.includes("sandbox list")) return { status: 0, stdout: "No sandboxes found." }; if (normalized.includes("provider get")) return { status: 1 }; return normalized.includes("sandbox get") && normalized.includes("my-assistant") ? { status: 0, stdout: Buffer.from("Name: my-assistant\nId: sbx-4f2a91c0d7\n"), stderr: Buffer.alloc(0) } : { status: 0 }; }; @@ -536,7 +536,7 @@ registry.registerSandbox({ name: "my-assistant", messaging: { schemaVersion: 1, registry.addExtraProvider("my-assistant-extra-telegram-bot-token-agent-a"); registry.addExtraProvider("my-assistant-extra-telegram-bot-token-agent-b"); runner.run = (command) => { const normalized = _n(command); - commands.push({ command: normalized }); + commands.push({ command: normalized }); if (normalized.includes("sandbox list")) return { status: 0, stdout: "No sandboxes found." }; const providerGet = normalized.match(/provider get -g nemoclaw ([^ ]+)$/)?.[1]; if (providerGet === process.env.NEMOCLAW_TEST_FAIL_PROVIDER) return { status: 2, stderr: "transport unavailable" }; if (providerGet && revisions.has(providerGet)) return { status: 0, stdout: "Name: " + providerGet + "\nType: " + (providerGet === "compatible-endpoint" ? "openai" : "generic") + "\nCredential keys: " + credentialKeys[providerGet] + "\nConfig keys: " + (providerGet === "compatible-endpoint" ? "OPENAI_BASE_URL" : "") + "\n" }; const refresh = normalized.match(/provider update -g nemoclaw ([^ ]+)$/)?.[1]; @@ -706,7 +706,7 @@ registry.registerSandbox({ }); runner.run = (command, opts = {}) => { const normalized = _n(command); - commands.push({ command: normalized, env: opts.env || null }); + commands.push({ command: normalized, env: opts.env || null }); if (normalized.includes("sandbox list")) return { status: 0, stdout: "No sandboxes found." }; if (normalized.includes("provider get -g nemoclaw my-assistant-telegram-bridge")) return { status: 0, stdout: "Name: my-assistant-telegram-bridge\nType: generic\nCredential keys: TELEGRAM_BOT_TOKEN\nConfig keys: \n" }; if (normalized.includes("provider get")) return { status: 1 }; return normalized.includes("sandbox get") && normalized.includes("my-assistant") ? { status: 0, stdout: Buffer.from("Name: my-assistant\nId: sbx-4f2a91c0d7\n"), stderr: Buffer.alloc(0) } : { status: 0 }; @@ -859,7 +859,7 @@ const commands = []; let dockerfileContent; const registerCalls = []; runner.run = (command, opts = {}) => { const normalized = _n(command); - commands.push({ command: normalized, env: opts.env || null }); + commands.push({ command: normalized, env: opts.env || null }); if (normalized.includes("sandbox list")) return { status: 0, stdout: "No sandboxes found." }; if (normalized.includes("provider get")) return { status: 1 }; return normalized.includes("sandbox get") && normalized.includes("my-assistant") ? { status: 0, stdout: Buffer.from("Name: my-assistant\nId: sbx-4f2a91c0d7\n"), stderr: Buffer.alloc(0) } : { status: 0 }; }; @@ -1020,7 +1020,7 @@ const commands = []; let dockerfileContent; const registerCalls = []; runner.run = (command, opts = {}) => { const normalized = _n(command); - commands.push({ command: normalized, env: opts.env || null }); + commands.push({ command: normalized, env: opts.env || null }); if (normalized.includes("sandbox list")) return { status: 0, stdout: "No sandboxes found." }; if (normalized.includes("provider get")) return { status: 1 }; return normalized.includes("sandbox get") && normalized.includes("my-assistant") ? { status: 0, stdout: Buffer.from("Name: my-assistant\nId: sbx-4f2a91c0d7\n"), stderr: Buffer.alloc(0) } : { status: 0 }; }; @@ -1348,7 +1348,7 @@ const { EventEmitter } = require("node:events"); const commands = []; runner.run = (command, opts = {}) => { - commands.push({ command: _n(command), env: opts.env || null }); + commands.push({ command: _n(command), env: opts.env || null }); if (_n(command).includes("sandbox list")) return { status: 0, stdout: "No sandboxes found." }; // provider-get returns not-found so messaging providers are created fresh if (_n(command).includes("provider get")) return { status: 1 }; return _n(command).includes("sandbox get") && _n(command).includes("my-assistant") ? { status: 0, stdout: Buffer.from("Name: my-assistant\nId: sbx-4f2a91c0d7\n"), stderr: Buffer.alloc(0) } : { status: 0 }; @@ -1485,7 +1485,7 @@ const { EventEmitter } = require("node:events"); const commands = []; runner.run = (command, opts = {}) => { - commands.push({ command: _n(command), env: opts.env || null }); + commands.push({ command: _n(command), env: opts.env || null }); if (_n(command).includes("sandbox list")) return { status: 0, stdout: "No sandboxes found." }; return _n(command).includes("sandbox get") && _n(command).includes("my-assistant") ? { status: 0, stdout: Buffer.from("Name: my-assistant\nId: sbx-4f2a91c0d7\n"), stderr: Buffer.alloc(0) } : { status: 0 }; }; runner.runCapture = (command) => { diff --git a/test/process-recovery-supervisor-relaunch.test.ts b/test/process-recovery-supervisor-relaunch.test.ts index 59ae878a3ef..07b6697fc5b 100644 --- a/test/process-recovery-supervisor-relaunch.test.ts +++ b/test/process-recovery-supervisor-relaunch.test.ts @@ -77,6 +77,7 @@ function composedRelaunchTransaction( .fn() .mockReturnValueOnce("old-container-id") .mockReturnValue("replacement-container-id"); + const runOpenshell = vi.fn(() => ({ status: 0, stdout: "No sandboxes found.\n" })); const relaunchManagedSupervisorSessionImpl = vi.fn( (sandboxName: string, options: Parameters[1]) => relaunchManagedSupervisorSession(sandboxName, { @@ -109,6 +110,7 @@ function composedRelaunchTransaction( }; }), removeBackup: vi.fn(() => true), + runOpenshell, recreate: vi.fn(() => ({ applied: true as const, oldContainerId: "old-container-id", @@ -127,7 +129,7 @@ function composedRelaunchTransaction( }, }), ); - return { finalizeTransaction, relaunchManagedSupervisorSessionImpl }; + return { finalizeTransaction, relaunchManagedSupervisorSessionImpl, runOpenshell }; } function scriptedPinnedGatewayRecovery( @@ -401,7 +403,7 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { mockOpenClawSandbox("recovered-box"); setImmediateRecoveryPolling(); const order: string[] = []; - const { finalizeTransaction, relaunchManagedSupervisorSessionImpl } = + const { finalizeTransaction, relaunchManagedSupervisorSessionImpl, runOpenshell } = composedRelaunchTransaction(order); const requestGatewaySupervisorAction = vi.fn((_name: string, action: string) => action === "recover" ? { status: 1, stdout: "", stderr: "SUPERVISOR_NOT_RUNNING" } : null, @@ -451,7 +453,12 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { expect(order).toEqual(["restore-state", "post-restore-restart", "commit-container"]); expect(finalizeTransaction).toHaveBeenCalledOnce(); expect(finalizeTransaction).toHaveBeenCalledWith( - expect.objectContaining({ supervisorReady: true }), + expect.objectContaining({ + lifecycleReleaseTimeoutSecs: 900, + sandboxName: "recovered-box", + supervisorReady: true, + }), + { runOpenshell }, ); }); @@ -464,8 +471,10 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { const dockerRm = vi.fn(() => ({ status: 0 })); const dockerStart = vi.fn(() => ({ status: 0 })); const finalizeTransaction = vi.fn( - (options: Parameters[0]) => - finalizeDockerGpuPatchBackup(options, { dockerStop, dockerRm, dockerStart }), + ( + options: Parameters[0], + deps: Parameters[1], + ) => finalizeDockerGpuPatchBackup(options, { ...deps, dockerStop, dockerRm, dockerStart }), ); const { relaunchManagedSupervisorSessionImpl } = composedRelaunchTransaction( order, @@ -569,6 +578,21 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { finalPinnedAction: () => ACCEPTED_MANAGED_PROBE, finalReadinessReady: true, }, + { + condition: "OpenShell does not release the sandbox name", + finalizeOutcome: () => ({ + backupRemoved: true, + lifecycleReleaseObserved: false, + replacementRestarted: false, + replacementStoppedForCommit: true, + rolledBack: false, + stateRestored: true, + }), + expectedDetail: "OpenShell did not release the sandbox name", + expectedReadinessCalls: 1, + finalPinnedAction: () => ACCEPTED_MANAGED_PROBE, + finalReadinessReady: true, + }, { condition: "Docker cannot start the replacement container", finalizeOutcome: () => ({