diff --git a/src/lib/actions/sandbox/mcp-bridge-status-resolution.test.ts b/src/lib/actions/sandbox/mcp-bridge-status-resolution.test.ts index aba649976e9..3543b0161cf 100644 --- a/src/lib/actions/sandbox/mcp-bridge-status-resolution.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-status-resolution.test.ts @@ -146,7 +146,7 @@ ${body} return { status: result.status, stdout: result.stdout }; } -describe("MCP status wire-level credential-resolution probe", () => { +describe("MCP status wire-level credential-resolution probe", { timeout: 15_000 }, () => { it("probes by default for a single named server and surfaces the wire failure (#6379)", () => { const home = createTempHome("nemoclaw-mcp-resolution-single-"); const { stdout } = runHarness( diff --git a/src/lib/actions/sandbox/policy-channel-list.test.ts b/src/lib/actions/sandbox/policy-channel-list.test.ts index cbd64716204..113367e45e3 100644 --- a/src/lib/actions/sandbox/policy-channel-list.test.ts +++ b/src/lib/actions/sandbox/policy-channel-list.test.ts @@ -15,6 +15,8 @@ const moduleMocks = vi.hoisted(() => ({ listCustomPresets: vi.fn<(sandboxName: string) => PresetInfo[]>(), getAppliedPresets: vi.fn<(sandboxName: string) => string[]>(), getGatewayPresets: vi.fn<(sandboxName: string) => string[] | null>(), + isDockerRuntimeDown: vi.fn<(sandboxName: string) => boolean>(), + printDockerRuntimeDownGuidance: vi.fn(), })); vi.mock("../../state/registry", async (importOriginal) => ({ @@ -31,6 +33,12 @@ vi.mock("../../policy", async (importOriginal) => ({ getGatewayPresets: moduleMocks.getGatewayPresets, })); +vi.mock("./gateway-failure-classifier", async (importOriginal) => ({ + ...(await importOriginal()), + isDockerRuntimeDown: moduleMocks.isDockerRuntimeDown, + printDockerRuntimeDownGuidance: moduleMocks.printDockerRuntimeDownGuidance, +})); + import { listSandboxPolicies } from "./policy-channel"; const POLICY_PRESETS: PresetInfo[] = [ @@ -78,6 +86,7 @@ beforeEach(() => { moduleMocks.getCustomPolicies.mockReturnValue([]); moduleMocks.listPresets.mockReturnValue(POLICY_PRESETS); moduleMocks.listCustomPresets.mockReturnValue([]); + moduleMocks.isDockerRuntimeDown.mockReturnValue(false); }); afterEach(() => { diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index c2d1ef88df7..5a8765c7006 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -4,7 +4,6 @@ // Interactive onboarding wizard — 8 steps from zero to running sandbox. // Supports non-interactive mode via --non-interactive flag or // NEMOCLAW_NON_INTERACTIVE=1 env var for CI/CD pipelines. - const { envInt, LOCAL_INFERENCE_TIMEOUT_SECS, @@ -166,7 +165,6 @@ const fs = require("fs"); const os = require("os"); const path = require("path"); 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"); @@ -2300,7 +2298,6 @@ async function createSandboxWithBaseImageResolution( preparedBuildContext: PreparedSandboxBuildContext | null = null, ) { step(6, 8, "Creating sandbox"); - const sandboxName = validateName( sandboxNameOverride ?? (await promptValidatedSandboxName(agent)), "sandbox name", @@ -2334,7 +2331,6 @@ async function createSandboxWithBaseImageResolution( getApiForwardPort: () => getDashboardForwardPort(chatUiUrl), }); const hermesDashboardState = hermesDashboardForwarding.resolveStateForPort(effectivePort); - const { messagingTokenDefs, extraPlaceholderKeys, @@ -2746,7 +2742,8 @@ async function createSandboxWithBaseImageResolution( messagingTokenDefs, reusableMessagingChannels, reusableMessagingProviders, - extraProviders: reconcileRegisteredExtraProviders(GATEWAY_NAME, { runOpenshell }), + // biome-ignore format: keep src/lib/onboard.ts net-neutral for growth guardrail. + extraProviders: createIntent?.extraProviders ?? reconcileRegisteredExtraProviders(GATEWAY_NAME, { runOpenshell }), hermesToolGateways, sandboxGpuConfig: effectiveSandboxGpuConfig, dockerDriverGateway, @@ -4510,6 +4507,7 @@ async function runOnboard(opts: OnboardOptions = {}): Promise { requestedDcodeAutoApprovalMode: runtimeControlRequests.requestedDcodeAutoApprovalMode, authoritativePolicyTier: opts.authoritativeResumeConfig === true ? (opts.policyTier ?? null) : null, + recreateSandbox: isRecreateSandbox, controlUiPort: _preflightDashboardPort, rootDir: ROOT, }, @@ -4551,6 +4549,8 @@ async function runOnboard(opts: OnboardOptions = {}): Promise { selectResourceProfileForSandbox({ isNonInteractive, note, prompt, promptOrDefault }), stopStaleDashboardListenersForSandbox, listRegistrySandboxes: registry.listSandboxes, + reconcileRegisteredExtraProviders: (gatewayName) => + reconcileRegisteredExtraProviders(gatewayName, { runOpenshell }), createSandbox: preparedDcodeRuntime.bindCreateSandbox( createSandboxWithBaseImageResolution.bind(null, baseImageResolutionContext), ), diff --git a/src/lib/onboard/extra-provider-diagnostic-parser.ts b/src/lib/onboard/extra-provider-diagnostic-parser.ts new file mode 100644 index 00000000000..bf8d3ff06cb --- /dev/null +++ b/src/lib/onboard/extra-provider-diagnostic-parser.ts @@ -0,0 +1,237 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +const DIAGNOSTIC_PREFIXES = ["error:", "rpc error:", "status:"]; +const NOT_FOUND_SUFFIXES = new Set([ + "not found", + "notfound", + "is not found", + "is notfound", + "was not found", + "was notfound", +]); + +// Parser helpers deliberately default to null/false for malformed or unknown +// diagnostics. The caller treats that as `ambiguous-diagnostic`, preserving the +// provider and emitting one redacted aggregate warning for observability. +function stripAnsi(value: string): string { + return value.replace(/\x1B\[[0-?]*[ -/]*[@-~]/gu, ""); +} + +function stripIssueMarker(text: string): string { + const trimmed = text.trimStart(); + return trimmed.startsWith("×") ? trimmed.slice(1).trimStart() : trimmed; +} + +function stripIssueDecoration(line: string): string { + const trimmed = stripAnsi(line).trim(); + const withoutPipe = trimmed.startsWith("│") ? trimmed.slice(1).trimStart() : trimmed; + return stripIssueMarker(withoutPipe); +} + +function joinDiagnosticLines(lines: string[]): string { + return lines + .reduce((message, line) => { + const part = line.trim(); + if (!part) return message; + return message.endsWith("-") ? `${message}${part}` : `${message} ${part}`; + }, "") + .trim(); +} + +function stripDiagnosticPrefixes(line: string): string { + let text = stripIssueDecoration(line); + for (let attempt = 0; attempt < 10; attempt += 1) { + const lower = text.toLowerCase(); + const prefix = DIAGNOSTIC_PREFIXES.find((candidate) => lower.startsWith(candidate)); + if (!prefix) return text; + text = stripIssueMarker(text.slice(prefix.length)); + } + return text; +} + +function readQuotedValue(text: string, searchStart = 0): { value: string; end: number } | null { + const quoteIndex = ["'", '"', "`"] + .map((quote) => ({ quote, index: text.indexOf(quote, searchStart) })) + .filter(({ index }) => index >= 0) + .sort((left, right) => left.index - right.index)[0]; + if (!quoteIndex) return null; + const end = text.indexOf(quoteIndex.quote, quoteIndex.index + 1); + return end >= 0 ? { value: text.slice(quoteIndex.index + 1, end), end: end + 1 } : null; +} + +function lineReportsMissingGateway(line: string): boolean { + const lower = line.replace(/'[^']*'|"[^"]*"|`[^`]*`/gu, "").toLowerCase(); + return ( + lower.includes("unknown gateway") || + lower.includes("no such gateway") || + lower.includes("notfound: gateway") || + (lower.includes("gateway") && + (lower.includes("does not exist") || + lower.includes("not found") || + lower.includes("notfound"))) + ); +} + +function structuredStatusValue(line: string): string | null { + const lower = line.toLowerCase(); + for (const key of ["status", "code"]) { + const keyIndex = lower.indexOf(key); + if (keyIndex < 0) continue; + let cursor = keyIndex + key.length; + while (/\s/u.test(line[cursor] ?? "")) cursor += 1; + if (line[cursor] !== ":" && line[cursor] !== "=") continue; + cursor += 1; + while (/[\s"']/u.test(line[cursor] ?? "")) cursor += 1; + if (line.slice(cursor).toLowerCase().startsWith("some requested entity was not found")) { + return "notfound"; + } + const start = cursor; + while (/[a-z_-]/iu.test(line[cursor] ?? "")) cursor += 1; + return line.slice(start, cursor); + } + return null; +} + +function normalizeStatus(value: string): string { + return value.replaceAll("_", "").replaceAll("-", "").toLowerCase(); +} + +function normalizedNotFoundSuffix(value: string): string { + return value + .replace(/[.!]+$/u, "") + .trim() + .toLowerCase(); +} + +function providerNameFromNotFoundLine(line: string): string | null { + return ( + providerNameFromNotFoundText(stripDiagnosticPrefixes(line)) ?? providerNameFromMessage(line) + ); +} + +function providerNameFromNotFoundText(text: string): string | null { + let hasNotFoundStatusPrefix = false; + if (text.toLowerCase().startsWith("notfound:")) { + text = text.slice("notfound:".length).trimStart(); + hasNotFoundStatusPrefix = true; + } + const providerPrefix = "provider "; + if (!text.toLowerCase().startsWith(providerPrefix)) return null; + const quoted = readQuotedValue(text, providerPrefix.length); + if (!quoted) return null; + const suffix = normalizedNotFoundSuffix(text.slice(quoted.end)); + return suffix === "" && hasNotFoundStatusPrefix + ? quoted.value + : NOT_FOUND_SUFFIXES.has(suffix) + ? quoted.value + : null; +} + +function providerNameFromMessage(line: string): string | null { + const text = stripDiagnosticPrefixes(line); + const markerIndex = text.toLowerCase().indexOf("message:"); + if (markerIndex < 0) return null; + let cursor = markerIndex + "message:".length; + while (/\s/u.test(text[cursor] ?? "")) cursor += 1; + const quote = text[cursor]; + if (quote === "'" || quote === '"' || quote === "`") { + const end = text.indexOf(quote, cursor + 1); + if (end < 0) return null; + return providerNameFromNotFoundText(text.slice(cursor + 1, end)); + } + return providerNameFromNotFoundText(text.slice(cursor)); +} + +function readMessageValue(line: string): string | null { + const text = stripDiagnosticPrefixes(line); + const markerIndex = text.toLowerCase().indexOf("message:"); + if (markerIndex < 0) return null; + const quoted = readQuotedValue(text, markerIndex + "message:".length); + if (quoted) return quoted.value; + return text.slice(markerIndex + "message:".length).trim(); +} + +function lineReportsTargetedProviderGetNotFound(line: string): boolean { + const text = stripDiagnosticPrefixes(line); + if (normalizedNotFoundSuffix(text) === "provider not found") return true; + const status = structuredStatusValue(line); + if (!status || normalizeStatus(status) !== "notfound") return false; + return normalizedNotFoundSuffix(readMessageValue(line) ?? "") === "provider not found"; +} + +function commandNameAfterMarker(text: string, marker: string): string | null { + const markerIndex = text.toLowerCase().indexOf(marker.toLowerCase()); + if (markerIndex < 0) return null; + let cursor = markerIndex + marker.length; + while (/\s/u.test(text[cursor] ?? "")) cursor += 1; + const start = cursor; + while (cursor < text.length && !/[\s`]/u.test(text[cursor] ?? "")) cursor += 1; + return cursor > start ? text.slice(start, cursor) : null; +} + +function wrappedIssueDiagnosticMatches( + issueDiagnostic: string, + providerName: string, +): boolean | null { + const text = stripDiagnosticPrefixes(issueDiagnostic).replace(/^\s*×\s*/u, ""); + const lower = text.toLowerCase(); + const providerIndex = lower.indexOf("provider "); + const hasWrappedIssueShape = + providerIndex >= 0 && + lower.includes(" not found and ") && + lower.includes(" is not a recognized provider type"); + if (!hasWrappedIssueShape) return null; + + const firstProvider = readQuotedValue(text, providerIndex); + const secondProvider = firstProvider + ? readQuotedValue(text, lower.indexOf(" and ", firstProvider.end)) + : null; + const commandProvider = commandNameAfterMarker(text, "--name "); + return ( + firstProvider?.value === providerName && + secondProvider?.value === providerName && + (commandProvider === null || commandProvider === providerName) + ); +} + +/** + * Accept only diagnostics that bind "not found" to this exact quoted provider. + * + * OpenShell currently renders both `provider 'name' not found` and the gRPC + * ordering `NotFound: provider "name"`. Keeping these shapes narrow matters: + * gateway failures can mention the provider being queried, but must remain + * indeterminate so onboarding does not silently drop a healthy attachment. + */ +export function reportsExactProviderNotFound( + output: string, + providerName: string, + diagnosticLimit: number, +): boolean { + const lines = output.slice(0, diagnosticLimit).split(/\r?\n/); + const diagnosticLines = lines.map(stripIssueDecoration).filter(Boolean); + if (diagnosticLines.length === 0) return false; + if (diagnosticLines.some(lineReportsMissingGateway)) return false; + if ( + diagnosticLines.some((line) => { + const status = structuredStatusValue(line); + return Boolean(status && normalizeStatus(status) !== "notfound"); + }) + ) { + return false; + } + + const wrappedIssueMatch = wrappedIssueDiagnosticMatches( + joinDiagnosticLines(diagnosticLines), + providerName, + ); + if (wrappedIssueMatch !== null) return wrappedIssueMatch; + if ( + diagnosticLines.length === 1 && + lineReportsTargetedProviderGetNotFound(diagnosticLines[0] ?? "") + ) { + return true; + } + + return diagnosticLines.every((line) => providerNameFromNotFoundLine(line) === providerName); +} diff --git a/src/lib/onboard/extra-provider-reconciliation-diagnostics.test.ts b/src/lib/onboard/extra-provider-reconciliation-diagnostics.test.ts new file mode 100644 index 00000000000..79ed8677d69 --- /dev/null +++ b/src/lib/onboard/extra-provider-reconciliation-diagnostics.test.ts @@ -0,0 +1,262 @@ +// 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 { LIMIT, reconcile } from "./extra-provider-reconciliation.test-fixtures"; + +const exactWrappedDiagnostic = [ + "Error: × provider 'stale-provider' not found and 'stale-provider' is not a recognized", + " │ provider type. Create it first with `openshell provider create --type", + " │ --name stale-provider`", +].join("\n"); + +describe("reconcileRegisteredExtraProviders diagnostics", () => { + it.each([ + { + label: "single-quoted CLI", + provider: "stale-provider", + stderr: "Error: provider 'stale-provider' not found", + }, + { + label: "double-quoted gRPC", + provider: "stale-provider", + stderr: 'rpc error: NotFound: provider "stale-provider"', + }, + { + label: "wrapped OpenShell issue diagnostic", + provider: "stale-provider", + stderr: exactWrappedDiagnostic, + }, + { + label: "wrapped OpenShell issue diagnostic with split provider token", + provider: "e2e-stale-extra-provider", + stderr: [ + "Error: × provider 'e2e-stale-extra-provider' not found and 'e2e-stale-extra-", + " │ provider' is not a recognized provider type. Create it first with", + " │ `openshell provider create --type --name e2e-stale-extra-provider`", + ].join("\n"), + }, + { + label: "short OpenShell issue diagnostic", + provider: "e2e-stale-extra-provider", + stderr: "Error: × provider 'e2e-stale-extra-provider' not found", + }, + { + label: "colored OpenShell issue diagnostic", + provider: "colored-provider", + stderr: + "\u001b[1m\u001b[31mError:\u001b[39m\u001b[0m × provider 'colored-provider' not found", + }, + { + label: "wrapped OpenShell provider-get issue diagnostic without a remediation command", + provider: "e2e-stale-extra-provider", + stderr: [ + "Error: × provider 'e2e-stale-extra-provider' not found and 'e2e-stale-extra-", + " │ provider' is not a recognized provider type.", + ].join("\n"), + }, + { + label: "wrapped OpenShell issue diagnostic with split remediation provider token", + provider: "e2e-resume-stale-extra-provider", + stderr: [ + "Error: × provider 'e2e-resume-stale-extra-provider' not found and 'e2e-resume-", + " │ stale-extra-provider' is not a recognized provider type. Create it first", + " │ with `openshell provider create --type --name e2e-resume-stale-", + " │ extra-provider`", + ].join("\n"), + }, + { + label: "wrapped OpenShell provider-get issue diagnostic after runner prefix", + provider: "e2e-stale-extra-provider", + stderr: [ + "OpenShell command failed while probing provider:", + "Error: × provider 'e2e-stale-extra-provider' not found and 'e2e-stale-extra-", + " │ provider' is not a recognized provider type. Create it first with", + " │ `openshell provider create --type --name e2e-stale-extra-provider`", + ].join("\n"), + }, + { + label: "OpenShell issue diagnostic with structured not-found code and message", + provider: "stale-provider", + stderr: + "Error: × code: 'Some requested entity was not found', message: \"provider 'stale-provider' not found\"", + }, + { + label: "OpenShell provider-get generic not-found diagnostic", + provider: "stale-provider", + stderr: + "Error: × code: 'Some requested entity was not found', message: \"provider not found\"", + }, + { + label: "short targeted provider-get generic not-found diagnostic", + provider: "stale-provider", + stderr: "Error: provider not found", + }, + { + label: "provider name containing gateway", + provider: "my-gateway-provider", + stderr: "Error: provider 'my-gateway-provider' not found", + }, + ])("accepts exact $label not-found diagnostics (#6501)", ({ provider, stderr }) => { + expect(reconcile([provider], { [provider]: { status: 1, stderr } })).toEqual([]); + }); + + it.each([ + [ + "mismatched wrapped provider name", + "Error: × provider 'stale-provider' not found and 'other-provider' is not a recognized provider type. Create it first with `openshell provider create --type --name stale-provider`", + ], + [ + "mismatched wrapped create command name", + "Error: × provider 'stale-provider' not found and 'stale-provider' is not a recognized provider type. Create it first with `openshell provider create --type --name other-provider`", + ], + [ + "gateway missing", + "Error: gateway 'nemoclaw' not found while checking provider 'stale-provider'", + ], + [ + "gateway and provider missing", + "Error: gateway 'nemoclaw' not found; provider 'stale-provider' not found", + ], + [ + "structured not-found code but gateway message", + "Error: × code: 'Some requested entity was not found', message: \"gateway 'nemoclaw' not found while checking provider 'stale-provider'\"", + ], + ["transport plus provider text", "transport error\nError: provider 'stale-provider' not found"], + ["transport plus generic provider text", "transport error\nError: provider not found"], + [ + "conflicting structured status", + "Error: status: Unavailable, message: \"provider 'stale-provider' not found\"", + ], + ])("preserves providers for ambiguous diagnostics: %s (#6501)", (_label, stderr) => { + expect( + reconcile(["stale-provider"], { + "stale-provider": { status: 1, stderr }, + }), + ).toEqual(["stale-provider"]); + }); + + it("uses composite output only when stderr and stdout are empty (#6501)", () => { + const diagnostic = Buffer.from("Error: provider 'stale-provider' not found"); + + expect( + reconcile(["stale-provider"], { + "stale-provider": { + status: 1, + output: [null, Buffer.alloc(0), diagnostic], + stderr: Buffer.alloc(0), + stdout: Buffer.alloc(0), + }, + }), + ).toEqual([]); + }); + + it("bounds diagnostics before parsing and warns without leaking provider names (#6501)", () => { + const warn = vi.fn(); + const recorded = ["at-limit-provider", "ambiguous-provider"]; + + expect( + reconcile( + recorded, + { + "at-limit-provider": { + status: 1, + stderr: Buffer.from("Error: provider 'at-limit-provider' not found".padEnd(LIMIT, " ")), + }, + "ambiguous-provider": { + status: 1, + stderr: `Error: provider '${"a".repeat(63 * 1024)}`, + }, + }, + { warn }, + ), + ).toEqual(recorded); + expect(warn).toHaveBeenCalledWith( + " Warning: extra-provider reconciliation preserved indeterminate attachments " + + "(providerCount=2; reasonClasses=ambiguous-diagnostic,diagnostic-capture-limit).", + ); + expect(warn.mock.calls[0]?.[0]).not.toContain("at-limit-provider"); + expect(warn.mock.calls[0]?.[0]).not.toContain("ambiguous-provider"); + }); + + it("redacts exact diagnostic provider names from warnings (#6501)", () => { + const warn = vi.fn(); + + expect( + reconcile( + ["exact-provider", "named-ambiguous-provider"], + { + "exact-provider": { + status: 1, + stderr: "Error: provider 'exact-provider' not found", + }, + "named-ambiguous-provider": { + status: 1, + stderr: + "Error: status: Unavailable, message: \"provider 'named-ambiguous-provider' not found\"", + }, + }, + { warn }, + ), + ).toEqual(["named-ambiguous-provider"]); + expect(warn).toHaveBeenCalledWith( + " Warning: extra-provider reconciliation preserved indeterminate attachments " + + "(providerCount=1; reasonClasses=ambiguous-diagnostic).", + ); + expect(warn.mock.calls[0]?.[0]).not.toContain("exact-provider"); + expect(warn.mock.calls[0]?.[0]).not.toContain("named-ambiguous-provider"); + }); + + it("keeps branch priority deterministic for conflicting diagnostics (#6501)", () => { + expect( + reconcile(["stale-provider"], { + "stale-provider": { + status: 1, + stderr: [ + "Error: gateway 'nemoclaw' not found", + "Error: provider 'stale-provider' not found", + ].join("\n"), + }, + }), + ).toEqual(["stale-provider"]); + expect( + reconcile(["stale-provider"], { + "stale-provider": { + status: 1, + stderr: [ + "Error: code: Unavailable", + 'rpc error: NotFound: provider "stale-provider"', + ].join("\n"), + }, + }), + ).toEqual(["stale-provider"]); + }); + + it("preserves providers when the not-found diagnostic uses different casing (#6501)", () => { + expect( + reconcile(["tavily-search"], { + "tavily-search": { + status: 1, + stderr: "Error: provider 'Tavily-Search' not found", + }, + }), + ).toEqual(["tavily-search"]); + }); + + it("parses adversarial diagnostics within a bounded budget (#6501)", () => { + const adversarial = [ + `${"error: ".repeat(2_000)}provider 'redos-provider' not found`, + `Error: provider '${"a".repeat(8_000)}`, + `${"gateway ".repeat(1_000)}provider 'redos-provider' not found`, + `status: unavailable ${"provider 'redos-provider' not found ".repeat(1_000)}`, + ].join("\n"); + const started = performance.now(); + + expect( + reconcile(["redos-provider"], { + "redos-provider": { status: 1, stderr: adversarial }, + }), + ).toEqual(["redos-provider"]); + expect(performance.now() - started).toBeLessThan(100); + }); +}); diff --git a/src/lib/onboard/extra-provider-reconciliation-probes.test.ts b/src/lib/onboard/extra-provider-reconciliation-probes.test.ts new file mode 100644 index 00000000000..07a2d087d13 --- /dev/null +++ b/src/lib/onboard/extra-provider-reconciliation-probes.test.ts @@ -0,0 +1,107 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { afterEach, describe, expect, it, vi } from "vitest"; +import { reconcileRegisteredExtraProviders } from "./extra-provider-reconciliation"; +import { + missing, + ok, + type ProbeResult, + reconcile, +} from "./extra-provider-reconciliation.test-fixtures"; + +afterEach(() => { + vi.unstubAllEnvs(); +}); + +describe("reconcileRegisteredExtraProviders probe outcomes", () => { + it("preserves providers for thrown, timed-out, process-error, and nonstandard probes (#6501)", () => { + const warn = vi.fn(); + const recorded = [ + "thrown-provider", + "timed-out-provider", + "nonstandard-exit-provider", + "buffer-error-provider", + ]; + + expect( + reconcile( + recorded, + { + "thrown-provider": () => { + throw new Error("gateway process unavailable"); + }, + "timed-out-provider": { + status: null, + stderr: missing("timed-out-provider").stderr, + }, + "nonstandard-exit-provider": { + status: 7, + stderr: missing("nonstandard-exit-provider").stderr, + }, + "buffer-error-provider": { + status: 1, + error: new Error("spawnSync ENOBUFS"), + stderr: missing("buffer-error-provider").stderr, + }, + }, + { warn }, + ), + ).toEqual(recorded); + expect(warn).toHaveBeenCalledWith( + " Warning: extra-provider reconciliation preserved indeterminate attachments " + + "(providerCount=4; reasonClasses=probe-process-error,probe-threw,timeout-or-signal,unexpected-exit).", + ); + }); + + it("bounds aggregate probe latency and preserves names left after the deadline (#6501)", () => { + let now = 0; + const timeouts: number[] = []; + const warn = vi.fn(); + const runOpenshell = vi.fn((_args: string[], options?: Record) => { + const timeout = Number(options?.timeout); + timeouts.push(timeout); + now += timeout; + return { status: null, stderr: "provider process timed out" }; + }); + const recorded = ["provider-1", "provider-2", "provider-3", "provider-4", "provider-5"]; + + expect( + reconcileRegisteredExtraProviders("nemoclaw", { + listExtraProviders: () => [...recorded], + nowMs: () => now, + removeExtraProvider: () => true, + runOpenshell, + warn, + }), + ).toEqual(recorded); + expect(runOpenshell).toHaveBeenCalledTimes(3); + expect(timeouts).toEqual([5_000, 5_000, 5_000]); + expect(warn).toHaveBeenCalledWith( + " Warning: extra-provider reconciliation preserved indeterminate attachments " + + "(providerCount=5; reasonClasses=aggregate-time-budget,timeout-or-signal).", + ); + }); + + it("enforces gateway containment and requires a gateway name before probing (#6501)", () => { + const runOpenshell = vi.fn((): ProbeResult => ok()); + vi.stubEnv("OPENSHELL_GATEWAY_ENDPOINT", "https://other.example.test"); + + expect(() => + reconcileRegisteredExtraProviders("nemoclaw", { + listExtraProviders: () => ["custom-provider"], + removeExtraProvider: () => true, + runOpenshell, + }), + ).toThrow(/OPENSHELL_GATEWAY_ENDPOINT is set/); + vi.unstubAllEnvs(); + expect(() => + reconcileRegisteredExtraProviders("", { + listExtraProviders: () => ["custom-provider"], + removeExtraProvider: () => true, + runOpenshell, + }), + ).toThrow("OpenShell gateway name is required."); + expect(runOpenshell).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/onboard/extra-provider-reconciliation.test-fixtures.ts b/src/lib/onboard/extra-provider-reconciliation.test-fixtures.ts new file mode 100644 index 00000000000..d16e2194846 --- /dev/null +++ b/src/lib/onboard/extra-provider-reconciliation.test-fixtures.ts @@ -0,0 +1,39 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { vi } from "vitest"; +import { reconcileRegisteredExtraProviders } from "./extra-provider-reconciliation"; + +export type ProbeResult = { + status: number | null; + error?: Error; + output?: unknown; + stdout?: unknown; + stderr?: unknown; +}; + +export const LIMIT = 64 * 1024; + +export const ok = (): ProbeResult => ({ status: 0, stdout: "" }); + +export const missing = (name: string): ProbeResult => ({ + status: 1, + stderr: `Error: provider '${name}' not found`, +}); + +export function reconcile( + recorded: string[], + responses: Record ProbeResult)> = {}, + extra: Partial[1]> = {}, +): string[] { + return reconcileRegisteredExtraProviders("nemoclaw", { + listExtraProviders: () => [...recorded], + removeExtraProvider: () => true, + runOpenshell: vi.fn((args: string[]): ProbeResult => { + const response = responses[args.at(-1) ?? ""]; + return typeof response === "function" ? response() : (response ?? ok()); + }), + warn: () => undefined, + ...extra, + }); +} diff --git a/src/lib/onboard/extra-provider-reconciliation.test.ts b/src/lib/onboard/extra-provider-reconciliation.test.ts index 397d0671757..6c5aa77e7b5 100644 --- a/src/lib/onboard/extra-provider-reconciliation.test.ts +++ b/src/lib/onboard/extra-provider-reconciliation.test.ts @@ -1,94 +1,76 @@ // 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 { afterEach, describe, expect, it, vi } from "vitest"; import { reconcileRegisteredExtraProviders } from "./extra-provider-reconciliation"; - -type RunResult = { status: number | null; stderr?: string; stdout?: string | Buffer }; +import { + LIMIT, + missing, + ok, + type ProbeResult, + reconcile, +} from "./extra-provider-reconciliation.test-fixtures"; + +afterEach(() => { + vi.unstubAllEnvs(); +}); describe("reconcileRegisteredExtraProviders", () => { - it("keeps every user-owned extra whose exact name appears in one scoped gateway list", () => { - const runOpenshell = vi.fn( - (_args: string[]): RunResult => ({ - status: 0, - stdout: Buffer.from( - " tavily-search\nbrave-search\ncustom-provider\nmy-slack-bridge\nunrelated\n", - ), - }), - ); - const recorded = ["tavily-search", "brave-search", "custom-provider", "my-slack-bridge"]; - - const result = reconcileRegisteredExtraProviders("nemoclaw", { - runOpenshell, - listExtraProviders: () => recorded, - }); - - expect(result).toEqual(recorded); - expect(runOpenshell).toHaveBeenCalledOnce(); - expect(runOpenshell).toHaveBeenCalledWith(["provider", "list", "-g", "nemoclaw", "--names"], { - ignoreError: true, - stdio: ["ignore", "pipe", "pipe"], - suppressOutput: true, - }); - }); - - it("omits a stale tavily record only from the current create plan (#6501)", () => { - const recorded = ["tavily-search", "custom-provider"]; + it("skips gateway probes when no extra provider is recorded (#6501)", () => { + const runOpenshell = vi.fn((): ProbeResult => ok()); - const result = reconcileRegisteredExtraProviders("nemoclaw", { - runOpenshell: vi.fn(() => ({ status: 0, stdout: "custom-provider\n" })), - listExtraProviders: () => recorded, - }); - - expect(result).toEqual(["custom-provider"]); - expect(recorded).toEqual(["tavily-search", "custom-provider"]); - }); - - it("matches complete names instead of prefixes or provider-name heuristics", () => { - const result = reconcileRegisteredExtraProviders("nemoclaw", { - runOpenshell: vi.fn(() => ({ - status: 0, - stdout: "tavily-search-backup\ncustom-provider-v2\n", - })), - listExtraProviders: () => ["tavily-search", "custom-provider"], - }); - - expect(result).toEqual([]); + expect( + reconcileRegisteredExtraProviders("nemoclaw", { + listExtraProviders: () => [], + runOpenshell, + }), + ).toEqual([]); + expect(runOpenshell).not.toHaveBeenCalled(); }); - it("preserves every recorded extra when the gateway list exits nonzero", () => { - const recorded = ["tavily-search", "brave-search", "custom-provider"]; - - const result = reconcileRegisteredExtraProviders("nemoclaw", { - runOpenshell: vi.fn(() => ({ status: 1, stderr: "gateway unavailable" })), - listExtraProviders: () => recorded, + it("probes every recorded provider exactly and never trusts provider-list snapshots (#6501)", () => { + const recorded = Array.from({ length: 128 }, (_value, index) => `custom-provider-${index}`); + const calls: Array<{ + args: string[]; + options: Record | undefined; + }> = []; + const removeExtraProvider = vi.fn(() => true); + const runOpenshell = vi.fn((args: string[], options?: Record) => { + calls.push({ args, options }); + return args.at(-1) === "custom-provider-127" ? missing("custom-provider-127") : ok(); }); - expect(result).toEqual(recorded); - }); - - it("preserves every recorded extra when the gateway list throws", () => { - const recorded = ["tavily-search", "my-slack-bridge"]; - - const result = reconcileRegisteredExtraProviders("nemoclaw", { - runOpenshell: vi.fn(() => { - throw new Error("spawn failed"); + expect( + reconcileRegisteredExtraProviders("nemoclaw", { + listExtraProviders: () => [...recorded], + removeExtraProvider, + runOpenshell, }), - listExtraProviders: () => recorded, + ).toEqual(recorded.slice(0, -1)); + expect(removeExtraProvider).toHaveBeenCalledWith("custom-provider-127"); + expect(calls).toHaveLength(recorded.length); + expect(calls.some(({ args }) => args.includes("list") || args.includes("--names"))).toBe(false); + expect(calls[0]).toEqual({ + args: ["provider", "get", "-g", "nemoclaw", "custom-provider-0"], + options: { + ignoreError: true, + maxBuffer: LIMIT, + stdio: ["ignore", "pipe", "pipe"], + suppressOutput: true, + timeout: 5_000, + }, }); - - expect(result).toEqual(recorded); }); - it("does not query the gateway when no extras are recorded", () => { - const runOpenshell = vi.fn(); - + it("keeps healthy providers and omits only exact provider-specific not-found diagnostics (#6501)", () => { expect( - reconcileRegisteredExtraProviders("nemoclaw", { - runOpenshell, - listExtraProviders: () => [], + reconcile(["healthy-provider", "stale-provider", "indeterminate-provider"], { + "stale-provider": { + status: 1, + stderr: Buffer.from("Error: provider 'stale-provider' not found\n"), + }, + "indeterminate-provider": missing("some-other-provider"), }), - ).toEqual([]); - expect(runOpenshell).not.toHaveBeenCalled(); + ).toEqual(["healthy-provider", "indeterminate-provider"]); }); }); diff --git a/src/lib/onboard/extra-provider-reconciliation.ts b/src/lib/onboard/extra-provider-reconciliation.ts index 833bb375e37..1f1f89641d0 100644 --- a/src/lib/onboard/extra-provider-reconciliation.ts +++ b/src/lib/onboard/extra-provider-reconciliation.ts @@ -2,29 +2,50 @@ // SPDX-License-Identifier: Apache-2.0 import { assertNoOpenShellGatewayEndpointOverride } from "../openshell-gateway-endpoint-guard"; +import { reportsExactProviderNotFound } from "./extra-provider-diagnostic-parser"; type ExtraProviderRunOpenshell = ( args: string[], opts?: Record, ) => { status: number | null; - stdout?: string | Buffer | null; - stderr?: string | Buffer | null; + error?: Error; + output?: unknown; + stdout?: unknown; + stderr?: unknown; }; export type ReconcileExtraProvidersDeps = { runOpenshell?: ExtraProviderRunOpenshell; listExtraProviders?: () => string[]; + removeExtraProvider?: (name: string) => boolean; + nowMs?: () => number; + warn?: (message: string) => void; }; +type IndeterminateProbeReason = + | "aggregate-time-budget" + | "ambiguous-diagnostic" + | "diagnostic-capture-limit" + | "probe-process-error" + | "probe-threw" + | "timeout-or-signal" + | "unexpected-exit"; + function defaultRunOpenshell( args: string[], opts?: Record, ): ReturnType { const runtime = require("../adapters/openshell/runtime") as { - runOpenshell: ExtraProviderRunOpenshell; + getOpenshellBinary: () => string; + }; + const { run } = require("../runner") as { + run: ( + command: string[], + options?: Record, + ) => ReturnType; }; - return runtime.runOpenshell(args, opts); + return run([runtime.getOpenshellBinary(), ...args], opts); } function defaultListExtraProviders(): string[] { @@ -34,19 +55,101 @@ function defaultListExtraProviders(): string[] { return listExtraProviders(); } -function outputText(value: string | Buffer | null | undefined): string { +function defaultRemoveExtraProvider(name: string): boolean { + const { removeExtraProvider } = require("../state/registry") as { + removeExtraProvider: (name: string) => boolean; + }; + return removeExtraProvider(name); +} + +function outputText(value: unknown): string { if (typeof value === "string") return value; - return value?.toString() ?? ""; + if (Buffer.isBuffer(value)) return value.toString(); + if (Array.isArray(value)) return value.map(outputText).filter(Boolean).join("\n"); + return value === null || value === undefined ? "" : String(value); +} + +const PROVIDER_PROBE_TIMEOUT_MS = 5_000; +const PROVIDER_PROBE_DIAGNOSTIC_LIMIT = 64 * 1024; +const PROVIDER_RECONCILIATION_BUDGET_MS = 15_000; + +function monotonicNowMs(): number { + return Number(process.hrtime.bigint() / 1_000_000n); +} + +type ProviderProbeOutcome = { + keep: boolean; + reason?: IndeterminateProbeReason; +}; + +type ProviderProbeContext = { + gatewayName: string; + name: string; + runOpenshell: ExtraProviderRunOpenshell; + nowMs: () => number; + deadlineMs: number; +}; + +function diagnosticPartsFromProbeResult(result: ReturnType): string[] { + const primaryDiagnosticParts = [result.stderr, result.stdout].map(outputText).filter(Boolean); + return primaryDiagnosticParts.length > 0 + ? primaryDiagnosticParts + : [outputText(result.output)].filter(Boolean); +} + +function probeExtraProvider(context: ProviderProbeContext): ProviderProbeOutcome { + const remainingMs = context.deadlineMs - context.nowMs(); + if (remainingMs <= 0) return { keep: true, reason: "aggregate-time-budget" }; + + let result: ReturnType; + try { + result = context.runOpenshell(["provider", "get", "-g", context.gatewayName, context.name], { + ignoreError: true, + maxBuffer: PROVIDER_PROBE_DIAGNOSTIC_LIMIT, + stdio: ["ignore", "pipe", "pipe"], + suppressOutput: true, + timeout: Math.max(1, Math.min(PROVIDER_PROBE_TIMEOUT_MS, Math.floor(remainingMs))), + }); + } catch { + return { keep: true, reason: "probe-threw" }; + } + if (result.error) return { keep: true, reason: "probe-process-error" }; + if (result.status === 0) return { keep: true }; + // OpenShell CLI command errors use exit 1. A null status means timeout or + // signal termination, while any other exit is outside this diagnostic + // contract; both are indeterminate and must preserve the provider. + if (result.status === null) return { keep: true, reason: "timeout-or-signal" }; + if (result.status !== 1) return { keep: true, reason: "unexpected-exit" }; + + const diagnosticParts = diagnosticPartsFromProbeResult(result); + if (diagnosticParts.some((part) => Buffer.byteLength(part) >= PROVIDER_PROBE_DIAGNOSTIC_LIMIT)) { + return { keep: true, reason: "diagnostic-capture-limit" }; + } + return reportsExactProviderNotFound( + diagnosticParts.join("\n"), + context.name, + PROVIDER_PROBE_DIAGNOSTIC_LIMIT, + ) + ? { keep: false } + : { keep: true, reason: "ambiguous-diagnostic" }; } /** - * Reconcile user-owned registry extras with one authoritative gateway list (#6501). + * Reconcile user-owned registry extras with strict provider-specific probes (#6501). + * + * Each recorded name is checked independently in the selected gateway. Only an + * exact provider-specific not-found diagnostic omits that name from this sandbox + * create and prunes it from the local extra-provider registry, so retries and + * `--fresh` starts no longer inherit the stale attachment. Successful probes and + * every indeterminate outcome (including throws, timeouts, transport failures, + * and missing-gateway diagnostics) preserve the recorded name. Probes share an + * aggregate time budget; any names left after that budget are preserved. Sandbox + * creation is still the final authority if gateway state changes after a probe. + * Indeterminate outcomes emit one aggregate warning containing reason classes + * and a count, never gateway names, provider names, or raw diagnostics. * - * A successful list is safe to filter against because provider names are matched - * exactly; there are no reserved search-provider names or diagnostic heuristics. - * A failed or thrown list preserves every recorded name so an unavailable gateway - * cannot silently change sandbox-create intent. Local registry state is never - * mutated: a provider omitted for this create remains available for later retry. + * Removal condition: delete this defensive prune once OpenShell/NemoClaw gateway + * reset owns extra-provider lifecycle cleanup before sandbox creation (#6501). */ export function reconcileRegisteredExtraProviders( gatewayName: string, @@ -58,23 +161,42 @@ export function reconcileRegisteredExtraProviders( assertNoOpenShellGatewayEndpointOverride(); const runOpenshell = deps.runOpenshell ?? defaultRunOpenshell; - let result: ReturnType; - try { - result = runOpenshell(["provider", "list", "-g", gatewayName, "--names"], { - ignoreError: true, - stdio: ["ignore", "pipe", "pipe"], - suppressOutput: true, + const removeExtraProvider = deps.removeExtraProvider ?? defaultRemoveExtraProvider; + const nowMs = deps.nowMs ?? monotonicNowMs; + const warn = deps.warn ?? ((message: string) => console.warn(message)); + const deadlineMs = nowMs() + PROVIDER_RECONCILIATION_BUDGET_MS; + const indeterminateReasons = new Set(); + let indeterminateProviderCount = 0; + + const recordIndeterminate = (reason: IndeterminateProbeReason): void => { + indeterminateReasons.add(reason); + indeterminateProviderCount += 1; + }; + + const reconciled: string[] = []; + for (const name of recorded) { + const outcome = probeExtraProvider({ + gatewayName, + name, + runOpenshell, + nowMs, + deadlineMs, }); - } catch { - return recorded; + if (outcome.reason) recordIndeterminate(outcome.reason); + if (outcome.keep) { + reconciled.push(name); + } else { + removeExtraProvider(name); + } } - if (result.status !== 0) return recorded; - - const gatewayNames = new Set( - outputText(result.stdout) - .split("\n") - .map((name) => name.trim()) - .filter(Boolean), - ); - return recorded.filter((name) => gatewayNames.has(name)); + + if (indeterminateProviderCount > 0) { + warn( + " Warning: extra-provider reconciliation preserved indeterminate attachments " + + `(providerCount=${indeterminateProviderCount}; ` + + `reasonClasses=${[...indeterminateReasons].sort().join(",")}).`, + ); + } + + return reconciled; } diff --git a/src/lib/onboard/gateway-recovery.test.ts b/src/lib/onboard/gateway-recovery.test.ts index d31985fae1e..3ca4d02651c 100644 --- a/src/lib/onboard/gateway-recovery.test.ts +++ b/src/lib/onboard/gateway-recovery.test.ts @@ -40,6 +40,7 @@ function createDeps(overrides: Partial = {}): GatewayRecove startGatewayWithOptions: vi.fn( async () => undefined, ) as GatewayRecoveryDeps["startGatewayWithOptions"], + shouldPatchCoredns: () => false, // Tests assert the plain-CLI fallback path by default; the Linux // Docker-driver branch is opted into explicitly per case. isLinuxDockerDriverGatewayEnabled: () => false, diff --git a/src/lib/onboard/gateway-recovery.ts b/src/lib/onboard/gateway-recovery.ts index c51442bf15f..fbec2303249 100644 --- a/src/lib/onboard/gateway-recovery.ts +++ b/src/lib/onboard/gateway-recovery.ts @@ -60,6 +60,9 @@ export type GatewayRecoveryDeps = { // to the production implementations. isGatewayHealthy?: typeof isGatewayHealthy; isGatewayHttpReady?: typeof isGatewayHttpReady; + getContainerRuntime?: typeof getContainerRuntime; + shouldPatchCoredns?: typeof shouldPatchCoredns; + runCorednsPatch?(gatewayName: string): void; // Injected clock reader for deadline-driven tests. Defaults to Date.now. // A test can pair a virtual sleeper (that advances a captured value) with // this reader to drive deterministic deadline expiration without real @@ -231,11 +234,15 @@ async function startTargetGatewayForRecovery( if (healthy) { process.env.OPENSHELL_GATEWAY = gatewayName; - const runtime = getContainerRuntime(); - if (shouldPatchCoredns(runtime)) { - run(["bash", path.join(SCRIPTS, "fix-coredns.sh"), gatewayName], { - ignoreError: true, - }); + const runtime = (deps.getContainerRuntime ?? getContainerRuntime)(); + if ((deps.shouldPatchCoredns ?? shouldPatchCoredns)(runtime)) { + const runCorednsPatch = + deps.runCorednsPatch ?? + ((targetGatewayName: string) => + run(["bash", path.join(SCRIPTS, "fix-coredns.sh"), targetGatewayName], { + ignoreError: true, + })); + runCorednsPatch(gatewayName); } return; } diff --git a/src/lib/onboard/machine/core-flow-phases.test.ts b/src/lib/onboard/machine/core-flow-phases.test.ts index c3efd474638..7c7728d2b94 100644 --- a/src/lib/onboard/machine/core-flow-phases.test.ts +++ b/src/lib/onboard/machine/core-flow-phases.test.ts @@ -160,6 +160,7 @@ function createPhases( }, sandbox: { resumeAgentChanged: false, + recreateSandbox: () => false, controlUiPort: null, rootDir: "/repo", }, @@ -205,6 +206,7 @@ function createPhases( selectResourceProfileForSandbox: vi.fn(async () => null), stopStaleDashboardListenersForSandbox: vi.fn(), listRegistrySandboxes: () => ({ sandboxes: [] }), + reconcileRegisteredExtraProviders: vi.fn(() => []), createSandbox: vi.fn(async () => "created-sandbox"), updateSandboxRegistry: vi.fn(), getSandboxAgentRegistryFields: () => ({ agent: "openclaw" }), diff --git a/src/lib/onboard/machine/core-flow-phases.ts b/src/lib/onboard/machine/core-flow-phases.ts index 957807b4d3d..0f62a83fda1 100644 --- a/src/lib/onboard/machine/core-flow-phases.ts +++ b/src/lib/onboard/machine/core-flow-phases.ts @@ -38,6 +38,7 @@ export interface CoreOnboardFlowPhaseOptions< requestedObservabilityEnabled?: boolean | null; requestedDcodeAutoApprovalMode?: DcodeAutoApprovalMode | null; authoritativePolicyTier?: string | null; + recreateSandbox: (requested?: boolean) => boolean; controlUiPort: number | null; rootDir: string; }; @@ -118,6 +119,7 @@ export function createCoreOnboardFlowPhases< resumeAgentChanged: options.sandbox.resumeAgentChanged, requestedObservabilityEnabled: options.sandbox.requestedObservabilityEnabled, requestedDcodeAutoApprovalMode: options.sandbox.requestedDcodeAutoApprovalMode, + recreateSandbox: options.sandbox.recreateSandbox, session: context.session, sandboxName: context.sandboxName, model: context.model, diff --git a/src/lib/onboard/machine/handlers/sandbox-dcode-selection.test.ts b/src/lib/onboard/machine/handlers/sandbox-dcode-selection.test.ts index cb2b51ba130..b95c4d4a8e6 100644 --- a/src/lib/onboard/machine/handlers/sandbox-dcode-selection.test.ts +++ b/src/lib/onboard/machine/handlers/sandbox-dcode-selection.test.ts @@ -71,6 +71,7 @@ describe("handleSandboxState live DCode selection", () => { observabilityEnabled: true, observabilityRequestedExplicitly: true, dcodeAutoApprovalMode: "disabled", + extraProviders: [], }); }); @@ -188,6 +189,7 @@ describe("handleSandboxState live DCode selection", () => { toolDisclosure: "progressive", observabilityEnabled: false, dcodeAutoApprovalMode: "disabled", + extraProviders: [], }); expect(calls.removeSandbox).not.toHaveBeenCalled(); }); @@ -208,6 +210,7 @@ describe("handleSandboxState live DCode selection", () => { toolDisclosure: "progressive", observabilityEnabled: false, dcodeAutoApprovalMode: "disabled", + extraProviders: [], }); }); diff --git a/src/lib/onboard/machine/handlers/sandbox-recreate-resume.test.ts b/src/lib/onboard/machine/handlers/sandbox-recreate-resume.test.ts new file mode 100644 index 00000000000..4a0aac95de5 --- /dev/null +++ b/src/lib/onboard/machine/handlers/sandbox-recreate-resume.test.ts @@ -0,0 +1,86 @@ +// 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 { createSession } from "../../../state/onboard-session"; +import { handleSandboxState } from "./sandbox"; +import { baseOptions, createDeps, makeMinimalPlan } from "./sandbox-test-fixtures"; + +vi.mock("../../messaging-channel-setup", () => ({ + detectMessagingChannelsFromEnv: vi.fn(() => []), +})); + +describe("handleSandboxState resume recreation", () => { + it("honors explicit recreate requests for completed ready sandboxes", async () => { + const session = createSession({ + sandboxName: "saved", + messagingPlan: makeMinimalPlan("saved", "openclaw", ["slack"]), + }); + session.steps.sandbox.status = "complete"; + const { deps, calls } = createDeps({ + getSandboxReuseState: () => "ready", + reconcileRegisteredExtraProviders: vi.fn(() => ["healthy-extra-provider"]), + getSandboxRegistryEntry: () => ({ + name: "saved", + provider: "provider", + model: "model", + endpointUrl: null, + preferredInferenceApi: "openai-completions", + toolDisclosure: "progressive", + fromDockerfile: null, + hermesAuthMethod: null, + }), + }); + calls.createSandbox.mockResolvedValue("saved"); + + const result = await handleSandboxState({ + ...baseOptions(deps, session), + resume: true, + sandboxName: "saved", + recreateSandbox: () => true, + }); + + expect(calls.skipped).not.toHaveBeenCalled(); + expect(calls.note).toHaveBeenCalledWith( + " [resume] Recreate sandbox requested; recreating sandbox.", + ); + expect(deps.reconcileRegisteredExtraProviders).toHaveBeenCalledWith("nemoclaw"); + expect(calls.removeSandbox).not.toHaveBeenCalled(); + expect(calls.createSandbox).toHaveBeenCalledTimes(1); + const createSandboxCall = calls.createSandbox.mock.calls[0] as unknown[]; + expect(createSandboxCall[4]).toBe("saved"); + expect(createSandboxCall[14]).toMatchObject({ + extraProviders: ["healthy-extra-provider"], + recreate: true, + }); + expect(result.sandboxName).toBe("saved"); + }); + + it("passes an authoritative empty extra-provider list after reconciliation prunes stale names", async () => { + const session = createSession({ + sandboxName: "saved", + messagingPlan: makeMinimalPlan("saved", "openclaw", ["slack"]), + }); + session.steps.sandbox.status = "complete"; + const { deps, calls } = createDeps({ + getSandboxReuseState: () => "missing", + reconcileRegisteredExtraProviders: vi.fn(() => []), + }); + calls.createSandbox.mockResolvedValue("saved"); + + await handleSandboxState({ + ...baseOptions(deps, session), + resume: true, + sandboxName: "saved", + }); + + expect(deps.reconcileRegisteredExtraProviders).toHaveBeenCalledWith("nemoclaw"); + expect(calls.createSandbox).toHaveBeenCalledTimes(1); + const createSandboxCall = calls.createSandbox.mock.calls[0] as unknown[]; + expect(createSandboxCall[14]).toMatchObject({ + extraProviders: [], + recreate: true, + }); + }); +}); diff --git a/src/lib/onboard/machine/handlers/sandbox-resume.test.ts b/src/lib/onboard/machine/handlers/sandbox-resume.test.ts index 28a98810171..28fbc3c49b4 100644 --- a/src/lib/onboard/machine/handlers/sandbox-resume.test.ts +++ b/src/lib/onboard/machine/handlers/sandbox-resume.test.ts @@ -18,6 +18,7 @@ function resumeSignals(overrides: Partial = {}): SandboxRe inferenceRouteConfigChanged: false, webSearchConfigChanged: false, sandboxGpuConfigChanged: false, + recreateSandboxRequested: false, messagingChannelConfigChanged: false, hermesToolGatewayConfigChanged: false, toolDisclosureMigrationNeeded: false, @@ -35,6 +36,7 @@ describe("decideSandboxResume", () => { it.each([ ["agent", { resumeAgentChanged: true }, false], ["web search", { webSearchConfigChanged: true }, true], + ["explicit recreate", { recreateSandboxRequested: true }, false], ["sandbox GPU", { sandboxGpuConfigChanged: true }, true], ["messaging", { messagingChannelConfigChanged: true }, true], ["Hermes tool gateway", { hermesToolGatewayConfigChanged: true }, true], diff --git a/src/lib/onboard/machine/handlers/sandbox-resume.ts b/src/lib/onboard/machine/handlers/sandbox-resume.ts index 73d95259294..f6ff02d3c05 100644 --- a/src/lib/onboard/machine/handlers/sandbox-resume.ts +++ b/src/lib/onboard/machine/handlers/sandbox-resume.ts @@ -19,6 +19,7 @@ export interface SandboxResumeSignals { readonly inferenceRouteConfigChanged: boolean; readonly webSearchConfigChanged: boolean; readonly sandboxGpuConfigChanged: boolean; + readonly recreateSandboxRequested: boolean; readonly messagingChannelConfigChanged: boolean; readonly hermesToolGatewayConfigChanged: boolean; readonly observabilityChanged?: boolean; @@ -135,6 +136,7 @@ function canReuseSandbox(signals: SandboxResumeSignals): boolean { !signals.inferenceSelectionChanged && !signals.webSearchConfigChanged && !signals.sandboxGpuConfigChanged && + !signals.recreateSandboxRequested && !signals.messagingChannelConfigChanged && !signals.hermesToolGatewayConfigChanged && !signals.observabilityChanged && @@ -196,6 +198,13 @@ function compatibilityResumeDecision(signals: SandboxResumeSignals): SandboxResu function runtimeConfigurationResumeDecision( signals: SandboxResumeSignals, ): SandboxResumeDecision | null { + if (signals.recreateSandboxRequested) { + return { + kind: "recreate", + note: " [resume] Recreate sandbox requested; recreating sandbox.", + removeRegistryEntry: false, + }; + } if (signals.webSearchConfigChanged) { return { kind: "recreate", diff --git a/src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts b/src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts index 8280b538a1c..294a3e212e8 100644 --- a/src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts +++ b/src/lib/onboard/machine/handlers/sandbox-test-fixtures.ts @@ -115,6 +115,7 @@ export function createDeps( promptName: vi.fn(async () => "my-assistant"), selectResourceProfile: vi.fn(async () => null as ResourceProfile | null), stopStale: vi.fn(), + reconcileRegisteredExtraProviders: vi.fn(() => [] as string[]), createSandbox: vi.fn(async () => "my-assistant"), updateSandbox: vi.fn(), complete: vi.fn(async (_stepName: string, updates: SessionUpdates) => { @@ -185,6 +186,7 @@ export function createDeps( selectResourceProfileForSandbox: calls.selectResourceProfile, stopStaleDashboardListenersForSandbox: calls.stopStale, listRegistrySandboxes: () => ({ sandboxes: [{ name: "old" }] }), + reconcileRegisteredExtraProviders: calls.reconcileRegisteredExtraProviders, createSandbox: calls.createSandbox, updateSandboxRegistry: calls.updateSandbox, getSandboxAgentRegistryFields: () => ({ agent: null }), @@ -226,6 +228,7 @@ export function baseOptions( resume: false, fresh: false, resumeAgentChanged: false, + recreateSandbox: () => false, gatewayName: "nemoclaw", session, sandboxName: null, diff --git a/src/lib/onboard/machine/handlers/sandbox-tool-disclosure.test.ts b/src/lib/onboard/machine/handlers/sandbox-tool-disclosure.test.ts index 86146839a99..b9d6b85f8bb 100644 --- a/src/lib/onboard/machine/handlers/sandbox-tool-disclosure.test.ts +++ b/src/lib/onboard/machine/handlers/sandbox-tool-disclosure.test.ts @@ -106,7 +106,7 @@ describe("handleSandboxState tool disclosure", () => { expect(calls.removeSandbox).not.toHaveBeenCalled(); expect(calls.createSandbox).toHaveBeenCalledWith( - expect.anything(), + { type: "nvidia" }, "model", "provider", "openai-completions", @@ -120,7 +120,12 @@ describe("handleSandboxState tool disclosure", () => { null, [], null, - { recreate: true, toolDisclosure: requestedMode, observabilityEnabled: false }, + { + recreate: true, + toolDisclosure: requestedMode, + observabilityEnabled: false, + extraProviders: [], + }, ); }); diff --git a/src/lib/onboard/machine/handlers/sandbox.test.ts b/src/lib/onboard/machine/handlers/sandbox.test.ts index 7bff515e3c6..789875da10e 100644 --- a/src/lib/onboard/machine/handlers/sandbox.test.ts +++ b/src/lib/onboard/machine/handlers/sandbox.test.ts @@ -74,6 +74,7 @@ describe("handleSandboxState", () => { recreate: false, toolDisclosure: "progressive", observabilityEnabled: false, + extraProviders: [], }, ); expect(calls.updateSandbox).toHaveBeenCalledWith( @@ -433,6 +434,7 @@ describe("handleSandboxState", () => { recreate: false, toolDisclosure: "progressive", observabilityEnabled: false, + extraProviders: [], }, ); expect(result.hermesToolGateways).toEqual(["nous-audio"]); @@ -542,6 +544,7 @@ describe("handleSandboxState", () => { recreate: true, toolDisclosure: "progressive", observabilityEnabled: false, + extraProviders: [], }, ); }); @@ -749,6 +752,7 @@ describe("handleSandboxState", () => { recreate: true, toolDisclosure: "progressive", observabilityEnabled: false, + extraProviders: [], }, ); expect(result.webSearchConfigChanged).toBe(true); @@ -867,6 +871,7 @@ describe("handleSandboxState", () => { recreate: true, toolDisclosure: "progressive", observabilityEnabled: false, + extraProviders: [], }, ); expect(result.webSearchConfig).toBeNull(); diff --git a/src/lib/onboard/machine/handlers/sandbox.ts b/src/lib/onboard/machine/handlers/sandbox.ts index 25afabaa315..d3e9dee7f23 100644 --- a/src/lib/onboard/machine/handlers/sandbox.ts +++ b/src/lib/onboard/machine/handlers/sandbox.ts @@ -64,6 +64,7 @@ export interface SandboxStateOptions< resumeAgentChanged: boolean; requestedObservabilityEnabled?: boolean | null; requestedDcodeAutoApprovalMode?: DcodeAutoApprovalMode | null; + recreateSandbox: (requested?: boolean) => boolean; gatewayName: string; session: Session | null; sandboxName: string | null; @@ -152,6 +153,7 @@ export interface SandboxStateOptions< selectResourceProfileForSandbox(): Promise; stopStaleDashboardListenersForSandbox(sandboxes: unknown[], sandboxName: string): void; listRegistrySandboxes(): { sandboxes: unknown[] }; + reconcileRegisteredExtraProviders(gatewayName: string): readonly string[]; createSandbox( gpu: Gpu, model: string, @@ -421,6 +423,7 @@ class SandboxStateFlow< sandboxGpuConfigChanged: state.sandboxName ? this.deps.hasSandboxGpuDrift(state.sandboxName, this.options.sandboxGpuConfig) : false, + recreateSandboxRequested: this.options.recreateSandbox(false), messagingChannelConfigChanged: !this.deps.messagingChannelConfigsEqual( effectiveMessagingConfig, storedMessagingConfig, @@ -611,6 +614,7 @@ class SandboxStateFlow< private buildSandboxCreateIntent( state: SandboxStepState, decision: SandboxCreationDecision, + extraProviders: readonly string[], ): SandboxCreateIntent { return { recreate: decision.kind !== "create", @@ -627,6 +631,7 @@ class SandboxStateFlow< ...(this.options.authoritativePolicyTier ? { policyTier: this.options.authoritativePolicyTier } : {}), + extraProviders, }; } @@ -641,6 +646,8 @@ class SandboxStateFlow< state.webSearchConfig as unknown as SharedWebSearchConfig | null, this.options.hermesToolGateways, ); + const extraProviders = this.deps.reconcileRegisteredExtraProviders(this.options.gatewayName); + const createIntent = this.buildSandboxCreateIntent(state, decision, extraProviders); const resourceProfile = await this.deps.selectResourceProfileForSandbox(); const createAndRecord = async (): Promise> => { this.assertGatewayRouteCompatible(requestedSandboxName); @@ -659,7 +666,6 @@ class SandboxStateFlow< current.messagingPlan = messagingPlan; return current; }); - const createIntent = this.buildSandboxCreateIntent(state, decision); const sandboxName = await withSandboxPhaseTrace( requestedSandboxName, this.options.provider, diff --git a/src/lib/onboard/sandbox-create-plan-extra-providers.test.ts b/src/lib/onboard/sandbox-create-plan-extra-providers.test.ts new file mode 100644 index 00000000000..c68d370385b --- /dev/null +++ b/src/lib/onboard/sandbox-create-plan-extra-providers.test.ts @@ -0,0 +1,71 @@ +// 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 { missing, reconcile } from "./extra-provider-reconciliation.test-fixtures"; +import { prepareSandboxCreatePlan } from "./sandbox-create-plan"; +import type { SandboxGpuCreateConfig } from "./sandbox-gpu-create"; + +const sandboxGpuConfig: SandboxGpuCreateConfig = { + sandboxGpuEnabled: false, + sandboxGpuDevice: null, +}; + +function providerArgs(args: string[]): string[] { + return args + .map((arg, index) => (arg === "--provider" ? args[index + 1] : null)) + .filter((value): value is string => value !== null); +} + +function buildPlan( + extraProviders: readonly string[] = ["brave-search", "custom-provider", "brave-search"], +) { + return prepareSandboxCreatePlan({ + basePolicyPath: "/repo/policy.yaml", + buildCtx: "/tmp/nemoclaw-build-1", + sandboxName: "sandbox", + channels: [], + enabledChannels: [], + disabledChannelNames: new Set(), + messagingTokenDefs: [], + reusableMessagingChannels: [], + reusableMessagingProviders: [], + extraProviders, + hermesToolGateways: [], + sandboxGpuConfig, + dockerDriverGateway: true, + appendResourceFlags: vi.fn(), + runProviderPreDeleteCleanup: vi.fn(), + upsertMessagingProviders: vi.fn(() => []), + getMessagingChannelForEnvKey: () => null, + getHermesToolGatewayProviderName: vi.fn(), + deps: { + resolveDockerGpuSandboxCreatePlan: vi.fn(() => ({ + useDockerGpuPatch: false, + logMessage: null, + })), + prepareInitialSandboxCreatePolicy: vi.fn(() => ({ + policyPath: "/tmp/policy.yaml", + appliedPresets: [], + })), + buildSandboxGpuCreateArgs: vi.fn(() => []), + }, + }); +} + +describe("prepareSandboxCreatePlan extra providers", () => { + it("keeps reconciled extra providers stable across retry create plans (#6501)", () => { + const reconciledExtraProviders = reconcile( + ["brave-search", "tavily-search", "custom-provider", "brave-search"], + { + "tavily-search": missing("tavily-search"), + }, + ); + const firstProviders = providerArgs(buildPlan(reconciledExtraProviders).createArgs); + const retryProviders = providerArgs(buildPlan(reconciledExtraProviders).createArgs); + + expect(reconciledExtraProviders).toEqual(["brave-search", "custom-provider", "brave-search"]); + expect(firstProviders).toEqual(["brave-search", "custom-provider"]); + expect(retryProviders).toEqual(firstProviders); + }); +}); diff --git a/src/lib/onboard/sandbox-create-plan.test.ts b/src/lib/onboard/sandbox-create-plan.test.ts index 3a135c25e64..fd8f3b6c820 100644 --- a/src/lib/onboard/sandbox-create-plan.test.ts +++ b/src/lib/onboard/sandbox-create-plan.test.ts @@ -74,7 +74,10 @@ function expectCredentialBindingFailure({ sandboxGpuLogMessage: null, policyTier: null, }); - const preparePolicy = vi.fn(() => ({ policyPath: "/tmp/policy.yaml", appliedPresets: [] })); + const preparePolicy = vi.fn(() => ({ + policyPath: "/tmp/policy.yaml", + appliedPresets: [], + })); const appendResources = vi.fn(); const cleanupProviders = vi.fn(); const upsertProviders = vi.fn(() => []); @@ -456,8 +459,16 @@ describe("prepareSandboxCreatePlan", () => { enabledChannels: ["telegram", "slack", "whatsapp"], disabledChannelNames: new Set(["slack"]), messagingTokenDefs: [ - { name: "sandbox-telegram-bridge", envKey: "TELEGRAM_BOT_TOKEN", token: "telegram" }, - { name: "sandbox-slack-bridge", envKey: "SLACK_BOT_TOKEN", token: "slack" }, + { + name: "sandbox-telegram-bridge", + envKey: "TELEGRAM_BOT_TOKEN", + token: "telegram", + }, + { + name: "sandbox-slack-bridge", + envKey: "SLACK_BOT_TOKEN", + token: "slack", + }, ], reusableMessagingChannels: ["slack", "whatsapp"], reusableMessagingProviders: ["sandbox-slack-bridge", "sandbox-existing-whatsapp"], @@ -488,7 +499,13 @@ describe("prepareSandboxCreatePlan", () => { }); expect(upsertMessagingProviders).toHaveBeenCalledWith( - [{ name: "sandbox-telegram-bridge", envKey: "TELEGRAM_BOT_TOKEN", token: "telegram" }], + [ + { + name: "sandbox-telegram-bridge", + envKey: "TELEGRAM_BOT_TOKEN", + token: "telegram", + }, + ], { replaceExisting: true }, ); expect(result.activeMessagingChannels).toEqual(["telegram", "whatsapp"]); diff --git a/src/lib/onboard/types.ts b/src/lib/onboard/types.ts index 0f4068dc943..5c52fbb2ce8 100644 --- a/src/lib/onboard/types.ts +++ b/src/lib/onboard/types.ts @@ -64,6 +64,8 @@ export interface SandboxCreateIntent { readonly endpointUrl?: string | null; /** Internal authoritative rebuild tier used before replacement registration completes. */ readonly policyTier?: string | null; + /** Gateway-level extra providers reconciled immediately before sandbox creation. */ + readonly extraProviders?: readonly string[]; } export type OnboardOptions = { diff --git a/test/e2e/fixtures/extra-providers-registry.ts b/test/e2e/fixtures/extra-providers-registry.ts new file mode 100644 index 00000000000..254a5fcb2ba --- /dev/null +++ b/test/e2e/fixtures/extra-providers-registry.ts @@ -0,0 +1,38 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +export const REGISTRY_FILE = path.join(os.homedir(), ".nemoclaw", "sandboxes.json"); + +export function readRegistry(): { extraProviders?: unknown; [key: string]: unknown } { + return fs.existsSync(REGISTRY_FILE) + ? (JSON.parse(fs.readFileSync(REGISTRY_FILE, "utf8")) as { + extraProviders?: unknown; + [key: string]: unknown; + }) + : { sandboxes: {}, defaultSandbox: null }; +} + +export function readExtraProviders(): string[] { + const value = readRegistry().extraProviders; + return Array.isArray(value) + ? value.filter((item): item is string => typeof item === "string") + : []; +} + +export function updateExtraProviders(update: (providers: Set) => void): string[] { + const registry = readRegistry(); + const providers = new Set(readExtraProviders()); + update(providers); + const sorted = [...providers].sort(); + const nextRegistry = Object.assign( + Object.fromEntries(Object.entries(registry).filter(([key]) => key !== "extraProviders")), + sorted.length > 0 ? { extraProviders: sorted } : {}, + ); + fs.mkdirSync(path.dirname(REGISTRY_FILE), { recursive: true }); + fs.writeFileSync(REGISTRY_FILE, `${JSON.stringify(nextRegistry, null, 2)}\n`, "utf8"); + return sorted; +} diff --git a/test/e2e/fixtures/gateway-providers.ts b/test/e2e/fixtures/gateway-providers.ts new file mode 100644 index 00000000000..f9930157710 --- /dev/null +++ b/test/e2e/fixtures/gateway-providers.ts @@ -0,0 +1,88 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { resultText } from "./clients/command.ts"; +import type { HostCliClient } from "./clients/host.ts"; +import type { SandboxClient } from "./clients/sandbox.ts"; +import { expect } from "./e2e-test.ts"; + +const PROVIDER_NAME = /^[A-Za-z0-9][A-Za-z0-9._-]*$/u; +const CREDENTIAL_ENV = /^[A-Z_][A-Z0-9_]*$/u; + +function shellQuote(value: string): string { + return `'${value.replaceAll("'", "'\\''")}'`; +} + +function assertProviderName(providerName: string): void { + if (!PROVIDER_NAME.test(providerName)) { + throw new Error(`Unsafe OpenShell provider name: ${providerName}`); + } +} + +export async function upsertGenericGatewayProvider( + host: HostCliClient, + providerName: string, + options: { + artifactName: string; + credentialEnv: string; + env: NodeJS.ProcessEnv; + redactionValues?: string[]; + }, +): Promise { + assertProviderName(providerName); + if (!CREDENTIAL_ENV.test(options.credentialEnv)) { + throw new Error(`Unsafe provider credential env name: ${options.credentialEnv}`); + } + if (!options.env[options.credentialEnv]) { + throw new Error(`Missing provider credential env value: ${options.credentialEnv}`); + } + + const provider = shellQuote(providerName); + const credential = shellQuote(options.credentialEnv); + const result = await host.command( + "bash", + [ + "-lc", + [ + "set -euo pipefail", + `if openshell provider get -g nemoclaw ${provider} >/dev/null 2>&1; then`, + ` openshell provider update -g nemoclaw ${provider} --credential ${credential}`, + "else", + ` openshell provider create -g nemoclaw --name ${provider} --type generic --credential ${credential}`, + "fi", + ].join("\n"), + ], + { + artifactName: options.artifactName, + env: options.env, + redactionValues: options.redactionValues ?? [], + timeoutMs: 60_000, + }, + ); + expect(result.exitCode, resultText(result)).toBe(0); +} + +export async function expectSandboxProviderAttachment( + sandbox: SandboxClient, + sandboxName: string, + providerName: string, + expected: "present" | "absent", + options: { artifactName: string; env: NodeJS.ProcessEnv }, +): Promise { + assertProviderName(providerName); + const attachments = await sandbox.openshell( + ["sandbox", "provider", "list", "-g", "nemoclaw", sandboxName], + { + artifactName: options.artifactName, + env: options.env, + timeoutMs: 60_000, + }, + ); + expect(attachments.exitCode, resultText(attachments)).toBe(0); + const providerNames = resultText(attachments).split(/\s+/u); + if (expected === "present") { + expect(providerNames).toContain(providerName); + } else { + expect(providerNames).not.toContain(providerName); + } +} diff --git a/test/e2e/live/onboard-repair.test.ts b/test/e2e/live/onboard-repair.test.ts index e1d9a433aa1..94b809934b4 100644 --- a/test/e2e/live/onboard-repair.test.ts +++ b/test/e2e/live/onboard-repair.test.ts @@ -14,13 +14,22 @@ import { createCorporateCaFixture, } from "../fixtures/corporate-ca.ts"; import { expect, test } from "../fixtures/e2e-test.ts"; +import { readExtraProviders, updateExtraProviders } from "../fixtures/extra-providers-registry.ts"; import { startFakeOpenAiCompatibleServer } from "../fixtures/fake-openai-compatible.ts"; +import { + expectSandboxProviderAttachment, + upsertGenericGatewayProvider, +} from "../fixtures/gateway-providers.ts"; import { CLI_ENTRYPOINT } from "../fixtures/paths.ts"; import type { ShellProbeResult } from "../fixtures/shell-probe.ts"; const SANDBOX_NAME = process.env.NEMOCLAW_SANDBOX_NAME ?? "e2e-repair"; const OTHER_SANDBOX_NAME = process.env.NEMOCLAW_OTHER_SANDBOX_NAME ?? "e2e-repair-other"; const SESSION_FILE = path.join(os.homedir(), ".nemoclaw", "onboard-session.json"); +const STALE_EXTRA_PROVIDER = "e2e-stale-extra-provider"; +const LIVE_EXTRA_PROVIDER = "e2e-live-extra-provider"; +const EXTRA_PROVIDER_TOKEN_ENV = "NEMOCLAW_E2E_EXTRA_PROVIDER_TOKEN"; +const EXTRA_PROVIDER_TOKEN = "e2e-extra-provider-token"; const LIVE_TIMEOUT_MS = 70 * 60_000; validateSandboxName(SANDBOX_NAME); @@ -83,6 +92,13 @@ async function cleanup(host: HostCliClient, sandbox: SandboxClient): Promise undefined); + await sandbox + .openshell(["provider", "delete", "-g", "nemoclaw", LIVE_EXTRA_PROVIDER], { + artifactName: "cleanup-live-extra-provider-delete", + env: env({ [EXTRA_PROVIDER_TOKEN_ENV]: EXTRA_PROVIDER_TOKEN }), + timeoutMs: 60_000, + }) + .catch(() => undefined); await sandbox .openshell(["gateway", "destroy", "-g", "nemoclaw"], { artifactName: "cleanup-gateway-destroy", @@ -90,6 +106,10 @@ async function cleanup(host: HostCliClient, sandbox: SandboxClient): Promise undefined); + updateExtraProviders((providers) => { + providers.delete(STALE_EXTRA_PROVIDER); + providers.delete(LIVE_EXTRA_PROVIDER); + }); fs.rmSync(SESSION_FILE, { force: true }); } @@ -119,6 +139,8 @@ test("onboard repair resumes missing sandbox and rejects conflicting resume inpu contracts: [ "forced policy-step failure leaves a resumable session", "resume recreates a recorded sandbox that was removed underneath it", + "resume repair filters stale extra-provider records while preserving live attachments", + "resume repair proves recreated sandbox provider attachments are selectively reconciled", "REQUESTS_CA_BUNDLE fallback corporate CA source is baked and merged after repair", "resume rejects a different requested sandbox name", "resume rejects provider/model overrides that conflict with recorded state", @@ -166,6 +188,21 @@ test("onboard repair resumes missing sandbox and rejects conflicting resume inpu }); expect(sandboxAfterFailure.exitCode, resultText(sandboxAfterFailure)).toBe(0); + await upsertGenericGatewayProvider(host, LIVE_EXTRA_PROVIDER, { + artifactName: "phase-1-live-extra-provider-upsert", + credentialEnv: EXTRA_PROVIDER_TOKEN_ENV, + env: env({ [EXTRA_PROVIDER_TOKEN_ENV]: EXTRA_PROVIDER_TOKEN }), + redactionValues: [EXTRA_PROVIDER_TOKEN], + }); + const seededExtraProviders = updateExtraProviders((providers) => { + providers.add(STALE_EXTRA_PROVIDER); + providers.add(LIVE_EXTRA_PROVIDER); + }); + await artifacts.writeJson("phase-1-extra-providers-seeded.json", seededExtraProviders); + expect(seededExtraProviders).toEqual( + expect.arrayContaining([LIVE_EXTRA_PROVIDER, STALE_EXTRA_PROVIDER]), + ); + await sandbox.openshell(["sandbox", "delete", SANDBOX_NAME], { artifactName: "phase-2-delete-recorded-sandbox", env: env(), @@ -186,6 +223,17 @@ test("onboard repair resumes missing sandbox and rejects conflicting resume inpu expect(resultText(repair)).toContain("[resume] Skipping preflight (cached)"); expect(resultText(repair)).toContain("Recorded sandbox state is unavailable; recreating it"); expect(resultText(repair)).toContain("Creating sandbox"); + const reconciledExtraProviders = readExtraProviders(); + expect(reconciledExtraProviders).toContain(LIVE_EXTRA_PROVIDER); + expect(reconciledExtraProviders).not.toContain(STALE_EXTRA_PROVIDER); + await expectSandboxProviderAttachment(sandbox, SANDBOX_NAME, LIVE_EXTRA_PROVIDER, "present", { + artifactName: "phase-2-sandbox-provider-list-live-after-repair", + env: env(), + }); + await expectSandboxProviderAttachment(sandbox, SANDBOX_NAME, STALE_EXTRA_PROVIDER, "absent", { + artifactName: "phase-2-sandbox-provider-list-stale-after-repair", + env: env(), + }); const status = await nemoclaw(host, [SANDBOX_NAME, "status"], "phase-2-status-after-repair"); expect(status.exitCode, resultText(status)).toBe(0); diff --git a/test/e2e/live/onboard-resume.test.ts b/test/e2e/live/onboard-resume.test.ts index 6d078ad340a..3a8da8892f0 100644 --- a/test/e2e/live/onboard-resume.test.ts +++ b/test/e2e/live/onboard-resume.test.ts @@ -15,10 +15,19 @@ import { createCorporateCaFixture, } from "../fixtures/corporate-ca.ts"; import { expect, test } from "../fixtures/e2e-test.ts"; +import { + readExtraProviders, + REGISTRY_FILE, + updateExtraProviders, +} from "../fixtures/extra-providers-registry.ts"; import { type FakeOpenAiCompatibleServer, startFakeOpenAiCompatibleServer, } from "../fixtures/fake-openai-compatible.ts"; +import { + expectSandboxProviderAttachment, + upsertGenericGatewayProvider, +} from "../fixtures/gateway-providers.ts"; import { CLI_ENTRYPOINT } from "../fixtures/paths.ts"; // Disruption-recovery contract — regression for #446. @@ -35,10 +44,13 @@ import { CLI_ENTRYPOINT } from "../fixtures/paths.ts"; // registry, migration ledger, or new shared helper. const SESSION_FILE = path.join(os.homedir(), ".nemoclaw", "onboard-session.json"); -const REGISTRY_FILE = path.join(os.homedir(), ".nemoclaw", "sandboxes.json"); const SANDBOX_NAME = process.env.NEMOCLAW_SANDBOX_NAME ?? "e2e-resume"; const FAKE_COMPATIBLE_AUTH_VALUE = "e2e-compatible-auth-value"; const FAKE_COMPATIBLE_MODEL = "test-model"; +const STALE_EXTRA_PROVIDER = "e2e-resume-stale-extra-provider"; +const LIVE_EXTRA_PROVIDER = "e2e-resume-live-extra-provider"; +const EXTRA_PROVIDER_TOKEN_ENV = "NEMOCLAW_E2E_EXTRA_PROVIDER_TOKEN"; +const EXTRA_PROVIDER_TOKEN = "e2e-resume-extra-provider-token"; validateSandboxName(SANDBOX_NAME); // 15 minutes per onboard run; matches NEMOCLAW_E2E_DEFAULT_TIMEOUT in the @@ -133,7 +145,7 @@ function expectHermeticCompatibleEndpointUsed( // The e2e-live Vitest project owns the NEMOCLAW_RUN_LIVE_E2E collection gate, // so accidental cli-test-shard discovery cannot run this without real // `openshell`, Docker, or a sandbox-reachable fake OpenAI-compatible endpoint. -test("onboard-resume: interrupted onboard then --resume completes without redoing cached steps", async ({ +test("onboard-resume: interrupted onboard then --resume can recreate with cached setup", async ({ artifacts, cleanup, host, @@ -151,7 +163,9 @@ test("onboard-resume: interrupted onboard then --resume completes without redoin corporateCaSource: corporateCa.sourceLabel, contracts: [ "forced policy-step failure leaves a resumable session", - "resume completes without redoing cached preflight/gateway/sandbox steps", + "resume recreates the sandbox on request without redoing cached preflight/gateway steps", + "resume sandbox recreation filters stale extra providers while preserving live attachments", + "resume proves recreated sandbox provider attachments are selectively reconciled", "host trust-store anchor corporate CA source is baked and merged after resume", "implicit resume is detected and --fresh suppresses that auto-resume", ], @@ -240,6 +254,11 @@ test("onboard-resume: interrupted onboard then --resume completes without redoin env: probeEnv, timeoutMs: 30_000, }); + await sandbox.openshell(["provider", "delete", "-g", "nemoclaw", LIVE_EXTRA_PROVIDER], { + artifactName: "pre-cleanup-live-extra-provider-delete", + env: { ...probeEnv, [EXTRA_PROVIDER_TOKEN_ENV]: EXTRA_PROVIDER_TOKEN }, + timeoutMs: 60_000, + }); await sandbox.openshell(["gateway", "destroy", "-g", "nemoclaw"], { artifactName: "pre-cleanup-openshell-gateway-destroy", env: probeEnv, @@ -266,12 +285,21 @@ test("onboard-resume: interrupted onboard then --resume completes without redoin env: cleanupEnv, timeoutMs: 30_000, }); + await sandbox.openshell(["provider", "delete", "-g", "nemoclaw", LIVE_EXTRA_PROVIDER], { + artifactName: "cleanup-live-extra-provider-delete", + env: { ...cleanupEnv, [EXTRA_PROVIDER_TOKEN_ENV]: EXTRA_PROVIDER_TOKEN }, + timeoutMs: 60_000, + }); await sandbox.openshell(["gateway", "destroy", "-g", "nemoclaw"], { artifactName: "cleanup-openshell-gateway-destroy", env: cleanupEnv, timeoutMs: 60_000, }); fs.rmSync(SESSION_FILE, { force: true }); + updateExtraProviders((providers) => { + providers.delete(STALE_EXTRA_PROVIDER); + providers.delete(LIVE_EXTRA_PROVIDER); + }); const sandboxAfterCleanup = await sandbox.openshell(["sandbox", "get", SANDBOX_NAME], { artifactName: "cleanup-openshell-sandbox-get-after-delete", @@ -371,10 +399,26 @@ test("onboard-resume: interrupted onboard then --resume completes without redoin await artifacts.writeJson("phase-2-fake-openai-compatible-requests.json", fake.requests()); expectHermeticCompatibleEndpointUsed(fake, onboardingRequestOffset); + await upsertGenericGatewayProvider(host, LIVE_EXTRA_PROVIDER, { + artifactName: "phase-2-live-extra-provider-upsert", + credentialEnv: EXTRA_PROVIDER_TOKEN_ENV, + env: { ...buildAvailabilityProbeEnv(), [EXTRA_PROVIDER_TOKEN_ENV]: EXTRA_PROVIDER_TOKEN }, + redactionValues: [EXTRA_PROVIDER_TOKEN], + }); + const seededExtraProviders = updateExtraProviders((providers) => { + providers.add(STALE_EXTRA_PROVIDER); + providers.add(LIVE_EXTRA_PROVIDER); + }); + await artifacts.writeJson("phase-2-extra-providers-seeded.json", seededExtraProviders); + expect(seededExtraProviders).toEqual( + expect.arrayContaining([LIVE_EXTRA_PROVIDER, STALE_EXTRA_PROVIDER]), + ); + // ────────────────────────────────────────────────────────────────── // Phase 3: resume — NVIDIA_INFERENCE_API_KEY and COMPATIBLE_API_KEY are // removed from env so the resume run must hydrate the credential from the - // gateway/session state. + // gateway/session state, then recreate the sandbox with stale extra-provider + // attachments filtered out for this create attempt. // ────────────────────────────────────────────────────────────────── const resumeEnv: NodeJS.ProcessEnv = { ...buildAvailabilityProbeEnv(), @@ -387,7 +431,7 @@ test("onboard-resume: interrupted onboard then --resume completes without redoin expect(resumeEnv.COMPATIBLE_API_KEY).toBeUndefined(); const resumeRun = await host.command( "node", - [CLI_ENTRYPOINT, "onboard", "--resume", "--non-interactive"], + [CLI_ENTRYPOINT, "onboard", "--resume", "--recreate-sandbox", "--non-interactive"], { artifactName: "phase-3-onboard-resume", env: resumeEnv, @@ -400,17 +444,28 @@ test("onboard-resume: interrupted onboard then --resume completes without redoin // Assertion: resume-exit-0. expect(resumeRun.exitCode, resumeText).toBe(0); - // Assertion: resume-skipped-{preflight,gateway,sandbox}-log. + // Assertion: resume-skipped-{preflight,gateway}-log and recreates sandbox. expect(resumeText).toContain("[resume] Skipping preflight (cached)"); expect(resumeText).toContain("[resume] Skipping gateway (running)"); - expect(resumeText).toContain(`[resume] Skipping sandbox (${SANDBOX_NAME})`); + expect(resumeText).toContain(`Deleting and recreating sandbox '${SANDBOX_NAME}'`); + expect(resumeText).toContain(`Sandbox '${SANDBOX_NAME}' created`); - // Assertion: resume-no-{preflight,gateway,sandbox}-redo. Current CLI output + // Assertion: resume-no-{preflight,gateway}-redo. Current CLI output // still prints phase headings before the resume-skip decisions, so assert // the skip evidence and absence of redo-only success strings instead of // rejecting headings that now frame the skipped phases. - expect(resumeText).not.toContain("Sandbox '" + SANDBOX_NAME + "' created"); expect(resumeText).not.toContain("Starting OpenShell Docker-driver gateway..."); + const reconciledExtraProviders = readExtraProviders(); + expect(reconciledExtraProviders).toContain(LIVE_EXTRA_PROVIDER); + expect(reconciledExtraProviders).not.toContain(STALE_EXTRA_PROVIDER); + await expectSandboxProviderAttachment(sandbox, SANDBOX_NAME, LIVE_EXTRA_PROVIDER, "present", { + artifactName: "phase-3-sandbox-provider-list-live-after-resume", + env: buildAvailabilityProbeEnv(), + }); + await expectSandboxProviderAttachment(sandbox, SANDBOX_NAME, STALE_EXTRA_PROVIDER, "absent", { + artifactName: "phase-3-sandbox-provider-list-stale-after-resume", + env: buildAvailabilityProbeEnv(), + }); // Assertion: resume-inference-handled — first onboard completed through // openclaw before failing at policies. Inference was already configured diff --git a/test/e2e/mock-parity.json b/test/e2e/mock-parity.json index cb4307863ef..5ffc07fe892 100644 --- a/test/e2e/mock-parity.json +++ b/test/e2e/mock-parity.json @@ -1,5 +1,29 @@ { "$comment": "SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.\nSPDX-License-Identifier: Apache-2.0", "version": 1, - "entries": [] + "entries": [ + { + "live": "test/e2e/live/onboard-repair.test.ts", + "fast": [ + "src/lib/onboard/extra-provider-reconciliation-diagnostics.test.ts", + "src/lib/onboard/extra-provider-reconciliation-probes.test.ts", + "src/lib/onboard/extra-provider-reconciliation.test.ts", + "src/lib/onboard/machine/handlers/sandbox-resume.test.ts", + "src/lib/onboard/sandbox-create-plan-extra-providers.test.ts", + "test/onboard-extra-provider-reconciliation.test.ts" + ] + }, + { + "live": "test/e2e/live/onboard-resume.test.ts", + "fast": [ + "src/lib/onboard/extra-provider-reconciliation-diagnostics.test.ts", + "src/lib/onboard/extra-provider-reconciliation-probes.test.ts", + "src/lib/onboard/extra-provider-reconciliation.test.ts", + "src/lib/onboard/machine/handlers/sandbox-recreate-resume.test.ts", + "src/lib/onboard/machine/handlers/sandbox-resume.test.ts", + "src/lib/onboard/sandbox-create-plan-extra-providers.test.ts", + "test/onboard-extra-provider-reconciliation.test.ts" + ] + } + ] } diff --git a/test/onboard-extra-provider-reconciliation.test.ts b/test/onboard-extra-provider-reconciliation.test.ts index b89da34f7c8..f0c6ce4d98c 100644 --- a/test/onboard-extra-provider-reconciliation.test.ts +++ b/test/onboard-extra-provider-reconciliation.test.ts @@ -17,7 +17,7 @@ const onboardScriptMocksPath = JSON.stringify( ); describe("onboard extra-provider reconciliation", () => { - it("attaches live user extras, skips stale names, and preserves registry state (#6501)", { + it("attaches live user extras, prunes stale names, and converges registry state (#6501)", { timeout: 90_000, }, () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-extra-provider-reconcile-")); @@ -35,6 +35,9 @@ describe("onboard extra-provider reconciliation", () => { const credentialsPath = JSON.stringify( path.join(repoRoot, "src", "lib", "credentials", "store.ts"), ); + const sandboxBaseImagePath = JSON.stringify( + path.join(repoRoot, "src", "lib", "sandbox-base-image.ts"), + ); fs.mkdirSync(fakeBin, { recursive: true }); writeOkOpenshell(fakeBin); @@ -44,6 +47,7 @@ const runner = require(${runnerPath}); const registry = require(${registryPath}); const preflight = require(${preflightPath}); const credentials = require(${credentialsPath}); +const sandboxBaseImage = require(${sandboxBaseImagePath}); const childProcess = require("node:child_process"); const { EventEmitter } = require("node:events"); const _n = (command) => (Array.isArray(command) ? command.join(" ") : String(command)).replace(/'/g, ""); @@ -57,8 +61,17 @@ registry.addExtraProvider("my-slack-bridge"); runner.run = (command, opts = {}) => { const normalized = _n(command); commands.push({ command: normalized, env: opts.env || null }); - if (normalized.includes("provider list -g nemoclaw --names")) { - return { status: 0, stdout: "brave-search\ncustom-provider\nmy-slack-bridge\n" }; + if (normalized.includes("provider get -g nemoclaw tavily-search")) { + const stderr = Buffer.from("Error: provider 'tavily-search' not found"); + return { + status: 1, + stderr, + stdout: Buffer.alloc(0), + output: [null, Buffer.alloc(0), stderr], + }; + } + if (normalized.includes("provider get -g nemoclaw ")) { + return { status: 0, stdout: "" }; } return { status: 0 }; }; @@ -79,6 +92,12 @@ registry.setDefault = () => true; registry.removeSandbox = () => true; preflight.checkPortAvailable = async () => ({ ok: true }); credentials.prompt = async () => ""; +sandboxBaseImage.resolveSandboxBaseImage = () => ({ + ref: "ghcr.io/nvidia/nemoclaw/sandbox-base@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + digest: "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + source: "latest", + glibcVersion: "2.39", +}); childProcess.spawn = (...args) => { const child = new EventEmitter(); @@ -101,9 +120,12 @@ const { createSandbox } = require(${onboardPath}); (async () => { process.env.OPENSHELL_GATEWAY = "nemoclaw"; - const sandboxName = await createSandbox(null, "gpt-5.4"); + const sandboxNames = [ + await createSandbox(null, "gpt-5.4"), + await createSandbox(null, "gpt-5.4"), + ]; console.log(JSON.stringify({ - sandboxName, + sandboxNames, commands, extraProviders: registry.listExtraProviders(), })); @@ -134,26 +156,53 @@ const { createSandbox } = require(${onboardPath}); .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"); + assert.deepEqual(payload.sandboxNames, ["my-assistant", "my-assistant"]); - const createCommand = payload.commands.find((entry: CommandEntry) => + const createCommands = payload.commands.filter((entry: CommandEntry) => entry.command.includes("sandbox create"), ); - assert.ok(createCommand, "expected sandbox create command"); + assert.equal(createCommands.length, 2, "expected one sandbox create command per attempt"); + const [createCommand, retryCreateCommand] = createCommands; assert.match(createCommand.command, /--provider brave-search/); assert.match(createCommand.command, /--provider custom-provider/); assert.match(createCommand.command, /--provider my-slack-bridge/); assert.doesNotMatch(createCommand.command, /--provider tavily-search/); + assert.deepEqual( + createCommand.command.match(/--provider\s+\S+/g), + retryCreateCommand.command.match(/--provider\s+\S+/g), + "retry must preserve the exact filtered provider arguments", + ); - const providerLists = payload.commands.filter((entry: CommandEntry) => - entry.command.includes("provider list -g nemoclaw --names"), + const providerProbes = payload.commands.filter((entry: CommandEntry) => + entry.command.includes("provider get -g nemoclaw "), + ); + assert.deepEqual( + providerProbes + .map((entry: CommandEntry) => + entry.command.slice(entry.command.indexOf("provider get -g nemoclaw ")), + ) + .sort(), + [ + "provider get -g nemoclaw brave-search", + "provider get -g nemoclaw brave-search", + "provider get -g nemoclaw custom-provider", + "provider get -g nemoclaw custom-provider", + "provider get -g nemoclaw my-slack-bridge", + "provider get -g nemoclaw my-slack-bridge", + "provider get -g nemoclaw tavily-search", + ].sort(), + ); + assert.equal( + payload.commands.some((entry: CommandEntry) => + entry.command.includes("provider list -g nemoclaw --names"), + ), + false, + "provider-list snapshots must not control extra-provider attachment", ); - assert.equal(providerLists.length, 1, "expected one gateway-scoped provider list"); assert.deepEqual(payload.extraProviders, [ "brave-search", "custom-provider", "my-slack-bridge", - "tavily-search", ]); } finally { fs.rmSync(tmpDir, { recursive: true, force: true });