fix(opencode-go): route muse-spark-1.3-contributor over Responses with Zen Go tool-surface guards - #1
fix(opencode-go): route muse-spark-1.3-contributor over Responses with Zen Go tool-surface guards#1guhcostan wants to merge 1 commit into
Conversation
…h Zen Go tool-surface guards
|
Opened against the fork by mistake; superseded by lidge-jun#3312. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 1c4c9946bd
ℹ️ 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".
| if (!node.$ref.startsWith("#/") && node.$ref !== "#" && node.$ref !== "#/") return false; | ||
| const target = lookupLocalJsonPointer(root, node.$ref); | ||
| if (target === undefined) return false; | ||
| return visit(target, [...stack, node.$ref]); |
There was a problem hiding this comment.
For a Muse request with a long but acyclic local $ref chain—for example, thousands of shallow $defs entries that each reference the next—visit recurses once per reference without any depth or node budget. Such JSON remains shallow enough to parse and serialize, but this walk can exhaust Bun's call stack before the upstream request is built, turning a caller-controlled tool schema into a failed request or process crash. Use the bounded/iterative schema-walk pattern already employed by the other adapter schema normalizers.
Useful? React with 👍 / 👎.
| function isMuseSparkGatewayModel(modelId: unknown): boolean { | ||
| const normalized = typeof modelId === "string" ? modelId.trim().toLowerCase() : ""; | ||
| const base = normalized.includes("/") ? normalized.split("/").pop() ?? normalized : normalized; | ||
| return base === "muse-spark-1.2-contributor" || base === "muse-spark-1.3-contributor"; |
There was a problem hiding this comment.
This predicate checks only the model ID, while its callers run for every noncanonical Responses provider. Consequently, a custom provider serving muse-spark-1.2-contributor or muse-spark-1.3-contributor at a different gateway will also lose long-named and recursive-schema tools even if that gateway accepts them; the restriction was only established for the Zen Go destination. Pass the provider into this guard and require the Zen Go base URL before mutating the tool surface.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| if (isPlainObject(next.tool_choice) && typeof next.tool_choice.name === "string" && dropped.has(next.tool_choice.name)) { | ||
| next = { ...next, tool_choice: "auto" }; |
There was a problem hiding this comment.
When the caller uses an allowed_tools selector, the removed function name is nested under tool_choice.tools[], so this direct .name check does nothing; the identical check in the recursive-schema filter has the same problem. The outgoing request therefore retains a selector for an omitted tool—and for the length case still contains the rejected over-64-character name—so Zen Go can return the same 400 this guard was intended to prevent. Reconcile both direct and allowed_tools choices, preserving remaining entries and refusing a required choice when none remain.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| } | ||
| if (dropped.size > 0) { | ||
| // eslint-disable-next-line no-console | ||
| console.warn(`[opencodex] muse-spark: dropped ${dropped.size} tool(s) with recursive schemas rejected by Zen Go: ${[...dropped].join(", ")}`); |
There was a problem hiding this comment.
Whenever a recursive schema is dropped, this warning writes the exact caller-controlled tool.name field from the request body into persistent process/service logs. MCP tool names can contain server, workspace, tenant, or account-derived identifiers, so this violates the repository's prohibition on logging request-body/account data; emit only the count or use the existing redacted diagnostic mechanism.
AGENTS.md reference: AGENTS.md:L345-L346
Useful? React with 👍 / 👎.
1 participant
Summary
Makes
opencode-go/muse-spark-1.3-contributorusable through the proxy (it 400/500'd on every Codex request before):src/providers/registry.ts): route 1.3 overopenai-responses(probed 2026-09-02:/chat/completions-> upstream 500,/responses-> 200), declare its real effort ladder (none..xhigh, nomaxrung per the gateway error message), and map Codexmax->xhighso default-max callers stop taking a gateway 400.src/adapters/openai-responses.ts): generalize the 1.2-only Muse Spark guards to 1.3 via a sharedisMuseSparkGatewayModelpredicate (matches bare orprovider/-namespaced ids), and add two outbound-only guards for limits Zen Go enforces but OpenAI tolerates in practice here:function/customdeclarations with names > 64 chars (Codex MCP tools reach 67 chars; whole turn 400'd otherwise),functiondeclarations with cyclic local$refschemas (Recursive JSON schemas are not currently supported; diamond $refs sharing one$defsentry are kept). Atool_choicenaming a dropped tool falls back toauto.tests/muse-spark-web-search-compat.test.ts(1.3 strip, registry wire default, name-length drop/keep/additional_tools/tool_choice, cyclic-drop/diamond-keep/scoping).Out of scope on purpose: context-window / modality claims for 1.3 (unverified), and the unrelated Codex-CLI websocket UX.
Verification
bun test tests/muse-spark-web-search-compat.test.ts— 13 pass, 0 fail (6 new assertions failed before the source change = RED, pass after).bun run typecheck— clean.bun run privacy:scan— passed (new logs emit counts + dropped tool names only, no bodies/keys).bun run test:changed— 14071 pass / 11 skip / 1 fail; the single failure (codex-shim.test.tslease-fd timing) passes isolated both with and without this change, so it reads as load flakiness, not a regression.codex execwithopencode-go/muse-spark-1.3-contributorand-1.2-contributorreturn OK (previously 500/400),gpt-5.6-solstill OK, and a real shell-tool round trip on 1.3 completes.Checklist