fix(catalog,bridge): write every slug once and scope declared-tool membership by wire (#4736, #4735)

scripts/test-layout/layout.json

@@ -286,20 +286,24 @@
     "cancel-body-on-abort.test.ts": "server",
     "catalog-auto-refresh-scheduler.test.ts": "codex-integration",
     "catalog-cursor-search.test.ts": "codex-integration",
+    "catalog-duplicate-slug-dedup.test.ts": "codex-integration",
     "catalog-free-pricing-status.test.ts": "codex-integration",
     "catalog-full-picker-order.test.ts": "codex-integration",
     "catalog-gated-native-suppression-reason.test.ts": "codex-integration",
     "catalog-go-exact-efforts.test.ts": "codex-integration",
     "catalog-hub-context-window.test.ts": "codex-integration",
     "catalog-input-modality-enum.test.ts": "codex-integration",
     "catalog-llamacpp-capabilities.test.ts": "codex-integration",
+    "catalog-modelalias-unique-sync.test.ts": "codex-integration",
     "catalog-oauth-observation.test.ts": "codex-integration",
     "catalog-remote-pull.test.ts": "codex-integration",
     "catalog-retain-models.test.ts": "codex-integration",
     "catalog-seed-window-fill.test.ts": "codex-integration",
+    "catalog-slug-uniqueness-boundary.test.ts": "codex-integration",
     "catalog-verbosity-default.test.ts": "codex-integration",
     "catalog-vision-sidecar-modalities.test.ts": "codex-integration",
     "catalog-zero-credit-picker.test.ts": "codex-integration",
+    "chat-completions-deferred-tools.test.ts": "responses",
     "chat-completions-endpoint.test.ts": "responses",
     "chat-conversation-affinity.test.ts": "responses",
     "chat-inbound-reasoning-none.test.ts": "responses",

src/bridge/response-json.ts

@@ -68,6 +68,8 @@ function buildResponseJSONWithBudget(
     toolNsMap?: Map<string, { namespace: string; name: string; freeform?: true }>;
     /** Request-visible tool names. When present, an upstream call outside this set fails closed. */
     declaredToolNames?: ReadonlySet<string>;
+    /** See `bridgeToResponsesSSE`: enforcement is separate from normalization (#4735). */
+    enforceDeclaredToolNames?: boolean;
     /** Declared parameter schema per tool name; repairs integral-float integer args (#1611). */
     toolParameterSchemas?: ReadonlyMap<string, Record<string, unknown>>;
     freeformToolNames?: Set<string>;
@@ -432,7 +434,11 @@ function buildResponseJSONWithBudget(
         }
         flushToolCall();
         const effectiveName = normalizeDeclaredToolName(e.name, options?.declaredToolNames);
-        if (options?.declaredToolNames && !options.declaredToolNames.has(effectiveName)) {
+        if (
+          options?.declaredToolNames
+          && options.enforceDeclaredToolNames !== false
+          && !options.declaredToolNames.has(effectiveName)
+        ) {
           errorEvent = {
             type: "error",
             message: `routed provider emitted undeclared client tool "${effectiveName}"; only request-declared tools may be called`,

src/bridge/sse.ts

@@ -88,6 +88,20 @@ export function bridgeToResponsesSSE(
     onUsage?: (usage: OcxUsage | undefined) => void;
     /** Request-visible tool names. When present, an upstream call outside this set fails closed. */
     declaredToolNames?: ReadonlySet<string>;
+    /**
+     * Whether `declaredToolNames` is an authorization boundary this proxy enforces, or only the
+     * catalog used to normalize provider-invented names back to declared ones.
+     *
+     * Defaults to enforcing. The chat and Anthropic inbound wires set it false: those specs make
+     * the server relay a tool call and leave execution or refusal to the client's own runner, and
+     * harnesses on them legitimately defer part of their catalog (#4735).
+     *
+     * It is a separate flag rather than simply withholding `declaredToolNames`, because the set
+     * also drives `normalizeDeclaredToolName` and `declaresCodeModeExec`. Passing `undefined`
+     * turns those off too, so a provider that invents `default.lookup` for a declared `lookup`
+     * would reach the client under the invented name instead of the normalized one.
+     */
+    enforceDeclaredToolNames?: boolean;
     /** Declared parameter schema per tool name; repairs integral-float integer args (#1611). */
     toolParameterSchemas?: ReadonlyMap<string, Record<string, unknown>>;
     /**
@@ -1008,7 +1022,11 @@ export function bridgeToResponsesSSE(
                 : undefined;
               const mapped = toolNsMap?.get(effectiveName);
               const realName = mapped?.name ?? effectiveName;
-              if (options?.declaredToolNames && !options.declaredToolNames.has(effectiveName)) {
+              if (
+                options?.declaredToolNames
+                && options.enforceDeclaredToolNames !== false
+                && !options.declaredToolNames.has(effectiveName)
+              ) {
                 const failure = responseError(
                   502,
                   "upstream_error",

src/codex/catalog/aggregation.ts

@@ -34,7 +34,7 @@ import upstreamModelsSnapshot from "../data/upstream-models.json";
 
 
 import { catalogModelSlug } from "./parsing";
-import type { CatalogModel } from "./parsing";
+import type { CatalogModel, RawEntry } from "./parsing";
 
 export const openAiApiCollisionWarnings = new Set<string>();
 
@@ -222,6 +222,85 @@ export function safeCatalogWarningLabel(value: string): string {
     .slice(0, 200);
 }
 
+/**
+ * Keep the first row of each slug and drop the rest (#4730).
+ *
+ * First-win is the only answer that agrees with the ordering already decided upstream: the merge
+ * ranks rows, so its first occurrence is the row it chose. Distinct slugs are never touched — an
+ * alias row and the canonical routed row of the same provider model are two different public names
+ * and both survive — and a row without a string slug passes through untouched.
+ */
+export function dedupeCatalogEntriesBySlug(models: RawEntry[]): RawEntry[] {
+  const seen = new Set<string>();
+  const out: RawEntry[] = [];
+  for (const entry of models) {
+    if (typeof entry.slug !== "string") {
+      out.push(entry);
+      continue;
+    }
+    if (seen.has(entry.slug)) continue;
+    seen.add(entry.slug);
+    out.push(entry);
+  }
+  return out;
+}
+
+/**
+ * Every slug this proxy writes into the Codex catalog must appear exactly once (#4730).
+ *
+ * The cost of breaking it is the whole file: a slug-unique validating consumer refuses the catalog
+ * outright, so one duplicated row takes every model with it. A 2.56.0 report carried 507 rows for
+ * 72 unique slugs, every duplicate byte-identical, and Codex rejected the file as `source-invalid`.
+ *
+ * This is a write-boundary invariant rather than a repair of one producer, and that distinction is
+ * deliberate: the reported catalog is evidence that some emit path can double a row, but nothing in
+ * this tree has been shown to be that path, and a guard that only covered the producer someone
+ * guessed at would leave the file corruptible by the next one. Both writers that serialize a merged
+ * catalog call this as their LAST mutation — `writeRetainedCatalogSync` and the management
+ * convergence commit — so uniqueness holds for the exact bytes that land on disk.
+ *
+ * Ordering is load-bearing. Running the guard before the effort clamp would be unsound:
+ * `clampCatalogModelsToObservedCodexSupport` splices whole rows out when an exact-reserve ladder
+ * clamps empty, so dropping a later same-slug row first can leave the slug with no row at all once
+ * the surviving one is spliced.
+ *
+ * @param models - The finished row list, already clamped and finalized.
+ * @param warn - Whether to report on `console.warn`. The convergence path merges under
+ *   `warningPolicy: "suppress"` and stays silent for the same reason.
+ * @returns The original array when it was already unique, so an unchanged catalog stays a no-op
+ *   write; otherwise a first-win copy.
+ */
+export function enforceCatalogSlugUniqueness(models: RawEntry[], warn: boolean): RawEntry[] {
+  const deduped = dedupeCatalogEntriesBySlug(models);
+  if (deduped.length === models.length) return models;
+  if (warn) {
+    // A dropped row that differs from the kept one means two emit paths disagree about the same
+    // slug's content. First-win still stands, but the operator needs to see WHICH slugs diverged
+    // instead of silently losing data. The baseline is the row the dedupe actually keeps — the
+    // FIRST occurrence — so the reported divergence is measured against what lands on disk.
+    const keptBySlug = new Map<string, RawEntry>();
+    for (const entry of models) {
+      if (typeof entry.slug !== "string" || keptBySlug.has(entry.slug)) continue;
+      keptBySlug.set(entry.slug, entry);
+    }
+    const divergentSlugs = new Set<string>();
+    for (const entry of models) {
+      if (typeof entry.slug !== "string") continue;
+      const kept = keptBySlug.get(entry.slug);
+      if (kept && kept !== entry && JSON.stringify(kept) !== JSON.stringify(entry)) {
+        divergentSlugs.add(entry.slug);
+      }
+    }
+    const divergentNote = divergentSlugs.size > 0
+      ? `; divergent content on: ${[...divergentSlugs].slice(0, 5).map(safeCatalogWarningLabel).join(", ")}${divergentSlugs.size > 5 ? ", …" : ""}`
+      : "";
+    console.warn(
+      `[opencodex] catalog sync dropped ${models.length - deduped.length} duplicate slug row(s), keeping the first occurrence of each slug (#4730)${divergentNote}.`,
+    );
+  }
+  return deduped;
+}
+
 export function comboCatalogWarningSignature(
   combo: NormalizedComboConfig,
   members: readonly CatalogModel[],

src/codex/catalog/retained-sync.ts

@@ -51,7 +51,7 @@ import { bundledCatalogCacheState, loadBundledCodexCatalog } from "./bundled";
 import { isMultiAgentV2Enabled } from "../features";
 import { clampCatalogModelsToCodexSupport } from "./effort";
 import { filterCatalogVisibleModels, gatherRoutedModels, type CatalogGatherProviderModelOutcome } from "./provider-fetch";
-import { exactComboCatalogSlugs, type ComboCatalogOmission } from "./aggregation";
+import { dedupeCatalogEntriesBySlug, enforceCatalogSlugUniqueness, exactComboCatalogSlugs, type ComboCatalogOmission } from "./aggregation";
 import {
   withCatalogWriteSerialization,
   type CatalogWritePermit,
@@ -522,6 +522,9 @@ function writeRetainedCatalogSync({
   });
   clampCatalogModelsToCodexSupport(catalog.models);
   finalizeAutoReviewModelOverride(catalog.models, catalogModelsForMerge, config);
+  // Last mutation before serialization; see `enforceCatalogSlugUniqueness` for why the ordering
+  // against the effort clamp is load-bearing rather than cosmetic.
+  catalog.models = enforceCatalogSlugUniqueness(catalog.models, true);
 
   const added = goEntries.length + accountBoundEntries.length;
   const content = `${JSON.stringify(catalog, null, 2)}\n`;
@@ -553,6 +556,11 @@ function writeRetainedCatalogSync({
   };
 }
 
+// Re-exported so the #4730 unit regression keeps importing the guard from the sync module it
+// guards; the implementation lives in ./aggregation because the management convergence commit
+// is the second writer that has to apply the identical rule.
+export { dedupeCatalogEntriesBySlug };
+
 export async function syncCatalogModels(
   config: OcxConfig,
   options?: CodexCatalogSyncOptions,

src/codex/convergence.ts

@@ -49,7 +49,7 @@ import {
   orderForSubagents,
   } from "./catalog/sync";
   import { multiAgentV2EnabledFromConfigText } from "./features";
-  import { exactComboCatalogSlugs } from "./catalog/aggregation";
+  import { enforceCatalogSlugUniqueness, exactComboCatalogSlugs } from "./catalog/aggregation";
   import {
   isNativeAliasCatalogEntry,
   accountBoundNativeOpenAiSlugs,
@@ -386,7 +386,12 @@ function prepareCatalog(
       : null,
   );
   finalizeAutoReviewModelOverride(mergedModels, catalogModels, config);
-  catalog.models = mergedModels;
+  // The second writer of this file. A dashboard model toggle, a combo edit, or a Codex account
+  // login reaches `convergeCodexCatalog` and commits through `fixedCommit`, never through
+  // `writeRetainedCatalogSync`, so the #4730 uniqueness guard has to stand here too or the same
+  // `source-invalid` rejection returns by a different route. Silent because this merge runs under
+  // `warningPolicy: "suppress"`.
+  catalog.models = enforceCatalogSlugUniqueness(mergedModels, false);
   return catalog;
 }
 

src/server/responses/adapter-delivery.ts

@@ -99,6 +99,7 @@ export async function deliverAdapterResponse(
         stallTimeoutSec: config.stallTimeoutSec,
         hideThinkingSummary: parsed.options.hideThinkingSummary,
         declaredToolNames,
+        enforceDeclaredToolNames: options.inboundWire !== "chat" && options.inboundWire !== "anthropic",
       toolParameterSchemas,
         ...(options.onFirstOutput ? { onFirstOutput: options.onFirstOutput } : {}),
         ...(routedCompaction ? { compaction: true } : {}),
@@ -174,6 +175,7 @@ export async function deliverAdapterResponse(
       hideThinkingSummary: parsed.options.hideThinkingSummary,
       toolNsMap,
       declaredToolNames,
+      enforceDeclaredToolNames: options.inboundWire !== "chat" && options.inboundWire !== "anthropic",
       toolParameterSchemas,
       freeformToolNames,
       toolSearchToolNames,

src/server/responses/run-turn-execution.ts

@@ -374,6 +374,7 @@ export async function executeResponsesRunTurn(
           stallTimeoutSec: config.stallTimeoutSec,
           hideThinkingSummary: parsed.options.hideThinkingSummary,
           declaredToolNames,
+          enforceDeclaredToolNames: inboundWire !== "chat" && inboundWire !== "anthropic",
           toolParameterSchemas,
           ...(options.onFirstOutput ? { onFirstOutput: options.onFirstOutput } : {}),
           ...(routedCompaction ? { compaction: true } : {}),
@@ -444,6 +445,7 @@ export async function executeResponsesRunTurn(
       hideThinkingSummary: parsed.options.hideThinkingSummary,
       toolNsMap,
       declaredToolNames,
+      enforceDeclaredToolNames: inboundWire !== "chat" && inboundWire !== "anthropic",
       toolParameterSchemas,
       freeformToolNames,
       toolSearchToolNames,

structure/adapters/compatibility-contracts.md

@@ -78,3 +78,19 @@ before dotted aliases are added. A conflicting explicit namespace is never overw
 restoration retains the existing lowered-kind handling because custom tools are lowered to
 functions before the adapter constructs its alias map; ordinary argument repair independently
 checks the original declaration kind.
+
+## Undeclared-tool refusal is an inbound-protocol claim
+
+Whether a routed provider's call to an undeclared tool is refused depends on the inbound protocol,
+not on the adapter or the upstream protocol. The `responses` inbound protocol refuses it and ends
+the turn, which is the #1700 contract. The `chat` and `anthropic` inbound protocols relay it,
+because those specs place validation and execution with the client's own tool runner.
+
+A manifest claiming a disposition for tool-call delivery therefore names its inbound protocol. The
+same provider, base URL, adapter, and authentication mode produce `passthrough` on `chat` and
+`anthropic` and `unsupported` on `responses` for the identical undeclared call, which is exactly
+the inference the narrow-subject rule above exists to prevent.
+
+Tool-name normalization is not scoped this way and runs on every inbound protocol, so a
+provider-invented `default.` namespace resolves back to the declared tool regardless of subject.
+The contract is stated in full in [Responses Transport](../transports/responses.md).

structure/transports/responses.md

@@ -445,6 +445,45 @@ Arguments, user text, and schema property names are never rewritten.
 
 > Decision record: [ADR-0043](../decisions/ADR-0043-responses-http-sse.md)
 
+### Declared-tool membership by inbound wire
+
+`declaredToolNames` carries the request's tool catalog into both bridges, and it does two separate
+jobs that are separately controlled.
+
+Normalization runs on every inbound wire. `normalizeDeclaredToolName` and `declaresCodeModeExec` in
+`src/types/tools.ts` read the same set to map a provider-invented `default.` namespace back to the
+declared bare tool and to rewrite code-mode helper names into the declared `exec`. Both return their
+input unchanged when the set is absent, so the set reaches the bridge on every wire and enforcement
+is expressed by a separate flag rather than by withholding it.
+
+Membership enforcement is that flag, `enforceDeclaredToolNames`, and only the `responses` inbound
+wire enforces. A routed provider that names a tool the request never declared ends the turn there:
+`src/bridge/sse.ts` emits `response.failed` and `src/bridge/response-json.ts` returns a failed
+response, both carrying `undeclared client tool`. That is the #1700 contract and it stands. Codex
+executes a top-level tool call, so a hallucinated `apply_patch` — which under code mode exists only
+as a nested `tools.apply_patch(...)` helper inside `exec` — is refused before it reaches the
+runtime, where it previously surfaced as a bare `aborted` with the file untouched.
+
+The `chat` and `anthropic` inbound wires relay the call instead. This is a deliberate reversal of
+#1700's scope for those two wires, not an oversight. Both vendor specs make the client's own runner
+responsible for validating a tool call and then executing or denying it, and harnesses on those
+endpoints defer part of their catalog to conserve prompt tokens and discover the rest at runtime.
+Enforcing membership against a partial catalog killed those streams mid-turn with a 502 and cost the
+caller the whole turn. This proxy executes no tool call on any wire, so scoping enforcement off
+these two moves the decision to the party that already makes it rather than removing it.
+
+An explicitly empty catalog still authorizes nothing on the wire that enforces. A request declaring
+an empty tool list is making a statement rather than omitting one, which is how the passthrough
+guard reads it through `clientExplicitWireToolCatalog` in
+`src/server/responses/passthrough-dispatch.ts`.
+
+The passthrough guard is not wire-scoped. `undeclaredToolGuardActive` gates namespace normalization
+and continuation-state suppression as well as the refusal, and it stands down only for
+`authMode: "forward"` and for a request that declares no catalog at all.
+
+`src/server/responses/run-turn-execution.ts` and `src/server/responses/adapter-delivery.ts` set the
+flag from `inboundWire` on the streaming, buffered, and JSON paths alike, so the three cannot drift.
+
 ### Passthrough SSE stream shapes (#314)
 
 Native passthrough SSE has TWO shapes, selected per request in