diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 9c348d06862..2e4b3ccadad 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -163,7 +163,7 @@ const pRetry = require("p-retry"); const runner: typeof import("./runner") = require("./runner"); const { ROOT, SCRIPTS, redact, run, runCapture, runCaptureEx, runFile, validateName } = runner; const braveProviderProfile: typeof import("./onboard/brave-provider-profile") = require("./onboard/brave-provider-profile"); -const { runSandboxProviderPreDeleteCleanup } = +const { reconcileRegisteredExtraProviders, runSandboxProviderPreDeleteCleanup } = require("./onboard/sandbox-provider-cleanup") as typeof import("./onboard/sandbox-provider-cleanup"); const nameValidation: typeof import("./name-validation") = require("./name-validation"); const { getNameValidationGuidance } = nameValidation; @@ -2749,7 +2749,7 @@ async function createSandboxWithBaseImageResolution( messagingTokenDefs, reusableMessagingChannels, reusableMessagingProviders, - extraProviders: registry.listExtraProviders(), + extraProviders: reconcileRegisteredExtraProviders({ runOpenshell, gatewayName: GATEWAY_NAME }), hermesToolGateways, sandboxGpuConfig: effectiveSandboxGpuConfig, dockerDriverGateway, diff --git a/src/lib/onboard/extra-provider-reconciliation.ts b/src/lib/onboard/extra-provider-reconciliation.ts new file mode 100644 index 00000000000..56184a8d127 --- /dev/null +++ b/src/lib/onboard/extra-provider-reconciliation.ts @@ -0,0 +1,123 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { assertNoOpenShellGatewayEndpointOverride } from "../openshell-gateway-endpoint-guard"; +import type { SandboxProviderRunOpenshell } from "./sandbox-provider-cleanup"; + +export type ReconcileExtraProvidersDeps = { + runOpenshell?: SandboxProviderRunOpenshell; + /** + * Scope existence probes to this gateway (`provider get -g `), + * mirroring the gateway-scoped runner the other onboarding provider + * probes use. When set, the same endpoint-override guard applies. + */ + gatewayName?: string; + listExtraProviders?: () => string[]; + forgetExtraProvider?: (name: string) => boolean; + warn?: (message: string) => void; +}; + +/** + * Diagnostic shapes for "the probed provider does not exist": both the CLI's + * `provider 'X' not found` and the gRPC-style `NotFound: provider "X"` + * orderings. Anchored to the word "provider" on the same line so missing- + * sandbox or missing-gateway errors never count as a provider-not-found. + */ +const PROVIDER_NOT_FOUND_RE = + /provider[^\n]{0,200}?(?:\bNotFound\b|\bnot\s+found\b)|(?:\bNotFound\b|\bnot\s+found\b)(?::|\s)[^\n]{0,200}?\bprovider\b/i; + +function toText(value: string | Buffer | null | undefined): string { + if (typeof value === "string") return value; + if (value && typeof (value as Buffer).toString === "function") { + return (value as Buffer).toString(); + } + return ""; +} + +function defaultRunOpenshell( + args: string[], + opts?: Record, +): ReturnType { + const runtime = require("../adapters/openshell/runtime") as { + runOpenshell: SandboxProviderRunOpenshell; + }; + return runtime.runOpenshell(args, opts); +} + +function defaultListExtraProviders(): string[] { + const { listExtraProviders } = require("../state/registry") as { + listExtraProviders: () => string[]; + }; + return listExtraProviders(); +} + +function defaultForgetExtraProvider(name: string): boolean { + const { removeExtraProvider } = require("../state/registry") as { + removeExtraProvider: (name: string) => boolean; + }; + return removeExtraProvider(name); +} + +// SOURCE_OF_TRUTH_REVIEW (extra-provider registry vs gateway drift, #6501): +// invalid state = the host registry records an extra provider (written by +// `credentials add` → `addExtraProvider`) that the gateway no longer knows, +// created by gateway-side `provider delete` or pointing the CLI at a rebuilt +// gateway — neither path can update the host record because OpenShell emits +// no provider-deletion signal the CLI could observe. Passing the dangling +// name to `sandbox create --provider` then fails every subsequent onboard +// with "provider not found", even when the user declined the feature that +// once created it. Reconciling at consumption time recovers regardless of +// how the desync happened. Regression proof lives in +// test/extra-provider-reconciliation.test.ts and the spawn-level onboard +// test in test/onboard-extra-provider-prune.test.ts. Remove this helper when +// OpenShell exposes a structured provider-deletion event (or the registry +// stops mirroring gateway provider state). +/** + * Resolve the registry-recorded extra providers that sandbox creation may + * attach, dropping records the gateway no longer knows about (#6501). + * + * Each recorded name is probed with `provider get` (the same existence + * check `upsertProvider` uses); a record is pruned only when the gateway + * explicitly answers "provider … not found". Any other failure — gateway + * down, timeout, unexpected diagnostic — keeps the record (fail-open, with + * a debug note for diagnosability) so a real outage still surfaces through + * the sandbox-create diagnostics instead of silently dropping a healthy + * provider. + */ +export function reconcileRegisteredExtraProviders( + deps: ReconcileExtraProvidersDeps = {}, +): string[] { + const recorded = (deps.listExtraProviders ?? defaultListExtraProviders)(); + if (recorded.length === 0) return recorded; + if (deps.gatewayName) assertNoOpenShellGatewayEndpointOverride(); + const gatewayArgs = deps.gatewayName ? ["-g", deps.gatewayName] : []; + const runOpenshell = deps.runOpenshell ?? defaultRunOpenshell; + const warn = deps.warn ?? ((message: string) => console.warn(message)); + const forget = deps.forgetExtraProvider ?? defaultForgetExtraProvider; + return recorded.filter((name) => { + const result = runOpenshell(["provider", "get", ...gatewayArgs, name], { + ignoreError: true, + stdio: ["ignore", "pipe", "pipe"], + suppressOutput: true, + }); + if (result.status === 0) return true; + const output = `${toText(result.stdout)}${toText(result.stderr)}`; + if (!PROVIDER_NOT_FOUND_RE.test(output)) { + console.debug( + `reconcileRegisteredExtraProviders: keeping '${name}' — existence probe failed without a provider-not-found diagnostic (fail-open).`, + ); + return true; + } + warn( + ` Skipping recorded provider '${name}': not registered with the OpenShell gateway. ` + + `Removing the stale local record; recreate it with 'nemoclaw credentials add' if needed.`, + ); + try { + forget(name); + } catch { + // A registry write failure must not abort onboarding — the dangling + // record is still skipped for this run and re-pruned on the next one. + } + return false; + }); +} diff --git a/src/lib/onboard/sandbox-provider-cleanup.ts b/src/lib/onboard/sandbox-provider-cleanup.ts index 68f18b3ab4c..9725d8b4991 100644 --- a/src/lib/onboard/sandbox-provider-cleanup.ts +++ b/src/lib/onboard/sandbox-provider-cleanup.ts @@ -4,6 +4,11 @@ import { listMessagingProviderSuffixes } from "../messaging/channels"; import { NAME_MAX_LENGTH, NAME_VALID_PATTERN } from "../name-validation"; +export { + type ReconcileExtraProvidersDeps, + reconcileRegisteredExtraProviders, +} from "./extra-provider-reconciliation"; + export type SandboxProviderRunOpenshell = ( args: string[], opts?: Record, diff --git a/test/extra-provider-reconciliation.test.ts b/test/extra-provider-reconciliation.test.ts new file mode 100644 index 00000000000..2b78115576c --- /dev/null +++ b/test/extra-provider-reconciliation.test.ts @@ -0,0 +1,132 @@ +// 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 { reconcileRegisteredExtraProviders } from "../src/lib/onboard/extra-provider-reconciliation.js"; + +type Argv = string[]; +type RunResult = { status: number | null; stderr?: string; stdout?: string | Buffer }; + +function buildRunOpenshell( + responses: Map, + defaultResponse: RunResult = { status: 0 }, +) { + const calls: Argv[] = []; + const fn = vi.fn((args: Argv, _opts?: Record) => { + calls.push(args); + const key = args.join(" "); + return responses.get(key) ?? defaultResponse; + }); + return { runOpenshell: fn, calls }; +} + +describe("reconcileRegisteredExtraProviders", () => { + it("returns the empty set without querying the gateway when nothing is recorded", () => { + const { runOpenshell, calls } = buildRunOpenshell(new Map()); + const forget = vi.fn(); + + const result = reconcileRegisteredExtraProviders({ + runOpenshell, + listExtraProviders: () => [], + forgetExtraProvider: forget, + }); + + expect(result).toEqual([]); + expect(calls).toEqual([]); + expect(forget).not.toHaveBeenCalled(); + }); + + it("keeps recorded providers that the gateway confirms via a scoped 'provider get'", () => { + const responses = new Map([ + ["provider get -g nemoclaw tavily-search", { status: 0, stdout: "name: tavily-search\n" }], + ]); + const { runOpenshell, calls } = buildRunOpenshell(responses, { status: 1 }); + const forget = vi.fn(); + const warn = vi.fn(); + + const result = reconcileRegisteredExtraProviders({ + runOpenshell, + gatewayName: "nemoclaw", + listExtraProviders: () => ["tavily-search"], + forgetExtraProvider: forget, + warn, + }); + + expect(result).toEqual(["tavily-search"]); + expect(calls).toEqual([["provider", "get", "-g", "nemoclaw", "tavily-search"]]); + expect(forget).not.toHaveBeenCalled(); + expect(warn).not.toHaveBeenCalled(); + }); + + it("skips, warns about, and forgets a recorded provider the gateway reports not found (#6501)", () => { + const responses = new Map([ + [ + "provider get brave-search", + { status: 1, stderr: "Error: provider 'brave-search' not found\n" }, + ], + [ + "provider get tavily-search", + { status: 1, stderr: 'rpc error: NotFound: provider "tavily-search"\n' }, + ], + ]); + const { runOpenshell } = buildRunOpenshell(responses); + const forget = vi.fn(); + const warn = vi.fn(); + + const result = reconcileRegisteredExtraProviders({ + runOpenshell, + listExtraProviders: () => ["brave-search", "tavily-search"], + forgetExtraProvider: forget, + warn, + }); + + expect(result).toEqual([]); + expect(forget.mock.calls.map((c) => c[0])).toEqual(["brave-search", "tavily-search"]); + const messages = warn.mock.calls.map((c) => c[0] as string); + expect(messages[0]).toContain("'brave-search'"); + expect(messages[1]).toContain("'tavily-search'"); + expect(messages[1]).toContain("nemoclaw credentials add"); + }); + + it("keeps the recorded set unchanged when the probe fails without a not-found diagnostic", () => { + const responses = new Map([ + ["provider get tavily-search", { status: 1, stderr: "gateway not running" }], + ]); + const { runOpenshell } = buildRunOpenshell(responses); + const forget = vi.fn(); + const warn = vi.fn(); + + const result = reconcileRegisteredExtraProviders({ + runOpenshell, + listExtraProviders: () => ["tavily-search"], + forgetExtraProvider: forget, + warn, + }); + + expect(result).toEqual(["tavily-search"]); + expect(forget).not.toHaveBeenCalled(); + expect(warn).not.toHaveBeenCalled(); + }); + + it("prunes on a Buffer not-found diagnostic while keeping confirmed bridge providers", () => { + const responses = new Map([ + ["provider get my-slack-bridge", { status: 0 }], + [ + "provider get tavily-search", + { status: 1, stdout: Buffer.from("provider 'tavily-search' not found\n") }, + ], + ]); + const { runOpenshell } = buildRunOpenshell(responses); + const forget = vi.fn(); + + const result = reconcileRegisteredExtraProviders({ + runOpenshell, + listExtraProviders: () => ["my-slack-bridge", "tavily-search"], + forgetExtraProvider: forget, + }); + + expect(result).toEqual(["my-slack-bridge"]); + expect(forget.mock.calls.map((c) => c[0])).toEqual(["tavily-search"]); + }); +}); diff --git a/test/onboard-extra-provider-prune.test.ts b/test/onboard-extra-provider-prune.test.ts new file mode 100644 index 00000000000..0a738826ee9 --- /dev/null +++ b/test/onboard-extra-provider-prune.test.ts @@ -0,0 +1,147 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { describe, it } from "vitest"; +import { writeOkOpenshell } from "./helpers/onboard-openshell-fixture"; + +type CommandEntry = { + command: string; + env?: Record; +}; + +const repoRoot = path.join(import.meta.dirname, ".."); +const onboardScriptMocksPath = JSON.stringify( + path.join(repoRoot, "test", "helpers", "onboard-script-mocks.cjs"), +); + +describe("onboard extra provider pruning", () => { + it("prunes a dangling tavily-search provider record before sandbox create (#6501)", { + timeout: 90_000, + }, async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-onboard-prune-provider-")); + const fakeBin = path.join(tmpDir, "bin"); + const scriptPath = path.join(tmpDir, "prune-provider-check.js"); + const onboardPath = JSON.stringify(path.join(repoRoot, "src", "lib", "onboard.ts")); + const runnerPath = JSON.stringify(path.join(repoRoot, "src", "lib", "runner.ts")); + const registryPath = JSON.stringify(path.join(repoRoot, "src", "lib", "state", "registry.ts")); + const preflightPath = JSON.stringify( + path.join(repoRoot, "src", "lib", "onboard", "preflight.ts"), + ); + const credentialsPath = JSON.stringify( + path.join(repoRoot, "src", "lib", "credentials", "store.ts"), + ); + + fs.mkdirSync(fakeBin, { recursive: true }); + writeOkOpenshell(fakeBin); + + const script = String.raw` +const runner = require(${runnerPath}); +const _n = (c) => (Array.isArray(c) ? c.join(" ") : String(c)).replace(/'/g, ""); +const registry = require(${registryPath}); +const preflight = require(${preflightPath}); +const credentials = require(${credentialsPath}); +const childProcess = require("node:child_process"); +const { EventEmitter } = require("node:events"); + +const commands = []; +registry.addExtraProvider("tavily-search"); +runner.run = (command, opts = {}) => { + const cmd = _n(command); + commands.push({ command: cmd, env: opts.env || null }); + // The gateway does not know tavily-search: the reconcile probe must see a + // provably dangling record (#6501). + if (cmd.includes("provider get") && cmd.includes("tavily-search")) { + return { status: 1, stdout: "", stderr: "Error: provider 'tavily-search' not found" }; + } + return { status: 0 }; +}; +runner.runCapture = (command) => { + if (_n(command).includes("sandbox get my-assistant")) return ""; + if (_n(command).includes("sandbox list")) return "my-assistant Ready"; + { + const mockedCapture = require(${onboardScriptMocksPath}).mockOnboardRunCapture(command); + if (mockedCapture !== null) return mockedCapture; + } + if (_n(command).includes("forward list")) return "my-assistant 127.0.0.1 18789 12345 running"; + return ""; +}; +registry.registerSandbox = () => true; +registry.updateSandbox = () => true; +registry.setDefault = () => true; +registry.removeSandbox = () => true; +preflight.checkPortAvailable = async () => ({ ok: true }); +credentials.prompt = async () => ""; + +childProcess.spawn = (...args) => { + const child = new EventEmitter(); + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + child.unref = () => {}; + child.pid = 4242; + commands.push({ command: _n([args[0], ...(Array.isArray(args[1]) ? args[1] : [])]), env: args[2]?.env || null }); + process.nextTick(() => { + child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); + child.emit("close", 0); + }); + return child; +}; + +const { createSandbox } = require(${onboardPath}); + +(async () => { + process.env.OPENSHELL_GATEWAY = "nemoclaw"; + const sandboxName = await createSandbox(null, "gpt-5.4"); + console.log(JSON.stringify({ sandboxName, commands, extraProviders: registry.listExtraProviders() })); +})().catch((error) => { + console.error(error); + process.exit(1); +}); +`; + fs.writeFileSync(scriptPath, script); + + const result = spawnSync(process.execPath, [scriptPath], { + cwd: repoRoot, + encoding: "utf-8", + env: { + ...process.env, + HOME: tmpDir, + PATH: `${fakeBin}:${process.env.PATH || ""}`, + NEMOCLAW_NON_INTERACTIVE: "1", + }, + }); + + assert.equal(result.status, 0, result.stderr); + const payloadLine = result.stdout + .trim() + .split("\n") + .slice() + .reverse() + .find((line) => line.startsWith("{") && line.endsWith("}")); + assert.ok(payloadLine, `expected JSON payload in stdout:\n${result.stdout}`); + const payload = JSON.parse(payloadLine); + assert.equal(payload.sandboxName, "my-assistant"); + const createCommand = payload.commands.find((entry: CommandEntry) => + entry.command.includes("sandbox create"), + ); + assert.ok(createCommand, "expected sandbox create command"); + assert.match(createCommand.command, /nemoclaw-start/); + assert.doesNotMatch(createCommand.command, /--provider tavily-search/); + const probeCommand = payload.commands.find( + (entry: CommandEntry) => + entry.command.includes("provider get") && entry.command.includes("tavily-search"), + ); + assert.ok(probeCommand, "expected a gateway-scoped provider existence probe"); + assert.match(probeCommand.command, /provider get -g \S+ tavily-search/); + assert.deepEqual( + payload.extraProviders, + [], + "expected the dangling tavily-search record to be removed from the registry", + ); + assert.match(result.stderr, /Skipping recorded provider 'tavily-search'/); + }); +}); diff --git a/test/sandbox-provider-cleanup.test.ts b/test/sandbox-provider-cleanup.test.ts index 761bc8c734f..424e00080d8 100644 --- a/test/sandbox-provider-cleanup.test.ts +++ b/test/sandbox-provider-cleanup.test.ts @@ -14,7 +14,7 @@ import { } from "../src/lib/onboard/sandbox-provider-cleanup.js"; type Argv = string[]; -type RunResult = { status: number | null; stderr?: string; stdout?: string }; +type RunResult = { status: number | null; stderr?: string; stdout?: string | Buffer }; function buildRunOpenshell( responses: Map,