diff --git a/ci/source-architecture-budget.json b/ci/source-architecture-budget.json index 82856349185..3e1e1ea1c96 100644 --- a/ci/source-architecture-budget.json +++ b/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, diff --git a/src/lib/actions/sandbox/connect-forward-recovery-guidance.test.ts b/src/lib/actions/sandbox/connect-forward-recovery-guidance.test.ts new file mode 100644 index 00000000000..d1eaade2dd9 --- /dev/null +++ b/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>); + + 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); + }); +}); diff --git a/src/lib/onboard/forward-start-recovery-retry.test.ts b/src/lib/onboard/forward-start-recovery-retry.test.ts new file mode 100644 index 00000000000..301d4f2aa95 --- /dev/null +++ b/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) => { + 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))), + ); + }); +}); diff --git a/test/support/connect-flow-test-harness.ts b/test/support/connect-flow-test-harness.ts index c295ba8551b..44c072de82c 100644 --- a/test/support/connect-flow-test-harness.ts +++ b/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;