fix(sandbox): retry forward start through OpenShell's readiness handoff

ci/source-architecture-budget.json

@@ -40,6 +40,7 @@
       "src/lib/actions/sandbox/connect.ts": 43,
       "src/lib/actions/sandbox/destroy.ts": 28,
       "src/lib/actions/sandbox/doctor.ts": 27,
+      "src/lib/actions/sandbox/forward-recovery.ts": 21,
       "src/lib/actions/sandbox/gateway-state.ts": 21,
       "src/lib/actions/sandbox/status-snapshot.ts": 19,
       "src/lib/actions/sandbox/policy-channel.ts": 30,

src/lib/actions/sandbox/connect-forward-recovery-guidance.test.ts

@@ -0,0 +1,144 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+import { afterEach, beforeEach, describe, expect, it, type MockInstance, vi } from "vitest";
+
+import { createConnectHarness } from "../../../../test/support/connect-flow-test-harness";
+import * as registry from "../../state/registry";
+import { primaryForwardRecoveryGuidance } from "./process-recovery";
+
+describe("connect forward recovery guidance", () => {
+  let exitSpy: MockInstance;
+
+  beforeEach(() => {
+    vi.stubEnv("NEMOCLAW_TEST_NO_SLEEP", "1");
+    exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number | string | null) => {
+      throw new Error(`process.exit(${code ?? 0})`);
+    }) as never);
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+    vi.unstubAllEnvs();
+  });
+
+  it.each([
+    [
+      "readiness retry exhaustion",
+      "forward-readiness-retry-limit",
+      "openshell sandbox get -g 'nemoclaw' 'alpha'` can report Ready or Running before forwarding is ready. Run `openshell forward list --gateway 'nemoclaw'`. If port 18789 has no owner, run `openshell forward start --background 18789 'alpha' --gateway 'nemoclaw'` to read the current OpenShell error",
+      true,
+    ],
+    [
+      "port ownership conflict",
+      "port-ownership-conflict",
+      "identify the current owner of port 18789 before you change either sandbox",
+      false,
+    ],
+    [
+      "unavailable forward state",
+      "forward-state-unavailable",
+      "After OpenShell reports forward state",
+      false,
+    ],
+    [
+      "unverified forward ownership",
+      "forward-ownership-unverified",
+      "confirm that 'alpha' owns port 18789",
+      false,
+    ],
+    [
+      "listener retry exhaustion",
+      "forward-listener-retry-limit",
+      "If port 18789 has no owner",
+      true,
+    ],
+    [
+      "rejected forward start",
+      "forward-start-failure",
+      "to read the OpenShell error. Correct the error",
+      true,
+    ],
+  ] as const)(
+    "prints recovery guidance for %s (#10640)",
+    async (_caseName, reason, expectedGuidance, includesManualStart) => {
+      const processCheck = {
+        checked: true,
+        wasRunning: true,
+        recovered: false,
+        forwardRecovered: false,
+        forwardRecoveryFailed: true,
+        forwardRecoveryFailureDetail: `classified ${reason}`,
+      };
+      const harness = createConnectHarness({ processCheck });
+      harness.checkAndRecoverSpy.mockImplementation((_sandboxName: unknown, options: unknown) => {
+        (
+          options as {
+            onForwardRecoveryFailure?: (failure: {
+              port: number;
+              reason: typeof reason;
+              sandboxName: string;
+            }) => void;
+          }
+        ).onForwardRecoveryFailure?.({ port: 18789, reason, sandboxName: "alpha" });
+        return processCheck;
+      });
+
+      await expect(harness.connectSandbox("alpha", { probeOnly: true })).rejects.toThrow(
+        "process.exit(1)",
+      );
+
+      const errorOutput = harness.errorSpy.mock.calls
+        .map((call) => String(call[0] ?? ""))
+        .join("\n");
+      expect(errorOutput).toContain(expectedGuidance);
+      expect(errorOutput.includes("openshell forward start --background 18789")).toBe(
+        includesManualStart,
+      );
+      expect(exitSpy).toHaveBeenCalledWith(1);
+    },
+  );
+
+  it("scopes recovery commands to the sandbox gateway (#10640)", () => {
+    vi.spyOn(registry, "getSandbox").mockReturnValue({
+      gatewayPort: 18080,
+      name: "alpha",
+    } as NonNullable<ReturnType<typeof registry.getSandbox>>);
+
+    const guidance = primaryForwardRecoveryGuidance(
+      "alpha",
+      18789,
+      "forward-readiness-retry-limit",
+    );
+
+    expect(guidance).toContain("openshell sandbox get -g 'nemoclaw-18080' 'alpha'");
+    expect(guidance).toContain("openshell forward list --gateway 'nemoclaw-18080'");
+    expect(guidance).toContain(
+      "openshell forward start --background 18789 'alpha' --gateway 'nemoclaw-18080'",
+    );
+  });
+
+  it("uses inspection guidance for an auxiliary forward failure (#10640)", async () => {
+    const harness = createConnectHarness({
+      processCheck: {
+        checked: true,
+        wasRunning: true,
+        recovered: false,
+        forwardRecovered: false,
+        forwardRecoveryFailed: true,
+        forwardRecoveryFailureDetail: "the messaging webhook host forward failed",
+        forwardRecoveryFailureScope: "auxiliary",
+      },
+    });
+
+    await expect(harness.connectSandbox("alpha", { probeOnly: true })).rejects.toThrow(
+      "process.exit(1)",
+    );
+
+    const errorOutput = harness.errorSpy.mock.calls.map((call) => String(call[0] ?? "")).join("\n");
+    expect(errorOutput).toContain("the messaging webhook host forward failed");
+    expect(errorOutput).toContain("openshell forward list --gateway 'nemoclaw'");
+    expect(errorOutput).not.toContain("openshell forward start");
+    expect(exitSpy).toHaveBeenCalledWith(1);
+  });
+});

