Repository navigation
fix: emit provider-safe JSON Schema for update_goal_plan tool input - #70
Conversation
The V2 update_goal_plan registration passed raw zod objects from planToolArgs as the tool input, so hosts serialized zod internals (def/checks/shape/optional plus null-valued keywords like maxLength/format) into the JSON Schema sent to providers. Providers that strictly validate tool schemas (e.g. Mistral mistral-large-4) reject those requests with 'Invalid tool schema', breaking every session that includes the goal tools on such models. Convert PlanToolSchema once via z.toJSONSchema and hand each registration a deep copy so hosts or test frameworks that mutate a received tool input cannot corrupt the shared schema. Runtime argument validation is unchanged: planFromTool still parses through PlanToolSchema.
danyel117
left a comment
There was a problem hiding this comment.
The code change matches #69, and the local lint, typecheck, 363 tests, build, and pack checks pass. I found no high or medium code issue.
Before merge, please update the PR description to name the AI model and agent harness used, or explicitly say the change was manual, as required by CONTRIBUTING.md. The fork's CI is still awaiting workflow approval; I will check the full CI run after it starts.
|
Author of #68 here: happy to consolidate the fix into this PR rather than merge competing implementations. Both changes address #69; your live Mistral verification and registration-isolation coverage make this a good final candidate. Our complementary verification: the fix passed an isolated lifecycle smoke on OpenCode 2.0.25 with a deterministic local model, and Ajv validated the update_goal_plan schema in all 10 actual model requests. This was not live OpenAI API testing. If useful, our regression test in test/server-v2.test.ts checks the serialized nested plan schema: required fields, strict objects, array limits, task statuses, nullable/optional evidence, and defaulted decisions. Feel free to reuse those assertions from #68 (commits b88c9e2 and e44a291). @danyel117, I support selecting #70 as the final fix. Once you confirm consolidation, I will close #68 as superseded. Thanks for reviewing both! |
…ld dist with minimal diff Co-authored-by: abeisleem <abeisleem@users.noreply.github.com>
|
@abeisleem Thanks for the graceful consolidation — agreed, and @danyel117 we're happy for #68 to close as superseded by this one. What I took from #68 and what I deliberately kept different: Ported from #68 (with attribution in the test file):
Kept as-is here, with rationale:
Also ported the @danyel117 The PR description now names the AI model and agent harness per CONTRIBUTING.md: developed with OpenCode + GLM 5.3 (Mistral), with the provider-level reproduction and end-to-end verification done manually from the CLI. The rebuilt |
|
Final review of 716cb37: the added nested-plan assertions cover required fields, bounds, statuses, nullable evidence, and defaults; the disclosure is complete, and the bundled dist matches a fresh build. I found no remaining high or medium issue. I ran the full local gate (lint, typecheck, 364 tests, build, pack dry-run), and all five CI checks passed. #70 consolidates the schema fix and the complementary coverage from #68; no maintainer code correction was needed. |
Fixes #69. Consolidates #68 (thanks @abeisleem — the nested plan schema regression test is ported from your commits b88c9e2/e44a291 with attribution in the test file).
Problem
goalToolsV2registeredupdate_goal_planwithinput: v2ObjectSchema(planToolArgs), whereplanToolArgsholds raw zod schemas. Hosts serialize those as-is, so the tool schema sent to providers contains zod internals (def,checks,shape,optional,isFinite, …) and null values for keywords that must be integers or strings ("maxLength": null,"format": null). Providers that strictly validate tool schemas reject the request — reproductions so far: Mistralmistral-large-4(400 Invalid tool schema), OpenAI Responses (invalid_function_parameters), and Gemini/Claude on Google Vertex and Bedrock (see #69). Since the plugin registers the goal tools into every session (including subagents), any session on a strictly-validating model dies before the first token, even with no active goal.Change
PlanToolSchemato JSON Schema once at module level withz.toJSONSchema(PlanToolSchema, { io: "input", unrepresentable: "any" }).io: "input"keeps the schema provider-facing: optionalrevisit_evidencestays out ofrequired, anddecisionskeeps itsdefault.goalToolsV2hands each registration a deep copy of that schema. A shared mutable object would let any host (or test framework — Bun'stoMatchObjectdemonstrably mutates and even unfreezes the object it receives) corrupt the schema for every other registration.toolmap keeps its zodargsunchanged, matching the V1 plugin API contract.planFromToolstill validates throughPlanToolSchema.parse.Tests
properties/required/additionalProperties.anyOf, and defaulted decisions.bun run lint,bun run typecheck,bun test(364 pass),npm pack --dry-run.dist/server.jsis committed with a minimal 9-line diff, functionally identical tobun run build.Verification
End-to-end against the live Mistral API with OpenCode v2.0.24 through a request-capturing local proxy:
400 Invalid tool schemaon everymistral-large-4session.update_goal_planschema is clean JSON Schema, and calling the tool round-trips correctly (proper domain response fromplanFromTool).Tooling attribution (per CONTRIBUTING.md)
Developed with the OpenCode agent harness using GLM 5.3 (
zai-glm-5-3via the Mistral API). The provider-level reproduction and verification (direct Mistral API calls, the request-capturing proxy, and the end-to-end smoke runs) were performed manually from the CLI; the captured request payloads and API error strings quoted in #70 and #69 are from those manual runs.