fix(catalog): preserve config edits during auto-refresh

src/codex/catalog-auto-refresh.ts

@@ -65,11 +65,17 @@ async function tick(): Promise<void> {
   const entryGeneration = generation;
   try {
     const {
+      armDetachedConfigBaseline,
       loadConfig,
       isCatalogAutoRefreshEnabled,
       resolveCatalogAutoRefreshIntervalMs,
     } = await import("../config");
     const config = loadConfig();
+    // Convergence can persist model-discovery fields after awaiting provider /models.
+    // Arm this independently loaded snapshot as detached so the save rebases every
+    // field — listener binding and disk-only keys included — against what is on disk
+    // by then, and concurrent hand edits survive the tick.
+    armDetachedConfigBaseline(config);
     if (!isCatalogAutoRefreshEnabled(config)) return;
     const configured = resolveCatalogAutoRefreshIntervalMs(config);
     // 0 is dormant: the section stays configured but this tick must not converge,

src/config.ts

@@ -133,7 +133,7 @@ export {
   withExpectedConfigGenerationSync,
 } from "./config/mutation-lock";
 export {
-  armClaudeCodeBaseline,
+  armClaudeCodeBaseline, armDetachedConfigBaseline,
   adoptPersistedProviderIntoLiveConfig,
   claudeCodeBaselineArmed,
   reconcileLiveConfigFromDisk,

src/config/live-reconcile.ts

@@ -47,6 +47,16 @@ type PersistedServerBinding = Pick<OcxConfig, "port" | "hostname">;
 
 const persistedLiveServerBinding = new WeakMap<OcxConfig, PersistedServerBinding>();
 
+/**
+ * Config instances nobody holds long-term — the catalog auto-refresh tick's
+ * per-tick `loadConfig()` snapshot. A detached snapshot cannot express a
+ * deliberate deletion and owns no live listener socket, so the live policy's
+ * `hostname`/`port` and disk-only-key skips would only discard concurrent hand
+ * edits wholesale. Keyed on the instance so the mode cannot leak into the
+ * long-lived server config.
+ */
+const detachedConfigSnapshots = new WeakSet<OcxConfig>();
+
 /**
  * Arm the baseline for a long-lived config. MANDATORY at `startServer`, not lazy on
  * first save — arming lazily would lose exactly the hand edit made before that first
@@ -57,6 +67,17 @@ export function armClaudeCodeBaseline(config: OcxConfig): void {
   claudeCodeBaseline.set(config, structuredClone(config.claudeCode));
 }
 
+/**
+ * Arm a freshly loaded config instance no long-lived server owns. The save path
+ * then reconciles every field — the listener binding and keys that exist only
+ * on disk included — against this arming baseline, so a concurrent hand edit is
+ * adopted rather than overwritten by the snapshot's stale values.
+ */
+export function armDetachedConfigBaseline(config: OcxConfig): void {
+  armClaudeCodeBaseline(config);
+  detachedConfigSnapshots.add(config);
+}
+
 /**
  * Adopt one schema-validated provider that was read from the authoritative disk
  * config into a long-lived server config without rebasing any unrelated field.
@@ -124,6 +145,42 @@ type IndexedCustomModels = {
   byId: Map<string, Record<string, unknown>>;
 };
 
+function indexDisabledModels(value: ConfigMergeValue): { order: string[]; members: Set<string> } | null {
+  if (value === MISSING_CONFIG_VALUE) return { order: [], members: new Set() };
+  if (!Array.isArray(value)) return null;
+  const members = new Set<string>();
+  for (const item of value) {
+    if (typeof item !== "string" || members.has(item)) return null;
+    members.add(item);
+  }
+  return { order: value as string[], members };
+}
+
+/**
+ * Merge disabled-model lists by membership instead of treating the array as one
+ * opaque leaf: a slug either side deleted stays deleted (deletion wins over the
+ * other side's unchanged retention) while slugs added on either side are kept.
+ * Discovery only appends, so live additions are its arrivals; a hand edit that
+ * hid or un-hid a model mid-refresh survives the discovery save.
+ */
+function reconcileDisabledModels(
+  baseline: ConfigMergeValue,
+  live: ConfigMergeValue,
+  persisted: ConfigMergeValue,
+): ConfigMergeValue | null {
+  const baselineSet = indexDisabledModels(baseline);
+  const liveSet = indexDisabledModels(live);
+  const persistedSet = indexDisabledModels(persisted);
+  if (!baselineSet || !liveSet || !persistedSet) return null;
+  const order = [...liveSet.order, ...persistedSet.order.filter(id => !liveSet.members.has(id))];
+  const merged: string[] = [];
+  for (const id of order) {
+    if (baselineSet.members.has(id) && (!liveSet.members.has(id) || !persistedSet.members.has(id))) continue;
+    merged.push(id);
+  }
+  return merged;
+}
+
 function indexCustomModels(value: ConfigMergeValue): IndexedCustomModels | null {
   if (!Array.isArray(value)) return null;
   const order: string[] = [];
@@ -192,7 +249,10 @@ function reconcileConfigRecord(
       : key === "customModels"
         ? reconcileCustomModels(baselineValue, liveValue, persistedValue)
           ?? reconcileConfigValue(baselineValue, liveValue, persistedValue)
-        : reconcileConfigValue(baselineValue, liveValue, persistedValue, key === "providers");
+        : key === "disabledModels"
+          ? reconcileDisabledModels(baselineValue, liveValue, persistedValue)
+            ?? reconcileConfigValue(baselineValue, liveValue, persistedValue)
+          : reconcileConfigValue(baselineValue, liveValue, persistedValue, key === "providers");
     if (merged === MISSING_CONFIG_VALUE) delete live[key];
     else live[key] = merged;
   }
@@ -302,7 +362,11 @@ function readPersistedServerBinding(
 }
 
 /**
- * The save entry point for every writer holding a LIVE server config.
+ * The save entry point for every writer holding a LIVE server config, and for a
+ * detached snapshot armed through {@link armDetachedConfigBaseline}. The live
+ * policy keeps `hostname`/`port` and disk-only keys out of the merge; a detached
+ * snapshot has neither hazard, so it rebases every field against its arming
+ * baseline instead.
  *
  * Conflict policy, chosen deliberately:
  * - disk changed, we did not → their hand edit wins;
@@ -312,8 +376,9 @@ function readPersistedServerBinding(
  *   live state edited that same row;
  * - file missing/unreadable → save what we have, no throw.
  *
- * Custom-model rows are merged by their stable `id`, preserving independent
- * edits and deletions across stale whole-config saves.
+ * Custom-model rows are merged by their stable `id`, and `disabledModels` by
+ * member, preserving independent edits and deletions across stale whole-config
+ * saves.
  */
 export function saveConfigPreservingClaudeCode(config: OcxConfig): void {
   const pinError = configReasoningPinsConfigError(config);
@@ -327,36 +392,51 @@ export function saveConfigPreservingClaudeCode(config: OcxConfig): void {
     if (baseline && onDisk !== undefined) {
       const persistedDiagnostics = configDiagnosticsFromRaw(JSON.stringify(onDisk));
       if (persistedDiagnostics.source === "file") {
-        const deletedKeys = configRebaseDeletionKeys(config);
-        const provenanceExists = configHasRebaseProvenance(config);
-        // Only keys this live config is actually known to have diverged on may be
-        // rebased. The baseline is captured once when the server arms it, so any key
-        // that appeared on disk afterwards — through saveConfig(), a hand edit, or
-        // another process — is absent from the baseline as well as from the live
-        // config. Reconciling those keys reads "live never changed this" and adopts
-        // the disk value, which resurrects a field the live writer had deliberately
-        // deleted (#1462 regression: PUT /api/grok/selection with an empty list).
-        // Restrict the merge to keys the baseline knew about, plus keys the live
-        // config still carries; a key that exists only on disk is left to the
-        // ordinary whole-config write below.
-        const rebaseableKeys = new Set([
-          ...Object.keys(baseline as unknown as Record<string, unknown>),
-          ...Object.keys(config as unknown as Record<string, unknown>),
-          ...(provenanceExists
-            ? Object.keys(persistedDiagnostics.config as unknown as Record<string, unknown>)
-            : []),
-        ]);
-        const skipped = new Set(["hostname", "port", "claudeCode", CONFIG_REBASE_PROVENANCE_KEY]);
-        for (const key of Object.keys(persistedDiagnostics.config as unknown as Record<string, unknown>)) {
-          if (!rebaseableKeys.has(key)) skipped.add(key);
+        // A detached snapshot diverged only where this pipeline mutated it (model
+        // discovery fields for the auto-refresh tick): it cannot express a
+        // deliberate deletion, so the disk-only-key and listener-binding skips
+        // below would only discard concurrent hand edits. It merges every top-level
+        // key — including configRebaseProvenance, so a cooperating writer's
+        // deletion marker adopted from disk is honored instead of silently dropped —
+        // and reads deletion intent AFTER the merge, from the marker disk actually
+        // carries now rather than a stale one the snapshot loaded.
+        const detached = detachedConfigSnapshots.has(config);
+        const deletedKeys = detached ? null : configRebaseDeletionKeys(config);
+        const skipped = detached
+          ? new Set(["claudeCode"])
+          : new Set(["hostname", "port", "claudeCode", CONFIG_REBASE_PROVENANCE_KEY]);
+        if (!detached) {
+          const provenanceExists = configHasRebaseProvenance(config);
+          // Only keys this live config is actually known to have diverged on may be
+          // rebased. The baseline is captured once when the server arms it, so any key
+          // that appeared on disk afterwards — through saveConfig(), a hand edit, or
+          // another process — is absent from the baseline as well as from the live
+          // config. Reconciling those keys reads "live never changed this" and adopts
+          // the disk value, which resurrects a field the live writer had deliberately
+          // deleted (#1462 regression: PUT /api/grok/selection with an empty list).
+          // Restrict the merge to keys the baseline knew about, plus keys the live
+          // config still carries; a key that exists only on disk is left to the
+          // ordinary whole-config write below.
+          const rebaseableKeys = new Set([
+            ...Object.keys(baseline as unknown as Record<string, unknown>),
+            ...Object.keys(config as unknown as Record<string, unknown>),
+            ...(provenanceExists
+              ? Object.keys(persistedDiagnostics.config as unknown as Record<string, unknown>)
+              : []),
+          ]);
+          for (const key of Object.keys(persistedDiagnostics.config as unknown as Record<string, unknown>)) {
+            if (!rebaseableKeys.has(key)) skipped.add(key);
+          }
         }
         reconcileConfigRecord(
           config as unknown as Record<string, unknown>,
           baseline as unknown as Record<string, unknown>,
           persistedDiagnostics.config as unknown as Record<string, unknown>,
           skipped,
         );
-        for (const key of deletedKeys) delete (config as unknown as Record<string, unknown>)[key];
+        for (const key of deletedKeys ?? configRebaseDeletionKeys(config)) {
+          delete (config as unknown as Record<string, unknown>)[key];
+        }
       }
     }
     if (claudeCodeBaseline.has(config)) {

structure/config.md

@@ -521,7 +521,7 @@ being treated as a text model by one and an image target by the other.
 
 ## Catalog auto-refresh
 
-`catalogAutoRefresh` on `src/types/config.ts` stores an optional `enabled` / `intervalMinutes` section that defaults off: an absent key, an explicit false, and a malformed value all leave the scheduler dormant. `src/config/feature-flags.ts` resolves the cadence; an explicit `intervalMinutes: 0` keeps the unref'd timer idle, and any other value is clamped up to 15 minutes because upstream `/models` caches have not moved below that and a shorter tick only multiplies rate-limit exposure. `src/codex/catalog-auto-refresh.ts` is the module-singleton interval `src/server/background-lifecycle.ts` starts beside the quota reset poller; a tick that is enabled and non-dormant drives the same catalog-only converge funnel management mutations drive. The last-outcome record lives in `src/codex/catalog-refresh-status.ts` (when the tick finished, the normalized `CatalogDisposition`, whether the served model set changed, consecutive failures) and carries no provider or account detail.
+`catalogAutoRefresh` on `src/types/config.ts` stores an optional `enabled` / `intervalMinutes` section that defaults off: an absent key, an explicit false, and a malformed value all leave the scheduler dormant. `src/config/feature-flags.ts` resolves the cadence; an explicit `intervalMinutes: 0` keeps the unref'd timer idle, and any other value is clamped up to 15 minutes because upstream `/models` caches have not moved below that and a shorter tick only multiplies rate-limit exposure. `src/codex/catalog-auto-refresh.ts` is the module-singleton interval `src/server/background-lifecycle.ts` starts beside the quota reset poller; a tick that is enabled and non-dormant drives the same catalog-only converge funnel management mutations drive. Each tick arms its independently loaded config snapshot as a detached baseline before provider work, so the discovery save rebases every field — the listener binding and sections absent from the snapshot included — against the disk state at save time and concurrent hand edits survive the tick — `disabledModels` merges by member, so an overlapping visibility edit survives alongside the discovery additions. The last-outcome record lives in `src/codex/catalog-refresh-status.ts` (when the tick finished, the normalized `CatalogDisposition`, whether the served model set changed, consecutive failures) and carries no provider or account detail.
 
 ## Aggregate request metrics export
 

tests/codex-integration/catalog-auto-refresh-scheduler.test.ts

@@ -1,5 +1,5 @@
 import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test";
-import { mkdtempSync, writeFileSync } from "node:fs";
+import { mkdtempSync, readFileSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import {
@@ -18,7 +18,9 @@ import {
   CATALOG_AUTO_REFRESH_MIN_INTERVAL_MS,
   getConfigPath,
   getDefaultConfig,
+  saveConfigPreservingClaudeCode,
 } from "../../src/config";
+import type { OcxConfig } from "../../src/types";
 import {
   installIsolatedCodexHome,
   type IsolatedCodexHome,
@@ -35,7 +37,7 @@ let previousOpenCodexHome: string | undefined;
 let openCodexHome = "";
 let isolatedCodexHome: IsolatedCodexHome | null = null;
 let convergeFactoryCalls = 0;
-let convergeImpl: () => Promise<CatalogOnlyOutcome> = async () => COMMITTED_CATALOG_ONLY;
+let convergeImpl: (config: OcxConfig) => Promise<CatalogOnlyOutcome> = async () => COMMITTED_CATALOG_ONLY;
 let convergeSpy: { mockRestore(): void } | null = null;
 let releaseHanging: ((outcome: CatalogOnlyOutcome) => void) | null = null;
 let pendingTick: Promise<unknown> | null = null;
@@ -68,9 +70,9 @@ beforeEach(() => {
   pendingTick = null;
   // The tick's only converge seam is a dynamic import of management-convergence.
   // Stub it so an enabled fixture cannot spend a live /models call or rewrite the catalog.
-  convergeSpy = spyOn(managementConvergence, "createManagementConvergeCodex").mockImplementation(() => {
+  convergeSpy = spyOn(managementConvergence, "createManagementConvergeCodex").mockImplementation((config) => {
     convergeFactoryCalls += 1;
-    return convergeImpl;
+    return () => convergeImpl(config);
   });
 });
 
@@ -168,6 +170,48 @@ describe("catalog auto-refresh scheduler", () => {
     expect(lastCatalogAutoRefreshOutcome()).toBeNull();
   });
 
+  test("a tick preserves a config hand edit made while convergence is in flight", async () => {
+    writeCatalogAutoRefreshConfig({ enabled: true, intervalMinutes: 60 });
+    convergeImpl = async (config) => {
+      const onDisk = JSON.parse(readFileSync(getConfigPath(), "utf8")) as OcxConfig;
+      onDisk.catalogAutoRefresh = { enabled: false, intervalMinutes: 60 };
+      writeFileSync(getConfigPath(), JSON.stringify(onDisk), "utf8");
+      saveConfigPreservingClaudeCode(config);
+      return COMMITTED_CATALOG_ONLY;
+    };
+
+    await runCatalogAutoRefreshTickForTests();
+
+    const persisted = JSON.parse(readFileSync(getConfigPath(), "utf8")) as OcxConfig;
+    expect(persisted.catalogAutoRefresh?.enabled).toBe(false);
+  });
+
+  test("a tick preserves listener and newly added fields edited while convergence is in flight", async () => {
+    // The general live-save policy leaves hostname/port and disk-only keys out of
+    // the rebase. For the tick's detached snapshot those skips would discard the
+    // hand edit wholesale, so arming it detached reconciles every field instead.
+    writeCatalogAutoRefreshConfig({ enabled: true, intervalMinutes: 60 });
+    convergeImpl = async (config) => {
+      const onDisk = JSON.parse(readFileSync(getConfigPath(), "utf8")) as OcxConfig;
+      onDisk.port = 10101;
+      onDisk.hostname = "127.0.0.2";
+      onDisk.metricsExport = { enabled: true };
+      writeFileSync(getConfigPath(), JSON.stringify(onDisk), "utf8");
+      // The same save convergeCodexCatalog performs after mutating discovery fields.
+      config.disabledModels = ["xai:grok-0"];
+      saveConfigPreservingClaudeCode(config);
+      return COMMITTED_CATALOG_ONLY;
+    };
+
+    await runCatalogAutoRefreshTickForTests();
+
+    const persisted = JSON.parse(readFileSync(getConfigPath(), "utf8")) as OcxConfig;
+    expect(persisted.port).toBe(10101);
+    expect(persisted.hostname).toBe("127.0.0.2");
+    expect(persisted.metricsExport?.enabled).toBe(true);
+    expect(persisted.disabledModels).toEqual(["xai:grok-0"]);
+  });
+
   test("an overlapping tick returns immediately without a second converge", async () => {
     writeCatalogAutoRefreshConfig({ enabled: true, intervalMinutes: 60 });