Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 130 additions & 12 deletions src/server/responses-self-named-namespace-scrub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,130 @@ function isPlainObject(value: unknown): value is Record<string, unknown> {
return !!value && typeof value === "object" && !Array.isArray(value);
}

export interface SelfNamedNamespaceScrubAuthorization {
customToolCallNames: ReadonlySet<string>;
functionCallNames: ReadonlySet<string>;
}

function collectBareToolSpecs(
bareCustomNames: Set<string>,
bareFunctionNames: Set<string>,
sameNameNamespacedCustomNames: Set<string>,
sameNameNamespacedFunctionNames: Set<string>,
specs: unknown,
): void {
if (!Array.isArray(specs)) return;
for (const spec of specs) {
if (!isPlainObject(spec)) continue;
if (spec.type === "namespace" && Array.isArray(spec.tools)) {
const namespace = typeof spec.name === "string" ? spec.name : undefined;
for (const inner of spec.tools) {
if (!isPlainObject(inner) || typeof inner.name !== "string") continue;
if (namespace !== "functions" && namespace === inner.name) {
if (inner.type === "custom") sameNameNamespacedCustomNames.add(inner.name);
else if (inner.type === "function") sameNameNamespacedFunctionNames.add(inner.name);
}
if (namespace === "functions") {
if (inner.type === "custom") bareCustomNames.add(inner.name);
else if (inner.type === "function") bareFunctionNames.add(inner.name);
}
}
continue;
}
// `buildTools` (parser.ts) also accepts the Chat-shaped `{ type: "function", function: { name } }`
// declaration, and the undeclared-tool guard authorizes it the same way. Reading only
// `spec.name` here left such a function out of the raw-body set, so the intersection dropped
// it and a self-named echo for it reached Codex again.
const nestedFunction = spec.type === "function" && isPlainObject(spec.function) ? spec.function : undefined;
const name = typeof spec.name === "string" && spec.name.length > 0
? spec.name
: typeof nestedFunction?.name === "string" && nestedFunction.name.length > 0
? nestedFunction.name
: undefined;
if (!name) continue;
const namespace = typeof spec.namespace === "string" ? spec.namespace : undefined;
if (namespace !== "functions" && namespace === name) {
if (spec.type === "custom") sameNameNamespacedCustomNames.add(name);
else if (spec.type === "function") sameNameNamespacedFunctionNames.add(name);
}
if (!namespace || namespace === "functions") {
if (spec.type === "custom") bareCustomNames.add(name);
else if (spec.type === "function") bareFunctionNames.add(name);
}
}
}

/** Bare custom tools authorized by this turn, scoped to each response call type. */
export function collectSelfNamedNamespaceScrubAuthorization(
body: unknown,
authorizedBareCustomToolNames: ReadonlySet<string>,
authorizedBareFunctionToolNames: ReadonlySet<string>,
): SelfNamedNamespaceScrubAuthorization {
const bareCustomNames = new Set<string>();
const bareFunctionNames = new Set<string>();
const sameNameNamespacedCustomNames = new Set<string>();
const sameNameNamespacedFunctionNames = new Set<string>();
if (isPlainObject(body)) {
collectBareToolSpecs(
bareCustomNames,
bareFunctionNames,
sameNameNamespacedCustomNames,
sameNameNamespacedFunctionNames,
body.tools,
);
if (Array.isArray(body.input)) {
for (const item of body.input) {
if (
isPlainObject(item)
&& (item.type === "additional_tools" || item.type === "tool_search_output")
) {
collectBareToolSpecs(
bareCustomNames,
bareFunctionNames,
sameNameNamespacedCustomNames,
sameNameNamespacedFunctionNames,
item.tools,
);
}
}
}
}
const authorizedCustomNames = [...bareCustomNames]
.filter(name => authorizedBareCustomToolNames.has(name));
const authorizedFunctionNames = new Set([
...authorizedCustomNames,
...[...bareFunctionNames].filter(name => authorizedBareFunctionToolNames.has(name)),
]);
return {
customToolCallNames: new Set(
authorizedCustomNames.filter(name => !sameNameNamespacedCustomNames.has(name)),
),
functionCallNames: new Set(
[...authorizedFunctionNames].filter(name => !sameNameNamespacedFunctionNames.has(name)),
),
};
}

