From aa8851f38d460878029cb29a6dee9fc5d8d589c6 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 7 Aug 2026 16:45:27 +0900 Subject: [PATCH 1/6] fix(codex): propagate routed DeepSeek and GLM effort Add model-scoped reasoning-summary capability defaults for the registry-backed DeepSeek V4 and evidence-backed GLM models so Codex keeps sending the selected reasoning object. Merge the registry map per key, with defaults spread first and user configuration second. This preserves an explicit false for one model without suppressing defaults for every other model, and creates a detached object instead of aliasing registry metadata. Cover the catalog contract, partial override behavior, conservative no-opt-in default, and red-green production ablation for #1100. --- src/providers/derive.ts | 6 +++ src/providers/registry.ts | 13 +++++ tests/codex-catalog.test.ts | 101 ++++++++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+) diff --git a/src/providers/derive.ts b/src/providers/derive.ts index b63c9fa7d8..eaa7aaeadd 100644 --- a/src/providers/derive.ts +++ b/src/providers/derive.ts @@ -280,6 +280,12 @@ export function enrichProviderFromRegistry(name: string, prov: OcxProviderConfig // the entry so an explicit user value stays distinguishable from the default. if (prov.supportsServiceTier === undefined && entry.supportsServiceTier !== undefined) prov.supportsServiceTier = entry.supportsServiceTier; if (prov.preserveResponsesReasoningContent === undefined && entry.preserveResponsesReasoningContent !== undefined) prov.preserveResponsesReasoningContent = entry.preserveResponsesReasoningContent; + if (entry.modelSupportsReasoningSummaries) { + prov.modelSupportsReasoningSummaries = { + ...entry.modelSupportsReasoningSummaries, + ...(prov.modelSupportsReasoningSummaries ?? {}), + }; + } // Registry-only repair policy (#938): fill only when the runtime provider has // no explicit policy, and deep-clone so saved/user values never alias the // registry constant. diff --git a/src/providers/registry.ts b/src/providers/registry.ts index 20518adb46..037f3c8481 100644 --- a/src/providers/registry.ts +++ b/src/providers/registry.ts @@ -196,6 +196,8 @@ export interface ProviderRegistryEntry { supportsServiceTier?: boolean; /** Registry default for plaintext reasoning replay; see `OcxProviderConfig.preserveResponsesReasoningContent`. Registry-only like `supportsServiceTier`. */ preserveResponsesReasoningContent?: boolean; + /** Registry defaults for per-model Codex reasoning propagation; explicit user keys win during enrichment. */ + modelSupportsReasoningSummaries?: Record; modelDiscovery?: ProviderModelDiscoverySpec; contextWindow?: number; modelContextWindows?: Record; @@ -1104,6 +1106,12 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ ...Object.fromEntries(OPENCODE_GO_THINKING_TOGGLE_MODELS.map(id => [id, THINKING_TOGGLE_MAP])), ...Object.fromEntries(DEEPSEEK_THINKING_MODELS.map(id => [id, deepseekReasoningMapFor(id)])), }, + modelSupportsReasoningSummaries: { + "glm-5.2": true, + "glm-5.1": true, + "glm-5": true, + ...Object.fromEntries(DEEPSEEK_THINKING_MODELS.map(id => [id, true])), + }, thinkingToggleModels: OPENCODE_GO_THINKING_TOGGLE_MODELS, thinkingBudgetModels: THINKING_BUDGET_MODELS, noReasoningModels: ["kimi-k2.7-code", "kimi-k2.7-code-highspeed"], @@ -1340,6 +1348,7 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ */ modelReasoningEfforts: Object.fromEntries(DEEPSEEK_THINKING_MODELS.map(id => [id, deepseekThinkingEffortsFor(id)])), modelReasoningEffortMap: Object.fromEntries(DEEPSEEK_THINKING_MODELS.map(id => [id, deepseekReasoningMapFor(id)])), + modelSupportsReasoningSummaries: Object.fromEntries(DEEPSEEK_THINKING_MODELS.map(id => [id, true])), preserveReasoningContentModels: DEEPSEEK_THINKING_MODELS, // Issue #88: every DeepSeek API model is text-only input (no image support upstream) — the // vision sidecar describes attached images for them, and the catalog advertises image input @@ -1653,6 +1662,7 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ modelSuffixBracketStrip: true, noVisionModels: ZAI_GLM_52_MODELS, modelReasoningEfforts: Object.fromEntries(ZAI_GLM_52_MODELS.map(id => [id, ZAI_GLM_52_REASONING_EFFORTS])), + modelSupportsReasoningSummaries: Object.fromEntries(ZAI_GLM_52_MODELS.map(id => [id, true])), preserveReasoningContentModels: ZAI_GLM_52_MODELS, }, // Zhipu's domestic BigModel platform: OpenAI-compatible pay-as-you-go on open.bigmodel.cn — a @@ -1689,6 +1699,9 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ modelReasoningEffortMap: Object.fromEntries( ZHIPU_BIGMODEL_THINKING_TOGGLE_MODELS.map(id => [id, THINKING_TOGGLE_MAP]), ), + modelSupportsReasoningSummaries: Object.fromEntries( + ZHIPU_BIGMODEL_THINKING_TOGGLE_MODELS.map(id => [id, true]), + ), preserveReasoningContentModels: ZHIPU_BIGMODEL_THINKING_TOGGLE_MODELS, // No liveModels: GET /api/paas/v4/models has not been observed to answer on this host, and a // false live claim yields an empty picker at runtime. Flip it on once someone verifies it. diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index 1f6896f8a2..771ca83714 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -2387,6 +2387,107 @@ describe("Codex catalog routed normalization", () => { expect(routed?.supports_reasoning_summaries).toBe(true); }); + test("built-in DeepSeek and GLM effort models opt into Codex reasoning propagation (#1100)", async () => { + const expected = [ + { slug: "deepseek/deepseek-v4-flash", efforts: ["low", "high", "max", "ultra"] }, + { slug: "deepseek/deepseek-v4-pro", efforts: ["high", "max", "ultra"] }, + { slug: "opencode-go/deepseek-v4-flash", efforts: ["low", "high", "max", "ultra"] }, + { slug: "opencode-go/deepseek-v4-pro", efforts: ["high", "max", "ultra"] }, + { slug: "opencode-go/glm-5.2", efforts: ["low", "medium", "high", "xhigh", "max", "ultra"] }, + { slug: "opencode-go/glm-5.1", efforts: ["low", "medium", "high", "xhigh", "max", "ultra"] }, + { slug: "opencode-go/glm-5", efforts: ["low", "medium", "high", "xhigh", "max", "ultra"] }, + { slug: "zai/glm-5.2", efforts: ["low", "medium", "high", "xhigh", "max", "ultra"] }, + { slug: "zai/glm-5.2[1m]", efforts: ["low", "medium", "high", "xhigh", "max", "ultra"] }, + { slug: "zhipu-bigmodel/glm-4.6", efforts: ["low", "medium", "high", "xhigh", "max", "ultra"] }, + { slug: "zhipu-bigmodel/glm-4.7", efforts: ["low", "medium", "high", "xhigh", "max", "ultra"] }, + { slug: "zhipu-bigmodel/glm-5", efforts: ["low", "medium", "high", "xhigh", "max", "ultra"] }, + { slug: "zhipu-bigmodel/glm-5.1", efforts: ["low", "medium", "high", "xhigh", "max", "ultra"] }, + ]; + const models = await gatherRoutedModels({ + providers: { + deepseek: { + adapter: "openai-chat", + baseUrl: "https://api.deepseek.com", + authMode: "key", + apiKey: "sk-test", + liveModels: false, + models: ["deepseek-v4-flash", "deepseek-v4-pro"], + }, + "opencode-go": { + adapter: "openai-chat", + baseUrl: "https://opencode.ai/zen/go/v1", + authMode: "key", + apiKey: "sk-test", + liveModels: false, + models: ["deepseek-v4-flash", "deepseek-v4-pro", "glm-5.2", "glm-5.1", "glm-5"], + }, + zai: { + adapter: "openai-chat", + baseUrl: "https://api.z.ai/api/coding/paas/v4", + authMode: "key", + apiKey: "sk-test", + liveModels: false, + models: ["glm-5.2", "glm-5.2[1m]"], + }, + "zhipu-bigmodel": { + adapter: "openai-chat", + baseUrl: "https://open.bigmodel.cn/api/paas/v4", + authMode: "key", + apiKey: "sk-test", + liveModels: false, + models: ["glm-4.6", "glm-4.7", "glm-5", "glm-5.1"], + }, + }, + }); + const entries = buildCatalogEntries(nativeTemplate(), [], models); + + for (const item of expected) { + const routed = entries.find(entry => entry.slug === item.slug); + expect( + (routed?.supported_reasoning_levels as Array<{ effort: string }> | undefined)?.map(level => level.effort), + ).toEqual(item.efforts); + expect(routed?.supports_reasoning_summaries).toBe(true); + } + }); + + test("explicit per-model overrides survive registry backfill", () => { + const provider: OcxConfig["providers"][string] = { + adapter: "openai-chat", + baseUrl: "https://api.deepseek.com", + authMode: "key", + modelSupportsReasoningSummaries: { "deepseek-v4-flash": false }, + }; + + enrichProviderFromRegistry("deepseek", provider); + + expect(provider.modelSupportsReasoningSummaries).toEqual({ + "deepseek-v4-flash": false, + "deepseek-v4-pro": true, + }); + }); + + test("routed effort ladders without an opt-in stay conservative about summaries (#1100)", async () => { + const models = await gatherRoutedModels({ + providers: { + plain: { + adapter: "openai-chat", + baseUrl: "https://plain.example.test/v1", + authMode: "key", + liveModels: false, + models: ["effort-model"], + modelReasoningEfforts: { "effort-model": ["low", "high"] }, + }, + }, + }); + const routed = buildCatalogEntries(nativeTemplate(), [], models) + .find(entry => entry.slug === "plain/effort-model"); + + expect( + (routed?.supported_reasoning_levels as Array<{ effort: string }> | undefined)?.map(level => level.effort), + ).toEqual(["low", "high", "max", "ultra"]); + expect(routed?.supports_reasoning_summaries).toBe(false); + }); + test("generated jawcode snapshot is restricted to mapped providers", () => { expect(resolveJawcodeProvider("kimi")).toBe("moonshot"); expect(resolveJawcodeProvider("nanogpt")).toBeUndefined(); From 6429e9c41541c99d09b2e2c79c842cdc4db4574e Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 7 Aug 2026 16:49:24 +0900 Subject: [PATCH 2/6] fix(acl): give one harden sequence a 30s envelope, not 5s (#1156) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a machine where icacls is slow — Defender real-time scanning, a roaming profile, a domain-controller round trip — a complete ACL sequence could not finish inside the 5-second envelope. The harden failed closed, the native-main owner published a permanent `unavailable`, and every native request returned 503 until the user restarted. One correction to the issue's framing: PR #1135's retry is not the problem. Owner-level recovery calls hardenSecret again and does receive a fresh deadline. The defect is that one complete sequence — `/grant:r`, `/inheritance:r`, `/remove:g`, plus the conditional `/findsid` verification — had only five seconds for all of it. Raises the default to 30s and keeps everything else: the 60s cap, the OPENCODEX_ACL_TIMEOUT_MS override, the clamp, and the shared-envelope structure. Independent per-command budgets were rejected: with /findsid fallbacks they multiply into the multi-minute startup stall the shared budget was introduced to prevent. The cost is stated in the source comment rather than hidden. Because loadConfig hardens three paths sequentially, the timeout-path worst case at load becomes ~90s, and the owner path ~60.25s. Both need icacls to be pathologically slow on every call; a healthy machine finishes in milliseconds. A slow start is recoverable, a permanent 503 is not, and the failure stays fail-closed either way. Four existing tests depended on the 5s default while actually asserting something else — envelope sharing, fresh-budget-on-second-call, recovery cardinality. Each now pins OPENCODEX_ACL_TIMEOUT_MS explicitly so it tests its real subject, and beforeEach/afterEach isolate the variable so a stray value in a developer's environment cannot change what any of them assert. The new test deliberately does not pin: it exercises the shipped default with 13s of slow-but-successful work. Confirmed to fail with the default reverted to 5s. --- src/lib/windows-secret-acl.ts | 18 ++++++++++-- tests/windows-secret-acl.test.ts | 49 ++++++++++++++++++++++++++++++-- 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/src/lib/windows-secret-acl.ts b/src/lib/windows-secret-acl.ts index f1b72bce85..9549d39870 100644 --- a/src/lib/windows-secret-acl.ts +++ b/src/lib/windows-secret-acl.ts @@ -229,9 +229,23 @@ export interface HardenOptions { * timeout retry and the diagnostic verification pass (no per-attempt fresh budget: * loadConfig hardens dir+config+auth sequentially, so per-attempt budgets stack * into multi-minute startup stalls). Override with OPENCODEX_ACL_TIMEOUT_MS - * (integer ms, clamped to [1000, 60000]; invalid values fall back to 5000). + * (integer ms, clamped to [1000, 60000]; invalid values fall back to 30000). + * + * The default was 5s until #1156. One envelope has to cover the whole sequence — + * `/grant:r`, `/inheritance:r`, `/remove:g`, plus the conditional `/findsid` + * verification — and on machines where icacls is slow (Defender real-time scanning, + * roaming profiles, a domain-controller round trip) 5s ran out mid-sequence. The + * harden then failed closed, the native-main owner published a permanent + * `unavailable`, and every native request returned 503 until restart. A slow start + * is recoverable; that is not. + * + * The cost is honest and worth stating: because loadConfig hardens three paths + * sequentially, the timeout-path worst case at load is ~90s, and the owner path + * (initial call + one recovery) is ~60.25s. Both require icacls to be + * pathologically slow on every call; a healthy machine finishes in milliseconds + * and sees no change. Operators who prefer the old bound can set the env override. */ -const HARDEN_DEADLINE_DEFAULT_MS = 5_000; +const HARDEN_DEADLINE_DEFAULT_MS = 30_000; const HARDEN_DEADLINE_MIN_MS = 1_000; const HARDEN_DEADLINE_MAX_MS = 60_000; diff --git a/tests/windows-secret-acl.test.ts b/tests/windows-secret-acl.test.ts index acf3cb343f..9ac911a14f 100644 --- a/tests/windows-secret-acl.test.ts +++ b/tests/windows-secret-acl.test.ts @@ -39,11 +39,21 @@ import { NATIVE_MAIN_OWNER_DB, retainNativeMainOwner } from "../src/codex/native let testDir = ""; +// The default harden budget is production policy (#1156 raised it to 30s), not a value tests +// should silently inherit. Isolate the override here so a test that cares about a specific +// budget pins it explicitly, and a stray value in the developer's environment cannot change +// what any of these assert. +let previousAclTimeout: string | undefined; + beforeEach(() => { + previousAclTimeout = process.env.OPENCODEX_ACL_TIMEOUT_MS; + delete process.env.OPENCODEX_ACL_TIMEOUT_MS; testDir = mkdtempSync(join(tmpdir(), "ocx-acl-test-")); }); afterEach(() => { + if (previousAclTimeout === undefined) delete process.env.OPENCODEX_ACL_TIMEOUT_MS; + else process.env.OPENCODEX_ACL_TIMEOUT_MS = previousAclTimeout; if (testDir && existsSync(testDir)) rmSync(testDir, { recursive: true, force: true }); testDir = ""; }); @@ -354,6 +364,10 @@ describe("icacls failure paths (injected seams)", () => { }); test("all icacls steps share one deadline and a timed-out path is not retried this process", () => { + // Pinned to the pre-#1156 budget: this test is about the SHARING of one envelope across + // steps, not about how large the envelope is. Without the pin it would silently stop + // timing out at the 30s default and assert nothing. + process.env.OPENCODEX_ACL_TIMEOUT_MS = "5000"; const filePath = secretFile(); let now = 0; const budgets: number[] = []; @@ -373,6 +387,30 @@ describe("icacls failure paths (injected seams)", () => { expect(budgets.length).toBe(1); }); + test("slow successful ACL steps fit the default harden envelope (#1156)", () => { + // The reported failure: on a machine where icacls is slow, the whole sequence could not + // finish inside one 5s envelope, the harden failed closed, and the native-main owner + // published a permanent `unavailable` — every native request then 503'd until restart. + // No pin here on purpose: this test exists to exercise the SHIPPED default. + resetHardenedStateForTests(); + const filePath = secretFile("slow-default-envelope.json"); + let now = 0; + const steps: string[] = []; + + setNowForTests(() => now); + setIcaclsRunnerForTests(args => { + if (args.includes("/grant:r")) { steps.push("/grant:r"); now += 2_000; } + else if (args.includes("/inheritance:r")) { steps.push("/inheritance:r"); now += 11_000; } + else if (args.includes("/remove:g")) { steps.push("/remove:g"); } + return ok; + }); + + // 13s of slow-but-successful work: impossible under the old 5s default, comfortable + // under 30s with margin left for the conditional /findsid verification. + expect(hardenSecretPath(filePath, { required: true })).toEqual({ ok: true }); + expect(steps).toEqual(["/grant:r", "/inheritance:r", "/remove:g"]); + }); + test("a timeout diagnostic no longer claims filesystem non-support (issue #160)", () => { setIcaclsRunnerForTests(() => timeout); let message = ""; @@ -459,10 +497,10 @@ describe("icacls failure paths (injected seams)", () => { expect(budgets[0]).toBeGreaterThan(500); budgets.length = 0; - process.env.OPENCODEX_ACL_TIMEOUT_MS = "5000ms"; // malformed → default 5000 + process.env.OPENCODEX_ACL_TIMEOUT_MS = "5000ms"; // malformed → default 30000 (#1156) hardenSecretPath(secretFile("env-c.json"), { required: true }); - expect(budgets[0]).toBeLessThanOrEqual(5_000); - expect(budgets[0]).toBeGreaterThan(4_000); + expect(budgets[0]).toBeLessThanOrEqual(30_000); + expect(budgets[0]).toBeGreaterThan(29_000); } finally { if (prev === undefined) delete process.env.OPENCODEX_ACL_TIMEOUT_MS; else process.env.OPENCODEX_ACL_TIMEOUT_MS = prev; @@ -607,6 +645,8 @@ describe("async hardenSecretPath (issue #612)", () => { }); test("a required timeout preserves ETIMEDOUT and one explicit recovery gets a fresh budget", async () => { + // Pinned: this asserts that a SECOND call gets a fresh envelope, not the envelope's size. + process.env.OPENCODEX_ACL_TIMEOUT_MS = "5000"; const target = secretFile("one-time-recovery.json"); let now = 0; let grantCalls = 0; @@ -639,6 +679,9 @@ describe("async hardenSecretPath (issue #612)", () => { }); test("the explicit timeout recovery cannot be consumed more than once", async () => { + // Pinned: this asserts recovery CARDINALITY. At the 30s default the first call would + // succeed on its internal retry and the cardinality claim would never be exercised. + process.env.OPENCODEX_ACL_TIMEOUT_MS = "5000"; const target = secretFile("consumed-recovery.json"); let now = 0; let grantCalls = 0; From 2f242bb7cc05debbd802bb639adb3505675204fb Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 7 Aug 2026 16:59:19 +0900 Subject: [PATCH 3/6] fix(codex): reach custom-named providers, and keep summary defaults off disk (#1100) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit found the first commit fixed the canonical provider ids but missed the shape the issue was actually reported against. REACHING CUSTOM PROVIDERS. `enrichProviderFromRegistry` matches on the provider NAME. The reporter's row is a hand-added provider literally called "GLM" pointing at a vendor endpoint we recognize. Routing worked, so the row looked healthy, but no registry id is called "GLM" — so every piece of registry metadata was skipped, the effort ladder was advertised with summaries left false, and Codex dropped the inbound reasoning object. Exactly the bug, on the exact configuration that was reported. On the name-lookup miss we now fall back to `registryEntryForProviderDestination`, which already answers "which vendor endpoint is this row talking to" and is restricted to fixed key destinations — no templated or overridable base URL can be claimed by it. Scope is deliberately one field: a custom row keeps its own identity for everything else. PER-KEY, EVERYWHERE. The fallback first bailed whenever the user had any map, which recreated the whole-record bug the per-key merge exists to prevent: one model's flag would have suppressed every sibling default. Both paths now share `applyReasoningSummaryDefaults`, so an explicit value — including `false` — wins for its own key and nothing else. NOT PERSISTED. `enrichProviderFromCatalog` feeds a config about to be written to disk. Writing today's registry defaults there would freeze them as the user's own overrides, so a later correction — learning a model's backend rejects summary delivery — would never reach anyone who created their provider first, and they would keep getting 400s with no way to know why. It now restores exactly what the caller submitted. Catalog gathering enriches a detached runtime clone, so the defaults still apply where they matter. KNOWN REMAINING GAP: the reporter's endpoint, open.bigmodel.cn/api/coding/paas/v4, is in no registry entry — only /api/paas/v4 is. Closing that route needs a new registry entry with its own id (glm/glm-cn are bound in FREE_PROVIDER_DIRECTORY), its own evidence-backed model set, and registry-parity updates. That is a provider addition, not a bug fix, so it stays out of this stack and is recorded in the plan unit instead. Each new test was confirmed to fail with its production change reverted. --- src/oauth/key-providers.ts | 12 ++++++ src/providers/derive.ts | 56 ++++++++++++++++++++++---- tests/codex-catalog.test.ts | 80 +++++++++++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 7 deletions(-) diff --git a/src/oauth/key-providers.ts b/src/oauth/key-providers.ts index 150a80b2fa..f48e56b3eb 100644 --- a/src/oauth/key-providers.ts +++ b/src/oauth/key-providers.ts @@ -18,9 +18,21 @@ export const KEY_LOGIN_PROVIDERS: Record = deriveKeyLo * `noReasoningModels`, `defaultModel`) onto a provider config being created, for any field the * caller didn't already supply. Lets the vision/reasoning classification actually reach the saved * config (the GUI/API only send adapter/baseUrl/apiKey/defaultModel). No-op for unknown names. + * + * `modelSupportsReasoningSummaries` is deliberately excluded from what gets persisted. It is + * registry-only metadata resolved at runtime, and this function feeds a config that is about to + * be written to disk. Persisting today's registry defaults would freeze them as the user's own + * overrides: a later registry correction — say we learn a model's backend rejects summary + * delivery — would never reach anyone who created their provider before the correction, and they + * would keep getting upstream 400s with no way to know why. Catalog gathering enriches a + * detached runtime clone, so the defaults still apply where they matter. */ export function enrichProviderFromCatalog(name: string, prov: OcxProviderConfig): void { + const hadOwnSummaries = Object.hasOwn(prov, "modelSupportsReasoningSummaries"); + const submittedSummaries = prov.modelSupportsReasoningSummaries; enrichProviderFromRegistry(name, prov); + if (hadOwnSummaries) prov.modelSupportsReasoningSummaries = submittedSummaries; + else delete prov.modelSupportsReasoningSummaries; } export function isKeyLoginProvider(name: string): boolean { diff --git a/src/providers/derive.ts b/src/providers/derive.ts index eaa7aaeadd..c947c0b4f8 100644 --- a/src/providers/derive.ts +++ b/src/providers/derive.ts @@ -2,6 +2,7 @@ import type { CodexAccountMode, OcxProviderConfig } from "../types"; import { PROVIDER_REGISTRY, providerMatchesRegistryTransport, + registryEntryForProviderDestination, type ProviderRegistryEntry, } from "./registry"; @@ -243,9 +244,55 @@ export function deriveProviderPresets(): DerivedProviderPreset[] { return [...dedupePresets(presets), customPreset()]; } +/** + * Merge registry reasoning-summary defaults PER KEY, letting explicit user values win. + * + * Not a whole-Record `=== undefined` fill like the scalars around it: a user who sets one + * model's flag creates a defined Record, and a whole-object check would then suppress every + * registry default for that provider. Spreading registry-first also preserves an explicit + * `false` — someone who disabled summaries for a model because their backend 400s on it keeps + * that. The result is a fresh object, so saved config never aliases the registry constant. + */ +function applyReasoningSummaryDefaults( + prov: OcxProviderConfig, + defaults: Readonly> | undefined, +): void { + if (!defaults) return; + prov.modelSupportsReasoningSummaries = { + ...defaults, + ...(prov.modelSupportsReasoningSummaries ?? {}), + }; +} + +/** + * Last-resort enrichment for a provider whose NAME matches no registry id. + * + * #1100 was reported against a hand-added provider called "GLM" pointing at a vendor endpoint + * we recognize. Routing worked, so the row looked healthy, but every piece of registry metadata + * was skipped and the reasoning ladder was advertised without summary support — exactly the + * inconsistency that makes Codex drop the inbound reasoning object. + * + * Deliberately narrow: only the reasoning-summary map, and only via + * `registryEntryForProviderDestination`, which matches fixed key destinations and refuses + * templated or overridable base URLs. A custom row keeps its own identity for everything else. + */ +function enrichReasoningSummariesByDestination(prov: OcxProviderConfig): void { + const destination = registryEntryForProviderDestination(prov); + applyReasoningSummaryDefaults(prov, destination?.modelSupportsReasoningSummaries); +} + export function enrichProviderFromRegistry(name: string, prov: OcxProviderConfig): void { const entry = PROVIDER_REGISTRY.find(row => row.id === name); - if (!entry || !providerMatchesRegistryTransport(name, prov)) return; + if (!entry || !providerMatchesRegistryTransport(name, prov)) { + // Name lookup failed, but the row may still point at a vendor route we know. #1100 was + // reported against a hand-added provider literally named "GLM": routing worked, yet every + // piece of registry metadata was skipped because no registry id is called "GLM". + // `registryEntryForProviderDestination` answers the question that actually matters here — + // which vendor endpoint is this row talking to — and is already restricted to fixed key + // destinations, so a templated or overridable base URL cannot be claimed by it. + enrichReasoningSummariesByDestination(prov); + return; + } const seed = providerConfigSeed(entry); if (prov.apiKeyTransport === undefined && seed.apiKeyTransport !== undefined) prov.apiKeyTransport = seed.apiKeyTransport; if (!prov.defaultModel && seed.defaultModel) prov.defaultModel = seed.defaultModel; @@ -280,12 +327,7 @@ export function enrichProviderFromRegistry(name: string, prov: OcxProviderConfig // the entry so an explicit user value stays distinguishable from the default. if (prov.supportsServiceTier === undefined && entry.supportsServiceTier !== undefined) prov.supportsServiceTier = entry.supportsServiceTier; if (prov.preserveResponsesReasoningContent === undefined && entry.preserveResponsesReasoningContent !== undefined) prov.preserveResponsesReasoningContent = entry.preserveResponsesReasoningContent; - if (entry.modelSupportsReasoningSummaries) { - prov.modelSupportsReasoningSummaries = { - ...entry.modelSupportsReasoningSummaries, - ...(prov.modelSupportsReasoningSummaries ?? {}), - }; - } + applyReasoningSummaryDefaults(prov, entry.modelSupportsReasoningSummaries); // Registry-only repair policy (#938): fill only when the runtime provider has // no explicit policy, and deep-clone so saved/user values never alias the // registry constant. diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index 771ca83714..8f979fafe8 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -24,6 +24,7 @@ import { import type { OcxConfig } from "../src/types"; import type { NormalizedComboConfig } from "../src/combos/types"; import { enrichProviderFromRegistry } from "../src/providers/derive"; +import { enrichProviderFromCatalog } from "../src/oauth/key-providers"; import { handleManagementAPI } from "../src/server/management-api"; import { OAUTH_PROVIDERS } from "../src/oauth"; @@ -2450,6 +2451,85 @@ describe("Codex catalog routed normalization", () => { } }); + test("a custom-named provider on a known vendor endpoint still gets the opt-in (#1100)", () => { + // The reporter's actual shape: a hand-added provider literally named "GLM". Routing worked, + // so the row looked healthy, but no registry id is called "GLM" and every piece of registry + // metadata was skipped — the ladder was advertised with summaries left false, which is the + // exact inconsistency that makes Codex drop the inbound reasoning object. Testing only + // canonical provider ids would have missed this entirely. + const custom: OcxConfig["providers"][string] = { + adapter: "openai-chat", + baseUrl: "https://api.z.ai/api/coding/paas/v4", + authMode: "key", + }; + enrichProviderFromRegistry("GLM", custom); + expect(custom.modelSupportsReasoningSummaries?.["glm-5.2"]).toBe(true); + + // Same for a renamed row pointing at the BigModel pay-as-you-go endpoint. + const renamed: OcxConfig["providers"][string] = { + adapter: "openai-chat", + baseUrl: "https://open.bigmodel.cn/api/paas/v4", + authMode: "key", + }; + enrichProviderFromRegistry("my-glm", renamed); + expect(renamed.modelSupportsReasoningSummaries?.["glm-4.6"]).toBe(true); + }); + + test("the destination fallback never claims an unrelated custom endpoint (#1100)", () => { + // The fallback matches by vendor endpoint. A provider pointing somewhere we do not + // recognize must stay untouched — silently opting a random backend into summary delivery + // would produce upstream 400s the user never asked for. + const unknown: OcxConfig["providers"][string] = { + adapter: "openai-chat", + baseUrl: "https://api.example.invalid/v1", + authMode: "key", + }; + enrichProviderFromRegistry("GLM", unknown); + expect(unknown.modelSupportsReasoningSummaries).toBeUndefined(); + + // An explicit user value wins PER KEY — it does not suppress the other registry defaults. + // An earlier revision of this fallback bailed whenever any user map existed, which + // recreated the whole-record bug the per-key merge was written to avoid: setting one + // model's flag would silently disable the opt-in for every sibling model. + const opinionated: OcxConfig["providers"][string] = { + adapter: "openai-chat", + baseUrl: "https://api.z.ai/api/coding/paas/v4", + authMode: "key", + modelSupportsReasoningSummaries: { "glm-5.2": false }, + }; + enrichProviderFromRegistry("GLM", opinionated); + expect(opinionated.modelSupportsReasoningSummaries).toEqual({ + "glm-5.2": false, + "glm-5.2[1m]": true, + }); + }); + + test("registry summary defaults are never persisted into saved config (#1100)", () => { + // enrichProviderFromCatalog feeds a config that is about to be written to disk. Persisting + // today's registry defaults would freeze them as the user's own overrides, so a later + // registry correction — e.g. learning a model's backend rejects summary delivery — would + // never reach anyone who created their provider first. + const created: OcxConfig["providers"][string] = { + adapter: "openai-chat", + baseUrl: "https://api.deepseek.com", + authMode: "key", + }; + enrichProviderFromCatalog("deepseek", created); + expect(created.modelSupportsReasoningSummaries).toBeUndefined(); + // Other registry seeding still reaches the saved config. + expect(created.models?.length).toBeGreaterThan(0); + + // A value the user actually submitted is preserved verbatim. + const submitted: OcxConfig["providers"][string] = { + adapter: "openai-chat", + baseUrl: "https://api.deepseek.com", + authMode: "key", + modelSupportsReasoningSummaries: { "deepseek-v4-flash": false }, + }; + enrichProviderFromCatalog("deepseek", submitted); + expect(submitted.modelSupportsReasoningSummaries).toEqual({ "deepseek-v4-flash": false }); + }); + test("explicit per-model overrides survive registry backfill", () => { const provider: OcxConfig["providers"][string] = { adapter: "openai-chat", From 392179e704495f17800af4cf7a7b97fdd8fabd8a Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 7 Aug 2026 17:01:08 +0900 Subject: [PATCH 4/6] docs(devlog): record the #1100 audit findings and the deferred BigModel coding endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first implementation passed its tests and was still wrong about the reported configuration: enrichment matches on provider NAME, and the reporter's row is a hand-added provider called "GLM". Recording the failure mode because it generalizes — canonical-id tests were green against a configuration no user had. Also records why the BigModel coding endpoint is deferred rather than fixed here, with the safety analysis for adding it later. --- .../020_routed_reasoning_effort.md | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/devlog/_plan/260807_untouched_bug_stack/020_routed_reasoning_effort.md b/devlog/_plan/260807_untouched_bug_stack/020_routed_reasoning_effort.md index cffde07fd5..99b9b82907 100644 --- a/devlog/_plan/260807_untouched_bug_stack/020_routed_reasoning_effort.md +++ b/devlog/_plan/260807_untouched_bug_stack/020_routed_reasoning_effort.md @@ -109,3 +109,45 @@ provider-scoped or ladder-inferred. `tests/codex-catalog.test.ts` is also touched by PR #1119. If that PR lands first, rebase onto it rather than duplicating its cases. + +## What audit changed after implementation + +The first implementation fixed the canonical provider ids and passed its tests, +and was still wrong about the reported case. Recording why, because the failure +mode generalizes. + +`enrichProviderFromRegistry` matches on the provider NAME. The reporter's row is +a hand-added provider literally called `GLM`. Routing worked, so nothing looked +broken — but no registry id is called `GLM`, so the metadata never arrived. The +tests substituted canonical ids (`zai`, `zhipu-bigmodel`) and were green against +a configuration no user had. + +Fix: on the name-lookup miss, fall back to +`registryEntryForProviderDestination`, which matches by vendor endpoint and is +already restricted to fixed key destinations. + +Two further corrections from the same audit: + +- The fallback originally bailed whenever the user had any map, recreating the + whole-record bug the per-key merge was written to prevent. +- `enrichProviderFromCatalog` persists what it enriches, so registry defaults + were being frozen into saved config as user overrides. + +## Deferred: the reporter's exact endpoint + +`https://open.bigmodel.cn/api/coding/paas/v4` appears in no registry entry — +only `/api/paas/v4` does, as `zhipu-bigmodel`. The coding path exists solely in +`FREE_PROVIDER_DIRECTORY` as `glm-cn`. + +Closing that route needs a new registry entry, and the audit confirmed it would +be safe with a distinct id (`glm` and `glm-cn` are both already bound, and +reusing either would retarget an existing config's endpoint — the warning at +`registry.ts:1668-1676`). It also needs `preserveCustomDestination: true`, its +own evidence-backed model set rather than the pay-as-you-go GLM 4.6–5.1 +metadata, and updates to `EXPECTED_KEY_PROVIDER_IDS` in +`tests/provider-registry-parity.test.ts`. + +That is a provider addition, not a bug fix. It stays out of this stack +deliberately: the destination fallback already fixes every custom-named row on +an endpoint we know, and mixing a new vendor entry into a bug-fix chain would +expand the review surface past what a reviewer can check in one pass. From 07e7525b869611080b0ee0c0eb3e87feccc62540 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sat, 8 Aug 2026 01:41:22 +0900 Subject: [PATCH 5/6] fix(providers): register BigModel's Coding Plan endpoint, which #1100 was actually reported on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The destination fallback added here matched two GLM routes: Z.AI's coding plan and BigModel's pay-as-you-go `/api/paas/v4`. The configuration in the issue is neither. It is `https://open.bigmodel.cn/api/coding/paas/v4` — a third endpoint with no registry row — so the lookup found nothing, `modelSupportsReasoningSummaries` stayed unset, and Codex kept dropping the reasoning object. Effort still displayed as `-`. The test hid this. It was captioned "the reporter's actual shape" and used provider name "GLM" and model glm-5.2, both correct, but substituted Z.AI's host. It passed against a route that already worked while the reported one stayed broken. Found in pre-merge review, not by the suite. A prefix match on `open.bigmodel.cn` would have covered both endpoints in one line. It is also how a config pointed at one vendor route inherits another route's metadata, which is the failure the exact-endpoint rule exists to prevent — so this is a separate row. Two details that are not arbitrary. The id is not `glm-cn`, which the free-provider directory already binds to this same path; registering it here would let routedProviderConfig() canonicalize a saved `glm-cn` config onto our baseUrl. And the model list follows Z.AI's coding-plan set rather than the pay-as-you-go one, because this endpoint is the subscription product and glm-5.2 only exists on that side. Ablation: pointing the new row's baseUrl elsewhere makes the reproduction test red. Refs #1100 --- src/providers/registry.ts | 36 ++++++++++++++++++++++++++ tests/codex-catalog.test.ts | 23 ++++++++++++---- tests/provider-registry-parity.test.ts | 7 +++-- 3 files changed, 59 insertions(+), 7 deletions(-) diff --git a/src/providers/registry.ts b/src/providers/registry.ts index 037f3c8481..e1dc32e400 100644 --- a/src/providers/registry.ts +++ b/src/providers/registry.ts @@ -1707,6 +1707,42 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ // false live claim yields an empty picker at runtime. Flip it on once someone verifies it. note: "Domestic BigModel pay-as-you-go endpoint (open.bigmodel.cn)", }, + // BigModel's Coding Plan is a SEPARATE endpoint from the pay-as-you-go row above, and that is + // the whole reason this one exists. #1100 was reported against + // `https://open.bigmodel.cn/api/coding/paas/v4`; the row above covers only `/api/paas/v4`, so + // destination enrichment matched nothing, `modelSupportsReasoningSummaries` stayed unset, and + // Codex kept dropping the inbound reasoning object — effort displayed as `-`. + // + // A prefix or fuzzy endpoint match would have been the shortcut. It is also how a config + // pointed at one vendor route silently inherits another route's metadata, so endpoints stay + // exact and each one gets its own row. + // + // The id is NOT `glm-cn`, which the free-provider directory already binds to this same coding + // path: registering it here would let routedProviderConfig() canonicalize a saved `glm-cn` + // config onto this baseUrl. Same reasoning as `zhipu-bigmodel` above. + // + // Models follow Z.AI's coding-plan list rather than the pay-as-you-go one. This endpoint is + // the subscription product, and the reporter's `glm-5.2` is only on that side. + { + id: "zhipu-bigmodel-coding", + label: "Zhipu AI — BigModel Coding Plan", + baseUrl: "https://open.bigmodel.cn/api/coding/paas/v4", + adapter: "openai-chat", + authKind: "key", + dashboardUrl: "https://bigmodel.cn/console/usercenter/apikeys", + defaultModel: "glm-5.2", + models: ["glm-5.2", "glm-5.2[1m]", "glm-5.1", "glm-5", "glm-4.6"], + jawcodeBundle: "zai", + modelContextWindows: { "glm-5.2": 1_000_000, "glm-5.2[1m]": 1_000_000 }, + modelSuffixBracketStrip: true, + noVisionModels: ZAI_GLM_52_MODELS, + modelReasoningEfforts: Object.fromEntries(ZAI_GLM_52_MODELS.map(id => [id, ZAI_GLM_52_REASONING_EFFORTS])), + modelSupportsReasoningSummaries: Object.fromEntries(ZAI_GLM_52_MODELS.map(id => [id, true])), + preserveReasoningContentModels: ZAI_GLM_52_MODELS, + // No liveModels: the same reasoning as the pay-as-you-go row — an unverified live claim + // yields an empty picker at runtime. + note: "Domestic BigModel Coding Plan endpoint (open.bigmodel.cn)", + }, { id: "nanogpt", label: "NanoGPT", baseUrl: "https://nano-gpt.com/api/v1", adapter: "openai-chat", authKind: "key", dashboardUrl: "https://nano-gpt.com/api" }, { id: "synthetic", label: "Synthetic", baseUrl: "https://api.synthetic.new/openai/v1", adapter: "openai-chat", authKind: "key", dashboardUrl: "https://synthetic.new" }, // SiliconFlow publishes an OpenAI-compatible chat endpoint and a dynamic model catalog. Do not diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index 8f979fafe8..7e515f8416 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -2452,11 +2452,24 @@ describe("Codex catalog routed normalization", () => { }); test("a custom-named provider on a known vendor endpoint still gets the opt-in (#1100)", () => { - // The reporter's actual shape: a hand-added provider literally named "GLM". Routing worked, - // so the row looked healthy, but no registry id is called "GLM" and every piece of registry - // metadata was skipped — the ladder was advertised with summaries left false, which is the - // exact inconsistency that makes Codex drop the inbound reasoning object. Testing only - // canonical provider ids would have missed this entirely. + // The reporter's ACTUAL configuration, verbatim from #1100: a hand-added provider literally + // named "GLM", model glm-5.2, on BigModel's Coding Plan endpoint. Routing worked, so the row + // looked healthy, but no registry id is called "GLM" and every piece of registry metadata was + // skipped — the ladder was advertised with summaries left false, which is the exact + // inconsistency that makes Codex drop the inbound reasoning object. + // + // This case used to substitute Z.AI's coding endpoint while claiming to be the reporter's + // shape. That passed while the reported configuration stayed broken: `/api/coding/paas/v4` + // on open.bigmodel.cn had no registry row at all, so the destination lookup found nothing. + const reported: OcxConfig["providers"][string] = { + adapter: "openai-chat", + baseUrl: "https://open.bigmodel.cn/api/coding/paas/v4", + authMode: "key", + }; + enrichProviderFromRegistry("GLM", reported); + expect(reported.modelSupportsReasoningSummaries?.["glm-5.2"]).toBe(true); + + // Z.AI's own Coding Plan endpoint is a different vendor route and keeps working. const custom: OcxConfig["providers"][string] = { adapter: "openai-chat", baseUrl: "https://api.z.ai/api/coding/paas/v4", diff --git a/tests/provider-registry-parity.test.ts b/tests/provider-registry-parity.test.ts index 37bc5467a4..94581a5300 100644 --- a/tests/provider-registry-parity.test.ts +++ b/tests/provider-registry-parity.test.ts @@ -32,7 +32,7 @@ function nativeTemplate(): Record { const EXPECTED_KEY_PROVIDER_IDS = [ "anthropic-apikey", "openai-apikey", "umans", "opencode-go", "neuralwatt", "openrouter", "cline-pass", "cline", "orcarouter", "bizrouter", "groq", "google", "google-vertex", "azure-openai", "deepseek", "cerebras", "deepinfra", "hyperbolic", "nscale", "vultr", "baseten", "commandcode", "sambanova", "nebius", "digitalocean", "scaleway", "together", "fireworks", "firepass", "moonshot", - "huggingface", "nvidia", "venice", "zai", "zhipu-bigmodel", "nanogpt", "synthetic", "siliconflow", "qwen-cloud", "tencent-coding-plan", + "huggingface", "nvidia", "venice", "zai", "zhipu-bigmodel", "zhipu-bigmodel-coding", "nanogpt", "synthetic", "siliconflow", "qwen-cloud", "tencent-coding-plan", "volcengine", "volcengine-coding-plan", "volcengine-agent-plan", "qianfan", "alibaba", "alibaba-token-plan", "alibaba-token-plan-intl", "parallel", "zenmux", "litellm", "ollama-cloud", "mistral", "minimax", "minimax-cn", "kimi-code", "opencode-zen", "vercel-ai-gateway", "opencode-free", "xiaomi", "kilo", "mimo-free", "cloudflare-ai-gateway", "cloudflare-workers-ai", "gitlab-duo", @@ -338,7 +338,9 @@ describe("provider registry parity", () => { .map(entry => entry.id); expect(zai?.modelContextWindows).toEqual({ "glm-5.2": 1_000_000, "glm-5.2[1m]": 1_000_000 }); expect(providerConfigSeed(zai!).modelSuffixBracketStrip).toBe(true); - expect(optedInProviders).toEqual(["kimi", "zai", "kimi-code"]); + // `zhipu-bigmodel-coding` opts in for the same reason `zai` does: it serves the same + // bracketed GLM ids, and that vendor's OpenAI path returns 400 code 1211 for them. + expect(optedInProviders).toEqual(["kimi", "zai", "zhipu-bigmodel-coding", "kimi-code"]); const config: OcxConfig = { port: 10100, @@ -763,6 +765,7 @@ describe("provider registry parity", () => { minimax: "minimax", "minimax-cn": "minimax", "zhipu-bigmodel": "zai", + "zhipu-bigmodel-coding": "zai", }); expect(resolveJawcodeProvider("gemini")).toBe("google"); expect(resolveJawcodeProvider("minimax-cn")).toBe("minimax"); From aca150b357101734ada307a3244b2a97875e8ab3 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sat, 8 Aug 2026 02:09:44 +0900 Subject: [PATCH 6/6] test(codex): accept SQLite contention as a pre-approval race, matching the product code The serialization test excluded two pre-approval race outcomes and treated everything else as a seam failure. A macOS CI run produced a third: `SQLiteError: database is locked` on stderr, which failed the build. That contradicts the runtime. `configGenerationFailureReason` classifies that exact message as "busy" rather than a database fault, and the storage and history paths do the same. So the product already treats it as ordinary contention while this test called it a defect. Found by a red CI run rather than by the suite, which is the part worth noting: the test enumerated the races it had seen instead of the races the code recognizes, so a third one was a matter of timing rather than of whether it could happen. The two-process seam itself is unchanged and still fails on a genuine convergence error. --- .../codex-retained-root-serialization.test.ts | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tests/codex-retained-root-serialization.test.ts b/tests/codex-retained-root-serialization.test.ts index c4bf147cc8..b7bebef5d8 100644 --- a/tests/codex-retained-root-serialization.test.ts +++ b/tests/codex-retained-root-serialization.test.ts @@ -495,13 +495,23 @@ test("two processes at the post-approval management seam serialize instead of in for (const result of results) { // A process can lose a race BEFORE approval and never reach the seam at all. - // Both known cases come from `saveConfigPreservingClaudeCode`: the config - // mutation lock is already held, or two cold processes create the ownership - // file at once. Neither says anything about catalog convergence, so they are - // excluded here — but only these two, so a genuine seam failure still fails. + // The known cases come from `saveConfigPreservingClaudeCode`: the config mutation + // lock is already held, two cold processes create the ownership file at once, or + // SQLite refuses the transaction outright while another process holds it. None of + // them say anything about catalog convergence, so they are excluded here — but only + // these, so a genuine seam failure still fails. + // + // The third case was found by a CI failure on macOS, not by this suite. The lock + // helper normally wraps busy errors in `ConfigMutationLockError`, but the raw + // `SQLiteError: database is locked` can still reach stderr from a path that has not + // wrapped it yet. `configGenerationFailureReason` already classifies that exact + // message as "busy" rather than a database fault, so treating it as a seam failure + // here contradicted the product code and turned ordinary contention into a red build. if (result.exitCode !== 0) { const preApproval = result.stderr.includes("CONFIG_MUTATION_LOCK_UNAVAILABLE") - || (result.stderr.includes("EEXIST") && result.stderr.includes("createOwnership")); + || (result.stderr.includes("EEXIST") && result.stderr.includes("createOwnership")) + || /database (?:is|table is) locked/i.test(result.stderr) + || result.stderr.includes("SQLITE_BUSY"); expect({ preApproval, stderr: result.stderr }).toMatchObject({ preApproval: true }); continue; }