diff --git a/docs/inference/configure-inference-timeouts.mdx b/docs/inference/configure-inference-timeouts.mdx index 4f9ece74ce3..643b65efc17 100644 --- a/docs/inference/configure-inference-timeouts.mdx +++ b/docs/inference/configure-inference-timeouts.mdx @@ -20,7 +20,7 @@ Use the error location to select the correct setting. |---|---|---| | `NEMOCLAW_AGENT_TIMEOUT` | OpenClaw per-request inference | `600` seconds | | `NEMOCLAW_LOCAL_INFERENCE_TIMEOUT` | Ollama, vLLM, NIM, and compatible-endpoint validation during onboarding | `180` seconds | -| `NEMOCLAW_SANDBOX_READY_TIMEOUT` | Image build, gateway upload, and in-sandbox boot after sandbox creation | `180` seconds | +| `NEMOCLAW_SANDBOX_READY_TIMEOUT` | Image build, gateway upload, and in-sandbox boot after creation; OpenShell command re-registration after policy application or after OpenClaw or Hermes managed recovery recreates the sandbox | `180` seconds | The readiness timeout does not govern inference requests or provider validation. @@ -64,12 +64,25 @@ This variable does not extend the later sandbox-readiness wait. Raise `NEMOCLAW_SANDBOX_READY_TIMEOUT` when onboarding creates the sandbox but image build, upload, or boot exceeds 180 seconds. This can occur during a first run with cold caches or on a remote VM over a slow link. +Onboarding also uses this budget to confirm that the sandbox can execute commands again after applying policy presets. + + + +The same budget applies when `start` or `recover` transactionally recreates a managed sandbox and waits for OpenShell to re-register it. + + ```bash export NEMOCLAW_SANDBOX_READY_TIMEOUT=600 $$nemoclaw onboard ``` + + +For an existing sandbox, export the variable before the `start` or `recover` command that performs the recreation. + + + Raise both onboarding budgets when the provider probe and the later sandbox creation phase are slow. ```bash diff --git a/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx b/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx index 8266cbc9c11..841a53ec90d 100644 --- a/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx +++ b/docs/manage-sandboxes/recover-rebuild-sandboxes.mdx @@ -84,6 +84,8 @@ For a local Docker-driver sandbox whose container still uses the legacy keepaliv NemoClaw keeps the previous container available until the managed controller proves the supervisor topology, gateway health, and settle check, and attempts to restore it if that proof fails. The recreation preserves mounted sandbox state, but a committed swap does not retain changes stored only in the previous container's writable layer. +After a transactional recreation, NemoClaw uses the `NEMOCLAW_SANDBOX_READY_TIMEOUT` budget (180 seconds by default) for OpenShell to re-register the sandbox before starting the primary dashboard or API host forward. +A definitive managed-health failure still stops immediately; if re-registration does not complete within the budget, the forward stays stopped. 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/docs/manage-sandboxes/run-sandboxes.mdx b/docs/manage-sandboxes/run-sandboxes.mdx index 64ba68e29a2..1c4b4724335 100644 --- a/docs/manage-sandboxes/run-sandboxes.mdx +++ b/docs/manage-sandboxes/run-sandboxes.mdx @@ -118,8 +118,10 @@ Stop a sandbox's container to free CPU, memory, and GPU resources without losing $$nemoclaw stop ``` -Workspace files, credentials, network policies, and the registry entry are preserved; only the container stops running. +Workspace files, credentials, network policies, and the registry entry are preserved. +The container stops running. +After the container stops, NemoClaw attempts to stop that sandbox's host dashboard forward. The shared host gateway and tunnel services keep serving other sandboxes. diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 95703aa0e84..ed7eaf614e5 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1186,7 +1186,7 @@ Terminal agents do not have a gateway runtime and fail as unsupported. ### `$$nemoclaw stop` Stop the sandbox's Docker container while preserving all of its state. -Workspace files, credentials, network policies, the registry entry, and the OpenShell sandbox record stay in place; only the container stops running. +Workspace files, credentials, network policies, the registry entry, and the OpenShell sandbox record stay in place. Use this to free CPU, memory, and GPU resources without destroying the sandbox; use [`$$nemoclaw destroy`](#$$nemoclaw-name-destroy) when you want to delete it instead. ```bash @@ -1195,8 +1195,17 @@ $$nemoclaw my-assistant stop For OpenClaw-managed gateways, the command first asks the in-sandbox gateway to shut down its channels gracefully; agent-managed gateways (for example Hermes) are supervised inside the sandbox and shut down with the container's stop signal. Then the container stops; a container stuck in a crash loop is stopped the same way, which also disarms its restart policy. + +After the container stops, NemoClaw attempts to stop that sandbox's host dashboard forward. +If the container does not stop, NemoClaw leaves the dashboard forward running. + The shared host gateway, tunnel services, and any local NIM inference container serve other sandboxes and keep running. + +Stopping an already-stopped sandbox succeeds and attempts to remove any leftover dashboard forward for that sandbox. + + Stopping an already-stopped sandbox succeeds without changes. + The command controls the local container directly, so it is available for local-container drivers (the default Docker driver and the vm driver) and unavailable for remote drivers such as kubernetes; if the Docker daemon itself is unreachable, the command reports the outage instead of guessing at container state. ### `$$nemoclaw start` @@ -3663,27 +3672,40 @@ Defaults are sized for typical hardware; override only if you see false-positive | `NEMOCLAW_SANDBOX_EXEC_TIMEOUT_MS` | per call site (typically `15000`) | Overrides the default timeout for `openshell sandbox exec` calls issued by recovery and lifecycle helpers. Integer milliseconds; non-positive or non-numeric values fall back to the per-call-site default. | | `NEMOCLAW_STATUS_PROBE_TIMEOUT_MS` | built-in default | Overrides the timeout for the OpenShell status probe used by `$$nemoclaw status`. Integer milliseconds; non-positive or non-numeric values fall back to the default. | -### Onboard Timeouts +### Onboard and Sandbox Readiness Timeouts The following environment variables tune onboard-time wall-clock limits. +`NEMOCLAW_SANDBOX_READY_TIMEOUT` also covers OpenShell command re-registration after onboarding applies policy presets. + + +`NEMOCLAW_SANDBOX_READY_TIMEOUT` also applies when managed recovery transactionally recreates an existing sandbox. + Set them before running `$$nemoclaw onboard` if a slow connection or large model pull risks tripping the default. | Variable | Default | Purpose | |----------|---------|---------| | `NEMOCLAW_OLLAMA_PULL_TIMEOUT` | `1800` (30 minutes) | Wall-clock timeout for `ollama pull` during onboard, in seconds. Accepts integer or float values. Already-downloaded layers are kept; re-running the pull resumes them. | | `NEMOCLAW_LOCAL_INFERENCE_TIMEOUT` | `180` | Wall-clock timeout for the inference-server validation probe during onboard, in seconds. Raise on slow networks or for very large prompts. | -| `NEMOCLAW_SANDBOX_READY_TIMEOUT` | `180` | Wall-clock timeout for the post-create readiness wait, in seconds. Raise when the sandbox image build, gateway upload, or in-sandbox boot exceeds the default (typical on 70B+ models, first-time gateway uploads over slow links, or DGX Station / remote-VM first runs). When the deadline expires onboarding deletes the orphaned sandbox and prints the retry hint. | +| `NEMOCLAW_SANDBOX_READY_TIMEOUT` | `180` | Wall-clock timeout for post-create readiness and OpenShell command re-registration after policy application, in seconds. Raise when the sandbox image build, gateway upload, in-sandbox boot, or post-policy re-registration exceeds the default (typical on 70B+ models, first-time gateway uploads over slow links, or DGX Station / remote-VM first runs). When the post-create deadline expires, onboarding deletes an orphaned sandbox and prints the retry hint. | | `NEMOCLAW_SANDBOX_READY_ERROR_DEBOUNCE` | `30` | Consecutive `Error`-phase polls (2s apart, so ~60s by default) the post-create readiness wait tolerates before treating `Error` as terminal. The gateway can briefly report a just-created sandbox in `Error` while it re-registers the sandbox (seen on DGX Spark); the debounce lets that transient recover to `Ready`. `Failed` and `CrashLoopBackOff` always fail immediately. Set to `1` to restore fast-fail on the first `Error` poll. | + + +For managed recovery, the same timeout covers OpenShell re-registration after transactional recreation. +When the deadline expires, the primary dashboard or API host forward stays stopped. + + + ```bash export NEMOCLAW_OLLAMA_PULL_TIMEOUT=3600 export NEMOCLAW_SANDBOX_READY_TIMEOUT=600 $$nemoclaw onboard ``` -If a timeout fires, onboarding emits the elapsed budget plus a hint to raise the relevant variable. +If the Ollama pull or post-create readiness timeout fires, onboarding emits the elapsed budget plus a hint to raise the relevant variable. The Ollama pull preserves its partial download for the next attempt. -The readiness wait deletes the orphaned sandbox first so the next `$$nemoclaw onboard` starts clean. +The post-create readiness wait deletes the orphaned sandbox first so the next `$$nemoclaw onboard` starts clean. +A post-policy re-registration failure leaves the sandbox in place and reports that OpenShell did not re-register it. ### Lifecycle Behavior Flags diff --git a/docs/reference/troubleshooting.mdx b/docs/reference/troubleshooting.mdx index 9413d576540..b9f267ac761 100644 --- a/docs/reference/troubleshooting.mdx +++ b/docs/reference/troubleshooting.mdx @@ -1485,6 +1485,9 @@ openshell sandbox list $$nemoclaw status ``` +If onboarding instead reports that the sandbox "did not re-register with OpenShell after policy application," the same timeout controls that post-policy command-readiness probe. +Raise the budget before retrying, then inspect the same gateway and sandbox status if re-registration still fails. + ### Sandbox onboard fails with "entered Error phase before it became ready" Onboarding ends with: diff --git a/src/lib/actions/sandbox/forward-recovery.ts b/src/lib/actions/sandbox/forward-recovery.ts index e0d1686c9f6..d13c93ab7f2 100644 --- a/src/lib/actions/sandbox/forward-recovery.ts +++ b/src/lib/actions/sandbox/forward-recovery.ts @@ -1,8 +1,14 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { spawnSync } from "node:child_process"; + +import { resolveOpenshell } from "../../adapters/openshell/resolve"; import { captureOpenshell, isCommandTimeout, runOpenshell } from "../../adapters/openshell/runtime"; -import { OPENSHELL_PROBE_TIMEOUT_MS } from "../../adapters/openshell/timeouts"; +import { + OPENSHELL_OPERATION_TIMEOUT_MS, + OPENSHELL_PROBE_TIMEOUT_MS, +} from "../../adapters/openshell/timeouts"; import * as agentRuntime from "../../agent/runtime"; import { DASHBOARD_PORT } from "../../core/ports"; import { waitUntil } from "../../core/wait"; @@ -11,9 +17,12 @@ import type { SandboxMessagingHostForwardPlan } from "../../messaging/manifest"; import { hydrateDerivedSandboxMessagingPlanFields } from "../../messaging/persistence"; import { parseSandboxMessagingPlan } from "../../messaging/plan-validation"; import { isRemoteDashboardBindRequested } from "../../onboard/dockerfile-remote-dashboard-bind-contract"; +import { resolveSandboxGatewayName } from "../../onboard/gateway-binding"; import { isWsl } from "../../platform"; +import { ROOT } from "../../state/paths"; import * as registry from "../../state/registry"; import { parseForwardList } from "../../state/sandbox-session"; +import { buildSubprocessEnv } from "../../subprocess-env"; import { classifyForwardHealthWithReachability, isLocalForwardReachable, @@ -38,10 +47,52 @@ type SandboxForwardRecoveryOptions = { isWsl?: boolean; }; +type DashboardForwardStopRunner = ( + args: string[], + options: { ignoreError: true; stdio: "ignore"; timeout: number }, +) => { status?: number | null }; + +const FORWARD_RELEASE_TIMEOUT_MS = 5_000; +const FORWARD_RELEASE_POLL_MS = 250; + function isValidPort(value: unknown): value is number { return typeof value === "number" && Number.isInteger(value) && value >= 1 && value <= 65535; } +function runDashboardForwardStopBestEffort( + args: string[], + options: { timeout: number }, +): { status?: number | null } { + try { + const openshellBinary = resolveOpenshell(); + if (!openshellBinary) return { status: 1 }; + return spawnSync(openshellBinary, args, { + cwd: ROOT, + env: buildSubprocessEnv(), + stdio: "ignore", + timeout: options.timeout, + }); + } catch { + // The container lifecycle action has already completed; cleanup must not + // replace that result when OpenShell cannot be launched. + return { status: 1 }; + } +} + +function confirmDashboardForwardReleased( + port: number, + isForwardReachable: (port: number) => boolean, +): boolean { + const now = Date.now; + return waitUntil(() => !isForwardReachable(port), { + deadlineMs: now() + FORWARD_RELEASE_TIMEOUT_MS, + initialIntervalMs: FORWARD_RELEASE_POLL_MS, + maxIntervalMs: FORWARD_RELEASE_POLL_MS, + backoffFactor: 1, + now, + }); +} + export function resolveSandboxDashboardPort( sandboxName: string, deps: SandboxPortDeps = {}, @@ -61,6 +112,48 @@ export function resolveSandboxDashboardPort( return DASHBOARD_PORT; } +/** + * Tear down the host-side dashboard port-forward this sandbox created. + * + * `stop` stops the container but must also release the forward it spawned; + * leaving it alive orphans an `ssh -L` listener on the dashboard port, which + * `status` then misreports as a foreign `sandbox_dashboard_port_conflict` and + * which `start`/`recover` contend with (#7227). Best-effort: a stop must still + * free container resources when openshell is unreachable, so errors are ignored + * — mirroring the sandbox- and gateway-scoped forward cleanup used elsewhere. + * OpenShell may return before its SSH listener exits, so successful commands + * also receive a bounded host-port release wait. + */ +export function teardownSandboxDashboardForward( + sandboxName: string, + deps: { + getSandbox?: typeof registry.getSandbox; + isLocalForwardReachable?: typeof isLocalForwardReachable; + resolveSandboxDashboardPort?: typeof resolveSandboxDashboardPort; + resolveSandboxGatewayName?: typeof resolveSandboxGatewayName; + runOpenshell?: DashboardForwardStopRunner; + } = {}, +): void { + try { + const getSandbox = deps.getSandbox ?? registry.getSandbox; + const sandbox = getSandbox(sandboxName); + if (!sandbox) return; + const gatewayName = (deps.resolveSandboxGatewayName ?? resolveSandboxGatewayName)(sandbox); + const resolvePort = deps.resolveSandboxDashboardPort ?? resolveSandboxDashboardPort; + const port = resolvePort(sandboxName, { getSandbox: () => sandbox }); + const run = deps.runOpenshell ?? runDashboardForwardStopBestEffort; + const result = run(["forward", "stop", String(port), sandboxName, "--gateway", gatewayName], { + ignoreError: true, + stdio: "ignore", + timeout: OPENSHELL_OPERATION_TIMEOUT_MS, + }); + if (result.status !== 0) return; + confirmDashboardForwardReleased(port, deps.isLocalForwardReachable ?? isLocalForwardReachable); + } catch { + // Defense in depth for injected or future runners: teardown is best-effort. + } +} + /** * Re-establish the dashboard port forward to the sandbox. * Uses the recorded dashboard port when available, including custom ports for diff --git a/src/lib/actions/sandbox/process-recovery.test.ts b/src/lib/actions/sandbox/process-recovery.test.ts index bfab0cdd343..7b83588ce1c 100644 --- a/src/lib/actions/sandbox/process-recovery.test.ts +++ b/src/lib/actions/sandbox/process-recovery.test.ts @@ -13,9 +13,34 @@ import { const OPENSHELL_SANDBOX_NOT_READY_STDERR = `Error: × code: 'The system is not in a state required for the operation's │ execution', message: "sandbox is not ready" `; +const OPENSHELL_SUPERVISOR_NOT_CONNECTED_STDERR = `Error: × code: 'The service is currently unavailable', message: "supervisor + │ relay failed: status: Unavailable, message: \\"supervisor session not + │ connected\\", details: [], metadata: MetadataMap { headers: {} }" +`; +const OPENSHELL_SUPERVISOR_DISCONNECTED_STDERR = `Error: × code: 'The service is currently unavailable', message: "supervisor + │ relay failed: status: Unavailable, message: \\"supervisor session + │ disconnected\\", details: [], metadata: MetadataMap { headers: {} }" +`; +const OPENSHELL_RELAY_OPEN_TIMED_OUT_STDERR = `Error: × status: DeadlineExceeded, message: "relay + │ open timed out", details: [], metadata: MetadataMap { headers: {} } +`; +const OPENSHELL_SUPERVISOR_RELAY_CHANNEL_TIMED_OUT_STDERR = `Error: × code: 'The service is currently unavailable', message: "supervisor + │ relay failed: status: DeadlineExceeded, message: \\"relay channel timed + │ out\\", details: [], metadata: MetadataMap { headers: {} }" +`; +const OPENSHELL_RELAY_TARGET_NOT_FOUND_STDERR = `Error: × code: 'The service is currently unavailable', message: "No such file + │ or directory (os error 2)" +`; +const OPENSHELL_RELAY_TARGET_REFUSED_STDERR = `Error: × code: 'The service is currently unavailable', message: "Connection + │ refused (os error 111)" +`; describe("recreated sandbox OpenShell readiness", () => { - it("retries only the structured not-ready state until OpenShell accepts the sandbox", () => { + afterEach(() => { + vi.unstubAllEnvs(); + }); + + it("retries the structured not-ready state until OpenShell accepts the sandbox", () => { const notReady = { status: 1, output: OPENSHELL_SANDBOX_NOT_READY_STDERR.trim(), @@ -52,12 +77,110 @@ describe("recreated sandbox OpenShell readiness", () => { expect(sleeps).toEqual([3, 3]); }); - it("fails immediately on an unknown OpenShell error", () => { + it("retries the exact supervisor reconnect states exposed during direct recreation", () => { + const reconnecting = [ + OPENSHELL_SUPERVISOR_NOT_CONNECTED_STDERR, + OPENSHELL_SUPERVISOR_DISCONNECTED_STDERR, + ].map((stderr) => ({ + status: 1, + output: stderr.trim(), + stdout: "", + stderr, + })); + const captureOpenshellImpl = vi + .fn() + .mockReturnValueOnce(reconnecting[0]) + .mockReturnValueOnce(reconnecting[1]) + .mockReturnValueOnce({ status: 0, output: "", stdout: "", stderr: "" }); + const beforeProbe = vi.fn(() => true); + const sleeps: number[] = []; + + expect( + waitForRecreatedSandboxOpenShellReady("recreated-box", { + beforeProbe, + captureOpenshellImpl, + intervalSeconds: 3, + sleepImpl: (seconds) => sleeps.push(seconds), + timeoutSeconds: 6, + }), + ).toBe(true); + expect(captureOpenshellImpl).toHaveBeenCalledTimes(3); + expect(beforeProbe).toHaveBeenCalledTimes(3); + expect(sleeps).toEqual([3, 3]); + }); + + it.each([ + OPENSHELL_RELAY_OPEN_TIMED_OUT_STDERR, + OPENSHELL_SUPERVISOR_RELAY_CHANNEL_TIMED_OUT_STDERR, + ])("retries when the connected supervisor misses OpenShell's relay deadline (#7227)", (stderr) => { + const captureOpenshellImpl = vi + .fn() + .mockReturnValueOnce({ + status: 1, + output: stderr.trim(), + stdout: "", + stderr, + }) + .mockReturnValueOnce({ status: 0, output: "", stdout: "", stderr: "" }); + const beforeProbe = vi.fn(() => true); + const sleeps: number[] = []; + + expect( + waitForRecreatedSandboxOpenShellReady("recreated-box", { + beforeProbe, + captureOpenshellImpl, + intervalSeconds: 3, + sleepImpl: (seconds) => sleeps.push(seconds), + timeoutSeconds: 30, + }), + ).toBe(true); + expect(beforeProbe).toHaveBeenCalledTimes(2); + expect(captureOpenshellImpl).toHaveBeenCalledTimes(2); + expect(sleeps).toEqual([3]); + }); + + it.each([ + OPENSHELL_RELAY_TARGET_NOT_FOUND_STDERR, + OPENSHELL_RELAY_TARGET_REFUSED_STDERR, + ])("retries while the replacement supervisor's local relay target starts (#7273)", (stderr) => { + const captureOpenshellImpl = vi + .fn() + .mockReturnValueOnce({ + status: 1, + output: stderr.trim(), + stdout: "", + stderr, + }) + .mockReturnValueOnce({ status: 0, output: "", stdout: "", stderr: "" }); + const beforeProbe = vi.fn(() => true); + const sleeps: number[] = []; + + expect( + waitForRecreatedSandboxOpenShellReady("recreated-box", { + beforeProbe, + captureOpenshellImpl, + intervalSeconds: 3, + sleepImpl: (seconds) => sleeps.push(seconds), + timeoutSeconds: 30, + }), + ).toBe(true); + expect(beforeProbe).toHaveBeenCalledTimes(2); + expect(captureOpenshellImpl).toHaveBeenCalledTimes(2); + expect(sleeps).toEqual([3]); + }); + + it.each([ + `Error: × status: DeadlineExceeded, message: "policy update timed out"`, + `Error: × code: 'The service is currently unavailable', message: "supervisor + │ relay failed: status: DeadlineExceeded, message: \\"relay requester timed + │ out\\", details: [], metadata: MetadataMap { headers: {} }"`, + `Error: × code: 'The service is currently unavailable', message: "permission denied"`, + ])("does not retry an unrelated OpenShell error", (stderr) => { const captureOpenshellImpl = vi.fn(() => ({ status: 1, - output: "permission denied", + output: stderr, stdout: "", - stderr: "permission denied", + stderr, })); const sleeps: number[] = []; @@ -73,14 +196,12 @@ describe("recreated sandbox OpenShell readiness", () => { expect(sleeps).toEqual([]); }); - it("does not retry an outcome-uncertain OpenShell timeout", () => { - const timeoutError = Object.assign(new Error("timed out"), { code: "ETIMEDOUT" }); + it("fails immediately on an unknown OpenShell error", () => { const captureOpenshellImpl = vi.fn(() => ({ - status: null, - output: "", + status: 1, + output: "permission denied", stdout: "", - stderr: "", - error: timeoutError, + stderr: "permission denied", })); const sleeps: number[] = []; @@ -96,6 +217,35 @@ describe("recreated sandbox OpenShell readiness", () => { expect(sleeps).toEqual([]); }); + it("retries the no-op OpenShell readiness probe after a command timeout (#7273)", () => { + const timeoutError = Object.assign(new Error("timed out"), { code: "ETIMEDOUT" }); + const captureOpenshellImpl = vi + .fn() + .mockReturnValueOnce({ + status: null, + output: "", + stdout: "", + stderr: "", + error: timeoutError, + }) + .mockReturnValueOnce({ status: 0, output: "", stdout: "", stderr: "" }); + const beforeProbe = vi.fn(() => true); + const sleeps: number[] = []; + + expect( + waitForRecreatedSandboxOpenShellReady("recreated-box", { + beforeProbe, + captureOpenshellImpl, + intervalSeconds: 3, + sleepImpl: (seconds) => sleeps.push(seconds), + timeoutSeconds: 30, + }), + ).toBe(true); + expect(beforeProbe).toHaveBeenCalledTimes(2); + expect(captureOpenshellImpl).toHaveBeenCalledTimes(2); + expect(sleeps).toEqual([3]); + }); + it("rechecks the pinned managed guard before every readiness retry", () => { const captureOpenshellImpl = vi.fn(() => ({ status: 1, @@ -119,6 +269,101 @@ describe("recreated sandbox OpenShell readiness", () => { expect(captureOpenshellImpl).toHaveBeenCalledOnce(); expect(sleeps).toEqual([3]); }); + + it("retries an inconclusive managed guard within the readiness deadline", () => { + const captureOpenshellImpl = vi.fn(() => ({ + status: 0, + output: "", + stdout: "", + stderr: "", + })); + const beforeProbe = vi.fn().mockReturnValueOnce(null).mockReturnValueOnce(true); + const sleeps: number[] = []; + + expect( + waitForRecreatedSandboxOpenShellReady("recreated-box", { + beforeProbe, + captureOpenshellImpl, + intervalSeconds: 3, + sleepImpl: (seconds) => sleeps.push(seconds), + timeoutSeconds: 6, + }), + ).toBe(true); + expect(beforeProbe).toHaveBeenCalledTimes(2); + expect(captureOpenshellImpl).toHaveBeenCalledOnce(); + expect(sleeps).toEqual([3]); + }); + + it("fails closed on a definitive managed guard failure without probing OpenShell", () => { + const captureOpenshellImpl = vi.fn(() => ({ + status: 0, + output: "", + stdout: "", + stderr: "", + })); + const beforeProbe = vi.fn(() => false); + const sleeps: number[] = []; + + expect( + waitForRecreatedSandboxOpenShellReady("recreated-box", { + beforeProbe, + captureOpenshellImpl, + intervalSeconds: 3, + sleepImpl: (seconds) => sleeps.push(seconds), + timeoutSeconds: 6, + }), + ).toBe(false); + expect(beforeProbe).toHaveBeenCalledOnce(); + expect(captureOpenshellImpl).not.toHaveBeenCalled(); + expect(sleeps).toEqual([]); + }); + + it("fails when the managed guard stays inconclusive until the deadline", () => { + const captureOpenshellImpl = vi.fn(() => ({ + status: 0, + output: "", + stdout: "", + stderr: "", + })); + const beforeProbe = vi.fn(() => null); + const sleeps: number[] = []; + + expect( + waitForRecreatedSandboxOpenShellReady("recreated-box", { + beforeProbe, + captureOpenshellImpl, + intervalSeconds: 3, + sleepImpl: (seconds) => sleeps.push(seconds), + timeoutSeconds: 6, + }), + ).toBe(false); + expect(beforeProbe).toHaveBeenCalledTimes(3); + expect(captureOpenshellImpl).not.toHaveBeenCalled(); + expect(sleeps).toEqual([3, 3]); + }); + + it("does not let the legacy gateway timeout shorten the sandbox readiness budget (#7273)", () => { + vi.stubEnv("NEMOCLAW_GATEWAY_RECOVERY_WAIT_SECONDS", "1"); + vi.stubEnv("NEMOCLAW_SANDBOX_READY_TIMEOUT", "6"); + const captureOpenshellImpl = vi.fn(() => ({ + status: 1, + output: OPENSHELL_SANDBOX_NOT_READY_STDERR.trim(), + stdout: "", + stderr: OPENSHELL_SANDBOX_NOT_READY_STDERR, + })); + const sleeps: number[] = []; + + expect( + waitForRecreatedSandboxOpenShellReady("recreated-box", { + captureOpenshellImpl, + intervalSeconds: 3, + sleepImpl: (seconds) => sleeps.push(seconds), + timeoutSeconds: Number(process.env.NEMOCLAW_SANDBOX_READY_TIMEOUT), + }), + ).toBe(false); + expect(captureOpenshellImpl).toHaveBeenCalledTimes(3); + expect(sleeps).toEqual([3, 3]); + }); }); describe("confirmRecoveredSandboxGatewayManaged scope", () => { diff --git a/src/lib/actions/sandbox/process-recovery.ts b/src/lib/actions/sandbox/process-recovery.ts index 9024fc530cb..ee0258b5de0 100644 --- a/src/lib/actions/sandbox/process-recovery.ts +++ b/src/lib/actions/sandbox/process-recovery.ts @@ -16,6 +16,7 @@ import { OPENSHELL_PROBE_TIMEOUT_MS } from "../../adapters/openshell/timeouts"; import * as agentRuntime from "../../agent/runtime"; import { G, R } from "../../cli/terminal-style"; import { sleepSeconds, waitUntil } from "../../core/wait"; +import { SANDBOX_READY_TIMEOUT_SECS } from "../../onboard/env"; import { ROOT, shellQuote } from "../../runner"; import { isDirectSandboxFallbackUnavailableError, @@ -585,46 +586,115 @@ function readNonNegativeNumberEnv(name: string, fallback: number): number { } const OPENSHELL_SANDBOX_NOT_READY = `Error: code: 'The system is not in a state required for the operation's execution', message: "sandbox is not ready"`; +const OPENSHELL_SERVICE_UNAVAILABLE = "code: 'The service is currently unavailable'"; +const OPENSHELL_RELAY_OPEN_TIMED_OUT = 'message: "relay open timed out"'; +const OPENSHELL_SUPERVISOR_RELAY_DEADLINE = "supervisor relay failed: status: DeadlineExceeded"; +const OPENSHELL_RELAY_CHANNEL_TIMED_OUT = "relay channel timed out"; +const OPENSHELL_RELAY_TARGET_NOT_FOUND = 'message: "No such file or directory (os error 2)"'; +const OPENSHELL_RELAY_TARGET_REFUSED = 'message: "Connection refused (os error 111)"'; function normalizeOpenshellStructuredError(value: string): string { return stripAnsi(value).replace(/[×│]/gu, " ").replace(/\s+/gu, " ").trim(); } -function isExactlyRetryableOpenshellSandboxNotReady( - result: ReturnType, -): boolean { +function hasRetryableOpenshellResultShape(result: ReturnType): boolean { return ( result.status === 1 && !result.error && String(result.stdout ?? "").trim() === "" && - normalizeOpenshellStructuredError(String(result.stderr ?? "")) === OPENSHELL_SANDBOX_NOT_READY + String(result.stderr ?? "").trim() !== "" ); } +function isRetryableOpenshellReRegistrationState( + result: ReturnType, +): boolean { + if (!hasRetryableOpenshellResultShape(result)) return false; + const error = normalizeOpenshellStructuredError(String(result.stderr)); + if (error === OPENSHELL_SANDBOX_NOT_READY) return true; + + // OpenShell 0.0.85 can keep the recreated sandbox's cached phase at Ready + // while its replacement supervisor session is still registering. The exec + // RPC can fail before a session connects, after a session disconnects, while + // the replacement supervisor's local SSH relay target is starting, or after + // the session connects but does not claim its reverse relay within OpenShell's + // 10-second relay deadline. These exact results are control-plane + // re-registration states; all other OpenShell failures remain terminal. + // NemoClaw cannot repair this OpenShell-owned phase/session state without + // bypassing the control plane. Remove these matches when supported OpenShell + // versions publish Ready only after the replacement session and relay are + // usable, or report the standard sandbox-not-ready state until then. + const sessionUnavailable = + error.includes(OPENSHELL_SERVICE_UNAVAILABLE) && + error.includes("supervisor relay failed: status: Unavailable") && + (error.includes("supervisor session not connected") || + error.includes("supervisor session disconnected")); + const relayChannelTimedOut = + error.includes(OPENSHELL_SERVICE_UNAVAILABLE) && + error.includes(OPENSHELL_SUPERVISOR_RELAY_DEADLINE) && + error.includes(OPENSHELL_RELAY_CHANNEL_TIMED_OUT); + const relayTargetUnavailable = + error.includes(OPENSHELL_SERVICE_UNAVAILABLE) && + (error.includes(OPENSHELL_RELAY_TARGET_NOT_FOUND) || + error.includes(OPENSHELL_RELAY_TARGET_REFUSED)); + return ( + sessionUnavailable || + relayChannelTimedOut || + relayTargetUnavailable || + error.includes(OPENSHELL_RELAY_OPEN_TIMED_OUT) + ); +} + +type RecreatedSandboxOpenShellReadinessFailure = + | "managed-health-definitive-failure" + | "managed-health-inconclusive-timeout" + | "openshell-readiness-failure"; + +type RecreatedSandboxOpenShellReadinessResult = + | { ready: true } + | { failure: RecreatedSandboxOpenShellReadinessFailure; ready: false }; + +type RecreatedSandboxOpenShellReadyOptions = { + captureOpenshellImpl?: typeof captureOpenshell; + beforeProbe?: (timeoutMs: number) => boolean | null; + intervalSeconds?: number; + nowImpl?: () => number; + sleepImpl?: (seconds: number) => void; + timeoutSeconds?: number; +}; + +function recreatedSandboxOpenShellReadinessFailureDetail( + failure: RecreatedSandboxOpenShellReadinessFailure, +): string { + 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"; + 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"; + case "openshell-readiness-failure": + return "the recreated sandbox did not become ready in OpenShell, so the primary dashboard/API host forward was not started"; + } +} + /** * Wait until OpenShell has re-registered a directly recreated sandbox as * ready. This probe deliberately has no direct-Docker or SSH fallback: it is * proving control-plane readiness, not authorizing the already completed * replacement-container recovery. */ -export function waitForRecreatedSandboxOpenShellReady( +function waitForRecreatedSandboxOpenShellReadyResult( sandboxName: string, - options: { - captureOpenshellImpl?: typeof captureOpenshell; - beforeProbe?: (timeoutMs: number) => boolean; - intervalSeconds?: number; - nowImpl?: () => number; - sleepImpl?: (seconds: number) => void; - timeoutSeconds?: number; - } = {}, -): boolean { + options: RecreatedSandboxOpenShellReadyOptions = {}, +): RecreatedSandboxOpenShellReadinessResult { const capture = options.captureOpenshellImpl ?? captureOpenshell; const now = options.nowImpl ?? Date.now; const sleep = options.sleepImpl ?? sleepSeconds; - const timeoutSeconds = readNonNegativeNumberEnv( - "NEMOCLAW_GATEWAY_RECOVERY_WAIT_SECONDS", - options.timeoutSeconds ?? 30, - ); + const timeoutSeconds = + typeof options.timeoutSeconds === "number" && + Number.isFinite(options.timeoutSeconds) && + options.timeoutSeconds >= 0 + ? options.timeoutSeconds + : readNonNegativeNumberEnv("NEMOCLAW_GATEWAY_RECOVERY_WAIT_SECONDS", 30); const intervalSeconds = readNonNegativeNumberEnv( "NEMOCLAW_GATEWAY_RECOVERY_POLL_INTERVAL_SECONDS", options.intervalSeconds ?? 3, @@ -637,25 +707,60 @@ export function waitForRecreatedSandboxOpenShellReady( for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { const preGuardRemainingMs = deadlineMs - now(); - if (attempt > 1 && preGuardRemainingMs <= 0) return false; + if (attempt > 1 && preGuardRemainingMs <= 0) { + return { failure: "managed-health-inconclusive-timeout", ready: false }; + } const guardBudgetMs = Math.max(1, Math.min(OPENSHELL_PROBE_TIMEOUT_MS, preGuardRemainingMs)); - if (options.beforeProbe?.(guardBudgetMs) === false) return false; + const guardResult = options.beforeProbe?.(guardBudgetMs); + if (guardResult === false) { + return { failure: "managed-health-definitive-failure", ready: false }; + } + if (guardResult === null) { + if (attempt === maxAttempts) { + return { failure: "managed-health-inconclusive-timeout", ready: false }; + } + const postGuardRemainingMs = deadlineMs - now(); + if (postGuardRemainingMs <= 0) { + return { failure: "managed-health-inconclusive-timeout", ready: false }; + } + sleep(Math.min(intervalSeconds * 1000, postGuardRemainingMs) / 1000); + continue; + } const remainingMs = deadlineMs - now(); - if (attempt > 1 && remainingMs <= 0) return false; + if (attempt > 1 && remainingMs <= 0) { + return { failure: "openshell-readiness-failure", ready: false }; + } const result = capture(["sandbox", "exec", "--name", sandboxName, "--", "true"], { ignoreError: true, includeStderr: true, includeStreams: true, timeout: Math.max(1, Math.min(OPENSHELL_PROBE_TIMEOUT_MS, remainingMs)), }); - if (result.status === 0 && !result.error) return true; - if (!isExactlyRetryableOpenshellSandboxNotReady(result)) return false; - if (attempt === maxAttempts) return false; + if (result.status === 0 && !result.error) return { ready: true }; + // This probe executes only `true`, so an OpenShell process timeout has no + // mutation outcome to reconcile. Treat that exact timeout as inconclusive + // and retry behind the pinned managed-health guard on the next iteration. + // All other unexpected OpenShell failures remain definitive. + if (!isRetryableOpenshellReRegistrationState(result) && !isCommandTimeout(result)) { + return { failure: "openshell-readiness-failure", ready: false }; + } + if (attempt === maxAttempts) { + return { failure: "openshell-readiness-failure", ready: false }; + } const postProbeRemainingMs = deadlineMs - now(); - if (postProbeRemainingMs <= 0) return false; + if (postProbeRemainingMs <= 0) { + return { failure: "openshell-readiness-failure", ready: false }; + } sleep(Math.min(intervalSeconds * 1000, postProbeRemainingMs) / 1000); } - return false; + return { failure: "openshell-readiness-failure", ready: false }; +} + +export function waitForRecreatedSandboxOpenShellReady( + sandboxName: string, + options: RecreatedSandboxOpenShellReadyOptions = {}, +): boolean { + return waitForRecreatedSandboxOpenShellReadyResult(sandboxName, options).ready; } function gatewayRecoveryTimeoutSeconds( @@ -1055,20 +1160,22 @@ function checkAndRecoverSandboxProcessesWithoutHostLock( let relaunchedIdentityRejected = false; const confirmRelaunchedManagedHealth = relaunch ? (timeout = OPENSHELL_PROBE_TIMEOUT_MS) => { - let confirmed = false; try { - confirmed = - confirmRecoveredSandboxGatewayManaged(sandboxName, { - requestGatewaySupervisorActionImpl: (name, action) => - requestManagedProbe(name, action, timeout), - }) === true; + const confirmed = confirmRecoveredSandboxGatewayManaged(sandboxName, { + requestGatewaySupervisorActionImpl: (name, action) => + requestManagedProbe(name, action, timeout), + }); + if (confirmed === false) relaunchedIdentityRejected = true; + return confirmed; } catch { - confirmed = false; + relaunchedIdentityRejected = true; + return false; } - relaunchedIdentityRejected ||= !confirmed; - return confirmed; } : null; + const confirmRelaunchedManagedHealthForForward = relaunch + ? () => confirmRelaunchedManagedHealth?.() === true + : null; // Wait for gateway to bind its HTTP port before declaring success. The // recovered process can be alive before the OpenAI-compatible API is ready. let gatewayReady = false; @@ -1134,28 +1241,38 @@ function checkAndRecoverSandboxProcessesWithoutHostLock( } } } - if ( - relaunch && - !waitForRecreatedSandboxOpenShellReadyImpl(sandboxName, { - beforeProbe: (timeoutMs) => confirmRelaunchedManagedHealth?.(timeoutMs) === true, - timeoutSeconds: gatewayRecoveryTimeoutSeconds(recoveryAgent), - }) - ) { + const readinessFailureDetail = relaunch + ? (() => { + const readinessOptions: RecreatedSandboxOpenShellReadyOptions = { + beforeProbe: (timeoutMs) => confirmRelaunchedManagedHealth?.(timeoutMs) ?? null, + timeoutSeconds: SANDBOX_READY_TIMEOUT_SECS, + }; + 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); + })() + : null; + if (readinessFailureDetail) { return { checked: true, wasRunning: false, recovered: true, forwardRecovered: false, forwardRecoveryFailed: true, - forwardRecoveryFailureDetail: - "the recreated sandbox did not become ready in OpenShell, so the primary dashboard/API host forward was not started", + forwardRecoveryFailureDetail: readinessFailureDetail, }; } const mcpRefusal = processRecoveryMcpReconciliationRefusal(sandboxName, false); if (mcpRefusal) return mcpRefusal; const forwardRecovered = ensureSandboxPortForward(sandboxName, { - afterSuccess: confirmRelaunchedManagedHealth ?? undefined, - beforeStart: confirmRelaunchedManagedHealth ?? undefined, + afterSuccess: confirmRelaunchedManagedHealthForForward ?? undefined, + beforeStart: confirmRelaunchedManagedHealthForForward ?? undefined, isWsl: isWslOverride, }); if (!forwardRecovered && relaunchedIdentityRejected) { diff --git a/src/lib/actions/sandbox/stop.test.ts b/src/lib/actions/sandbox/stop.test.ts index d2ebcb2a209..77ec0b8519e 100644 --- a/src/lib/actions/sandbox/stop.test.ts +++ b/src/lib/actions/sandbox/stop.test.ts @@ -4,6 +4,7 @@ import { describe, expect, it, vi } from "vitest"; import type { SandboxEntry } from "../../state/registry"; +import { teardownSandboxDashboardForward } from "./forward-recovery"; import { type SandboxStopDeps, stopSandbox } from "./stop"; function sandbox(values: Partial = {}): SandboxEntry { @@ -26,6 +27,8 @@ function harness(overrides: Partial = {}) { >(() => [container("openshell-my-sandbox", true)]); const stopSandboxChannels = vi.fn>(); const dockerStop = vi.fn>(() => ({ status: 0 })); + const teardownSandboxDashboardForward = + vi.fn>(); const log = vi.fn<(message: string) => void>(); const warn = vi.fn<(message: string) => void>(); const deps: SandboxStopDeps = { @@ -34,6 +37,7 @@ function harness(overrides: Partial = {}) { printDockerRuntimeDownGuidance, findLabeledSandboxContainers, stopSandboxChannels, + teardownSandboxDashboardForward, dockerStop, log, warn, @@ -42,6 +46,7 @@ function harness(overrides: Partial = {}) { return { deps, dockerStop, + teardownSandboxDashboardForward, findLabeledSandboxContainers, getSandbox, isDockerRuntimeDown, @@ -52,6 +57,78 @@ function harness(overrides: Partial = {}) { }; } +describe("teardownSandboxDashboardForward", () => { + it("stops only the selected sandbox's resolved dashboard forward on its gateway (#7227)", () => { + const getSandbox = vi.fn(() => + sandbox({ + dashboardPort: 19443, + gatewayName: "nemoclaw-18080", + gatewayPort: 18080, + }), + ); + const resolveSandboxDashboardPort = vi.fn(() => 19443); + const runOpenshell = vi.fn(() => ({ status: 0 })); + const isLocalForwardReachable = vi + .fn<() => boolean>() + .mockReturnValueOnce(true) + .mockReturnValueOnce(false); + + expect(() => + teardownSandboxDashboardForward("selected-sandbox", { + getSandbox, + isLocalForwardReachable, + resolveSandboxDashboardPort, + runOpenshell, + }), + ).not.toThrow(); + + expect(resolveSandboxDashboardPort).toHaveBeenCalledWith( + "selected-sandbox", + expect.objectContaining({ getSandbox: expect.any(Function) }), + ); + expect(runOpenshell).toHaveBeenCalledWith( + ["forward", "stop", "19443", "selected-sandbox", "--gateway", "nemoclaw-18080"], + { + ignoreError: true, + stdio: "ignore", + timeout: 30_000, + }, + ); + expect(isLocalForwardReachable).toHaveBeenCalledTimes(2); + expect(isLocalForwardReachable).toHaveBeenNthCalledWith(1, 19443); + expect(isLocalForwardReachable).toHaveBeenNthCalledWith(2, 19443); + }); + + it("does not throw when OpenShell cannot be launched (#7227)", () => { + const runOpenshell = vi.fn(() => { + throw new Error("spawn openshell ENOENT"); + }); + + expect(() => + teardownSandboxDashboardForward("selected-sandbox", { + getSandbox: () => sandbox(), + resolveSandboxDashboardPort: () => 19443, + runOpenshell, + }), + ).not.toThrow(); + }); + + it("does not probe the port when OpenShell reports cleanup failure (#7227)", () => { + const isLocalForwardReachable = vi.fn(() => false); + + expect(() => + teardownSandboxDashboardForward("selected-sandbox", { + getSandbox: () => sandbox(), + isLocalForwardReachable, + resolveSandboxDashboardPort: () => 19443, + runOpenshell: () => ({ status: 1 }), + }), + ).not.toThrow(); + + expect(isLocalForwardReachable).not.toHaveBeenCalled(); + }); +}); + describe("stopSandbox", () => { it("gracefully stops in-sandbox channels before stopping the container (#6026)", () => { const h = harness(); @@ -72,6 +149,57 @@ describe("stopSandbox", () => { ); }); + it("tears down the host-side dashboard port-forward after stopping the container (#7227)", () => { + const h = harness(); + + const result = stopSandbox("my-sandbox", h.deps); + + expect(result.exitCode).toBe(0); + expect(h.teardownSandboxDashboardForward).toHaveBeenCalledWith("my-sandbox"); + // Release the forward only after the container is stopped, never before. + expect(h.dockerStop.mock.invocationCallOrder[0]).toBeLessThan( + h.teardownSandboxDashboardForward.mock.invocationCallOrder[0], + ); + }); + + it("keeps a successful stop successful when dashboard cleanup cannot launch (#7227)", () => { + const teardownSandboxDashboardForward = vi.fn(() => { + throw new Error("spawn openshell EACCES"); + }); + const h = harness({ teardownSandboxDashboardForward }); + + const result = stopSandbox("my-sandbox", h.deps); + + expect(result.exitCode).toBe(0); + expect(teardownSandboxDashboardForward).toHaveBeenCalledWith("my-sandbox"); + expect(h.warn).toHaveBeenCalledWith( + " Warning: could not release the dashboard port-forward: spawn openshell EACCES", + ); + }); + + it("does not release the dashboard forward when the container failed to stop (#7227)", () => { + const h = harness({ dockerStop: vi.fn(() => ({ status: 1 })) }); + + const result = stopSandbox("my-sandbox", h.deps); + + expect(result.exitCode).toBe(1); + expect(h.teardownSandboxDashboardForward).not.toHaveBeenCalled(); + }); + + it("releases a leftover dashboard forward for an already-stopped sandbox — idempotent (#7227)", () => { + const h = harness({ + findLabeledSandboxContainers: vi.fn(() => [container("openshell-my-sandbox", false)]), + }); + + const result = stopSandbox("my-sandbox", h.deps); + + // No container to stop, but a repeated stop must still converge on no + // leftover dashboard listener (e.g. a forward orphaned by an earlier stop). + expect(result.exitCode).toBe(0); + expect(h.dockerStop).not.toHaveBeenCalled(); + expect(h.teardownSandboxDashboardForward).toHaveBeenCalledWith("my-sandbox"); + }); + it("routes channel-stop reporter lines through the action's log and warn (#6026)", () => { const h = harness(); h.stopSandboxChannels.mockImplementation((_name, channelDeps) => { @@ -248,6 +376,7 @@ describe("stopSandbox", () => { expect(result.exitCode).toBe(1); expect(h.dockerStop).toHaveBeenCalledTimes(2); + expect(h.teardownSandboxDashboardForward).not.toHaveBeenCalled(); expect(h.dockerStop).toHaveBeenNthCalledWith( 2, "openshell-my-sandbox-nemoclaw-gpu-backup-1700000000000", diff --git a/src/lib/actions/sandbox/stop.ts b/src/lib/actions/sandbox/stop.ts index 1fc5e20376d..53cc1489ecd 100644 --- a/src/lib/actions/sandbox/stop.ts +++ b/src/lib/actions/sandbox/stop.ts @@ -5,6 +5,7 @@ import { CLI_NAME } from "../../cli/branding"; import { findLabeledSandboxContainers } from "../../onboard/docker-driver-sandbox-recovery"; import * as registry from "../../state/registry"; import { stopSandboxChannels } from "../../tunnel/sandbox-gateway-stop"; +import { teardownSandboxDashboardForward } from "./forward-recovery"; import { isDockerRuntimeDown, printDockerRuntimeDownGuidance } from "./gateway-failure-classifier"; // Lazy adapter accessor, same pattern as docker-driver-sandbox-recovery.ts: @@ -28,6 +29,19 @@ function isAtRest(status: string): boolean { return AT_REST_STATUS_PREFIXES.some((prefix) => status.startsWith(prefix)); } +function teardownDashboardForwardBestEffort( + sandboxName: string, + teardown: typeof teardownSandboxDashboardForward, + warn: (message: string) => void, +): void { + try { + teardown(sandboxName); + } catch (error) { + const detail = error instanceof Error ? error.message : String(error); + warn(` Warning: could not release the dashboard port-forward: ${detail}`); + } +} + export type SandboxLifecycleResult = { exitCode: number; message?: string; @@ -39,6 +53,7 @@ export interface SandboxStopDeps { printDockerRuntimeDownGuidance?: typeof printDockerRuntimeDownGuidance; findLabeledSandboxContainers?: typeof findLabeledSandboxContainers; stopSandboxChannels?: typeof stopSandboxChannels; + teardownSandboxDashboardForward?: typeof teardownSandboxDashboardForward; dockerStop?: DockerStopFn; log?: (message: string) => void; warn?: (message: string) => void; @@ -138,6 +153,15 @@ export function stopSandbox( const stoppable = containers.filter((container) => !isAtRest(container.status)); if (stoppable.length === 0) { log(` Sandbox '${sandboxName}' is already stopped.`); + // Idempotent teardown: an earlier stop may have left the dashboard forward + // alive (e.g. openshell was unreachable then, or the forward was orphaned by + // a raw `docker stop`). Release it here too so a repeated stop always + // converges on no leftover listener (#7227). + teardownDashboardForwardBestEffort( + sandboxName, + deps.teardownSandboxDashboardForward ?? teardownSandboxDashboardForward, + warn, + ); log(` Start it again with '${CLI_NAME} ${sandboxName} start'.`); return { exitCode: 0 }; } @@ -181,6 +205,17 @@ export function stopSandbox( }; } + // Release the host-side dashboard port-forward this sandbox created. Without + // this, the `ssh -L` listener stays alive after the container is stopped, so + // `status` misreports the cleanly-stopped sandbox as a foreign + // `sandbox_dashboard_port_conflict` and `start`/`recover` contend with the + // still-held port (#7227). Best-effort — the container is already stopped. + teardownDashboardForwardBestEffort( + sandboxName, + deps.teardownSandboxDashboardForward ?? teardownSandboxDashboardForward, + warn, + ); + log(` Sandbox '${sandboxName}' stopped. Workspace state is preserved.`); log(` Start it again with '${CLI_NAME} ${sandboxName} start'.`); return { exitCode: 0 }; diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index d782dd9226a..4ee22db0d43 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -3833,7 +3833,6 @@ const computeSetupPresetSuggestions = ( tierName, options, ); - async function setupPoliciesWithSelection( sandboxName: string, options: SetupPolicySelectionOptions = {}, @@ -3848,6 +3847,7 @@ async function setupPoliciesWithSelection( note, isNonInteractive, waitForSandboxReady, + waitForSandboxControlPlaneReady: finalizationHandlerDeps.waitForSandboxControlPlaneReady, syncPresetSelection, selectPolicyTier, setPolicyTier: (s, t) => registry.updateSandbox(s, { policyTier: t }), @@ -4585,8 +4585,8 @@ async function runOnboard(opts: OnboardOptions = {}): Promise { webSearchEnabled: (config) => braveProviderProfile.shouldEnableBraveWebSearch(config), }, finalizationDeps: { - ensureAgentDashboardForward: (name, selectedAgent) => - selectedAgent ? ensureAgentDashboardForward(name, selectedAgent) : 0, + // biome-ignore format: keep src/lib/onboard.ts net-neutral for growth guardrail. + ensureAgentDashboardForward: (name, selectedAgent) => selectedAgent ? ensureAgentDashboardForward(name, selectedAgent) : ensureDashboardForward(name, process.env.CHAT_UI_URL), setDefaultSandbox: registry.setDefault, verifyWebSearchInsideSandbox, recordPostVerifyStarted, diff --git a/src/lib/onboard/finalization-deps.ts b/src/lib/onboard/finalization-deps.ts index d74d03ca123..750779bccd0 100644 --- a/src/lib/onboard/finalization-deps.ts +++ b/src/lib/onboard/finalization-deps.ts @@ -7,6 +7,14 @@ // avoid an import cycle: connect.ts and process-recovery.ts both pull in // onboard helpers, so they must not be statically imported here. export const finalizationHandlerDeps = { + waitForSandboxControlPlaneReady(name: string): boolean { + const processRecovery: typeof import("../actions/sandbox/process-recovery") = + require("../actions/sandbox/process-recovery"); + const { SANDBOX_READY_TIMEOUT_SECS }: typeof import("./env") = require("./env"); + return processRecovery.waitForRecreatedSandboxOpenShellReady(name, { + timeoutSeconds: SANDBOX_READY_TIMEOUT_SECS, + }); + }, checkAndRecoverSandboxProcesses(name: string, options: { quiet: boolean }): void { const processRecovery: typeof import("../actions/sandbox/process-recovery") = require("../actions/sandbox/process-recovery"); diff --git a/src/lib/onboard/machine/final-flow-phases.runtime.test.ts b/src/lib/onboard/machine/final-flow-phases.runtime.test.ts index 6bab3532b33..16f3ee0c76d 100644 --- a/src/lib/onboard/machine/final-flow-phases.runtime.test.ts +++ b/src/lib/onboard/machine/final-flow-phases.runtime.test.ts @@ -63,7 +63,14 @@ describe("final onboard flow runtime boundary", () => { expect(recordStateResult).not.toHaveBeenCalled(); expect(recordInvalidatedStateResult).not.toHaveBeenCalled(); - expect(order).toEqual(["openclaw", "policies", "disarm", "set-default", "verify"]); + expect(order).toEqual([ + "openclaw", + "policies", + "disarm", + "set-default", + "agent-forward", + "verify", + ]); expect(harness.getSession()).toMatchObject({ status: "complete", sandboxName: "my-sandbox", @@ -110,7 +117,14 @@ describe("final onboard flow runtime boundary", () => { expect(recordStateResult).toHaveBeenCalled(); expect(recordInvalidatedStateResult).toHaveBeenCalled(); - expect(order).toEqual(["openclaw", "policies", "disarm", "set-default", "verify"]); + expect(order).toEqual([ + "openclaw", + "policies", + "disarm", + "set-default", + "agent-forward", + "verify", + ]); expect(harness.getSession()).toMatchObject({ status: "complete", sandboxName: "my-sandbox", @@ -231,6 +245,7 @@ describe("final onboard flow runtime boundary", () => { "policies", "disarm", "set-default", + "agent-forward", "verify:slack,discord", ]); }); @@ -296,7 +311,14 @@ describe("final onboard flow runtime boundary", () => { }), ).rejects.toThrow("verification failed"); - expect(order).toEqual(["openclaw", "policies", "disarm", "set-default", "verify"]); + expect(order).toEqual([ + "openclaw", + "policies", + "disarm", + "set-default", + "agent-forward", + "verify", + ]); expect(printDashboard).not.toHaveBeenCalled(); expect(harness.getSession()).toMatchObject({ status: "in_progress", diff --git a/src/lib/onboard/machine/final-flow-phases.test.ts b/src/lib/onboard/machine/final-flow-phases.test.ts index bc00f35fb97..c7f556753c6 100644 --- a/src/lib/onboard/machine/final-flow-phases.test.ts +++ b/src/lib/onboard/machine/final-flow-phases.test.ts @@ -21,7 +21,7 @@ describe("final onboard flow phases", () => { const policiesResult = await policiesPhase.run(branchResult.context); await finalizationPhase.run(policiesResult.context); - expect(order).toEqual(["openclaw", "policies", "set-default", "verify"]); + expect(order).toEqual(["openclaw", "policies", "set-default", "agent-forward", "verify"]); }); it("carries merged policy messaging channels into the final flow context", async () => { @@ -85,7 +85,14 @@ describe("final onboard flow phases", () => { }, }); - expect(order).toEqual(["openclaw", "policies", "disarm", "set-default", "verify"]); + expect(order).toEqual([ + "openclaw", + "policies", + "disarm", + "set-default", + "agent-forward", + "verify", + ]); expect(recorded).toEqual(["policies", "finalizing", "complete"]); }); }); diff --git a/src/lib/onboard/machine/handlers/finalization.test.ts b/src/lib/onboard/machine/handlers/finalization.test.ts index 1130ccfc0ef..dda440ae41d 100644 --- a/src/lib/onboard/machine/handlers/finalization.test.ts +++ b/src/lib/onboard/machine/handlers/finalization.test.ts @@ -160,6 +160,36 @@ describe("handleFinalizationState", () => { }); }); + it("restores the default OpenClaw dashboard forward after process recovery", async () => { + let forwardLive = true; + const recoverProcesses = vi.fn(() => { + forwardLive = false; + }); + const ensureDashboard = vi.fn(() => { + forwardLive = true; + return 18789; + }); + const verify = vi.fn(async () => ({ ok: forwardLive })); + const { deps } = createDeps({ + checkAndRecoverSandboxProcesses: recoverProcesses, + ensureAgentDashboardForward: ensureDashboard, + verifyDeployment: verify, + isDeploymentHealthy: vi.fn((result) => result.ok), + }); + + const result = await handleFinalizationState(baseOptions(deps)); + + expect(ensureDashboard).toHaveBeenCalledWith("my-assistant", null); + expect(ensureDashboard.mock.invocationCallOrder[0]).toBeGreaterThan( + recoverProcesses.mock.invocationCallOrder[1], + ); + expect(ensureDashboard.mock.invocationCallOrder[0]).toBeLessThan( + verify.mock.invocationCallOrder[0], + ); + expect(result.deploymentHealthy).toBe(true); + expect(result.stateResult.type).toBe("complete"); + }); + it("ensures agent dashboard forwarding before completion for non-OpenClaw agents", async () => { const { deps, calls } = createDeps(); const agent = { name: "hermes" }; @@ -193,13 +223,13 @@ describe("handleFinalizationState", () => { const recoveryOrders = calls.recoverProcesses.mock.invocationCallOrder; const refreshOrder = calls.ensureAgentDashboard.mock.invocationCallOrder[0]; expect(recoveryOrders).toHaveLength(2); - expect(refreshOrder).toBeLessThan(recoveryOrders[0]); expect(recoveryOrders[1]).toBeGreaterThan(calls.warmupScopeUpgrade.mock.invocationCallOrder[0]); expect(recoveryOrders[1]).toBeGreaterThan( calls.autoPairScopeApproval.mock.invocationCallOrder[0], ); expect(recoveryOrders[1]).toBeGreaterThan(calls.verifyWebSearch.mock.invocationCallOrder[0]); - expect(recoveryOrders[1]).toBeLessThan(calls.verify.mock.invocationCallOrder[0]); + expect(refreshOrder).toBeGreaterThan(recoveryOrders[1]); + expect(refreshOrder).toBeLessThan(calls.verify.mock.invocationCallOrder[0]); }); it("skips dashboard and gateway verification for terminal agents without forwards", async () => { diff --git a/src/lib/onboard/machine/handlers/finalization.ts b/src/lib/onboard/machine/handlers/finalization.ts index 747c7143369..8d22878a862 100644 --- a/src/lib/onboard/machine/handlers/finalization.ts +++ b/src/lib/onboard/machine/handlers/finalization.ts @@ -22,7 +22,7 @@ export interface FinalizationStateOptions; webSearchEnabled: boolean; deps: { - ensureAgentDashboardForward(sandboxName: string, agent: NonNullable): number; + ensureAgentDashboardForward(sandboxName: string, agent: Agent): number; /** * Mark this sandbox as the default. Called here (not at sandbox creation) so * a cancel at the policy-preset step never leaves an unconfigured sandbox @@ -143,10 +143,6 @@ export async function handleFinalizationState); - } - const allStagedMigrated = stagedLegacyKeys.length > 0 && stagedLegacyKeys.every((key) => migratedLegacyKeys.has(key)); const unmigratedLegacyKeys = stagedLegacyKeys.filter((key) => !migratedLegacyKeys.has(key)); @@ -188,6 +184,9 @@ export async function handleFinalizationState "balanced"); + const setPolicyTier = vi.fn(); + const syncPresetSelection = vi.fn(); + const waitForSandboxReady = vi.fn(() => true); + const waitForSandboxControlPlaneReady = vi.fn(() => controlPlaneReady); + const onSelection = vi.fn(); + const deps = { + policies: { + setupPolicyPresetSupported: vi.fn(() => true), + listSetupPolicyPresets: vi.fn(() => [{ name: "observability-otlp-local" }]), + listCustomPresets: vi.fn(() => []), + getAppliedPresets: vi.fn(() => []), + customPresetOwnsNetworkPolicyKey: vi.fn(() => false), + removeBuiltinPresetAttribution: vi.fn(), + clampSetupPolicyPresetNames: vi.fn((names: string[]) => [...names]), + }, + tiers: { + resolveTierPresets: vi.fn((tierName: string) => + tierName === "balanced" ? [{ name: "observability-otlp-local" }] : [], + ), + getTier: vi.fn(() => ({})), + }, + localInferenceProviders: [], + step: vi.fn(), + note: vi.fn(), + isNonInteractive: vi.fn(() => true), + waitForSandboxReady, + waitForSandboxControlPlaneReady, + syncPresetSelection, + selectPolicyTier, + setPolicyTier, + getRecordedPolicyTier: vi.fn(() => null), + selectTierPresetsAndAccess: vi.fn(async () => []), + parsePolicyPresetEnv: vi.fn(() => []), + env: { NEMOCLAW_POLICY_MODE: "suggested" }, + } satisfies SetupPolicySelectionDeps; + return { + deps, + onSelection, + selectPolicyTier, + setPolicyTier, + syncPresetSelection, + waitForSandboxControlPlaneReady, + waitForSandboxReady, + }; +} + +const setupOptions = { + selectedPresets: null, + tierName: "restricted", + agent: "langchain-deepagents-code", + observabilityEnabled: true, +}; + describe("policy selection after interrupted onboarding", () => { - it("reuses the recorded restricted tier before policy presets exist", async () => { - const selectPolicyTier = vi.fn(async () => "balanced"); - const setPolicyTier = vi.fn(); - const syncPresetSelection = vi.fn(); - const onSelection = vi.fn(); - const deps = { - policies: { - setupPolicyPresetSupported: vi.fn(() => true), - listSetupPolicyPresets: vi.fn(() => [{ name: "observability-otlp-local" }]), - listCustomPresets: vi.fn(() => []), - getAppliedPresets: vi.fn(() => []), - customPresetOwnsNetworkPolicyKey: vi.fn(() => false), - removeBuiltinPresetAttribution: vi.fn(), - clampSetupPolicyPresetNames: vi.fn((names: string[]) => [...names]), - }, - tiers: { - resolveTierPresets: vi.fn((tierName: string) => - tierName === "balanced" ? [{ name: "observability-otlp-local" }] : [], - ), - getTier: vi.fn(() => ({})), - }, - localInferenceProviders: [], - step: vi.fn(), - note: vi.fn(), - isNonInteractive: vi.fn(() => true), - waitForSandboxReady: vi.fn(() => true), - syncPresetSelection, + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("reuses the recorded tier and waits for sandbox re-registration after applying it (#7228)", async () => { + const { + deps, + onSelection, selectPolicyTier, setPolicyTier, - getRecordedPolicyTier: vi.fn(() => null), - selectTierPresetsAndAccess: vi.fn(async () => []), - parsePolicyPresetEnv: vi.fn(() => []), - env: { NEMOCLAW_POLICY_MODE: "suggested" }, - } satisfies SetupPolicySelectionDeps; - + syncPresetSelection, + waitForSandboxControlPlaneReady, + waitForSandboxReady, + } = createPolicySelectionHarness(); await expect( setupPoliciesWithSelection(deps, "alpha", { - selectedPresets: null, - tierName: "restricted", - agent: "langchain-deepagents-code", - observabilityEnabled: true, + ...setupOptions, onSelection, }), ).resolves.toEqual([]); @@ -55,5 +86,35 @@ describe("policy selection after interrupted onboarding", () => { expect(setPolicyTier).toHaveBeenCalledWith("alpha", "restricted"); expect(onSelection).toHaveBeenCalledWith([]); expect(syncPresetSelection).toHaveBeenCalledWith("alpha", [], []); + expect(waitForSandboxReady).toHaveBeenCalledTimes(2); + expect(waitForSandboxReady.mock.invocationCallOrder[0]).toBeLessThan( + syncPresetSelection.mock.invocationCallOrder[0], + ); + expect(waitForSandboxReady.mock.invocationCallOrder[1]).toBeGreaterThan( + syncPresetSelection.mock.invocationCallOrder[0], + ); + expect(waitForSandboxControlPlaneReady).toHaveBeenCalledOnce(); + expect(waitForSandboxControlPlaneReady.mock.invocationCallOrder[0]).toBeGreaterThan( + waitForSandboxReady.mock.invocationCallOrder[1], + ); + }); + + it("fails closed when sandbox command execution does not recover after policy application (#7228)", async () => { + const exit = vi.spyOn(process, "exit").mockImplementation((code) => { + throw new Error(`process.exit(${code})`); + }); + const { deps, syncPresetSelection, waitForSandboxControlPlaneReady } = + createPolicySelectionHarness(false); + + await expect(setupPoliciesWithSelection(deps, "alpha", setupOptions)).rejects.toThrow( + "process.exit(1)", + ); + + expect(syncPresetSelection).toHaveBeenCalledWith("alpha", [], []); + expect(waitForSandboxControlPlaneReady).toHaveBeenCalledWith("alpha"); + expect(waitForSandboxControlPlaneReady.mock.invocationCallOrder[0]).toBeGreaterThan( + syncPresetSelection.mock.invocationCallOrder[0], + ); + expect(exit).toHaveBeenCalledWith(1); }); }); diff --git a/src/lib/onboard/policy-selection.ts b/src/lib/onboard/policy-selection.ts index 73ab05461c0..10f523ff681 100644 --- a/src/lib/onboard/policy-selection.ts +++ b/src/lib/onboard/policy-selection.ts @@ -96,6 +96,7 @@ export type SetupPolicySelectionDeps = { note: (message: string) => void; isNonInteractive: () => boolean; waitForSandboxReady: (sandboxName: string) => boolean; + waitForSandboxControlPlaneReady: (sandboxName: string) => boolean; syncPresetSelection: ( sandboxName: string, currentAppliedPresets: string[], @@ -230,6 +231,23 @@ export async function setupPoliciesWithSelection( return chosen; } +function requireSandboxReady( + deps: SetupPolicySelectionDeps, + sandboxName: string, + stage: "before" | "after", +): void { + if (!deps.waitForSandboxReady(sandboxName)) { + console.error(` Sandbox '${sandboxName}' was not ready ${stage} policy application.`); + process.exit(1); + } + if (stage === "after" && !deps.waitForSandboxControlPlaneReady(sandboxName)) { + console.error( + ` Sandbox '${sandboxName}' did not re-register with OpenShell after policy application.`, + ); + process.exit(1); + } +} + async function setupPoliciesWithSelectionInner( deps: SetupPolicySelectionDeps, sandboxName: string, @@ -343,12 +361,10 @@ async function setupPoliciesWithSelectionInner( if (selectedPresets !== null) { const resumeSelection = chosen || []; if (onSelection) onSelection(resumeSelection); - if (!deps.waitForSandboxReady(sandboxName)) { - console.error(` Sandbox '${sandboxName}' was not ready for policy application.`); - process.exit(1); - } + requireSandboxReady(deps, sandboxName, "before"); deps.note(` [resume] Reapplying policy presets: ${resumeSelection.join(", ")}`); deps.syncPresetSelection(sandboxName, currentAppliedPresets, resumeSelection); + requireSandboxReady(deps, sandboxName, "after"); return resumeSelection; } @@ -450,12 +466,10 @@ async function setupPoliciesWithSelectionInner( } if (onSelection) onSelection(chosen); - if (!deps.waitForSandboxReady(sandboxName)) { - console.error(` Sandbox '${sandboxName}' was not ready for policy application.`); - process.exit(1); - } + requireSandboxReady(deps, sandboxName, "before"); deps.note(` [non-interactive] Applying policy presets: ${chosen.join(", ")}`); deps.syncPresetSelection(sandboxName, currentAppliedPresets, chosen); + requireSandboxReady(deps, sandboxName, "after"); return chosen; } @@ -489,13 +503,11 @@ async function setupPoliciesWithSelectionInner( ); if (onSelection) onSelection(interactiveChoice); - if (!deps.waitForSandboxReady(sandboxName)) { - console.error(` Sandbox '${sandboxName}' was not ready for policy application.`); - process.exit(1); - } + requireSandboxReady(deps, sandboxName, "before"); const accessByName: Record = {}; for (const preset of resolvedPresets) accessByName[preset.name] = preset.access; deps.syncPresetSelection(sandboxName, currentAppliedPresets, interactiveChoice, accessByName); + requireSandboxReady(deps, sandboxName, "after"); return interactiveChoice; } diff --git a/test/e2e/live/double-onboard.test.ts b/test/e2e/live/double-onboard.test.ts index 45bc6fe5cb9..d25a4528b32 100644 --- a/test/e2e/live/double-onboard.test.ts +++ b/test/e2e/live/double-onboard.test.ts @@ -289,11 +289,12 @@ function forwardOwnerForPort(output: string, port: string): string | undefined { async function waitForForwardOwner( sandbox: SandboxClient, port: string, - owner: string, + owner: string | undefined, artifactPrefix: string, -): Promise<{ owner: string | undefined; output: string }> { +): Promise<{ owner: string | undefined; output: string; querySucceeded: boolean }> { let observedOwner: string | undefined; let lastOutput = ""; + let querySucceeded = false; for (let attempt = 1; attempt <= PROBE_ATTEMPTS; attempt += 1) { const result = await sandbox.openshell(["forward", "list"], { artifactName: `${artifactPrefix}-attempt-${attempt}`, @@ -301,11 +302,12 @@ async function waitForForwardOwner( timeoutMs: 30_000, }); lastOutput = resultText(result); - observedOwner = forwardOwnerForPort(lastOutput, port); - if (observedOwner === owner) break; + querySucceeded = result.exitCode === 0 && !result.timedOut; + observedOwner = querySucceeded ? forwardOwnerForPort(lastOutput, port) : undefined; + if (querySucceeded && observedOwner === owner) break; if (attempt < PROBE_ATTEMPTS) await sleep(PROBE_DELAY_MS); } - return { owner: observedOwner, output: lastOutput }; + return { owner: observedOwner, output: lastOutput, querySucceeded }; } function hasOwn(object: object, key: string): boolean { @@ -448,6 +450,7 @@ test("double-onboard: reuses gateway, preserves sibling sandbox, and recovers st "onboard first sandbox", "re-onboard same sandbox on existing gateway", "onboard sibling sandbox with isolated dashboard", + "stop sibling sandbox without disturbing the first forward", "recover sandbox from stale registry", "validate gateway-stop lifecycle guidance", "remove double-onboard resources", @@ -523,6 +526,7 @@ test("double-onboard: reuses gateway, preserves sibling sandbox, and recovers st "first onboard creates a sandbox and NemoClaw gateway", "same-name recreate reuses the healthy gateway without port conflicts", "different-name onboard preserves the first sandbox and allocates distinct dashboard forwards", + "stopping one sandbox releases only its dashboard forward and reports the container stopped", "stale OpenShell deletion preserves registry metadata through status/connect and rebuild recovers it", "status after gateway stop gives explicit lifecycle guidance without deleting registry state", ], @@ -679,6 +683,55 @@ test("double-onboard: reuses gateway, preserves sibling sandbox, and recovers st ); expect(retainedForwardA.owner, retainedForwardA.output).toBe(SANDBOX_A); + progress.phase("stop sibling sandbox without disturbing the first forward"); + const stopB = await command(host, [SANDBOX_B, "stop"], { + artifactName: "phase-4-nemoclaw-stop-sandbox-b", + env: commandEnv(), + timeoutMs: 60_000, + }); + expect(stopB.exitCode, resultText(stopB)).toBe(0); + + const releasedForwardB = await waitForForwardOwner( + sandbox, + portB ?? "", + undefined, + "phase-4-openshell-forward-list-b-after-stop", + ); + expect(releasedForwardB.querySucceeded, releasedForwardB.output).toBe(true); + expect(releasedForwardB.owner, releasedForwardB.output).toBeUndefined(); + + const stoppedStatusB = await command(host, [SANDBOX_B, "status"], { + artifactName: "phase-4-nemoclaw-status-sandbox-b-after-stop", + env: commandEnv(), + timeoutMs: 60_000, + }); + const stoppedStatusTextB = resultText(stoppedStatusB); + expect(stoppedStatusB.exitCode, stoppedStatusTextB).toBe(1); + expect(stoppedStatusTextB).toContain("sandbox_container_stopped"); + expect(stoppedStatusTextB).not.toContain("sandbox_dashboard_port_conflict"); + + const retainedForwardAAfterStop = await waitForForwardOwner( + sandbox, + portA ?? "", + SANDBOX_A, + "phase-4-openshell-forward-list-a-after-b-stop", + ); + expect(retainedForwardAAfterStop.owner, retainedForwardAAfterStop.output).toBe(SANDBOX_A); + + const startB = await command(host, [SANDBOX_B, "start"], { + artifactName: "phase-4-nemoclaw-start-sandbox-b", + env: commandEnv(), + timeoutMs: PHASE_TIMEOUT_MS, + }); + expect(startB.exitCode, resultText(startB)).toBe(0); + const restoredForwardBAfterStart = await waitForForwardOwner( + sandbox, + portB ?? "", + SANDBOX_B, + "phase-4-openshell-forward-list-b-after-start", + ); + expect(restoredForwardBAfterStart.owner, restoredForwardBAfterStart.output).toBe(SANDBOX_B); + progress.phase("recover sandbox from stale registry"); // Phase 5: direct OpenShell deletion leaves a stale registry entry that // status/connect preserve and rebuild can recover. @@ -795,6 +848,15 @@ test("double-onboard: reuses gateway, preserves sibling sandbox, and recovers st thirdOnboardPreservedSibling: sandboxAAfterThird.exitCode === 0 && sandboxBAfterThird.exitCode === 0, distinctDashboardPorts: Boolean(portA && portB && portA !== portB), + selectedStopReleasedOnlySelectedForward: + stopB.exitCode === 0 && + releasedForwardB.querySucceeded && + releasedForwardB.owner === undefined && + retainedForwardAAfterStop.owner === SANDBOX_A && + stoppedStatusTextB.includes("sandbox_container_stopped") && + !stoppedStatusTextB.includes("sandbox_dashboard_port_conflict") && + startB.exitCode === 0 && + restoredForwardBAfterStart.owner === SANDBOX_B, staleRegistryRecovered: rebuild.exitCode === 0, gatewayStopGuidance: /Recovered NemoClaw gateway runtime|gateway is no longer configured after restart\/rebuild|gateway is still refusing connections after restart|gateway trust material rotated after restart/.test( diff --git a/test/onboard-preset-diff.test.ts b/test/onboard-preset-diff.test.ts index 86d0c94bdb0..0da3c71ee1f 100644 --- a/test/onboard-preset-diff.test.ts +++ b/test/onboard-preset-diff.test.ts @@ -85,6 +85,7 @@ async function runPolicyScenario({ note: () => undefined, isNonInteractive: () => true, waitForSandboxReady: () => true, + waitForSandboxControlPlaneReady: () => true, syncPresetSelection: (_sandboxName, current, selected) => { const currentSet = new Set(current); const selectedSet = new Set(selected); diff --git a/test/policy-tiers-onboard.test.ts b/test/policy-tiers-onboard.test.ts index 5aa50cf097f..801d8db5ccc 100644 --- a/test/policy-tiers-onboard.test.ts +++ b/test/policy-tiers-onboard.test.ts @@ -146,6 +146,7 @@ function createSetupHarness({ note: (message) => notes.push(message), isNonInteractive: () => nonInteractive, waitForSandboxReady: () => true, + waitForSandboxControlPlaneReady: () => true, syncPresetSelection: (sandboxName, current, selected, accessByName) => { syncCalls.push({ sandboxName, diff --git a/test/process-recovery-supervisor-relaunch.test.ts b/test/process-recovery-supervisor-relaunch.test.ts index ef262ce6aef..af899316aad 100644 --- a/test/process-recovery-supervisor-relaunch.test.ts +++ b/test/process-recovery-supervisor-relaunch.test.ts @@ -14,12 +14,16 @@ afterEach(() => { vi.unstubAllEnvs(); }); -function mockOpenClawSandbox(sandboxName: string) { +function mockOpenClawSandbox(sandboxName: string, healthTimeoutSeconds = 30) { vi.spyOn(agentRuntime, "getSessionAgent").mockReturnValue({ name: "openclaw", displayName: "OpenClaw", forwardPort: 18789, - healthProbe: { url: "http://127.0.0.1:18789/health", port: 18789, timeout_seconds: 30 }, + healthProbe: { + url: "http://127.0.0.1:18789/health", + port: 18789, + timeout_seconds: healthTimeoutSeconds, + }, } as never); vi.spyOn(registry, "getSandbox").mockReturnValue({ name: sandboxName, @@ -212,8 +216,96 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { expect(finalize).toHaveBeenCalledWith(true); }); - it("retains a healthy replacement but does not start a forward when OpenShell stays unready", () => { - mockOpenClawSandbox("unready-box"); + it("retries a busy pinned managed probe before starting the replacement forward", () => { + mockOpenClawSandbox("busy-recovered-box"); + vi.stubEnv("NEMOCLAW_GATEWAY_RECOVERY_POLL_INTERVAL_SECONDS", "0"); + vi.stubEnv("NEMOCLAW_GATEWAY_RECOVERY_WAIT_SECONDS", "1"); + vi.stubEnv("NEMOCLAW_GATEWAY_RECOVERY_SETTLE_SECONDS", "0"); + vi.stubEnv("NEMOCLAW_FORWARD_RECOVERY_WAIT_MS", "0"); + const finalize = vi.fn((supervisorReady: boolean) => + supervisorReady + ? { backupRemoved: true, rolledBack: false } + : { backupRemoved: false, rolledBack: true }, + ); + const relaunchManagedSupervisorSessionImpl = vi.fn(() => ({ + containerId: "replacement-container-id", + finalize, + })); + const requestGatewaySupervisorAction = vi.fn((_name: string, action: string) => + action === "recover" ? { status: 1, stdout: "", stderr: "SUPERVISOR_NOT_RUNNING" } : null, + ); + const acceptedProbe = { + status: 0, + stdout: "GATEWAY_PID=4242\n", + stderr: "", + }; + const requestPinnedGatewaySupervisorAction = vi + .fn() + .mockReturnValueOnce(acceptedProbe) + .mockReturnValueOnce({ status: 1, stdout: "", stderr: "SUPERVISOR_BUSY" }) + .mockReturnValue(acceptedProbe); + let forwardStarted = false; + vi.spyOn(forwardHealth, "isLocalForwardReachable").mockImplementation(() => forwardStarted); + const captureOpenshell = vi + .spyOn(openshellRuntime, "captureOpenshell") + .mockImplementation((args) => { + const command = args.join(" "); + const responses = { + "sandbox exec --name busy-recovered-box -- true": () => ({ + status: 0, + output: "", + stdout: "", + stderr: "", + }), + "forward list": () => ({ + status: 0, + output: forwardStarted + ? "SANDBOX BIND PORT PID STATUS\nbusy-recovered-box 127.0.0.1 18789 12345 running" + : "SANDBOX BIND PORT PID STATUS", + }), + }; + return ( + responses[command 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 busy-recovered-box"; + return { status: 0 } as never; + }); + + const result = checkAndRecoverSandboxProcesses("busy-recovered-box", { + quiet: true, + isSandboxGatewayRunningImpl: () => false, + requestGatewaySupervisorAction, + requestPinnedGatewaySupervisorAction, + relaunchManagedSupervisorSessionImpl, + }); + + expect(result).toMatchObject({ + checked: true, + wasRunning: false, + recovered: true, + forwardRecovered: true, + }); + expect(requestPinnedGatewaySupervisorAction).toHaveBeenCalledTimes(5); + expect(captureOpenshell).toHaveBeenCalledWith( + ["sandbox", "exec", "--name", "busy-recovered-box", "--", "true"], + expect.objectContaining({ ignoreError: true }), + ); + expect(finalize).toHaveBeenCalledWith(true); + expect(runOpenshell).toHaveBeenCalledWith( + ["forward", "start", "--background", "18789", "busy-recovered-box"], + expect.objectContaining({ ignoreError: true }), + ); + }); + + it("uses the sandbox readiness budget after a longer gateway health wait (#7273)", () => { + mockOpenClawSandbox("unready-box", 600); setImmediateRecoveryPolling(); const finalize = vi.fn(() => ({ backupRemoved: true, rolledBack: false })); const relaunchManagedSupervisorSessionImpl = vi.fn(() => ({ @@ -254,11 +346,59 @@ describe("checkAndRecoverSandboxProcesses supervisor relaunch", () => { expect(finalize).toHaveBeenCalledWith(true); expect(waitForRecreatedSandboxOpenShellReadyImpl).toHaveBeenCalledWith( "unready-box", - expect.objectContaining({ beforeProbe: expect.any(Function), timeoutSeconds: 30 }), + expect.objectContaining({ beforeProbe: expect.any(Function), timeoutSeconds: 180 }), ); expect(runOpenshell).not.toHaveBeenCalled(); }); + 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 relaunchManagedSupervisorSessionImpl = vi.fn(() => ({ + containerId: "replacement-container-id", + finalize, + })); + const requestGatewaySupervisorAction = vi.fn(() => ({ + status: 1, + stdout: "", + stderr: "SUPERVISOR_NOT_RUNNING", + })); + const acceptedProbe = { + status: 0, + stdout: "GATEWAY_PID=4242\n", + stderr: "", + }; + const requestPinnedGatewaySupervisorAction = vi + .fn() + .mockReturnValueOnce(acceptedProbe) + .mockReturnValue({ + status: 1, + stdout: "", + stderr: "SUPERVISOR_UNAVAILABLE", + }); + const captureOpenshell = vi.spyOn(openshellRuntime, "captureOpenshell"); + + const result = checkAndRecoverSandboxProcesses("managed-failed-box", { + quiet: true, + isSandboxGatewayRunningImpl: () => false, + requestGatewaySupervisorAction, + requestPinnedGatewaySupervisorAction, + relaunchManagedSupervisorSessionImpl, + }); + + expect(result).toMatchObject({ + checked: true, + wasRunning: false, + recovered: true, + forwardRecovered: false, + forwardRecoveryFailed: true, + forwardRecoveryFailureDetail: expect.stringContaining("failed the managed health guard"), + }); + expect(finalize).toHaveBeenCalledWith(true); + expect(captureOpenshell).not.toHaveBeenCalled(); + }); + it("rejects a healthy forward when the replacement identity changes after readiness", () => { mockOpenClawSandbox("drifted-box"); vi.mocked(agentRuntime.getSessionAgent).mockReturnValue({