src/lib/onboard/forward-start-recovery-retry.test.ts

@@ -0,0 +1,235 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+import fs from "node:fs";
+import path from "node:path";
+
+import { afterEach, describe, expect, it, vi } from "vitest";
+
+import { runBackgroundForwardStartWithReadinessRetry } from "./forward-start";
+import * as tempFiles from "./temp-files";
+
+const SANDBOX_NOT_READY_FORWARD_DIAGNOSTIC = `Error:   × code: 'The system is not in a state required for the operation's
+  │ execution', message: "sandbox is not ready"
+`;
+
+afterEach(() => {
+  vi.restoreAllMocks();
+});
+
+describe("runBackgroundForwardStartWithReadinessRetry", () => {
+  it("retries the readiness handoff until OpenShell accepts the forward", () => {
+    const rejections = [SANDBOX_NOT_READY_FORWARD_DIAGNOSTIC, SANDBOX_NOT_READY_FORWARD_DIAGNOSTIC];
+    const runForwardStart = vi.fn((stdio: "ignore" | ["ignore", number, number]) => {
+      const diagnostic = rejections.shift() ?? "";
+      fs.writeSync((stdio as ["ignore", number, number])[1], diagnostic);
+      return { status: Number(diagnostic !== "") };
+    });
+
+    const outcome = runBackgroundForwardStartWithReadinessRetry({
+      runForwardStart,
+      isListenerReachable: () => false,
+      isRetryAllowed: () => true,
+      sleepMs: () => {},
+    });
+
+    expect(outcome.status).toBe(0);
+    expect(runForwardStart).toHaveBeenCalledTimes(3);
+  });
+
+  it("stops retrying as soon as the caller withdraws permission", () => {
+    const runForwardStart = vi.fn((stdio: "ignore" | ["ignore", number, number]) => {
+      fs.writeSync((stdio as ["ignore", number, number])[1], SANDBOX_NOT_READY_FORWARD_DIAGNOSTIC);
+      return { status: 1 };
+    });
+
+    const outcome = runBackgroundForwardStartWithReadinessRetry({
+      runForwardStart,
+      isListenerReachable: () => false,
+      isRetryAllowed: () => false,
+      sleepMs: () => {},
+    });
+
+    expect(outcome.status).toBe(1);
+    expect(outcome.failureReason).toBe("retry-not-allowed");
+    expect(runForwardStart).toHaveBeenCalledOnce();
+  });
+
+  it.each([
+    {
+      diagnostic: "ssh exited before local forward listener opened",
+      expectedAttempts: 4,
+      expectedFailure: "listener-retry-limit",
+      name: "listener failure",
+    },
+    {
+      diagnostic: SANDBOX_NOT_READY_FORWARD_DIAGNOSTIC,
+      expectedAttempts: 13,
+      expectedFailure: "readiness-retry-limit",
+      name: "readiness handoff",
+    },
+  ] as const)(
+    "keeps the $name retry limit",
+    ({ diagnostic, expectedAttempts, expectedFailure }) => {
+      const runForwardStart = vi.fn((stdio: "ignore" | ["ignore", number, number]) => {
+        fs.writeSync((stdio as ["ignore", number, number])[1], diagnostic);
+        return { status: 1 };
+      });
+      const sleepMs = vi.fn();
+
+      const outcome = runBackgroundForwardStartWithReadinessRetry({
+        runForwardStart,
+        isListenerReachable: () => false,
+        isRetryAllowed: () => true,
+        sleepMs,
+      });
+
+      expect(outcome).toEqual({ status: 1, failureReason: expectedFailure });
+      expect(runForwardStart).toHaveBeenCalledTimes(expectedAttempts);
+      expect(sleepMs).toHaveBeenCalledTimes(expectedAttempts - 1);
+    },
+  );
+
+  it("does not restart when the listener opens during the settle", () => {
+    let listenerReachable = false;
+    const runForwardStart = vi.fn((stdio: "ignore" | ["ignore", number, number]) => {
+      fs.writeSync((stdio as ["ignore", number, number])[1], SANDBOX_NOT_READY_FORWARD_DIAGNOSTIC);
+      return { status: 1 };
+    });
+
+    const outcome = runBackgroundForwardStartWithReadinessRetry({
+      runForwardStart,
+      isListenerReachable: () => listenerReachable,
+      isRetryAllowed: () => true,
+      sleepMs: () => {
+        listenerReachable = true;
+      },
+    });
+
+    expect(outcome).toEqual({ status: 1, failureReason: "listener-reachable" });
+    expect(runForwardStart).toHaveBeenCalledOnce();
+  });
+
+  it("rechecks retry permission after the settle", () => {
+    const runForwardStart = vi.fn((stdio: "ignore" | ["ignore", number, number]) => {
+      fs.writeSync((stdio as ["ignore", number, number])[1], SANDBOX_NOT_READY_FORWARD_DIAGNOSTIC);
+      return { status: 1 };
+    });
+    const isRetryAllowed = vi.fn().mockReturnValueOnce(true).mockReturnValue(false);
+
+    const outcome = runBackgroundForwardStartWithReadinessRetry({
+      runForwardStart,
+      isListenerReachable: () => false,
+      isRetryAllowed,
+      sleepMs: () => {},
+    });
+
+    expect(outcome).toEqual({ status: 1, failureReason: "retry-not-allowed" });
+    expect(runForwardStart).toHaveBeenCalledOnce();
+    expect(isRetryAllowed).toHaveBeenCalledTimes(2);
+  });
+
+  it("still starts the forward when no diagnostic file can be created", () => {
+    vi.spyOn(tempFiles, "secureTempFile").mockImplementation(() => {
+      throw new Error("no space left on device");
+    });
+    const runForwardStart = vi.fn(() => ({ status: 0 }));
+
+    const outcome = runBackgroundForwardStartWithReadinessRetry({
+      runForwardStart,
+      isListenerReachable: () => false,
+      isRetryAllowed: () => true,
+      sleepMs: () => {},
+    });
+
+    expect(outcome.status).toBe(0);
+    expect(outcome.failureReason).toBeUndefined();
+    expect(runForwardStart).toHaveBeenCalledWith("ignore");
+  });
+
+  it("does not follow a pre-existing diagnostic link", () => {
+    const secureTempFile = tempFiles.secureTempFile;
+    const targetPath = secureTempFile("nemoclaw-forward-target", ".log");
+    fs.writeFileSync(targetPath, "sentinel", { mode: 0o600 });
+    vi.spyOn(tempFiles, "secureTempFile").mockImplementation((prefix, extension) => {
+      const diagnosticPath = secureTempFile(prefix, extension);
+      fs.symlinkSync(targetPath, diagnosticPath, "file");
+      return diagnosticPath;
+    });
+    const runForwardStart = vi.fn(() => ({ status: 0 }));
+
+    try {
+      const outcome = runBackgroundForwardStartWithReadinessRetry({
+        runForwardStart,
+        isListenerReachable: () => false,
+        isRetryAllowed: () => true,
+        sleepMs: () => {},
+      });
+
+      expect(outcome).toEqual({ status: 0 });
+      expect(runForwardStart).toHaveBeenCalledWith("ignore");
+      expect(fs.readFileSync(targetPath, "utf8")).toBe("sentinel");
+    } finally {
+      tempFiles.cleanupTempDir(targetPath, "nemoclaw-forward-target");
+    }
+  });
+
+  it("classifies the opened diagnostic after its path is replaced", () => {
+    const secureTempFile = tempFiles.secureTempFile;
+    const targetPath = secureTempFile("nemoclaw-forward-target", ".log");
+    fs.writeFileSync(targetPath, SANDBOX_NOT_READY_FORWARD_DIAGNOSTIC, { mode: 0o600 });
+    let diagnosticPath: string | undefined;
+    vi.spyOn(tempFiles, "secureTempFile").mockImplementation((prefix, extension) => {
+      diagnosticPath = secureTempFile(prefix, extension);
+      return diagnosticPath;
+    });
+    const runForwardStart = vi.fn((stdio: "ignore" | ["ignore", number, number]) => {
+      const handle = (stdio as ["ignore", number, number])[1];
+      fs.writeSync(handle, "Error: gateway authentication failed");
+      fs.unlinkSync(String(diagnosticPath));
+      fs.symlinkSync(targetPath, String(diagnosticPath), "file");
+      return { status: 1 };
+    });
+
+    try {
+      const outcome = runBackgroundForwardStartWithReadinessRetry({
+        runForwardStart,
+        isListenerReachable: () => false,
+        isRetryAllowed: () => true,
+        sleepMs: () => {},
+      });
+
+      expect(outcome).toEqual({ status: 1, failureReason: "forward-start-failure" });
+      expect(runForwardStart).toHaveBeenCalledOnce();
+    } finally {
+      tempFiles.cleanupTempDir(targetPath, "nemoclaw-forward-target");
+    }
+  });
+
+  it("preserves a successful start when diagnostic cleanup fails", () => {
+    const cleanupTempDir = tempFiles.cleanupTempDir;
+    const cleanupSpy = vi
+      .spyOn(tempFiles, "cleanupTempDir")
+      .mockImplementation((...args: Parameters<typeof tempFiles.cleanupTempDir>) => {
+        cleanupTempDir(...args);
+        throw new Error("directory is busy");
+      });
+    const warningSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+    const runForwardStart = vi.fn(() => ({ status: 0 }));
+
+    const outcome = runBackgroundForwardStartWithReadinessRetry({
+      runForwardStart,
+      isListenerReachable: () => false,
+      isRetryAllowed: () => true,
+      sleepMs: () => {},
+    });
+
+    expect(outcome).toEqual({ status: 0 });
+    expect(cleanupSpy).toHaveBeenCalledOnce();
+    const diagnosticPath = cleanupSpy.mock.calls[0]?.[0];
+    expect(diagnosticPath).toBeDefined();
+    expect(warningSpy).toHaveBeenCalledWith(
+      expect.stringContaining(path.dirname(String(diagnosticPath))),
+    );
+  });
+});

test/support/connect-flow-test-harness.ts

@@ -112,6 +112,7 @@ export type ConnectHarnessOptions = {
     forwardRecovered?: boolean;
     forwardRecoveryFailed?: boolean;
     forwardRecoveryFailureDetail?: string;
+    forwardRecoveryFailureScope?: "auxiliary";
     recoveryFailureDetail?: string;
     secretBoundaryRefused?: boolean;
     secretBoundaryReason?: SecretBoundaryRefusalReason;