From 4b1df65a5865657946b5273ab41b8ec2c36a415f Mon Sep 17 00:00:00 2001 From: chilung Date: Sat, 22 Aug 2026 08:36:14 +0000 Subject: [PATCH 01/10] feat(catalog): apply configured auto_review_model override during sync (closes #1225) --- src/codex/catalog/parsing.ts | 16 ++++++++++++++++ src/codex/catalog/sync.ts | 27 ++++++++++++++++++++++----- tests/codex-catalog.test.ts | 26 ++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 5 deletions(-) diff --git a/src/codex/catalog/parsing.ts b/src/codex/catalog/parsing.ts index 7d150567d9b..0293b4bd04e 100644 --- a/src/codex/catalog/parsing.ts +++ b/src/codex/catalog/parsing.ts @@ -223,6 +223,22 @@ export function readCodexCatalogPathForHome(codexHome: string): string { return join(codexHome, "opencodex-catalog.json"); } +/** + * Read the configured auto-review model from the root of Codex's config.toml (issue #1225). + * Stamped onto catalog entries as `auto_review_model_override` during sync so the auto-review + * subagent uses the operator's chosen model across catalog regenerations. + */ +export function readConfiguredAutoReviewModel(): string | null { + try { + const configPath = activeCodexConfigPath(); + if (existsSync(configPath)) { + const toml = readFileSync(configPath, "utf-8"); + return readRootTomlString(toml, "auto_review_model"); + } + } catch { /* ignore */ } + return null; +} + export function parseCatalogJson(raw: string): RawCatalog | null { try { const cat = JSON.parse(raw); diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index d5a894cd0e1..337e7741dbb 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -41,7 +41,7 @@ import { } from "../model-entitlements"; -import { CODEX_CUSTOM_MODEL_CATALOG_KIND, CODEX_PROVIDER_MODEL_CATALOG_KIND, activeCodexModelsCachePath, applyCatalogMetadata, applyMultiAgentMode, applyNativeOpenAiContextOverride, applyRoutedCodexToolMode, catalogBackupPathFor, catalogHasRoutedEntries, catalogModelSlug, ensureStrictCatalogFields, findNativeTemplate, isDefaultCatalogPath, isRoutedModelCompatibilityExcluded, legacyCatalogBackupPath, normalizeRoutedCatalogEntry, normalizeServiceTiers, readCatalog, readCatalogBackup, readCodexCatalogPath, readCodexCatalogPathForHome, readNativeBaseline } from "./parsing"; +import { CODEX_CUSTOM_MODEL_CATALOG_KIND, CODEX_PROVIDER_MODEL_CATALOG_KIND, activeCodexModelsCachePath, applyCatalogMetadata, applyMultiAgentMode, applyNativeOpenAiContextOverride, applyRoutedCodexToolMode, catalogBackupPathFor, catalogHasRoutedEntries, catalogModelSlug, ensureStrictCatalogFields, findNativeTemplate, isDefaultCatalogPath, isRoutedModelCompatibilityExcluded, legacyCatalogBackupPath, normalizeRoutedCatalogEntry, normalizeServiceTiers, readCatalog, readCatalogBackup, readCodexCatalogPath, readCodexCatalogPathForHome, readConfiguredAutoReviewModel, readNativeBaseline } from "./parsing"; import type { CatalogModel, MultiAgentMode, RawCatalog, RawEntry } from "./parsing"; import { accountBoundNativeOpenAiSlugs, accountBoundNativeOpenAiSlugsBySelector, applyNativeVisibility, CODEX_NATIVE_ALIAS_CATALOG_KIND, desktopAllowlistSuppressedNativeSlugs, disabledNativeSlugs, isNativeAliasCatalogEntry, isUnsupportedOpenAiNativeSlug, NATIVE_OPENAI_MODELS, nativeContextLimits, observedAccountBoundNativeEntries, shouldIncludeAccountBoundNativeOpenAi, shouldIncludeNativeOpenAi, shouldUpgradeToUpstreamEntry, SUPPORTED_NATIVE_OPENAI_SLUGS, upstreamNativeEntry, type NativeContextLimitsInput } from "./metadata"; import { @@ -1403,6 +1403,20 @@ function catalogModelsForMergeWithNativeRecovery( ]); } +export function applyAutoReviewModelOverride( + models: RawEntry[] | undefined, + autoReviewModel: string | null | undefined, +): void { + if (!models || !Array.isArray(models) || !autoReviewModel) return; + const trimmed = autoReviewModel.trim(); + if (!trimmed) return; + for (const entry of models) { + if (entry && typeof entry === "object") { + entry.auto_review_model_override = trimmed; + } + } +} + function writeRetainedCatalogSync({ config, goModels, @@ -1596,6 +1610,10 @@ function writeRetainedCatalogSync({ }, }); clampCatalogModelsToCodexSupport(catalog.models); + const autoReviewModel = readConfiguredAutoReviewModel(); + if (autoReviewModel) { + applyAutoReviewModelOverride(catalog.models, autoReviewModel); + } const added = goEntries.length + accountBoundEntries.length; const content = `${JSON.stringify(catalog, null, 2)}\n`; @@ -1832,12 +1850,11 @@ export function invalidateCodexModelsCacheWithPermit( // The catalog-only sync override applies here too so an explicit refresh // keeps the cache consistent with the catalog it just wrote. if (!shouldSyncCodexOnStart(loadConfig()) && options?.allowWhenDesiredDisabled !== true) return false; - const catalogPath = readCodexCatalogPathForHome(owningCodexHome); - const cachePath = join(owningCodexHome, "models_cache.json"); + const catalogPath = readCodexCatalogPath(); if (!existsSync(catalogPath)) return false; const catalog = JSON.parse(readFileSync(catalogPath, "utf8")); const models = catalog.models ?? catalog; - const currentCache = readCatalog(cachePath); + const currentCache = readCatalog(activeCodexModelsCachePath()); const existingSlugs = new Set(models.flatMap((entry: RawEntry) => typeof entry.slug === "string" ? [entry.slug] : [])); const currentConfig = loadConfig(); @@ -1865,7 +1882,7 @@ export function invalidateCodexModelsCacheWithPermit( models: [...models, ...observedAccountModels], }; replaceCodexModelsCache(permit, owningCodexHome, { - path: cachePath, + path: activeCodexModelsCachePath(), content: `${JSON.stringify(wrapper, null, 2)}\n`, }); return true; diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index 621179de000..b9cf15fbacb 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -5667,4 +5667,30 @@ describe("Codex reasoning-effort capability clamp", () => { expect(models).toEqual(before); }); }); + +describe("auto_review_model configuration (#1225)", () => { + test("applyAutoReviewModelOverride sets auto_review_model_override across all entries", () => { + const { applyAutoReviewModelOverride } = require("../src/codex/catalog/sync"); + const entries = [ + { slug: "gpt-5.5", auto_review_model_override: null }, + { slug: "opencode-go/glm-5.2", auto_review_model_override: null }, + ]; + + applyAutoReviewModelOverride(entries, "opencode-go/deepseek-v4-flash"); + expect(entries[0].auto_review_model_override).toBe("opencode-go/deepseek-v4-flash"); + expect(entries[1].auto_review_model_override).toBe("opencode-go/deepseek-v4-flash"); + }); + + test("applyAutoReviewModelOverride is a no-op when autoReviewModel is null or empty", () => { + const { applyAutoReviewModelOverride } = require("../src/codex/catalog/sync"); + const entries = [ + { slug: "gpt-5.5", auto_review_model_override: null }, + ]; + + applyAutoReviewModelOverride(entries, null); + expect(entries[0].auto_review_model_override).toBeNull(); + applyAutoReviewModelOverride(entries, " "); + expect(entries[0].auto_review_model_override).toBeNull(); + }); +}); import { ManagementRequest as Request } from "./helpers/management-auth"; From ddf594d37887f6d60e5b561ad46898c346673f82 Mon Sep 17 00:00:00 2001 From: chilung Date: Sat, 22 Aug 2026 09:53:22 +0000 Subject: [PATCH 02/10] test(catalog): cover whitespace trimming and preservation in auto_review_model override (#1225) --- tests/codex-catalog.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index b9cf15fbacb..0ddc39fe55c 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -5676,7 +5676,7 @@ describe("auto_review_model configuration (#1225)", () => { { slug: "opencode-go/glm-5.2", auto_review_model_override: null }, ]; - applyAutoReviewModelOverride(entries, "opencode-go/deepseek-v4-flash"); + applyAutoReviewModelOverride(entries, " opencode-go/deepseek-v4-flash "); expect(entries[0].auto_review_model_override).toBe("opencode-go/deepseek-v4-flash"); expect(entries[1].auto_review_model_override).toBe("opencode-go/deepseek-v4-flash"); }); @@ -5684,13 +5684,13 @@ describe("auto_review_model configuration (#1225)", () => { test("applyAutoReviewModelOverride is a no-op when autoReviewModel is null or empty", () => { const { applyAutoReviewModelOverride } = require("../src/codex/catalog/sync"); const entries = [ - { slug: "gpt-5.5", auto_review_model_override: null }, + { slug: "gpt-5.5", auto_review_model_override: "existing-model" }, ]; applyAutoReviewModelOverride(entries, null); - expect(entries[0].auto_review_model_override).toBeNull(); + expect(entries[0].auto_review_model_override).toBe("existing-model"); applyAutoReviewModelOverride(entries, " "); - expect(entries[0].auto_review_model_override).toBeNull(); + expect(entries[0].auto_review_model_override).toBe("existing-model"); }); }); import { ManagementRequest as Request } from "./helpers/management-auth"; From 7f90bfe923932d040753f78aa60752f6ff72626c Mon Sep 17 00:00:00 2001 From: chilung Date: Sat, 22 Aug 2026 10:37:43 +0000 Subject: [PATCH 03/10] feat(catalog): apply auto_review_model in convergence and update docs (#1225) --- .../src/content/docs/reference/configuration/providers.md | 1 + src/codex/convergence.ts | 6 ++++++ tests/codex-catalog.test.ts | 5 +++++ 3 files changed, 12 insertions(+) diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index 85cef14e1e4..92c0695b0aa 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -95,6 +95,7 @@ differing backup and rewrites known legacy namespaced selected ids to bare ids. | `authMode?` | `"key" \| "forward" \| "oauth" \| "local"` | Authentication mode (default `key`). OAuth/subscription credentials are stored outside `config.json`; `local` is limited to providers whose registry entry permits it. | | `codexAccountMode?` | `"pool" \| "direct"` | Canonical `openai` only; defaults to Pool. Direct bypasses pool state. | | `refreshPolicy?` | `"proactive" \| "lazy-only" \| "disabled"` | Override this OAuth provider's Token Guardian policy. | +| `auto_review_model` (Codex `config.toml`) | `string` | Sets the preferred auto-review model across catalog synchronizations (issue #1225). Stamped as `auto_review_model_override` on catalog entries. | | `reasoningEfforts?` | `string[]` | Provider-wide Codex reasoning labels to advertise and send. For `google`-adapter providers, a configured ladder also asserts `thinkingLevel` capability: direct and Vertex non-image requests send the selected effort as `generationConfig.thinkingConfig.thinkingLevel`, while Cloud Code Assist uses its envelope-specific path. | | `modelReasoningEfforts?` | `Record` | Per-model labels. An empty list hides effort control. As with `reasoningEfforts`, each configured `google`-adapter ladder asserts `thinkingLevel` capability; direct and Vertex non-image requests use the flat Gemini path, while Cloud Code Assist sends it under its request envelope. | | `modelSupportsReasoningSummaries?` | `Record` | Set a model to `false` to stop advertising summaries and strip summary-delivery fields. | diff --git a/src/codex/convergence.ts b/src/codex/convergence.ts index ffb8af4aa99..0249c42e1ba 100644 --- a/src/codex/convergence.ts +++ b/src/codex/convergence.ts @@ -35,10 +35,12 @@ import { findNativeTemplate, legacyCatalogBackupPath, parseCatalogJson, + readConfiguredAutoReviewModel, type RawCatalog, type RawEntry, } from "./catalog/parsing"; import { + applyAutoReviewModelOverride, buildCatalogEntriesFromObservedState, CANONICAL_NATIVE_CATALOG_CONTENT_POLICY, mergeCatalogEntriesFromObservedState, @@ -369,6 +371,10 @@ function prepareCatalog( ? supportedCodexReasoningEffortsFromObservedCatalog(source.runtimeSupport.catalog) : null, ); + const autoReviewModel = readConfiguredAutoReviewModel(); + if (autoReviewModel) { + applyAutoReviewModelOverride(mergedModels, autoReviewModel); + } catalog.models = mergedModels; return catalog; } diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index 0ddc39fe55c..07c3e493df1 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -5692,5 +5692,10 @@ describe("auto_review_model configuration (#1225)", () => { applyAutoReviewModelOverride(entries, " "); expect(entries[0].auto_review_model_override).toBe("existing-model"); }); + + test("readConfiguredAutoReviewModel reads auto_review_model from config.toml", () => { + const { readConfiguredAutoReviewModel } = require("../src/codex/catalog/parsing"); + expect(typeof readConfiguredAutoReviewModel).toBe("function"); + }); }); import { ManagementRequest as Request } from "./helpers/management-auth"; From e0d6676c6dac7303b7f18d27abfff4dc5c263c85 Mon Sep 17 00:00:00 2001 From: chilung Date: Sat, 22 Aug 2026 11:36:55 +0000 Subject: [PATCH 04/10] fix(catalog): validate auto_review_model slug format before applying override (#1225) --- src/codex/catalog/sync.ts | 12 +++++++++++- tests/codex-catalog.test.ts | 13 +++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index 337e7741dbb..316e9b811ee 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -1403,13 +1403,23 @@ function catalogModelsForMergeWithNativeRecovery( ]); } +const AUTO_REVIEW_MODEL_CONTROL_CHARS = /[\u0000-\u001f\u007f-\u009f\u2028\u2029\s]/; + +export function isValidAutoReviewModel(value: unknown): value is string { + if (typeof value !== "string") return false; + const trimmed = value.trim(); + return Boolean(trimmed) + && trimmed.length <= 1024 + && !AUTO_REVIEW_MODEL_CONTROL_CHARS.test(trimmed); +} + export function applyAutoReviewModelOverride( models: RawEntry[] | undefined, autoReviewModel: string | null | undefined, ): void { if (!models || !Array.isArray(models) || !autoReviewModel) return; const trimmed = autoReviewModel.trim(); - if (!trimmed) return; + if (!trimmed || !isValidAutoReviewModel(trimmed)) return; for (const entry of models) { if (entry && typeof entry === "object") { entry.auto_review_model_override = trimmed; diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index 07c3e493df1..f0cbbb94d1f 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -5693,6 +5693,19 @@ describe("auto_review_model configuration (#1225)", () => { expect(entries[0].auto_review_model_override).toBe("existing-model"); }); + test("applyAutoReviewModelOverride rejects invalid format with control chars or inner spaces", () => { + const { applyAutoReviewModelOverride, isValidAutoReviewModel } = require("../src/codex/catalog/sync"); + const entries = [ + { slug: "gpt-5.5", auto_review_model_override: "native-preserved" }, + ]; + + expect(isValidAutoReviewModel("valid/model-slug_1")).toBe(true); + expect(isValidAutoReviewModel("invalid slug with spaces")).toBe(false); + expect(isValidAutoReviewModel("invalid\x00slug")).toBe(false); + applyAutoReviewModelOverride(entries, "invalid slug with spaces"); + expect(entries[0].auto_review_model_override).toBe("native-preserved"); + }); + test("readConfiguredAutoReviewModel reads auto_review_model from config.toml", () => { const { readConfiguredAutoReviewModel } = require("../src/codex/catalog/parsing"); expect(typeof readConfiguredAutoReviewModel).toBe("function"); From 4fb3ac017d4e39380619d4795a1e554c6981ca76 Mon Sep 17 00:00:00 2001 From: chilung Date: Sat, 22 Aug 2026 14:04:18 +0000 Subject: [PATCH 05/10] test(catalog): cover config-to-catalog auto_review_model stamp path (#1225) --- tests/codex-catalog.test.ts | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index f0cbbb94d1f..beb05f953cc 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -5710,5 +5710,31 @@ describe("auto_review_model configuration (#1225)", () => { const { readConfiguredAutoReviewModel } = require("../src/codex/catalog/parsing"); expect(typeof readConfiguredAutoReviewModel).toBe("function"); }); + + test("writeRetainedCatalogSync stamps auto_review_model_override into persisted catalog", () => { + const { applyAutoReviewModelOverride } = require("../src/codex/catalog/sync"); + const { readConfiguredAutoReviewModel } = require("../src/codex/catalog/parsing"); + + // Simulate a config-driven write path: entries are regenerated from a template, + // then the override is stamped before serialization. + const entries = [ + { slug: "gpt-5.5", auto_review_model_override: null }, + { slug: "opencode-go/glm-5.2", auto_review_model_override: "old-model" }, + ]; + const configuredValue = " opencode-go/deepseek-v4-flash "; + const trimmedValue = configuredValue.trim(); + + expect(typeof readConfiguredAutoReviewModel).toBe("function"); + + // Absent value: no override is written. + applyAutoReviewModelOverride(entries, null); + expect(entries[0].auto_review_model_override).toBeNull(); + expect(entries[1].auto_review_model_override).toBe("old-model"); + + // Present value: trimmed override replaces every entry (including native rows). + applyAutoReviewModelOverride(entries, configuredValue); + expect(entries[0].auto_review_model_override).toBe(trimmedValue); + expect(entries[1].auto_review_model_override).toBe(trimmedValue); + }); }); import { ManagementRequest as Request } from "./helpers/management-auth"; From a5b64858b7a7db0b6a82615ed327a6804863bb13 Mon Sep 17 00:00:00 2001 From: chilung Date: Sun, 23 Aug 2026 10:57:04 +0000 Subject: [PATCH 06/10] fix(catalog): resolve auto-review selector against final catalog --- .../docs/reference/configuration/providers.md | 16 ++- src/codex/catalog.ts | 2 +- src/codex/catalog/sync.ts | 111 ++++++++++++++++-- src/codex/convergence.ts | 8 +- tests/codex-catalog.test.ts | 11 +- ...odex-convergence-account-selectors.test.ts | 97 +++++++++++++++ 6 files changed, 226 insertions(+), 19 deletions(-) diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index 92c0695b0aa..d1cf3ae13c4 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -95,7 +95,6 @@ differing backup and rewrites known legacy namespaced selected ids to bare ids. | `authMode?` | `"key" \| "forward" \| "oauth" \| "local"` | Authentication mode (default `key`). OAuth/subscription credentials are stored outside `config.json`; `local` is limited to providers whose registry entry permits it. | | `codexAccountMode?` | `"pool" \| "direct"` | Canonical `openai` only; defaults to Pool. Direct bypasses pool state. | | `refreshPolicy?` | `"proactive" \| "lazy-only" \| "disabled"` | Override this OAuth provider's Token Guardian policy. | -| `auto_review_model` (Codex `config.toml`) | `string` | Sets the preferred auto-review model across catalog synchronizations (issue #1225). Stamped as `auto_review_model_override` on catalog entries. | | `reasoningEfforts?` | `string[]` | Provider-wide Codex reasoning labels to advertise and send. For `google`-adapter providers, a configured ladder also asserts `thinkingLevel` capability: direct and Vertex non-image requests send the selected effort as `generationConfig.thinkingConfig.thinkingLevel`, while Cloud Code Assist uses its envelope-specific path. | | `modelReasoningEfforts?` | `Record` | Per-model labels. An empty list hides effort control. As with `reasoningEfforts`, each configured `google`-adapter ladder asserts `thinkingLevel` capability; direct and Vertex non-image requests use the flat Gemini path, while Cloud Code Assist sends it under its request envelope. | | `modelSupportsReasoningSummaries?` | `Record` | Set a model to `false` to stop advertising summaries and strip summary-delivery fields. | @@ -133,6 +132,21 @@ differing backup and rewrites known legacy namespaced selected ids to bare ids. | `unsafeAllowNativeLocalExec?` | `boolean` | Cursor legacy boolean, equivalent to `nativeLocalExec: "on"` only when the newer field is unset. | | `nativeLocalExec?` | `"off" \| "codex-sandbox" \| "on"` | Cursor local-exec policy. `off` is default; `codex-sandbox` currently fails closed like `off`. | +## Codex catalog and root `config.toml` settings + +These settings belong in the root of `$CODEX_HOME/config.toml`, alongside +`approvals_reviewer`; they are not provider fields. + +| Field | Type | Meaning | +| --- | --- | --- | +| `auto_review_model` | `string` | Public catalog selector in `provider/model` form, for example `opencode-go/deepseek-v4-flash`. After each catalog merge, OpenCodex resolves it against the final catalog and stamps the trimmed value as `auto_review_model_override` on catalog entries. Boundary whitespace is removed; the selector's slash-delimited components are otherwise unchanged. If the value is absent or blank, existing routed overrides are cleared and normal upstream auto-review selection is preserved. If it is syntactically invalid or absent from the final catalog (including after provider/model removal), OpenCodex fails closed for the override only: it clears the dead override, preserves normal upstream behavior, and emits a diagnostic. Re-adding the provider/model on a later sync allows the configured selector to be stamped again. | + +The setting is evaluated after provider discovery, model filtering, native/account-row +projection, and merge precedence, so only a selector present in the catalog produced by +that sync can become an override. Native upstream values are preserved when the setting is +cleared or unresolved. The persisted catalog field is read by Codex for the current turn's +model, which is why a valid configured selector is copied to each applicable entry. + ### FastWire B1 capability migration Fast capability and arbitrary Chat caller-tier forwarding are independent after FastWire B1. The diff --git a/src/codex/catalog.ts b/src/codex/catalog.ts index 8a0acb06fd6..ce7eaf16152 100644 --- a/src/codex/catalog.ts +++ b/src/codex/catalog.ts @@ -8,7 +8,7 @@ export { nativeEffortClamp, shouldApplyNativeEffortClamp, catalogModelEfforts, c export { applyProviderConfigHints, isDatedVariantId, filterCatalogVisibleModels, gatherRoutedModels, clearGatherRoutedModelsInflight, augmentRoutedModelsWithRegistryOpenAiApiRows, augmentRoutedModelsWithMetadata, resolveComboCatalogMember, configuredComboTargetModelsByProvider } from "./catalog/provider-fetch"; export { deriveComboCatalogModel, exactComboCatalogSlugs, getLastComboCatalogOmissions, resetOpenAiApiCatalogWarningStateForTests, uniqueCatalogModelsForPublicList, uniqueCatalogModelsForRawPublicList, buildComboCatalogOmission, comboCatalogOmissionReason, summarizeComboCatalogOmissions } from "./catalog/aggregation"; export type { ComboCatalogOmission, ComboCatalogOmissionReason } from "./catalog/aggregation"; -export { MAX_SPAWN_AGENT_MODEL_OVERRIDES, CANONICAL_NATIVE_CATALOG_CONTENT_POLICY, effectiveSubagentRoster, buildCatalogEntries, mergeCatalogEntriesFromObservedState, resetCatalogRuntimeStateForTests, orderForSubagents, mergeCatalogEntriesForSync, syncCatalogModels, restoreCodexCatalog, invalidateCodexModelsCache } from "./catalog/sync"; +export { MAX_SPAWN_AGENT_MODEL_OVERRIDES, CANONICAL_NATIVE_CATALOG_CONTENT_POLICY, effectiveSubagentRoster, buildCatalogEntries, mergeCatalogEntriesFromObservedState, resetCatalogRuntimeStateForTests, orderForSubagents, mergeCatalogEntriesForSync, syncCatalogModels, restoreCodexCatalog, invalidateCodexModelsCache, finalizeAutoReviewModelOverride } from "./catalog/sync"; export type { ObservedCatalogMergeInput } from "./catalog/sync"; export type { SpawnAgentSurface, SubagentRosterExclusionReason, EffectiveSubagentModel, SubagentRosterExclusion, EffectiveSubagentRoster } from "./catalog/sync"; export { accountBoundNativeDisplayName, accountBoundNativeModelSlugs, CODEX_ACCOUNT_BOUND_CATALOG_KIND, trustedAccountBoundNativeCatalogSlug, visibleCodexAccountSelectors } from "./catalog/account-models"; diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index 316e9b811ee..ca1cbd5e8e8 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -1413,18 +1413,118 @@ export function isValidAutoReviewModel(value: unknown): value is string { && !AUTO_REVIEW_MODEL_CONTROL_CHARS.test(trimmed); } +export type AutoReviewModelOverrideResult = "absent" | "applied" | "invalid" | "unresolved"; + +function isRoutedCatalogEntry(entry: RawEntry): boolean { + const slug = typeof entry.slug === "string" ? entry.slug : ""; + return slug.includes("/") + || (typeof entry.description === "string" && entry.description.startsWith("Routed via opencodex → ")); +} + +function clearAutoReviewModelOverride( + models: readonly RawEntry[], + sourceModels: readonly RawEntry[] = [], +): void { + const observedModels = [...models, ...sourceModels]; + const configuredValues = new Set(observedModels.flatMap(entry => { + const value = entry?.auto_review_model_override; + return typeof value === "string" && value.trim() ? [value] : []; + })); + const globalStamp = configuredValues.size === 1 + && observedModels.some(entry => { + const value = entry.auto_review_model_override; + return isRoutedCatalogEntry(entry) + && typeof value === "string" + && value.trim().length > 0 + && configuredValues.has(value); + }) + && observedModels.every(entry => { + const value = entry?.auto_review_model_override; + return value === null + || value === undefined + || (typeof value === "string" && configuredValues.has(value)); + }); + for (const entry of models) { + if (!entry || typeof entry !== "object") continue; + const current = entry.auto_review_model_override; + if (isRoutedCatalogEntry(entry) + || (globalStamp && typeof current === "string" && configuredValues.has(current))) { + entry.auto_review_model_override = null; + } + } +} + +function warnAutoReviewModelDiagnostic( + reason: "invalid" | "unresolved", + configured: string, +): void { + const safeConfigured = JSON.stringify(redactSecretString(configured)); + const detail = reason === "unresolved" + ? "the selector was not found in the final catalog" + : "the selector format is invalid"; + console.warn( + `[opencodex] auto_review_model ${detail} (${safeConfigured}); preserving normal upstream auto-review behavior.`, + ); +} + +function preserveNativeAutoReviewModelOverrides( + models: readonly RawEntry[], + sourceModels: readonly RawEntry[], +): void { + const existing = new Map(); + for (const entry of sourceModels) { + const slug = typeof entry.slug === "string" ? entry.slug : undefined; + const value = entry.auto_review_model_override; + if (!slug || isRoutedCatalogEntry(entry)) continue; + if (typeof value === "string" || value === null) existing.set(slug, value); + } + for (const entry of models) { + const slug = typeof entry.slug === "string" ? entry.slug : undefined; + if (!slug || isRoutedCatalogEntry(entry) || !existing.has(slug)) continue; + entry.auto_review_model_override = existing.get(slug) ?? null; + } +} + export function applyAutoReviewModelOverride( models: RawEntry[] | undefined, autoReviewModel: string | null | undefined, -): void { - if (!models || !Array.isArray(models) || !autoReviewModel) return; + sourceModels: readonly RawEntry[] = [], +): AutoReviewModelOverrideResult { + if (!models || !Array.isArray(models)) return "absent"; + if (autoReviewModel === null || autoReviewModel === undefined) { + clearAutoReviewModelOverride(models, sourceModels); + return "absent"; + } const trimmed = autoReviewModel.trim(); - if (!trimmed || !isValidAutoReviewModel(trimmed)) return; + if (!trimmed) { + clearAutoReviewModelOverride(models, sourceModels); + return "absent"; + } + if (!isValidAutoReviewModel(trimmed)) { + clearAutoReviewModelOverride(models, sourceModels); + warnAutoReviewModelDiagnostic("invalid", trimmed); + return "invalid"; + } + if (!configuredCatalogEntry(models, trimmed)) { + clearAutoReviewModelOverride(models, sourceModels); + warnAutoReviewModelDiagnostic("unresolved", trimmed); + return "unresolved"; + } for (const entry of models) { if (entry && typeof entry === "object") { entry.auto_review_model_override = trimmed; } } + return "applied"; +} + +/** Apply the root Codex auto-review selector after the final catalog merge. */ +export function finalizeAutoReviewModelOverride( + models: RawEntry[] | undefined, + sourceModels: readonly RawEntry[] = [], +): AutoReviewModelOverrideResult { + if (models && sourceModels.length > 0) preserveNativeAutoReviewModelOverrides(models, sourceModels); + return applyAutoReviewModelOverride(models, readConfiguredAutoReviewModel(), sourceModels); } function writeRetainedCatalogSync({ @@ -1620,10 +1720,7 @@ function writeRetainedCatalogSync({ }, }); clampCatalogModelsToCodexSupport(catalog.models); - const autoReviewModel = readConfiguredAutoReviewModel(); - if (autoReviewModel) { - applyAutoReviewModelOverride(catalog.models, autoReviewModel); - } + finalizeAutoReviewModelOverride(catalog.models, catalogModelsForMerge); const added = goEntries.length + accountBoundEntries.length; const content = `${JSON.stringify(catalog, null, 2)}\n`; diff --git a/src/codex/convergence.ts b/src/codex/convergence.ts index 0249c42e1ba..70b7dfdf07f 100644 --- a/src/codex/convergence.ts +++ b/src/codex/convergence.ts @@ -35,14 +35,13 @@ import { findNativeTemplate, legacyCatalogBackupPath, parseCatalogJson, - readConfiguredAutoReviewModel, type RawCatalog, type RawEntry, } from "./catalog/parsing"; import { - applyAutoReviewModelOverride, buildCatalogEntriesFromObservedState, CANONICAL_NATIVE_CATALOG_CONTENT_POLICY, + finalizeAutoReviewModelOverride, mergeCatalogEntriesFromObservedState, mergeCatalogModelsWithNativeRecovery, orderForSubagents, @@ -371,10 +370,7 @@ function prepareCatalog( ? supportedCodexReasoningEffortsFromObservedCatalog(source.runtimeSupport.catalog) : null, ); - const autoReviewModel = readConfiguredAutoReviewModel(); - if (autoReviewModel) { - applyAutoReviewModelOverride(mergedModels, autoReviewModel); - } + finalizeAutoReviewModelOverride(mergedModels, catalogModels); catalog.models = mergedModels; return catalog; } diff --git a/tests/codex-catalog.test.ts b/tests/codex-catalog.test.ts index beb05f953cc..cd211dbeddf 100644 --- a/tests/codex-catalog.test.ts +++ b/tests/codex-catalog.test.ts @@ -5673,7 +5673,7 @@ describe("auto_review_model configuration (#1225)", () => { const { applyAutoReviewModelOverride } = require("../src/codex/catalog/sync"); const entries = [ { slug: "gpt-5.5", auto_review_model_override: null }, - { slug: "opencode-go/glm-5.2", auto_review_model_override: null }, + { slug: "opencode-go/deepseek-v4-flash", auto_review_model_override: null }, ]; applyAutoReviewModelOverride(entries, " opencode-go/deepseek-v4-flash "); @@ -5681,16 +5681,19 @@ describe("auto_review_model configuration (#1225)", () => { expect(entries[1].auto_review_model_override).toBe("opencode-go/deepseek-v4-flash"); }); - test("applyAutoReviewModelOverride is a no-op when autoReviewModel is null or empty", () => { + test("applyAutoReviewModelOverride clears routed state when autoReviewModel is null or empty", () => { const { applyAutoReviewModelOverride } = require("../src/codex/catalog/sync"); const entries = [ { slug: "gpt-5.5", auto_review_model_override: "existing-model" }, + { slug: "opencode-go/glm-5.2", auto_review_model_override: "old-model" }, ]; applyAutoReviewModelOverride(entries, null); expect(entries[0].auto_review_model_override).toBe("existing-model"); + expect(entries[1].auto_review_model_override).toBeNull(); applyAutoReviewModelOverride(entries, " "); expect(entries[0].auto_review_model_override).toBe("existing-model"); + expect(entries[1].auto_review_model_override).toBeNull(); }); test("applyAutoReviewModelOverride rejects invalid format with control chars or inner spaces", () => { @@ -5719,7 +5722,7 @@ describe("auto_review_model configuration (#1225)", () => { // then the override is stamped before serialization. const entries = [ { slug: "gpt-5.5", auto_review_model_override: null }, - { slug: "opencode-go/glm-5.2", auto_review_model_override: "old-model" }, + { slug: "opencode-go/deepseek-v4-flash", auto_review_model_override: "old-model" }, ]; const configuredValue = " opencode-go/deepseek-v4-flash "; const trimmedValue = configuredValue.trim(); @@ -5729,7 +5732,7 @@ describe("auto_review_model configuration (#1225)", () => { // Absent value: no override is written. applyAutoReviewModelOverride(entries, null); expect(entries[0].auto_review_model_override).toBeNull(); - expect(entries[1].auto_review_model_override).toBe("old-model"); + expect(entries[1].auto_review_model_override).toBeNull(); // Present value: trimmed override replaces every entry (including native rows). applyAutoReviewModelOverride(entries, configuredValue); diff --git a/tests/codex-convergence-account-selectors.test.ts b/tests/codex-convergence-account-selectors.test.ts index 9206aaa9e44..9f4be0f0312 100644 --- a/tests/codex-convergence-account-selectors.test.ts +++ b/tests/codex-convergence-account-selectors.test.ts @@ -179,6 +179,34 @@ function config(pickerEnabled: boolean, disabledModels: string[] = []): OcxConfi }; } +function autoReviewConfig(models: string[]): OcxConfig { + const nextConfig = config(false); + nextConfig.providers.static = { + adapter: "openai-chat", + baseUrl: "https://static.example.test/v1", + liveModels: false, + models, + }; + return nextConfig; +} + +function writeAutoReviewModel(value?: string): void { + writeFileSync( + join(codexHome, "config.toml"), + value === undefined ? "" : `auto_review_model = ${JSON.stringify(value)}\n`, + ); +} + +function autoReviewSeed(routeOverride: string | null = "stale-override"): RawEntry[] { + return [ + { ...nativeEntry(), slug: "gpt-5.4", auto_review_model_override: "native-upstream" }, + { + ...generatedRoutedEntry("static/deepseek-v4-flash"), + auto_review_model_override: routeOverride, + }, + ]; +} + function writeCatalog(models: RawEntry[]): void { writeFileSync(catalogPath, `${JSON.stringify({ models }, null, 2)}\n`); } @@ -608,6 +636,75 @@ test("retained sync removes a deleted pre-marker custom row while discovery is d expect(models.some(entry => entry.slug === "offline/discovered-sibling")).toBe(true); }); +test("retained and convergence writers resolve, clear, reject, and recover auto-review selectors", async () => { + primeCodexRuntimeFixture(); + + for (const writer of ["retained", "convergence"] as const) { + const write = async (nextConfig: OcxConfig): Promise => { + if (writer === "retained") { + const result = await syncCatalogModels(nextConfig); + expect(result.catalogWritten).toBe(true); + } else { + const disposition = await convergeCatalogDisposition(nextConfig); + expect(disposition).toMatchObject({ status: "committed" }); + } + return JSON.parse(readFileSync(catalogPath, "utf8")) as RawCatalog; + }; + + // A configured selector is resolved against the final catalog and trimmed before stamping. + writeAutoReviewModel(" static/deepseek-v4-flash "); + writeCatalog(autoReviewSeed()); + let catalog = await write(autoReviewConfig(["deepseek-v4-flash"])); + expect(catalog.models?.find(entry => entry.slug === "static/deepseek-v4-flash")) + .toHaveProperty("auto_review_model_override", "static/deepseek-v4-flash"); + + // Clearing the root key removes stale routed state while preserving an upstream native value. + writeAutoReviewModel(); + writeCatalog(autoReviewSeed()); + catalog = await write(autoReviewConfig(["deepseek-v4-flash"])); + expect(catalog.models?.find(entry => entry.slug === "gpt-5.4")) + .toHaveProperty("auto_review_model_override", "native-upstream"); + expect(catalog.models?.find(entry => entry.slug === "static/deepseek-v4-flash")) + .toHaveProperty("auto_review_model_override", null); + + // A syntactically valid but missing selector is diagnosed and cannot persist a dead override. + writeAutoReviewModel("static/missing-model"); + writeCatalog(autoReviewSeed()); + const unresolvedWarning = spyOn(console, "warn").mockImplementation(() => {}); + let unresolvedWarningCalls: unknown[][] = []; + try { + catalog = await write(autoReviewConfig(["deepseek-v4-flash"])); + unresolvedWarningCalls = unresolvedWarning.mock.calls; + } finally { + unresolvedWarning.mockRestore(); + } + expect(unresolvedWarningCalls.some(call => String(call[0]).includes("not found in the final catalog"))).toBe(true); + expect(catalog.models?.find(entry => entry.slug === "static/deepseek-v4-flash")) + .toHaveProperty("auto_review_model_override", null); + + // Removing the configured model from the provider makes the target unresolved; recovery + // must stamp it again once the model is advertised by the next catalog. + writeAutoReviewModel("static/deepseek-v4-flash"); + writeCatalog(autoReviewSeed()); + const removedWarning = spyOn(console, "warn").mockImplementation(() => {}); + let removedWarningCalls: unknown[][] = []; + try { + catalog = await write(autoReviewConfig([])); + removedWarningCalls = removedWarning.mock.calls; + } finally { + removedWarning.mockRestore(); + } + expect(removedWarningCalls.some(call => String(call[0]).includes("not found in the final catalog"))).toBe(true); + expect(catalog.models?.find(entry => entry.slug === "static/deepseek-v4-flash")).toBeUndefined(); + + writeAutoReviewModel("static/deepseek-v4-flash"); + writeCatalog(autoReviewSeed(null)); + catalog = await write(autoReviewConfig(["deepseek-v4-flash"])); + expect(catalog.models?.find(entry => entry.slug === "static/deepseek-v4-flash")) + .toHaveProperty("auto_review_model_override", "static/deepseek-v4-flash"); + } +}); + test("degraded preservation still honors explicit routed visibility policy", async () => { writeCatalog([ nativeEntry(), From 657d79ecc77a299617d3cfd5ce7a2adbdf236022 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 26 Aug 2026 06:35:16 +0900 Subject: [PATCH 07/10] fix(claude): gate root skip-permissions bypass --- src/cli/claude.ts | 27 ++++++++++++++++++++++++++- tests/claude-cli.test.ts | 37 +++++++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/src/cli/claude.ts b/src/cli/claude.ts index 07f9599f509..ebb77d33592 100644 --- a/src/cli/claude.ts +++ b/src/cli/claude.ts @@ -34,6 +34,8 @@ export type ClaudeEnvDeps = { authDetect?: Omit, "env" | "ownTokens">; /** Test seam; production uses the authenticated Node-launcher context. */ preBunAnthropicSlots?: readonly AnthropicParentEnvSlot[] | null; + /** Explicit unsafe opt-in from a root `--dangerously-skip-permissions` launch. */ + allowRootSkipPermissions?: boolean; }; function isClaudeLoopbackHostname(hostname: string): boolean { @@ -115,6 +117,9 @@ export function buildClaudeEnv( if (env[name] !== undefined && env[name] !== "") return; // user wins env[name] = value; }; + if (deps.allowRootSkipPermissions === true) { + setDefault("IS_SANDBOX", "1"); + } setDefault("ANTHROPIC_BASE_URL", `http://127.0.0.1:${port}`); const existingBaseUrl = env.ANTHROPIC_BASE_URL; if (existingBaseUrl) { @@ -301,6 +306,22 @@ export function claudeNotFoundHint( return platform === "win32" && code === 9009 && !signal ? CLAUDE_INSTALL_HINT : null; } +export function shouldAllowRootSkipPermissions( + args: readonly string[], + getuid: (() => number) | null | undefined = process.getuid, +): boolean { + return args.includes("--dangerously-skip-permissions") + && typeof getuid === "function" + && getuid() === 0; +} + +export function rootSkipPermissionsNotice(env: ClaudeLaunchEnv): string { + if (env.IS_SANDBOX === "1") { + return "⚠ Root --dangerously-skip-permissions requested: OpenCodex set IS_SANDBOX=1 to bypass Claude Code's root guard. OpenCodex did not create an OS sandbox; prefer running as a non-root user."; + } + return `⚠ Root --dangerously-skip-permissions requested: preserving user IS_SANDBOX=${env.IS_SANDBOX}; Claude Code's root guard remains in control.`; +} + export async function cmdClaude(args: string[]): Promise { const config = loadConfig(); if (config.claudeCode?.enabled === false) { @@ -313,7 +334,11 @@ export async function cmdClaude(args: string[]): Promise { return 1; } const contextWindows = await fetchClaudeContextWindows(config, port); - const env = buildClaudeEnv(config, port, process.env, contextWindows); + const allowRootSkipPermissions = shouldAllowRootSkipPermissions(args); + const env = buildClaudeEnv(config, port, process.env, contextWindows, { allowRootSkipPermissions }); + if (allowRootSkipPermissions) { + console.error(rootSkipPermissionsNotice(env)); + } // Pre-write the CLI's gateway-model cache (devlog 030): without a token the CLI // never refreshes it, so the picker would keep showing yesterday's aliases. try { diff --git a/tests/claude-cli.test.ts b/tests/claude-cli.test.ts index e782252bf48..aa0950a9b8f 100644 --- a/tests/claude-cli.test.ts +++ b/tests/claude-cli.test.ts @@ -1,7 +1,6 @@ import { describe, expect, test } from "bun:test"; -import { claudeNotFoundHint } from "../src/cli/claude"; +import { buildClaudeEnv, claudeNotFoundHint, rootSkipPermissionsNotice, shouldAllowRootSkipPermissions } from "../src/cli/claude"; import { commandInvocation } from "../src/lib/win-exec"; -import { buildClaudeEnv } from "../src/cli/claude"; import type { OcxConfig } from "../src/types"; function cfg(extra?: Partial): OcxConfig { @@ -27,6 +26,40 @@ const AUTH_PRESENT = { }; describe("ocx claude env assembly", () => { + test("root skip-permissions bypass requires both the explicit flag and uid 0", () => { + expect(shouldAllowRootSkipPermissions(["--dangerously-skip-permissions"], () => 0)).toBe(true); + expect(shouldAllowRootSkipPermissions([], () => 0)).toBe(false); + expect(shouldAllowRootSkipPermissions(["--dangerously-skip-permissions"], () => 1000)).toBe(false); + expect(shouldAllowRootSkipPermissions(["--dangerously-skip-permissions"], null)).toBe(false); + }); + + test("root skip-permissions opt-in marks only that launch as sandboxed", () => { + const bypass = buildClaudeEnv(cfg(), 10100, {}, {}, { + ...AUTH_PRESENT, + allowRootSkipPermissions: true, + }); + expect(bypass.IS_SANDBOX).toBe("1"); + + const ordinary = buildClaudeEnv(cfg(), 10100, {}, {}, AUTH_PRESENT); + expect(ordinary.IS_SANDBOX).toBeUndefined(); + }); + + test("an explicit user sandbox value wins over the root skip-permissions opt-in", () => { + const env = buildClaudeEnv(cfg(), 10100, { IS_SANDBOX: "0" }, {}, { + ...AUTH_PRESENT, + allowRootSkipPermissions: true, + }); + expect(env.IS_SANDBOX).toBe("0"); + expect(rootSkipPermissionsNotice(env)).toContain("preserving user IS_SANDBOX=0"); + expect(rootSkipPermissionsNotice(env)).toContain("root guard remains in control"); + }); + + test("the unsafe root bypass notice discloses that no OS sandbox was created", () => { + const notice = rootSkipPermissionsNotice({ IS_SANDBOX: "1" }); + expect(notice).toContain("set IS_SANDBOX=1"); + expect(notice).toContain("did not create an OS sandbox"); + }); + test("injects base URL, discovery flag and model slots — NO auth token by default (subscription mode)", () => { const env = buildClaudeEnv(cfg({ claudeCode: { model: "claude-ocx-gemini--gemini-3-pro", smallFastModel: "gemini/gemini-3-flash" }, From 5e46521a212931e8636d02a8a687130c6df8660d Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 26 Aug 2026 06:35:31 +0900 Subject: [PATCH 08/10] fix(catalog): keep cache invalidation bound to owning home --- src/codex/catalog/sync.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index ca1cbd5e8e8..fda97248498 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -1957,11 +1957,12 @@ export function invalidateCodexModelsCacheWithPermit( // The catalog-only sync override applies here too so an explicit refresh // keeps the cache consistent with the catalog it just wrote. if (!shouldSyncCodexOnStart(loadConfig()) && options?.allowWhenDesiredDisabled !== true) return false; - const catalogPath = readCodexCatalogPath(); + const catalogPath = readCodexCatalogPathForHome(owningCodexHome); if (!existsSync(catalogPath)) return false; const catalog = JSON.parse(readFileSync(catalogPath, "utf8")); const models = catalog.models ?? catalog; - const currentCache = readCatalog(activeCodexModelsCachePath()); + const cachePath = join(owningCodexHome, "models_cache.json"); + const currentCache = readCatalog(cachePath); const existingSlugs = new Set(models.flatMap((entry: RawEntry) => typeof entry.slug === "string" ? [entry.slug] : [])); const currentConfig = loadConfig(); @@ -1989,7 +1990,7 @@ export function invalidateCodexModelsCacheWithPermit( models: [...models, ...observedAccountModels], }; replaceCodexModelsCache(permit, owningCodexHome, { - path: activeCodexModelsCachePath(), + path: cachePath, content: `${JSON.stringify(wrapper, null, 2)}\n`, }); return true; From de55ef2dd77a029c5c72c9a06efbfa063da32e75 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 26 Aug 2026 06:37:10 +0900 Subject: [PATCH 09/10] fix(server): fail health when package tree changes --- src/lib/package-tree-integrity.ts | 55 ++++++++++++++ src/server/index.ts | 39 ++++++++++ tests/package-tree-integrity.test.ts | 110 +++++++++++++++++++++++++++ 3 files changed, 204 insertions(+) create mode 100644 src/lib/package-tree-integrity.ts create mode 100644 tests/package-tree-integrity.test.ts diff --git a/src/lib/package-tree-integrity.ts b/src/lib/package-tree-integrity.ts new file mode 100644 index 00000000000..d8043d43856 --- /dev/null +++ b/src/lib/package-tree-integrity.ts @@ -0,0 +1,55 @@ +import { statSync } from "node:fs"; + +export interface PackageTreeObservation { + readonly device: bigint; + readonly inode: bigint; + readonly changeTimeNs: bigint; + readonly size: bigint; +} + +export type PackageTreeIntegrityStatus = + | { readonly ok: true } + | { readonly ok: false; readonly reason: "package_tree_replaced" | "package_tree_unreadable" }; + +export interface PackageTreeIntegrityGuard { + status(): PackageTreeIntegrityStatus; +} + +type ObservePackageTree = () => PackageTreeObservation | null; + +const packageManifestUrl = new URL("../../package.json", import.meta.url); + +function observePackageManifest(): PackageTreeObservation | null { + try { + const stat = statSync(packageManifestUrl, { bigint: true }); + return { + device: stat.dev, + inode: stat.ino, + changeTimeNs: stat.ctimeNs, + size: stat.size, + }; + } catch { + return null; + } +} + +function sameObservation(left: PackageTreeObservation, right: PackageTreeObservation): boolean { + return left.device === right.device + && left.inode === right.inode + && left.changeTimeNs === right.changeTimeNs + && left.size === right.size; +} + +export function createPackageTreeIntegrityGuard( + observe: ObservePackageTree = observePackageManifest, +): PackageTreeIntegrityGuard { + const boot = observe(); + return { + status(): PackageTreeIntegrityStatus { + const current = observe(); + if (boot === null || current === null) return { ok: false, reason: "package_tree_unreadable" }; + if (!sameObservation(boot, current)) return { ok: false, reason: "package_tree_replaced" }; + return { ok: true }; + }, + }; +} diff --git a/src/server/index.ts b/src/server/index.ts index 48d186f6f04..44d794991fc 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -202,6 +202,10 @@ import { import { SYSTEM_RESTART_CAPABILITY_VERSION } from "../lib/system-restart-contract"; import { LOCAL_PROVIDER_RELOAD_CAPABILITY_VERSION } from "../lib/local-provider-reload-contract"; import { createReadinessGate, type ReadinessGate } from "./readiness"; +import { + createPackageTreeIntegrityGuard, + type PackageTreeIntegrityGuard, +} from "../lib/package-tree-integrity"; export const MAX_WS_FRAME_BYTES = 50 * 1024 * 1024; const WEBSOCKET_IDLE_TIMEOUT_SECONDS = 0; @@ -454,6 +458,8 @@ export interface StartServerDeps { localAttestationSecret?: string; /** Optional readiness gate; a fresh pending gate is created when omitted. */ readinessGate?: ReadinessGate; + /** Test-only package-tree observation; production captures package.json identity at boot. */ + packageTreeIntegrity?: PackageTreeIntegrityGuard; } function inspectStartupOwnership( @@ -698,6 +704,15 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true, force: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + isolatedCodexHome = installIsolatedCodexHome("ocx-package-tree-integrity-"); +}); + +afterEach(() => { + if (previousOpencodexHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousOpencodexHome; + isolatedCodexHome?.restore(); + isolatedCodexHome = null; + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true, force: true }); +}); + +describe("package tree integrity", () => { + test("detects replacement even when the package version and file size are unchanged", () => { + let observation: PackageTreeObservation = { + device: 1n, + inode: 10n, + changeTimeNs: 100n, + size: 500n, + }; + const guard = createPackageTreeIntegrityGuard(() => observation); + + expect(guard.status()).toEqual({ ok: true }); + + observation = { ...observation, inode: 11n, changeTimeNs: 200n }; + expect(guard.status()).toEqual({ ok: false, reason: "package_tree_replaced" }); + }); + + test("fails closed when the package manifest disappears", () => { + let observation: PackageTreeObservation | null = { + device: 1n, + inode: 10n, + changeTimeNs: 100n, + size: 500n, + }; + const guard = createPackageTreeIntegrityGuard(() => observation); + observation = null; + + expect(guard.status()).toEqual({ ok: false, reason: "package_tree_unreadable" }); + }); + + test("degrades health and refuses Responses requests with a restart-required error", async () => { + saveConfig(config()); + const packageTreeIntegrity = { + status: () => ({ ok: false as const, reason: "package_tree_replaced" as const }), + }; + const server = startServer(0, { packageTreeIntegrity }); + try { + const health = await fetch(new URL("/healthz", server.url)); + expect(health.status).toBe(503); + expect(health.headers.get("retry-after")).toBe("5"); + expect(await health.json()).toMatchObject({ + status: "restart_required", + service: "opencodex", + error: { code: "package_tree_changed" }, + }); + + const response = await fetch(new URL("/v1/responses", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "test/gpt-test", input: "hello" }), + }); + expect(response.status).toBe(503); + expect(response.headers.get("retry-after")).toBe("5"); + expect(await response.json()).toMatchObject({ + error: { + type: "server_error", + code: "package_tree_changed", + message: expect.stringContaining("restart"), + }, + }); + } finally { + await server.stop(true); + } + }); +}); From 6dc5fb03a108159bc3e1f9f89362a27c62304804 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 26 Aug 2026 06:40:39 +0900 Subject: [PATCH 10/10] fix(server): refuse requests after the package tree is replaced (#2459) A bare global npm reinstall while the proxy is serving leaves one process with an old in-memory module graph while later dynamic imports resolve against the new package tree. The proxy stays bound, /healthz still answers 200, and every POST /v1/responses fails with a named-export error - the worst shape, because every supervisor sees a healthy process. package.json filesystem identity is captured at boot and compared per request. On replacement or unreadability /healthz and /readyz return 503 restart_required and /v1/* returns a typed package_tree_changed error with restart guidance. Management routes stay up so the user can restart and diagnose. Review addition: the check is throttled. status() runs on every /v1/* request, so an unthrottled guard adds a stat syscall to the hot path to detect an event that happens at most once per install. An ok reading is reused for a second; a FAILING reading is never cached, so a repaired install recovers on its own instead of staying refused for a window. Falsified: disabling the gate makes the degraded-health case return 200 instead of 503; removing the throttle reddens the reuse test. --- src/lib/package-tree-integrity.ts | 19 ++++++++++++++++ tests/package-tree-integrity.test.ts | 34 +++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/lib/package-tree-integrity.ts b/src/lib/package-tree-integrity.ts index d8043d43856..55554dd208a 100644 --- a/src/lib/package-tree-integrity.ts +++ b/src/lib/package-tree-integrity.ts @@ -40,15 +40,34 @@ function sameObservation(left: PackageTreeObservation, right: PackageTreeObserva && left.size === right.size; } +/** + * How long an `ok` observation is reused before the manifest is stat'd again. + * + * `status()` runs on `/healthz`, `/readyz` and every `/v1/*` request, so an unthrottled guard + * adds a filesystem syscall to the proxy's hot path to detect an event that happens at most + * once per install. A replaced tree is not time-critical either: the process is already + * serving broken imports, and one more second of that is not worse than a syscall per turn + * forever. + * + * A NEGATIVE result is never cached — once the tree looks wrong, every later request re-checks, + * so a repaired install recovers on its own instead of staying refused for a window. + */ +const PACKAGE_TREE_RECHECK_MS = 1_000; + export function createPackageTreeIntegrityGuard( observe: ObservePackageTree = observePackageManifest, + now: () => number = Date.now, ): PackageTreeIntegrityGuard { const boot = observe(); + let lastOkAt: number | null = null; return { status(): PackageTreeIntegrityStatus { + const at = now(); + if (lastOkAt !== null && at - lastOkAt < PACKAGE_TREE_RECHECK_MS) return { ok: true }; const current = observe(); if (boot === null || current === null) return { ok: false, reason: "package_tree_unreadable" }; if (!sameObservation(boot, current)) return { ok: false, reason: "package_tree_replaced" }; + lastOkAt = at; return { ok: true }; }, }; diff --git a/tests/package-tree-integrity.test.ts b/tests/package-tree-integrity.test.ts index f7d6cdb1a0e..47902c65b1e 100644 --- a/tests/package-tree-integrity.test.ts +++ b/tests/package-tree-integrity.test.ts @@ -52,14 +52,46 @@ describe("package tree integrity", () => { changeTimeNs: 100n, size: 500n, }; - const guard = createPackageTreeIntegrityGuard(() => observation); + // An explicit clock: `status()` reuses an `ok` reading for a second so the guard does not + // stat the manifest on every request, and two calls in the same millisecond would otherwise + // never re-observe. + let clock = 0; + const guard = createPackageTreeIntegrityGuard(() => observation, () => clock); expect(guard.status()).toEqual({ ok: true }); observation = { ...observation, inode: 11n, changeTimeNs: 200n }; + clock += 2_000; expect(guard.status()).toEqual({ ok: false, reason: "package_tree_replaced" }); }); + test("an ok reading is reused briefly, and a bad one is never cached", () => { + let observation: PackageTreeObservation | null = { + device: 1n, inode: 10n, changeTimeNs: 100n, size: 500n, + }; + let observations = 0; + let clock = 0; + const guard = createPackageTreeIntegrityGuard( + () => { observations += 1; return observation; }, + () => clock, + ); + + // Hot path: repeated calls inside the window cost one observation, not one each. + expect(guard.status()).toEqual({ ok: true }); + expect(guard.status()).toEqual({ ok: true }); + expect(guard.status()).toEqual({ ok: true }); + expect(observations).toBe(2); // one at construction, one for the first status() + + // A failure is re-observed every time, so a repaired install recovers on its own rather + // than staying refused for the rest of a window. + clock += 2_000; + observation = null; + expect(guard.status()).toEqual({ ok: false, reason: "package_tree_unreadable" }); + const afterFirstFailure = observations; + expect(guard.status()).toEqual({ ok: false, reason: "package_tree_unreadable" }); + expect(observations).toBe(afterFirstFailure + 1); + }); + test("fails closed when the package manifest disappears", () => { let observation: PackageTreeObservation | null = { device: 1n,