From 073c512b0ddbb4cf705234cdeb7b231b24f788f8 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 13:22:31 -0700 Subject: [PATCH 01/19] fix: sanitize Gemini schemas and MCP notifications (fixes #754) --- crates/jcode-base/src/mcp/client.rs | 2 +- crates/jcode-base/src/mcp/protocol.rs | 19 +++++++++++++ crates/jcode-base/src/mcp/protocol_tests.rs | 10 +++++++ .../src/antigravity_tests.rs | 28 ++++++++++++++----- crates/jcode-provider-antigravity/src/lib.rs | 23 +++++++++++++-- crates/jcode-provider-gemini/src/lib.rs | 23 +++++++++++++++ 6 files changed, 95 insertions(+), 10 deletions(-) diff --git a/crates/jcode-base/src/mcp/client.rs b/crates/jcode-base/src/mcp/client.rs index c8e65b4d40..b14b24166f 100644 --- a/crates/jcode-base/src/mcp/client.rs +++ b/crates/jcode-base/src/mcp/client.rs @@ -312,7 +312,7 @@ impl McpClient { } // Send initialized notification - let notif = JsonRpcRequest::new(0, "notifications/initialized", None); + let notif = JsonRpcNotification::new("notifications/initialized", None); let msg = serde_json::to_string(¬if)? + "\n"; self.handle.writer_tx.send(msg).await?; diff --git a/crates/jcode-base/src/mcp/protocol.rs b/crates/jcode-base/src/mcp/protocol.rs index a1de3e2245..d59b279038 100644 --- a/crates/jcode-base/src/mcp/protocol.rs +++ b/crates/jcode-base/src/mcp/protocol.rs @@ -24,6 +24,25 @@ impl JsonRpcRequest { } } +/// JSON-RPC notification (a request without an `id`). +#[derive(Debug, Clone, Serialize)] +pub struct JsonRpcNotification { + pub jsonrpc: &'static str, + pub method: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub params: Option, +} + +impl JsonRpcNotification { + pub fn new(method: impl Into, params: Option) -> Self { + Self { + jsonrpc: "2.0", + method: method.into(), + params, + } + } +} + /// JSON-RPC response #[derive(Debug, Clone, Deserialize)] pub struct JsonRpcResponse { diff --git a/crates/jcode-base/src/mcp/protocol_tests.rs b/crates/jcode-base/src/mcp/protocol_tests.rs index dba544a122..a29b007706 100644 --- a/crates/jcode-base/src/mcp/protocol_tests.rs +++ b/crates/jcode-base/src/mcp/protocol_tests.rs @@ -41,6 +41,16 @@ fn test_json_rpc_request_serialization() { assert!(json.contains("\"method\":\"tools/list\"")); } +#[test] +fn test_json_rpc_notification_serialization_omits_id() { + let notification = JsonRpcNotification::new("notifications/initialized", None); + let value = serde_json::to_value(notification).unwrap(); + assert_eq!(value["jsonrpc"], "2.0"); + assert_eq!(value["method"], "notifications/initialized"); + assert!(value.get("id").is_none()); + assert!(value.get("params").is_none()); +} + #[test] fn test_json_rpc_response_deserialization() { let json = r#"{"jsonrpc":"2.0","id":1,"result":{"tools":[]}}"#; diff --git a/crates/jcode-provider-antigravity-runtime/src/antigravity_tests.rs b/crates/jcode-provider-antigravity-runtime/src/antigravity_tests.rs index bb596c9f64..baec21a2f5 100644 --- a/crates/jcode-provider-antigravity-runtime/src/antigravity_tests.rs +++ b/crates/jcode-provider-antigravity-runtime/src/antigravity_tests.rs @@ -477,9 +477,9 @@ fn strip_numeric_schema_bounds_drops_array_and_string_and_object_bounds() { } #[test] -fn antigravity_compatible_schema_passes_gemini_through_unchanged() { - // Gemini is the native backend path; it accepts everything jcode emits, so - // the schema must be byte-identical (combiners and numeric bounds intact). +fn antigravity_compatible_schema_only_strips_property_names_for_gemini() { + // Gemini keeps combiners and bounds, but generateContent rejects the + // `propertyNames` keyword even when it is nested. let schema = serde_json::json!({ "type": "object", "properties": { @@ -489,14 +489,28 @@ fn antigravity_compatible_schema_passes_gemini_through_unchanged() { { "items": { "type": "string" }, "type": "array" } ] }, - "tool_calls": { "type": "array", "minItems": 1, "maxItems": 10 } + "tool_calls": { "type": "array", "minItems": 1, "maxItems": 10 }, + "data": { + "type": "object", + "propertyNames": { "type": "string" }, + "additionalProperties": { "type": "string" } + } } }); + let out = antigravity_compatible_schema(&schema, "gemini-3-flash"); + assert!(out["properties"]["data"].get("propertyNames").is_none()); + assert_eq!( + out["properties"]["status_filter"], + schema["properties"]["status_filter"] + ); + assert_eq!( + out["properties"]["tool_calls"], + schema["properties"]["tool_calls"] + ); assert_eq!( - antigravity_compatible_schema(&schema, "gemini-3-flash"), - schema, - "Gemini path must not rewrite the schema" + out["properties"]["data"]["additionalProperties"], + serde_json::json!({ "type": "string" }) ); } diff --git a/crates/jcode-provider-antigravity/src/lib.rs b/crates/jcode-provider-antigravity/src/lib.rs index 7f54bf1d57..d5d05a5bec 100644 --- a/crates/jcode-provider-antigravity/src/lib.rs +++ b/crates/jcode-provider-antigravity/src/lib.rs @@ -465,10 +465,11 @@ pub fn model_is_gemini(model: &str) -> bool { /// `maxProperties` for these models. These are advisory bounds the model does /// not need to satisfy a call, so dropping them is safe. /// -/// Gemini (the native path) is returned unchanged. +/// Gemini (the native path) rejects `propertyNames`, so that keyword is +/// removed recursively while the rest of the schema is preserved. pub fn antigravity_compatible_schema(schema: &Value, model: &str) -> Value { if model_is_gemini(model) { - return schema.clone(); + return strip_schema_key(schema, "propertyNames"); } if model_is_claude(model) { return flatten_schema_combiners(schema); @@ -479,6 +480,24 @@ pub fn antigravity_compatible_schema(schema: &Value, model: &str) -> Value { strip_numeric_schema_bounds(&flatten_schema_combiners(schema)) } +fn strip_schema_key(schema: &Value, rejected_key: &str) -> Value { + match schema { + Value::Object(map) => Value::Object( + map.iter() + .filter(|(key, _)| key.as_str() != rejected_key) + .map(|(key, value)| (key.clone(), strip_schema_key(value, rejected_key))) + .collect(), + ), + Value::Array(items) => Value::Array( + items + .iter() + .map(|value| strip_schema_key(value, rejected_key)) + .collect(), + ), + _ => schema.clone(), + } +} + /// Numeric JSON Schema bounds an OpenAI-compatible Antigravity bridge corrupts /// when round-tripping through a protobuf `int64` field. const NUMERIC_SCHEMA_BOUND_KEYS: &[&str] = &[ diff --git a/crates/jcode-provider-gemini/src/lib.rs b/crates/jcode-provider-gemini/src/lib.rs index 35a1b0c6f1..d53c15dd3b 100644 --- a/crates/jcode-provider-gemini/src/lib.rs +++ b/crates/jcode-provider-gemini/src/lib.rs @@ -460,6 +460,7 @@ const GEMINI_UNSUPPORTED_SCHEMA_KEYS: &[&str] = &[ "$defs", "definitions", "$comment", + "propertyNames", ]; fn gemini_compatible_schema(schema: &Value) -> Value { @@ -744,6 +745,28 @@ mod tests { use super::*; use serde_json::json; + #[test] + fn compatible_schema_strips_nested_property_names() { + let schema = json!({ + "type": "object", + "properties": { + "data": { + "type": "object", + "propertyNames": { "type": "string" }, + "additionalProperties": { "type": "string" } + } + } + }); + let out = gemini_compatible_schema(&schema); + assert!(out["properties"]["data"].get("propertyNames").is_none()); + // Existing Gemini normalization still drops additionalProperties. + assert!( + out["properties"]["data"] + .get("additionalProperties") + .is_none() + ); + } + #[test] fn fallback_models_skip_current_model() { assert_eq!( From 336bf4d9a78cfecdb88e8482ecef0c63d97e4f20 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 14:54:02 -0700 Subject: [PATCH 02/19] fix(usage): show Anthropic model-scoped limits --- crates/jcode-base/src/usage.rs | 13 +++++++ crates/jcode-base/src/usage/cache.rs | 31 +++++++++++++++- crates/jcode-base/src/usage/model.rs | 34 ++++++++++++++++- crates/jcode-base/src/usage/tests.rs | 55 ++++++++++++++++++++++++++++ 4 files changed, 131 insertions(+), 2 deletions(-) diff --git a/crates/jcode-base/src/usage.rs b/crates/jcode-base/src/usage.rs index 0d15e47ff0..5c8e5c4d66 100644 --- a/crates/jcode-base/src/usage.rs +++ b/crates/jcode-base/src/usage.rs @@ -118,6 +118,19 @@ async fn fetch_anthropic_usage_data(access_token: String, cache_key: String) -> .as_ref() .and_then(|w| w.utilization) .map(usage_percent_to_ratio), + model_scoped: data + .limits + .into_iter() + .filter(|limit| limit.kind.as_deref() == Some("weekly_scoped")) + .filter_map(|limit| { + let model_name = limit.scope?.model?.display_name?; + Some(ModelScopedUsageWindow { + model_name, + utilization: usage_percent_to_ratio(limit.percent?), + resets_at: limit.resets_at, + }) + }) + .collect(), extra_usage_enabled: data .extra_usage .as_ref() diff --git a/crates/jcode-base/src/usage/cache.rs b/crates/jcode-base/src/usage/cache.rs index c35d147fba..45019856a4 100644 --- a/crates/jcode-base/src/usage/cache.rs +++ b/crates/jcode-base/src/usage/cache.rs @@ -1,5 +1,8 @@ use super::openai_helpers::{classify_openai_limits, usage_percent_to_ratio}; -use super::{AccountUsageSnapshot, OpenAIUsageData, ProviderUsage, UsageData, UsageLimit}; +use super::{ + AccountUsageSnapshot, ModelScopedUsageWindow, OpenAIUsageData, ProviderUsage, UsageData, + UsageLimit, +}; use std::collections::HashMap; use std::time::Instant; @@ -142,6 +145,13 @@ pub(super) fn provider_report_from_usage_data( resets_at: data.seven_day_resets_at.clone(), }); } + for window in &data.model_scoped { + limits.push(UsageLimit { + name: format!("7-day {} window", window.model_name), + usage_percent: window.utilization * 100.0, + resets_at: window.resets_at.clone(), + }); + } let mut extra_info = Vec::new(); extra_info.push(( @@ -184,6 +194,24 @@ pub(super) fn usage_data_from_provider_report(report: &ProviderUsage) -> UsageDa .limits .iter() .find(|limit| limit.name == "7-day Opus window"); + let model_scoped = report + .limits + .iter() + .filter_map(|limit| { + let model_name = limit + .name + .strip_prefix("7-day ")? + .strip_suffix(" window")?; + if model_name == "Opus" { + return None; + } + Some(ModelScopedUsageWindow { + model_name: model_name.to_string(), + utilization: usage_percent_to_ratio(limit.usage_percent), + resets_at: limit.resets_at.clone(), + }) + }) + .collect(); let extra_usage_enabled = report.extra_info.iter().find_map(|(key, value)| { if key == "Extra usage (long context)" { Some(value == "enabled") @@ -202,6 +230,7 @@ pub(super) fn usage_data_from_provider_report(report: &ProviderUsage) -> UsageDa .unwrap_or(0.0), seven_day_resets_at: seven_day.and_then(|limit| limit.resets_at.clone()), seven_day_opus: seven_day_opus.map(|limit| usage_percent_to_ratio(limit.usage_percent)), + model_scoped, extra_usage_enabled: extra_usage_enabled.unwrap_or(false), fetched_at: Some(Instant::now()), last_error: None, diff --git a/crates/jcode-base/src/usage/model.rs b/crates/jcode-base/src/usage/model.rs index bebb86ed26..39810f4eb0 100644 --- a/crates/jcode-base/src/usage/model.rs +++ b/crates/jcode-base/src/usage/model.rs @@ -58,6 +58,8 @@ pub struct UsageData { pub seven_day_resets_at: Option, /// Seven-day Opus utilization (0.0-1.0) pub seven_day_opus: Option, + /// Model-specific weekly windows returned by newer OAuth usage responses. + pub model_scoped: Vec, /// Whether extra usage (long context, etc.) is enabled pub extra_usage_enabled: bool, /// Last fetch time @@ -72,7 +74,9 @@ impl UsageData { if usage_reset_passed([ self.five_hour_resets_at.as_deref(), self.seven_day_resets_at.as_deref(), - ]) { + ]) || self.model_scoped.iter().any(|window| { + usage_reset_passed([window.resets_at.as_deref()]) + }) { return true; } @@ -110,15 +114,43 @@ impl UsageData { } } +#[derive(Debug, Clone, Default, PartialEq)] +pub struct ModelScopedUsageWindow { + pub model_name: String, + /// Utilization as a fraction in [0.0, 1.0]. + pub utilization: f32, + pub resets_at: Option, +} + /// API response structures #[derive(Deserialize, Debug)] pub(super) struct UsageResponse { pub(super) five_hour: Option, pub(super) seven_day: Option, pub(super) seven_day_opus: Option, + #[serde(default)] + pub(super) limits: Vec, pub(super) extra_usage: Option, } +#[derive(Deserialize, Debug)] +pub(super) struct StructuredUsageLimit { + pub(super) kind: Option, + pub(super) percent: Option, + pub(super) resets_at: Option, + pub(super) scope: Option, +} + +#[derive(Deserialize, Debug)] +pub(super) struct UsageLimitScope { + pub(super) model: Option, +} + +#[derive(Deserialize, Debug)] +pub(super) struct UsageLimitModel { + pub(super) display_name: Option, +} + #[derive(Deserialize, Debug)] pub(super) struct UsageWindow { pub(super) utilization: Option, diff --git a/crates/jcode-base/src/usage/tests.rs b/crates/jcode-base/src/usage/tests.rs index cda8ea633a..02fcce6152 100644 --- a/crates/jcode-base/src/usage/tests.rs +++ b/crates/jcode-base/src/usage/tests.rs @@ -720,3 +720,58 @@ fn test_activity_sweeper_skips_sources_with_dedicated_reports() { "some-custom-endpoint" )); } + +#[test] +fn anthropic_model_scoped_fable_limit_survives_report_cache_roundtrip() { + let usage = UsageData { + five_hour: 0.1, + seven_day: 0.2, + model_scoped: vec![ModelScopedUsageWindow { + model_name: "Fable".to_string(), + utilization: 0.73, + resets_at: Some("2026-08-10T00:00:00Z".to_string()), + }], + ..Default::default() + }; + + let report = provider_report_from_usage_data("Anthropic".to_string(), &usage); + let fable = report + .limits + .iter() + .find(|limit| limit.name == "7-day Fable window") + .expect("Fable limit should be displayed"); + assert_eq!(fable.usage_percent, 73.0); + assert_eq!(fable.resets_at.as_deref(), Some("2026-08-10T00:00:00Z")); + + let restored = usage_data_from_provider_report(&report); + assert_eq!(restored.model_scoped, usage.model_scoped); +} + +#[test] +fn anthropic_usage_response_deserializes_structured_fable_limit() { + let response: UsageResponse = serde_json::from_str( + r#"{ + "five_hour": {"utilization": 12.0, "resets_at": "2026-08-04T00:00:00Z"}, + "seven_day": {"utilization": 34.0, "resets_at": "2026-08-10T00:00:00Z"}, + "limits": [{ + "kind": "weekly_scoped", + "percent": 56.0, + "resets_at": "2026-08-11T00:00:00Z", + "scope": {"model": {"display_name": "Fable"}} + }] + }"#, + ) + .expect("structured Anthropic usage payload should deserialize"); + + let fable = response.limits.first().expect("model-scoped limit"); + assert_eq!(fable.kind.as_deref(), Some("weekly_scoped")); + assert_eq!(fable.percent, Some(56.0)); + assert_eq!( + fable + .scope + .as_ref() + .and_then(|scope| scope.model.as_ref()) + .and_then(|model| model.display_name.as_deref()), + Some("Fable") + ); +} From 83860a1d63b0cb4c82f84700652f9883a9872329 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 15:27:42 -0700 Subject: [PATCH 03/19] style(usage): format model-scoped limits --- crates/jcode-base/src/usage/cache.rs | 5 +---- crates/jcode-base/src/usage/model.rs | 8 +++++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/crates/jcode-base/src/usage/cache.rs b/crates/jcode-base/src/usage/cache.rs index 45019856a4..043babe5f7 100644 --- a/crates/jcode-base/src/usage/cache.rs +++ b/crates/jcode-base/src/usage/cache.rs @@ -198,10 +198,7 @@ pub(super) fn usage_data_from_provider_report(report: &ProviderUsage) -> UsageDa .limits .iter() .filter_map(|limit| { - let model_name = limit - .name - .strip_prefix("7-day ")? - .strip_suffix(" window")?; + let model_name = limit.name.strip_prefix("7-day ")?.strip_suffix(" window")?; if model_name == "Opus" { return None; } diff --git a/crates/jcode-base/src/usage/model.rs b/crates/jcode-base/src/usage/model.rs index 39810f4eb0..691add9635 100644 --- a/crates/jcode-base/src/usage/model.rs +++ b/crates/jcode-base/src/usage/model.rs @@ -74,9 +74,11 @@ impl UsageData { if usage_reset_passed([ self.five_hour_resets_at.as_deref(), self.seven_day_resets_at.as_deref(), - ]) || self.model_scoped.iter().any(|window| { - usage_reset_passed([window.resets_at.as_deref()]) - }) { + ]) || self + .model_scoped + .iter() + .any(|window| usage_reset_passed([window.resets_at.as_deref()])) + { return true; } From 76ec0ae2f3fe5e921258a47d18191da053acb2f9 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 15:27:42 -0700 Subject: [PATCH 04/19] fix(desktop2): hide completed todo marker glyph --- crates/jcode-desktop2/src/transcript.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/jcode-desktop2/src/transcript.rs b/crates/jcode-desktop2/src/transcript.rs index 2ce10e3d5f..46c688545e 100644 --- a/crates/jcode-desktop2/src/transcript.rs +++ b/crates/jcode-desktop2/src/transcript.rs @@ -1426,7 +1426,17 @@ fn todo_spans( } // A finished task recedes: the strikethrough says "done" and the // faint ink stops five done lines from shouting over the two left. + // The `โ€ข ` marker keeps its width but loses its ink: the scene draws + // a state dot in that column, and the glyph showed through the ring. + // Kept in the source (rather than stripped) so the text keeps its + // column and a copied task still reads as a list item. BlockKind::ListItem { .. } => { + if let Some(first) = spans.first_mut() + && first.range.start == 0 + && first.role == StyleRole::Dim + { + first.color = Some(Color::TRANSPARENT); + } for span in &mut spans { if span.strikethrough { span.color = Some(theme.faint); From 88a19f38e50e312cc8781f7648b37e078c415a1d Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 15:28:32 -0700 Subject: [PATCH 05/19] release: v0.67.1 --- Cargo.lock | 2 +- Cargo.toml | 2 +- changelog/index.json | 4 ++++ changelog/v0.67.1.json | 13 +++++++++++++ 4 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 changelog/v0.67.1.json diff --git a/Cargo.lock b/Cargo.lock index c7bd950ea6..d5122c45d2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3311,7 +3311,7 @@ checksum = "92ecc6618181def0457392ccd0ee51198e065e016d1d527a7ac1b6dc7c1f09d2" [[package]] name = "jcode" -version = "0.67.0" +version = "0.67.1" dependencies = [ "anyhow", "async-stream", diff --git a/Cargo.toml b/Cargo.toml index e43399a0a0..d4b4025aff 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "jcode" -version = "0.67.0" +version = "0.67.1" description = "Possibly the greatest coding agent ever built โ€” blazing-fast TUI, multi-model, swarm coordination, 30+ tools" edition = "2024" autobins = false diff --git a/changelog/index.json b/changelog/index.json index 3a77c96e92..125f0578aa 100644 --- a/changelog/index.json +++ b/changelog/index.json @@ -1,5 +1,9 @@ { "entries": [ + { + "version": "0.67.1", + "date": "2026-08-03" + }, { "version": "0.67.0", "date": "2026-08-03" diff --git a/changelog/v0.67.1.json b/changelog/v0.67.1.json new file mode 100644 index 0000000000..d8283539f1 --- /dev/null +++ b/changelog/v0.67.1.json @@ -0,0 +1,13 @@ +{ + "version": "0.67.1", + "date": "2026-08-03", + "title": "Provider reliability fixes", + "improvements": [ + "Anthropic usage now shows model-specific weekly limits alongside account-wide windows" + ], + "fixes": [ + "Gemini tool schemas are sanitized for provider compatibility", + "MCP notifications no longer cause request-handling failures", + "Completed desktop todo items render without a duplicate marker glyph" + ] +} From 7d7137048e7adcfa5b3259eba1d868a9ca7b9df1 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 15:29:05 -0700 Subject: [PATCH 06/19] feat(sdk): bundle platform jcode runtimes --- .github/workflows/publish-typescript-sdk.yml | 31 +++++++++ scripts/prepare_sdk_runtime_packages.sh | 28 ++++++++ scripts/test_sdk_package.sh | 39 ++++++++++- sdk/npm/darwin-arm64/README.md | 3 + sdk/npm/darwin-arm64/bin/.gitkeep | 0 sdk/npm/darwin-arm64/package.json | 27 ++++++++ sdk/npm/darwin-x64/README.md | 3 + sdk/npm/darwin-x64/bin/.gitkeep | 0 sdk/npm/darwin-x64/package.json | 27 ++++++++ sdk/npm/linux-arm64/README.md | 3 + sdk/npm/linux-arm64/bin/.gitkeep | 0 sdk/npm/linux-arm64/package.json | 27 ++++++++ sdk/npm/linux-x64/README.md | 3 + sdk/npm/linux-x64/bin/.gitkeep | 0 sdk/npm/linux-x64/package.json | 27 ++++++++ sdk/npm/win32-arm64/README.md | 3 + sdk/npm/win32-arm64/bin/.gitkeep | 0 sdk/npm/win32-arm64/package.json | 27 ++++++++ sdk/npm/win32-x64/README.md | 3 + sdk/npm/win32-x64/bin/.gitkeep | 0 sdk/npm/win32-x64/package.json | 27 ++++++++ sdk/typescript/README.md | 9 ++- sdk/typescript/RELEASING.md | 8 ++- sdk/typescript/package-lock.json | 72 +++++++++++++++++++- sdk/typescript/package.json | 10 ++- sdk/typescript/src/binary.ts | 39 +++++++++++ sdk/typescript/src/index.ts | 1 + sdk/typescript/src/launch.ts | 16 +++-- sdk/typescript/test/launch.test.ts | 8 +++ 29 files changed, 428 insertions(+), 13 deletions(-) create mode 100755 scripts/prepare_sdk_runtime_packages.sh create mode 100644 sdk/npm/darwin-arm64/README.md create mode 100644 sdk/npm/darwin-arm64/bin/.gitkeep create mode 100644 sdk/npm/darwin-arm64/package.json create mode 100644 sdk/npm/darwin-x64/README.md create mode 100644 sdk/npm/darwin-x64/bin/.gitkeep create mode 100644 sdk/npm/darwin-x64/package.json create mode 100644 sdk/npm/linux-arm64/README.md create mode 100644 sdk/npm/linux-arm64/bin/.gitkeep create mode 100644 sdk/npm/linux-arm64/package.json create mode 100644 sdk/npm/linux-x64/README.md create mode 100644 sdk/npm/linux-x64/bin/.gitkeep create mode 100644 sdk/npm/linux-x64/package.json create mode 100644 sdk/npm/win32-arm64/README.md create mode 100644 sdk/npm/win32-arm64/bin/.gitkeep create mode 100644 sdk/npm/win32-arm64/package.json create mode 100644 sdk/npm/win32-x64/README.md create mode 100644 sdk/npm/win32-x64/bin/.gitkeep create mode 100644 sdk/npm/win32-x64/package.json create mode 100644 sdk/typescript/src/binary.ts diff --git a/.github/workflows/publish-typescript-sdk.yml b/.github/workflows/publish-typescript-sdk.yml index d4993290a6..b4df303de0 100644 --- a/.github/workflows/publish-typescript-sdk.yml +++ b/.github/workflows/publish-typescript-sdk.yml @@ -2,6 +2,10 @@ name: Publish TypeScript SDK on: workflow_dispatch: + inputs: + jcode_release_tag: + description: Released jcode tag whose binaries should be bundled + required: true permissions: contents: read @@ -41,5 +45,32 @@ jobs: - name: Validate SDK run: npm run check + - name: Download released jcode runtimes + working-directory: ${{ github.workspace }} + env: + GH_TOKEN: ${{ github.token }} + run: | + mkdir -p sdk-runtime-assets + gh release download "${{ inputs.jcode_release_tag }}" --dir sdk-runtime-assets \ + --pattern 'jcode-linux-x86_64.tar.gz' \ + --pattern 'jcode-linux-aarch64.tar.gz' \ + --pattern 'jcode-macos-x86_64.tar.gz' \ + --pattern 'jcode-macos-aarch64.tar.gz' \ + --pattern 'jcode-windows-x86_64.tar.gz' \ + --pattern 'jcode-windows-aarch64.tar.gz' + + - name: Prepare platform packages + working-directory: ${{ github.workspace }} + run: bash scripts/prepare_sdk_runtime_packages.sh sdk-runtime-assets + + # These must exist before the main package is published. npm selects only + # the package matching the consumer's os/cpu from optionalDependencies. + - name: Publish platform runtime packages + working-directory: ${{ github.workspace }} + run: | + for package in sdk/npm/*; do + npm publish "$package" --access public --provenance + done + - name: Publish with npm provenance run: npm publish --access public --provenance diff --git a/scripts/prepare_sdk_runtime_packages.sh b/scripts/prepare_sdk_runtime_packages.sh new file mode 100755 index 0000000000..144439eb14 --- /dev/null +++ b/scripts/prepare_sdk_runtime_packages.sh @@ -0,0 +1,28 @@ +#!/usr/bin/env bash +# Populate the platform npm packages from jcode release tarballs. +set -euo pipefail + +if [ "$#" -ne 1 ]; then + echo "usage: $0 " >&2 + exit 2 +fi + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +assets="$(cd "$1" && pwd)" + +prepare() { + local package="$1" archive="$2" archived_binary="$3" installed_binary="$4" + local package_dir="$repo_root/sdk/npm/$package" + rm -rf "$package_dir/bin" + mkdir -p "$package_dir/bin" + tar -xzf "$assets/$archive" -C "$package_dir/bin" + mv "$package_dir/bin/$archived_binary" "$package_dir/bin/$installed_binary" + chmod +x "$package_dir/bin/$installed_binary" +} + +prepare linux-x64 jcode-linux-x86_64.tar.gz jcode-linux-x86_64 jcode +prepare linux-arm64 jcode-linux-aarch64.tar.gz jcode-linux-aarch64 jcode +prepare darwin-x64 jcode-macos-x86_64.tar.gz jcode-macos-x86_64 jcode +prepare darwin-arm64 jcode-macos-aarch64.tar.gz jcode-macos-aarch64 jcode +prepare win32-x64 jcode-windows-x86_64.tar.gz jcode-windows-x86_64.exe jcode.exe +prepare win32-arm64 jcode-windows-aarch64.tar.gz jcode-windows-aarch64.exe jcode.exe diff --git a/scripts/test_sdk_package.sh b/scripts/test_sdk_package.sh index b00c7bc273..d6f85c7747 100755 --- a/scripts/test_sdk_package.sh +++ b/scripts/test_sdk_package.sh @@ -26,7 +26,30 @@ echo "packed $tarball" echo "== installing into a fresh consumer ==" cd "$work" npm init -y --silent >/dev/null -npm install "$tarball" --no-audit --no-fund --silent +runtime_tarball="" +if command -v jcode >/dev/null 2>&1 && [ "$(uname -s)" = Linux ]; then + case "$(uname -m)" in + x86_64) runtime_package=linux-x64 ;; + aarch64) runtime_package=linux-arm64 ;; + *) runtime_package="" ;; + esac + if [ -n "$runtime_package" ]; then + runtime_stage="$work/runtime-stage" + mkdir -p "$runtime_stage/bin" + cp "$repo_root/sdk/npm/$runtime_package/package.json" "$runtime_stage/" + cp "$repo_root/sdk/npm/$runtime_package/README.md" "$runtime_stage/" + cp "$(command -v jcode)" "$runtime_stage/bin/jcode" + chmod +x "$runtime_stage/bin/jcode" + runtime_tarball="$(cd "$runtime_stage" && npm pack --silent)" + runtime_tarball="$runtime_stage/$runtime_tarball" + fi +fi + +if [ -n "$runtime_tarball" ]; then + npm install "$runtime_tarball" "$tarball" --no-audit --no-fund --silent +else + npm install "$tarball" --no-audit --no-fund --silent +fi echo "== ESM import ==" node --input-type=module -e ' @@ -37,6 +60,18 @@ if (API_VERSION_MAJOR !== 1) throw new Error("unexpected protocol version"); console.log("esm ok"); ' +if [ -n "$runtime_tarball" ]; then + echo "== bundled runtime resolution ==" + node --input-type=module -e ' + import { bundledJcodeBinary } from "@1jehuang/jcode-sdk"; + const binary = bundledJcodeBinary(); + if (!binary || !binary.includes("@1jehuang/jcode-linux-")) { + throw new Error(`platform runtime was not resolved: ${binary}`); + } + console.log(`runtime ok: ${binary}`); + ' +fi + echo "== CJS require ==" node --input-type=commonjs -e ' const sdk = require("@1jehuang/jcode-sdk"); @@ -123,7 +158,7 @@ if command -v jcode >/dev/null 2>&1; then import { JcodeClient } from "@1jehuang/jcode-sdk"; import fs from "node:fs"; -// No `binary` option: resolve `jcode` from PATH, exactly like a consumer. +// No `binary` option: use the platform npm package, exactly like a consumer. const client = await JcodeClient.launch({ workingDir: process.cwd() }); const home = client.instanceHome; diff --git a/sdk/npm/darwin-arm64/README.md b/sdk/npm/darwin-arm64/README.md new file mode 100644 index 0000000000..1d4108dacc --- /dev/null +++ b/sdk/npm/darwin-arm64/README.md @@ -0,0 +1,3 @@ +# @1jehuang/jcode-darwin-arm64 + +Platform runtime used by `@1jehuang/jcode-sdk`. Install the SDK rather than this package directly. diff --git a/sdk/npm/darwin-arm64/bin/.gitkeep b/sdk/npm/darwin-arm64/bin/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/sdk/npm/darwin-arm64/package.json b/sdk/npm/darwin-arm64/package.json new file mode 100644 index 0000000000..61338ceadd --- /dev/null +++ b/sdk/npm/darwin-arm64/package.json @@ -0,0 +1,27 @@ +{ + "name": "@1jehuang/jcode-darwin-arm64", + "version": "1.1.0", + "description": "Jcode runtime binary for darwin arm64", + "license": "MIT", + "os": [ + "darwin" + ], + "cpu": [ + "arm64" + ], + "files": [ + "bin", + "README.md" + ], + "exports": { + "./package.json": "./package.json" + }, + "repository": { + "type": "git", + "url": "git+https://github.com/1jehuang/jcode.git", + "directory": "sdk/npm/darwin-arm64" + }, + "publishConfig": { + "access": "public" + } +} diff --git a/sdk/npm/darwin-x64/README.md b/sdk/npm/darwin-x64/README.md new file mode 100644 index 0000000000..7c8983abe1 --- /dev/null +++ b/sdk/npm/darwin-x64/README.md @@ -0,0 +1,3 @@ +# @1jehuang/jcode-darwin-x64 + +Platform runtime used by `@1jehuang/jcode-sdk`. Install the SDK rather than this package directly. diff --git a/sdk/npm/darwin-x64/bin/.gitkeep b/sdk/npm/darwin-x64/bin/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/sdk/npm/darwin-x64/package.json b/sdk/npm/darwin-x64/package.json new file mode 100644 index 0000000000..68dc2edfb8 --- /dev/null +++ b/sdk/npm/darwin-x64/package.json @@ -0,0 +1,27 @@ +{ + "name": "@1jehuang/jcode-darwin-x64", + "version": "1.1.0", + "description": "Jcode runtime binary for darwin x64", + "license": "MIT", + "os": [ + "darwin" + ], + "cpu": [ + "x64" + ], + "files": [ + "bin", + "README.md" + ], + "exports": { + "./package.json": "./package.json" + }, + "repository": { + "type": "git", + "url": "git+https://github.com/1jehuang/jcode.git", + "directory": "sdk/npm/darwin-x64" + }, + "publishConfig": { + "access": "public" + } +} diff --git a/sdk/npm/linux-arm64/README.md b/sdk/npm/linux-arm64/README.md new file mode 100644 index 0000000000..00852cfcb1 --- /dev/null +++ b/sdk/npm/linux-arm64/README.md @@ -0,0 +1,3 @@ +# @1jehuang/jcode-linux-arm64 + +Platform runtime used by `@1jehuang/jcode-sdk`. Install the SDK rather than this package directly. diff --git a/sdk/npm/linux-arm64/bin/.gitkeep b/sdk/npm/linux-arm64/bin/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/sdk/npm/linux-arm64/package.json b/sdk/npm/linux-arm64/package.json new file mode 100644 index 0000000000..2ae5467292 --- /dev/null +++ b/sdk/npm/linux-arm64/package.json @@ -0,0 +1,27 @@ +{ + "name": "@1jehuang/jcode-linux-arm64", + "version": "1.1.0", + "description": "Jcode runtime binary for linux arm64", + "license": "MIT", + "os": [ + "linux" + ], + "cpu": [ + "arm64" + ], + "files": [ + "bin", + "README.md" + ], + "exports": { + "./package.json": "./package.json" + }, + "repository": { + "type": "git", + "url": "git+https://github.com/1jehuang/jcode.git", + "directory": "sdk/npm/linux-arm64" + }, + "publishConfig": { + "access": "public" + } +} diff --git a/sdk/npm/linux-x64/README.md b/sdk/npm/linux-x64/README.md new file mode 100644 index 0000000000..5108e3ca79 --- /dev/null +++ b/sdk/npm/linux-x64/README.md @@ -0,0 +1,3 @@ +# @1jehuang/jcode-linux-x64 + +Platform runtime used by `@1jehuang/jcode-sdk`. Install the SDK rather than this package directly. diff --git a/sdk/npm/linux-x64/bin/.gitkeep b/sdk/npm/linux-x64/bin/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/sdk/npm/linux-x64/package.json b/sdk/npm/linux-x64/package.json new file mode 100644 index 0000000000..55ae8deba7 --- /dev/null +++ b/sdk/npm/linux-x64/package.json @@ -0,0 +1,27 @@ +{ + "name": "@1jehuang/jcode-linux-x64", + "version": "1.1.0", + "description": "Jcode runtime binary for linux x64", + "license": "MIT", + "os": [ + "linux" + ], + "cpu": [ + "x64" + ], + "files": [ + "bin", + "README.md" + ], + "exports": { + "./package.json": "./package.json" + }, + "repository": { + "type": "git", + "url": "git+https://github.com/1jehuang/jcode.git", + "directory": "sdk/npm/linux-x64" + }, + "publishConfig": { + "access": "public" + } +} diff --git a/sdk/npm/win32-arm64/README.md b/sdk/npm/win32-arm64/README.md new file mode 100644 index 0000000000..9b5633eec6 --- /dev/null +++ b/sdk/npm/win32-arm64/README.md @@ -0,0 +1,3 @@ +# @1jehuang/jcode-win32-arm64 + +Platform runtime used by `@1jehuang/jcode-sdk`. Install the SDK rather than this package directly. diff --git a/sdk/npm/win32-arm64/bin/.gitkeep b/sdk/npm/win32-arm64/bin/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/sdk/npm/win32-arm64/package.json b/sdk/npm/win32-arm64/package.json new file mode 100644 index 0000000000..8565e1b2ba --- /dev/null +++ b/sdk/npm/win32-arm64/package.json @@ -0,0 +1,27 @@ +{ + "name": "@1jehuang/jcode-win32-arm64", + "version": "1.1.0", + "description": "Jcode runtime binary for win32 arm64", + "license": "MIT", + "os": [ + "win32" + ], + "cpu": [ + "arm64" + ], + "files": [ + "bin", + "README.md" + ], + "exports": { + "./package.json": "./package.json" + }, + "repository": { + "type": "git", + "url": "git+https://github.com/1jehuang/jcode.git", + "directory": "sdk/npm/win32-arm64" + }, + "publishConfig": { + "access": "public" + } +} diff --git a/sdk/npm/win32-x64/README.md b/sdk/npm/win32-x64/README.md new file mode 100644 index 0000000000..eb7d13de78 --- /dev/null +++ b/sdk/npm/win32-x64/README.md @@ -0,0 +1,3 @@ +# @1jehuang/jcode-win32-x64 + +Platform runtime used by `@1jehuang/jcode-sdk`. Install the SDK rather than this package directly. diff --git a/sdk/npm/win32-x64/bin/.gitkeep b/sdk/npm/win32-x64/bin/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/sdk/npm/win32-x64/package.json b/sdk/npm/win32-x64/package.json new file mode 100644 index 0000000000..2960d2301a --- /dev/null +++ b/sdk/npm/win32-x64/package.json @@ -0,0 +1,27 @@ +{ + "name": "@1jehuang/jcode-win32-x64", + "version": "1.1.0", + "description": "Jcode runtime binary for win32 x64", + "license": "MIT", + "os": [ + "win32" + ], + "cpu": [ + "x64" + ], + "files": [ + "bin", + "README.md" + ], + "exports": { + "./package.json": "./package.json" + }, + "repository": { + "type": "git", + "url": "git+https://github.com/1jehuang/jcode.git", + "directory": "sdk/npm/win32-x64" + }, + "publishConfig": { + "access": "public" + } +} diff --git a/sdk/typescript/README.md b/sdk/typescript/README.md index 35933e0dca..f5cb6bc4c2 100644 --- a/sdk/typescript/README.md +++ b/sdk/typescript/README.md @@ -26,7 +26,11 @@ npm run build ## Requirements -jcode must be installed, and Node 20 or newer. +Node 20 or newer. The SDK installs the correct jcode runtime for supported +macOS, Linux, and Windows architectures as an optional platform package, so a +separate jcode installation is not normally required. If optional dependencies +are disabled, `launch()` falls back to `jcode` on `PATH`; `binary` can also +select a specific executable. macOS and Linux are exercised end to end in CI. Windows builds and is wired up (the bridge listens on a named pipe rather than a Unix socket, and the SDK @@ -35,7 +39,8 @@ treat it as untested rather than unsupported and please report what breaks. `launch()` needs nothing else: it starts its own daemon and bridge. `connect()` needs a bridge already running, which the user starts once and leaves running. -The bridge ships in the released binary, so no Rust toolchain is needed: +The bridge ships in the runtime package, so no Rust toolchain is needed. To use +`connect()` with the user's global jcode, start its bridge: ```bash jcode api-bridge diff --git a/sdk/typescript/RELEASING.md b/sdk/typescript/RELEASING.md index 9f92e44aab..0b942533d3 100644 --- a/sdk/typescript/RELEASING.md +++ b/sdk/typescript/RELEASING.md @@ -25,8 +25,14 @@ bash ../../scripts/test_sdk_package.sh # the tarball as a consumer sees it npm publish # publishConfig already sets public access ``` +Use the **Publish TypeScript SDK** workflow and provide an existing jcode release +tag. It downloads that release's six runtime artifacts, publishes the matching +platform packages first, and then publishes the SDK. All seven package manifests +must have the same version. The main package uses exact optional dependency +versions so an SDK release can never silently pick up a different runtime. + `prepack` rebuilds `dist/` from a clean slate, so a stale build cannot be -published. `files` limits the tarball to `dist`, `README.md`, and `LICENSE`; +published. `files` limits the main tarball to `dist`, `README.md`, and `LICENSE`; confirm with `npm pack --dry-run`. ## Verifying a published release diff --git a/sdk/typescript/package-lock.json b/sdk/typescript/package-lock.json index 0bfb44a886..47ad513800 100644 --- a/sdk/typescript/package-lock.json +++ b/sdk/typescript/package-lock.json @@ -1,12 +1,12 @@ { "name": "@1jehuang/jcode-sdk", - "version": "1.0.1", + "version": "1.1.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@1jehuang/jcode-sdk", - "version": "1.0.1", + "version": "1.1.0", "license": "MIT", "dependencies": { "ajv": "^8.20.0" @@ -17,6 +17,14 @@ }, "engines": { "node": ">=20" + }, + "optionalDependencies": { + "@1jehuang/jcode-darwin-arm64": "1.1.0", + "@1jehuang/jcode-darwin-x64": "1.1.0", + "@1jehuang/jcode-linux-arm64": "1.1.0", + "@1jehuang/jcode-linux-x64": "1.1.0", + "@1jehuang/jcode-win32-arm64": "1.1.0", + "@1jehuang/jcode-win32-x64": "1.1.0" } }, "node_modules/@types/node": { @@ -102,6 +110,66 @@ "integrity": "sha512-iwDZqg0QAGrg9Rav5H4n0M64c3mkR59cJ6wQp+7C4nI0gsmExaedaYLNO44eT4AtBBwjbTiGPMlt2Md0T9H9JQ==", "dev": true, "license": "MIT" + }, + "node_modules/@1jehuang/jcode-darwin-arm64": { + "version": "1.1.0", + "optional": true, + "os": [ + "darwin" + ], + "cpu": [ + "arm64" + ] + }, + "node_modules/@1jehuang/jcode-darwin-x64": { + "version": "1.1.0", + "optional": true, + "os": [ + "darwin" + ], + "cpu": [ + "x64" + ] + }, + "node_modules/@1jehuang/jcode-linux-arm64": { + "version": "1.1.0", + "optional": true, + "os": [ + "linux" + ], + "cpu": [ + "arm64" + ] + }, + "node_modules/@1jehuang/jcode-linux-x64": { + "version": "1.1.0", + "optional": true, + "os": [ + "linux" + ], + "cpu": [ + "x64" + ] + }, + "node_modules/@1jehuang/jcode-win32-arm64": { + "version": "1.1.0", + "optional": true, + "os": [ + "win32" + ], + "cpu": [ + "arm64" + ] + }, + "node_modules/@1jehuang/jcode-win32-x64": { + "version": "1.1.0", + "optional": true, + "os": [ + "win32" + ], + "cpu": [ + "x64" + ] } } } diff --git a/sdk/typescript/package.json b/sdk/typescript/package.json index 1ae2449949..0bc2c11340 100644 --- a/sdk/typescript/package.json +++ b/sdk/typescript/package.json @@ -1,6 +1,6 @@ { "name": "@1jehuang/jcode-sdk", - "version": "1.0.1", + "version": "1.1.0", "description": "TypeScript SDK for the jcode harness API (protocol v1)", "license": "MIT", "type": "module", @@ -56,5 +56,13 @@ }, "dependencies": { "ajv": "^8.20.0" + }, + "optionalDependencies": { + "@1jehuang/jcode-darwin-arm64": "1.1.0", + "@1jehuang/jcode-darwin-x64": "1.1.0", + "@1jehuang/jcode-linux-arm64": "1.1.0", + "@1jehuang/jcode-linux-x64": "1.1.0", + "@1jehuang/jcode-win32-arm64": "1.1.0", + "@1jehuang/jcode-win32-x64": "1.1.0" } } diff --git a/sdk/typescript/src/binary.ts b/sdk/typescript/src/binary.ts new file mode 100644 index 0000000000..7e1f58c26e --- /dev/null +++ b/sdk/typescript/src/binary.ts @@ -0,0 +1,39 @@ +import { createRequire } from "node:module"; +import path from "node:path"; + +const require = createRequire(import.meta.url); + +const PLATFORM_PACKAGES: Record = { + "linux-x64": "@1jehuang/jcode-linux-x64", + "linux-arm64": "@1jehuang/jcode-linux-arm64", + "darwin-x64": "@1jehuang/jcode-darwin-x64", + "darwin-arm64": "@1jehuang/jcode-darwin-arm64", + "win32-x64": "@1jehuang/jcode-win32-x64", + "win32-arm64": "@1jehuang/jcode-win32-arm64", +}; + +/** The optional npm package containing the runtime for this machine. */ +export function platformBinaryPackage( + platform = process.platform, + arch = process.arch, +): string | undefined { + return PLATFORM_PACKAGES[`${platform}-${arch}`]; +} + +/** + * Resolve the jcode executable installed as an optional platform dependency. + * Returns undefined on unsupported platforms or when optional dependencies + * were deliberately omitted, allowing launch() to fall back to PATH. + */ +export function bundledJcodeBinary(): string | undefined { + const packageName = platformBinaryPackage(); + if (!packageName) return undefined; + try { + const manifest = require.resolve(`${packageName}/package.json`); + return path.join(path.dirname(manifest), "bin", process.platform === "win32" ? "jcode.exe" : "jcode"); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === "MODULE_NOT_FOUND") return undefined; + throw error; + } +} diff --git a/sdk/typescript/src/index.ts b/sdk/typescript/src/index.ts index 9e72673d6c..856d064813 100644 --- a/sdk/typescript/src/index.ts +++ b/sdk/typescript/src/index.ts @@ -22,6 +22,7 @@ export { userAppConfigDir, } from "./launch.js"; export type { LaunchOptions, LaunchedInstance } from "./launch.js"; +export { bundledJcodeBinary, platformBinaryPackage } from "./binary.js"; export { JcodeClient, unixSocketTransport } from "./client.js"; export type { ConnectOptions, diff --git a/sdk/typescript/src/launch.ts b/sdk/typescript/src/launch.ts index 1f7d8e37fa..27b24bcad6 100644 --- a/sdk/typescript/src/launch.ts +++ b/sdk/typescript/src/launch.ts @@ -17,6 +17,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { HarnessError } from "./errors.js"; +import { bundledJcodeBinary, platformBinaryPackage } from "./binary.js"; /** * Files inherited from the user's jcode home when logins are inherited. @@ -167,7 +168,7 @@ export interface LaunchOptions { * supply credentials yourself. */ inheritLogins?: boolean; - /** Path to the jcode binary. Defaults to `jcode` on PATH. */ + /** Path to the jcode binary. Defaults to the npm-bundled runtime, then `jcode` on PATH. */ binary?: string; /** Extra environment variables for the instance. */ env?: Record; @@ -479,6 +480,7 @@ function removeInstanceHome(home: string): void { * connections. */ export async function launchInstance(options: LaunchOptions = {}): Promise { + const binary = options.binary ?? bundledJcodeBinary() ?? "jcode"; const ephemeral = options.jcodeHome === undefined; const jcodeHome = options.jcodeHome ?? @@ -512,7 +514,7 @@ export async function launchInstance(options: LaunchOptions = {}): Promise { + const { platformBinaryPackage } = await import("../dist/index.js"); + assert.equal(platformBinaryPackage("linux", "x64"), "@1jehuang/jcode-linux-x64"); + assert.equal(platformBinaryPackage("darwin", "arm64"), "@1jehuang/jcode-darwin-arm64"); + assert.equal(platformBinaryPackage("win32", "x64"), "@1jehuang/jcode-win32-x64"); + assert.equal(platformBinaryPackage("freebsd", "x64"), undefined); +}); + /** * Cleanup must never follow a symlink out of the instance home. * From 62b1d3745c0c5b492442b65aa7ea9363170e36f5 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 15:52:13 -0700 Subject: [PATCH 07/19] feat: add fast macOS release path --- crates/jcode-tui/src/tui/app/commands.rs | 37 +++++++ crates/jcode-tui/src/tui/app/input_help.rs | 3 + .../src/tui/app/remote/key_handling.rs | 8 ++ .../src/tui/app/state_ui_input_helpers.rs | 4 + .../app/tests/commands_accounts_01/part_01.rs | 40 ++++++++ scripts/quick-release.sh | 99 ++++++++++++++++++- 6 files changed, 187 insertions(+), 4 deletions(-) diff --git a/crates/jcode-tui/src/tui/app/commands.rs b/crates/jcode-tui/src/tui/app/commands.rs index 2aa7bd1845..307c2681d7 100644 --- a/crates/jcode-tui/src/tui/app/commands.rs +++ b/crates/jcode-tui/src/tui/app/commands.rs @@ -1680,6 +1680,11 @@ pub(super) fn handle_session_command(app: &mut App, trimmed: &str) -> bool { return true; } + if trimmed == "/fast-macos-release" { + handle_fast_macos_release_command_local(app); + return true; + } + if trimmed == "/remote-release" { handle_remote_release_command_local(app); return true; @@ -2163,6 +2168,13 @@ pub(super) fn build_fast_release_prompt() -> String { ) } +pub(super) fn build_fast_macos_release_prompt() -> String { + build_release_prompt( + "Before editing Cargo.toml or the changelog for the version bump, run scripts/quick-release.sh --prepare-fast-macos v. It must cross-build and record the macOS arm64 binary with the future release identity while the release metadata is still unchanged.", + "Then run scripts/quick-release.sh --fast-macos-local v. It must validate and publish the prepared macOS arm64 asset and GitHub release immediately, while CI replaces it with the signoff artifact and adds macOS Intel, Linux, Windows, FreeBSD, signatures, and final checksums. If preparation is stale or the release-metadata commit contains code changes, stop instead of publishing a binary that differs from the tag.", + ) +} + pub(super) fn build_remote_release_prompt() -> String { build_release_prompt( "", @@ -2235,6 +2247,14 @@ pub(super) fn fast_release_launch_notice(interrupted: bool) -> String { } } +pub(super) fn fast_macos_release_launch_notice(interrupted: bool) -> String { + if interrupted { + "๐Ÿ‘‰ Interrupting and starting logical commits + push + fast macOS release...".to_string() + } else { + "๐Ÿš€ Starting logical commits + push + fast macOS release...".to_string() + } +} + pub(super) fn remote_release_launch_notice(interrupted: bool) -> String { if interrupted { "๐Ÿ‘‰ Interrupting and starting logical commits + push + remote release...".to_string() @@ -2288,6 +2308,23 @@ fn handle_fast_release_command_local(app: &mut App) { } } +fn handle_fast_macos_release_command_local(app: &mut App) { + let prompt = build_fast_macos_release_prompt(); + if app.is_processing { + super::commands_improve::interrupt_and_queue_synthetic_message( + app, + prompt, + "Interrupting for /fast-macos-release...", + fast_macos_release_launch_notice(true), + ); + } else { + app.push_display_message(DisplayMessage::system(fast_macos_release_launch_notice( + false, + ))); + super::commands_improve::start_synthetic_user_turn(app, prompt); + } +} + fn handle_remote_release_command_local(app: &mut App) { let prompt = build_remote_release_prompt(); if app.is_processing { diff --git a/crates/jcode-tui/src/tui/app/input_help.rs b/crates/jcode-tui/src/tui/app/input_help.rs index ad0ce99993..c1c1be59cb 100644 --- a/crates/jcode-tui/src/tui/app/input_help.rs +++ b/crates/jcode-tui/src/tui/app/input_help.rs @@ -70,6 +70,9 @@ impl App { "fast-release" | "cut-release" | "commit-push-release" => { "/fast-release\nSame as /commit-push, then publish the release as quickly as possible from the local Linux machine.\n\nThe agent picks the semver bump and first runs scripts/quick-release.sh --prepare-fast before changing Cargo.toml. This refreshes and records the warm target/selfdev Linux binary without invalidating the cache for a version change. The agent then makes one release-metadata commit containing Cargo.toml, Cargo.lock, and the changelog, pushes it, and runs scripts/quick-release.sh --fast-local. That command wraps the prepared binary with the release identity, publishes Linux and the GitHub release immediately, and lets CI replace it with the portable Linux build while adding every other platform and final signoff assets. /cut-release is a compatibility alias." } + "fast-macos-release" => { + "/fast-macos-release\nSame as /commit-push, but prepare and publish macOS arm64 as quickly as possible from the local Linux machine.\n\nThe agent first runs scripts/quick-release.sh --prepare-fast-macos before changing Cargo.toml. This cross-builds macOS arm64 with the future release identity and records its source commit and checksum. After the release-metadata commit, scripts/quick-release.sh --fast-macos-local validates and publishes that asset immediately. CI then replaces it with the signoff build and adds macOS Intel and every other platform. Requires osxcross." + } "remote-release" => { "/remote-release\nSame as /commit-push, then push the release tag without running any local build.\n\nThe agent picks the semver bump, updates Cargo.toml/Cargo.lock and the changelog, commits and pushes, then runs scripts/quick-release.sh --remote. GitHub Actions builds, signs, checksums, and publishes every platform; the release remains a draft until the remote gates pass." } diff --git a/crates/jcode-tui/src/tui/app/remote/key_handling.rs b/crates/jcode-tui/src/tui/app/remote/key_handling.rs index 36c1be956f..eace5e148b 100644 --- a/crates/jcode-tui/src/tui/app/remote/key_handling.rs +++ b/crates/jcode-tui/src/tui/app/remote/key_handling.rs @@ -1945,6 +1945,7 @@ async fn handle_remote_key_internal( || trimmed == "/commit-push" || trimmed == "/commit-and-push" || trimmed == "/fast-release" + || trimmed == "/fast-macos-release" || trimmed == "/remote-release" || trimmed == "/cut-release" || trimmed == "/commit-push-release" @@ -1957,11 +1958,14 @@ async fn handle_remote_key_internal( "/fast-release" | "/cut-release" | "/commit-push-release" ); let is_remote_release = trimmed == "/remote-release"; + let is_fast_macos_release = trimmed == "/fast-macos-release"; let is_push = trimmed != "/commit"; let prompt = if is_triage { app_mod::commands::build_triage_prompt( trimmed.strip_prefix("/triage").unwrap_or_default(), ) + } else if is_fast_macos_release { + app_mod::commands::build_fast_macos_release_prompt() } else if is_fast_release { app_mod::commands::build_fast_release_prompt() } else if is_remote_release { @@ -1974,6 +1978,8 @@ async fn handle_remote_key_internal( let launch_notice = |interrupted: bool| { if is_triage { app_mod::commands::triage_launch_notice(interrupted) + } else if is_fast_macos_release { + app_mod::commands::fast_macos_release_launch_notice(interrupted) } else if is_fast_release { app_mod::commands::fast_release_launch_notice(interrupted) } else if is_remote_release { @@ -1986,6 +1992,8 @@ async fn handle_remote_key_internal( }; let cmd_label = if is_triage { "/triage" + } else if is_fast_macos_release { + "/fast-macos-release" } else if is_fast_release { "/fast-release" } else if is_remote_release { diff --git a/crates/jcode-tui/src/tui/app/state_ui_input_helpers.rs b/crates/jcode-tui/src/tui/app/state_ui_input_helpers.rs index c6dfbc88b1..857b889eda 100644 --- a/crates/jcode-tui/src/tui/app/state_ui_input_helpers.rs +++ b/crates/jcode-tui/src/tui/app/state_ui_input_helpers.rs @@ -76,6 +76,10 @@ const REGISTERED_COMMANDS: &[RegisteredCommand] = &[ "/fast-release", "Publish Linux immediately from the warm selfdev cache; CI adds other platforms", ), + RegisteredCommand::public( + "/fast-macos-release", + "Publish a prepared macOS arm64 build immediately; CI adds other platforms", + ), RegisteredCommand::public("/remote", "Reach this session from another machine"), RegisteredCommand::public( "/remote-release", diff --git a/crates/jcode-tui/src/tui/app/tests/commands_accounts_01/part_01.rs b/crates/jcode-tui/src/tui/app/tests/commands_accounts_01/part_01.rs index 5082debb5c..a929248021 100644 --- a/crates/jcode-tui/src/tui/app/tests/commands_accounts_01/part_01.rs +++ b/crates/jcode-tui/src/tui/app/tests/commands_accounts_01/part_01.rs @@ -650,6 +650,46 @@ fn test_fast_release_prompt_uses_selfdev_cache() { assert!(prepare < bump); } +#[test] +fn test_fast_macos_release_command_uses_prepared_cross_build() { + let mut app = create_test_app(); + app.input = "/fast-macos-release".to_string(); + app.submit_input(); + + assert!(app.is_processing); + assert!(app.pending_turn); + let notice = app + .display_messages() + .last() + .expect("missing launch notice"); + assert!(notice.content.contains("fast macOS release")); + + let prompt = super::commands::build_fast_macos_release_prompt(); + assert!(prompt.contains("quick-release.sh --prepare-fast-macos")); + assert!(prompt.contains("quick-release.sh --fast-macos-local")); + assert!(prompt.contains("macOS arm64")); + let prepare = prompt.find("--prepare-fast-macos").unwrap(); + let bump = prompt.find("Bump the version").unwrap(); + assert!(prepare < bump); +} + +#[test] +fn test_help_topic_shows_fast_macos_release_details() { + let mut app = create_test_app(); + app.input = "/help fast-macos-release".to_string(); + app.submit_input(); + + let msg = app + .display_messages() + .last() + .expect("missing help response"); + assert_eq!(msg.role, "system"); + assert!(msg.content.contains("/fast-macos-release")); + assert!(msg.content.contains("--prepare-fast-macos")); + assert!(msg.content.contains("--fast-macos-local")); + assert!(msg.content.contains("osxcross")); +} + #[test] fn test_remote_release_command_uses_tag_only_ci_path() { let mut app = create_test_app(); diff --git a/scripts/quick-release.sh b/scripts/quick-release.sh index 3de067405b..5dcc76b36b 100755 --- a/scripts/quick-release.sh +++ b/scripts/quick-release.sh @@ -5,12 +5,16 @@ set -euo pipefail # - --prepare-fast: refresh target/selfdev before the release metadata commit. # - --fast-local: package that prepared binary, publish Linux immediately, then # let CI replace it with portable/signoff assets and add every other platform. +# - --prepare-fast-macos/--fast-macos-local: do the same for macOS arm64 using +# the local osxcross cache. # - --remote: push the tag immediately and let CI gate publication. # - default: build Linux + macOS locally and stage them on the CI-owned draft. # # Usage: # scripts/quick-release.sh --prepare-fast v0.5.5 # warm selfdev before bump # scripts/quick-release.sh --fast-local v0.5.5 # package it, public now +# scripts/quick-release.sh --prepare-fast-macos v0.5.5 +# scripts/quick-release.sh --fast-macos-local v0.5.5 # scripts/quick-release.sh --remote v0.5.5 # tag now, CI-gated publication # scripts/quick-release.sh v0.5.5 # local Linux + macOS draft # scripts/quick-release.sh --dry-run v0.5.5 # standard local build only @@ -28,6 +32,14 @@ while [[ "${1:-}" == --* ]]; do [[ "$MODE" == "standard" ]] || { echo "Error: release modes cannot be combined." >&2; exit 1; } MODE="prepare-fast" ;; + --fast-macos|--fast-macos-local) + [[ "$MODE" == "standard" ]] || { echo "Error: release modes cannot be combined." >&2; exit 1; } + MODE="fast-macos-local" + ;; + --prepare-fast-macos) + [[ "$MODE" == "standard" ]] || { echo "Error: release modes cannot be combined." >&2; exit 1; } + MODE="prepare-fast-macos" + ;; --remote|--ci-only) [[ "$MODE" == "standard" ]] || { echo "Error: release modes cannot be combined." >&2; exit 1; } MODE="remote" @@ -38,7 +50,7 @@ while [[ "${1:-}" == --* ]]; do ;; *) echo "Error: Unknown option: $1" >&2 - echo "Usage: scripts/quick-release.sh [--prepare-fast | --fast-local | --remote | --dry-run] [title]" >&2 + echo "Usage: scripts/quick-release.sh [--prepare-fast | --fast-local | --prepare-fast-macos | --fast-macos-local | --remote | --dry-run] [title]" >&2 exit 1 ;; esac @@ -50,7 +62,7 @@ if $DRY_RUN && [[ "$MODE" == "remote" ]]; then exit 1 fi -VERSION="${1:?Usage: scripts/quick-release.sh [--prepare-fast | --fast-local | --remote | --dry-run] [title]}" +VERSION="${1:?Usage: scripts/quick-release.sh [--prepare-fast | --fast-local | --prepare-fast-macos | --fast-macos-local | --remote | --dry-run] [title]}" TITLE="${2:-$VERSION}" VERSION_NUM="${VERSION#v}" @@ -71,6 +83,13 @@ case "$MODE" in required_commands+=(file strip tar gzip sha256sum) $DRY_RUN || required_commands+=(gh) ;; + prepare-fast-macos) + required_commands+=(cargo file sha256sum) + ;; + fast-macos-local) + required_commands+=(file tar gzip sha256sum) + $DRY_RUN || required_commands+=(gh) + ;; standard) required_commands+=(cargo docker file) $DRY_RUN || required_commands+=(gh) @@ -80,7 +99,7 @@ for cmd in "${required_commands[@]}"; do command -v "$cmd" &>/dev/null || { echo "Error: $cmd not found."; exit 1; } done -if [[ "$MODE" == "standard" ]]; then +if [[ "$MODE" == "standard" || "$MODE" == "prepare-fast-macos" ]]; then [[ -f "$HOME/.cargo/env" ]] && source "$HOME/.cargo/env" export PATH="$HOME/.osxcross/bin:$PATH" if ! command -v aarch64-apple-darwin23.5-clang &>/dev/null; then @@ -92,7 +111,7 @@ fi working_tree_changes="$(git status --porcelain)" if [[ -n "$working_tree_changes" ]]; then case "$MODE" in - fast-local|prepare-fast) + fast-local|prepare-fast|fast-macos-local|prepare-fast-macos) echo "Error: --$MODE requires a clean working tree so the binary matches committed source." >&2 printf '%s\n' "$working_tree_changes" >&2 exit 1 @@ -115,6 +134,13 @@ if [[ "$MODE" == "fast-local" || "$MODE" == "prepare-fast" ]]; then fi fi +if [[ "$MODE" == "fast-macos-local" || "$MODE" == "prepare-fast-macos" ]]; then + if [[ "$(uname -s)" != "Linux" || "$(uname -m)" != "x86_64" ]]; then + echo "Error: fast macOS release currently uses osxcross and must run on Linux x86_64." >&2 + exit 1 + fi +fi + echo "=== Quick Release: $VERSION ($MODE) ===" echo "" @@ -198,6 +224,27 @@ if [[ "$MODE" == "prepare-fast" ]]; then exit 0 fi +if [[ "$MODE" == "prepare-fast-macos" ]]; then + echo "โ–ธ Refreshing the macOS arm64 build before the version bump..." + JCODE_RELEASE_BUILD=1 JCODE_BUILD_SEMVER="$VERSION_NUM" \ + CARGO_INCREMENTAL=0 CARGO_BUILD_JOBS="${CARGO_BUILD_JOBS:-1}" \ + cargo build --release --target aarch64-apple-darwin --bin jcode + source_bin="target/aarch64-apple-darwin/release/jcode" + [[ -x "$source_bin" ]] || { echo "Error: macOS binary not found: $source_bin" >&2; exit 1; } + file "$source_bin" | grep -q 'Mach-O 64-bit' || { echo "Error: bad macOS binary" >&2; exit 1; } + prepared_marker="target/aarch64-apple-darwin/release/fast-macos-release-prepared" + { + printf 'version=%s\n' "$VERSION_NUM" + printf 'commit=%s\n' "$(git rev-parse HEAD)" + printf 'binary_sha256=%s\n' "$(sha256sum "$source_bin" | cut -d' ' -f1)" + } > "$prepared_marker" + echo "" + echo "=== Fast macOS release prepared in $(elapsed)s ===" + echo " โœ… macOS arm64 binary recorded for $VERSION at $(git rev-parse --short HEAD)" + echo " Next: commit only Cargo.toml, Cargo.lock, and changelog release metadata, then run --fast-macos-local." + exit 0 +fi + if [[ "$MODE" == "remote" ]]; then tag_and_push echo "" @@ -296,6 +343,50 @@ WRAPPER exit 0 fi +if [[ "$MODE" == "fast-macos-local" ]]; then + echo "โ–ธ Validating the prepared macOS arm64 build..." + source_bin="target/aarch64-apple-darwin/release/jcode" + prepared_marker="target/aarch64-apple-darwin/release/fast-macos-release-prepared" + [[ -x "$source_bin" ]] || { echo "Error: macOS binary not found: $source_bin" >&2; exit 1; } + [[ -f "$prepared_marker" ]] || { + echo "Error: fast macOS release was not prepared. Run scripts/quick-release.sh --prepare-fast-macos $VERSION before the release metadata commit." >&2 + exit 1 + } + prepared_version="$(sed -n 's/^version=//p' "$prepared_marker")" + prepared_commit="$(sed -n 's/^commit=//p' "$prepared_marker")" + prepared_sha256="$(sed -n 's/^binary_sha256=//p' "$prepared_marker")" + [[ "$prepared_version" == "$VERSION_NUM" ]] || { echo "Error: prepared version $prepared_version does not match $VERSION_NUM." >&2; exit 1; } + [[ "$prepared_commit" == "$(git rev-parse HEAD^)" ]] || { echo "Error: prepared macOS binary commit is not the parent of the release commit." >&2; exit 1; } + [[ "$prepared_sha256" == "$(sha256sum "$source_bin" | cut -d' ' -f1)" ]] || { echo "Error: prepared macOS binary changed after preparation." >&2; exit 1; } + unexpected_release_files="$(git diff-tree --no-commit-id --name-only -r HEAD | grep -Ev '^(Cargo\.toml|Cargo\.lock|changelog/)' || true)" + [[ -z "$unexpected_release_files" ]] || { + echo "Error: the release metadata commit contains code or unsupported files:" >&2 + printf '%s\n' "$unexpected_release_files" >&2 + exit 1 + } + cp "$source_bin" "$DIST/jcode-macos-aarch64" + chmod +x "$DIST/jcode-macos-aarch64" + file "$DIST/jcode-macos-aarch64" | grep -q 'Mach-O 64-bit' || { echo "Error: bad macOS binary" >&2; exit 1; } + (cd "$DIST" && tar czf jcode-macos-aarch64.tar.gz jcode-macos-aarch64) + (cd "$DIST" && sha256sum jcode-macos-aarch64.tar.gz > SHA256SUMS) + + if $DRY_RUN; then + echo "Fast macOS dry run complete in $(elapsed)s. Artifacts in: $DIST" + trap - EXIT + exit 0 + fi + tag_and_push + echo "โ–ธ Publishing immediate macOS arm64 release..." + ensure_release_draft + gh release upload "$VERSION" "$DIST/jcode-macos-aarch64.tar.gz" "$DIST/SHA256SUMS" --clobber + gh release edit "$VERSION" --draft=false --latest + echo "" + echo "=== Fast macOS release published in $(elapsed)s ===" + echo " โœ… macOS arm64: public now from the prepared osxcross build" + echo " โณ CI: replacing it with the signoff build and adding Linux, macOS Intel, Windows, FreeBSD, signatures, and final checksums" + exit 0 +fi + # Standard local distribution build: Linux + macOS in parallel. echo "โ–ธ Building Linux x86_64 + macOS aarch64 in parallel..." ( From 6567e073e4c7fb4432fc9a479cdb0797599e4353 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 15:55:43 -0700 Subject: [PATCH 08/19] fix(tui): render latex in white --- crates/jcode-tui-markdown/src/lib.rs | 6 ++---- .../jcode-tui-markdown/src/markdown_latex_image.rs | 3 ++- .../src/markdown_tests/cases/latex_streaming.rs | 12 +++--------- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/crates/jcode-tui-markdown/src/lib.rs b/crates/jcode-tui-markdown/src/lib.rs index e84b1fde98..77256022f8 100644 --- a/crates/jcode-tui-markdown/src/lib.rs +++ b/crates/jcode-tui-markdown/src/lib.rs @@ -481,10 +481,8 @@ fn rendered_rule_width(max_width: Option) -> usize { // Colors matching ui.rs palette use jcode_tui_workspace::color_support::rgb; -const MATH_FOREGROUND: (u8, u8, u8) = (100, 160, 255); -/// Inline math sits inside prose, so it matches the body text brightness with -/// only a gentle blue tint instead of the saturated display-math blue. -const MATH_INLINE_FOREGROUND: (u8, u8, u8) = (185, 200, 225); +const MATH_FOREGROUND: (u8, u8, u8) = (255, 255, 255); +const MATH_INLINE_FOREGROUND: (u8, u8, u8) = (255, 255, 255); fn code_bg() -> Color { rgb(45, 45, 45) diff --git a/crates/jcode-tui-markdown/src/markdown_latex_image.rs b/crates/jcode-tui-markdown/src/markdown_latex_image.rs index 8b33857e23..ce116e7c75 100644 --- a/crates/jcode-tui-markdown/src/markdown_latex_image.rs +++ b/crates/jcode-tui-markdown/src/markdown_latex_image.rs @@ -10,7 +10,8 @@ use std::sync::{LazyLock, Mutex, OnceLock, mpsc}; use std::time::Duration; use wait_timeout::ChildExt; -const RENDERER_VERSION: u8 = 4; +// Bump whenever rendering output changes so stale images are not reused. +const RENDERER_VERSION: u8 = 5; const MAX_SOURCE_BYTES: usize = 32 * 1024; const COMMAND_TIMEOUT: Duration = Duration::from_secs(8); const FOREGROUND: (u8, u8, u8) = super::MATH_FOREGROUND; diff --git a/crates/jcode-tui-markdown/src/markdown_tests/cases/latex_streaming.rs b/crates/jcode-tui-markdown/src/markdown_tests/cases/latex_streaming.rs index af74e47923..4755188c5a 100644 --- a/crates/jcode-tui-markdown/src/markdown_tests/cases/latex_streaming.rs +++ b/crates/jcode-tui-markdown/src/markdown_tests/cases/latex_streaming.rs @@ -10,15 +10,9 @@ fn exact_multiline_latex_response() -> &'static str { } #[test] -fn latex_foreground_is_saturated_blue_and_styles_inline_math() { - assert_eq!(MATH_FOREGROUND, (100, 160, 255)); - assert!(MATH_FOREGROUND.2 > MATH_FOREGROUND.1); - assert!(MATH_FOREGROUND.1 > MATH_FOREGROUND.0); - // Inline math blends with prose: near body-text brightness with a light - // blue tint, not the saturated display-math blue. - assert_eq!(MATH_INLINE_FOREGROUND, (185, 200, 225)); - assert!(MATH_INLINE_FOREGROUND.2 > MATH_INLINE_FOREGROUND.1); - assert!(MATH_INLINE_FOREGROUND.1 > MATH_INLINE_FOREGROUND.0); +fn latex_foreground_is_white_and_styles_inline_math() { + assert_eq!(MATH_FOREGROUND, (255, 255, 255)); + assert_eq!(MATH_INLINE_FOREGROUND, (255, 255, 255)); let lines = with_streaming_render_context(|| render_markdown("Inline $x^2$ math.")); let math_spans: Vec<_> = lines From d06b769c5fbae30b6e5e11991295e854a36abda8 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:03:40 -0700 Subject: [PATCH 09/19] fix(tui): normalize dvipng math colors --- .../src/markdown_latex_image.rs | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/crates/jcode-tui-markdown/src/markdown_latex_image.rs b/crates/jcode-tui-markdown/src/markdown_latex_image.rs index ce116e7c75..979c0cf9c3 100644 --- a/crates/jcode-tui-markdown/src/markdown_latex_image.rs +++ b/crates/jcode-tui-markdown/src/markdown_latex_image.rs @@ -11,7 +11,7 @@ use std::time::Duration; use wait_timeout::ChildExt; // Bump whenever rendering output changes so stale images are not reused. -const RENDERER_VERSION: u8 = 5; +const RENDERER_VERSION: u8 = 6; const MAX_SOURCE_BYTES: usize = 32 * 1024; const COMMAND_TIMEOUT: Duration = Duration::from_secs(8); const FOREGROUND: (u8, u8, u8) = super::MATH_FOREGROUND; @@ -546,7 +546,7 @@ fn render_artifact_in( .map_err(|e| format!("write LaTeX source: {e}"))?; let dpi_arg = dpi.to_string(); - let foreground_arg = format!("rgb {} {} {}", FOREGROUND.0, FOREGROUND.1, FOREGROUND.2); + let foreground_arg = dvipng_rgb_arg(FOREGROUND); let dvi_result = run_command( &toolchain.latex, [ @@ -596,6 +596,17 @@ fn render_artifact_in( load_artifact(&cache_path) } +fn dvipng_rgb_arg((red, green, blue): (u8, u8, u8)) -> String { + // dvipng uses the dvips color syntax, whose RGB components are in 0..=1. + // Passing byte values such as `255 255 255` wraps to almost-black output. + format!( + "rgb {:.6} {:.6} {:.6}", + f32::from(red) / 255.0, + f32::from(green) / 255.0, + f32::from(blue) / 255.0 + ) +} + fn render_with_pdf_toolchain( toolchain: &Toolchain, working_dir: &Path, @@ -818,6 +829,18 @@ mod tests { assert_ne!(cache_key("x", false, 240), cache_key("x", false, 312)); } + #[test] + fn dvipng_foreground_uses_normalized_rgb_components() { + assert_eq!( + dvipng_rgb_arg((255, 255, 255)), + "rgb 1.000000 1.000000 1.000000" + ); + assert_eq!( + dvipng_rgb_arg((0, 128, 255)), + "rgb 0.000000 0.501961 1.000000" + ); + } + #[cfg(feature = "mermaid-renderer")] #[test] fn image_placeholder_extracts_math_copy_target_with_source_delimiters() { From 68300b8a83cee115c15f560c6cf46f192a62b3e4 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:41:57 -0700 Subject: [PATCH 10/19] feat(schema): replace per-provider deny-lists with a dialect system 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. --- Cargo.toml | 1 + crates/jcode-app-core/Cargo.toml | 1 + crates/jcode-app-core/src/tool/tests.rs | 89 ++++ .../Cargo.toml | 1 + .../src/antigravity_tests.rs | 29 +- .../src/lib.rs | 114 +++- crates/jcode-provider-antigravity/Cargo.toml | 1 + crates/jcode-provider-antigravity/src/lib.rs | 68 +-- .../src/gemini_tests.rs | 6 +- crates/jcode-provider-gemini/Cargo.toml | 1 + crates/jcode-provider-gemini/src/lib.rs | 77 +-- crates/jcode-schema-dialect/Cargo.toml | 17 + .../jcode-schema-dialect/src/conformance.rs | 284 ++++++++++ crates/jcode-schema-dialect/src/dialect.rs | 432 +++++++++++++++ crates/jcode-schema-dialect/src/keyword.rs | 131 +++++ crates/jcode-schema-dialect/src/lib.rs | 503 ++++++++++++++++++ crates/jcode-schema-dialect/src/quirks.rs | 223 ++++++++ crates/jcode-schema-dialect/src/registry.rs | 252 +++++++++ crates/jcode-schema-dialect/src/rejection.rs | 218 ++++++++ 19 files changed, 2312 insertions(+), 136 deletions(-) create mode 100644 crates/jcode-schema-dialect/Cargo.toml create mode 100644 crates/jcode-schema-dialect/src/conformance.rs create mode 100644 crates/jcode-schema-dialect/src/dialect.rs create mode 100644 crates/jcode-schema-dialect/src/keyword.rs create mode 100644 crates/jcode-schema-dialect/src/lib.rs create mode 100644 crates/jcode-schema-dialect/src/quirks.rs create mode 100644 crates/jcode-schema-dialect/src/registry.rs create mode 100644 crates/jcode-schema-dialect/src/rejection.rs diff --git a/Cargo.toml b/Cargo.toml index d4b4025aff..2b1053b015 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,6 +49,7 @@ members = [ "crates/jcode-provider-metadata", "crates/jcode-provider-env", "crates/jcode-provider-core", + "crates/jcode-schema-dialect", "crates/jcode-provider-bedrock", "crates/jcode-provider-anthropic", "crates/jcode-provider-antigravity", diff --git a/crates/jcode-app-core/Cargo.toml b/crates/jcode-app-core/Cargo.toml index 22d0321dbf..d0bdf12b7d 100644 --- a/crates/jcode-app-core/Cargo.toml +++ b/crates/jcode-app-core/Cargo.toml @@ -61,6 +61,7 @@ jcode-agent-runtime = { path = "../jcode-agent-runtime" } jcode-ambient-types = { path = "../jcode-ambient-types" } jcode-notify-email = { path = "../jcode-notify-email" } jcode-provider-core = { path = "../jcode-provider-core" } +jcode-schema-dialect = { path = "../jcode-schema-dialect" } # NOTE: jcode-app-core does NOT depend on any jcode-tui-* crate. They were # unused dead dependency edges here (the TUI declares them itself). Removing # them stops a jcode-tui-* edit from cascading a recompile through app-core. diff --git a/crates/jcode-app-core/src/tool/tests.rs b/crates/jcode-app-core/src/tool/tests.rs index 6605e69caf..0d17cfd37a 100644 --- a/crates/jcode-app-core/src/tool/tests.rs +++ b/crates/jcode-app-core/src/tool/tests.rs @@ -1444,3 +1444,92 @@ async fn test_single_output_ceiling_is_absolute_not_only_proportional() { ); } } + +/// Every built-in tool, normalized for every provider dialect, must be +/// sendable. +/// +/// This is the guard the recurring schema-outage class never had. #446, #495, +/// #543, #655, #687, #713 and #754 were each discovered by a user whose +/// provider had gone down, then fixed by appending one keyword to one +/// provider's deny-list. Nothing checked the *other* providers for the same +/// construct, which is exactly how #754 hit Gemini through Antigravity months +/// after the same class was fixed for OpenAI. +/// +/// Running the real registry through every registered dialect turns "some +/// provider is about to break" into a failing test on the commit that +/// introduces it. +#[tokio::test] +async fn tool_schemas_are_sendable_to_every_provider_dialect() { + let provider: Arc = Arc::new(MockProvider); + let registry = Registry::new(provider).await; + let defs = registry.definitions(None).await; + assert!(!defs.is_empty(), "the sweep must not pass vacuously"); + + let mut failures = Vec::new(); + for spec in jcode_schema_dialect::registry::ALL { + for def in &defs { + let normalized = jcode_schema_dialect::dialect::apply(&def.input_schema, spec); + for error in + jcode_schema_dialect::must_not_contain_unsupported_constructs(&normalized, spec) + { + failures.push(format!("[{}] tool `{}` {error}", spec.id, def.name)); + } + // Over-stripping is the hazard an allow-list introduces: a dialect + // that forgot to list `description` would produce requests that + // succeed while silently deleting every tool's prompt text. + for error in jcode_schema_dialect::must_preserve_meaning(&def.input_schema, &normalized) + { + failures.push(format!( + "[{}] tool `{}` lost meaning: {error}", + spec.id, def.name + )); + } + } + } + + assert!( + failures.is_empty(), + "tool schemas are not sendable to every provider:\n{}", + failures.join("\n") + ); +} + +/// The sweep above must fail when a tool really does carry a construct a +/// provider rejects, otherwise it is decorative. Feeds the exact +/// `@playwright/mcp` schema from #754 through the same checker to prove the +/// detection works end to end. +#[test] +fn the_dialect_sweep_catches_the_issue_754_schema() { + let hostile = serde_json::json!({ + "type": "object", + "properties": { + "data": { + "type": "object", + "additionalProperties": { "type": "string" }, + "propertyNames": { "type": "string" } + } + } + }); + + let unnormalized = jcode_schema_dialect::must_not_contain_unsupported_constructs( + &hostile, + &jcode_schema_dialect::registry::GEMINI, + ); + assert!( + unnormalized.iter().any(|e| e.message.contains("propertyNames")), + "the checker must flag the raw schema, got {unnormalized:?}" + ); + + let normalized = jcode_schema_dialect::dialect::apply( + &hostile, + &jcode_schema_dialect::registry::GEMINI, + ); + assert!( + jcode_schema_dialect::must_not_contain_unsupported_constructs( + &normalized, + &jcode_schema_dialect::registry::GEMINI, + ) + .is_empty(), + "and must pass once normalized" + ); +} diff --git a/crates/jcode-provider-antigravity-runtime/Cargo.toml b/crates/jcode-provider-antigravity-runtime/Cargo.toml index 551fc69137..1f4f4ea7e5 100644 --- a/crates/jcode-provider-antigravity-runtime/Cargo.toml +++ b/crates/jcode-provider-antigravity-runtime/Cargo.toml @@ -21,6 +21,7 @@ jcode-message-types = { path = "../jcode-message-types" } jcode-provider-antigravity = { path = "../jcode-provider-antigravity" } jcode-provider-core = { path = "../jcode-provider-core" } jcode-provider-gemini = { path = "../jcode-provider-gemini" } +jcode-schema-dialect = { path = "../jcode-schema-dialect" } reqwest = { version = "0.12", default-features = false, features = ["json", "stream", "charset", "http2", "system-proxy", "rustls-tls", "rustls-tls-native-roots"] } serde_json = "1" tokio = { version = "1", features = ["sync", "time", "rt"] } diff --git a/crates/jcode-provider-antigravity-runtime/src/antigravity_tests.rs b/crates/jcode-provider-antigravity-runtime/src/antigravity_tests.rs index baec21a2f5..48bb47d5f3 100644 --- a/crates/jcode-provider-antigravity-runtime/src/antigravity_tests.rs +++ b/crates/jcode-provider-antigravity-runtime/src/antigravity_tests.rs @@ -477,9 +477,11 @@ fn strip_numeric_schema_bounds_drops_array_and_string_and_object_bounds() { } #[test] -fn antigravity_compatible_schema_only_strips_property_names_for_gemini() { - // Gemini keeps combiners and bounds, but generateContent rejects the - // `propertyNames` keyword even when it is nested. +fn antigravity_gemini_uses_the_same_dialect_as_the_native_gemini_provider() { + // Antigravity's Gemini route reaches the same `generateContent` validator + // as the native Gemini provider, so it must apply the identical dialect. + // Historically it applied a laxer one and inherited every Gemini schema + // outage a release late (#754), which is what the shared dialect prevents. let schema = serde_json::json!({ "type": "object", "properties": { @@ -499,7 +501,23 @@ fn antigravity_compatible_schema_only_strips_property_names_for_gemini() { }); let out = antigravity_compatible_schema(&schema, "gemini-3-flash"); + + // Byte-identical to what the native Gemini provider would send. + assert_eq!( + out, + jcode_provider_gemini::gemini_compatible_schema(&schema), + "the two Gemini routes must not drift apart" + ); + + // Both keywords Gemini rejects are gone, at depth. assert!(out["properties"]["data"].get("propertyNames").is_none()); + assert!( + out["properties"]["data"] + .get("additionalProperties") + .is_none() + ); + + // Everything load-bearing survives: combiner branches, types, bounds. assert_eq!( out["properties"]["status_filter"], schema["properties"]["status_filter"] @@ -508,10 +526,7 @@ fn antigravity_compatible_schema_only_strips_property_names_for_gemini() { out["properties"]["tool_calls"], schema["properties"]["tool_calls"] ); - assert_eq!( - out["properties"]["data"]["additionalProperties"], - serde_json::json!({ "type": "string" }) - ); + assert_eq!(out["properties"]["data"]["type"], "object"); } #[test] diff --git a/crates/jcode-provider-antigravity-runtime/src/lib.rs b/crates/jcode-provider-antigravity-runtime/src/lib.rs index e7a28877f3..7f27d7c117 100644 --- a/crates/jcode-provider-antigravity-runtime/src/lib.rs +++ b/crates/jcode-provider-antigravity-runtime/src/lib.rs @@ -231,6 +231,60 @@ impl AntigravityProvider { jcode_base::provider::antigravity::fetch_catalog_snapshot(&self.client).await } + /// Recover from a tool-schema rejection by learning what the backend + /// refused and re-sending the turn without it. + /// + /// Because jcode advertises every tool on every request, one construct the + /// backend dislikes 400s the whole session rather than one tool, and the + /// historical fix has been to ship a new deny-list entry (#754, #687, #543, + /// #446). `jcode-schema-dialect` instead parses the construct out of the + /// error, persists it, and normalization strips it from then on, so the + /// user's next turn works instead of their next upgrade. + /// + /// Returns `None` when the error is not a recoverable schema rejection, so + /// the caller falls through to its other error handling. The quirk store + /// only reports a construct as newly-learned once, which is what bounds + /// this to a single retry per distinct construct. + #[expect( + clippy::too_many_arguments, + reason = "mirrors generate_content's explicit per-request settings so the retry re-sends an identical turn" + )] + async fn retry_after_schema_rejection( + &self, + error: &str, + model: &str, + messages: &[Message], + tools: &[ToolDefinition], + system: &str, + resume_session_id: Option<&str>, + signature_policy: jcode_provider_gemini::SignaturePolicy, + ) -> Option> { + let resolved = self.resolve_model_for_request(model); + let dialect = jcode_provider_antigravity::antigravity_dialect(&resolved); + match jcode_schema_dialect::recover_from_error(error, dialect) { + jcode_schema_dialect::RecoveryAction::NotSchemaRelated => None, + jcode_schema_dialect::RecoveryAction::Unrecoverable { hint } => { + jcode_base::logging::warn(&format!("Antigravity tool-schema rejection: {hint}")); + None + } + jcode_schema_dialect::RecoveryAction::RetryWithoutConstruct { description } => { + jcode_base::logging::warn(&format!("Antigravity {description}")); + Some( + self.generate_content( + model, + messages, + tools, + system, + resume_session_id, + false, + signature_policy, + ) + .await, + ) + } + } + } + #[expect( clippy::too_many_arguments, reason = "the Code Assist call threads explicit per-request settings, including the signature policy, without hidden state" @@ -449,32 +503,58 @@ impl Provider for AntigravityProvider { { Ok(response) => response, Err(err) => { - if !jcode_provider_gemini::is_missing_thought_signature_error(&err.to_string()) - { - let _ = tx.send(Err(err)).await; - return; - } - jcode_base::logging::warn( - "Antigravity rejected unsigned function calls; retrying with tool calls downgraded to text", - ); - signature_policy = - jcode_provider_gemini::SignaturePolicy::DowngradeToolCallsToText; - match provider - .generate_content( + // A tool schema the backend rejects 400s every single turn, + // so the provider is unusable until jcode ships a new + // deny-list entry. Instead, learn the rejected construct + // from the error, persist it, and retry the same turn + // without it. See `jcode-schema-dialect`. + if let Some(retried) = provider + .retry_after_schema_rejection( + &err.to_string(), &model, &messages, &tools, &system, resume_session_id.as_deref(), - false, signature_policy, ) .await { - Ok(response) => response, - Err(retry_err) => { - let _ = tx.send(Err(retry_err)).await; - return; + match retried { + Ok(response) => response, + Err(retry_err) => { + let _ = tx.send(Err(retry_err)).await; + return; + } + } + } else if !jcode_provider_gemini::is_missing_thought_signature_error( + &err.to_string(), + ) { + let _ = tx.send(Err(err)).await; + return; + } else { + jcode_base::logging::warn( + "Antigravity rejected unsigned function calls; retrying with tool calls downgraded to text", + ); + signature_policy = + jcode_provider_gemini::SignaturePolicy::DowngradeToolCallsToText; + match provider + .generate_content( + &model, + &messages, + &tools, + &system, + resume_session_id.as_deref(), + false, + signature_policy, + ) + .await + { + Ok(response) => response, + Err(retry_err) => { + let _ = tx.send(Err(retry_err)).await; + return; + } } } } diff --git a/crates/jcode-provider-antigravity/Cargo.toml b/crates/jcode-provider-antigravity/Cargo.toml index 734d445736..fbfd3a78d6 100644 --- a/crates/jcode-provider-antigravity/Cargo.toml +++ b/crates/jcode-provider-antigravity/Cargo.toml @@ -6,5 +6,6 @@ edition = "2024" [dependencies] chrono = { version = "0.4", features = ["serde"] } jcode-provider-gemini = { path = "../jcode-provider-gemini" } +jcode-schema-dialect = { path = "../jcode-schema-dialect" } serde = { version = "1", features = ["derive"] } serde_json = "1" diff --git a/crates/jcode-provider-antigravity/src/lib.rs b/crates/jcode-provider-antigravity/src/lib.rs index d5d05a5bec..9b8f883cfd 100644 --- a/crates/jcode-provider-antigravity/src/lib.rs +++ b/crates/jcode-provider-antigravity/src/lib.rs @@ -439,63 +439,39 @@ pub fn remap_unsupported_model(model: &str) -> &str { } /// Whether a resolved Antigravity model id targets a Gemini model. -/// -/// Gemini is the backend's native path and accepts every JSON Schema construct -/// jcode emits, so no schema rewriting is needed for these models. pub fn model_is_gemini(model: &str) -> bool { model.trim().to_ascii_lowercase().starts_with("gemini") } -/// Normalize a tool-parameter JSON schema for the Antigravity backend path that -/// the resolved model uses. +/// The schema dialect the Antigravity backend will validate a request against, +/// which depends on which upstream the resolved model is routed to. /// -/// The Antigravity Cloud Code backend multiplexes several upstreams behind one -/// `generateContent` endpoint, and each upstream validates tool schemas -/// differently. jcode's emitted schemas are valid JSON Schema draft 2020-12 -/// (verified against the metaschema), but two upstreams reject specific -/// constructs after their own re-translation: +/// The Cloud Code backend multiplexes several upstreams behind one +/// `generateContent` endpoint, and each validates tool schemas differently: /// +/// - **Gemini** (native): an OpenAPI 3.0 subset. Rejects draft keywords such as +/// `propertyNames` (#754) and `required` naming an undeclared property (#655). /// - **Claude** (Gemini->Anthropic translation): rejects combiners -/// (`anyOf`/`oneOf`/`allOf`) with HTTP 400 "must match JSON Schema draft -/// 2020-12". We collapse each combiner to its first branch. -/// - **gpt-oss / other OpenAI-compatible bridges**: round-trip numeric schema -/// bounds through a protobuf `int64`, which proto3 JSON re-encodes as a -/// string, then reject it ("'10' is not of type 'integer'"). We drop -/// `minItems`/`maxItems`/`minLength`/`maxLength`/`minProperties`/ -/// `maxProperties` for these models. These are advisory bounds the model does -/// not need to satisfy a call, so dropping them is safe. -/// -/// Gemini (the native path) rejects `propertyNames`, so that keyword is -/// removed recursively while the rest of the schema is preserved. -pub fn antigravity_compatible_schema(schema: &Value, model: &str) -> Value { +/// (`anyOf`/`oneOf`/`allOf`) at any depth with HTTP 400 "must match JSON +/// Schema draft 2020-12". +/// - **gpt-oss / other OpenAI-compatible bridges**: round-trip numeric bounds +/// through a protobuf `int64`, which proto3 JSON re-encodes as a string, then +/// reject the string ("'10' is not of type 'integer'"). +pub fn antigravity_dialect(model: &str) -> &'static jcode_schema_dialect::DialectSpec { if model_is_gemini(model) { - return strip_schema_key(schema, "propertyNames"); - } - if model_is_claude(model) { - return flatten_schema_combiners(schema); + &jcode_schema_dialect::registry::GEMINI + } else if model_is_claude(model) { + &jcode_schema_dialect::registry::ANTIGRAVITY_CLAUDE + } else { + &jcode_schema_dialect::registry::ANTIGRAVITY_BRIDGE } - // Non-Gemini, non-Claude models (e.g. gpt-oss) reach an OpenAI-compatible - // bridge that mangles numeric bounds; also flatten combiners defensively - // since those bridges share Anthropic's strictness about them. - strip_numeric_schema_bounds(&flatten_schema_combiners(schema)) } -fn strip_schema_key(schema: &Value, rejected_key: &str) -> Value { - match schema { - Value::Object(map) => Value::Object( - map.iter() - .filter(|(key, _)| key.as_str() != rejected_key) - .map(|(key, value)| (key.clone(), strip_schema_key(value, rejected_key))) - .collect(), - ), - Value::Array(items) => Value::Array( - items - .iter() - .map(|value| strip_schema_key(value, rejected_key)) - .collect(), - ), - _ => schema.clone(), - } +/// Normalize a tool-parameter JSON schema for the Antigravity backend path the +/// resolved model uses. See [`antigravity_dialect`] for the per-upstream rules +/// and `jcode-schema-dialect` for why the subsets are allow-lists. +pub fn antigravity_compatible_schema(schema: &Value, model: &str) -> Value { + jcode_schema_dialect::normalize(schema, antigravity_dialect(model)) } /// Numeric JSON Schema bounds an OpenAI-compatible Antigravity bridge corrupts diff --git a/crates/jcode-provider-gemini-runtime/src/gemini_tests.rs b/crates/jcode-provider-gemini-runtime/src/gemini_tests.rs index 1026b2cf07..3a3c4bef49 100644 --- a/crates/jcode-provider-gemini-runtime/src/gemini_tests.rs +++ b/crates/jcode-provider-gemini-runtime/src/gemini_tests.rs @@ -499,8 +499,12 @@ fn build_tools_rewrites_const_for_gemini_schema_compatibility() { let parameters = &built[0].function_declarations[0].parameters; assert!(!schema_contains_key(parameters, "const")); + // Gemini's schema proto models `anyOf` and has no `oneOf` field, so a + // passed-through `oneOf` is an "Unknown name" HTTP 400. The two mean the + // same thing for tool parameters, so the dialect renames rather than drops. + assert!(!schema_contains_key(parameters, "oneOf")); assert_eq!( - parameters["properties"]["tool_calls"]["items"]["oneOf"][0]["properties"]["tool"]["enum"], + parameters["properties"]["tool_calls"]["items"]["anyOf"][0]["properties"]["tool"]["enum"], json!(["read"]) ); } diff --git a/crates/jcode-provider-gemini/Cargo.toml b/crates/jcode-provider-gemini/Cargo.toml index cf57bb90c9..24d33874fe 100644 --- a/crates/jcode-provider-gemini/Cargo.toml +++ b/crates/jcode-provider-gemini/Cargo.toml @@ -5,6 +5,7 @@ edition = "2024" [dependencies] jcode-message-types = { path = "../jcode-message-types" } +jcode-schema-dialect = { path = "../jcode-schema-dialect" } anyhow = "1" serde = { version = "1", features = ["derive"] } serde_json = "1" diff --git a/crates/jcode-provider-gemini/src/lib.rs b/crates/jcode-provider-gemini/src/lib.rs index d53c15dd3b..33e86db65c 100644 --- a/crates/jcode-provider-gemini/src/lib.rs +++ b/crates/jcode-provider-gemini/src/lib.rs @@ -448,72 +448,19 @@ pub fn build_tools(tools: &[ToolDefinition]) -> Option> { }]) } -/// JSON Schema keywords the Gemini Code Assist `generateContent` endpoint -/// rejects outright (HTTP 400 "Unknown name ... Cannot find field"). Gemini -/// accepts only an OpenAPI 3.0 subset for `function_declarations.parameters`, -/// so these draft-style keywords must be stripped before sending. -const GEMINI_UNSUPPORTED_SCHEMA_KEYS: &[&str] = &[ - "additionalProperties", - "$schema", - "$id", - "$ref", - "$defs", - "definitions", - "$comment", - "propertyNames", -]; - -fn gemini_compatible_schema(schema: &Value) -> Value { - match schema { - Value::Object(map) => { - let mut out = serde_json::Map::new(); - for (key, value) in map { - // Drop draft-JSON-Schema keywords the Gemini API does not model; - // leaving them in fails the whole request with HTTP 400. - if GEMINI_UNSUPPORTED_SCHEMA_KEYS.contains(&key.as_str()) { - continue; - } - if key == "const" { - out.insert( - "enum".to_string(), - Value::Array(vec![gemini_compatible_schema(value)]), - ); - } else { - out.insert(key.clone(), gemini_compatible_schema(value)); - } - } - prune_dangling_required(&mut out); - Value::Object(out) - } - Value::Array(items) => Value::Array(items.iter().map(gemini_compatible_schema).collect()), - _ => schema.clone(), - } -} - -/// Drop `required` entries that name a property the same object does not define. -/// -/// Gemini validates this and rejects the whole request with HTTP 400 -/// ("required fields ['label'] are not defined in the schema properties"), while -/// OpenAI-compatible providers accept it. Multi-action tools legitimately write -/// `anyOf` branches that only constrain a discriminator and require a property -/// declared in the parent schema, which is what tripped this (issue #655). +/// Normalize a tool-parameter schema for Gemini `generateContent`, which +/// accepts only an OpenAPI 3.0 subset and rejects the whole request with +/// HTTP 400 ("Unknown name ... Cannot find field") over a single draft-style +/// keyword anywhere in any tool. /// -/// Objects without a `properties` map are left alone: there `required` cannot be -/// checked locally and Gemini does not reject it. -fn prune_dangling_required(out: &mut serde_json::Map) { - let Some(Value::Object(properties)) = out.get("properties") else { - return; - }; - let defined: Vec = properties.keys().cloned().collect(); - if let Some(Value::Array(required)) = out.get_mut("required") { - required.retain(|name| match name.as_str() { - Some(name) => defined.iter().any(|known| known == name), - None => false, - }); - if required.is_empty() { - out.remove("required"); - } - } +/// The accepted subset, the recursion, and the structural rewrites (dangling +/// `required` pruning for #655, `const` -> `enum`) all live in +/// `jcode-schema-dialect` so that every provider shares one implementation and +/// one set of regression tests. See that crate's docs for why this is an +/// allow-list: a deny-list can only contain keywords that have already taken +/// the provider down for somebody. +pub fn gemini_compatible_schema(schema: &Value) -> Value { + jcode_schema_dialect::normalize(schema, &jcode_schema_dialect::registry::GEMINI) } #[derive(Debug, Clone, Serialize)] diff --git a/crates/jcode-schema-dialect/Cargo.toml b/crates/jcode-schema-dialect/Cargo.toml new file mode 100644 index 0000000000..1472d73b7f --- /dev/null +++ b/crates/jcode-schema-dialect/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "jcode-schema-dialect" +version = "0.1.0" +edition = "2024" +publish = false + +[lib] +name = "jcode_schema_dialect" +path = "src/lib.rs" + +[dependencies] +dirs = "5" +serde = { version = "1", features = ["derive"] } +serde_json = "1" + +[dev-dependencies] +tempfile = "3" diff --git a/crates/jcode-schema-dialect/src/conformance.rs b/crates/jcode-schema-dialect/src/conformance.rs new file mode 100644 index 0000000000..ed4c137b47 --- /dev/null +++ b/crates/jcode-schema-dialect/src/conformance.rs @@ -0,0 +1,284 @@ +//! Checking that a normalized schema is actually safe to send. +//! +//! Normalization is only half the guarantee: it is easy to write a dialect that +//! drops the right keyword and still emits something a provider rejects for a +//! second reason. This module states the properties a normalized schema must +//! hold so they can be asserted over the *real* tool registry in CI, where they +//! catch a bad dialect edit before a user does. +//! +//! Both directions matter and both have shipped as bugs: +//! `must_not_contain_unsupported_constructs` catches under-stripping (#754, +//! #687), and `must_preserve_meaning` catches over-stripping, which is the +//! failure mode a keyword allow-list newly makes possible. + +use crate::dialect::DialectSpec; +use crate::keyword::{KeywordRole, LOAD_BEARING_KEYWORDS, keyword_role}; +use serde_json::Value; + +/// A property violation found in a normalized schema. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ConformanceError { + /// JSON path to the offending node, e.g. `$.properties.data`. + pub path: String, + pub message: String, +} + +impl std::fmt::Display for ConformanceError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}: {}", self.path, self.message) + } +} + +/// Assert a normalized schema contains nothing `spec` is known to reject. +/// +/// This is the invariant whose violation is an outage: any surviving keyword +/// outside the dialect's allow-list would 400 the entire tool catalog. +pub fn must_not_contain_unsupported_constructs( + schema: &Value, + spec: &DialectSpec, +) -> Vec { + let mut errors = Vec::new(); + walk(schema, spec, "$", &mut errors); + errors +} + +fn walk(schema: &Value, spec: &DialectSpec, path: &str, errors: &mut Vec) { + match schema { + Value::Object(map) => { + for (key, value) in map { + if !spec.supports(key) { + errors.push(ConformanceError { + path: path.to_string(), + message: format!("keyword `{key}` is not in dialect `{}`", spec.id), + }); + } + if key == "format" + && let Some(format) = value.as_str() + && !spec.supports_format(format) + { + errors.push(ConformanceError { + path: path.to_string(), + message: format!("format `{format}` is not in dialect `{}`", spec.id), + }); + } + if spec.transforms.flatten_all_combiners + && matches!(key.as_str(), "anyOf" | "oneOf" | "allOf") + { + errors.push(ConformanceError { + path: path.to_string(), + message: format!( + "combiner `{key}` survived a dialect that flattens all combiners" + ), + }); + } + if spec.transforms.prune_dangling_required && key == "required" { + check_required(map, value, path, errors); + } + + let child_path = format!("{path}.{key}"); + match keyword_role(key) { + KeywordRole::SubschemaMap => { + if let Value::Object(children) = value { + for (name, child) in children { + walk(child, spec, &format!("{child_path}.{name}"), errors); + } + } + } + KeywordRole::SubschemaArray => { + if let Value::Array(items) = value { + for (idx, item) in items.iter().enumerate() { + walk(item, spec, &format!("{child_path}[{idx}]"), errors); + } + } + } + KeywordRole::Subschema => walk(value, spec, &child_path, errors), + KeywordRole::Data => {} + } + } + if spec.transforms.require_properties_on_objects + && matches!(map.get("type"), Some(Value::String(t)) if t == "object") + && !map.contains_key("properties") + { + errors.push(ConformanceError { + path: path.to_string(), + message: "object schema is missing `properties`".to_string(), + }); + } + } + Value::Array(items) => { + for (idx, item) in items.iter().enumerate() { + walk(item, spec, &format!("{path}[{idx}]"), errors); + } + } + _ => {} + } +} + +fn check_required( + map: &serde_json::Map, + required: &Value, + path: &str, + errors: &mut Vec, +) { + let Some(Value::Object(properties)) = map.get("properties") else { + return; + }; + let Some(names) = required.as_array() else { + return; + }; + for name in names.iter().filter_map(Value::as_str) { + if !properties.contains_key(name) { + errors.push(ConformanceError { + path: path.to_string(), + message: format!("`required` names `{name}`, which this object does not declare"), + }); + } + } +} + +/// Assert normalization did not destroy what the tool means. +/// +/// An allow-list makes over-stripping the new hazard: a dialect that forgot to +/// list `description` would silently delete every tool's prompt text and the +/// requests would still succeed, so nothing else would catch it. Compares the +/// normalized schema against its source. +pub fn must_preserve_meaning(original: &Value, normalized: &Value) -> Vec { + let mut errors = Vec::new(); + compare(original, normalized, "$", &mut errors); + errors +} + +fn compare(original: &Value, normalized: &Value, path: &str, errors: &mut Vec) { + let (Some(original_map), Some(normalized_map)) = (original.as_object(), normalized.as_object()) + else { + return; + }; + + for key in LOAD_BEARING_KEYWORDS { + // A combiner flatten legitimately moves properties around, so only + // absence is checked, not equality. + if original_map.contains_key(*key) && !normalized_map.contains_key(*key) { + // `required` is the one load-bearing keyword a dialect may legally + // shrink, because Gemini rejects names it cannot resolve (#655). + if *key == "required" { + continue; + } + errors.push(ConformanceError { + path: path.to_string(), + message: format!("load-bearing keyword `{key}` was dropped"), + }); + } + } + + if let (Some(Value::Object(original_props)), Some(Value::Object(normalized_props))) = ( + original_map.get("properties"), + normalized_map.get("properties"), + ) { + for (name, original_child) in original_props { + match normalized_props.get(name) { + Some(normalized_child) => compare( + original_child, + normalized_child, + &format!("{path}.properties.{name}"), + errors, + ), + None => errors.push(ConformanceError { + path: path.to_string(), + message: format!("property `{name}` disappeared"), + }), + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{dialect, registry}; + use serde_json::json; + + /// The property that matters: whatever goes in, what comes out is clean. + #[test] + fn normalizing_a_hostile_schema_satisfies_the_dialect() { + let hostile = json!({ + "type": "object", + "properties": { + "url": { "type": "string", "format": "uri" }, + "ids": { "type": "array", "uniqueItems": true, "items": { "type": "string" } }, + "data": { + "type": "object", + "propertyNames": { "type": "string" }, + "additionalProperties": { "type": "string" } + }, + "weird": { "type": "string", "x-vendor": { "nested": true } } + }, + "required": ["url", "ghost"], + "$schema": "https://json-schema.org/draft/2020-12/schema" + }); + + for spec in registry::ALL { + let normalized = dialect::apply(&hostile, spec); + let errors = must_not_contain_unsupported_constructs(&normalized, spec); + assert!( + errors.is_empty(), + "dialect `{}` emitted an unsendable schema:\n{}", + spec.id, + errors + .iter() + .map(ToString::to_string) + .collect::>() + .join("\n") + ); + } + } + + #[test] + fn normalization_never_destroys_tool_meaning() { + let schema = json!({ + "type": "object", + "description": "a tool", + "properties": { + "path": { "type": "string", "description": "where" }, + "count": { "type": "integer", "minimum": 1 } + }, + "required": ["path"] + }); + for spec in registry::ALL { + let normalized = dialect::apply(&schema, spec); + let errors = must_preserve_meaning(&schema, &normalized); + assert!( + errors.is_empty(), + "dialect `{}` lost meaning:\n{}", + spec.id, + errors + .iter() + .map(ToString::to_string) + .collect::>() + .join("\n") + ); + assert_eq!(normalized["properties"]["path"]["description"], "where"); + } + } + + /// The checker must actually fail on a bad schema, or the sweep above is + /// passing vacuously. + #[test] + fn the_checker_rejects_an_unnormalized_schema() { + let raw = json!({ + "type": "object", + "properties": { "data": { "type": "object", "propertyNames": { "type": "string" } } } + }); + let errors = must_not_contain_unsupported_constructs(&raw, ®istry::GEMINI); + assert!( + errors.iter().any(|e| e.message.contains("propertyNames")), + "expected propertyNames to be flagged, got {errors:?}" + ); + + let stripped = json!({ "type": "object", "properties": {} }); + let lost = must_preserve_meaning(&raw, &stripped); + assert!( + lost.iter().any(|e| e.message.contains("disappeared")), + "expected the lost property to be flagged, got {lost:?}" + ); + } +} diff --git a/crates/jcode-schema-dialect/src/dialect.rs b/crates/jcode-schema-dialect/src/dialect.rs new file mode 100644 index 0000000000..d40cfe7f92 --- /dev/null +++ b/crates/jcode-schema-dialect/src/dialect.rs @@ -0,0 +1,432 @@ +//! Provider schema dialects, expressed as data. +//! +//! # Why this exists +//! +//! jcode advertises the *whole* tool array on every request, so one construct a +//! provider dislikes does not degrade one tool, it 400s every turn and the +//! provider becomes unusable. That has now happened at least eight times, each +//! with a different keyword and each fixed the same way: append the keyword to +//! that provider's hand-written deny-list and ship a release. +//! +//! | Issue | Provider | Construct | +//! |-------|----------|-----------| +//! | #446 | LM Studio | object schema without `properties` | +//! | #495 | OpenRouter | top-level `anyOf` | +//! | #543 | OpenAI | `format: "uri"` | +//! | #655 | Gemini | `required` naming an undeclared property | +//! | #687 | OpenAI | `uniqueItems` | +//! | #713 | OpenAI | property with no `type` | +//! | #754 | Gemini | `propertyNames` | +//! +//! A deny-list can only ever list what has already broken for somebody, so the +//! next unlisted keyword from the next MCP server is the next outage. The two +//! changes here break that cycle: +//! +//! 1. **Allow-lists, not deny-lists.** A dialect declares what its provider is +//! known to accept. Anything else is dropped, so an unknown construct is +//! inert by default rather than fatal by default. +//! 2. **One recursion.** Every dialect shares [`apply`], which walks schemas +//! using [`crate::keyword`] classification. Adding provider knowledge is a +//! data change, and a fix to the walk fixes it for all providers at once. + +use crate::keyword::{KeywordRole, LOAD_BEARING_KEYWORDS, is_droppable, keyword_role}; +use serde_json::{Map, Value}; + +/// Structural rewrites a dialect can request. These are the transformations +/// that cannot be expressed as "keep this keyword", because they change shape +/// rather than remove a key. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub struct DialectTransforms { + /// Collapse `anyOf`/`oneOf`/`allOf` at the schema root into one object + /// whose properties union every branch (Anthropic, OpenRouter: #495). + pub flatten_top_level_combiners: bool, + /// Collapse every combiner anywhere to its first branch (the Antigravity + /// Gemini->Anthropic bridge, which rejects them at any depth). + pub flatten_all_combiners: bool, + /// Insert `properties: {}` into object-typed schemas that lack it, which + /// strict validators require (LM Studio: #446). + pub require_properties_on_objects: bool, + /// Drop `required` entries naming a property the same object does not + /// declare, which Gemini rejects outright (#655). + pub prune_dangling_required: bool, + /// Rewrite `const: X` as `enum: [X]` for dialects that model only `enum`. + pub const_as_enum: bool, + /// Rewrite `oneOf` as `anyOf` for dialects that model only `anyOf`. + pub one_of_as_any_of: bool, +} + +/// A provider's accepted JSON Schema subset plus the rewrites needed to reach +/// it. Construct these as `const` tables next to the provider they describe. +#[derive(Clone, Debug)] +pub struct DialectSpec { + /// Stable id used for quirk persistence and diagnostics, e.g. `"gemini"`. + pub id: &'static str, + /// Keywords the provider is known to accept. Everything else is dropped. + pub supported_keywords: &'static [&'static str], + /// String `format` values the provider accepts. Empty means "any format". + pub supported_string_formats: &'static [&'static str], + /// Structural rewrites. + pub transforms: DialectTransforms, +} + +/// Keywords every dialect gets for free: the load-bearing set plus the +/// annotations no provider has ever objected to. +const UNIVERSAL_KEYWORDS: &[&str] = &["type", "properties", "items", "required", "enum", "title"]; + +impl DialectSpec { + /// Whether this dialect keeps `key`. + pub fn supports(&self, key: &str) -> bool { + UNIVERSAL_KEYWORDS.contains(&key) || self.supported_keywords.contains(&key) + } + + /// Whether a `format` value survives. + pub fn supports_format(&self, format: &str) -> bool { + self.supported_string_formats.is_empty() || self.supported_string_formats.contains(&format) + } + + /// Check the spec is coherent. A dialect that forgets a load-bearing + /// keyword would silently strip meaning from every tool, so this is + /// asserted for all registered dialects by the crate's own tests rather + /// than left to be discovered in production. + pub fn validate(&self) -> Result<(), String> { + for key in LOAD_BEARING_KEYWORDS { + if !self.supports(key) { + return Err(format!( + "dialect `{}` does not support load-bearing keyword `{key}`", + self.id + )); + } + } + for key in self.supported_keywords { + if UNIVERSAL_KEYWORDS.contains(key) { + return Err(format!( + "dialect `{}` redundantly lists universal keyword `{key}`", + self.id + )); + } + } + Ok(()) + } +} + +/// Extra keywords to drop beyond the dialect's own allow-list, learned at +/// runtime from a real provider rejection (see [`crate::quirks`]). +#[derive(Clone, Debug, Default)] +pub struct LearnedQuirks { + pub rejected_keywords: Vec, + pub rejected_formats: Vec, +} + +impl LearnedQuirks { + pub fn is_empty(&self) -> bool { + self.rejected_keywords.is_empty() && self.rejected_formats.is_empty() + } +} + +/// Normalize a tool-parameter schema into `spec`'s dialect. +pub fn apply(schema: &Value, spec: &DialectSpec) -> Value { + apply_with_quirks(schema, spec, &LearnedQuirks::default()) +} + +/// [`apply`], additionally honoring keywords learned from live rejections. +pub fn apply_with_quirks(schema: &Value, spec: &DialectSpec, quirks: &LearnedQuirks) -> Value { + let mut out = walk(schema, spec, quirks); + + if spec.transforms.flatten_all_combiners { + out = flatten_all_combiners(&out); + } else if spec.transforms.flatten_top_level_combiners { + flatten_top_level_combiners(&mut out); + } + + // A tool with no parameters is `{}` or a non-object in some MCP servers; + // strict validators want an object schema (#446). + if spec.transforms.require_properties_on_objects { + match out.as_object_mut() { + Some(map) if map.is_empty() => { + map.insert("type".into(), Value::String("object".into())); + map.insert("properties".into(), Value::Object(Map::new())); + } + None => { + out = serde_json::json!({ "type": "object", "properties": {} }); + } + _ => {} + } + } + + out +} + +fn walk(schema: &Value, spec: &DialectSpec, quirks: &LearnedQuirks) -> Value { + match schema { + Value::Object(map) => { + let mut out = Map::new(); + for (key, value) in map { + // Renames happen before the support check, because the dialect + // supports the *renamed* keyword, not the source one. Checking + // first would drop `oneOf` (and with it an entire branch set) + // on the very dialects whose whole point is to accept it as + // `anyOf`. + let out_key = if key == "oneOf" && spec.transforms.one_of_as_any_of { + "anyOf" + } else { + key.as_str() + }; + let dropped_by_quirk = quirks.rejected_keywords.iter().any(|k| k == key); + // Load-bearing keywords survive even a learned rejection: if a + // provider truly rejects `type`, dropping it would produce a + // meaningless tool, and the right answer is a real fix. + if (!spec.supports(out_key) || dropped_by_quirk) && is_droppable(key) { + if key == "const" && spec.transforms.const_as_enum { + out.insert( + "enum".into(), + // A `const` value is data, so it moves verbatim. + Value::Array(vec![value.clone()]), + ); + } + continue; + } + if key == "format" { + let ok = value + .as_str() + .map(|f| { + spec.supports_format(f) + && !quirks.rejected_formats.iter().any(|r| r == f) + }) + .unwrap_or(false); + if !ok { + continue; + } + out.insert(key.clone(), value.clone()); + continue; + } + + let normalized = match keyword_role(key) { + KeywordRole::SubschemaMap => match value { + Value::Object(children) => Value::Object( + children + .iter() + .map(|(name, child)| (name.clone(), walk(child, spec, quirks))) + .collect(), + ), + other => other.clone(), + }, + KeywordRole::SubschemaArray => match value { + Value::Array(items) => { + Value::Array(items.iter().map(|i| walk(i, spec, quirks)).collect()) + } + other => walk(other, spec, quirks), + }, + KeywordRole::Subschema => walk(value, spec, quirks), + KeywordRole::Data => value.clone(), + }; + match out.get_mut(out_key) { + // `oneOf` renamed onto an existing `anyOf`: merge branches + // rather than clobbering one of them. + Some(Value::Array(existing)) => { + if let Value::Array(incoming) = normalized { + existing.extend(incoming); + } + } + _ => { + out.insert(out_key.to_string(), normalized); + } + } + } + + if spec.transforms.prune_dangling_required { + prune_dangling_required(&mut out); + } + if spec.transforms.require_properties_on_objects && is_object_typed(&out) { + out.entry("properties".to_string()) + .or_insert_with(|| Value::Object(Map::new())); + } + Value::Object(out) + } + Value::Array(items) => Value::Array(items.iter().map(|i| walk(i, spec, quirks)).collect()), + _ => schema.clone(), + } +} + +fn is_object_typed(map: &Map) -> bool { + match map.get("type") { + Some(Value::String(t)) => t == "object", + Some(Value::Array(types)) => types.iter().any(|t| t.as_str() == Some("object")), + _ => false, + } +} + +/// Drop `required` names the same object does not declare (#655). +fn prune_dangling_required(out: &mut Map) { + let Some(Value::Object(properties)) = out.get("properties") else { + return; + }; + let defined: Vec = properties.keys().cloned().collect(); + if let Some(Value::Array(required)) = out.get_mut("required") { + required.retain(|name| { + name.as_str() + .is_some_and(|name| defined.iter().any(|known| known == name)) + }); + if required.is_empty() { + out.remove("required"); + } + } +} + +/// Collapse root combiners into one object schema whose properties union every +/// branch. Runtime tool deserialization stays the authority on which +/// combination is actually valid. +fn flatten_top_level_combiners(schema: &mut Value) { + let Some(output) = schema.as_object_mut() else { + return; + }; + let mut merged_properties = output + .get("properties") + .and_then(Value::as_object) + .cloned() + .unwrap_or_default(); + let mut all_of_required: Vec = Vec::new(); + let mut saw_combiner = false; + + for keyword in ["oneOf", "anyOf", "allOf"] { + let Some(branches) = output.remove(keyword).and_then(|v| match v { + Value::Array(items) => Some(items), + _ => None, + }) else { + continue; + }; + saw_combiner = true; + for branch in branches { + let Some(branch) = branch.as_object() else { + continue; + }; + if let Some(properties) = branch.get("properties").and_then(Value::as_object) { + for (name, property) in properties { + merged_properties + .entry(name.clone()) + .or_insert_with(|| property.clone()); + } + } + // Only `allOf` branches all apply, so only their `required` is safe + // to promote; promoting an `anyOf` branch's would demand fields the + // caller may legitimately omit. + if keyword == "allOf" + && let Some(required) = branch.get("required").and_then(Value::as_array) + { + for name in required.iter().filter_map(Value::as_str) { + if !all_of_required.iter().any(|e| e == name) { + all_of_required.push(name.to_string()); + } + } + } + } + } + + if !saw_combiner { + return; + } + + output.insert("type".into(), Value::String("object".into())); + output.insert("properties".into(), Value::Object(merged_properties)); + if !all_of_required.is_empty() { + let required = output + .entry("required".to_string()) + .or_insert_with(|| Value::Array(Vec::new())); + if let Value::Array(required) = required { + for name in all_of_required { + if !required.iter().any(|e| e.as_str() == Some(&name)) { + required.push(Value::String(name)); + } + } + } + } + // The union object may now require a name only one branch declared. + if let Value::Object(map) = schema { + prune_dangling_required(map); + } +} + +/// Combine two declarations of the same property name, keeping every key +/// either side declares. Neither is strictly more authoritative: the branch +/// narrows the type, the parent carries the description, and a tool needs both. +fn merge_property(parent: &Value, branch: &Value) -> Value { + let (Some(parent_map), Some(branch_map)) = (parent.as_object(), branch.as_object()) else { + return parent.clone(); + }; + let mut merged = branch_map.clone(); + for (key, value) in parent_map { + merged.entry(key.clone()).or_insert_with(|| value.clone()); + } + Value::Object(merged) +} + +/// Collapse every combiner, at any depth, into the schema that contains it. +/// +/// The branch is chosen (first, since branches are ordered by primacy) but its +/// *siblings are merged*, not discarded. Merging matters most for `properties`: +/// a multi-action tool declares its shared parameters on the parent object and +/// only its discriminator constraints in the branches, so taking the branch's +/// property map alone would silently drop nearly every parameter the tool +/// accepts. That was live for the `swarm` tool on Antigravity's Claude and +/// bridge routes until the registry-wide conformance sweep caught it. +fn flatten_all_combiners(schema: &Value) -> Value { + match schema { + Value::Object(map) => { + for combiner in ["anyOf", "oneOf", "allOf"] { + if let Some(Value::Array(branches)) = map.get(combiner) + && let Some(first) = branches.first() + { + let mut flattened = match flatten_all_combiners(first) { + Value::Object(branch) => branch, + other => return other, + }; + for (key, value) in map { + if key == combiner { + continue; + } + let sibling = flatten_all_combiners(value); + match (flattened.get_mut(key), &sibling) { + // Union the parent's declared properties with the + // branch's. The parent wins on a name collision: + // a branch usually redeclares a shared property + // only to narrow it (an `enum` on a discriminator) + // and omits the description, so preferring the + // branch would delete prompt-visible text. + (Some(Value::Object(existing)), Value::Object(incoming)) + if key == "properties" => + { + for (name, property) in incoming { + match existing.get(name) { + Some(parent) => { + existing.insert( + name.clone(), + merge_property(parent, property), + ); + } + None => { + existing.insert(name.clone(), property.clone()); + } + } + } + } + (Some(_), _) => {} + (None, _) => { + flattened.insert(key.clone(), sibling); + } + } + } + // A branch may require a discriminator the merged object + // does not declare, which the downstream validator rejects. + prune_dangling_required(&mut flattened); + return Value::Object(flattened); + } + } + Value::Object( + map.iter() + .map(|(k, v)| (k.clone(), flatten_all_combiners(v))) + .collect(), + ) + } + Value::Array(items) => Value::Array(items.iter().map(flatten_all_combiners).collect()), + _ => schema.clone(), + } +} diff --git a/crates/jcode-schema-dialect/src/keyword.rs b/crates/jcode-schema-dialect/src/keyword.rs new file mode 100644 index 0000000000..2f66fe64bf --- /dev/null +++ b/crates/jcode-schema-dialect/src/keyword.rs @@ -0,0 +1,131 @@ +//! JSON Schema keyword classification. +//! +//! Every provider-specific schema rewrite in jcode has to answer the same two +//! questions about a key it encounters: *is this a schema keyword or a property +//! name?* and *does its value contain more schemas?* Getting either wrong is how +//! a property literally named `uniqueItems` gets stripped, or how a nested +//! `propertyNames` survives a top-level filter (issue #754). +//! +//! Answering them once, here, is what lets the rest of this crate express a +//! provider dialect as data instead of as another bespoke recursion. + +/// Where a keyword's value sits on the "is it a schema?" spectrum. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum KeywordRole { + /// The value is one subschema (`items`, `not`, `propertyNames`). It may also + /// legally be a boolean, which recursion passes through untouched. + Subschema, + /// The value is a map of name -> subschema (`properties`, `$defs`). The map + /// keys are user-controlled names, never keywords. + SubschemaMap, + /// The value is an array of subschemas (`anyOf`, `prefixItems`). + SubschemaArray, + /// The value is plain data that must never be walked as a schema (`enum`, + /// `const`, `required`, `default`, `examples`). + Data, +} + +/// Keywords whose value is a map of name -> subschema. +const SUBSCHEMA_MAP_KEYWORDS: &[&str] = &[ + "properties", + "patternProperties", + "$defs", + "definitions", + "dependentSchemas", +]; + +/// Keywords whose value is a single subschema (or a boolean schema). +const SUBSCHEMA_KEYWORDS: &[&str] = &[ + "items", + "additionalItems", + "additionalProperties", + "contains", + "propertyNames", + "not", + "if", + "then", + "else", + "unevaluatedItems", + "unevaluatedProperties", + "contentSchema", +]; + +/// Keywords whose value is an array of subschemas. +const SUBSCHEMA_ARRAY_KEYWORDS: &[&str] = &["allOf", "anyOf", "oneOf", "prefixItems"]; + +/// Keywords whose value is data, not a schema. Recursing into these is how a +/// user's `enum: ["items"]` or `default: {"type": "x"}` gets corrupted. +const DATA_KEYWORDS: &[&str] = &["enum", "const", "default", "examples", "required"]; + +/// Classify a schema keyword by what its value contains. +pub fn keyword_role(key: &str) -> KeywordRole { + if SUBSCHEMA_MAP_KEYWORDS.contains(&key) { + KeywordRole::SubschemaMap + } else if SUBSCHEMA_KEYWORDS.contains(&key) { + KeywordRole::Subschema + } else if SUBSCHEMA_ARRAY_KEYWORDS.contains(&key) { + KeywordRole::SubschemaArray + } else if DATA_KEYWORDS.contains(&key) { + KeywordRole::Data + } else { + // Unknown keywords (vendor extensions, future drafts) are treated as + // data: walking into them can only corrupt them, and a dialect that + // does not support them drops them wholesale anyway. + KeywordRole::Data + } +} + +/// Keywords that carry the *meaning* of a schema rather than an extra +/// constraint or annotation on it. +/// +/// A dialect may drop any keyword it does not support, which is the whole point +/// of the allow-list: a construct jcode has never seen cannot brick a provider. +/// But dropping one of these would silently change what the tool accepts, so +/// they are never droppable, and a dialect that omits one is a bug caught by +/// [`crate::dialect::DialectSpec::validate`]. +pub const LOAD_BEARING_KEYWORDS: &[&str] = &[ + "type", + "properties", + "items", + "required", + "enum", + "description", +]; + +/// Whether a keyword may be removed from a schema without changing which +/// instances the tool will accept at execution time. +/// +/// Removing a validation keyword (`uniqueItems`, `minItems`, `propertyNames`) +/// only widens what the *model* is told it may send; the tool or MCP server +/// still validates the real call, so the constraint is not actually lost. +pub fn is_droppable(key: &str) -> bool { + !LOAD_BEARING_KEYWORDS.contains(&key) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn schema_bearing_keywords_are_classified() { + assert_eq!(keyword_role("properties"), KeywordRole::SubschemaMap); + assert_eq!(keyword_role("propertyNames"), KeywordRole::Subschema); + assert_eq!(keyword_role("anyOf"), KeywordRole::SubschemaArray); + assert_eq!(keyword_role("enum"), KeywordRole::Data); + } + + #[test] + fn unknown_keywords_are_data_so_recursion_cannot_corrupt_them() { + assert_eq!(keyword_role("x-vendor-thing"), KeywordRole::Data); + assert_eq!(keyword_role("$anchor"), KeywordRole::Data); + } + + #[test] + fn load_bearing_keywords_are_not_droppable() { + for key in LOAD_BEARING_KEYWORDS { + assert!(!is_droppable(key), "{key} must never be dropped"); + } + assert!(is_droppable("uniqueItems")); + assert!(is_droppable("propertyNames")); + } +} diff --git a/crates/jcode-schema-dialect/src/lib.rs b/crates/jcode-schema-dialect/src/lib.rs new file mode 100644 index 0000000000..888b85b593 --- /dev/null +++ b/crates/jcode-schema-dialect/src/lib.rs @@ -0,0 +1,503 @@ +//! Provider JSON Schema compatibility, as a system rather than a patch queue. +//! +//! # The recurring bug +//! +//! jcode sends its entire tool array on every request. Providers each accept a +//! different subset of JSON Schema, so a single construct emitted by one MCP +//! server does not degrade one tool: it 400s every turn and takes the provider +//! offline. Issues #446, #495, #543, #655, #687, #711, #713 and #754 are all +//! this same bug with a different keyword, and each was fixed by appending that +//! keyword to a hand-written per-provider deny-list. +//! +//! A deny-list only contains what has already broken for a user, so that loop +//! cannot converge. This crate replaces it with three layers: +//! +//! 1. **Prevention** ([`registry`], [`dialect`]) - each provider declares the +//! subset it *accepts*. Unknown constructs are dropped instead of forwarded, +//! so a keyword nobody has seen yet is inert rather than fatal. +//! 2. **Recovery** ([`rejection`]) - when a provider rejects something anyway, +//! its error text is parsed into the offending keyword so the turn can be +//! retried without it, instead of failing in front of the user. +//! 3. **Memory** ([`quirks`]) - the learned rejection is persisted, so it costs +//! one wasted round trip ever, not one per request, and the fix propagates +//! without waiting for a jcode release. +//! +//! # Adding a provider +//! +//! Add a [`dialect::DialectSpec`] to [`registry`] listing the keywords it is +//! observed to accept, then call [`normalize`] where tools are built. Do not +//! write a new recursion: [`dialect::apply`] handles keyword classification, +//! and a bug fixed there is fixed for every provider at once. + +pub mod conformance; +pub mod dialect; +pub mod keyword; +pub mod quirks; +pub mod registry; +pub mod rejection; + +pub use conformance::{ + ConformanceError, must_not_contain_unsupported_constructs, must_preserve_meaning, +}; +pub use dialect::{DialectSpec, DialectTransforms, LearnedQuirks}; +pub use keyword::{KeywordRole, keyword_role}; +pub use rejection::{SchemaRejection, classify, is_schema_error}; + +use serde_json::Value; + +/// Normalize a tool-parameter schema for `spec`, applying anything already +/// learned about that provider. This is the entry point provider crates use. +pub fn normalize(schema: &Value, spec: &DialectSpec) -> Value { + let learned = quirks::learned_for(spec.id); + if learned.is_empty() { + dialect::apply(schema, spec) + } else { + dialect::apply_with_quirks(schema, spec, &learned) + } +} + +/// What a caller should do after a provider rejected a request. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum RecoveryAction { + /// Not a schema problem; handle the error normally. + NotSchemaRelated, + /// A schema rejection naming something new. The tool schemas have been + /// re-normalized to exclude it, so retrying the request should succeed. + RetryWithoutConstruct { + /// Human-readable description for the log line. + description: String, + }, + /// A schema rejection that retrying cannot fix, either because it names + /// something jcode already strips or because it names nothing actionable. + /// The message explains what to add to the dialect. + Unrecoverable { hint: String }, +} + +/// Classify a provider failure and, when it is a learnable schema rejection, +/// record it so the next [`normalize`] call strips it. +/// +/// Returns [`RecoveryAction::RetryWithoutConstruct`] at most once per distinct +/// construct per dialect, which is what bounds the retry loop: a second +/// rejection naming the same keyword reports as unrecoverable instead of +/// retrying forever. +pub fn recover_from_error(message: &str, spec: &DialectSpec) -> RecoveryAction { + let Some(rejection) = rejection::classify(message) else { + if rejection::is_schema_error(message) { + return RecoveryAction::Unrecoverable { + hint: format!( + "provider `{}` rejected the tool schemas but did not name the construct; \ + the raw error is the only diagnostic available", + spec.id + ), + }; + } + return RecoveryAction::NotSchemaRelated; + }; + + let mut learned_something = false; + let mut described = Vec::new(); + + if let Some(keyword) = rejection.keyword.as_deref() { + // A load-bearing keyword cannot be dropped without destroying the + // tool's meaning, so this is a real bug rather than a quirk to absorb. + if !keyword::is_droppable(keyword) { + return RecoveryAction::Unrecoverable { + hint: format!( + "provider `{}` rejected the load-bearing keyword `{keyword}`, which cannot be \ + stripped without changing what the tool accepts; the dialect's transforms \ + need a real fix", + spec.id + ), + }; + } + if quirks::record_keyword(spec.id, keyword) { + learned_something = true; + described.push(format!("keyword `{keyword}`")); + } + } + if let Some(format) = rejection.format.as_deref() + && quirks::record_format(spec.id, format) + { + learned_something = true; + described.push(format!("format `{format}`")); + } + + if learned_something { + let tool = rejection + .tool + .as_deref() + .map(|t| format!(" (from tool `{t}`)")) + .unwrap_or_default(); + return RecoveryAction::RetryWithoutConstruct { + description: format!( + "provider `{}` rejected {}{tool}; stripping it and retrying, and remembering it \ + for future requests", + spec.id, + described.join(" and "), + ), + }; + } + + RecoveryAction::Unrecoverable { + hint: if rejection.is_actionable() { + format!( + "provider `{}` rejected {} again after it was already being stripped, so the \ + construct is not what actually failed", + spec.id, + rejection + .keyword + .or(rejection.format) + .unwrap_or_else(|| "a schema construct".into()) + ) + } else { + format!( + "provider `{}` reported a schema error naming no actionable construct", + spec.id + ) + }, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + /// Redirect this test thread's quirk store so parallel cases cannot see + /// each other's learned rejections. + fn isolate_quirks(name: &str) -> tempfile::TempDir { + let dir = tempfile::tempdir().unwrap(); + quirks::use_test_path(dir.path().join(format!("{name}.json"))); + dir + } + + /// The exact schema from #754 (`@playwright/mcp`'s `browser_drop`) must + /// survive the Gemini dialect with the offending keyword gone and the rest + /// of the tool intact. + #[test] + fn issue_754_playwright_schema_is_accepted_by_the_gemini_dialect() { + let schema = json!({ + "type": "object", + "properties": { + "data": { + "type": "object", + "additionalProperties": { "type": "string" }, + "propertyNames": { "type": "string" }, + "description": "Data to drop, as a map of MIME type to string value" + } + }, + "required": ["data"] + }); + let out = dialect::apply(&schema, ®istry::GEMINI); + assert!(!rejection::schema_contains_keyword(&out, "propertyNames")); + assert!(!rejection::schema_contains_keyword( + &out, + "additionalProperties" + )); + assert_eq!(out["properties"]["data"]["type"], "object"); + assert_eq!( + out["properties"]["data"]["description"], + "Data to drop, as a map of MIME type to string value" + ); + assert_eq!(out["required"], json!(["data"])); + } + + /// #687: `uniqueItems` is stripped for OpenAI but the property itself, its + /// type, and its description survive. + #[test] + fn issue_687_unique_items_is_stripped_without_losing_the_property() { + let schema = json!({ + "type": "object", + "properties": { + "ids": { + "type": "array", + "uniqueItems": true, + "items": { "type": "string" }, + "description": "Channel ids" + } + } + }); + let out = dialect::apply(&schema, ®istry::OPENAI); + assert!(out["properties"]["ids"].get("uniqueItems").is_none()); + assert_eq!(out["properties"]["ids"]["items"]["type"], "string"); + assert_eq!(out["properties"]["ids"]["description"], "Channel ids"); + } + + /// #543: an unsupported `format` goes, a supported one stays. + #[test] + fn issue_543_unsupported_format_is_stripped_but_supported_ones_survive() { + let schema = json!({ + "type": "object", + "properties": { + "url": { "type": "string", "format": "uri" }, + "when": { "type": "string", "format": "date-time" } + } + }); + let out = dialect::apply(&schema, ®istry::OPENAI); + assert!(out["properties"]["url"].get("format").is_none()); + assert_eq!(out["properties"]["url"]["type"], "string"); + assert_eq!(out["properties"]["when"]["format"], "date-time"); + } + + /// #446: a bare no-argument object schema gains `properties`. + #[test] + fn issue_446_bare_object_schema_gains_properties() { + let out = dialect::apply(&json!({ "type": "object" }), ®istry::OPENROUTER); + assert_eq!(out["properties"], json!({})); + let empty = dialect::apply(&json!({}), ®istry::OPENROUTER); + assert_eq!(empty["type"], "object"); + assert_eq!(empty["properties"], json!({})); + } + + /// #495: a top-level combiner is flattened into one object for providers + /// whose upstream rejects it. + #[test] + fn issue_495_top_level_combiner_is_flattened() { + let schema = json!({ + "type": "object", + "properties": { "action": { "type": "string" } }, + "anyOf": [ + { "properties": { "label": { "type": "string" } }, "required": ["label"] }, + { "properties": { "target": { "type": "string" } } } + ] + }); + let out = dialect::apply(&schema, ®istry::OPENROUTER); + assert!(out.get("anyOf").is_none()); + assert_eq!(out["properties"]["action"]["type"], "string"); + assert_eq!(out["properties"]["label"]["type"], "string"); + assert_eq!(out["properties"]["target"]["type"], "string"); + } + + /// #655: `required` naming a property this object does not declare is + /// dropped for Gemini, since Gemini 400s on it. + #[test] + fn issue_655_dangling_required_is_pruned_for_gemini() { + let schema = json!({ + "type": "object", + "properties": { "action": { "type": "string" } }, + "required": ["action", "label"] + }); + let out = dialect::apply(&schema, ®istry::GEMINI); + assert_eq!(out["required"], json!(["action"])); + } + + /// The regression that made #754 a bug in the first place: a top-level-only + /// filter left the nested copy in place. + #[test] + fn stripping_reaches_arbitrarily_deep_nesting() { + let schema = json!({ + "type": "object", + "properties": { + "a": { "type": "array", "items": { + "type": "object", "properties": { + "b": { "type": "object", "propertyNames": { "type": "string" } } + } + }} + } + }); + let out = dialect::apply(&schema, ®istry::GEMINI); + assert!(!rejection::schema_contains_keyword(&out, "propertyNames")); + } + + /// #713's sibling hazard: a *property* named like a keyword must not be + /// mistaken for the keyword and deleted. + #[test] + fn a_property_named_like_a_keyword_is_never_stripped() { + let schema = json!({ + "type": "object", + "properties": { + "uniqueItems": { "type": "boolean", "description": "a real field" }, + "propertyNames": { "type": "string" } + } + }); + let openai = dialect::apply(&schema, ®istry::OPENAI); + assert_eq!(openai["properties"]["uniqueItems"]["type"], "boolean"); + let gemini = dialect::apply(&schema, ®istry::GEMINI); + assert_eq!(gemini["properties"]["propertyNames"]["type"], "string"); + } + + /// Enum values are data, so a value that happens to name a keyword must + /// survive verbatim. + #[test] + fn enum_values_that_look_like_keywords_survive() { + let schema = json!({ + "type": "object", + "properties": { + "mode": { "type": "string", "enum": ["uniqueItems", "propertyNames"] } + } + }); + let out = dialect::apply(&schema, ®istry::GEMINI); + assert_eq!( + out["properties"]["mode"]["enum"], + json!(["uniqueItems", "propertyNames"]) + ); + } + + #[test] + fn recovery_learns_then_refuses_to_loop() { + let _dir = isolate_quirks("recover"); + let message = "invalid_request_error (invalid_function_parameters): Invalid schema for function 'mcp__x__y': In context=('properties', 'ids'), 'somethingNew' is not permitted."; + + match recover_from_error(message, ®istry::OPENAI) { + RecoveryAction::RetryWithoutConstruct { description } => { + assert!(description.contains("somethingNew"), "{description}"); + } + other => panic!("expected a retry, got {other:?}"), + } + + // The same rejection a second time means stripping it did not help, so + // the caller must not retry again. + assert!(matches!( + recover_from_error(message, ®istry::OPENAI), + RecoveryAction::Unrecoverable { .. } + )); + + } + + #[test] + fn a_learned_keyword_is_stripped_by_later_normalization() { + let _dir = isolate_quirks("learned"); + let schema = json!({ + "type": "object", + "properties": { "ids": { "type": "array", "vendorThing": true } } + }); + // `vendorThing` is unknown to the dialect, so prevention already drops + // it; use a keyword the dialect explicitly supports to prove the + // learned layer adds power beyond the allow-list. + assert!(rejection::schema_contains_keyword( + &json!({ "minItems": 1 }), + "minItems" + )); + let supported = json!({ + "type": "object", + "properties": { "ids": { "type": "array", "minItems": 1 } } + }); + assert!(rejection::schema_contains_keyword( + &normalize(&supported, ®istry::OPENAI), + "minItems" + )); + + assert!(quirks::record_keyword("openai", "minItems")); + assert!(!rejection::schema_contains_keyword( + &normalize(&supported, ®istry::OPENAI), + "minItems" + )); + let _ = schema; + + } + + #[test] + fn a_load_bearing_rejection_is_reported_not_absorbed() { + let _dir = isolate_quirks("loadbearing"); + let action = recover_from_error( + "GenerateContentRequest.tools[0].function_declarations[3].parameters: required fields ['label'] are not defined in the schema properties", + ®istry::GEMINI, + ); + match action { + RecoveryAction::Unrecoverable { hint } => assert!(hint.contains("required"), "{hint}"), + other => panic!("required must never be silently dropped, got {other:?}"), + } + } + + #[test] + fn non_schema_errors_are_passed_through() { + assert_eq!( + recover_from_error("HTTP 503 upstream unavailable", ®istry::OPENAI), + RecoveryAction::NotSchemaRelated + ); + } + + /// Found by the registry-wide conformance sweep, not by a user: flattening + /// a combiner used to keep only the chosen branch's `properties`, so the + /// `swarm` tool reached Antigravity's Claude route advertising 3 of its 44 + /// parameters. Requests still succeeded, which is why nothing caught it. + #[test] + fn flattening_a_combiner_keeps_the_parent_properties() { + let schema = json!({ + "type": "object", + "properties": { + "action": { "type": "string", "description": "what to do" }, + "target": { "type": "string", "description": "on what" }, + "limit": { "type": "integer" } + }, + "required": ["action"], + "anyOf": [ + { + "properties": { "action": { "enum": ["spawn"] } }, + "required": ["action", "prompt"] + }, + { "properties": { "action": { "enum": ["stop"] } } } + ] + }); + + let out = dialect::apply(&schema, ®istry::ANTIGRAVITY_CLAUDE); + + // Every parameter the tool actually accepts is still advertised. + for name in ["action", "target", "limit"] { + assert!( + out["properties"].get(name).is_some(), + "property `{name}` was dropped: {out}" + ); + } + // The branch's narrowing survives alongside the parent's description. + assert_eq!(out["properties"]["action"]["enum"], json!(["spawn"])); + assert_eq!(out["properties"]["action"]["description"], "what to do"); + assert_eq!(out["properties"]["target"]["description"], "on what"); + // No combiner survives for a route that rejects them at any depth. + assert!(out.get("anyOf").is_none()); + // And the branch's `required` cannot name a property that is now absent. + assert_eq!(out["required"], json!(["action"])); + } + + /// A keyword the dialect accepts only under a different name must be + /// renamed, never dropped. Getting the order wrong deleted the entire + /// `oneOf` branch set (and with it the `batch` tool's whole call shape) + /// before it could be renamed to `anyOf`. + #[test] + fn a_renamed_keyword_survives_instead_of_being_dropped() { + let schema = json!({ + "type": "object", + "properties": { + "tool_calls": { + "type": "array", + "items": { + "oneOf": [ + { "type": "object", "properties": { + "tool": { "type": "string", "const": "read" } + }} + ] + } + } + } + }); + let out = dialect::apply(&schema, ®istry::GEMINI); + let items = &out["properties"]["tool_calls"]["items"]; + assert!(items.get("oneOf").is_none(), "oneOf must be renamed"); + assert_eq!( + items["anyOf"][0]["properties"]["tool"]["enum"], + json!(["read"]), + "the branch and its const->enum rewrite must both survive: {out}" + ); + } + + /// The same sweep's second finding: a branch that redeclares a shared + /// property to narrow it omits the description, so preferring the branch + /// wholesale deleted prompt-visible text from the tool. + #[test] + fn a_narrowing_branch_never_deletes_the_parent_description() { + let schema = json!({ + "type": "object", + "properties": { "label": { "type": "string", "description": "shown on the chip" } }, + "anyOf": [{ "properties": { "label": { "minLength": 1 } } }] + }); + for spec in [®istry::ANTIGRAVITY_CLAUDE, ®istry::ANTIGRAVITY_BRIDGE] { + let out = dialect::apply(&schema, spec); + assert_eq!( + out["properties"]["label"]["description"], "shown on the chip", + "dialect `{}` dropped the description", + spec.id + ); + assert_eq!(out["properties"]["label"]["type"], "string"); + } + } +} diff --git a/crates/jcode-schema-dialect/src/quirks.rs b/crates/jcode-schema-dialect/src/quirks.rs new file mode 100644 index 0000000000..e24b895df0 --- /dev/null +++ b/crates/jcode-schema-dialect/src/quirks.rs @@ -0,0 +1,223 @@ +//! Persisting what a provider rejected, so it is only learned once. +//! +//! Recovering inside a turn (see [`crate::rejection`]) fixes the immediate +//! failure but costs a wasted round trip on every subsequent request, and the +//! knowledge dies with the process. Writing it to `~/.jcode/schema-quirks.json` +//! makes the second request the fast path and means the fleet self-heals in +//! front of a provider change instead of waiting for a jcode release. +//! +//! The file is advisory: a corrupt or unreadable store degrades to "learn it +//! again", never to a failure. + +use crate::dialect::LearnedQuirks; +use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; +use std::path::PathBuf; +use std::sync::{Mutex, OnceLock}; + +#[derive(Clone, Debug, Default, Serialize, Deserialize)] +struct QuirkEntry { + #[serde(default, skip_serializing_if = "Vec::is_empty")] + rejected_keywords: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + rejected_formats: Vec, +} + +#[derive(Clone, Debug, Default, Serialize, Deserialize)] +struct QuirkFile { + #[serde(default)] + dialects: BTreeMap, +} + +fn store_path() -> Option { + if let Some(override_path) = test_override() { + return Some(override_path); + } + if let Ok(explicit) = std::env::var("JCODE_SCHEMA_QUIRKS_PATH") { + return Some(PathBuf::from(explicit)); + } + let home = if let Ok(jcode_home) = std::env::var("JCODE_HOME") { + PathBuf::from(jcode_home) + } else { + dirs::home_dir()?.join(".jcode") + }; + Some(home.join("schema-quirks.json")) +} + +#[cfg(test)] +thread_local! { + static TEST_PATH: std::cell::RefCell> = const { std::cell::RefCell::new(None) }; +} + +#[cfg(test)] +fn test_override() -> Option { + TEST_PATH.with(|path| path.borrow().clone()) +} + +#[cfg(not(test))] +fn test_override() -> Option { + None +} + +/// Point this thread's quirk store at `path`. Tests run in parallel and the +/// store is otherwise process-global, so redirecting per thread is what keeps +/// them from racing each other (and avoids mutating process env, which is +/// `unsafe` in edition 2024). +#[cfg(test)] +pub fn use_test_path(path: PathBuf) { + TEST_PATH.with(|slot| *slot.borrow_mut() = Some(path)); + reset_cache_for_tests(); +} + +/// Cached store contents, keyed by path so that redirected stores (tests, an +/// alternate `JCODE_HOME`) never read each other's state. +fn cache() -> &'static Mutex> { + static CACHE: OnceLock>> = OnceLock::new(); + CACHE.get_or_init(|| Mutex::new(BTreeMap::new())) +} + +fn load_file(path: &PathBuf) -> QuirkFile { + std::fs::read_to_string(path) + .ok() + .and_then(|raw| serde_json::from_str(&raw).ok()) + .unwrap_or_default() +} + +/// Run `f` against the cached store for the active path, persisting when it +/// reports a change. +fn with_store(f: impl FnOnce(&mut QuirkFile) -> (T, bool)) -> T { + let Some(path) = store_path() else { + return f(&mut QuirkFile::default()).0; + }; + let mut guard = cache().lock().unwrap_or_else(|e| e.into_inner()); + let file = guard + .entry(path.clone()) + .or_insert_with(|| load_file(&path)); + let (result, changed) = f(file); + if changed { + // Best-effort persistence: an unwritable home must not break the turn + // that just recovered. + if let Ok(serialized) = serde_json::to_string_pretty(file) { + if let Some(parent) = path.parent() { + let _ = std::fs::create_dir_all(parent); + } + let _ = std::fs::write(&path, serialized); + } + } + result +} + +/// Quirks learned so far for `dialect_id`. +pub fn learned_for(dialect_id: &str) -> LearnedQuirks { + with_store(|file| { + let learned = file + .dialects + .get(dialect_id) + .map(|entry| LearnedQuirks { + rejected_keywords: entry.rejected_keywords.clone(), + rejected_formats: entry.rejected_formats.clone(), + }) + .unwrap_or_default(); + (learned, false) + }) +} + +/// Record a keyword `dialect_id` rejected. Returns `true` when this is new +/// information, which is the caller's signal that a retry is worth attempting. +pub fn record_keyword(dialect_id: &str, keyword: &str) -> bool { + record(dialect_id, Some(keyword), None) +} + +/// Record a `format` value `dialect_id` rejected. +pub fn record_format(dialect_id: &str, format: &str) -> bool { + record(dialect_id, None, Some(format)) +} + +fn record(dialect_id: &str, keyword: Option<&str>, format: Option<&str>) -> bool { + with_store(|file| { + let entry = file.dialects.entry(dialect_id.to_string()).or_default(); + let mut changed = false; + if let Some(keyword) = keyword + && !entry.rejected_keywords.iter().any(|k| k == keyword) + { + entry.rejected_keywords.push(keyword.to_string()); + changed = true; + } + if let Some(format) = format + && !entry.rejected_formats.iter().any(|f| f == format) + { + entry.rejected_formats.push(format.to_string()); + changed = true; + } + (changed, changed) + }) +} + +/// Drop every learned quirk for a dialect. Exposed for tests and for a manual +/// reset after a provider fixes its validator. +pub fn forget(dialect_id: &str) { + with_store(|file| { + let removed = file.dialects.remove(dialect_id).is_some(); + ((), removed) + }) +} + +/// Drop the cached copy of the *active* store so a later read re-reads the +/// file, simulating a restart. Scoped to the active path so a test running in +/// parallel cannot evict another test's (or the real store's) cache. +#[doc(hidden)] +pub fn reset_cache_for_tests() { + let Some(path) = store_path() else { + return; + }; + cache() + .lock() + .unwrap_or_else(|e| e.into_inner()) + .remove(&path); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn learns_persists_and_forgets() { + let dir = tempfile::tempdir().unwrap(); + use_test_path(dir.path().join("schema-quirks.json")); + + assert!(learned_for("gemini").is_empty()); + + assert!(record_keyword("gemini", "propertyNames")); + // Learning the same thing twice is not news, so no retry is triggered. + assert!(!record_keyword("gemini", "propertyNames")); + assert!(record_format("openai", "uri")); + + let learned = learned_for("gemini"); + assert_eq!(learned.rejected_keywords, vec!["propertyNames".to_string()]); + assert!(learned.rejected_formats.is_empty()); + + // Survives a process restart. + reset_cache_for_tests(); + assert_eq!( + learned_for("openai").rejected_formats, + vec!["uri".to_string()] + ); + + forget("openai"); + reset_cache_for_tests(); + assert!(learned_for("openai").is_empty()); + assert!(!learned_for("gemini").is_empty()); + } + + /// A store that cannot be written must degrade to "learn it again", never + /// to a failed turn. + #[test] + fn an_unwritable_store_still_reports_what_it_learned() { + use_test_path(PathBuf::from("/proc/definitely-not-writable/quirks.json")); + assert!(record_keyword("gemini", "somethingNew")); + assert_eq!( + learned_for("gemini").rejected_keywords, + vec!["somethingNew".to_string()] + ); + } +} diff --git a/crates/jcode-schema-dialect/src/registry.rs b/crates/jcode-schema-dialect/src/registry.rs new file mode 100644 index 0000000000..eb59b1b0ee --- /dev/null +++ b/crates/jcode-schema-dialect/src/registry.rs @@ -0,0 +1,252 @@ +//! The dialect table: what each provider is known to accept. +//! +//! Every entry is evidence-based. A keyword is listed as supported because +//! jcode has observed the provider accept it in a live request, not because the +//! spec says it should. That asymmetry is deliberate: the cost of omitting a +//! keyword the provider would have accepted is a slightly less expressive tool +//! schema, while the cost of listing one it rejects is every request failing. + +use crate::dialect::{DialectSpec, DialectTransforms}; + +/// Annotations and validation keywords the OpenAI function-parameters subset +/// accepts. Derived from the deny-list this replaces (#543, #687, #711, #713) +/// inverted against the keywords jcode and common MCP servers actually emit. +pub const OPENAI: DialectSpec = DialectSpec { + id: "openai", + supported_keywords: &[ + "description", + "default", + "examples", + "format", + "anyOf", + "oneOf", + "allOf", + "$defs", + "definitions", + "$ref", + "$schema", + "const", + "additionalProperties", + "patternProperties", + "prefixItems", + "minimum", + "maximum", + "exclusiveMinimum", + "exclusiveMaximum", + "multipleOf", + "minLength", + "maxLength", + "pattern", + "minItems", + "maxItems", + "nullable", + ], + // #543: unknown formats such as `uri` fail the strict validator. + supported_string_formats: &[ + "date-time", + "time", + "date", + "duration", + "email", + "hostname", + "ipv4", + "ipv6", + "uuid", + ], + transforms: DialectTransforms { + one_of_as_any_of: true, + ..DEFAULT_TRANSFORMS + }, +}; + +/// Gemini `generateContent` accepts an OpenAPI 3.0 subset for +/// `function_declarations.parameters`, which is much narrower than OpenAI's: +/// draft keywords like `$ref`/`$defs`/`additionalProperties`/`propertyNames` +/// are rejected with HTTP 400 (#754), and it validates `required` against the +/// same object's `properties` (#655). +pub const GEMINI: DialectSpec = DialectSpec { + id: "gemini", + supported_keywords: &[ + "description", + "default", + "format", + "anyOf", + "nullable", + "minimum", + "maximum", + "minItems", + "maxItems", + "minLength", + "maxLength", + "pattern", + "example", + ], + supported_string_formats: &[], + transforms: DialectTransforms { + prune_dangling_required: true, + const_as_enum: true, + one_of_as_any_of: true, + ..DEFAULT_TRANSFORMS + }, +}; + +/// Anthropic accepts full draft 2020-12 inside properties but rejects +/// combiners at the input schema's top level. +pub const ANTHROPIC: DialectSpec = DialectSpec { + id: "anthropic", + supported_keywords: &[ + "description", + "default", + "examples", + "format", + "anyOf", + "oneOf", + "allOf", + "not", + "if", + "then", + "else", + "$defs", + "definitions", + "$ref", + "$schema", + "$comment", + "const", + "additionalProperties", + "patternProperties", + "propertyNames", + "prefixItems", + "contains", + "uniqueItems", + "minimum", + "maximum", + "exclusiveMinimum", + "exclusiveMaximum", + "multipleOf", + "minLength", + "maxLength", + "pattern", + "minItems", + "maxItems", + "minProperties", + "maxProperties", + "dependentRequired", + "dependentSchemas", + "nullable", + ], + supported_string_formats: &[], + transforms: DialectTransforms { + flatten_top_level_combiners: true, + ..DEFAULT_TRANSFORMS + }, +}; + +/// OpenRouter forwards to whichever upstream serves the model, so it must +/// satisfy the strictest of them: Anthropic-family top-level combiner +/// rejection (#495) plus LM Studio's demand for `properties` (#446). +pub const OPENROUTER: DialectSpec = DialectSpec { + id: "openrouter", + supported_keywords: OPENAI.supported_keywords, + supported_string_formats: &[], + transforms: DialectTransforms { + flatten_top_level_combiners: true, + require_properties_on_objects: true, + ..DEFAULT_TRANSFORMS + }, +}; + +/// The Antigravity Cloud Code backend translating Gemini schemas to Anthropic, +/// which rejects combiners at *any* depth. +pub const ANTIGRAVITY_CLAUDE: DialectSpec = DialectSpec { + id: "antigravity-claude", + supported_keywords: ANTHROPIC.supported_keywords, + supported_string_formats: &[], + transforms: DialectTransforms { + flatten_all_combiners: true, + ..DEFAULT_TRANSFORMS + }, +}; + +/// Antigravity's OpenAI-compatible bridge (gpt-oss and friends), which +/// round-trips numeric bounds through a protobuf `int64` and then rejects the +/// string it produced, so those bounds must not be sent at all. +pub const ANTIGRAVITY_BRIDGE: DialectSpec = DialectSpec { + id: "antigravity-bridge", + supported_keywords: &[ + "description", + "default", + "examples", + "format", + "anyOf", + "oneOf", + "allOf", + "$defs", + "definitions", + "$ref", + "const", + "additionalProperties", + "patternProperties", + "minimum", + "maximum", + "pattern", + "nullable", + ], + supported_string_formats: &[], + transforms: DialectTransforms { + flatten_all_combiners: true, + ..DEFAULT_TRANSFORMS + }, +}; + +const DEFAULT_TRANSFORMS: DialectTransforms = DialectTransforms { + flatten_top_level_combiners: false, + flatten_all_combiners: false, + require_properties_on_objects: false, + prune_dangling_required: false, + const_as_enum: false, + one_of_as_any_of: false, +}; + +/// Every registered dialect, for conformance sweeps. +pub const ALL: &[&DialectSpec] = &[ + &OPENAI, + &GEMINI, + &ANTHROPIC, + &OPENROUTER, + &ANTIGRAVITY_CLAUDE, + &ANTIGRAVITY_BRIDGE, +]; + +/// Look up a dialect by its stable id (used by the quirk store and CLI). +pub fn by_id(id: &str) -> Option<&'static DialectSpec> { + ALL.iter().copied().find(|spec| spec.id == id) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn every_registered_dialect_is_coherent() { + for spec in ALL { + spec.validate().unwrap_or_else(|err| panic!("{err}")); + } + } + + #[test] + fn dialect_ids_are_unique() { + let mut ids: Vec<&str> = ALL.iter().map(|s| s.id).collect(); + ids.sort_unstable(); + let before = ids.len(); + ids.dedup(); + assert_eq!(before, ids.len(), "duplicate dialect id"); + } + + #[test] + fn lookup_round_trips() { + for spec in ALL { + assert_eq!(by_id(spec.id).map(|s| s.id), Some(spec.id)); + } + assert!(by_id("nope").is_none()); + } +} diff --git a/crates/jcode-schema-dialect/src/rejection.rs b/crates/jcode-schema-dialect/src/rejection.rs new file mode 100644 index 0000000000..6e8c98c5fd --- /dev/null +++ b/crates/jcode-schema-dialect/src/rejection.rs @@ -0,0 +1,218 @@ +//! Recognizing a schema rejection in a provider's error text. +//! +//! The allow-list in [`crate::registry`] shrinks the blast radius of an unknown +//! construct, but it cannot be complete: a provider may reject something jcode +//! believed was fine. When that happens the failure is currently a hard 400 the +//! user reports as a GitHub issue days later. Parsing the error instead lets the +//! same turn recover, and lets jcode report exactly which keyword to add. +//! +//! The three shapes below are verbatim from the filed issues, so the parser is +//! tested against real provider output rather than invented strings. + +use serde_json::Value; + +/// What a provider objected to. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct SchemaRejection { + /// The offending JSON Schema keyword, when the provider names one. + pub keyword: Option, + /// The offending `format` value, when the provider names one. + pub format: Option, + /// The tool whose schema was rejected, when the provider names one. + pub tool: Option, +} + +impl SchemaRejection { + /// Whether this rejection identifies something actionable to strip. A + /// rejection we cannot attribute is not worth retrying: the retry would + /// send a byte-identical request. + pub fn is_actionable(&self) -> bool { + self.keyword.is_some() || self.format.is_some() + } +} + +/// Extract a schema rejection from a provider error message. +/// +/// Returns `None` for errors that are not about tool schemas, so callers can +/// use this as the retry predicate directly. +pub fn classify(message: &str) -> Option { + let tool = extract_tool(message); + + // Gemini / Antigravity (#754): + // Invalid JSON payload received. Unknown name "propertyNames" at + // 'request.tools[0].function_declarations[32].parameters.properties[0].value': + // Cannot find field. + if message.contains("Cannot find field") + && let Some(name) = extract_quoted_after(message, "Unknown name") + { + return Some(SchemaRejection { + keyword: Some(name), + format: None, + tool, + }); + } + + // OpenAI (#543): 'uri' is not a valid format. + if let Some(format) = extract_quoted_before(message, "is not a valid format") { + return Some(SchemaRejection { + keyword: None, + format: Some(format), + tool, + }); + } + + // OpenAI (#687): 'uniqueItems' is not permitted. + if let Some(keyword) = extract_quoted_before(message, "is not permitted") { + return Some(SchemaRejection { + keyword: Some(keyword), + format: None, + tool, + }); + } + + // Gemini (#655): required fields ['label'] are not defined in the schema + // properties. Structural, not a keyword, but still a schema rejection the + // caller can act on by re-normalizing with pruning enabled. + if message.contains("are not defined in the schema properties") { + return Some(SchemaRejection { + keyword: Some("required".to_string()), + format: None, + tool, + }); + } + + // Anthropic / OpenRouter (#495) and the Antigravity Claude bridge. + if message.contains("does not support oneOf, allOf, or anyOf") + || (message.contains("input_schema") && message.contains("JSON Schema draft 2020-12")) + { + return Some(SchemaRejection { + keyword: Some("anyOf".to_string()), + format: None, + tool, + }); + } + + None +} + +/// Whether an error is *about* tool schemas at all, even if unattributable. +/// Used to label a hard failure with a useful hint instead of a raw 400. +pub fn is_schema_error(message: &str) -> bool { + if classify(message).is_some() { + return true; + } + let lowered = message.to_ascii_lowercase(); + lowered.contains("invalid_function_parameters") + || lowered.contains("invalid schema for function") + || (lowered.contains("function_declarations") && lowered.contains("invalid")) +} + +fn extract_tool(message: &str) -> Option { + // OpenAI: Invalid schema for function 'mcp__firecrawl__firecrawl_map': ... + extract_quoted_after(message, "for function") +} + +/// Trim the quoting artifacts left by a provider that embedded a JSON document +/// inside its own error string, which is how the real Antigravity 400 arrives +/// (`Unknown name \"propertyNames\"`). +fn clean_token(token: &str) -> Option { + let token = token.trim().trim_matches('\\').trim(); + (!token.is_empty()).then(|| token.to_string()) +} + +/// The first single- or double-quoted token appearing after `marker`. +fn extract_quoted_after(message: &str, marker: &str) -> Option { + let start = message.find(marker)? + marker.len(); + let rest = &message[start..]; + let quote = rest.find(['\'', '"'])?; + let quote_char = rest[quote..].chars().next()?; + let after = &rest[quote + quote_char.len_utf8()..]; + let end = after.find(quote_char)?; + clean_token(&after[..end]) +} + +/// The last single- or double-quoted token appearing before `marker`. +fn extract_quoted_before(message: &str, marker: &str) -> Option { + let end = message.find(marker)?; + let head = message[..end].trim_end(); + let close = head.rfind(['\'', '"'])?; + let quote_char = head[close..].chars().next()?; + let open = head[..close].rfind(quote_char)?; + clean_token(&head[open + quote_char.len_utf8()..close]) +} + +/// Whether a schema still contains the construct a provider rejected. Used to +/// avoid a pointless retry when the rejection names something jcode did not +/// send (which would otherwise loop). +pub fn schema_contains_keyword(schema: &Value, keyword: &str) -> bool { + match schema { + Value::Object(map) => map + .iter() + .any(|(key, value)| key == keyword || schema_contains_keyword(value, keyword)), + Value::Array(items) => items.iter().any(|i| schema_contains_keyword(i, keyword)), + _ => false, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_the_real_gemini_unknown_name_400() { + let message = r#"Antigravity generateContent failed (HTTP 400 Bad Request): "Invalid JSON payload received. Unknown name \"propertyNames\" at 'request.tools[0].function_declarations[32].parameters.properties[0].value': Cannot find field.""#; + let rejection = classify(message).expect("recognized"); + assert_eq!(rejection.keyword.as_deref(), Some("propertyNames")); + assert!(rejection.is_actionable()); + } + + #[test] + fn parses_the_real_openai_unsupported_format_400() { + let message = "invalid_request_error (invalid_function_parameters): Invalid schema for function 'mcp__firecrawl__firecrawl_map': In context=('properties', 'url'), 'uri' is not a valid format."; + let rejection = classify(message).expect("recognized"); + assert_eq!(rejection.format.as_deref(), Some("uri")); + assert_eq!( + rejection.tool.as_deref(), + Some("mcp__firecrawl__firecrawl_map") + ); + } + + #[test] + fn parses_the_real_openai_unpermitted_keyword_400() { + let message = "invalid_request_error (invalid_function_parameters): Invalid schema for function 'mcp__tubealfred__youtube_channels_batch': In context=('properties', 'ids'), 'uniqueItems' is not permitted."; + let rejection = classify(message).expect("recognized"); + assert_eq!(rejection.keyword.as_deref(), Some("uniqueItems")); + } + + #[test] + fn parses_the_real_gemini_dangling_required_400() { + let message = "GenerateContentRequest.tools[0].function_declarations[3].parameters: required fields ['label'] are not defined in the schema properties"; + assert_eq!( + classify(message).unwrap().keyword.as_deref(), + Some("required") + ); + } + + #[test] + fn parses_the_real_anthropic_top_level_combiner_400() { + let message = "input_schema does not support oneOf, allOf, or anyOf at the top level"; + assert_eq!(classify(message).unwrap().keyword.as_deref(), Some("anyOf")); + } + + #[test] + fn ignores_errors_that_are_not_about_schemas() { + assert!(classify("HTTP 429 Too Many Requests").is_none()); + assert!(classify("Function call is missing a thought_signature").is_none()); + assert!(!is_schema_error("connection reset by peer")); + } + + #[test] + fn detects_a_keyword_nested_anywhere() { + let schema = serde_json::json!({ + "type": "object", + "properties": { "data": { "propertyNames": { "type": "string" } } } + }); + assert!(schema_contains_keyword(&schema, "propertyNames")); + assert!(!schema_contains_keyword(&schema, "uniqueItems")); + } +} From f03d2c7db2a9adf0e66afe3e3cde5115006119b4 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:42:09 -0700 Subject: [PATCH 11/19] fix(test): repair four build and test gates that were failing on master 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 c98c9cc61 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. --- .../src/server/client_lifecycle_tests.rs | 14 +++++ crates/jcode-app-core/src/tool/todo.rs | 12 ++-- src/cli/commands/menubar.rs | 2 +- src/cli/commands_tests.rs | 56 ++++++++++++------- 4 files changed, 58 insertions(+), 26 deletions(-) diff --git a/crates/jcode-app-core/src/server/client_lifecycle_tests.rs b/crates/jcode-app-core/src/server/client_lifecycle_tests.rs index 0afa2177d1..4faae5a713 100644 --- a/crates/jcode-app-core/src/server/client_lifecycle_tests.rs +++ b/crates/jcode-app-core/src/server/client_lifecycle_tests.rs @@ -288,6 +288,14 @@ async fn cancel_without_local_task_still_signals_session_control() { soft_interrupt_queue, stop_signal.clone(), ); + // The point of this path is a turn this connection does not own (attach + // after reload, server-initiated turn). Without a registered active turn + // the cancel is a deliberate no-op, because arming the signal with nothing + // running only kills the *next* message. + let _active_turn = crate::turn_cancel_registry::register_active_turn( + "session_detached_cancel", + InterruptSignal::new(), + ); let (client_event_tx, mut client_event_rx) = mpsc::unbounded_channel::(); let swarm_members = Arc::new(RwLock::new(HashMap::new())); let swarms_by_id = Arc::new(RwLock::new(HashMap::new())); @@ -348,6 +356,12 @@ async fn deferred_cancel_reset_does_not_erase_newer_cancel() { Arc::clone(&soft_interrupt_queue), stop_signal.clone(), ); + // A turn owned by another connection is what makes this the signalling + // path rather than the idle no-op; see the sibling test. + let _active_turn = crate::turn_cancel_registry::register_active_turn( + "session_detached_cancel_race", + InterruptSignal::new(), + ); let (client_event_tx, _client_event_rx) = mpsc::unbounded_channel::(); let swarm_members = Arc::new(RwLock::new(HashMap::new())); let swarms_by_id = Arc::new(RwLock::new(HashMap::new())); diff --git a/crates/jcode-app-core/src/tool/todo.rs b/crates/jcode-app-core/src/tool/todo.rs index bb99f49829..5a6e4d5be1 100644 --- a/crates/jcode-app-core/src/tool/todo.rs +++ b/crates/jcode-app-core/src/tool/todo.rs @@ -679,7 +679,7 @@ impl Tool for TodoTool { "understands_user_intent": { "type": "string", "enum": ["uncertain", "partial", "clear", "complete"], - "description": "How well you understand what the user actually wants. Report uncertain or partial when guessing at intent." + "description": "How well you understand what the user wants. Report uncertain or partial when guessing." } } }, @@ -716,16 +716,16 @@ impl Tool for TodoTool { "autonomy": { "type": "string", "enum": ["requested_only", "necessary_followthrough", "proactive", "stewardship"], - "description": "How far beyond the literal request the work extended. Assess honestly from completed work and consequential adjacent follow-through." + "description": "How far beyond the literal request the work went. Assess from what was completed." }, "iteration_maturity": { "type": "string", "enum": ["not_started", "exploring", "improving", "plateau_unproven", "outcome_reached", "constraints_exhausted", "plateau_confirmed", "budget_exhausted"], - "description": "How far the feedback loop has actually been exercised and the evidence-based reason, if any, that further iteration should stop. Assess the current state honestly rather than predicting a future result." + "description": "How far the feedback loop was actually exercised, and any evidence-based reason to stop iterating." }, "stopping_evidence": { "type": "string", - "description": "Concrete evidence supporting the reported iteration maturity when a stopping claim is made. Name relevant attempts, observations, remaining hypotheses, or an actual constraint or budget." + "description": "Evidence for the reported iteration_maturity: attempts, observations, or a real budget limit." } } } @@ -918,8 +918,8 @@ mod tests { .get("description") .and_then(Value::as_str) .expect("alignment score should describe representation coverage"); - assert!(alignment_description.contains("what the user actually wants")); - assert!(alignment_description.contains("when guessing at intent")); + assert!(alignment_description.contains("what the user wants")); + assert!(alignment_description.contains("when guessing")); // The detailed calibration rubric moved out of the always-on schema // into deferred turn-finish continuation messages, which are paid only // when the completed turn needs another quality pass. diff --git a/src/cli/commands/menubar.rs b/src/cli/commands/menubar.rs index 760f318d4b..fb90044c38 100644 --- a/src/cli/commands/menubar.rs +++ b/src/cli/commands/menubar.rs @@ -866,7 +866,7 @@ mod tests { priority: "high".to_string(), id: "menubar-label".to_string(), group: Some("Meaningful menu labels".to_string()), - confidence: Some(95), + confidence: Some(crate::todo::ConfidenceState::Plausible), completion_confidence: None, confidence_history: Vec::new(), blocked_by: Vec::new(), diff --git a/src/cli/commands_tests.rs b/src/cli/commands_tests.rs index 81251e0d50..8e32c00e19 100644 --- a/src/cli/commands_tests.rs +++ b/src/cli/commands_tests.rs @@ -4,6 +4,7 @@ use crate::message::{Message, StreamEvent, ToolDefinition}; use crate::provider::ModelRoute; use crate::provider::{EventStream, Provider}; use crate::tool::Registry; +use crate::todo::ConfidenceState; use async_trait::async_trait; use std::io::{Read, Write}; use std::sync::Arc; @@ -126,6 +127,12 @@ fn test_parse_tailscale_dns_name_invalid_json() { #[test] fn configured_auth_test_targets_only_include_configured_supported_providers() { let _guard = crate::storage::lock_test_env(); + // OpenRouter has no OAuth state to set: its availability is read straight + // from `OPENROUTER_API_KEY` (or `openrouter.env`). Setting it here is what + // makes the expectation below independent of whoever runs the test having + // a key exported. + let saved_openrouter_key = std::env::var("OPENROUTER_API_KEY").ok(); + crate::env::set_var("OPENROUTER_API_KEY", "test-key"); let status = AuthStatus { anthropic: ProviderAuth { @@ -139,11 +146,17 @@ fn configured_auth_test_targets_only_include_configured_supported_providers() { google: AuthState::Expired, copilot: AuthState::Available, cursor: AuthState::NotConfigured, + openrouter: AuthState::Available, ..AuthStatus::default() }; let targets = configured_auth_test_targets(&status); + match saved_openrouter_key { + Some(key) => crate::env::set_var("OPENROUTER_API_KEY", key), + None => crate::env::remove_var("OPENROUTER_API_KEY"), + } + assert!(targets.contains(&ResolvedAuthTestTarget::Detailed(AuthTestTarget::Claude))); assert!(targets.contains(&ResolvedAuthTestTarget::Detailed(AuthTestTarget::Copilot))); assert!(targets.contains(&ResolvedAuthTestTarget::Detailed(AuthTestTarget::Gemini))); @@ -251,8 +264,8 @@ fn test_todo( id: &str, status: &str, priority: &str, - confidence: Option, - completion_confidence: Option, + confidence: Option, + completion_confidence: Option, ) -> crate::todo::TodoItem { crate::todo::TodoItem { id: id.to_string(), @@ -268,8 +281,8 @@ fn test_todo( #[test] fn run_auto_poke_followup_targets_below_threshold_todos() { let todos = vec![ - test_todo("a", "completed", "high", Some(90), Some(90)), - test_todo("b", "completed", "low", Some(80), Some(80)), + test_todo("a", "completed", "high", Some(ConfidenceState::Plausible), Some(ConfidenceState::Plausible)), + test_todo("b", "completed", "low", Some(ConfidenceState::Plausible), Some(ConfidenceState::Plausible)), ]; let followup = build_run_auto_poke_follow_up_from_todos(&todos, false, None); @@ -292,8 +305,8 @@ fn run_auto_poke_followup_targets_below_threshold_todos() { #[test] fn run_auto_poke_followup_challenges_abrupt_confidence_once() { - let mut todo = test_todo("a", "completed", "high", Some(0), Some(100)); - todo.confidence_history = vec![0, 100]; + let mut todo = test_todo("a", "completed", "high", Some(ConfidenceState::Speculative), Some(ConfidenceState::Verified)); + todo.confidence_history = vec![ConfidenceState::Speculative, ConfidenceState::Verified]; let todos = [todo]; match build_run_auto_poke_follow_up_from_todos(&todos, false, None) { @@ -319,11 +332,16 @@ fn run_auto_poke_followup_silent_when_confident_and_earned() { // summary anyway; now we spend no tokens and end the run. let todos = vec![ { - let mut todo = test_todo("a", "completed", "high", Some(100), Some(100)); - todo.confidence_history = vec![70, 80, 90, 100]; + let mut todo = test_todo("a", "completed", "high", Some(ConfidenceState::Verified), Some(ConfidenceState::Verified)); + todo.confidence_history = vec![ + ConfidenceState::Plausible, + ConfidenceState::Plausible, + ConfidenceState::Validated, + ConfidenceState::Verified, + ]; todo }, - test_todo("b", "completed", "low", Some(98), Some(98)), + test_todo("b", "completed", "low", Some(ConfidenceState::Validated), Some(ConfidenceState::Validated)), ]; assert!(build_run_auto_poke_follow_up_from_todos(&todos, false, None).is_none()); } @@ -331,8 +349,8 @@ fn run_auto_poke_followup_silent_when_confident_and_earned() { #[test] fn run_auto_poke_followup_prioritizes_incomplete_todos() { let todos = vec![ - test_todo("a", "completed", "high", Some(95), Some(95)), - test_todo("b", "in_progress", "medium", Some(80), None), + test_todo("a", "completed", "high", Some(ConfidenceState::Plausible), Some(ConfidenceState::Plausible)), + test_todo("b", "in_progress", "medium", Some(ConfidenceState::Plausible), None), ]; let followup = build_run_auto_poke_follow_up_from_todos(&todos, false, None); @@ -353,7 +371,7 @@ fn run_auto_poke_followup_prioritizes_incomplete_todos() { /// the deferred quality review must reach that path too, not only the TUI. #[test] fn run_auto_poke_delivers_the_deferred_gate_digest_before_confidence() { - let todos = vec![test_todo("a", "completed", "high", Some(80), Some(80))]; + let todos = vec![test_todo("a", "completed", "high", Some(ConfidenceState::Plausible), Some(ConfidenceState::Plausible))]; // Without a digest, the confidence gate is what fires. assert!(matches!( build_run_auto_poke_follow_up_from_todos(&todos, false, None), @@ -377,7 +395,7 @@ fn run_auto_poke_delivers_the_deferred_gate_digest_before_confidence() { /// turn to actually end rather than interrupting mid-flight. #[test] fn run_auto_poke_prefers_incomplete_todos_over_the_gate_digest() { - let todos = vec![test_todo("a", "in_progress", "high", Some(80), None)]; + let todos = vec![test_todo("a", "in_progress", "high", Some(ConfidenceState::Plausible), None)]; assert!(matches!( build_run_auto_poke_follow_up_from_todos( &todos, @@ -405,12 +423,12 @@ fn open_todos_do_not_consume_the_pending_gate_digest() { &[crate::todo::GateObservation { kind: crate::todo::GateObservationKind::IntentUnderstanding, group: None, - score: Some(70), + state: Some("partial".to_string()), }], ) .expect("append"); - let open = vec![test_todo("a", "in_progress", "high", Some(80), None)]; + let open = vec![test_todo("a", "in_progress", "high", Some(ConfidenceState::Plausible), None)]; assert!(matches!( build_run_auto_poke_follow_up_from_todos( &open, @@ -427,7 +445,7 @@ fn open_todos_do_not_consume_the_pending_gate_digest() { ); // Once the work closes, the reminder is still there to deliver. - let done = vec![test_todo("a", "completed", "high", Some(80), Some(100))]; + let done = vec![test_todo("a", "completed", "high", Some(ConfidenceState::Plausible), Some(ConfidenceState::Verified))]; match build_run_auto_poke_follow_up_from_todos( &done, false, @@ -466,7 +484,7 @@ fn take_run_gate_digest_consumes_the_log_and_respects_delivery() { &[crate::todo::GateObservation { kind: crate::todo::GateObservationKind::IntentUnderstanding, group: None, - score: Some(70), + state: Some("partial".to_string()), }], ) .expect("append"); @@ -493,7 +511,7 @@ fn take_run_gate_digest_consumes_the_log_and_respects_delivery() { #[test] fn run_auto_poke_followup_rechecks_completion_confidence_until_it_passes() { - let needs_validation = vec![test_todo("a", "completed", "high", Some(80), Some(80))]; + let needs_validation = vec![test_todo("a", "completed", "high", Some(ConfidenceState::Plausible), Some(ConfidenceState::Plausible))]; assert!(matches!( build_run_auto_poke_follow_up_from_todos(&needs_validation, false, None), Some(RunAutoPokeFollowUp::ConfidenceSummary { .. }) @@ -503,7 +521,7 @@ fn run_auto_poke_followup_rechecks_completion_confidence_until_it_passes() { Some(RunAutoPokeFollowUp::ConfidenceSummary { .. }) )); - let validated = vec![test_todo("a", "completed", "high", Some(80), Some(100))]; + let validated = vec![test_todo("a", "completed", "high", Some(ConfidenceState::Plausible), Some(ConfidenceState::Verified))]; assert!(matches!( build_run_auto_poke_follow_up_from_todos(&validated, false, None), Some(RunAutoPokeFollowUp::ConfidenceSummary { From 5e1804c33afe8f2a59e5177e0499d7192cb94c27 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:47:14 -0700 Subject: [PATCH 12/19] fix(schema): Antigravity's non-Gemini routes still speak the Gemini payload 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). --- .../jcode-provider-gemini-runtime/Cargo.toml | 4 + .../tests/mcp_schema_end_to_end.rs | 168 ++++++++++++++++++ crates/jcode-schema-dialect/src/registry.rs | 41 +++-- 3 files changed, 197 insertions(+), 16 deletions(-) create mode 100644 crates/jcode-provider-gemini-runtime/tests/mcp_schema_end_to_end.rs diff --git a/crates/jcode-provider-gemini-runtime/Cargo.toml b/crates/jcode-provider-gemini-runtime/Cargo.toml index 8be7c40f8b..a4d79d5ddb 100644 --- a/crates/jcode-provider-gemini-runtime/Cargo.toml +++ b/crates/jcode-provider-gemini-runtime/Cargo.toml @@ -28,6 +28,10 @@ tokio-stream = "0.1" uuid = { version = "1", features = ["v4"] } [dev-dependencies] +jcode-provider-antigravity = { path = "../jcode-provider-antigravity" } +jcode-provider-openai = { path = "../jcode-provider-openai" } +jcode-schema-dialect = { path = "../jcode-schema-dialect" } +serde = { version = "1", features = ["derive"] } # The migrated gemini tests use jcode-base's test-env sandbox (lock_test_env). jcode-base = { path = "../jcode-base", default-features = false, features = ["test-support"] } tempfile = "3" diff --git a/crates/jcode-provider-gemini-runtime/tests/mcp_schema_end_to_end.rs b/crates/jcode-provider-gemini-runtime/tests/mcp_schema_end_to_end.rs new file mode 100644 index 0000000000..b154ea6ea0 --- /dev/null +++ b/crates/jcode-provider-gemini-runtime/tests/mcp_schema_end_to_end.rs @@ -0,0 +1,168 @@ +//! End-to-end: a hostile MCP tool schema must reach every provider's wire +//! format in a form that provider accepts. +//! +//! The unit tests in `jcode-schema-dialect` work on schema values, and the +//! sweep in `jcode-app-core` works on jcode's own tools. Neither covers the +//! path that actually broke in #754: a third-party MCP server's `inputSchema` +//! is deserialized verbatim, becomes a `ToolDefinition`, and is serialized into +//! a provider request. This starts from the raw JSON an MCP server puts on the +//! wire and asserts on the bytes jcode sends to the provider. + +use jcode_base::mcp::McpToolDef; +use jcode_message_types::ToolDefinition; + +/// Verbatim `tools/list` payload from `@playwright/mcp`, the server that took +/// Antigravity + Gemini down in #754. `browser_drop` declares `propertyNames`, +/// which `generateContent` rejects with HTTP 400 for the whole request. +const PLAYWRIGHT_TOOLS_LIST: &str = r#"{ + "tools": [ + { + "name": "browser_drop", + "description": "Drop data onto an element", + "inputSchema": { + "type": "object", + "properties": { + "element": { "type": "string", "description": "Human-readable element description" }, + "data": { + "type": "object", + "additionalProperties": { "type": "string" }, + "propertyNames": { "type": "string" }, + "description": "Data to drop, as a map of MIME type to string value" + } + }, + "required": ["element", "data"] + } + } + ] +}"#; + +fn playwright_tool_definitions() -> Vec { + #[derive(serde::Deserialize)] + struct ToolsList { + tools: Vec, + } + + let listed: ToolsList = serde_json::from_str(PLAYWRIGHT_TOOLS_LIST).expect("MCP tools/list"); + listed + .tools + .into_iter() + .map(|tool| ToolDefinition { + name: format!("mcp__playwright__{}", tool.name), + description: tool.description.unwrap_or_default(), + input_schema: tool.input_schema, + }) + .collect() +} + +/// Recursively search serialized request JSON for a key, the way a provider's +/// validator does when it reports "Unknown name X at ...". +fn contains_key(value: &serde_json::Value, key: &str) -> bool { + match value { + serde_json::Value::Object(map) => map + .iter() + .any(|(k, v)| k == key || contains_key(v, key)), + serde_json::Value::Array(items) => items.iter().any(|i| contains_key(i, key)), + _ => false, + } +} + +#[test] +fn playwright_schema_reaches_gemini_without_the_keyword_that_caused_issue_754() { + let defs = playwright_tool_definitions(); + let built = jcode_provider_gemini::build_tools(&defs).expect("gemini tools"); + let wire = serde_json::to_value(&built).expect("serialize gemini tools"); + + assert!( + !contains_key(&wire, "propertyNames"), + "the keyword that 400s generateContent is still on the wire: {wire}" + ); + // `additionalProperties` is rejected by the same endpoint and was already + // stripped before #754; assert it too so a dialect edit cannot regress it. + assert!(!contains_key(&wire, "additionalProperties"), "{wire}"); + + // The tool is still usable: both parameters, their types, and the + // prompt-visible descriptions survive. + let parameters = &built[0].function_declarations[0].parameters; + assert_eq!(parameters["properties"]["element"]["type"], "string"); + assert_eq!(parameters["properties"]["data"]["type"], "object"); + assert_eq!( + parameters["properties"]["data"]["description"], + "Data to drop, as a map of MIME type to string value" + ); + assert_eq!( + parameters["required"], + serde_json::json!(["element", "data"]) + ); +} + +#[test] +fn playwright_schema_reaches_every_antigravity_route_cleanly() { + let defs = playwright_tool_definitions(); + let schema = &defs[0].input_schema; + + // The route that actually reported #754, plus the sibling upstreams that + // the same request path can be dispatched to. + for model in ["gemini-3-flash", "claude-sonnet-4-5", "gpt-oss-120b"] { + let normalized = jcode_provider_antigravity::antigravity_compatible_schema(schema, model); + assert!( + !contains_key(&normalized, "propertyNames"), + "model `{model}` still sends propertyNames: {normalized}" + ); + assert_eq!( + normalized["properties"]["element"]["description"], + "Human-readable element description", + "model `{model}` lost a description" + ); + } +} + +#[test] +fn playwright_schema_reaches_openai_without_an_unsupported_construct() { + let defs = playwright_tool_definitions(); + let built = jcode_provider_openai::request::build_tools(&defs); + let wire = serde_json::to_value(&built).expect("serialize openai tools"); + + // OpenAI's strict subset rejects `propertyNames` too (it is on the + // deny-list that #687 extended). + assert!(!contains_key(&wire, "propertyNames"), "{wire}"); + assert_eq!(wire[0]["name"], "mcp__playwright__browser_drop"); + assert_eq!( + wire[0]["parameters"]["properties"]["element"]["description"], + "Human-readable element description" + ); +} + +/// The conformance checker agrees with the concrete assertions above. If a +/// future dialect edit makes a schema unsendable, this reports which keyword +/// and where, rather than only that some assertion failed. +#[test] +fn the_normalized_playwright_schema_conforms_to_every_dialect() { + let defs = playwright_tool_definitions(); + let schema = &defs[0].input_schema; + + for spec in jcode_schema_dialect::registry::ALL { + let normalized = jcode_schema_dialect::dialect::apply(schema, spec); + let errors = + jcode_schema_dialect::must_not_contain_unsupported_constructs(&normalized, spec); + assert!( + errors.is_empty(), + "dialect `{}` would send an unacceptable schema:\n{}", + spec.id, + errors + .iter() + .map(ToString::to_string) + .collect::>() + .join("\n") + ); + let lost = jcode_schema_dialect::must_preserve_meaning(schema, &normalized); + assert!( + lost.is_empty(), + "dialect `{}` lost tool meaning:\n{}", + spec.id, + lost.iter() + .map(ToString::to_string) + .collect::>() + .join("\n") + ); + } +} diff --git a/crates/jcode-schema-dialect/src/registry.rs b/crates/jcode-schema-dialect/src/registry.rs index eb59b1b0ee..8b5c9196bd 100644 --- a/crates/jcode-schema-dialect/src/registry.rs +++ b/crates/jcode-schema-dialect/src/registry.rs @@ -155,45 +155,54 @@ pub const OPENROUTER: DialectSpec = DialectSpec { }, }; -/// The Antigravity Cloud Code backend translating Gemini schemas to Anthropic, -/// which rejects combiners at *any* depth. +/// Antigravity's Claude route: the Cloud Code backend translates the request +/// to Anthropic *after* parsing it. +/// +/// The keyword set is Gemini's, not Anthropic's, because every Antigravity +/// request is a `generateContent` payload whatever model it names. A keyword +/// outside the Gemini schema proto is rejected while parsing the payload +/// ("Invalid JSON payload received. Unknown name ...", #754), so it never +/// reaches the Anthropic translation that would have accepted it. On top of +/// that, the translation rejects combiners at any depth. pub const ANTIGRAVITY_CLAUDE: DialectSpec = DialectSpec { id: "antigravity-claude", - supported_keywords: ANTHROPIC.supported_keywords, + supported_keywords: GEMINI.supported_keywords, supported_string_formats: &[], transforms: DialectTransforms { flatten_all_combiners: true, + prune_dangling_required: true, + const_as_enum: true, + one_of_as_any_of: true, ..DEFAULT_TRANSFORMS }, }; -/// Antigravity's OpenAI-compatible bridge (gpt-oss and friends), which -/// round-trips numeric bounds through a protobuf `int64` and then rejects the -/// string it produced, so those bounds must not be sent at all. +/// Antigravity's OpenAI-compatible bridge (gpt-oss and friends). +/// +/// Gemini's payload subset again (see [`ANTIGRAVITY_CLAUDE`]), minus the +/// numeric bounds this bridge corrupts: it round-trips them through a protobuf +/// `int64`, which proto3 JSON re-encodes as a string, and then rejects the +/// string it just produced ("'10' is not of type 'integer'"). The bounds are +/// advisory, so dropping them costs nothing at call time. pub const ANTIGRAVITY_BRIDGE: DialectSpec = DialectSpec { id: "antigravity-bridge", supported_keywords: &[ "description", "default", - "examples", "format", "anyOf", - "oneOf", - "allOf", - "$defs", - "definitions", - "$ref", - "const", - "additionalProperties", - "patternProperties", + "nullable", "minimum", "maximum", "pattern", - "nullable", + "example", ], supported_string_formats: &[], transforms: DialectTransforms { flatten_all_combiners: true, + prune_dangling_required: true, + const_as_enum: true, + one_of_as_any_of: true, ..DEFAULT_TRANSFORMS }, }; From 2151cee535ab9acc5fc5293b885dd2d1a88d04d6 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:50:51 -0700 Subject: [PATCH 13/19] chore: lock jcode-schema-dialect dependency --- Cargo.lock | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index d5122c45d2..06669c1cfe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3407,6 +3407,7 @@ dependencies = [ "jcode-pdf", "jcode-plan", "jcode-provider-core", + "jcode-schema-dialect", "jcode-selfdev-types", "jcode-session-types", "jcode-setup-hints", @@ -3823,6 +3824,7 @@ version = "0.1.0" dependencies = [ "chrono", "jcode-provider-gemini", + "jcode-schema-dialect", "serde", "serde_json", ] @@ -3839,6 +3841,7 @@ dependencies = [ "jcode-provider-antigravity", "jcode-provider-core", "jcode-provider-gemini", + "jcode-schema-dialect", "reqwest 0.12.28", "serde_json", "tempfile", @@ -4004,6 +4007,7 @@ version = "0.1.0" dependencies = [ "anyhow", "jcode-message-types", + "jcode-schema-dialect", "serde", "serde_json", ] @@ -4017,8 +4021,11 @@ dependencies = [ "chrono", "jcode-base", "jcode-message-types", + "jcode-provider-antigravity", "jcode-provider-core", "jcode-provider-gemini", + "jcode-provider-openai", + "jcode-schema-dialect", "reqwest 0.12.28", "serde", "serde_json", @@ -4121,6 +4128,16 @@ dependencies = [ "unicode-width", ] +[[package]] +name = "jcode-schema-dialect" +version = "0.1.0" +dependencies = [ + "dirs", + "serde", + "serde_json", + "tempfile", +] + [[package]] name = "jcode-sdk" version = "0.1.0" From e31ebaad155d33cdea2ceb508b55e927d9b44380 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:56:30 -0700 Subject: [PATCH 14/19] feat(gemini): self-heal a tool-schema rejection on the native route too 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. --- .../jcode-provider-gemini-runtime/Cargo.toml | 2 +- .../jcode-provider-gemini-runtime/src/lib.rs | 73 ++++++++++++++++++- .../tests/mcp_schema_end_to_end.rs | 33 +++++++++ 3 files changed, 105 insertions(+), 3 deletions(-) diff --git a/crates/jcode-provider-gemini-runtime/Cargo.toml b/crates/jcode-provider-gemini-runtime/Cargo.toml index a4d79d5ddb..692026f368 100644 --- a/crates/jcode-provider-gemini-runtime/Cargo.toml +++ b/crates/jcode-provider-gemini-runtime/Cargo.toml @@ -20,6 +20,7 @@ jcode-base = { path = "../jcode-base", default-features = false } jcode-message-types = { path = "../jcode-message-types" } jcode-provider-core = { path = "../jcode-provider-core" } jcode-provider-gemini = { path = "../jcode-provider-gemini" } +jcode-schema-dialect = { path = "../jcode-schema-dialect" } reqwest = { version = "0.12", default-features = false, features = ["json", "stream", "charset", "http2", "system-proxy", "rustls-tls", "rustls-tls-native-roots"] } serde = { version = "1", features = ["derive"] } serde_json = "1" @@ -30,7 +31,6 @@ uuid = { version = "1", features = ["v4"] } [dev-dependencies] jcode-provider-antigravity = { path = "../jcode-provider-antigravity" } jcode-provider-openai = { path = "../jcode-provider-openai" } -jcode-schema-dialect = { path = "../jcode-schema-dialect" } serde = { version = "1", features = ["derive"] } # The migrated gemini tests use jcode-base's test-env sandbox (lock_test_env). jcode-base = { path = "../jcode-base", default-features = false, features = ["test-support"] } diff --git a/crates/jcode-provider-gemini-runtime/src/lib.rs b/crates/jcode-provider-gemini-runtime/src/lib.rs index 8ba32c4203..d2890b4fac 100644 --- a/crates/jcode-provider-gemini-runtime/src/lib.rs +++ b/crates/jcode-provider-gemini-runtime/src/lib.rs @@ -504,6 +504,50 @@ impl GeminiProvider { .context("Failed to parse Gemini operation response") } + /// Recover from a tool-schema rejection by learning what + /// `generateContent` refused and re-sending the turn without it. + /// + /// jcode advertises every tool on every request, so one construct the + /// endpoint dislikes 400s the whole session rather than one tool. The + /// historical fix was to append the keyword to a deny-list and ship a + /// release (#754, #655); this recovers in the same turn instead, and + /// `jcode-schema-dialect` remembers it so later requests never send it. + /// + /// Returns `None` when the error is not a recoverable schema rejection, + /// so the caller falls through to its normal error handling. The quirk + /// store reports a construct as newly-learned only once, which is what + /// bounds this to a single retry per distinct construct. + #[expect( + clippy::too_many_arguments, + reason = "mirrors generate_content so the retry re-sends an identical turn" + )] + async fn retry_after_schema_rejection( + &self, + error: &str, + state: &GeminiRuntimeState, + model: &str, + messages: &[Message], + tools: &[ToolDefinition], + system: &str, + resume_session_id: Option<&str>, + ) -> Option> { + let dialect = &jcode_schema_dialect::registry::GEMINI; + match jcode_schema_dialect::recover_from_error(error, dialect) { + jcode_schema_dialect::RecoveryAction::NotSchemaRelated => None, + jcode_schema_dialect::RecoveryAction::Unrecoverable { hint } => { + jcode_base::logging::warn(&format!("Gemini tool-schema rejection: {hint}")); + None + } + jcode_schema_dialect::RecoveryAction::RetryWithoutConstruct { description } => { + jcode_base::logging::warn(&format!("Gemini {description}")); + Some( + self.generate_content(state, model, messages, tools, system, resume_session_id) + .await, + ) + } + } + } + async fn generate_content( &self, state: &GeminiRuntimeState, @@ -727,8 +771,33 @@ impl Provider for GeminiProvider { } } Err(err) => { - let _ = tx.send(Err(err)).await; - return; + // A tool schema `generateContent` rejects 400s every turn, + // so the provider is unusable until jcode ships a new + // keyword. Learn the rejected construct from the error, + // persist it, and retry this turn without it. See + // `jcode-schema-dialect`. + match provider + .retry_after_schema_rejection( + &err.to_string(), + &state, + &model, + &messages, + &tools, + &system, + resume_session_id.as_deref(), + ) + .await + { + Some(Ok(response)) => response, + Some(Err(retry_err)) => { + let _ = tx.send(Err(retry_err)).await; + return; + } + None => { + let _ = tx.send(Err(err)).await; + return; + } + } } }; diff --git a/crates/jcode-provider-gemini-runtime/tests/mcp_schema_end_to_end.rs b/crates/jcode-provider-gemini-runtime/tests/mcp_schema_end_to_end.rs index b154ea6ea0..854cca6f16 100644 --- a/crates/jcode-provider-gemini-runtime/tests/mcp_schema_end_to_end.rs +++ b/crates/jcode-provider-gemini-runtime/tests/mcp_schema_end_to_end.rs @@ -166,3 +166,36 @@ fn the_normalized_playwright_schema_conforms_to_every_dialect() { ); } } + +/// The recovery layer must trigger on the error string the runtime actually +/// builds, not on the idealized provider sentence. +/// +/// `generate_content` wraps the HTTP body as "Gemini request {method} failed +/// (HTTP 400): {body}", where the body is the raw JSON error envelope with its +/// quotes backslash-escaped, and anyhow adds a "Caused by" layer on top. A +/// classifier matching only the clean provider text would compile, pass its own +/// unit tests, and never once fire in production. +#[test] +fn recovery_triggers_on_the_error_string_the_runtime_really_builds() { + let runtime_error = format!( + "Gemini request {} failed (HTTP {}): {}", + "generateContent", 400, GEMINI_400_BODY + ); + let with_context = format!( + "{runtime_error}\n\nCaused by:\n Gemini request to \ + https://cloudcode-pa.googleapis.com/v1internal:generateContent failed" + ); + + let rejection = jcode_schema_dialect::classify(&with_context) + .expect("the runtime's own error string must be recognized as a schema rejection"); + assert_eq!( + rejection.keyword.as_deref(), + Some("propertyNames"), + "recovery must extract the construct from the wrapped, escaped error" + ); +} + +/// Verbatim HTTP 400 envelope from the Antigravity/Gemini `generateContent` +/// endpoint as quoted in issue #754, including the backslash-escaped quotes +/// that survive into the error string. +const GEMINI_400_BODY: &str = r#"{"error":{"code":400,"message":"Invalid JSON payload received. Unknown name \"propertyNames\" at 'request.tools[0].function_declarations[32].parameters.properties[0].value': Cannot find field.","status":"INVALID_ARGUMENT"}}"#; From 1e29ed7d59c71e26d6f590c386c3c8fc01c82e51 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 17:04:20 -0700 Subject: [PATCH 15/19] fix macOS notification click session routing --- crates/jcode-tui/src/tui/app/turn_notify.rs | 127 +++++++++++++++++++- 1 file changed, 121 insertions(+), 6 deletions(-) diff --git a/crates/jcode-tui/src/tui/app/turn_notify.rs b/crates/jcode-tui/src/tui/app/turn_notify.rs index 98964f82d0..db106835ed 100644 --- a/crates/jcode-tui/src/tui/app/turn_notify.rs +++ b/crates/jcode-tui/src/tui/app/turn_notify.rs @@ -7,7 +7,10 @@ //! default it fires only while the terminal window is unfocused. use super::App; +use base64::Engine as _; use crate::todo::TodoItem; +#[cfg(target_os = "macos")] +use std::io::Write; /// Maximum characters of assistant text shown in the notification body. /// Notification banners truncate aggressively; keep the payload tight. @@ -65,12 +68,17 @@ impl App { ); let sound = cfg.turn_complete_sound.trim(); let sound = (!sound.is_empty()).then_some(sound); - crate::notifications::send_desktop_notification_rich( - ¬ification.title, - notification.subtitle.as_deref(), - ¬ification.body, - sound, - ); + if !send_originating_terminal_notification( + ¬ification, + self.active_client_session_id().unwrap_or("unknown"), + ) { + crate::notifications::send_desktop_notification_rich( + ¬ification.title, + notification.subtitle.as_deref(), + ¬ification.body, + sound, + ); + } } fn runtime_mode_allows_turn_notifications(&self) -> bool { @@ -87,6 +95,88 @@ impl App { } } +/// Ask the terminal to create the notification when it has a native protocol. +/// +/// On macOS, a notification emitted by `osascript` belongs to the helper +/// process, so clicking it cannot identify, much less focus, the terminal pane +/// that owns this session. Kitty and iTerm notifications retain that origin and +/// Notification Center consequently takes the user back to the exact window +/// and pane that emitted them. +#[cfg(target_os = "macos")] +fn send_originating_terminal_notification( + notification: &TurnNotification, + session_id: &str, +) -> bool { + let term_program = std::env::var("TERM_PROGRAM").unwrap_or_default(); + let term = std::env::var("TERM").unwrap_or_default(); + let sequence = if term_program.eq_ignore_ascii_case("kitty") || term == "xterm-kitty" { + kitty_notification_sequence(notification, session_id) + } else if term_program.eq_ignore_ascii_case("iTerm.app") { + iterm_notification_sequence(notification) + } else { + return false; + }; + + // This runs on the TUI event thread, after the completed turn has rendered, + // so one atomic write and flush cannot interleave with a frame draw. + let mut stdout = std::io::stdout().lock(); + stdout.write_all(sequence.as_bytes()).is_ok() && stdout.flush().is_ok() +} + +#[cfg(not(target_os = "macos"))] +fn send_originating_terminal_notification( + _notification: &TurnNotification, + _session_id: &str, +) -> bool { + false +} + +fn notification_text(notification: &TurnNotification) -> String { + match notification.subtitle.as_deref() { + Some(subtitle) => format!("{}\n{}", subtitle, notification.body), + None => notification.body.clone(), + } +} + +fn osc_safe(text: &str) -> String { + text.chars() + .filter(|ch| !ch.is_control()) + .collect() +} + +fn kitty_notification_id(session_id: &str) -> String { + let safe: String = session_id + .chars() + .filter(|ch| ch.is_ascii_alphanumeric() || matches!(ch, '_' | '-' | '+' | '.')) + .take(128) + .collect(); + format!("jcode-turn-{}", if safe.is_empty() { "unknown" } else { &safe }) +} + +fn kitty_notification_sequence(notification: &TurnNotification, session_id: &str) -> String { + // OSC 99 is Kitty's desktop-notification protocol. Notifications are tied + // to the originating Kitty window, which is what makes click-to-focus work. + // Base64 is required because the body can contain a newline and OSC 99's + // unencoded form forbids every C0/C1 control character. + let encoder = base64::engine::general_purpose::STANDARD; + let title = encoder.encode(notification.title.as_bytes()); + let body = encoder.encode(notification_text(notification).as_bytes()); + let id = kitty_notification_id(session_id); + format!( + "\x1b]99;i={id}:d=0:e=1:p=title;{title}\x1b\\\x1b]99;i={id}:d=1:e=1:p=body;{body}\x1b\\" + ) +} + +fn iterm_notification_sequence(notification: &TurnNotification) -> String { + // iTerm2's OSC 9 notification is likewise associated with its source tab. + let text = osc_safe(&format!( + "{}: {}", + notification.title, + notification_text(notification) + )); + format!("\x1b]9;{text}\x07") +} + fn load_session_todos(session_id: &str) -> Vec { crate::todo::load_todos(session_id).unwrap_or_default() } @@ -412,4 +502,29 @@ mod tests { assert_eq!(format_duration_compact(3600.0), "1h"); assert_eq!(format_duration_compact(3725.0), "1h 2m"); } + + #[test] + fn kitty_notification_is_one_completed_clickable_message() { + let n = TurnNotification { + title: "jcode ยท fox".to_string(), + subtitle: Some("2/3 todos".to_string()), + body: "Finished parser".to_string(), + }; + assert_eq!( + kitty_notification_sequence(&n, "session:fox/123"), + "\x1b]99;i=jcode-turn-sessionfox123:d=0:e=1:p=title;amNvZGUgwrcgZm94\x1b\\\x1b]99;i=jcode-turn-sessionfox123:d=1:e=1:p=body;Mi8zIHRvZG9zCkZpbmlzaGVkIHBhcnNlcg==\x1b\\" + ); + } + + #[test] + fn terminal_notification_payload_strips_osc_terminators() { + let n = TurnNotification { + title: "unsafe\x1b] title".to_string(), + subtitle: None, + body: "body\x07text".to_string(), + }; + let sequence = iterm_notification_sequence(&n); + assert_eq!(sequence.matches('\x07').count(), 1); + assert!(!sequence.contains("\x1b] title")); + } } From 4581aec38b17fde255e66f20a583fa39ab988429 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 17:07:05 -0700 Subject: [PATCH 16/19] fix(schema): learn every keyword a 400 names, not just the first 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. --- .../tests/mcp_schema_end_to_end.rs | 2 +- crates/jcode-schema-dialect/src/lib.rs | 10 +- crates/jcode-schema-dialect/src/rejection.rs | 114 +++++++++++++++--- 3 files changed, 104 insertions(+), 22 deletions(-) diff --git a/crates/jcode-provider-gemini-runtime/tests/mcp_schema_end_to_end.rs b/crates/jcode-provider-gemini-runtime/tests/mcp_schema_end_to_end.rs index 854cca6f16..0478fbfb58 100644 --- a/crates/jcode-provider-gemini-runtime/tests/mcp_schema_end_to_end.rs +++ b/crates/jcode-provider-gemini-runtime/tests/mcp_schema_end_to_end.rs @@ -189,7 +189,7 @@ fn recovery_triggers_on_the_error_string_the_runtime_really_builds() { let rejection = jcode_schema_dialect::classify(&with_context) .expect("the runtime's own error string must be recognized as a schema rejection"); assert_eq!( - rejection.keyword.as_deref(), + rejection.keyword(), Some("propertyNames"), "recovery must extract the construct from the wrapped, escaped error" ); diff --git a/crates/jcode-schema-dialect/src/lib.rs b/crates/jcode-schema-dialect/src/lib.rs index 888b85b593..d3d1e85aa9 100644 --- a/crates/jcode-schema-dialect/src/lib.rs +++ b/crates/jcode-schema-dialect/src/lib.rs @@ -97,7 +97,12 @@ pub fn recover_from_error(message: &str, spec: &DialectSpec) -> RecoveryAction { let mut learned_something = false; let mut described = Vec::new(); - if let Some(keyword) = rejection.keyword.as_deref() { + // Learn every keyword the response named, not just the first. A Gemini 400 + // reports one `fieldViolations` entry per bad keyword, so learning them one + // per turn would burn a failed request for each (observed live: a single + // response naming both `dependentRequired` and `unevaluatedItems`). + for keyword in &rejection.keywords { + let keyword = keyword.as_str(); // A load-bearing keyword cannot be dropped without destroying the // tool's meaning, so this is a real bug rather than a quirk to absorb. if !keyword::is_droppable(keyword) { @@ -145,7 +150,8 @@ pub fn recover_from_error(message: &str, spec: &DialectSpec) -> RecoveryAction { construct is not what actually failed", spec.id, rejection - .keyword + .keyword() + .map(ToString::to_string) .or(rejection.format) .unwrap_or_else(|| "a schema construct".into()) ) diff --git a/crates/jcode-schema-dialect/src/rejection.rs b/crates/jcode-schema-dialect/src/rejection.rs index 6e8c98c5fd..7ace23612b 100644 --- a/crates/jcode-schema-dialect/src/rejection.rs +++ b/crates/jcode-schema-dialect/src/rejection.rs @@ -14,8 +14,14 @@ use serde_json::Value; /// What a provider objected to. #[derive(Clone, Debug, PartialEq, Eq)] pub struct SchemaRejection { - /// The offending JSON Schema keyword, when the provider names one. - pub keyword: Option, + /// Every offending JSON Schema keyword the provider named. + /// + /// Plural because a real Gemini 400 reports a `fieldViolations` array and + /// names each bad keyword separately. Learning only the first would cost + /// one failed turn per bad keyword, so a schema with three of them would + /// look like the recovery layer was broken. Observed live: a single + /// response naming both `dependentRequired` and `unevaluatedItems`. + pub keywords: Vec, /// The offending `format` value, when the provider names one. pub format: Option, /// The tool whose schema was rejected, when the provider names one. @@ -23,11 +29,16 @@ pub struct SchemaRejection { } impl SchemaRejection { + /// The first named keyword, for callers that only need one. + pub fn keyword(&self) -> Option<&str> { + self.keywords.first().map(String::as_str) + } + /// Whether this rejection identifies something actionable to strip. A /// rejection we cannot attribute is not worth retrying: the retry would /// send a byte-identical request. pub fn is_actionable(&self) -> bool { - self.keyword.is_some() || self.format.is_some() + !self.keywords.is_empty() || self.format.is_some() } } @@ -42,20 +53,21 @@ pub fn classify(message: &str) -> Option { // Invalid JSON payload received. Unknown name "propertyNames" at // 'request.tools[0].function_declarations[32].parameters.properties[0].value': // Cannot find field. - if message.contains("Cannot find field") - && let Some(name) = extract_quoted_after(message, "Unknown name") - { - return Some(SchemaRejection { - keyword: Some(name), - format: None, - tool, - }); + if message.contains("Cannot find field") { + let names = extract_all_quoted_after(message, "Unknown name"); + if !names.is_empty() { + return Some(SchemaRejection { + keywords: names, + format: None, + tool, + }); + } } // OpenAI (#543): 'uri' is not a valid format. if let Some(format) = extract_quoted_before(message, "is not a valid format") { return Some(SchemaRejection { - keyword: None, + keywords: Vec::new(), format: Some(format), tool, }); @@ -64,7 +76,7 @@ pub fn classify(message: &str) -> Option { // OpenAI (#687): 'uniqueItems' is not permitted. if let Some(keyword) = extract_quoted_before(message, "is not permitted") { return Some(SchemaRejection { - keyword: Some(keyword), + keywords: vec![keyword], format: None, tool, }); @@ -75,7 +87,7 @@ pub fn classify(message: &str) -> Option { // caller can act on by re-normalizing with pruning enabled. if message.contains("are not defined in the schema properties") { return Some(SchemaRejection { - keyword: Some("required".to_string()), + keywords: vec!["required".to_string()], format: None, tool, }); @@ -86,7 +98,7 @@ pub fn classify(message: &str) -> Option { || (message.contains("input_schema") && message.contains("JSON Schema draft 2020-12")) { return Some(SchemaRejection { - keyword: Some("anyOf".to_string()), + keywords: vec!["anyOf".to_string()], format: None, tool, }); @@ -131,6 +143,26 @@ fn extract_quoted_after(message: &str, marker: &str) -> Option { clean_token(&after[..end]) } +/// Every distinct quoted token following any occurrence of `marker`. +/// +/// A Gemini 400 repeats "Unknown name X" once per `fieldViolations` entry, and +/// the top-level `message` duplicates the first one, so this both collects all +/// of them and deduplicates. +fn extract_all_quoted_after(message: &str, marker: &str) -> Vec { + let mut found: Vec = Vec::new(); + let mut cursor = 0usize; + while let Some(offset) = message[cursor..].find(marker) { + let start = cursor + offset; + if let Some(token) = extract_quoted_after(&message[start..], marker) + && !found.contains(&token) + { + found.push(token); + } + cursor = start + marker.len(); + } + found +} + /// The last single- or double-quoted token appearing before `marker`. fn extract_quoted_before(message: &str, marker: &str) -> Option { let end = message.find(marker)?; @@ -162,7 +194,7 @@ mod tests { fn parses_the_real_gemini_unknown_name_400() { let message = r#"Antigravity generateContent failed (HTTP 400 Bad Request): "Invalid JSON payload received. Unknown name \"propertyNames\" at 'request.tools[0].function_declarations[32].parameters.properties[0].value': Cannot find field.""#; let rejection = classify(message).expect("recognized"); - assert_eq!(rejection.keyword.as_deref(), Some("propertyNames")); + assert_eq!(rejection.keyword(), Some("propertyNames")); assert!(rejection.is_actionable()); } @@ -181,14 +213,14 @@ mod tests { fn parses_the_real_openai_unpermitted_keyword_400() { let message = "invalid_request_error (invalid_function_parameters): Invalid schema for function 'mcp__tubealfred__youtube_channels_batch': In context=('properties', 'ids'), 'uniqueItems' is not permitted."; let rejection = classify(message).expect("recognized"); - assert_eq!(rejection.keyword.as_deref(), Some("uniqueItems")); + assert_eq!(rejection.keyword(), Some("uniqueItems")); } #[test] fn parses_the_real_gemini_dangling_required_400() { let message = "GenerateContentRequest.tools[0].function_declarations[3].parameters: required fields ['label'] are not defined in the schema properties"; assert_eq!( - classify(message).unwrap().keyword.as_deref(), + classify(message).unwrap().keyword(), Some("required") ); } @@ -196,7 +228,7 @@ mod tests { #[test] fn parses_the_real_anthropic_top_level_combiner_400() { let message = "input_schema does not support oneOf, allOf, or anyOf at the top level"; - assert_eq!(classify(message).unwrap().keyword.as_deref(), Some("anyOf")); + assert_eq!(classify(message).unwrap().keyword(), Some("anyOf")); } #[test] @@ -215,4 +247,48 @@ mod tests { assert!(schema_contains_keyword(&schema, "propertyNames")); assert!(!schema_contains_keyword(&schema, "uniqueItems")); } + + /// Captured live from the Antigravity `generateContent` endpoint, not + /// transcribed from an issue: a single 400 whose `fieldViolations` array + /// names two different bad keywords, with the first also duplicated into + /// the top-level `message`. + /// + /// Learning only the first would spend one failed turn per bad keyword, + /// which is what this response actually did before the fix. + const LIVE_MULTI_VIOLATION_400: &str = r#"Antigravity generateContent failed (HTTP 400 Bad Request): { + "error": { + "code": 400, + "message": "Invalid JSON payload received. Unknown name \"dependentRequired\" at 'request.tools[0].function_declarations[14].parameters.properties[3].value': Cannot find field.", + "status": "INVALID_ARGUMENT", + "details": [ + { + "@type": "type.googleapis.com/google.rpc.BadRequest", + "fieldViolations": [ + { + "field": "request.tools[0].function_declarations[14].parameters.properties[3].value", + "description": "Invalid JSON payload received. Unknown name \"dependentRequired\" at 'request.tools[0].function_declarations[14].parameters.properties[3].value': Cannot find field." + }, + { + "field": "request.tools[0].function_declarations[14].parameters.properties[3].value", + "description": "Invalid JSON payload received. Unknown name \"unevaluatedItems\" at 'request.tools[0].function_declarations[14].parameters.properties[3].value': Cannot find field." + } + ] + } + ] + } +}"#; + + #[test] + fn every_keyword_in_a_multi_violation_400_is_learned_at_once() { + let rejection = classify(LIVE_MULTI_VIOLATION_400).expect("recognized"); + assert_eq!( + rejection.keywords, + vec![ + "dependentRequired".to_string(), + "unevaluatedItems".to_string() + ], + "both violations must be learned from one response, deduplicated" + ); + } + } From 373b576f1d52025676804455dd04d111e448e280 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 17:13:25 -0700 Subject: [PATCH 17/19] fix(openai): stop claiming strict for a schema OpenAI rejects (fixes #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. --- crates/jcode-app-core/src/tool/tests.rs | 9 +++ .../jcode-provider-core/src/openai_schema.rs | 78 +++++++++++++++++++ .../jcode-schema-dialect/src/conformance.rs | 58 ++++++++++++++ crates/jcode-schema-dialect/src/lib.rs | 1 + 4 files changed, 146 insertions(+) diff --git a/crates/jcode-app-core/src/tool/tests.rs b/crates/jcode-app-core/src/tool/tests.rs index 0d17cfd37a..e85d286e6a 100644 --- a/crates/jcode-app-core/src/tool/tests.rs +++ b/crates/jcode-app-core/src/tool/tests.rs @@ -1466,6 +1466,15 @@ async fn tool_schemas_are_sendable_to_every_provider_dialect() { assert!(!defs.is_empty(), "the sweep must not pass vacuously"); let mut failures = Vec::new(); + // Not per-dialect: no provider *rejects* a property that declares no type, + // but OpenAI refuses `strict` for the whole catalog over one (#713), so a + // built-in tool acquiring one would silently cost every OpenAI-route agent + // its structured-output guarantees. + for def in &defs { + for error in jcode_schema_dialect::untyped_properties(&def.input_schema) { + failures.push(format!("tool `{}` {error}", def.name)); + } + } for spec in jcode_schema_dialect::registry::ALL { for def in &defs { let normalized = jcode_schema_dialect::dialect::apply(&def.input_schema, spec); diff --git a/crates/jcode-provider-core/src/openai_schema.rs b/crates/jcode-provider-core/src/openai_schema.rs index dde20daeaf..b5231e1f2d 100644 --- a/crates/jcode-provider-core/src/openai_schema.rs +++ b/crates/jcode-provider-core/src/openai_schema.rs @@ -255,6 +255,17 @@ pub fn schema_supports_strict(schema: &Value) -> bool { return false; } } + // A declared property that says nothing about its type is legal JSON + // Schema (an empty schema accepts anything) and Anthropic takes it, but + // OpenAI's strict validator rejects the whole catalog over it (#713: + // cua-driver's `set_config.value`, whose type genuinely depends on + // `key`). Strict eligibility must fail closed here: the schema is still + // sent, just without `strict: true`, so the tool stays usable. + if let Some(Value::Object(properties)) = map.get("properties") + && properties.values().any(|property| !declares_a_type(property)) + { + return false; + } map.values().all(schema_supports_strict) } @@ -266,6 +277,22 @@ pub fn schema_supports_strict(schema: &Value) -> bool { } } +/// Whether a subschema says anything about what it accepts. +/// +/// `true` for a boolean schema: `true`/`false` are complete JSON Schemas whose +/// meaning is unambiguous, unlike an object that simply omits `type`. +fn declares_a_type(schema: &Value) -> bool { + let Some(map) = schema.as_object() else { + return schema.is_boolean(); + }; + const TYPE_BEARING_KEYWORDS: &[&str] = &[ + "type", "enum", "const", "anyOf", "oneOf", "allOf", "$ref", "properties", "items", + ]; + TYPE_BEARING_KEYWORDS + .iter() + .any(|keyword| map.contains_key(*keyword)) +} + fn schema_is_object_typed(map: &serde_json::Map) -> bool { match map.get("type") { Some(Value::String(t)) => t == "object", @@ -690,4 +717,55 @@ mod tests { ); assert_eq!(normalized["properties"]["not"]["type"], json!("string")); } + + /// Issue #713: `cua-driver`'s `set_config.value` declares a description and + /// no type, because its type genuinely depends on the sibling `key`. That + /// is legal JSON Schema and Anthropic accepts it, but OpenAI's strict + /// validator rejects the entire tool catalog over it, so every + /// OpenAI-route agent died on its first turn. + /// + /// The fix is to fail strict eligibility closed rather than to rewrite the + /// schema: the tool is still advertised with its real shape, just without + /// `strict: true`. + #[test] + fn issue_713_a_property_without_a_type_disqualifies_strict_mode() { + let schema = serde_json::json!({ + "type": "object", + "properties": { + "key": { "type": "string" }, + "value": { "description": "JSON type depends on the key." } + } + }); + assert!( + !schema_supports_strict(&openai_compatible_schema(&schema)), + "a typeless property must not be sent as a strict schema" + ); + } + + /// The counterpart: failing closed must not become failing always, or every + /// well-formed tool silently loses strict mode and the structured-output + /// guarantees that come with it. + #[test] + fn a_fully_typed_schema_still_qualifies_for_strict_mode() { + let schema = serde_json::json!({ + "type": "object", + "properties": { + "path": { "type": "string", "description": "where" }, + "count": { "type": "integer" }, + "mode": { "enum": ["fast", "slow"] }, + "nested": { + "type": "object", + "properties": { "inner": { "type": "boolean" } } + }, + "either": { "anyOf": [{ "type": "string" }, { "type": "integer" }] }, + "anything": true + }, + "required": ["path"] + }); + assert!( + schema_supports_strict(&openai_compatible_schema(&schema)), + "every property declares its shape, so strict must stay available" + ); + } + } diff --git a/crates/jcode-schema-dialect/src/conformance.rs b/crates/jcode-schema-dialect/src/conformance.rs index ed4c137b47..0effe169d6 100644 --- a/crates/jcode-schema-dialect/src/conformance.rs +++ b/crates/jcode-schema-dialect/src/conformance.rs @@ -23,6 +23,64 @@ pub struct ConformanceError { pub message: String, } +/// Report properties that declare nothing about what they accept. +/// +/// Legal JSON Schema (an empty schema accepts any instance) and fine for +/// Anthropic, but OpenAI's strict validator rejects the whole tool catalog over +/// it (#713). Unlike the keyword checks this is not per-dialect: no dialect +/// *rejects* a typeless property, the OpenAI path just must not claim `strict` +/// for it. Reporting it separately keeps jcode's own tools from acquiring one +/// silently, since that would cost every OpenAI-route agent its strict +/// structured-output guarantees. +pub fn untyped_properties(schema: &Value) -> Vec { + fn declares_a_type(schema: &Value) -> bool { + let Some(map) = schema.as_object() else { + return schema.is_boolean(); + }; + ["type", "enum", "const", "anyOf", "oneOf", "allOf", "$ref", "properties", "items"] + .iter() + .any(|keyword| map.contains_key(*keyword)) + } + + fn walk(schema: &Value, path: &str, errors: &mut Vec) { + let Some(map) = schema.as_object() else { + if let Some(items) = schema.as_array() { + for (idx, item) in items.iter().enumerate() { + walk(item, &format!("{path}[{idx}]"), errors); + } + } + return; + }; + if let Some(Value::Object(properties)) = map.get("properties") { + for (name, property) in properties { + let child = format!("{path}.properties.{name}"); + if !declares_a_type(property) { + errors.push(ConformanceError { + path: child.clone(), + message: "property declares no type, enum, or combiner, so the OpenAI \ + strict validator would reject the whole catalog" + .to_string(), + }); + } + walk(property, &child, errors); + } + } + for (key, value) in map { + if key == "properties" { + continue; + } + match keyword_role(key) { + KeywordRole::Data => {} + _ => walk(value, &format!("{path}.{key}"), errors), + } + } + } + + let mut errors = Vec::new(); + walk(schema, "$", &mut errors); + errors +} + impl std::fmt::Display for ConformanceError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{}: {}", self.path, self.message) diff --git a/crates/jcode-schema-dialect/src/lib.rs b/crates/jcode-schema-dialect/src/lib.rs index d3d1e85aa9..565d06991b 100644 --- a/crates/jcode-schema-dialect/src/lib.rs +++ b/crates/jcode-schema-dialect/src/lib.rs @@ -38,6 +38,7 @@ pub mod rejection; pub use conformance::{ ConformanceError, must_not_contain_unsupported_constructs, must_preserve_meaning, + untyped_properties, }; pub use dialect::{DialectSpec, DialectTransforms, LearnedQuirks}; pub use keyword::{KeywordRole, keyword_role}; From e1e47ad870976af4ab96e8ce9cd1b7b714d59863 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 17:19:43 -0700 Subject: [PATCH 18/19] fix(openai): fail strict closed for the remaining #711 constructs (fixes #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. --- crates/jcode-app-core/src/tool/tests.rs | 42 ++++++ .../jcode-provider-core/src/openai_schema.rs | 132 ++++++++++++++++++ 2 files changed, 174 insertions(+) diff --git a/crates/jcode-app-core/src/tool/tests.rs b/crates/jcode-app-core/src/tool/tests.rs index e85d286e6a..c00f9b2fdb 100644 --- a/crates/jcode-app-core/src/tool/tests.rs +++ b/crates/jcode-app-core/src/tool/tests.rs @@ -1542,3 +1542,45 @@ fn the_dialect_sweep_catches_the_issue_754_schema() { "and must pass once normalized" ); } + +/// Failing strict eligibility closed for #711/#713 must not quietly cost jcode's +/// own tools their strict mode, since that would drop the structured-output +/// guarantees on every OpenAI-route tool call with nothing to notice. +/// +/// The four tools listed below were already non-strict before that change, for +/// reasons unrelated to it (`batch` declares `additionalProperties: true` so its +/// sub-call payloads stay open-world; the others carry open maps or untyped +/// action payloads). Pinning the exact set is what makes this a regression +/// detector: a fifth name appearing means a stricter rule went too far, and a +/// name disappearing means a tool became strict-eligible and the list is stale. +#[tokio::test] +async fn only_the_known_open_world_tools_are_ineligible_for_openai_strict_mode() { + /// Built-ins that legitimately cannot be strict. Verified against master + /// before the #711/#713 eligibility changes, so this is pre-existing. + const KNOWN_OPEN_WORLD_TOOLS: &[&str] = &["batch", "browser", "initiative", "swarm"]; + + let provider: Arc = Arc::new(MockProvider); + let registry = Registry::new(provider).await; + let defs = registry.definitions(None).await; + assert!(!defs.is_empty(), "the sweep must not pass vacuously"); + + let mut ineligible: Vec = Vec::new(); + for def in &defs { + let compatible = + jcode_provider_core::openai_schema::openai_compatible_schema(&def.input_schema); + if !jcode_provider_core::openai_schema::schema_supports_strict(&compatible) { + ineligible.push(def.name.clone()); + } + } + ineligible.sort(); + + let expected: Vec = KNOWN_OPEN_WORLD_TOOLS + .iter() + .map(ToString::to_string) + .collect(); + assert_eq!( + ineligible, expected, + "the set of strict-ineligible built-in tools changed; a new name means an \ + eligibility rule is too aggressive, a missing name means this list is stale" + ); +} diff --git a/crates/jcode-provider-core/src/openai_schema.rs b/crates/jcode-provider-core/src/openai_schema.rs index b5231e1f2d..085c60e6c9 100644 --- a/crates/jcode-provider-core/src/openai_schema.rs +++ b/crates/jcode-provider-core/src/openai_schema.rs @@ -267,6 +267,42 @@ pub fn schema_supports_strict(schema: &Value) -> bool { return false; } + // The same rule inside a combiner. A branch that only adds a + // constraint (`{"required": ["memory_id"]}`) or only an enum, with no + // type of its own, cannot satisfy OpenAI's strict object requirements + // and fails the whole catalog (#711, observed on a real MCP catalog). + // + // Branches are held to a stricter bar than properties: a bare `enum` + // is enough to describe a property, but a combiner branch must name a + // `type` (OpenAI reports "schema must have a 'type' key"), which is the + // same rule the registry-wide sweep already enforces on jcode's own + // tools. + for combiner in ["anyOf", "oneOf", "allOf"] { + if let Some(Value::Array(branches)) = map.get(combiner) + && branches.iter().any(|branch| !branch_names_a_type(branch)) + { + return false; + } + } + + // An array whose `items` are unconstrained: strict mode requires the + // element shape to be known (#711). + let is_array_typed = match map.get("type") { + Some(Value::String(t)) => t == "array", + Some(Value::Array(types)) => types.iter().any(|v| v.as_str() == Some("array")), + _ => false, + }; + if is_array_typed && !map.get("items").is_some_and(declares_a_type) { + return false; + } + + // A `$ref` that survived normalization points at a definition the + // request does not carry (`$defs` is stripped for some paths), so the + // strict validator cannot resolve it (#711). + if map.contains_key("$ref") { + return false; + } + map.values().all(schema_supports_strict) } @@ -293,6 +329,23 @@ fn declares_a_type(schema: &Value) -> bool { .any(|keyword| map.contains_key(*keyword)) } +/// Whether a combiner branch names a concrete `type`. +/// +/// Stricter than [`declares_a_type`]: OpenAI accepts a property described only +/// by an `enum`, but rejects a *branch* that does not name a `type`, which is +/// what #711's `mcp__cirqul__create_object` hit with an enum-only `anyOf` +/// branch. A nested combiner is accepted here and validated on recursion. +fn branch_names_a_type(branch: &Value) -> bool { + let Some(map) = branch.as_object() else { + return branch.is_boolean(); + }; + map.contains_key("type") + || map.contains_key("$ref") + || ["anyOf", "oneOf", "allOf"] + .iter() + .any(|combiner| map.contains_key(*combiner)) +} + fn schema_is_object_typed(map: &serde_json::Map) -> bool { match map.get("type") { Some(Value::String(t)) => t == "object", @@ -768,4 +821,83 @@ mod tests { ); } + + /// Issue #711, reproduced independently against master before fixing: four + /// constructs from a real MCP catalog that jcode marked `strict: true` and + /// OpenAI then rejected, failing the entire tool catalog. + /// + /// Each is legal JSON Schema that Anthropic accepts, so the fix is to fail + /// strict eligibility closed rather than to rewrite the schema. + #[test] + fn issue_711_constructs_openai_rejects_do_not_claim_strict() { + let cases: &[(&str, serde_json::Value)] = &[ + ( + // mcp__cirqul__correct_memory: a branch that only adds a + // constraint cannot satisfy strict object requirements. + "constraint-only anyOf branch", + serde_json::json!({ + "type": "object", + "properties": { "memory_id": { "type": "string" } }, + "anyOf": [ { "required": ["memory_id"] } ] + }), + ), + ( + // mcp__cirqul__create_object: OpenAI wants a `type` key on a + // branch even when an enum already pins the values. + "enum-only anyOf branch without a type", + serde_json::json!({ + "type": "object", + "properties": { + "kind": { "anyOf": [ { "enum": ["a", "b"] }, { "type": "string" } ] } + } + }), + ), + ( + "array with unconstrained items", + serde_json::json!({ + "type": "object", + "properties": { "tags": { "type": "array" } } + }), + ), + ( + // `$defs` is stripped on some paths, so a surviving `$ref` + // cannot be resolved by the validator. + "unresolvable $ref", + serde_json::json!({ + "type": "object", + "properties": { "node": { "$ref": "#/$defs/missing" } } + }), + ), + ]; + + for (label, schema) in cases { + assert!( + !schema_supports_strict(&openai_compatible_schema(schema)), + "{label} must not be sent as a strict schema" + ); + } + } + + /// Failing closed must not become failing always: every construct below is + /// well formed, and losing strict mode for them would quietly drop the + /// structured-output guarantees on every OpenAI-route tool call. + #[test] + fn well_formed_schemas_keep_strict_mode() { + let schema = serde_json::json!({ + "type": "object", + "properties": { + "path": { "type": "string", "description": "where" }, + "tags": { "type": "array", "items": { "type": "string" } }, + "mode": { "enum": ["fast", "slow"] }, + "either": { "anyOf": [ { "type": "string" }, { "type": "integer" } ] }, + "nested": { "type": "object", "properties": { "x": { "type": "boolean" } } } + }, + "required": ["path"] + }); + assert!( + schema_supports_strict(&openai_compatible_schema(&schema)), + "a well-formed schema must keep strict mode" + ); + } + } From 7872b456859af23302c9413db702450e0e9fd5e8 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Mon, 3 Aug 2026 17:26:00 -0700 Subject: [PATCH 19/19] fix(anthropic): reroute exhausted Fable quota to Opus --- crates/jcode-base/src/usage/accessors.rs | 9 ++++ crates/jcode-base/src/usage/model.rs | 16 ++++++ crates/jcode-base/src/usage/tests.rs | 25 +++++++++ .../src/anthropic_tests.rs | 21 ++++++++ .../src/lib.rs | 52 ++++++++++++++++++- 5 files changed, 121 insertions(+), 2 deletions(-) diff --git a/crates/jcode-base/src/usage/accessors.rs b/crates/jcode-base/src/usage/accessors.rs index f18e61bda2..a403dd741e 100644 --- a/crates/jcode-base/src/usage/accessors.rs +++ b/crates/jcode-base/src/usage/accessors.rs @@ -353,6 +353,15 @@ async fn fetch_usage_for_account( fetch_anthropic_usage_data(access_token, cache_key).await } +/// Fetch the current Anthropic OAuth usage for an already-resolved access +/// token. This is used on the request path when model-scoped quota affects +/// routing. Unlike [`get`], it waits for the first fetch instead of returning +/// an empty snapshot while a background refresh starts. +pub async fn fetch_usage_for_access_token(access_token: &str) -> Result { + let cache_key = anthropic_usage_cache_key(access_token, None); + fetch_anthropic_usage_data(access_token.to_string(), cache_key).await +} + /// Get usage data synchronously (returns cached data, triggers refresh if stale) pub fn get_sync() -> UsageData { // Try to get cached data diff --git a/crates/jcode-base/src/usage/model.rs b/crates/jcode-base/src/usage/model.rs index 691add9635..5be47ba222 100644 --- a/crates/jcode-base/src/usage/model.rs +++ b/crates/jcode-base/src/usage/model.rs @@ -69,6 +69,22 @@ pub struct UsageData { } impl UsageData { + /// Whether Anthropic reports an exhausted model-scoped weekly window for + /// `model`. The API currently uses display names such as `Fable`, while + /// callers use catalog ids such as `claude-fable-5`, so compare normalized + /// model families rather than requiring the strings to be identical. + pub fn model_scoped_exhausted(&self, model: &str) -> bool { + let model = model.to_ascii_lowercase(); + self.model_scoped.iter().any(|window| { + let scope = window.model_name.to_ascii_lowercase(); + window.utilization >= 0.99 + && ((model.contains("fable") && scope.contains("fable")) + || (model.contains("opus") && scope.contains("opus")) + || (model.contains("sonnet") && scope.contains("sonnet")) + || (model.contains("haiku") && scope.contains("haiku"))) + }) + } + /// Check if data is stale and should be refreshed pub fn is_stale(&self) -> bool { if usage_reset_passed([ diff --git a/crates/jcode-base/src/usage/tests.rs b/crates/jcode-base/src/usage/tests.rs index 02fcce6152..8b2bc0b0a2 100644 --- a/crates/jcode-base/src/usage/tests.rs +++ b/crates/jcode-base/src/usage/tests.rs @@ -775,3 +775,28 @@ fn anthropic_usage_response_deserializes_structured_fable_limit() { Some("Fable") ); } + +#[test] +fn anthropic_model_scoped_exhaustion_matches_display_name_to_catalog_id() { + let usage = UsageData { + model_scoped: vec![ModelScopedUsageWindow { + model_name: "Fable".to_string(), + utilization: 1.0, + resets_at: Some("2026-08-11T00:00:00Z".to_string()), + }], + ..Default::default() + }; + + assert!(usage.model_scoped_exhausted("claude-fable-5")); + assert!(!usage.model_scoped_exhausted("claude-opus-5")); + + let below_limit = UsageData { + model_scoped: vec![ModelScopedUsageWindow { + model_name: "Claude Fable 5".to_string(), + utilization: 0.98, + resets_at: None, + }], + ..Default::default() + }; + assert!(!below_limit.model_scoped_exhausted("claude-fable-5")); +} diff --git a/crates/jcode-provider-anthropic-runtime/src/anthropic_tests.rs b/crates/jcode-provider-anthropic-runtime/src/anthropic_tests.rs index f9e32e3ea1..ae8f87a981 100644 --- a/crates/jcode-provider-anthropic-runtime/src/anthropic_tests.rs +++ b/crates/jcode-provider-anthropic-runtime/src/anthropic_tests.rs @@ -1830,6 +1830,27 @@ fn anthropic_quality_rank_orders_opus_before_haiku_and_retired_last() { ); } +#[test] +fn fable_quota_fallback_selects_the_best_available_opus() { + let fallback = AnthropicProvider::best_available_opus_model("claude-fable-5") + .expect("the curated Anthropic catalog should contain an Opus fallback"); + assert!( + fallback.contains("claude-opus"), + "unexpected fallback: {fallback}" + ); + + let candidates = jcode_base::provider::cached_anthropic_model_ids() + .unwrap_or_else(jcode_base::provider::known_anthropic_model_ids); + let best_rank = candidates + .iter() + .filter(|model| model.to_ascii_lowercase().contains("claude-opus")) + .filter(|model| !anthropic_model_is_retired(model)) + .map(|model| anthropic_model_quality_rank(model)) + .min() + .expect("available Opus model"); + assert_eq!(anthropic_model_quality_rank(&fallback), best_rank); +} + #[test] fn ping_keepalive_emits_streaming_phase_event() { // Issue #451: during silent reasoning phases, `ping` events can be the diff --git a/crates/jcode-provider-anthropic-runtime/src/lib.rs b/crates/jcode-provider-anthropic-runtime/src/lib.rs index 02cf80be51..ddf76da6cc 100644 --- a/crates/jcode-provider-anthropic-runtime/src/lib.rs +++ b/crates/jcode-provider-anthropic-runtime/src/lib.rs @@ -386,6 +386,48 @@ impl AnthropicProvider { usage.five_hour >= 0.99 && usage.seven_day >= 0.99 } + fn best_available_opus_model(exclude: &str) -> Option { + let mut models = jcode_base::provider::cached_anthropic_model_ids() + .unwrap_or_else(jcode_base::provider::known_anthropic_model_ids); + models.retain(|model| { + let key = strip_1m_suffix(model).to_ascii_lowercase(); + key.contains("claude-opus") + && strip_1m_suffix(model) != strip_1m_suffix(exclude) + && !anthropic_model_is_retired(model) + }); + models.sort_by_key(|model| anthropic_model_quality_rank(model)); + models.into_iter().next() + } + + async fn model_after_oauth_quota_check( + &self, + token: &str, + is_oauth: bool, + selected_model: String, + ) -> String { + if !is_oauth || !selected_model.to_ascii_lowercase().contains("fable") { + return selected_model; + } + let Ok(usage) = jcode_base::usage::fetch_usage_for_access_token(token).await else { + return selected_model; + }; + if !usage.model_scoped_exhausted(&selected_model) { + return selected_model; + } + let Some(fallback) = Self::best_available_opus_model(&selected_model) else { + return selected_model; + }; + jcode_base::logging::warn(&format!( + "Anthropic OAuth model-scoped weekly quota for '{}' is exhausted; routing to '{}'", + selected_model, fallback + )); + *self + .model + .write() + .unwrap_or_else(|poisoned| poisoned.into_inner()) = fallback.clone(); + fallback + } + /// Resolve a usable access token (OAuth or API key) and whether it is OAuth. /// /// Exposed for the provider-doctor's native Claude driver so it can validate @@ -1020,11 +1062,14 @@ impl Provider for AnthropicProvider { ) .await?; } - let model = self + let selected_model = self .model .read() .unwrap_or_else(|poisoned| poisoned.into_inner()) .clone(); + let model = self + .model_after_oauth_quota_check(&token, is_oauth, selected_model) + .await; let api_model = strip_1m_suffix(&model).to_string(); // Format request @@ -1381,11 +1426,14 @@ impl Provider for AnthropicProvider { ) .await?; } - let model = self + let selected_model = self .model .read() .unwrap_or_else(|poisoned| poisoned.into_inner()) .clone(); + let model = self + .model_after_oauth_quota_check(&token, is_oauth, selected_model) + .await; let api_model = strip_1m_suffix(&model).to_string(); // Format request