Repository navigation
fix(onboard): probe registered extra providers exactly #6596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit
Hold shift + click to select a range
b8204d2
fix(onboard): probe registered extra providers exactly
apurvvkumaria c66486b
test(onboard): keep provider probe fixtures linear
apurvvkumaria a55ce25
test(onboard): remove obsolete provider-list mock
apurvvkumaria 7ebcab1
fix(onboard): recognize wrapped provider errors
apurvvkumaria a0ea6e6
fix(onboard): handle real probe output safely
apurvvkumaria cb7977f
fix(onboard): surface indeterminate provider probes
apurvvkumaria b23bfea
test(onboard): cover stale extra providers on resume repair
prekshivyas c0b2d16
Merge remote-tracking branch 'origin/main' into codex/6501-strict-ext…
prekshivyas f2516d3
test(onboard): avoid repair conditional growth
prekshivyas d6b1248
merge(main): sync v0.0.80 branch
cv ca48d51
Merge branch 'main' into codex/6501-strict-extra-provider-reconciliation
prekshivyas 5bc41db
Merge remote branch 'codex/6501-strict-extra-provider-reconciliation'
prekshivyas 043cfcd
test(onboard): cover resume stale provider reconciliation
prekshivyas e32a26c
fix(onboard): parse wrapped provider diagnostics
prekshivyas 4fd6a33
fix(onboard): harden extra provider reconciliation
prekshivyas d458c26
fix(onboard): prune stale extra providers
prekshivyas 14d1fc8
fix(onboard): honor resume sandbox recreation
prekshivyas 78fa183
chore(onboard): keep entrypoint size neutral
prekshivyas d6adbfd
test(onboard): split resume recreation coverage
prekshivyas 85d092d
fix(onboard): parse issue-form provider diagnostics
prekshivyas 81668f2
fix(onboard): parse provider get stale diagnostics
prekshivyas f4f7be4
fix(onboard): parse structured provider not-found diagnostics
prekshivyas a52b5ac
fix(onboard): reconcile extra providers in machine create
prekshivyas 04e17df
chore(onboard): keep create wiring net-neutral
prekshivyas d1b7085
Merge remote-tracking branch 'origin/main' into codex/6501-strict-ext…
prekshivyas 7e07ab3
test(e2e): record onboard mock parity
prekshivyas 7ea6dcd
fix(onboard): preserve empty reconciled extra providers
prekshivyas 5671f09
test(e2e): prove extra provider resume filtering
prekshivyas 974ac86
fix(onboard): accept wrapped provider not-found diagnostics
prekshivyas ed92e4c
Merge remote-tracking branch 'origin/main' into codex/6501-strict-ext…
prekshivyas d0fbf2d
fix(onboard): prune targeted missing extra providers
prekshivyas e69a6a0
test(sandbox): budget MCP status subprocess suite
prekshivyas 1a6885b
test(onboard): cover extra provider retry stability
prekshivyas cb0a4c8
test(onboard): isolate gateway recovery coredns patch
prekshivyas bc4a194
test(policy): isolate policy list docker guidance
prekshivyas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,94 +1,256 @@ | ||
| // 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 }; | ||
| type ProbeResult = { | ||
| status: number | null; | ||
| output?: string | Buffer | null; | ||
| stdout?: string | Buffer | null; | ||
| stderr?: string | Buffer | null; | ||
| }; | ||
|
|
||
| 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", | ||
| ), | ||
| it("does not probe the gateway when no extra provider is recorded (#6501)", () => { | ||
| const runOpenshell = vi.fn((): ProbeResult => ({ status: 0 })); | ||
|
|
||
| expect( | ||
| reconcileRegisteredExtraProviders("nemoclaw", { | ||
| listExtraProviders: () => [], | ||
| runOpenshell, | ||
| }), | ||
| ).toEqual([]); | ||
| expect(runOpenshell).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("probes beyond the first hundred records and omits an exact stale provider (#6501)", () => { | ||
| const recorded = Object.freeze( | ||
| Array.from({ length: 128 }, (_value, index) => `custom-provider-${index}`), | ||
| ); | ||
| const recorded = ["tavily-search", "brave-search", "custom-provider", "my-slack-bridge"]; | ||
| const calls: Array<{ args: string[]; options: Record<string, unknown> | undefined }> = []; | ||
| const runOpenshell = vi.fn((args: string[], options?: Record<string, unknown>): ProbeResult => { | ||
| calls.push({ args, options }); | ||
| if (args.at(-1) === "custom-provider-127") { | ||
| return { status: 1, stderr: "Error: provider 'custom-provider-127' not found" }; | ||
| } | ||
| return { status: 0, stdout: "" }; | ||
| }); | ||
|
|
||
| const result = reconcileRegisteredExtraProviders("nemoclaw", { | ||
| const reconciled = reconcileRegisteredExtraProviders("nemoclaw", { | ||
| listExtraProviders: () => [...recorded], | ||
| 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, | ||
| expect(reconciled).toEqual(recorded.slice(0, -1)); | ||
| expect(calls).toHaveLength(recorded.length); | ||
| expect(calls.every(({ args }) => args[0] === "provider" && args[1] === "get")).toBe(true); | ||
| 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: 64 * 1024, | ||
| stdio: ["ignore", "pipe", "pipe"], | ||
| suppressOutput: true, | ||
| timeout: 5_000, | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| it("omits a stale tavily record only from the current create plan (#6501)", () => { | ||
| const recorded = ["tavily-search", "custom-provider"]; | ||
| it("keeps healthy custom providers even when successful get output is empty (#6501)", () => { | ||
| const recorded = Object.freeze(["custom-provider", "my-slack-bridge"]); | ||
| const runOpenshell = vi.fn((): ProbeResult => ({ status: 0, stdout: "", stderr: "" })); | ||
|
|
||
| const result = reconcileRegisteredExtraProviders("nemoclaw", { | ||
| runOpenshell: vi.fn(() => ({ status: 0, stdout: "custom-provider\n" })), | ||
| listExtraProviders: () => recorded, | ||
| const reconciled = reconcileRegisteredExtraProviders("nemoclaw", { | ||
| listExtraProviders: () => [...recorded], | ||
| runOpenshell, | ||
| }); | ||
|
|
||
| expect(result).toEqual(["custom-provider"]); | ||
| expect(recorded).toEqual(["tavily-search", "custom-provider"]); | ||
| expect(reconciled).toEqual(recorded); | ||
| expect(recorded).toEqual(["custom-provider", "my-slack-bridge"]); | ||
| expect(runOpenshell).toHaveBeenCalledTimes(2); | ||
| }); | ||
|
|
||
| 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"], | ||
| it("omits only the entry with an exact provider-specific not-found diagnostic (#6501)", () => { | ||
| const recorded = Object.freeze([ | ||
| "healthy-provider", | ||
| "stale-provider", | ||
| "indeterminate-provider", | ||
| ]); | ||
| const runOpenshell = vi.fn((args: string[]): ProbeResult => { | ||
| const providerName = args.at(-1); | ||
| if (providerName === "stale-provider") { | ||
| return { | ||
| status: 1, | ||
| stderr: Buffer.from("Error: provider 'stale-provider' not found\n"), | ||
| }; | ||
| } | ||
| if (providerName === "indeterminate-provider") { | ||
| return { status: 1, stderr: "Error: provider 'some-other-provider' not found" }; | ||
| } | ||
| return { status: 0 }; | ||
| }); | ||
|
|
||
| expect(result).toEqual([]); | ||
| const reconciled = reconcileRegisteredExtraProviders("nemoclaw", { | ||
| listExtraProviders: () => [...recorded], | ||
| runOpenshell, | ||
| }); | ||
|
|
||
| expect(reconciled).toEqual(["healthy-provider", "indeterminate-provider"]); | ||
| expect(recorded).toEqual(["healthy-provider", "stale-provider", "indeterminate-provider"]); | ||
| }); | ||
|
|
||
| it("preserves every recorded extra when the gateway list exits nonzero", () => { | ||
| const recorded = ["tavily-search", "brave-search", "custom-provider"]; | ||
| it("accepts the exact gRPC not-found ordering without using broad name heuristics (#6501)", () => { | ||
| const runOpenshell = vi.fn( | ||
| (): ProbeResult => ({ | ||
| status: 1, | ||
| stderr: 'rpc error: NotFound: provider "stale-provider"', | ||
| }), | ||
| ); | ||
|
|
||
| expect( | ||
| reconcileRegisteredExtraProviders("nemoclaw", { | ||
| listExtraProviders: () => ["stale-provider"], | ||
| runOpenshell, | ||
| }), | ||
| ).toEqual([]); | ||
| }); | ||
|
|
||
| const result = reconcileRegisteredExtraProviders("nemoclaw", { | ||
| runOpenshell: vi.fn(() => ({ status: 1, stderr: "gateway unavailable" })), | ||
| listExtraProviders: () => recorded, | ||
| it("compares captured names byte-for-byte while matching keywords case-insensitively (#6501)", () => { | ||
| const runOpenshell = vi.fn((args: string[]): ProbeResult => { | ||
| if (args.at(-1) === "ProviderA") { | ||
| return { status: 1, stderr: "Error: provider 'providera' not found" }; | ||
| } | ||
| return { status: 1, stderr: "ERROR: PROVIDER 'ProviderB' NOT FOUND" }; | ||
| }); | ||
|
|
||
| expect(result).toEqual(recorded); | ||
| expect( | ||
| reconcileRegisteredExtraProviders("nemoclaw", { | ||
| listExtraProviders: () => ["ProviderA", "ProviderB"], | ||
| runOpenshell, | ||
| }), | ||
| ).toEqual(["ProviderA"]); | ||
| }); | ||
|
|
||
| it("preserves every recorded extra when the gateway list throws", () => { | ||
| const recorded = ["tavily-search", "my-slack-bridge"]; | ||
| it("preserves providers when a missing-gateway diagnostic merely mentions them (#6501)", () => { | ||
| const diagnostics = [ | ||
| "Error: gateway 'nemoclaw' not found while checking provider 'custom-provider'", | ||
| "Error: gateway not found: provider 'custom-provider' was requested", | ||
| 'Error: status: NotFound, message: "gateway not found"; requested provider "custom-provider"', | ||
| "Error: gateway 'nemoclaw' not found; provider 'custom-provider' not found during lookup", | ||
| "Unknown gateway 'nemoclaw' while checking provider 'custom-provider'", | ||
| "No such gateway 'nemoclaw'; provider 'custom-provider' not found", | ||
| "Transport closed while checking provider 'custom-provider' not found", | ||
| "transport error\nError: provider 'custom-provider' not found", | ||
| "authentication failed\nError: provider 'custom-provider' not found", | ||
| ]; | ||
|
|
||
| const result = reconcileRegisteredExtraProviders("nemoclaw", { | ||
| runOpenshell: vi.fn(() => { | ||
| throw new Error("spawn failed"); | ||
| }), | ||
| listExtraProviders: () => recorded, | ||
| for (const diagnostic of diagnostics) { | ||
| const runOpenshell = vi.fn((): ProbeResult => ({ status: 1, stderr: diagnostic })); | ||
| expect( | ||
| reconcileRegisteredExtraProviders("nemoclaw", { | ||
| listExtraProviders: () => ["custom-provider"], | ||
| runOpenshell, | ||
| }), | ||
| diagnostic, | ||
| ).toEqual(["custom-provider"]); | ||
| } | ||
| }); | ||
|
|
||
| it("fails open for thrown, timed-out, and otherwise ambiguous probes (#6501)", () => { | ||
| const runOpenshell = vi.fn((args: string[]): ProbeResult => { | ||
| switch (args.at(-1)) { | ||
| case "thrown-provider": | ||
| throw new Error("gateway process unavailable"); | ||
| case "timed-out-provider": | ||
| return { | ||
| status: null, | ||
| stderr: "operation timed out: provider 'timed-out-provider' not found", | ||
| }; | ||
| case "nonstandard-exit-provider": | ||
| return { | ||
| status: 7, | ||
| stderr: "provider 'nonstandard-exit-provider' not found", | ||
| }; | ||
| case "ambiguous-provider": | ||
| return { status: 1, stderr: "provider lookup failed" }; | ||
| case "unavailable-provider": | ||
| return { | ||
| status: 1, | ||
| stderr: | ||
| "Error: status: Unavailable, message: \"provider 'unavailable-provider' not found\"", | ||
| }; | ||
| default: | ||
| return { status: 0 }; | ||
| } | ||
| }); | ||
| const recorded = [ | ||
| "thrown-provider", | ||
| "timed-out-provider", | ||
| "nonstandard-exit-provider", | ||
| "ambiguous-provider", | ||
| "unavailable-provider", | ||
| ]; | ||
|
|
||
| expect(result).toEqual(recorded); | ||
| expect( | ||
| reconcileRegisteredExtraProviders("nemoclaw", { | ||
| listExtraProviders: () => [...recorded], | ||
| runOpenshell, | ||
| }), | ||
| ).toEqual(recorded); | ||
| }); | ||
|
|
||
| it("does not query the gateway when no extras are recorded", () => { | ||
| const runOpenshell = vi.fn(); | ||
| it("bounds aggregate probe latency and preserves names left after the deadline (#6501)", () => { | ||
| let now = 0; | ||
| const timeouts: number[] = []; | ||
| const runOpenshell = vi.fn( | ||
| (_args: string[], options?: Record<string, unknown>): ProbeResult => { | ||
| 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, | ||
| runOpenshell, | ||
| listExtraProviders: () => [], | ||
| }), | ||
| ).toEqual([]); | ||
| ).toEqual(recorded); | ||
| expect(runOpenshell).toHaveBeenCalledTimes(3); | ||
| expect(timeouts).toEqual([5_000, 5_000, 5_000]); | ||
| }); | ||
|
|
||
| it("enforces gateway containment before making any nonempty provider probe (#6501)", () => { | ||
| vi.stubEnv("OPENSHELL_GATEWAY_ENDPOINT", "https://other.example.test"); | ||
| const runOpenshell = vi.fn((): ProbeResult => ({ status: 0 })); | ||
|
|
||
| expect(() => | ||
| reconcileRegisteredExtraProviders("nemoclaw", { | ||
| listExtraProviders: () => ["custom-provider"], | ||
| runOpenshell, | ||
| }), | ||
| ).toThrow(/OPENSHELL_GATEWAY_ENDPOINT is set/); | ||
| expect(runOpenshell).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("requires a gateway name when recorded providers need reconciliation (#6501)", () => { | ||
| const runOpenshell = vi.fn((): ProbeResult => ({ status: 0 })); | ||
|
|
||
| expect(() => | ||
| reconcileRegisteredExtraProviders("", { | ||
| listExtraProviders: () => ["custom-provider"], | ||
| runOpenshell, | ||
| }), | ||
| ).toThrow("OpenShell gateway name is required."); | ||
| expect(runOpenshell).not.toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.