diff --git a/ci/source-architecture-budget.json b/ci/source-architecture-budget.json index e729c6452e0..5651e7a43bc 100644 --- a/ci/source-architecture-budget.json +++ b/ci/source-architecture-budget.json @@ -6,7 +6,7 @@ "src/lib/actions/sandbox/mcp-bridge-contracts.ts": 25, "src/lib/actions/sandbox/process-recovery.ts": 27, "src/lib/adapters/docker/index.ts": 43, - "src/lib/adapters/openshell/client.ts": 20, + "src/lib/adapters/openshell/client.ts": 19, "src/lib/adapters/openshell/resolve.ts": 27, "src/lib/adapters/openshell/runtime.ts": 55, "src/lib/adapters/openshell/timeouts.ts": 39, diff --git a/src/commands/credentials.test.ts b/src/commands/credentials.test.ts index 3acbc5b47fd..1975bb10e68 100644 --- a/src/commands/credentials.test.ts +++ b/src/commands/credentials.test.ts @@ -86,6 +86,7 @@ describe("credentials oclif adapter source coverage", () => { ["provider", "list", "--names"], { ignoreError: true, + maxBuffer: 64 * 1024, stdio: ["ignore", "pipe", "pipe"], timeout: 30_000, }, @@ -259,6 +260,7 @@ describe("credentials oclif adapter source coverage", () => { ["provider", "profile", "export", "openai", "--output", "json"], { ignoreError: true, + maxBuffer: 64 * 1024, suppressOutput: true, stdio: ["ignore", "pipe", "pipe"], timeout: 30_000, @@ -292,6 +294,7 @@ describe("credentials oclif adapter source coverage", () => { ["provider", "profile", "export", "openai", "--output", "json"], { ignoreError: true, + maxBuffer: 64 * 1024, suppressOutput: true, stdio: ["ignore", "pipe", "pipe"], timeout: 30_000, @@ -341,12 +344,14 @@ describe("credentials oclif adapter source coverage", () => { ).toEqual([ { ignoreError: true, + maxBuffer: 64 * 1024, suppressOutput: true, stdio: ["ignore", "pipe", "pipe"], timeout: 30_000, }, { ignoreError: true, + maxBuffer: 64 * 1024, suppressOutput: true, stdio: ["ignore", "pipe", "pipe"], timeout: 30_000, diff --git a/src/lib/actions/inference-set-failure-handling.test.ts b/src/lib/actions/inference-set-failure-handling.test.ts index d4c80a517bf..a4a76abec8e 100644 --- a/src/lib/actions/inference-set-failure-handling.test.ts +++ b/src/lib/actions/inference-set-failure-handling.test.ts @@ -188,7 +188,7 @@ describe("runInferenceSet failure handling", () => { expect(deps.calls.captureOpenshell).toHaveBeenNthCalledWith( 2, ["provider", "list", "--names"], - { ignoreError: true, maxBuffer: 64 * 1024, timeout: 5_000 }, + { ignoreError: true, includeStreams: true, maxBuffer: 64 * 1024, timeout: 5_000 }, ); expect(deps.calls.writeSandboxConfig).not.toHaveBeenCalled(); expect(deps.calls.updateSandbox).not.toHaveBeenCalled(); diff --git a/src/lib/actions/inference-set-no-auth-compatible.test.ts b/src/lib/actions/inference-set-no-auth-compatible.test.ts index 7b634c68c71..ee36808be77 100644 --- a/src/lib/actions/inference-set-no-auth-compatible.test.ts +++ b/src/lib/actions/inference-set-no-auth-compatible.test.ts @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { describe, expect, it, vi } from "vitest"; +import type { OpenShellProviderAdapter } from "../adapters/openshell/provider-adapter"; import type { SandboxEntry } from "../state/registry"; import { runInferenceSet } from "./inference-set"; import { @@ -174,6 +175,38 @@ describe("runInferenceSet on a loopback no-auth compatible endpoint", () => { expect(deps.calls.writeSandboxConfig).not.toHaveBeenCalled(); }); + it("refuses incomplete typed provider identity before route mutation (#9806)", async () => { + const captureOpenshell = noAuthProviderCapture(); + const providerAdapter = { + getProvider: vi.fn(async () => ({ + ok: true as const, + value: { + name: "compatible-endpoint", + type: "openai", + credentialKeys: [NO_AUTH_CREDENTIAL_ENV], + configKeys: ["OPENAI_BASE_URL"], + revision: null, + }, + })), + } as unknown as OpenShellProviderAdapter; + const deps = createDeps({ + config: CONFIG, + entry: noAuthEntry(), + session: noAuthSession(), + captureOpenshell, + providerAdapter, + }); + + await expect( + runInferenceSet({ provider: "compatible-endpoint", model: "model-b" }, deps), + ).rejects.toThrow(/without a revision/); + + expect(inferenceSetArgs(captureOpenshell)).toEqual([]); + expect(providerMutationArgs(captureOpenshell)).toEqual([]); + expect(deps.calls.probeSandboxRoute).not.toHaveBeenCalled(); + expect(deps.calls.updateSandbox).not.toHaveBeenCalled(); + }); + it("refuses an absent provider before selecting the route with no endpoint options", async () => { const captureOpenshell = noAuthProviderCapture({ initiallyPresent: false }); const deps = createDeps({ diff --git a/src/lib/actions/inference-set-provider-diagnostics.test.ts b/src/lib/actions/inference-set-provider-diagnostics.test.ts index 094d82256d6..ff73ed0fcce 100644 --- a/src/lib/actions/inference-set-provider-diagnostics.test.ts +++ b/src/lib/actions/inference-set-provider-diagnostics.test.ts @@ -2,6 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 import { describe, expect, it, vi } from "vitest"; +import type { + OpenShellProviderAdapter, + OpenShellProviderResult, + OpenShellProviderInventory, +} from "../adapters/openshell/provider-adapter"; import { classifyGatewayProviderNames, isBridgeProviderName } from "../credentials/provider-list"; import { queryRegisteredGatewayProviders } from "./inference-set-provider-diagnostics"; @@ -9,21 +14,30 @@ const STATIC_WARNING = " ⚠ Could not query registered OpenShell providers while formatting the failure."; describe("inference set provider diagnostics", () => { - it("returns sorted gateway credentials and excludes messaging providers (#5924)", () => { - const captureOpenshell = vi.fn(() => ({ - status: 0, - output: "nvidia-prod\nalpha-telegram-bridge\nanthropic-prod\n", - })); + function adapterWithList( + result: OpenShellProviderResult, + ): OpenShellProviderAdapter { + return { + listProviders: vi.fn(async () => result), + } as unknown as OpenShellProviderAdapter; + } + + it("returns sorted gateway credentials and excludes messaging providers (#5924)", async () => { + const providerAdapter = adapterWithList({ + ok: true, + value: { + names: ["nvidia-prod", "alpha-telegram-bridge", "anthropic-prod"], + }, + }); const log = vi.fn(); - expect(queryRegisteredGatewayProviders({ captureOpenshell, log })).toEqual([ + await expect(queryRegisteredGatewayProviders({ providerAdapter, log })).resolves.toEqual([ "anthropic-prod", "nvidia-prod", ]); - expect(captureOpenshell).toHaveBeenCalledWith(["provider", "list", "--names"], { - ignoreError: true, - maxBuffer: 64 * 1024, - timeout: 5_000, + expect(providerAdapter.listProviders).toHaveBeenCalledWith({ + target: { kind: "selected" }, + timeoutMs: 5_000, }); expect(log).not.toHaveBeenCalled(); }); @@ -33,9 +47,10 @@ describe("inference set provider diagnostics", () => { bridgeNames: [], credentialNames: [], }); - expect( - classifyGatewayProviderNames(["alpha-telegram-bridge", "alpha-slack-app"]), - ).toEqual({ bridgeNames: ["alpha-telegram-bridge", "alpha-slack-app"], credentialNames: [] }); + expect(classifyGatewayProviderNames(["alpha-telegram-bridge", "alpha-slack-app"])).toEqual({ + bridgeNames: ["alpha-telegram-bridge", "alpha-slack-app"], + credentialNames: [], + }); expect(isBridgeProviderName("alpha-discord-bridge")).toBe(true); expect(isBridgeProviderName("nvidia-prod")).toBe(false); }); @@ -43,35 +58,33 @@ describe("inference set provider diagnostics", () => { it.each([ { name: "thrown capture error", - capture: () => { + list: async () => { throw new Error("query-secret"); }, }, { name: "timeout", - capture: () => ({ - status: null, - output: "partial-timeout-provider", - error: Object.assign(new Error("query-secret"), { code: "ETIMEDOUT" }), + list: async () => ({ + ok: false as const, + error: { kind: "timeout" as const, message: "safe timeout" }, }), }, { - name: "buffer overflow", - capture: () => ({ - status: null, - output: "partial-overflow-provider", - error: Object.assign(new Error("query-secret"), { code: "ENOBUFS" }), + name: "command failure", + list: async () => ({ + ok: false as const, + error: { kind: "command" as const, reason: "failed" as const, message: "safe failure" }, }), }, - { - name: "nonzero status", - capture: () => ({ status: 17, output: "query-secret" }), - }, - ])("uses the static fallback for $name", ({ capture }) => { - const captureOpenshell = vi.fn(capture); + ])("uses the static fallback for $name", async ({ list }) => { + const providerAdapter = { + listProviders: vi.fn(list), + } as unknown as OpenShellProviderAdapter; const log = vi.fn(); - expect(queryRegisteredGatewayProviders({ captureOpenshell, log })).toBeUndefined(); + await expect( + queryRegisteredGatewayProviders({ providerAdapter, log }), + ).resolves.toBeUndefined(); expect(log).toHaveBeenCalledWith(STATIC_WARNING); expect(log).not.toHaveBeenCalledWith(expect.stringContaining("query-secret")); }); diff --git a/src/lib/actions/inference-set-provider-diagnostics.ts b/src/lib/actions/inference-set-provider-diagnostics.ts index 64c8c70a663..a41fa5c3030 100644 --- a/src/lib/actions/inference-set-provider-diagnostics.ts +++ b/src/lib/actions/inference-set-provider-diagnostics.ts @@ -1,37 +1,32 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import type { CaptureOpenshellOptions, CaptureOpenshellResult } from "../adapters/openshell/client"; -import { parseCliOpenShellProviderNames } from "../adapters/openshell/provider-command"; +import type { CaptureOpenshellResult } from "../adapters/openshell/client"; +import type { OpenShellProviderAdapter } from "../adapters/openshell/provider-adapter"; +import { selectedOpenShellGateway } from "../adapters/openshell/sandbox-observer"; import { classifyGatewayProviderNames } from "../credentials/provider-list"; import { buildOpenshellInferenceSetFailureMessage, - OPEN_SHELL_FAILURE_CAPTURE_MAX_BUFFER, openshellReportsProviderNotFound, } from "./inference-set-error"; const OPEN_SHELL_DIAGNOSTIC_TIMEOUT_MS = 5_000; interface ProviderDiagnosticDeps { - captureOpenshell: ( - args: string[], - opts?: Pick, - ) => CaptureOpenshellResult; + providerAdapter: OpenShellProviderAdapter; log: (message: string) => void; } -export function queryRegisteredGatewayProviders( +export async function queryRegisteredGatewayProviders( deps: ProviderDiagnosticDeps, -): string[] | undefined { +): Promise { try { - const result = deps.captureOpenshell(["provider", "list", "--names"], { - ignoreError: true, - maxBuffer: OPEN_SHELL_FAILURE_CAPTURE_MAX_BUFFER, - timeout: OPEN_SHELL_DIAGNOSTIC_TIMEOUT_MS, + const result = await deps.providerAdapter.listProviders({ + target: selectedOpenShellGateway(), + timeoutMs: OPEN_SHELL_DIAGNOSTIC_TIMEOUT_MS, }); - if (result.status === 0) { - return classifyGatewayProviderNames(parseCliOpenShellProviderNames(result.output)) - .credentialNames; + if (result.ok) { + return classifyGatewayProviderNames(result.value.names).credentialNames; } } catch (_error: unknown) { // #5924: intentionally treat every thrown query or parsing error identically. @@ -42,11 +37,11 @@ export function queryRegisteredGatewayProviders( return undefined; } -export function buildInferenceSetFailure( +export async function buildInferenceSetFailure( setResult: CaptureOpenshellResult, provider: string, deps: ProviderDiagnosticDeps, -): { exitCode: number; message: string } { +): Promise<{ exitCode: number; message: string }> { const stderr = typeof setResult.stderr === "string" ? setResult.stderr : ""; const stdout = typeof setResult.stdout === "string" ? setResult.stdout : ""; const providerNotFound = openshellReportsProviderNotFound(`${stderr}\n${stdout}`, provider); @@ -56,7 +51,9 @@ export function buildInferenceSetFailure( message: buildOpenshellInferenceSetFailureMessage({ exitCode, providerNotFound, - registeredProviders: providerNotFound ? queryRegisteredGatewayProviders(deps) : undefined, + registeredProviders: providerNotFound + ? await queryRegisteredGatewayProviders(deps) + : undefined, stderr, stdout, }), diff --git a/src/lib/actions/inference-set-provider.test.ts b/src/lib/actions/inference-set-provider.test.ts index a9f653ef7e2..6a531c8776a 100644 --- a/src/lib/actions/inference-set-provider.test.ts +++ b/src/lib/actions/inference-set-provider.test.ts @@ -2,9 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 import { afterEach, describe, expect, it, vi } from "vitest"; +import type { OpenShellProviderAdapter } from "../adapters/openshell/provider-adapter"; +import { createCliOpenShellProviderAdapter } from "../adapters/openshell/provider-adapter-cli"; import { OPENSHELL_OPERATION_TIMEOUT_MS } from "../adapters/openshell/provider-command"; import type { InferenceSetDeps } from "./inference-set"; -import { __test, prepareInferenceSetProviderBinding } from "./inference-set-provider"; +import { prepareInferenceSetProviderBinding } from "./inference-set-provider"; import type { HttpsPinProviderBinding } from "./inference-set-route-containment"; const PROVIDER_ID = "11111111-2222-4333-8444-555555555555"; @@ -66,10 +68,25 @@ function captureSequence( ) as InferenceSetDeps["captureOpenshell"] & ReturnType; } +function providerAdapterFromCapture( + captureOpenshell: InferenceSetDeps["captureOpenshell"], +): OpenShellProviderAdapter { + return createCliOpenShellProviderAdapter({ + run: (args, options) => + captureOpenshell(args, { + ...(options.env ? { env: options.env } : {}), + ignoreError: true, + includeStreams: true, + ...(options.maxBuffer ? { maxBuffer: options.maxBuffer } : {}), + timeout: options.timeout, + }), + }); +} + describe("inference set provider binding", () => { afterEach(() => vi.unstubAllEnvs()); - it("updates an owned provider with only the route token in invocation-local env", () => { + it("updates an owned provider with only the route token in invocation-local env", async () => { vi.stubEnv("COMPATIBLE_API_KEY", "real-upstream-secret"); const before = providerOutput({ resourceVersion: 4 }); const after = providerOutput({ resourceVersion: 5 }); @@ -79,13 +96,13 @@ describe("inference set provider binding", () => { { status: 0, stdout: after, stderr: "", output: after }, ]); - const mutation = prepareInferenceSetProviderBinding({ + const mutation = await prepareInferenceSetProviderBinding({ gatewayName: "nemoclaw", providerName: "compatible-endpoint", binding: binding(), - captureOpenshell: capture, + providerAdapter: providerAdapterFromCapture(capture), }); - mutation.commit(); + await mutation.commit(); expect(capture.mock.calls[1][0]).toEqual([ "provider", @@ -122,7 +139,7 @@ describe("inference set provider binding", () => { expect(JSON.stringify(binding())).not.toContain("real-upstream-secret"); }); - it("creates an absent provider and verifies its new identity", () => { + it("creates an absent provider and verifies its new identity", async () => { const after = providerOutput({ resourceVersion: 1 }); const capture = captureSequence([ { status: 1, stdout: "", stderr: "Provider 'compatible-endpoint' not found" }, @@ -130,19 +147,19 @@ describe("inference set provider binding", () => { { status: 0, stdout: after, stderr: "" }, ]); - expect(() => + await expect( prepareInferenceSetProviderBinding({ gatewayName: "nemoclaw", providerName: "compatible-endpoint", binding: binding(), - captureOpenshell: capture, + providerAdapter: providerAdapterFromCapture(capture), }), - ).not.toThrow(); + ).resolves.toBeDefined(); expect(capture.mock.calls[1][0]).toContain("profile"); expect(capture.mock.calls[2][0]).toContain("create"); }); - it("stops before an OpenAI provider mutation when profile registration fails (#9895)", () => { + it("stops before an OpenAI provider mutation when profile registration fails (#9895)", async () => { const before = providerOutput({ resourceVersion: 4 }); const responses = [ { status: 0, stdout: before, stderr: "", output: before }, @@ -157,20 +174,20 @@ describe("inference set provider binding", () => { })(), ) as InferenceSetDeps["captureOpenshell"] & ReturnType; - const mutation = prepareInferenceSetProviderBinding({ + const mutation = await prepareInferenceSetProviderBinding({ gatewayName: "nemoclaw", providerName: "compatible-endpoint", binding: binding(), - captureOpenshell: capture, + providerAdapter: providerAdapterFromCapture(capture), }); - expect(() => mutation.commit()).toThrow( + await expect(mutation.commit()).rejects.toThrow( "could not import the checked-in 'openai' inference provider profile", ); expect(capture.mock.calls.map(([args]) => args[1])).toEqual(["get", "profile", "profile"]); }); - it("does not register the OpenAI profile before an Anthropic provider mutation", () => { + it("does not register the OpenAI profile before an Anthropic provider mutation", async () => { const after = providerOutput({ resourceVersion: 1, providerName: "compatible-anthropic-endpoint", @@ -184,17 +201,17 @@ describe("inference set provider binding", () => { { status: 0, stdout: after, stderr: "", output: after }, ]); - prepareInferenceSetProviderBinding({ + await prepareInferenceSetProviderBinding({ gatewayName: "nemoclaw", providerName: "compatible-anthropic-endpoint", binding: binding({ providerType: "anthropic", credentialEnv: "ANTHROPIC_API_KEY" }), - captureOpenshell: capture, + providerAdapter: providerAdapterFromCapture(capture), }); expect(capture.mock.calls.map(([args]) => args[1])).toEqual(["get", "create", "get"]); }); - it("creates a provider after the OpenShell 0.0.99 generic lookup miss (#7725)", () => { + it("creates a provider after the OpenShell 0.0.99 generic lookup miss (#7725)", async () => { const after = providerOutput({ resourceVersion: 1 }); const capture = captureSequence([ { @@ -207,11 +224,11 @@ describe("inference set provider binding", () => { { status: 0, stdout: after, stderr: "" }, ]); - const mutation = prepareInferenceSetProviderBinding({ + const mutation = await prepareInferenceSetProviderBinding({ gatewayName: "nemoclaw", providerName: "compatible-endpoint", binding: binding(), - captureOpenshell: capture, + providerAdapter: providerAdapterFromCapture(capture), }); expect(mutation.action).toBe("create"); @@ -219,7 +236,7 @@ describe("inference set provider binding", () => { expect(capture.mock.calls[2][0]).toContain("create"); }); - it("removes a newly created provider when the caller rolls back", () => { + it("removes a newly created provider when the caller rolls back", async () => { const after = providerOutput({ resourceVersion: 1 }); const capture = captureSequence([ { status: 1, stdout: "", stderr: "Provider 'compatible-endpoint' not found" }, @@ -229,13 +246,13 @@ describe("inference set provider binding", () => { { status: 1, stdout: "", stderr: "Provider 'compatible-endpoint' not found" }, ]); - const mutation = prepareInferenceSetProviderBinding({ + const mutation = await prepareInferenceSetProviderBinding({ gatewayName: "nemoclaw", providerName: "compatible-endpoint", binding: binding(), - captureOpenshell: capture, + providerAdapter: providerAdapterFromCapture(capture), }); - mutation.rollback(); + await mutation.rollback(); expect(mutation.action).toBe("create"); expect(capture.mock.calls[4][0]).toEqual([ @@ -250,39 +267,38 @@ describe("inference set provider binding", () => { it.each([ ["same resource version", PROVIDER_ID, 4], ["delete and recreate", "aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee", 5], - ])("fails closed on update identity drift: %s", (_label, id, resourceVersion) => { + ])("fails closed on update identity drift: %s", async (_label, id, resourceVersion) => { const capture = captureSequence([ { status: 0, stdout: providerOutput({ resourceVersion: 4 }), stderr: "" }, { status: 0, stdout: "", stderr: "" }, { status: 0, stdout: providerOutput({ id, resourceVersion }), stderr: "" }, ]); - expect(() => - prepareInferenceSetProviderBinding({ - gatewayName: "nemoclaw", - providerName: "compatible-endpoint", - binding: binding(), - captureOpenshell: capture, - }).commit(), - ).toThrow("may be partial"); + const mutation = await prepareInferenceSetProviderBinding({ + gatewayName: "nemoclaw", + providerName: "compatible-endpoint", + binding: binding(), + providerAdapter: providerAdapterFromCapture(capture), + }); + await expect(mutation.commit()).rejects.toThrow("may be partial"); }); - it("fails closed when provider metadata is malformed or foreign", () => { + it("fails closed when provider metadata is malformed or foreign", async () => { const malformed = providerOutput({ resourceVersion: 4, credentialKey: "FOREIGN_TOKEN" }); const capture = captureSequence([{ status: 0, stdout: malformed, stderr: "" }]); - expect(() => + await expect( prepareInferenceSetProviderBinding({ gatewayName: "nemoclaw", providerName: "compatible-endpoint", binding: binding(), - captureOpenshell: capture, + providerAdapter: providerAdapterFromCapture(capture), }), - ).toThrow("malformed, foreign"); + ).rejects.toThrow("malformed, foreign"); expect(capture).toHaveBeenCalledTimes(1); }); - it("treats a nonzero mutation as ambiguous and never infers success from post-state", () => { + it("treats a nonzero mutation as ambiguous and never infers success from post-state", async () => { const before = providerOutput({ resourceVersion: 4 }); const after = providerOutput({ resourceVersion: 5 }); const capture = captureSequence([ @@ -291,17 +307,16 @@ describe("inference set provider binding", () => { { status: 0, stdout: after, stderr: "" }, ]); - expect(() => - prepareInferenceSetProviderBinding({ - gatewayName: "nemoclaw", - providerName: "compatible-endpoint", - binding: binding(), - captureOpenshell: capture, - }).commit(), - ).toThrow("may have partially applied"); + const mutation = await prepareInferenceSetProviderBinding({ + gatewayName: "nemoclaw", + providerName: "compatible-endpoint", + binding: binding(), + providerAdapter: providerAdapterFromCapture(capture), + }); + await expect(mutation.commit()).rejects.toThrow("may have partially applied"); }); - it("keeps route credentials isolated across independent invocations", () => { + it("keeps route credentials isolated across independent invocations", async () => { const mutations: Array = []; const makeCapture = (id: string): InferenceSetDeps["captureOpenshell"] => { let version = 1; @@ -321,18 +336,24 @@ describe("inference set provider binding", () => { }; }; - prepareInferenceSetProviderBinding({ + const first = await prepareInferenceSetProviderBinding({ gatewayName: "gateway-a", providerName: "compatible-endpoint", binding: binding({ token: "route-token-a" }), - captureOpenshell: makeCapture("aaaaaaaa-2222-4333-8444-555555555555"), - }).commit(); - prepareInferenceSetProviderBinding({ + providerAdapter: providerAdapterFromCapture( + makeCapture("aaaaaaaa-2222-4333-8444-555555555555"), + ), + }); + await first.commit(); + const second = await prepareInferenceSetProviderBinding({ gatewayName: "gateway-b", providerName: "compatible-endpoint", binding: binding({ token: "route-token-b", routeId: "route-b" }), - captureOpenshell: makeCapture("bbbbbbbb-2222-4333-8444-555555555555"), - }).commit(); + providerAdapter: providerAdapterFromCapture( + makeCapture("bbbbbbbb-2222-4333-8444-555555555555"), + ), + }); + await second.commit(); expect(mutations).toEqual([ { COMPATIBLE_API_KEY: "route-token-a" }, @@ -340,15 +361,105 @@ describe("inference set provider binding", () => { ]); }); - it("parses styled identity fields but rejects duplicates and invalid versions", () => { - expect( - __test.parseProviderVersion( - "\u001b[2mId:\u001b[0m 11111111-2222-4333-8444-555555555555\n\u001b[2mResource version:\u001b[0m 7", - ), - ).toEqual({ id: PROVIDER_ID, resourceVersion: 7 }); - expect( - __test.parseProviderVersion(`Id: ${PROVIDER_ID}\nId: ${PROVIDER_ID}\nResource version: 7`), - ).toBeNull(); - expect(__test.parseProviderVersion(`Id: ${PROVIDER_ID}\nResource version: 0`)).toBeNull(); + it("makes update decisions from typed provider results (#9806)", async () => { + const getProvider = vi + .fn() + .mockResolvedValueOnce({ + ok: true, + value: { + name: "compatible-endpoint", + type: "openai", + credentialKeys: ["COMPATIBLE_API_KEY"], + configKeys: ["OPENAI_BASE_URL"], + revision: { id: PROVIDER_ID, resourceVersion: 4 }, + }, + }) + .mockResolvedValueOnce({ + ok: true, + value: { + name: "compatible-endpoint", + type: "openai", + credentialKeys: ["COMPATIBLE_API_KEY"], + configKeys: ["OPENAI_BASE_URL"], + revision: { id: PROVIDER_ID, resourceVersion: 5 }, + }, + }); + const updateProvider = vi.fn(async () => ({ + ok: true, + value: { state: "updated" }, + })); + const providerAdapter = { + getProvider, + updateProvider, + ensureEndpointlessProviderProfile: vi.fn(async () => ({ + ok: true as const, + value: { state: "ready" as const }, + })), + } as unknown as OpenShellProviderAdapter; + + const mutation = await prepareInferenceSetProviderBinding({ + gatewayName: "nemoclaw-18080", + providerName: "compatible-endpoint", + binding: binding(), + providerAdapter, + }); + await mutation.commit(); + + expect(updateProvider).toHaveBeenCalledWith({ + target: { kind: "named", gatewayName: "nemoclaw-18080" }, + providerName: "compatible-endpoint", + credentials: [{ name: "COMPATIBLE_API_KEY", value: "route-token-a" }], + config: [ + { + key: "OPENAI_BASE_URL", + value: "http://host.openshell.internal:11438/route/route-a/v1", + }, + ], + }); + }); + + it("does not infer absence from a typed authentication failure (#9806)", async () => { + const providerAdapter = { + getProvider: vi.fn(async () => ({ + ok: false as const, + error: { + kind: "authentication" as const, + message: "OpenShell could not authenticate the provider operation.", + }, + })), + } as unknown as OpenShellProviderAdapter; + + await expect( + prepareInferenceSetProviderBinding({ + gatewayName: "nemoclaw", + providerName: "compatible-endpoint", + binding: binding(), + providerAdapter, + }), + ).rejects.toThrow("no provider mutation was attempted"); + }); + + it("stops before update when typed metadata has no revision evidence (#9806)", async () => { + const providerAdapter = { + getProvider: vi.fn(async () => ({ + ok: true as const, + value: { + name: "compatible-endpoint", + type: "openai", + credentialKeys: ["COMPATIBLE_API_KEY"], + configKeys: ["OPENAI_BASE_URL"], + revision: null, + }, + })), + } as unknown as OpenShellProviderAdapter; + + await expect( + prepareInferenceSetProviderBinding({ + gatewayName: "nemoclaw", + providerName: "compatible-endpoint", + binding: binding(), + providerAdapter, + }), + ).rejects.toThrow("without a revision"); }); }); diff --git a/src/lib/actions/inference-set-provider.ts b/src/lib/actions/inference-set-provider.ts index 2d1a8cc855b..0cffcc6fa06 100644 --- a/src/lib/actions/inference-set-provider.ts +++ b/src/lib/actions/inference-set-provider.ts @@ -1,16 +1,21 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { type CaptureOpenshellResult, stripAnsi } from "../adapters/openshell/client"; import { - checkOpenAiInferenceProviderProfile, + endpointlessProviderProfileFailureMessages, + endpointlessProviderProfilePath, OPENAI_GATEWAY_PROVIDER_TYPE, } from "../adapters/openshell/provider-profile"; +import type { + OpenShellProviderAdapter, + OpenShellProviderError, + OpenShellProviderMetadata, +} from "../adapters/openshell/provider-adapter"; +import { createCliOpenShellProviderAdapter } from "../adapters/openshell/provider-adapter-cli"; +import { namedOpenShellGateway } from "../adapters/openshell/sandbox-observer"; +import { REPOSITORY_ROOT } from "../core/repository-root"; import { retryUntilAsync } from "../core/retry"; -import { - matchesGatewayProviderBinding, - parseGatewayProviderMetadata, -} from "../onboard/gateway-provider-metadata"; +import { matchesGatewayProviderBinding } from "../onboard/gateway-provider-metadata"; import { assertHermesPortableCommandUnavailable } from "../onboard/experimental/portable-agent-lifecycle"; import { CURRENT_RUNTIME_PROVIDER_BUNDLES, @@ -20,11 +25,7 @@ import { requireRuntimeProviderMutationAuthority, } from "../onboard/runtime-provider/access"; import type { SandboxEntry } from "../state/registry"; -import { - InferenceSetError, - OPEN_SHELL_FAILURE_CAPTURE_MAX_BUFFER, - openshellReportsProviderNotFound, -} from "./inference-set-error"; +import { InferenceSetError } from "./inference-set-error"; import type { InferenceSetProviderBinding } from "./inference-set-route-containment"; import type { SandboxInferenceInvocationInput, @@ -33,6 +34,11 @@ import type { export type { RuntimeProviderBundleRegistry }; export { RuntimeProviderSelectionError }; +export type InferenceSetProviderAdapter = OpenShellProviderAdapter; + +export function createDefaultInferenceSetProviderAdapter(): OpenShellProviderAdapter { + return createCliOpenShellProviderAdapter(); +} export type InferenceSetSandboxRouteProbe = ( input: SandboxInferenceInvocationInput, @@ -115,17 +121,6 @@ export function assertInferenceSetCommandAvailable(sandboxName: string): void { assertHermesPortableCommandUnavailable(sandboxName, "inference:set"); } -type CaptureProviderCommand = ( - args: string[], - options: { - ignoreError: true; - includeStreams: true; - maxBuffer: number; - timeout?: number; - env?: NodeJS.ProcessEnv; - }, -) => CaptureOpenshellResult; - type ProviderSurface = { type: "openai" | "anthropic"; configKey: "OPENAI_BASE_URL" | "ANTHROPIC_BASE_URL"; @@ -135,77 +130,33 @@ type ProviderObservation = | { kind: "absent" } | { kind: "present"; - id: string; - resourceVersion: number; - metadata: NonNullable>; + metadata: OpenShellProviderMetadata; } - | { kind: "error"; status: number | null }; + | { kind: "error"; error: OpenShellProviderError }; -function providerSurface(providerType: InferenceSetProviderBinding["providerType"]): ProviderSurface { +function providerSurface( + providerType: InferenceSetProviderBinding["providerType"], +): ProviderSurface { return providerType === "anthropic" ? { type: "anthropic", configKey: "ANTHROPIC_BASE_URL" } : { type: "openai", configKey: "OPENAI_BASE_URL" }; } -function resultText(result: CaptureOpenshellResult): string { - // includeStreams=true normally makes `output` a duplicate aggregate of - // stdout/stderr. Parse the split streams when present and use `output` only - // as the compatibility fallback so strict duplicate-field checks keep - // working on normal OpenShell results. - const hasStreams = result.stdout !== undefined || result.stderr !== undefined; - const combined = hasStreams - ? `${result.stdout ?? ""}\n${result.stderr ?? ""}` - : String(result.output ?? ""); - return Buffer.from(combined, "utf8") - .subarray(0, OPEN_SHELL_FAILURE_CAPTURE_MAX_BUFFER) - .toString("utf8"); -} - -function parseProviderVersion(output: string): { id: string; resourceVersion: number } | null { - const clean = stripAnsi(output); - const ids = Array.from(clean.matchAll(/^\s*Id:\s*([A-Za-z0-9._:-]{1,128})\s*$/gimu)); - const versions = Array.from(clean.matchAll(/^\s*Resource version:\s*([0-9]+)\s*$/gimu)); - if (ids.length !== 1 || versions.length !== 1) return null; - const resourceVersion = Number(versions[0][1]); - if (!Number.isSafeInteger(resourceVersion) || resourceVersion < 1) return null; - return { id: ids[0][1], resourceVersion }; -} - -function inspectProvider( - captureOpenshell: CaptureProviderCommand, +async function inspectProvider( + providerAdapter: OpenShellProviderAdapter, gatewayName: string, providerName: string, -): ProviderObservation { - const result = captureOpenshell(["provider", "get", "-g", gatewayName, providerName], { - ignoreError: true, - includeStreams: true, - maxBuffer: OPEN_SHELL_FAILURE_CAPTURE_MAX_BUFFER, +): Promise { + const result = await providerAdapter.getProvider({ + target: namedOpenShellGateway(gatewayName), + providerName, }); - const output = resultText(result); - if (result.status !== 0) { - return providerLookupReportsNotFound(output, providerName) + if (!result.ok) { + return result.error.kind === "command" && result.error.reason === "not_found" ? { kind: "absent" } - : { kind: "error", status: result.status }; + : { kind: "error", error: result.error }; } - const metadata = parseGatewayProviderMetadata(output); - const version = parseProviderVersion(output); - if (!metadata || !version) return { kind: "error", status: result.status }; - return { kind: "present", ...version, metadata }; -} - -function providerLookupReportsNotFound(output: string, providerName: string): boolean { - if (openshellReportsProviderNotFound(output, providerName)) return true; - // OpenShell 0.0.99 omits the name only from this exact-name `provider get` - // command. Keep the route-update parser strict because its output can name - // a different missing provider. - return stripAnsi(output) - .toLowerCase() - .split("\n") - .some( - (line) => - /code:\s*['"]some requested entity was not found['"]/u.test(line) && - /message:\s*['"]provider not found['"]/u.test(line), - ); + return { kind: "present", metadata: result.value }; } function expectedShape(providerName: string, surface: ProviderSurface, credentialEnv: string) { @@ -243,7 +194,7 @@ function assertProviderOwnership(options: { } if (observation.kind === "error") { throw new InferenceSetError( - `Could not inspect provider '${providerName}' (status ${observation.status ?? "unknown"}); no provider mutation was attempted.`, + `Could not inspect provider '${providerName}'; no provider mutation was attempted. ${observation.error.message}`, 1, ); } @@ -261,34 +212,19 @@ function assertProviderOwnership(options: { return "update"; } -function mutationArgs(options: { - action: "create" | "update"; - gatewayName: string; - providerName: string; - surface: ProviderSurface; - credentialEnv: string; - baseUrl: string; -}): string[] { - const args = - options.action === "create" - ? [ - "provider", - "create", - "-g", - options.gatewayName, - "--name", - options.providerName, - "--type", - options.surface.type, - ] - : ["provider", "update", "-g", options.gatewayName, options.providerName]; - args.push( - "--credential", - options.credentialEnv, - "--config", - `${options.surface.configKey}=${options.baseUrl}`, - ); - return args; +function profileFailureMessage(error: OpenShellProviderError): string { + if (error.kind !== "command") return error.message; + const reason = + error.reason === "profile_import_failed" + ? "import-failed" + : error.reason === "profile_export_failed" + ? "export-failed" + : error.reason === "profile_incompatible" + ? "incompatible" + : null; + return reason + ? endpointlessProviderProfileFailureMessages(reason).join("\n").trim() + : error.message; } /** @@ -298,19 +234,26 @@ function mutationArgs(options: { * that name, so a same-name foreign or malformed binding has to be rejected * before the selection, not only when a provider mutation is prepared. */ -export function assertInferenceSetProviderOwnership(options: { +export async function assertInferenceSetProviderOwnership(options: { gatewayName: string; providerName: string; providerType: InferenceSetProviderBinding["providerType"]; credentialEnv: string; - captureOpenshell: CaptureProviderCommand; -}): void { + providerAdapter: OpenShellProviderAdapter; +}): Promise { + const observation = await inspectProvider( + options.providerAdapter, + options.gatewayName, + options.providerName, + ); + if (observation.kind === "present" && observation.metadata.revision == null) { + throw new InferenceSetError( + `Could not inspect provider '${options.providerName}'; no inference route mutation was attempted. OpenShell returned provider metadata without a revision.`, + 1, + ); + } assertProviderOwnership({ - observation: inspectProvider( - options.captureOpenshell, - options.gatewayName, - options.providerName, - ), + observation, providerName: options.providerName, surface: providerSurface(options.providerType), credentialEnv: options.credentialEnv, @@ -318,17 +261,24 @@ export function assertInferenceSetProviderOwnership(options: { }); } -export function prepareInferenceSetProviderBinding(options: { +export type PreparedInferenceSetProviderBinding = Readonly<{ + action: "create" | "update"; + commit: () => Promise; + rollback: () => Promise; +}>; + +export async function prepareInferenceSetProviderBinding(options: { gatewayName: string; providerName: string; binding: InferenceSetProviderBinding; - captureOpenshell: CaptureProviderCommand; + providerAdapter: OpenShellProviderAdapter; /** False when only onboarding can rebuild this provider's binding. */ allowCreate?: boolean; -}): { action: "create" | "update"; commit: () => void; rollback: () => void } { - const { gatewayName, providerName, binding, captureOpenshell } = options; +}): Promise { + const { gatewayName, providerName, binding, providerAdapter } = options; + const target = namedOpenShellGateway(gatewayName); const surface = providerSurface(binding.providerType); - const before = inspectProvider(captureOpenshell, gatewayName, providerName); + const before = await inspectProvider(providerAdapter, gatewayName, providerName); const action = assertProviderOwnership({ observation: before, providerName, @@ -336,57 +286,59 @@ export function prepareInferenceSetProviderBinding(options: { credentialEnv: binding.credentialEnv, allowCreate: options.allowCreate !== false, }); + if (action === "update" && (before.kind !== "present" || before.metadata.revision == null)) { + throw new InferenceSetError( + `Could not inspect provider '${providerName}'; no provider mutation was attempted. OpenShell returned provider metadata without a revision.`, + 1, + ); + } - const apply = (): void => { + const apply = async (): Promise => { if (surface.type === OPENAI_GATEWAY_PROVIDER_TYPE) { - const profile = checkOpenAiInferenceProviderProfile({ - runOpenshell: (args, runnerOptions) => - captureOpenshell( - args[0] === "provider" && args[1] === "profile" - ? [args[0], args[1], "-g", gatewayName, ...args.slice(2)] - : args, - { - ignoreError: true, - includeStreams: true, - maxBuffer: OPEN_SHELL_FAILURE_CAPTURE_MAX_BUFFER, - timeout: runnerOptions?.timeout, - }, - ), + const profile = await providerAdapter.ensureEndpointlessProviderProfile({ + target, + profileType: OPENAI_GATEWAY_PROVIDER_TYPE, + inferenceCapable: true, + profilePath: endpointlessProviderProfilePath(REPOSITORY_ROOT, OPENAI_GATEWAY_PROVIDER_TYPE), }); if (!profile.ok) { - throw new InferenceSetError(profile.messages.join("\n").trim(), 1); + throw new InferenceSetError(profileFailureMessage(profile.error), 1); } } - const result = captureOpenshell( - mutationArgs({ - action, - gatewayName, - providerName, - surface, - credentialEnv: binding.credentialEnv, - baseUrl: binding.baseUrl, - }), - { - ignoreError: true, - includeStreams: true, - maxBuffer: OPEN_SHELL_FAILURE_CAPTURE_MAX_BUFFER, - env: { [binding.credentialEnv]: binding.token }, - }, - ); - const after = inspectProvider(captureOpenshell, gatewayName, providerName); - if (result.status !== 0) { + const credentials = [{ name: binding.credentialEnv, value: binding.token }]; + const config = [{ key: surface.configKey, value: binding.baseUrl }]; + const result = + action === "create" + ? await providerAdapter.createProvider({ + target, + name: providerName, + type: surface.type, + credentials, + config, + fromExisting: false, + }) + : await providerAdapter.updateProvider({ + target, + providerName, + credentials, + config, + }); + const after = await inspectProvider(providerAdapter, gatewayName, providerName); + if (!result.ok) { throw new InferenceSetError( - `Failed to ${action} provider '${providerName}' on gateway '${gatewayName}' (status ${result.status ?? "unknown"}). ` + + `Failed to ${action} provider '${providerName}' on gateway '${gatewayName}'. ` + `The provider command may have partially applied; retry this command or re-run onboarding to converge the requested binding.`, 1, ); } if ( after.kind !== "present" || + after.metadata.revision == null || (action === "update" && (before.kind !== "present" || - after.id !== before.id || - after.resourceVersion <= before.resourceVersion)) || + before.metadata.revision == null || + after.metadata.revision.id !== before.metadata.revision.id || + after.metadata.revision.resourceVersion <= before.metadata.revision.resourceVersion)) || !matchesGatewayProviderBinding( after.metadata, expectedShape(providerName, surface, binding.credentialEnv), @@ -404,22 +356,21 @@ export function prepareInferenceSetProviderBinding(options: { return { action, commit: apply, - rollback: () => {}, + rollback: async () => {}, }; } - apply(); + await apply(); return { action, - commit: () => {}, - rollback: () => { - const result = captureOpenshell(["provider", "delete", "-g", gatewayName, providerName], { - ignoreError: true, - includeStreams: true, - maxBuffer: OPEN_SHELL_FAILURE_CAPTURE_MAX_BUFFER, + commit: async () => {}, + rollback: async () => { + const result = await providerAdapter.deleteProvider({ + target, + providerName, }); - const restored = inspectProvider(captureOpenshell, gatewayName, providerName); - if (result.status !== 0 || restored.kind !== "absent") { + const restored = await inspectProvider(providerAdapter, gatewayName, providerName); + if (!result.ok || restored.kind !== "absent") { throw new InferenceSetError( `Failed to remove newly created provider '${providerName}' after inference selection failed.`, 1, @@ -431,8 +382,5 @@ export function prepareInferenceSetProviderBinding(options: { export const __test = { inspectProvider, - parseProviderVersion, providerSurface, - providerLookupReportsNotFound, - mutationArgs, }; diff --git a/src/lib/actions/inference-set.test-support.ts b/src/lib/actions/inference-set.test-support.ts index 98f575c18de..18a78ec9206 100644 --- a/src/lib/actions/inference-set.test-support.ts +++ b/src/lib/actions/inference-set.test-support.ts @@ -2,6 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 import { vi } from "vitest"; +import type { OpenShellProviderAdapter } from "../adapters/openshell/provider-adapter"; +import { createCliOpenShellProviderAdapter } from "../adapters/openshell/provider-adapter-cli"; import type { ValidationResult } from "../inference/local"; import type { AgentConfigTarget } from "../sandbox/config"; import type { ConfigObject, ConfigValue } from "../security/credential-filter"; @@ -165,6 +167,7 @@ export function createDeps(options: { session?: Session | null; openshellStatus?: number; captureOpenshell?: InferenceSetDeps["captureOpenshell"]; + providerAdapter?: OpenShellProviderAdapter; localValidation?: ValidationResult; localReachable?: boolean; contextWindow?: number | null; @@ -243,7 +246,7 @@ export function createDeps(options: { ), resolveCredentialValue: vi.fn( options.resolveCredentialValue ?? - ((credentialEnv: string) => process.env[credentialEnv] ?? ""), + ((credentialEnv: string) => process.env[credentialEnv] ?? "test-credential-value"), ), ensureHttpsPinRuntimeAdapter: vi.fn( options.ensureHttpsPinRuntimeAdapter ?? @@ -275,6 +278,25 @@ export function createDeps(options: { await operation()), ), }; + const providerAdapter = + options.providerAdapter ?? + createCliOpenShellProviderAdapter({ + run: (args, runOptions) => { + const result = calls.captureOpenshell(args, { + ...(runOptions.env ? { env: runOptions.env } : {}), + ignoreError: true, + includeStreams: true, + ...(runOptions.maxBuffer ? { maxBuffer: runOptions.maxBuffer } : {}), + timeout: runOptions.timeout, + }); + return { + status: result.status, + stdout: result.stdout || result.stderr ? result.stdout : result.output, + stderr: result.stderr, + ...("error" in result && result.error ? { error: result.error } : {}), + }; + }, + }); return { getDefaultSandbox: () => defaultSandbox, getSandbox: (name: string) => sandboxes[name] ?? null, @@ -290,6 +312,7 @@ export function createDeps(options: { seedHermesDashboardConfig: calls.seedHermesDashboardConfig, prepareRunOpenshell: calls.prepareRunOpenshell, captureOpenshell: calls.captureOpenshell, + providerAdapter, appendAuditEntry: calls.appendAuditEntry, log: calls.log, isLocalInferenceProvider: (provider) => diff --git a/src/lib/actions/inference-set.ts b/src/lib/actions/inference-set.ts index edb26b9056f..23ce94cacc1 100644 --- a/src/lib/actions/inference-set.ts +++ b/src/lib/actions/inference-set.ts @@ -27,10 +27,7 @@ import { resolveReasoningEffortRequest, } from "../inference/selection"; import { resolveSandboxGatewayName } from "../onboard/gateway-binding"; -import { - matchesGatewayProviderBinding, - parseGatewayProviderMetadata, -} from "../onboard/gateway-provider-metadata"; +import { matchesGatewayProviderBinding } from "../onboard/gateway-provider-metadata"; import { ensureLocalProviderReachable } from "../onboard/local-inference-topology"; import { assertNoOpenShellGatewayEndpointOverride, @@ -73,9 +70,11 @@ import { } from "./inference-set-gateway-restart"; import { type InferenceSetSandboxRouteProbe, + type InferenceSetProviderAdapter, assertInferenceSetCommandAvailable, assertInferenceSetProviderOwnership, prepareInferenceSetProviderBinding, + createDefaultInferenceSetProviderAdapter, probeInferenceSetSandboxRoute, probeInferenceSetSandboxRouteUntilConverged, type RuntimeProviderBundleRegistry, @@ -163,6 +162,7 @@ export interface InferenceSetDeps extends InferenceGatewayRestartDeps { "env" | "ignoreError" | "includeStreams" | "maxBuffer" | "timeout" >, ) => CaptureOpenshellResult; + providerAdapter: InferenceSetProviderAdapter; isLocalInferenceProvider: (provider: string) => boolean; validateLocalProvider: (provider: string) => ValidationResult; ensureLocalProviderReachable: (provider: string) => boolean; @@ -267,6 +267,7 @@ function defaultDeps(): InferenceSetDeps { getOpenshellBinary(); }, captureOpenshell: (args, opts) => captureOpenshell(args, opts), + providerAdapter: createDefaultInferenceSetProviderAdapter(), appendAuditEntry, log: console.log, isLocalInferenceProvider: (provider) => @@ -709,7 +710,7 @@ function getPreferredInferenceApi(config: ConfigObject): string | null { return typeof inferenceProvider.api === "string" ? inferenceProvider.api : null; } -function assertHermesCompatibleAnthropicOpenAiProvider( +async function assertHermesCompatibleAnthropicOpenAiProvider( sandboxName: string, agentName: string, gatewayName: string, @@ -717,7 +718,7 @@ function assertHermesCompatibleAnthropicOpenAiProvider( endpointUrl: string | null, deps: InferenceSetDeps, httpsPinProviderBinding: { providerType: "openai" | "anthropic" } | null = null, -): void { +): Promise { if ( agentName !== "hermes" || provider !== "compatible-anthropic-endpoint" || @@ -727,15 +728,13 @@ function assertHermesCompatibleAnthropicOpenAiProvider( } if (httpsPinProviderBinding?.providerType === "openai") return; - const result = deps.captureOpenshell(["provider", "get", "-g", gatewayName, provider], { - ignoreError: true, - includeStreams: true, - maxBuffer: OPEN_SHELL_FAILURE_CAPTURE_MAX_BUFFER, + const result = await deps.providerAdapter.getProvider({ + target: { kind: "named", gatewayName }, + providerName: provider, }); - const output = result.output || `${result.stdout ?? ""}\n${result.stderr ?? ""}`; - const metadata = result.status === 0 ? parseGatewayProviderMetadata(output) : null; if ( - matchesGatewayProviderBinding(metadata, { + result.ok && + matchesGatewayProviderBinding(result.value, { name: provider, type: "openai", credentialKey: "COMPATIBLE_ANTHROPIC_API_KEY", @@ -991,7 +990,7 @@ async function runInferenceSetWithoutHostLock( // `inference set` changes the selected route but cannot change a gateway // provider's protocol type. Fail before mutation when a legacy Anthropic // registration would make the required Hermes OpenAI frontend unroutable. - assertHermesCompatibleAnthropicOpenAiProvider( + await assertHermesCompatibleAnthropicOpenAiProvider( sandboxName, agentName, preparedRoute.gatewayName, @@ -1041,7 +1040,8 @@ async function runInferenceSetWithoutHostLock( let appliedProvider = false; let appliedInferenceSelection = false; let restoredSelectionAfterProviderFailure = false; - let providerMutation: ReturnType | null = null; + let providerMutation: Awaited> | null = + null; const restorePreviousInferenceSelection = (): string | null => { let restoreResult: CaptureOpenshellResult; try { @@ -1070,11 +1070,11 @@ async function runInferenceSetWithoutHostLock( try { const providerBinding = httpsPinProviderBinding ?? directProviderBinding; if (providerBinding) { - providerMutation = prepareInferenceSetProviderBinding({ + providerMutation = await prepareInferenceSetProviderBinding({ gatewayName: preparedRoute.gatewayName, providerName: provider, binding: providerBinding, - captureOpenshell: deps.captureOpenshell, + providerAdapter: deps.providerAdapter, allowCreate: !loopbackNoAuthProxyRoute, }); if (directProviderBinding && providerMutation.action === "update") { @@ -1102,12 +1102,12 @@ async function runInferenceSetWithoutHostLock( // no-auth proxy token that OpenShell holds and sends on selection, and // its host-side verification is skipped, so confirm the durable binding // is still the one onboarding registered before selecting it. - assertInferenceSetProviderOwnership({ + await assertInferenceSetProviderOwnership({ gatewayName: preparedRoute.gatewayName, providerName: provider, providerType: preMutationInferenceApi === "anthropic-messages" ? "anthropic" : "openai", credentialEnv: sandboxCustomCompatibleCredentialEnv(entry, provider), - captureOpenshell: deps.captureOpenshell, + providerAdapter: deps.providerAdapter, }); } if (providerMutation) { @@ -1148,13 +1148,13 @@ async function runInferenceSetWithoutHostLock( setResult = setInferenceRoute(); } if (setResult.status !== 0) { - const failure = buildInferenceSetFailure(setResult, provider, deps); + const failure = await buildInferenceSetFailure(setResult, provider, deps); throw new InferenceSetError(failure.message, failure.exitCode); } appliedInferenceSelection = true; if (providerMutation) { try { - providerMutation.commit(); + await providerMutation.commit(); appliedProvider = true; } catch (providerError) { const providerDetail = @@ -1462,7 +1462,7 @@ async function runInferenceSetWithoutHostLock( const exitCode = error instanceof InferenceSetError ? error.exitCode : 1; if (!appliedInferenceSelection) { try { - providerMutation.rollback(); + await providerMutation.rollback(); } catch (rollbackError) { const rollbackDetail = rollbackError instanceof Error ? rollbackError.message : String(rollbackError); diff --git a/src/lib/adapters/openshell/provider-adapter-cli.test.ts b/src/lib/adapters/openshell/provider-adapter-cli.test.ts index c5e4dda9d52..dedcdd9f356 100644 --- a/src/lib/adapters/openshell/provider-adapter-cli.test.ts +++ b/src/lib/adapters/openshell/provider-adapter-cli.test.ts @@ -23,6 +23,7 @@ describe("CLI OpenShell provider adapter", () => { ).resolves.toEqual({ ok: true, value: { names: ["zeta", "alpha"] } }); expect(run).toHaveBeenCalledWith(["provider", "list", "-g", "nemoclaw-18080", "--names"], { ignoreError: true, + maxBuffer: 64 * 1024, stdio: ["ignore", "pipe", "pipe"], timeout: 4_321, }); @@ -34,7 +35,9 @@ describe("CLI OpenShell provider adapter", () => { 0, [ "Name: search-prod", + "Id: 11111111-2222-4333-8444-555555555555", "Type: tavily", + "Resource version: 7", "Credential keys: TAVILY_API_KEY", "Config keys: ", ].join("\n"), @@ -55,6 +58,10 @@ describe("CLI OpenShell provider adapter", () => { type: "tavily", credentialKeys: ["TAVILY_API_KEY"], configKeys: [], + revision: { + id: "11111111-2222-4333-8444-555555555555", + resourceVersion: 7, + }, }, }); expect(run).toHaveBeenCalledWith(["provider", "get", "-g", "nemoclaw-18080", "search-prod"], { @@ -66,6 +73,72 @@ describe("CLI OpenShell provider adapter", () => { }); }); + it("rejects provider metadata with incomplete revision evidence (#9806)", async () => { + const run = vi.fn(() => + captured( + 0, + [ + "Name: search-prod", + "Id: 11111111-2222-4333-8444-555555555555", + "Type: tavily", + "Credential keys: TAVILY_API_KEY", + "Config keys: ", + ].join("\n"), + ), + ); + const adapter = createCliOpenShellProviderAdapter({ run }); + + await expect( + adapter.getProvider({ + target: namedOpenShellGateway("nemoclaw"), + providerName: "search-prod", + }), + ).resolves.toEqual({ + ok: false, + error: { kind: "schema", message: "OpenShell returned invalid provider metadata." }, + }); + }); + + it.each([ + [ + "duplicate identity", + [ + "Name: search-prod", + "Id: first-id", + "Id: second-id", + "Type: tavily", + "Resource version: 7", + "Credential keys: TAVILY_API_KEY", + "Config keys: ", + ].join("\n"), + ], + [ + "invalid resource version", + [ + "Name: search-prod", + "Id: provider-id", + "Type: tavily", + "Resource version: 0", + "Credential keys: TAVILY_API_KEY", + "Config keys: ", + ].join("\n"), + ], + ])("rejects provider metadata with %s (#9806)", async (_case, output) => { + const adapter = createCliOpenShellProviderAdapter({ + run: vi.fn(() => captured(0, output)), + }); + + await expect( + adapter.getProvider({ + target: selectedOpenShellGateway(), + providerName: "search-prod", + }), + ).resolves.toEqual({ + ok: false, + error: { kind: "schema", message: "OpenShell returned invalid provider metadata." }, + }); + }); + it("distinguishes an exact missing provider from a missing gateway (#9806)", async () => { const run = vi .fn() diff --git a/src/lib/adapters/openshell/provider-adapter-cli.ts b/src/lib/adapters/openshell/provider-adapter-cli.ts index 936609018d4..eebd0559259 100644 --- a/src/lib/adapters/openshell/provider-adapter-cli.ts +++ b/src/lib/adapters/openshell/provider-adapter-cli.ts @@ -233,7 +233,14 @@ export function createCliOpenShellProviderAdapter( }); const listProviders: OpenShellProviderAdapter["listProviders"] = async (request) => { - const result = invoke(["provider", "list", "--names"], request); + const result = invoke( + ["provider", "list", "--names"], + request, + undefined, + 2, + false, + PROVIDER_GET_DIAGNOSTIC_LIMIT, + ); const error = commandError(result); if (error) return failure(error); return success({ @@ -362,7 +369,14 @@ export function createCliOpenShellProviderAdapter( inferenceCapable: request.inferenceCapable, profilePath: request.profilePath, runOpenshell: (args, options) => - invoke(args, request, undefined, 2, options?.suppressOutput === true), + invoke( + args, + request, + undefined, + 2, + options?.suppressOutput === true, + PROVIDER_GET_DIAGNOSTIC_LIMIT, + ), }); if (result.ok) return success({ state: "ready" }); const reason = diff --git a/src/lib/adapters/openshell/provider-adapter.ts b/src/lib/adapters/openshell/provider-adapter.ts index e496f592308..c127259175c 100644 --- a/src/lib/adapters/openshell/provider-adapter.ts +++ b/src/lib/adapters/openshell/provider-adapter.ts @@ -53,6 +53,10 @@ export type OpenShellProviderMetadata = Readonly<{ type: string; credentialKeys: readonly string[]; configKeys: readonly string[]; + revision?: Readonly<{ + id: string; + resourceVersion: number; + }> | null; }>; export type OpenShellProviderProfileInspection = Readonly<{ diff --git a/src/lib/adapters/openshell/provider-metadata-cli.ts b/src/lib/adapters/openshell/provider-metadata-cli.ts index 10b34cee20b..c02b65bcd2d 100644 --- a/src/lib/adapters/openshell/provider-metadata-cli.ts +++ b/src/lib/adapters/openshell/provider-metadata-cli.ts @@ -15,12 +15,21 @@ const ANSI_CSI_PATTERN = /\x1B\[[0-?]*[ -/]*[@-~]/gu; const LEADING_FIELD_LABEL_RESET_PATTERN = /^(?:\x1B\[0m)*[ \t]*/u; const UNSAFE_FIELD_VALUE_CONTROL_PATTERN = /[\x00-\x08\x0A-\x1F\x7F-\x9F]/u; -type ProviderField = "Name" | "Type" | "Credential keys" | "Config keys"; +type ProviderField = + | "Name" + | "Id" + | "Type" + | "Resource version" + | "Credential keys" + | "Config keys"; -const PROVIDER_FIELD_PATTERN = /^\s*(Name|Type|Credential keys|Config keys):\s*(.*?)\s*$/iu; +const PROVIDER_FIELD_PATTERN = + /^\s*(Name|Id|Type|Resource version|Credential keys|Config keys):\s*(.*?)\s*$/iu; const CANONICAL_PROVIDER_FIELDS = new Map([ ["name", "Name"], + ["id", "Id"], ["type", "Type"], + ["resource version", "Resource version"], ["credential keys", "Credential keys"], ["config keys", "Config keys"], ]); @@ -100,5 +109,28 @@ export function parseCliOpenShellProviderMetadata( const credentialKeys = parseProviderKeys(credentialKeysValue); const configKeys = parseProviderKeys(configKeysValue); - return credentialKeys && configKeys ? { name, type, credentialKeys, configKeys } : null; + if (!credentialKeys || !configKeys) return null; + + const id = fields.get("Id"); + const resourceVersionValue = fields.get("Resource version"); + if ((id === undefined) !== (resourceVersionValue === undefined)) return null; + if (id === undefined || resourceVersionValue === undefined) { + return { name, type, credentialKeys, configKeys, revision: null }; + } + const resourceVersion = Number(resourceVersionValue); + if ( + !isValidCliOpenShellProviderIdentifier(id) || + !/^[0-9]+$/u.test(resourceVersionValue) || + !Number.isSafeInteger(resourceVersion) || + resourceVersion < 1 + ) { + return null; + } + return { + name, + type, + credentialKeys, + configKeys, + revision: { id, resourceVersion }, + }; } diff --git a/src/lib/onboard/gateway-provider-metadata.ts b/src/lib/onboard/gateway-provider-metadata.ts index ba16d0608fb..747b949e29e 100644 --- a/src/lib/onboard/gateway-provider-metadata.ts +++ b/src/lib/onboard/gateway-provider-metadata.ts @@ -11,7 +11,7 @@ import type { OpenShellProviderMetadata } from "../adapters/openshell/provider-a const PROVIDER_PROBE_DIAGNOSTIC_LIMIT = 64 * 1024; const PROVIDER_PROBE_TIMEOUT_MS = 5_000; -export type GatewayProviderMetadata = OpenShellProviderMetadata; +export type GatewayProviderMetadata = Omit; export type GatewayProviderBinding = { name: string; @@ -129,7 +129,10 @@ function providerCommandOutput(result: GatewayProviderCommandResult): string { * selected-provider context and cannot authorize credential reuse by itself. */ export function parseGatewayProviderMetadata(output: string): GatewayProviderMetadata | null { - return parseCliOpenShellProviderMetadata(output); + const metadata = parseCliOpenShellProviderMetadata(output); + if (!metadata) return null; + const { revision: _revision, ...legacyMetadata } = metadata; + return legacyMetadata; } function inspectGatewayCredentialBinding( diff --git a/test/package-contract/cli/credentials-cli-command.test.ts b/test/package-contract/cli/credentials-cli-command.test.ts index 33c56d83edb..b6ff3aafad3 100644 --- a/test/package-contract/cli/credentials-cli-command.test.ts +++ b/test/package-contract/cli/credentials-cli-command.test.ts @@ -207,6 +207,7 @@ describe("credentials oclif commands", () => { opts: { env: expect.any(Object), ignoreError: true, + maxBuffer: 64 * 1024, replaceEnv: true, stdio: ["ignore", "pipe", "pipe"], timeout: 30_000,