diff --git a/ci/source-architecture-budget.json b/ci/source-architecture-budget.json index 40d38eb732f..263a2e5e642 100644 --- a/ci/source-architecture-budget.json +++ b/ci/source-architecture-budget.json @@ -6,10 +6,10 @@ "src/lib/actions/sandbox/mcp-bridge-contracts.ts": 26, "src/lib/actions/sandbox/process-recovery.ts": 27, "src/lib/adapters/docker/index.ts": 43, - "src/lib/adapters/openshell/client.ts": 23, + "src/lib/adapters/openshell/client.ts": 20, "src/lib/adapters/openshell/resolve.ts": 27, "src/lib/adapters/openshell/runtime.ts": 54, - "src/lib/adapters/openshell/timeouts.ts": 38, + "src/lib/adapters/openshell/timeouts.ts": 39, "src/lib/agent/defs.ts": 33, "src/lib/cli/branding.ts": 87, "src/lib/cli/nemoclaw-oclif-command.ts": 107, @@ -37,10 +37,11 @@ "defaultMax": 20, "maxByFile": { "src/lib/actions/inference-set.ts": 32, - "src/lib/actions/sandbox/connect.ts": 42, + "src/lib/actions/sandbox/connect.ts": 43, "src/lib/actions/sandbox/destroy.ts": 29, "src/lib/actions/sandbox/doctor.ts": 29, - "src/lib/actions/sandbox/status-snapshot.ts": 20, + "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/process-recovery.ts": 21, "src/lib/actions/sandbox/rebuild-pipeline.ts": 29, diff --git a/src/lib/actions/maintenance.test.ts b/src/lib/actions/maintenance.test.ts index e1f9b4b647d..89b84bc035c 100644 --- a/src/lib/actions/maintenance.test.ts +++ b/src/lib/actions/maintenance.test.ts @@ -8,8 +8,6 @@ const mocks = vi.hoisted(() => ({ getSandbox: vi.fn(), backupSandboxState: vi.fn(), captureSandboxListWithGatewayPreflightOrExit: vi.fn(), - parseReadySandboxNames: vi.fn(), - parseLiveSandboxNames: vi.fn(), dockerListImagesFormat: vi.fn().mockReturnValue(""), dockerRmi: vi.fn(), prompt: vi.fn(), @@ -25,6 +23,19 @@ const mocks = vi.hoisted(() => ({ withPortableHostFence: vi.fn(), })); +let readySandboxNames = new Set(); +let liveSandboxNames = new Set(); + +function sandboxInventory() { + return { + sandboxes: [...new Set([...readySandboxNames, ...liveSandboxNames])].map((name) => ({ + name, + phase: null, + readiness: readySandboxNames.has(name) ? ("ready" as const) : ("not_ready" as const), + })), + }; +} + async function runSandboxMutationAction( _sandboxName: string, action: () => unknown, @@ -57,10 +68,6 @@ vi.mock("./sandbox/snapshot/backup-authority", () => ({ vi.mock("../openshell-sandbox-list", () => ({ captureSandboxListWithGatewayPreflightOrExit: mocks.captureSandboxListWithGatewayPreflightOrExit, })); -vi.mock("../runtime-recovery", () => ({ - parseReadySandboxNames: mocks.parseReadySandboxNames, - parseLiveSandboxNames: mocks.parseLiveSandboxNames, -})); // GATEWAY_PORT is baked from NEMOCLAW_GATEWAY_PORT at module load. Pin it so // the #6520 orphan-classification tests (which run the real gateway-binding // resolvers against literal ports) don't invert on a shell that exports a @@ -108,15 +115,14 @@ describe("backupAll", () => { vi.clearAllMocks(); mocks.backupStartedSandboxState.mockReset(); delete process.env.NEMOCLAW_REQUIRE_ALL_SANDBOX_BACKUPS; - mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue({ - status: 0, - output: "sb-good\nsb-bad\n", - }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-good", "sb-bad"])); + readySandboxNames = new Set(["sb-good", "sb-bad"]); + liveSandboxNames = new Set(); + mocks.captureSandboxListWithGatewayPreflightOrExit.mockImplementation(async () => + sandboxInventory(), + ); // Defaults keep every pre-#6520 case on its original path: no sandbox is // gateway-observed (so orphan classification is decided by the absence // gate alone) and no container is ever definitively absent. - mocks.parseLiveSandboxNames.mockReturnValue(new Set()); mocks.isSandboxContainerDefinitivelyAbsent.mockReturnValue(false); mocks.startStoppedSandboxContainerForBackup.mockReturnValue(null); mocks.returnSandboxContainerToStopped.mockReturnValue(true); @@ -208,7 +214,7 @@ describe("backupAll", () => { ], defaultSandbox: "alpha", }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["alpha", "beta"])); + readySandboxNames = new Set(["alpha", "beta"]); mocks.backupSandboxState.mockImplementation((name: string) => ({ success: true, backedUpDirs: ["workspace"], @@ -238,7 +244,7 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-good" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-good"])); + readySandboxNames = new Set(["sb-good"]); mocks.backupSandboxState.mockReturnValue({ success: true, backedUpDirs: ["workspace"], @@ -271,7 +277,7 @@ describe("backupAll", () => { sandboxes: [{ name: "alpha" }, { name: "beta" }], defaultSandbox: "alpha", }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["alpha", "beta"])); + readySandboxNames = new Set(["alpha", "beta"]); mocks.withSandboxMutationLock .mockRejectedValueOnce(new Error("Timed out waiting for the sandbox mutation lock")) .mockImplementation(runSandboxMutationAction); @@ -308,7 +314,7 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-stopped" }], defaultSandbox: "sb-stopped", }); - mocks.parseReadySandboxNames.mockReturnValue(new Set()); + readySandboxNames = new Set(); mocks.startStoppedSandboxContainerForBackup.mockReturnValue({ containerName: "openshell-sb-stopped-abc", }); @@ -349,7 +355,7 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-bad" }, { name: "sb-good" }, { name: "sb-stopped" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-bad", "sb-good"])); + readySandboxNames = new Set(["sb-bad", "sb-good"]); mocks.backupSandboxState.mockImplementation((name: string) => name === "sb-bad" ? { @@ -393,7 +399,7 @@ describe("backupAll", () => { sandboxes: [{ name: "alpha" }, { name: "beta" }], defaultSandbox: "alpha", }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["alpha", "beta"])); + readySandboxNames = new Set(["alpha", "beta"]); const events: string[] = []; mocks.withSandboxMutationLock.mockImplementation( async (name: string, action: () => unknown) => { @@ -464,7 +470,7 @@ describe("backupAll", () => { sandboxes: [{ name: "alpha" }], defaultSandbox: "alpha", }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["alpha"])); + readySandboxNames = new Set(["alpha"]); mocks.openBackupShieldsWindow.mockReturnValue({ relocked: false, wasLocked: true }); mocks.backupSandboxState.mockReturnValue({ success: false, @@ -494,7 +500,7 @@ describe("backupAll", () => { sandboxes: [{ name: "alpha" }, { name: "beta" }], defaultSandbox: "alpha", }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["alpha", "beta"])); + readySandboxNames = new Set(["alpha", "beta"]); mocks.openBackupShieldsWindow.mockImplementation((name: string) => name === "alpha" ? null : { relocked: false, wasLocked: false }, ); @@ -527,7 +533,7 @@ describe("backupAll", () => { sandboxes: [{ name: "alpha" }, { name: "beta" }], defaultSandbox: "alpha", }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["alpha", "beta"])); + readySandboxNames = new Set(["alpha", "beta"]); mocks.openBackupShieldsWindow.mockReturnValue({ relocked: false, wasLocked: true }); mocks.backupSandboxState.mockReturnValue({ success: true, @@ -554,7 +560,7 @@ describe("backupAll", () => { sandboxes: [{ name: "alpha" }], defaultSandbox: "alpha", }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["alpha"])); + readySandboxNames = new Set(["alpha"]); mocks.openBackupShieldsWindow.mockReturnValue({ relocked: false, wasLocked: true }); const backupError = new Error("EACCES: permission denied, open '/var/backups/state'"); mocks.backupSandboxState.mockImplementation(() => { @@ -591,7 +597,7 @@ describe("backupAll", () => { sandboxes: [{ name: "alpha" }], defaultSandbox: "alpha", }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["alpha"])); + readySandboxNames = new Set(["alpha"]); mocks.openBackupShieldsWindow.mockReturnValue({ relocked: false, wasLocked: true }); const orphanMessage = "Agent 'alpha' not found: /agents/alpha/manifest.yaml"; mocks.backupSandboxState.mockImplementation(() => { @@ -621,7 +627,7 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-good" }, { name: "sb-stopped" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-good"])); + readySandboxNames = new Set(["sb-good"]); mocks.backupSandboxState.mockReturnValue({ success: true, backedUpDirs: ["workspace"], @@ -654,7 +660,7 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-good" }, { name: "sb-stopped" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-good"])); + readySandboxNames = new Set(["sb-good"]); mocks.backupSandboxState.mockReturnValue({ success: true, backedUpDirs: ["workspace"], @@ -701,7 +707,7 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-stopped" }], defaultSandbox: "sb-stopped", }); - mocks.parseReadySandboxNames.mockReturnValue(new Set()); + readySandboxNames = new Set(); const events: string[] = []; let lockActive = false; mocks.withSandboxMutationLock.mockImplementation( @@ -772,7 +778,7 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-stopped" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set()); + readySandboxNames = new Set(); mocks.startStoppedSandboxContainerForBackup.mockReturnValue({ containerName: "openshell-sb-stopped-abc", }); @@ -805,7 +811,7 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-stopped" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set()); + readySandboxNames = new Set(); mocks.startStoppedSandboxContainerForBackup.mockReturnValue({ containerName: "openshell-sb-stopped-abc", }); @@ -835,7 +841,7 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-stopped" }], defaultSandbox: "sb-stopped", }); - mocks.parseReadySandboxNames.mockReturnValue(new Set()); + readySandboxNames = new Set(); let lockActive = false; mocks.withSandboxMutationLock.mockImplementation( async (_name: string, action: () => unknown) => { @@ -890,7 +896,7 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-stopped" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set()); + readySandboxNames = new Set(); mocks.startStoppedSandboxContainerForBackup.mockReturnValue({ containerName: "openshell-sb-stopped-abc", }); @@ -912,7 +918,7 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-stopped" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set()); + readySandboxNames = new Set(); mocks.startStoppedSandboxContainerForBackup.mockReturnValue(null); process.env.NEMOCLAW_REQUIRE_ALL_SANDBOX_BACKUPS = "1"; const logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); @@ -963,11 +969,8 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-bad" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-bad"])); - mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue({ - status: 0, - output: "sb-bad\n", - }); + readySandboxNames = new Set(["sb-bad"]); + mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue(sandboxInventory()); mocks.backupSandboxState.mockImplementation(() => { throw new Error("Agent 'orphan' not found: /agents/orphan/manifest.yaml"); @@ -990,7 +993,7 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-orphan" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-orphan"])); + readySandboxNames = new Set(["sb-orphan"]); mocks.backupSandboxState.mockImplementation(() => { throw new Error("Agent 'orphan' not found: /agents/orphan/manifest.yaml"); }); @@ -1015,11 +1018,8 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-bad" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-bad"])); - mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue({ - status: 0, - output: "sb-bad\n", - }); + readySandboxNames = new Set(["sb-bad"]); + mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue(sandboxInventory()); mocks.backupSandboxState.mockImplementation(() => { throw new Error("EACCES: permission denied, open '/var/backups/state'"); @@ -1040,11 +1040,8 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-bad" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-bad"])); - mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue({ - status: 0, - output: "sb-bad\n", - }); + readySandboxNames = new Set(["sb-bad"]); + mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue(sandboxInventory()); mocks.backupSandboxState.mockImplementation(() => { throw new Error("Agent 'phantom' not found"); @@ -1064,11 +1061,8 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-bad" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-bad"])); - mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue({ - status: 0, - output: "sb-bad\n", - }); + readySandboxNames = new Set(["sb-bad"]); + mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue(sandboxInventory()); mocks.backupSandboxState.mockImplementation(() => { throw new Error("Agent 'phantom' not found: /agents/phantom/binary"); @@ -1125,7 +1119,7 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-bad" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-bad"])); + readySandboxNames = new Set(["sb-bad"]); mocks.backupSandboxState.mockReturnValue({ success: false, unreachable: true, @@ -1157,11 +1151,8 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-bad" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-bad"])); - mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue({ - status: 0, - output: "sb-bad\n", - }); + readySandboxNames = new Set(["sb-bad"]); + mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue(sandboxInventory()); mocks.backupSandboxState.mockImplementation(() => ({ success: false, unreachable: true, @@ -1203,8 +1194,8 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-good" }, { name: "sb-stranded" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["sb-good"])); - mocks.parseLiveSandboxNames.mockReturnValue(new Set(["sb-good"])); + readySandboxNames = new Set(["sb-good"]); + liveSandboxNames = new Set(["sb-good"]); mocks.isSandboxContainerDefinitivelyAbsent.mockImplementation( (name: string) => name === "sb-stranded", ); @@ -1255,8 +1246,8 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-other", gatewayPort: 9999 }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set()); - mocks.parseLiveSandboxNames.mockReturnValue(new Set()); + readySandboxNames = new Set(); + liveSandboxNames = new Set(); mocks.isSandboxContainerDefinitivelyAbsent.mockReturnValue(true); process.env.NEMOCLAW_REQUIRE_ALL_SANDBOX_BACKUPS = "1"; const logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); @@ -1285,8 +1276,8 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-reconnecting" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set()); - mocks.parseLiveSandboxNames.mockReturnValue(new Set()); + readySandboxNames = new Set(); + liveSandboxNames = new Set(); mocks.isSandboxContainerDefinitivelyAbsent.mockReturnValue(false); process.env.NEMOCLAW_REQUIRE_ALL_SANDBOX_BACKUPS = "1"; const logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); @@ -1312,16 +1303,12 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-flapping" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set()); + readySandboxNames = new Set(); mocks.captureSandboxListWithGatewayPreflightOrExit - .mockResolvedValueOnce({ status: 0, output: "" }) + .mockResolvedValueOnce({ sandboxes: [] }) .mockResolvedValueOnce({ - status: 0, - output: "sb-flapping openshell 2026-07-21 10:00:00 Ready\n", + sandboxes: [{ name: "sb-flapping", phase: null, readiness: "ready" }], }); - mocks.parseLiveSandboxNames.mockImplementation((output: string) => - output.includes("sb-flapping") ? new Set(["sb-flapping"]) : new Set(), - ); mocks.isSandboxContainerDefinitivelyAbsent.mockReturnValue(true); process.env.NEMOCLAW_REQUIRE_ALL_SANDBOX_BACKUPS = "1"; const logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); @@ -1345,12 +1332,9 @@ describe("backupAll", () => { sandboxes: [{ name: "sb-flapping" }], defaultSandbox: null, }); - mocks.parseReadySandboxNames.mockReturnValue(new Set()); - mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue({ - status: 0, - output: "", - }); - mocks.parseLiveSandboxNames.mockReturnValue(new Set()); + readySandboxNames = new Set(); + mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue(sandboxInventory()); + liveSandboxNames = new Set(); mocks.isSandboxContainerDefinitivelyAbsent.mockReturnValueOnce(true).mockReturnValueOnce(false); process.env.NEMOCLAW_REQUIRE_ALL_SANDBOX_BACKUPS = "1"; const logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); diff --git a/src/lib/actions/maintenance.ts b/src/lib/actions/maintenance.ts index e13da54b113..14751692712 100644 --- a/src/lib/actions/maintenance.ts +++ b/src/lib/actions/maintenance.ts @@ -21,7 +21,6 @@ import { import { SANDBOX_IMAGE_REPOS } from "../domain/sandbox/image-tag"; import { resolveGatewayName, resolveSandboxGatewayName } from "../onboard/gateway-binding"; import { captureSandboxListWithGatewayPreflightOrExit } from "../openshell-sandbox-list"; -import { parseLiveSandboxNames, parseReadySandboxNames } from "../runtime-recovery"; import { withSandboxMutationLock } from "../state/mcp-lifecycle-lock"; import * as registry from "../state/registry"; import * as sandboxState from "../state/sandbox"; @@ -303,7 +302,11 @@ async function backupAllWithoutPortableAuthority(): Promise { }, { gatewayName: selectedGatewayName }, ); - const readyNames = parseReadySandboxNames(liveList.output || ""); + const readyNames = new Set( + liveList.sandboxes + .filter((sandbox) => sandbox.readiness === "ready") + .map((sandbox) => sandbox.name), + ); // Source-of-truth review (#6520): // // - Invalid state: a sandbox the selected gateway does not observe, whose @@ -332,7 +335,7 @@ async function backupAllWithoutPortableAuthority(): Promise { // candidate the gateway observes again reverts to a genuine strict skip. const orphanNames = new Set( classifyOrphanedRegistrySandboxes(sandboxes, { - observedNames: parseLiveSandboxNames(liveList.output || ""), + observedNames: new Set(liveList.sandboxes.map((sandbox) => sandbox.name)), reconnectedNames: new Set(), selectedGatewayName, resolveGatewayBinding: resolveSandboxGatewayName, @@ -448,7 +451,7 @@ async function backupAllWithoutPortableAuthority(): Promise { }, { gatewayName: selectedGatewayName }, ); - const observedOnRecheck = parseLiveSandboxNames(confirmation.output || ""); + const observedOnRecheck = new Set(confirmation.sandboxes.map((sandbox) => sandbox.name)); confirmedStranded = strandedOrphans.filter( (name) => !observedOnRecheck.has(name) && isSandboxContainerDefinitivelyAbsent(name), ); diff --git a/src/lib/actions/sandbox/agent/passthrough-ollama-recovery.test.ts b/src/lib/actions/sandbox/agent/passthrough-ollama-recovery.test.ts index 7594cd04110..d8516b67e5f 100644 --- a/src/lib/actions/sandbox/agent/passthrough-ollama-recovery.test.ts +++ b/src/lib/actions/sandbox/agent/passthrough-ollama-recovery.test.ts @@ -147,9 +147,11 @@ function makePassthroughDeps( getSandbox: ((name) => ({ name, agent: "openclaw", ...route })) as NonNullable< AgentPassthroughDeps["getSandbox"] >, - ensureLive: (async () => ({ state: "present", output: "Phase: Ready" })) as NonNullable< - AgentPassthroughDeps["ensureLive"] - >, + ensureLive: (async () => ({ + state: "present", + phase: "Ready", + output: "Phase: Ready", + })) as NonNullable, execNonJson: ((): never => { events.push("dispatch"); throw new Error("__exit:0"); diff --git a/src/lib/actions/sandbox/agent/passthrough-shields-warning.test.ts b/src/lib/actions/sandbox/agent/passthrough-shields-warning.test.ts index 7b62d05544e..8a1cd55dbaa 100644 --- a/src/lib/actions/sandbox/agent/passthrough-shields-warning.test.ts +++ b/src/lib/actions/sandbox/agent/passthrough-shields-warning.test.ts @@ -10,7 +10,7 @@ import type { ShieldsAutoRestoreReadResult } from "../../../shields/audit"; const execMock = vi.hoisted(() => vi.fn(async () => {})); const ensureLiveMock = vi.hoisted(() => - vi.fn(async () => ({ state: "present", output: "Phase: Ready" }) as { output?: string }), + vi.fn(async () => ({ state: "present", phase: "Ready", output: "Phase: Ready" })), ); const getSandboxMock = vi.hoisted(() => vi.fn(() => ({ agent: "openclaw" }))); const listAgentsMock = vi.hoisted(() => vi.fn(() => ["langchain-deepagents-code", "openclaw"])); @@ -210,12 +210,10 @@ describe("runAgentPassthrough shields-relock warning", () => { it("does not consult OpenClaw relock history for terminal-runtime passthroughs (#5922)", async () => { getSandboxMock.mockReturnValueOnce({ agent: "langchain-deepagents-code" }); - const getRecentShieldsAutoRestore = vi.fn( - (): ShieldsAutoRestoreReadResult => ({ - kind: "event", - event: { timestamp: new Date().toISOString(), timeoutSeconds: 20 }, - }), - ); + const getRecentShieldsAutoRestore = vi.fn((): ShieldsAutoRestoreReadResult => ({ + kind: "event", + event: { timestamp: new Date().toISOString(), timeoutSeconds: 20 }, + })); const { writes, proc } = makeProcMock(); await runAgentPassthrough( diff --git a/src/lib/actions/sandbox/agent/passthrough.test.ts b/src/lib/actions/sandbox/agent/passthrough.test.ts index 9ba1369f180..ddb49b96d15 100644 --- a/src/lib/actions/sandbox/agent/passthrough.test.ts +++ b/src/lib/actions/sandbox/agent/passthrough.test.ts @@ -5,7 +5,14 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const execMock = vi.hoisted(() => vi.fn(async () => {})); const ensureLiveMock = vi.hoisted(() => - vi.fn(async () => ({ state: "present", output: "Phase: Ready" }) as { output?: string }), + vi.fn( + async () => + ({ state: "present", phase: "Ready", output: "Phase: Ready" }) as { + state: string; + phase: string | null; + output: string; + }, + ), ); const getSandboxMock = vi.hoisted(() => vi.fn( @@ -164,10 +171,7 @@ describe("runAgentPassthrough", () => { headless_command: "python3 /app/run_with_harness.py", }, }); - await runAgentPassthrough( - "alpha", - { extraArgs: ["start", "--task-id", "demo"] }, - ); + await runAgentPassthrough("alpha", { extraArgs: ["start", "--task-id", "demo"] }); expect(execMock).toHaveBeenCalledWith( "alpha", ["python3", "/app/run_with_harness.py", "start", "--task-id", "demo"], @@ -599,7 +603,11 @@ describe("runAgentPassthrough", () => { }); it("prints recovery hints with exit 1 before selector rejection for the literal stopped-sandbox repro `agent -m ping` (#5655)", async () => { - ensureLiveMock.mockResolvedValueOnce({ output: "Phase: Error" }); + ensureLiveMock.mockResolvedValueOnce({ + state: "present", + phase: "Error", + output: "Phase: Error", + }); getSandboxMock.mockReturnValueOnce({ agent: "openclaw" }); const { writes, exit, proc } = makeProcMock(); await expect( @@ -652,7 +660,11 @@ describe("runAgentPassthrough", () => { }); it("rejects with exit 1 + recovery hints when sandbox phase is non-Ready", async () => { - ensureLiveMock.mockResolvedValueOnce({ output: "Phase: Error" }); + ensureLiveMock.mockResolvedValueOnce({ + state: "present", + phase: "Error", + output: "Phase: Error", + }); getSandboxMock.mockReturnValueOnce({ agent: "openclaw" }); const { writes, exit, proc } = makeProcMock(); await expect( @@ -673,8 +685,12 @@ describe("runAgentPassthrough", () => { expect(all).toMatch(/onboard --resume/); }); - it("fails closed with exit 2 when ensureLive returns output without a parseable Phase line, never invoking exec", async () => { - ensureLiveMock.mockResolvedValueOnce({ output: "Name: alpha\n(no phase line here)\n" }); + it("fails closed with exit 2 when ensureLive returns no observed phase, never invoking exec", async () => { + ensureLiveMock.mockResolvedValueOnce({ + state: "present", + phase: null, + output: "Name: alpha\n(no phase line here)\n", + }); getSandboxMock.mockReturnValueOnce({ agent: "openclaw" }); const { writes, exit, proc } = makeProcMock(); await expect( @@ -781,10 +797,15 @@ describe("runAgentNonJsonPassthrough", () => { const { proc } = makeNonJsonProcMock(); const runDispatchMock = makeDispatchMock("PONG\n", "", 0); await expect( - runAgentNonJsonPassthrough("my-sb", ["openclaw", "agent", "--agent", "main", "-m", "ping"], proc, { - getOpenshellBinary: stubBinary, - runDispatch: runDispatchMock, - }), + runAgentNonJsonPassthrough( + "my-sb", + ["openclaw", "agent", "--agent", "main", "-m", "ping"], + proc, + { + getOpenshellBinary: stubBinary, + runDispatch: runDispatchMock, + }, + ), ).rejects.toThrow("__exit:0"); expect(buildOpenshellExecArgsMock.mock.calls[0]?.[2]?.timeoutSeconds).toBeUndefined(); }); diff --git a/src/lib/actions/sandbox/agent/passthrough.ts b/src/lib/actions/sandbox/agent/passthrough.ts index 8fdbd7574e3..318ccea1a34 100644 --- a/src/lib/actions/sandbox/agent/passthrough.ts +++ b/src/lib/actions/sandbox/agent/passthrough.ts @@ -116,7 +116,6 @@ import { CLI_NAME } from "../../../cli/branding"; import { isStdinTty } from "../../../core/stdin"; import { resolveSandboxHermesApiPort } from "../../../onboard/hermes-api-port"; import type { ShieldsAutoRestoreReadResult } from "../../../shields/audit"; -import { parseSandboxPhase } from "../../../state/gateway"; import * as registry from "../../../state/registry"; import { buildOpenshellExecArgs, @@ -541,7 +540,7 @@ export async function runAgentPassthrough( if (!command) return; const ensureLive = deps.ensureLive ?? ensureLiveSandboxOrExit; const state = await ensureLive(sandboxName, { allowNonReadyPhase: true }); - const phase = parseSandboxPhase(state?.output ?? ""); + const phase = state?.phase ?? null; if (!phase) { rejectUnparseablePhase(sandboxName, proc); } diff --git a/src/lib/actions/sandbox/connect.ts b/src/lib/actions/sandbox/connect.ts index d1e3b0cdc29..60cb0321e26 100644 --- a/src/lib/actions/sandbox/connect.ts +++ b/src/lib/actions/sandbox/connect.ts @@ -2,6 +2,13 @@ // SPDX-License-Identifier: Apache-2.0 import { spawnSync } from "node:child_process"; +import { createCliOpenShellSandboxObserver } from "../../adapters/openshell/sandbox-observer-cli"; +import { + namedOpenShellGateway, + type OpenShellSandboxError, + type OpenShellSandboxObservation, + type OpenShellSandboxObserver, +} from "../../adapters/openshell/sandbox-observer"; import { resolveOpenshell } from "../../adapters/openshell/resolve"; import { captureOpenshell, @@ -43,13 +50,6 @@ import { isWsl } from "../../platform"; import { ROOT } from "../../runner"; import * as sandboxVersion from "../../sandbox/version"; import { redact, redactFull } from "../../security/redact"; -import { - isSandboxReady, - isTerminalSandboxPhase, - parseSandboxPhase, - parseSandboxStatus, - TERMINAL_SANDBOX_PHASES, -} from "../../state/gateway"; import type { SandboxEntry } from "../../state/registry"; import * as registry from "../../state/registry"; import { @@ -85,6 +85,8 @@ import { printGatewayLifecycleHint, qualifyPortableAgentLifecycleAuthority, recoverPortableDemoSandboxLifecycleForConnect, + isTerminalSandboxPhase, + TERMINAL_SANDBOX_PHASES, requireHermesPortableActiveLifecycleAuthority, startStoppedSandboxContainerForProbeRecovery, withConnectSandboxLifecycleLock, @@ -137,11 +139,6 @@ type SpawnLikeResult = { signal?: NodeJS.Signals | null; }; -type SandboxListProbe = { - status: number | null; - output: string; -}; - export type SandboxInferenceRouteProbe = { healthy: boolean; broken: boolean; @@ -570,8 +567,38 @@ function failConnectReadinessGatewayUnavailable(sandboxName: string, detailOutpu process.exit(1); } -function outputShowsGatewayUnavailable(output = ""): boolean { - return GATEWAY_UNAVAILABLE_RE.test(output); +function failConnectReadinessObservation(sandboxName: string, error: OpenShellSandboxError): never { + if (error.kind === "transport") { + failConnectReadinessGatewayUnavailable(sandboxName, error.message); + } + + console.error(""); + switch (error.kind) { + case "authentication": + console.error( + ` OpenShell could not authenticate while checking sandbox '${sandboxName}' readiness.`, + ); + console.error(" Restore authentication for the sandbox's recorded gateway, then retry."); + break; + case "schema": + console.error( + ` The OpenShell CLI and gateway schemas do not match; cannot verify sandbox '${sandboxName}' readiness.`, + ); + console.error(" Use matching supported OpenShell CLI and gateway versions, then retry."); + break; + case "timeout": + console.error(` The OpenShell readiness request for sandbox '${sandboxName}' timed out.`); + console.error(" Check gateway health and retry after it responds."); + break; + case "command": + console.error(` The OpenShell readiness request for sandbox '${sandboxName}' failed.`); + console.error( + ` Run \`${CLI_NAME} ${sandboxName} status\` to inspect the failure before retrying.`, + ); + break; + } + console.error(` ${error.message}`); + process.exit(1); } // Fail fast with Docker-outage guidance instead of polling to the readiness @@ -1187,10 +1214,7 @@ function exitWithConnectSpawnResult(sandboxName: string, result: SpawnLikeResult type WaitForSandboxReadyOptions = { allowInitialErrorAfterStart?: boolean; allowDockerRuntimeInspection?: boolean; - captureSandboxList?: ( - args: string[], - options: { readonly ignoreError: true; readonly timeout: number }, - ) => ReturnType; + observer?: OpenShellSandboxObserver; defaultTimeoutSec?: number; retryCommand?: string; successLogs?: readonly string[]; @@ -1212,17 +1236,20 @@ const START_INITIAL_ERROR_GRACE_POLLS = 10; export const SANDBOX_REPAIR_READY_TIMEOUT_SEC = 300; /** Wait for a sandbox to become ready, exiting with recovery guidance on terminal failure. */ -export function waitForSandboxReadyOrExit( +export async function waitForSandboxReadyOrExit( sandboxName: string, { allowInitialErrorAfterStart = false, allowDockerRuntimeInspection = true, - captureSandboxList = captureOpenshell, + observer = createCliOpenShellSandboxObserver({ + capture: captureOpenshell, + defaultTimeoutMs: OPENSHELL_PROBE_TIMEOUT_MS, + }), defaultTimeoutSec = 120, retryCommand = "connect", successLogs = [], }: WaitForSandboxReadyOptions = {}, -): void { +): Promise { const rawTimeout = process.env.NEMOCLAW_CONNECT_TIMEOUT; let timeout = defaultTimeoutSec; if (rawTimeout !== undefined) { @@ -1241,27 +1268,26 @@ export function waitForSandboxReadyOrExit( const gatewayName = getSandboxTargetGatewayName(sandboxName); const elapsedSec = () => Math.floor((Date.now() - startedAt) / 1000); const remainingMs = () => Math.max(1, deadline - Date.now()); - const runSandboxList = (): SandboxListProbe => { + const observeSandbox = async (): Promise => { // Gateway selection is process-global and another CLI can change it while // this command waits. Pin each poll to the registry-recorded owner so a // same-named sandbox on a sibling gateway cannot satisfy readiness. - const result = captureSandboxList(["sandbox", "list", "-g", gatewayName], { - ignoreError: true, - timeout: remainingMs(), + const result = await observer.listSandboxes({ + target: namedOpenShellGateway(gatewayName), + timeoutMs: remainingMs(), }); - return { status: result.status, output: result.output }; + if (!result.ok) { + if (result.error.kind === "timeout" && Date.now() >= deadline) return null; + failConnectReadinessObservation(sandboxName, result.error); + } + return result.value.sandboxes.find((sandbox) => sandbox.name === sandboxName) ?? null; }; - const listProbe = runSandboxList(); - const listCommandFailed = listProbe.status !== 0; - if (listCommandFailed && outputShowsGatewayUnavailable(listProbe.output)) { - failConnectReadinessGatewayUnavailable(sandboxName, listProbe.output); - } - const list = listProbe.output; - if (isSandboxReady(list, sandboxName)) return; + const initial = await observeSandbox(); + if (initial?.readiness === "ready") return; - const status = parseSandboxStatus(list, sandboxName); - if (!listCommandFailed && status && /^unknown$/i.test(status)) { + const status = initial?.phase ?? null; + if (status && /^unknown$/i.test(status)) { failIfGatewayBlocksConnectReadiness(sandboxName); } let remainingInitialErrorGracePolls = @@ -1283,21 +1309,18 @@ export function waitForSandboxReadyOrExit( while (Date.now() < deadline) { const sleepFor = Math.min(interval, remainingMs() / 1000); if (sleepFor <= 0) break; - spawnSync("sleep", [String(sleepFor)]); - const pollProbe = runSandboxList(); - const pollCommandFailed = pollProbe.status !== 0; - if (pollCommandFailed && outputShowsGatewayUnavailable(pollProbe.output)) { - failConnectReadinessGatewayUnavailable(sandboxName, pollProbe.output); + if (process.env.VITEST !== "true" && process.env.NEMOCLAW_TEST_NO_SLEEP !== "1") { + await new Promise((resolve) => setTimeout(resolve, sleepFor * 1000)); } - const poll = pollProbe.output; + const poll = await observeSandbox(); const elapsed = elapsedSec(); - if (isSandboxReady(poll, sandboxName)) { + if (poll?.readiness === "ready") { ready = true; break; } - const parsedCur = parseSandboxStatus(poll, sandboxName); + const parsedCur = poll?.phase ?? null; const cur = parsedCur || "unknown"; - if (!pollCommandFailed && parsedCur && /^unknown$/i.test(parsedCur)) { + if (parsedCur && /^unknown$/i.test(parsedCur)) { failIfGatewayBlocksConnectReadiness(sandboxName); } if (cur !== "unknown") everSeen = true; @@ -1486,7 +1509,7 @@ async function runConnectEntryPreflight( gatewayRecovery: probeOnly ? "observe" : "recover", }), ); - const livePhase = parseSandboxPhase(live.output || ""); + const livePhase = live.phase ?? null; if ( livePhase && livePhase !== "Ready" && @@ -1611,11 +1634,14 @@ export async function prepareInteractiveSession(sandboxName: string): Promise<{ } // Ensure Ollama auth proxy is running (recovers from host reboots) if (!hermesPortable) ensureOllamaAuthProxy(); - waitForSandboxReadyOrExit(sandboxName, { + await waitForSandboxReadyOrExit(sandboxName, { allowDockerRuntimeInspection: !hermesPortable, - captureSandboxList: hermesPortable - ? (args, captureOptions) => - captureHermesPortableOpenShell(sandboxName, args, captureOptions) + observer: hermesPortable + ? createCliOpenShellSandboxObserver({ + capture: (args, captureOptions) => + captureHermesPortableOpenShell(sandboxName, args, captureOptions), + defaultTimeoutMs: OPENSHELL_PROBE_TIMEOUT_MS, + }) : undefined, successLogs: [" Sandbox is ready. Connecting..."], }); @@ -1783,12 +1809,15 @@ async function prepareConnectSandboxWithinLifecycleFence( startStoppedSandboxContainerForProbeRecovery(sandboxName), ); } - probeTiming!.measure("gateway", () => + await probeTiming!.measureAsync("gateway", () => waitForSandboxReadyOrExit(sandboxName, { allowDockerRuntimeInspection: !hermesPortable, - captureSandboxList: hermesPortable - ? (args, captureOptions) => - captureHermesPortableOpenShell(sandboxName, args, captureOptions) + observer: hermesPortable + ? createCliOpenShellSandboxObserver({ + capture: (args, captureOptions) => + captureHermesPortableOpenShell(sandboxName, args, captureOptions), + defaultTimeoutMs: OPENSHELL_PROBE_TIMEOUT_MS, + }) : undefined, defaultTimeoutSec: SANDBOX_REPAIR_READY_TIMEOUT_SEC, retryCommand: "connect --probe-only", diff --git a/src/lib/actions/sandbox/doctor-flow.test.ts b/src/lib/actions/sandbox/doctor-flow.test.ts index c3d299f7090..c7f1147a987 100644 --- a/src/lib/actions/sandbox/doctor-flow.test.ts +++ b/src/lib/actions/sandbox/doctor-flow.test.ts @@ -951,7 +951,7 @@ describe("runSandboxDoctor flow", () => { gatewayName: "nemoclaw-19080", }); expect(harness.captureOpenShellSpy).toHaveBeenCalledWith( - ["sandbox", "list"], + ["sandbox", "list", "-g", "nemoclaw-19080"], expect.any(Object), ); expect(harness.probeSandboxInferenceGatewayHealthSpy).toHaveBeenCalledWith("alpha"); diff --git a/src/lib/actions/sandbox/doctor-observation-failure.test.ts b/src/lib/actions/sandbox/doctor-observation-failure.test.ts new file mode 100644 index 00000000000..17659f07b13 --- /dev/null +++ b/src/lib/actions/sandbox/doctor-observation-failure.test.ts @@ -0,0 +1,149 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { OpenShellSandboxError } from "../../adapters/openshell/sandbox-observer"; + +const mocks = vi.hoisted(() => ({ + listSandboxes: vi.fn(), +})); + +vi.mock("../../adapters/openshell/sandbox-observer-cli", () => ({ + createCliOpenShellSandboxObserver: () => ({ listSandboxes: mocks.listSandboxes }), + stripOpenShellCliAnsi: (value: string) => value, +})); + +vi.mock("../../adapters/openshell/resolve", () => ({ + resolveOpenshell: () => "/usr/bin/openshell", +})); + +vi.mock("../../adapters/openshell/runtime", () => ({ + captureOpenshell: () => ({ status: 0, output: "" }), +})); + +vi.mock("../../agent/defs", () => ({ + getAgentRuntimeKind: () => "gateway", + loadAgent: () => ({ name: "openclaw" }), +})); + +vi.mock("../../gateway-runtime-action", () => ({ + getNamedGatewayLifecycleState: () => ({ + state: "healthy_named", + status: "Status: Connected", + gatewayInfo: "Gateway: nemoclaw-19080", + }), + recoverNamedGatewayRuntime: vi.fn(), +})); + +vi.mock("../../onboard/gateway-binding", () => ({ + resolveGatewayName: () => "nemoclaw-19080", + resolveSandboxGatewayName: () => "nemoclaw-19080", +})); + +vi.mock("../../onboard/runtime-provider/access", () => ({ + CURRENT_RUNTIME_PROVIDER_BUNDLES: [], + RuntimeProviderSelectionError: class RuntimeProviderSelectionError extends Error {}, + requireRuntimeProviderBundle: vi.fn(), + resolveCurrentRuntimeProviderBundle: () => ({ + preflightDoctor: { + inspectHost: () => ({ + group: "Host", + label: "Runtime provider", + status: "ok", + detail: "available", + }), + }, + }), +})); + +vi.mock("../../state/registry", () => ({ + getSandbox: () => null, + getBaselineExclusionTransition: () => null, + getBaselineExclusions: () => [], +})); + +vi.mock("./doctor-inference", () => ({ + collectInferenceChecks: () => [], + collectManagedLlamaCppDoctorChecks: () => [], + resolveDoctorReasoningEffort: () => undefined, +})); + +vi.mock("./doctor-system-checks", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + cloudflaredDoctorCheck: () => ({ + group: "Local services", + label: "cloudflared", + status: "info", + detail: "not inspected", + }), + inspectSandboxDoctorPortableAuthority: () => ({ kind: "absent" }), + ollamaDoctorCheck: () => ({ + group: "Local services", + label: "Ollama", + status: "info", + detail: "not inspected", + }), + shouldInspectLegacyGatewayContainer: () => false, + withSandboxDoctorLifecycleLock: async ( + _sandboxName: string, + operation: () => Promise, + ) => await operation(), + }; +}); + +import { runSandboxDoctor } from "./doctor"; + +describe("doctor live sandbox observation", () => { + beforeEach(() => { + mocks.listSandboxes.mockReset(); + }); + + it.each<{ + label: string; + error: OpenShellSandboxError; + expectedDetail: string; + expectedHint: string; + }>([ + { + label: "authentication", + error: { + kind: "authentication", + message: "OpenShell could not authenticate the sandbox observation.", + }, + expectedDetail: "OpenShell could not authenticate the sandbox observation.", + expectedHint: "restore OpenShell authentication for gateway 'nemoclaw-19080'", + }, + { + label: "transport", + error: { + kind: "transport", + reason: "unreachable", + message: "OpenShell could not reach the selected gateway.", + }, + expectedDetail: "OpenShell could not reach the selected gateway.", + expectedHint: "run `openshell status`, restore gateway 'nemoclaw-19080'", + }, + ])( + "reports a failed $label observation without classifying the sandbox as absent (#9803)", + async ({ error, expectedDetail, expectedHint }) => { + mocks.listSandboxes.mockResolvedValue({ ok: false, error }); + + const report = await runSandboxDoctor("alpha", ["--json"], { quietJson: true }); + const liveSandbox = report?.checks.find( + (check) => check.group === "Sandbox" && check.label === "Live sandbox", + ); + + expect(liveSandbox).toMatchObject({ + status: "fail", + detail: expect.stringContaining(expectedDetail), + hint: expect.stringContaining(expectedHint), + }); + const rendered = `${liveSandbox?.detail ?? ""}\n${liveSandbox?.hint ?? ""}`; + expect(rendered).not.toContain("not present"); + expect(rendered).not.toContain("recreate"); + expect(rendered).not.toContain("credential-value"); + }, + ); +}); diff --git a/src/lib/actions/sandbox/doctor.ts b/src/lib/actions/sandbox/doctor.ts index 83ccf86f424..68d61a1c441 100644 --- a/src/lib/actions/sandbox/doctor.ts +++ b/src/lib/actions/sandbox/doctor.ts @@ -3,7 +3,14 @@ import fs from "node:fs"; import path from "node:path"; -import { stripAnsi } from "../../adapters/openshell/client"; +import { + createCliOpenShellSandboxObserver, + stripOpenShellCliAnsi, +} from "../../adapters/openshell/sandbox-observer-cli"; +import { + namedOpenShellGateway, + type OpenShellSandboxError, +} from "../../adapters/openshell/sandbox-observer"; import { resolveOpenshell } from "../../adapters/openshell/resolve"; import { captureOpenshell } from "../../adapters/openshell/runtime"; import { OPENSHELL_PROBE_TIMEOUT_MS } from "../../adapters/openshell/timeouts"; @@ -31,7 +38,6 @@ import { type BaselineExclusionRuntimeStatus, } from "../../policy/baseline-exclusion"; import { ROOT } from "../../runner"; -import { parseLiveSandboxNames } from "../../runtime-recovery"; import * as sandboxVersion from "../../sandbox/version"; import * as shields from "../../shields"; import type { SandboxEntry } from "../../state/registry"; @@ -59,8 +65,6 @@ import { import { cloudflaredDoctorCheck, dockerInspectGateway, - findSandboxListLine, - inferSandboxReadyFromLine, inspectSandboxDoctorPortableAuthority, ollamaDoctorCheck, oneLine, @@ -242,7 +246,7 @@ async function probeOpenShellGateway( connected: boolean; }> { const lifecycle = await gatewayLifecycle(gatewayName, recoverGateway); - const cleanStatus = stripAnsi(lifecycle?.status || ""); + const cleanStatus = stripOpenShellCliAnsi(lifecycle?.status || ""); const connected = lifecycle?.state === "healthy_named"; return { connected, @@ -262,11 +266,11 @@ function liveSandboxDetail( sandboxName: string, present: boolean, ready: boolean | null, - line: string | null, + phase: string | null, ): string { if (!present) return `${sandboxName} not present in live OpenShell sandbox list`; if (ready) return `${sandboxName} present (Ready)`; - return `${sandboxName} present${line ? ` (${oneLine(line)})` : ""}`; + return `${sandboxName} present${phase ? ` (${oneLine(phase)})` : ""}`; } function liveSandboxHint( @@ -281,15 +285,52 @@ function liveSandboxHint( return `run \`${CLI_NAME} ${sandboxName} status\` or \`${CLI_NAME} ${sandboxName} logs --follow\``; } -function liveSandboxCheck(sandboxName: string): SandboxProbe { - const list = captureOpenshell(["sandbox", "list"], { - ignoreError: true, - timeout: OPENSHELL_PROBE_TIMEOUT_MS, +function liveSandboxObservationFailureHint( + sandboxName: string, + gatewayName: string, + error: OpenShellSandboxError, +): string { + switch (error.kind) { + case "authentication": + return `restore OpenShell authentication for gateway '${gatewayName}', then retry`; + case "transport": + return error.reason === "identity_mismatch" + ? `run \`${CLI_NAME} ${sandboxName} status\` to inspect the recorded gateway identity, then retry` + : `run \`openshell status\`, restore gateway '${gatewayName}', then retry`; + case "schema": + return "use matching supported OpenShell CLI and gateway versions, then retry"; + case "timeout": + return `check that gateway '${gatewayName}' responds, then retry`; + case "command": + return `run \`openshell sandbox list -g ${gatewayName}\` and correct the reported command failure`; + } +} + +async function liveSandboxCheck(sandboxName: string, gatewayName: string): Promise { + const list = await createCliOpenShellSandboxObserver({ + capture: captureOpenshell, + defaultTimeoutMs: OPENSHELL_PROBE_TIMEOUT_MS, + }).listSandboxes({ + target: namedOpenShellGateway(gatewayName), + timeoutMs: OPENSHELL_PROBE_TIMEOUT_MS, }); - const liveNames = parseLiveSandboxNames(list.output || ""); - const present = list.status === 0 && liveNames.has(sandboxName); - const line = findSandboxListLine(list.output || "", sandboxName); - const ready = inferSandboxReadyFromLine(line); + if (!list.ok) { + return { + reachable: false, + checks: [ + { + group: "Sandbox", + label: "Live sandbox", + status: "fail", + detail: `OpenShell sandbox observation failed: ${oneLine(list.error.message)}`, + hint: liveSandboxObservationFailureHint(sandboxName, gatewayName, list.error), + }, + ], + }; + } + const observed = list.value.sandboxes.find((sandbox) => sandbox.name === sandboxName) ?? null; + const present = observed !== null; + const ready = observed ? observed.readiness === "ready" : null; const reachable = present && ready === true; return { reachable, @@ -298,19 +339,22 @@ function liveSandboxCheck(sandboxName: string): SandboxProbe { group: "Sandbox", label: "Live sandbox", status: reachable ? "ok" : "fail", - detail: liveSandboxDetail(sandboxName, present, ready, line), + detail: liveSandboxDetail(sandboxName, present, ready, observed?.phase ?? null), hint: liveSandboxHint(sandboxName, present, ready), }, ], }; } -function collectSandboxReadinessChecks( +async function collectSandboxReadinessChecks( sandboxName: string, + gatewayName: string | null, openshellBin: ReturnType, openshellConnected: boolean, -): SandboxProbe { - if (openshellBin && openshellConnected) return liveSandboxCheck(sandboxName); +): Promise { + if (gatewayName && openshellBin && openshellConnected) { + return liveSandboxCheck(sandboxName, gatewayName); + } if (!openshellBin) return { checks: [], reachable: false }; return { reachable: false, @@ -563,7 +607,12 @@ async function collectDoctorChecks( }, ], }; - const sandbox = collectSandboxReadinessChecks(sandboxName, host.openshellBin, gateway.connected); + const sandbox = await collectSandboxReadinessChecks( + sandboxName, + gatewayName, + host.openshellBin, + gateway.connected, + ); const route = resolveInferenceRoute(sb, host.openshellBin, gateway.connected); return [ ...host.checks, diff --git a/src/lib/actions/sandbox/gateway-state-drift.test.ts b/src/lib/actions/sandbox/gateway-state-drift.test.ts index feab9617159..0a4b8105016 100644 --- a/src/lib/actions/sandbox/gateway-state-drift.test.ts +++ b/src/lib/actions/sandbox/gateway-state-drift.test.ts @@ -218,36 +218,38 @@ describe("sandbox gateway state drift guard", () => { }, expected: "gateway exists in metadata, but its API is refusing connections after restart", }, - ])("preserves registry state when the named gateway reports $lifecycle.state", async ({ - lifecycle, - expected, - }) => { - detectPreflightIssueSpy.mockReturnValue(null); - getSandboxSpy.mockReturnValue({ - name: "alpha", - gatewayName: "nemoclaw", - gatewayPort: 8080, - }); - captureOpenshellSpy.mockReturnValue({ - status: 1, - output: 'Error: status: NotFound, message: "sandbox not found"', - }); - getNamedGatewayLifecycleStateSpy.mockReturnValue(lifecycle); - - await expect(gatewayState.ensureLiveSandboxOrExit("alpha")).rejects.toThrow("process.exit(1)"); - - expect(errorSpy.mock.calls.flat().join("\n")).toContain(expected); - expect(removeSandboxSpy).not.toHaveBeenCalled(); - }); + ])( + "preserves registry state when the named gateway reports $lifecycle.state", + async ({ lifecycle, expected }) => { + detectPreflightIssueSpy.mockReturnValue(null); + getSandboxSpy.mockReturnValue({ + name: "alpha", + gatewayName: "nemoclaw", + gatewayPort: 8080, + }); + captureOpenshellSpy.mockReturnValue({ + status: 1, + output: 'Error: status: NotFound, message: "sandbox not found"', + }); + getNamedGatewayLifecycleStateSpy.mockReturnValue(lifecycle); + + await expect(gatewayState.ensureLiveSandboxOrExit("alpha")).rejects.toThrow( + "process.exit(1)", + ); + + expect(errorSpy.mock.calls.flat().join("\n")).toContain(expected); + expect(removeSandboxSpy).not.toHaveBeenCalled(); + }, + ); - it("propagates schema mismatch after selecting the named gateway", () => { + it("propagates schema mismatch after selecting the named gateway", async () => { getNamedGatewayLifecycleStateSpy.mockReturnValue({ state: "connected_other", activeGateway: "openshell", status: "Gateway: openshell\nStatus: Connected", }); - const lookup = gatewayState.reconcileMissingAgainstNamedGateway("alpha", { + const lookup = await gatewayState.reconcileMissingAgainstNamedGateway("alpha", { state: "missing", output: "NotFound", }); @@ -293,7 +295,7 @@ describe("sandbox gateway state drift guard", () => { expect(recoverNamedGatewayRuntimeSpy).toHaveBeenCalledWith({ gatewayName: "nemoclaw-8090" }); }); - it("classifies the `sandbox has no spec` gRPC reply as a missing sandbox so the named-gateway reconciler can retry on the owning gateway", () => { + it("classifies the `sandbox has no spec` gRPC reply as a missing sandbox so the named-gateway reconciler can retry on the owning gateway", async () => { detectPreflightIssueSpy.mockReturnValue(null); captureOpenshellSpy.mockReturnValue({ status: 1, @@ -301,10 +303,10 @@ describe("sandbox gateway state drift guard", () => { 'status: Internal, message: "sandbox has no spec", details: [], metadata: MetadataMap {}', }); - const lookup = gatewayState.getSandboxGatewayState("alpha"); + const lookup = await gatewayState.getSandboxGatewayState("alpha"); expect(lookup.state).toBe("missing"); - expect(lookup.output).toContain("sandbox has no spec"); + expect(lookup.output).not.toContain("sandbox has no spec"); }); it("classifies the same gRPC reply as `missing` on the async status-probe path so the live `nemoclaw status` lookup goes through the named-gateway reconciler too", async () => { @@ -318,10 +320,10 @@ describe("sandbox gateway state drift guard", () => { const lookup = await gatewayState.getSandboxGatewayStateForStatus("alpha"); expect(lookup.state).toBe("missing"); - expect(lookup.output).toContain("sandbox has no spec"); + expect(lookup.output).not.toContain("sandbox has no spec"); }); - it("selects the sandbox's owning gateway and retries when the active gateway is a sibling that has no spec for it", () => { + it("selects the sandbox's owning gateway and retries when the active gateway is a sibling that has no spec for it", async () => { detectPreflightIssueSpy.mockReturnValue(null); getSandboxSpy.mockReturnValue({ name: "instance-a", @@ -338,7 +340,7 @@ describe("sandbox gateway state drift guard", () => { output: "Sandbox:\n Name: instance-a\n Phase: Ready", }); - const retry = gatewayState.reconcileMissingAgainstNamedGateway("instance-a", { + const retry = await gatewayState.reconcileMissingAgainstNamedGateway("instance-a", { state: "missing", output: 'status: Internal, message: "sandbox has no spec"', }); diff --git a/src/lib/actions/sandbox/gateway-state-hints.test.ts b/src/lib/actions/sandbox/gateway-state-hints.test.ts index 3c24ff9f729..d755cc09f30 100644 --- a/src/lib/actions/sandbox/gateway-state-hints.test.ts +++ b/src/lib/actions/sandbox/gateway-state-hints.test.ts @@ -125,6 +125,53 @@ describe("printGatewayLifecycleHint multi-instance hints", () => { expect(lines.join("\n")).toContain(expected); }); + it.each([ + { + label: "unreachable transport", + result: { status: 1, output: "Connection refused credential-value" }, + expected: "gateway 'nemoclaw' is not reachable", + }, + { + label: "gateway identity", + result: { status: 1, output: "handshake verification failed credential-value" }, + expected: "gateway identity drift after restart", + }, + { + label: "authentication", + result: { status: 1, output: "authentication failed credential-value" }, + expected: "restore its authentication before retrying", + }, + { + label: "timeout", + result: { + status: null, + output: "credential-value", + error: Object.assign(new Error("credential-value"), { code: "ETIMEDOUT" }), + }, + expected: "did not answer before the sandbox observation timeout", + }, + ])( + "prints typed $label guidance without raw diagnostics (#9803)", + async ({ result, expected }) => { + captureOpenshellSpy.mockReturnValue(result); + const lines: string[] = []; + vi.spyOn(console, "error").mockImplementation((line = "") => { + lines.push(String(line)); + }); + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`process.exit(${code ?? 0})`); + }) as never); + + await expect( + gatewayState.ensureLiveSandboxOrExit("instance-a", { gatewayRecovery: "observe" }), + ).rejects.toThrow("process.exit(1)"); + + expect(lines.join("\n")).toContain(expected); + expect(lines.join("\n")).not.toContain("credential-value"); + expect(exitSpy).toHaveBeenCalledWith(1); + }, + ); + it("classifies a failed post-recovery handshake as identity drift", async () => { recoverNamedGatewayRuntimeSpy.mockResolvedValue({ recovered: true, via: "start" }); const getState = vi @@ -132,7 +179,8 @@ describe("printGatewayLifecycleHint multi-instance hints", () => { .mockResolvedValueOnce({ state: "gateway_error", output: "transport error" }) .mockResolvedValueOnce({ state: "gateway_error", - output: "transport error: handshake verification failed", + output: "The selected gateway identity does not match the recorded identity.", + transportReason: "identity_mismatch", }); const lookup = await gatewayState.getReconciledSandboxGatewayState("instance-a", { getState }); @@ -317,20 +365,19 @@ describe("printGatewayLifecycleHint multi-instance hints", () => { expectedState: "gateway_error", expectedGatewayRecoveryFailed: true, }, - ])("maps failed gateway recovery to $expectedState", async ({ - lifecycle, - expectedState, - expectedGatewayRecoveryFailed, - }) => { - getNamedGatewayLifecycleStateSpy.mockReturnValue(lifecycle); - - const lookup = await gatewayState.getReconciledSandboxGatewayState("instance-a", { - getState: async () => ({ state: "gateway_error", output: "transport error" }), - }); + ])( + "maps failed gateway recovery to $expectedState", + async ({ lifecycle, expectedState, expectedGatewayRecoveryFailed }) => { + getNamedGatewayLifecycleStateSpy.mockReturnValue(lifecycle); - expect(lookup.state).toBe(expectedState); - expect(lookup.gatewayRecoveryFailed).toBe(expectedGatewayRecoveryFailed); - }); + const lookup = await gatewayState.getReconciledSandboxGatewayState("instance-a", { + getState: async () => ({ state: "gateway_error", output: "transport error" }), + }); + + expect(lookup.state).toBe(expectedState); + expect(lookup.gatewayRecoveryFailed).toBe(expectedGatewayRecoveryFailed); + }, + ); it("prints reconnect and recreate guidance when identity drift persists", async () => { captureOpenshellSpy.mockReturnValue({ @@ -405,7 +452,9 @@ describe("printGatewayLifecycleHint multi-instance hints", () => { ).rejects.toThrow("process.exit(1)"); const output = lines.join("\n"); - expect(output).toContain("This sandbox-scoped command will not restart the shared host gateway"); + expect(output).toContain( + "This sandbox-scoped command will not restart the shared host gateway", + ); expect(output).toContain("Start the gateway again with `nemoclaw onboard`."); expect(output).not.toContain("openshell gateway start"); expect(recoverNamedGatewayRuntimeSpy).not.toHaveBeenCalled(); diff --git a/src/lib/actions/sandbox/gateway-state-owning-gateway.test.ts b/src/lib/actions/sandbox/gateway-state-owning-gateway.test.ts index fec4dc749b1..bbbbd423e89 100644 --- a/src/lib/actions/sandbox/gateway-state-owning-gateway.test.ts +++ b/src/lib/actions/sandbox/gateway-state-owning-gateway.test.ts @@ -32,7 +32,7 @@ describe("getReconciledSandboxGatewayState owning-gateway guard", () => { vi.restoreAllMocks(); }); - it("pins both the sandbox and policy RPCs to the recorded owner", () => { + it("pins both the sandbox and policy RPCs to the recorded owner", async () => { vi.spyOn(gatewayDrift, "detectOpenShellStateRpcPreflightIssue").mockReturnValue(null); vi.spyOn(gatewayDrift, "detectOpenShellStateRpcResultIssue").mockReturnValue(null); const capture = vi @@ -40,7 +40,7 @@ describe("getReconciledSandboxGatewayState owning-gateway guard", () => { .mockReturnValueOnce({ status: 0, output: "Policy:\nPhase: Ready" } as never) .mockReturnValueOnce({ status: 0, output: "version: 1" } as never); - const result = getSandboxGatewayState("beta", "nemoclaw-8091"); + const result = await getSandboxGatewayState("beta", "nemoclaw-8091"); expect(result.state).toBe("present"); expect(capture).toHaveBeenNthCalledWith( @@ -55,7 +55,7 @@ describe("getReconciledSandboxGatewayState owning-gateway guard", () => { ); }); - it("classifies the owner-scoped Internal no-spec response as missing", () => { + it("classifies the owner-scoped Internal no-spec response as missing", async () => { vi.spyOn(gatewayDrift, "detectOpenShellStateRpcPreflightIssue").mockReturnValue(null); vi.spyOn(gatewayDrift, "detectOpenShellStateRpcResultIssue").mockReturnValue(null); const capture = vi.spyOn(openshellRuntime, "captureOpenshell").mockReturnValue({ @@ -63,7 +63,7 @@ describe("getReconciledSandboxGatewayState owning-gateway guard", () => { output: 'status: Internal, message: "sandbox has no spec"', } as never); - expect(getSandboxGatewayState("beta", "nemoclaw-8091")).toMatchObject({ + await expect(getSandboxGatewayState("beta", "nemoclaw-8091")).resolves.toMatchObject({ state: "missing", }); expect(capture).toHaveBeenCalledWith( @@ -101,7 +101,7 @@ describe("getReconciledSandboxGatewayState owning-gateway guard", () => { const syncCapture = vi.spyOn(openshellRuntime, "captureOpenshell"); const asyncCapture = vi.spyOn(openshellRuntime, "captureOpenshellForStatus"); - expect(getSandboxGatewayState("beta", "nemoclaw-8091")).toMatchObject({ + await expect(getSandboxGatewayState("beta", "nemoclaw-8091")).resolves.toMatchObject({ state: "gateway_endpoint_override", output: expect.stringContaining("OPENSHELL_GATEWAY_ENDPOINT is set"), }); diff --git a/src/lib/actions/sandbox/gateway-state.ts b/src/lib/actions/sandbox/gateway-state.ts index ed0a6547ed4..6e6a200cebb 100644 --- a/src/lib/actions/sandbox/gateway-state.ts +++ b/src/lib/actions/sandbox/gateway-state.ts @@ -12,7 +12,8 @@ import { } from "../../gateway-runtime-action"; import { gatewayStartGuidance } from "../../gateway-start-guidance"; import { assertNoOpenShellGatewayEndpointOverride } from "../../openshell-gateway-endpoint-guard"; -import { isTerminalSandboxPhase, parseSandboxPhase } from "../../state/gateway"; +import { isTerminalSandboxPhase, TERMINAL_SANDBOX_PHASES } from "../../state/gateway"; +export { isTerminalSandboxPhase, TERMINAL_SANDBOX_PHASES }; import { withMcpLifecycleLock, withMcpLifecycleLockSync, @@ -30,10 +31,20 @@ const { pruneKnownHostsEntries } = require("../../onboard/known-hosts") as { }; import { dockerStart } from "../../adapters/docker/container"; -import { stripAnsi } from "../../adapters/openshell/client"; +import { + createCliOpenShellSandboxLookup, + stripOpenShellCliAnsi, + type CliOpenShellSandboxLookup, +} from "../../adapters/openshell/sandbox-observer-cli"; +import { + namedOpenShellGateway, + selectedOpenShellGateway, + type OpenShellSandboxError, + type OpenShellSandboxErrorKind, + type OpenShellSandboxTransportReason, +} from "../../adapters/openshell/sandbox-observer"; import { detectOpenShellStateRpcPreflightIssue, - detectOpenShellStateRpcResultIssue, formatOpenShellStateRpcIssue, type OpenShellStateRpcIssue, } from "../../adapters/openshell/gateway-drift"; @@ -72,9 +83,12 @@ import { isDockerRuntimeDown, printDockerRuntimeDownGuidance } from "./gateway-f export type SandboxGatewayState = { state: string; output: string; + phase?: string | null; activeGateway?: string | null; recoveredGateway?: boolean; recoveryVia?: string | null; + observationErrorKind?: OpenShellSandboxErrorKind; + transportReason?: OpenShellSandboxTransportReason; gatewayRecoveryFailed?: boolean; /** * True when active Docker-driver sandbox recovery (#4423 part 2) @@ -210,13 +224,6 @@ function gatewayEndpointOverrideState(): SandboxGatewayState | null { } } -/** Canonical OpenShell response classifier for an absent sandbox record. */ -export function isMissingSandboxGatewayOutput(output = ""): boolean { - return /\bNotFound\b|\bNot Found\b|sandbox not found|sandbox has no spec/i.test( - stripAnsi(String(output)), - ); -} - function formatGatewaySchemaMismatchOutput( issue: OpenShellStateRpcIssue, action: string, @@ -227,7 +234,7 @@ function formatGatewaySchemaMismatchOutput( export function mergeLivePolicyIntoSandboxOutput(output: string, livePolicyOutput: string): string { const rawLines = String(output).split("\n"); - const cleanLines = stripAnsi(String(output)).split("\n"); + const cleanLines = stripOpenShellCliAnsi(String(output)).split("\n"); const policyLineIdx = cleanLines.findIndex((line: string) => line.trim() === "Policy:"); if (policyLineIdx === -1) return output; @@ -243,7 +250,7 @@ export function mergeLivePolicyIntoSandboxOutput(output: string, livePolicyOutpu suffixLineIdx === -1 ? "" : `\n${rawLines.slice(suffixLineIdx).join("\n").replace(/\n+$/u, "")}`; - const cleanLivePolicy = stripAnsi(String(livePolicyOutput)); + const cleanLivePolicy = stripOpenShellCliAnsi(String(livePolicyOutput)); const delimIdx = cleanLivePolicy.search(/^---\s*$/m); const metadataPart = delimIdx !== -1 ? cleanLivePolicy.slice(0, delimIdx) : ""; const yamlPart = @@ -270,10 +277,47 @@ export function mergeLivePolicyIntoSandboxOutput(output: string, livePolicyOutpu } /** Query sandbox presence and return its output with the live enforced policy. */ -export function getSandboxGatewayState( +function sandboxObservationTarget(gatewayName?: string) { + return gatewayName ? namedOpenShellGateway(gatewayName) : selectedOpenShellGateway(); +} + +function schemaMismatchState(action: string): SandboxGatewayState { + return { + state: "gateway_schema_mismatch", + output: formatOpenShellStateRpcIssue( + { kind: "protobuf_mismatch", drift: null, output: "" }, + { action }, + ).join("\n"), + }; +} + +function sandboxObservationErrorState( + error: OpenShellSandboxError, + action: string, +): SandboxGatewayState { + if (error.kind === "schema") return schemaMismatchState(action); + if (error.kind === "transport") { + return { + state: "gateway_error", + output: error.message, + observationErrorKind: error.kind, + transportReason: error.reason, + }; + } + if (error.kind === "authentication" || error.kind === "timeout") { + return { state: "gateway_error", output: error.message, observationErrorKind: error.kind }; + } + return { state: "unknown_error", output: error.message, observationErrorKind: error.kind }; +} + +export async function getSandboxGatewayState( sandboxName: string, gatewayName?: string, -): SandboxGatewayState { + lookupSandbox: CliOpenShellSandboxLookup = createCliOpenShellSandboxLookup({ + capture: captureOpenshell, + defaultTimeoutMs: OPENSHELL_PROBE_TIMEOUT_MS, + }), +): Promise { const endpointOverride = gatewayEndpointOverrideState(); if (endpointOverride) return endpointOverride; const preflightIssue = detectOpenShellStateRpcPreflightIssue({ gatewayName }); @@ -286,20 +330,22 @@ export function getSandboxGatewayState( ), }; } - const result = captureOpenshell(gatewayScopedArgs(["sandbox", "get", sandboxName], gatewayName), { - timeout: OPENSHELL_PROBE_TIMEOUT_MS, + const observed = await lookupSandbox({ + sandboxName, + target: sandboxObservationTarget(gatewayName), + timeoutMs: OPENSHELL_PROBE_TIMEOUT_MS, }); - let output = result.output; - const resultIssue = detectOpenShellStateRpcResultIssue(result, { gatewayName }); - if (resultIssue) { - return { - state: "gateway_schema_mismatch", - output: formatOpenShellStateRpcIssue(resultIssue, { - action: `verifying sandbox '${sandboxName}' against OpenShell`, - }).join("\n"), - }; - } - if (result.status === 0) { + const lookup = observed.result; + const action = `verifying sandbox '${sandboxName}' against OpenShell`; + if (!lookup.ok) return sandboxObservationErrorState(lookup.error, action); + if (lookup.value.state === "missing") { + return { state: "missing", output: "OpenShell did not find the sandbox." }; + } + // Preserve the current CLI-formatted status display without putting it in + // the transport-neutral observation contract. Presence and phase decisions + // do not parse this text. + let output = observed.displayOutput; + if (lookup.value.state === "present") { const livePolicy = captureOpenshell( gatewayScopedArgs(["policy", "get", "--full", sandboxName], gatewayName), { @@ -310,29 +356,18 @@ export function getSandboxGatewayState( if (livePolicy.status === 0 && livePolicy.output.trim()) { output = mergeLivePolicyIntoSandboxOutput(output, livePolicy.output); } - return { state: "present", output }; - } - // `sandbox has no spec` is the gRPC reply when the queried gateway does not - // know about this sandbox. On an unscoped lookup that can be an ambient - // sibling; an owner-scoped lookup means the sandbox is genuinely absent - // from its recorded gateway. Both remain `missing`, and reconciliation uses - // the presence of the explicit owner pin to distinguish those cases. - if (isMissingSandboxGatewayOutput(output)) { - return { state: "missing", output }; - } - if ( - /transport error|Connection refused|handshake verification failed|Missing gateway auth token|device identity required/i.test( - output, - ) - ) { - return { state: "gateway_error", output }; + return { state: "present", output, phase: lookup.value.sandbox.phase }; } - return { state: "unknown_error", output }; + return { state: "unknown_error", output: "OpenShell returned an unknown sandbox state." }; } export async function getSandboxGatewayStateForStatus( sandboxName: string, gatewayName?: string, + lookupSandbox: CliOpenShellSandboxLookup = createCliOpenShellSandboxLookup({ + capture: captureOpenshellForStatus, + defaultTimeoutMs: OPENSHELL_PROBE_TIMEOUT_MS, + }), ): Promise { const timeoutMs = getStatusProbeTimeoutMs(); const endpointOverride = gatewayEndpointOverrideState(); @@ -348,30 +383,28 @@ export async function getSandboxGatewayStateForStatus( ), }; } - const result = await captureOpenshellForStatus( - gatewayScopedArgs(["sandbox", "get", sandboxName], gatewayName), - { - timeout: timeoutMs, - }, - ); - let output = result.output; - const resultIssue = detectOpenShellStateRpcResultIssue(result, { gatewayName, timeoutMs }); - if (resultIssue) { - return { - state: "gateway_schema_mismatch", - output: formatOpenShellStateRpcIssue(resultIssue, { - action: `checking status for sandbox '${sandboxName}'`, - command: `${CLI_NAME} ${sandboxName} status`, - }).join("\n"), - }; - } - if (isCommandTimeout(result)) { + const observed = await lookupSandbox({ + sandboxName, + target: sandboxObservationTarget(gatewayName), + timeoutMs, + }); + const lookup = observed.result; + const action = `checking status for sandbox '${sandboxName}'`; + if (!lookup.ok && lookup.error.kind === "timeout") { return { state: "status_probe_timeout", output: ` Live sandbox status probe timed out after ${Math.ceil(timeoutMs / 1000)}s. Local registry data is shown above.`, }; } - if (result.status === 0) { + if (!lookup.ok) return sandboxObservationErrorState(lookup.error, action); + if (lookup.value.state === "missing") { + return { state: "missing", output: "OpenShell did not find the sandbox." }; + } + // Preserve the current CLI-formatted status display without putting it in + // the transport-neutral observation contract. Presence and phase decisions + // do not parse this text. + let output = observed.displayOutput; + if (lookup.value.state === "present") { const livePolicy = await captureOpenshellForStatus( gatewayScopedArgs(["policy", "get", "--full", sandboxName], gatewayName), { @@ -382,19 +415,9 @@ export async function getSandboxGatewayStateForStatus( if (!isCommandTimeout(livePolicy) && livePolicy.status === 0 && livePolicy.output.trim()) { output = mergeLivePolicyIntoSandboxOutput(output, livePolicy.output); } - return { state: "present", output }; - } - if (isMissingSandboxGatewayOutput(output)) { - return { state: "missing", output }; - } - if ( - /transport error|Connection refused|handshake verification failed|Missing gateway auth token|device identity required/i.test( - output, - ) - ) { - return { state: "gateway_error", output }; + return { state: "present", output, phase: lookup.value.sandbox.phase }; } - return { state: "unknown_error", output }; + return { state: "unknown_error", output: "OpenShell returned an unknown sandbox state." }; } /** @@ -407,11 +430,11 @@ export async function getSandboxGatewayStateForStatus( * already came from the recorded owner, so ambient selection is ignored and * only the existing Docker-side recovery path is considered. */ -export function reconcileMissingAgainstNamedGateway( +export async function reconcileMissingAgainstNamedGateway( sandboxName: string, missingLookup: SandboxGatewayState, pinnedGatewayName?: string, -): SandboxGatewayState { +): Promise { const targetGatewayName = pinnedGatewayName ?? getSandboxTargetGatewayName(sandboxName); if (pinnedGatewayName) { // The owner-scoped RPC reached this exact gateway and reported NotFound. @@ -424,7 +447,7 @@ export function reconcileMissingAgainstNamedGateway( ignoreError: true, timeout: OPENSHELL_OPERATION_TIMEOUT_MS, }); - const retry = getSandboxGatewayState(sandboxName, targetGatewayName); + const retry = await getSandboxGatewayState(sandboxName, targetGatewayName); if (retry.state === "present") { return { ...retry, recoveredGateway: true, recoveryVia: "select" }; } @@ -481,11 +504,11 @@ export function reconcileMissingAgainstNamedGateway( * `missing` lookup unchanged so the caller's existing non-destructive * guidance fires. */ -function tryRecoverDockerDriverSandbox( +async function tryRecoverDockerDriverSandbox( sandboxName: string, missingLookup: SandboxGatewayState, gatewayName?: string, -): SandboxGatewayState { +): Promise { let recovery: DockerDriverRecoveryResult; try { recovery = recoverDockerDriverSandbox(sandboxName); @@ -497,7 +520,7 @@ function tryRecoverDockerDriverSandbox( } // Recovery succeeded against Docker; re-query OpenShell so the // returned state reflects what the gateway sees post-restart. - const retried = getSandboxGatewayState(sandboxName, gatewayName); + const retried = await getSandboxGatewayState(sandboxName, gatewayName); return { ...retried, recoveredSandbox: true, @@ -536,7 +559,7 @@ export function printGatewayLifecycleHint( sandboxName = "", writer: (message: string) => void = console.error, ): void { - const cleanOutput = stripAnsi(output); + const cleanOutput = stripOpenShellCliAnsi(output); const targetGatewayName = getSandboxTargetGatewayName(sandboxName); // The gateway-side gRPC reply `sandbox has no spec` is returned when the // active OpenShell gateway does not know about the sandbox — which on a @@ -605,6 +628,47 @@ export function printGatewayLifecycleHint( } } +/** Print recovery guidance from a typed observation error or legacy CLI output. */ +export function printSandboxGatewayStateHint( + lookup: Pick, + sandboxName: string, + writer: (message: string) => void = console.error, +): void { + const targetGatewayName = getSandboxTargetGatewayName(sandboxName); + switch (lookup.observationErrorKind) { + case "transport": + if (lookup.transportReason === "identity_mismatch") { + writer(" This looks like gateway identity drift after restart."); + writer( + " Existing sandboxes may still be recorded locally, but the selected gateway identity no longer matches the identity recorded before restart.", + ); + writer( + ` Re-establish the ${CLI_DISPLAY_NAME} gateway runtime first. If the sandbox stays unreachable, recreate only that sandbox with \`${CLI_NAME} onboard\`.`, + ); + return; + } + writer( + ` The sandbox '${sandboxName}' may still exist, but gateway '${targetGatewayName}' is not reachable.`, + ); + writer(" Check `openshell status`, verify the active gateway, and retry."); + return; + case "authentication": + writer(" OpenShell could not authenticate the sandbox observation."); + writer(" Verify the active gateway and restore its authentication before retrying."); + return; + case "timeout": + writer(" The OpenShell gateway did not answer before the sandbox observation timeout."); + writer(" Check `openshell status` and retry after the gateway responds."); + return; + case "command": + writer(" The OpenShell sandbox observation command failed."); + writer(" Run `openshell status`, inspect the gateway, and retry."); + return; + default: + printGatewayLifecycleHint(lookup.output, sandboxName, writer); + } +} + export type GatewayRecoveryMode = "observe" | "recover"; export async function getReconciledSandboxGatewayState( @@ -660,7 +724,7 @@ export async function getReconciledSandboxGatewayState( if (retried.state === "present" || retried.state === "missing") { return { ...retried, recoveredGateway: true, recoveryVia: recovery.via || null }; } - if (/handshake verification failed/i.test(retried.output)) { + if (retried.transportReason === "identity_mismatch") { return { state: "identity_drift", output: retried.output, @@ -671,7 +735,7 @@ export async function getReconciledSandboxGatewayState( return { ...retried, recoveredGateway: true, recoveryVia: recovery.via || null }; } const latestLifecycle = getNamedGatewayLifecycleState(recoveryGatewayName); - const latestStatus = stripAnsi(latestLifecycle.status || ""); + const latestStatus = stripOpenShellCliAnsi(latestLifecycle.status || ""); if (/No gateway configured/i.test(latestStatus)) { return { state: "gateway_missing_after_restart", @@ -753,7 +817,7 @@ export async function ensureLiveSandboxOrExit( selectOwningGateway, }); if (lookup.state === "present") { - const phase = parseSandboxPhase(lookup.output || ""); + const phase = lookup.phase ?? null; if (!allowNonReadyPhase && phase && phase !== "Ready" && phase !== "Running") { // Don't steer toward rebuild when the host Docker daemon is down: the // sandbox is fine and recreating it cannot succeed until Docker is back @@ -894,7 +958,7 @@ export async function ensureLiveSandboxOrExit( if (lookup.output) { console.error(lookup.output); } - printGatewayLifecycleHint(lookup.output, sandboxName); + printSandboxGatewayStateHint(lookup, sandboxName); console.error( ` This sandbox-scoped command will not restart the shared host gateway. ${gatewayStartGuidance(getSandboxTargetGatewayName(sandboxName))} Then retry this command.`, ); @@ -917,7 +981,7 @@ export async function ensureLiveSandboxOrExit( if (lookup.output) { console.error(lookup.output); } - printGatewayLifecycleHint(lookup.output, sandboxName); + printSandboxGatewayStateHint(lookup, sandboxName); console.error(" Check `openshell status` and the active gateway, then retry."); return exit(1); } diff --git a/src/lib/actions/sandbox/rebuild-dcode-recovered-provider.test.ts b/src/lib/actions/sandbox/rebuild-dcode-recovered-provider.test.ts index 72cba356e9b..dd1e33371dd 100644 --- a/src/lib/actions/sandbox/rebuild-dcode-recovered-provider.test.ts +++ b/src/lib/actions/sandbox/rebuild-dcode-recovered-provider.test.ts @@ -64,7 +64,9 @@ describe("rebuildSandbox DCode recovered provider", () => { const harness = createRebuildFlowHarness({ agentName: "langchain-deepagents-code", sandboxEntry: makeDcodeSandboxEntry(), - sandboxListOutput: "alpha Error", + sandboxInventory: { + sandboxes: [{ name: "alpha", phase: "Error", readiness: "terminal" }], + }, preDeleteLatestManifest: recoveryManifest, }); configureDcodeSession(harness); diff --git a/src/lib/actions/sandbox/rebuild-dcode-recovery.test.ts b/src/lib/actions/sandbox/rebuild-dcode-recovery.test.ts index a6a8312fd40..ff8dc935287 100644 --- a/src/lib/actions/sandbox/rebuild-dcode-recovery.test.ts +++ b/src/lib/actions/sandbox/rebuild-dcode-recovery.test.ts @@ -25,7 +25,9 @@ describe("rebuildSandbox DCode flow: recovery", () => { const harness = createRebuildFlowHarness({ agentName: "langchain-deepagents-code", sandboxEntry: makeDcodeSandboxEntry(), - sandboxListOutput: "alpha Error", + sandboxInventory: { + sandboxes: [{ name: "alpha", phase: "Error", readiness: "terminal" }], + }, preDeleteLatestManifest: recoveryManifest, }); configureDcodeSession(harness); @@ -64,7 +66,7 @@ describe("rebuildSandbox DCode flow: recovery", () => { customPolicies: [customPolicy], policyPresetsFinalized: true, }, - sandboxListOutput: "", + sandboxInventory: { sandboxes: [] }, reconciledSandboxGatewayState: { state: "missing", output: "" }, }); configureDcodeSession(harness); diff --git a/src/lib/actions/sandbox/rebuild-flow-credential-preflight.test.ts b/src/lib/actions/sandbox/rebuild-flow-credential-preflight.test.ts index b8048ecb79a..c0f907c1711 100644 --- a/src/lib/actions/sandbox/rebuild-flow-credential-preflight.test.ts +++ b/src/lib/actions/sandbox/rebuild-flow-credential-preflight.test.ts @@ -162,7 +162,9 @@ describe("rebuildSandbox flow: credential preflight", () => { }, hydrateCredentialEnv: () => "host-provider-key", runOpenshell: providerRuntime([]), - sandboxListOutput: "alpha Error", + sandboxInventory: { + sandboxes: [{ name: "alpha", phase: "Error", readiness: "terminal" }], + }, }); configureSession(harness, "compatible-endpoint", "COMPATIBLE_API_KEY", { endpointUrl: "https://inference.example.test/v1", diff --git a/src/lib/actions/sandbox/rebuild-flow-helpers.ts b/src/lib/actions/sandbox/rebuild-flow-helpers.ts index c3217515e7c..877874b8baa 100644 --- a/src/lib/actions/sandbox/rebuild-flow-helpers.ts +++ b/src/lib/actions/sandbox/rebuild-flow-helpers.ts @@ -2,10 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { dockerRmi } from "../../adapters/docker/image"; -import { - detectOpenShellStateRpcResultIssue, - printOpenShellStateRpcIssue, -} from "../../adapters/openshell/gateway-drift"; +import { printOpenShellStateRpcIssue } from "../../adapters/openshell/gateway-drift"; import { loadAgent } from "../../agent/defs"; import { bindLocalAgentBaseImageHandoffToResolution, @@ -33,7 +30,6 @@ import { captureSandboxListWithGatewayRecovery, printSandboxListFailureWithRecoveryContext, } from "../../openshell-sandbox-list"; -import { parseLiveSandboxNames } from "../../runtime-recovery"; import { parseContentAddressedSandboxBaseImageId, type SandboxBaseImageResolutionMetadata, @@ -46,7 +42,7 @@ import * as sandboxState from "../../state/sandbox"; import * as userManagedFilesProbe from "../../state/user-managed-files-probe"; import { getReconciledSandboxGatewayState, - printGatewayLifecycleHint, + printSandboxGatewayStateHint, printWrongGatewayActiveGuidance, } from "./gateway-state"; import { openRebuildShieldsWindow, type RebuildShieldsWindow } from "./rebuild-shields"; @@ -164,28 +160,25 @@ export async function resolveRebuildLiveState( const liveRecovery = await captureSandboxListWithGatewayRecovery({ gatewayName: recordedGateway, }); - const isLive = liveRecovery.result; - log( - `openshell sandbox list exit=${isLive.status}, output=${(isLive.output || "").substring(0, 200)}`, - ); - const liveListIssue = detectOpenShellStateRpcResultIssue(isLive, { - gatewayName: recordedGateway, - }); - if (liveListIssue) { - printOpenShellStateRpcIssue(liveListIssue, { - action: `rebuilding sandbox '${sandboxName}'`, - command: `${CLI_NAME} ${sandboxName} rebuild`, - }); + const observed = liveRecovery.result; + if (!observed.ok && observed.error.kind === "schema") { + printOpenShellStateRpcIssue( + { kind: "protobuf_mismatch", drift: null, output: "" }, + { + action: `rebuilding sandbox '${sandboxName}'`, + command: `${CLI_NAME} ${sandboxName} rebuild`, + }, + ); bail("OpenShell gateway schema mismatch."); return null; } - if (isLive.status !== 0) { + if (!observed.ok) { printSandboxListFailureWithRecoveryContext(liveRecovery); - bail("Failed to query running sandboxes from OpenShell.", isLive.status || 1); + bail("Failed to query running sandboxes from OpenShell.", 1); return null; } - const liveNames = parseLiveSandboxNames(isLive.output || ""); + const liveNames = new Set(observed.value.sandboxes.map((sandbox) => sandbox.name)); log(`Live sandboxes: ${Array.from(liveNames).join(", ") || "(none)"}`); if (liveNames.has(sandboxName)) return { staleRecovery: false, staleRegistrySnapshot: null }; @@ -257,7 +250,7 @@ export async function resolveRebuildLiveState( ` Sandbox '${sandboxName}' is not visible on gateway '${recordedGateway}' and its live state could not be confirmed.`, ); console.error(" Your local registry entry has been preserved — nothing was removed."); - printGatewayLifecycleHint(reconciled.output || "", sandboxName, console.error); + printSandboxGatewayStateHint(reconciled, sandboxName, console.error); } bail(`Could not confirm live state of '${sandboxName}' (gateway not in a known-good state).`); return null; diff --git a/src/lib/actions/sandbox/rebuild-flow-lifecycle.test.ts b/src/lib/actions/sandbox/rebuild-flow-lifecycle.test.ts index 8a7b2c54391..06621396c43 100644 --- a/src/lib/actions/sandbox/rebuild-flow-lifecycle.test.ts +++ b/src/lib/actions/sandbox/rebuild-flow-lifecycle.test.ts @@ -529,7 +529,7 @@ network_policies: it("disposes the base-image handoff when live-state preflight fails (#7144)", async () => { const disposeImageRef = vi.fn(() => true); const harness = createRebuildFlowHarness({ - sandboxListOutput: "", + sandboxInventory: { sandboxes: [] }, reconciledSandboxGatewayState: { state: "unknown", output: "indeterminate" }, baseImagePreflight: { ok: true, diff --git a/src/lib/actions/sandbox/rebuild-gateway-drift.test.ts b/src/lib/actions/sandbox/rebuild-gateway-drift.test.ts index f291319fea2..869e0ba4010 100644 --- a/src/lib/actions/sandbox/rebuild-gateway-drift.test.ts +++ b/src/lib/actions/sandbox/rebuild-gateway-drift.test.ts @@ -167,45 +167,50 @@ describe("rebuild gateway drift preflight", () => { recordedPort: 9000, activeGateway: "nemoclaw", }, - ])("recovers $recordedGateway as stale even while $activeGateway is ambiently active, since the sandbox RPC is gateway-pinned (#4497)", async ({ - recordedGateway, - recordedPort, - activeGateway, - }) => { - const entry = makeSandboxEntry(recordedGateway, recordedPort); - const registrySnapshot = { sandboxes: { alpha: entry } }; - vi.mocked(registry.getSandbox).mockReturnValue(entry as never); - vi.mocked(registryPersistence.load).mockReturnValue(registrySnapshot as never); - captureOpenshellSpy - .mockReturnValueOnce({ status: 0, output: "" }) - .mockReturnValueOnce({ status: 1, output: "Error: × Not Found: sandbox not found" }); - getNamedGatewayLifecycleStateSpy.mockReturnValue({ - state: "connected_other", - activeGateway, - status: `Gateway: ${activeGateway}\nStatus: Connected`, - } as never); - const behaviorLog = vi.fn(); - - const result = await resolveRebuildLiveState("alpha", entry, behaviorLog, bail); - - expect(result).toEqual({ staleRecovery: true, staleRegistrySnapshot: registrySnapshot }); - expect(result?.staleRegistrySnapshot).not.toBe(registrySnapshot); - expect(getNamedGatewayLifecycleStateSpy).not.toHaveBeenCalled(); - expect(runOpenshellSpy).toHaveBeenCalledWith( - ["gateway", "select", recordedGateway], - expect.objectContaining({ ignoreError: true }), - ); - expect(captureOpenshellSpy).toHaveBeenNthCalledWith(1, ["sandbox", "list"]); - expect(captureOpenshellSpy).toHaveBeenNthCalledWith( - 2, - ["sandbox", "get", "-g", recordedGateway, "alpha"], - expect.anything(), - ); - expect(recoverDockerDriverSandboxSpy).toHaveBeenCalledWith("alpha"); - expect(registryPersistence.load).toHaveBeenCalledOnce(); - expect(logSpy.mock.calls.flat().join("\n")).toContain("absent from the live OpenShell gateway"); - expect(behaviorLog.mock.calls.flat().join("\n")).toContain("Stale-sandbox recovery"); - }); + ])( + "recovers $recordedGateway as stale even while $activeGateway is ambiently active, since the sandbox RPC is gateway-pinned (#4497)", + async ({ recordedGateway, recordedPort, activeGateway }) => { + const entry = makeSandboxEntry(recordedGateway, recordedPort); + const registrySnapshot = { sandboxes: { alpha: entry } }; + vi.mocked(registry.getSandbox).mockReturnValue(entry as never); + vi.mocked(registryPersistence.load).mockReturnValue(registrySnapshot as never); + captureOpenshellSpy + .mockReturnValueOnce({ status: 0, output: "" }) + .mockReturnValueOnce({ status: 1, output: "Error: × Not Found: sandbox not found" }); + getNamedGatewayLifecycleStateSpy.mockReturnValue({ + state: "connected_other", + activeGateway, + status: `Gateway: ${activeGateway}\nStatus: Connected`, + } as never); + const behaviorLog = vi.fn(); + + const result = await resolveRebuildLiveState("alpha", entry, behaviorLog, bail); + + expect(result).toEqual({ staleRecovery: true, staleRegistrySnapshot: registrySnapshot }); + expect(result?.staleRegistrySnapshot).not.toBe(registrySnapshot); + expect(getNamedGatewayLifecycleStateSpy).not.toHaveBeenCalled(); + expect(runOpenshellSpy).toHaveBeenCalledWith( + ["gateway", "select", recordedGateway], + expect.objectContaining({ ignoreError: true }), + ); + expect(captureOpenshellSpy).toHaveBeenNthCalledWith( + 1, + ["sandbox", "list", "-g", recordedGateway], + expect.objectContaining({ ignoreError: true }), + ); + expect(captureOpenshellSpy).toHaveBeenNthCalledWith( + 2, + ["sandbox", "get", "-g", recordedGateway, "alpha"], + expect.anything(), + ); + expect(recoverDockerDriverSandboxSpy).toHaveBeenCalledWith("alpha"); + expect(registryPersistence.load).toHaveBeenCalledOnce(); + expect(logSpy.mock.calls.flat().join("\n")).toContain( + "absent from the live OpenShell gateway", + ); + expect(behaviorLog.mock.calls.flat().join("\n")).toContain("Stale-sandbox recovery"); + }, + ); it("removes one exactly labeled Docker orphan before a registry-only rebuild (#8720)", async () => { const entry = { ...makeSandboxEntry(), openshellDriver: "docker" }; @@ -279,82 +284,87 @@ describe("rebuild gateway drift preflight", () => { removeResult: { status: 0 }, removeCalls: 1, }, - ])("fails closed before registry recovery on Docker orphan $failure (#8720)", async ({ - queryResults, - removeResult, - removeCalls, - }) => { - const entry = { ...makeSandboxEntry(), openshellDriver: "docker" }; - vi.mocked(registry.getSandbox).mockReturnValue(entry as never); - captureOpenshellSpy - .mockReturnValueOnce({ status: 0, output: "" }) - .mockReturnValueOnce({ status: 1, output: "Error: sandbox not found" }); - queryDockerContainersSpy.mockReturnValueOnce(queryResults[0] as never); - queryDockerContainersSpy.mockReturnValueOnce((queryResults[1] ?? queryResults[0]) as never); - forceRemoveDockerContainerSpy.mockReturnValue(removeResult); - - await expect(resolveRebuildLiveState("alpha", entry, vi.fn(), bail)).rejects.toThrow( - "Stale-recovery Docker orphan cleanup failed", - ); - - expect(forceRemoveDockerContainerSpy).toHaveBeenCalledTimes(removeCalls); - expect(registryPersistence.load).not.toHaveBeenCalled(); - }); + ])( + "fails closed before registry recovery on Docker orphan $failure (#8720)", + async ({ queryResults, removeResult, removeCalls }) => { + const entry = { ...makeSandboxEntry(), openshellDriver: "docker" }; + vi.mocked(registry.getSandbox).mockReturnValue(entry as never); + captureOpenshellSpy + .mockReturnValueOnce({ status: 0, output: "" }) + .mockReturnValueOnce({ status: 1, output: "Error: sandbox not found" }); + queryDockerContainersSpy.mockReturnValueOnce(queryResults[0] as never); + queryDockerContainersSpy.mockReturnValueOnce((queryResults[1] ?? queryResults[0]) as never); + forceRemoveDockerContainerSpy.mockReturnValue(removeResult); + + await expect(resolveRebuildLiveState("alpha", entry, vi.fn(), bail)).rejects.toThrow( + "Stale-recovery Docker orphan cleanup failed", + ); + + expect(forceRemoveDockerContainerSpy).toHaveBeenCalledTimes(removeCalls); + expect(registryPersistence.load).not.toHaveBeenCalled(); + }, + ); it.each([ { gatewayName: "nemoclaw", gatewayPort: 8080 }, { gatewayName: "nemoclaw-12345", gatewayPort: 12345 }, - ])("recovers $gatewayName and returns stale state after confirming the sandbox is absent (#4497)", async ({ - gatewayName, - gatewayPort, - }) => { - const entry = makeSandboxEntry(gatewayName, gatewayPort); - const registrySnapshot = { sandboxes: { alpha: entry } }; - vi.mocked(registry.getSandbox).mockReturnValue(entry as never); - vi.mocked(registryPersistence.load).mockReturnValue(registrySnapshot as never); - captureOpenshellSpy - .mockReturnValueOnce({ - status: 1, - output: "client error (Connect): Connection refused", - }) - .mockReturnValueOnce({ status: 0, output: "beta Ready" }) - .mockReturnValueOnce({ status: 1, output: "Error: × Not Found: sandbox not found" }); - getNamedGatewayLifecycleStateSpy.mockReturnValue({ - state: "healthy_named", - activeGateway: gatewayName, - status: `Gateway: ${gatewayName}\nStatus: Connected`, - } as never); - const behaviorLog = vi.fn(); - - const result = await resolveRebuildLiveState("alpha", entry, behaviorLog, bail); - - expect(result).toEqual({ - staleRecovery: true, - staleRegistrySnapshot: registrySnapshot, - }); - expect(result?.staleRegistrySnapshot).not.toBe(registrySnapshot); - expect(recoverNamedGatewayRuntimeSpy).toHaveBeenCalledTimes(2); - expect(recoverNamedGatewayRuntimeSpy).toHaveBeenNthCalledWith(1, { - gatewayName, - recoverableStates: recoveryStates, - }); - expect(recoverNamedGatewayRuntimeSpy).toHaveBeenNthCalledWith(2, { - gatewayName, - recoverableStates: recoveryStates, - }); - expect(captureOpenshellSpy).toHaveBeenNthCalledWith(1, ["sandbox", "list"]); - expect(captureOpenshellSpy).toHaveBeenNthCalledWith(2, ["sandbox", "list"]); - expect(captureOpenshellSpy).toHaveBeenNthCalledWith( - 3, - ["sandbox", "get", "-g", gatewayName, "alpha"], - expect.anything(), - ); - expect(getNamedGatewayLifecycleStateSpy).not.toHaveBeenCalled(); - expect(recoverDockerDriverSandboxSpy).toHaveBeenCalledWith("alpha"); - expect(registryPersistence.load).toHaveBeenCalledOnce(); - expect(logSpy.mock.calls.flat().join("\n")).toContain("absent from the live OpenShell gateway"); - expect(behaviorLog.mock.calls.flat().join("\n")).toContain("Stale-sandbox recovery"); - }); + ])( + "recovers $gatewayName and returns stale state after confirming the sandbox is absent (#4497)", + async ({ gatewayName, gatewayPort }) => { + const entry = makeSandboxEntry(gatewayName, gatewayPort); + const registrySnapshot = { sandboxes: { alpha: entry } }; + vi.mocked(registry.getSandbox).mockReturnValue(entry as never); + vi.mocked(registryPersistence.load).mockReturnValue(registrySnapshot as never); + captureOpenshellSpy + .mockReturnValueOnce({ + status: 1, + output: "client error (Connect): Connection refused", + }) + .mockReturnValueOnce({ status: 0, output: "beta Ready" }) + .mockReturnValueOnce({ status: 1, output: "Error: × Not Found: sandbox not found" }); + getNamedGatewayLifecycleStateSpy.mockReturnValue({ + state: "healthy_named", + activeGateway: gatewayName, + status: `Gateway: ${gatewayName}\nStatus: Connected`, + } as never); + const behaviorLog = vi.fn(); + + const result = await resolveRebuildLiveState("alpha", entry, behaviorLog, bail); + + expect(result).toEqual({ + staleRecovery: true, + staleRegistrySnapshot: registrySnapshot, + }); + expect(result?.staleRegistrySnapshot).not.toBe(registrySnapshot); + expect(recoverNamedGatewayRuntimeSpy).toHaveBeenCalledOnce(); + expect(recoverNamedGatewayRuntimeSpy).toHaveBeenCalledWith({ + gatewayName, + recoverableStates: recoveryStates, + }); + expect(captureOpenshellSpy).toHaveBeenNthCalledWith( + 1, + ["sandbox", "list", "-g", gatewayName], + expect.objectContaining({ ignoreError: true }), + ); + expect(captureOpenshellSpy).toHaveBeenNthCalledWith( + 2, + ["sandbox", "list", "-g", gatewayName], + expect.objectContaining({ ignoreError: true }), + ); + expect(captureOpenshellSpy).toHaveBeenNthCalledWith( + 3, + ["sandbox", "get", "-g", gatewayName, "alpha"], + expect.anything(), + ); + expect(getNamedGatewayLifecycleStateSpy).not.toHaveBeenCalled(); + expect(recoverDockerDriverSandboxSpy).toHaveBeenCalledWith("alpha"); + expect(registryPersistence.load).toHaveBeenCalledOnce(); + expect(logSpy.mock.calls.flat().join("\n")).toContain( + "absent from the live OpenShell gateway", + ); + expect(behaviorLog.mock.calls.flat().join("\n")).toContain("Stale-sandbox recovery"); + }, + ); it("fails without a sandbox-list retry after a generic query error", async () => { const entry = makeSandboxEntry(); @@ -367,13 +377,12 @@ describe("rebuild gateway drift preflight", () => { "Failed to query running sandboxes from OpenShell.", ); - expect(recoverNamedGatewayRuntimeSpy).toHaveBeenCalledOnce(); - expect(recoverNamedGatewayRuntimeSpy).toHaveBeenCalledWith({ - gatewayName: "nemoclaw", - recoverableStates: recoveryStates, - }); + expect(recoverNamedGatewayRuntimeSpy).not.toHaveBeenCalled(); expect(captureOpenshellSpy).toHaveBeenCalledOnce(); - expect(captureOpenshellSpy).toHaveBeenCalledWith(["sandbox", "list"]); + expect(captureOpenshellSpy).toHaveBeenCalledWith( + ["sandbox", "list", "-g", "nemoclaw"], + expect.objectContaining({ ignoreError: true }), + ); expect(getNamedGatewayLifecycleStateSpy).not.toHaveBeenCalled(); expect(recoverDockerDriverSandboxSpy).not.toHaveBeenCalled(); expect(registryPersistence.load).not.toHaveBeenCalled(); diff --git a/src/lib/actions/sandbox/rebuild-prepared-recovery.test.ts b/src/lib/actions/sandbox/rebuild-prepared-recovery.test.ts index 68875abda42..dd3cc163f44 100644 --- a/src/lib/actions/sandbox/rebuild-prepared-recovery.test.ts +++ b/src/lib/actions/sandbox/rebuild-prepared-recovery.test.ts @@ -15,7 +15,9 @@ describe("prepared rebuild recovery", () => { it("restores the validated pre-upgrade manifest without taking a second backup (#6114)", async () => { const harness = createRebuildFlowHarness({ applyPreset: () => true, - sandboxListOutput: "alpha Error", + sandboxInventory: { + sandboxes: [{ name: "alpha", phase: "Error", readiness: "terminal" }], + }, }); const recoveryManifest = makePreparedRecoveryManifest(); @@ -56,7 +58,9 @@ describe("prepared rebuild recovery", () => { it("carries confirmed legacy managed-image recovery through the delete edge (#6114)", async () => { const harness = createRebuildFlowHarness({ applyPreset: () => true, - sandboxListOutput: "alpha Error", + sandboxInventory: { + sandboxes: [{ name: "alpha", phase: "Error", readiness: "terminal" }], + }, sandboxEntry: { nemoclawVersion: null }, managedImageEvidence: false, }); @@ -84,7 +88,9 @@ describe("prepared rebuild recovery", () => { it("rejects an ambiguous legacy image without the scoped recovery capability (#6114)", async () => { const harness = createRebuildFlowHarness({ - sandboxListOutput: "alpha Error", + sandboxInventory: { + sandboxes: [{ name: "alpha", phase: "Error", readiness: "terminal" }], + }, sandboxEntry: { nemoclawVersion: null }, managedImageEvidence: false, }); @@ -102,7 +108,9 @@ describe("prepared rebuild recovery", () => { it("rejects recorded custom-image evidence despite the scoped recovery capability (#6114)", async () => { const harness = createRebuildFlowHarness({ - sandboxListOutput: "alpha Error", + sandboxInventory: { + sandboxes: [{ name: "alpha", phase: "Error", readiness: "terminal" }], + }, sandboxEntry: { nemoclawVersion: null, fromDockerfile: "/tmp/custom.Dockerfile", diff --git a/src/lib/actions/sandbox/rebuild-resume-snapshot.test.ts b/src/lib/actions/sandbox/rebuild-resume-snapshot.test.ts index e0ed83daa15..fd41d7a4778 100644 --- a/src/lib/actions/sandbox/rebuild-resume-snapshot.test.ts +++ b/src/lib/actions/sandbox/rebuild-resume-snapshot.test.ts @@ -121,7 +121,12 @@ describe("rebuild resume snapshot repair", () => { attempted: false, }), vi.spyOn(sandboxList, "captureSandboxListWithGatewayRecovery").mockResolvedValue({ - result: { status: 0, output: "alpha Ready" }, + result: { + ok: true, + value: { + sandboxes: [{ name: "alpha", phase: "Ready", readiness: "ready" }], + }, + }, recoveryAttempted: false, recoverySucceeded: false, }), diff --git a/src/lib/actions/sandbox/start-wait.test.ts b/src/lib/actions/sandbox/start-wait.test.ts index 9f1f7bc1703..9a8086dbbe9 100644 --- a/src/lib/actions/sandbox/start-wait.test.ts +++ b/src/lib/actions/sandbox/start-wait.test.ts @@ -1,67 +1,122 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { createConnectHarness } from "../../../../test/support/connect-flow-test-harness"; describe("sandbox start readiness", () => { - it("waits through the stopped sandbox Error phase after start (#9753)", () => { + it("waits through the stopped sandbox Error phase after start (#9753)", async () => { const harness = createConnectHarness({ listOutputs: ["alpha Error", "alpha Provisioning", "alpha Ready"], }); - expect(() => + await expect( harness.waitForSandboxReadyOrExit("alpha", { allowInitialErrorAfterStart: true }), - ).not.toThrow(); + ).resolves.toBeUndefined(); expect(harness.captureOpenshellSpy).toHaveBeenCalledTimes(3); }); - it("keeps Error terminal outside the post-start grace period (#9753)", () => { + it("keeps Error terminal outside the post-start grace period (#9753)", async () => { const harness = createConnectHarness({ listOutputs: ["alpha Error"] }); - expect(() => harness.waitForSandboxReadyOrExit("alpha")).toThrow( + await expect(harness.waitForSandboxReadyOrExit("alpha")).rejects.toThrow( 'process.exit unexpectedly called with "1"', ); expect(harness.captureOpenshellSpy).toHaveBeenCalledTimes(1); }); - it("ends the post-start Error grace after the phase advances (#9753)", () => { + it("ends the post-start Error grace after the phase advances (#9753)", async () => { const harness = createConnectHarness({ listOutputs: ["alpha Error", "alpha Provisioning", "alpha Error"], }); - expect(() => + await expect( harness.waitForSandboxReadyOrExit("alpha", { allowInitialErrorAfterStart: true }), - ).toThrow('process.exit unexpectedly called with "1"'); + ).rejects.toThrow('process.exit unexpectedly called with "1"'); expect(harness.captureOpenshellSpy).toHaveBeenCalledTimes(3); }); - it("fails after the stopped sandbox Error phase remains terminal (#9753)", () => { + it("fails after the stopped sandbox Error phase remains terminal (#9753)", async () => { const harness = createConnectHarness({ listOutputs: Array.from({ length: 11 }, () => "alpha Error"), }); - expect(() => + await expect( harness.waitForSandboxReadyOrExit("alpha", { allowInitialErrorAfterStart: true }), - ).toThrow('process.exit unexpectedly called with "1"'); + ).rejects.toThrow('process.exit unexpectedly called with "1"'); expect(harness.captureOpenshellSpy).toHaveBeenCalledTimes(11); }); it.each(["Failed", "CrashLoopBackOff"])( "fails immediately when start reports the terminal %s phase (#9753)", - (phase) => { + async (phase) => { const harness = createConnectHarness({ listOutputs: [`alpha ${phase}`] }); - expect(() => + await expect( harness.waitForSandboxReadyOrExit("alpha", { allowInitialErrorAfterStart: true }), - ).toThrow('process.exit unexpectedly called with "1"'); + ).rejects.toThrow('process.exit unexpectedly called with "1"'); expect(harness.captureOpenshellSpy).toHaveBeenCalledTimes(1); }, ); + + it.each([ + { + error: { + kind: "authentication", + message: "OpenShell could not authenticate the sandbox observation.", + }, + guidance: "could not authenticate", + }, + { + error: { + kind: "schema", + message: "The OpenShell CLI and gateway sandbox schemas do not match.", + }, + guidance: "schemas do not match", + }, + { + error: { kind: "timeout", message: "OpenShell sandbox observation timed out." }, + guidance: "readiness request for sandbox 'alpha' timed out", + }, + { + error: { + kind: "command", + reason: "failed", + message: "The OpenShell sandbox observation failed.", + }, + guidance: "readiness request for sandbox 'alpha' failed", + }, + { + error: { + kind: "transport", + reason: "unreachable", + message: "OpenShell could not reach the selected gateway.", + }, + guidance: "gateway is not running or unreachable", + }, + ] as const)( + "prints accurate $error.kind readiness failure guidance (#9803)", + async (testCase) => { + const harness = createConnectHarness(); + const observer = { + listSandboxes: vi.fn().mockResolvedValue({ ok: false, error: testCase.error }), + } as never; + + await expect(harness.waitForSandboxReadyOrExit("alpha", { observer })).rejects.toThrow( + 'process.exit unexpectedly called with "1"', + ); + + const output = harness.errorSpy.mock.calls.flat().join("\n"); + expect(output).toContain(testCase.guidance); + expect(output.includes("gateway is not running or unreachable")).toBe( + testCase.error.kind === "transport", + ); + }, + ); }); diff --git a/src/lib/actions/sandbox/start.test.ts b/src/lib/actions/sandbox/start.test.ts index 57e87d2ab8b..e79b8683e35 100644 --- a/src/lib/actions/sandbox/start.test.ts +++ b/src/lib/actions/sandbox/start.test.ts @@ -7,6 +7,7 @@ import path from "node:path"; import { describe, expect, it, vi } from "vitest"; import { testTimeoutOptions } from "../../../../test/helpers/timeouts"; +import type { OpenShellSandboxObserver } from "../../adapters/openshell/sandbox-observer"; import { createDockerRuntimeProviderBundle, createKubernetesRuntimeProviderBundle, @@ -115,12 +116,12 @@ function harness(overrides: Partial = {}) { } describe("startSandbox", () => { - it("restores sealed access before recovering sandbox processes (#8112)", () => { + it("restores sealed access before recovering sandbox processes (#8112)", async () => { const restoreAccess = vi.fn(); const recovery = SUCCESSFUL_RECOVERY; const restoreProcesses = vi.fn(() => recovery); - const result = restoreStoppedSandboxStartupState("my-sandbox", { + const result = await restoreStoppedSandboxStartupState("my-sandbox", { agent: "openclaw", restoreLockedStartupAccess: restoreAccess, waitForSandboxReady: vi.fn(), @@ -135,11 +136,11 @@ describe("startSandbox", () => { expect(result).toBe(recovery); }); - it("keeps Hermes sealed state untouched while recovering sandbox processes (#8112)", () => { + it("keeps Hermes sealed state untouched while recovering sandbox processes (#8112)", async () => { const restoreAccess = vi.fn(); const restoreProcesses = vi.fn(() => SUCCESSFUL_RECOVERY); - restoreStoppedSandboxStartupState("my-sandbox", { + await restoreStoppedSandboxStartupState("my-sandbox", { agent: "hermes", restoreLockedStartupAccess: restoreAccess, waitForSandboxReady: vi.fn(), @@ -150,12 +151,12 @@ describe("startSandbox", () => { expect(restoreProcesses).toHaveBeenCalledWith("my-sandbox"); }); - it("waits for OpenShell readiness after restoring sealed access and before recovering sandbox processes (#8978)", () => { + it("waits for OpenShell readiness after restoring sealed access and before recovering sandbox processes (#8978)", async () => { const restoreAccess = vi.fn(); const waitForSandboxReady = vi.fn(); const restoreProcesses = vi.fn(() => SUCCESSFUL_RECOVERY); - restoreStoppedSandboxStartupState("my-sandbox", { + await restoreStoppedSandboxStartupState("my-sandbox", { agent: "openclaw", restoreLockedStartupAccess: restoreAccess, waitForSandboxReady, @@ -171,11 +172,11 @@ describe("startSandbox", () => { ); }); - it("waits for OpenShell readiness before recovering Hermes sandbox processes (#8978)", () => { + it("waits for OpenShell readiness before recovering Hermes sandbox processes (#8978)", async () => { const waitForSandboxReady = vi.fn(); const restoreProcesses = vi.fn(() => SUCCESSFUL_RECOVERY); - restoreStoppedSandboxStartupState("my-sandbox", { + await restoreStoppedSandboxStartupState("my-sandbox", { agent: "hermes", restoreLockedStartupAccess: vi.fn(), waitForSandboxReady, @@ -222,16 +223,30 @@ describe("startSandbox", () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-start-readiness-")); vi.stubEnv("HOME", home); const listOutputs = ["my-sandbox Error", "my-sandbox Provisioning", "my-sandbox Ready"]; - const captureSandboxList = vi.fn(() => ({ - status: 0, - output: listOutputs.shift() ?? "my-sandbox Ready", - stdout: "", - stderr: "", - })); + const listSandboxes = vi.fn(async () => { + const output = listOutputs.shift() ?? "my-sandbox Ready"; + const phase = output.split(/\s+/u)[1] ?? null; + return { + ok: true, + value: { + sandboxes: [ + { + name: "my-sandbox", + phase, + readiness: + phase === "Ready" ? "ready" : phase === "Error" ? "terminal" : "not_ready", + }, + ], + }, + }; + }); + const observer: OpenShellSandboxObserver = { + listSandboxes, + }; const restoreProcesses = vi.fn(() => SUCCESSFUL_RECOVERY); const h = harness({ allowDockerRuntimeInspection: false, - captureSandboxList, + observer, environment: { ...process.env, HOME: home }, restoreLockedStartupAccess: vi.fn(), restoreProcessState: restoreProcesses, @@ -242,10 +257,10 @@ describe("startSandbox", () => { const result = await startSandbox("my-sandbox", h.deps); expect(result.exitCode).toBe(0); - expect(captureSandboxList).toHaveBeenCalledTimes(3); + expect(listSandboxes).toHaveBeenCalledTimes(3); expect(restoreProcesses).toHaveBeenCalledWith("my-sandbox"); expect(h.verifyGateway).toHaveBeenCalledWith("my-sandbox"); - expect(captureSandboxList.mock.invocationCallOrder[2]).toBeLessThan( + expect(listSandboxes.mock.invocationCallOrder[2]).toBeLessThan( restoreProcesses.mock.invocationCallOrder[0], ); expect(restoreProcesses.mock.invocationCallOrder[0]).toBeLessThan( @@ -443,17 +458,20 @@ describe("startSandbox", () => { }, /did not become ready in OpenShell/iu, ], - ] as const)("propagates an actionable %s %s failure (#8662)", async (agent, _layer, recovery, expected) => { - const h = harness(); - h.getSandbox.mockReturnValue(sandbox({ agent })); - h.restoreStartupState.mockReturnValue(recovery); + ] as const)( + "propagates an actionable %s %s failure (#8662)", + async (agent, _layer, recovery, expected) => { + const h = harness(); + h.getSandbox.mockReturnValue(sandbox({ agent })); + h.restoreStartupState.mockReturnValue(recovery); - const failure = await startSandbox("my-sandbox", h.deps).catch((error) => String(error)); - expect(failure).toMatch(expected); - expect(failure).toMatch(/nemoclaw my-sandbox recover/iu); - expect(failure).not.toContain(REDACTED_TOKEN); - expect(h.verifyGateway).not.toHaveBeenCalled(); - }); + const failure = await startSandbox("my-sandbox", h.deps).catch((error) => String(error)); + expect(failure).toMatch(expected); + expect(failure).toMatch(/nemoclaw my-sandbox recover/iu); + expect(failure).not.toContain(REDACTED_TOKEN); + expect(h.verifyGateway).not.toHaveBeenCalled(); + }, + ); it("does not claim preservation when startup recovery reports a failed rollback (#9364)", async () => { const h = harness(); @@ -676,24 +694,24 @@ describe("startSandbox", () => { expect(h.verifyGateway).not.toHaveBeenCalled(); }); - it.each([ - "unknown-runtime", - "mxc-not-installed", - ])("fails closed for unregistered provider %s without lifecycle side effects", async (providerId) => { - const h = harness(); - h.getSandbox.mockReturnValue(sandbox({ openshellDriver: providerId })); + it.each(["unknown-runtime", "mxc-not-installed"])( + "fails closed for unregistered provider %s without lifecycle side effects", + async (providerId) => { + const h = harness(); + h.getSandbox.mockReturnValue(sandbox({ openshellDriver: providerId })); - const result = await startSandbox("my-sandbox", h.deps); + const result = await startSandbox("my-sandbox", h.deps); - expect(result.exitCode).toBe(1); - expect(result.message).toContain(providerId); - expect(result.message).toContain("has no registered lifecycle provider"); - expect(h.findLabeledSandboxContainers).not.toHaveBeenCalled(); - expect(h.dockerUnpause).not.toHaveBeenCalled(); - expect(h.recoverDockerDriverSandbox).not.toHaveBeenCalled(); - expect(h.restoreStartupState).not.toHaveBeenCalled(); - expect(h.verifyGateway).not.toHaveBeenCalled(); - }); + expect(result.exitCode).toBe(1); + expect(result.message).toContain(providerId); + expect(result.message).toContain("has no registered lifecycle provider"); + expect(h.findLabeledSandboxContainers).not.toHaveBeenCalled(); + expect(h.dockerUnpause).not.toHaveBeenCalled(); + expect(h.recoverDockerDriverSandbox).not.toHaveBeenCalled(); + expect(h.restoreStartupState).not.toHaveBeenCalled(); + expect(h.verifyGateway).not.toHaveBeenCalled(); + }, + ); it.each([ ["null driver", sandbox({ openshellDriver: null })], diff --git a/src/lib/actions/sandbox/start.ts b/src/lib/actions/sandbox/start.ts index 0ece1124902..c80831a5aa4 100644 --- a/src/lib/actions/sandbox/start.ts +++ b/src/lib/actions/sandbox/start.ts @@ -1,8 +1,8 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import type { OpenShellSandboxObserver } from "../../adapters/openshell/sandbox-observer"; import { cliName } from "../../onboard/branding"; -import type { captureOpenshell } from "../../adapters/openshell/runtime"; import { CURRENT_RUNTIME_PROVIDER_BUNDLES, type RuntimeProviderBundleRegistry, @@ -42,20 +42,17 @@ function restoreLockedStartupAccess(sandboxName: string): void { } /** Wait for a just-started sandbox while tolerating its bounded transient Error phase. */ -function waitForSandboxReady( +async function waitForSandboxReady( sandboxName: string, - captureSandboxList?: ( - args: string[], - options: { readonly ignoreError: true; readonly timeout: number }, - ) => ReturnType, + observer?: OpenShellSandboxObserver, allowDockerRuntimeInspection = true, -): void { +): Promise { const { waitForSandboxReadyOrExit, SANDBOX_REPAIR_READY_TIMEOUT_SEC } = require("./connect") as typeof import("./connect"); - waitForSandboxReadyOrExit(sandboxName, { + await waitForSandboxReadyOrExit(sandboxName, { allowInitialErrorAfterStart: true, allowDockerRuntimeInspection, - ...(captureSandboxList ? { captureSandboxList } : {}), + ...(observer ? { observer } : {}), defaultTimeoutSec: SANDBOX_REPAIR_READY_TIMEOUT_SEC, retryCommand: "start", }); @@ -64,33 +61,32 @@ function waitForSandboxReady( export interface SandboxStartupStateDeps { agent?: SandboxEntry["agent"]; restoreLockedStartupAccess?: (sandboxName: string) => void; - waitForSandboxReady?: (sandboxName: string) => void; + waitForSandboxReady?: (sandboxName: string) => void | Promise; restoreProcessState?: (sandboxName: string) => SandboxStartupRecoveryResult; } -export function restoreStoppedSandboxStartupState( +export async function restoreStoppedSandboxStartupState( sandboxName: string, deps: SandboxStartupStateDeps = {}, -): SandboxStartupRecoveryResult { +): Promise { if ((deps.agent ?? "openclaw") === "openclaw") { (deps.restoreLockedStartupAccess ?? restoreLockedStartupAccess)(sandboxName); } - (deps.waitForSandboxReady ?? waitForSandboxReady)(sandboxName); + await (deps.waitForSandboxReady ?? waitForSandboxReady)(sandboxName); return (deps.restoreProcessState ?? restoreProcessState)(sandboxName); } export interface SandboxStartDeps { allowDockerRuntimeInspection?: boolean; - captureSandboxList?: ( - args: string[], - options: { readonly ignoreError: true; readonly timeout: number }, - ) => ReturnType; + observer?: OpenShellSandboxObserver; environment?: NodeJS.ProcessEnv; getSandbox?: typeof registry.getSandbox; restoreLockedStartupAccess?: (sandboxName: string) => void; restoreProcessState?: (sandboxName: string) => SandboxStartupRecoveryResult; runtimeProviders?: RuntimeProviderBundleRegistry; - restoreStartupState?: (sandboxName: string) => SandboxStartupRecoveryResult; + restoreStartupState?: ( + sandboxName: string, + ) => SandboxStartupRecoveryResult | Promise; waitForManagedGatewaySupervisor?: (sandboxName: string) => boolean; verifyGateway?: (sandboxName: string) => Promise; probeInferenceInvocation?: typeof probeSandboxInferenceInvocation; @@ -221,15 +217,11 @@ async function startSandboxWithinLifecycleFence( restoreLockedStartupAccess: deps.restoreLockedStartupAccess, restoreProcessState: deps.restoreProcessState, waitForSandboxReady: (readyName) => - waitForSandboxReady( - readyName, - deps.captureSandboxList, - deps.allowDockerRuntimeInspection, - ), + waitForSandboxReady(readyName, deps.observer, deps.allowDockerRuntimeInspection), })); let recovery: SandboxStartupRecoveryResult; try { - recovery = restoreStartupState(name); + recovery = await restoreStartupState(name); } catch (error) { throw startupRecoveryError(name, error); } @@ -247,7 +239,7 @@ async function startSandboxWithinLifecycleFence( } if (supervisorReady) { try { - recovery = restoreStartupState(name); + recovery = await restoreStartupState(name); } catch (error) { throw startupRecoveryError(name, error); } diff --git a/src/lib/actions/sandbox/status-lookup-rendering.ts b/src/lib/actions/sandbox/status-lookup-rendering.ts index e14fdba1eac..0785c151930 100644 --- a/src/lib/actions/sandbox/status-lookup-rendering.ts +++ b/src/lib/actions/sandbox/status-lookup-rendering.ts @@ -9,7 +9,7 @@ import { isTerminalSandboxPhase } from "../../state/gateway"; import { getSandboxDockerRuntime } from "./docker-health"; import { isDockerRuntimeDown, printDockerRuntimeDownGuidance } from "./gateway-failure-classifier"; import type { SandboxGatewayState } from "./gateway-state"; -import { printGatewayLifecycleHint, printWrongGatewayActiveGuidance } from "./gateway-state"; +import { printSandboxGatewayStateHint, printWrongGatewayActiveGuidance } from "./gateway-state"; import { getSandboxTargetGatewayName } from "./gateway-target"; import { printGatewayFailureLayerHeader, @@ -223,7 +223,7 @@ async function printUnknownGatewayLookupStatus({ console.log(lookup.output); } await printGatewayFailureLayerHeader(sandboxName, effectivePreflight.failureLayer); - printGatewayLifecycleHint(lookup.output, sandboxName, console.log); + printSandboxGatewayStateHint(lookup, sandboxName, console.log); deferSandboxLifecycleExit(1); } diff --git a/src/lib/actions/sandbox/status-snapshot-recovery.test.ts b/src/lib/actions/sandbox/status-snapshot-recovery.test.ts index 48a1f768c66..b8ec24c683d 100644 --- a/src/lib/actions/sandbox/status-snapshot-recovery.test.ts +++ b/src/lib/actions/sandbox/status-snapshot-recovery.test.ts @@ -55,6 +55,7 @@ const healthyRoute: SandboxInferenceRouteHealth = { function recoveredLookup() { return Promise.resolve({ state: "present", + phase: "Ready", output: "Phase: Ready", recoveredSandbox: true, recoverySandboxVia: "started-stopped-original", @@ -90,6 +91,7 @@ describe("collectSandboxStatusSnapshot Docker recovery", () => { reconcile: () => Promise.resolve({ state: "present" as const, + phase: "Ready", output: "Phase: Ready", }), }; @@ -111,6 +113,7 @@ describe("collectSandboxStatusSnapshot Docker recovery", () => { reconcile: () => Promise.resolve({ state: "present" as const, + phase: "Ready", output: "Phase: Ready", }), }; @@ -124,29 +127,30 @@ describe("collectSandboxStatusSnapshot Docker recovery", () => { expect(deps.probeSandboxInferenceGatewayHealthImpl).not.toHaveBeenCalled(); }); - it.each([ - "Provisioning", - "Failed", - ])("keeps the existing %s phase diagnosis ahead of markerless recovery (#7824)", async (phase) => { - const deps = { - ...snapshotDeps({ - checked: true, - wasRunning: false, - recovered: false, - forwardRecovered: false, - }), - reconcile: () => - Promise.resolve({ - state: "present" as const, - output: `Phase: ${phase}`, + it.each(["Provisioning", "Failed"])( + "keeps the existing %s phase diagnosis ahead of markerless recovery (#7824)", + async (phase) => { + const deps = { + ...snapshotDeps({ + checked: true, + wasRunning: false, + recovered: false, + forwardRecovered: false, }), - }; - - const snapshot = await collectSandboxStatusSnapshot("alpha", { deps }); - - expect(deps.recoverSandboxProcesses).not.toHaveBeenCalled(); - expect(snapshot.lookup.state).toBe("present"); - }); + reconcile: () => + Promise.resolve({ + state: "present" as const, + phase, + output: `Phase: ${phase}`, + }), + }; + + const snapshot = await collectSandboxStatusSnapshot("alpha", { deps }); + + expect(deps.recoverSandboxProcesses).not.toHaveBeenCalled(); + expect(snapshot.lookup.state).toBe("present"); + }, + ); it("keeps a host preflight failure ahead of markerless recovery (#7824)", async () => { const deps = { @@ -159,6 +163,7 @@ describe("collectSandboxStatusSnapshot Docker recovery", () => { reconcile: () => Promise.resolve({ state: "present" as const, + phase: "Ready", output: "Phase: Ready", }), }; diff --git a/src/lib/actions/sandbox/status-snapshot.ts b/src/lib/actions/sandbox/status-snapshot.ts index ccdfa74fcdb..5d89ac63bbf 100644 --- a/src/lib/actions/sandbox/status-snapshot.ts +++ b/src/lib/actions/sandbox/status-snapshot.ts @@ -32,7 +32,6 @@ import { resolveSandboxGatewayName } from "../../onboard/gateway-binding"; import { getBaselineExclusionRuntimeStatus } from "../../policy"; import type { BaselineExclusionRuntimeStatus } from "../../policy/baseline-exclusion"; import { redact } from "../../security/redact"; -import { parseSandboxPhase } from "../../state/gateway"; import * as registry from "../../state/registry"; import { buildGatewayInferenceGetArgs, @@ -269,7 +268,7 @@ export function resolveSandboxStatusAgent(agentName = "openclaw"): SandboxStatus type ReconcileSandboxGatewayState = (sandboxName: string) => Promise; type ProbeTerminalRuntimeHealth = (sandboxName: string) => TerminalRuntimeOomProbeResult; type RecoverSandboxProcesses = - typeof import("./status/process-recovery")["checkAndRecoverSandboxProcesses"]; + (typeof import("./status/process-recovery"))["checkAndRecoverSandboxProcesses"]; type SandboxProcessRecoveryResult = ReturnType; type SandboxProcessRecoveryFailure = { @@ -435,7 +434,7 @@ export async function collectSandboxStatusSnapshot( lookup.state === "present" && sb?.openshellDriver === "docker" && (sb.agent ?? "openclaw") === "openclaw" && - parseSandboxPhase(lookup.output || "") === "Ready" && + lookup.phase === "Ready" && !opts.preflight?.failure; let recoveredManagedGateway = false; if ( @@ -535,13 +534,13 @@ export async function collectSandboxStatusSnapshot( recorded: routeDriftPlan.recorded, canConnect: Boolean( sb && - gatewayName && - canSandboxGatewayRouteRealign( - sandboxName, - sb, - gatewayName, - (opts.deps?.listSandboxes ?? registry.listSandboxes)().sandboxes, - ), + gatewayName && + canSandboxGatewayRouteRealign( + sandboxName, + sb, + gatewayName, + (opts.deps?.listSandboxes ?? registry.listSandboxes)().sandboxes, + ), ), } : null; @@ -717,7 +716,7 @@ async function buildSandboxStatusReport( terminalRuntimeHealth, } = snapshot; const dockerRuntime = lookup.state === "present" ? getSandboxDockerRuntime(sandboxName) : null; - const phase = lookup.state === "present" ? parseSandboxPhase(lookup.output || "") : null; + const phase = lookup.state === "present" ? (lookup.phase ?? null) : null; const effectivePreflight = withoutTerminalPhasePreflight( snapshot.postRecoveryPreflight ?? preflight, phase, diff --git a/src/lib/actions/sandbox/status.ts b/src/lib/actions/sandbox/status.ts index f33b4ef81ca..0d64b62f974 100644 --- a/src/lib/actions/sandbox/status.ts +++ b/src/lib/actions/sandbox/status.ts @@ -5,7 +5,6 @@ import { printOpenShellStateRpcIssue } from "../../adapters/openshell/gateway-dr import { CLI_NAME } from "../../cli/branding"; import { deferSandboxLifecycleExit, isSandboxLifecycleDeferredExit } from "../../core/process-exit"; import { inspectManagedLlamaCppStatus } from "../../inference/llama-cpp/managed-status"; -import { parseSandboxPhase } from "../../state/gateway"; import { withMcpLifecycleLock } from "../../state/mcp-lifecycle-lock-acquisition"; import * as registry from "../../state/registry"; import { getSandboxDockerRuntime } from "./docker-health"; @@ -196,7 +195,7 @@ async function showLegacySandboxStatus(sandboxName: string): Promise { // Resolve the docker-driver container once: reused for the paused-container // recovery hint (#4495) and the Docker health line below (#3975). const dockerRuntime = lookup.state === "present" ? getSandboxDockerRuntime(sandboxName) : null; - const phase = lookup.state === "present" ? parseSandboxPhase(lookup.output || "") : null; + const phase = lookup.state === "present" ? (lookup.phase ?? null) : null; const effectivePreflight = withoutTerminalPhasePreflight( snapshot.postRecoveryPreflight ?? preflight, phase, diff --git a/src/lib/actions/upgrade-sandboxes-preflight.test.ts b/src/lib/actions/upgrade-sandboxes-preflight.test.ts index 70f701599c7..379c5620413 100644 --- a/src/lib/actions/upgrade-sandboxes-preflight.test.ts +++ b/src/lib/actions/upgrade-sandboxes-preflight.test.ts @@ -13,8 +13,6 @@ const mocks = vi.hoisted(() => ({ getLatestBackup: vi.fn(), getVersion: vi.fn(), listSandboxes: vi.fn(), - parseLiveSandboxEntries: vi.fn(), - parseReadySandboxNames: vi.fn(), prompt: vi.fn(), shouldSkipUpgradeConfirmation: vi.fn(), splitRebuildableSandboxes: vi.fn(), @@ -36,10 +34,6 @@ vi.mock("../openshell-sandbox-list", () => ({ captureNamedGatewaySandboxListReadOnly: mocks.captureNamedGatewaySandboxListReadOnly, captureSandboxListWithGatewayPreflightOrExit: mocks.captureSandboxListWithGatewayPreflightOrExit, })); -vi.mock("../runtime-recovery", () => ({ - parseLiveSandboxEntries: mocks.parseLiveSandboxEntries, - parseReadySandboxNames: mocks.parseReadySandboxNames, -})); vi.mock("../sandbox/version", () => ({ checkAgentVersion: mocks.checkAgentVersion })); vi.mock("../state/registry", () => ({ isPublishedSandboxRegistration: (entry: { pendingRouteReservation?: true }) => @@ -56,20 +50,15 @@ describe("upgrade-sandboxes gateway preflight adapter (#6237)", () => { vi.stubEnv("NEMOCLAW_RESTORE_LATEST_BACKUP_ON_RECREATE", ""); vi.spyOn(upgradeSandboxesDependencies, "getGatewayPort").mockReturnValue(8080); vi.spyOn(upgradeSandboxesDependencies, "rebuildSandbox").mockResolvedValue(undefined); - mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue({ - status: 0, - output: "alpha Ready", - }); - mocks.captureNamedGatewaySandboxListReadOnly.mockReturnValue({ - status: 0, - output: "alpha Ready", - }); + const inventory = { + sandboxes: [{ name: "alpha", phase: null, readiness: "ready" as const }], + }; + mocks.captureSandboxListWithGatewayPreflightOrExit.mockResolvedValue(inventory); + mocks.captureNamedGatewaySandboxListReadOnly.mockResolvedValue(inventory); mocks.getVersion.mockReturnValue("0.0.74"); mocks.listSandboxes.mockReturnValue({ sandboxes: [{ name: "alpha", provider: "nvidia-prod", model: "nemotron" }], }); - mocks.parseLiveSandboxEntries.mockReturnValue([{ name: "alpha", phase: "Ready" }]); - mocks.parseReadySandboxNames.mockReturnValue(new Set(["alpha"])); mocks.classifyUpgradeableSandboxes.mockReturnValue({ stale: [], unknown: [] }); mocks.shouldSkipUpgradeConfirmation.mockReturnValue(true); mocks.splitRebuildableSandboxes.mockReturnValue({ rebuildable: [], stopped: [] }); diff --git a/src/lib/actions/upgrade-sandboxes-recovery.test.ts b/src/lib/actions/upgrade-sandboxes-recovery.test.ts index 6fc72c05295..6fd1b46b7ae 100644 --- a/src/lib/actions/upgrade-sandboxes-recovery.test.ts +++ b/src/lib/actions/upgrade-sandboxes-recovery.test.ts @@ -3,6 +3,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; +import { parseCliOpenShellSandboxInventory } from "../adapters/openshell/sandbox-observer-cli"; import * as coreVersion from "../core/version"; import * as sandboxList from "../openshell-sandbox-list"; import * as sandboxVersion from "../sandbox/version"; @@ -40,6 +41,10 @@ function makeManifest(sandboxName: string, agentType: ManifestAgentType = "openc }; } +function sandboxInventory(output: string) { + return parseCliOpenShellSandboxInventory(output); +} + function createRecoveryHarness( names: string[], options: { @@ -91,19 +96,17 @@ function createRecoveryHarness( vi.spyOn(coreVersion, "getVersion").mockReturnValue("0.0.71"); const liveListSpy = vi .spyOn(sandboxList, "captureSandboxListWithGatewayPreflightOrExit") - .mockResolvedValue({ - status: 0, - output: options.liveOutput ?? names.map((name) => `${name} Error`).join("\n"), - }); + .mockResolvedValue( + sandboxInventory(options.liveOutput ?? names.map((name) => `${name} Error`).join("\n")), + ); // #7279: check mode observes gateways through the read-only helper instead of // the recovering preflight; keep both stubbed so a check-mode run never hits // the real openshell adapter. const readOnlyListSpy = vi .spyOn(sandboxList, "captureNamedGatewaySandboxListReadOnly") - .mockReturnValue({ - status: 0, - output: options.liveOutput ?? names.map((name) => `${name} Error`).join("\n"), - }); + .mockResolvedValue( + sandboxInventory(options.liveOutput ?? names.map((name) => `${name} Error`).join("\n")), + ); vi.spyOn(registry, "listSandboxes").mockReturnValue({ defaultSandbox: null, sandboxes: names.map((name) => ({ @@ -714,8 +717,8 @@ describe("upgrade-sandboxes prepared backup recovery (#6114)", () => { staleNames: ["reconnecting-box"], }); harness.liveListSpy - .mockResolvedValueOnce({ status: 0, output: "other-box Ready" }) - .mockResolvedValueOnce({ status: 0, output: "reconnecting-box Ready" }); + .mockResolvedValueOnce(sandboxInventory("other-box Ready")) + .mockResolvedValueOnce(sandboxInventory("reconnecting-box Ready")); await expect(harness.upgradeSandboxes({ auto: true })).resolves.toBeUndefined(); @@ -777,8 +780,8 @@ describe("upgrade-sandboxes prepared backup recovery (#6114)", () => { liveOutput: "other-box Ready", }); harness.liveListSpy - .mockResolvedValueOnce({ status: 0, output: "other-box Ready" }) - .mockResolvedValueOnce({ status: 0, output: "still-other-box Ready" }); + .mockResolvedValueOnce(sandboxInventory("other-box Ready")) + .mockResolvedValueOnce(sandboxInventory("still-other-box Ready")); await expect(harness.upgradeSandboxes({ auto: true })).resolves.toBeUndefined(); @@ -801,8 +804,8 @@ describe("upgrade-sandboxes prepared backup recovery (#6114)", () => { const harness = createRecoveryHarness(["healthy-box"], { gatewayPort: 12345 }); harness.liveListSpy.mockImplementation(async (...args: unknown[]) => (args[1] as { gatewayName?: string } | undefined)?.gatewayName === targetGatewayName - ? { status: 0, output: "healthy-box Ready" } - : { status: 0, output: "default-other-box Ready" }, + ? sandboxInventory("healthy-box Ready") + : sandboxInventory("default-other-box Ready"), ); await expect(harness.upgradeSandboxes({ auto: true })).resolves.toBeUndefined(); @@ -868,8 +871,8 @@ describe("upgrade-sandboxes prepared backup recovery (#6114)", () => { staleNames: ["reconnecting-box"], }); harness.liveListSpy - .mockResolvedValueOnce({ status: 0, output: "other-box Ready" }) - .mockResolvedValueOnce({ status: 0, output: "reconnecting-box Ready" }); + .mockResolvedValueOnce(sandboxInventory("other-box Ready")) + .mockResolvedValueOnce(sandboxInventory("reconnecting-box Ready")); await expect(harness.upgradeSandboxes({ auto: true })).resolves.toBeUndefined(); @@ -888,8 +891,8 @@ describe("upgrade-sandboxes prepared backup recovery (#6114)", () => { liveOutput: "other-box Ready", }); harness.liveListSpy - .mockResolvedValueOnce({ status: 0, output: "other-box Ready" }) - .mockResolvedValueOnce({ status: 0, output: `orphaned-box ${phase}` }); + .mockResolvedValueOnce(sandboxInventory("other-box Ready")) + .mockResolvedValueOnce(sandboxInventory(`orphaned-box ${phase}`)); await expect(harness.upgradeSandboxes({ auto: true })).resolves.toBeUndefined(); diff --git a/src/lib/actions/upgrade-sandboxes.ts b/src/lib/actions/upgrade-sandboxes.ts index 3b5035f4166..c04caba748e 100644 --- a/src/lib/actions/upgrade-sandboxes.ts +++ b/src/lib/actions/upgrade-sandboxes.ts @@ -26,7 +26,6 @@ import { captureNamedGatewaySandboxListReadOnly, captureSandboxListWithGatewayPreflightOrExit, } from "../openshell-sandbox-list"; -import { parseLiveSandboxEntries, parseReadySandboxNames } from "../runtime-recovery"; import * as sandboxVersion from "../sandbox/version"; import { diagnosticPreview, isValidName, NAME_ALLOWED_FORMAT } from "../sandbox-name-contract"; import * as registry from "../state/registry"; @@ -205,11 +204,15 @@ async function confirmAbsentRecoveryCandidates( }; // #7279: a read-only check must never recover/select the gateway. const confirmation = checkOnly - ? captureNamedGatewaySandboxListReadOnly(context, selectedGatewayName) + ? await captureNamedGatewaySandboxListReadOnly(context, selectedGatewayName) : await captureSandboxListWithGatewayPreflightOrExit(context, { gatewayName: selectedGatewayName, }); - const confirmedLiveNames = parseReadySandboxNames(confirmation.output || ""); + const confirmedLiveNames = new Set( + confirmation.sandboxes + .filter((sandbox) => sandbox.readiness === "ready") + .map((sandbox) => sandbox.name), + ); return absentCandidates.filter((sandbox) => !confirmedLiveNames.has(sandbox.name)); } @@ -298,21 +301,23 @@ export async function upgradeSandboxes( command: `${CLI_NAME} upgrade-sandboxes`, }; const liveResult = checkOnly - ? captureNamedGatewaySandboxListReadOnly(liveListContext, selectedGatewayName) + ? await captureNamedGatewaySandboxListReadOnly(liveListContext, selectedGatewayName) : await captureSandboxListWithGatewayPreflightOrExit(liveListContext, { gatewayName: selectedGatewayName, }); - const liveNames = parseReadySandboxNames(liveResult.output || ""); + const liveNames = new Set( + liveResult.sandboxes + .filter((sandbox) => sandbox.readiness === "ready") + .map((sandbox) => sandbox.name), + ); // Sandboxes the selected gateway observes in a non-Ready phase. Absence from // the selected gateway and stale Ready/Running rows are handled by // isPreparedRecoveryCandidate, which recovers them only when they resolve to // the selected gateway. const nonReadyLiveNames = new Set( - parseLiveSandboxEntries(liveResult.output || "") - .filter( - (entry) => entry.phase !== null && entry.phase !== "Ready" && entry.phase !== "Running", - ) - .map((entry) => entry.name), + liveResult.sandboxes + .filter((sandbox) => sandbox.phase !== null && sandbox.readiness !== "ready") + .map((sandbox) => sandbox.name), ); // Classify sandboxes as stale, unknown, or current. Pass the running NemoClaw diff --git a/src/lib/adapters/openshell/sandbox-observer-cli.test.ts b/src/lib/adapters/openshell/sandbox-observer-cli.test.ts new file mode 100644 index 00000000000..30a3fa74b5a --- /dev/null +++ b/src/lib/adapters/openshell/sandbox-observer-cli.test.ts @@ -0,0 +1,221 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it, vi } from "vitest"; + +import { + createCliOpenShellSandboxLookup, + createCliOpenShellSandboxObserver as createObserver, + type CapturedSandboxCommandResult, + type CliOpenShellSandboxObserverDeps, + parseCliOpenShellSandboxInventory, +} from "./sandbox-observer-cli"; +import { namedOpenShellGateway, selectedOpenShellGateway } from "./sandbox-observer"; + +function createCliOpenShellSandboxObserver( + deps: Omit, +) { + return createObserver({ ...deps, defaultTimeoutMs: 15_000 }); +} + +function captured( + status: number | null, + stdout = "", + stderr = "", + error?: Error, +): CapturedSandboxCommandResult { + return { + status, + output: `${stdout}${stderr}`.trim(), + stdout, + stderr, + ...(error ? { error } : {}), + }; +} + +describe("CLI OpenShell sandbox observer", () => { + it("targets a named gateway and returns typed list observations (#9803)", async () => { + const capture = vi.fn(() => + captured( + 0, + [ + "NAME CREATED PHASE", + "alpha 2m Ready", + "beta 1m Provisioning", + "gamma 30s CrashLoopBackOff", + ].join("\n"), + ), + ); + const observer = createCliOpenShellSandboxObserver({ capture }); + + const result = await observer.listSandboxes({ + target: namedOpenShellGateway("nemoclaw-18080"), + timeoutMs: 4_321, + }); + + expect(capture).toHaveBeenCalledWith(["sandbox", "list", "-g", "nemoclaw-18080"], { + ignoreError: true, + includeStderr: true, + includeStreams: true, + timeout: 4_321, + }); + expect(result).toEqual({ + ok: true, + value: { + sandboxes: [ + { name: "alpha", phase: "Ready", readiness: "ready" }, + { name: "beta", phase: "Provisioning", readiness: "not_ready" }, + { name: "gamma", phase: "CrashLoopBackOff", readiness: "terminal" }, + ], + }, + }); + }); + + it("contains table and ANSI compatibility inside the CLI implementation (#9803)", () => { + expect( + parseCliOpenShellSandboxInventory( + "\u001b[1mNAME\u001b[0m CREATED PHASE\n" + + "\u001b[1malpha\u001b[0m 2m \u001b[32mReady\u001b[0m\n" + + "beta Ready NotReady 1m ago\n" + + "gamma unknown 1m ago\n" + + "delta Ready 2026-03-24 10:00:00 Provisioning\n" + + "epsilon 1m Running\n" + + "Error: command failed\n" + + "No sandboxes found.", + ), + ).toEqual({ + sandboxes: [ + { name: "alpha", phase: "Ready", readiness: "ready" }, + { name: "beta", phase: "NotReady", readiness: "not_ready" }, + { name: "gamma", phase: "Unknown", readiness: "terminal" }, + { name: "delta", phase: "Provisioning", readiness: "not_ready" }, + { name: "epsilon", phase: "Running", readiness: "ready" }, + ], + }); + }); + + it("parses successful list output from stdout without treating stderr as inventory (#9803)", async () => { + const observer = createCliOpenShellSandboxObserver({ + capture: () => captured(0, "alpha Ready", "warning text"), + }); + + await expect(observer.listSandboxes({ target: selectedOpenShellGateway() })).resolves.toEqual({ + ok: true, + value: { + sandboxes: [{ name: "alpha", phase: "Ready", readiness: "ready" }], + }, + }); + }); + + it("keeps formatted get output on an explicit CLI-only compatibility path (#9803)", async () => { + const capture = vi.fn(() => captured(0, "\u001b[1mName:\u001b[0m alpha\nPhase: Running\n")); + const lookup = createCliOpenShellSandboxLookup({ capture }); + + const result = await lookup({ + sandboxName: "alpha", + target: namedOpenShellGateway("nemoclaw"), + timeoutMs: 1_000, + }); + + expect(capture).toHaveBeenCalledWith(["sandbox", "get", "-g", "nemoclaw", "alpha"], { + ignoreError: true, + includeStderr: true, + includeStreams: true, + timeout: 1_000, + }); + expect(result).toEqual({ + result: { + ok: true, + value: { + state: "present", + sandbox: { name: "alpha", phase: "Running", readiness: "ready" }, + }, + }, + displayOutput: "Name: alpha\nPhase: Running", + }); + }); + + it("canonicalizes lookup phase tokens case-insensitively (#9803)", async () => { + const lookup = createCliOpenShellSandboxLookup({ + capture: () => captured(0, "Name: alpha\nPhase: ready\n"), + }); + + await expect( + lookup({ sandboxName: "alpha", target: selectedOpenShellGateway() }), + ).resolves.toEqual({ + result: { + ok: true, + value: { + state: "present", + sandbox: { name: "alpha", phase: "Ready", readiness: "ready" }, + }, + }, + displayOutput: "Name: alpha\nPhase: ready", + }); + }); + + it("keeps a missing sandbox distinct from authentication failure (#9803)", async () => { + const capture = vi + .fn() + .mockReturnValueOnce(captured(1, "", "sandbox has no spec: NotFound")) + .mockReturnValueOnce( + captured(1, "", "Error: authentication failed: sandbox not found: bearer value"), + ); + const lookup = createCliOpenShellSandboxLookup({ capture }); + const request = { sandboxName: "alpha", target: selectedOpenShellGateway() }; + + await expect(lookup(request)).resolves.toEqual({ + result: { ok: true, value: { state: "missing" } }, + displayOutput: "", + }); + await expect(lookup(request)).resolves.toEqual({ + result: { + ok: false, + error: { + kind: "authentication", + message: "OpenShell could not authenticate the sandbox observation.", + }, + }, + displayOutput: "", + }); + }); + + it.each([ + ["transport", "unreachable", captured(1, "", "client error (Connect): Connection refused")], + ["transport", "unreachable", captured(1, "", "Status: Disconnected")], + ["transport", "identity_mismatch", captured(1, "", "handshake verification failed")], + ["schema", undefined, captured(1, "", "protobuf decode error: invalid wire type")], + ["command", "failed", captured(7, "", "unexpected opaque failure")], + ["command", "invalid_request", captured(2, "", "unknown option")], + ] as const)( + "maps %s failures without retaining CLI diagnostics (#9803)", + async (kind, reason, value) => { + const observer = createCliOpenShellSandboxObserver({ capture: () => value }); + + const result = await observer.listSandboxes({ target: selectedOpenShellGateway() }); + + expect(result.ok).toBe(false); + const mapped = result as Extract; + expect(mapped.error.kind).toBe(kind); + expect(mapped.error).toMatchObject(reason ? { reason } : {}); + expect(mapped.error.message).not.toContain(value.stderr); + }, + ); + + it("maps a subprocess timeout without retaining its command text (#9803)", async () => { + const timeout = new Error( + "spawn openshell sandbox list token-value timed out", + ) as NodeJS.ErrnoException; + timeout.code = "ETIMEDOUT"; + const observer = createCliOpenShellSandboxObserver({ + capture: () => captured(null, "", "credential-bearing detail", timeout), + }); + + await expect( + observer.listSandboxes({ target: selectedOpenShellGateway(), timeoutMs: 25 }), + ).resolves.toEqual({ + ok: false, + error: { kind: "timeout", message: "OpenShell sandbox observation timed out." }, + }); + }); +}); diff --git a/src/lib/adapters/openshell/sandbox-observer-cli.ts b/src/lib/adapters/openshell/sandbox-observer-cli.ts new file mode 100644 index 00000000000..c264083b066 --- /dev/null +++ b/src/lib/adapters/openshell/sandbox-observer-cli.ts @@ -0,0 +1,265 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { + type ListOpenShellSandboxesRequest, + type LookupOpenShellSandboxRequest, + type OpenShellGatewayTarget, + type OpenShellSandboxError, + type OpenShellSandboxInventory, + type OpenShellSandboxLookup, + type OpenShellSandboxObservation, + type OpenShellSandboxObserver, + type OpenShellSandboxResult, +} from "./sandbox-observer"; +import { OPENSHELL_PROBE_TIMEOUT_MS } from "./timeouts"; + +const ANSI_RE = /\x1b\[[0-9;]*m/gu; + +const READY_PHASES = new Set(["Ready", "Running"]); +const TERMINAL_PHASES = new Set([ + "CrashLoopBackOff", + "Error", + "Evicted", + "Failed", + "ImagePullBackOff", + "Unknown", +]); +const KNOWN_PHASES = new Set([ + ...READY_PHASES, + ...TERMINAL_PHASES, + "Creating", + "Deleting", + "NotReady", + "Pending", + "Provisioning", + "Terminating", +]); +const CANONICAL_PHASES = new Map( + [...KNOWN_PHASES].map((phase) => [phase.toLowerCase(), phase] as const), +); + +function isOpenShellSandboxSchemaMismatch(output: string): boolean { + return ( + /invalid wire type/iu.test(output) || /proto(?:buf)?(?: decode| schema| wire)/iu.test(output) + ); +} + +export type CapturedSandboxCommandResult = Readonly<{ + status: number | null; + output: string; + stdout?: string; + stderr?: string; + error?: Error; +}>; + +export type CaptureSandboxCommand = ( + args: string[], + options: { + ignoreError: true; + includeStderr: true; + includeStreams: true; + timeout: number; + }, +) => CapturedSandboxCommandResult | Promise; + +export type CliOpenShellSandboxObserverDeps = Readonly<{ + capture: CaptureSandboxCommand; + defaultTimeoutMs?: number; +}>; + +export type CliOpenShellSandboxLookupResult = Readonly<{ + result: OpenShellSandboxResult; + displayOutput: string; +}>; + +export type CliOpenShellSandboxLookup = ( + request: LookupOpenShellSandboxRequest, +) => Promise; + +function readinessForPhase(phase: string | null): OpenShellSandboxObservation["readiness"] { + if (phase && READY_PHASES.has(phase)) return "ready"; + if (phase && TERMINAL_PHASES.has(phase)) return "terminal"; + return "not_ready"; +} + +export function stripOpenShellCliAnsi(value = ""): string { + return String(value).replace(ANSI_RE, ""); +} + +function observation(name: string, phase: string | null): OpenShellSandboxObservation { + return { name, phase, readiness: readinessForPhase(phase) }; +} + +function isNonSandboxRow(line: string, firstColumn: string): boolean { + return ( + firstColumn === "NAME" || + line === "No sandboxes found" || + line === "No sandboxes found." || + /^Error:/iu.test(line) || + isOpenShellSandboxSchemaMismatch(line) + ); +} + +export function parseCliOpenShellSandboxInventory(output: string): OpenShellSandboxInventory { + const sandboxes: OpenShellSandboxObservation[] = []; + for (const rawLine of stripOpenShellCliAnsi(output).split(/\r?\n/u)) { + const line = rawLine.trim(); + if (!line) continue; + const columns = line.split(/\s+/u); + const name = columns[0]; + if (!name || isNonSandboxRow(line, name)) continue; + let phase: string | null = null; + for (const column of columns.slice(1)) { + phase = CANONICAL_PHASES.get(column.toLowerCase()) ?? phase; + } + sandboxes.push(observation(name, phase)); + } + return { sandboxes }; +} + +function parseCliOpenShellSandboxPhase(output: string): string | null { + const match = stripOpenShellCliAnsi(output).match(/^\s*Phase:\s+(\S+)/mu); + const phase = match?.[1] ?? null; + return phase ? (CANONICAL_PHASES.get(phase.toLowerCase()) ?? phase) : null; +} + +function targetArgs( + command: "get" | "list", + target: OpenShellGatewayTarget, + sandboxName?: string, +): string[] { + const args = ["sandbox", command]; + if (target.kind === "named") args.push("-g", target.gatewayName); + if (sandboxName) args.push(sandboxName); + return args; +} + +function commandOutput(result: CapturedSandboxCommandResult): string { + return `${result.stderr ?? ""}\n${result.stdout ?? result.output ?? ""}`.trim(); +} + +function successfulCommandOutput(result: CapturedSandboxCommandResult): string { + return stripOpenShellCliAnsi(result.stdout ?? result.output); +} + +function commandError(result: CapturedSandboxCommandResult): OpenShellSandboxError | null { + const output = stripOpenShellCliAnsi(commandOutput(result)); + const errorCode = (result.error as NodeJS.ErrnoException | undefined)?.code; + if (errorCode === "ETIMEDOUT") { + return { kind: "timeout", message: "OpenShell sandbox observation timed out." }; + } + if (isOpenShellSandboxSchemaMismatch(output)) { + return { + kind: "schema", + message: "The OpenShell CLI and gateway sandbox schemas do not match.", + }; + } + if ( + /\b(?:authentication failed|unauthorized|forbidden|permission denied|missing gateway auth token|device identity required|invalid token|expired token)\b/iu.test( + output, + ) + ) { + return { + kind: "authentication", + message: "OpenShell could not authenticate the sandbox observation.", + }; + } + if (/\bhandshake verification failed\b/iu.test(output)) { + return { + kind: "transport", + reason: "identity_mismatch", + message: "The selected OpenShell gateway identity does not match the recorded identity.", + }; + } + if ( + /\b(?:connection refused|client error \(connect\)|tcp connect error|transport error|connection reset|connection aborted|connection closed|no active gateway|no gateway configured)\b|status:\s*disconnected/iu.test( + output, + ) + ) { + return { + kind: "transport", + reason: "unreachable", + message: "OpenShell could not reach the selected gateway.", + }; + } + if (result.status !== 0) { + return { + kind: "command", + reason: result.status === 2 ? "invalid_request" : "failed", + message: "The OpenShell sandbox observation failed.", + }; + } + return null; +} + +function isMissingSandboxOutput(output: string): boolean { + return /\bNotFound\b|\bNot Found\b|sandbox not found|sandbox has no spec/iu.test( + stripOpenShellCliAnsi(output), + ); +} + +function success(value: T): OpenShellSandboxResult { + return { ok: true, value }; +} + +function failure(error: OpenShellSandboxError): OpenShellSandboxResult { + return { ok: false, error }; +} + +/** + * CLI-only compatibility lookup for the legacy status display. Presence and + * phase decisions must use `result`; `displayOutput` remains a CLI-only + * presentation compatibility path. + */ +export function createCliOpenShellSandboxLookup( + deps: Pick, +): CliOpenShellSandboxLookup { + return async (request) => { + const result = await deps.capture(targetArgs("get", request.target, request.sandboxName), { + ignoreError: true, + includeStderr: true, + includeStreams: true, + timeout: request.timeoutMs ?? deps.defaultTimeoutMs ?? OPENSHELL_PROBE_TIMEOUT_MS, + }); + const output = commandOutput(result); + const error = commandError(result); + if (error && error.kind !== "command") { + return { result: failure(error), displayOutput: "" }; + } + if (result.status !== 0 && isMissingSandboxOutput(output)) { + return { result: success({ state: "missing" }), displayOutput: "" }; + } + if (error) return { result: failure(error), displayOutput: "" }; + const displayOutput = successfulCommandOutput(result).trim(); + return { + result: success({ + state: "present", + sandbox: observation(request.sandboxName, parseCliOpenShellSandboxPhase(displayOutput)), + }), + displayOutput, + }; + }; +} + +export function createCliOpenShellSandboxObserver( + deps: CliOpenShellSandboxObserverDeps, +): OpenShellSandboxObserver { + const capture = deps.capture; + + const listSandboxes = async ( + request: ListOpenShellSandboxesRequest, + ): Promise> => { + const result = await capture(targetArgs("list", request.target), { + ignoreError: true, + includeStderr: true, + includeStreams: true, + timeout: request.timeoutMs ?? deps.defaultTimeoutMs ?? OPENSHELL_PROBE_TIMEOUT_MS, + }); + const error = commandError(result); + if (error) return failure(error); + return success(parseCliOpenShellSandboxInventory(successfulCommandOutput(result))); + }; + + return { listSandboxes }; +} diff --git a/src/lib/adapters/openshell/sandbox-observer.ts b/src/lib/adapters/openshell/sandbox-observer.ts new file mode 100644 index 00000000000..45102d10b3b --- /dev/null +++ b/src/lib/adapters/openshell/sandbox-observer.ts @@ -0,0 +1,74 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +export type OpenShellGatewayTarget = { kind: "named"; gatewayName: string } | { kind: "selected" }; + +export type OpenShellSandboxReadiness = "ready" | "not_ready" | "terminal"; + +export type OpenShellSandboxObservation = Readonly<{ + name: string; + phase: string | null; + readiness: OpenShellSandboxReadiness; +}>; + +export type OpenShellSandboxInventory = Readonly<{ + sandboxes: readonly OpenShellSandboxObservation[]; +}>; + +export type OpenShellSandboxLookup = + | Readonly<{ state: "present"; sandbox: OpenShellSandboxObservation }> + | Readonly<{ state: "missing" }>; + +export type OpenShellSandboxErrorKind = + | "authentication" + | "command" + | "schema" + | "timeout" + | "transport"; + +export type OpenShellSandboxTransportReason = "identity_mismatch" | "unreachable"; + +export type OpenShellSandboxError = + | Readonly<{ + kind: Exclude; + message: string; + }> + | Readonly<{ + kind: "transport"; + reason: OpenShellSandboxTransportReason; + message: string; + }> + | Readonly<{ + kind: "command"; + reason: "failed" | "invalid_request"; + message: string; + }>; + +export type OpenShellSandboxResult = + | Readonly<{ ok: true; value: T }> + | Readonly<{ ok: false; error: OpenShellSandboxError }>; + +export type ListOpenShellSandboxesRequest = Readonly<{ + target: OpenShellGatewayTarget; + timeoutMs?: number; +}>; + +export type LookupOpenShellSandboxRequest = ListOpenShellSandboxesRequest & + Readonly<{ + sandboxName: string; + }>; + +/** Transport-neutral sandbox observation capabilities used by NemoClaw. */ +export interface OpenShellSandboxObserver { + listSandboxes( + request: ListOpenShellSandboxesRequest, + ): Promise>; +} + +export function namedOpenShellGateway(gatewayName: string): OpenShellGatewayTarget { + return { kind: "named", gatewayName }; +} + +export function selectedOpenShellGateway(): OpenShellGatewayTarget { + return { kind: "selected" }; +} diff --git a/src/lib/openshell-sandbox-list.test.ts b/src/lib/openshell-sandbox-list.test.ts index d9fa697e0f8..74232075ea8 100644 --- a/src/lib/openshell-sandbox-list.test.ts +++ b/src/lib/openshell-sandbox-list.test.ts @@ -11,7 +11,6 @@ const mocks = vi.hoisted(() => ({ detectResultIssue: vi.fn(), printIssue: vi.fn(), recoverNamedGatewayRuntime: vi.fn(), - stripAnsi: vi.fn((value: string) => value), })); vi.mock("./adapters/openshell/gateway-drift", () => ({ @@ -19,9 +18,6 @@ vi.mock("./adapters/openshell/gateway-drift", () => ({ detectOpenShellStateRpcResultIssue: mocks.detectResultIssue, printOpenShellStateRpcIssue: mocks.printIssue, })); -vi.mock("./adapters/openshell/client", () => ({ - stripAnsi: mocks.stripAnsi, -})); vi.mock("./adapters/openshell/runtime", () => ({ captureOpenshell: mocks.captureOpenshell, })); @@ -32,13 +28,25 @@ vi.mock("./gateway-runtime-action", () => ({ import { captureNamedGatewaySandboxListReadOnly, captureSandboxListWithGatewayPreflightOrExit, + captureSandboxListWithGatewayRecovery, } from "./openshell-sandbox-list"; +import type { + OpenShellSandboxInventory, + OpenShellSandboxObserver, + OpenShellSandboxResult, +} from "./adapters/openshell/sandbox-observer"; const context = { action: "checking sandbox state", command: "nemoclaw test-command", }; +function observerReturning( + result: OpenShellSandboxResult, +): OpenShellSandboxObserver { + return { listSandboxes: vi.fn().mockResolvedValue(result) }; +} + const imageDriftIssue: OpenShellStateRpcIssue = { kind: "image_drift", drift: { @@ -94,17 +102,22 @@ describe("sandbox list gateway preflight and recovery (#6237)", () => { it("returns the successful sandbox list without gateway recovery", async () => { const result = await captureSandboxListWithGatewayPreflightOrExit(context); - expect(result).toEqual({ status: 0, output: "alpha Ready" }); + expect(result).toEqual({ + sandboxes: [{ name: "alpha", phase: "Ready", readiness: "ready" }], + }); expect(mocks.captureOpenshell).toHaveBeenCalledOnce(); - expect(mocks.captureOpenshell).toHaveBeenCalledWith(["sandbox", "list"]); + expect(mocks.captureOpenshell).toHaveBeenCalledWith( + ["sandbox", "list"], + expect.objectContaining({ ignoreError: true, includeStreams: true, timeout: 15_000 }), + ); expect(mocks.recoverNamedGatewayRuntime).not.toHaveBeenCalled(); expect(exitSpy).not.toHaveBeenCalled(); }); - it("proves and recovers an explicit gateway instead of the current selection (#6114)", async () => { + it("recovers an unreachable explicit gateway after its scoped observation (#6114)", async () => { const options = { gatewayName: "nemoclaw-12345" }; mocks.captureOpenshell - .mockReturnValueOnce({ status: 1, output: "client error (Connect): Connection refused" }) + .mockReturnValueOnce({ status: 1, output: "Status: Disconnected" }) .mockReturnValueOnce({ status: 0, output: "alpha Ready" }); const result = await captureSandboxListWithGatewayPreflightOrExit(context, options); @@ -119,40 +132,28 @@ describe("sandbox list gateway preflight and recovery (#6237)", () => { "connected_other", ], }; - expect(mocks.recoverNamedGatewayRuntime).toHaveBeenNthCalledWith(1, expectedRecoveryOptions); - expect(mocks.recoverNamedGatewayRuntime).toHaveBeenNthCalledWith(2, expectedRecoveryOptions); - expect(mocks.detectResultIssue).toHaveBeenCalledWith(result, options); + expect(mocks.recoverNamedGatewayRuntime).toHaveBeenCalledOnce(); + expect(mocks.recoverNamedGatewayRuntime).toHaveBeenCalledWith(expectedRecoveryOptions); + expect(mocks.captureOpenshell).toHaveBeenCalledWith( + ["sandbox", "list", "-g", "nemoclaw-12345"], + expect.anything(), + ); expect(exitSpy).not.toHaveBeenCalled(); }); - it("fails closed when target selection fails while a sibling list is healthy (#6114)", async () => { + it("observes a healthy explicit gateway without mutating gateway state (#6114)", async () => { const options = { gatewayName: "nemoclaw-12345" }; - mocks.recoverNamedGatewayRuntime.mockResolvedValueOnce({ - recovered: false, - attempted: true, - before: { state: "connected_other", activeGateway: "nemoclaw" }, - after: { state: "connected_other", activeGateway: "nemoclaw" }, + mocks.captureOpenshell.mockReturnValue({ status: 0, output: "alpha Ready" }); + + await expect(captureSandboxListWithGatewayPreflightOrExit(context, options)).resolves.toEqual({ + sandboxes: [{ name: "alpha", phase: "Ready", readiness: "ready" }], }); - // This is the process-global sibling output that must never be accepted - // after the target gateway select/verification fails. - mocks.captureOpenshell.mockReturnValue({ status: 0, output: "default-box Ready" }); - const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); - await expect(captureSandboxListWithGatewayPreflightOrExit(context, options)).rejects.toThrow( - "process.exit(1)", + expect(mocks.captureOpenshell).toHaveBeenCalledWith( + ["sandbox", "list", "-g", "nemoclaw-12345"], + expect.anything(), ); - - expect(mocks.recoverNamedGatewayRuntime).toHaveBeenCalledWith({ - gatewayName: "nemoclaw-12345", - recoverableStates: [ - "missing_named", - "named_unhealthy", - "named_unreachable", - "connected_other", - ], - }); - expect(mocks.captureOpenshell).not.toHaveBeenCalled(); - expect(errorSpy.mock.calls.flat().join("\n")).toContain("recovery did not complete"); + expect(mocks.recoverNamedGatewayRuntime).not.toHaveBeenCalled(); }); it("recovers a disconnected gateway once and retries the sandbox list", async () => { @@ -162,7 +163,9 @@ describe("sandbox list gateway preflight and recovery (#6237)", () => { const result = await captureSandboxListWithGatewayPreflightOrExit(context); - expect(result).toEqual({ status: 0, output: "alpha Ready" }); + expect(result).toEqual({ + sandboxes: [{ name: "alpha", phase: "Ready", readiness: "ready" }], + }); expect(mocks.recoverNamedGatewayRuntime).toHaveBeenCalledWith({ recoverableStates: [ "missing_named", @@ -172,8 +175,16 @@ describe("sandbox list gateway preflight and recovery (#6237)", () => { ], }); expect(mocks.captureOpenshell).toHaveBeenCalledTimes(2); - expect(mocks.captureOpenshell).toHaveBeenNthCalledWith(1, ["sandbox", "list"]); - expect(mocks.captureOpenshell).toHaveBeenNthCalledWith(2, ["sandbox", "list"]); + expect(mocks.captureOpenshell).toHaveBeenNthCalledWith( + 1, + ["sandbox", "list"], + expect.anything(), + ); + expect(mocks.captureOpenshell).toHaveBeenNthCalledWith( + 2, + ["sandbox", "list"], + expect.anything(), + ); }); it("classifies protobuf mismatch from the retry before generic failure handling", async () => { @@ -193,13 +204,16 @@ describe("sandbox list gateway preflight and recovery (#6237)", () => { expect(mocks.captureOpenshell).toHaveBeenCalledTimes(2); expect(mocks.recoverNamedGatewayRuntime).toHaveBeenCalledOnce(); - expect(mocks.printIssue).toHaveBeenCalledWith(issue, context); + expect(mocks.printIssue).toHaveBeenCalledWith( + { kind: "protobuf_mismatch", drift: null, output: "" }, + context, + ); expect(errorSpy).not.toHaveBeenCalledWith( expect.stringContaining("Failed to query running sandboxes"), ); }); - it("preserves a generic failure status from the single retry", async () => { + it("preserves invalid-request exit behavior from the single retry", async () => { mocks.captureOpenshell .mockReturnValueOnce({ status: 1, output: "client error (Connect): Connection refused" }) .mockReturnValueOnce({ status: 2, output: "unknown option: --json" }); @@ -244,6 +258,29 @@ describe("sandbox list gateway preflight and recovery (#6237)", () => { expect(errorSpy.mock.calls.flat().join("\n")).toContain("Failed to query running sandboxes"); }); + it("does not mutate a named gateway after an identity mismatch (#9803)", async () => { + const result = { + ok: false, + error: { + kind: "transport", + reason: "identity_mismatch", + message: "The selected OpenShell gateway identity does not match the recorded identity.", + }, + } as const; + + await expect( + captureSandboxListWithGatewayRecovery({ + gatewayName: "nemoclaw-12345", + observer: observerReturning(result), + }), + ).resolves.toEqual({ + result, + recoveryAttempted: false, + recoverySucceeded: false, + }); + expect(mocks.recoverNamedGatewayRuntime).not.toHaveBeenCalled(); + }); + it("classifies protobuf mismatch before recovery or generic failure handling", async () => { const issue: OpenShellStateRpcIssue = { kind: "protobuf_mismatch", @@ -257,7 +294,10 @@ describe("sandbox list gateway preflight and recovery (#6237)", () => { "process.exit(1)", ); - expect(mocks.printIssue).toHaveBeenCalledWith(issue, context); + expect(mocks.printIssue).toHaveBeenCalledWith( + { kind: "protobuf_mismatch", drift: null, output: "" }, + context, + ); expect(mocks.recoverNamedGatewayRuntime).not.toHaveBeenCalled(); expect(errorSpy).not.toHaveBeenCalledWith( expect.stringContaining("Failed to query running sandboxes"), @@ -282,44 +322,97 @@ describe("read-only named-gateway sandbox list (#7279)", () => { vi.restoreAllMocks(); }); - it("lists the named gateway with -g and never recovers or selects", () => { - const result = captureNamedGatewaySandboxListReadOnly(context, "nemoclaw-18080"); + it("lists the named gateway with -g and never recovers or selects", async () => { + const result = await captureNamedGatewaySandboxListReadOnly(context, "nemoclaw-18080"); expect(mocks.captureOpenshell).toHaveBeenCalledWith( ["sandbox", "list", "-g", "nemoclaw-18080"], - { ignoreError: true }, + expect.objectContaining({ ignoreError: true, includeStreams: true, timeout: 15_000 }), ); expect(mocks.recoverNamedGatewayRuntime).not.toHaveBeenCalled(); - expect(result).toEqual({ status: 0, output: "alpha Ready" }); + expect(result).toEqual({ + sandboxes: [{ name: "alpha", phase: "Ready", readiness: "ready" }], + }); }); - it("stays non-fatal when the recorded gateway is down", () => { + it("stays non-fatal when the recorded gateway is down", async () => { mocks.captureOpenshell.mockReturnValue({ status: 1, output: "tcp connect error: Connection refused", }); - const result = captureNamedGatewaySandboxListReadOnly(context, "nemoclaw-18080"); + const result = await captureNamedGatewaySandboxListReadOnly(context, "nemoclaw-18080"); - expect(result.status).toBe(1); + expect(result).toEqual({ sandboxes: [] }); expect(exitSpy).not.toHaveBeenCalled(); expect(mocks.recoverNamedGatewayRuntime).not.toHaveBeenCalled(); }); - it("still exits on a state-RPC result drift issue", () => { - mocks.detectResultIssue.mockReturnValue(imageDriftIssue); + it.each([ + { + label: "authentication", + error: { + kind: "authentication", + message: "OpenShell could not authenticate the sandbox observation.", + }, + exitCode: 1, + }, + { + label: "identity mismatch", + error: { + kind: "transport", + reason: "identity_mismatch", + message: "The selected OpenShell gateway identity does not match the recorded identity.", + }, + exitCode: 1, + }, + { + label: "invalid command", + error: { + kind: "command", + reason: "invalid_request", + message: "OpenShell rejected the sandbox observation request.", + }, + exitCode: 2, + }, + ] as const)( + "fails closed on a $label observation failure (#9803)", + async ({ error, exitCode }) => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + + await expect( + captureNamedGatewaySandboxListReadOnly( + context, + "nemoclaw-18080", + observerReturning({ ok: false, error }), + ), + ).rejects.toThrow(`process.exit(${exitCode})`); + + expect(errorSpy.mock.calls.flat().join("\n")).toContain(error.message); + expect(mocks.recoverNamedGatewayRuntime).not.toHaveBeenCalled(); + }, + ); + + it("still exits on a state-RPC result drift issue", async () => { + mocks.captureOpenshell.mockReturnValue({ + status: 1, + output: "Sandbox.metadata: invalid wire type value: 6", + }); - expect(() => captureNamedGatewaySandboxListReadOnly(context, "nemoclaw-18080")).toThrow( + await expect(captureNamedGatewaySandboxListReadOnly(context, "nemoclaw-18080")).rejects.toThrow( "process.exit(1)", ); - expect(mocks.printIssue).toHaveBeenCalledWith(imageDriftIssue, context); + expect(mocks.printIssue).toHaveBeenCalledWith( + { kind: "protobuf_mismatch", drift: null, output: "" }, + context, + ); expect(mocks.recoverNamedGatewayRuntime).not.toHaveBeenCalled(); }); - it("exits before listing on a preflight drift issue", () => { + it("exits before listing on a preflight drift issue", async () => { mocks.detectPreflightIssue.mockReturnValue(hostProcessDriftIssue); - expect(() => captureNamedGatewaySandboxListReadOnly(context, "nemoclaw-18080")).toThrow( + await expect(captureNamedGatewaySandboxListReadOnly(context, "nemoclaw-18080")).rejects.toThrow( "process.exit(1)", ); expect(mocks.captureOpenshell).not.toHaveBeenCalled(); diff --git a/src/lib/openshell-sandbox-list.ts b/src/lib/openshell-sandbox-list.ts index 76754b586e7..11d891cbf1a 100644 --- a/src/lib/openshell-sandbox-list.ts +++ b/src/lib/openshell-sandbox-list.ts @@ -1,16 +1,22 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { stripAnsi } from "./adapters/openshell/client"; import { detectOpenShellStateRpcPreflightIssue, - detectOpenShellStateRpcResultIssue, printOpenShellStateRpcIssue, } from "./adapters/openshell/gateway-drift"; +import { createCliOpenShellSandboxObserver } from "./adapters/openshell/sandbox-observer-cli"; +import { + namedOpenShellGateway, + selectedOpenShellGateway, + type OpenShellSandboxInventory, + type OpenShellSandboxObserver, + type OpenShellSandboxResult, +} from "./adapters/openshell/sandbox-observer"; import { captureOpenshell } from "./adapters/openshell/runtime"; import { recoverNamedGatewayRuntime } from "./gateway-runtime-action"; -type SandboxListResult = ReturnType; +type SandboxListResult = OpenShellSandboxResult; export type SandboxListPreflightContext = { action: string; @@ -25,24 +31,21 @@ export type SandboxListRecoveryResult = { export type CaptureSandboxListWithGatewayRecoveryOptions = { gatewayName?: string; + observer?: OpenShellSandboxObserver; }; -export function isRecoverableSandboxListGatewayFailure( - result: SandboxListResult, - options: CaptureSandboxListWithGatewayRecoveryOptions = {}, -): boolean { - if (result.status === 0 || detectOpenShellStateRpcResultIssue(result, options)) { - return false; - } - const output = stripAnsi(String(result.output || "")); - return /Connection refused|client error \(Connect\)|tcp connect error|No active gateway|No gateway configured|Status:\s*Disconnected/i.test( - output, - ); +function isRecoverableObservedSandboxListGatewayFailure(result: SandboxListResult): boolean { + return !result.ok && result.error.kind === "transport" && result.error.reason === "unreachable"; } export async function captureSandboxListWithGatewayRecovery( options: CaptureSandboxListWithGatewayRecoveryOptions = {}, ): Promise { + const observer = + options.observer ?? + createCliOpenShellSandboxObserver({ + capture: captureOpenshell, + }); const recoveryOptions: Parameters[0] = { recoverableStates: ["missing_named", "named_unhealthy", "named_unreachable", "connected_other"], }; @@ -50,29 +53,15 @@ export async function captureSandboxListWithGatewayRecovery( recoveryOptions.gatewayName = options.gatewayName; } - // An explicit target must be proven healthy and active before an unscoped - // `sandbox list` can be trusted. OpenShell otherwise leaves a failed select - // on the current sibling gateway, whose successful list would be unsafe - // evidence for destructive recovery decisions (#6114). - let targetRecoveryAttempted = false; - if (options.gatewayName) { - const targetRecovery = await recoverNamedGatewayRuntime(recoveryOptions); - targetRecoveryAttempted = targetRecovery.attempted === true; - if (!targetRecovery.recovered) { - return { - result: { status: 1, output: "" }, - recoveryAttempted: targetRecovery.attempted === true, - recoverySucceeded: false, - }; - } - } - - const initial = captureOpenshell(["sandbox", "list"]); - if (!isRecoverableSandboxListGatewayFailure(initial, options)) { + const target = options.gatewayName + ? namedOpenShellGateway(options.gatewayName) + : selectedOpenShellGateway(); + const initial = await observer.listSandboxes({ target }); + if (!isRecoverableObservedSandboxListGatewayFailure(initial)) { return { result: initial, - recoveryAttempted: targetRecoveryAttempted, - recoverySucceeded: targetRecoveryAttempted, + recoveryAttempted: false, + recoverySucceeded: false, }; } @@ -82,7 +71,7 @@ export async function captureSandboxListWithGatewayRecovery( } return { - result: captureOpenshell(["sandbox", "list"]), + result: await observer.listSandboxes({ target }), recoveryAttempted: true, recoverySucceeded: true, }; @@ -91,24 +80,28 @@ export async function captureSandboxListWithGatewayRecovery( export async function captureSandboxListWithGatewayPreflightOrExit( context: SandboxListPreflightContext, options: CaptureSandboxListWithGatewayRecoveryOptions = {}, -): Promise { - const preflightIssue = detectOpenShellStateRpcPreflightIssue(options); +): Promise { + const preflightOptions = options.gatewayName ? { gatewayName: options.gatewayName } : {}; + const preflightIssue = detectOpenShellStateRpcPreflightIssue(preflightOptions); if (preflightIssue) { printOpenShellStateRpcIssue(preflightIssue, context); process.exit(1); } const recovery = await captureSandboxListWithGatewayRecovery(options); - const resultIssue = detectOpenShellStateRpcResultIssue(recovery.result, options); - if (resultIssue) { - printOpenShellStateRpcIssue(resultIssue, context); + if (!recovery.result.ok && recovery.result.error.kind === "schema") { + printOpenShellStateRpcIssue({ kind: "protobuf_mismatch", drift: null, output: "" }, context); process.exit(1); } - if (recovery.result.status !== 0) { + if (!recovery.result.ok) { printSandboxListFailureWithRecoveryContext(recovery); - process.exit(recovery.result.status || 1); + process.exit( + recovery.result.error.kind === "command" && recovery.result.error.reason === "invalid_request" + ? 2 + : 1, + ); } - return recovery.result; + return recovery.result.value; } /** @@ -120,10 +113,13 @@ export async function captureSandboxListWithGatewayPreflightOrExit( * but a down or unreachable gateway is non-fatal — its empty output makes the * sandbox report as unobserved instead of triggering a gateway start. */ -export function captureNamedGatewaySandboxListReadOnly( +export async function captureNamedGatewaySandboxListReadOnly( context: SandboxListPreflightContext, gatewayName: string, -): SandboxListResult { + observer: OpenShellSandboxObserver = createCliOpenShellSandboxObserver({ + capture: captureOpenshell, + }), +): Promise { const options: CaptureSandboxListWithGatewayRecoveryOptions = { gatewayName }; const preflightIssue = detectOpenShellStateRpcPreflightIssue(options); if (preflightIssue) { @@ -131,13 +127,20 @@ export function captureNamedGatewaySandboxListReadOnly( process.exit(1); } - const result = captureOpenshell(["sandbox", "list", "-g", gatewayName], { ignoreError: true }); - const resultIssue = detectOpenShellStateRpcResultIssue(result, options); - if (resultIssue) { - printOpenShellStateRpcIssue(resultIssue, context); + const result = await observer.listSandboxes({ target: namedOpenShellGateway(gatewayName) }); + if (result.ok) return result.value; + if (result.error.kind === "transport" && result.error.reason === "unreachable") { + return { sandboxes: [] }; + } + if (result.error.kind === "schema") { + printOpenShellStateRpcIssue({ kind: "protobuf_mismatch", drift: null, output: "" }, context); process.exit(1); } - return result; + console.error(" Failed to query running sandboxes from OpenShell."); + console.error(` ${result.error.message}`); + process.exit( + result.error.kind === "command" && result.error.reason === "invalid_request" ? 2 : 1, + ); } export function printSandboxListFailureWithRecoveryContext( diff --git a/src/lib/registry-recovery-action.test.ts b/src/lib/registry-recovery-action.test.ts index 6b0f344f9a7..85e3b7ea417 100644 --- a/src/lib/registry-recovery-action.test.ts +++ b/src/lib/registry-recovery-action.test.ts @@ -50,10 +50,6 @@ vi.mock("./state/onboard-session.js", () => ({ loadSession: vi.fn(), })); -vi.mock("./runtime-recovery.js", () => ({ - parseLiveSandboxEntries: vi.fn(() => [] as Array<{ name: string; phase: string | null }>), -})); - vi.mock("./runner.js", () => ({ validateName: (name: string) => { if (!/^[a-z]([a-z0-9-]*[a-z0-9])?$/.test(name)) { @@ -70,7 +66,6 @@ import { recoverNamedGatewayRuntime, } from "./gateway-runtime-action.js"; import { recoverRegistryEntries } from "./registry-recovery-action.js"; -import { parseLiveSandboxEntries } from "./runtime-recovery.js"; import { loadSession } from "./state/onboard-session.js"; function resetRegistryRecoveryDependencyMocks(): void { @@ -84,8 +79,7 @@ function resetRegistryRecoveryDependencyMocks(): void { .mockReturnValue({ state: "missing_named" } as never); vi.mocked(captureOpenshell) .mockReset() - .mockReturnValue({ output: "", status: 0 } as never); - vi.mocked(parseLiveSandboxEntries).mockReset().mockReturnValue([]); + .mockReturnValue({ output: "No sandboxes found.", status: 0 } as never); } describe("recoverRegistryEntries seed-time guard (#2753)", () => { @@ -346,7 +340,10 @@ describe("recoverRegistryEntries empty-registry live gateway recovery (#5714)", // is not an authoritative agent source; the real agent is reconciled by a // follow-up `nemoclaw status`. vi.mocked(getNamedGatewayLifecycleState).mockReturnValue({ state: "healthy_named" } as never); - vi.mocked(parseLiveSandboxEntries).mockReturnValue([{ name: "dcode-station", phase: "Ready" }]); + vi.mocked(captureOpenshell).mockReturnValue({ + output: "dcode-station Ready", + status: 0, + } as never); const result = await recoverRegistryEntries(); @@ -395,7 +392,7 @@ describe("recoverRegistryEntries empty-registry live gateway recovery (#5714)", }, } as never); vi.mocked(getNamedGatewayLifecycleState).mockReturnValue({ state: "healthy_named" } as never); - vi.mocked(parseLiveSandboxEntries).mockReturnValue([{ name: "live-x", phase: "Ready" }]); + vi.mocked(captureOpenshell).mockReturnValue({ output: "live-x Ready", status: 0 } as never); const result = await recoverRegistryEntries(); @@ -413,7 +410,10 @@ describe("recoverRegistryEntries empty-registry live gateway recovery (#5714)", // and permanently misclassify a Deep Agents/Hermes sandbox. Recovery is // display-only: the on-disk registry must stay empty. vi.mocked(getNamedGatewayLifecycleState).mockReturnValue({ state: "healthy_named" } as never); - vi.mocked(parseLiveSandboxEntries).mockReturnValue([{ name: "dcode-station", phase: "Ready" }]); + vi.mocked(captureOpenshell).mockReturnValue({ + output: "dcode-station Ready", + status: 0, + } as never); await recoverRegistryEntries(); @@ -430,7 +430,10 @@ describe("recoverRegistryEntries empty-registry live gateway recovery (#5714)", state: "connected_other", activeGateway: "nemoclaw-8092", } as never); - vi.mocked(parseLiveSandboxEntries).mockReturnValue([{ name: "dcode-station", phase: "Ready" }]); + vi.mocked(captureOpenshell).mockReturnValue({ + output: "dcode-station Ready", + status: 0, + } as never); const result = await recoverRegistryEntries(); @@ -446,7 +449,6 @@ describe("recoverRegistryEntries empty-registry live gateway recovery (#5714)", output: "transport error: connection reset", status: 1, } as never); - vi.mocked(parseLiveSandboxEntries).mockReturnValue([{ name: "transport", phase: null }]); const result = await recoverRegistryEntries(); @@ -462,7 +464,10 @@ describe("recoverRegistryEntries empty-registry live gateway recovery (#5714)", state: "connected_other", activeGateway: "some-other-project", } as never); - vi.mocked(parseLiveSandboxEntries).mockReturnValue([{ name: "foreign-sbox", phase: "Ready" }]); + vi.mocked(captureOpenshell).mockReturnValue({ + output: "foreign-sbox Ready", + status: 0, + } as never); const result = await recoverRegistryEntries(); @@ -475,7 +480,10 @@ describe("recoverRegistryEntries empty-registry live gateway recovery (#5714)", // effect of listing: it inspects the lifecycle directly and never calls // the mutating recoverNamedGatewayRuntime path. vi.mocked(getNamedGatewayLifecycleState).mockReturnValue({ state: "healthy_named" } as never); - vi.mocked(parseLiveSandboxEntries).mockReturnValue([{ name: "dcode-station", phase: "Ready" }]); + vi.mocked(captureOpenshell).mockReturnValue({ + output: "dcode-station Ready", + status: 0, + } as never); await recoverRegistryEntries(); diff --git a/src/lib/registry-recovery-action.ts b/src/lib/registry-recovery-action.ts index 513d531c8c0..fbae893846a 100644 --- a/src/lib/registry-recovery-action.ts +++ b/src/lib/registry-recovery-action.ts @@ -2,6 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 import { resolveOpenshell } from "./adapters/openshell/resolve"; +import { createCliOpenShellSandboxObserver } from "./adapters/openshell/sandbox-observer-cli"; +import { namedOpenShellGateway } from "./adapters/openshell/sandbox-observer"; import { captureOpenshell } from "./adapters/openshell/runtime"; import { OPENSHELL_PROBE_TIMEOUT_MS } from "./adapters/openshell/timeouts"; import { GATEWAY_PORT } from "./core/ports"; @@ -16,7 +18,6 @@ import { import { withGatewayRouteMutationLock } from "./inference/gateway-route-mutation-lock"; import { resolveGatewayName, resolveSandboxGatewayName } from "./onboard/gateway-binding"; import { validateName } from "./runner"; -import { parseLiveSandboxEntries } from "./runtime-recovery"; import * as onboardSession from "./state/onboard-session"; import type { SandboxEntry } from "./state/registry"; import * as registry from "./state/registry"; @@ -333,18 +334,20 @@ async function recoverRegistryFromLiveGateway( // Provisioning or absent from the live gateway (#7105). `-g` targets the // named gateway without selecting it, matching the readiness poll in // `connect` and `captureNamedGatewaySandboxListReadOnly`. - const liveList = captureOpenshell(["sandbox", "list", "-g", gatewayName], { - ignoreError: true, - timeout: OPENSHELL_PROBE_TIMEOUT_MS, + const liveList = await createCliOpenShellSandboxObserver({ + capture: captureOpenshell, + defaultTimeoutMs: OPENSHELL_PROBE_TIMEOUT_MS, + }).listSandboxes({ + target: namedOpenShellGateway(gatewayName), + timeoutMs: OPENSHELL_PROBE_TIMEOUT_MS, }); // Only trust the output of a clean `sandbox list`. On a non-zero/failed probe - // (timeout, transport error) OpenShell may print free-form text whose first - // token parseLiveSandboxEntries would otherwise mistake for a sandbox name. - if (liveList.status !== 0) { + // (timeout, transport error) the typed observer returns an error instead of + // treating command diagnostics as sandbox rows. + if (!liveList.ok) { return { recoveredFromGateway: 0, ephemeralSandboxes: [] }; } - const liveEntries = parseLiveSandboxEntries(liveList.output); - for (const { name, phase } of liveEntries) { + for (const { name, phase } of liveList.value.sandboxes) { const metadata = metadataByName.get(name) || undefined; if (readOnly) { // Unseeded recovery: surface the live sandbox for THIS `list` only and do diff --git a/src/lib/registry-recovery-seeded-paths.test.ts b/src/lib/registry-recovery-seeded-paths.test.ts index adc3cc4167d..c16ee2562dc 100644 --- a/src/lib/registry-recovery-seeded-paths.test.ts +++ b/src/lib/registry-recovery-seeded-paths.test.ts @@ -51,10 +51,6 @@ vi.mock("./state/onboard-session.js", () => ({ loadSession: vi.fn(), })); -vi.mock("./runtime-recovery.js", () => ({ - parseLiveSandboxEntries: vi.fn(), -})); - vi.mock("./runner.js", async () => { const actual = await vi.importActual("./runner.js"); return { ROOT: actual.ROOT, validateName: actual.validateName }; @@ -67,7 +63,6 @@ import { recoverNamedGatewayRuntime, } from "./gateway-runtime-action.js"; import { recoverRegistryEntries } from "./registry-recovery-action.js"; -import { parseLiveSandboxEntries } from "./runtime-recovery.js"; import { loadSession } from "./state/onboard-session.js"; const gammaEntry = (policies: string[]): SandboxEntry => ({ @@ -103,8 +98,7 @@ function resetSeededRecoveryMocks(): void { .mockReturnValue({ state: "missing_named" } as never); vi.mocked(captureOpenshell) .mockReset() - .mockReturnValue({ output: "live sandboxes", status: 0 } as never); - vi.mocked(parseLiveSandboxEntries).mockReset().mockReturnValue([]); + .mockReturnValue({ output: "No sandboxes found.", status: 0 } as never); } describe("recoverRegistryEntries seeded recovery paths", () => { @@ -114,10 +108,10 @@ describe("recoverRegistryEntries seeded recovery paths", () => { mockRegistryState.sandboxes.gamma = gammaEntry(["npm"]); mockRegistryState.defaultSandbox = "gamma"; vi.mocked(loadSession).mockReturnValue(completedSession("alpha", ["pypi"])); - vi.mocked(parseLiveSandboxEntries).mockReturnValue([ - { name: "alpha", phase: "Ready" }, - { name: "beta", phase: "Ready" }, - ]); + vi.mocked(captureOpenshell).mockReturnValue({ + output: "alpha Ready\nbeta Ready", + status: 0, + } as never); const result = await recoverRegistryEntries(); @@ -141,7 +135,7 @@ describe("recoverRegistryEntries seeded recovery paths", () => { }; mockRegistryState.defaultSandbox = "gamma"; vi.mocked(loadSession).mockReturnValue(completedSession("alpha", [])); - vi.mocked(parseLiveSandboxEntries).mockReturnValue([{ name: "alpha", phase: "Ready" }]); + vi.mocked(captureOpenshell).mockReturnValue({ output: "alpha Ready", status: 0 } as never); const result = await recoverRegistryEntries(); @@ -174,7 +168,7 @@ describe("recoverRegistryEntries seeded recovery paths", () => { sandbox: { status: "complete", startedAt: null, completedAt: null, error: null }, }, } as never); - vi.mocked(parseLiveSandboxEntries).mockReturnValue([{ name: "alpha", phase: "Ready" }]); + vi.mocked(captureOpenshell).mockReturnValue({ output: "alpha Ready", status: 0 } as never); await recoverRegistryEntries({ requestedSandboxName: "missing-sandbox" }); @@ -191,10 +185,10 @@ describe("recoverRegistryEntries seeded recovery paths", () => { mockRegistryState.sandboxes.gamma = gammaEntry([]); mockRegistryState.defaultSandbox = "gamma"; vi.mocked(loadSession).mockReturnValue(completedSession("Alpha", [])); - vi.mocked(parseLiveSandboxEntries).mockReturnValue([ - { name: "alpha", phase: "Ready" }, - { name: "Bad_Name", phase: "Ready" }, - ]); + vi.mocked(captureOpenshell).mockReturnValue({ + output: "alpha Ready\nBad_Name Ready", + status: 0, + } as never); const result = await recoverRegistryEntries(); @@ -220,7 +214,10 @@ describe("recoverRegistryEntries seeded recovery paths", () => { }, } as never); vi.mocked(getNamedGatewayLifecycleState).mockReturnValue({ state: "healthy_named" } as never); - vi.mocked(parseLiveSandboxEntries).mockReturnValue([{ name: "dcode-station", phase: "Ready" }]); + vi.mocked(captureOpenshell).mockReturnValue({ + output: "dcode-station Ready", + status: 0, + } as never); const result = await recoverRegistryEntries(); @@ -240,7 +237,7 @@ describe("recoverRegistryEntries seeded recovery paths", () => { }); it("persists a requested live sandbox and makes it the default", async () => { - vi.mocked(parseLiveSandboxEntries).mockReturnValue([{ name: "alpha", phase: "Ready" }]); + vi.mocked(captureOpenshell).mockReturnValue({ output: "alpha Ready", status: 0 } as never); const result = await recoverRegistryEntries({ requestedSandboxName: "alpha" }); @@ -253,7 +250,7 @@ describe("recoverRegistryEntries seeded recovery paths", () => { }); it("keeps a missing requested sandbox absent while recovering other live entries", async () => { - vi.mocked(parseLiveSandboxEntries).mockReturnValue([{ name: "alpha", phase: "Ready" }]); + vi.mocked(captureOpenshell).mockReturnValue({ output: "alpha Ready", status: 0 } as never); const result = await recoverRegistryEntries({ requestedSandboxName: "beta" }); @@ -267,9 +264,10 @@ describe("recoverRegistryEntries seeded recovery paths", () => { it("blocks route mutation after seeded recovery persists a live row without route metadata (#6315)", async () => { mockRegistryState.sandboxes.gamma = gammaEntry([]); mockRegistryState.defaultSandbox = "gamma"; - vi.mocked(parseLiveSandboxEntries).mockReturnValue([ - { name: "recovered-live", phase: "Ready" }, - ]); + vi.mocked(captureOpenshell).mockReturnValue({ + output: "recovered-live Ready", + status: 0, + } as never); await recoverRegistryEntries({ requestedSandboxName: "missing-sandbox" }); expect(mockRegistryState.sandboxes["recovered-live"]).toMatchObject({ @@ -313,13 +311,10 @@ describe("recoverRegistryEntries seeded recovery paths", () => { vi.mocked(captureOpenshell).mockImplementation( (args: string[]) => ({ - output: args.includes("-g") ? "scoped-list" : "host-wide-list", + output: args.includes("-g") ? "No sandboxes found." : "hermes-station Ready", status: 0, }) as never, ); - vi.mocked(parseLiveSandboxEntries).mockImplementation((output?: string) => - output === "scoped-list" ? [] : [{ name: "hermes-station", phase: "Ready" }], - ); mockRegistryState.sandboxes.gamma = gammaEntry([]); mockRegistryState.defaultSandbox = "gamma"; @@ -341,15 +336,10 @@ describe("recoverRegistryEntries seeded recovery paths", () => { vi.mocked(captureOpenshell).mockImplementation( (args: string[]) => ({ - output: args.includes("-g") ? "scoped-list" : "host-wide-list", + output: args.includes("-g") ? "target-sandbox Ready" : "sibling-sandbox Ready", status: 0, }) as never, ); - vi.mocked(parseLiveSandboxEntries).mockImplementation((output?: string) => - output === "scoped-list" - ? [{ name: "target-sandbox", phase: "Ready" }] - : [{ name: "sibling-sandbox", phase: "Ready" }], - ); const result = await recoverRegistryEntries(); diff --git a/src/lib/runtime-recovery.ts b/src/lib/runtime-recovery.ts index 0361fc7a24c..fd4b6479512 100644 --- a/src/lib/runtime-recovery.ts +++ b/src/lib/runtime-recovery.ts @@ -6,72 +6,22 @@ * output and determine recovery strategy. */ -const ANSI_RE = /\x1b\[[0-9;]*m/g; -const SANDBOX_PHASES = new Set(["Ready", "Running", "NotReady", "Provisioning", "Error"]); -// Broader phase vocabulary for surfacing the live PHASE on #5714 recovered list -// rows. Unions the lifecycle phases above with the terminal/failure phases used -// elsewhere (see state/gateway.ts TERMINAL_SANDBOX_PHASES) plus common -// transient phases, so a recovered row reports the real phase (e.g. Failed, -// CrashLoopBackOff, Creating) instead of "unknown". Kept separate from -// SANDBOX_PHASES so parseReadySandboxNames' Ready/Running gate is unchanged. -const LIVE_SANDBOX_DISPLAY_PHASES = new Set([ - "Ready", - "Running", - "NotReady", - "Provisioning", - "Creating", - "Pending", - "Deleting", - "Terminating", - "Error", - "Failed", - "CrashLoopBackOff", - "ImagePullBackOff", - "Evicted", - "Unknown", -]); - -/** Strip ANSI color escape sequences from CLI output. */ -function stripAnsi(text: string | null | undefined): string { - return String(text || "").replace(ANSI_RE, ""); -} +import { + parseCliOpenShellSandboxInventory, + stripOpenShellCliAnsi, +} from "./adapters/openshell/sandbox-observer-cli"; /** Detect an OpenShell protobuf/wire schema-mismatch error in command output. */ export function isOpenShellProtobufSchemaMismatch(output = ""): boolean { - const clean = stripAnsi(output); + const clean = stripOpenShellCliAnsi(output); return /invalid wire type/i.test(clean) || /proto(?:buf)?(?: decode| schema| wire)/i.test(clean); } -/** Whether a `sandbox list` line is a header/empty/error row rather than a sandbox. */ -function isNonSandboxRow(line: string, firstCol: string): boolean { - if (firstCol === "NAME") return true; - if (line === "No sandboxes found" || line === "No sandboxes found.") return true; - if (/^Error:/i.test(line)) return true; - if (isOpenShellProtobufSchemaMismatch(line)) return true; - return false; -} - -/** Extract the phase token from a `sandbox list` row's columns (compact or trailing). */ -function parseSandboxListPhase(cols: string[]): string | null { - const compactPhase = cols[1]; - if (cols.length <= 3 && SANDBOX_PHASES.has(compactPhase)) return compactPhase; - const trailingPhase = cols.at(-1); - return trailingPhase && SANDBOX_PHASES.has(trailingPhase) ? trailingPhase : null; -} - /** Parse the set of all live sandbox names from `openshell sandbox list` output. */ export function parseLiveSandboxNames(listOutput = ""): Set { - const clean = stripAnsi(listOutput); - const names = new Set(); - for (const rawLine of clean.split("\n")) { - const line = rawLine.trim(); - if (!line) continue; - const cols = line.split(/\s+/); - if (!cols[0]) continue; - if (isNonSandboxRow(line, cols[0])) continue; - names.add(cols[0]); - } - return names; + return new Set( + parseCliOpenShellSandboxInventory(listOutput).sandboxes.map((sandbox) => sandbox.name), + ); } export interface LiveSandboxEntry { @@ -86,39 +36,17 @@ export interface LiveSandboxEntry { * output for any other (e.g. agent) metadata it does not contain. */ export function parseLiveSandboxEntries(listOutput = ""): LiveSandboxEntry[] { - const clean = stripAnsi(listOutput); - const entries: LiveSandboxEntry[] = []; - for (const rawLine of clean.split("\n")) { - const line = rawLine.trim(); - if (!line) continue; - const cols = line.split(/\s+/); - if (!cols[0]) continue; - if (isNonSandboxRow(line, cols[0])) continue; - // Scan every column after the name for a known phase token so we read the - // phase regardless of column layout — trailing (`NAME CREATED PHASE`), - // compact (`NAME PHASE`), or with an age suffix (`NAME PHASE 2m ago`). The - // first column is the name and is never a phase. Uses the broader display - // vocabulary so terminal/transient phases (Failed, Creating, …) are kept. - const phase = cols.slice(1).find((col) => LIVE_SANDBOX_DISPLAY_PHASES.has(col)) ?? null; - entries.push({ name: cols[0], phase }); - } - return entries; + return parseCliOpenShellSandboxInventory(listOutput).sandboxes.map(({ name, phase }) => ({ + name, + phase, + })); } /** Parse the set of sandbox names in a Ready/Running phase from `sandbox list` output. */ export function parseReadySandboxNames(listOutput = ""): Set { - const clean = stripAnsi(listOutput); - const names = new Set(); - for (const rawLine of clean.split("\n")) { - const line = rawLine.trim(); - if (!line) continue; - const cols = line.split(/\s+/); - if (!cols[0]) continue; - if (isNonSandboxRow(line, cols[0])) continue; - const phase = parseSandboxListPhase(cols); - const isReadyOrRunning = phase === "Ready" || phase === "Running"; - if (phase === "NotReady" || !isReadyOrRunning) continue; - names.add(cols[0]); - } - return names; + return new Set( + parseCliOpenShellSandboxInventory(listOutput) + .sandboxes.filter((sandbox) => sandbox.readiness === "ready") + .map((sandbox) => sandbox.name), + ); } diff --git a/test/cli/connect-readiness.test.ts b/test/cli/connect-readiness.test.ts index e977a475c34..487becb062a 100644 --- a/test/cli/connect-readiness.test.ts +++ b/test/cli/connect-readiness.test.ts @@ -187,6 +187,7 @@ describe("CLI connect readiness", () => { expect(r.out).not.toContain("Timed out after 1s"); const calls = fs.readFileSync(markerFile, "utf8").trim().split("\n").filter(Boolean); expect(calls).toContain("status"); + expect(calls).toContain("sandbox list -g nemoclaw"); expect(calls).not.toContain("should-not-connect"); }, testTimeout(15_000), diff --git a/test/cli/doctor-gateway-token.test.ts b/test/cli/doctor-gateway-token.test.ts index d9ccfebba19..77697cfe759 100644 --- a/test/cli/doctor-gateway-token.test.ts +++ b/test/cli/doctor-gateway-token.test.ts @@ -112,7 +112,7 @@ describe("CLI dispatch", () => { 'case "$*" in', ' "status") printf "Server Status\\n\\n Gateway: nemoclaw\\n Status: Connected\\n"; exit 0 ;;', ' "gateway info -g nemoclaw") printf "Gateway: nemoclaw\\n"; exit 0 ;;', - ' "sandbox list") printf "NAME STATUS\\nalpha Creating\\n"; exit 0 ;;', + ' "sandbox list -g nemoclaw") printf "NAME STATUS\\nalpha Creating\\n"; exit 0 ;;', ' "inference get") printf "Provider: nvidia-prod\\nModel: test-model\\n"; exit 0 ;;', "esac", ]); @@ -141,7 +141,7 @@ describe("CLI dispatch", () => { 'case "$*" in', ' "status") printf "Server Status\\n\\n Gateway: nemoclaw\\n Status: Connected\\n"; exit 0 ;;', ' "gateway info -g nemoclaw") printf "Gateway: nemoclaw\\n"; exit 0 ;;', - ' "sandbox list") printf "NAME STATUS\\nalpha Ready\\n"; exit 0 ;;', + ' "sandbox list -g nemoclaw") printf "NAME STATUS\\nalpha Ready\\n"; exit 0 ;;', ' "inference get") printf "Provider: nvidia-prod\\nModel: test-model\\n"; exit 0 ;;', "esac", ]); @@ -205,7 +205,7 @@ describe("CLI dispatch", () => { 'case "$*" in', ' "status") printf "Server Status\\n\\n Gateway: nemoclaw-8090\\n Status: Connected\\n"; exit 0 ;;', ' "gateway info -g nemoclaw-8090") printf "Gateway: nemoclaw-8090\\n"; exit 0 ;;', - ' "sandbox list") printf "NAME STATUS\\nalpha Ready\\n"; exit 0 ;;', + ' "sandbox list -g nemoclaw-8090") printf "NAME STATUS\\nalpha Ready\\n"; exit 0 ;;', ' "inference get") printf "Provider: nvidia-prod\\nModel: test-model\\n"; exit 0 ;;', "esac", ]); @@ -242,7 +242,7 @@ describe("CLI dispatch", () => { 'case "$*" in', ' "status") printf "Server Status\\n\\n Gateway: nemoclaw\\n Status: Connected\\n"; exit 0 ;;', ' "gateway info -g nemoclaw") printf "Gateway: nemoclaw\\n"; exit 0 ;;', - ' "sandbox list") printf "NAME STATUS\\nalpha Ready\\n"; exit 0 ;;', + ' "sandbox list -g nemoclaw") printf "NAME STATUS\\nalpha Ready\\n"; exit 0 ;;', ' "inference get") printf "Provider: nvidia-prod\\nModel: test-model\\n"; exit 0 ;;', "esac", ]); @@ -272,7 +272,7 @@ describe("CLI dispatch", () => { 'case "$*" in', ' "status") printf "Server Status\\n\\n Gateway: nemoclaw\\n Status: Connected\\n"; exit 0 ;;', ' "gateway info -g nemoclaw") printf "Gateway: nemoclaw\\n"; exit 0 ;;', - ' "sandbox list") printf "NAME STATUS\\nalpha Ready\\n"; exit 0 ;;', + ' "sandbox list -g nemoclaw") printf "NAME STATUS\\nalpha Ready\\n"; exit 0 ;;', ' "inference get") printf "Provider: nvidia-prod\\nModel: test-model\\n"; exit 0 ;;', "esac", ]); @@ -325,7 +325,7 @@ describe("CLI dispatch", () => { ' "gateway info -g nemoclaw") printf "Gateway: nemoclaw\\n"; exit 0 ;;', ' "gateway select nemoclaw") exit 1 ;;', ' "gateway start --name nemoclaw --port 8080") exit 1 ;;', - ' "sandbox list") echo "should not query sandbox list" >> "$marker_file"; exit 0 ;;', + ' "sandbox list -g nemoclaw") echo "should not query sandbox list" >> "$marker_file"; exit 0 ;;', "esac", ], ); @@ -365,7 +365,7 @@ describe("CLI dispatch", () => { 'case "$*" in', ' "status") printf "Server Status\\n\\n Gateway: nemoclaw\\n Status: Connected\\n"; exit 0 ;;', ' "gateway info -g nemoclaw") printf "Gateway: nemoclaw\\n"; exit 0 ;;', - ' "sandbox list") printf "NAME STATUS\\nalpha Ready\\n"; exit 0 ;;', + ' "sandbox list -g nemoclaw") printf "NAME STATUS\\nalpha Ready\\n"; exit 0 ;;', ' "inference get") printf "Provider: nvidia-prod\\nModel: test-model\\n"; exit 0 ;;', "esac", ], @@ -407,7 +407,7 @@ describe("CLI dispatch", () => { 'case "$*" in', ' "status") printf "Server Status\\n\\n Gateway: nemoclaw\\n Status: Connected\\n"; exit 0 ;;', ' "gateway info -g nemoclaw") printf "Gateway: nemoclaw\\n"; exit 0 ;;', - ' "sandbox list") printf "NAME STATUS\\nalpha Ready\\n"; exit 0 ;;', + ' "sandbox list -g nemoclaw") printf "NAME STATUS\\nalpha Ready\\n"; exit 0 ;;', ' "inference get") printf "Provider: nvidia-prod\\nModel: test-model\\n"; exit 0 ;;', "esac", ]); @@ -438,7 +438,7 @@ describe("CLI dispatch", () => { ' "gateway info -g nemoclaw") printf "Gateway: nemoclaw\\n"; exit 0 ;;', ' "gateway select nemoclaw") exit 1 ;;', ' "gateway start --name nemoclaw --port 8080") exit 1 ;;', - ' "sandbox list") echo "queried wrong gateway sandbox list" >> "$marker_file"; exit 0 ;;', + ' "sandbox list -g nemoclaw") echo "queried wrong gateway sandbox list" >> "$marker_file"; exit 0 ;;', "esac", ]); @@ -464,7 +464,7 @@ describe("CLI dispatch", () => { 'case "$*" in', ' "status") printf "Server Status\\n\\n Gateway: nemoclaw\\n Status: Connected\\n"; exit 0 ;;', ' "gateway info -g nemoclaw") printf "Gateway: nemoclaw\\n"; exit 0 ;;', - ` "sandbox list") printf "NAME STATUS\\n${sandboxName} Ready\\n"; exit 0 ;;`, + ` "sandbox list -g nemoclaw") printf "NAME STATUS\\n${sandboxName} Ready\\n"; exit 0 ;;`, ' "inference get") printf "Provider: nvidia-prod\\nModel: test-model\\n"; exit 0 ;;', "esac", ], @@ -506,7 +506,7 @@ describe("CLI dispatch", () => { 'case "$*" in', ' "status") printf "Server Status\\n\\n Gateway: nemoclaw\\n Status: Connected\\n"; exit 0 ;;', ' "gateway info -g nemoclaw") printf "Gateway: nemoclaw\\n"; exit 0 ;;', - ` "sandbox list") printf "NAME STATUS\\n${sandboxName} Ready\\n"; exit 0 ;;`, + ` "sandbox list -g nemoclaw") printf "NAME STATUS\\n${sandboxName} Ready\\n"; exit 0 ;;`, ' "inference get") printf "Provider: nvidia-prod\\nModel: test-model\\n"; exit 0 ;;', "esac", ], diff --git a/test/helpers/rebuild-flow-dcode-harness.ts b/test/helpers/rebuild-flow-dcode-harness.ts index c1b2f8fab2f..c1097ac7e13 100644 --- a/test/helpers/rebuild-flow-dcode-harness.ts +++ b/test/helpers/rebuild-flow-dcode-harness.ts @@ -3,6 +3,7 @@ import { type MockInstance, vi } from "vitest"; import type { GatewayRestartResult } from "../../src/lib/actions/sandbox/gateway-restart"; +import type { OpenShellSandboxInventory } from "../../src/lib/adapters/openshell/sandbox-observer"; import { makePreparedRecoveryManifest } from "../../src/lib/actions/sandbox/rebuild-flow-test-fixtures"; import { agentDefs, @@ -94,7 +95,7 @@ export type RebuildFlowOverrides = { sandboxEntry?: Record; sandboxEntryReads?: Array | null>; sessionSandboxName?: string; - sandboxListOutput?: string; + sandboxInventory?: OpenShellSandboxInventory; backupPolicyPresets?: string[]; gatewayPresets?: string[]; verificationUnavailableAfterPresetRemoval?: boolean; @@ -239,7 +240,14 @@ export function createRebuildFlowHarness(overrides: RebuildFlowOverrides = {}): resolveGatewayAuthority, ); vi.spyOn(sandboxList, "captureSandboxListWithGatewayRecovery").mockResolvedValue({ - result: { status: 0, output: overrides.sandboxListOutput ?? "alpha Ready" }, + result: { + ok: true, + value: overrides.sandboxInventory ?? { + sandboxes: [{ name: "alpha", phase: "Ready", readiness: "ready" }], + }, + }, + recoveryAttempted: false, + recoverySucceeded: false, }); vi.spyOn(resolve, "resolveOpenshell").mockReturnValue(null); vi.spyOn(dockerImage, "dockerBuild").mockReturnValue({ status: 0 }); diff --git a/test/helpers/rebuild-flow-generic-harness.ts b/test/helpers/rebuild-flow-generic-harness.ts index 4dc7812dac9..b68923d638a 100644 --- a/test/helpers/rebuild-flow-generic-harness.ts +++ b/test/helpers/rebuild-flow-generic-harness.ts @@ -105,9 +105,17 @@ export function createRebuildFlowHarness(overrides: RebuildFlowOverrides = {}): ); vi.spyOn(sandboxList, "captureSandboxListWithGatewayRecovery").mockResolvedValue({ result: { - status: 0, - output: overrides.sandboxListOutput ?? (overrides.staleRecovery ? "" : "alpha Ready"), + ok: true, + value: + overrides.sandboxInventory ?? + (overrides.staleRecovery + ? { sandboxes: [] } + : { + sandboxes: [{ name: "alpha", phase: "Ready", readiness: "ready" }], + }), }, + recoveryAttempted: false, + recoverySucceeded: false, }); vi.spyOn(gatewayState, "getReconciledSandboxGatewayState").mockResolvedValue( overrides.reconciledSandboxGatewayState ?? { diff --git a/test/helpers/rebuild-flow-test-support.ts b/test/helpers/rebuild-flow-test-support.ts index 57b2dd7e9f5..cab63d41c99 100644 --- a/test/helpers/rebuild-flow-test-support.ts +++ b/test/helpers/rebuild-flow-test-support.ts @@ -4,6 +4,7 @@ import { type MockInstance, vi } from "vitest"; import type { GatewayRestartResult } from "../../src/lib/actions/sandbox/gateway-restart"; import type { SandboxGatewayState } from "../../src/lib/actions/sandbox/gateway-state"; +import type { OpenShellSandboxInventory } from "../../src/lib/adapters/openshell/sandbox-observer"; import type { finalizePreparedRebuildImageMessagingPlan, RebuildImagePreflightResult, @@ -80,7 +81,7 @@ export type RebuildFlowOverrides = { sandboxEntry?: Record; sandboxBaseImageLabelsOutput?: string; sessionSandboxName?: string; - sandboxListOutput?: string; + sandboxInventory?: OpenShellSandboxInventory; defaultSandbox?: string | null; preDeleteSandboxEntry?: Record; preDeleteDefaultSandbox?: string | null; diff --git a/test/process-recovery/rebuild-stale-recovery.test.ts b/test/process-recovery/rebuild-stale-recovery.test.ts index 7d0a422b526..8b775f39d07 100644 --- a/test/process-recovery/rebuild-stale-recovery.test.ts +++ b/test/process-recovery/rebuild-stale-recovery.test.ts @@ -95,7 +95,7 @@ describe("stale sandbox rebuild recovery (#4497)", () => { // refuse to recreate from scratch, or it would destroy live workspace // state in multi-gateway setups (#4497 / #4645). const harness = createRebuildFlowHarness({ - sandboxListOutput: "", + sandboxInventory: { sandboxes: [] }, reconciledSandboxGatewayState: { state: "wrong_gateway_active", output: "Gateway: other-gw", @@ -129,7 +129,7 @@ describe("stale sandbox rebuild recovery (#4497)", () => { // preserve the registry entry. const harness = createRebuildFlowHarness({ sandboxEntry: { gatewayName: "nemoclaw-9000", gatewayPort: 9000 }, - sandboxListOutput: "", + sandboxInventory: { sandboxes: [] }, reconciledSandboxGatewayState: { state: "wrong_gateway_active", output: "Gateway: nemoclaw",