From 8946ad55525a66f378aa9a1175b2556bb414262e Mon Sep 17 00:00:00 2001 From: Valera Brizhatiuk <19537764+plombeer31@users.noreply.github.com> Date: Wed, 2 Sep 2026 13:41:21 +0300 Subject: [PATCH 1/3] feat(providers): probe streaming native-tool contract at setup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A live API key proves the account, not the route. `/v1/models` proves reachability, and the pre-save key check proves one non-streaming completion with no tools — neither exercises what an Atomic turn actually is: a streamed chat completion carrying a `tools` payload from which a native tool call must come back whole. Routes that pass both and then fail a turn are common (STREAM_EARLY_EOF, HTTP 400 only once `tools` is in the body, forced tool choice refused, incomplete tool-call deltas), and until now the operator's first message was the test. Adds a conformance probe next to the existing key check, built as a three-rung ladder so a healthy route costs exactly one request: 1. forced named tool, streaming, tools payload — a complete tool call here is the whole answer; 2. `tool_choice: auto` with the same payload, reached only when rung 1 was refused for a reason that is not the key, the quota or the model — a tool call now means it was the forcing the route refused; 3. the same streamed completion with no tools at all, reached only when both tool requests were refused — if this answers, the route works and it is specifically `tools` it rejects. Rung 3 is an experiment rather than a regex over error wording, which is not stable across providers; "refuses with tools, answers without them" is. A text answer under `auto` stays inconclusive and is never reported as "tools unsupported". The synthetic function is a diagnostic fixture: nothing registers it, nothing dispatches it, and its name sits outside the dotted namespace every built-in tool uses so it cannot shadow one. Details are bounded and credential-scrubbed — the exact key by match, key-shaped strings by pattern — and that redaction is now shared with the key check. Wired into the two setup paths (Providers wizard save, first-run cloud onboarding) as advisory only: it can never refuse a save, so a custom endpoint that blocks synthetic probes stays configurable, but a route it could not prove no longer reports as a working backend. `/llm check` runs it on demand against a provider that is already saved. It never runs on a turn path. --- .../verify/accumulate-probe-stream.test.ts | 120 ++++++ .../verify/accumulate-probe-stream.ts | 104 +++++ .../verify/classify-contract-probe.test.ts | 232 +++++++++++ .../verify/classify-contract-probe.ts | 134 ++++++ .../provider/verify/contract-probe-types.ts | 179 ++++++++ src/llm/provider/verify/index.ts | 28 ++ .../provider/verify/redact-provider-detail.ts | 42 ++ .../verify/run-contract-probe.test.ts | 313 ++++++++++++++ src/llm/provider/verify/run-contract-probe.ts | 389 ++++++++++++++++++ .../provider/verify/verify-provider-key.ts | 19 +- .../commands/slash-command-handler.test.ts | 15 + src/tui/commands/slash-command-handler.ts | 14 +- .../components/cloud-provider-onboarding.tsx | 19 +- src/tui/providers/contract-probe-target.ts | 110 +++++ src/tui/providers/describe-contract-probe.ts | 59 +++ .../providers/probe-wizard-contract.test.ts | 170 ++++++++ src/tui/providers/probe-wizard-contract.ts | 101 +++++ src/tui/providers/providers-actions.ts | 13 + .../providers/providers-orchestrator.test.ts | 67 +++ src/tui/providers/providers-orchestrator.ts | 126 +++++- src/tui/providers/providers-wizard-target.ts | 4 +- src/tui/submit-handler.ts | 7 + src/tui/tui-app.tsx | 7 + src/tui/tui-command.ts | 2 + 24 files changed, 2252 insertions(+), 22 deletions(-) create mode 100644 src/llm/provider/verify/accumulate-probe-stream.test.ts create mode 100644 src/llm/provider/verify/accumulate-probe-stream.ts create mode 100644 src/llm/provider/verify/classify-contract-probe.test.ts create mode 100644 src/llm/provider/verify/classify-contract-probe.ts create mode 100644 src/llm/provider/verify/contract-probe-types.ts create mode 100644 src/llm/provider/verify/redact-provider-detail.ts create mode 100644 src/llm/provider/verify/run-contract-probe.test.ts create mode 100644 src/llm/provider/verify/run-contract-probe.ts create mode 100644 src/tui/providers/contract-probe-target.ts create mode 100644 src/tui/providers/describe-contract-probe.ts create mode 100644 src/tui/providers/probe-wizard-contract.test.ts create mode 100644 src/tui/providers/probe-wizard-contract.ts diff --git a/src/llm/provider/verify/accumulate-probe-stream.test.ts b/src/llm/provider/verify/accumulate-probe-stream.test.ts new file mode 100644 index 00000000..f80b1720 --- /dev/null +++ b/src/llm/provider/verify/accumulate-probe-stream.test.ts @@ -0,0 +1,120 @@ +import { describe, expect, it } from "vitest"; + +import { accumulateProbeStream } from "./accumulate-probe-stream.js"; + +function sse(...events: unknown[]): string { + return events + .map((event) => + typeof event === "string" ? `data: ${event}\n\n` : `data: ${JSON.stringify(event)}\n\n`, + ) + .join(""); +} + +function toolCallChunk( + parts: { name?: string; arguments?: string; index?: number }, + finishReason: string | null = null, +): Record { + return { + model: "probe-model", + choices: [ + { + delta: { + tool_calls: [ + { + index: parts.index ?? 0, + id: "call_1", + type: "function", + function: { + ...(parts.name !== undefined ? { name: parts.name } : {}), + ...(parts.arguments !== undefined ? { arguments: parts.arguments } : {}), + }, + }, + ], + }, + ...(finishReason ? { finish_reason: finishReason } : {}), + }, + ], + }; +} + +describe("accumulateProbeStream", () => { + it("assembles a native tool call split across deltas", () => { + const observation = accumulateProbeStream( + sse( + toolCallChunk({ name: "atomic_contract_probe", arguments: '{"ok"' }), + toolCallChunk({ arguments: ":true}" }), + toolCallChunk({}, "tool_calls"), + "[DONE]", + ), + ); + expect(observation.toolCalls).toEqual([ + { index: 0, name: "atomic_contract_probe", arguments: '{"ok":true}' }, + ]); + expect(observation.sawToolCallDelta).toBe(true); + expect(observation.terminalObserved).toBe(true); + expect(observation.finishReason).toBe("tool_calls"); + }); + + it("keeps a tool-call delta that never carried a function name", () => { + // The production stream consumer drops this call, because a nameless + // call cannot be dispatched. The probe has to see it: "deltas came + // but assembled into nothing" is the diagnosis, and a probe that + // dropped it would report the far friendlier "no tool call". + const observation = accumulateProbeStream( + sse(toolCallChunk({ arguments: '{"ok":true}' }), toolCallChunk({}, "tool_calls")), + ); + expect(observation.sawToolCallDelta).toBe(true); + expect(observation.toolCalls[0]?.name).toBe(""); + }); + + it("does not mistake a repeated whole name for fragments", () => { + // Anthropic-compatible endpoints resend the full name every delta. + const observation = accumulateProbeStream( + sse( + toolCallChunk({ name: "atomic_contract_probe", arguments: "{" }), + toolCallChunk({ name: "atomic_contract_probe", arguments: "}" }), + toolCallChunk({}, "tool_calls"), + ), + ); + expect(observation.toolCalls[0]?.name).toBe("atomic_contract_probe"); + }); + + it("collects plain assistant text and its finish reason", () => { + const observation = accumulateProbeStream( + sse( + { choices: [{ delta: { content: "I can " } }] }, + { choices: [{ delta: { content: "help." }, finish_reason: "stop" }] }, + "[DONE]", + ), + ); + expect(observation.text).toBe("I can help."); + expect(observation.sawToolCallDelta).toBe(false); + expect(observation.terminalObserved).toBe(true); + }); + + it("reports no terminal signal when the body simply stops", () => { + const observation = accumulateProbeStream( + sse(toolCallChunk({ name: "atomic_contract_probe", arguments: '{"ok"' })), + ); + expect(observation.terminalObserved).toBe(false); + expect(observation.finishReason).toBeNull(); + }); + + it("reads a final event that has no trailing blank line", () => { + // Providers and proxies close the response right after the terminal + // event often enough that treating it as noise would turn healthy + // routes into false early-EOF reports. + const observation = accumulateProbeStream( + `${sse({ choices: [{ delta: { content: "hi" }, finish_reason: "stop" }] })}data: [DONE]`, + ); + expect(observation.terminalObserved).toBe(true); + }); + + it("survives a truncated JSON payload without inventing content", () => { + const observation = accumulateProbeStream( + `${sse({ choices: [{ delta: { content: "hi" } }] })}data: {"choices":[{"delta":{"too`, + ); + expect(observation.text).toBe("hi"); + expect(observation.terminalObserved).toBe(false); + }); +}); diff --git a/src/llm/provider/verify/accumulate-probe-stream.ts b/src/llm/provider/verify/accumulate-probe-stream.ts new file mode 100644 index 00000000..0b5d8121 --- /dev/null +++ b/src/llm/provider/verify/accumulate-probe-stream.ts @@ -0,0 +1,104 @@ +/** + * What actually came back over a probe's SSE stream. + * + * The probe reads the whole (small, bounded) body and then replays it + * through `parseOpenAiSseEvent` and `mergeToolName` — the very parser + * and name-merge rule a real turn uses. Reimplementing either here + * would let the probe and the turn disagree about what a provider sent, + * which is the one thing a conformance check must never do. + * + * It stops short of `createOpenAiStreamConsumer` on purpose. That + * consumer answers "what should the agent act on", and to do it it + * *drops* a tool call whose function name never arrived. A probe needs + * the opposite: knowing that tool-call deltas were streamed but never + * assembled into a callable tool is the whole diagnosis of a route with + * malformed deltas. + */ + +import { mergeToolName } from "../openai/openai-stream-consumer.js"; +import { parseOpenAiSseEvent } from "../openai/parse-sse-chunk.js"; +import { createReasoningExtractor } from "../openai/reasoning-extractor.js"; + +export interface ProbeToolCallObservation { + readonly index: number; + /** Empty when deltas for this index never carried a function name. */ + readonly name: string; + /** Concatenated argument fragments, exactly as they arrived. */ + readonly arguments: string; +} + +export interface ProbeStreamObservation { + /** Assistant text, for the auto-mode "answered in prose" case. */ + readonly text: string; + readonly toolCalls: readonly ProbeToolCallObservation[]; + /** A `tool_calls` delta was seen at all — even one naming nothing. */ + readonly sawToolCallDelta: boolean; + readonly finishReason: string | null; + /** + * The provider said it was finished: an explicit `finish_reason` on + * some chunk, or a `[DONE]` event. A body that simply stops carries + * neither, and that is precisely `STREAM_EARLY_EOF`. Same rule the + * stream consumer applies before it trusts a tool call. + */ + readonly terminalObserved: boolean; +} + +export function accumulateProbeStream(sse: string): ProbeStreamObservation { + // The probe never asks for reasoning, and no probe verdict depends on + // it, so the no-op extractor keeps the parser call honest without + // pulling provider reasoning formats into the check. + const reasoning = createReasoningExtractor("none"); + const calls = new Map(); + let text = ""; + let sawToolCallDelta = false; + let finishReason: string | null = null; + let terminalObserved = false; + + for (const rawEvent of splitSseEvents(sse)) { + const chunk = parseOpenAiSseEvent(rawEvent, reasoning, ""); + text += chunk.delta; + if (chunk.finishReason !== null) { + finishReason = chunk.finishReason; + terminalObserved = true; + } + if (chunk.done) terminalObserved = true; + if (chunk.toolArgsDelta === true) sawToolCallDelta = true; + for (const delta of chunk.toolCallDeltas) { + const current = calls.get(delta.index) ?? { name: "", arguments: "" }; + if (delta.function?.name) { + current.name = mergeToolName(current.name, delta.function.name); + } + if (delta.function?.arguments) { + current.arguments += delta.function.arguments; + } + calls.set(delta.index, current); + } + } + + return { + text, + toolCalls: [...calls.entries()] + .sort(([a], [b]) => a - b) + .map(([index, call]) => ({ + index, + name: call.name, + arguments: call.arguments, + })), + sawToolCallDelta, + finishReason, + terminalObserved, + }; +} + +/** + * SSE events are blank-line separated. A trailing chunk with no closing + * blank line still counts: providers and proxies routinely close the + * response straight after the terminal event, and treating that last + * event as noise would turn a clean finish into a false early EOF. + */ +function splitSseEvents(sse: string): string[] { + return sse + .split("\n\n") + .map((part) => part.trim()) + .filter((part) => part.length > 0); +} diff --git a/src/llm/provider/verify/classify-contract-probe.test.ts b/src/llm/provider/verify/classify-contract-probe.test.ts new file mode 100644 index 00000000..d2f1a9a6 --- /dev/null +++ b/src/llm/provider/verify/classify-contract-probe.test.ts @@ -0,0 +1,232 @@ +import { describe, expect, it } from "vitest"; + +import { accumulateProbeStream } from "./accumulate-probe-stream.js"; +import { + classifyContractProbeHttpFailure, + classifyProbeStream, + contractProbeFailureIsTerminal, +} from "./classify-contract-probe.js"; +import { + CONTRACT_PROBE_TOOL_NAME, + contractProbeFoundDefect, + contractProbeProvesToolSupport, + contractProbeToolDefinition, +} from "./contract-probe-types.js"; +import { DEFAULT_TOOL_DESCRIPTORS } from "../../../prompt/tool-descriptors.js"; + +function stream(body: string): ReturnType { + return accumulateProbeStream(body); +} + +const CALL_EVENT = + `data: ${JSON.stringify({ + choices: [ + { + delta: { + tool_calls: [ + { + index: 0, + id: "call_1", + type: "function", + function: { name: CONTRACT_PROBE_TOOL_NAME, arguments: '{"ok":true}' }, + }, + ], + }, + }, + ], + })}\n\n`; +const FINISH_EVENT = `data: ${JSON.stringify({ choices: [{ delta: {}, finish_reason: "tool_calls" }] })}\n\ndata: [DONE]\n\n`; + +describe("classifyContractProbeHttpFailure", () => { + it("names an authentication refusal", () => { + expect( + classifyContractProbeHttpFailure(401, '{"error":"No auth credentials found"}'), + ).toBe("endpoint_auth_failed"); + }); + + it("buckets quota exhaustion and gateway throttling together", () => { + expect( + classifyContractProbeHttpFailure(429, '{"error":{"code":"insufficient_quota"}}'), + ).toBe("quota_or_routing_failed"); + expect(classifyContractProbeHttpFailure(402, "Insufficient credits")).toBe( + "quota_or_routing_failed", + ); + expect(classifyContractProbeHttpFailure(429, "slow down")).toBe( + "quota_or_routing_failed", + ); + }); + + it("names an unknown model", () => { + expect(classifyContractProbeHttpFailure(404, "no such model")).toBe( + "model_unavailable", + ); + expect( + classifyContractProbeHttpFailure(400, '{"error":"model does not exist"}'), + ).toBe("model_unavailable"); + }); + + it("leaves an unexplained 400 open rather than blaming tools", () => { + // This is the case the ladder exists for: a bare 400 with `tools` in + // the body proves nothing on its own, and guessing from wording is + // exactly what makes a diagnosis wrong. + expect(classifyContractProbeHttpFailure(400, "Bad Request")).toBe("provider_error"); + expect(contractProbeFailureIsTerminal("provider_error")).toBe(false); + }); + + it("treats key, quota and model refusals as terminal", () => { + expect(contractProbeFailureIsTerminal("endpoint_auth_failed")).toBe(true); + expect(contractProbeFailureIsTerminal("quota_or_routing_failed")).toBe(true); + expect(contractProbeFailureIsTerminal("model_unavailable")).toBe(true); + }); +}); + +describe("classifyProbeStream", () => { + it("accepts one complete native tool call", () => { + expect( + classifyProbeStream(stream(CALL_EVENT + FINISH_EVENT), "required_named"), + ).toBe("tools_supported"); + }); + + it("accepts a forced call that carried no arguments", () => { + // The synthetic schema requires no field, so empty arguments are a + // legal answer; calling them malformed would fail healthy routes. + const empty = `data: ${JSON.stringify({ + choices: [ + { + delta: { + tool_calls: [ + { + index: 0, + type: "function", + function: { name: CONTRACT_PROBE_TOOL_NAME, arguments: "" }, + }, + ], + }, + }, + ], + })}\n\n`; + expect(classifyProbeStream(stream(empty + FINISH_EVENT), "required_named")).toBe( + "tools_supported", + ); + }); + + it("calls a text answer under auto inconclusive, never unsupported", () => { + const text = `data: ${JSON.stringify({ + choices: [{ delta: { content: "Sure!" }, finish_reason: "stop" }], + })}\n\ndata: [DONE]\n\n`; + expect(classifyProbeStream(stream(text), "auto")).toBe( + "inconclusive_no_tool_call", + ); + }); + + it("calls the same text answer under a forced choice a route defect", () => { + const text = `data: ${JSON.stringify({ + choices: [{ delta: { content: "Sure!" }, finish_reason: "stop" }], + })}\n\ndata: [DONE]\n\n`; + expect(classifyProbeStream(stream(text), "required_named")).toBe( + "forced_tool_choice_ignored", + ); + }); + + it("reports a stream that ended before announcing it was done", () => { + // A complete-looking call in a truncated body is not trustworthy: + // argument fragments may still have been in flight. + expect(classifyProbeStream(stream(CALL_EVENT), "required_named")).toBe( + "stream_early_eof", + ); + }); + + it("reports arguments that are not JSON", () => { + const truncated = `data: ${JSON.stringify({ + choices: [ + { + delta: { + tool_calls: [ + { + index: 0, + type: "function", + function: { name: CONTRACT_PROBE_TOOL_NAME, arguments: '{"ok":' }, + }, + ], + }, + }, + ], + })}\n\n`; + expect(classifyProbeStream(stream(truncated + FINISH_EVENT), "required_named")).toBe( + "malformed_tool_call", + ); + }); + + it("reports tool-call deltas that never named a function", () => { + const nameless = `data: ${JSON.stringify({ + choices: [ + { + delta: { + tool_calls: [{ index: 0, type: "function", function: { arguments: "{}" } }], + }, + }, + ], + })}\n\n`; + expect(classifyProbeStream(stream(nameless + FINISH_EVENT), "required_named")).toBe( + "malformed_tool_call", + ); + }); + + it("reports a call naming a function that was never offered", () => { + const wrong = `data: ${JSON.stringify({ + choices: [ + { + delta: { + tool_calls: [ + { + index: 0, + type: "function", + function: { name: "os.fs.read", arguments: "{}" }, + }, + ], + }, + }, + ], + })}\n\n`; + expect(classifyProbeStream(stream(wrong + FINISH_EVENT), "required_named")).toBe( + "malformed_tool_call", + ); + }); +}); + +describe("the synthetic probe tool", () => { + it("is not a tool this agent can dispatch", () => { + // The probe reads the call and throws it away; nothing dispatches + // it. This pins the other half of that promise: the name cannot + // collide with a real tool, now or after the catalog grows. + const registered = DEFAULT_TOOL_DESCRIPTORS.map((d) => d.name); + expect(registered).not.toContain(CONTRACT_PROBE_TOOL_NAME); + }); + + it("offers a schema with no required field", () => { + const fn = (contractProbeToolDefinition().function ?? {}) as { + name: string; + parameters: { required: string[] }; + }; + expect(fn.name).toBe(CONTRACT_PROBE_TOOL_NAME); + expect(fn.parameters.required).toEqual([]); + }); + + it("treats only a complete tool call as proven support", () => { + expect(contractProbeProvesToolSupport("tools_supported")).toBe(true); + for (const status of [ + "inconclusive_no_tool_call", + "stream_early_eof", + "malformed_tool_call", + "tools_payload_rejected", + "provider_error", + ] as const) { + expect(contractProbeProvesToolSupport(status)).toBe(false); + } + }); + + it("does not count an inconclusive auto answer as a defect", () => { + expect(contractProbeFoundDefect("inconclusive_no_tool_call")).toBe(false); + expect(contractProbeFoundDefect("stream_early_eof")).toBe(true); + }); +}); diff --git a/src/llm/provider/verify/classify-contract-probe.ts b/src/llm/provider/verify/classify-contract-probe.ts new file mode 100644 index 00000000..86060ee6 --- /dev/null +++ b/src/llm/provider/verify/classify-contract-probe.ts @@ -0,0 +1,134 @@ +/** + * Turning one probe request into a verdict about the *route*. + * + * Two classifiers live here, and the split matters: + * + * - `classifyContractProbeHttpFailure` reads a refusal. It answers + * only the questions a single status code and body can settle — + * credential, quota, model — and delegates that reading to + * `classifyVerifyResponse`, so the contract probe and the key check + * can never disagree about what a 402 or a Gemini 400 means. + * - `classifyProbeStream` reads a stream that was accepted, and + * settles whether a dispatchable native tool call actually arrived. + * + * Nothing here guesses "tools are unsupported" from error wording. + * Provider phrasing for that is not stable enough to hang a verdict on, + * and it does not need to be: the runner establishes it by experiment — + * refuse with tools, answer without them — which is both stronger + * evidence and the same evidence a human would gather by hand. + */ + +import { classifyVerifyResponse } from "./classify-verify-response.js"; +import { + CONTRACT_PROBE_TOOL_NAME, + type ProbeToolChoiceMode, + type ProviderContractStatus, +} from "./contract-probe-types.js"; +import type { ProbeStreamObservation } from "./accumulate-probe-stream.js"; + +/** + * Classify a non-2xx answer to a probe request. + * + * Only the terminal classes are named here (see + * `contractProbeFailureIsTerminal`). Anything else comes back as + * `provider_error`, which the runner reads as "not settled yet" and + * follows up on with the next rung of the ladder. + */ +export function classifyContractProbeHttpFailure( + httpStatus: number, + body: string, +): ProviderContractStatus { + const verdict = classifyVerifyResponse(httpStatus, body); + if (verdict.kind === "retry_next_model") return "model_unavailable"; + // The key check's "resend with the other max-tokens field" hint is + // meaningless here: the probe deliberately sends no token cap (see + // `run-contract-probe`), so a body naming those fields is the route + // complaining about something else it read in our request. + if (verdict.kind === "retry_token_field") return "provider_error"; + switch (verdict.status) { + case "invalid_key": + return "endpoint_auth_failed"; + case "no_balance": + case "rate_limited": + // One bucket on purpose: from a setup screen, "you are out of + // credit" and "this gateway is throttling you" lead to the same + // action — sort out the account, then probe again. + return "quota_or_routing_failed"; + case "model_unavailable": + return "model_unavailable"; + default: + return "provider_error"; + } +} + +/** + * `true` when the refusal already explains itself and no further + * request can teach us anything about tool support. Retrying a dead + * key without `tools` would only spend another request to be told the + * same thing. + */ +export function contractProbeFailureIsTerminal( + status: ProviderContractStatus, +): boolean { + return ( + status === "endpoint_auth_failed" || + status === "quota_or_routing_failed" || + status === "model_unavailable" + ); +} + +/** + * Read an accepted stream. + * + * Order is deliberate. A stream that never announced its own end is + * judged first and unconditionally: a tool call assembled out of a + * truncated body may be missing argument fragments that were still in + * flight, so trusting it is exactly the mistake + * `applyToolCallTerminationSafety` exists to prevent on the real path. + */ +export function classifyProbeStream( + observation: ProbeStreamObservation, + mode: ProbeToolChoiceMode, +): ProviderContractStatus { + if (!observation.terminalObserved) return "stream_early_eof"; + + if (observation.sawToolCallDelta) { + const call = + observation.toolCalls.find((c) => c.name === CONTRACT_PROBE_TOOL_NAME) ?? + observation.toolCalls[0]; + // Deltas arrived and still produced nothing callable. On the real + // path this is the failure that surfaces as `tool not registered in + // this agent`, several minutes into a turn. + if (!call || call.name.length === 0) return "malformed_tool_call"; + // Only one function was offered, so any other name is the route + // inventing one — the call could never be dispatched. + if (call.name !== CONTRACT_PROBE_TOOL_NAME) return "malformed_tool_call"; + if (!argumentsAreDispatchable(call.arguments)) return "malformed_tool_call"; + return "tools_supported"; + } + + // No tool call at all. What that means depends entirely on what we + // asked for, and conflating the two cases is the specific mistake + // this probe is built to avoid. + return mode === "required_named" + ? "forced_tool_choice_ignored" + : "inconclusive_no_tool_call"; +} + +/** + * Arguments Atomic could actually hand to a tool. Empty is fine — the + * probe's schema requires no field, and a model answering a forced call + * with nothing to say legitimately sends `""` or `{}`. Anything else + * has to parse as a JSON object; a truncated `{"ok":` is the shape a + * route with incomplete deltas produces. + */ +function argumentsAreDispatchable(raw: string): boolean { + const trimmed = raw.trim(); + if (trimmed.length === 0) return true; + try { + const parsed: unknown = JSON.parse(trimmed); + return typeof parsed === "object" && parsed !== null && !Array.isArray(parsed); + } catch { + return false; + } +} diff --git a/src/llm/provider/verify/contract-probe-types.ts b/src/llm/provider/verify/contract-probe-types.ts new file mode 100644 index 00000000..b021513a --- /dev/null +++ b/src/llm/provider/verify/contract-probe-types.ts @@ -0,0 +1,179 @@ +/** + * Shapes for the provider *contract* probe — the second question a + * setup flow has to ask. + * + * `verify-provider-key` settles whether the credential is live and + * funded. It cannot settle whether the route behind that credential can + * run an Atomic turn, because a turn is not a one-token completion: it + * is a **streamed** chat completion carrying a `tools` payload, from + * which a **native tool call** has to come back whole. Routes exist that + * pass the key check and then fail exactly one of those: the stream ends + * early, HTTP 400 appears only once `tools` is in the body, forced tool + * choice is refused, or the tool-call deltas arrive incomplete. + * + * Kept free of config and UI imports for the same reason the key check + * is: the wizard, onboarding and an explicit "test this provider" + * action all have to be able to run it. + */ + +/** + * The tool the probe offers. It is a *diagnostic fixture*, never an + * Atomic tool: nothing registers it, nothing dispatches it, and the + * model's call to it is read and thrown away. The name is deliberately + * outside the dotted namespace every built-in tool uses + * (`os.fs.read`, `browser.navigate`, …) so it cannot shadow a real one, + * and it stays inside the `^[A-Za-z0-9_-]{1,64}$` shape the strict + * providers validate function names against. + */ +export const CONTRACT_PROBE_TOOL_NAME = "atomic_contract_probe"; + +/** + * Which tool-choice mode a probe request ran in. `required_named` is the + * primary instrument — it is the only mode where "no tool call" is a + * real answer about the route rather than about the model's mood. + */ +export type ProbeToolChoiceMode = "required_named" | "auto"; + +export type ProviderContractStatus = + /** One complete native tool call came back over the stream. */ + | "tools_supported" + /** + * The route took the tools payload but chose to answer in prose under + * `tool_choice: auto`. That is a legal answer for a model, so it says + * nothing about whether the route can emit tool calls at all. + */ + | "inconclusive_no_tool_call" + /** + * The route accepted a forced named tool choice and then answered + * text anyway — it advertises the parameter without honoring it. + */ + | "forced_tool_choice_ignored" + /** + * The route refused the request while forcing a named tool, but ran + * the same tools payload under `auto` and produced a tool call. + */ + | "forced_tool_choice_rejected" + /** + * The route refused every request that carried `tools`, and answered + * the same streamed completion once `tools` was removed. + */ + | "tools_payload_rejected" + /** The configured model is unknown to this route. */ + | "model_unavailable" + /** The endpoint refused the credential; nothing else was learned. */ + | "endpoint_auth_failed" + /** Out of quota, or the gateway could not route to a backend. */ + | "quota_or_routing_failed" + /** The stream closed with neither a finish reason nor `[DONE]`. */ + | "stream_early_eof" + /** + * Tool-call deltas arrived but never assembled into a dispatchable + * call: no function name, a name nobody offered, or arguments that + * are not JSON. + */ + | "malformed_tool_call" + /** No HTTP response at all: DNS, refused connection, TLS, offline. */ + | "unreachable" + /** The probe's own deadline fired first. */ + | "timeout" + /** The operator (or the caller) aborted the probe. */ + | "cancelled" + /** The route failed in a way that says nothing about tool support. */ + | "provider_error"; + +export interface ProviderContractProbeTarget { + /** Service name for user-facing wording ("OpenRouter", "Groq"). */ + readonly label: string; + /** API root without the version prefix, already normalized. */ + readonly baseUrl: string; + /** Version prefix the service uses: `/v1`, Gemini's `/v1beta/openai`. */ + readonly apiPathPrefix: string; + /** Trimmed key, or `""` for a service that authenticates without one. */ + readonly apiKey: string; + /** + * The model the operator is about to run turns with. Not a cheap + * stand-in: route limitations are per-model, so probing anything else + * would answer a question nobody asked. + */ + readonly model: string; + readonly extraHeaders?: Record; +} + +export interface ProviderContractProbeResult { + readonly status: ProviderContractStatus; + readonly probedModel: string; + /** Status of the request the verdict came from, `null` if none did. */ + readonly httpStatus: number | null; + /** The mode the verdict came from; `null` when no request was made. */ + readonly toolChoiceMode: ProbeToolChoiceMode | null; + /** + * A bounded, credential-scrubbed excerpt of what the provider said — + * enough for a status line and the log, never the whole body. + */ + readonly detail: string; + readonly latencyMs: number; + /** How many HTTP requests the probe spent reaching this verdict. */ + readonly requests: number; +} + +/** + * The one verdict that means "this route can run a turn". Everything + * else is either a failure or an open question, and neither may be + * reported as proven compatibility. + */ +export function contractProbeProvesToolSupport( + status: ProviderContractStatus, +): boolean { + return status === "tools_supported"; +} + +/** + * `true` when the probe learned something about the *route* rather than + * about the model's whim. Used to decide whether a warning is worth + * showing: an inconclusive auto-mode answer is not a defect to report. + */ +export function contractProbeFoundDefect( + status: ProviderContractStatus, +): boolean { + return ( + status === "forced_tool_choice_ignored" || + status === "forced_tool_choice_rejected" || + status === "tools_payload_rejected" || + status === "model_unavailable" || + status === "endpoint_auth_failed" || + status === "quota_or_routing_failed" || + status === "stream_early_eof" || + status === "malformed_tool_call" + ); +} + +/** + * The synthetic function definition, in the exact shape + * `buildOpenAiChatBody` puts real tools in, so a route that validates + * tool schemas judges this one by the same rules it will judge Atomic's. + * + * No required properties: a model answering a forced call with empty + * arguments is honoring the contract, and demanding a field would turn + * that legal answer into a false "malformed" verdict. + */ +export function contractProbeToolDefinition(): Record { + return { + type: "function", + function: { + name: CONTRACT_PROBE_TOOL_NAME, + description: + "Diagnostic no-op used to check that this endpoint can emit a native tool call. Has no effect.", + parameters: { + type: "object", + properties: { + ok: { + type: "boolean", + description: "Always true.", + }, + }, + required: [], + additionalProperties: false, + }, + }, + }; +} diff --git a/src/llm/provider/verify/index.ts b/src/llm/provider/verify/index.ts index a100e465..87a43b75 100644 --- a/src/llm/provider/verify/index.ts +++ b/src/llm/provider/verify/index.ts @@ -1,12 +1,40 @@ +export { + accumulateProbeStream, + type ProbeStreamObservation, + type ProbeToolCallObservation, +} from "./accumulate-probe-stream.js"; +export { + classifyContractProbeHttpFailure, + classifyProbeStream, + contractProbeFailureIsTerminal, +} from "./classify-contract-probe.js"; export { classifyVerifyResponse, classifyVerifyTransportError, type VerifyResponseVerdict, } from "./classify-verify-response.js"; +export { + CONTRACT_PROBE_TOOL_NAME, + contractProbeFoundDefect, + contractProbeProvesToolSupport, + contractProbeToolDefinition, + type ProbeToolChoiceMode, + type ProviderContractProbeResult, + type ProviderContractProbeTarget, + type ProviderContractStatus, +} from "./contract-probe-types.js"; export { cheapestPaidOpenRouterModel, pickProbeModels, } from "./pick-probe-models.js"; +export { + PROVIDER_DETAIL_MAX_LEN, + redactProviderDetail, +} from "./redact-provider-detail.js"; +export { + PROVIDER_CONTRACT_PROBE_TIMEOUT_MS, + runProviderContractProbe, +} from "./run-contract-probe.js"; export { PROVIDER_VERIFY_TIMEOUT_MS, verifyProviderKey, diff --git a/src/llm/provider/verify/redact-provider-detail.ts b/src/llm/provider/verify/redact-provider-detail.ts new file mode 100644 index 00000000..641bf568 --- /dev/null +++ b/src/llm/provider/verify/redact-provider-detail.ts @@ -0,0 +1,42 @@ +/** + * What a provider said, made safe to put on a status line and in a log. + * + * Two rules, both learned the hard way from error bodies: + * + * - Providers echo the offending credential back. The key we sent is + * therefore removed by exact match, and anything else shaped like an + * API key is removed by pattern — a proxy in front of the service can + * quote a *different* key than the one under test, and the exact + * match would sail straight past it. + * - The body itself is never reproduced whole. A verdict needs a + * sentence of evidence, not a provider's entire JSON, which on some + * gateways carries request echoes and upstream headers. + */ + +/** Same cap the OpenAI HTTP layer uses when folding a body into an error. */ +export const PROVIDER_DETAIL_MAX_LEN = 300; + +/** + * Vendor key shapes common enough to be worth removing on sight: + * OpenAI-style `sk-…`, OpenRouter's `sk-or-…`, Google's `AIza…`, and a + * bearer token quoted out of an echoed header. Deliberately narrow — + * a pattern loose enough to catch every possible secret would redact + * model ids and error codes along with them. + */ +const KEY_SHAPED = [ + /\bsk-[A-Za-z0-9_-]{6,}/g, + /\bAIza[A-Za-z0-9_-]{10,}/g, + /\b[Bb]earer\s+[A-Za-z0-9._-]{8,}/g, +]; + +export function redactProviderDetail( + detail: string, + apiKey = "", + maxLen: number = PROVIDER_DETAIL_MAX_LEN, +): string { + // Short strings are not keys; splitting on one would shred ordinary + // words out of the message. + let out = apiKey.length >= 8 ? detail.split(apiKey).join("***") : detail; + for (const pattern of KEY_SHAPED) out = out.replace(pattern, "***"); + return out.slice(0, maxLen); +} diff --git a/src/llm/provider/verify/run-contract-probe.test.ts b/src/llm/provider/verify/run-contract-probe.test.ts new file mode 100644 index 00000000..0d39d62f --- /dev/null +++ b/src/llm/provider/verify/run-contract-probe.test.ts @@ -0,0 +1,313 @@ +import { describe, expect, it, vi } from "vitest"; + +import { + CONTRACT_PROBE_TOOL_NAME, + type ProviderContractProbeTarget, +} from "./contract-probe-types.js"; +import { runProviderContractProbe } from "./run-contract-probe.js"; + +const TARGET: ProviderContractProbeTarget = { + label: "OmniRoute", + baseUrl: "https://route.example", + apiPathPrefix: "/v1", + apiKey: "sk-secret-probe-key", + model: "vendor/some-model", +}; + +function sseEvent(payload: unknown): string { + return `data: ${JSON.stringify(payload)}\n\n`; +} + +const TOOL_CALL_STREAM = + sseEvent({ + choices: [ + { + delta: { + tool_calls: [ + { + index: 0, + id: "call_1", + type: "function", + function: { name: CONTRACT_PROBE_TOOL_NAME, arguments: '{"ok"' }, + }, + ], + }, + }, + ], + }) + + sseEvent({ + choices: [ + { + delta: { + tool_calls: [{ index: 0, function: { arguments: ":true}" } }], + }, + }, + ], + }) + + sseEvent({ choices: [{ delta: {}, finish_reason: "tool_calls" }] }) + + "data: [DONE]\n\n"; + +const TEXT_STREAM = + sseEvent({ choices: [{ delta: { content: "Happy to help." } }] }) + + sseEvent({ choices: [{ delta: {}, finish_reason: "stop" }] }) + + "data: [DONE]\n\n"; + +/** Truncated: the arguments never close and nothing announces the end. */ +const EARLY_EOF_STREAM = sseEvent({ + choices: [ + { + delta: { + tool_calls: [ + { + index: 0, + type: "function", + function: { name: CONTRACT_PROBE_TOOL_NAME, arguments: '{"ok"' }, + }, + ], + }, + }, + ], +}); + +const MALFORMED_STREAM = + sseEvent({ + choices: [ + { + delta: { + tool_calls: [ + { + index: 0, + type: "function", + function: { name: CONTRACT_PROBE_TOOL_NAME, arguments: '{"ok":' }, + }, + ], + }, + }, + ], + }) + sseEvent({ choices: [{ delta: {}, finish_reason: "tool_calls" }] }); + +function streamResponse(body: string): Response { + return new Response(body, { + status: 200, + headers: { "content-type": "text/event-stream" }, + }); +} + +function errorResponse(status: number, body: unknown): Response { + return new Response(typeof body === "string" ? body : JSON.stringify(body), { + status, + headers: { "content-type": "application/json" }, + }); +} + +/** A fetch that answers the calls in order and records the bodies sent. */ +function scriptedFetch(responses: readonly (() => Response)[]): { + fetchImpl: typeof fetch; + bodies: () => Record[]; + calls: () => number; +} { + const sent: Record[] = []; + let index = 0; + const fetchImpl = vi.fn(async (_url: unknown, init?: RequestInit) => { + sent.push(JSON.parse(String(init?.body ?? "{}")) as Record); + const next = responses[index] ?? responses[responses.length - 1]; + index += 1; + return next!(); + }) as unknown as typeof fetch; + return { + fetchImpl, + bodies: () => sent, + calls: () => index, + }; +} + +describe("runProviderContractProbe", () => { + it("proves support from one forced, streamed native tool call", async () => { + const script = scriptedFetch([() => streamResponse(TOOL_CALL_STREAM)]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + }); + + expect(result.status).toBe("tools_supported"); + expect(result.toolChoiceMode).toBe("required_named"); + expect(result.probedModel).toBe("vendor/some-model"); + // A healthy route costs exactly one request: no probe ladder, and + // nothing that could run per turn. + expect(script.calls()).toBe(1); + expect(result.requests).toBe(1); + + const body = script.bodies()[0]!; + expect(body.stream).toBe(true); + expect(body.model).toBe("vendor/some-model"); + expect(body.tool_choice).toEqual({ + type: "function", + function: { name: CONTRACT_PROBE_TOOL_NAME }, + }); + expect(JSON.stringify(body.tools)).toContain(CONTRACT_PROBE_TOOL_NAME); + }); + + it("reports a forced tool choice the route refuses but tools it accepts", async () => { + const script = scriptedFetch([ + () => errorResponse(400, { error: "tool_choice of type function is not supported" }), + () => streamResponse(TOOL_CALL_STREAM), + ]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + }); + + expect(result.status).toBe("forced_tool_choice_rejected"); + expect(result.httpStatus).toBe(400); + expect(script.calls()).toBe(2); + expect(script.bodies()[1]!.tool_choice).toBe("auto"); + }); + + it("calls a plain text answer under auto inconclusive, not unsupported", async () => { + const script = scriptedFetch([ + () => errorResponse(400, { error: "unexpected parameter" }), + () => streamResponse(TEXT_STREAM), + ]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + }); + + expect(result.status).toBe("inconclusive_no_tool_call"); + expect(result.toolChoiceMode).toBe("auto"); + }); + + it("proves the tools payload is the problem by answering without it", async () => { + // Endpoint success with no tools, refusal with them: the route works, + // and it is specifically `tools` it will not take. + const script = scriptedFetch([ + () => errorResponse(400, "Bad Request"), + () => errorResponse(400, "Bad Request"), + () => streamResponse(TEXT_STREAM), + ]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + }); + + expect(result.status).toBe("tools_payload_rejected"); + expect(script.calls()).toBe(3); + // The control request is the same streamed completion minus tools. + const control = script.bodies()[2]!; + expect(control.stream).toBe(true); + expect(control.tools).toBeUndefined(); + expect(control.tool_choice).toBeUndefined(); + }); + + it("blames the route, not tools, when the no-tools control fails too", async () => { + const script = scriptedFetch([ + () => errorResponse(500, "upstream exploded"), + () => errorResponse(500, "upstream exploded"), + () => errorResponse(500, "upstream exploded"), + ]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + }); + + expect(result.status).toBe("provider_error"); + expect(result.httpStatus).toBe(500); + }); + + it("reports a stream that ended early", async () => { + const script = scriptedFetch([() => streamResponse(EARLY_EOF_STREAM)]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + }); + + expect(result.status).toBe("stream_early_eof"); + expect(script.calls()).toBe(1); + }); + + it("reports malformed tool-call deltas", async () => { + const script = scriptedFetch([() => streamResponse(MALFORMED_STREAM)]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + }); + + expect(result.status).toBe("malformed_tool_call"); + }); + + it("stops at a quota refusal instead of climbing the ladder", async () => { + const script = scriptedFetch([ + () => errorResponse(429, { error: { code: "insufficient_quota" } }), + ]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + }); + + expect(result.status).toBe("quota_or_routing_failed"); + // Nothing another request could add: the account, not the route. + expect(script.calls()).toBe(1); + }); + + it("stops at an authentication refusal", async () => { + const script = scriptedFetch([ + () => errorResponse(401, { error: "No auth credentials found" }), + ]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + }); + + expect(result.status).toBe("endpoint_auth_failed"); + expect(script.calls()).toBe(1); + }); + + it("stops at an unknown model", async () => { + const script = scriptedFetch([ + () => errorResponse(404, { error: "model not found" }), + ]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + }); + + expect(result.status).toBe("model_unavailable"); + expect(script.calls()).toBe(1); + }); + + it("reports an unreachable endpoint without claiming anything about tools", async () => { + const fetchImpl = vi.fn(async () => { + throw new TypeError("fetch failed"); + }) as unknown as typeof fetch; + const result = await runProviderContractProbe(TARGET, { fetchImpl }); + + expect(result.status).toBe("unreachable"); + expect(result.httpStatus).toBeNull(); + }); + + it("keeps credentials and whole response bodies out of the detail", async () => { + const leak = `${"x".repeat(400)} key=${TARGET.apiKey} other=sk-someoneelseskey123`; + const script = scriptedFetch([() => errorResponse(401, leak)]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + }); + + expect(result.detail).not.toContain(TARGET.apiKey); + expect(result.detail).not.toContain("sk-someoneelseskey123"); + expect(result.detail.length).toBeLessThanOrEqual(300); + }); + + it("refuses to probe with no model rather than inventing one", async () => { + const script = scriptedFetch([() => streamResponse(TOOL_CALL_STREAM)]); + const result = await runProviderContractProbe( + { ...TARGET, model: " " }, + { fetchImpl: script.fetchImpl }, + ); + + expect(result.status).toBe("model_unavailable"); + expect(script.calls()).toBe(0); + }); + + it("reports a cancelled probe as cancelled", async () => { + const controller = new AbortController(); + controller.abort(); + const script = scriptedFetch([() => streamResponse(TOOL_CALL_STREAM)]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + signal: controller.signal, + }); + + expect(result.status).toBe("cancelled"); + expect(script.calls()).toBe(0); + }); +}); diff --git a/src/llm/provider/verify/run-contract-probe.ts b/src/llm/provider/verify/run-contract-probe.ts new file mode 100644 index 00000000..f9983b9f --- /dev/null +++ b/src/llm/provider/verify/run-contract-probe.ts @@ -0,0 +1,389 @@ +/** + * Prove — or fail to prove — that a route can run an Atomic turn. + * + * A turn is a streamed chat completion that carries a `tools` payload + * and gets a native tool call back. `/v1/models` proves none of that, + * and neither does the one-token key check: both are non-streaming, both + * send no tools. Routes that pass them and then fail a real turn are + * common enough that the failure has a name in the field reports + * (`STREAM_EARLY_EOF`, "HTTP 400, but only with tools"). + * + * So this sends the real thing, once, with a synthetic function nobody + * has registered, and reads what comes back. + * + * ## The ladder + * + * Each rung only runs when the one above it left the question open, so + * a healthy route costs exactly one request: + * + * 1. **forced named tool**, streaming, tools payload. A complete tool + * call here is the whole answer: `tools_supported`. + * 2. **`tool_choice: auto`**, same tools payload — reached only when + * rung 1 was *refused* for a reason that is not the key, the quota + * or the model. A tool call now means the payload is fine and it + * was the forcing the route would not take. + * 3. **no tools at all**, same model, same streaming transport — + * reached only when both tool requests were refused. If this + * answers, the route works and it is specifically `tools` it + * rejects; if it fails too, the failure was never about tools. + * + * Rung 3 is what turns "HTTP 400" into a sentence an operator can act + * on, and it is deliberately an experiment rather than a regex over the + * error body: provider wording for "tools unsupported" is not stable, + * but "refuses with tools, answers without them" is unambiguous. + * + * ## What it never does + * + * The synthetic call is read and discarded. It is never looked up in, + * dispatched to, or registered with the tool registry — the probe does + * not import it and could not reach it. And nothing here runs per turn: + * the only callers are setup-time or an explicit operator request. + */ + +import { openAiFetch, type OpenAiHttpDeps } from "../openai/openai-http.js"; +import { + accumulateProbeStream, + type ProbeStreamObservation, +} from "./accumulate-probe-stream.js"; +import { + classifyContractProbeHttpFailure, + classifyProbeStream, + contractProbeFailureIsTerminal, +} from "./classify-contract-probe.js"; +import { isAbortError, classifyVerifyTransportError } from "./classify-verify-response.js"; +import { + CONTRACT_PROBE_TOOL_NAME, + contractProbeToolDefinition, + type ProbeToolChoiceMode, + type ProviderContractProbeResult, + type ProviderContractProbeTarget, + type ProviderContractStatus, +} from "./contract-probe-types.js"; +import { redactProviderDetail } from "./redact-provider-detail.js"; + +/** + * Whole-probe budget, not per request. Longer than the key check's 8s + * because this one waits for a model to actually generate, but still + * short enough that a wizard screen does not feel hung: a route that + * cannot produce a two-field tool call inside this is a finding in + * itself. + */ +export const PROVIDER_CONTRACT_PROBE_TIMEOUT_MS = 20_000; + +/** Ceiling on the SSE body we buffer. A probe answer is a few hundred bytes. */ +const MAX_STREAM_BYTES = 64 * 1024; + +/** The whole ladder: forced → auto → no-tools. Never more than that. */ +const MAX_PROBE_REQUESTS = 3; + +const PROBE_PROMPT = + `Call the ${CONTRACT_PROBE_TOOL_NAME} function with ok set to true. ` + + `Do not answer in words.`; + +export async function runProviderContractProbe( + target: ProviderContractProbeTarget, + opts: { + signal?: AbortSignal; + timeoutMs?: number; + fetchImpl?: typeof fetch; + } = {}, +): Promise { + const startedAt = Date.now(); + const budgetMs = opts.timeoutMs ?? PROVIDER_CONTRACT_PROBE_TIMEOUT_MS; + const deadline = startedAt + budgetMs; + const model = target.model.trim(); + const state = { requests: 0 }; + + const emit = ( + status: ProviderContractStatus, + httpStatus: number | null, + mode: ProbeToolChoiceMode | null, + detail: string, + ): ProviderContractProbeResult => ({ + status, + probedModel: model, + httpStatus, + toolChoiceMode: mode, + detail: redactProviderDetail(detail, target.apiKey), + latencyMs: Date.now() - startedAt, + requests: state.requests, + }); + + if (model.length === 0) { + return emit("model_unavailable", null, null, "no model configured to probe"); + } + + const run = async (body: Record): Promise => + runRung(target, body, { + deadline, + state, + ...(opts.signal ? { signal: opts.signal } : {}), + ...(opts.fetchImpl ? { fetchImpl: opts.fetchImpl } : {}), + }); + + // Rung 1 — the real contract, forced. + const forced = await run(probeBody(model, "required_named")); + if (forced.kind === "aborted") { + return emit(forced.status, null, "required_named", forced.detail); + } + if (forced.kind === "stream") { + return emit( + classifyProbeStream(forced.observation, "required_named"), + forced.httpStatus, + "required_named", + streamDetail(forced.observation), + ); + } + const forcedStatus = classifyContractProbeHttpFailure( + forced.httpStatus, + forced.body, + ); + if (contractProbeFailureIsTerminal(forcedStatus)) { + return emit(forcedStatus, forced.httpStatus, "required_named", forced.body); + } + + // Rung 2 — same payload, no forcing. + const auto = await run(probeBody(model, "auto")); + if (auto.kind === "aborted") { + return emit(auto.status, null, "auto", auto.detail); + } + if (auto.kind === "stream") { + const autoStatus = classifyProbeStream(auto.observation, "auto"); + // Tools work; it was the forced choice rung 1 asked for that this + // route would not take. Reporting rung 2's own verdict here would + // hide the actual limitation behind a cheerful "supported". + if (autoStatus === "tools_supported") { + return emit( + "forced_tool_choice_rejected", + forced.httpStatus, + "required_named", + forced.body, + ); + } + return emit(autoStatus, auto.httpStatus, "auto", streamDetail(auto.observation)); + } + const autoStatus = classifyContractProbeHttpFailure(auto.httpStatus, auto.body); + if (contractProbeFailureIsTerminal(autoStatus)) { + return emit(autoStatus, auto.httpStatus, "auto", auto.body); + } + + // Rung 3 — the control. Same model, same streaming transport, no tools. + const control = await run(probeBody(model, null)); + if (control.kind === "aborted") { + return emit(control.status, null, null, control.detail); + } + if (control.kind === "stream") { + // It answers without tools and refuses with them. That is the + // finding, stated from evidence rather than from error wording. + return emit("tools_payload_rejected", auto.httpStatus, "auto", auto.body); + } + const controlStatus = classifyContractProbeHttpFailure( + control.httpStatus, + control.body, + ); + // The control failed too, so nothing here was ever about tools. + return emit( + contractProbeFailureIsTerminal(controlStatus) ? controlStatus : "provider_error", + control.httpStatus, + null, + control.body, + ); +} + +type RungOutcome = + | { kind: "stream"; httpStatus: number; observation: ProbeStreamObservation } + | { kind: "http_error"; httpStatus: number; body: string } + | { + kind: "aborted"; + status: Extract< + ProviderContractStatus, + "timeout" | "cancelled" | "unreachable" | "provider_error" + >; + detail: string; + }; + +async function runRung( + target: ProviderContractProbeTarget, + body: Record, + ctx: { + deadline: number; + state: { requests: number }; + signal?: AbortSignal; + fetchImpl?: typeof fetch; + }, +): Promise { + if (ctx.signal?.aborted) { + return { kind: "aborted", status: "cancelled", detail: "probe cancelled" }; + } + if (ctx.state.requests >= MAX_PROBE_REQUESTS) { + return { + kind: "aborted", + status: "provider_error", + detail: "probe budget spent", + }; + } + const remainingMs = ctx.deadline - Date.now(); + if (remainingMs <= 0) { + return { kind: "aborted", status: "timeout", detail: "probe deadline reached" }; + } + ctx.state.requests += 1; + + const deps: OpenAiHttpDeps = { + baseUrl: target.baseUrl, + apiKey: target.apiKey, + extraHeaders: target.extraHeaders ?? {}, + requestTimeoutMs: remainingMs, + fetchImpl: ctx.fetchImpl ?? fetch, + label: target.label, + }; + + let res: Response; + try { + res = await openAiFetch( + deps, + `${target.apiPathPrefix}/chat/completions`, + body, + { ...(ctx.signal ? { signal: ctx.signal } : {}) }, + true, + "POST", + ); + } catch (err) { + if (ctx.signal?.aborted || isAbortError(err)) { + return { kind: "aborted", status: "cancelled", detail: "probe cancelled" }; + } + // Transport failures are read by the key check's classifier, so a + // refused connection or an expired deadline means the same thing in + // both checks. Only its `cancelled` verdict is unreachable here — + // that case is handled above. + const transport = classifyVerifyTransportError(err); + const status = + transport === "timeout" || transport === "unreachable" + ? transport + : "provider_error"; + return { + kind: "aborted", + status, + detail: err instanceof Error ? err.message : String(err), + }; + } + + if (!res.ok) { + const text = await res.text().catch(() => ""); + return { kind: "http_error", httpStatus: res.status, body: text }; + } + + // `openAiFetch`'s own timeout covers the connect only — it clears the + // timer the moment headers arrive. A route that opens a stream and + // then stalls forever would hang here, so the body read carries the + // remaining budget itself. + const sse = await readStreamBounded(res, ctx.deadline - Date.now()); + if (sse.timedOut && sse.text.length === 0) { + return { + kind: "aborted", + status: "timeout", + detail: "no stream data before deadline", + }; + } + return { + kind: "stream", + httpStatus: res.status, + observation: accumulateProbeStream(sse.text), + }; +} + +/** + * The probe request, in the same shape `buildOpenAiChatBody` gives a + * real turn — including `parallel_tool_calls`, which some routes + * validate and which a turn always sends alongside tools. + * + * `mode === null` is the control: no tools, no tool choice, otherwise + * identical, so a difference in outcome can only be the tools payload. + * + * No `max_tokens`. The one-token key check has to cap spend and + * therefore has to guess between `max_tokens` and + * `max_completion_tokens` (newer OpenAI models reject the former). + * Here a cap would risk truncating the very tool call being measured, + * reporting malformed deltas that were in fact our own doing — and one + * forced call to a single-field function is cheap enough uncapped. + */ +function probeBody( + model: string, + mode: ProbeToolChoiceMode | null, +): Record { + const body: Record = { + model, + messages: [{ role: "user", content: PROBE_PROMPT }], + temperature: 0, + stream: true, + }; + if (mode === null) return body; + body.tools = [contractProbeToolDefinition()]; + body.parallel_tool_calls = true; + body.tool_choice = + mode === "required_named" + ? { type: "function", function: { name: CONTRACT_PROBE_TOOL_NAME } } + : "auto"; + return body; +} + +/** + * A one-line summary of what the stream contained. Deliberately not the + * body: the assistant text is the model's own words about a synthetic + * function and has no diagnostic value worth logging. + */ +function streamDetail(observation: ProbeStreamObservation): string { + const names = observation.toolCalls + .map((call) => (call.name.length > 0 ? call.name : "")) + .join(", "); + return [ + `finish_reason=${observation.finishReason ?? "none"}`, + `terminal=${observation.terminalObserved}`, + `tool_call_deltas=${observation.sawToolCallDelta}`, + `tool_calls=[${names}]`, + `text_chars=${observation.text.length}`, + ].join(" "); +} + +/** + * Buffer the SSE body under a byte ceiling and a deadline, cancelling + * the stream rather than leaving a socket open behind us. + */ +async function readStreamBounded( + res: Response, + budgetMs: number, +): Promise<{ text: string; timedOut: boolean }> { + if (!res.body) return { text: await res.text().catch(() => ""), timedOut: false }; + const reader = res.body.getReader(); + const decoder = new TextDecoder(); + let text = ""; + let timedOut = false; + let timer: ReturnType | undefined; + try { + const deadline = + budgetMs > 0 + ? new Promise<"deadline">((resolve) => { + timer = setTimeout(() => resolve("deadline"), budgetMs); + }) + : Promise.resolve<"deadline">("deadline"); + for (;;) { + const next = await Promise.race([reader.read(), deadline]); + if (next === "deadline") { + timedOut = true; + break; + } + if (next.done) { + text += decoder.decode(); + break; + } + text += decoder.decode(next.value, { stream: true }); + if (text.length >= MAX_STREAM_BYTES) break; + } + } catch { + // A stream that breaks mid-body is exactly the early-EOF case: keep + // what arrived and let the classifier see that it never terminated. + } finally { + if (timer) clearTimeout(timer); + void reader.cancel().catch(() => {}); + } + return { text, timedOut }; +} diff --git a/src/llm/provider/verify/verify-provider-key.ts b/src/llm/provider/verify/verify-provider-key.ts index 8a9f8f10..5ab7e98c 100644 --- a/src/llm/provider/verify/verify-provider-key.ts +++ b/src/llm/provider/verify/verify-provider-key.ts @@ -19,6 +19,10 @@ import { classifyVerifyTransportError, isAbortError, } from "./classify-verify-response.js"; +import { + redactProviderDetail, + PROVIDER_DETAIL_MAX_LEN, +} from "./redact-provider-detail.js"; import type { ProviderVerifyResult, ProviderVerifyStatus, @@ -35,8 +39,6 @@ export const PROVIDER_VERIFY_TIMEOUT_MS = 8_000; /** model → other token field → next model. Never more than that. */ const MAX_VERIFY_REQUESTS = 3; -/** Provider error bodies are quoted back bounded, same cap as the HTTP layer. */ -const VERIFY_DETAIL_MAX_LEN = 300; export async function verifyProviderKey( target: ProviderVerifyTarget, @@ -168,7 +170,7 @@ function probeBody( async function readBounded(res: Response): Promise { const text = await res.text().catch(() => ""); - return text.slice(0, VERIFY_DETAIL_MAX_LEN); + return text.slice(0, PROVIDER_DETAIL_MAX_LEN); } function result( @@ -183,16 +185,7 @@ function result( status, probedModel, httpStatus, - detail: redactKey(detail, apiKey).slice(0, VERIFY_DETAIL_MAX_LEN), + detail: redactProviderDetail(detail, apiKey), latencyMs: Date.now() - startedAt, }; } - -/** - * Some providers echo the offending credential back in the error body, - * and this detail is headed for a status line and the log file. - */ -function redactKey(detail: string, apiKey: string): string { - if (apiKey.length < 8) return detail; - return detail.split(apiKey).join("***"); -} diff --git a/src/tui/commands/slash-command-handler.test.ts b/src/tui/commands/slash-command-handler.test.ts index 65e52e2d..83e02b8d 100644 --- a/src/tui/commands/slash-command-handler.test.ts +++ b/src/tui/commands/slash-command-handler.test.ts @@ -263,6 +263,21 @@ describe("dispatchSlashCommand", () => { ]); }); + it("asks for a contract probe of the active provider on /llm check", () => { + const result = dispatchSlashCommand("/llm check"); + expect(result.actions).toEqual([ + { type: "providers_contract_probe_requested", providerId: null }, + ]); + // The operator is told a request is about to be spent on their key. + expect(result.systemMessage).toContain("one request"); + }); + + it("names /llm check in the usage line so it is discoverable", () => { + const result = dispatchSlashCommand("/llm nonsense"); + expect(result.actions).toEqual([]); + expect(result.systemMessage).toContain("/llm check"); + }); + it("signals triggerLocalModelsStatus for /models status", () => { const result = dispatchSlashCommand("/models status"); expect(result.triggerLocalModelsStatus).toBe(true); diff --git a/src/tui/commands/slash-command-handler.ts b/src/tui/commands/slash-command-handler.ts index 2bb2d2c1..121ad895 100644 --- a/src/tui/commands/slash-command-handler.ts +++ b/src/tui/commands/slash-command-handler.ts @@ -697,8 +697,20 @@ function dispatchLlmSub(rawArgs: string): SlashDispatchResult { { type: "providers_refresh_requested" }, ]); } + // `/llm check` exercises the real turn contract against the active + // provider — streaming, tools payload, a forced native tool call — + // and reports what came back. A reachable `/v1/models` says nothing + // about any of that, so until now the first real message was the + // test. Explicit only: it spends requests against the operator's own + // account and must never run on a turn path. + if (/^check$/i.test(argPart)) { + return pureActions([{ type: "providers_contract_probe_requested", providerId: null }], { + systemMessage: + "checking the active provider's streaming tool-call contract — this sends one request", + }); + } return pureActions([], { - systemMessage: "usage: /llm | /llm provider | /llm fallback", + systemMessage: "usage: /llm | /llm provider | /llm check | /llm fallback", }); } diff --git a/src/tui/components/cloud-provider-onboarding.tsx b/src/tui/components/cloud-provider-onboarding.tsx index 7fcf3b90..7854ad05 100644 --- a/src/tui/components/cloud-provider-onboarding.tsx +++ b/src/tui/components/cloud-provider-onboarding.tsx @@ -14,6 +14,7 @@ import { createProvidersWizardState } from "../providers/providers-wizard-state. import type { ProvidersWizardState } from "../providers/providers-wizard-state.js"; import { saveProviderWizardToConfig } from "../providers/save-provider-wizard.js"; import { verifyWizardBeforeSave } from "../providers/verify-wizard-before-save.js"; +import { probeWizardContract } from "../providers/probe-wizard-contract.js"; import { theme } from "../theme/theme.js"; import { ProvidersWizard } from "./providers-wizard.js"; @@ -86,8 +87,24 @@ export function CloudProviderOnboarding(props: { setSubmitting(false); return; } + // The key is good; whether the route can run a turn is a + // separate question, and first-run is exactly where getting it + // wrong costs the most — the operator's first message is + // otherwise the test. Advisory only: it cannot stop the save, + // and it rides the same abort, so Esc still abandons the whole + // submit with nothing written. + const contract = await probeWizardContract(nextWizard, { + signal: abort.signal, + }); + if (!checkStillWanted(abort)) return; saveProviderWizardToConfig(nextWizard); - props.onFinished("saved_cloud", gate.warning ?? undefined); + const notes = [gate.warning, contract.warning].filter( + (note): note is string => Boolean(note), + ); + props.onFinished( + "saved_cloud", + notes.length > 0 ? notes.join(" ") : undefined, + ); } catch (err) { // An abandoned run does not get to report a failure either: it // would paint over the screen the operator was handed back, and diff --git a/src/tui/providers/contract-probe-target.ts b/src/tui/providers/contract-probe-target.ts new file mode 100644 index 00000000..cd926373 --- /dev/null +++ b/src/tui/providers/contract-probe-target.ts @@ -0,0 +1,110 @@ +/** + * What to send the contract probe, derived from a wizard run — or why + * there is nothing here to probe. + * + * Same endpoint resolution the key check uses (`endpointForKind`, + * `apiKeyForWizard`), with one deliberate difference: the model. + * + * `pickProbeModels` picks the *cheapest paid* model it can find, because + * the key check's question is "can this account pay for a token". The + * contract probe's question is "can the route I am about to use run a + * turn", and route limitations are per-model — a gateway can stream + * native tool calls for one model and refuse `tools` outright for + * another. Probing anything but the configured model would answer a + * question nobody asked. + * + * The skip cases are named rather than collapsed into `null` because an + * explicitly requested check has to report them: "this is a server on + * your own machine" and "this provider has no key yet" are different + * answers, and telling an operator the second when the first is true is + * how a diagnostic tool loses their trust. + */ + +import type { ProviderContractProbeTarget } from "../../llm/provider/verify/index.js"; +import { isLocalProviderUrl } from "./is-local-provider-url.js"; +import { + apiKeyForWizard, + chosenModelForWizard, + endpointForKind, + providerLabelForWizard, + wizardKeyIsOptional, +} from "./providers-wizard-target.js"; +import type { ProvidersWizardState } from "./providers-wizard-state.js"; + +export type ContractProbeSkipReason = + /** The wizard has not settled on a provider kind yet. */ + | "no_kind" + /** A subscription CLI: a subprocess, not an HTTP contract. */ + | "cli_backed" + /** A server on the operator's own machine. */ + | "local_endpoint" + /** No key resolved, and this service is not one that works without one. */ + | "no_api_key" + /** No model chosen, so there is nothing to probe *with*. */ + | "no_model"; + +export type ContractProbeTargetResolution = + | { readonly kind: "target"; readonly target: ProviderContractProbeTarget } + | { readonly kind: "skipped"; readonly reason: ContractProbeSkipReason }; + +export function contractProbeTargetForWizard( + wizard: ProvidersWizardState, +): ContractProbeTargetResolution { + const kind = wizard.kind; + if (!kind) return skip("no_kind"); + // A CLI-backed provider speaks its vendor's own protocol through a + // subprocess; there is no OpenAI-compatible endpoint here to hold to + // this contract, and inventing one would probe a URL it never uses. + if (kind === "claude-cli" || kind === "codex-cli") return skip("cli_backed"); + + const apiKey = apiKeyForWizard(wizard)?.trim() ?? ""; + // Keyless is legitimate for local servers and keyless-listing + // services; a missing key everywhere else is already refused by the + // key screen, and probing without one would only re-report that. + if (!apiKey && !wizardKeyIsOptional(wizard)) return skip("no_api_key"); + + const endpoint = endpointForKind(kind, wizard); + // A server on this machine is the operator's own: reachable, free to + // call, and a probe against it says more about their llama-server + // flags than about a provider. The key check skips it for the same + // reason. + if (isLocalProviderUrl(endpoint.baseUrl)) return skip("local_endpoint"); + + const model = chosenModelForWizard(wizard).trim(); + if (!model) return skip("no_model"); + + return { + kind: "target", + target: { + label: providerLabelForWizard(wizard), + baseUrl: endpoint.baseUrl, + apiPathPrefix: endpoint.apiPathPrefix, + apiKey, + model, + ...(endpoint.extraHeaders ? { extraHeaders: endpoint.extraHeaders } : {}), + }, + }; +} + +/** Why nothing was probed, in the operator's words. */ +export function describeContractProbeSkip( + reason: ContractProbeSkipReason, + label: string, +): string { + switch (reason) { + case "cli_backed": + return `${label} runs through a CLI, not an HTTP endpoint — there is no streaming tool contract to check.`; + case "local_endpoint": + return `${label} is a server on this machine — nothing to check against a provider.`; + case "no_api_key": + return `${label} has no API key yet, so the contract check has nothing to authenticate with.`; + case "no_model": + return `${label} has no chat model set, so there is nothing to run the contract check with.`; + default: + return `${label} is not configured far enough to run a contract check.`; + } +} + +function skip(reason: ContractProbeSkipReason): ContractProbeTargetResolution { + return { kind: "skipped", reason }; +} diff --git a/src/tui/providers/describe-contract-probe.ts b/src/tui/providers/describe-contract-probe.ts new file mode 100644 index 00000000..c1df4bfb --- /dev/null +++ b/src/tui/providers/describe-contract-probe.ts @@ -0,0 +1,59 @@ +/** + * One sentence per contract-probe verdict, in the voice the key check + * and the cloud error path already use: who answered, what happened, + * what to do about it. + * + * Three rules the wording has to keep: + * + * - Never say "incompatible" for something the probe did not establish. + * `inconclusive_no_tool_call` is the model declining to call a + * pointless function, which is legal behaviour, not a broken route. + * - Never say "compatible" for anything but a completed tool call. + * - Always name the next move. `HTTP 400` on a setup screen reads as a + * product failure; "answers fine until `tools` is in the request" + * reads as a route to change. + */ + +import type { ProviderContractProbeResult } from "../../llm/provider/verify/index.js"; + +export function describeContractProbeOutcome( + result: ProviderContractProbeResult, + label: string, +): string { + const who = `"${label}"`; + const on = ` on ${result.probedModel}`; + switch (result.status) { + case "tools_supported": + return `${who} streamed a native tool call${on} — this route can run a turn.`; + case "inconclusive_no_tool_call": + return `${who} answered in text instead of calling a tool${on}. Inconclusive: it would not take a forced tool choice, so whether it can emit tool calls is still unknown.`; + case "forced_tool_choice_ignored": + return `${who} accepted a forced tool choice${on} and answered in text anyway. Turns that must call a tool may loop or stall on this route.`; + case "forced_tool_choice_rejected": + return `${who} refuses a forced tool choice${on} but does emit tool calls without one${statusSuffix(result)}. Usable, with less control over when tools fire.`; + case "tools_payload_rejected": + return `${who} answers this model until "tools" is in the request, then refuses it${statusSuffix(result)}. Pick another model or route — Atomic sends tools on every turn.`; + case "model_unavailable": + return `${who} does not recognise ${result.probedModel}${statusSuffix(result)}. Pick a model this route actually serves.`; + case "endpoint_auth_failed": + return `${who} rejected the key when asked for a streamed tool call${statusSuffix(result)}. Tool support is still untested.`; + case "quota_or_routing_failed": + return `${who} could not run the check — out of quota, or no backend to route to${statusSuffix(result)}. Tool support is still untested.`; + case "stream_early_eof": + return `${who} closed the stream${on} before finishing the answer. Turns will end mid-tool-call on this route.`; + case "malformed_tool_call": + return `${who} streamed tool-call deltas${on} that never formed a callable tool. Atomic would fail the turn with a tool it cannot look up.`; + case "unreachable": + return `Could not reach ${who} for the contract check. Tool support is untested — check the connection or the base URL.`; + case "timeout": + return `${who} did not finish the contract check in time${on}. Tool support is untested.`; + case "cancelled": + return `Contract check cancelled. Tool support is untested.`; + default: + return `${who} failed the contract check${statusSuffix(result)}. Tool support is untested — this looks like the route, not your setup.`; + } +} + +function statusSuffix(result: ProviderContractProbeResult): string { + return result.httpStatus === null ? "" : ` (${result.httpStatus})`; +} diff --git a/src/tui/providers/probe-wizard-contract.test.ts b/src/tui/providers/probe-wizard-contract.test.ts new file mode 100644 index 00000000..dae85423 --- /dev/null +++ b/src/tui/providers/probe-wizard-contract.test.ts @@ -0,0 +1,170 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { CONTRACT_PROBE_TOOL_NAME } from "../../llm/provider/verify/index.js"; +import { probeWizardContract } from "./probe-wizard-contract.js"; +import { createProvidersWizardState } from "./providers-wizard-state.js"; +import type { + ProvidersWizardKind, + ProvidersWizardState, +} from "./providers-wizard-state.js"; + +const ENV_KEYS = [ + "OPENROUTER_API_KEY", + "AIMLAPI_API_KEY", + "GEMINI_API_KEY", + "OPENAI_COMPAT_API_KEY", + "OPENAI_API_KEY", + "LMSTUDIO_API_KEY", +] as const; + +function wizard( + kind: ProvidersWizardKind, + overrides: Partial = {}, +): ProvidersWizardState { + return { + ...createProvidersWizardState("add", { kind }), + phase: "api_key", + apiKeyBuffer: "sk-test-key", + ...overrides, + }; +} + +function sseEvent(payload: unknown): string { + return `data: ${JSON.stringify(payload)}\n\n`; +} + +const TOOL_CALL_STREAM = + sseEvent({ + choices: [ + { + delta: { + tool_calls: [ + { + index: 0, + id: "call_1", + type: "function", + function: { name: CONTRACT_PROBE_TOOL_NAME, arguments: '{"ok":true}' }, + }, + ], + }, + }, + ], + }) + + sseEvent({ choices: [{ delta: {}, finish_reason: "tool_calls" }] }) + + "data: [DONE]\n\n"; + +beforeEach(() => { + for (const key of ENV_KEYS) delete process.env[key]; +}); +afterEach(() => { + vi.unstubAllGlobals(); + for (const key of ENV_KEYS) delete process.env[key]; +}); + +describe("probeWizardContract", () => { + it("proves a route that streams a native tool call", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => new Response(TOOL_CALL_STREAM, { status: 200 })), + ); + const outcome = await probeWizardContract(wizard("openrouter")); + expect(outcome.proven).toBe(true); + expect(outcome.warning).toBeNull(); + expect(outcome.summary).toContain("can run a turn"); + }); + + it("warns without blocking when the route refuses the tools payload", async () => { + // Refuses with tools twice, answers the no-tools control: the + // route works, and it is `tools` it will not take. + const bodies: string[] = []; + vi.stubGlobal( + "fetch", + vi.fn(async (_url: unknown, init?: RequestInit) => { + bodies.push(String(init?.body ?? "")); + if (bodies.length <= 2) return new Response("Bad Request", { status: 400 }); + return new Response( + sseEvent({ choices: [{ delta: { content: "hi" }, finish_reason: "stop" }] }), + { status: 200 }, + ); + }), + ); + const outcome = await probeWizardContract(wizard("aimlapi")); + expect(outcome.proven).toBe(false); + expect(outcome.warning).toContain('"tools"'); + // Advisory only: nothing here can refuse a save. + expect(outcome.result?.status).toBe("tools_payload_rejected"); + }); + + it("does not call an inconclusive auto answer a failure of the route", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async (_url: unknown, init?: RequestInit) => { + const body = String(init?.body ?? ""); + if (body.includes('"tool_choice":{')) { + return new Response("Bad Request", { status: 400 }); + } + return new Response( + sseEvent({ choices: [{ delta: { content: "Sure!" }, finish_reason: "stop" }] }), + { status: 200 }, + ); + }), + ); + const outcome = await probeWizardContract(wizard("openrouter")); + expect(outcome.result?.status).toBe("inconclusive_no_tool_call"); + expect(outcome.warning).toContain("Inconclusive"); + // "Unproven" is not "incompatible", and the wording must not drift. + expect(outcome.warning).not.toContain("cannot"); + }); + + it("never calls out for a server on this machine", async () => { + const fetchMock = vi.fn(async () => new Response(TOOL_CALL_STREAM, { status: 200 })); + vi.stubGlobal("fetch", fetchMock); + const outcome = await probeWizardContract( + wizard("openai-compatible", { + apiKeyBuffer: "", + baseUrlLine: "http://127.0.0.1:8000", + }), + ); + expect(outcome.proven).toBe(false); + // A skip is not a warning: a local server is not a defect to report. + expect(outcome.warning).toBeNull(); + expect(outcome.skipped).toBe("local_endpoint"); + expect(outcome.summary).toContain("server on this machine"); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("never calls out for a CLI-backed provider", async () => { + const fetchMock = vi.fn(async () => new Response(TOOL_CALL_STREAM, { status: 200 })); + vi.stubGlobal("fetch", fetchMock); + const outcome = await probeWizardContract(wizard("claude-cli")); + expect(outcome.skipped).toBe("cli_backed"); + expect(outcome.warning).toBeNull(); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("says a key is missing rather than pretending there is nothing to check", async () => { + const fetchMock = vi.fn(async () => new Response(TOOL_CALL_STREAM, { status: 200 })); + vi.stubGlobal("fetch", fetchMock); + const outcome = await probeWizardContract( + wizard("openrouter", { apiKeyBuffer: "" }), + ); + expect(outcome.skipped).toBe("no_api_key"); + expect(outcome.summary).toContain("no API key"); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("probes the model the operator picked, not a cheap stand-in", async () => { + const bodies: string[] = []; + vi.stubGlobal( + "fetch", + vi.fn(async (_url: unknown, init?: RequestInit) => { + bodies.push(String(init?.body ?? "")); + return new Response(TOOL_CALL_STREAM, { status: 200 }); + }), + ); + await probeWizardContract( + wizard("openrouter", { selectedChatModelId: "vendor/chosen-model" }), + ); + expect(bodies[0]).toContain("vendor/chosen-model"); + }); +}); diff --git a/src/tui/providers/probe-wizard-contract.ts b/src/tui/providers/probe-wizard-contract.ts new file mode 100644 index 00000000..529d5fae --- /dev/null +++ b/src/tui/providers/probe-wizard-contract.ts @@ -0,0 +1,101 @@ +/** + * The setup-time contract check, run once per provider save. + * + * Deliberately separate from `verifyWizardBeforeSave`, and deliberately + * unable to stop a save: + * + * - The key gate answers "is this credential usable", and a dead key is + * worth refusing because nothing downstream can work without one. + * - This answers "can this route run a turn", and the honest response + * to "no" is a warning, not a refusal. Some providers block synthetic + * probes outright; a custom endpoint the operator knows works must + * still be savable, and the operator is the one who decides whether + * to live with a route that fires tools only under `auto`. + * + * What it must not do is let a failed probe pass for a proven one — the + * caller keys "this install has a working cloud backend" off a clean + * result, so an unproven route reports as unproven. + */ + +import { + contractProbeProvesToolSupport, + runProviderContractProbe, + type ProviderContractProbeResult, +} from "../../llm/provider/verify/index.js"; +import { + contractProbeTargetForWizard, + describeContractProbeSkip, + type ContractProbeSkipReason, +} from "./contract-probe-target.js"; +import { describeContractProbeOutcome } from "./describe-contract-probe.js"; +import { providerLabelForWizard } from "./providers-wizard-target.js"; +import type { ProvidersWizardState } from "./providers-wizard-state.js"; + +export interface WizardContractProbeOutcome { + /** + * `true` only when a complete native tool call came back. Anything + * else — including a probe that never ran — leaves the route + * unproven, and callers must treat unproven as unproven. + */ + readonly proven: boolean; + /** + * What to show the operator, or `null` when there is nothing worth + * saying: the route passed, or there was nothing here to probe. + */ + readonly warning: string | null; + /** + * One sentence for the operator, whatever happened — a clean pass, a + * defect, or the reason no probe ran. An explicitly requested check + * has to be able to report all three; a save only wants `warning`. + */ + readonly summary: string; + /** Set when no request was made, saying which case this was. */ + readonly skipped: ContractProbeSkipReason | null; + /** The raw verdict, for callers that log or branch on it. */ + readonly result: ProviderContractProbeResult | null; +} + +/** + * Tighter than the probe module's own budget. This one runs with an + * operator watching a wizard screen: a route slow enough to blow + * through it has told us something already, and the save can proceed + * with the warning rather than holding the screen. + */ +export const WIZARD_CONTRACT_PROBE_TIMEOUT_MS = 12_000; + +export async function probeWizardContract( + wizard: ProvidersWizardState, + opts: { signal?: AbortSignal; timeoutMs?: number } = {}, +): Promise { + const resolved = contractProbeTargetForWizard(wizard); + if (resolved.kind === "skipped") { + return { + proven: false, + // Not a warning: none of the skip cases is a defect to act on, + // and a wizard that reported one would cry wolf on every local + // server it ever saved. + warning: null, + summary: describeContractProbeSkip( + resolved.reason, + providerLabelForWizard(wizard), + ), + skipped: resolved.reason, + result: null, + }; + } + const target = resolved.target; + + const result = await runProviderContractProbe(target, { + ...(opts.signal ? { signal: opts.signal } : {}), + timeoutMs: opts.timeoutMs ?? WIZARD_CONTRACT_PROBE_TIMEOUT_MS, + }); + const proven = contractProbeProvesToolSupport(result.status); + const summary = describeContractProbeOutcome(result, target.label); + return { + proven, + warning: proven ? null : summary, + summary, + skipped: null, + result, + }; +} diff --git a/src/tui/providers/providers-actions.ts b/src/tui/providers/providers-actions.ts index 9eb5cc21..97040b16 100644 --- a/src/tui/providers/providers-actions.ts +++ b/src/tui/providers/providers-actions.ts @@ -89,6 +89,19 @@ export type ProvidersAction = generation: number; error: string; } + | { + /** + * Run the side-effect-free provider contract probe against + * `providerId` (`null` = active text provider), on explicit + * operator request (`/llm check`). Handled by + * `ProvidersOrchestrator.runContractProbe`; `submit-handler` + * routes it through `onProvidersContractProbeRequested` for the + * same reason as the picker request above — a dispatched reducer + * action never reaches the event bus the orchestrator listens on. + */ + type: "providers_contract_probe_requested"; + providerId: string | null; + } | { type: "providers_wizard_updated"; wizard: ProvidersWizardState } | { type: "providers_wizard_closed" } | { type: "providers_wizard_submit_started" } diff --git a/src/tui/providers/providers-orchestrator.test.ts b/src/tui/providers/providers-orchestrator.test.ts index 1e47cf99..fb2de59a 100644 --- a/src/tui/providers/providers-orchestrator.test.ts +++ b/src/tui/providers/providers-orchestrator.test.ts @@ -363,6 +363,73 @@ describe("ProvidersOrchestrator.completeWizard", () => { expect(failure?.error).toContain("rejected this key"); }); + it("runs the contract probe on explicit request and reports the verdict", async () => { + currentConfig = { + llm: { + activeTextProvider: "openrouter", + activeEmbeddingProvider: "local-llama-embed", + toolTransport: "auto", + providers: [ + { + id: "openrouter", + kind: "openrouter", + apiKey: "sk-saved-key", + defaultChatModel: "vendor/configured-model", + }, + ], + }, + } as AtomicAgentConfig; + const bodies: string[] = []; + vi.stubGlobal( + "fetch", + vi.fn(async (_url: unknown, init?: RequestInit) => { + bodies.push(String(init?.body ?? "")); + return new Response( + `data: ${JSON.stringify({ + choices: [ + { + delta: { + tool_calls: [ + { + index: 0, + type: "function", + function: { + name: "atomic_contract_probe", + arguments: '{"ok":true}', + }, + }, + ], + }, + }, + ], + })}\n\ndata: ${JSON.stringify({ + choices: [{ delta: {}, finish_reason: "tool_calls" }], + })}\n\ndata: [DONE]\n\n`, + { status: 200 }, + ); + }), + ); + const { ProvidersOrchestrator } = await importFreshOrchestrator(); + const bus = fakeBus(); + const orchestrator = new ProvidersOrchestrator(fakeRuntime(), bus as never); + + await orchestrator.runContractProbe(null); + + const lines = bus.emit.mock.calls + .map((call) => call[0] as { type: string; line?: string }) + .filter((action) => action.type === "providers_status") + .map((action) => action.line ?? ""); + expect(lines.some((line) => line.includes("can run a turn"))).toBe(true); + // The configured model, streamed, with the tools payload — the real + // turn contract, not a cheap stand-in. + expect(bodies[0]).toContain("vendor/configured-model"); + expect(bodies[0]).toContain('"stream":true'); + expect(bodies[0]).toContain("atomic_contract_probe"); + // Nothing was written and nothing was activated: this is a read-only + // check an operator can run whenever they like. + expect(bodies).toHaveLength(1); + }); + it("hands the cancel back to the wizard while a check is in flight", async () => { currentConfig = configWithGemini(); let releaseFetch: () => void = () => {}; diff --git a/src/tui/providers/providers-orchestrator.ts b/src/tui/providers/providers-orchestrator.ts index 67e18892..aea038ca 100644 --- a/src/tui/providers/providers-orchestrator.ts +++ b/src/tui/providers/providers-orchestrator.ts @@ -31,7 +31,11 @@ import { isProvidersAction } from "./providers-actions.js"; import type { ProviderRow } from "./providers-panel-state.js"; import { saveProviderWizardToConfig } from "./save-provider-wizard.js"; import { verifyWizardBeforeSave } from "./verify-wizard-before-save.js"; -import { wizardKindForSubscriptionCli } from "./providers-wizard-state.js"; +import { probeWizardContract } from "./probe-wizard-contract.js"; +import { + createProvidersWizardState, + wizardKindForSubscriptionCli, +} from "./providers-wizard-state.js"; import type { ProvidersWizardKind, ProvidersWizardState, @@ -51,6 +55,13 @@ export class ProvidersOrchestrator { /** Aborts the pre-save key check when the operator presses Esc. */ private wizardVerifyAbort: AbortController | null = null; + /** + * Guards `/llm check` against a second run while one is in flight. + * The probe spends real requests against a route the operator may be + * paying per token for; a held-down key must not queue three of them. + */ + private contractProbeRunning = false; + constructor( private readonly runtime: AgentRuntime, private readonly bus: TuiEventBus & { emit(action: unknown): void }, @@ -365,13 +376,25 @@ export class ProvidersOrchestrator { // A cancel already put the wizard back in an editable state; a // late verdict from the abandoned check must not overwrite it. if (abort.signal.aborted) return; - // The check is over; from here Esc has nothing to cancel and must - // not interrupt the save that follows. - this.wizardVerifyAbort = null; if (!gate.proceed) { + this.wizardVerifyAbort = null; this.bus.emit({ type: "providers_wizard_failed", error: gate.error }); return; } + // A live key proves the account, not the route. Exercise the real + // contract once — streaming, tools payload, forced native tool + // call — before this provider is reported as working, so a route + // that only fails with `tools` in the body is named here instead + // of on the operator's first message. + // + // It cannot refuse the save (see `probe-wizard-contract`), but it + // runs while Esc can still abandon the whole submit, so nothing + // has reached disk yet if the operator gives up on a slow route. + const contract = await probeWizardContract(wizard, { signal: abort.signal }); + if (abort.signal.aborted) return; + // Both checks are over; from here Esc has nothing to cancel and + // must not interrupt the save that follows. + this.wizardVerifyAbort = null; const built = saveProviderWizardToConfig(wizard); const exists = this.runtime.providerRegistry .listIds() @@ -392,7 +415,12 @@ export class ProvidersOrchestrator { // an unreachable check is not a proven backend. A warned save that // turns out to work reports on a later verified save instead. // Only the provider id travels — never the key or the base URL. - if (gate.warning === null) { + // + // The contract probe joins the same gate: a route that could not + // stream a native tool call has not been shown to run a turn, and + // reporting it as a working backend would be the silent + // "fully compatible" this check exists to prevent. + if (gate.warning === null && contract.warning === null) { this.runtime.reportModelConfigured(built.entry.id, "cloud"); } if (gate.warning) { @@ -401,6 +429,10 @@ export class ProvidersOrchestrator { this.bus.emit({ type: "providers_status", line: gate.warning }); this.bus.emit({ type: "runtime_info", line: gate.warning }); } + if (contract.warning) { + this.bus.emit({ type: "providers_status", line: contract.warning }); + this.bus.emit({ type: "runtime_info", line: contract.warning }); + } this.bus.emit({ type: "runtime_info", line: `Active text provider: ${built.entry.id} (${built.entry.defaultChatModel ?? "default model"}). Chat uses cloud native tools now.`, @@ -432,6 +464,90 @@ export class ProvidersOrchestrator { } } + /** + * Run the contract probe against a provider that is already saved + * (`null` = the active text provider), on explicit request — the + * `/llm check` command. + * + * This is the second of exactly two ways the probe ever runs: here, + * and once per wizard save. It is never on a turn path. A route can + * degrade after setup (a gateway drops a backend, an account runs + * dry), and until now the only way to find out was to send a real + * message and watch it fail. + * + * The provider is described to the probe through a `configure` wizard + * state — the same object the panel builds when the operator presses + * `c` on that row — so the endpoint, key and model resolution is + * literally the wizard's, not a second implementation that could + * drift from it. + */ + async runContractProbe(providerId: string | null): Promise { + if (this.contractProbeRunning) { + this.bus.emit({ + type: "providers_status", + line: "A provider check is already running.", + }); + return; + } + const config = getConfig(); + const resolved = resolveLlmConfig(config); + const id = providerId ?? resolved.activeTextProvider; + const provider = id ? resolved.providers.find((p) => p.id === id) : undefined; + if (!id || !provider) { + this.bus.emit({ + type: "providers_status", + line: "No provider to check — configure one first.", + }); + return; + } + const fileEntry = config.llm?.providers.find((e) => e.id === id); + const kind = configureWizardKindForRow({ + kind: provider.kind, + ...(fileEntry?.subscriptionCli + ? { subscriptionCli: { cli: fileEntry.subscriptionCli.cli } } + : {}), + }); + const chatModel = fileEntry?.defaultChatModel ?? fileEntry?.model ?? null; + const wizard = kind + ? { + ...createProvidersWizardState("configure", { + providerId: id, + kind, + ...(fileEntry?.baseUrl ? { baseUrl: fileEntry.baseUrl } : {}), + ...(chatModel ? { chatModel } : {}), + }), + // The factory prefills `chatModelLine` only for CLI-backed + // kinds — a cloud reconfigure re-picks its model on the model + // screen. Nothing is being re-picked here, so the saved model + // is pinned directly; without it the probe would silently + // test the kind's default instead of the model this provider + // actually runs, and report a verdict about the wrong route. + ...(chatModel ? { selectedChatModelId: chatModel } : {}), + } + : null; + + this.contractProbeRunning = true; + this.bus.emit({ type: "providers_busy", busy: true }); + try { + // Every path here says what actually happened, including the ones + // where no request went out. Reporting a skip as a pass would be + // the silent "fully compatible" this whole check exists to stop. + const line = wizard + ? (await probeWizardContract(wizard)).summary + : `"${id}" has no provider kind that can be contract-checked.`; + this.bus.emit({ type: "providers_status", line }); + this.bus.emit({ type: "runtime_info", line }); + } catch (err) { + this.bus.emit({ + type: "providers_status", + line: err instanceof Error ? err.message : String(err), + }); + } finally { + this.contractProbeRunning = false; + this.bus.emit({ type: "providers_busy", busy: false }); + } + } + async removeProviderById(id: string): Promise { this.bus.emit({ type: "providers_busy", busy: true }); try { diff --git a/src/tui/providers/providers-wizard-target.ts b/src/tui/providers/providers-wizard-target.ts index f1b906e4..30c6f048 100644 --- a/src/tui/providers/providers-wizard-target.ts +++ b/src/tui/providers/providers-wizard-target.ts @@ -212,7 +212,7 @@ export function providerLabelForWizard(wizard: ProvidersWizardState): string { } /** The model this wizard run is about to save, before any defaulting. */ -function chosenModelForWizard(wizard: ProvidersWizardState): string { +export function chosenModelForWizard(wizard: ProvidersWizardState): string { const typed = wizard.chatModelLine.trim(); if (wizard.selectedChatModelId) return wizard.selectedChatModelId; if (typed.length > 0) return typed; @@ -222,7 +222,7 @@ function chosenModelForWizard(wizard: ProvidersWizardState): string { return OPENAI_COMPAT_DEFAULT_CHAT_MODEL; } -function endpointForKind( +export function endpointForKind( kind: ProvidersWizardKind, wizard: ProvidersWizardState, ): { baseUrl: string; apiPathPrefix: string; extraHeaders?: Record } { diff --git a/src/tui/submit-handler.ts b/src/tui/submit-handler.ts index e73eca52..64dcbbd7 100644 --- a/src/tui/submit-handler.ts +++ b/src/tui/submit-handler.ts @@ -201,6 +201,13 @@ export function runSlashCommand( callbacks.onProvidersChatModelPickerRequested?.(action.providerId); continue; } + if (action.type === "providers_contract_probe_requested") { + // Same wiring rule again for `/llm check`: the probe lives on + // `ProvidersOrchestrator.runContractProbe`, which only the + // callback layer can reach. + callbacks.onProvidersContractProbeRequested?.(action.providerId); + continue; + } if (action.type === "providers_inline_models_ensure_requested") { // Same wiring rule for the inline Cloud-pane model list (`/model`): // the catalog ensure must reach diff --git a/src/tui/tui-app.tsx b/src/tui/tui-app.tsx index b38cbbb4..739d090c 100644 --- a/src/tui/tui-app.tsx +++ b/src/tui/tui-app.tsx @@ -436,6 +436,13 @@ export interface TuiAppCallbacks { * above: only the callback layer reaches the orchestrator's bus. */ onProvidersInlineModelsEnsureRequested?(providerId: string | null): void; + /** + * `/llm check`: run the provider contract probe against `providerId` + * (`null` = active text provider). Callback for the same reason as + * the two above. Explicit request only — the probe spends real + * requests and never runs on a turn path. + */ + onProvidersContractProbeRequested?(providerId: string | null): void; /** Providers tab / LLM panel: switch the active embedding provider. */ onProvidersSetActiveEmbedding?(id: string): void; /** Providers tab / LLM panel: select an exact embedding model. */ diff --git a/src/tui/tui-command.ts b/src/tui/tui-command.ts index 7d6b5e08..3b0f91bf 100644 --- a/src/tui/tui-command.ts +++ b/src/tui/tui-command.ts @@ -540,6 +540,8 @@ export async function tuiCommand(args: string[]): Promise { void orchestrator.providers.openChatModelPicker(providerId), onProvidersInlineModelsEnsureRequested: (providerId) => void orchestrator.providers.ensureInlineModels(providerId), + onProvidersContractProbeRequested: (providerId) => + void orchestrator.providers.runContractProbe(providerId), onProvidersSetActiveEmbedding: (id) => void orchestrator.providers.setActiveEmbedding(id), onProvidersSelectEmbeddingModel: (providerId, modelId) => From b070c39310fc8b4754eb8372d7e6ed18936cadfd Mon Sep 17 00:00:00 2001 From: Valera Brizhatiuk <19537764+plombeer31@users.noreply.github.com> Date: Wed, 2 Sep 2026 14:24:48 +0300 Subject: [PATCH 2/3] fix(providers): stop the probe reporting our own limits as route defects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the contract probe found several verdicts that describe something Atomic did, not something the route did. Each one reaches an operator as a sentence about their provider, so each is a bug. * **Our deadline was reported as `stream_early_eof`.** The timeout flag was discarded the moment any byte arrived, so a queued OpenRouter request (it sends `: OPENROUTER PROCESSING` while it waits) or a slow first token was classified as "closed the stream before finishing the answer. Turns will end mid-tool-call on this route" — and blocked from `reportModelConfigured`. The flag now survives: only a stream that announced its own end before the timer fired is classified at all, anything else is `timeout`, which is what it always was. * **An abort mid-stream was reported the same way.** Once headers had arrived the reader's rejection was swallowed and the partial body classified, so cancelling produced an invented route defect. The read now races the caller's signal, which also means Esc gets the screen back at once instead of at the deadline. * **Forced-tool-choice verdicts warned about a request Atomic never makes.** `step-executor` sends `tool_choice: "auto"` on every turn, deliberately, with production-observed reasons written down beside it. A route that refuses a forced choice and streams a complete call under `auto` therefore runs every real turn — it is now `proven`, not a warning that suppresses "this install has a working backend". A route that *accepts* forcing and ignores it no longer stops the ladder either: rung 2 asks the way a turn asks, and only if that also produces nothing is `forced_tool_choice_ignored` reported. * **The probe sent no `max_tokens`, which every turn sends.** `buildOpenAiChatBody` sets it unconditionally and has no `max_completion_tokens` fallback, so a model that rejects the field fails every turn — exactly the class of failure this probe exists to catch, and one it passed. The cap is now in the body, generous enough (1024) that it cannot truncate a one-field tool call, a `retry_token_field` refusal is reported as the new `token_cap_rejected` instead of being retried in a shape Atomic never uses, and `finish_reason: "length"` is read as inconclusive so our own cap can never be reported as a route defect either. Smaller corrections from the same review: * `parallel_tool_calls` is no longer hardcoded: the wizard resolves what a turn would send (`agent.maxParallelToolCalls > 1`). * Keyless-listing presets (Nous, Novita, Ollama Cloud, SambaNova, Sarvam) are skipped rather than probed with an empty credential and told their absent key "was rejected" — that flag is about listing models, not about completions. * `contractProbeFoundDefect` is gone. It had no caller and its doc described a warning policy the shipped code contradicts. * One timeout constant instead of two: the 20s default no caller passed described a budget that never existed. * The unreachable `MAX_PROBE_REQUESTS` guard is gone; the ladder is three fixed rungs and the deadline is the real bound. * `/llm check` is now in the `/llm` menu description, and prints the redacted provider detail when the route did not pass — until now that string was computed, scrubbed and never shown to anyone. --- .../verify/classify-contract-probe.test.ts | 16 +- .../verify/classify-contract-probe.ts | 42 +++- .../provider/verify/contract-probe-types.ts | 67 ++--- src/llm/provider/verify/index.ts | 1 - .../verify/run-contract-probe.test.ts | 180 +++++++++++++- src/llm/provider/verify/run-contract-probe.ts | 230 +++++++++++++----- src/tui/menu/menu-registry.test.ts | 8 +- src/tui/menu/menu-registry.ts | 2 +- src/tui/providers/contract-probe-target.ts | 28 ++- src/tui/providers/describe-contract-probe.ts | 10 +- .../providers/probe-wizard-contract.test.ts | 45 ++++ src/tui/providers/probe-wizard-contract.ts | 30 +-- src/tui/providers/providers-orchestrator.ts | 20 +- 13 files changed, 539 insertions(+), 140 deletions(-) diff --git a/src/llm/provider/verify/classify-contract-probe.test.ts b/src/llm/provider/verify/classify-contract-probe.test.ts index d2f1a9a6..8a0e0129 100644 --- a/src/llm/provider/verify/classify-contract-probe.test.ts +++ b/src/llm/provider/verify/classify-contract-probe.test.ts @@ -8,7 +8,6 @@ import { } from "./classify-contract-probe.js"; import { CONTRACT_PROBE_TOOL_NAME, - contractProbeFoundDefect, contractProbeProvesToolSupport, contractProbeToolDefinition, } from "./contract-probe-types.js"; @@ -216,17 +215,26 @@ describe("the synthetic probe tool", () => { expect(contractProbeProvesToolSupport("tools_supported")).toBe(true); for (const status of [ "inconclusive_no_tool_call", + "forced_tool_choice_ignored", "stream_early_eof", "malformed_tool_call", "tools_payload_rejected", + "token_cap_rejected", "provider_error", ] as const) { expect(contractProbeProvesToolSupport(status)).toBe(false); } }); - it("does not count an inconclusive auto answer as a defect", () => { - expect(contractProbeFoundDefect("inconclusive_no_tool_call")).toBe(false); - expect(contractProbeFoundDefect("stream_early_eof")).toBe(true); + it("counts a route that only refuses the forcing as proven", () => { + // Atomic never sends a forced tool choice (`step-executor` sends + // `auto` on every turn), so a route that refuses one and streams a + // complete call without it runs every real turn. Marking it + // unproven would warn operators about a bug they cannot hit and + // withhold "this install has a working backend" from an install + // that has one. + expect(contractProbeProvesToolSupport("forced_tool_choice_rejected")).toBe( + true, + ); }); }); diff --git a/src/llm/provider/verify/classify-contract-probe.ts b/src/llm/provider/verify/classify-contract-probe.ts index 86060ee6..4b524039 100644 --- a/src/llm/provider/verify/classify-contract-probe.ts +++ b/src/llm/provider/verify/classify-contract-probe.ts @@ -40,11 +40,13 @@ export function classifyContractProbeHttpFailure( ): ProviderContractStatus { const verdict = classifyVerifyResponse(httpStatus, body); if (verdict.kind === "retry_next_model") return "model_unavailable"; - // The key check's "resend with the other max-tokens field" hint is - // meaningless here: the probe deliberately sends no token cap (see - // `run-contract-probe`), so a body naming those fields is the route - // complaining about something else it read in our request. - if (verdict.kind === "retry_token_field") return "provider_error"; + // The key check answers this hint by resending with the other field. + // The probe must not: it sends the same `max_tokens` a turn sends + // (see `run-contract-probe`), and `buildOpenAiChatBody` has no + // `max_completion_tokens` fallback to switch to. Retrying would prove + // a route works in a shape Atomic never uses and report a pass for a + // route whose every turn 400s. So the refusal is the verdict. + if (verdict.kind === "retry_token_field") return "token_cap_rejected"; switch (verdict.status) { case "invalid_key": return "endpoint_auth_failed"; @@ -73,7 +75,10 @@ export function contractProbeFailureIsTerminal( return ( status === "endpoint_auth_failed" || status === "quota_or_routing_failed" || - status === "model_unavailable" + status === "model_unavailable" || + // Every rung carries the same token cap, so the next one would be + // refused for the same reason — and it is a real finding already. + status === "token_cap_rejected" ); } @@ -92,6 +97,14 @@ export function classifyProbeStream( ): ProviderContractStatus { if (!observation.terminalObserved) return "stream_early_eof"; + // The probe's own `max_tokens` ran out. A verbose or thinking model + // can spend it honestly before it gets to a tool call, or in the + // middle of one, so everything below this line would be blaming the + // route for a limit we set. The one thing still worth reading is a + // call that arrived *complete* despite the cut — that is proof, and + // it is checked below by the same rules as any other. + const truncatedByOurCap = observation.finishReason === "length"; + if (observation.sawToolCallDelta) { const call = observation.toolCalls.find((c) => c.name === CONTRACT_PROBE_TOOL_NAME) ?? @@ -99,14 +112,19 @@ export function classifyProbeStream( // Deltas arrived and still produced nothing callable. On the real // path this is the failure that surfaces as `tool not registered in // this agent`, several minutes into a turn. - if (!call || call.name.length === 0) return "malformed_tool_call"; - // Only one function was offered, so any other name is the route - // inventing one — the call could never be dispatched. - if (call.name !== CONTRACT_PROBE_TOOL_NAME) return "malformed_tool_call"; - if (!argumentsAreDispatchable(call.arguments)) return "malformed_tool_call"; - return "tools_supported"; + const dispatchable = + call !== undefined && + call.name.length > 0 && + // Only one function was offered, so any other name is the route + // inventing one — the call could never be dispatched. + call.name === CONTRACT_PROBE_TOOL_NAME && + argumentsAreDispatchable(call.arguments); + if (dispatchable) return "tools_supported"; + return truncatedByOurCap ? "inconclusive_no_tool_call" : "malformed_tool_call"; } + if (truncatedByOurCap) return "inconclusive_no_tool_call"; + // No tool call at all. What that means depends entirely on what we // asked for, and conflating the two cases is the specific mistake // this probe is built to avoid. diff --git a/src/llm/provider/verify/contract-probe-types.ts b/src/llm/provider/verify/contract-probe-types.ts index b021513a..3d55b8fa 100644 --- a/src/llm/provider/verify/contract-probe-types.ts +++ b/src/llm/provider/verify/contract-probe-types.ts @@ -38,14 +38,18 @@ export type ProviderContractStatus = /** One complete native tool call came back over the stream. */ | "tools_supported" /** - * The route took the tools payload but chose to answer in prose under - * `tool_choice: auto`. That is a legal answer for a model, so it says - * nothing about whether the route can emit tool calls at all. + * The route took the tools payload but produced no callable tool for + * a reason that says nothing about the route: it answered in prose + * under `tool_choice: auto` (legal for a model), or the probe's own + * `max_tokens` cut the answer short. */ | "inconclusive_no_tool_call" /** * The route accepted a forced named tool choice and then answered - * text anyway — it advertises the parameter without honoring it. + * text anyway — it advertises the parameter without honoring it — and + * called nothing under `auto` either. The second half matters: `auto` + * is the only mode a turn uses, so a route that ignores forcing but + * calls tools without it is reported as working, not as this. */ | "forced_tool_choice_ignored" /** @@ -58,6 +62,13 @@ export type ProviderContractStatus = * the same streamed completion once `tools` was removed. */ | "tools_payload_rejected" + /** + * The route refused the `max_tokens` cap every Atomic turn carries + * (newer OpenAI models want `max_completion_tokens` instead). Nothing + * was learned about tools, and nothing needed to be: the turn path + * has no second field to try, so it would fail the same way. + */ + | "token_cap_rejected" /** The configured model is unknown to this route. */ | "model_unavailable" /** The endpoint refused the credential; nothing else was learned. */ @@ -97,6 +108,14 @@ export interface ProviderContractProbeTarget { */ readonly model: string; readonly extraHeaders?: Record; + /** + * What a turn would put in `parallel_tool_calls` for this provider — + * the executor's cap and the provider's declared capability, resolved + * by the caller because neither is visible from here. Defaults to + * `true`, which is what `buildOpenAiChatBody` sends for a provider + * that declares nothing. + */ + readonly parallelToolCalls?: boolean; } export interface ProviderContractProbeResult { @@ -117,34 +136,26 @@ export interface ProviderContractProbeResult { } /** - * The one verdict that means "this route can run a turn". Everything - * else is either a failure or an open question, and neither may be - * reported as proven compatibility. + * The verdicts that mean "this route can run a turn". Everything else + * is either a failure or an open question, and neither may be reported + * as proven compatibility. + * + * Two of them qualify, because a turn is a narrower thing than the + * probe's primary instrument. `step-executor` sends + * `tool_choice: "auto"` on every request and never a forced or named + * choice — deliberately, with production-observed reasons written down + * beside it (Alibaba's Qwen-thinking gate answers `400 InvalidParameter` + * to a forced choice at all). A route that refuses the forcing and then + * streams a complete native tool call under `auto` therefore runs every + * real Atomic turn correctly, and calling it unproven would warn + * operators who are not hitting any bug. The forced rung stays first + * because it is the only mode in which "no tool call" is a statement + * about the route rather than about the model's mood. */ export function contractProbeProvesToolSupport( status: ProviderContractStatus, ): boolean { - return status === "tools_supported"; -} - -/** - * `true` when the probe learned something about the *route* rather than - * about the model's whim. Used to decide whether a warning is worth - * showing: an inconclusive auto-mode answer is not a defect to report. - */ -export function contractProbeFoundDefect( - status: ProviderContractStatus, -): boolean { - return ( - status === "forced_tool_choice_ignored" || - status === "forced_tool_choice_rejected" || - status === "tools_payload_rejected" || - status === "model_unavailable" || - status === "endpoint_auth_failed" || - status === "quota_or_routing_failed" || - status === "stream_early_eof" || - status === "malformed_tool_call" - ); + return status === "tools_supported" || status === "forced_tool_choice_rejected"; } /** diff --git a/src/llm/provider/verify/index.ts b/src/llm/provider/verify/index.ts index 87a43b75..560e7a45 100644 --- a/src/llm/provider/verify/index.ts +++ b/src/llm/provider/verify/index.ts @@ -15,7 +15,6 @@ export { } from "./classify-verify-response.js"; export { CONTRACT_PROBE_TOOL_NAME, - contractProbeFoundDefect, contractProbeProvesToolSupport, contractProbeToolDefinition, type ProbeToolChoiceMode, diff --git a/src/llm/provider/verify/run-contract-probe.test.ts b/src/llm/provider/verify/run-contract-probe.test.ts index 0d39d62f..1b7879e6 100644 --- a/src/llm/provider/verify/run-contract-probe.test.ts +++ b/src/llm/provider/verify/run-contract-probe.test.ts @@ -86,6 +86,38 @@ const MALFORMED_STREAM = ], }) + sseEvent({ choices: [{ delta: {}, finish_reason: "tool_calls" }] }); +/** + * A response whose body opens, says something, and then never ends — + * a queued OpenRouter request (`: OPENROUTER PROCESSING`) or a model + * still thinking about its first token. `stalled` resolves once the + * first chunk has been handed over, so a test can act mid-stream. + */ +function stallingStreamResponse(preamble: string): { + response: () => Response; + firstChunkRead: Promise; +} { + let seen = () => {}; + const firstChunkRead = new Promise((resolve) => { + seen = resolve; + }); + const body = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(preamble)); + // Never closed and never enqueued again: the socket is open and + // the route is thinking. + setTimeout(seen, 0); + }, + }); + return { + response: () => + new Response(body, { + status: 200, + headers: { "content-type": "text/event-stream" }, + }), + firstChunkRead, + }; +} + function streamResponse(body: string): Response { return new Response(body, { status: 200, @@ -276,7 +308,11 @@ describe("runProviderContractProbe", () => { }); it("keeps credentials and whole response bodies out of the detail", async () => { - const leak = `${"x".repeat(400)} key=${TARGET.apiKey} other=sk-someoneelseskey123`; + // Both keys sit inside the first 300 characters, so the length cap + // cannot be what hides them: only redaction can. (The rules + // themselves are pinned in `redact-provider-detail.test.ts`.) + const leak = + `key=${TARGET.apiKey} other=sk-someoneelseskey123 ` + "x".repeat(400); const script = scriptedFetch([() => errorResponse(401, leak)]); const result = await runProviderContractProbe(TARGET, { fetchImpl: script.fetchImpl, @@ -284,6 +320,7 @@ describe("runProviderContractProbe", () => { expect(result.detail).not.toContain(TARGET.apiKey); expect(result.detail).not.toContain("sk-someoneelseskey123"); + expect(result.detail).toContain("key=***"); expect(result.detail.length).toBeLessThanOrEqual(300); }); @@ -310,4 +347,145 @@ describe("runProviderContractProbe", () => { expect(result.status).toBe("cancelled"); expect(script.calls()).toBe(0); }); + it("sends the token cap a real turn sends", async () => { + const script = scriptedFetch([() => streamResponse(TOOL_CALL_STREAM)]); + await runProviderContractProbe(TARGET, { fetchImpl: script.fetchImpl }); + + // `buildOpenAiChatBody` puts `max_tokens` on every turn and has no + // second field to fall back on, so a probe that left it out could + // pass on a route where every real message 400s. + expect(script.bodies()[0]!.max_tokens).toBeTypeOf("number"); + expect(script.bodies()[0]!.parallel_tool_calls).toBe(true); + }); + + it("sends the parallel_tool_calls the caller says a turn would send", async () => { + const script = scriptedFetch([() => streamResponse(TOOL_CALL_STREAM)]); + await runProviderContractProbe( + { ...TARGET, parallelToolCalls: false }, + { fetchImpl: script.fetchImpl }, + ); + + expect(script.bodies()[0]!.parallel_tool_calls).toBe(false); + }); + + it("reports a rejected token cap instead of retrying with the other field", async () => { + const script = scriptedFetch([ + () => + errorResponse(400, { + error: { + message: + "Unsupported parameter: 'max_tokens' is not supported with this model. Use 'max_completion_tokens' instead.", + }, + }), + ]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + }); + + // The turn path has no `max_completion_tokens` fallback, so this is + // the verdict, not a request to resend: every real turn would be + // refused the same way. + expect(result.status).toBe("token_cap_rejected"); + expect(script.calls()).toBe(1); + }); + + it("settles a route that ignores forcing by asking the way a turn asks", async () => { + const script = scriptedFetch([ + () => streamResponse(TEXT_STREAM), + () => streamResponse(TOOL_CALL_STREAM), + ]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + }); + + // Rung 1 was accepted and ignored, which says nothing about the + // mode Atomic runs in. Rung 2 asks in that mode and gets a complete + // tool call: the route works, and warning about the forced-choice + // quirk would be warning about a request Atomic never makes. + expect(result.status).toBe("tools_supported"); + expect(result.toolChoiceMode).toBe("auto"); + expect(script.calls()).toBe(2); + expect(script.bodies()[1]!.tool_choice).toBe("auto"); + }); + + it("reports an ignored forcing when auto declines as well", async () => { + const script = scriptedFetch([ + () => streamResponse(TEXT_STREAM), + () => streamResponse(TEXT_STREAM), + ]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + }); + + expect(result.status).toBe("forced_tool_choice_ignored"); + expect(script.calls()).toBe(2); + }); + + it("does not blame tools when rung 1 streamed and rung 2 failed", async () => { + const script = scriptedFetch([ + () => streamResponse(TEXT_STREAM), + () => errorResponse(500, "upstream exploded"), + () => streamResponse(TEXT_STREAM), + ]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + }); + + // The first rung carried `tools` and streamed, so the no-tools + // control could not attribute anything to them: spending a third + // request could only produce a wrong sentence. + expect(result.status).toBe("provider_error"); + expect(script.calls()).toBe(2); + }); + + it("calls a cap-truncated answer inconclusive, not a route defect", async () => { + const truncated = + sseEvent({ choices: [{ delta: { content: "Let me think about" } }] }) + + sseEvent({ choices: [{ delta: {}, finish_reason: "length" }] }) + + "data: [DONE]\n\n"; + const script = scriptedFetch([() => streamResponse(truncated)]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + }); + + // Our own `max_tokens` ended that answer. Reporting it as "ignored + // a forced tool choice" would blame the route for our limit. + expect(result.status).toBe("inconclusive_no_tool_call"); + }); + + it("calls its own deadline a timeout, even once bytes have arrived", async () => { + // OpenRouter's real queue keepalive, then silence. Under the old + // rule ("timed out with zero bytes") this comment alone turned a + // slow route into `stream_early_eof` — "turns will end + // mid-tool-call" — a defect invented by our own budget. + const stalling = stallingStreamResponse(": OPENROUTER PROCESSING\n\n"); + const script = scriptedFetch([stalling.response]); + const result = await runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + timeoutMs: 250, + }); + + expect(result.status).toBe("timeout"); + expect(script.calls()).toBe(1); + }); + + it("reports an abort that lands mid-stream as cancelled", async () => { + const stalling = stallingStreamResponse( + sseEvent({ choices: [{ delta: { content: "thinking" } }] }), + ); + const script = scriptedFetch([stalling.response]); + const controller = new AbortController(); + const running = runProviderContractProbe(TARGET, { + fetchImpl: script.fetchImpl, + signal: controller.signal, + timeoutMs: 5_000, + }); + await stalling.firstChunkRead; + controller.abort(); + + // The stream is open and half-read: without the signal in the read + // race this is a partial body, which classifies as + // `stream_early_eof` — a route defect we caused by giving up. + await expect(running).resolves.toMatchObject({ status: "cancelled" }); + }); }); diff --git a/src/llm/provider/verify/run-contract-probe.ts b/src/llm/provider/verify/run-contract-probe.ts index f9983b9f..8420962c 100644 --- a/src/llm/provider/verify/run-contract-probe.ts +++ b/src/llm/provider/verify/run-contract-probe.ts @@ -18,14 +18,21 @@ * * 1. **forced named tool**, streaming, tools payload. A complete tool * call here is the whole answer: `tools_supported`. - * 2. **`tool_choice: auto`**, same tools payload — reached only when - * rung 1 was *refused* for a reason that is not the key, the quota - * or the model. A tool call now means the payload is fine and it - * was the forcing the route would not take. + * 2. **`tool_choice: auto`**, same tools payload — reached when rung 1 + * was *refused* for a reason that is not the key, the quota or the + * model, or when it was *accepted and ignored*. This is the mode a + * real turn runs in (`step-executor` sends `auto` on every request, + * deliberately — several providers reject a forced choice outright), + * so a tool call here is proof about the shape Atomic actually uses: + * after a refusal it means only the forcing was unacceptable, and + * after an ignored forcing it means the route emits tool calls + * regardless. * 3. **no tools at all**, same model, same streaming transport — - * reached only when both tool requests were refused. If this + * reached only when both tool *requests* were refused. If this * answers, the route works and it is specifically `tools` it - * rejects; if it fails too, the failure was never about tools. + * rejects; if it fails too, the failure was never about tools. It is + * skipped when rung 1 streamed, because a stream already proved the + * route takes `tools` and the control could only mislead. * * Rung 3 is what turns "HTTP 400" into a sentence an operator can act * on, and it is deliberately an experiment rather than a regex over the @@ -64,17 +71,36 @@ import { redactProviderDetail } from "./redact-provider-detail.js"; /** * Whole-probe budget, not per request. Longer than the key check's 8s * because this one waits for a model to actually generate, but still - * short enough that a wizard screen does not feel hung: a route that - * cannot produce a two-field tool call inside this is a finding in - * itself. + * short enough that a wizard screen does not feel hung — every caller + * runs it with an operator watching, which is why this is the only + * budget in the module: a second, laxer default nobody passes would + * only describe a timeout that never happens. + * + * Blowing it is *our* verdict, not the route's: see `runRung`, which + * reports `timeout` rather than inventing a stream defect out of a slow + * route. */ -export const PROVIDER_CONTRACT_PROBE_TIMEOUT_MS = 20_000; +export const PROVIDER_CONTRACT_PROBE_TIMEOUT_MS = 12_000; /** Ceiling on the SSE body we buffer. A probe answer is a few hundred bytes. */ const MAX_STREAM_BYTES = 64 * 1024; -/** The whole ladder: forced → auto → no-tools. Never more than that. */ -const MAX_PROBE_REQUESTS = 3; +/** + * The cap a real turn always carries. `buildOpenAiChatBody` sets + * `max_tokens` on every request unconditionally and has no + * `max_completion_tokens` fallback, so a route that refuses the field + * refuses every Atomic turn — exactly the class of failure this probe + * exists to catch, and one it would miss by leaving the cap out. + * + * The value only has to be far above what one call to a single-field + * function costs; what a route validates is the field, not the number. + * Generous on purpose: a thinking model can spend hundreds of tokens + * before it calls anything, and a cap that truncated the answer would + * report our own doing as a route defect. `classifyProbeStream` guards + * the remainder of that risk by reading `finish_reason: "length"` as + * inconclusive. + */ +const PROBE_MAX_TOKENS = 1024; const PROBE_PROMPT = `Call the ${CONTRACT_PROBE_TOOL_NAME} function with ok set to true. ` + @@ -122,28 +148,47 @@ export async function runProviderContractProbe( }); // Rung 1 — the real contract, forced. - const forced = await run(probeBody(model, "required_named")); + const forced = await run(probeBody(target, model, "required_named")); if (forced.kind === "aborted") { return emit(forced.status, null, "required_named", forced.detail); } + // What rung 1 left open, in the two shapes rung 2 has to tell apart. + // Exactly one of these is set by the time rung 2 runs; anything else + // rung 1 saw was already the verdict and returned above. + let forcedRefusal: { httpStatus: number; body: string } | null = null; + let forcedIgnored: { httpStatus: number; detail: string } | null = null; if (forced.kind === "stream") { - return emit( - classifyProbeStream(forced.observation, "required_named"), + const forcedStatus = classifyProbeStream(forced.observation, "required_named"); + if (forcedStatus !== "forced_tool_choice_ignored") { + return emit( + forcedStatus, + forced.httpStatus, + "required_named", + streamDetail(forced.observation), + ); + } + // The route took `tool_choice` and then answered prose anyway. That + // is a real observation about the route, but it is not yet an + // answer about Atomic: turns never force a tool (`step-executor` + // sends `auto`), so what still has to be settled is whether this + // route emits tool calls in the mode it will actually be run in. + forcedIgnored = { + httpStatus: forced.httpStatus, + detail: streamDetail(forced.observation), + }; + } else { + const forcedStatus = classifyContractProbeHttpFailure( forced.httpStatus, - "required_named", - streamDetail(forced.observation), + forced.body, ); - } - const forcedStatus = classifyContractProbeHttpFailure( - forced.httpStatus, - forced.body, - ); - if (contractProbeFailureIsTerminal(forcedStatus)) { - return emit(forcedStatus, forced.httpStatus, "required_named", forced.body); + if (contractProbeFailureIsTerminal(forcedStatus)) { + return emit(forcedStatus, forced.httpStatus, "required_named", forced.body); + } + forcedRefusal = { httpStatus: forced.httpStatus, body: forced.body }; } - // Rung 2 — same payload, no forcing. - const auto = await run(probeBody(model, "auto")); + // Rung 2 — same payload, no forcing. This is the request a turn makes. + const auto = await run(probeBody(target, model, "auto")); if (auto.kind === "aborted") { return emit(auto.status, null, "auto", auto.detail); } @@ -151,13 +196,27 @@ export async function runProviderContractProbe( const autoStatus = classifyProbeStream(auto.observation, "auto"); // Tools work; it was the forced choice rung 1 asked for that this // route would not take. Reporting rung 2's own verdict here would - // hide the actual limitation behind a cheerful "supported". - if (autoStatus === "tools_supported") { + // hide the limitation, so it keeps its own status — one that says + // "usable", because `auto` is all a turn ever sends. + if (autoStatus === "tools_supported" && forcedRefusal) { return emit( "forced_tool_choice_rejected", - forced.httpStatus, + forcedRefusal.httpStatus, + "required_named", + forcedRefusal.body, + ); + } + // Forcing ignored *and* nothing called under `auto`: two requests + // and not one tool call. Still not proof that the route cannot emit + // them — a model may simply keep declining a pointless function — + // but "it ignores a forced choice" is the sharper of the two + // observations, so that is the one reported. + if (forcedIgnored && autoStatus === "inconclusive_no_tool_call") { + return emit( + "forced_tool_choice_ignored", + forcedIgnored.httpStatus, "required_named", - forced.body, + forcedIgnored.detail, ); } return emit(autoStatus, auto.httpStatus, "auto", streamDetail(auto.observation)); @@ -166,9 +225,15 @@ export async function runProviderContractProbe( if (contractProbeFailureIsTerminal(autoStatus)) { return emit(autoStatus, auto.httpStatus, "auto", auto.body); } + if (forcedIgnored) { + // Rung 1 streamed, so this route demonstrably accepts `tools`; the + // no-tools control could not attribute rung 2's refusal to them and + // would only spend a request to reach a wrong sentence. + return emit(autoStatus, auto.httpStatus, "auto", auto.body); + } // Rung 3 — the control. Same model, same streaming transport, no tools. - const control = await run(probeBody(model, null)); + const control = await run(probeBody(target, model, null)); if (control.kind === "aborted") { return emit(control.status, null, null, control.detail); } @@ -215,13 +280,6 @@ async function runRung( if (ctx.signal?.aborted) { return { kind: "aborted", status: "cancelled", detail: "probe cancelled" }; } - if (ctx.state.requests >= MAX_PROBE_REQUESTS) { - return { - kind: "aborted", - status: "provider_error", - detail: "probe budget spent", - }; - } const remainingMs = ctx.deadline - Date.now(); if (remainingMs <= 0) { return { kind: "aborted", status: "timeout", detail: "probe deadline reached" }; @@ -275,38 +333,51 @@ async function runRung( // `openAiFetch`'s own timeout covers the connect only — it clears the // timer the moment headers arrive. A route that opens a stream and // then stalls forever would hang here, so the body read carries the - // remaining budget itself. - const sse = await readStreamBounded(res, ctx.deadline - Date.now()); - if (sse.timedOut && sse.text.length === 0) { + // remaining budget itself, and the operator's abort as well. + const sse = await readStreamBounded( + res, + ctx.deadline - Date.now(), + ctx.signal, + ); + if (sse.aborted || ctx.signal?.aborted) { + return { kind: "aborted", status: "cancelled", detail: "probe cancelled" }; + } + const observation = accumulateProbeStream(sse.text); + // Our own deadline is not a route defect. A single byte is enough to + // put text in the buffer — OpenRouter sends `: OPENROUTER PROCESSING` + // while a request is queued, and a reasoning model can take seconds + // over its first token — so keying this off "no bytes arrived" would + // report every slow route as `stream_early_eof` ("turns will end + // mid-tool-call"), a defect we invented by giving up first. Only a + // stream that announced its own end before the timer fired is a + // complete observation worth classifying. + if (sse.timedOut && !observation.terminalObserved) { return { kind: "aborted", status: "timeout", - detail: "no stream data before deadline", + detail: `no complete stream before deadline (${sse.text.length} bytes read)`, }; } - return { - kind: "stream", - httpStatus: res.status, - observation: accumulateProbeStream(sse.text), - }; + return { kind: "stream", httpStatus: res.status, observation }; } /** * The probe request, in the same shape `buildOpenAiChatBody` gives a - * real turn — including `parallel_tool_calls`, which some routes - * validate and which a turn always sends alongside tools. + * real turn — the streamed transport, the tools payload, + * `parallel_tool_calls`, and the `max_tokens` cap a turn always carries. + * Sending anything less would let a route pass the probe and then fail + * the first message on a field the probe never showed it. * * `mode === null` is the control: no tools, no tool choice, otherwise * identical, so a difference in outcome can only be the tools payload. * - * No `max_tokens`. The one-token key check has to cap spend and - * therefore has to guess between `max_tokens` and - * `max_completion_tokens` (newer OpenAI models reject the former). - * Here a cap would risk truncating the very tool call being measured, - * reporting malformed deltas that were in fact our own doing — and one - * forced call to a single-field function is cheap enough uncapped. + * The one field a turn sends that this cannot is `parallel_tool_calls`' + * *value* — a turn computes it from the provider's declared capability + * and the executor's cap. The target carries it when the caller knows + * it; `true` is what a wizard-saved cloud provider gets by default. */ function probeBody( + target: ProviderContractProbeTarget, model: string, mode: ProbeToolChoiceMode | null, ): Record { @@ -314,11 +385,12 @@ function probeBody( model, messages: [{ role: "user", content: PROBE_PROMPT }], temperature: 0, + max_tokens: PROBE_MAX_TOKENS, stream: true, }; if (mode === null) return body; body.tools = [contractProbeToolDefinition()]; - body.parallel_tool_calls = true; + body.parallel_tool_calls = target.parallelToolCalls ?? true; body.tool_choice = mode === "required_named" ? { type: "function", function: { name: CONTRACT_PROBE_TOOL_NAME } } @@ -345,19 +417,30 @@ function streamDetail(observation: ProbeStreamObservation): string { } /** - * Buffer the SSE body under a byte ceiling and a deadline, cancelling - * the stream rather than leaving a socket open behind us. + * Buffer the SSE body under a byte ceiling, a deadline and the caller's + * abort, cancelling the stream rather than leaving a socket open behind + * us. + * + * All three outcomes are reported separately because they mean + * different things about the route: a body that simply stopped is the + * early EOF the probe is hunting for, while a deadline or an abort is + * something *we* did and must never be dressed up as one. */ async function readStreamBounded( res: Response, budgetMs: number, -): Promise<{ text: string; timedOut: boolean }> { - if (!res.body) return { text: await res.text().catch(() => ""), timedOut: false }; + signal?: AbortSignal, +): Promise<{ text: string; timedOut: boolean; aborted: boolean }> { + if (!res.body) { + return { text: await res.text().catch(() => ""), timedOut: false, aborted: false }; + } const reader = res.body.getReader(); const decoder = new TextDecoder(); let text = ""; let timedOut = false; + let aborted = false; let timer: ReturnType | undefined; + let onAbort: (() => void) | undefined; try { const deadline = budgetMs > 0 @@ -365,8 +448,25 @@ async function readStreamBounded( timer = setTimeout(() => resolve("deadline"), budgetMs); }) : Promise.resolve<"deadline">("deadline"); + // A stalled stream does not reject on abort by itself on every + // transport, so the signal is raced rather than waited for: an + // operator pressing Esc must get the screen back now, not at the + // deadline. + const cancelled = new Promise<"aborted">((resolve) => { + if (!signal) return; + if (signal.aborted) { + resolve("aborted"); + return; + } + onAbort = () => resolve("aborted"); + signal.addEventListener("abort", onAbort, { once: true }); + }); for (;;) { - const next = await Promise.race([reader.read(), deadline]); + const next = await Promise.race([reader.read(), deadline, cancelled]); + if (next === "aborted") { + aborted = true; + break; + } if (next === "deadline") { timedOut = true; break; @@ -378,12 +478,16 @@ async function readStreamBounded( text += decoder.decode(next.value, { stream: true }); if (text.length >= MAX_STREAM_BYTES) break; } - } catch { + } catch (err) { // A stream that breaks mid-body is exactly the early-EOF case: keep // what arrived and let the classifier see that it never terminated. + // Unless it broke because the caller aborted the request, which is + // not a fact about the route at all. + if (signal?.aborted || isAbortError(err)) aborted = true; } finally { if (timer) clearTimeout(timer); + if (onAbort) signal?.removeEventListener("abort", onAbort); void reader.cancel().catch(() => {}); } - return { text, timedOut }; + return { text, timedOut, aborted }; } diff --git a/src/tui/menu/menu-registry.test.ts b/src/tui/menu/menu-registry.test.ts index 748e90d5..a05da31e 100644 --- a/src/tui/menu/menu-registry.test.ts +++ b/src/tui/menu/menu-registry.test.ts @@ -145,10 +145,12 @@ const V0_2_2_SLASH_COMMANDS = [ }, { name: "llm", - // Updated when the Fallback pane got its deep link: the palette must - // advertise all four panes, not the three that predate it. + // Updated when the Fallback pane got its deep link, and again for + // `/llm check`: every subcommand the handler answers has to be + // reachable from here, or it exists only for whoever types an + // invalid one and reads the usage line. description: - "open LLM Local/Cloud/External/Fallback panel · `/llm provider ` switch text provider · `/llm fallback` edit the fallover chain", + "open LLM Local/Cloud/External/Fallback panel · `/llm provider ` switch text provider · `/llm check` test the active route's streaming tool contract · `/llm fallback` edit the fallover chain", }, { name: "mcp", diff --git a/src/tui/menu/menu-registry.ts b/src/tui/menu/menu-registry.ts index 96040018..b63242ba 100644 --- a/src/tui/menu/menu-registry.ts +++ b/src/tui/menu/menu-registry.ts @@ -290,7 +290,7 @@ export const MENU: readonly MenuNode[] = [ slash: { name: "llm", description: - "open LLM Local/Cloud/External/Fallback panel · `/llm provider ` switch text provider · `/llm fallback` edit the fallover chain", + "open LLM Local/Cloud/External/Fallback panel · `/llm provider ` switch text provider · `/llm check` test the active route's streaming tool contract · `/llm fallback` edit the fallover chain", rank: 24, }, section: "manage", diff --git a/src/tui/providers/contract-probe-target.ts b/src/tui/providers/contract-probe-target.ts index cd926373..efbe9da6 100644 --- a/src/tui/providers/contract-probe-target.ts +++ b/src/tui/providers/contract-probe-target.ts @@ -20,6 +20,7 @@ * how a diagnostic tool loses their trust. */ +import { getConfig } from "../../config/index.js"; import type { ProviderContractProbeTarget } from "../../llm/provider/verify/index.js"; import { isLocalProviderUrl } from "./is-local-provider-url.js"; import { @@ -27,7 +28,6 @@ import { chosenModelForWizard, endpointForKind, providerLabelForWizard, - wizardKeyIsOptional, } from "./providers-wizard-target.js"; import type { ProvidersWizardState } from "./providers-wizard-state.js"; @@ -57,19 +57,24 @@ export function contractProbeTargetForWizard( // this contract, and inventing one would probe a URL it never uses. if (kind === "claude-cli" || kind === "codex-cli") return skip("cli_backed"); - const apiKey = apiKeyForWizard(wizard)?.trim() ?? ""; - // Keyless is legitimate for local servers and keyless-listing - // services; a missing key everywhere else is already refused by the - // key screen, and probing without one would only re-report that. - if (!apiKey && !wizardKeyIsOptional(wizard)) return skip("no_api_key"); - const endpoint = endpointForKind(kind, wizard); // A server on this machine is the operator's own: reachable, free to // call, and a probe against it says more about their llama-server // flags than about a provider. The key check skips it for the same - // reason. + // reason. Checked before the key, because a local server having none + // is the *reason* it has none. if (isLocalProviderUrl(endpoint.baseUrl)) return skip("local_endpoint"); + const apiKey = apiKeyForWizard(wizard)?.trim() ?? ""; + // No key, no probe — including for the presets `wizardKeyIsOptional` + // lets through. That flag means "this service lists its models + // without a key", which is true of Nous, Novita, Ollama Cloud, + // SambaNova and Sarvam and says nothing about completions: they all + // answer a keyless one with a 401. Probing anyway would spend a + // request to tell the operator their key "was rejected" when no key + // was ever sent. + if (!apiKey) return skip("no_api_key"); + const model = chosenModelForWizard(wizard).trim(); if (!model) return skip("no_model"); @@ -81,6 +86,13 @@ export function contractProbeTargetForWizard( apiPathPrefix: endpoint.apiPathPrefix, apiKey, model, + // What `buildOpenAiChatBody` would put in `parallel_tool_calls` + // for a provider saved from this wizard. The wizard has no screen + // for the per-provider `supportsTools` flag, so the executor's cap + // is the only half of the turn's expression that can differ here + // (`step-executor`: `maxParallelToolCalls > 1 && + // supportsParallelTools`). + parallelToolCalls: getConfig().agent.maxParallelToolCalls > 1, ...(endpoint.extraHeaders ? { extraHeaders: endpoint.extraHeaders } : {}), }, }; diff --git a/src/tui/providers/describe-contract-probe.ts b/src/tui/providers/describe-contract-probe.ts index c1df4bfb..b9642615 100644 --- a/src/tui/providers/describe-contract-probe.ts +++ b/src/tui/providers/describe-contract-probe.ts @@ -9,6 +9,10 @@ * `inconclusive_no_tool_call` is the model declining to call a * pointless function, which is legal behaviour, not a broken route. * - Never say "compatible" for anything but a completed tool call. + * - Never describe a limitation Atomic cannot hit. A turn always sends + * `tool_choice: "auto"` (see `step-executor`), so a route that + * refuses a *forced* choice and streams a call without one is + * working, and its sentence has to read that way. * - Always name the next move. `HTTP 400` on a setup screen reads as a * product failure; "answers fine until `tools` is in the request" * reads as a route to change. @@ -28,9 +32,11 @@ export function describeContractProbeOutcome( case "inconclusive_no_tool_call": return `${who} answered in text instead of calling a tool${on}. Inconclusive: it would not take a forced tool choice, so whether it can emit tool calls is still unknown.`; case "forced_tool_choice_ignored": - return `${who} accepted a forced tool choice${on} and answered in text anyway. Turns that must call a tool may loop or stall on this route.`; + return `${who} took a forced tool choice${on} and answered in text anyway, and called nothing under "auto" either. Inconclusive, but no request has yet produced a tool call on this route.`; case "forced_tool_choice_rejected": - return `${who} refuses a forced tool choice${on} but does emit tool calls without one${statusSuffix(result)}. Usable, with less control over when tools fire.`; + return `${who} refuses a forced tool choice${on} but streams native tool calls without one${statusSuffix(result)} — which is all Atomic ever asks for, so this route can run a turn.`; + case "token_cap_rejected": + return `${who} rejected the "max_tokens" cap Atomic puts on every request${statusSuffix(result)}. This model wants "max_completion_tokens"; real turns would fail the same way, so pick another model or route.`; case "tools_payload_rejected": return `${who} answers this model until "tools" is in the request, then refuses it${statusSuffix(result)}. Pick another model or route — Atomic sends tools on every turn.`; case "model_unavailable": diff --git a/src/tui/providers/probe-wizard-contract.test.ts b/src/tui/providers/probe-wizard-contract.test.ts index dae85423..c03cb533 100644 --- a/src/tui/providers/probe-wizard-contract.test.ts +++ b/src/tui/providers/probe-wizard-contract.test.ts @@ -167,4 +167,49 @@ describe("probeWizardContract", () => { ); expect(bodies[0]).toContain("vendor/chosen-model"); }); + it("treats a route that only refuses the forcing as one that can run a turn", async () => { + // Refuses `tool_choice: {type:"function"}`, streams a complete call + // under `auto` — which is the only mode `step-executor` ever sends. + // Every real turn on this route works, so warning about it would be + // warning about a bug the operator is not having. + vi.stubGlobal( + "fetch", + vi.fn(async (_url: unknown, init?: RequestInit) => { + const body = String(init?.body ?? ""); + if (body.includes('"tool_choice":{')) { + return new Response( + JSON.stringify({ error: "tool_choice does not support being set to object" }), + { status: 400 }, + ); + } + return new Response(TOOL_CALL_STREAM, { status: 200 }); + }), + ); + const outcome = await probeWizardContract(wizard("openrouter")); + + expect(outcome.result?.status).toBe("forced_tool_choice_rejected"); + expect(outcome.proven).toBe(true); + expect(outcome.warning).toBeNull(); + expect(outcome.summary).toContain("can run a turn"); + }); + + it("does not tell a keyless-listing service its absent key was rejected", async () => { + // `wizardKeyIsOptional` is true for these presets because they list + // models without a key — not because a completion works without + // one. Probing anyway earns a 401 and the sentence "rejected the + // key" about a key nobody sent. + const fetchMock = vi.fn(async () => new Response("Unauthorized", { status: 401 })); + vi.stubGlobal("fetch", fetchMock); + const outcome = await probeWizardContract( + wizard("openai-compatible", { + apiKeyBuffer: "", + presetId: "nous", + baseUrlLine: "https://inference-api.nousresearch.com", + }), + ); + + expect(outcome.skipped).toBe("no_api_key"); + expect(outcome.summary).toContain("no API key"); + expect(fetchMock).not.toHaveBeenCalled(); + }); }); diff --git a/src/tui/providers/probe-wizard-contract.ts b/src/tui/providers/probe-wizard-contract.ts index 529d5fae..d02ad014 100644 --- a/src/tui/providers/probe-wizard-contract.ts +++ b/src/tui/providers/probe-wizard-contract.ts @@ -8,13 +8,17 @@ * worth refusing because nothing downstream can work without one. * - This answers "can this route run a turn", and the honest response * to "no" is a warning, not a refusal. Some providers block synthetic - * probes outright; a custom endpoint the operator knows works must - * still be savable, and the operator is the one who decides whether - * to live with a route that fires tools only under `auto`. + * probes outright, and a custom endpoint the operator knows works + * must still be savable. * * What it must not do is let a failed probe pass for a proven one — the * caller keys "this install has a working cloud backend" off a clean - * result, so an unproven route reports as unproven. + * result, so an unproven route reports as unproven. Nor may it warn + * about something a turn cannot hit: `contractProbeProvesToolSupport` + * counts a route that refuses a *forced* tool choice and streams a call + * under `auto` as proven, because `auto` is the only mode Atomic ever + * sends, and a warning there would be about a bug the operator is not + * having. */ import { @@ -40,7 +44,10 @@ export interface WizardContractProbeOutcome { readonly proven: boolean; /** * What to show the operator, or `null` when there is nothing worth - * saying: the route passed, or there was nothing here to probe. + * saying: the route ran a turn's worth of work, or there was nothing + * here to probe. `null` is also what the caller's + * "report a working backend" gate keys off, so a probe that ran and + * did not prove the route always fills this in. */ readonly warning: string | null; /** @@ -55,14 +62,6 @@ export interface WizardContractProbeOutcome { readonly result: ProviderContractProbeResult | null; } -/** - * Tighter than the probe module's own budget. This one runs with an - * operator watching a wizard screen: a route slow enough to blow - * through it has told us something already, and the save can proceed - * with the warning rather than holding the screen. - */ -export const WIZARD_CONTRACT_PROBE_TIMEOUT_MS = 12_000; - export async function probeWizardContract( wizard: ProvidersWizardState, opts: { signal?: AbortSignal; timeoutMs?: number } = {}, @@ -85,9 +84,12 @@ export async function probeWizardContract( } const target = resolved.target; + // No budget of its own: `PROVIDER_CONTRACT_PROBE_TIMEOUT_MS` is + // already sized for an operator watching a wizard screen, and this is + // the only entry point the probe has. const result = await runProviderContractProbe(target, { ...(opts.signal ? { signal: opts.signal } : {}), - timeoutMs: opts.timeoutMs ?? WIZARD_CONTRACT_PROBE_TIMEOUT_MS, + ...(opts.timeoutMs !== undefined ? { timeoutMs: opts.timeoutMs } : {}), }); const proven = contractProbeProvesToolSupport(result.status); const summary = describeContractProbeOutcome(result, target.label); diff --git a/src/tui/providers/providers-orchestrator.ts b/src/tui/providers/providers-orchestrator.ts index aea038ca..4d660bd0 100644 --- a/src/tui/providers/providers-orchestrator.ts +++ b/src/tui/providers/providers-orchestrator.ts @@ -532,11 +532,25 @@ export class ProvidersOrchestrator { // Every path here says what actually happened, including the ones // where no request went out. Reporting a skip as a pass would be // the silent "fully compatible" this whole check exists to stop. - const line = wizard - ? (await probeWizardContract(wizard)).summary - : `"${id}" has no provider kind that can be contract-checked.`; + const outcome = wizard ? await probeWizardContract(wizard) : null; + const line = + outcome?.summary ?? + `"${id}" has no provider kind that can be contract-checked.`; this.bus.emit({ type: "providers_status", line }); this.bus.emit({ type: "runtime_info", line }); + // What the route actually said, on the one surface where it is + // worth the noise: this command was typed to diagnose something, + // and a verdict sentence alone leaves the operator guessing which + // 400 they are looking at. Bounded and credential-scrubbed at the + // source (`redactProviderDetail`), and only when there is a + // problem — a passing route's stream summary tells nobody + // anything. + if (outcome?.result && !outcome.proven && outcome.result.detail.length > 0) { + this.bus.emit({ + type: "runtime_info", + line: `Route said: ${outcome.result.detail}`, + }); + } } catch (err) { this.bus.emit({ type: "providers_status", From 6322a0d2d7301d97519e49c1b3739a2b256d57e4 Mon Sep 17 00:00:00 2001 From: Valera Brizhatiuk <19537764+plombeer31@users.noreply.github.com> Date: Wed, 2 Sep 2026 14:25:01 +0300 Subject: [PATCH 3/3] test(providers): cover the probe's wiring, its gate and its redaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three claims the feature is sold on had no test that could fail. Both save paths could be deleted wholesale and every test still passed: the wizard's `completeWizard` tests all stop at a refused key, so nothing reached the probe call, the gate, or the note the operator gets. `completeWizard` now runs end to end with only the disk writes stubbed — real key check, real probe, real gate — in two shapes: a route that streams a native tool call is saved *and* reported as a working backend, and a route whose stream ends early is saved, warned about, and not reported. First-run onboarding gets the same pair. Deleting either call site, or weakening the gate to `gate.warning === null`, now fails four tests. The redaction test was vacuous: 400 filler characters sat in front of the credentials, so the 300-character cap satisfied both assertions on its own and the test passed with `redactProviderDetail` reduced to a plain slice. The fixture now puts both keys inside the cap, and `redact-provider-detail.test.ts` pins each rule on strings short enough that truncation cannot do the work — including the pattern pass, the one capability this adds over the key check's exact-key removal, which nothing in the repo detected the loss of. Verified by mutation, each of these now fails at least one test: redaction disabled; the `KEY_SHAPED` loop deleted; the probe removed from either save path; the gate weakened; the old "timed out with zero bytes" rule restored; the abort dropped from the read race; `max_tokens` removed from the probe body; `forced_tool_choice_rejected` dropped from the proven set. --- .../verify/redact-provider-detail.test.ts | 78 +++++++++ .../cloud-provider-onboarding.test.tsx | 116 +++++++++++++ .../providers/providers-orchestrator.test.ts | 155 ++++++++++++++++++ 3 files changed, 349 insertions(+) create mode 100644 src/llm/provider/verify/redact-provider-detail.test.ts diff --git a/src/llm/provider/verify/redact-provider-detail.test.ts b/src/llm/provider/verify/redact-provider-detail.test.ts new file mode 100644 index 00000000..2c4a8433 --- /dev/null +++ b/src/llm/provider/verify/redact-provider-detail.test.ts @@ -0,0 +1,78 @@ +/** + * Redaction is the only thing standing between a provider's error body + * and a status line, so each rule is exercised on a string short enough + * that the length cap cannot do the work for it. A test whose fixture + * is longer than `PROVIDER_DETAIL_MAX_LEN` passes with redaction + * removed entirely, which is worse than no test at all. + */ + +import { describe, expect, it } from "vitest"; + +import { + PROVIDER_DETAIL_MAX_LEN, + redactProviderDetail, +} from "./redact-provider-detail.js"; + +const KEY = "sk-ours-1234567890"; + +describe("redactProviderDetail", () => { + it("removes the key we sent, wherever the provider echoed it", () => { + const detail = redactProviderDetail( + `invalid key ${KEY} for org (header: Bearer ${KEY})`, + KEY, + ); + + expect(detail).not.toContain(KEY); + expect(detail.length).toBeLessThan(PROVIDER_DETAIL_MAX_LEN); + }); + + it("removes a key-shaped string that is not the one under test", () => { + // The case the exact-match rule cannot reach: a gateway quoting the + // upstream credential it uses on our behalf. Short on purpose — + // truncation must not be what hides this. + const detail = redactProviderDetail( + "upstream rejected sk-or-v1-abcdef0123456789 (routed)", + KEY, + ); + + expect(detail).not.toContain("sk-or-v1-abcdef0123456789"); + expect(detail).toContain("upstream rejected"); + expect(detail).toContain("(routed)"); + }); + + it("removes Google keys and quoted bearer tokens", () => { + const detail = redactProviderDetail( + 'API key AIzaSyD-0123456789abcdef invalid; sent "Bearer ghp_0123456789abcd"', + "", + ); + + expect(detail).not.toContain("AIzaSyD-0123456789abcdef"); + expect(detail).not.toContain("ghp_0123456789abcd"); + }); + + it("leaves an ordinary provider message readable", () => { + // The other half of the contract: a pattern loose enough to redact + // model ids and error codes would make every verdict unreadable. + const message = + "400 InvalidParameter: tool_choice does not support being set to object"; + expect(redactProviderDetail(message, KEY)).toBe(message); + expect(redactProviderDetail("model deepseek-v4-flash not found", KEY)).toBe( + "model deepseek-v4-flash not found", + ); + }); + + it("never lets a whole body through, redacted or not", () => { + const body = `{"error":{"message":"${"detail ".repeat(200)}"}}`; + expect(redactProviderDetail(body, KEY)).toHaveLength( + PROVIDER_DETAIL_MAX_LEN, + ); + }); + + it("ignores a key too short to be one, rather than shredding words", () => { + // `split(apiKey).join("***")` on a 3-character "key" would cut the + // message to pieces; the length floor is what stops it. + expect(redactProviderDetail("the model was not found", "the")).toBe( + "the model was not found", + ); + }); +}); diff --git a/src/tui/components/cloud-provider-onboarding.test.tsx b/src/tui/components/cloud-provider-onboarding.test.tsx index f90697f1..40064972 100644 --- a/src/tui/components/cloud-provider-onboarding.test.tsx +++ b/src/tui/components/cloud-provider-onboarding.test.tsx @@ -61,6 +61,9 @@ vi.mock("../providers/verify-wizard-before-save.js", async (importOriginal) => { }); const currentConfig = { + // The contract probe reads `agent.maxParallelToolCalls` to send the + // `parallel_tool_calls` a real turn would send. + agent: { maxParallelToolCalls: 8 }, llm: { activeTextProvider: "local-llama", activeEmbeddingProvider: "local-llama-embed", @@ -125,6 +128,72 @@ function stubGatedProbe(status = 429): ProbeGate { }; } +/** + * A fetch that lets the key check pass and answers the contract probe — + * the streamed request carrying `tools` — with `sse`. + */ +function stubProbeFetch(sse: string): { probeBodies: () => string[] } { + const probeBodies: string[] = []; + vi.stubGlobal( + "fetch", + vi.fn(async (url: unknown, init?: RequestInit) => { + if (!String(url).includes("/chat/completions")) { + return new Response(JSON.stringify({ data: [] }), { status: 200 }); + } + const body = String(init?.body ?? "{}"); + if (!body.includes('"stream":true')) { + return new Response( + JSON.stringify({ choices: [{ message: { content: "ok" } }] }), + { status: 200 }, + ); + } + probeBodies.push(body); + return new Response(sse, { status: 200 }); + }), + ); + return { probeBodies: () => probeBodies }; +} + +/** A complete native tool call: the one verdict that proves the route. */ +const PROBE_TOOL_CALL_SSE = + `data: ${JSON.stringify({ + choices: [ + { + delta: { + tool_calls: [ + { + index: 0, + type: "function", + function: { + name: "atomic_contract_probe", + arguments: '{"ok":true}', + }, + }, + ], + }, + }, + ], + })}\n\ndata: ${JSON.stringify({ + choices: [{ delta: {}, finish_reason: "tool_calls" }], + })}\n\ndata: [DONE]\n\n`; + +/** Truncated mid-argument, with nothing announcing the end. */ +const PROBE_EARLY_EOF_SSE = `data: ${JSON.stringify({ + choices: [ + { + delta: { + tool_calls: [ + { + index: 0, + type: "function", + function: { name: "atomic_contract_probe", arguments: '{"ok"' }, + }, + ], + }, + }, + ], +})}\n\n`; + async function flush(times = 6): Promise { for (let i = 0; i < times; i += 1) { await new Promise((resolve) => setImmediate(resolve)); @@ -321,3 +390,50 @@ describe("CloudProviderOnboarding cancellation", () => { unmount(); }); }); + +describe("CloudProviderOnboarding contract probe", () => { + beforeEach(() => { + saveMock.mockClear(); + gateOverrides.length = 0; + // The key itself is not what these tests are about; they are about + // what happens between a good key and the save. + gateOverrides.push(async () => ({ proceed: true, warning: null })); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("exercises the streaming tool contract before finishing first-run", async () => { + const probe = stubProbeFetch(PROBE_TOOL_CALL_SSE); + const { stdin, onFinished, unmount } = await mountAtSubmitPoint(); + + stdin.write("\r"); + await flush(12); + + // First-run is where a route that cannot run a turn costs the most: + // the operator's very first message would otherwise be the test. + expect(probe.probeBodies()).toHaveLength(1); + expect(probe.probeBodies()[0]).toContain("atomic_contract_probe"); + expect(saveMock).toHaveBeenCalledTimes(1); + expect(onFinished).toHaveBeenCalledWith("saved_cloud", undefined); + unmount(); + }); + + it("carries the probe's verdict into the finish note without blocking the save", async () => { + stubProbeFetch(PROBE_EARLY_EOF_SSE); + const { stdin, onFinished, unmount } = await mountAtSubmitPoint(); + + stdin.write("\r"); + await flush(12); + + // Advisory, not a gate: the provider is saved either way, and the + // operator is told what the route did instead of finding out on + // their first message. + expect(saveMock).toHaveBeenCalledTimes(1); + expect(onFinished).toHaveBeenCalledTimes(1); + expect(onFinished.mock.calls[0]?.[0]).toBe("saved_cloud"); + expect(String(onFinished.mock.calls[0]?.[1])).toContain("closed the stream"); + unmount(); + }); +}); diff --git a/src/tui/providers/providers-orchestrator.test.ts b/src/tui/providers/providers-orchestrator.test.ts index fb2de59a..ff0b08e5 100644 --- a/src/tui/providers/providers-orchestrator.test.ts +++ b/src/tui/providers/providers-orchestrator.test.ts @@ -15,8 +15,17 @@ vi.mock("../../config/index.js", async (importOriginal) => { let currentConfig: AtomicAgentConfig; +/** + * Enough of the real config for the paths under test: the contract + * probe reads `agent.maxParallelToolCalls` to send the + * `parallel_tool_calls` a turn would send, so a fixture without it + * would fail for a reason no user has. + */ +const AGENT_CONFIG = { maxParallelToolCalls: 8 } as AtomicAgentConfig["agent"]; + function configWithGemini(): AtomicAgentConfig { return { + agent: AGENT_CONFIG, llm: { activeTextProvider: "gemini", activeEmbeddingProvider: "local-llama-embed", @@ -294,6 +303,7 @@ describe("ProvidersOrchestrator.completeWizard", () => { afterEach(() => { vi.unstubAllGlobals(); vi.unstubAllEnvs(); + vi.doUnmock("../persist-llm-provider.js"); }); function wizardFor(kind: "openrouter" | "aimlapi"): ProvidersWizardState { @@ -363,8 +373,153 @@ describe("ProvidersOrchestrator.completeWizard", () => { expect(failure?.error).toContain("rejected this key"); }); + /** + * The save path with only the disk writes stubbed: the real key + * check, the real contract probe and the real gate between them. + * Everything above this point in the file stops at a refused key, so + * without it nothing ever reaches the code that decides whether this + * install may be reported as having a working backend. + */ + async function importOrchestratorWithStubbedDisk() { + vi.doMock("../persist-llm-provider.js", async (importOriginal) => { + const original = + await importOriginal(); + return { + ...original, + upsertLlmProvider: vi.fn(), + writeProviderApiKeyToDotenv: vi.fn(), + setActiveTextProviderInConfig: vi.fn(), + }; + }); + return importFreshOrchestrator(); + } + + /** + * Answers the pre-save key check with a live key, and hands the probe + * request — the streamed one, carrying `tools` — to the caller. + */ + function stubSaveFetch(probeAnswer: () => Response) { + const bodies: Record[] = []; + vi.stubGlobal( + "fetch", + vi.fn(async (url: unknown, init?: RequestInit) => { + if (!String(url).includes("/chat/completions")) { + return new Response(JSON.stringify({ data: [] }), { status: 200 }); + } + const body = JSON.parse(String(init?.body ?? "{}")) as Record< + string, + unknown + >; + bodies.push(body); + if (body.stream !== true) { + return new Response( + JSON.stringify({ choices: [{ message: { content: "ok" } }] }), + { status: 200 }, + ); + } + return probeAnswer(); + }), + ); + return { bodies: () => bodies }; + } + + const PROBE_TOOL_CALL_SSE = + `data: ${JSON.stringify({ + choices: [ + { + delta: { + tool_calls: [ + { + index: 0, + type: "function", + function: { + name: "atomic_contract_probe", + arguments: '{"ok":true}', + }, + }, + ], + }, + }, + ], + })}\n\ndata: ${JSON.stringify({ + choices: [{ delta: {}, finish_reason: "tool_calls" }], + })}\n\ndata: [DONE]\n\n`; + + /** Truncated mid-argument, with nothing announcing the end. */ + const PROBE_EARLY_EOF_SSE = `data: ${JSON.stringify({ + choices: [ + { + delta: { + tool_calls: [ + { + index: 0, + type: "function", + function: { name: "atomic_contract_probe", arguments: '{"ok"' }, + }, + ], + }, + }, + ], + })}\n\n`; + + function wizardWithModel(): ProvidersWizardState { + return { ...wizardFor("openrouter"), selectedChatModelId: "vendor/picked-model" }; + } + + it("probes the route on save and reports a backend only once it is proven", async () => { + currentConfig = configWithGemini(); + const fetches = stubSaveFetch( + () => new Response(PROBE_TOOL_CALL_SSE, { status: 200 }), + ); + const { ProvidersOrchestrator } = await importOrchestratorWithStubbedDisk(); + const bus = fakeBus(); + const runtime = fakeRuntime(); + const orchestrator = new ProvidersOrchestrator(runtime, bus as never); + + await orchestrator.completeWizard(wizardWithModel()); + + // The key check, then the turn contract itself: streamed, with the + // tools payload, on the model the operator picked. + const probeBody = fetches.bodies()[1]; + expect(probeBody?.stream).toBe(true); + expect(probeBody?.model).toBe("vendor/picked-model"); + expect(JSON.stringify(probeBody?.tools)).toContain("atomic_contract_probe"); + + const types = bus.emit.mock.calls.map((call) => (call[0] as { type: string }).type); + expect(types).toContain("providers_wizard_succeeded"); + expect(runtime.reportModelConfigured).toHaveBeenCalledWith( + "openrouter", + "cloud", + ); + }); + + it("saves but reports no working backend when the probe fails", async () => { + currentConfig = configWithGemini(); + stubSaveFetch(() => new Response(PROBE_EARLY_EOF_SSE, { status: 200 })); + const { ProvidersOrchestrator } = await importOrchestratorWithStubbedDisk(); + const bus = fakeBus(); + const runtime = fakeRuntime(); + const orchestrator = new ProvidersOrchestrator(runtime, bus as never); + + await orchestrator.completeWizard(wizardWithModel()); + + // The key is live, so the save stands — the probe is advisory and + // may never refuse one. + const types = bus.emit.mock.calls.map((call) => (call[0] as { type: string }).type); + expect(types).toContain("providers_wizard_succeeded"); + // But the route was never shown to run a turn, so "this install has + // a working cloud backend" must not be claimed on its behalf. + expect(runtime.reportModelConfigured).not.toHaveBeenCalled(); + const lines = bus.emit.mock.calls + .map((call) => call[0] as { type: string; line?: string }) + .filter((action) => action.type === "providers_status") + .map((action) => action.line ?? ""); + expect(lines.some((line) => line.includes("closed the stream"))).toBe(true); + }); + it("runs the contract probe on explicit request and reports the verdict", async () => { currentConfig = { + agent: AGENT_CONFIG, llm: { activeTextProvider: "openrouter", activeEmbeddingProvider: "local-llama-embed",