From de124f4319a5456298d7d4c098e2b6971fb66973 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 29 Apr 2026 10:59:15 -0400 Subject: [PATCH 1/4] refactor(arch): add named timeout constants for openshell execution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce src/lib/openshell-timeouts.ts with categorized timeout constants for all openshell child-process calls: - OPENSHELL_PROBE_TIMEOUT_MS (15s) — read-only queries - OPENSHELL_OPERATION_TIMEOUT_MS (30s) — mutating commands - OPENSHELL_HEAVY_TIMEOUT_MS (60s) — destructive/long-running ops - OPENSHELL_DOWNLOAD_TIMEOUT_MS (30s) — file transfers over SSH Uses the same OPENSHELL_PROBE_TIMEOUT_MS name introduced locally in PR #2454 so that PR can import from the shared module on rebase. This is Phase 1 of #2562 — pure constants, no behavioral change. Refs: #2562 --- src/lib/openshell-timeouts.test.ts | 40 ++++++++++++++++++++++++++++++ src/lib/openshell-timeouts.ts | 28 +++++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 src/lib/openshell-timeouts.test.ts create mode 100644 src/lib/openshell-timeouts.ts diff --git a/src/lib/openshell-timeouts.test.ts b/src/lib/openshell-timeouts.test.ts new file mode 100644 index 00000000000..580ccf97dc6 --- /dev/null +++ b/src/lib/openshell-timeouts.test.ts @@ -0,0 +1,40 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { + OPENSHELL_DOWNLOAD_TIMEOUT_MS, + OPENSHELL_HEAVY_TIMEOUT_MS, + OPENSHELL_OPERATION_TIMEOUT_MS, + OPENSHELL_PROBE_TIMEOUT_MS, +} from "./openshell-timeouts"; + +describe("openshell-timeouts", () => { + it("exports positive integer constants", () => { + const constants = [ + OPENSHELL_PROBE_TIMEOUT_MS, + OPENSHELL_OPERATION_TIMEOUT_MS, + OPENSHELL_HEAVY_TIMEOUT_MS, + OPENSHELL_DOWNLOAD_TIMEOUT_MS, + ]; + + for (const value of constants) { + expect(value).toBeTypeOf("number"); + expect(value).toBeGreaterThan(0); + expect(Number.isInteger(value)).toBe(true); + } + }); + + it("maintains expected ordering: PROBE < OPERATION <= DOWNLOAD < HEAVY", () => { + expect(OPENSHELL_PROBE_TIMEOUT_MS).toBeLessThan(OPENSHELL_OPERATION_TIMEOUT_MS); + expect(OPENSHELL_OPERATION_TIMEOUT_MS).toBeLessThanOrEqual(OPENSHELL_DOWNLOAD_TIMEOUT_MS); + expect(OPENSHELL_DOWNLOAD_TIMEOUT_MS).toBeLessThanOrEqual(OPENSHELL_HEAVY_TIMEOUT_MS); + }); + + it("uses the same probe constant name as PR #2454 for forward compatibility", () => { + // PR #2454 introduces OPENSHELL_PROBE_TIMEOUT_MS = 15_000 locally. + // This ensures the shared module stays aligned so #2454 can import it after rebase. + expect(OPENSHELL_PROBE_TIMEOUT_MS).toBe(15_000); + }); +}); diff --git a/src/lib/openshell-timeouts.ts b/src/lib/openshell-timeouts.ts new file mode 100644 index 00000000000..5fc0f3ae4e7 --- /dev/null +++ b/src/lib/openshell-timeouts.ts @@ -0,0 +1,28 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Named timeout constants for openshell child-process execution. + * + * Every openshell CLI call should use one of these categories rather than + * raw millisecond literals. This ensures consistent behaviour across all + * call sites and makes it easy to tune timeouts from a single location. + * + * Categories: + * PROBE — read-only queries that should return instantly (list, status, info, ssh-config) + * OPERATION — mutating commands (provider CRUD, forward start/stop, gateway select) + * HEAVY — destructive or long-running (sandbox delete, gateway destroy, build) + * DOWNLOAD — file transfers over the sandbox SSH tunnel (config download) + */ + +/** Quick probe — sandbox list, status, gateway info, forward list, ssh-config */ +export const OPENSHELL_PROBE_TIMEOUT_MS = 15_000; + +/** Mutating operations — provider create/delete, gateway select, forward start/stop */ +export const OPENSHELL_OPERATION_TIMEOUT_MS = 30_000; + +/** Heavy operations — sandbox delete, gateway destroy, full build */ +export const OPENSHELL_HEAVY_TIMEOUT_MS = 60_000; + +/** Sandbox download (config file fetch over SSH) */ +export const OPENSHELL_DOWNLOAD_TIMEOUT_MS = 30_000; From d8fe1a31aec7bbd09fa9e2b83883c47054187621 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 29 Apr 2026 16:27:42 -0400 Subject: [PATCH 2/4] =?UTF-8?q?fix:=20apply=20CodeRabbit=20suggestion=20?= =?UTF-8?q?=E2=80=94=20use=20strict=20comparison=20for=20DOWNLOAD=20<=20HE?= =?UTF-8?q?AVY=20ordering?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/lib/openshell-timeouts.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/openshell-timeouts.test.ts b/src/lib/openshell-timeouts.test.ts index 580ccf97dc6..6202cfcc0b0 100644 --- a/src/lib/openshell-timeouts.test.ts +++ b/src/lib/openshell-timeouts.test.ts @@ -29,7 +29,7 @@ describe("openshell-timeouts", () => { it("maintains expected ordering: PROBE < OPERATION <= DOWNLOAD < HEAVY", () => { expect(OPENSHELL_PROBE_TIMEOUT_MS).toBeLessThan(OPENSHELL_OPERATION_TIMEOUT_MS); expect(OPENSHELL_OPERATION_TIMEOUT_MS).toBeLessThanOrEqual(OPENSHELL_DOWNLOAD_TIMEOUT_MS); - expect(OPENSHELL_DOWNLOAD_TIMEOUT_MS).toBeLessThanOrEqual(OPENSHELL_HEAVY_TIMEOUT_MS); + expect(OPENSHELL_DOWNLOAD_TIMEOUT_MS).toBeLessThan(OPENSHELL_HEAVY_TIMEOUT_MS); }); it("uses the same probe constant name as PR #2454 for forward compatibility", () => { From 6e67943c528b50218ca80df0366d49277de7dc0b Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 29 Apr 2026 16:45:20 -0400 Subject: [PATCH 3/4] refactor(arch): bound status/recovery hot path with probe timeouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add timeout: OPENSHELL_PROBE_TIMEOUT_MS to 15 unbounded captureOpenshell/runOpenshell calls in the status, recovery, and connect paths of nemoclaw.ts: - hasNoLiveSandboxes → sandbox list - executeSandboxCommand → sandbox ssh-config - recoverRegistryFromLiveGateway → sandbox list - getNamedGatewayLifecycleState → status + gateway info - recoverNamedGatewayRuntime → gateway select (x2) - getSandboxGatewayState → sandbox get + policy get - reconcileMissingAgainstNamedGateway → gateway select - recoverRegistryEntries → sandbox list - showStatus / listSandboxes / sandboxStatus → inference get (x4) - isGatewayAlive → sandbox list - providerExists → provider get This is Phase 2 of #2562 — the critical path that caused the #2398 E2E hang and blocks re-landing #2390. Refs: #2562 --- src/nemoclaw.ts | 77 +++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 62 insertions(+), 15 deletions(-) diff --git a/src/nemoclaw.ts b/src/nemoclaw.ts index d3755dabc3b..61ffcd02be3 100644 --- a/src/nemoclaw.ts +++ b/src/nemoclaw.ts @@ -94,6 +94,7 @@ import { knownChannelNames, persistChannelTokens, } from "./lib/sandbox-channels"; +import { OPENSHELL_PROBE_TIMEOUT_MS } from "./lib/openshell-timeouts"; const onboardProviders = require("./lib/onboard-providers"); // ── Global commands (derived from command registry) ────────────── @@ -182,7 +183,10 @@ function cleanupGatewayAfterLastSandbox() { } function hasNoLiveSandboxes() { - const liveList = captureOpenshell(["sandbox", "list"], { ignoreError: true }); + const liveList = captureOpenshell(["sandbox", "list"], { + ignoreError: true, + timeout: OPENSHELL_PROBE_TIMEOUT_MS, + }); if (liveList.status !== 0) { return false; } @@ -218,6 +222,7 @@ function getInstalledOpenshellVersionOrNull() { function executeSandboxCommand(sandboxName: string, command: string): SandboxCommandResult | null { const sshConfigResult = captureOpenshell(["sandbox", "ssh-config", sandboxName], { ignoreError: true, + timeout: OPENSHELL_PROBE_TIMEOUT_MS, }); if (sshConfigResult.status !== 0) return null; @@ -507,7 +512,10 @@ async function recoverRegistryFromLiveGateway( } let recoveredFromGateway = 0; - const liveList = captureOpenshell(["sandbox", "list"], { ignoreError: true }); + const liveList = captureOpenshell(["sandbox", "list"], { + ignoreError: true, + timeout: OPENSHELL_PROBE_TIMEOUT_MS, + }); const liveNames = Array.from(parseLiveSandboxNames(liveList.output)); for (const name of liveNames) { const metadata = metadataByName.get(name) || undefined; @@ -569,8 +577,10 @@ function getActiveGatewayName(output = ""): string | null { } function getNamedGatewayLifecycleState() { - const status = captureOpenshell(["status"]); - const gatewayInfo = captureOpenshell(["gateway", "info", "-g", "nemoclaw"]); + const status = captureOpenshell(["status"], { timeout: OPENSHELL_PROBE_TIMEOUT_MS }); + const gatewayInfo = captureOpenshell(["gateway", "info", "-g", "nemoclaw"], { + timeout: OPENSHELL_PROBE_TIMEOUT_MS, + }); const cleanStatus = stripAnsi(status.output); const activeGateway = getActiveGatewayName(status.output); const connected = /^\s*Status:\s*Connected\b/im.test(cleanStatus); @@ -625,7 +635,10 @@ async function recoverNamedGatewayRuntime() { return { recovered: true, before, after: before, attempted: false }; } - runOpenshell(["gateway", "select", "nemoclaw"], { ignoreError: true }); + runOpenshell(["gateway", "select", "nemoclaw"], { + ignoreError: true, + timeout: OPENSHELL_PROBE_TIMEOUT_MS, + }); let after = getNamedGatewayLifecycleState(); if (after.state === "healthy_named") { process.env.OPENSHELL_GATEWAY = "nemoclaw"; @@ -643,7 +656,10 @@ async function recoverNamedGatewayRuntime() { // Fall through to the lifecycle re-check below so we preserve the // existing recovery result shape and emit the correct classification. } - runOpenshell(["gateway", "select", "nemoclaw"], { ignoreError: true }); + runOpenshell(["gateway", "select", "nemoclaw"], { + ignoreError: true, + timeout: OPENSHELL_PROBE_TIMEOUT_MS, + }); after = getNamedGatewayLifecycleState(); if (after.state === "healthy_named") { process.env.OPENSHELL_GATEWAY = "nemoclaw"; @@ -656,7 +672,9 @@ async function recoverNamedGatewayRuntime() { /** Query sandbox presence and return its output with the live enforced policy. */ function getSandboxGatewayState(sandboxName: string) { - const result = captureOpenshell(["sandbox", "get", sandboxName]); + const result = captureOpenshell(["sandbox", "get", sandboxName], { + timeout: OPENSHELL_PROBE_TIMEOUT_MS, + }); let output = result.output; if (result.status === 0) { // `openshell sandbox get` returns the immutable baseline policy from sandbox @@ -666,6 +684,7 @@ function getSandboxGatewayState(sandboxName: string) { // Sandbox info above it. (#1132) const livePolicy = captureOpenshell(["policy", "get", "--full", sandboxName], { ignoreError: true, + timeout: OPENSHELL_PROBE_TIMEOUT_MS, }); if (livePolicy.status === 0 && livePolicy.output.trim()) { const rawLines = String(output).split("\n"); @@ -727,7 +746,10 @@ function reconcileMissingAgainstNamedGateway( ) { const lifecycle = getNamedGatewayLifecycleState(); if (lifecycle.state === "connected_other") { - runOpenshell(["gateway", "select", "nemoclaw"], { ignoreError: true }); + runOpenshell(["gateway", "select", "nemoclaw"], { + ignoreError: true, + timeout: OPENSHELL_PROBE_TIMEOUT_MS, + }); const retry = getSandboxGatewayState(sandboxName); if (retry.state === "present") { return { ...retry, recoveredGateway: true, recoveryVia: "select" }; @@ -1153,7 +1175,10 @@ function debug(args: string[]) { ); return undefined; } - const liveList = captureOpenshell(["sandbox", "list"], { ignoreError: true }); + const liveList = captureOpenshell(["sandbox", "list"], { + ignoreError: true, + timeout: OPENSHELL_PROBE_TIMEOUT_MS, + }); if (liveList.status === 0 && !parseLiveSandboxNames(liveList.output).has(defaultSandbox)) { console.error( `${_RD}Warning:${R} default sandbox '${defaultSandbox}' exists in the local registry but not in OpenShell.`, @@ -1399,7 +1424,10 @@ function makeConflictProbe() { let gatewayAlive: boolean | null = null; const isGatewayAlive = (): boolean => { if (gatewayAlive === null) { - const result = captureOpenshell(["sandbox", "list"], { ignoreError: true }); + const result = captureOpenshell(["sandbox", "list"], { + ignoreError: true, + timeout: OPENSHELL_PROBE_TIMEOUT_MS, + }); gatewayAlive = result.status === 0; } return gatewayAlive; @@ -1407,7 +1435,10 @@ function makeConflictProbe() { return { providerExists: (name: string) => { if (!isGatewayAlive()) return "error"; - const result = captureOpenshell(["provider", "get", name], { ignoreError: true }); + const result = captureOpenshell(["provider", "get", name], { + ignoreError: true, + timeout: OPENSHELL_PROBE_TIMEOUT_MS, + }); return result.status === 0 ? "present" : "absent"; }, }; @@ -1457,7 +1488,12 @@ function showStatus() { showStatusCommand({ listSandboxes: () => registry.listSandboxes(), getLiveInference: () => - parseGatewayInference(captureOpenshell(["inference", "get"], { ignoreError: true }).output), + parseGatewayInference( + captureOpenshell(["inference", "get"], { + ignoreError: true, + timeout: OPENSHELL_PROBE_TIMEOUT_MS, + }).output, + ), showServiceStatus, checkMessagingBridgeHealth, backfillAndFindOverlaps, @@ -1483,7 +1519,12 @@ async function listSandboxes(): Promise { await listSandboxesCommand({ recoverRegistryEntries: () => recoverRegistryEntries(), getLiveInference: () => - parseGatewayInference(captureOpenshell(["inference", "get"], { ignoreError: true }).output), + parseGatewayInference( + captureOpenshell(["inference", "get"], { + ignoreError: true, + timeout: OPENSHELL_PROBE_TIMEOUT_MS, + }).output, + ), loadLastSession: () => onboardSession.loadSession(), getActiveSessionCount: sessionDeps ? (name: string) => { @@ -1559,7 +1600,10 @@ async function sandboxConnect( const sb = registry.getSandbox(sandboxName); if (sb && sb.provider && sb.model) { const live = parseGatewayInference( - captureOpenshell(["inference", "get"], { ignoreError: true }).output, + captureOpenshell(["inference", "get"], { + ignoreError: true, + timeout: OPENSHELL_PROBE_TIMEOUT_MS, + }).output, ); if (!live || live.provider !== sb.provider || live.model !== sb.model) { console.log( @@ -1699,7 +1743,10 @@ async function sandboxConnect( async function sandboxStatus(sandboxName: string) { const sb = registry.getSandbox(sandboxName); const live = parseGatewayInference( - captureOpenshell(["inference", "get"], { ignoreError: true }).output, + captureOpenshell(["inference", "get"], { + ignoreError: true, + timeout: OPENSHELL_PROBE_TIMEOUT_MS, + }).output, ); const currentModel = (live && live.model) || (sb && sb.model) || "unknown"; const currentProvider = (live && live.provider) || (sb && sb.provider) || "unknown"; From fdc59a0f37fbca901354bb2bf3d14d5e5db95112 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 29 Apr 2026 17:06:34 -0400 Subject: [PATCH 4/4] fix(review): use OPENSHELL_OPERATION_TIMEOUT_MS for gateway select calls Address CodeRabbit review feedback: gateway select is a mutating operation, not a read-only probe. Switch the 3 gateway select calls from OPENSHELL_PROBE_TIMEOUT_MS (15s) to OPENSHELL_OPERATION_TIMEOUT_MS (30s) for correct timeout classification. Refs: #2562 --- src/nemoclaw.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/nemoclaw.ts b/src/nemoclaw.ts index 61ffcd02be3..8f224a6e801 100644 --- a/src/nemoclaw.ts +++ b/src/nemoclaw.ts @@ -94,7 +94,10 @@ import { knownChannelNames, persistChannelTokens, } from "./lib/sandbox-channels"; -import { OPENSHELL_PROBE_TIMEOUT_MS } from "./lib/openshell-timeouts"; +import { + OPENSHELL_OPERATION_TIMEOUT_MS, + OPENSHELL_PROBE_TIMEOUT_MS, +} from "./lib/openshell-timeouts"; const onboardProviders = require("./lib/onboard-providers"); // ── Global commands (derived from command registry) ────────────── @@ -637,7 +640,7 @@ async function recoverNamedGatewayRuntime() { runOpenshell(["gateway", "select", "nemoclaw"], { ignoreError: true, - timeout: OPENSHELL_PROBE_TIMEOUT_MS, + timeout: OPENSHELL_OPERATION_TIMEOUT_MS, }); let after = getNamedGatewayLifecycleState(); if (after.state === "healthy_named") { @@ -658,7 +661,7 @@ async function recoverNamedGatewayRuntime() { } runOpenshell(["gateway", "select", "nemoclaw"], { ignoreError: true, - timeout: OPENSHELL_PROBE_TIMEOUT_MS, + timeout: OPENSHELL_OPERATION_TIMEOUT_MS, }); after = getNamedGatewayLifecycleState(); if (after.state === "healthy_named") { @@ -748,7 +751,7 @@ function reconcileMissingAgainstNamedGateway( if (lifecycle.state === "connected_other") { runOpenshell(["gateway", "select", "nemoclaw"], { ignoreError: true, - timeout: OPENSHELL_PROBE_TIMEOUT_MS, + timeout: OPENSHELL_OPERATION_TIMEOUT_MS, }); const retry = getSandboxGatewayState(sandboxName); if (retry.state === "present") {