Resolve local MCP refs in Code Mode tool schemas#31901
Conversation
|
I have read the CLA Document and I hereby sign the CLA Alex Phonpradith seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf1be25414
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } else { | ||
| root.pointer(pointer) | ||
| }; | ||
| let rendered = target.map(|target| self.render(target)); |
There was a problem hiding this comment.
When an MCP/server schema reuses a large $defs entry many times (or a DAG of refs fans out), this recursively renders the target inline for every $ref without any total-size or memoization budget; the active-path guard only stops cycles. That can turn a compact tool schema into a single code-mode exec declaration well over 1k/10k tokens, which is injected into the model context, so please add a hard cap or avoid repeated inline expansion.
AGENTS.md reference: AGENTS.md:L91-L100
Useful? React with 👍 / 👎.
| .get("$ref") | ||
| .and_then(JsonValue::as_str) | ||
| .and_then(|reference| { | ||
| let pointer = reference.strip_prefix('#')?; |
There was a problem hiding this comment.
For schemas that use a valid URI-fragment $ref such as #/$defs/Foo%20Bar (or percent-encoded //~ characters), this keeps the raw fragment and later passes it to serde_json::Value::pointer, so the lookup misses the definition and the generated exec declaration falls back to unknown. The input-schema pruning path already accepts decoded local definition refs, so these schemas can reach code mode with their definitions preserved but still lose their types here; decode the fragment before applying JSON-pointer lookup.
Useful? React with 👍 / 👎.
| .map(|variant| self.render(variant)) | ||
| .collect::<Vec<_>>(); | ||
| if !rendered.is_empty() { | ||
| return rendered.join(" & "); |
There was a problem hiding this comment.
When an allOf branch is a $ref to a union-producing definition (for example a oneOf or multi-value enum), self.render(variant) now returns a string like A | B and this join emits A | B & C. TypeScript parses that as A | (B & C), not (A | B) & C, so the generated code-mode declaration makes the referenced branch appear valid without the other allOf constraints. Parenthesize union operands before joining with &.
Useful? React with 👍 / 👎.
1 participant
Summary
$refvalues against the schema root when Code Mode renders TypeScript tool declarations.#/$defs/...and#/definitions/..., including RFC 6901 escaped path segments.$refsibling descriptions and intersect renderable sibling constraints.unknown, so generated declarations stay finite.structuredContent.results.itemsrefs instead of renderingArray<unknown>.Why
MCP schemas that use local or recursive refs currently lose their useful shape in Codex Code Mode. For the SE-8141 fixture, the model sees recursive query fields and structured result items as
unknown, which weakens first-call argument construction.Related: SE-8141, CLI-3774, CONNECT-521.
Scope and limitations
This is a narrow Codex Code Mode fix. It does not change schema normalization or the Functions/Responses v3 renderer.
Recursive rendering is intentionally a finite prefix, not a full recursive TypeScript alias: references deeper than two occurrences on the same active path remain
unknown. The depth bound prevents infinite output, but repeated branching can still increase prompt size versus today's renderer. Before making this ready for review, owners should confirm the bound with prompt-size measurements and the saved SE-8141 fixture.Validation
just test -p codex-code-mode-protocol— 26 passedjust fix -p codex-code-mode-protocoljust fmt