diff --git a/devlog/_plan/260920_meaning_preservation_batch/050_lane_e.md b/devlog/_plan/260920_meaning_preservation_batch/050_lane_e.md new file mode 100644 index 00000000000..154a0a639e9 --- /dev/null +++ b/devlog/_plan/260920_meaning_preservation_batch/050_lane_e.md @@ -0,0 +1,128 @@ +# Lane E — output budgets, queue memory, per-key policy + +Status: OPEN. Branch `codex/260920-lane-e-budgets-key-policy` against `dev`, one pull request. +Covers phase 2 bundles 10, 11 and 12 from [010_phase2.md](010_phase2.md). + +## What each bundle turned out to be + +### 10 — Devin output budget and the history ceiling + +Two defects, not one. The adapter forwarded only a caller-supplied +`max_output_tokens`, and Codex never sends one, so every `devin/*` turn was capped at +the cloud-direct encoder's 8192 fallback however the provider was configured. The +escape hatch was closed too: no OAuth preset declares `defaultMaxOutputTokens` or +`modelMaxOutputTokens`, so the delete-when-preset-undefined branch in +`applyOAuthPresetCatalog` was the only branch either field ever took and a +hand-edited value was gone before the next startup finished. Both had to move, or +wiring the adapter alone would have been unreachable in practice. + +The resolver reads the caller's explicit value, then the configured per-model cap, +then the provider default, then nothing — leaving the encoder fallback. It never +reads `contextWindow` or `modelContextWindows`: CompletionConfiguration #2 is the +output cap and #3 is the context window, and collapsing them would ask Cognition to +generate a whole window of output. + +The history ceiling is the other half and stays a separate quantity. #5189 is +carried with attribution: it derives the coding-agent projected-history bound from +the declared context window in characters. That bounds replayed history memory; +nothing there decides how long a reply may run. + +No retry change was needed. Source review of `stated-reset-retry.ts` and +`upstream-retry.ts` confirms an upstream `incomplete / max_output_tokens` is a +successful streaming response that has already emitted events, so it matches none +of the replay conditions. The repeated identical attempts in #5190 are the client's. + +### 11 — adapter event queue memory + +PR #5182 had the right idea and the wrong number. Its 1 MiB aggregate default +aborts a legitimate turn: a synchronous producer fills the queue before its +consumer is scheduled, and the image loop does exactly that with over a million +one-character deltas that coalesce into roughly 1.2 MB of retained text. Its own CI +proved it, which is why it sits at `CHANGES_REQUESTED`. + +The work is carried with attribution and reshaped around two budgets rather than +one, because a stalled consumer and a malformed event are different failures and an +operator reading the terminal error should learn which happened. Accounting is now +exact by construction: each queued item records what it was charged, so a merge +pays only for appended text, a refused event is priced before anything is retained +and never charged, and the terminal record explaining a refusal is admitted past +the budget it reports but still charged and released. `retainedCodeUnits()` exposes +the counter so the regressions assert it reaches zero rather than inferring it from +an abort that happened to fire. + +Retention is measured by a bounded walk of own enumerable properties rather than a +per-variant table. A table would be exhaustive over `AdapterEvent`, which is the +union class `AGENTS.md` records: a member added on another branch would silently +stop being counted. + +### 12 — per-admission-key model and provider scope + +The security question is where the check goes, not what it compares. A scope +evaluated against the client's string authorizes one destination and reaches +another, because alias resolution, policy and combo selection, subagent fallback +and compaction override all rewrite that string. So the scope names destinations +and is applied to the resolved route. + +On the Responses path every route produced by the request — direct name, alias, +policy, combo child, shadow-intercept target and both subagent-fallback re-routes — +passes through one capture point, which is where the check sits. Chat and Messages +translate into that path; their native lanes and the compaction route send without +re-entering it, so each applies the same predicate itself. `/v1/models` filters by +the same predicate, and that filter is explicitly not the boundary. + +A malformed scope drops the key rather than degrading to `undefined` like every +other field on the record, because degrading a permission field reads as "allowed +everything". + +Out of scope and deliberately not started: Redis, a full multi-tenant conversion, +and any budget or RPM/TPM system. + +#### What the scope does not cover, stated rather than implied + +An adversarial review of the branch found authenticated data-plane endpoints that +spend provider quota without resolving a model through the router, so the scope +does not reach them: + +- `/v1/images/generations` and `/v1/images/edits`, +- `/v1/audio/transcriptions` and its streaming form, +- `/v1/live`, `/v1/realtime/calls` and the standalone realtime sockets, +- the non-account-qualified branch of `/v1/alpha/search`, which forwards the caller's + model to a search sidecar without routing it. + +The account-qualified search branch does route a model and is checked. The rest +need a destination definition this lane does not own — an image or audio endpoint +has a fixed-purpose model rather than a routed one — and inventing one here would +be the multi-tenant expansion this batch rules out. They are recorded so the +contract is not read as broader than it is. + +The review also found that resolving an OpenAI virtual model rewrites +`route.modelId` to the wire id after the initial check. That one was a real hole in +the stated contract and is fixed: the settled route is re-checked after +normalization, so the id that is billed is the id that was authorized. + +## Verification + +Static source review plus exact-head hosted CI. No local suite, individual test, +typecheck, build, install or live `ocx` execution was run — those are NOT RUN, not +passing. + +Union-defect classes checked before pushing. No file in the touched set carries a +`file-size-baseline.json` cap; the largest, `src/oauth/index.ts` and +`src/server/index/serve-options.ts`, stay under the 2000-line new-file threshold. +The two new test files are registered in both `scripts/test-layout/layout.json` and +`tests/fixtures/test-layout-expected.json`. Nothing here restates a count or +enumerates a union. + +## Ownership + +Lane E owns the adapter event queue budget and the Devin and coding-agent limits. +Retry classification — `sendCount`, the send budget, the stage and cause vocabulary — +is lane C's and is untouched. + +## Carried work + +- #5182 (luvs01) — adapter event queue backlog budget. +- #5189 (mdwsk88) — coding-agent projected-history ceiling. + +Both carry a `Co-authored-by` trailer in the branch commit. Neither original pull +request is closed here; the coordinator handles that after this lane lands. diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index ef1c68922bb..7522f69ddae 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1566,7 +1566,9 @@ "web-search-sidecar-429.test.ts": "web-search", "management-google-tool-schema-policy.test.ts": "server", "codex-shim-destroyed-probe.test.ts": "codex-integration", - "client-runtime.test.ts": "clients" + "client-runtime.test.ts": "clients", + "devin-output-budget.test.ts": "providers", + "api-key-model-scope.test.ts": "server" }, "migrated": [ "adapters", diff --git a/src/adapters/coding-agent/protocol.ts b/src/adapters/coding-agent/protocol.ts index 779e88d14f7..628341bb8d1 100644 --- a/src/adapters/coding-agent/protocol.ts +++ b/src/adapters/coding-agent/protocol.ts @@ -21,6 +21,30 @@ export const MAX_STREAM_LINE_BYTES = 8 * 1024 * 1024; export const MAX_STREAM_TOTAL_BYTES = 64 * 1024 * 1024; /** Hard ceiling on projected conversation history text (characters) to prevent runaway memory. */ export const MAX_PROJECTED_HISTORY_CHARS = 200_000; +/** + * Chars-per-token ratio for deriving the projected-history ceiling from the model context + * window. Sits between the English/code (~4 chars per token) and CJK (~1.5) extremes: the + * ceiling is a runaway-memory bound and a coarse guard against cutting history the window can + * hold, not a token accounting - the caller-side compaction line stays the token authority. + */ +const PROJECTED_HISTORY_CHARS_PER_TOKEN = 3; +/** Absolute ceiling on a window-derived history cap, so runaway metadata cannot unbound stdin. */ +const MAX_PROJECTED_HISTORY_DERIVED_CHARS = 4_000_000; + +/** + * Projected-history character ceiling for a turn, derived from the declared model context + * window. A missing or non-finite window keeps the legacy flat cap, and the derivation never + * lowers the cap below it: small windows change nothing, while large windows scale (a 1M-token + * model keeps 3M characters) until the hard ceiling. The flat 200k cap predates window + * metadata and cut long replays to roughly 50k-130k tokens of content regardless of the model. + */ +export function projectedHistoryCharLimit(contextWindowTokens: number | undefined): number { + if (typeof contextWindowTokens !== "number" || !Number.isFinite(contextWindowTokens) || contextWindowTokens <= 0) { + return MAX_PROJECTED_HISTORY_CHARS; + } + const derived = contextWindowTokens * PROJECTED_HISTORY_CHARS_PER_TOKEN; + return Math.min(Math.max(derived, MAX_PROJECTED_HISTORY_CHARS), MAX_PROJECTED_HISTORY_DERIVED_CHARS); +} export class CodingAgentStreamLimitError extends Error { constructor(message: string) { @@ -395,7 +419,7 @@ export function buildSystemPrompt(parsed: OcxParsedRequest): string | undefined * prior conversation turns are structured as bounded context text with tool results as text, * clearly demarcated from the current user request. Codex retains tool control; vendor tools are never invoked. */ -export function buildConversationInput(parsed: OcxParsedRequest): string[] { +export function buildConversationInput(parsed: OcxParsedRequest, options: { maxHistoryChars?: number } = {}): string[] { const nonDev = parsed.context.messages.filter(m => m.role !== "developer"); if (nonDev.length === 0) { return [JSON.stringify({ type: "user", message: { role: "user", content: [{ type: "text", text: "" }] } })]; @@ -475,10 +499,11 @@ export function buildConversationInput(parsed: OcxParsedRequest): string[] { const imageBlocks: WireContentPart[] = [...historyImageBlocks, ...currentImageBlocks]; + const maxHistoryChars = options.maxHistoryChars ?? MAX_PROJECTED_HISTORY_CHARS; let historyText = historyMessages.map(formatMessageForHistory).filter(Boolean).join("\n\n"); - if (historyText.length > MAX_PROJECTED_HISTORY_CHARS) { + if (historyText.length > maxHistoryChars) { historyText = `[Earlier conversation history truncated for length...]\n\n` + - historyText.slice(historyText.length - MAX_PROJECTED_HISTORY_CHARS); + historyText.slice(historyText.length - maxHistoryChars); } const combinedText = `Prior conversation context:\n\n${historyText}\n\nCurrent user request:\n\n${currentRequestText}`; diff --git a/src/adapters/coding-agent/turn.ts b/src/adapters/coding-agent/turn.ts index a264ce188fa..f13b1ff0ea3 100644 --- a/src/adapters/coding-agent/turn.ts +++ b/src/adapters/coding-agent/turn.ts @@ -1,8 +1,9 @@ import { execFileSync, spawn as nodeSpawn, type ChildProcess, type SpawnOptions } from "node:child_process"; import type { AdapterEvent, OcxParsedRequest, OcxProviderConfig } from "../../types"; import { commandInvocation } from "../../lib/win-exec"; +import { modelRecordValue } from "../../reasoning-effort"; import type { IncomingMeta } from "../base"; -import { buildConversationInput, CodingAgentProtocolError, mapStreamMessageToEvents, readJsonLines, type StreamParseState } from "./protocol"; +import { buildConversationInput, CodingAgentProtocolError, mapStreamMessageToEvents, projectedHistoryCharLimit, readJsonLines, type StreamParseState } from "./protocol"; import { resolveCodingAgentBinary, resolveProfileByBaseUrl, type CodingAgentProviderProfile, type WhichFn } from "./profile"; /** Injectable spawn for tests; production uses node:child_process. */ @@ -264,7 +265,14 @@ export async function runCodingAgentTurn(input: CodingAgentTurnInput): Promise { /* EPIPE if the CLI exits early; surfaced via close/stderr */ }); - for (const line of buildConversationInput(parsed)) stdin.write(`${line}\n`); + // The projected history scales with the model context window on the routed provider row + // (catalog and config metadata merged): a 1M-token model keeps 3M characters of replay + // where the flat cap cut it near 50k-130k tokens of content. Absent metadata keeps the + // flat cap. + const historyCharLimit = projectedHistoryCharLimit( + modelRecordValue(provider.modelContextWindows, parsed.modelId) ?? provider.contextWindow, + ); + for (const line of buildConversationInput(parsed, { maxHistoryChars: historyCharLimit })) stdin.write(`${line}\n`); stdin.end(); } const stdout = child.stdout; diff --git a/src/adapters/devin.ts b/src/adapters/devin.ts index 5832b2be8b7..c1c67bdd409 100644 --- a/src/adapters/devin.ts +++ b/src/adapters/devin.ts @@ -192,6 +192,35 @@ async function resolveWireModelUid( */ export const resolveWireModelUidForTests = resolveWireModelUid; +const positiveTokenCount = (value: unknown): number | undefined => + typeof value === "number" && Number.isSafeInteger(value) && value > 0 ? value : undefined; + +/** + * Read a per-model token count for the exact UID selected for this turn. + * + * Tries the selected UID and then its collapsed base id, preferring the + * canonical spelling and accepting dotted or case-folded saved hints — the same + * normalization the inference request applies to the model id. Where several + * spellings match one id, the smallest wins: a ceiling stated twice is + * satisfied by the lower statement. + */ +function devinModelTokenHint( + record: Record | undefined, + modelUid: string, +): number | undefined { + if (!record) return undefined; + for (const id of [modelUid, collapseDevinModelUid(modelUid)]) { + const exact = Object.hasOwn(record, id) ? positiveTokenCount(record[id]) : undefined; + if (exact !== undefined) return exact; + const matches = Object.entries(record) + .filter(([key]) => normalizeDevinModelId(key).toLowerCase() === id.toLowerCase()) + .map(([, value]) => positiveTokenCount(value)) + .filter((value): value is number => value !== undefined); + if (matches.length > 0) return Math.min(...matches); + } + return undefined; +} + /** * Resolve the INPUT ceiling for the exact UID selected for this turn. Catalog * ClientModelConfig #18 and CompletionConfiguration #3 both carry input tokens; @@ -204,33 +233,50 @@ function resolveDevinMaxInputTokens( modelUid: string, liveWindow?: number, ): number | undefined { - const positive = (value: unknown): number | undefined => - typeof value === "number" && Number.isSafeInteger(value) && value > 0 ? value : undefined; - const baseId = collapseDevinModelUid(modelUid); - const configured = (record: Record | undefined): number | undefined => { - if (!record) return undefined; - for (const id of [modelUid, baseId]) { - // Prefer the canonical spelling; retain dotted/case-folded saved hints, - // matching the model-id normalization used for the inference request. - const exact = Object.hasOwn(record, id) ? positive(record[id]) : undefined; - if (exact !== undefined) return exact; - const matches = Object.entries(record) - .filter(([key]) => normalizeDevinModelId(key).toLowerCase() === id.toLowerCase()) - .map(([, value]) => positive(value)) - .filter((value): value is number => value !== undefined); - if (matches.length > 0) return Math.min(...matches); - } - return undefined; - }; - const contextHint = configured(provider.modelContextWindows) ?? positive(provider.contextWindow); - const inputHint = configured(provider.modelMaxInputTokens); - const ceilings = [positive(liveWindow), contextHint, inputHint] + const contextHint = devinModelTokenHint(provider.modelContextWindows, modelUid) + ?? positiveTokenCount(provider.contextWindow); + const inputHint = devinModelTokenHint(provider.modelMaxInputTokens, modelUid); + const ceilings = [positiveTokenCount(liveWindow), contextHint, inputHint] .filter((value): value is number => value !== undefined); return ceilings.length > 0 ? Math.min(...ceilings) : undefined; } -/** Pure test seam; runtime uses the same resolver immediately before dispatch. */ +/** + * Resolve the OUTPUT ceiling for this turn, highest authority first: + * + * 1. the caller's explicit `max_output_tokens`, forwarded unchanged — an + * explicit cap is a request, so a small one is never widened into a + * configured larger one; + * 2. the configured per-model cap (`modelMaxOutputTokens`), read through the + * same UID-aware hint lookup the input ceiling uses; + * 3. the provider-wide `defaultMaxOutputTokens`; + * 4. undefined, which leaves the cloud-direct encoder's own 8192 fallback in + * place for a provider that configured nothing. + * + * This is NOT the history ceiling, and the two must not collapse into one + * number. CompletionConfiguration #2 is the output cap and #3 is the context + * window, so feeding a context window into this resolver would ask Cognition to + * generate a whole window's worth of output. Nothing here reads + * `contextWindow` or `modelContextWindows` for that reason. + * + * Step 1 keeps the caller's raw value rather than `positiveTokenCount`: the + * inbound parser owns what a caller may send, and re-filtering here would + * silently promote a rejected value to a configured cap the caller never asked + * for. + */ +function resolveDevinMaxOutputTokens( + provider: OcxProviderConfig, + modelUid: string, + requested: number | undefined, +): number | undefined { + if (typeof requested === "number") return requested; + return devinModelTokenHint(provider.modelMaxOutputTokens, modelUid) + ?? positiveTokenCount(provider.defaultMaxOutputTokens); +} + +/** Pure test seams; runtime uses the same resolvers immediately before dispatch. */ export const resolveDevinMaxInputTokensForTests = resolveDevinMaxInputTokens; +export const resolveDevinMaxOutputTokensForTests = resolveDevinMaxOutputTokens; export class DevinMissingCredentialError extends Error { constructor() { @@ -590,6 +636,9 @@ export function createDevinAdapter( const maxInputTokens = resolveDevinMaxInputTokens( provider, modelUid, catalog?.byUid.get(modelUid)?.contextWindow, ); + const maxOutputTokens = resolveDevinMaxOutputTokens( + provider, modelUid, parsed.options.maxOutputTokens, + ); // The reset-retry wrapper waits out a 429 that states its own recovery // delay ("limit will reset in 35 seconds") and replays the identical // request — but only while zero events have been yielded, so a @@ -606,7 +655,7 @@ export function createDevinAdapter( // input hint used to force every model through the 128k default. completionOpts: { ...(maxInputTokens !== undefined ? { maxInputTokens } : {}), - ...(typeof parsed.options.maxOutputTokens === "number" ? { maxOutputTokens: parsed.options.maxOutputTokens } : {}), + ...(maxOutputTokens !== undefined ? { maxOutputTokens } : {}), ...(typeof parsed.options.temperature === "number" ? { temperature: parsed.options.temperature } : {}), ...(typeof parsed.options.topP === "number" ? { topP: parsed.options.topP } : {}), }, diff --git a/src/adapters/run-turn-queue.ts b/src/adapters/run-turn-queue.ts index 76024b32662..23c1cd50f18 100644 --- a/src/adapters/run-turn-queue.ts +++ b/src/adapters/run-turn-queue.ts @@ -6,11 +6,107 @@ export const PREFLIGHT_HEARTBEAT_RETAIN_LIMIT = 16; /** * Coalescing threshold for adjacent text/thinking deltas buffered with no - * waiting reader (UTF-16 code units). This is a merge-size ceiling, not a - * byte-memory cap: a single oversized incoming event stays one item. + * waiting reader (UTF-16 code units). This is a merge-size ceiling; the two + * retention budgets below are what bound memory. */ export const COALESCE_MAX_CHUNK_LENGTH = 64 * 1024; +/** + * Retained-string budget for the WHOLE queue, in UTF-16 code units. + * + * This bounds what the queue is holding at one moment, not how much a turn + * streams: every dequeue gives its charge back, so a long healthy stream with a + * consumer attached never accumulates and is never capped by total length. + * + * 32 MiB is deliberately far above any single legitimate burst. A synchronous + * producer can legally fill the queue before its consumer is scheduled — the + * image loop does exactly that with over a million one-character deltas, which + * coalesce into roughly 1.2 MB of retained text — so a budget near that size + * aborts healthy turns rather than stalled ones. The previous effective bound + * was the 1024-event cap times the 64 KiB merge ceiling, so 64 MiB; this halves + * it while leaving that legitimate burst an order of magnitude of headroom. + */ +export const DEFAULT_MAX_BACKLOG_CODE_UNITS = 32 * 1024 * 1024; + +/** + * Retained-string budget for ONE queued event, in UTF-16 code units. + * + * Separate from the aggregate on purpose, because the two describe different + * failures. Passing the aggregate means the consumer is not keeping up. Passing + * this one means a single event is malformed or unbounded, which stays true + * however empty the queue is, so it must be refused even with the whole + * aggregate free. They also report different terminal messages, so an operator + * reading the turn's error learns which happened. + * + * Like the aggregate, this governs what the queue RETAINS. An event handed + * straight to a waiting consumer is never held here, so neither budget applies + * to it: refusing it would abort a turn over memory this queue does not own, + * and the consumer's own per-event bound governs that payload instead. + */ +export const DEFAULT_MAX_EVENT_CODE_UNITS = 8 * 1024 * 1024; + +const BACKLOG_EXCEEDED_MESSAGE = "consumer stalled: adapter event backlog exceeded — turn aborted"; +const EVENT_TOO_LARGE_MESSAGE = "adapter event exceeds the single-event retained-string budget — turn aborted"; + +/** + * Bound on how far the retention measure walks into one event. AdapterEvent is + * a plain-data union, but two of its members carry open provider-shaped bags + * (`providerState`, `usage.rawUsage`) whose depth no type here controls. The + * ceilings keep a single push O(1)-ish rather than O(whatever an adapter + * attached), and under-counting a pathological object is the safe direction: + * the event-count cap still bounds how many of them can be retained. + */ +const RETENTION_MAX_DEPTH = 8; +const RETENTION_MAX_NODES = 4096; + +/** + * Retained UTF-16 code units carried by one event's string payload. + * + * Measured by walking own enumerable properties rather than by naming each + * variant's string fields: a hand-written per-variant table is exhaustive over + * a union, so adding an event type on one branch while a consumer lands on + * another produces a measure that silently stops counting the new payload. + * The walk is the derived answer and needs no update when the union grows. + * + * `type` is skipped because it is the discriminant, identical for every event + * of a kind and not payload anyone is buffering. + */ +export function retainedEventCodeUnits(event: AdapterEvent): number { + // Defensive: this measures values an adapter produced. A malformed emission + // has to become a terminal event, not a TypeError thrown out of push() with + // the queue half-updated. + if (!event || typeof event !== "object") return 0; + let total = 0; + let nodes = 0; + const seen = new Set(); + const visit = (value: unknown, depth: number): void => { + if (typeof value === "string") { + total += value.length; + return; + } + if (!value || typeof value !== "object" || depth >= RETENTION_MAX_DEPTH || seen.has(value)) return; + seen.add(value); + for (const nested of Object.values(value)) { + if (nodes++ >= RETENTION_MAX_NODES) return; + visit(nested, depth + 1); + } + }; + for (const [key, value] of Object.entries(event)) { + if (key === "type") continue; + if (nodes++ >= RETENTION_MAX_NODES) break; + visit(value, 1); + } + return total; +} + +function positiveBudget(value: number | undefined, fallback: number, name: string): number { + const resolved = value ?? fallback; + if (!Number.isSafeInteger(resolved) || resolved <= 0) { + throw new RangeError(`${name} must be a positive safe integer`); + } + return resolved; +} + export interface AdapterEventQueue { /** * Returns true when the event was merged into the buffered tail instead of @@ -22,6 +118,13 @@ export interface AdapterEventQueue { close(): void; stream(): AsyncIterable; collect(): Promise; + /** + * Retained string payload the queue is currently holding, in UTF-16 code + * units. Exposed so a caller — and a regression — can assert the counter + * returns to zero on every terminal path instead of inferring it from an + * abort that happened to fire. + */ + retainedCodeUnits(): number; } export interface AdapterEventPreflight { @@ -73,47 +176,74 @@ export async function preflightAdapterEvents( export function createAdapterEventQueue(opts?: { maxBacklog?: number; + maxBacklogCodeUnits?: number; + maxEventCodeUnits?: number; onBacklogExceeded?: () => void; }): AdapterEventQueue { const queued: AdapterEvent[] = []; + /** + * What each queued item was charged, in lockstep with `queued`. Releasing the + * recorded charge rather than re-measuring is what makes the accounting exact + * on every path: a merge, a terminal record admitted past the budget and a + * plain event all give back precisely what they took, so the counter cannot + * drift positive (a leak) or negative (a budget the next turn gets for free). + */ + const charged: number[] = []; const readers: QueueReader[] = []; const maxBacklog = opts?.maxBacklog ?? 1_024; + const maxBacklogCodeUnits = positiveBudget(opts?.maxBacklogCodeUnits, DEFAULT_MAX_BACKLOG_CODE_UNITS, "maxBacklogCodeUnits"); + const maxEventCodeUnits = positiveBudget(opts?.maxEventCodeUnits, DEFAULT_MAX_EVENT_CODE_UNITS, "maxEventCodeUnits"); + let retained = 0; let closed = false; // Merge an incoming delta into the buffered tail when no reader is waiting. - // The backlog cap counts events, not tokens, so a detached or briefly - // stalled consumer (e.g. a Codex app mid-reconnect whose disconnect Bun has - // not yet delivered) used to hit the cap within seconds of token-granular - // streaming and abort a healthy turn. Adjacent same-phase text deltas, - // adjacent thinking deltas, and consecutive heartbeats carry no ordering - // information between themselves, so merging them preserves every consumer - // contract while making the cap approximate buffered items again. + // The event cap counts events, not tokens, so a detached or briefly stalled + // consumer (e.g. a Codex app mid-reconnect whose disconnect Bun has not yet + // delivered) used to hit it within seconds of token-granular streaming and + // abort a healthy turn. Adjacent same-phase text deltas, adjacent thinking + // deltas, and consecutive heartbeats carry no ordering information between + // themselves, so merging them preserves every consumer contract while making + // the cap approximate buffered items again. // Pushed objects may be retained by adapters, so the tail is REPLACED with - // a fresh object — never mutated (alias safety). - const coalesceIntoTail = (event: AdapterEvent): boolean => { - const tail = queued[queued.length - 1]; - if (!tail) return false; + // a fresh object — never mutated (alias safety). Returning the replacement + // instead of installing it lets push price the merge before committing to it. + const planTailMerge = (tail: AdapterEvent, event: AdapterEvent): AdapterEvent | null => { if (event.type === "heartbeat") { - if (tail.type !== "heartbeat") return false; + if (tail.type !== "heartbeat") return null; // Heartbeats carry no ordering between themselves, but the replay-unsafe // marker is not ordering — it is a latch. Dropping the incoming event // would discard the only record that Cursor already performed a local // side effect, and preflight would then permit an OAuth replay of it. if (event.replayUnsafe === true && tail.replayUnsafe !== true) { - queued[queued.length - 1] = { type: "heartbeat", replayUnsafe: true }; + return { type: "heartbeat", replayUnsafe: true }; } - return true; + return tail; } if (event.type === "text_delta" && tail.type === "text_delta" && tail.phase === event.phase) { - if (tail.text.length + event.text.length > COALESCE_MAX_CHUNK_LENGTH) return false; - queued[queued.length - 1] = { type: "text_delta", text: tail.text + event.text, phase: tail.phase }; - return true; + if (tail.text.length + event.text.length > COALESCE_MAX_CHUNK_LENGTH) return null; + return { type: "text_delta", text: tail.text + event.text, phase: tail.phase }; } if (event.type === "thinking_delta" && tail.type === "thinking_delta") { - if (tail.thinking.length + event.thinking.length > COALESCE_MAX_CHUNK_LENGTH) return false; - queued[queued.length - 1] = { type: "thinking_delta", thinking: tail.thinking + event.thinking }; - return true; + if (tail.thinking.length + event.thinking.length > COALESCE_MAX_CHUNK_LENGTH) return null; + return { type: "thinking_delta", thinking: tail.thinking + event.thinking }; } + return null; + }; + + /** + * Record why the turn is ending and close. The terminal error is admitted + * past both budgets — refusing to retain the explanation of a refusal would + * leave the consumer with a silent truncation — but it is charged like any + * other item so the counter stays exact through the final drain. + */ + const abortWith = (message: string): false => { + opts?.onBacklogExceeded?.(); + const terminal: AdapterEvent = { type: "error", message }; + const cost = retainedEventCodeUnits(terminal); + queued.push(terminal); + charged.push(cost); + retained += cost; + close(); return false; }; @@ -121,17 +251,35 @@ export function createAdapterEventQueue(opts?: { if (closed) return false; const reader = readers.shift(); if (reader) { + // Handed straight to a waiting consumer, so the queue retains nothing and + // charges nothing. Neither budget applies to an event it never holds. reader({ done: false, value: event }); return false; } - if (coalesceIntoTail(event)) return true; - if (queued.length >= maxBacklog) { - opts?.onBacklogExceeded?.(); - queued.push({ type: "error", message: "consumer stalled: adapter event backlog exceeded — turn aborted" }); - close(); - return false; + const tail = queued[queued.length - 1]; + const merged = tail ? planTailMerge(tail, event) : null; + if (merged && tail) { + const replacement = retainedEventCodeUnits(merged); + if (replacement > maxEventCodeUnits) return abortWith(EVENT_TOO_LARGE_MESSAGE); + // Charge only what the backlog actually gains. A merge keeps the tail's + // own fields, so the incoming event's duplicated phase is never retained + // twice and an unchanged tail costs nothing at all. + const delta = replacement - charged[charged.length - 1]!; + if (delta > maxBacklogCodeUnits - retained) return abortWith(BACKLOG_EXCEEDED_MESSAGE); + queued[queued.length - 1] = merged; + charged[charged.length - 1] = replacement; + retained += delta; + return true; } + const cost = retainedEventCodeUnits(event); + if (cost > maxEventCodeUnits) return abortWith(EVENT_TOO_LARGE_MESSAGE); + // Both refusals are priced before anything is retained, so an event that is + // turned away is never charged for. + if (queued.length >= maxBacklog) return abortWith(BACKLOG_EXCEEDED_MESSAGE); + if (cost > maxBacklogCodeUnits - retained) return abortWith(BACKLOG_EXCEEDED_MESSAGE); queued.push(event); + charged.push(cost); + retained += cost; return false; }; @@ -147,6 +295,7 @@ export function createAdapterEventQueue(opts?: { while (true) { const next = queued.shift(); if (next) { + retained -= charged.shift() ?? 0; yield next; continue; } @@ -165,5 +314,5 @@ export function createAdapterEventQueue(opts?: { return events; }; - return { push, close, stream, collect }; + return { push, close, stream, collect, retainedCodeUnits: () => retained }; } diff --git a/src/cli/access.ts b/src/cli/access.ts index 51b351f9042..9cc0ec091a2 100644 --- a/src/cli/access.ts +++ b/src/cli/access.ts @@ -12,6 +12,8 @@ import { const USAGE = `Usage: ocx access key [list] [--json] ocx access key create [name] [--json] + ocx access key get [--json] + ocx access key set [--allow-provider ]... [--allow-model ]... [--clear] [--json] ocx access key rotate [--json] ocx access key rotate commit [--json] ocx access key rotate abort [--json] @@ -65,6 +67,51 @@ function formatKeyRows(payload: Record, keys: Array 0 ? [...lines, "", ...footer] : lines; } +/** + * Repeatable option values, in the order given. + * + * takeOption removes one occurrence, so a scope with several entries needs the + * loop: reading it once would silently keep only the first `--allow-model` and + * write a narrower scope than the operator typed. + */ +function takeAllOptions(args: string[], name: string): string[] { + const values: string[] = []; + for (;;) { + const value = takeOption(args, name); + if (value === undefined) break; + values.push(value); + } + return values; +} + +/** + * Find a key by id or by name, without ever reading the secret. + * + * The management API keys every mutation by id, so a name has to be resolved + * here. An ambiguous name is refused rather than resolved to the first match: + * silently scoping one of two keys that share a name is the kind of mistake + * only discovered when the wrong client stops working. + */ +function findKeyRow(keys: Array>, selector: string): Record { + const wanted = selector.trim().toLowerCase(); + const byId = keys.filter(entry => String(entry.id ?? "").toLowerCase() === wanted); + if (byId.length === 1) return byId[0]!; + const byName = keys.filter(entry => String(entry.name ?? "").trim().toLowerCase() === wanted); + if (byName.length === 1) return byName[0]!; + if (byName.length > 1) throw new CliUsageError("key name " + selector + " is ambiguous; use the id", USAGE); + throw new CliUsageError("no API key matches " + selector, USAGE); +} + +function scopeLines(entry: Record): string[] { + const list = (value: unknown): string => + Array.isArray(value) && value.length > 0 ? (value as string[]).join(", ") : "(any)"; + return [ + "API key " + String(entry.name ?? "") + " (" + String(entry.id ?? "") + ")", + " allowed providers: " + list(entry.allowedProviders), + " allowed models: " + list(entry.allowedModels), + ]; +} + async function key(argv: string[], deps: RuntimeApiDeps): Promise { const args = [...argv]; const action = (args.shift() ?? "list").toLowerCase(); @@ -90,6 +137,46 @@ async function key(argv: string[], deps: RuntimeApiDeps): Promise { ]); return; } + if (action === "get") { + const selector = args.shift(); + if (!selector) throw new CliUsageError("key id or name is required", USAGE); + rejectArgs(args, USAGE); + const result = await runtimeRequest>("/api/keys", {}, deps); + const entry = findKeyRow(Array.isArray(result.keys) ? result.keys as Array> : [], selector); + // The list response carries the masked prefix and never the secret, so the + // row is safe to print as-is under --json. + printData(entry, wantsJson, scopeLines(entry)); + return; + } + if (action === "set") { + const selector = args.shift(); + if (!selector) throw new CliUsageError("key id or name is required", USAGE); + const clear = takeFlag(args, "--clear"); + const providers = takeAllOptions(args, "--allow-provider"); + const models = takeAllOptions(args, "--allow-model"); + rejectArgs(args, USAGE); + if (!clear && providers.length === 0 && models.length === 0) { + throw new CliUsageError("set requires --allow-provider, --allow-model, or --clear", USAGE); + } + const listed = await runtimeRequest>("/api/keys", {}, deps); + const target = findKeyRow(Array.isArray(listed.keys) ? listed.keys as Array> : [], selector); + // A set REPLACES the named dimension rather than appending to it, and + // --clear removes both. Naming one dimension leaves the other alone, so + // narrowing providers cannot accidentally widen models. + const body: Record = { id: target.id }; + if (clear) { + body.allowedProviders = null; + body.allowedModels = null; + } + if (providers.length > 0) body.allowedProviders = providers; + if (models.length > 0) body.allowedModels = models; + const result = await runtimeRequest>("/api/keys", { + method: "PATCH", + body: JSON.stringify(body), + }, deps); + printData(result, wantsJson, scopeLines(result)); + return; + } if (action === "rotate") { const operation = args[0] === "commit" || args[0] === "abort" ? args.shift()! : "start"; const id = args.shift(); diff --git a/src/config/schema/leaf-validators.ts b/src/config/schema/leaf-validators.ts index f3160644b08..93edd25d0fc 100644 --- a/src/config/schema/leaf-validators.ts +++ b/src/config/schema/leaf-validators.ts @@ -649,6 +649,13 @@ export const apiKeyEntrySchema = z.object({ createdAt: z.string().catch(""), // A damaged overlap record must never discard the still-authoritative key. pendingRotation: pendingApiKeyRotationSchema.optional().catch(undefined), + // Deliberately NOT `.catch`ed, unlike every field above. Degrading a damaged + // scope to `undefined` would silently widen the key to the whole catalog, + // which is the one direction a permission field must never fail. Letting the + // record fail instead drops the key, so a corrupted scope stops that client + // rather than promoting it. + allowedProviders: z.array(z.string().trim().min(1).max(256)).optional(), + allowedModels: z.array(z.string().trim().min(1).max(256)).optional(), }).passthrough(); /** diff --git a/src/oauth/index.ts b/src/oauth/index.ts index f33a6855c0b..5d8e00ef472 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -1273,6 +1273,29 @@ const OAUTH_RECONCILE_FIELDS: (keyof OcxProviderConfig)[] = [ // existing rows through enrichProviderFromRegistry, which is fill-only and // preserves explicit saved values. +/** + * Output-budget fields an OAuth preset may refresh but must never erase. + * + * These stay on the reconcile list so a preset that does declare a budget still + * refreshes the saved row. What changes is the other branch: when the preset + * declares nothing, the operator's value survives instead of being deleted. + * + * Without that, the fields behaved as if they could not be configured at all. + * No OAuth preset seeds either one, so the delete branch was the only branch + * these two ever took, and a hand-edited `defaultMaxOutputTokens` was gone + * before the first turn of the next startup — leaving the adapter's own + * fallback as the only reachable output cap (#5190). + * + * Scoped to the output budget on purpose. The input side (`contextWindow`, + * `modelContextWindows`) describes what the account's models are, which the + * preset and live discovery do own; an output budget is a spend decision the + * operator makes. + */ +const OAUTH_PRESERVE_WHEN_PRESET_UNSET: ReadonlySet = new Set([ + "defaultMaxOutputTokens", + "modelMaxOutputTokens", +]); + const GOOGLE_ANTIGRAVITY_PROVIDER = "google-antigravity"; const GOOGLE_ANTIGRAVITY_LIVE_DISCOVERY_VERSION = 2 as const; @@ -1312,7 +1335,7 @@ function applyOAuthPresetCatalog( if (JSON.stringify(provider[field]) === JSON.stringify(preset[field])) continue; if (preset[field] !== undefined) { provider[field] = cloneProviderField(preset[field]) as never; - } else { + } else if (!OAUTH_PRESERVE_WHEN_PRESET_UNSET.has(field)) { delete provider[field]; } } diff --git a/src/server/admission-model-scope.ts b/src/server/admission-model-scope.ts new file mode 100644 index 00000000000..4ffe9380827 --- /dev/null +++ b/src/server/admission-model-scope.ts @@ -0,0 +1,166 @@ +import type { DataPlaneAdmission } from "./auth-cors"; +import type { OcxApiKeyEntry, OcxConfig } from "../types"; + +/** + * Per-admission-key model and provider scope. + * + * A hub that serves several clients with their own `ocx_data_…` keys needs a + * mail or cron key to be unable to spend a coding key's Grok or Claude quota. + * Hub-level model selection cannot express that: it hides a model from + * everyone or from no one (#5049). + * + * Two rules decide what this is and is not: + * + * A scope names DESTINATIONS, not selectors. It is evaluated against the + * resolved route — the provider and model a turn will actually bill — never + * against the string the client sent. A client-supplied label is not a + * permission subject: alias resolution, a combo pick, a policy fallback and a + * compaction override all rewrite that string, so a scope checked before them + * would authorize one destination and reach another. + * + * A scope is never management authority. It narrows which models an inference + * key may call and grants nothing else; reading or editing a scope stays on + * the management credential. + */ +export interface AdmissionModelScope { + /** Resolved provider names this key may reach. Empty means every provider. */ + readonly providers: readonly string[]; + /** Resolved destinations this key may reach. Empty means every model. */ + readonly models: readonly string[]; +} + +/** The resolved destination a scope decision is made about. */ +export interface ScopedRoute { + readonly providerName: string; + readonly modelId: string; +} + +const normalize = (value: string): string => value.trim().toLowerCase(); + +function normalizedList(values: readonly string[] | undefined): readonly string[] { + if (!Array.isArray(values)) return []; + const seen = new Set(); + for (const value of values) { + if (typeof value !== "string") continue; + const normalized = normalize(value); + if (normalized) seen.add(normalized); + } + return [...seen]; +} + +/** Read the scope a stored key declares. An entry with neither list is unrestricted. */ +export function admissionModelScopeOf(entry: Pick): AdmissionModelScope | undefined { + const providers = normalizedList(entry.allowedProviders); + const models = normalizedList(entry.allowedModels); + return providers.length === 0 && models.length === 0 ? undefined : { providers, models }; +} + +/** + * The scope that applies to one request, or undefined when nothing is scoped. + * + * Only a configured key carries a scope. The environment token and loopback + * admission have no stored record to attach one to, so they stay unrestricted; + * an operator who wants them narrowed issues a configured key instead. + */ +export function resolveAdmissionModelScope( + config: Pick, + admission: DataPlaneAdmission | undefined, +): AdmissionModelScope | undefined { + if (!admission || admission.kind !== "configured") return undefined; + const entry = (config.apiKeys ?? []).find(key => key.id === admission.keyId); + return entry ? admissionModelScopeOf(entry) : undefined; +} + +/** + * Does this scope admit this resolved destination? + * + * The two lists are independent conditions and both must hold when both are + * declared: a key allowed one provider and one model may not reach that + * model on a different provider, which is what a combo child or a policy + * fallback would otherwise do while the requested selector stayed the same. + * + * A model entry matches the bare resolved model id or the fully qualified + * `provider/model` form, so an operator can scope one model everywhere or + * pin it to a single provider without a second field. + */ +export function routeAllowedByScope( + scope: AdmissionModelScope | undefined, + route: ScopedRoute, +): boolean { + if (!scope) return true; + const provider = normalize(route.providerName); + const model = normalize(route.modelId); + if (scope.providers.length > 0 && !scope.providers.includes(provider)) return false; + if (scope.models.length === 0) return true; + return scope.models.includes(model) || scope.models.includes(provider + "/" + model); +} + +/** + * A request that asked for a destination its key may not reach. + * + * Carries the selector the client sent rather than the destination it resolved + * to: the caller needs to know which of its own requests was refused, and a + * key that may not reach a provider has no business learning that an alias it + * named points there. The resolved destination goes to the server log. + */ +export class AdmissionModelDeniedError extends Error { + readonly requestedModel: string; + readonly deniedProvider: string; + readonly deniedModel: string; + constructor(requestedModel: string, route: ScopedRoute) { + super("model " + requestedModel + " is not allowed for this API key"); + this.name = "AdmissionModelDeniedError"; + this.requestedModel = requestedModel; + this.deniedProvider = route.providerName; + this.deniedModel = route.modelId; + } +} + +/** Stable wire type for a scope refusal. */ +export const MODEL_NOT_ALLOWED_FOR_KEY = "model_not_allowed_for_key"; + +/** The HTTP body a scope refusal returns. 403: authenticated, not permitted. */ +export function admissionModelDeniedBody(error: AdmissionModelDeniedError): { + error: { type: string; message: string; model: string }; +} { + return { + error: { + type: MODEL_NOT_ALLOWED_FOR_KEY, + message: error.message, + model: error.requestedModel, + }, + }; +} + +/** + * The refusal a scoped request gets: 403, not 404. + * + * The key authenticated; it simply may not reach this destination. Reporting + * "not found" instead would tell a client its credential is wrong and invite + * it to retry with another, and would make an operator's own denial + * indistinguishable from a typo in the model name. + */ +export function admissionModelDeniedResponse(error: AdmissionModelDeniedError): Response { + return new Response(JSON.stringify(admissionModelDeniedBody(error)), { + status: 403, + headers: { "Content-Type": "application/json" }, + }); +} + +/** + * Refuse a resolved destination this key may not reach. + * + * Every request-path site that produces or re-produces a route calls this, so + * the direct name, an alias, a combo child, a policy or subagent fallback and + * a compaction override are all checked at the point they become concrete + * rather than once at the front door. + */ +export function assertRouteAllowedByScope( + scope: AdmissionModelScope | undefined, + requestedModel: string, + route: ScopedRoute, +): void { + if (!routeAllowedByScope(scope, route)) { + throw new AdmissionModelDeniedError(requestedModel, route); + } +} diff --git a/src/server/chat-completions.ts b/src/server/chat-completions.ts index 8a611e70e4b..4f4209d11c0 100644 --- a/src/server/chat-completions.ts +++ b/src/server/chat-completions.ts @@ -46,6 +46,12 @@ import { providerConsumesCallerAuthorization } from "../providers/caller-authori import { captureExplicitOpenAiCallerAuth } from "../providers/openai-sidecar"; import { captureCallerDirectAuth } from "../providers/caller-authorization"; import type { AdmissionLease } from "../lib/admission"; +import { + admissionModelDeniedResponse, + AdmissionModelDeniedError, + assertRouteAllowedByScope, + resolveAdmissionModelScope, +} from "./admission-model-scope"; import type { DataPlaneAdmission } from "./auth-cors"; import { tryClaimNativeMainProfileForTurn } from "../codex/native-main-admission"; import { @@ -150,6 +156,10 @@ async function handleChatCompletionsWithBudget( let chatNativeRoute: ReturnType | null = null; try { const route = routeModel(config, chatBody.model as string, evidenceFromBody(chatBody)); + // The native Chat lane sends without re-entering the Responses path, so it + // has to apply the key's scope itself. Translated traffic is checked where + // every rewrite converges instead. + assertRouteAllowedByScope(resolveAdmissionModelScope(config, logIds?.admission), requestedModel, route); // Preserve the routed destination for Go recognition, then settle the wire before // deriving protocol-scoped affinity. Recognition must not inspect the flipped adapter. const routedProvider = route.provider; @@ -182,6 +192,11 @@ async function handleChatCompletionsWithBudget( // effort, failover, and per-attempt telemetry run before any native Chat send. if (!route.combo && !effortRow && isNativeChatRouteEligible(route, chatBody, config)) chatNativeRoute = route; } catch (err) { + if (err instanceof AdmissionModelDeniedError) { + logCtx.requestedModel = requestedModel; + if (logIds) addFinalRequestLog(logIds.requestId, logIds.start, logCtx, 403, { closeReason: "non_stream" }); + return admissionModelDeniedResponse(err); + } if (err instanceof UnknownRoutingPolicyError) { logCtx.requestedModel = requestedModel; if (logIds) addFinalRequestLog(logIds.requestId, logIds.start, logCtx, 404, { closeReason: "non_stream" }); diff --git a/src/server/claude-messages.ts b/src/server/claude-messages.ts index b73199dd15a..b27ec73c91e 100644 --- a/src/server/claude-messages.ts +++ b/src/server/claude-messages.ts @@ -7,6 +7,12 @@ * unchanged. The Responses output (SSE or JSON) is converted back to Anthropic shape. */ import { FORWARD_HEADERS } from "../adapters/openai-responses"; +import { + admissionModelDeniedResponse, + AdmissionModelDeniedError, + assertRouteAllowedByScope, + resolveAdmissionModelScope, +} from "./admission-model-scope"; import { jsonUtf8Bytes } from "../lib/json-byte-size"; import { sseFieldValue } from "../lib/sse-decoder"; import { enforceAnthropicImageLimits, sniffImageDimensions } from "../adapters/anthropic-image-guard"; @@ -815,6 +821,13 @@ async function handleClaudeMessagesWithBudget( // verified live 2026-07-11). Strip them for that route; routed providers keep them. try { const route = routeModel(config, internalBody.model as string, evidenceFromBody(internalBody)); + // Same reason as the native Chat lane: this route can be sent from here, so + // the key's scope is applied before the wire is settled. + assertRouteAllowedByScope( + resolveAdmissionModelScope(config, logIds?.admission), + String(internalBody.model ?? ""), + route, + ); // Settle the wire once so the sampling decision below reads the effective // adapter rather than the provider-wide default (#404). route.staticPolicy = captureRouteStaticPolicy( @@ -847,6 +860,11 @@ async function handleClaudeMessagesWithBudget( if (ladder !== undefined && ladder.length === 0) delete internalBody.reasoning; } } catch (err) { + if (err instanceof AdmissionModelDeniedError) { + logCtx.requestedModel = requestedModel; + if (logIds) addFinalRequestLog(logIds.requestId, logIds.start, logCtx, 403, { closeReason: "non_stream" }); + return admissionModelDeniedResponse(err); + } if (err instanceof UnknownRoutingPolicyError) { logCtx.requestedModel = requestedModel; if (logIds) addFinalRequestLog(logIds.requestId, logIds.start, logCtx, 404, { closeReason: "non_stream" }); diff --git a/src/server/index/serve-options.ts b/src/server/index/serve-options.ts index 657f5f0b5b9..6d3ed873339 100644 --- a/src/server/index/serve-options.ts +++ b/src/server/index/serve-options.ts @@ -100,6 +100,7 @@ import { withCors, withManagementCors, } from "../auth-cors"; +import { resolveAdmissionModelScope, routeAllowedByScope } from "../admission-model-scope"; import { disableResponsesRequestTimeout, handleResponses, @@ -1101,6 +1102,17 @@ export function createServeOptions(ctx: ServeOptionsContext) { return disabledModels.has(id) ? [] : [{ id, metadataId }]; }) ); + // What a scoped key may see, filtered by the same predicate that refuses + // it on the data plane, so the catalog and the send path cannot disagree. + // This is a convenience, never the boundary: hiding a row only stops a + // client that reads the catalog first, which is why the refusal lives on + // the request path and this filter reuses it rather than replacing it. + // Filtering happens where the resolved provider and model are still in + // hand -- a published id is a selector, and re-resolving one here would + // re-run combo selection just to render a list. + const listScope = resolveAdmissionModelScope(config, admission); + const listAllows = (providerName: string, modelId: string): boolean => + routeAllowedByScope(listScope, { providerName, modelId }); // The projection is opt-in. Keep the default path free of Cursor install detection, // and resolve the bundle table once for the whole list rather than once per row. const effortRowsEnabled = config.cursorEffortRows === true; @@ -1133,7 +1145,9 @@ export function createServeOptions(ctx: ServeOptionsContext) { effortRowKnownIds, )); }; - const routedRows = await Promise.all(uniqueCatalogModelsForRawPublicList(goOrdered).map(async m => { + const routedRows = await Promise.all(uniqueCatalogModelsForRawPublicList(goOrdered) + .filter(m => listAllows(m.provider, m.id)) + .map(async m => { // Same rule as the anthropic branch: with the global fast switch on, a client // that has no Fast toggle is offered the fast identity directly. An operator // alias is an explicit decision and still wins. @@ -1180,8 +1194,12 @@ export function createServeOptions(ctx: ServeOptionsContext) { )); })); const data = [ - ...visibleNatives.flatMap(id => expandedNativeModelRow(id)), - ...visibleAccountNatives.flatMap(({ id, metadataId }) => expandedNativeModelRow(id, metadataId)), + ...visibleNatives + .filter(id => listAllows(OPENAI_CODEX_PROVIDER_ID, id)) + .flatMap(id => expandedNativeModelRow(id)), + ...visibleAccountNatives + .filter(({ metadataId }) => listAllows(OPENAI_CODEX_PROVIDER_ID, metadataId)) + .flatMap(({ id, metadataId }) => expandedNativeModelRow(id, metadataId)), ...routedRows.flat(), ]; return jsonResponse({ object: "list", data }, 200, req, policy); diff --git a/src/server/management/oauth-account-routes.ts b/src/server/management/oauth-account-routes.ts index 4cd52004665..0cb29ff8acd 100644 --- a/src/server/management/oauth-account-routes.ts +++ b/src/server/management/oauth-account-routes.ts @@ -869,6 +869,10 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise< name: k.name, prefix: k.key.slice(0, 17) + "...", createdAt: k.createdAt, + // Scope is metadata, not secret: an operator has to be able to read + // what a key may reach without minting a replacement to find out. + ...(k.allowedProviders ? { allowedProviders: [...k.allowedProviders] } : {}), + ...(k.allowedModels ? { allowedModels: [...k.allowedModels] } : {}), ...(k.pendingRotation ? { pendingRotation: { id: k.pendingRotation.id, createdAt: k.pendingRotation.createdAt, @@ -952,15 +956,46 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise< const body = await readJsonBody(req); if (!body) return jsonResponse({ error: "invalid body" }, 400, req, config); if (typeof body.id !== "string" || !body.id) return jsonResponse({ error: "id required" }, 400, req, config); - const nameField = validateKeyName(body.name, { required: true }); - if ("error" in nameField) return jsonResponse({ error: nameField.error }, 400, req, config); const entry = (config.apiKeys ?? []).find(k => k.id === body.id); if (!entry) return jsonResponse({ error: "key not found" }, 404, req, config); - entry.name = nameField.value; + // Rename and scope are independent edits. A scope-only PATCH must not have + // to restate the name, and a rename must not silently widen a scope, so + // each field is applied only when the caller actually sent it. + const renaming = body.name !== undefined; + const scopingProviders = body.allowedProviders !== undefined; + const scopingModels = body.allowedModels !== undefined; + if (!renaming && !scopingProviders && !scopingModels) { + return jsonResponse({ error: "name, allowedProviders or allowedModels required" }, 400, req, config); + } + if (renaming) { + const nameField = validateKeyName(body.name, { required: true }); + if ("error" in nameField) return jsonResponse({ error: nameField.error }, 400, req, config); + entry.name = nameField.value; + } + for (const [field, sent] of [["allowedProviders", scopingProviders], ["allowedModels", scopingModels]] as const) { + if (!sent) continue; + const value = body[field]; + // `null` and `[]` both clear the list back to unrestricted; anything else + // must be a list of non-empty strings, because a silently ignored malformed + // scope would read as "allowed everything" to whoever set it. + if (value === null) { delete entry[field]; continue; } + if (!Array.isArray(value) || value.some(item => typeof item !== "string" || !item.trim() || item.length > 256)) { + return jsonResponse({ error: `${field} must be a list of non-empty names` }, 400, req, config); + } + const normalized = [...new Set((value as string[]).map(item => item.trim()))]; + if (normalized.length === 0) delete entry[field]; + else entry[field] = normalized; + } saveConfigPreservingClaudeCode(config); reconcileLiveStateStores(); // Never echo key material from a rename. - return jsonResponse({ id: entry.id, name: entry.name, createdAt: entry.createdAt }, 200, req, config); + return jsonResponse({ + id: entry.id, + name: entry.name, + createdAt: entry.createdAt, + ...(entry.allowedProviders ? { allowedProviders: [...entry.allowedProviders] } : {}), + ...(entry.allowedModels ? { allowedModels: [...entry.allowedModels] } : {}), + }, 200, req, config); } if (url.pathname === "/api/keys" && req.method === "DELETE") { diff --git a/src/server/responses/compact.ts b/src/server/responses/compact.ts index 92e394751bc..89e40123789 100644 --- a/src/server/responses/compact.ts +++ b/src/server/responses/compact.ts @@ -1,4 +1,10 @@ import { capturePoolQuotaWriter } from "../../codex/account-store"; +import { + admissionModelDeniedResponse, + AdmissionModelDeniedError, + assertRouteAllowedByScope, + resolveAdmissionModelScope, +} from "../admission-model-scope"; import type { Server } from "bun"; import { bridgeToResponsesSSE, buildResponseJSON, formatErrorResponse, type ResponsesTerminalStatus } from "../../bridge"; import { @@ -655,7 +661,12 @@ export async function handleResponsesCompact( // routes ordinary turns elsewhere (#2901); the compaction-scoped router // may land that on the configured default provider instead of 404. route = routeCompactionModel(config, compactModel, evidenceFromBody(raw)); + // A compaction override picks the model, not the caller, so the key's scope + // is applied to what the override resolved to rather than to the selector + // the client sent. + assertRouteAllowedByScope(resolveAdmissionModelScope(config, admission), compactRequestedModel, route); } catch (err) { + if (err instanceof AdmissionModelDeniedError) return admissionModelDeniedResponse(err); if (err instanceof NoEligiblePolicyCandidateError) { // Persist the evaluation trace (per-candidate exclusions + the // no-eligible reason) so a failed compact policy request stays diff --git a/src/server/responses/request-prepare.ts b/src/server/responses/request-prepare.ts index 4d94d6a71ad..a96a672ada1 100644 --- a/src/server/responses/request-prepare.ts +++ b/src/server/responses/request-prepare.ts @@ -110,6 +110,13 @@ import { CODEX_RESERVE_OPT_IN_REQUIRED_MESSAGE, } from "../../codex/loopback-target"; import { checkComboTargetInputAdmission, checkInputAdmission } from "./input-admission"; +import { + admissionModelDeniedResponse, + AdmissionModelDeniedError, + assertRouteAllowedByScope, + resolveAdmissionModelScope, + routeAllowedByScope, +} from "../admission-model-scope"; import { nativeContextLimits } from "../../codex/catalog"; import { streamingContextOverflowResponse } from "./context-overflow"; import { @@ -411,7 +418,18 @@ export async function prepareResponsesRequest( let route: RouteResult; let credentialDomainWasRewritten = false; + // The selector the caller actually sent, captured before shadow interception + // or a subagent fallback rewrites it, so a refusal names the client's own + // request rather than a destination it never asked for. + const inboundSelector = parsed.modelId; + const admissionScope = resolveAdmissionModelScope(config, options.admission); const captureInboundRoutePolicy = (candidate: RouteResult): RouteResult => { + // Every route this request path produces passes through here: the direct + // name, an alias, a policy or combo selection, a compaction override, a + // shadow-intercept target and both subagent-fallback re-routes. Checking + // the key's scope at this one point is what stops a rewrite from reaching + // a destination the front door would have refused. + assertRouteAllowedByScope(admissionScope, inboundSelector, candidate); candidate.staticPolicy = captureRouteStaticPolicy( candidate.providerName, candidate.modelId, @@ -474,6 +492,7 @@ export async function prepareResponsesRequest( } logCtx.routeDecision = route.routeDecision; } catch (err) { + if (err instanceof AdmissionModelDeniedError) return admissionModelDeniedResponse(err); if (err instanceof NoAvailableComboTargetsError) { return comboUnavailable(err.comboId); } @@ -673,6 +692,7 @@ export async function prepareResponsesRequest( credentialDomainWasRewritten = true; logCtx.routeDecision = route.routeDecision; } catch (err) { + if (err instanceof AdmissionModelDeniedError) return admissionModelDeniedResponse(err); if (err instanceof NoAvailableComboTargetsError) { return comboUnavailable(err.comboId); } @@ -873,6 +893,7 @@ export async function prepareResponsesRequest( credentialDomainWasRewritten = true; logCtx.routeDecision = route.routeDecision; } catch (err) { + if (err instanceof AdmissionModelDeniedError) return admissionModelDeniedResponse(err); if (err instanceof NoAvailableComboTargetsError) { return comboUnavailable(err.comboId); } @@ -1021,6 +1042,14 @@ export async function prepareResponsesRequest( inboundTransport: options.inboundTransport, claudeGoAffinity: options.claudeGoAffinity, }); + // Normalization is the last thing that can move the destination: resolving an + // OpenAI virtual model rewrites route.modelId to the wire id that will + // actually be billed. A scope checked only before this would authorize the + // public selector and send the wire model, so the settled route is checked + // once more here. + if (!routeAllowedByScope(admissionScope, route)) { + return admissionModelDeniedResponse(new AdmissionModelDeniedError(inboundSelector, route)); + } // Attribute local auth/cooldown failures to the public selector too; exact auth may fail before // the normal post-resolution provider label is assigned. if (route.codexAccountNamespace) { diff --git a/src/server/search.ts b/src/server/search.ts index e66a51e24b7..6f9143ddaca 100644 --- a/src/server/search.ts +++ b/src/server/search.ts @@ -25,6 +25,12 @@ import { codexAccountNamespaceForModel } from "../codex/account-namespace-match" import { NATIVE_RESERVE_MODEL } from "../codex/catalog/native-models"; import { isCodexReserveRequestEligible } from "../codex/loopback-target"; import type { DataPlaneAdmission } from "./auth-cors"; +import { + admissionModelDeniedResponse, + AdmissionModelDeniedError, + resolveAdmissionModelScope, + routeAllowedByScope, +} from "./admission-model-scope"; import { formatCodexProviderForLog } from "../codex/routing"; import { signalWithTimeout } from "../lib/abort"; import { readBoundedResponseBytes } from "../lib/bounded-body"; @@ -87,6 +93,12 @@ export async function handleSearch( if (!route.codexAccountId || route.codexAccountNamespace !== accountNamespace) { return formatErrorResponse(400, "invalid_request_error", "Invalid Codex account-qualified search model"); } + // This branch resolves a model through the router and bills the account it + // names, so a scoped key is held to the same destination rule it is held + // to on the inference path. + if (!routeAllowedByScope(resolveAdmissionModelScope(config, admission), route)) { + return admissionModelDeniedResponse(new AdmissionModelDeniedError(model, route)); + } exactAccount = { accountId: route.codexAccountId, modelId: route.modelId }; logCtx.provider = `${route.providerName}-${accountNamespace}`; logCtx.routeDecision = route.routeDecision; diff --git a/src/types/config.ts b/src/types/config.ts index 977bdb266f6..bd6959ce5ab 100644 --- a/src/types/config.ts +++ b/src/types/config.ts @@ -253,6 +253,21 @@ export interface OcxApiKeyEntry { key: string; createdAt: string; pendingRotation?: OcxPendingApiKeyRotation; + /** + * Resolved provider names this key may reach. Absent or empty means every + * provider, which is what every existing key has, so adding the field + * changes nothing until an operator sets one. + */ + allowedProviders?: string[]; + /** + * Resolved destinations this key may reach, as a bare model id or a + * `provider/model` pair. Absent or empty means every model. + * + * These name destinations, not the selectors a client sends: they are + * checked after alias, combo, fallback and compaction resolution, because + * that is the only point at which the model about to be billed is known. + */ + allowedModels?: string[]; } export interface OcxPendingApiKeyRotation { diff --git a/tests/adapters/run-turn-queue.test.ts b/tests/adapters/run-turn-queue.test.ts index e983ab6a00a..b1f821d4818 100644 --- a/tests/adapters/run-turn-queue.test.ts +++ b/tests/adapters/run-turn-queue.test.ts @@ -1,5 +1,13 @@ import { describe, expect, test } from "bun:test"; -import { COALESCE_MAX_CHUNK_LENGTH, createAdapterEventQueue, PREFLIGHT_HEARTBEAT_RETAIN_LIMIT, preflightAdapterEvents } from "../../src/adapters/run-turn-queue"; +import { + COALESCE_MAX_CHUNK_LENGTH, + createAdapterEventQueue, + DEFAULT_MAX_BACKLOG_CODE_UNITS, + DEFAULT_MAX_EVENT_CODE_UNITS, + PREFLIGHT_HEARTBEAT_RETAIN_LIMIT, + preflightAdapterEvents, + retainedEventCodeUnits, +} from "../../src/adapters/run-turn-queue"; import type { AdapterEvent } from "../../src/types"; const text = (value: string): AdapterEvent => ({ type: "text_delta", text: value }); @@ -340,3 +348,173 @@ describe("run-turn adapter event preflight", () => { expect(cancelled).toBe(1); }); }); + +describe("run-turn adapter event queue retained-payload budgets", () => { + const BACKLOG_EXCEEDED = "consumer stalled: adapter event backlog exceeded — turn aborted"; + const EVENT_TOO_LARGE = "adapter event exceeds the single-event retained-string budget — turn aborted"; + + test("a stalled consumer is bounded by retained payload, not by the event count alone", async () => { + let backlogExceeded = 0; + const queue = createAdapterEventQueue({ + maxBacklogCodeUnits: 8, + maxEventCodeUnits: 8, + onBacklogExceeded: () => { backlogExceeded += 1; }, + }); + + // Each event on its own is within the per-event budget, so only the + // aggregate can be what refuses the second one. + expect(retainedEventCodeUnits(toolStart("0"))).toBeLessThanOrEqual(8); + queue.push(toolStart("0")); + queue.push(toolStart("1")); + + expect(backlogExceeded).toBe(1); + expect(await queue.collect()).toEqual([ + toolStart("0"), + { type: "error", message: BACKLOG_EXCEEDED }, + ]); + expect(queue.retainedCodeUnits()).toBe(0); + }); + + test("one oversized event is refused with its own cause even into an empty queue", async () => { + let backlogExceeded = 0; + const queue = createAdapterEventQueue({ + maxEventCodeUnits: 8, + onBacklogExceeded: () => { backlogExceeded += 1; }, + }); + + // The aggregate has its whole default budget free; this refusal is about + // the single event, and it has to say so rather than blame the consumer. + queue.push(text("x".repeat(9))); + + expect(backlogExceeded).toBe(1); + expect(await queue.collect()).toEqual([{ type: "error", message: EVENT_TOO_LARGE }]); + expect(queue.retainedCodeUnits()).toBe(0); + }); + + test("accumulated coalescing is charged by retained growth, not once per merged delta", async () => { + let backlogExceeded = 0; + const queue = createAdapterEventQueue({ + maxBacklogCodeUnits: 16, + onBacklogExceeded: () => { backlogExceeded += 1; }, + }); + + // Three 2-character deltas sharing one 10-character phase. Retained growth + // is 12 then +2 then +2; charging each whole event instead would bill 36 + // and abort a turn holding sixteen code units. + queue.push(phasedText("ab", "commentary")); + queue.push(phasedText("cd", "commentary")); + queue.push(phasedText("ef", "commentary")); + + expect(backlogExceeded).toBe(0); + expect(queue.retainedCodeUnits()).toBe(16); + queue.close(); + expect(await queue.collect()).toEqual([phasedText("abcdef", "commentary")]); + expect(queue.retainedCodeUnits()).toBe(0); + }); + + test("an event handed straight to a waiting consumer is never charged or capped", async () => { + let backlogExceeded = 0; + const queue = createAdapterEventQueue({ + maxBacklogCodeUnits: 4, + maxEventCodeUnits: 4, + onBacklogExceeded: () => { backlogExceeded += 1; }, + }); + const iterator = queue.stream()[Symbol.asyncIterator](); + + const pending = iterator.next(); + queue.push(text("x".repeat(1_000))); + + // The queue never held it, so neither budget has anything to say about it: + // both govern retained payload, and refusing this would abort a turn over + // memory the queue does not own. + expect(await pending).toEqual({ done: false, value: text("x".repeat(1_000)) }); + expect(backlogExceeded).toBe(0); + expect(queue.retainedCodeUnits()).toBe(0); + queue.close(); + }); + + test("a long synchronous burst well past one mebibyte still completes", async () => { + let backlogExceeded = 0; + const queue = createAdapterEventQueue({ + onBacklogExceeded: () => { backlogExceeded += 1; }, + }); + + // A synchronous producer legally fills the queue before its consumer is + // scheduled — the image loop does this with over a million one-character + // deltas, which coalesce into more than a mebibyte of retained text. A + // retained budget sized near that burst aborts healthy turns, so the + // default has to sit well above it. + const chunk = "x".repeat(64); + for (let i = 0; i < 20_000; i++) queue.push(text(chunk)); + queue.close(); + + const collected = await queue.collect(); + expect(backlogExceeded).toBe(0); + expect(collected.map(event => (event.type === "text_delta" ? event.text.length : 0)) + .reduce((sum, length) => sum + length, 0)).toBe(20_000 * 64); + expect(collected.every(event => event.type === "text_delta")).toBe(true); + expect(queue.retainedCodeUnits()).toBe(0); + }); + + test("draining after an abort releases exactly what was charged", async () => { + const queue = createAdapterEventQueue({ maxBacklogCodeUnits: 8, maxEventCodeUnits: 8 }); + + queue.push(toolStart("0")); + expect(queue.retainedCodeUnits()).toBeGreaterThan(0); + queue.push(toolStart("1")); + + // The terminal record is admitted past the budget it reports, and is then + // charged and released like any other item, so the counter lands on zero + // rather than on the size of an explanation nobody paid for. + const iterator = queue.stream()[Symbol.asyncIterator](); + expect(await iterator.next()).toEqual({ done: false, value: toolStart("0") }); + expect(await iterator.next()).toEqual({ done: false, value: { type: "error", message: BACKLOG_EXCEEDED } }); + expect(await iterator.next()).toEqual({ done: true, value: undefined }); + expect(queue.retainedCodeUnits()).toBe(0); + }); + + test("a cancel race leaves nothing charged behind", async () => { + const queue = createAdapterEventQueue({ maxBacklogCodeUnits: 64 }); + const iterator = queue.stream()[Symbol.asyncIterator](); + + queue.push(text("abcd")); + queue.push(thinking("wxyz")); + expect(await iterator.next()).toEqual({ done: false, value: text("abcd") }); + + // The consumer walks away mid-stream and the turn is closed underneath it. + await iterator.return?.(); + queue.close(); + queue.push(text("after close")); + + // What is still queued is still charged — and nothing more, so a second + // drain returns the counter to zero without a phantom balance. + expect(queue.retainedCodeUnits()).toBe(4); + expect(await queue.collect()).toEqual([thinking("wxyz")]); + expect(queue.retainedCodeUnits()).toBe(0); + }); + + test("the retention measure counts payload strings and skips the discriminant", () => { + expect(retainedEventCodeUnits(text("abcd"))).toBe(4); + expect(retainedEventCodeUnits(heartbeat)).toBe(0); + expect(retainedEventCodeUnits(phasedText("ab", "commentary"))).toBe(12); + // A malformed adapter emission has to become a terminal event, not a + // TypeError thrown out of push() with the queue half-updated. + expect(retainedEventCodeUnits(null as unknown as AdapterEvent)).toBe(0); + expect(retainedEventCodeUnits("oops" as unknown as AdapterEvent)).toBe(0); + // Nested provider-shaped payload is counted; a cycle terminates. + const cyclic: Record = { owner: "abc" }; + cyclic.self = cyclic; + expect(retainedEventCodeUnits({ type: "done", providerState: cyclic } as unknown as AdapterEvent)).toBe(3); + }); + + test("both budgets must be positive safe integers", () => { + const invalid = [Number.NaN, Number.POSITIVE_INFINITY, 0, -4, 2.5, Number.MAX_SAFE_INTEGER + 1]; + for (const value of invalid) { + expect(() => createAdapterEventQueue({ maxBacklogCodeUnits: value })) + .toThrow("maxBacklogCodeUnits must be a positive safe integer"); + expect(() => createAdapterEventQueue({ maxEventCodeUnits: value })) + .toThrow("maxEventCodeUnits must be a positive safe integer"); + } + expect(DEFAULT_MAX_EVENT_CODE_UNITS).toBeLessThan(DEFAULT_MAX_BACKLOG_CODE_UNITS); + }); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index d8a39f76b8b..308682ccebc 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1398,5 +1398,7 @@ "gui-codex-usage-score-parity.test.ts": "gui", "web-search-sidecar-429.test.ts": "web-search", "codex-shim-destroyed-probe.test.ts": "codex-integration", - "client-runtime.test.ts": "clients" + "client-runtime.test.ts": "clients", + "devin-output-budget.test.ts": "providers", + "api-key-model-scope.test.ts": "server" } diff --git a/tests/oauth/oauth-provider-reconcile.test.ts b/tests/oauth/oauth-provider-reconcile.test.ts index fe624a54f70..8e8a1b67afd 100644 --- a/tests/oauth/oauth-provider-reconcile.test.ts +++ b/tests/oauth/oauth-provider-reconcile.test.ts @@ -504,6 +504,43 @@ describe("OAuth provider reconciliation", () => { expect(config.providers.kimi.requiresReasoningPlaceholderModels).toEqual([]); }); + test("preserves an operator output budget that no OAuth preset declares", () => { + // The devin preset declares neither output field, so the delete branch was + // the only branch either of them ever took: a hand-edited budget was gone + // before the next startup finished, leaving the adapter's own fallback as + // the only reachable cap (#5190). Reconciliation still owns the input-side + // catalog, which is why modelContextWindows is asserted alongside. + const preset = OAUTH_PROVIDERS.devin.providerConfig; + expect(preset.defaultMaxOutputTokens).toBeUndefined(); + expect(preset.modelMaxOutputTokens).toBeUndefined(); + const config = { + port: 10100, + defaultProvider: "devin", + googleAntigravityStaticCatalogVersion: 1, + providers: { + devin: { + ...structuredClone(preset), + defaultMaxOutputTokens: 64_000, + modelMaxOutputTokens: { "swe-2": 32_000 }, + modelContextWindows: { "swe-2": 1 }, + }, + }, + } satisfies OcxConfig; + + reconcileOAuthProviders(config, false); + + expect(config.providers.devin.defaultMaxOutputTokens).toBe(64_000); + expect(config.providers.devin.modelMaxOutputTokens).toEqual({ "swe-2": 32_000 }); + expect(config.providers.devin.modelContextWindows).toEqual(preset.modelContextWindows!); + + // A preset that does declare a budget still refreshes the saved row: this + // is a narrower delete branch, not an exemption from reconciliation. + config.providers.devin.defaultMaxOutputTokens = 1; + reconcileOAuthProviders(config, false); + expect(config.providers.devin.defaultMaxOutputTokens).toBe(1); + expect(OAUTH_PROVIDERS.anthropic.providerConfig.defaultMaxOutputTokens).toBeGreaterThan(0); + }); + test("refreshes Grok 4.6 levels while runtime fills the default without overwriting user intent", () => { const home = mkdtempSync(join(tmpdir(), "ocx-grok-46-reconcile-")); homes.push(home); diff --git a/tests/providers/codebuddy-adapter.test.ts b/tests/providers/codebuddy-adapter.test.ts index ea877f4a3eb..f68322752c5 100644 --- a/tests/providers/codebuddy-adapter.test.ts +++ b/tests/providers/codebuddy-adapter.test.ts @@ -252,6 +252,37 @@ describe("codebuddy runTurn streams a headless turn", () => { expect(JSON.stringify(events)).not.toContain("secret-command"); }); + test("the projected history ceiling follows the model context window", async () => { + const stdout = [ + enc.encode('{"type":"system","subtype":"init"}\n'), + enc.encode('{"type":"result","subtype":"success","is_error":false,"usage":{"input_tokens":7,"output_tokens":2}}\n'), + ]; + const history = Array.from({ length: 5 }, (_, index) => ({ + role: "user" as const, + content: "EARLY-MARKER-" + String(index) + " " + "a".repeat(50_000), + timestamp: index, + })); + const messages = [...history, { role: "user", content: "final request", timestamp: 5 }]; + + const wide = fakeChild(stdout); + const wideAdapter = createCodeBuddyAdapter( + provider({ modelContextWindows: { "glm-5.3": 1_000_000 } }), + { spawn: () => wide as unknown as ChildProcess, which: () => "/usr/bin/codebuddy", killGraceMs: 20 }, + ); + await run(wideAdapter, parsed({ context: { messages } })); + expect(wide.written.join("")).toContain("EARLY-MARKER-0"); + expect(wide.written.join("")).not.toContain("truncated for length"); + + const flat = fakeChild(stdout); + const flatAdapter = createCodeBuddyAdapter( + provider(), + { spawn: () => flat as unknown as ChildProcess, which: () => "/usr/bin/codebuddy", killGraceMs: 20 }, + ); + await run(flatAdapter, parsed({ context: { messages } })); + expect(flat.written.join("")).not.toContain("EARLY-MARKER-0"); + expect(flat.written.join("")).toContain("truncated for length"); + }); + test.each(["Bash", "exec", "shell", "apply_patch"])("refuses a bare %s DSML invoke", name => { const events: AdapterEvent[] = []; const guarded = guardCodeBuddyScaffolding(event => events.push(event)); diff --git a/tests/providers/codebuddy-protocol.test.ts b/tests/providers/codebuddy-protocol.test.ts index a51cb1ea7b4..5c3b4c8db29 100644 --- a/tests/providers/codebuddy-protocol.test.ts +++ b/tests/providers/codebuddy-protocol.test.ts @@ -1,9 +1,11 @@ import { describe, expect, test } from "bun:test"; import { + MAX_PROJECTED_HISTORY_CHARS, buildConversationInput, buildInputLines, buildSystemPrompt, mapStreamMessageToEvents, + projectedHistoryCharLimit, readJsonLines, usageFromResult, } from "../../src/adapters/coding-agent/protocol"; @@ -352,3 +354,41 @@ describe("codebuddy conversation input builder (Strategy C projection)", () => { }); }); }); +describe("projected history ceiling derives from the model context window", () => { + test("absent or invalid window metadata keeps the legacy flat cap", () => { + expect(projectedHistoryCharLimit(undefined)).toBe(MAX_PROJECTED_HISTORY_CHARS); + expect(projectedHistoryCharLimit(Number.NaN)).toBe(MAX_PROJECTED_HISTORY_CHARS); + expect(projectedHistoryCharLimit(0)).toBe(MAX_PROJECTED_HISTORY_CHARS); + expect(projectedHistoryCharLimit(-1)).toBe(MAX_PROJECTED_HISTORY_CHARS); + }); + + test("a small window never lowers the cap below the legacy default", () => { + expect(projectedHistoryCharLimit(64_000)).toBe(MAX_PROJECTED_HISTORY_CHARS); + }); + + test("a large window scales the cap until the hard ceiling", () => { + expect(projectedHistoryCharLimit(128_000)).toBe(384_000); + expect(projectedHistoryCharLimit(1_000_000)).toBe(3_000_000); + expect(projectedHistoryCharLimit(Number.MAX_SAFE_INTEGER)).toBe(4_000_000); + }); + + test("buildConversationInput keeps the history a derived ceiling admits", () => { + const history = Array.from({ length: 5 }, (_, index) => ({ + role: "user" as const, + content: "EARLY-MARKER-" + String(index) + " " + "a".repeat(50_000), + timestamp: index, + })); + const messages = [...history, { role: "user", content: "current request", timestamp: 5 }]; + const parsed = parsedRequest({ context: { messages } }); + + const wide = buildConversationInput(parsed, { maxHistoryChars: projectedHistoryCharLimit(1_000_000) }) + .map(line => JSON.parse(line)); + expect(wide[0].message.content[0].text).toContain("EARLY-MARKER-0"); + expect(wide[0].message.content[0].text).not.toContain("truncated for length"); + + const flat = buildConversationInput(parsed).map(line => JSON.parse(line)); + expect(flat[0].message.content[0].text).not.toContain("EARLY-MARKER-0"); + expect(flat[0].message.content[0].text).toContain("truncated for length"); + expect(flat[0].message.content[0].text).toContain("current request"); + }); +}); diff --git a/tests/providers/devin-output-budget.test.ts b/tests/providers/devin-output-budget.test.ts new file mode 100644 index 00000000000..ebd0b277259 --- /dev/null +++ b/tests/providers/devin-output-budget.test.ts @@ -0,0 +1,183 @@ +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { createDevinAdapter, resolveDevinMaxOutputTokensForTests } from "../../src/adapters/devin"; +import { parseCatalogBuffer, setCachedCatalogForTests } from "../../src/adapters/devin/cloud-direct/catalog"; +import { devinCacheIdentity, invalidateSessionIdentity } from "../../src/adapters/devin/cloud-direct/chat"; +import { encodeMessage, encodeString, encodeVarintField, iterFields } from "../../src/adapters/devin/cloud-direct/wire"; +import { createTranslatorBudget } from "../../src/lib/translator-budget"; +import type { AdapterEvent, OcxParsedRequest, OcxProviderConfig } from "../../src/types"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +/** + * The output budget a Devin turn actually sends. + * + * CompletionConfiguration #2 is the output cap and #3 is the context window. + * Every assertion below reads both, because the defect these guard against is + * not "the number is wrong" but "the two meanings were collapsed into one": + * a caller that names no cap has to reach the configured output budget without + * the context window leaking into the field that decides how long the answer + * may run. + */ +describe("Devin output budget on the wire", () => { + const apiKey = "ocx-devin-output-fixture"; + const host = "https://server.codeium.com"; + const previousHome = process.env.OPENCODEX_HOME; + const previousFetch = globalThis.fetch; + let home = ""; + let requests: Buffer[] = []; + + function frame(body: Buffer, flags = 0): Buffer { + const header = Buffer.alloc(5); + header[0] = flags; + header.writeUInt32BE(body.length, 1); + return Buffer.concat([header, body]); + } + function fields(buf: Buffer) { + return new Map([...iterFields(buf)].map(field => [field.num, field])); + } + function seed(rows: Array<{ uid: string; window?: number }>): void { + const buffer = Buffer.concat(rows.map(row => encodeMessage(1, Buffer.concat([ + encodeString(1, row.uid), + encodeString(22, row.uid), + ...(row.window === undefined ? [] : [encodeVarintField(18, row.window)]), + encodeVarintField(4, 0), + ])))); + setCachedCatalogForTests(parseCatalogBuffer(buffer, apiKey, host)); + } + async function run( + provider: Partial = {}, + options: OcxParsedRequest["options"] = {}, + modelId = "swe-2-high", + ): Promise { + const adapter = createDevinAdapter({ ...provider, adapter: "devin", apiKey, baseUrl: host }); + const events: AdapterEvent[] = []; + await adapter.runTurn!({ + modelId, stream: true, + context: { messages: [{ role: "user", content: "hi", timestamp: 1 }] }, + options, + }, { headers: new Headers(), translatorBudget: createTranslatorBudget() }, + event => { events.push(event); }); + return events; + } + /** The completion configuration the one captured turn actually encoded. */ + function sentCompletion(): { output: bigint; context: bigint } { + expect(requests).toHaveLength(1); + const completion = fields(fields(requests[0]!).get(8)!.value as Buffer); + return { output: completion.get(2)!.value as bigint, context: completion.get(3)!.value as bigint }; + } + + beforeEach(() => { + home = mkdtempSync(join(tmpdir(), "ocx-devin-output-")); + process.env.OPENCODEX_HOME = home; + requests = []; + setCachedCatalogForTests(null); + seed([{ uid: "swe-2-high", window: 262_000 }, { uid: "swe-2-max", window: 1_000_000 }]); + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = String(input); + if (!url.endsWith("/GetChatMessage")) return new Response("unavailable", { status: 503 }); + requests.push(Buffer.from(await (init!.body as Blob).arrayBuffer()).subarray(5)); + return new Response(Buffer.concat([ + frame(Buffer.concat([encodeString(3, "ok"), encodeVarintField(5, 2)])), + frame(Buffer.from("{}"), 2), + ]), { headers: { "content-type": "application/connect+proto" } }); + }) as typeof fetch; + }); + afterEach(() => { + globalThis.fetch = previousFetch; + setCachedCatalogForTests(null); + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + invalidateSessionIdentity(devinCacheIdentity(apiKey, host)); + removeTreeWithRetry(home); + }); + + test("a caller that names no cap still reaches the configured provider budget", async () => { + // Codex never sends max_output_tokens, so this is the path every real turn + // takes. Before the budget was wired the encoder's 8192 was the only cap a + // devin turn could ever have, whatever the operator configured. + const events = await run({ defaultMaxOutputTokens: 64_000 }); + expect(events.some(event => event.type === "error")).toBe(false); + expect(sentCompletion().output).toBe(64_000n); + }); + + test("a per-model budget outranks the provider default and follows the selected variant", async () => { + await run({ + defaultMaxOutputTokens: 64_000, + modelMaxOutputTokens: { "swe-2-high": 32_000, "swe-2-max": 100_000 }, + }); + expect(sentCompletion().output).toBe(32_000n); + }); + + test("an explicit small cap survives a much larger configured budget", async () => { + await run( + { defaultMaxOutputTokens: 64_000, modelMaxOutputTokens: { "swe-2-high": 32_000 } }, + { maxOutputTokens: 64 }, + ); + expect(sentCompletion().output).toBe(64n); + }); + + test("the context window never becomes the output cap", async () => { + // The whole provider row describes input size and nothing else. The output + // field has to stay on the encoder default rather than inherit 262k, which + // would ask Cognition to generate an entire context window of tokens. + await run({ contextWindow: 200_000, modelContextWindows: { "swe-2-high": 180_000 } }); + const sent = sentCompletion(); + expect(sent.output).toBe(8192n); + expect(sent.context).toBe(180_000n); + }); + + test("a configured output budget does not disturb the input ceiling", async () => { + await run({ defaultMaxOutputTokens: 64_000 }); + expect(sentCompletion().context).toBe(262_000n); + }); +}); + +describe("Devin output budget resolution", () => { + const provider: Partial = { + adapter: "devin", + defaultMaxOutputTokens: 64_000, + modelMaxOutputTokens: { "swe-2-high": 32_000 }, + }; + + test("an explicit caller value is forwarded unchanged", () => { + expect(resolveDevinMaxOutputTokensForTests(provider as OcxProviderConfig, "swe-2-high", 64)).toBe(64); + }); + + test("an unconfigured provider leaves the encoder default in place", () => { + expect(resolveDevinMaxOutputTokensForTests({ adapter: "devin" } as OcxProviderConfig, "swe-2-high", undefined)) + .toBeUndefined(); + }); + + test("a dotted or case-folded saved hint still matches the selected uid", () => { + expect(resolveDevinMaxOutputTokensForTests( + { adapter: "devin", modelMaxOutputTokens: { "SWE.2-HIGH": 24_000 } } as OcxProviderConfig, + "swe-2-high", undefined, + )).toBe(24_000); + }); + + test("another variant's budget is never borrowed", () => { + expect(resolveDevinMaxOutputTokensForTests( + { adapter: "devin", modelMaxOutputTokens: { "swe-2-max": 100_000 } } as OcxProviderConfig, + "swe-2-high", undefined, + )).toBeUndefined(); + }); + + test.each([Number.NaN, Number.POSITIVE_INFINITY, 0, -1, 1.5, Number.MAX_SAFE_INTEGER + 1])( + "invalid configured metadata %p is ignored rather than encoded", + invalid => { + expect(resolveDevinMaxOutputTokensForTests( + { adapter: "devin", defaultMaxOutputTokens: invalid, modelMaxOutputTokens: { "swe-2": invalid } } as OcxProviderConfig, + "swe-2-high", undefined, + )).toBeUndefined(); + }, + ); + + test("the context window is not an output budget", () => { + expect(resolveDevinMaxOutputTokensForTests( + { adapter: "devin", contextWindow: 200_000, modelContextWindows: { "swe-2-high": 180_000 } } as OcxProviderConfig, + "swe-2-high", undefined, + )).toBeUndefined(); + }); +}); diff --git a/tests/server/api-key-model-scope.test.ts b/tests/server/api-key-model-scope.test.ts new file mode 100644 index 00000000000..51e45b7c8c6 --- /dev/null +++ b/tests/server/api-key-model-scope.test.ts @@ -0,0 +1,159 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +// The schema this exercises is reached through `src/config`, the entry the +// runtime evaluates first. Importing `config/schema/*` directly enters that +// module cycle from the wrong end and throws a TDZ ReferenceError. +import { loadConfig } from "../../src/config"; +import { + admissionModelDeniedBody, + AdmissionModelDeniedError, + admissionModelDeniedResponse, + admissionModelScopeOf, + assertRouteAllowedByScope, + MODEL_NOT_ALLOWED_FOR_KEY, + resolveAdmissionModelScope, + routeAllowedByScope, +} from "../../src/server/admission-model-scope"; +import { routeModel } from "../../src/router"; +import type { OcxConfig } from "../../src/types"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +const KEY = "ocx_data_" + "a".repeat(40); +const OTHER_KEY = "ocx_data_" + "b".repeat(40); + +const previousHome = process.env.OPENCODEX_HOME; +const homes: string[] = []; + +afterEach(() => { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + for (const home of homes.splice(0)) removeTreeWithRetry(home); +}); + +/** Load a hand-written config.json the way a startup would, and report the keys that survived. */ +function loadKeysFrom(apiKeys: unknown[]): string[] { + const home = mkdtempSync(join(tmpdir(), "ocx-key-scope-")); + homes.push(home); + process.env.OPENCODEX_HOME = home; + writeFileSync(join(home, "config.json"), JSON.stringify({ + port: 10100, + defaultProvider: "allowed", + providers: { allowed: { adapter: "openai-chat", baseUrl: "https://allowed.test/v1", models: ["small"] } }, + apiKeys, + })); + return (loadConfig().apiKeys ?? []).map(entry => entry.name); +} + +function configWithKey(scope: { allowedProviders?: string[]; allowedModels?: string[] }): Pick { + return { + apiKeys: [ + { id: "scoped", name: "mail", key: KEY, createdAt: "2026-01-01T00:00:00.000Z", ...scope }, + { id: "open", name: "coding", key: OTHER_KEY, createdAt: "2026-01-01T00:00:00.000Z" }, + ], + }; +} + +describe("per-key model and provider scope", () => { + test("a key with neither list is unrestricted", () => { + expect(admissionModelScopeOf({})).toBeUndefined(); + expect(admissionModelScopeOf({ allowedProviders: [], allowedModels: [] })).toBeUndefined(); + expect(routeAllowedByScope(undefined, { providerName: "xai", modelId: "grok-4.6" })).toBe(true); + }); + + test("only a configured key carries a scope", () => { + const config = configWithKey({ allowedProviders: ["zai-discount"] }); + expect(resolveAdmissionModelScope(config, { kind: "configured", keyId: "scoped", source: "bearer" })) + .toEqual({ providers: ["zai-discount"], models: [] }); + // The environment token and loopback have no stored record to attach a + // scope to, so narrowing them would be inventing a policy nobody wrote. + expect(resolveAdmissionModelScope(config, { kind: "environment", source: "bearer" })).toBeUndefined(); + expect(resolveAdmissionModelScope(config, { kind: "loopback", source: "loopback" })).toBeUndefined(); + expect(resolveAdmissionModelScope(config, undefined)).toBeUndefined(); + expect(resolveAdmissionModelScope(config, { kind: "configured", keyId: "open", source: "bearer" })).toBeUndefined(); + }); + + test("the two lists are independent conditions and both must hold", () => { + const scope = admissionModelScopeOf({ allowedProviders: ["zai-discount"], allowedModels: ["glm-5.3-flash"] })!; + expect(routeAllowedByScope(scope, { providerName: "zai-discount", modelId: "glm-5.3-flash" })).toBe(true); + // The allowed model on a forbidden provider is exactly what a combo child or + // a policy fallback reaches while the requested selector never changes. + expect(routeAllowedByScope(scope, { providerName: "openrouter", modelId: "glm-5.3-flash" })).toBe(false); + // The allowed provider carrying a forbidden model is the mirror case. + expect(routeAllowedByScope(scope, { providerName: "zai-discount", modelId: "grok-4.6" })).toBe(false); + }); + + test("a model entry matches bare or fully qualified, and folds case and spacing", () => { + const scope = admissionModelScopeOf({ allowedModels: [" ZAI-Discount/GLM-5.3-Flash "] })!; + expect(routeAllowedByScope(scope, { providerName: "zai-discount", modelId: "glm-5.3-flash" })).toBe(true); + // Pinned to that provider: the same model elsewhere is a different destination. + expect(routeAllowedByScope(scope, { providerName: "openrouter", modelId: "glm-5.3-flash" })).toBe(false); + const bare = admissionModelScopeOf({ allowedModels: ["glm-5.3-flash"] })!; + expect(routeAllowedByScope(bare, { providerName: "openrouter", modelId: "glm-5.3-flash" })).toBe(true); + }); + + test("an alias is judged by what it resolves to, not by the name the client sent", () => { + // The defect this guards: a bare alias names neither the provider nor the + // model, so a scope checked against the caller's string has nothing to + // match on and would authorize a destination it never saw. + const config = { + port: 10100, + defaultProvider: "allowed", + providers: { + allowed: { adapter: "openai-chat", baseUrl: "https://allowed.test/v1", models: ["small"] }, + forbidden: { + adapter: "openai-chat", baseUrl: "https://forbidden.test/v1", + models: ["expensive"], modelAliases: { expensive: "cheap" }, + }, + }, + } as unknown as OcxConfig; + const scope = admissionModelScopeOf({ allowedProviders: ["allowed"] })!; + + const routed = routeModel(config, "cheap"); + expect(routed).toMatchObject({ providerName: "forbidden", modelId: "expensive" }); + expect(routeAllowedByScope(scope, routed)).toBe(false); + expect(() => assertRouteAllowedByScope(scope, "cheap", routed)).toThrow(AdmissionModelDeniedError); + + const permitted = routeModel(config, "allowed/small"); + expect(() => assertRouteAllowedByScope(scope, "allowed/small", permitted)).not.toThrow(); + }); + + test("a refusal is 403 and names the caller's own selector", () => { + const error = new AdmissionModelDeniedError("gldf-flash", { providerName: "xai", modelId: "grok-4.6" }); + const body = admissionModelDeniedBody(error); + expect(body.error.type).toBe(MODEL_NOT_ALLOWED_FOR_KEY); + // The client learns which of its own requests was refused. A key that may + // not reach xai has no business learning that its alias points there. + expect(body.error.model).toBe("gldf-flash"); + expect(JSON.stringify(body)).not.toContain("grok-4.6"); + const response = admissionModelDeniedResponse(error); + // 403, not 404: the key authenticated and simply may not go there, and a + // 404 would tell the client its credential is wrong and invite a retry. + expect(response.status).toBe(403); + }); + + test("a malformed scope drops the key instead of widening it", () => { + const scoped = { key: KEY, id: "scoped", name: "mail", createdAt: "2026-01-01T00:00:00.000Z", allowedProviders: ["zai-discount"] }; + const open = { key: OTHER_KEY, id: "open", name: "coding", createdAt: "2026-01-01T00:00:00.000Z" }; + expect(loadKeysFrom([scoped, open])).toEqual(["mail", "coding"]); + + // Every other field on this record degrades to a default. These two must + // not: degrading a damaged permission field reads as "allowed everything", + // which is the one direction it can never fail. The damaged key is dropped + // and its still-valid neighbour survives, so one bad record does not take + // the whole array with it. + for (const damaged of [ + { ...scoped, allowedProviders: "zai-discount" }, + { ...scoped, allowedProviders: [""] }, + { ...scoped, allowedModels: [123] }, + { ...scoped, allowedModels: ["x".repeat(257)] }, + ]) { + expect(loadKeysFrom([damaged, open])).toEqual(["coding"]); + } + + // A degrading neighbour still degrades, so the fail-closed choice is scoped + // to the permission fields rather than hardening the whole record. + expect(loadKeysFrom([{ ...scoped, name: 7 }, open])).toHaveLength(2); + }); +});