diff --git a/ci/source-architecture-budget.json b/ci/source-architecture-budget.json index 09950a7365b..8f9515af132 100644 --- a/ci/source-architecture-budget.json +++ b/ci/source-architecture-budget.json @@ -8,14 +8,14 @@ "src/lib/adapters/docker/index.ts": 43, "src/lib/adapters/openshell/client.ts": 23, "src/lib/adapters/openshell/resolve.ts": 27, - "src/lib/adapters/openshell/runtime.ts": 52, + "src/lib/adapters/openshell/runtime.ts": 53, "src/lib/adapters/openshell/timeouts.ts": 37, "src/lib/agent/defs.ts": 32, "src/lib/cli/branding.ts": 86, "src/lib/cli/nemoclaw-oclif-command.ts": 106, "src/lib/cli/terminal-style.ts": 43, "src/lib/core/json-types.ts": 37, - "src/lib/core/ports.ts": 88, + "src/lib/core/ports.ts": 89, "src/lib/core/shell-quote.ts": 28, "src/lib/core/url-utils.ts": 27, "src/lib/core/wait.ts": 35, @@ -23,11 +23,11 @@ "src/lib/inference/config.ts": 29, "src/lib/inference/web-search.ts": 21, "src/lib/messaging/channels/index.ts": 25, - "src/lib/onboard/gateway-binding.ts": 49, + "src/lib/onboard/gateway-binding.ts": 50, "src/lib/runner.ts": 88, "src/lib/security/redact.ts": 52, "src/lib/state/onboard-session.ts": 36, - "src/lib/state/registry.ts": 99, + "src/lib/state/registry.ts": 100, "src/lib/state/state-root.ts": 20, "src/lib/subprocess-env.ts": 24, "src/lib/validation.ts": 25 @@ -46,7 +46,7 @@ "src/lib/actions/sandbox/rebuild-pipeline.ts": 28, "src/lib/actions/sandbox/snapshot.ts": 40, "src/lib/actions/uninstall/run-plan.ts": 26, - "src/lib/inference/onboard-probes.ts": 20, + "src/lib/inference/onboard-probes.ts": 21, "src/lib/inference/vllm.ts": 21, "src/lib/onboard.ts": 210, "src/lib/onboard/machine/handlers/sandbox.ts": 21, diff --git a/src/lib/inference/onboard-probes.test.ts b/src/lib/inference/onboard-probes.test.ts index b14f02f6af3..e805d217c45 100644 --- a/src/lib/inference/onboard-probes.test.ts +++ b/src/lib/inference/onboard-probes.test.ts @@ -26,6 +26,7 @@ const { isSandboxInternalUrl, probeOpenAiLikeEndpoint, RETRIABLE_HTTP_PROBE_STATUSES, + verifyOnboardInferenceSmoke, } = require("./onboard-probes"); const { assertEndpointResolvesPublic } = require("./endpoint-ssrf-preflight") as typeof import("./endpoint-ssrf-preflight"); @@ -1324,3 +1325,40 @@ exit 0 }, ); }); + +describe("onboard inference smoke abort cleanup", () => { + it("tears down the orphan managed gateway before exiting after a failed smoke", async () => { + const teardownOrphanManagedGatewayOnAbort = vi.fn(); + const exit = vi.spyOn(process, "exit").mockImplementation((() => undefined) as never); + const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + vi.stubEnv("VITEST", "false"); + + try { + await verifyOnboardInferenceSmoke( + { + endpointUrl: "https://inference.example.com/v1", + forceOpenAiLike: true, + model: "example/model", + provider: "example-provider", + }, + { + probeOpenAiLikeEndpointOptimized: vi.fn().mockResolvedValue({ + ok: false, + message: "smoke failed", + }), + teardownOrphanManagedGatewayOnAbort, + }, + ); + + expect(teardownOrphanManagedGatewayOnAbort).toHaveBeenCalledOnce(); + expect(exit).toHaveBeenCalledWith(1); + expect(teardownOrphanManagedGatewayOnAbort.mock.invocationCallOrder[0]).toBeLessThan( + exit.mock.invocationCallOrder[0], + ); + } finally { + vi.unstubAllEnvs(); + error.mockRestore(); + exit.mockRestore(); + } + }); +}); diff --git a/src/lib/inference/onboard-probes.ts b/src/lib/inference/onboard-probes.ts index d4b5f7a7f1b..c1cbe1bb999 100644 --- a/src/lib/inference/onboard-probes.ts +++ b/src/lib/inference/onboard-probes.ts @@ -1161,7 +1161,7 @@ export function shouldSmokeOpenAiLikeOnboardRoute( ); } -export async function verifyOnboardInferenceSmoke(options: any) { +export async function verifyOnboardInferenceSmoke(options: any, dependencies: any = {}) { if ( !options.forceOpenAiLike && !shouldSmokeOpenAiLikeOnboardRoute(options.provider, options.credentialEnv) @@ -1190,7 +1190,9 @@ export async function verifyOnboardInferenceSmoke(options: any) { const apiKey = credentialEnv ? resolveProviderCredential(credentialEnv) || getCredential(credentialEnv) || "" : ""; - const probe = await probeOpenAiLikeEndpointOptimized(endpointUrl, options.model, apiKey, { + const optimizedProbe = + dependencies.probeOpenAiLikeEndpointOptimized ?? probeOpenAiLikeEndpointOptimized; + const probe = await optimizedProbe(endpointUrl, options.model, apiKey, { authMode: getProbeAuthMode(options.provider), extraHeaders: getProbeExtraHeaders(options.provider), skipResponsesProbe: true, @@ -1217,8 +1219,11 @@ export async function verifyOnboardInferenceSmoke(options: any) { ); // #8952: tear down an unowned managed gateway before fatal exit. try { - const { teardownOrphanManagedGatewayOnAbort } = - require("../onboard/abort-gateway-teardown") as typeof import("../onboard/abort-gateway-teardown"); + const teardownOrphanManagedGatewayOnAbort = + dependencies.teardownOrphanManagedGatewayOnAbort ?? + ( + require("../onboard/gateway-destroy") as typeof import("../onboard/gateway-destroy") + ).teardownOrphanManagedGatewayOnAbort; teardownOrphanManagedGatewayOnAbort(); } catch (error) { // Helper never throws; this covers require/load failures only. diff --git a/src/lib/onboard/abort-gateway-teardown.test.ts b/src/lib/onboard/abort-gateway-teardown.test.ts index 95b2e4bf996..2415bbb4c38 100644 --- a/src/lib/onboard/abort-gateway-teardown.test.ts +++ b/src/lib/onboard/abort-gateway-teardown.test.ts @@ -6,7 +6,7 @@ import { describe, expect, it, vi } from "vitest"; import { gatewayHasRegisteredSandbox, teardownOrphanManagedGatewayOnAbort, -} from "./abort-gateway-teardown"; +} from "./gateway-destroy"; import { GatewayAuthorityError } from "./gateway-teardown-authority"; describe("gatewayHasRegisteredSandbox", () => { diff --git a/src/lib/onboard/abort-gateway-teardown.ts b/src/lib/onboard/abort-gateway-teardown.ts deleted file mode 100644 index 0d96112e20c..00000000000 --- a/src/lib/onboard/abort-gateway-teardown.ts +++ /dev/null @@ -1,151 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -/** - * Tear down an unowned NemoClaw-managed host gateway after onboard aborts - * once the gateway is up but before a sandbox is registered (#8952). - * The Docker-driver gateway inherits provider credentials from the onboard - * process environment, so the listener must not survive a failed run. - */ - -import { GATEWAY_PORT } from "../core/ports"; -import { listSandboxes as listRegisteredSandboxes } from "../state/registry"; -import { releaseManagedGatewayPort } from "../tunnel/gateway-port-release"; -import { resolveGatewayName, resolveSandboxGatewayName } from "./gateway-binding"; -import { isExternallySupervised } from "./gateway-ownership"; -import { - GatewayAuthorityError, - resolveGatewayTeardownAuthority, -} from "./gateway-teardown-authority"; - -export type AbortGatewayTeardownDeps = { - env?: NodeJS.ProcessEnv; - gatewayPort?: number; - gatewayName?: string; - listSandboxes?: typeof listRegisteredSandboxes; - resolveAuthority?: typeof resolveGatewayTeardownAuthority; - releaseManagedGatewayPort?: typeof releaseManagedGatewayPort; - removeGatewayRegistration?: (gatewayName: string) => void; - log?: (message: string) => void; - warn?: (message: string) => void; -}; - -function defaultRemoveGatewayRegistration(gatewayName: string): void { - // Lazy require keeps unit tests free of openshell binary resolution. - const runtime = - require("../adapters/openshell/runtime") as typeof import("../adapters/openshell/runtime"); - runtime.runOpenshell(["gateway", "remove", gatewayName], { - ignoreError: true, - stdio: ["ignore", "pipe", "pipe"], - }); -} - -/** - * True when any registered sandbox is bound to `gatewayName`. An unreadable - * binding fails closed (treat as owned) so abort teardown never guesses. - */ -export function gatewayHasRegisteredSandbox( - gatewayName: string, - listSandboxes: typeof listRegisteredSandboxes = listRegisteredSandboxes, -): boolean { - for (const sandbox of listSandboxes().sandboxes) { - try { - if (resolveSandboxGatewayName(sandbox) === gatewayName) return true; - } catch { - return true; - } - } - return false; -} - -/** - * Best-effort: stop the managed host gateway listener and remove its OpenShell - * registration when no sandbox still owns that gateway. Never throws — callers - * on fatal exit paths must still be able to `process.exit(1)` after a warn. - * - * @returns true when a teardown attempt ran (stop and/or remove). - */ -export function teardownOrphanManagedGatewayOnAbort(deps: AbortGatewayTeardownDeps = {}): boolean { - const log = deps.log ?? ((message: string) => console.error(message)); - const warn = deps.warn ?? ((message: string) => console.error(message)); - - try { - const env = deps.env ?? process.env; - const port = deps.gatewayPort ?? GATEWAY_PORT; - const gatewayName = deps.gatewayName ?? resolveGatewayName(port); - const listSandboxes = deps.listSandboxes ?? listRegisteredSandboxes; - const resolveAuthority = deps.resolveAuthority ?? resolveGatewayTeardownAuthority; - const release = deps.releaseManagedGatewayPort ?? releaseManagedGatewayPort; - const removeRegistration = deps.removeGatewayRegistration ?? defaultRemoveGatewayRegistration; - - if (gatewayHasRegisteredSandbox(gatewayName, listSandboxes)) { - return false; - } - - try { - const owner = resolveAuthority({ gatewayName, gatewayPort: port }, { env }); - if (isExternallySupervised(owner)) { - log( - ` Keeping externally supervised OpenShell gateway '${gatewayName}' running after onboard abort.`, - ); - return false; - } - } catch (error) { - if (error instanceof GatewayAuthorityError) { - warn(` Skipping gateway teardown after onboard abort: ${error.message}`); - return false; - } - warn( - ` Skipping gateway teardown after onboard abort: ${error instanceof Error ? error.message : String(error)}`, - ); - return false; - } - - log( - ` Onboard aborted before a sandbox was created; releasing managed gateway '${gatewayName}' so provider credentials do not remain in a live process.`, - ); - - let attempted = false; - let releaseConfirmed = false; - try { - const result = release({ port }); - attempted = true; - releaseConfirmed = result.released; - if (result.released && result.stopped.length > 0) { - log( - ` Released gateway port ${String(result.port)} (stopped host process ${result.stopped.join(", ")}).`, - ); - } else if (!result.released && !result.skipped) { - warn( - ` Gateway port ${String(result.port ?? port)} was not confirmed released after onboard abort. Inspect the listener and stop only the matching openshell-gateway process.`, - ); - } - } catch (error) { - attempted = true; - warn( - ` Gateway process stop after onboard abort failed: ${error instanceof Error ? error.message : String(error)}`, - ); - } - - // Keep the OpenShell registration when the listener may still be up — it is - // the supported recovery handle for a credential-bearing process. - if (!releaseConfirmed) return attempted; - - try { - removeRegistration(gatewayName); - attempted = true; - } catch (error) { - warn( - ` Gateway registration remove after onboard abort failed: ${error instanceof Error ? error.message : String(error)}`, - ); - } - - return attempted; - } catch (error) { - // Warn and continue to fatal exit; do not hide a still-live listener. - warn( - ` Gateway teardown after onboard abort failed: ${error instanceof Error ? error.message : String(error)}`, - ); - return false; - } -} diff --git a/src/lib/onboard/gateway-destroy.ts b/src/lib/onboard/gateway-destroy.ts index 3dc447913fa..78f4dbd5dd2 100644 --- a/src/lib/onboard/gateway-destroy.ts +++ b/src/lib/onboard/gateway-destroy.ts @@ -1,6 +1,16 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { GATEWAY_PORT } from "../core/ports"; +import { listSandboxes as listRegisteredSandboxes } from "../state/registry"; +import { releaseManagedGatewayPort } from "../tunnel/gateway-port-release"; +import { resolveGatewayName, resolveSandboxGatewayName } from "./gateway-binding"; +import { isExternallySupervised } from "./gateway-ownership"; +import { + GatewayAuthorityError, + resolveGatewayTeardownAuthority, +} from "./gateway-teardown-authority"; + export type RunOpenshell = ( args: string[], opts: { ignoreError: true }, @@ -19,6 +29,143 @@ export type DestroyGatewayDeps = { stopDockerDriverGatewayProcess: () => void; }; +/** + * Abort cleanup runs after the OpenShell gateway starts and before NemoClaw + * registers a sandbox. The Docker-driver gateway inherits provider credentials + * from onboarding, so an unowned listener must not survive a failed run. + */ +export type AbortGatewayTeardownDeps = { + env?: NodeJS.ProcessEnv; + gatewayPort?: number; + gatewayName?: string; + listSandboxes?: typeof listRegisteredSandboxes; + resolveAuthority?: typeof resolveGatewayTeardownAuthority; + releaseManagedGatewayPort?: typeof releaseManagedGatewayPort; + removeGatewayRegistration?: (gatewayName: string) => void; + log?: (message: string) => void; + warn?: (message: string) => void; +}; + +function defaultRemoveGatewayRegistration(gatewayName: string): void { + // Lazy require keeps unit tests free of openshell binary resolution. + const runtime = + require("../adapters/openshell/runtime") as typeof import("../adapters/openshell/runtime"); + runtime.runOpenshell(["gateway", "remove", gatewayName], { + ignoreError: true, + stdio: ["ignore", "pipe", "pipe"], + }); +} + +/** + * True when any registered sandbox is bound to `gatewayName`. An unreadable + * binding fails closed (treat as owned) so abort teardown never guesses. + */ +export function gatewayHasRegisteredSandbox( + gatewayName: string, + listSandboxes: typeof listRegisteredSandboxes = listRegisteredSandboxes, +): boolean { + for (const sandbox of listSandboxes().sandboxes) { + try { + if (resolveSandboxGatewayName(sandbox) === gatewayName) return true; + } catch { + return true; + } + } + return false; +} + +/** + * Best-effort: stop the managed host gateway listener and remove its OpenShell + * registration when no sandbox still owns that gateway. Never throws — callers + * on fatal exit paths must still be able to `process.exit(1)` after a warning. + * + * @returns true when a teardown attempt ran (stop and/or remove). + */ +export function teardownOrphanManagedGatewayOnAbort(deps: AbortGatewayTeardownDeps = {}): boolean { + const log = deps.log ?? ((message: string) => console.error(message)); + const warn = deps.warn ?? ((message: string) => console.error(message)); + + try { + const env = deps.env ?? process.env; + const port = deps.gatewayPort ?? GATEWAY_PORT; + const gatewayName = deps.gatewayName ?? resolveGatewayName(port); + const listSandboxes = deps.listSandboxes ?? listRegisteredSandboxes; + const resolveAuthority = deps.resolveAuthority ?? resolveGatewayTeardownAuthority; + const release = deps.releaseManagedGatewayPort ?? releaseManagedGatewayPort; + const removeRegistration = deps.removeGatewayRegistration ?? defaultRemoveGatewayRegistration; + + if (gatewayHasRegisteredSandbox(gatewayName, listSandboxes)) { + return false; + } + + try { + const owner = resolveAuthority({ gatewayName, gatewayPort: port }, { env }); + if (isExternallySupervised(owner)) { + log( + ` Keeping externally supervised OpenShell gateway '${gatewayName}' running after onboard abort.`, + ); + return false; + } + } catch (error) { + if (error instanceof GatewayAuthorityError) { + warn(` Skipping gateway teardown after onboard abort: ${error.message}`); + return false; + } + warn( + ` Skipping gateway teardown after onboard abort: ${error instanceof Error ? error.message : String(error)}`, + ); + return false; + } + + log( + ` Onboard aborted before a sandbox was created; releasing managed gateway '${gatewayName}' so provider credentials do not remain in a live process.`, + ); + + let attempted = false; + let releaseConfirmed = false; + try { + const result = release({ port }); + attempted = true; + releaseConfirmed = result.released; + if (result.released && result.stopped.length > 0) { + log( + ` Released gateway port ${String(result.port)} (stopped host process ${result.stopped.join(", ")}).`, + ); + } else if (!result.released && !result.skipped) { + warn( + ` Gateway port ${String(result.port ?? port)} was not confirmed released after onboard abort. Inspect the listener and stop only the matching openshell-gateway process.`, + ); + } + } catch (error) { + attempted = true; + warn( + ` Gateway process stop after onboard abort failed: ${error instanceof Error ? error.message : String(error)}`, + ); + } + + // Keep the OpenShell registration when the listener may still be up — it is + // the supported recovery handle for a credential-bearing process. + if (!releaseConfirmed) return attempted; + + try { + removeRegistration(gatewayName); + attempted = true; + } catch (error) { + warn( + ` Gateway registration remove after onboard abort failed: ${error instanceof Error ? error.message : String(error)}`, + ); + } + + return attempted; + } catch (error) { + // Warn and continue to fatal exit; do not hide a still-live listener. + warn( + ` Gateway teardown after onboard abort failed: ${error instanceof Error ? error.message : String(error)}`, + ); + return false; + } +} + export function destroyGatewayWithVolumeCleanup({ clearRegistry, dockerRemoveVolumesByPrefix, diff --git a/src/lib/onboard/inference-selection-validation.test.ts b/src/lib/onboard/inference-selection-validation.test.ts index df5127e1fbe..03ca4718c35 100644 --- a/src/lib/onboard/inference-selection-validation.test.ts +++ b/src/lib/onboard/inference-selection-validation.test.ts @@ -92,6 +92,7 @@ describe("inference selection validation", () => { const error = vi.spyOn(console, "error").mockImplementation(() => {}); const exit = vi.spyOn(process, "exit").mockImplementation((() => undefined) as never); const promptValidationRecovery = vi.fn(async () => "selection" as const); + const teardownOrphanManagedGatewayOnAbort = vi.fn(); const helpers = createInferenceSelectionValidationHelpers({ isNonInteractive: () => true, agentProductName: () => "OpenClaw", @@ -100,6 +101,7 @@ describe("inference selection validation", () => { ok: false, failures: [{ name: "Chat Completions API", httpStatus: 403 }], }), + teardownOrphanManagedGatewayOnAbort, promptValidationRecovery, }); @@ -115,6 +117,7 @@ describe("inference selection validation", () => { expect(exit).toHaveBeenCalledWith(1); expect(process.exitCode).toBe(1); expect(promptValidationRecovery).not.toHaveBeenCalled(); + expect(teardownOrphanManagedGatewayOnAbort).toHaveBeenCalledOnce(); expect(error.mock.calls.map((args) => args.join(" "))).toEqual([ " NVIDIA Endpoints endpoint validation failed.", " Validation probe summary: Chat Completions API: HTTP 403.", @@ -454,6 +457,7 @@ describe("inference selection validation", () => { probeAnthropicEndpoint, promptValidationRecovery: vi.fn(async () => "selection" as const), resolveEndpointHost: async () => [{ address: "169.254.169.254", family: 4 }], + teardownOrphanManagedGatewayOnAbort: vi.fn(), }); try { @@ -871,6 +875,7 @@ exit 0 probeOpenAiLikeEndpoint, promptValidationRecovery, resolveEndpointHost: async () => [{ address: "93.184.216.34", family: 4 }], + teardownOrphanManagedGatewayOnAbort: vi.fn(), }); try { diff --git a/src/lib/onboard/inference-selection-validation.ts b/src/lib/onboard/inference-selection-validation.ts index 976a3cb0161..c18218b0b15 100644 --- a/src/lib/onboard/inference-selection-validation.ts +++ b/src/lib/onboard/inference-selection-validation.ts @@ -158,7 +158,7 @@ export function createInferenceSelectionValidationHelpers( deps.teardownOrphanManagedGatewayOnAbort ?? (() => { const { teardownOrphanManagedGatewayOnAbort } = - require("./abort-gateway-teardown") as typeof import("./abort-gateway-teardown"); + require("./gateway-destroy") as typeof import("./gateway-destroy"); teardownOrphanManagedGatewayOnAbort(); }); teardown();