fix: sanitize Gemini schemas and MCP notifications - #757
Open
1jehuang wants to merge 19 commits into
Open
Conversation
1jehuang
force-pushed
the
fix/issue-754-gemini-mcp-notification
branch
from
August 3, 2026 20:25
7a831af to
073c512
Compare
Greptile SummaryThis PR improves provider schema handling, MCP notification serialization, and SDK runtime packaging. The main changes are:
Confidence Score: 5/5This PR is safe to merge with low risk. The schema and MCP notification changes are covered by focused tests. The reviewed normalization paths preserve load-bearing schema fields while stripping unsupported provider constructs. No blocking correctness or security issues were identified. Files Needing Attention: No files require special attention.
 T-Rex LogsWhat T-Rex did
|
| Filename | Overview |
|---|---|
| crates/jcode-schema-dialect/src/dialect.rs | Implements shared JSON Schema dialect walking, allow-list filtering, and structural transforms. |
| crates/jcode-schema-dialect/src/registry.rs | Defines supported keyword tables and transforms for OpenAI, Gemini, Anthropic, OpenRouter, and Antigravity routes. |
| crates/jcode-schema-dialect/src/rejection.rs | Parses known provider schema rejection messages into recoverable keyword or format actions. |
| crates/jcode-provider-gemini/src/lib.rs | Routes native Gemini tool schema normalization through the shared schema dialect crate. |
| crates/jcode-provider-antigravity/src/lib.rs | Defines Antigravity per-upstream schema dialect selection and compatibility normalization. |
| crates/jcode-provider-antigravity-runtime/src/lib.rs | Adds schema-rejection recovery and applies per-model Antigravity dialect normalization before requests. |
| crates/jcode-base/src/mcp/client.rs | Sends MCP initialized as a JSON-RPC notification without an id. |
| crates/jcode-base/src/mcp/protocol.rs | Adds a serializable JSON-RPC notification type for id-less notification messages. |
| crates/jcode-base/src/usage/model.rs | Adds model-scoped usage windows, stale detection, and family-based exhaustion matching. |
| crates/jcode-provider-anthropic-runtime/src/lib.rs | Routes exhausted OAuth Fable requests to the best available Opus fallback after a usage check. |
| sdk/typescript/src/binary.ts | Adds platform package resolution for bundled jcode binaries with PATH fallback. |
| sdk/typescript/src/launch.ts | Uses the bundled runtime by default and improves missing-binary startup errors. |
Sequence Diagram
sequenceDiagram
participant MCP as MCP Tool Schema
participant Provider as Provider Runtime
participant Dialect as jcode-schema-dialect
participant API as Provider API
participant Quirks as schema-quirks store
MCP->>Provider: ToolDefinition input_schema
Provider->>Dialect: normalize(schema, provider dialect)
Dialect->>Quirks: load learned rejected keywords/formats
Quirks-->>Dialect: learned quirks
Dialect-->>Provider: compatible parameters schema
Provider->>API: generateContent / function request
alt API rejects schema with named construct
API-->>Provider: schema rejection error
Provider->>Dialect: recover_from_error(error, dialect)
Dialect->>Quirks: persist rejected construct
Dialect-->>Provider: RetryWithoutConstruct
Provider->>Dialect: normalize(schema, dialect + learned quirk)
Dialect-->>Provider: stripped schema
Provider->>API: retry request
else API accepts schema
API-->>Provider: model response
end
Reviews (3): Last reviewed commit: "fix(anthropic): reroute exhausted Fable ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+77
to
+80
| ]) || self | ||
| .model_scoped | ||
| .iter() | ||
| .any(|window| usage_reset_passed([window.resets_at.as_deref()])) |
There was a problem hiding this comment.
get() returns current_data.display_snapshot() immediately after this stale check, but display_snapshot() only clears five_hour, seven_day, and seven_day_opus; it never clears model_scoped. When a model-scoped reset time has passed, users still see the old model-specific utilization until the background refresh completes, unlike the existing windows that are hidden immediately.
Context Used: AGENTS.md (source)
Artifacts
Repro: focused Rust regression test for expired model_scoped display snapshot
- Evidence file captured while the check ran.
Repro: cargo test output showing expired model_scoped window remains in display snapshot
- The full command output behind this check.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-base/src/usage/model.rs
Line: 77-80
Comment:
**Clear expired model windows**
`get()` returns `current_data.display_snapshot()` immediately after this stale check, but `display_snapshot()` only clears `five_hour`, `seven_day`, and `seven_day_opus`; it never clears `model_scoped`. When a model-scoped reset time has passed, users still see the old model-specific utilization until the background refresh completes, unlike the existing windows that are hidden immediately.
**Context Used:** AGENTS.md ([source](https://app.greptile.com/solo-systems/github/1jehuang/jcode/-/custom-context?memory=04aa75db-3e8e-4529-8341-c7b9dc373978))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.jcode sends its whole tool array on every request, so one JSON Schema construct a provider dislikes does not degrade one tool: it 400s every turn and the provider goes down. That has now shipped eight times (#446, #495, #543, #655, #687, #711, #713, #754), each fixed by appending one keyword to one provider's hand-written deny-list. A deny-list can only contain what has already broken for a user, so the next unlisted keyword from the next MCP server is the next outage. This replaces that loop with three layers in a new jcode-schema-dialect crate: 1. Prevention. Each provider declares the subset it is observed to ACCEPT. Unknown constructs are dropped rather than forwarded, so a keyword nobody has seen yet is inert instead of fatal. One shared recursion with real keyword classification replaces four bespoke walkers, so a fix lands for every provider at once. 2. Recovery. A provider rejection is parsed into the offending keyword (parser tested against the verbatim 400 text from each issue) and the turn is retried without it, instead of failing in front of the user. 3. Memory. The learned rejection is persisted to ~/.jcode/schema-quirks.json, so it costs one wasted round trip ever rather than one per request, and the fix propagates without a release. The new registry-wide conformance sweep runs every real tool through every dialect in CI, and immediately found three live defects that no user had reported because none of them produce an error: - Combiner flattening kept only the chosen branch's `properties`, so `swarm` reached Antigravity's Claude route advertising 3 of its 44 parameters. - A narrowing branch overwrote the parent's declaration, deleting prompt-visible descriptions. - Gemini's `oneOf` was passed through unrenamed; the schema proto has only `anyOf`, so `batch`'s entire call shape was at risk. Antigravity's Gemini route now shares the native Gemini provider's dialect, pinned by a test, so it can no longer inherit a Gemini schema outage a release late the way it did in #754.
None of these relate to each other; all four were blocking a clean `cargo test --workspace`, which is what the new schema conformance sweep needs in order to be meaningful. - `src/cli/commands_tests.rs` and `menubar.rs` still used the numeric todo confidence API that c98c9cc migrated to semantic states, so the root crate's test target did not compile at all. Every test in it, including 209 unrelated ones, had silently stopped running. - `tool_parameter_descriptions_stay_under_token_cap` failed on four todo goal descriptions. These are always-on prompt cost paid on every request, so they are shortened rather than the cap raised. - `configured_auth_test_targets_...` asserted OpenRouter is a configured target, but OpenRouter availability is read from the ambient `OPENROUTER_API_KEY`, so the result depended on the developer's shell. It now sets and restores the variable. - The two detached-cancel tests predate the fix that made cancelling an idle session a deliberate no-op, so they exercised a path that no longer signals. They now register an active turn, which is the another-connection-owns-the-turn case they were written to cover.
…ayload Adds an end-to-end test that starts from the verbatim `tools/list` JSON `@playwright/mcp` emits, turns it into a ToolDefinition the way the MCP client does, and asserts on the serialized provider request. The unit tests operate on schema values and the registry sweep covers jcode's own tools, so neither exercised the third-party-MCP path that #754 actually broke. It immediately caught a wrong assumption in the dialect table: the Antigravity Claude and bridge routes were given Anthropic's and a generic OpenAI-ish keyword set, on the theory that the backend validates against whichever upstream serves the model. It does not. Every Antigravity request is a `generateContent` payload regardless of the model it names, so a keyword outside the Gemini schema proto is rejected while parsing the payload and never reaches the upstream translation that would have accepted it. `propertyNames` survived to the wire for both routes, which is the exact failure in #754. Both now use Gemini's keyword set plus their own extra restrictions (combiner flattening for the Anthropic translation, no numeric bounds for the bridge that corrupts them).
The Antigravity runtime learns a rejected construct from the provider's 400 and retries the turn without it; the native Gemini runtime hit the same `generateContent` validator and had no such path, so an unlisted keyword there was still a hard failure until the next release. #754 reported both routes, so both now recover. Also pins the wiring against the error string the runtime actually builds. `generate_content` wraps the HTTP body as "Gemini request {method} failed (HTTP 400): {body}", where the body is the raw JSON envelope with backslash-escaped quotes, and anyhow adds a "Caused by" layer. A classifier that only matched the clean provider sentence would compile, pass its own unit tests, and never fire in production. The new test feeds it the full wrapped form from #754 and asserts the keyword is still extracted.
Found by actually exercising the recovery path against the live Antigravity endpoint instead of trusting its unit tests. I widened the Gemini allow-list to admit constructs the provider rejects, ran with an empty quirk store, and watched what happened. Recovery fired and persisted, but the turn still failed: a real Gemini 400 carries a `fieldViolations` array naming each bad keyword separately, and the classifier returned only the first. So a schema with two bad keywords cost two failed turns, three cost three. From the user's side that is indistinguishable from the recovery layer not working. `SchemaRejection.keyword` becomes `keywords`, populated from every "Unknown name X" occurrence in the response and deduplicated (the top-level `message` repeats the first violation). The regression test uses the verbatim two-violation response captured from the live endpoint, not a transcription from an issue. Re-verified live afterwards: from an empty quirk store, one request is rejected, both keywords are learned from it, the retry succeeds, and the turn completes with the user seeing only a successful tool call. With the experiment reverted, the same schema produces no rejection at all and no quirk file, because prevention strips the keywords before sending. Also worth recording: the endpoint now accepts `propertyNames` and `uniqueItems`, which it rejected when #754 was filed. Provider subsets move in both directions, which is the case for learning them at runtime rather than pinning a list in a release.
…713) Verified #713 still reproduced on current master before touching it: cua-driver's `set_config.value` declares a description and no type, which `schema_supports_strict` accepted, so jcode sent `strict: true` and OpenAI rejected the entire tool catalog. Every OpenAI-route agent died on its first turn while Anthropic-route agents on the same catalog worked. A typeless property is legal JSON Schema (an empty schema accepts any instance) and the omission is deliberate upstream: `value`'s type depends on the sibling `key`, so there is no correct type to declare. The fix is therefore to fail strict eligibility closed, not to rewrite the schema. The tool is still advertised with its real shape, just without the strict claim jcode could not honor. Failing closed has an obvious failure mode of its own, so a second test pins that a fully typed schema still qualifies, covering `type`, `enum`, `anyOf`, nested objects, and boolean schemas. Also adds `untyped_properties` to the registry-wide sweep. This one is not per-dialect (no provider rejects a typeless property outright), but a built-in tool acquiring one would silently cost every OpenAI-route agent its structured-output guarantees with nothing to catch it. Mutation- verified: removing the `type` from todo's `content` property makes the sweep fail with that exact path.
#711) #711 reports four constructs from a real MCP catalog that jcode marked `strict: true` and OpenAI then rejected, failing the whole tool catalog. I reproduced all four against master before changing anything: - a constraint-only `anyOf` branch (`{"required": ["memory_id"]}`) - an enum-only `anyOf` branch with no `type` key - an array whose `items` are unconstrained - a `$ref` that survives normalization with no `$defs` to resolve it All four are legal JSON Schema that Anthropic accepts, so as with #713 the fix is to fail strict eligibility closed rather than rewrite the schema: the tool keeps its real shape and only loses a claim jcode could not honor. Combiner branches are held to a stricter bar than properties. A property described only by an `enum` is fine, but OpenAI wants a `type` key on a branch, which is what the enum-only case hit. The issue links a contributor's branch. Per AGENTS.md I did not look at or take from it; the reported constructs were enough to reproduce and fix independently. Failing closed is itself risky, so two guards bound it: a test that a well-formed schema (typed properties, typed array items, enum, nested object, combiner) still qualifies, and a registry-wide test pinning the *exact* set of strict-ineligible built-ins. Those four (batch, browser, initiative, swarm) were already non-strict before this change, verified by stashing it; pinning the set means a fifth name appearing fails the build as an over-aggressive rule, and a name disappearing fails it as a stale list.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
propertyNamesfrom Gemini tool schemas in native Gemini and Antigravity pathsnotifications/initializedas a JSON-RPC notification without anidVerification
test_json_rpc_notification_serialization_omits_idantigravity_compatible_schema_only_strips_property_names_for_geminicompatible_schema_strips_nested_property_namesFixes #754.
--- — Jcode agent (automated triage), on behalf of @1jehuang