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/Cargo.lock b/Cargo.lock index c7bd950ea6..06669c1cfe 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", @@ -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" diff --git a/Cargo.toml b/Cargo.toml index e43399a0a0..2b1053b015 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 @@ -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/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" + ] +} 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/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/tests.rs b/crates/jcode-app-core/src/tool/tests.rs index 6605e69caf..c00f9b2fdb 100644 --- a/crates/jcode-app-core/src/tool/tests.rs +++ b/crates/jcode-app-core/src/tool/tests.rs @@ -1444,3 +1444,143 @@ 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(); + // 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); + 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" + ); +} + +/// 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-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/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-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/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/cache.rs b/crates/jcode-base/src/usage/cache.rs index c35d147fba..043babe5f7 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,21 @@ 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 +227,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..5be47ba222 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 @@ -67,12 +69,32 @@ 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([ 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 +132,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..8b2bc0b0a2 100644 --- a/crates/jcode-base/src/usage/tests.rs +++ b/crates/jcode-base/src/usage/tests.rs @@ -720,3 +720,83 @@ 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") + ); +} + +#[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-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); 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 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 bb596c9f64..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_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_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": { @@ -489,15 +491,42 @@ 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"); + + // 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"] + ); assert_eq!( - antigravity_compatible_schema(&schema, "gemini-3-flash"), - schema, - "Gemini path must not rewrite the schema" + out["properties"]["tool_calls"], + schema["properties"]["tool_calls"] ); + 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 7f54bf1d57..9b8f883cfd 100644 --- a/crates/jcode-provider-antigravity/src/lib.rs +++ b/crates/jcode-provider-antigravity/src/lib.rs @@ -439,44 +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) is returned unchanged. -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 schema.clone(); - } - 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)) +} + +/// 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-core/src/openai_schema.rs b/crates/jcode-provider-core/src/openai_schema.rs index dde20daeaf..085c60e6c9 100644 --- a/crates/jcode-provider-core/src/openai_schema.rs +++ b/crates/jcode-provider-core/src/openai_schema.rs @@ -255,6 +255,53 @@ 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; + } + + // 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) } @@ -266,6 +313,39 @@ 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)) +} + +/// 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", @@ -690,4 +770,134 @@ 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" + ); + } + + + /// 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" + ); + } + } diff --git a/crates/jcode-provider-gemini-runtime/Cargo.toml b/crates/jcode-provider-gemini-runtime/Cargo.toml index 8be7c40f8b..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" @@ -28,6 +29,9 @@ 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" } +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/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-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 new file mode 100644 index 0000000000..0478fbfb58 --- /dev/null +++ b/crates/jcode-provider-gemini-runtime/tests/mcp_schema_end_to_end.rs @@ -0,0 +1,201 @@ +//! 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") + ); + } +} + +/// 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(), + 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"}}"#; 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 35a1b0c6f1..33e86db65c 100644 --- a/crates/jcode-provider-gemini/src/lib.rs +++ b/crates/jcode-provider-gemini/src/lib.rs @@ -448,71 +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", -]; - -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)] @@ -744,6 +692,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!( 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..0effe169d6 --- /dev/null +++ b/crates/jcode-schema-dialect/src/conformance.rs @@ -0,0 +1,342 @@ +//! 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, +} + +/// 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) + } +} + +/// 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..565d06991b --- /dev/null +++ b/crates/jcode-schema-dialect/src/lib.rs @@ -0,0 +1,510 @@ +//! 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, + untyped_properties, +}; +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(); + + // 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) { + 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() + .map(ToString::to_string) + .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..8b5c9196bd --- /dev/null +++ b/crates/jcode-schema-dialect/src/registry.rs @@ -0,0 +1,261 @@ +//! 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 + }, +}; + +/// 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: 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). +/// +/// 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", + "format", + "anyOf", + "nullable", + "minimum", + "maximum", + "pattern", + "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 + }, +}; + +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..7ace23612b --- /dev/null +++ b/crates/jcode-schema-dialect/src/rejection.rs @@ -0,0 +1,294 @@ +//! 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 { + /// 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. + pub tool: Option, +} + +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.keywords.is_empty() || 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 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 { + keywords: Vec::new(), + format: Some(format), + tool, + }); + } + + // OpenAI (#687): 'uniqueItems' is not permitted. + if let Some(keyword) = extract_quoted_before(message, "is not permitted") { + return Some(SchemaRejection { + keywords: vec![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 { + keywords: vec!["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 { + keywords: vec!["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]) +} + +/// 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)?; + 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(), 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(), 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(), + 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(), 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")); + } + + /// 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" + ); + } + +} 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..979c0cf9c3 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 = 6; const MAX_SOURCE_BYTES: usize = 32 * 1024; const COMMAND_TIMEOUT: Duration = Duration::from_secs(8); const FOREGROUND: (u8, u8, u8) = super::MATH_FOREGROUND; @@ -545,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, [ @@ -595,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, @@ -817,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() { 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 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/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")); + } } 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/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..." ( 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. * 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 {