diff --git a/devlog/_plan/260927_merge_train_3/050_batch5.md b/devlog/_plan/260927_merge_train_3/050_batch5.md new file mode 100644 index 00000000000..aaef42bfb9f --- /dev/null +++ b/devlog/_plan/260927_merge_train_3/050_batch5.md @@ -0,0 +1,55 @@ +# B5 — narrowed carries, a Command Code retry default, and evidence-backed closes + +Base: `dev` `4b3737fc5c` (after B4 #6063). Branch `codex/train3-b5`. + +Previous D (B4): #6043 landed; #6044 held on its security blocker, #6051 on the owner's `.agents/` decision. The +coordinator asked the lane to continue until nothing in scope is landable. + +| Item | Plan | +|---|---| +| #5953 (codingbooo) → #5465 | Carry, then narrow `protectGlmSummaryBudget` as the maintainer round asked: Z.AI host only (from the provider base URL), caller effort `high`/`max` only, the checkpoint shape (summary instruction plus a `` transcript of at least 2000 characters), and each tiny cap field (≤1024) raised on its own to 8192. Negative tests for each boundary. | +| #5180 (issue, found through Aside) | With no `retryOn429` knob, a key-auth Command Code destination gets the patient same-key policy OpenCode Go already has, so a burst 429 on a long turn waits (honoring Retry-After) instead of failing to the client. An explicit `retryOn429`, including `enabled: false`, still wins; OAuth is never replayed. | +| #6027 (codingbooo) → #5569 | Carry, then fix the owner's three blockers: replace only when the whole body carries exactly one `` block (otherwise pass through); store a new snapshot only after `prepareResponsesRequest` reaches its success return, so a rejected first request pins nothing; share a snapshot without a principal only for loopback admission. Tests for each. | +| #4055 | Close as fixed for the reported Tailscale Serve case (12-hour identity sessions with sliding renewal, #2776), and correct the stale `management-api.md` sentence that says remote binds never get a session. | +| #3433 | Close with evidence: per-conversation identifiers are preserved at the forward boundary (#4365) and the managed Hermes export now sends one (#5742); 26 pinned tests pass. | + +Held: #6030 (draft; launchd PATH adoption drops non-PATH changes, two ratchet breaches, WinSW gap, conflict), +#4143 (needs the reporter's desktop routing details). + +## Audit (Kimi, NEAR-PASS) and folded decisions + +- #5180: the predicate is key auth plus the existing `isCanonicalCommandCodeBaseUrl`; a row repointed at a custom + relay keeps fail-fast unless `retryOn429` is set. +- #6027: `snapshotSkillsCatalogInBody` splits into a replace-only lookup before parsing and a store call at the + success return. Residuals named in the PR: a snapshot can be pinned by a turn whose upstream request later fails; + on a no-auth server bound beyond loopback, snapshots are keyed by conversation id alone, matching that server's + trust model. +- #5953: the gate reads effective effort after combo overrides; transcript shapes that #5465 does not show stay + unprotected, which is today's behavior. + +## Build and evidence + +| Commit | What | +|---|---| +| `4fda15d338` | #5180: canonical key-auth Command Code gets the patient same-key 429 policy; test fails without the fix | +| `3876151d01` | #5953 carried | +| `fb3faab205` | #5953 narrowed: Z.AI host, effective `high`/`max`, checkpoint transcript ≥ 2000 characters, per-field cap raise to 8192; negative tests per boundary | +| `25e01a8e2b` | #6027 carried (layout registries unioned, entry kept on an existing line) | +| `01b7e24d2d` | #6027 blockers: one-block rule, store at the success return, anonymous sharing only on a no-auth server; three tests that fail on the PR head | +| `ad3b374820`, `2256d097e6` | `management-api.md` describes remote dashboard sessions as shipped (#4055) | + +Closed during this cycle with evidence: #3433 (identifiers preserved at the forward boundary, #4365; managed Hermes +sends one, #5742; 26 pinned tests pass). + +Aside: #5953 shows two CHANGES_REQUESTED reviews (the overbreadth this batch narrows); #6027 shows the owner's +three-blocker review this batch answers; #5465, #5569 and #5180 pages captured. One Aside capture failed once with a +daemon `Aside.controlTab` error after an Aside update and succeeded on retry. + +Local proof at `2256d097e6`: typecheck, structure and privacy exit 0; six focused files 60 pass; `tests/adapters/openai` +591 pass; eight request-preparation files 108 pass. The full `tests/responses` directory shows 13 failures in +`responses-compaction-recovery.test.ts` that do not reproduce when that file runs alone (33 pass on this branch and on +`dev`); the same directory run on `dev` is recorded below. + +The full `tests/responses` run on `dev` `4b3737fc5c` shows the same 13 `responses-compaction-recovery` failures +(3567 pass, 13 fail), so they are cross-file interference in a directory run, not this batch; the branch run was 3576 +pass, 13 fail. diff --git a/docs-site/src/content/docs/guides/codex-prompt.md b/docs-site/src/content/docs/guides/codex-prompt.md index df6c5a770f8..9a4e776cc91 100644 --- a/docs-site/src/content/docs/guides/codex-prompt.md +++ b/docs-site/src/content/docs/guides/codex-prompt.md @@ -169,6 +169,37 @@ repair writes a backup before it touches anything. Changes apply to newly started sessions. A session already running keeps the prompt settings it started with. +## Keeping the skills catalog stable + +The proxy defaults to `skills.catalog_refresh: "per_session"`: the first +`` catalog received for a conversation is reused on later +requests in that conversation. This keeps skill discovery and `SKILL.md` edits +from changing that part of the upstream prompt cache prefix mid-session. +A request that carries more than one `` block is passed +through unchanged, and a request the proxy rejects does not set the catalog. + +To use the catalog supplied by the client on every turn, set this in opencodex's +`$OPENCODEX_HOME/config.json` (normally `~/.opencodex/config.json`), then restart +the proxy: + +```json +{ + "skills": { + "catalog_refresh": "per_turn" + } +} +``` + +The supported values are `"per_session"` (default) and `"per_turn"`. This is a +proxy setting, separate from Codex's `skills.include_instructions` toggle. +Requests without a reliable conversation identity use the catalog supplied by +the client. Snapshots are held in memory and do not survive a proxy restart. +They expire after four hours of inactivity and may be evicted when the bounded +cache fills. An initial catalog block larger than 512 KiB is forwarded without caching. +After expiry or eviction, the next received catalog becomes the new snapshot. +The dashboard's prompt preview still reads the current files; it does not show +the snapshot retained for an ongoing conversation. + ## What this page reads, and what it does not opencodex reads one configuration file — your `config.toml`. Codex resolves its diff --git a/docs-site/src/content/docs/reference/configuration/agents.md b/docs-site/src/content/docs/reference/configuration/agents.md index ac968ee7073..20165746900 100644 --- a/docs-site/src/content/docs/reference/configuration/agents.md +++ b/docs-site/src/content/docs/reference/configuration/agents.md @@ -8,6 +8,15 @@ routes, and limits delegated work. ## Agent fields +### Skills catalog refresh + +`skills.catalog_refresh` accepts `"per_session"` (the default) or `"per_turn"` +in opencodex's `config.json`. Session mode reuses the first received skills +instructions for a conversation, protecting the prompt cache prefix from catalog +changes between turns. Turn mode forwards the client's current catalog. +See [Keeping the skills catalog stable](/guides/codex-prompt/#keeping-the-skills-catalog-stable) +for configuration and snapshot lifetime details. + ### Astra roster upgrade On the first start after upgrading, existing `subagentModels` lists receive diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index c66261b0425..a440ec05c02 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -267,7 +267,7 @@ Providers can expose a built-in shorthand, such as `agy` for `google-antigravity | `responsesItemIdRepair?` | `{ message?: string[]; reasoning?: string[]; repairMissingTerminalIds?: boolean; repairInvalidIds?: boolean }` | Disabled-by-default downstream SSE repair for exact placeholder ids, missing terminal ids, and (with `repairInvalidIds`) message/reasoning ids missing the canonical `msg_`/`rs_` prefix. Function-call ids are never rewritten. Built-in DeepSeek enables the last two by default. | | `responsesSnapshotRepair?` | `boolean` | Disabled-by-default client-facing repair for sparse Responses lifecycle snapshots in SSE and JSON. Fills missing canonical status, output, and tool metadata while raw inspection and persistence remain unchanged. | | `webSearchBridge?` | `{ enabled?: boolean; backend?: "ollama" \| "openai" \| "anthropic" \| "xai" \| "gemini" \| "exa"; maxSearches?: number; timeoutMs?: number; endpoint?: string }` | Key-auth `openai-responses` passthrough providers only. Off by default. Codex always declares the hosted `web_search` tool, and the passthrough relays it on the assumption the destination executes it. A gateway that does not run hosted search answers with a `function_call` named `web_search` that nothing runs, and the undeclared-tool guard ends the turn. With `enabled: true` and an explicit `backend` OpenCodex intercepts that call, runs the search itself, feeds the result back to the same upstream, and shows Codex a hosted `web_search_call` cell. Never armed for `authMode: "forward"` (ChatGPT already searches) or for a provider that executes hosted search upstream. `backend` is required; there is no implicit default and a missing credential for the named backend leaves the bridge disarmed rather than falling through to another paid search. `ollama` reuses this provider's own API key on `POST /api/web_search`, so the origin must be `https://ollama.com` unless the operator names `endpoint` explicitly. `openai` / `anthropic` / `xai` / `gemini` / `exa` reuse the matching sidecar executor and that executor's own credential (`webSearchSidecar.exaApiKey` for Exa). The search model comes from `webSearchSidecar.model` only when `webSearchSidecar.backend` resolves to the same backend this bridge names; otherwise the bridge runs that backend's own default, because a model chosen for one vendor is rejected by another. An unset `webSearchSidecar.backend` resolves to `openai`, so an unset-backend model reaches an `openai` bridge and no other. There is no per-provider bridge model override. Streaming turns only. A turn that mixes `web_search` with another client tool call still fails closed rather than dropping the client's call. Assistant text such as XML-like `` prose is not executed. Defaults: `maxSearches: 3` (1..10), `timeoutMs: 60000` (1000..600000). | -| `retryOn429?` | `{ enabled?: boolean; attempts?: number; intervalMs?: number; maxIntervalMs?: number; respectRetryAfter?: boolean }` | API-key providers only (`authMode: "key"`). Opt-in same-target 429 retry: when `retryOn429` is absent the feature is off; object presence enables it unless `enabled: false`. On 429 the proxy waits (upstream `Retry-After` or the fixed interval) and replays the identical request on the same key before any key failover — across the main text-turn recovery loop, the Responses passthrough wire, the image/video bridge, the web-search sidecar, and terminal continuations. Only pre-stream HTTP 429 responses are eligible for replay; custom `runTurn` transports are outside the HTTP retry loop. `attempts` counts same-key replays after the first 429 (total sends = `attempts` + 1) and is one request-wide budget shared by the main recovery loop, the terminal-guard continuation, and bridge retries. Exhausting `attempts` only stops further same-key replays: normal key failover or final-error handling then applies per the available targets — on the key-auth passthrough wire there is no failover, so the exhausted 429 surfaces as-is. Codex itself never retries 429, so this is the only defense for single-key providers. Defaults: `enabled: true`, `attempts: 3`, `intervalMs: 5000`, `maxIntervalMs: 60000` (any single wait is capped at `maxIntervalMs`, itself capped at 600000), `respectRetryAfter: true`. | +| `retryOn429?` | `{ enabled?: boolean; attempts?: number; intervalMs?: number; maxIntervalMs?: number; respectRetryAfter?: boolean }` | API-key providers only (`authMode: "key"`). Opt-in same-target 429 retry: when `retryOn429` is absent the feature is off, except that key-auth OpenCode Go and Command Code (canonical endpoints) fall back to a patient policy (6 replays, 10 s interval, 60 s cap); object presence enables it unless `enabled: false`, which also turns that fallback off. On 429 the proxy waits (upstream `Retry-After` or the fixed interval) and replays the identical request on the same key before any key failover — across the main text-turn recovery loop, the Responses passthrough wire, the image/video bridge, the web-search sidecar, and terminal continuations. Only pre-stream HTTP 429 responses are eligible for replay; custom `runTurn` transports are outside the HTTP retry loop. `attempts` counts same-key replays after the first 429 (total sends = `attempts` + 1) and is one request-wide budget shared by the main recovery loop, the terminal-guard continuation, and bridge retries. Exhausting `attempts` only stops further same-key replays: normal key failover or final-error handling then applies per the available targets — on the key-auth passthrough wire there is no failover, so the exhausted 429 surfaces as-is. Codex itself never retries 429, so this is the only defense for single-key providers. Defaults: `enabled: true`, `attempts: 3`, `intervalMs: 5000`, `maxIntervalMs: 60000` (any single wait is capped at `maxIntervalMs`, itself capped at 600000), `respectRetryAfter: true`. | | `transientRetryOn5xx?` | `{ enabled?: boolean; attempts?: number }` | Key-auth `openai-chat` and `openai-responses` providers only. `authMode: "forward"` providers (the ChatGPT account pool) never read this option and keep the default ladder. Opt-in retry for pre-stream transient upstream statuses (500, 502, 503, 504, 520, 521, 522): absent means off, object presence enables it unless `enabled: false`. Covers the initial Responses request, the Responses passthrough lane and each of its recovery legs (OAuth-401 replay, same-target 429 replay, validated rebuild), the terminal-guard continuation, and native `/v1/chat/completions`. `attempts` is the TOTAL number of upstream sends allowed for one request including the first (1..10, default 3) — it is one budget shared with connection-reset recovery, so `3` means at most three real requests reach the provider. On the Responses passthrough lane the configured value is additionally intersected with the request-wide send allowance, so a value below that allowance narrows the ladder exactly while a value above it does not raise the bound. Waits use a fixed 400 ms exponential backoff capped at 5 s and honor `Retry-After`. Separate from `retryOn429`, which handles rate limiting; mid-stream failures are never replayed. | | `retryOnReset?` | `{ enabled?: boolean; replacements?: number }` | Native `openai-responses` providers, including `authMode: "forward"`. Opt-in replacement of a send that failed while the caller had observed nothing: absent means off, object presence enables it unless `enabled: false`. Covers both ambiguous stages — a connection that died before any response header, and an SSE body that died after the header while carrying only control events. A canonical ChatGPT upstream WebSocket that closed or errored after its create frame left, before any Responses event, is covered the same way, and its replacement is sent over HTTP. Only a self-contained request is ever replaced: `store: false`, complete `input`, no `previous_response_id`, `conversation` or `stream_id`, and only client-executed tools. `replacements` is the number of replacement sends ONE logical request may make across every leg and every combo child (1..2, default 1) — not a per-leg retry count and not a send budget, so a replacement still has to fit inside the send allowance the leg already had. A request that already emitted output or a tool call is never replaced, whatever this is set to. The replacement inference may still be billed if the origin had already started the first one, which is why this is off by default. | | `autoToolChoiceOnlyModels?` | `string[]` | Models whose `tool_choice` accepts only `auto` or `none`; forced choices are downgraded. | diff --git a/docs-site/src/content/docs/reference/management-api.md b/docs-site/src/content/docs/reference/management-api.md index 0a1b76c94e4..f10c36ba19b 100644 --- a/docs-site/src/content/docs/reference/management-api.md +++ b/docs-site/src/content/docs/reference/management-api.md @@ -45,9 +45,12 @@ On a loopback bind, the dashboard bootstrap can receive a short-lived `ocx_sessi Each session lasts five minutes and is bound to the exact dashboard origin. Safe requests must match that origin. Unsafe methods also require the browser `Origin` and the session's CSRF token. -Session issuance is disabled whenever data-plane authentication is required, which includes remote -binds. A remote operator must authenticate with the raw admin token; no loopback-style GUI session -is minted. +When data-plane authentication is required, which includes remote binds, the loopback bootstrap +does not mint a session. A remote dashboard gets a 12-hour session only through a trusted Tailscale +identity (`remoteGui.allowedTailscaleUsers` on the Tailscale management ingress) or a one-use +pairing grant; each authorized request extends it. Otherwise a remote operator authenticates with +the raw admin token, and the dashboard asks for it again after a reload because the session lives +only in page memory. See [Remote hub](/guides/remote-hub/). ## Common errors diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 53bcb676449..cb4c2759e87 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -173,7 +173,7 @@ "cli-kiro-auto-selection.test.ts": "cli", "codebuddy-live-models.test.ts": "providers", "kiro-auto-selection.test.ts": "providers/kiro", "kiro-quota-metrics.test.ts": "providers/kiro", "management-provider-request-pacing.test.ts": "server", "desktop-supervised-restart.test.ts": "clients", "cli-restart-handoff.test.ts": "cli", "restart-replacement.test.ts": "server", "deepseek-artifact-tool-schema.test.ts": "providers", - "client-config-export-output-limit.test.ts": "config", + "client-config-export-output-limit.test.ts": "config", "responses-skills-snapshot.test.ts": "responses", "openai-chat-serialized-tool-call-scaling.test.ts": "adapters/openai", "openai-chat-tool-call-id-remint.test.ts": "adapters/openai", "coding-agent-json-lines-scaling.test.ts": "providers", diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index cd2e12c6183..6835be4354c 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -1,11 +1,12 @@ import { hasShrinkableOpenAIChatImages, normalizeOpenAIChatImages } from "./openai-chat-images"; +import { protectGlmSummaryBudget, resolveMaxTokens } from "./openai-chat/summary-budget"; import { chatParallelToolCallsWireValue } from "./openai-chat/parallel-tool-calls"; import { applyExplicitChatReasoningWirePolicy } from "./openai-chat/reasoning-wire"; import type { AdapterRequest, IncomingMeta, ProviderAdapter } from "./base"; import type { AdapterEvent, OcxParsedRequest, OcxProviderConfig, OcxUsage } from "../types"; import { modelInList } from "../types"; import { createInlineThinkContentSplitter, splitInlineThinkContent } from "./inline-think-tags"; -import { mapReasoningEffort, modelRecordValue } from "../reasoning-effort"; +import { mapReasoningEffort } from "../reasoning-effort"; import { debugProviderDiagnostic } from "../lib/debug"; import { sseFieldValue } from "../lib/sse-decoder"; import { isDebugEnabled } from "../lib/debug-settings"; @@ -51,12 +52,6 @@ export { stripBracketedModelSuffix } from "./openai-chat/wire"; export { buildOpenAIChatPassthroughRequest } from "./openai-chat/passthrough"; export { formatOpenAIChatErrorBody } from "./openai-chat/errors"; -function resolveMaxTokens(provider: OcxProviderConfig, parsed: OcxParsedRequest): number | undefined { - return parsed.options.maxOutputTokens - ?? modelRecordValue(provider.modelMaxOutputTokens, parsed.modelId) - ?? provider.defaultMaxOutputTokens; -} - function thinkingBudgetForEffort(parsed: OcxParsedRequest, reasoningEffort: string, maxOutputTokens?: number): number | undefined { if (parsed.options.reasoning === "minimal") return 0; const maxBudget = maxOutputTokens ?? 32768; @@ -150,12 +145,14 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd body.stop = parsed.options.stopSequences; } const reasoningDisabled = modelInList(provider.noReasoningModels, parsed.modelId); - const reasoningEffort = mapReasoningEffort(provider, parsed.modelId, parsed.options.reasoning); + const requestedEffort = protectGlmSummaryBudget(body, provider.baseUrl, parsed.options.reasoning) + ? "low" : parsed.options.reasoning; + const reasoningEffort = mapReasoningEffort(provider, parsed.modelId, requestedEffort); const explicitReasoning = applyExplicitChatReasoningWirePolicy({ provider, modelId: parsed.modelId, hasTools: !!tools, - requestedEffort: parsed.options.reasoning, + requestedEffort, wireEffort: reasoningEffort, reasoningDisabled, body, diff --git a/src/adapters/openai-chat/passthrough.ts b/src/adapters/openai-chat/passthrough.ts index cedc86f8c46..ed01bf13a6c 100644 --- a/src/adapters/openai-chat/passthrough.ts +++ b/src/adapters/openai-chat/passthrough.ts @@ -1,3 +1,4 @@ +import { protectGlmSummaryBudget } from "./summary-budget"; import { openAIChatTransport, stripBracketedModelSuffix } from "./wire"; import type { AdapterRequest } from "../base"; import { frameAgentRouterMessages } from "../agentrouter"; @@ -72,6 +73,7 @@ export function buildOpenAIChatPassthroughRequest( for (const field of CHAT_PASSTHROUGH_FIELDS) { if (rawBody[field] !== undefined) body[field] = rawBody[field]; } + if (protectGlmSummaryBudget(body, provider.baseUrl, body.reasoning_effort)) body.reasoning_effort = "low"; const rawEfforts = modelRecordValue(provider.modelReasoningEfforts, modelId) ?? provider.reasoningEfforts; const reasoningDisabled = modelInList(provider.noReasoningModels, modelId) || rawEfforts?.length === 0; if (reasoningDisabled) { diff --git a/src/adapters/openai-chat/summary-budget.ts b/src/adapters/openai-chat/summary-budget.ts new file mode 100644 index 00000000000..2e044a46421 --- /dev/null +++ b/src/adapters/openai-chat/summary-budget.ts @@ -0,0 +1,72 @@ +import { modelRecordValue } from "../../reasoning-effort"; +import type { OcxParsedRequest, OcxProviderConfig } from "../../types"; + +export function resolveMaxTokens(provider: OcxProviderConfig, parsed: OcxParsedRequest): number | undefined { + return parsed.options.maxOutputTokens + ?? modelRecordValue(provider.modelMaxOutputTokens, parsed.modelId) + ?? provider.defaultMaxOutputTokens; +} + +function textContent(value: unknown): string | undefined { + if (typeof value === "string") return value; + if (!Array.isArray(value)) return undefined; + const text: string[] = []; + for (const part of value) { + if (!part || part.type !== "text" || typeof part.text !== "string") return undefined; + text.push(part.text); + } + return text.join("\n"); +} + +const PROTECTED_SUMMARY_CAP = 8192; +/** A real checkpoint carries the conversation it summarizes; a short probe is not one (#5465). */ +const MIN_CHECKPOINT_TRANSCRIPT_CHARS = 2000; + +/** The mitigation is scoped to Z.AI's own endpoints; another gateway serving the same model id is not. */ +function isZaiEndpoint(baseUrl: string | undefined): boolean { + if (!baseUrl) return false; + try { + const host = new URL(baseUrl).hostname.toLowerCase(); + return host === "z.ai" || host.endsWith(".z.ai"); + } catch { + return false; + } +} + +function isTinyCap(value: unknown): value is number { + return typeof value === "number" && Number.isInteger(value) && value >= 1 && value <= 1024; +} + +/** + * Aside's emergency checkpoint is a standalone summary, not an ordinary short answer (#5465). + * Runs at the physical Chat destination, after all combo effort overrides, so `effort` is the + * effective effort. Only the exhausting tiers (`high`/`max`) on Z.AI's GLM-5.3-Flash, for the + * two-message checkpoint shape with a real `` transcript, qualify. Each tiny cap + * field is raised on its own, so a caller's larger cap is never shrunk. + */ +export function protectGlmSummaryBudget( + body: Record, + baseUrl: string | undefined, + effort: unknown, +): boolean { + if (!isZaiEndpoint(baseUrl)) return false; + if (effort !== "high" && effort !== "max") return false; + if (typeof body.model !== "string" + || !/^(?:(?:zai|z-ai|zai-org)\/)?glm-5\.3-flash$/i.test(body.model)) return false; + if (body.tools !== undefined && (!Array.isArray(body.tools) || body.tools.length > 0)) return false; + if (!isTinyCap(body.max_tokens) && !isTinyCap(body.max_completion_tokens)) return false; + const messages = body.messages; + if (!Array.isArray(messages) || messages.length !== 2) return false; + const [system, user] = messages; + if (!system || !user || !["system", "developer"].includes(system.role) || user.role !== "user" + || system.tool_calls || user.tool_calls || system.function_call || user.function_call) return false; + const instruction = textContent(system.content); + const transcript = textContent(user.content); + if (instruction === undefined || transcript === undefined) return false; + if (!/\b(?:summari[sz](?:e|ation|er|ing)|summary|checkpoint)\b/i.test(instruction)) return false; + if (!/[\s\S]*<\/conversation>/i.test(transcript)) return false; + if (transcript.length < MIN_CHECKPOINT_TRANSCRIPT_CHARS) return false; + if (isTinyCap(body.max_tokens)) body.max_tokens = PROTECTED_SUMMARY_CAP; + if (isTinyCap(body.max_completion_tokens)) body.max_completion_tokens = PROTECTED_SUMMARY_CAP; + return true; +} diff --git a/src/config/diagnostics.ts b/src/config/diagnostics.ts index ca7b899e3d6..d1ab81747c4 100644 --- a/src/config/diagnostics.ts +++ b/src/config/diagnostics.ts @@ -63,6 +63,7 @@ import { runtimeRoleSchema, spendSchema, compactionRoutingSchema, + skillsConfigSchema, } from "./schema/leaf-validators"; export type ConfigDiagnostics = { @@ -594,6 +595,17 @@ export function metricsExportConfigError(value: unknown): string | null { return null; } + +function skillsConfigError(value: unknown): string | null { + const raw = rawConfigRecord(value); + if (!raw || !Object.hasOwn(raw, "skills") || raw.skills === undefined) return null; + const result = skillsConfigSchema.safeParse(raw.skills); + if (result.success) return null; + const issue = result.error.issues[0]; + const field = issue?.path.join("."); + return "schema_invalid: skills" + (field ? "." + field : "") + ": " + (issue?.message ?? "invalid configuration"); +} + export function validateConfigCandidate(value: unknown): { ok: true; config: OcxConfig } | { ok: false; error: string } { const compactionRouting = rawConfigRecord(value)?.compactionRouting; if (compactionRouting !== undefined && !compactionRoutingSchema.safeParse(compactionRouting).success) { @@ -625,7 +637,8 @@ export function validateConfigCandidate(value: unknown): { ok: true; config: Ocx ?? clientRolePairError(value) ?? loopbackListenerPortError(value) ?? managementIngressConfigError(value) - ?? metricsExportConfigError(value); + ?? metricsExportConfigError(value) + ?? skillsConfigError(value); if (boundaryError) return { ok: false, error: boundaryError }; const result = configSchema.safeParse(value); if (result.success) { diff --git a/src/config/schema/config-schema.ts b/src/config/schema/config-schema.ts index 48460d3667e..a855202a29a 100644 --- a/src/config/schema/config-schema.ts +++ b/src/config/schema/config-schema.ts @@ -16,6 +16,7 @@ import { remoteGuiConfigSchema, runtimeRoleSchema, spendSchema, + skillsConfigSchema, configuredCodexPoolAccountIds, apiKeyEntrySchema, asideProfileSyncSchema, @@ -78,6 +79,7 @@ export const configSchema = z.object({ // A malformed privacy block must never be read as "unmask": .catch(undefined) drops it and // emailMaskingEnabled then falls back to masked, which is also what an absent block means. privacy: z.object({ maskEmails: z.boolean().optional() }).strict().optional().catch(undefined), + skills: skillsConfigSchema.optional().catch(undefined), // Malformed hand edits disable this opt-in exporter. Live writes reject them in diagnostics.ts. metricsExport: z.object({ enabled: z.boolean().optional() }).strict().optional().catch(undefined), // Kept raw on purpose: `.catch(undefined)` would turn a mistyped `enabled` into "inherit", diff --git a/src/config/schema/leaf-validators.ts b/src/config/schema/leaf-validators.ts index 449fd54ce13..8de6060f5b2 100644 --- a/src/config/schema/leaf-validators.ts +++ b/src/config/schema/leaf-validators.ts @@ -1055,3 +1055,10 @@ export const spendSchema = z.object({ pool: spendScopeSchema.optional(), retentionDays: z.number().int().min(1).max(365).optional(), }).strict(); + +/** + * Runtime skills catalog configuration (#5569). + */ +export const skillsConfigSchema = z.object({ + catalog_refresh: z.enum(["per_session", "per_turn"]).optional(), +}).strict(); diff --git a/src/providers/key-failover.ts b/src/providers/key-failover.ts index c96b555029b..cbaca42739c 100644 --- a/src/providers/key-failover.ts +++ b/src/providers/key-failover.ts @@ -12,7 +12,7 @@ import { commitProviderApiKeySelection } from "./api-key-selection"; import type { ProviderApiKeySelection } from "../types/provider"; import { routedProviderConfig } from "../router"; import { getProviderRegistryEntry } from "./registry"; -import { normalizedBaseUrl } from "./quota/vendor-probes-key"; +import { isCanonicalCommandCodeBaseUrl, normalizedBaseUrl } from "./quota/vendor-probes-key"; import type { OcxConfig, OcxProviderConfig, RateLimitRetryPolicy, ResetReplayPolicy, TransientRetryPolicy } from "../types"; import { OPENCODE_GO_SESSION_HEADER } from "./opencode-go-transport"; import { resolveProviderTransport, type OcxProviderTransport } from "./xai-transport"; @@ -566,7 +566,8 @@ export function selectProactiveApiKeyTransport( * `enabled: false` to opt out). When the knob is absent, the OpenCode Go destination * (subscription traffic such as Muse Spark) falls back to a patient same-key policy so a * burst 429 waits and replays instead of surfacing to the client and aborting a long - * session; every other provider without the knob keeps today's fail-fast behavior. + * session; key-auth Command Code at its canonical endpoints gets the same fallback (#5180), and + * every other provider without the knob keeps today's fail-fast behavior. * OAuth/forward/local credentials are never replayed on the same token. The returned * policy is fully defaulted so callers never re-check fields. */ @@ -590,10 +591,17 @@ export function rateLimitRetryPolicyFor( respectRetryAfter: policy.respectRetryAfter ?? DEFAULT_RATE_LIMIT_RETRY.respectRetryAfter, }; } - // No explicit knob: patient fallback for the OpenCode Go destination only. + // No explicit knob: patient fallback for OpenCode Go and canonical Command Code only. if (provider.authMode !== undefined && provider.authMode !== "key") return null; - if (!isOpenCodeGoDestination(provider)) return null; - return { ...OPENCODE_GO_RATE_LIMIT_RETRY }; + if (isOpenCodeGoDestination(provider)) return { ...OPENCODE_GO_RATE_LIMIT_RETRY }; + // Command Code's Provider API rate-limits long muse-spark turns the same way (#5180): a single + // key cannot fail over, and the Codex client does not retry a 429, so the turn aborts. The key + // endpoint gets the same patient same-key policy. A row repointed at a custom relay is not the + // canonical endpoint and keeps fail-fast unless it sets `retryOn429`. + if (typeof provider.baseUrl === "string" && isCanonicalCommandCodeBaseUrl(provider.baseUrl.trim())) { + return { ...OPENCODE_GO_RATE_LIMIT_RETRY }; + } + return null; } /** diff --git a/src/server/responses/request-prepare.ts b/src/server/responses/request-prepare.ts index 66369040eb3..2ac1679a72f 100644 --- a/src/server/responses/request-prepare.ts +++ b/src/server/responses/request-prepare.ts @@ -22,6 +22,7 @@ import { reasoningReplayConversationIdFromResponsesRequest, } from "../request-log-conversation"; import { resolveContextPrincipal } from "../auth-cors"; +import { resolveSkillsSnapshotScopeKey, snapshotSkillsCatalogInBody } from "./skills-snapshot"; import { isShadowSourceModel, shadowSourceModelPrefix, @@ -314,6 +315,15 @@ export async function prepareResponsesRequest( ); } + const skillsSnapshotScopeKey = resolveSkillsSnapshotScopeKey({ + req, + config, + admission: options.admission, + promptCacheKeyIsSharedCohort: options.promptCacheKeyIsSharedCohort, + }); + // Substitutes a known snapshot now; a new catalog is only stored once the request is prepared. + const commitSkillsSnapshot = snapshotSkillsCatalogInBody(body, skillsSnapshotScopeKey, config); + let parsed: OcxParsedRequest; let toolBridgeMaps: ReturnType; try { @@ -1263,6 +1273,7 @@ export async function prepareResponsesRequest( ? admissionState.authCtx.accountId : config.activeCodexAccountId ?? null; + commitSkillsSnapshot?.(); return { inboundWire, translatorBudget, diff --git a/src/server/responses/skills-snapshot.ts b/src/server/responses/skills-snapshot.ts new file mode 100644 index 00000000000..0a310ae4a7a --- /dev/null +++ b/src/server/responses/skills-snapshot.ts @@ -0,0 +1,239 @@ +/** + * runtime skills catalog session snapshotting (#5569). + * + * Preserves the Anthropic/LLM prompt cache prefix across turns by freezing + * incoming for the duration of a trustworthy session. + * Gated by config `skills.catalog_refresh`: "per_session" (default) or "per_turn". + * + * Lifecycle & Bounds: + * - 4 hours idle TTL (sliding on access) + * - 1,000 maximum tracked sessions (LRU eviction) + * - 512 KiB maximum per snapshotted skills block + * - 8 MiB global retained byte bound across all sessions + */ +import type { OcxConfig, SkillsCatalogRefresh } from "../../types/config"; +import { isApiAuthRequired, resolveContextPrincipal, type DataPlaneAdmission } from "../auth-cors"; +import { + reasoningReplayConversationIdFromResponsesRequest, + sessionIdHeaderFromRequest, +} from "../request-log-conversation"; + +const SKILLS_BLOCK_GLOBAL_REGEX = /([\s\S]*?)<\/skills_instructions>/g; + +/** Maximum distinct sessions tracked in the memory LRU. */ +export const MAX_SNAPSHOT_SESSIONS = 1000; +/** Slide expiry after 4 hours of inactivity. */ +export const SNAPSHOT_TTL_MS = 4 * 60 * 60 * 1000; +/** Bounded byte ceiling per snapshotted skills block (512 KiB). */ +export const MAX_SKILLS_BLOCK_BYTES = 512 * 1024; +/** Global retained byte bound across all tracked sessions (8 MiB). */ +export const MAX_TOTAL_RETAINED_BYTES = 8 * 1024 * 1024; + +interface SnapshotEntry { + skillsBlock: string; // The full ... block + byteLength: number; + lastAccessed: number; +} + +const snapshotCache = new Map(); +let totalRetainedBytes = 0; + +function evictOldestEntry(): boolean { + const oldest = snapshotCache.entries().next().value; + if (!oldest) return false; + const [key, entry] = oldest; + totalRetainedBytes -= entry.byteLength; + snapshotCache.delete(key); + return true; +} + +export function resolveSkillsCatalogRefresh(config: OcxConfig | undefined): SkillsCatalogRefresh { + const configured = config?.skills?.catalog_refresh; + if (configured === "per_turn") return "per_turn"; + return "per_session"; +} + +export interface ResolveSkillsSessionScopeInput { + req: Request; + config: OcxConfig; + admission?: DataPlaneAdmission; + cursorConversationId?: string; + promptCacheKeyIsSharedCohort?: boolean; +} + +/** + * The principal a snapshot is scoped to. Without a named principal, only a server that requires + * no data-plane auth may share by conversation id (loopback admission, or an internal caller that + * passed none on such a server): its callers already share one trust domain, which on a no-auth + * server bound beyond loopback includes remote callers. Any other anonymous request gets no + * snapshot. + */ +function snapshotPrincipal(input: ResolveSkillsSessionScopeInput): string | null | undefined { + const principal = resolveContextPrincipal(input.req, input.config, input.admission); + if (principal) return principal; + if (input.admission) return input.admission.kind === "loopback" ? null : undefined; + return isApiAuthRequired(input.config) ? undefined : null; +} + +/** + * Resolves a trustworthy cache key for skills catalog snapshotting. + * Returns null if no specific, reliable thread/session identity is available, + * or if the identity comes from a shared cohort fallback. + */ +export function resolveSkillsSnapshotScopeKey(input: ResolveSkillsSessionScopeInput): string | null { + if (input.promptCacheKeyIsSharedCohort === true) { + return null; + } + + const parentThread = input.req.headers.get("x-codex-parent-thread-id")?.trim() || undefined; + const ownThreadId = input.req.headers.get("thread-id")?.trim() || undefined; + const cursorId = input.cursorConversationId?.trim() || undefined; + + // When a parent thread is present, subagents/children may share a root session-id header. + // To prevent cross-thread/sibling collapse or parent-level caching, require an explicit + // own thread-id (or cursor id). If parentThread is present without an own child thread, + // bypass snapshotting completely. + if (parentThread) { + const childId = ownThreadId ?? cursorId; + if (!childId || childId === parentThread) { + return null; + } + const qualifiedId = `${parentThread}\u0000${childId}`; + const principal = snapshotPrincipal(input); + if (principal === undefined) return null; + return JSON.stringify(["skills_catalog_snapshot_v1", principal, qualifiedId]); + } + + // Standalone conversation (no parent thread) + const standaloneId = reasoningReplayConversationIdFromResponsesRequest({ + threadIdHeader: ownThreadId, + cursorConversationId: cursorId, + sessionIdHeader: sessionIdHeaderFromRequest(input.req.headers), + }); + if (!standaloneId) { + return null; + } + const principal = snapshotPrincipal(input); + if (principal === undefined) return null; + return JSON.stringify(["skills_catalog_snapshot_v1", principal, standaloneId]); +} + +/** One text slot that may carry a catalog: `instructions` or a developer/system text part. */ +interface CatalogSlot { + text: string; + write(next: string): void; +} + +/** Every developer/system text slot, walked the same way the replacement writes. */ +function catalogSlots(body: Record): CatalogSlot[] { + const slots: CatalogSlot[] = []; + if (typeof body.instructions === "string") { + slots.push({ text: body.instructions, write: next => { body.instructions = next; } }); + } + if (!Array.isArray(body.input)) return slots; + for (const item of body.input) { + if (!item || typeof item !== "object") continue; + const it = item as Record; + // Restrict message item type: must be undefined or "message", so role-like tool objects are untouched + if (it.type !== undefined && it.type !== "message") continue; + // Only developer and system content is inspected/transformed + if (it.role !== "developer" && it.role !== "system") continue; + const content = it.content; + if (typeof content === "string") { + slots.push({ text: content, write: next => { it.content = next; } }); + } else if (Array.isArray(content)) { + for (const part of content) { + if (!part || typeof part !== "object") continue; + const p = part as Record; + // Restrict text parts to known text / input_text + if (p.type !== "text" && p.type !== "input_text") continue; + if (typeof p.text === "string") slots.push({ text: p.text, write: next => { p.text = next; } }); + } + } + } + return slots; +} + +function liveSnapshot(scopeKey: string, now: number): SnapshotEntry | undefined { + const existing = snapshotCache.get(scopeKey); + if (!existing) return undefined; + if (now - existing.lastAccessed > SNAPSHOT_TTL_MS) { + totalRetainedBytes -= existing.byteLength; + snapshotCache.delete(scopeKey); + return undefined; + } + existing.lastAccessed = now; + // Refresh Map order for true LRU behavior + snapshotCache.delete(scopeKey); + snapshotCache.set(scopeKey, existing); + return existing; +} + +function storeSnapshot(scopeKey: string, skillsBlock: string, now: number): void { + const blockBytes = Buffer.byteLength(skillsBlock, "utf8"); + if (blockBytes > MAX_SKILLS_BLOCK_BYTES || blockBytes > MAX_TOTAL_RETAINED_BYTES) return; + // A concurrent request of the same conversation may have stored first; the first catalog wins. + if (snapshotCache.has(scopeKey)) return; + // Evict oldest entries until under count ceiling AND under global byte ceiling + while ( + (snapshotCache.size >= MAX_SNAPSHOT_SESSIONS || totalRetainedBytes + blockBytes > MAX_TOTAL_RETAINED_BYTES) + && snapshotCache.size > 0 + ) { + if (!evictOldestEntry()) break; + } + if (totalRetainedBytes + blockBytes > MAX_TOTAL_RETAINED_BYTES) return; + snapshotCache.set(scopeKey, { skillsBlock, byteLength: blockBytes, lastAccessed: now }); + totalRetainedBytes += blockBytes; +} + +/** + * Reuses the session's snapshotted in developer/system content, preserving + * the prompt cache prefix across turns. User and assistant messages and tool calls are never + * modified. + * + * Only a body with exactly one catalog block across all of those slots takes part: with two or + * more there is no way to tell which one the snapshot stands for, so the body passes through + * untouched rather than rewriting every block to one catalog. + * + * A known snapshot is substituted at once, so parsing and the verbatim passthrough body both see + * it. A new catalog is only stored through the returned commit, which the caller runs once the + * request has passed parsing and admission, so a rejected first request pins nothing. + */ +export function snapshotSkillsCatalogInBody( + body: unknown, + scopeKey: string | null, + config: OcxConfig, + now: number = Date.now(), +): (() => void) | undefined { + if (!scopeKey) return undefined; + if (resolveSkillsCatalogRefresh(config) === "per_turn") return undefined; + if (!body || typeof body !== "object" || Array.isArray(body)) return undefined; + + let found: { slot: CatalogSlot; block: string } | undefined; + let blocks = 0; + for (const slot of catalogSlots(body as Record)) { + if (!slot.text.includes("")) continue; + for (const match of slot.text.matchAll(SKILLS_BLOCK_GLOBAL_REGEX)) { + blocks++; + found ??= { slot, block: match[0] }; + } + } + if (blocks !== 1 || !found) return undefined; + + const existing = liveSnapshot(scopeKey, now); + if (existing) { + const { slot, block } = found; + const at = slot.text.indexOf(block); + slot.write(slot.text.slice(0, at) + existing.skillsBlock + slot.text.slice(at + block.length)); + return undefined; + } + const incoming = found.block; + return () => storeSnapshot(scopeKey, incoming, now); +} + +/** Test helpers */ +export function resetSkillsSnapshotCacheForTests(): void { + snapshotCache.clear(); + totalRetainedBytes = 0; +} + diff --git a/src/types.ts b/src/types.ts index ff961d813e4..5281666884d 100644 --- a/src/types.ts +++ b/src/types.ts @@ -77,6 +77,8 @@ export type { OcxConnectedClientId, OcxClientConnectionConfig, OcxConfig, + SkillsCatalogRefresh, + OcxSkillsConfig, OcxAccountPoolRotationStrategy, OcxAccountPoolQuotaWindow, OcxComboCooldownWaitPolicy, diff --git a/src/types/config.ts b/src/types/config.ts index 6e7fa9a48ac..940aa2b06fd 100644 --- a/src/types/config.ts +++ b/src/types/config.ts @@ -362,6 +362,18 @@ export interface OcxConfigRebaseProvenance { deletedTopLevelKeys: string[]; } + +export type SkillsCatalogRefresh = "per_session" | "per_turn"; + +export interface OcxSkillsConfig { + /** + * Refresh policy for the runtime skills catalog (#5569). + * `per_session` (default): snapshots incoming `` on the first turn of a trustworthy session and reuses it across turns to preserve the Anthropic prompt cache. + * `per_turn`: re-derives/passes through incoming skills instructions every turn (previous behavior). + */ + catalog_refresh?: SkillsCatalogRefresh; +} + export type OcxRuntimeRole = "standalone" | "hub" | "client"; export interface OcxHubConfig { @@ -487,6 +499,8 @@ export interface OcxConfig { client?: OcxClientConnectionConfig; /** Operator-facing redaction policy for management and CLI projections. */ privacy?: OcxPrivacyConfig; + /** Runtime skills catalog session snapshotting settings (#5569). */ + skills?: OcxSkillsConfig; /** Opt-in process-local aggregate request metrics on the authenticated management plane. */ metricsExport?: { enabled?: boolean }; /** Opt in to one identical-turn retry when a Responses completion has no text or tool call. */ diff --git a/structure/config.md b/structure/config.md index 3ce49bf31f3..5088fbf4ffe 100644 --- a/structure/config.md +++ b/structure/config.md @@ -30,10 +30,10 @@ the [source-owned credential contract](codex-home.md#orca-source-owned-account-i `src/config/schema/compaction-recovery.ts` strictly validates opt-in `compactionRecovery`; invalid disk values disable it with a warning, while candidate writes reject them. The [failure-only contract](transports/responses-failover.md) leaves provider identity, accounts and client compaction unchanged. +`skills.catalog_refresh` in the proxy JSON configuration accepts `per_session` (the runtime default when absent) or `per_turn`. The former retains received skills instructions for a conversation; the latter passes through the current catalog. This is separate from Codex's `skills.include_instructions` TOML switch and does not change the live dashboard probe. See the [Responses snapshot contract](transports/responses.md#responses-httpsse). + Google providers may persist `googleToolSchemaPolicy` as `compatible` or `reject-lossy`. -`ocx provider add --google-tool-schema-policy` is one authoring path and is accepted only when the -effective adapter is `google`. Omission remains absent in `config.json`; the adapter resolves it to -`compatible` in memory. +`ocx provider add --google-tool-schema-policy` is one authoring path and is accepted only when the effective adapter is `google`. Omission remains absent in `config.json`; the adapter resolves it to `compatible` in memory. ### OpenCodex home and live process state diff --git a/structure/transports/responses.md b/structure/transports/responses.md index 8320d296bac..aa15f709724 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -11,7 +11,7 @@ Plaintext collaboration restoration treats a null namespace as absent, rejects n When a successful streamed native response has a missing or unrecognized non-JSON content type, the plaintext V2 path confirms a bounded Responses SSE prefix, under the server's `stallTimeoutSec` probe budget, before applying that restoration; an `application/json` body takes the bounded JSON path instead, and an unknown, stalled, or unreadable body retains the fail-closed response. ## Responses HTTP/SSE - +Responses request preparation stabilizes incoming `` under `skills.catalog_refresh`: `per_session` (default) reuses the first received catalog for a conversation; `per_turn` leaves the supplied catalog unchanged. Other instruction sections and user/tool content remain untouched. Requests without a reliable conversation identity bypass snapshots; shared prompt-cache cohorts are not conversation identities. Only a body with exactly one catalog block across its instructions and developer/system content takes part; two or more pass through unchanged. A known snapshot is substituted before parsing, but a new catalog is stored only when preparation reaches its success return, so a request rejected by parsing or admission pins nothing. Without a named principal, snapshots are shared by conversation id only on a server that requires no data-plane auth. Snapshots are process-local, expire after four idle hours, and use bounded LRU retention; oversized blocks bypass caching. The dashboard's `src/codex/prompt-layers.ts` and `src/codex/prompt-text-probe.ts` continue observing current files for previews and do not own session snapshots. `/v1/responses` is the main Codex-facing endpoint. The server parses Responses input, routes to a provider, lets the selected adapter speak the upstream protocol, then bridges adapter events back to Responses-compatible streaming output. For an opted-in key-auth provider, a hosted-search continuation stays bound to the API-key selection that served the first leg; the contract is the [hosted-search continuation binding](../providers-and-adapters.md#hosted-search-continuation-binding). diff --git a/structure/transports/streaming-health.md b/structure/transports/streaming-health.md index b87bd6444eb..3e01e3894d2 100644 --- a/structure/transports/streaming-health.md +++ b/structure/transports/streaming-health.md @@ -188,7 +188,7 @@ once the server observes the client disconnect (Bun propagates it asynchronously cancelled with 499 before any replay; because the propagation is async, a replay may precede the cancel if the interval elapses first (bounded by the same `attempts` budget). -OpenCode Go (`https://opencode.ai/zen/go/v1`, serving subscription traffic such as Muse Spark) ships a patient same-target fallback when no explicit `retryOn429` is configured: same-key wait-and-replay with a 10s interval and a 60s cap, `Retry-After` honored. Replays draw from the shared per-request send budget, so a burst typically absorbs a couple of paced sends before the 429 surfaces — without this, a single-key pool surfaced the first 429 immediately and the client’s own retry budget aborted the goal (`exceeded retry limit, last status: 429`). An explicit `retryOn429` — including `enabled: false` — always overrides the fallback; every other provider without the knob keeps fail-fast behavior. +OpenCode Go (`https://opencode.ai/zen/go/v1`, serving subscription traffic such as Muse Spark) ships a patient same-target fallback when no explicit `retryOn429` is configured: same-key wait-and-replay with a 10s interval and a 60s cap, `Retry-After` honored. Replays draw from the shared per-request send budget, so a burst typically absorbs a couple of paced sends before the 429 surfaces — without this, a single-key pool surfaced the first 429 immediately and the client’s own retry budget aborted the goal (`exceeded retry limit, last status: 429`). The same fallback covers key-auth Command Code at its canonical endpoints (`https://api.commandcode.ai/provider/v1` and the API root), where long muse-spark turns hit the same burst limit (#5180); OAuth rows are never replayed on the same token, and a row repointed at a custom relay keeps fail-fast. An explicit `retryOn429` — including `enabled: false` — always overrides the fallback; every other provider without the knob keeps fail-fast behavior. Provider-level `requestPacing` is the proactive companion to `retryOn429`. It reserves outbound request-start slots before transport work begins, so a known RPM ceiling does not have to fail once diff --git a/tests/adapters/openai/openai-chat-glm-summary.test.ts b/tests/adapters/openai/openai-chat-glm-summary.test.ts new file mode 100644 index 00000000000..b892d3a04f5 --- /dev/null +++ b/tests/adapters/openai/openai-chat-glm-summary.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, test } from "bun:test"; +import { buildOpenAIChatPassthroughRequest, createOpenAIChatAdapter } from "../../../src/adapters/openai-chat"; +import { chatCompletionsToResponsesBody } from "../../../src/chat/inbound"; +import { concreteComboRequestBody } from "../../../src/combos/request"; +import { parseRequest } from "../../../src/responses/parser"; +import type { OcxProviderConfig } from "../../../src/types"; + +const model = "glm-5.3-flash"; +const provider: OcxProviderConfig = { + adapter: "openai-chat", baseUrl: "https://api.z.ai/api/coding/paas/v4", + reasoningEfforts: ["low", "medium", "high", "max"], +}; +// Aside's emergency checkpoint: a summary instruction plus the transcript it summarizes (#5465). +const transcript = `${"User: implement the feature and keep the tests green.\n".repeat(60)}`; +const messages = [ + { role: "system", content: "You are a context-summarization assistant. Produce a checkpoint." }, + { role: "user", content: transcript }, +]; +function bodies(overrides: Record = {}, config = provider) { + const raw = { model, messages, max_tokens: 512, reasoning_effort: "max", ...overrides }; + const parsed = parseRequest(chatCompletionsToResponsesBody(raw)); + return [ + JSON.parse(createOpenAIChatAdapter(config).buildRequest(parsed).body), + JSON.parse(buildOpenAIChatPassthroughRequest(config, raw, String(raw.model), false).body), + ]; +} + +describe("GLM tiny standalone summary compatibility", () => { + test.each([1, 512, 819, 1024])("raises cap %i and lowers effort on both Chat paths", cap => { + for (const body of bodies({ max_tokens: cap })) { + expect(body.max_tokens).toBe(8192); + expect(body.reasoning_effort).toBe("low"); + expect(body.messages).toEqual(messages); + } + }); + test("final adapter wins after successive combo force overrides", () => { + let raw = chatCompletionsToResponsesBody({ model, messages, max_tokens: 819, reasoning_effort: "high" }); + for (const target of [{ provider: "proxy", model: "inner" }, { provider: "zai", model }]) { + raw = concreteComboRequestBody(raw, target, "max", provider.reasoningEfforts, "strict", "force"); + } + const parsed = parseRequest(raw); + parsed.modelId = model; + expect(parsed.options.reasoning).toBe("max"); + const body = JSON.parse(createOpenAIChatAdapter(provider).buildRequest(parsed).body); + expect(body.max_tokens).toBe(8192); + expect(body.reasoning_effort).toBe("low"); + expect(parsed.options.maxOutputTokens).toBe(819); + expect(parsed.options.reasoning).toBe("max"); + }); + test.each([0, -1, 1025, 4096, undefined])("preserves cap outside the mitigation: %s", cap => { + for (const body of bodies({ max_tokens: cap })) { + expect(body.max_tokens).toBe(cap); + expect(body.reasoning_effort).toBe("max"); + } + }); + test("does not change other models, ordinary prompts, tools, or ongoing conversations", () => { + for (const overrides of [ + { model: "glm-5.3" }, { model: "glm-5.3-flashx" }, { model: "gpt-5" }, + { messages: [{ role: "system", content: "Be helpful." }, { role: "user", content: "Summarize this article." }] }, + { messages: [...messages, { role: "assistant", content: "Previous checkpoint" }] }, + { tools: [{ type: "function", function: { name: "read", parameters: { type: "object", properties: {} } } }] }, + ]) for (const body of bodies(overrides)) { + expect(body.max_tokens).toBe(512); + expect(body.reasoning_effort).toBe("max"); + } + }); +}); + +describe("GLM summary mitigation stays inside its boundary (#5953 review)", () => { + const untouched = (body: Record, effort = "max") => { + expect(body.max_tokens).toBe(512); + expect(body.reasoning_effort).toBe(effort); + }; + test("another gateway serving the same model id is left alone", () => { + for (const baseUrl of ["https://example.com/v1", "https://evilz.ai/api/v4", "not a url"]) { + for (const body of bodies({}, { ...provider, baseUrl })) untouched(body); + } + }); + test("a summarization system prompt with an ordinary short user message is not a checkpoint", () => { + const probe = [messages[0], { role: "user", content: "Hello" }]; + for (const body of bodies({ messages: probe })) untouched(body); + }); + test("a checkpoint transcript under the minimum length is not rewritten", () => { + const short = [messages[0], { role: "user", content: "User: hi." }]; + for (const body of bodies({ messages: short })) untouched(body); + }); + test.each(["low", "medium"])("an effective %s effort is not overridden", effort => { + for (const body of bodies({ reasoning_effort: effort })) untouched(body, effort); + }); + test("each cap field is judged on its own", () => { + const [, mixedLarge] = bodies({ max_tokens: 512, max_completion_tokens: 2048 }); + expect(mixedLarge.max_tokens).toBe(8192); + expect(mixedLarge.max_completion_tokens).toBe(2048); + const [, mixedSmall] = bodies({ max_tokens: 4096, max_completion_tokens: 512 }); + expect(mixedSmall.max_tokens).toBe(4096); + expect(mixedSmall.max_completion_tokens).toBe(8192); + }); +}); diff --git a/tests/config/config-skills-catalog-refresh.test.ts b/tests/config/config-skills-catalog-refresh.test.ts new file mode 100644 index 00000000000..7ddac6ce389 --- /dev/null +++ b/tests/config/config-skills-catalog-refresh.test.ts @@ -0,0 +1,115 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + getConfigPath, + getDefaultConfig, + loadConfig, + saveConfig, + validateConfigCandidate, +} from "../../src/config"; +import { resolveSkillsCatalogRefresh } from "../../src/server/responses/skills-snapshot"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +let home = ""; +let previousHome: string | undefined; + +beforeEach(() => { + previousHome = process.env.OPENCODEX_HOME; + home = mkdtempSync(join(tmpdir(), "ocx-skills-config-")); + process.env.OPENCODEX_HOME = home; +}); + +afterEach(() => { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + removeTreeWithRetry(home); +}); + +function candidate(skills: unknown) { + return { + ...getDefaultConfig(), + defaultProvider: "xai", + providers: { + xai: { + adapter: "openai-responses", + baseUrl: "https://api.x.ai/v1", + }, + }, + skills, + }; +} + +test("validateConfigCandidate accepts valid skills configuration", () => { + const c1 = validateConfigCandidate(candidate({ catalog_refresh: "per_session" })); + expect(c1.ok).toBe(true); + if (c1.ok) { + expect(c1.config.skills?.catalog_refresh).toBe("per_session"); + } + + const c2 = validateConfigCandidate(candidate({ catalog_refresh: "per_turn" })); + expect(c2.ok).toBe(true); + if (c2.ok) { + expect(c2.config.skills?.catalog_refresh).toBe("per_turn"); + } + + const c3 = validateConfigCandidate(candidate({})); + expect(c3.ok).toBe(true); + + const c4 = validateConfigCandidate(candidate(undefined)); + expect(c4.ok).toBe(true); +}); + +test("validateConfigCandidate explicitly rejects invalid skills configuration", () => { + const badValue = validateConfigCandidate(candidate({ catalog_refresh: "invalid_refresh" })); + expect(badValue.ok).toBe(false); + if (!badValue.ok) { + expect(badValue.error).toContain("schema_invalid: skills.catalog_refresh"); + } + + const extraProp = validateConfigCandidate(candidate({ catalog_refresh: "per_session", extra: 123 })); + expect(extraProp.ok).toBe(false); + if (!extraProp.ok) { + expect(extraProp.error).toContain("schema_invalid: skills"); + } + + const nonObject = validateConfigCandidate(candidate("per_session")); + expect(nonObject.ok).toBe(false); + if (!nonObject.ok) { + expect(nonObject.error).toContain("schema_invalid: skills"); + } +}); + +test("resolveSkillsCatalogRefresh defaults to per_session", () => { + expect(resolveSkillsCatalogRefresh(undefined)).toBe("per_session"); + expect(resolveSkillsCatalogRefresh({} as any)).toBe("per_session"); + expect(resolveSkillsCatalogRefresh({ skills: {} } as any)).toBe("per_session"); + expect(resolveSkillsCatalogRefresh({ skills: { catalog_refresh: "per_session" } } as any)).toBe("per_session"); + expect(resolveSkillsCatalogRefresh({ skills: { catalog_refresh: "per_turn" } } as any)).toBe("per_turn"); +}); + +test("skills configuration persists to disk and loads correctly", () => { + const cfg = { + ...getDefaultConfig(), + skills: { catalog_refresh: "per_turn" as const }, + }; + saveConfig(cfg); + + const loaded = loadConfig(); + expect(loaded.skills?.catalog_refresh).toBe("per_turn"); +}); + +test("malformed hand-edited skills in config file degrades gracefully on load", () => { + const configPath = getConfigPath(); + const raw = JSON.stringify({ + ...getDefaultConfig(), + skills: { catalog_refresh: "bad_value" }, + }); + writeFileSync(configPath, raw, "utf8"); + + const loaded = loadConfig(); + expect(loaded.skills).toBeUndefined(); + expect(resolveSkillsCatalogRefresh(loaded)).toBe("per_session"); +}); + diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 2924eaf5ca9..89dfe34703d 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -19,6 +19,7 @@ "deepseek-artifact-tool-schema.test.ts": "providers", "client-config-export-output-limit.test.ts": "config", "codex-account-clear-paused.test.ts": "codex-integration", + "responses-skills-snapshot.test.ts": "responses", "openai-chat-serialized-tool-call-scaling.test.ts": "adapters/openai", "openai-chat-tool-call-id-remint.test.ts": "adapters/openai", "coding-agent-json-lines-scaling.test.ts": "providers", diff --git a/tests/helpers/responses-core-source.ts b/tests/helpers/responses-core-source.ts index 7460b64ada9..f803be42c3e 100644 --- a/tests/helpers/responses-core-source.ts +++ b/tests/helpers/responses-core-source.ts @@ -30,6 +30,7 @@ export const RESPONSES_CORE_MODULES = [ "core-combo.ts", "core-combo-native.ts", "request-prepare.ts", + "skills-snapshot.ts", "shadow-target-availability.ts", "compaction-routing.ts", "compaction-recovery.ts", diff --git a/tests/providers/rate-limit-retry.test.ts b/tests/providers/rate-limit-retry.test.ts index e028f6abc67..adc82ad669f 100644 --- a/tests/providers/rate-limit-retry.test.ts +++ b/tests/providers/rate-limit-retry.test.ts @@ -109,6 +109,28 @@ describe("rateLimitRetryPolicyFor", () => { } }); + // #5180: a single Command Code key cannot fail over and the Codex client does not retry a 429, + // so a burst on a long muse-spark turn aborted it. Both canonical endpoints wait patiently. + test("falls back to the patient policy for key-auth Command Code without the knob", () => { + const patient = { enabled: true, attempts: 6, intervalMs: 10_000, maxIntervalMs: 60_000, respectRetryAfter: true }; + for (const baseUrl of ["https://api.commandcode.ai/provider/v1", "https://api.commandcode.ai"]) { + expect(rateLimitRetryPolicyFor({ baseUrl, authMode: "key", adapter: "openai-chat" } as OcxProviderConfig)) + .toEqual(patient); + expect(rateLimitRetryPolicyFor({ baseUrl, adapter: "openai-chat" } as OcxProviderConfig)).toEqual(patient); + } + // OAuth Command Code is never replayed on the same token. + expect(rateLimitRetryPolicyFor({ + baseUrl: "https://api.commandcode.ai", authMode: "oauth", + } as OcxProviderConfig)).toBeNull(); + // An explicit opt-out wins, and a custom relay keeps fail-fast. + expect(rateLimitRetryPolicyFor({ + baseUrl: "https://api.commandcode.ai/provider/v1", retryOn429: { enabled: false }, + } as OcxProviderConfig)).toBeNull(); + expect(rateLimitRetryPolicyFor({ + baseUrl: "https://relay.example.test/provider/v1", authMode: "key", + } as OcxProviderConfig)).toBeNull(); + }); + test("honors explicit values", () => { expect(rateLimitRetryPolicyFor({ retryOn429: { attempts: 10, intervalMs: 1_000, maxIntervalMs: 5_000, respectRetryAfter: false }, diff --git a/tests/responses/responses-skills-snapshot.test.ts b/tests/responses/responses-skills-snapshot.test.ts new file mode 100644 index 00000000000..a1c8c07fdee --- /dev/null +++ b/tests/responses/responses-skills-snapshot.test.ts @@ -0,0 +1,173 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { getDefaultConfig } from "../../src/config/proxy-env"; +import { handleResponses } from "../../src/server/responses/core"; +import { resetSkillsSnapshotCacheForTests, resolveSkillsSnapshotScopeKey } from "../../src/server/responses/skills-snapshot"; +import type { OcxConfig } from "../../src/types"; +import { acquireOwnedSpendHome } from "../helpers/owned-spend-home"; + +const originalFetch = globalThis.fetch; +let releaseSpendHome: (() => void) | undefined; +const captured: string[] = []; + +beforeEach(() => { + releaseSpendHome = acquireOwnedSpendHome(); + resetSkillsSnapshotCacheForTests(); + captured.length = 0; +}); +afterEach(() => { + releaseSpendHome?.(); + releaseSpendHome = undefined; + globalThis.fetch = originalFetch; + resetSkillsSnapshotCacheForTests(); +}); + +function fixture(adapter: "openai-responses" | "anthropic"): OcxConfig { + globalThis.fetch = (async (_input, init) => { + captured.push(String(init?.body)); + return Response.json(adapter === "anthropic" ? { + id: "msg_skills", type: "message", role: "assistant", model: "fixture-model", + content: [{ type: "text", text: "done" }], stop_reason: "end_turn", + usage: { input_tokens: 10, output_tokens: 1 }, + } : { + id: "resp_skills", status: "completed", + output: [{ type: "message", role: "assistant", content: [{ type: "output_text", text: "done" }] }], + usage: { input_tokens: 10, output_tokens: 1, total_tokens: 11 }, + }); + }) as typeof fetch; + return { + ...getDefaultConfig(), + defaultProvider: "fixture", + providers: { + fixture: { adapter, baseUrl: "https://fixture.test/v1", authMode: "key", apiKey: "fixture-key" }, + }, + }; +} + +async function send(config: OcxConfig, catalog: string, thread?: string, surrounding = "outside", headers: Record = {}) { + const response = await handleResponses(new Request("http://localhost/v1/responses", { + method: "POST", + headers: { + "content-type": "application/json", + ...headers, + ...(thread ? { "thread-id": thread, "x-codex-parent-thread-id": "shared-parent" } : {}), + }, + body: JSON.stringify({ + model: "fixture/fixture-model", stream: false, + input: [ + { role: "developer", content: [{ type: "input_text", text: `${surrounding}${catalog}` }] }, + { role: "user", content: "Please help with user-example" }, + ], + }), + }), config, { model: "", provider: "" }); + const text = await response.text(); + expect({ status: response.status, ...(response.status !== 200 ? { text } : {}) }).toEqual({ status: 200 }); + return captured.at(-1)!; +} + +describe("skills catalog snapshots on the Responses request path", () => { + for (const adapter of ["openai-responses", "anthropic"] as const) { + test(`${adapter}: keeps the catalog stable while preserving surrounding instructions and sibling isolation`, async () => { + const config = fixture(adapter); + await send(config, "first-catalog", "child-a"); + const second = await send(config, "edited-catalog", "child-a", "updated-outside"); + expect(second).toContain("first-catalog"); + expect(second).not.toContain("edited-catalog"); + expect(second).toContain("updated-outside"); + expect(second).toContain("user-example"); + const sibling = await send(config, "sibling-catalog", "child-b"); + expect(sibling).toContain("sibling-catalog"); + expect(sibling).not.toContain("first-catalog"); + }); + } + + test("per_turn forwards changed catalogs", async () => { + const config = fixture("anthropic"); + config.skills = { catalog_refresh: "per_turn" }; + await send(config, "first-catalog", "turn-mode"); + expect(await send(config, "edited-catalog", "turn-mode")).toContain("edited-catalog"); + }); + + test("session header aliases reuse the same snapshot", async () => { + const config = fixture("anthropic"); + await send(config, "session-catalog", undefined, "outside", { session_id: "session-a" }); + const next = await send(config, "edited-catalog", undefined, "outside", { "session-id": "session-a" }); + expect(next).toContain("session-catalog"); + expect(next).not.toContain("edited-catalog"); + expect(await send(config, "new-session-catalog", undefined, "outside", { session_id: "session-b" })) + .toContain("new-session-catalog"); + }); + + test("requests without a conversation identity never share catalogs", async () => { + const config = fixture("anthropic"); + await send(config, "first-catalog"); + expect(await send(config, "edited-catalog")).toContain("edited-catalog"); + }); + + test("a parent-only routing identity cannot share sibling catalogs", async () => { + const config = fixture("anthropic"); + const headers = { "x-codex-parent-thread-id": "parent-without-child" }; + await send(config, "first-child-catalog", undefined, "outside", headers); + expect(await send(config, "second-child-catalog", undefined, "outside", headers)) + .toContain("second-child-catalog"); + }); +}); + +describe("skills catalog snapshot blockers from the #6027 review", () => { + function body(model: string, developerTexts: string[], extra: Record = {}) { + return JSON.stringify({ + model, stream: false, ...extra, + input: [ + ...developerTexts.map(text => ({ role: "developer", content: [{ type: "input_text", text }] })), + { role: "user", content: "hi" }, + ], + }); + } + async function raw( + config: OcxConfig, model: string, developerTexts: string[], thread: string, extra: Record = {}, + ) { + const response = await handleResponses(new Request("http://localhost/v1/responses", { + method: "POST", + headers: { "content-type": "application/json", "thread-id": thread }, + body: body(model, developerTexts, extra), + }), config, { model: "", provider: "" }); + await response.text(); + return { status: response.status, sent: captured.at(-1) }; + } + const block = (name: string) => `${name}`; + + test("a body with two catalog blocks passes through and pins nothing", async () => { + const config = fixture("openai-responses"); + const two = await raw(config, "fixture/fixture-model", [block("alpha"), block("beta")], "multi"); + expect(two.sent).toContain("alpha"); + expect(two.sent).toContain("beta"); + const one = await raw(config, "fixture/fixture-model", [block("gamma")], "multi"); + expect(one.sent).toContain("gamma"); + // A later two-block body is not rewritten to the stored catalog either. + const again = await raw(config, "fixture/fixture-model", [block("delta"), block("epsilon")], "multi"); + expect(again.sent).toContain("delta"); + expect(again.sent).toContain("epsilon"); + expect(again.sent).not.toContain("gamma"); + }); + + test("a rejected first request pins nothing", async () => { + const config = fixture("openai-responses"); + // The catalog is inspected before parsing; this body then fails parsing (tools must be an array). + const rejected = await raw(config, "fixture/fixture-model", [block("rejected")], "reject-first", { tools: "x" }); + expect(rejected.status).toBe(400); + const first = await raw(config, "fixture/fixture-model", [block("accepted")], "reject-first"); + expect(first.sent).toContain("accepted"); + const second = await raw(config, "fixture/fixture-model", [block("edited")], "reject-first"); + expect(second.sent).toContain("accepted"); + expect(second.sent).not.toContain("edited"); + }); + + test("anonymous callers share only on a server that requires no data-plane auth", () => { + const req = new Request("http://localhost/v1/responses", { headers: { "thread-id": "anon" } }); + const open = { ...getDefaultConfig(), hostname: "127.0.0.1" } as OcxConfig; + const remote = { ...getDefaultConfig(), hostname: "0.0.0.0" } as OcxConfig; + expect(resolveSkillsSnapshotScopeKey({ req, config: open, admission: { kind: "loopback", source: "loopback" } })) + .not.toBeNull(); + expect(resolveSkillsSnapshotScopeKey({ req, config: open })).not.toBeNull(); + expect(resolveSkillsSnapshotScopeKey({ req, config: remote })).toBeNull(); + }); +});