From 5a5edf9d5bd8f32f2228bf56be1d5badc65e76df Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 27 Sep 2026 14:21:41 +0900 Subject: [PATCH 01/12] docs(devlog): plan merge train round 3 batch 1 --- .../_plan/260927_merge_train_3/000_roadmap.md | 30 +++++++++++++++++++ .../_plan/260927_merge_train_3/010_batch1.md | 30 +++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 devlog/_plan/260927_merge_train_3/000_roadmap.md create mode 100644 devlog/_plan/260927_merge_train_3/010_batch1.md diff --git a/devlog/_plan/260927_merge_train_3/000_roadmap.md b/devlog/_plan/260927_merge_train_3/000_roadmap.md new file mode 100644 index 00000000000..a4a56c65274 --- /dev/null +++ b/devlog/_plan/260927_merge_train_3/000_roadmap.md @@ -0,0 +1,30 @@ +# Merge train round 3 — roadmap + +Inventory at `dev` `99d0a9400e` (2026-09-27, after round 2 in `devlog/_plan/260927_merge_train_2/`). Round 2's +outcome carries forward: its owner-decision list (#5831, #5964, #5956, #5995, #5800, #5912, #5879) and its blocked list +(#5977 unauthenticated status proof, #5893 Bun NO_PROXY patterns, #5927 security, #5925 needs a split, #5953 overbroad +override, #5539, #5497, #5947, #5782, #4222 and the stale feature drafts) stay out of this lane unless their authors +changed the premise. + +Goal: land the open bug fixes and non-GUI enhancements that opened after round 2's inventory, close what they resolve, +and bring open issues and PRs to at most 40 each (61 issues and 80 PRs at the start). One lane, serialized batches, +each rebased on the newest `dev`. + +Every carried PR is one squashed commit with the author or a `Co-authored-by` trailer. Each item's GitHub page is read +through Aside's signed-in browser (captures in `.tmp/aside/`, gitignored) before it is carried or closed. Kimi +subagents review each PR and audit each batch diff; auth, credential and link-relay changes get a dedicated security +review whose specifics stay in scratch. + +## Batches + +| Batch | PRs | Issues | +|---|---|---| +| B1 | #6041, #6019, #6015, #6011, #6006, #6026, #6034 (security review) | #6033, #6017, #6014, #4191 (only if the fix, not just a pin, lands), #6005, #5960, #6032 | +| B2 | luvs01 non-GUI bug fixes: #6048, #6047, #6046, #6038, #6036, #6035, #6057; #6022 (mdwsk88) | per PR | +| B3 | #6020 or #6056 (same quota-activation code; pick one), #6027, rebased #6050 #6049 #6037, #6042 (security review), #6030, #6003 | #6018, #5569 | +| B4 | bugs found through Aside that have no PR yet, implemented in this lane | per issue | + +## Out of scope + +GUI changes: #6043, #6007, #5983, #5905, #5950, #6025, #6010, and the GUI feature drafts. #6044 is a conflicting draft +that also edits a GUI test. diff --git a/devlog/_plan/260927_merge_train_3/010_batch1.md b/devlog/_plan/260927_merge_train_3/010_batch1.md new file mode 100644 index 00000000000..0a4ca773a5c --- /dev/null +++ b/devlog/_plan/260927_merge_train_3/010_batch1.md @@ -0,0 +1,30 @@ +# B1 — small bug fixes with owner-filed issues + +Base: `dev` `99d0a9400e`. Branch `codex/train3-b1`. + +| PR | Author | Issue | Change | Risk | +|---|---|---|---|---| +| #6041 | Ingwannu | #6033 | `abort_restart()` restores the pre-update wanted intent instead of forcing `wanted = true` (desktop/src-tauri exit.rs, updater.rs) | Tauri; hosted macOS/Windows/Linux desktop jobs | +| #6019 | Ingwannu | #6017 | macOS ACL parser stops trusting the `user:0`/`root` display name as root identity | plugin trust; must only tighten | +| #6015 | Ingwannu | #6014 | picker route test binds real listeners instead of probing then releasing ports | test only plus a runtime option | +| #6011 | Ingwannu | #4191 | pins established-WebSocket failure behavior with a test and ADR | issue closes only if behavior is fixed | +| #6006 | Ingwannu | #6005 | translated Anthropic output schemas claim `strict` only when strict-eligible | adapter contract | +| #6026 | codingbooo | #5960 | `ocx models` derives catalog price estimates when no manual price is set | CLI output | +| #6034 | Ingwannu | #6032 | link relay strips provider credential headers | security boundary; dedicated review | + +## Method + +1. Kimi review per PR (verdict LAND / LAND-WITH-FIXES / HOLD); #6034 also gets the security verdict. +2. Squash each PR onto the branch in the table order, preserving the author; fold review fixes as separate commits. +3. Reconcile test-layout registries and the file-size ratchet once for the batch. +4. Local: `bun install`, `bun run typecheck`, the focused test files each PR touches, `bun run structure:check`, + `bun run privacy:scan`. +5. Push, open the batch PR with the template, wait for exact-head CI, merge with `--admin --merge + --match-head-commit`, then close the source PRs and issues with the merge commit. + +## Aside evidence + +Captured to `.tmp/aside/` for every PR and issue above. All seven issues are owner-filed today with reproduction and +code pointers that match the PR premises. #4191's thread records the owner's position (Sep 21) that an established +WebSocket dying mid-turn is a failed leg rather than an SSE fallback, plus two contributor data sets (Sep 23) asking for +a fallback, so a test-and-ADR PR does not by itself resolve that issue. From 1c9f1fb015c56ead44d58161dd309e5ea8b36a01 Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Sun, 27 Sep 2026 14:21:42 +0900 Subject: [PATCH 02/12] test(claude): bind picker proxies without port probes (#6015) Carried from #6015 into merge train round 3. Co-authored-by: Ingwannu --- src/claude/intercept/runtime.ts | 7 +++- structure/clients/claude-desktop.md | 4 ++ .../claude-desktop-picker-routes.test.ts | 41 +++++++++---------- 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/claude/intercept/runtime.ts b/src/claude/intercept/runtime.ts index 82cd133ea16..4ca539ec070 100644 --- a/src/claude/intercept/runtime.ts +++ b/src/claude/intercept/runtime.ts @@ -117,6 +117,8 @@ export interface StartClaudeInterceptOptions { loadPickerRoutes?: () => Promise; /** Test seam: builds the picker runtime. */ createPicker?: (options: CreatePickerRuntimeOptions) => PickerRuntime; + /** Test seam: bind real CONNECT handlers on kernel-assigned ports without probe-and-release races. */ + startProxy?: typeof startConnectProxy; /** Test seams: the macOS `security` runner and platform for the picker runtime and controller. */ pickerSecurity?: SecurityRunner; pickerPlatform?: NodeJS.Platform; @@ -132,6 +134,7 @@ export async function startClaudeIntercept(options: StartClaudeInterceptOptio if (options.requestedPort === 0 && !explicitPort) return null; const configDir = options.configDir ?? getConfigDir(); const ca = await ensureLocalInterceptCaForStartup(configDir); + const startProxy = options.startProxy ?? startConnectProxy; const authToken = ensureClaudeInterceptProxyToken(configDir); const leaf = issueLocalInterceptLeaf(ca, CLAUDE_INTERCEPT_HOSTS); // Refresh an env we already own (e.g. a pre-auth proxy URL left by an upgrade) before the @@ -156,7 +159,7 @@ export async function startClaudeIntercept(options: StartClaudeInterceptOptio }); let proxy: ConnectProxyHandle; try { - proxy = await startConnectProxy(claudeInterceptProxyPort(options.config, options.publicPort), { + proxy = await startProxy(claudeInterceptProxyPort(options.config, options.publicPort), { interceptPort: listener.port!, // A real apply may recreate a missing token while this listener remains live. // Read current validated authority per CONNECT; absent/invalid means deny, not mint. @@ -187,7 +190,7 @@ export async function startClaudeIntercept(options: StartClaudeInterceptOptio const runtime = picker; const interceptPort = listener.port!; try { - pickerProxy = await startConnectProxy(claudePickerProxyPort(options.config, options.publicPort), { + pickerProxy = await startProxy(claudePickerProxyPort(options.config, options.publicPort), { interceptPort, // No authToken: Desktop's egressProxyUrl cannot present proxy credentials, so this // listener stays an unauthenticated loopback relay until the profile format can carry diff --git a/structure/clients/claude-desktop.md b/structure/clients/claude-desktop.md index 6f038c5dab5..d8dc2451c43 100644 --- a/structure/clients/claude-desktop.md +++ b/structure/clients/claude-desktop.md @@ -152,6 +152,10 @@ Code, trusting only the intercept CA) gets the `api.anthropic.com` intercept and blind, never the picker; a tunnel with Chromium's `Mozilla/` User-Agent (the app, trusting only the login keychain) is asked of the picker runtime (`src/claude/intercept/picker-runtime.ts`), which blind-tunnels every target except `claude.ai:443`. +Production always uses the configured adjacent ports. Lifecycle tests inject only the CONNECT +factory and bind the real handlers on kernel-assigned ports; this preserves request handling while +avoiding the false reservation created by probing and closing a port pair before the ephemeral TLS +listener starts. The injected factory does not change production port selection. The User-Agent is a routing hint, not a trust boundary: a client that fakes it reaches only what any local process already reaches (the `api.anthropic.com` intercept is on the Claude Code proxy too; the `claude.ai` relay verifies upstream and adds no credential) and breaks only its own TLS, diff --git a/tests/claude-integration/claude-desktop-picker-routes.test.ts b/tests/claude-integration/claude-desktop-picker-routes.test.ts index 9111785e46c..2e395d53c43 100644 --- a/tests/claude-integration/claude-desktop-picker-routes.test.ts +++ b/tests/claude-integration/claude-desktop-picker-routes.test.ts @@ -1,9 +1,9 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; -import { createServer } from "node:net"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { applyDesktopPickerProfile, inspectDesktopPickerProfile } from "../../src/claude/desktop-picker-profile"; +import { startConnectProxy } from "../../src/claude/intercept/connect-proxy"; import { pickerCaCertPath, pickerCaFingerprints } from "../../src/claude/intercept/picker-ca"; import type { PickerListenerOptions } from "../../src/claude/intercept/picker-listener"; import { createPickerRuntime } from "../../src/claude/intercept/picker-runtime"; @@ -21,6 +21,7 @@ let handle: ClaudeInterceptHandle | null = null; const previous: Record = {}; const ENV_KEYS = ["OPENCODEX_HOME", "OPENCODEX_CLAUDE_DESKTOP_CONFIG_DIR", "CLAUDE_CONFIG_DIR"] as const; const LISTENER_PORT = 45_679; +const REQUESTED_PROXY_PORT = 45_600; const INTERCEPT = { kind: "intercept", port: LISTENER_PORT }; const BLIND = { kind: "blind" }; @@ -49,28 +50,12 @@ const security: SecurityRunner = async args => { } }; -async function canBind(port: number): Promise { - return new Promise(resolve => { - const server = createServer(); - server.once("error", () => resolve(false)); - server.listen({ port, host: "127.0.0.1", exclusive: true }, () => server.close(() => resolve(true))); - }); -} - -async function freePortPair(): Promise { - for (let attempt = 0; attempt < 50; attempt += 1) { - const port = 20_000 + Math.floor(Math.random() * 30_000); - if (await canBind(port) && await canBind(port + 1)) return port; - } - throw new Error("no free port pair"); -} - /** Start the intercept pair with picker mode wired, as the server lifecycle does. */ async function startPicker(saved: OcxConfig, onDispatch?: (req: Request) => Response): Promise { writeFileSync(join(root, "config.json"), JSON.stringify(saved)); - const port = await freePortPair(); + const requestedProxyPorts: number[] = []; handle = await startClaudeIntercept({ - config: config({ claudeCode: { intercept: { port } } }), + config: config({ claudeCode: { intercept: { port: REQUESTED_PROXY_PORT } } }), publicPort: 10100, configDir: root, dispatch: async req => onDispatch?.(req) ?? new Response("unused"), @@ -78,6 +63,13 @@ async function startPicker(saved: OcxConfig, onDispatch?: (req: Request) => Resp loadPickerRoutes: async () => ({ nativeSlugs: [], routedModels: [{ provider: "xai", id: "grok-4.7", contextWindow: 256_000 }] }), pickerSecurity: security, pickerPlatform: "darwin", + // A probe that closes before startup does not reserve anything: the lifecycle's own + // ephemeral TLS listener or another process can take the observed pair. Bind the real proxy + // handlers directly on port 0 so the kernel owns both allocations until teardown. + startProxy: async (requestedPort, proxyOptions) => { + requestedProxyPorts.push(requestedPort); + return startConnectProxy(0, proxyOptions); + }, createPicker: options => createPickerRuntime({ ...options, startListener: (async (_: PickerListenerOptions) => ({ port: LISTENER_PORT, close: async () => {} })) as never, @@ -85,7 +77,14 @@ async function startPicker(saved: OcxConfig, onDispatch?: (req: Request) => Resp refreshIntervalMs: 3_600_000, }), }); - return port; + if (!handle || handle.pickerProxyPort === null || !getClaudePickerRuntime()) { + throw new Error("picker fixture did not start every runtime component"); + } + expect(requestedProxyPorts).toEqual([REQUESTED_PROXY_PORT, REQUESTED_PROXY_PORT + 1]); + const boundPorts = [handle.listener.port, handle.proxyPort, handle.pickerProxyPort]; + expect(boundPorts.every(port => typeof port === "number" && Number.isInteger(port) && port > 0)).toBe(true); + expect(new Set(boundPorts).size).toBe(boundPorts.length); + return handle.pickerProxyPort; } async function dispatch(path: string, init: RequestInit = {}, deps: Parameters[3] = {}) { @@ -146,7 +145,7 @@ describe("first-party turns picker mode on by default", () => { expect(applied.status).toBe(200); expect(applied.body.picker).toMatchObject({ effective: true, reason: "restart_required", trust: "trusted", profile: "applied" }); expect(decision()).toEqual(INTERCEPT); - expect(inspectDesktopPickerProfile()).toMatchObject({ kind: "applied", proxyUrl: `http://127.0.0.1:${port + 1}` }); + expect(inspectDesktopPickerProfile()).toMatchObject({ kind: "applied", proxyUrl: `http://127.0.0.1:${port}` }); expect(keychain.trusted).toBe(true); expect(persisted().claudeCode?.intercept?.picker).toBeUndefined(); From 5747a5c4b9dbe44faef7cbd2213d7abe9a21fe35 Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Sun, 27 Sep 2026 14:21:44 +0900 Subject: [PATCH 03/12] fix(link): strip provider credentials at relay (#6034) Carried from #6034 into merge train round 3. Co-authored-by: Ingwannu --- .../src/content/docs/guides/remote-link.md | 2 +- src/client/link-relay.ts | 5 ++- ...ADR-6032-link-relay-credential-boundary.md | 12 ++++++ structure/remote-link.md | 4 +- tests/clients/client-link-relay.test.ts | 38 ++++++++++++++++--- 5 files changed, 53 insertions(+), 8 deletions(-) create mode 100644 structure/decisions/ADR-6032-link-relay-credential-boundary.md diff --git a/docs-site/src/content/docs/guides/remote-link.md b/docs-site/src/content/docs/guides/remote-link.md index 38037482146..cf5973216fb 100644 --- a/docs-site/src/content/docs/guides/remote-link.md +++ b/docs-site/src/content/docs/guides/remote-link.md @@ -73,7 +73,7 @@ When a step fails, the dashboard shows the reason and, when SSH reported one, th ## Security -The Child uses the Home computer's providers and provider credentials through the link. The Home creates a separate link key for each Child; removing the link revokes that key. On the Child, the key stays inside OpenCodex: credentials that Codex or Claude Code send there are not forwarded to the Home, and any program on the Child that reaches `127.0.0.1:` uses the Home without a key, the same local trust a standalone install gives. Web pages from other sites are refused. Compare the host fingerprint before confirmation so a wrong machine or changed host key is not accepted by mistake. Dashboard sessions issued from a Tailscale identity cannot manage machine links. +The Child uses the Home computer's providers and provider credentials through the link. The Home creates a separate link key for each Child; removing the link revokes that key. On the Child, the key stays inside OpenCodex: credentials that Codex or Claude Code send there are not forwarded to the Home, including Bearer, Azure `api-key`, Anthropic-compatible `x-api-key`, and Google `x-goog-api-key` forms. Any program on the Child that reaches `127.0.0.1:` uses the Home without a key, the same local trust a standalone install gives. Web pages from other sites are refused. Compare the host fingerprint before confirmation so a wrong machine or changed host key is not accepted by mistake. Dashboard sessions issued from a Tailscale identity cannot manage machine links. ## CLI reference diff --git a/src/client/link-relay.ts b/src/client/link-relay.ts index 41fcd0f8eb2..d2a155631b8 100644 --- a/src/client/link-relay.ts +++ b/src/client/link-relay.ts @@ -93,7 +93,10 @@ const RESPONSE_OMITTED_HEADERS = new Set(["content-encoding", "content-length"]) * Caller credentials never cross the tunnel. The Child's own ChatGPT or Anthropic credential * stays on the Child, and the Home sees exactly one admission: the link key. */ -const CALLER_CREDENTIAL_HEADERS = ["authorization", "x-api-key", "x-opencodex-api-key", "chatgpt-account-id", "cookie"] as const; +const CALLER_CREDENTIAL_HEADERS = [ + "authorization", "api-key", "x-api-key", "x-goog-api-key", "x-opencodex-api-key", + "chatgpt-account-id", "cookie", +] as const; function jsonError(status: number, error: string, retry = false): Response { const headers = retry ? { "Retry-After": String(LINK_RELAY_RETRY_AFTER_SECONDS) } : undefined; diff --git a/structure/decisions/ADR-6032-link-relay-credential-boundary.md b/structure/decisions/ADR-6032-link-relay-credential-boundary.md new file mode 100644 index 00000000000..d7861d0d8ae --- /dev/null +++ b/structure/decisions/ADR-6032-link-relay-credential-boundary.md @@ -0,0 +1,12 @@ +# ADR-6032 — Child link relay credential boundary + +- Contract owner: [Remote Link](../remote-link.md) + +## Decision record + +- Purpose and intent: Ensure a Child sends only its link admission key across the machine tunnel, never a caller's provider credential. +- Existing implementation and constraints: The relay already removed Bearer, `x-api-key`, OpenCodex, account and cookie credentials before attaching the link key. Built-in Azure and Google adapters use the independent `api-key` and `x-goog-api-key` forms, which were not in that denylist and therefore survived ordinary end-to-end header forwarding. +- Alternatives considered: Strip every header containing `key` or `token`; reuse the broad log-redaction regex; extend the relay's explicit list with the provider credential forms it actually supports. +- Chosen approach: Add `api-key` and `x-goog-api-key` to the case-insensitive explicit relay denylist and exercise them through both header construction and the actual fetch boundary. +- Why this approach: A broad name heuristic could remove legitimate protocol headers such as `idempotency-key`. The explicit list closes the proven built-in adapter paths while preserving ordinary request metadata and the existing link-key wire contract. +- Benefits, costs and impact: Azure and Google caller keys remain on the Child, matching the existing Anthropic/OpenAI behavior. Custom credential header names still require deliberate review before they become supported provider authentication forms. diff --git a/structure/remote-link.md b/structure/remote-link.md index 18bf22612e9..e4edc2419e0 100644 --- a/structure/remote-link.md +++ b/structure/remote-link.md @@ -54,6 +54,8 @@ Codex keeps the standalone loopback routing: `routingTarget` in `src/client/conn `src/client/link-ingress.ts` is the link-mode data plane of the machine listener. A `/v1/responses` WebSocket upgrade answers `426 upgrade_required`, which codex-rs maps to its HTTP fallback, and no upgrade is ever relayed. Every relayed route first passes the standalone loopback Host and Origin gate (`isAllowedRequestOrigin` in `src/server/auth-cors.ts`), so a rebinding or cross-site page gets `403 origin_rejected` and nothing is fetched upstream. `/readyz` is answered locally. The link key is read from the service token file once, when the listener starts, and held in memory; the file must hold the key whose fingerprint the connection committed. While it does not, relayed routes answer `503 link_credential_unavailable` without an upstream fetch, and the file is read again at most once a second; a valid key is never re-read. The key is never logged or returned. -`src/client/link-relay.ts` forwards exactly the `linkRouteAllowed` routes from `src/link/routes.ts` through the tunnel. It drops the caller's `Authorization`, `x-api-key`, `x-opencodex-api-key`, `chatgpt-account-id` and `cookie` and sends the link key as `Authorization: Bearer`, the wire an `env_key` config sent; `GET /v1/usage` takes it as `x-opencodex-api-key`, the only header that route admits. The Home admits the key and serves the Child with its own accounts. The request body is streamed chunk by chunk with the caller's `Content-Length` and a byte-counting cap at the inbound limit (`resolveInboundBodyLimitBytes`, 256 MiB by default); a larger declared or streamed body answers 413. A lone `Transfer-Encoding: chunked` without `Content-Length` is admitted as a standalone admits it, because the listener has already de-chunked the body; any other Transfer-Encoding, or one next to a `Content-Length`, answers 400. The Home's response headers may take up to 300 seconds, and a caller abort ends the wait sooner. SSE passes through chunk by chunk with caller-abort propagation and a 300-second idle limit, other response bodies stream under the same byte cap, and the relay answers 503 with Retry-After while the tunnel is down. The client supervisor is the relay's tunnel gate (`LinkTunnelGate`): only while the tunnel is connecting or reconnecting (including the start of the client runtime) does a relayed request wait, for at most 15 seconds (`LINK_RELAY_HOLD_MS`) from its first wait and with at most 64 requests waiting, before it is forwarded once; a connected tunnel costs one `pending()` call per request, and a failed one answers 503 at once. A forward whose connection was refused sent nothing, so while the tunnel reconnects it may wait again and be sent again inside the same 15 seconds, provided the streamed body was never read or cancelled; any other failure (a reset, a timeout, a failure after the body started) is never replayed. Both the Child's machine listener and the Home's hub-link listener bind with `idleTimeout: 255`, the public listener's limit, so a held or slow turn is not cut by Bun's 10-second default. Like a standalone data route, a relayed request then lifts its own idle timer (`server.timeout(req, 0)` in `src/client/link-ingress.ts`), so a quiet stretch longer than 255 seconds inside a long generation is not cut either; the relay's header deadline, SSE idle limit and caller abort bound the wait instead. Hub transport keeps the 4 MiB management-relay listener bound and its default idle limit. Link mode waits for the configured port without signalling its holder, then binds there or fails; `src/client/runtime.ts` passes the cached link key, tunnel status and tunnel gate through `bindClientListener` to every bind attempt. Link mode turns the management relay off and refuses key rotation and revocation, which belong to the hub. +`src/client/link-relay.ts` forwards exactly the `linkRouteAllowed` routes from `src/link/routes.ts` through the tunnel. It drops the caller's `Authorization`, Azure `api-key`, Anthropic-compatible `x-api-key`, Google `x-goog-api-key`, `x-opencodex-api-key`, `chatgpt-account-id` and `cookie` and sends the link key as `Authorization: Bearer`, the wire an `env_key` config sent; `GET /v1/usage` takes it as `x-opencodex-api-key`, the only header that route admits. The explicit provider forms matter because the Child and Home are separate credential owners: caller provider credentials never cross the tunnel merely because they are not Bearer tokens. The Home admits the link key and serves the Child with its own accounts. The request body is streamed chunk by chunk with the caller's `Content-Length` and a byte-counting cap at the inbound limit (`resolveInboundBodyLimitBytes`, 256 MiB by default); a larger declared or streamed body answers 413. A lone `Transfer-Encoding: chunked` without `Content-Length` is admitted as a standalone admits it, because the listener has already de-chunked the body; any other Transfer-Encoding, or one next to a `Content-Length`, answers 400. The Home's response headers may take up to 300 seconds, and a caller abort ends the wait sooner. SSE passes through chunk by chunk with caller-abort propagation and a 300-second idle limit, other response bodies stream under the same byte cap, and the relay answers 503 with Retry-After while the tunnel is down. The client supervisor is the relay's tunnel gate (`LinkTunnelGate`): only while the tunnel is connecting or reconnecting (including the start of the client runtime) does a relayed request wait, for at most 15 seconds (`LINK_RELAY_HOLD_MS`) from its first wait and with at most 64 requests waiting, before it is forwarded once; a connected tunnel costs one `pending()` call per request, and a failed one answers 503 at once. A forward whose connection was refused sent nothing, so while the tunnel reconnects it may wait again and be sent again inside the same 15 seconds, provided the streamed body was never read or cancelled; any other failure (a reset, a timeout, a failure after the body started) is never replayed. Both the Child's machine listener and the Home's hub-link listener bind with `idleTimeout: 255`, the public listener's limit, so a held or slow turn is not cut by Bun's 10-second default. Like a standalone data route, a relayed request then lifts its own idle timer (`server.timeout(req, 0)` in `src/client/link-ingress.ts`), so a quiet stretch longer than 255 seconds inside a long generation is not cut either; the relay's header deadline, SSE idle limit and caller abort bound the wait instead. Hub transport keeps the 4 MiB management-relay listener bound and its default idle limit. Link mode waits for the configured port without signalling its holder, then binds there or fails; `src/client/runtime.ts` passes the cached link key, tunnel status and tunnel gate through `bindClientListener` to every bind attempt. Link mode turns the management relay off and refuses key rotation and revocation, which belong to the hub. + +> Decision record: [ADR-6032](decisions/ADR-6032-link-relay-credential-boundary.md) Regression coverage lives in `tests/clients/link-ssh-argv.test.ts`, `tests/clients/link-ssh-config.test.ts`, `tests/clients/link-tunnel-state.test.ts`, `tests/clients/link-store.test.ts`, `tests/clients/link-boundary.test.ts`, `tests/clients/link-routes.test.ts`, `tests/clients/client-link-connect.test.ts`, `tests/clients/client-link-relay.test.ts`, `tests/clients/client-machine-listener.test.ts`, `tests/clients/client-link-status.test.ts`, `tests/clients/client-link-runtime.test.ts`, `tests/codex-integration/injection-link-websocket.test.ts`, `tests/clients/link-supervisor.test.ts`, `tests/clients/link-status-projection.test.ts`, `tests/clients/link-admission-wait.test.ts`, `tests/clients/link-fingerprint.test.ts`, `tests/cli/cli-link.test.ts`, `tests/server/link-management-routes.test.ts`, `tests/server/link-join-route.test.ts`, `tests/server/link-listener-lifecycle.test.ts`, `tests/clients/client-link-teardown.test.ts` and `gui/tests/remote-link.test.tsx`. diff --git a/tests/clients/client-link-relay.test.ts b/tests/clients/client-link-relay.test.ts index a8cd6fce202..01bd7c0969f 100644 --- a/tests/clients/client-link-relay.test.ts +++ b/tests/clients/client-link-relay.test.ts @@ -106,10 +106,13 @@ describe("client link HTTP relay", () => { test("replaces every caller credential with the link key and filters hop-by-hop headers", () => { const caller = new Headers({ Authorization: "Bearer caller-chatgpt-oauth", + "Api-Key": "azure-caller", "X-OpenCodex-API-Key": "ocx_data_caller", "X-Api-Key": "sk-ant-caller", + "X-Goog-Api-Key": "google-caller", "ChatGPT-Account-Id": "acct-caller", Cookie: "session=caller", + "Idempotency-Key": "idem-1", "X-Trace": "trace-1", Connection: "keep-alive, X-Remove", "X-Remove": "secret", @@ -118,15 +121,18 @@ describe("client link HTTP relay", () => { }); const forwarded = forwardLinkRequestHeaders(caller, LINK_KEY, "/v1/responses"); expect(forwarded.get("authorization")).toBe(`Bearer ${LINK_KEY}`); + expect(forwarded.get("idempotency-key")).toBe("idem-1"); expect(forwarded.get("x-trace")).toBe("trace-1"); - for (const name of ["x-opencodex-api-key", "x-api-key", "chatgpt-account-id", "cookie", "connection", "keep-alive", "x-remove", "host", "content-length"]) { + for (const name of ["api-key", "x-opencodex-api-key", "x-api-key", "x-goog-api-key", "chatgpt-account-id", "cookie", "connection", "keep-alive", "x-remove", "host", "content-length"]) { expect(forwarded.get(name)).toBeNull(); } // /v1/usage admits only the dedicated header on the Home. const usage = forwardLinkRequestHeaders(caller, LINK_KEY, "/v1/usage"); expect(usage.get("x-opencodex-api-key")).toBe(LINK_KEY); expect(usage.get("authorization")).toBeNull(); + expect(usage.get("api-key")).toBeNull(); expect(usage.get("x-api-key")).toBeNull(); + expect(usage.get("x-goog-api-key")).toBeNull(); const response = sanitizeLinkResponseHeaders(new Headers({ Connection: "X-Response-Secret", @@ -146,18 +152,32 @@ describe("client link HTTP relay", () => { const fetchImpl = (async (_input, init) => { sent.push(new Headers(init?.headers)); return Response.json({ ok: true }); }) as typeof fetch; const response = await relayLinkDataRequest(relayRequest({ method: "POST", - headers: { Authorization: "Bearer caller-chatgpt-oauth", "ChatGPT-Account-Id": "acct-caller", "Content-Type": "application/json" }, + headers: { + Authorization: "Bearer caller-chatgpt-oauth", + "Api-Key": "azure-caller", + "X-Goog-Api-Key": "google-caller", + "ChatGPT-Account-Id": "acct-caller", + "Content-Type": "application/json", + }, body: "{}", }), target, { fetchImpl }); expect(response.status).toBe(200); expect(sent[0]?.get("authorization")).toBe(`Bearer ${LINK_KEY}`); + expect(sent[0]?.get("api-key")).toBeNull(); + expect(sent[0]?.get("x-goog-api-key")).toBeNull(); expect(sent[0]?.get("chatgpt-account-id")).toBeNull(); const usage = await relayLinkDataRequest(new Request("http://127.0.0.1:10100/v1/usage", { - headers: { Authorization: "Bearer caller-chatgpt-oauth" }, + headers: { + Authorization: "Bearer caller-chatgpt-oauth", + "Api-Key": "azure-caller", + "X-Goog-Api-Key": "google-caller", + }, }), target, { fetchImpl }); expect(usage.status).toBe(200); expect(sent[1]?.get("x-opencodex-api-key")).toBe(LINK_KEY); expect(sent[1]?.get("authorization")).toBeNull(); + expect(sent[1]?.get("api-key")).toBeNull(); + expect(sent[1]?.get("x-goog-api-key")).toBeNull(); }); test("rejects TE/CL ambiguity and oversized requests before outbound I/O", async () => { @@ -387,6 +407,8 @@ describe("client link HTTP relay", () => { path: new URL(req.url).pathname + new URL(req.url).search, host: req.headers.get("host"), authorization: req.headers.get("authorization"), + apiKey: req.headers.get("api-key"), + googleApiKey: req.headers.get("x-goog-api-key"), dedicated: req.headers.get("x-opencodex-api-key"), contentLength: req.headers.get("content-length"), transferEncoding: req.headers.get("transfer-encoding"), @@ -401,14 +423,20 @@ describe("client link HTTP relay", () => { const body = JSON.stringify({ input: "hello" }); const response = await fetch(new URL("/v1/responses?trace=1", machine.url), { method: "POST", - headers: { "Content-Type": "application/json", Authorization: "Bearer caller-chatgpt-oauth", "X-OpenCodex-API-Key": "ocx_data_caller" }, + headers: { + "Content-Type": "application/json", + Authorization: "Bearer caller-chatgpt-oauth", + "Api-Key": "azure-caller", + "X-Goog-Api-Key": "google-caller", + "X-OpenCodex-API-Key": "ocx_data_caller", + }, body, }); expect(response.status).toBe(200); expect(await response.json()).toEqual({ relayed: true }); expect(received).toEqual({ method: "POST", path: "/v1/responses?trace=1", host: `127.0.0.1:${hub.port}`, - authorization: `Bearer ${LINK_KEY}`, dedicated: null, + authorization: `Bearer ${LINK_KEY}`, apiKey: null, googleApiKey: null, dedicated: null, contentLength: String(body.length), transferEncoding: null, body, }); expect((await fetch(new URL("/v1/unknown", machine.url))).status).toBe(404); From 5f4784c21cda95277b5b0f72025680be2369e0ff Mon Sep 17 00:00:00 2001 From: codingbo Date: Sun, 27 Sep 2026 14:21:45 +0900 Subject: [PATCH 04/12] fix(cli): derive catalog price estimates for models without manual overrides (#6026) Carried from #6026 into merge train round 3. Co-authored-by: codingbo --- .../docs/reference/cli/providers-accounts.md | 12 +++++++-- src/cli/models-runtime.ts | 9 +++++-- src/cli/models.ts | 7 ++++- structure/runtime.md | 2 +- tests/cli/cli-models-price.test.ts | 27 +++++++++++++++++-- tests/cli/cli-models.test.ts | 23 ++++++++++++++++ 6 files changed, 72 insertions(+), 8 deletions(-) diff --git a/docs-site/src/content/docs/reference/cli/providers-accounts.md b/docs-site/src/content/docs/reference/cli/providers-accounts.md index e344104d868..9805136a375 100644 --- a/docs-site/src/content/docs/reference/cli/providers-accounts.md +++ b/docs-site/src/content/docs/reference/cli/providers-accounts.md @@ -657,6 +657,14 @@ catalog entries; `enable`, `disable`, and `provider` control visibility; `select provider allowlist; `context` controls provider context caps; and `shadow` manages background shadow-call interception. +Model prices are estimates in USD per million tokens. `ocx models --json` includes a +`price` object with `cost4` rates and their source; `ocx models price --json` keeps +`cost` for the saved override and reports resolved rates in `effectiveCost`. +Manual prices (including zero) take precedence, followed by the shared catalog and +verified official-price fallbacks. Unknown models return `null`; no price is invented. +Automatic defaults are derived on read and do not populate `modelCosts` in your config, +so catalog updates remain effective. Use `set-price` to save provider-specific rates. + Every per-model operation the dashboard offers is available here, so a headless install never needs the GUI to manage a catalog. `add`, `remove`, and `list-custom` work against the config file and apply to a running proxy through a catalog sync; the rest talk to the live management API and require the @@ -664,9 +672,9 @@ proxy to be running (`ocx start`, or an installed service). | Subcommand | Supported flags | Action | | --- | --- | --- | -| `list` (default) | `--provider `, `--json` | List models seeded in configured providers. | +| `list` (default) | `--provider `, `--json` | List models seeded in configured providers, with estimated input/output prices. | | `live` | `--provider `, `--json` | Read the running catalog, including models discovered at runtime. Rows are flagged `native`/`routed`, `custom`, and `enabled`/`disabled`. | -| `price ` | `--json` | Read the model's saved manual price override; no override means automatic pricing. | +| `price ` | `--json` | Read the saved manual override and effective price, including automatic catalog defaults. | | `set-price ` | `--input `, `--output `, `--cache-read `, `--cache-write `, `--auto`, `--json` | Set display prices in USD per 1M tokens. Input/output are required when setting; omitted cache rates become zero. `--auto` removes only this model's override. | | `add ` | `--display-name `, `--context-window `, `--modalities ` | Register a model the provider catalog does not advertise. | | `edit ` | `--model-id `, `--display-name `, `--context-window `, `--modalities `, `--json` | Edit a custom model. `-` clears a field; `0` clears the context window. | diff --git a/src/cli/models-runtime.ts b/src/cli/models-runtime.ts index 1be92b6c301..ad753f3002f 100644 --- a/src/cli/models-runtime.ts +++ b/src/cli/models-runtime.ts @@ -18,6 +18,7 @@ import { isValidProviderName } from "../config/provider-name"; import { isValidModelDiscoveryModelId } from "../providers/model-discovery-limits"; import { redactSecretString } from "../lib/redact"; import type { ProviderCostOverlay } from "../types"; +import { resolveMatchedPrice } from "../usage/cost"; import { MAX_COST4_RATE } from "../usage/expected-prices"; import { isValidCost4Rate } from "../usage/user-cost-overlays"; @@ -124,8 +125,12 @@ async function priceRequest(write: boolean, argv: string[], deps: RuntimeApiDeps if (!validPriceCost(stored)) throw new Error("Invalid model price response"); cost = { ...stored }; } - printData({ provider, modelId, cost }, wantsJson, [ - cost === null ? `${selector}: automatic pricing` : `${selector}: ${JSON.stringify(cost)} USD per 1M tokens`, + // The API map owns manual overrides; bundled defaults remain derived rather + // than being persisted as overrides that would mask later catalog updates. + const effectiveCost = cost ?? resolveMatchedPrice(provider, modelId, undefined, [])?.cost4 ?? null; + printData({ provider, modelId, cost, effectiveCost }, wantsJson, [ + effectiveCost === null ? `${selector}: automatic pricing (unknown)` + : `${selector}: ${JSON.stringify(effectiveCost)} USD per 1M tokens${cost === null ? " (automatic estimate)" : ""}`, ]); return; } diff --git a/src/cli/models.ts b/src/cli/models.ts index 0f917964085..791dbf70dc2 100644 --- a/src/cli/models.ts +++ b/src/cli/models.ts @@ -1,6 +1,7 @@ /** * `ocx models` subcommand — list configured models and manage custom models. */ +import { resolveMatchedPrice, type MatchedPrice } from "../usage/cost"; import { randomUUID } from "node:crypto"; import { createInterface } from "node:readline/promises"; import { syncModelsToCodex } from "../codex/sync"; @@ -85,6 +86,7 @@ interface ModelEntry { contextWindow: number | null; inputModalities: string[] | null; reasoningEfforts: string[] | null; + price: MatchedPrice | null; } /** @@ -132,6 +134,7 @@ function collectModels(config: OcxConfig, providerFilter?: string): ModelEntry[] contextWindow: configuredContextWindow(prov, model) ?? null, inputModalities: modalities, reasoningEfforts: efforts, + price: resolveMatchedPrice(provName, model), }); }; @@ -428,7 +431,9 @@ function handleConfiguredModels(args: string[]): void { for (const m of provModels) { const marker = m.isDefault ? " *" : ""; const ctx = m.contextWindow ? ` (${Math.round(m.contextWindow / 1000)}k)` : ""; - console.log(` ${m.model}${marker}${ctx}`); + const rates = m.price?.cost4; + const pricing = rates ? ` ~$${rates.input}/$${rates.output} input/output per 1M tokens` : " price unknown"; + console.log(` ${m.model}${marker}${ctx}${pricing}`); } console.log(); } diff --git a/structure/runtime.md b/structure/runtime.md index 3abe289fc4a..693f9926b41 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -26,7 +26,7 @@ Native steering follows [the shared WebSocket contract](transports/streaming-hea Responses admission and finalization are composed through the [core module ownership](transports/responses.md#core-module-ownership). Kiro's optional account-load admission is process-local and request-owned; its slot ends with the response body or cancellation. Other providers retain their admission path. -Catalog HTTP acquisition follows the [proxy-routing contract](catalog.md#remote-catalog-http-proxy-routing). +Catalog HTTP acquisition follows the [proxy-routing contract](catalog.md#remote-catalog-http-proxy-routing). CLI model lists expose shared catalog estimates as `price`; `models price` retains the saved override in `cost` and adds `effectiveCost`. Explicit zero overrides win; unknown prices remain null, and automatic defaults are never persisted to `modelCosts`. Covered by `tests/cli/cli-models.test.ts` and `tests/cli/cli-models-price.test.ts`. OAuth refresh coordination follows the [refresh-lock identity contract](catalog.md#accounts-namespaces-and-pool-rotation): a fresh unreadable lock remains held, and release requires matching descriptor identity. A failed path-identity probe preserves the refresh callback outcome. Cooperating lock metadata changes serialize through the existing SQLite mutation transaction; release keeps the descriptor open through identity comparison and any unlink, then closes it. Failed metadata writes remove only a matching owned path after successful coordination; unknown identity, failed probes or unavailable coordination retain the path for stale recovery. Async refresh work holds no metadata transaction. diff --git a/tests/cli/cli-models-price.test.ts b/tests/cli/cli-models-price.test.ts index 9adda799770..50aa79c07a0 100644 --- a/tests/cli/cli-models-price.test.ts +++ b/tests/cli/cli-models-price.test.ts @@ -45,19 +45,42 @@ describe("models manual price commands", () => { }); expect(result.code).toBe(0); expect(result.calls).toEqual([{ path: "/api/providers/custom-price/model-costs", method: "GET", body: undefined }]); - expect(JSON.parse(result.stdout)).toEqual({ provider: "custom-price", modelId: "org/model--fast", cost: COST }); + expect(JSON.parse(result.stdout)).toEqual({ provider: "custom-price", modelId: "org/model--fast", cost: COST, effectiveCost: COST }); }); test("missing own keys read as automatic, including prototype-shaped selectors", async () => { for (const modelId of ["missing", "__proto__", "constructor", "toString"]) { const result = await invoke("price", [`custom-price/${modelId}`, "--json"], { provider: "custom-price", modelCosts: {} }); expect(result.code).toBe(0); - expect(JSON.parse(result.stdout)).toEqual({ provider: "custom-price", modelId, cost: null }); + expect(JSON.parse(result.stdout)).toEqual({ provider: "custom-price", modelId, cost: null, effectiveCost: null }); } const automatic = await invoke("price", ["custom-price/missing"], { provider: "custom-price", modelCosts: {} }); expect(automatic.stdout).toContain("automatic pricing"); }); + test("automatic pricing exposes known vendor rates without creating an override", async () => { + const result = await invoke("price", ["custom-price/claude-sonnet-4-6", "--json"], { + provider: "custom-price", modelCosts: {}, + }); + expect(result.code).toBe(0); + const row = JSON.parse(result.stdout); + expect(row.cost).toBeNull(); + expect(row.effectiveCost).toEqual({ input: 3, output: 15, cacheRead: 0.3, cacheWrite: 3.75 }); + expect(result.calls).toHaveLength(1); + }); + + test("explicit zero prices override known automatic rates", async () => { + const zero = { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }; + const result = await invoke("price", ["custom-price/claude-sonnet-4-6", "--json"], { + provider: "custom-price", modelCosts: { "claude-sonnet-4-6": zero }, + }); + expect(JSON.parse(result.stdout)).toMatchObject({ cost: zero, effectiveCost: zero }); + const automatic = await invoke("price", ["custom-price/claude-sonnet-4-6"], { + provider: "custom-price", modelCosts: {}, + }); + expect(automatic.stdout).toContain("USD per 1M tokens (automatic estimate)"); + }); + test("set-price sends four numeric rates with omitted cache rates defaulted to zero", async () => { const result = await invoke("set-price", ["custom-price/org/model", "--input", "1.25", "--output", "5", "--json"]); expect(result.code).toBe(0); diff --git a/tests/cli/cli-models.test.ts b/tests/cli/cli-models.test.ts index 14c426c97d1..fcc99236780 100644 --- a/tests/cli/cli-models.test.ts +++ b/tests/cli/cli-models.test.ts @@ -60,6 +60,29 @@ describe("ocx models", () => { await warmModuleGraph({ graph: "cli-index/models", entry: cliPath }); }, COLD_SPAWN_WARMUP_HOOK_BUDGET_MS); + test("configured models expose automatic prices, overrides and unknowns without persisting defaults", () => { + const { dir } = freshConfig({ providers: { relay: { + adapter: "openai-chat", baseUrl: "https://example.com/v1", + models: ["claude-sonnet-4-6", "gpt-4o", "unknown-model"], + modelCosts: { "gpt-4o": { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 } }, + } } }); + try { + const before = readFileSync(join(dir, "config.json"), "utf8"); + const result = runCli(["models", "--json"], { OPENCODEX_HOME: dir }); + expect(result.status).toBe(0); + const rows = JSON.parse(result.stdout).models; + expect(rows[0].price.cost4).toEqual({ input: 3, output: 15, cacheRead: 0.3, cacheWrite: 3.75 }); + expect(rows[1].price).toMatchObject({ source: "user", cost4: { input: 0, output: 0 } }); + expect(rows[2].price).toBeNull(); + expect(readFileSync(join(dir, "config.json"), "utf8")).toBe(before); + const human = runCli(["models"], { OPENCODEX_HOME: dir }); + expect(human.stdout).toContain("~$3/$15 input/output per 1M tokens"); + expect(human.stdout).toContain("price unknown"); + } finally { + removeTreeWithRetry(dir); + } + }); + test("models lists all provider models", () => { const { dir } = freshConfig(); try { From e89de8f5329fe43e53e88cd0e6200e91d4a4803c Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Sun, 27 Sep 2026 14:21:52 +0900 Subject: [PATCH 05/12] fix(plugins): do not trust ACL display names as root (#6019) Carried from #6019 into merge train round 3. Co-authored-by: Ingwannu --- .../src/content/docs/guides/local-plugins.md | 7 +++--- src/plugins/loader.ts | 8 ++++--- structure/ops/plugins.md | 8 ++++--- tests/lib/plugin-loader.test.ts | 24 ++++++++++++++++++- 4 files changed, 37 insertions(+), 10 deletions(-) diff --git a/docs-site/src/content/docs/guides/local-plugins.md b/docs-site/src/content/docs/guides/local-plugins.md index b952aa91419..fee1e551152 100644 --- a/docs-site/src/content/docs/guides/local-plugins.md +++ b/docs-site/src/content/docs/guides/local-plugins.md @@ -31,9 +31,10 @@ Put plugin files in `plugins/` inside the opencodex home (`~/.opencodex/plugins/ `chmod go-w ~/.opencodex/plugins ~/.opencodex/plugins/*`; on systems whose default umask is `002`, check the parent directories too. On macOS, an ACL grant to another user or group that can write, delete, change permissions, or add/remove path entries blocks loading, even if the - mode is `0600`; inspect with `ls -le`. Read-only, deny, inheritance-only, and grants only to - the path owner, the running user, or root do not block loading. On Linux, extended ACLs are - checked when `getfacl` is installed. Without it, only owner and mode bits are verified. + mode is `0600`; inspect the path itself with `/bin/ls -lebd -- `. Read-only, deny, + inheritance-only, and grants only to the path owner or running user do not block loading. ACL + display names such as `root` or `0` are not treated as numeric UID proof. On Linux, extended + ACLs are checked when `getfacl` is installed. Without it, only owner and mode bits are verified. - On Windows automatic plugin loading is disabled until an ACL trust check is available. Restart the proxy after adding, changing or removing a plugin (`ocx service restart`, or stop and diff --git a/src/plugins/loader.ts b/src/plugins/loader.ts index b1b3e37adac..a9c2ffc57c7 100644 --- a/src/plugins/loader.ts +++ b/src/plugins/loader.ts @@ -81,10 +81,12 @@ export function macAclListingTrustError(listing: string, currentUser = userInfo( if (!entry) { unparseable = true; continue; } if (entry[2] === "deny") continue; const principal = entry[1]!; - // The file owner, this process's user, and root already control the path without an ACE. - // Require the `user:` prefix: a bare or group principal might include other users. + // The file owner and this process's user already control the path without an ACE. Require the + // `user:` prefix: a bare or group principal might include other users. macOS `ls` prints a + // resolved directory-record NAME here, so neither `0` nor `root` proves that the ACE is UID 0. + // On a genuinely root-owned path, `root` is still admitted by the owner comparison. if (principal.startsWith("user:") - && [owner, currentUser, "root", "0"].includes(principal.slice(5))) continue; + && [owner, currentUser].includes(principal.slice(5))) continue; const rights = entry[3]!.split(","); if (rights.includes("only_inherit")) continue; if (rights.some(right => !MAC_ACL_BENIGN_TOKENS.has(right))) return "has an access control list"; diff --git a/structure/ops/plugins.md b/structure/ops/plugins.md index 45886a169fb..263db4c938b 100644 --- a/structure/ops/plugins.md +++ b/structure/ops/plugins.md @@ -20,9 +20,11 @@ or signs them. can swap a checked path before it is imported; files are imported through the resolved directory. On macOS, `ls -lebd` must show no effective non-owner ACL grant that can write, delete, change permissions, or add/remove path entries on the file, plugin directory, or any ancestor. Denials, - grants only to the path owner, the running user, or root, read-only grants, and inheritance-only - entries on the inspected path are safe; inherited grants effective on a descendant are checked - at that descendant. A timed-out macOS inspection retries once only if its output is empty: + grants only to the path owner or running user, read-only grants, and inheritance-only entries on + the inspected path are safe; inherited grants effective on a descendant are checked at that + descendant. `ls` renders UUID-backed principals as Directory Services record names, so names + such as `root` or `0` never establish UID 0; root-owned paths still pass through the owner check. + A timed-out macOS inspection retries once only if its output is empty: observed unsafe grants refuse immediately, and any other partial output is incomplete and also refuses loading. Unknown grants or other inspection errors also refuse loading. Linux uses `getfacl` when installed and refuses extended diff --git a/tests/lib/plugin-loader.test.ts b/tests/lib/plugin-loader.test.ts index 1ec5dda1fde..48078a3a8b3 100644 --- a/tests/lib/plugin-loader.test.ts +++ b/tests/lib/plugin-loader.test.ts @@ -137,9 +137,31 @@ test("recorded macOS ls output rejects effective non-owner write grants", () => + " 0: group:everyone allow write\n"; const ownerNamedGroup = "drwx------@ 2 runner staff 64 Sep 27 07:50 /private/var/folders/ab/tmp/plugins\n" + " 0: group:runner allow add_file\n"; - for (const listing of [pluginDir, ownedAncestor, inheritedChild, pluginFile, ownerNamedGroup]) { + // `/bin/ls -lebd` renders resolved ACL record names, not numeric UIDs. A foreign record named + // `0` must not inherit root trust merely because its name looks like UID 0 (#6017). + const numericRecordName = "-rw-------@ 1 runner staff 64 Sep 27 07:50 /plugins/plugin.ts\n" + + " 0: user:0 allow write\n"; + const rootRecordName = "-rw-------@ 1 runner staff 64 Sep 27 07:50 /plugins/plugin.ts\n" + + " 0: user:root allow write\n"; + const unresolvedUuid = "-rw-------@ 1 runner staff 64 Sep 27 07:50 /plugins/plugin.ts\n" + + " 0: user:8D95C9F2-3B29-4B30-8932-C43D3AABC123 allow write\n"; + for (const listing of [ + pluginDir, ownedAncestor, inheritedChild, pluginFile, ownerNamedGroup, + numericRecordName, rootRecordName, unresolvedUuid, + ]) { expect(macAclListingTrustError(listing)).toBe("has an access control list"); } + // Bare principals are not identity-bearing user records. Keep the rights policy explicit so a + // future parser cleanup cannot accidentally grant them the owner/current-user exemption. + const bareBenignPrincipal = "-rw-------@ 1 runner staff 64 Sep 27 07:50 /plugins/plugin.ts\n" + + " 0: runner allow read\n"; + expect(macAclListingTrustError(bareBenignPrincipal)).toBeNull(); + const bareWritePrincipal = "-rw-------@ 1 runner staff 64 Sep 27 07:50 /plugins/plugin.ts\n" + + " 0: runner allow write\n"; + expect(macAclListingTrustError(bareWritePrincipal)).toBe("has an access control list"); + const numericCurrentUser = "-rw-------@ 1 0 staff 64 Sep 27 07:50 /plugins/plugin.ts\n" + + " 0: user:0 allow write\n"; + expect(macAclListingTrustError(numericCurrentUser, "0")).toBeNull(); expect(macAclListingTrustError("drwxr-xr-x+ 23 root wheel 736 Sep 27 07:50 /\n 0: unrecognized ACL entry\n")) .toBe("access control list inspection failed"); expect(macAclListingTrustError(`${pluginDir.split("\n")[0]}\n 0: group:everyone allow future_permission\n`)) From 14913d98ad16752933e5817a9d2022021289fd77 Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 27 Sep 2026 14:22:01 +0900 Subject: [PATCH 06/12] test(plugins): pin the current user in the foreign ACL principal cases --- tests/lib/plugin-loader.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/lib/plugin-loader.test.ts b/tests/lib/plugin-loader.test.ts index 48078a3a8b3..fc89aa93a75 100644 --- a/tests/lib/plugin-loader.test.ts +++ b/tests/lib/plugin-loader.test.ts @@ -149,7 +149,9 @@ test("recorded macOS ls output rejects effective non-owner write grants", () => pluginDir, ownedAncestor, inheritedChild, pluginFile, ownerNamedGroup, numericRecordName, rootRecordName, unresolvedUuid, ]) { - expect(macAclListingTrustError(listing)).toBe("has an access control list"); + // Pin the current user so a runner whose login is `root` or a record named `0` cannot turn + // these foreign-principal refusals into the current-user exemption. + expect(macAclListingTrustError(listing, "runner")).toBe("has an access control list"); } // Bare principals are not identity-bearing user records. Keep the rights policy explicit so a // future parser cleanup cannot accidentally grant them the owner/current-user exemption. From 91267a34bef55315749d87ea69e53f152ec86365 Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Sun, 27 Sep 2026 14:22:06 +0900 Subject: [PATCH 07/12] fix(desktop): preserve stopped intent after update failure (#6041) Carried from #6041 into merge train round 3. Co-authored-by: Ingwannu --- desktop/src-tauri/src/exit.rs | 105 ++++++++++++++++-- desktop/src-tauri/src/updater.rs | 30 ++++- .../ADR-6033-desktop-update-intent.md | 12 ++ structure/desktop-shell.md | 12 +- 4 files changed, 145 insertions(+), 14 deletions(-) create mode 100644 structure/decisions/ADR-6033-desktop-update-intent.md diff --git a/desktop/src-tauri/src/exit.rs b/desktop/src-tauri/src/exit.rs index eac40099499..5ee8e4bff10 100644 --- a/desktop/src-tauri/src/exit.rs +++ b/desktop/src-tauri/src/exit.rs @@ -146,6 +146,14 @@ pub struct Supervision { pub reason_set: bool, } +/// State restored when an update's coordinated restart is abandoned. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct AbortedRestart { + pub phase: ExitPhase, + /// Whether the person wanted a runtime before the drain or requested one while it ran. + pub runtime_was_wanted: bool, +} + impl Supervision { /// Nothing is in flight, nobody asked for the runtime to stop, and the app is not ending. pub fn allowed(self) -> bool { @@ -161,6 +169,14 @@ struct Inner { deferred: bool, /// See [`Supervision::wanted`]. Sticky: finishing a stop does not restore it. wanted: bool, + /// The intent an update temporarily replaced with `wanted=false`; consumed if it aborts. + restart_wanted: Option, +} + +fn remember_restart_intent(inner: &mut Inner, reason: ExitReason, prior_wanted: bool) { + if reason == ExitReason::CoordinatedRestart { + inner.restart_wanted.get_or_insert(prior_wanted); + } } /// The exit sequence's state, managed by the app. @@ -179,6 +195,7 @@ impl ExitCoordinator { hides_to_tray: TrayAvailability::assumed().hides_to_tray(), deferred: false, wanted: true, + restart_wanted: None, }), } } @@ -215,17 +232,20 @@ impl ExitCoordinator { /// [`ExitCoordinator::finish_stop`] is holding the phase. pub fn claim_drain(&self, fallback: ExitReason) -> Option { let mut inner = self.inner(); + let prior_wanted = inner.wanted; // A quit or an update is on its way, whoever ends up running the drain: the runtime it // stops is not one to bring back. inner.wanted = false; match inner.phase { ExitPhase::Idle => { let reason = *inner.reason.get_or_insert(fallback); + remember_restart_intent(&mut inner, reason, prior_wanted); inner.phase = ExitPhase::Draining; Some(reason) } ExitPhase::Spawning | ExitPhase::Stopping => { - inner.reason.get_or_insert(fallback); + let reason = *inner.reason.get_or_insert(fallback); + remember_restart_intent(&mut inner, reason, prior_wanted); inner.deferred = true; None } @@ -235,6 +255,7 @@ impl ExitCoordinator { // is the one thing a user with a runtime that would not stop cannot easily do. ExitPhase::DrainFailed | ExitPhase::OwnershipUnknown => { let reason = *inner.reason.get_or_insert(fallback); + remember_restart_intent(&mut inner, reason, prior_wanted); inner.phase = ExitPhase::Draining; Some(reason) } @@ -256,10 +277,12 @@ impl ExitCoordinator { /// An update drains before it installs. When the install then fails, or the drain itself did, /// the drain's phase used to be the end of the road: `Drained` is terminal, so no runtime could /// be started again and a bare window close quit the app. This returns the app to `Idle` with - /// no claimed reason and wants a runtime again, which is what a successful update would have - /// ended in too. It touches nothing unless an update's restart holds the phase: a quit is never - /// aborted, and a drain still running belongs to whoever runs it. Returns the phase it left. - pub fn abort_restart(&self) -> Option { + /// no claimed reason and restores the runtime intent the update temporarily suppressed. A + /// retry requested while the drain was in flight wins too: aborting an older update must not + /// overwrite newer user intent. It touches nothing unless an update's restart holds the phase: + /// a quit is never aborted, and a + /// drain still running belongs to whoever runs it. Returns the phase and restored intent. + pub fn abort_restart(&self) -> Option { let mut inner = self.inner(); let left = inner.phase; let restart = inner.reason == Some(ExitReason::CoordinatedRestart); @@ -273,8 +296,15 @@ impl ExitCoordinator { inner.phase = ExitPhase::Idle; inner.reason = None; inner.deferred = false; - inner.wanted = true; - Some(left) + // `resume` can arrive after the update captured its original intent. Preserve that newer + // request as well as the older snapshot; otherwise the abort races the startup retry and + // can leave a runtime stopped even though the person just asked for it. + let runtime_was_wanted = inner.wanted || inner.restart_wanted.take().unwrap_or(false); + inner.wanted = runtime_was_wanted; + Some(AbortedRestart { + phase: left, + runtime_was_wanted, + }) } /// What the runtime supervisor reads before it acts. @@ -624,7 +654,7 @@ fn hide_windows(app: &AppHandle) { #[cfg(test)] mod tests { use super::{ - decide, DrainVerdict, ExitCoordinator, ExitDecision, ExitPhase, ExitReason, + decide, AbortedRestart, DrainVerdict, ExitCoordinator, ExitDecision, ExitPhase, ExitReason, RestartReadiness, Supervision, }; use crate::tray_availability::TrayAvailability; @@ -707,7 +737,13 @@ mod tests { ); coordinator.finish_drain(verdict); let left = coordinator.phase(); - assert_eq!(coordinator.abort_restart(), Some(left)); + assert_eq!( + coordinator.abort_restart(), + Some(AbortedRestart { + phase: left, + runtime_was_wanted: true, + }) + ); assert_eq!(coordinator.phase(), ExitPhase::Idle); // A bare close hides again instead of quitting out of a terminal phase. assert_eq!(coordinator.decision(), ExitDecision::Hide); @@ -716,6 +752,57 @@ mod tests { } } + #[test] + fn a_failed_update_preserves_a_completed_tray_stop() { + let coordinator = ExitCoordinator::new(); + coordinator.set_tray(TrayAvailability::Available); + assert!(coordinator.begin_stop()); + assert_eq!(coordinator.finish_stop(), None); + assert!(!coordinator.supervision().wanted); + + assert_eq!( + coordinator.claim_drain(ExitReason::CoordinatedRestart), + Some(ExitReason::CoordinatedRestart) + ); + coordinator.finish_drain(DrainVerdict::Drained); + assert_eq!( + coordinator.abort_restart(), + Some(AbortedRestart { + phase: ExitPhase::Drained, + runtime_was_wanted: false, + }) + ); + assert_eq!(coordinator.phase(), ExitPhase::Idle); + assert_eq!(coordinator.decision(), ExitDecision::Hide); + assert!(!coordinator.supervision_allowed()); + } + + #[test] + fn a_startup_retry_during_an_update_drain_is_not_overwritten_by_abort() { + let coordinator = ExitCoordinator::new(); + coordinator.set_tray(TrayAvailability::Available); + assert!(coordinator.begin_stop()); + assert_eq!(coordinator.finish_stop(), None); + assert!(!coordinator.supervision().wanted); + + assert_eq!( + coordinator.claim_drain(ExitReason::CoordinatedRestart), + Some(ExitReason::CoordinatedRestart) + ); + coordinator.resume(); + // The ending claim still prevents supervision until the failed update is handed back. + assert!(!coordinator.supervision_allowed()); + coordinator.finish_drain(DrainVerdict::Drained); + assert_eq!( + coordinator.abort_restart(), + Some(AbortedRestart { + phase: ExitPhase::Drained, + runtime_was_wanted: true, + }) + ); + assert!(coordinator.supervision_allowed()); + } + #[test] fn a_quit_or_a_drain_in_flight_is_never_aborted() { let coordinator = ExitCoordinator::new(); diff --git a/desktop/src-tauri/src/updater.rs b/desktop/src-tauri/src/updater.rs index 9fd3595e5be..bcaf3349533 100644 --- a/desktop/src-tauri/src/updater.rs +++ b/desktop/src-tauri/src/updater.rs @@ -1,5 +1,5 @@ use crate::{ - exit::{ExitCoordinator, ExitPhase, RestartReadiness}, + exit::{AbortedRestart, ExitCoordinator, ExitPhase, RestartReadiness}, logging, tray, }; use serde::Serialize; @@ -422,8 +422,13 @@ pub async fn install(app: &AppHandle, update: Update) -> Result<(), String> { /// Hand a failed install back to a running app. True when the drain had already stopped the /// runtime, so the startup sequence has to bring one back; a drain that failed left it running. +/// Intent captured before the drain and a newer startup retry are both authoritative. fn after_install_failure(coordinator: &ExitCoordinator) -> bool { - coordinator.abort_restart() == Some(ExitPhase::Drained) + coordinator.abort_restart() + == Some(AbortedRestart { + phase: ExitPhase::Drained, + runtime_was_wanted: true, + }) } fn recover_after_failed_install(app: &AppHandle) { @@ -507,6 +512,7 @@ mod tests { DesktopUpdateState, InstallClaim, UiProjection, }; use crate::exit::{DrainVerdict, ExitCoordinator, ExitDecision, ExitReason}; + use crate::tray_availability::TrayAvailability; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{mpsc, Arc}; use tauri_utils::config::BundleType; @@ -533,6 +539,26 @@ mod tests { coordinator.finish_drain(DrainVerdict::Drained); assert!(!after_install_failure(&coordinator)); assert_eq!(coordinator.decision(), ExitDecision::Proceed); + + // A completed tray Stop remains the person's intent across repeated failed updates. + let coordinator = ExitCoordinator::new(); + coordinator.set_tray(TrayAvailability::Available); + assert!(coordinator.begin_stop()); + assert_eq!(coordinator.finish_stop(), None); + for _ in 0..2 { + coordinator.claim_drain(ExitReason::CoordinatedRestart); + coordinator.finish_drain(DrainVerdict::Drained); + assert!(!after_install_failure(&coordinator)); + assert!(!coordinator.supervision_allowed()); + assert_eq!(coordinator.decision(), ExitDecision::Hide); + } + + // A newer retry wins over the stopped intent that the update captured at claim time. + coordinator.claim_drain(ExitReason::CoordinatedRestart); + coordinator.resume(); + coordinator.finish_drain(DrainVerdict::Drained); + assert!(after_install_failure(&coordinator)); + assert!(coordinator.supervision_allowed()); } #[test] diff --git a/structure/decisions/ADR-6033-desktop-update-intent.md b/structure/decisions/ADR-6033-desktop-update-intent.md new file mode 100644 index 00000000000..98dcb3d9a44 --- /dev/null +++ b/structure/decisions/ADR-6033-desktop-update-intent.md @@ -0,0 +1,12 @@ +# ADR-6033 — failed desktop updates preserve runtime intent + +- Contract owner: [Desktop shell](../desktop-shell.md) + +## Decision record + +- Purpose and intent: Recover from a failed in-app update without undoing a person's earlier tray Stop. +- Existing implementation and constraints: A coordinated restart clears `wanted` before draining so the supervisor cannot race the update. Its abort path then unconditionally restored `wanted=true`, and the updater treated every `Drained` result as a request for immediate recovery. When the runtime was already tray-stopped, no child still produced `Drained`, so installer failure restarted a runtime that had been intentionally left off. +- Alternatives considered: Never recover after installer failure; infer intent from whether a child PID existed; snapshot the pre-drain `wanted` value inside the exit coordinator. +- Chosen approach: Capture pre-drain intent under the coordinator mutex when a coordinated restart first claims the sequence. When that settled restart is aborted, combine and consume the snapshot with any newer Resume request received during the drain. Immediate updater recovery requires both a `Drained` phase and this effective `wanted=true` intent. +- Why this approach: Process presence does not express user intent: an already-stopped runtime and one drained by the update are both absent. The coordinator is the existing authority for sticky Stop/Resume intent. Combining the captured value with its current value preserves newer user intent without a second race-prone read. +- Benefits, costs and impact: Failed updates still recover a runtime they stopped, while tray-stopped sessions remain idle unless the person explicitly retries startup during the drain. Download failures, successful installer restarts, quit ownership, and in-flight drains are unchanged. The snapshot is process-local and intentionally does not persist across a successful application restart. diff --git a/structure/desktop-shell.md b/structure/desktop-shell.md index 9bc2d8db553..55673192b64 100644 --- a/structure/desktop-shell.md +++ b/structure/desktop-shell.md @@ -168,8 +168,13 @@ restart asked for after `install` is never reached, and the package would be rep runtime still serving out of those files. A drain that did not complete refuses the install and leaves the update pending. Neither that refusal nor an install that fails after the drain strands the app: `ExitCoordinator::abort_restart` takes a coordinated restart's settled drain phase back to -idle with no claimed reason, so a close hides again and Quit works, and when the drain had stopped -the runtime the startup sequence brings one back in recovery mode. A quit's drain is never aborted. +idle with no claimed reason, so a close hides again and Quit works. When the drain had stopped the +runtime and it was wanted before the update **or** requested again while draining, the startup +sequence brings one back in recovery mode. A runtime already stopped from the tray stays stopped after a failed update unless the person +explicitly requests startup while that update drain is in flight; that newer request wins over the +captured stopped intent. A quit's drain is never aborted. + +> Decision record: [ADR-6033](decisions/ADR-6033-desktop-update-intent.md) The Tauri updater also publishes a bounded desktop snapshot over its identity-bound ProxyClient. A random process-session id travels in the embedded dashboard URL, and the dashboard requests GET /api/update/badge?surface=desktop&session=. A normal browser keeps the package badge. The shell posts each updater-state change and a 60-second heartbeat; if the proxy loses the snapshot or the shell stops, the desktop badge becomes unknown after 180 seconds. This display path never installs an update or replaces the signed Tauri result. The tray shows the same pending state: macOS draws a blue child NSView dot over the template status-item image; Windows/Linux swap a generated dotted PNG when a tray host exists. The Windows base glyph is unchanged. @@ -263,7 +268,8 @@ unreadable answers never count. It covers guest runtimes and an exit event that The exit coordinator's `wanted` intent keeps this from fighting the person. It is true from launch; the tray's Stop (when it takes the phase), a quit's drain and an update's drain clear it before the runtime's exit can arrive, finishing a stop does not restore it, and the failure page's retry sets it -again. A terminal +again. A coordinated update remembers the intent it temporarily clears: an aborted update restores a +previously wanted runtime, but never turns a completed tray Stop back on. A terminal `ocx stop` of the runtime this app started clears nothing, so the app starts it again after the backoff; the tray's Stop and Quit keep it stopped. The dashboard's own Stop, in the app's window or a browser, is refused with `desktop_supervised` while the app supervises the runtime From ead97d2e36383d7891a4b2c9ffda1e3be62557fd Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Sun, 27 Sep 2026 14:23:23 +0900 Subject: [PATCH 08/12] fix(claude): validate translated strict output schemas (#6006) Carried from #6006 into merge train round 3. Co-authored-by: Ingwannu --- .../src/content/docs/reference/adapters.md | 8 + scripts/test-layout/layout.json | 1 + src/adapters/anthropic-output-schema.ts | 117 +++++++++- src/claude/inbound-model-options.ts | 12 +- src/claude/inbound.ts | 2 +- ...slated-output-schema-strict-eligibility.md | 13 ++ structure/providers/chat-compat.md | 17 +- .../claude-output-schema-strict.test.ts | 215 ++++++++++++++++++ tests/fixtures/test-layout-expected.json | 1 + 9 files changed, 362 insertions(+), 24 deletions(-) create mode 100644 structure/decisions/ADR-5901-translated-output-schema-strict-eligibility.md create mode 100644 tests/claude-integration/claude-output-schema-strict.test.ts diff --git a/docs-site/src/content/docs/reference/adapters.md b/docs-site/src/content/docs/reference/adapters.md index a77af2706ce..5f15be73148 100644 --- a/docs-site/src/content/docs/reference/adapters.md +++ b/docs-site/src/content/docs/reference/adapters.md @@ -256,6 +256,14 @@ MiMo model Command Code serves. local reference remains resolvable. OpenAI envelope fields such as schema `name`, envelope `description`, and `strict` are not part of the Anthropic wire format. JSON object mode without a schema has no Anthropic equivalent and is not translated. + In the reverse direction, translated Anthropic output schemas retain the caller's original + schema. `strict: true` requires an object root without a root union and complete closed objects + throughout nested properties, array items, unions and definitions: `properties` must be an object, + `additionalProperties` must be `false`, and `required` must contain exactly its property names. + Incomplete objects, unknown schema keywords, and values outside OpenAI's documented strict + subset use explicit `strict: false`; the classifier uses an allowlist rather than chasing each + unsupported constraint separately. The proxy does not invent + required fields, close an open object, or discard the caller's schema merely to obtain strict mode. - Always sends `anthropic-version: 2023-06-01`. Streams `content_block_delta` (`text_delta`, `thinking_delta`, compatible `reasoning_delta`, `input_json_delta`). The SSE decoder preserves event state across fetch chunks and accepts a terminal `message_stop` without a trailing newline. diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index aecc89fbbc0..4856cd23048 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -477,6 +477,7 @@ "claude-inbound-cache-stabilize.test.ts": "claude-integration", "claude-inbound-debug.test.ts": "claude-integration", "claude-inbound.test.ts": "claude-integration", + "claude-output-schema-strict.test.ts": "claude-integration", "claude-intercept-integration.test.ts": "server", "claude-intercept-local-ca.test.ts": "claude-integration", "claude-intercept-model-bindings.test.ts": "claude-integration", diff --git a/src/adapters/anthropic-output-schema.ts b/src/adapters/anthropic-output-schema.ts index 122e89a8869..bc1758b30e3 100644 --- a/src/adapters/anthropic-output-schema.ts +++ b/src/adapters/anthropic-output-schema.ts @@ -136,8 +136,59 @@ export function isAnthropicOutputSchema(schema: Record): boolea } } +// Anthropic accepts `uri`, but OpenAI strict Structured Outputs documents a narrower +// string-format set. Keep this separate from SUPPORTED_STRING_FORMATS: changing the Anthropic +// normalizer to satisfy the translated OpenAI route would silently remove native guidance. +const OPENAI_STRICT_STRING_FORMATS = new Set([ + "date-time", "time", "date", "duration", "email", "hostname", "ipv4", "ipv6", "uuid", +]); + +const OPENAI_STRICT_COMMON_KEYWORDS = new Set([ + "$defs", "definitions", "$ref", "type", "title", "description", "enum", "const", "anyOf", +]); +const OPENAI_STRICT_OBJECT_KEYWORDS = new Set([ + "properties", "patternProperties", "required", "additionalProperties", +]); +const OPENAI_STRICT_STRING_KEYWORDS = new Set(["pattern", "format"]); +const OPENAI_STRICT_NUMBER_KEYWORDS = new Set([ + "multipleOf", "maximum", "exclusiveMaximum", "minimum", "exclusiveMinimum", +]); +const OPENAI_STRICT_ARRAY_KEYWORDS = new Set(["items", "minItems", "maxItems"]); +const OPENAI_STRICT_TYPES = new Set(["string", "number", "boolean", "integer", "object", "array", "null"]); +// Fine-tuned models implement a narrower documented Structured Outputs subset. Falling back to +// non-strict keeps the caller's schema intact; deleting these constraints would silently widen it. +const OPENAI_FINE_TUNED_UNSUPPORTED_KEYWORDS = new Set([ + "patternProperties", "pattern", "format", + ...OPENAI_STRICT_NUMBER_KEYWORDS, + "minItems", "maxItems", +]); + +function openAiStrictSchemaTypes(value: unknown): Set | null { + if (value === undefined) return new Set(); + const values = typeof value === "string" ? [value] : value; + if (!Array.isArray(values) || values.length === 0 + || values.some(type => typeof type !== "string" || !OPENAI_STRICT_TYPES.has(type))) return null; + const types = new Set(values); + if (types.size !== values.length) return null; + // OpenAI documents unions through anyOf; the type-array shorthand is reserved for nullable fields. + if (types.size > 1 && (types.size !== 2 || !types.has("null"))) return null; + return types; +} + +function hasOnlyOpenAiStrictKeywords(node: Record, types: Set): boolean { + for (const key of Object.keys(node)) { + if (OPENAI_STRICT_COMMON_KEYWORDS.has(key)) continue; + if (types.has("object") && OPENAI_STRICT_OBJECT_KEYWORDS.has(key)) continue; + if (types.has("string") && OPENAI_STRICT_STRING_KEYWORDS.has(key)) continue; + if ((types.has("number") || types.has("integer")) && OPENAI_STRICT_NUMBER_KEYWORDS.has(key)) continue; + if (types.has("array") && OPENAI_STRICT_ARRAY_KEYWORDS.has(key)) continue; + return false; + } + return true; +} + /** - * Does every object in this schema list ALL of its properties as required? + * Can this schema preserve its object contract under OpenAI strict mode? * * OpenAI's structured-output strict mode demands exactly that, and rejects anything else with * `'required' is required to be supplied and to be an array including every key in properties`. @@ -148,26 +199,70 @@ export function isAnthropicOutputSchema(schema: Record): boolea * would silently change the contract the caller asked for, so the only honest answer is to stop * claiming strict for these schemas -- the schema is still sent and still honoured as guidance. */ -export function satisfiesOpenAiStrictSchema(value: unknown): boolean { - if (Array.isArray(value)) return value.every(satisfiesOpenAiStrictSchema); - if (!value || typeof value !== "object") return true; - const node = value as Record; - // `allOf` is not supported under strict Structured Outputs at all, wherever it appears. - if ("allOf" in node) return false; +export function satisfiesOpenAiStrictSchema(value: unknown, fineTuned = false): boolean { + // Callers pass one schema node. Boolean/null/array schemas are outside the documented + // Structured Outputs subset; schema lists such as anyOf are validated explicitly below. + if (!isRecord(value)) return false; + const node = value; + // This is a compatibility proof, so it fails closed on unknown schema keywords instead of + // maintaining an inevitably incomplete denylist. The original schema is still forwarded. + const types = openAiStrictSchemaTypes(node.type); + if (!types || !hasOnlyOpenAiStrictKeywords(node, types)) return false; + if (fineTuned && Object.keys(node).some(key => OPENAI_FINE_TUNED_UNSUPPORTED_KEYWORDS.has(key))) { + return false; + } + if (Object.hasOwn(node, "$ref") && typeof node.$ref !== "string") return false; + if (Object.hasOwn(node, "title") && typeof node.title !== "string") return false; + if (Object.hasOwn(node, "description") && typeof node.description !== "string") return false; + if (Object.hasOwn(node, "enum") && (!Array.isArray(node.enum) || node.enum.length === 0)) return false; + if (Object.hasOwn(node, "format")) { + if (typeof node.format !== "string" || !OPENAI_STRICT_STRING_FORMATS.has(node.format)) { + return false; + } + } + if (Object.hasOwn(node, "pattern") && typeof node.pattern !== "string") return false; + for (const key of OPENAI_STRICT_NUMBER_KEYWORDS) { + const constraint = node[key]; + if (Object.hasOwn(node, key) && (typeof constraint !== "number" || !Number.isFinite(constraint))) return false; + } + for (const key of ["minItems", "maxItems"]) { + const constraint = node[key]; + if (Object.hasOwn(node, key) + && (typeof constraint !== "number" || !Number.isInteger(constraint) || constraint < 0)) return false; + } const properties = node.properties; - if (isRecord(properties)) { + const objectType = types.has("object"); + if (objectType || Object.hasOwn(node, "properties")) { // An object node must list every property in `required` AND close itself to extras. The // caller's schema is forwarded verbatim -- `isAnthropicOutputSchema` normalizes a CLONE for // its own acceptance check -- so an object that never said `additionalProperties: false` // reaches the wire without it and is refused, however complete its `required` is. - if (node.additionalProperties !== false) return false; + // Anthropic's acceptance probe fills a missing/malformed property map on its clone; + // that must not certify the unchanged bare object which actually reaches the wire. + if (!isRecord(properties) || node.additionalProperties !== false) return false; // Strict mode also requires `required` to be supplied at all, even for an empty // `properties` map, so a missing array is not the same as an empty one. if (!Array.isArray(node.required)) return false; const keys = Object.keys(properties); const required: unknown[] = node.required; const requiredKeys = new Set(required); - if (keys.some(key => !requiredKeys.has(key))) return false; + if (required.length !== keys.length || keys.some(key => !requiredKeys.has(key))) return false; + } + // Walk schema positions, not arbitrary JSON values: a property named `not` or an enum/const + // value containing `properties` is data, not another schema node to certify or reject. + for (const key of ["properties", "$defs", "definitions", "patternProperties"]) { + if (!Object.hasOwn(node, key)) continue; + const entries = node[key]; + if (!isRecord(entries) + || !Object.values(entries).every(entry => satisfiesOpenAiStrictSchema(entry, fineTuned))) return false; + } + if (Object.hasOwn(node, "items") && !satisfiesOpenAiStrictSchema(node.items, fineTuned)) return false; + if (Object.hasOwn(node, "anyOf")) { + const anyOf = node.anyOf; + if (!Array.isArray(anyOf) || anyOf.length === 0 + || !anyOf.every(entry => satisfiesOpenAiStrictSchema(entry, fineTuned))) { + return false; + } } - return Object.values(node).every(satisfiesOpenAiStrictSchema); + return true; } diff --git a/src/claude/inbound-model-options.ts b/src/claude/inbound-model-options.ts index 3441d275d0d..40e32a3157b 100644 --- a/src/claude/inbound-model-options.ts +++ b/src/claude/inbound-model-options.ts @@ -96,7 +96,13 @@ export function effortFromOutputConfig(outputConfig: unknown): string | undefine return typeof effort === "string" && OUTPUT_CONFIG_EFFORTS.has(effort) ? effort : undefined; } -export function formatFromOutputConfig(outputConfig: unknown): Rec | undefined { +function isFineTunedOpenAiTarget(model: string | undefined): boolean { + if (!model) return false; + const separator = model.lastIndexOf("/"); + return model.slice(separator + 1).startsWith("ft:"); +} + +export function formatFromOutputConfig(outputConfig: unknown, resolvedModel?: string): Rec | undefined { if (!isRec(outputConfig) || !isRec(outputConfig.format)) return undefined; const format = outputConfig.format; if ( @@ -115,7 +121,9 @@ export function formatFromOutputConfig(outputConfig: unknown): Rec | undefined { name: "response", schema: format.schema, // Strict Structured Outputs also needs an object at the root; a root anyOf/oneOf is refused. - strict: format.schema.type === "object" && satisfiesOpenAiStrictSchema(format.schema), + strict: format.schema.type === "object" + && !Object.hasOwn(format.schema, "anyOf") && !Object.hasOwn(format.schema, "oneOf") + && satisfiesOpenAiStrictSchema(format.schema, isFineTunedOpenAiTarget(resolvedModel)), }; } diff --git a/src/claude/inbound.ts b/src/claude/inbound.ts index 2aba1fdc787..1b190193362 100644 --- a/src/claude/inbound.ts +++ b/src/claude/inbound.ts @@ -460,7 +460,7 @@ function translateAnthropicRequest( if (Array.isArray(raw.stop_sequences) && raw.stop_sequences.length > 0) { body.stop = raw.stop_sequences.filter((s): s is string => typeof s === "string"); } - const outputConfigFormat = formatFromOutputConfig(raw.output_config); + const outputConfigFormat = formatFromOutputConfig(raw.output_config, body.model as string); if (outputConfigFormat) body.text = { format: outputConfigFormat }; let cacheKeySource: ClaudeCacheKeySource = null; if (isRec(raw.metadata) && typeof raw.metadata.user_id === "string") { diff --git a/structure/decisions/ADR-5901-translated-output-schema-strict-eligibility.md b/structure/decisions/ADR-5901-translated-output-schema-strict-eligibility.md new file mode 100644 index 00000000000..937becbfdc7 --- /dev/null +++ b/structure/decisions/ADR-5901-translated-output-schema-strict-eligibility.md @@ -0,0 +1,13 @@ +# ADR-5901 — decision recorded under "Anthropic structured-output compatibility" + +- Contract owner: [providers/chat-compat.md](../providers/chat-compat.md#anthropic-structured-output-compatibility) + +## Decision record + +- Purpose and intent: Avoid asserting OpenAI strict-mode compatibility for an unchanged Anthropic output schema whose objects do not satisfy the strict object contract. +- Existing implementation and constraints: The Anthropic acceptance probe normalizes a clone, including missing property maps and object closure. Translation forwards the original schema. Checking only existing property maps therefore certified a bare object that the destination would reject. Generic JSON recursion also confused property names and literal example/default objects with schema nodes. +- Alternatives considered: Normalize the forwarded schema by inventing closure and required fields; reject all such Anthropic input; retain the original schema and classify strict eligibility separately. +- Chosen approach: Preserve the schema unchanged and emit explicit non-strict mode when an object lacks a valid property map, closure or an exact required-name set, or any schema node uses a keyword/value outside OpenAI's documented subset. Use a per-type allowlist so new unsupported keywords fail closed without growing a denylist. Validate nested schema positions and nullable object types without traversing literal enum/const data. A root union remains non-strict even when accompanied by an object type. After Claude alias/model-map resolution, apply the documented narrower fine-tuned subset recursively to direct or provider-qualified `ft:` targets. Do not claim that this ingress decision predicts later provider aliases or combo selection. Keep Anthropic's broader native format acceptance separate. +- Why this approach: The caller's optional fields and open-object contract must not silently narrow just to satisfy another provider. Non-strict fallback is the existing translated-output contract; outright rejection would unnecessarily remove accepted requests. +- Benefits, costs and impact: Valid nested and empty closed objects plus documented string formats keep strict mode; incomplete or unsupported schemas no longer claim strict compatibility. Fine-tuned targets retain their schema but use non-strict mode for pattern/format, numeric, array-size and pattern-property constraints; otherwise eligible fine-tuned schemas remain strict. The helper is a compatibility classifier, not a complete JSON Schema validator or proof of every eventual routed model. Native Anthropic normalization and unrelated non-object/non-strict handling remain unchanged. +- Contract reference: [OpenAI Structured Outputs supported schemas](https://developers.openai.com/api/docs/guides/structured-outputs#supported-schemas), checked 2026-09-26. The documented unsupported composition keywords are `allOf`, `not`, `dependentRequired`, `dependentSchemas`, `if`, `then` and `else`; only `anyOf`, not `oneOf`, is in the supported type list. Other schema positions outside that documented subset also fall back to non-strict mode. diff --git a/structure/providers/chat-compat.md b/structure/providers/chat-compat.md index 9aaeceababe..b1eea210ff9 100644 --- a/structure/providers/chat-compat.md +++ b/structure/providers/chat-compat.md @@ -439,18 +439,15 @@ family shared by unrelated upstreams. ## Anthropic structured-output compatibility -The Anthropic adapter lowers Responses `text.format` and Chat Completions `response_format` JSON -Schema requests to `output_config.format`. The local transform follows Anthropic's TypeScript SDK -subset so upstream rejects neither OpenAI-only envelope fields nor unsupported schema constraints. -The adapter merges `format` into an existing adaptive-thinking `output_config` rather than replacing -it, so a compatible `output_config.effort` remains alongside the structured-output format. -Routed Anthropic Messages input carries `output_config.format` through internal `text.format`, so -stored-OAuth requests regain the same native format when the Anthropic adapter rebuilds the wire body. -Unsupported constraints remain in `description` as model guidance instead of disappearing. Root -`$defs` stay beside a root `$ref`, intentionally differing from the current SDK transform's early -`$ref` return so local references remain resolvable. +The Anthropic adapter lowers Responses `text.format` and Chat Completions `response_format` JSON Schema requests to `output_config.format`, following Anthropic's TypeScript SDK subset. +It merges `format` into the existing adaptive-thinking `output_config`, preserving compatible `output_config.effort`. Unsupported constraints remain in `description` as model guidance; root `$defs` remain beside a root `$ref` so local references resolve. +Routed Anthropic Messages input carries `output_config.format` through internal `text.format`; stored-OAuth requests regain that format when the Anthropic adapter rebuilds the wire body. +In that inbound direction, `src/adapters/anthropic-output-schema.ts` checks the original schema, not the normalized acceptance clone. Strict mode requires an object root without a root union, and every object must supply a property map, `additionalProperties: false`, and exactly matching `required` names. +The check traverses schema-valued properties, array items, unions and definitions; property names and literal enum/const values are not schema nodes. It admits only documented schema keywords for the node's declared type, so incomplete objects, unknown constraints and invalid keyword values retain their original schema with explicit `strict: false` instead of waiting for a denylist update. Fine-tuned `ft:` targets use OpenAI's narrower model-specific keyword subset after Claude alias/model-map resolution; later provider/combo routing remains outside this ingress proof. Anthropic's native acceptance still includes `uri`; only the translated OpenAI strict claim uses the narrower set. +`tests/claude-integration/claude-output-schema-strict.test.ts` covers the acceptance probe, inbound format, translated Responses parser, recursive object controls and caller-schema preservation. > Decision record: [ADR-0066](../decisions/ADR-0066-anthropic-structured-output-compatibility.md) +> Decision record: [ADR-5901](../decisions/ADR-5901-translated-output-schema-strict-eligibility.md) ## Reasoning display parity (hideThinkingSummary) diff --git a/tests/claude-integration/claude-output-schema-strict.test.ts b/tests/claude-integration/claude-output-schema-strict.test.ts new file mode 100644 index 00000000000..f5ffc6dc635 --- /dev/null +++ b/tests/claude-integration/claude-output-schema-strict.test.ts @@ -0,0 +1,215 @@ +import { describe, expect, test } from "bun:test"; +import { isAnthropicOutputSchema, satisfiesOpenAiStrictSchema } from "../../src/adapters/anthropic-output-schema"; +import { anthropicToResponsesBody } from "../../src/claude/inbound"; +import { formatFromOutputConfig } from "../../src/claude/inbound-model-options"; +import { parseRequest } from "../../src/responses/parser"; + +type Schema = Record; + +function closedObject(properties: Schema = {}): Schema { + return { type: "object", properties, required: Object.keys(properties), additionalProperties: false }; +} + +function expectTranslatedStrict(schema: Schema, strict: boolean): void { + const before = structuredClone(schema); + // Acceptance normalizes a clone; strict eligibility must describe the ORIGINAL wire schema. + expect(isAnthropicOutputSchema(schema)).toBe(true); + const format = formatFromOutputConfig({ format: { type: "json_schema", schema } }); + expect(format).toEqual({ type: "json_schema", name: "response", schema, strict }); + expect(format?.schema).toBe(schema); + const body = anthropicToResponsesBody({ + model: "claude-sonnet-5", max_tokens: 256, + messages: [{ role: "user", content: "Return JSON" }], + output_config: { format: { type: "json_schema", schema } }, + }); + expect(parseRequest(body).options.textFormat).toEqual({ type: "json_schema", name: "response", schema, strict }); + expect(schema).toEqual(before); +} + +describe("translated Anthropic structured output strict eligibility (#5901 follow-up)", () => { + test.each([ + { type: "object" }, + { type: "object", additionalProperties: false, required: [] }, + { type: "object", properties: null, additionalProperties: false, required: [] }, + { type: "object", properties: [], additionalProperties: false, required: [] }, + { type: "object", properties: "invalid", additionalProperties: false, required: [] }, + { type: "object", properties: {} }, + { type: "object", properties: {}, required: [] }, + { type: "object", properties: {}, required: [], additionalProperties: true }, + { type: "object", properties: {}, additionalProperties: false }, + ])("does not certify an incomplete object: %j", schema => { + expect(satisfiesOpenAiStrictSchema(schema)).toBe(false); + expectTranslatedStrict(schema, false); + }); + + test.each([undefined, null, "answer", [], ["answer", "extra"], ["answer", "answer"], [123]] + .map(required => ({ required })))( + "requires exactly the declared property names: %j", ({ required }) => { + const schema = { ...closedObject({ answer: { type: "string" } }), required }; + expect(satisfiesOpenAiStrictSchema(schema)).toBe(false); + expectTranslatedStrict(schema, false); + }, + ); + + test.each([ + { payload: null }, + { payload: { type: "object" } }, + { payload: { type: ["object", "null"] } }, + { payload: { type: "array", items: { type: "object" } } }, + { payload: { anyOf: [{ type: "null" }, { type: "object" }] } }, + { payload: { anyOf: [null, { type: "string" }] } }, + ])("checks object contracts in nested schema positions: %j", properties => { + const schema = closedObject(properties); + expect(satisfiesOpenAiStrictSchema(schema)).toBe(false); + expectTranslatedStrict(schema, false); + }); + + test("checks object contracts inside referenced definitions", () => { + const schema = { + ...closedObject({ payload: { $ref: "#/$defs/payload" } }), + $defs: { payload: { type: "object" } }, + }; + expectTranslatedStrict(schema, false); + }); + + test.each(["allOf", "oneOf", "not", "dependentRequired", "dependentSchemas", "if", "then", "else", + "additionalItems", "contains", "minContains", "maxContains", "prefixItems", "uniqueItems", + "propertyNames", "minProperties", "maxProperties", "unevaluatedItems", "unevaluatedProperties"])( + "does not claim unsupported strict keyword %s at a schema node", keyword => { + const unsupported = { + ...closedObject(), + [keyword]: keyword === "allOf" ? [closedObject()] : keyword === "uniqueItems" ? true : {}, + }; + expectTranslatedStrict(unsupported, false); + expectTranslatedStrict(closedObject({ payload: { type: "array", items: unsupported } }), false); + }, + ); + + test("an unsupported array constraint on a property survives with strict disabled", () => { + const schema = closedObject({ + tags: { type: "array", items: { type: "string" }, uniqueItems: true }, + }); + expectTranslatedStrict(schema, false); + }); + + test("unknown schema keywords fail closed instead of waiting for another denylist entry", () => { + for (const property of [ + { type: "string", contentEncoding: "base64" }, + { type: "string", minLength: 1 }, + { type: "string", default: "value" }, + { type: "string", examples: ["value"] }, + { type: "string", readOnly: true }, + ]) { + expectTranslatedStrict(closedObject({ value: property }), false); + } + }); + + test("OpenAI strict string formats use the documented subset without changing Anthropic acceptance", () => { + const uriSchema = closedObject({ resource: { type: "string", format: "uri" } }); + expectTranslatedStrict(uriSchema, false); + expect((uriSchema.properties as Schema).resource).toEqual({ type: "string", format: "uri" }); + + for (const format of [ + "date-time", "time", "date", "duration", "email", "hostname", "ipv4", "ipv6", "uuid", + ]) { + expectTranslatedStrict(closedObject({ value: { type: "string", format } }), true); + } + }); + + test("keeps complete empty, nested, array and nullable objects strict", () => { + expectTranslatedStrict(closedObject(), true); + const schema = { + ...closedObject({ + empty: closedObject(), + rows: { type: "array", items: closedObject({ answer: { type: "string" } }) }, + optionalValue: { ...closedObject(), type: ["object", "null"] }, + choice: { anyOf: [closedObject(), { type: "null" }] }, + referenced: { $ref: "#/$defs/payload" }, + }), + $defs: { payload: closedObject({ next: { $ref: "#/$defs/payload" } }) }, + }; + expectTranslatedStrict(schema, true); + }); + + test("keeps documented type-specific constraints strict", () => { + expectTranslatedStrict(closedObject({ + text: { type: "string", pattern: "^[a-z]+$", format: "hostname" }, + count: { type: "integer", minimum: 0, exclusiveMaximum: 10, multipleOf: 2 }, + rows: { type: "array", items: { type: "boolean" }, minItems: 1, maxItems: 3 }, + }), true); + }); + + test("fine-tuned targets preserve unsupported constraints with strict disabled", () => { + const schema = closedObject({ + nested: { + anyOf: [ + { type: "string", pattern: "^[a-z]+$" }, + { type: "array", items: { type: "integer", minimum: 0 }, maxItems: 3 }, + ], + }, + mapped: { $ref: "#/$defs/mapped" }, + }); + schema.$defs = { + mapped: { + type: "object", properties: {}, required: [], additionalProperties: false, + patternProperties: { "^x-": { type: "string" } }, + }, + }; + const before = structuredClone(schema); + + for (const model of ["ft:gpt-4.1-nano:org::name", "openai/ft:gpt-4.1-nano:org::name"]) { + expect(formatFromOutputConfig({ format: { type: "json_schema", schema } }, model)) + .toEqual({ type: "json_schema", name: "response", schema, strict: false }); + } + expect(formatFromOutputConfig({ format: { type: "json_schema", schema } }, "gpt-4.1-nano")) + .toEqual({ type: "json_schema", name: "response", schema, strict: true }); + expect(schema).toEqual(before); + }); + + test("resolved modelMap targets control fine-tuned strict eligibility", () => { + const constrained = closedObject({ value: { type: "string", format: "email" } }); + const raw = { + model: "claude-sonnet-5", max_tokens: 256, + messages: [{ role: "user", content: "Return JSON" }], + output_config: { format: { type: "json_schema", schema: constrained } }, + }; + const fineTuned = anthropicToResponsesBody(raw, { + modelMap: { "claude-sonnet-5": "openai/ft:gpt-4.1-nano:org::name" }, + }); + expect((fineTuned.text as { format: { strict: boolean } }).format.strict).toBe(false); + expect(fineTuned.model).toBe("openai/ft:gpt-4.1-nano:org::name"); + + const ordinary = anthropicToResponsesBody({ ...raw, output_config: { + format: { type: "json_schema", schema: closedObject({ value: { type: "string" } }) }, + } }, { + modelMap: { "claude-sonnet-5": "openai/ft:gpt-4.1-nano:org::name" }, + }); + expect((ordinary.text as { format: { strict: boolean } }).format.strict).toBe(true); + }); + + test("property names and literal data are not mistaken for schema keywords", () => { + const schema = closedObject({ + not: { type: "string" }, allOf: { type: "string" }, properties: { type: "string" }, + example: { + ...closedObject({ type: { type: "string" } }), + enum: [{ type: "object", not: {}, properties: null }], + }, + }); + expectTranslatedStrict(schema, true); + }); + + test.each(["anyOf", "oneOf"])("a root %s stays non-strict even with an explicit object type", keyword => { + expectTranslatedStrict({ ...closedObject(), [keyword]: [closedObject()] }, false); + }); + + test.each([{ type: "string" }, { type: "array", items: { type: "string" } }])( + "keeps non-object root behavior: %j", schema => expectTranslatedStrict(schema, false), + ); + + test("preserves optional fields and unsupported-schema rejection", () => { + expectTranslatedStrict({ ...closedObject({ answer: { type: "string" } }), required: [] }, false); + expect(formatFromOutputConfig({ format: { type: "json_schema", schema: { description: "no type" } } })) + .toBeUndefined(); + expect(formatFromOutputConfig({ format: { type: "json_object" } })).toBeUndefined(); + }); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index b44e92dbfec..d8bfa12451f 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -313,6 +313,7 @@ "claude-inbound-cache-stabilize.test.ts": "claude-integration", "claude-inbound-debug.test.ts": "claude-integration", "claude-inbound.test.ts": "claude-integration", + "claude-output-schema-strict.test.ts": "claude-integration", "claude-intercept-integration.test.ts": "server", "claude-intercept-local-ca.test.ts": "claude-integration", "claude-intercept-model-bindings.test.ts": "claude-integration", From 8f25206f3edbe73ae050b0677813d4d36709ea17 Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Sun, 27 Sep 2026 14:23:34 +0900 Subject: [PATCH 09/12] test(responses): pin established websocket fallback (#6011) Carried from #6011 into merge train round 3. Co-authored-by: Ingwannu --- .../docs/reference/configuration/server.md | 5 + ...ADR-4191-established-websocket-fallback.md | 14 +++ structure/transports/responses-failover.md | 11 ++ tests/responses/ws-ambiguous-resend.test.ts | 102 +++++++++++++++++- 4 files changed, 130 insertions(+), 2 deletions(-) create mode 100644 structure/decisions/ADR-4191-established-websocket-fallback.md diff --git a/docs-site/src/content/docs/reference/configuration/server.md b/docs-site/src/content/docs/reference/configuration/server.md index a9b694c93a3..c8e9f967ffb 100644 --- a/docs-site/src/content/docs/reference/configuration/server.md +++ b/docs-site/src/content/docs/reference/configuration/server.md @@ -90,6 +90,11 @@ refusal returns as soon as that grant is spent, the leg has no send left, or a r for any other reason. A request that already emitted output or a tool call keeps the refusal regardless. A caller that cancels mid-replacement gets the cancellation, not the refusal. +For WebSocket recovery, “before the first Responses event” is stricter than “before the +first text”: even `response.created`, a tool event or a usage-bearing response closes the +replacement window. Ping/pong and quota metadata alone do not. A later socket close cannot +turn an already settled cancellation or connect/silence timeout into an HTTP retry. + `noProxy` accepts either a comma-separated string or an array. Both forms add entries without replacing an inherited `NO_PROXY`: diff --git a/structure/decisions/ADR-4191-established-websocket-fallback.md b/structure/decisions/ADR-4191-established-websocket-fallback.md new file mode 100644 index 00000000000..1dec5e87b9f --- /dev/null +++ b/structure/decisions/ADR-4191-established-websocket-fallback.md @@ -0,0 +1,14 @@ +# ADR-4191 — decision recorded under "Ambiguous-resend gate" + +- Contract owner: [transports/responses-failover.md](../transports/responses-failover.md#ambiguous-resend-gate) +- Recorded: 2026-09-26 + +## Decision record + +- Purpose and intent: Recover an established Codex WebSocket that closes or errors before any semantic Responses event without replaying partially observed turns. +- Existing implementation and constraints: At dev `5518653a9a`, the implementation from `aed3bb8f42` already marks eligible socket deaths and asks the shared ambiguous-resend gate in passthrough dispatch. A successful WebSocket send can have started inference even if nothing came back. Liveness/prelude diagnostics and silence deadlines already exist independently. +- Alternatives considered: Add an unconditional SSE retry inside the exchange; add a second transport-local retry counter; replace requests after text-free response events; or retain the existing dispatch-owned replacement and strengthen regression coverage. +- Chosen approach: Keep the existing dispatch-owned HTTP-only replacement. Require the provider's `retryOnReset` policy, self-contained request judgment, unspent request-wide grant and available send budget. Record and charge the physical send at the ordinary boundary, after credential selection is revalidated. Extend deterministic coverage rather than add a duplicate transport path. +- Why: The exchange cannot independently authorize a new inference, reserve spend, refresh credentials or choose another account. The shared dispatch already owns those decisions and the request identity. Any semantic response event ends eligibility, including creation, tool and usage events before text. Quota control frames and pong events indicate liveness only. +- Advantages, costs and consequences: One default replacement remains available without weakening hosted-tool, cancellation, timeout, native-control or committed-output exclusions. The opt-in still accepts possible duplicate inference billing; absence of output does not prove non-execution. An explicit `replacements: 2` retains the existing shared-policy contract for a subsequent HTTP reset, rather than becoming a second WebSocket fallback. This change does not widen that policy or alter runtime behavior. +- Validation boundary: Focused tests are authored but not executed in this work item because the operator prohibited test/build/install execution. Existing implementation and source traces are evidence of placement, not claims that the added assertions pass. diff --git a/structure/transports/responses-failover.md b/structure/transports/responses-failover.md index 67e512b608e..0d5d4a310fd 100644 --- a/structure/transports/responses-failover.md +++ b/structure/transports/responses-failover.md @@ -105,6 +105,17 @@ is sorted exactly like the pre-header row's (see [ambiguous connection-reset replay boundary](#ambiguous-connection-reset-replay-boundary)) before it goes round the recovery loop again. +The boundary is the first non-control Responses event, not the first visible text delta: +`response.created`, output/tool events and response usage all close the WebSocket replacement +window. Quota metadata and ping/pong liveness alone do not. Cancellation and connect/silence +deadlines never acquire the socket-death marker, even if a late close follows them. +`tests/responses/ws-ambiguous-resend.test.ts` covers these boundaries through the exchange and +the existing HTTP-only dispatch, including request-field preservation and terminal fallback +answers. The replacement uses the shared credential-selection guard and physical-send ledger; +there is no transport-local retry budget or credential snapshot with independent authority. + +> Decision record: [ADR-4191](../decisions/ADR-4191-established-websocket-fallback.md) + ## Console upload rejection recovery `src/providers/opencode-zen-rate-limit.ts` recognizes the complete Console upload-rejection envelope only at the effective HTTPS opencode.ai Zen/Go generation endpoint. A provider row name cannot authorize another destination. The two recovery loops in `src/server/responses/core.ts` wait 800 ms and replay the captured serialized request once; cancellation, nonreplayable responses, other errors and a second upload rejection keep their failure semantics. The recovery kind is persisted as `console-go-upload-retry` and has a localized Logs label. diff --git a/tests/responses/ws-ambiguous-resend.test.ts b/tests/responses/ws-ambiguous-resend.test.ts index 38ddd5b8edb..36e12ae4cb1 100644 --- a/tests/responses/ws-ambiguous-resend.test.ts +++ b/tests/responses/ws-ambiguous-resend.test.ts @@ -116,7 +116,7 @@ const QUOTA_FRAME = JSON.stringify({ type: "codex.rate_limits", rate_limits: { primary: { used_percent: 10, window_minutes: 10080 } }, }); -/** The three ways a socket can die under the send before anything was promised to the client. */ +/** Control traffic proves liveness, not inference output or safe non-delivery. */ const SOCKET_DEATHS: Array<[string, (ws: FakeWebSocket) => void, "pre-header" | "protocol-prelude"]> = [ ["nothing came back", ws => { ws.emit("open", {}); @@ -131,6 +131,13 @@ const SOCKET_DEATHS: Array<[string, (ws: FakeWebSocket) => void, "pre-header" | ws.emit("open", {}); ws.emit("error", {}); }, "pre-header"], + ["pongs and metadata arrived without a Responses event", ws => { + ws.emit("open", {}); + ws.emit("pong", {}); + ws.emit("message", { data: QUOTA_FRAME }); + ws.emit("message", { data: JSON.stringify({ type: "codex.response.metadata", headers: {} }) }); + ws.emit("close", { code: 1006 }); + }, "protocol-prelude"], ]; const noFallback = (async () => { @@ -176,6 +183,23 @@ describe("the exchange records a socket that died under the send (#4191)", () => await expect(response.text()).rejects.toThrow("closed before a Responses terminal event"); }); + test("a connect deadline after send cannot become a socket-death replacement", async () => { + const abort = new AbortController(); + installFake(ws => { + ws.emit("open", {}); + abort.abort(new DOMException("connect deadline", "TimeoutError")); + // A late close must not replace the already settled deadline verdict. + ws.emit("close", { code: 1006 }); + }); + const response = await codexWsUpstreamFetch( + CODEX_URL, { ...streamingInit(), signal: abort.signal }, noFallback, + ); + expect(response.status).toBe(504); + expect(codexWsSocketDeathStage(response)).toBeUndefined(); + expect(readCodexWsStage(response)?.sent).toBe(true); + expect(FakeWebSocket.instances[0]!.sent).toHaveLength(1); + }); + test("a steering exchange's death is not offered: its channel may have sent more than the create", async () => { installFake(ws => { ws.emit("open", {}); @@ -251,9 +275,10 @@ describe("handleResponses replaces a dead socket's send once under retryOnReset config: OcxConfig, logCtx: RequestLogContext = { model: "", provider: "" }, sendBudget = createRequestExecutionBudget(), + abortSignal?: AbortSignal, ): Promise { takeSpendHome(); - return handleResponses(request, config, logCtx, { codexWsRuntimeIdentity: BOUNDED_WS_RUNTIME, sendBudget }); + return handleResponses(request, config, logCtx, { codexWsRuntimeIdentity: BOUNDED_WS_RUNTIME, sendBudget, abortSignal }); } describe("in pool mode", () => { @@ -382,6 +407,7 @@ describe("handleResponses replaces a dead socket's send once under retryOnReset test.each([ ["the provider grants nothing", {}, {}], ["the turn is stored upstream", { retryOnReset: {} }, { store: true }], + ["the request declares an upstream hosted tool", { retryOnReset: {} }, { tools: [{ type: "web_search" }] }], ])("the 502 stands and nothing else is sent when %s", async (_name, provider, body) => { installFake(SOCKET_DEATHS[0]![1]); const http = stubHttp(completed); @@ -392,6 +418,78 @@ describe("handleResponses replaces a dead socket's send once under retryOnReset expect(http).toHaveLength(0); }); + test.each([ + { type: "response.created", response: { id: "r-ws", status: "in_progress" } }, + { type: "response.output_text.delta", response_id: "r-ws", item_id: "item-ws", delta: "hello" }, + { type: "response.output_item.added", response_id: "r-ws", output_index: 0, + item: { id: "item-ws", type: "function_call", call_id: "call-ws", name: "lookup", arguments: "{}" } }, + { type: "response.in_progress", response: { id: "r-ws", usage: { input_tokens: 4, output_tokens: 1 } } }, + ])("$type forbids HTTP replacement even with an unused grant", async event => { + installFake(ws => { + ws.emit("open", {}); + ws.emit("message", { data: JSON.stringify(event) }); + ws.emit("close", { code: 1006 }); + }); + const http = stubHttp(completed); + const budget = createRequestExecutionBudget(); + const response = await send(turn(), forwardConfig({ retryOnReset: {} }), undefined, budget); + // Depending on preflight, this is a projected failure or a body error. Neither + // representation may turn an observed semantic event into another inference. + await response.text().catch(() => ""); + expect(FakeWebSocket.instances).toHaveLength(1); + expect(FakeWebSocket.instances[0]!.sent).toHaveLength(1); + expect(http).toHaveLength(0); + expect(budget.claimAmbiguousResend?.(1)).toBe(true); + }); + + test("cancellation after send wins over a late socket death without spending a grant", async () => { + const abort = new AbortController(); + installFake(ws => { + ws.emit("open", {}); + abort.abort(); + ws.emit("close", { code: 1006 }); + }); + const http = stubHttp(completed); + const budget = createRequestExecutionBudget(); + const response = await send(turn(), forwardConfig({ retryOnReset: {} }), undefined, budget, abort.signal); + expect(response.status).toBe(499); + expect(FakeWebSocket.instances[0]!.sent).toHaveLength(1); + expect(http).toHaveLength(0); + expect(budget.claimAmbiguousResend?.(1)).toBe(true); + }); + + test("HTTP replacement preserves the sent request's model, input, tools and instructions", async () => { + installFake(SOCKET_DEATHS[0]![1]); + const http = stubHttp(completed); + const response = await send(turn({ + instructions: "Use the supplied lookup tool only when needed.", + input: [{ role: "user", content: "hello" }], + tools: [{ type: "function", name: "lookup", parameters: { type: "object", properties: {} } }], + }), forwardConfig({ retryOnReset: {} })); + await response.text(); + expect(http).toHaveLength(1); + const frame = JSON.parse(FakeWebSocket.instances[0]!.sent[0]!); + const replacement = JSON.parse(http[0]!); + for (const field of ["model", "input", "instructions", "tools", "store"]) { + expect(replacement[field]).toEqual(frame[field]); + } + }); + + test.each(["response.failed", "response.incomplete"])("HTTP %s remains terminal, not a third send", async type => { + installFake(SOCKET_DEATHS[0]![1]); + const http = stubHttp(() => new Response(`event: ${type}\ndata: ${JSON.stringify({ + type, + response: { id: "r-http", status: type.slice("response.".length), output: [], + ...(type === "response.failed" + ? { error: { code: "server_error", message: "failed" } } + : { incomplete_details: { reason: "max_output_tokens" } }) }, + })}\n\n`, { headers: { "content-type": "text/event-stream" } })); + const response = await send(turn(), forwardConfig({ retryOnReset: {} })); + await response.text().catch(() => ""); + expect(FakeWebSocket.instances).toHaveLength(1); + expect(http).toHaveLength(1); + }); + test.each([ ["a status the client would retry", () => new Response("busy", { status: 503 })], ["a reset of its own", () => { throw Object.assign(new Error("socket hang up"), { code: "ECONNRESET" }); }], From 462990242ab67fa6b1d0090b82d2bc07b3e06081 Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 27 Sep 2026 14:24:22 +0900 Subject: [PATCH 10/12] fix(desktop): keep a retry made between update attempts A retried install that finds the drain already settled clears wanted again without replacing the update snapshot, so a startup retry recorded only in wanted was lost when that install failed. resume() now promotes a pending snapshot as well. Found in the round 3 review of #6041. --- desktop/src-tauri/src/exit.rs | 37 ++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/desktop/src-tauri/src/exit.rs b/desktop/src-tauri/src/exit.rs index 5ee8e4bff10..8950c338638 100644 --- a/desktop/src-tauri/src/exit.rs +++ b/desktop/src-tauri/src/exit.rs @@ -323,7 +323,14 @@ impl ExitCoordinator { /// A person asked for a runtime again (the startup page's retry). pub fn resume(&self) { - self.inner().wanted = true; + let mut inner = self.inner(); + inner.wanted = true; + // A pending update snapshot must learn about the request too. A retried install that finds + // the drain already settled clears `wanted` again without replacing the snapshot, so a + // retry recorded only in `wanted` would be lost when that install fails and aborts. + if let Some(snapshot) = inner.restart_wanted.as_mut() { + *snapshot = true; + } } /// Reserve the right to start a runtime. False once something else owns the phase. @@ -803,6 +810,34 @@ mod tests { assert!(coordinator.supervision_allowed()); } + #[test] + fn a_retry_between_update_attempts_survives_the_second_claim() { + let coordinator = ExitCoordinator::new(); + coordinator.set_tray(TrayAvailability::Available); + assert!(coordinator.begin_stop()); + assert_eq!(coordinator.finish_stop(), None); + + assert_eq!( + coordinator.claim_drain(ExitReason::CoordinatedRestart), + Some(ExitReason::CoordinatedRestart) + ); + coordinator.finish_drain(DrainVerdict::Drained); + coordinator.resume(); + // The next install attempt finds the drain settled and clears `wanted` again. + assert_eq!( + coordinator.claim_drain(ExitReason::CoordinatedRestart), + None + ); + assert_eq!( + coordinator.abort_restart(), + Some(AbortedRestart { + phase: ExitPhase::Drained, + runtime_was_wanted: true, + }) + ); + assert!(coordinator.supervision_allowed()); + } + #[test] fn a_quit_or_a_drain_in_flight_is_never_aborted() { let coordinator = ExitCoordinator::new(); From bfbcbd6f8133678caeecaa342c3fbe12ea5c208b Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 27 Sep 2026 14:24:22 +0900 Subject: [PATCH 11/12] fix(claude): refuse strict for a non-positive multipleOf --- src/adapters/anthropic-output-schema.ts | 3 +++ .../claude-integration/claude-output-schema-strict.test.ts | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/src/adapters/anthropic-output-schema.ts b/src/adapters/anthropic-output-schema.ts index bc1758b30e3..dccae46d211 100644 --- a/src/adapters/anthropic-output-schema.ts +++ b/src/adapters/anthropic-output-schema.ts @@ -225,6 +225,9 @@ export function satisfiesOpenAiStrictSchema(value: unknown, fineTuned = false): const constraint = node[key]; if (Object.hasOwn(node, key) && (typeof constraint !== "number" || !Number.isFinite(constraint))) return false; } + // JSON Schema requires a strictly positive divisor; a zero or negative one is a schema the + // destination rejects, so it cannot be certified strict. + if (Object.hasOwn(node, "multipleOf") && (node.multipleOf as number) <= 0) return false; for (const key of ["minItems", "maxItems"]) { const constraint = node[key]; if (Object.hasOwn(node, key) diff --git a/tests/claude-integration/claude-output-schema-strict.test.ts b/tests/claude-integration/claude-output-schema-strict.test.ts index f5ffc6dc635..ce045428038 100644 --- a/tests/claude-integration/claude-output-schema-strict.test.ts +++ b/tests/claude-integration/claude-output-schema-strict.test.ts @@ -139,6 +139,12 @@ describe("translated Anthropic structured output strict eligibility (#5901 follo }), true); }); + test.each([0, -2])("does not certify a non-positive multipleOf: %d", multipleOf => { + const schema = closedObject({ count: { type: "integer", multipleOf } }); + expect(satisfiesOpenAiStrictSchema(schema)).toBe(false); + expect(satisfiesOpenAiStrictSchema(closedObject({ count: { type: "integer", multipleOf: 2 } }))).toBe(true); + }); + test("fine-tuned targets preserve unsupported constraints with strict disabled", () => { const schema = closedObject({ nested: { From 2bfc18aca1ced5f5872284f943387621e446c5ba Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 27 Sep 2026 14:24:22 +0900 Subject: [PATCH 12/12] docs(structure): record the executed ws-ambiguous-resend proof and its scope in ADR-4191 --- structure/decisions/ADR-4191-established-websocket-fallback.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/structure/decisions/ADR-4191-established-websocket-fallback.md b/structure/decisions/ADR-4191-established-websocket-fallback.md index 1dec5e87b9f..0e92ad3652d 100644 --- a/structure/decisions/ADR-4191-established-websocket-fallback.md +++ b/structure/decisions/ADR-4191-established-websocket-fallback.md @@ -11,4 +11,4 @@ - Chosen approach: Keep the existing dispatch-owned HTTP-only replacement. Require the provider's `retryOnReset` policy, self-contained request judgment, unspent request-wide grant and available send budget. Record and charge the physical send at the ordinary boundary, after credential selection is revalidated. Extend deterministic coverage rather than add a duplicate transport path. - Why: The exchange cannot independently authorize a new inference, reserve spend, refresh credentials or choose another account. The shared dispatch already owns those decisions and the request identity. Any semantic response event ends eligibility, including creation, tool and usage events before text. Quota control frames and pong events indicate liveness only. - Advantages, costs and consequences: One default replacement remains available without weakening hosted-tool, cancellation, timeout, native-control or committed-output exclusions. The opt-in still accepts possible duplicate inference billing; absence of output does not prove non-execution. An explicit `replacements: 2` retains the existing shared-policy contract for a subsequent HTTP reset, rather than becoming a second WebSocket fallback. This change does not widen that policy or alter runtime behavior. -- Validation boundary: Focused tests are authored but not executed in this work item because the operator prohibited test/build/install execution. Existing implementation and source traces are evidence of placement, not claims that the added assertions pass. +- Validation boundary: `tests/responses/ws-ambiguous-resend.test.ts` pins the behavior that landed in #5675 (`aed3bb8f42`); it runs in the hosted test shards. The fallback covers a socket that dies after the create frame and before the first Responses event; a death after output started stays a failed leg, because replaying it risks a second inference.