/**
* Drop a tool-call `namespace` that merely repeats the call's own `name` (#3217).
*
* codex-rs resolves a client tool call as `ToolName::new(namespace, name)` and treats only
* `None | "" | "functions"` as the default namespace; anything else is concatenated into a flat
* name before routing. A backend answer of `{ name: "exec", namespace: "exec" }` therefore
* becomes `execexec`, which no client tool matches, and Codex re-issues the same call forever.
* That shape is never a legitimate identity — an MCP namespace is a server name, not the tool —
* so it is safe to scrub without consulting the declared catalog. The adapter fix that stops
* provoking the answer lives in `stripSparkCompatibility`; this is the belt to that suspender.
* The malformed Spark shape is scrubbed only when the current turn authorized a bare custom tool
* with that name. A genuine namespaced tool may intentionally use the same namespace and name.
* The adapter fix that stops provoking the answer lives in `stripSparkCompatibility`; this is the
* belt to that suspender.
*/
export function scrubSelfNamedToolCallNamespace(value: unknown): { value: unknown; changed: boolean } {
export function scrubSelfNamedToolCallNamespace(
value: unknown,
authorization: SelfNamedNamespaceScrubAuthorization,
): { value: unknown; changed: boolean } {
if (Array.isArray(value)) {
let changed = false;
const out = value.map(entry => {
const result = scrubSelfNamedToolCallNamespace(entry);
const result = scrubSelfNamedToolCallNamespace(entry, authorization);
changed ||= result.changed;
return result.value;
});
Expand All @@ -29,35 +137,45 @@ export function scrubSelfNamedToolCallNamespace(value: unknown): { value: unknow
let changed = false;
const out: Record<string, unknown> = {};
for (const [key, entry] of Object.entries(value)) {
const result = scrubSelfNamedToolCallNamespace(entry);
const result = scrubSelfNamedToolCallNamespace(entry, authorization);
out[key] = result.value;
changed ||= result.changed;
}
const authorizedNames = value.type === "custom_tool_call"
? authorization.customToolCallNames
: value.type === "function_call"
? authorization.functionCallNames
: undefined;
if (
(value.type === "custom_tool_call" || value.type === "function_call")
authorizedNames
&& typeof value.name === "string"
&& value.name.length > 0
&& value.namespace === value.name
&& authorizedNames.has(value.name)
) {
delete out.namespace;
changed = true;
}
return changed ? { value: out, changed: true } : { value, changed: false };
}

export function scrubSelfNamedToolCallNamespaceInJson(text: string): string {
export function scrubSelfNamedToolCallNamespaceInJson(
text: string,
authorization: SelfNamedNamespaceScrubAuthorization,
): string {
if (!text.includes("\"namespace\"")) return text;
let payload: unknown;
try {
payload = JSON.parse(text);
} catch {
return text;
}
const result = scrubSelfNamedToolCallNamespace(payload);
const result = scrubSelfNamedToolCallNamespace(payload, authorization);
return result.changed ? JSON.stringify(result.value) : text;
}

export function createSelfNamedToolCallNamespaceScrubRewrite(): SsePayloadRewrite {
return payload => scrubSelfNamedToolCallNamespaceInJson(payload);
export function createSelfNamedToolCallNamespaceScrubRewrite(
authorization: SelfNamedNamespaceScrubAuthorization,
): SsePayloadRewrite {
return payload => scrubSelfNamedToolCallNamespaceInJson(payload, authorization);
}

27 changes: 26 additions & 1 deletion src/server/responses/collaboration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,16 @@ export function buildToolBridgeMaps(parsed: OcxParsedRequest, budget?: Translato
/** Declared parameter schema per request-visible tool name (#1611 integer repair). */
toolParameterSchemas: Map<string, Record<string, unknown>>;
freeformToolNames: Set<string>;
bareCustomToolNames: Set<string>;
bareFunctionToolNames: Set<string>;
toolSearchToolNames: Set<string>;
} {
const toolNsMap = new Map<string, { namespace: string; name: string; freeform?: true }>();
const declaredToolNames = new Set<string>();
const toolParameterSchemas = new Map<string, Record<string, unknown>>();
const freeformToolNames = new Set<string>();
const bareCustomToolNames = new Set<string>();
const bareFunctionToolNames = new Set<string>();
const toolSearchToolNames = new Set<string>();
const requestedTools = parsed.context.tools ?? [];
const toolAllowed = toolChoiceToolPredicate(parsed.options.toolChoice, requestedTools);
Expand All @@ -133,6 +137,19 @@ export function buildToolBridgeMaps(parsed: OcxParsedRequest, budget?: Translato
if (t.freeform) {
budget?.chargeRetained(new TextEncoder().encode(t.name).byteLength, { kind: "retained_collectors" });
freeformToolNames.add(t.name);
if (!t.namespace || t.namespace === "functions") {
budget?.chargeRetained(new TextEncoder().encode(t.name).byteLength, { kind: "retained_collectors" });
bareCustomToolNames.add(t.name);
}
} else if (
!t.toolSearch
&& !t.webSearch
&& !t.imageGeneration
&& !t.videoGeneration
&& (!t.namespace || t.namespace === "functions")
) {
budget?.chargeRetained(new TextEncoder().encode(t.name).byteLength, { kind: "retained_collectors" });
bareFunctionToolNames.add(t.name);
}
if (t.toolSearch) {
budget?.chargeRetained(new TextEncoder().encode(t.name).byteLength, { kind: "retained_collectors" });
Expand Down Expand Up @@ -162,7 +179,15 @@ export function buildToolBridgeMaps(parsed: OcxParsedRequest, budget?: Translato
toolParameterSchemas.set(t.name, t.parameters);
}
}
return { toolNsMap, declaredToolNames, toolParameterSchemas, freeformToolNames, toolSearchToolNames };
return {
toolNsMap,
declaredToolNames,
toolParameterSchemas,
freeformToolNames,
bareCustomToolNames,
bareFunctionToolNames,
toolSearchToolNames,
};
}


Expand Down
13 changes: 11 additions & 2 deletions src/server/responses/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ import {
} from "../responses-image-gen-repair";
import { createResponsesModelPayloadRewrite, rewriteResponsesModelJson } from "../responses-model-rewrite";
import {
collectSelfNamedNamespaceScrubAuthorization,
createSelfNamedToolCallNamespaceScrubRewrite,
scrubSelfNamedToolCallNamespaceInJson,
} from "../responses-self-named-namespace-scrub";
Expand Down Expand Up @@ -3661,6 +3662,11 @@ async function handleResponsesInner(
parsed._rawBody,
replayedInputPrefixLength,
);
const selfNamedNamespaceScrubAuthorization = collectSelfNamedNamespaceScrubAuthorization(
clientToolAuthorizationBody,
toolBridgeMaps.bareCustomToolNames,
toolBridgeMaps.bareFunctionToolNames,
);
const clientExplicitWireToolCatalog = hasExplicitWireToolCatalog(clientToolAuthorizationBody);
const clientDeclaredWireToolNames = collectDeclaredWireToolNames(clientToolAuthorizationBody);
const clientDeclaredNamelessCallTypes = collectDeclaredNamelessClientCallTypes(
Expand Down Expand Up @@ -4646,7 +4652,7 @@ async function handleResponsesInner(
const payloadRewrites = [
createImageGenCallRestoreRewrite(imageGenCallAliases),
// #3217: a call whose namespace repeats its own name is unroutable in codex-rs.
createSelfNamedToolCallNamespaceScrubRewrite(),
createSelfNamedToolCallNamespaceScrubRewrite(selfNamedNamespaceScrubAuthorization),
routedNamespaceToolAliases.size > 0
? createRoutedNamespaceCallRestoreRewrite(routedNamespaceToolAliases)
: undefined,
Expand Down Expand Up @@ -4879,7 +4885,10 @@ async function handleResponsesInner(
inspectResponseLogJson(logCtx, text);
const clientJson = (() => {
const restoredNamespace = restoreRoutedNamespaceCallsInJson(
scrubSelfNamedToolCallNamespaceInJson(restoreImageGenCallsInJson(text, imageGenCallAliases)),
scrubSelfNamedToolCallNamespaceInJson(
restoreImageGenCallsInJson(text, imageGenCallAliases),
selfNamedNamespaceScrubAuthorization,
),
routedNamespaceToolAliases,
);
const restoredAuthorizedBareNamespace = restoreRoutedNamespaceCallsInJson(
Expand Down
Loading
Loading