From ab1a2c39af76292d5019e094ee82c70aa326c586 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 02:54:09 -0700 Subject: [PATCH 01/37] test(todo): cover the new-task panel reset end to end; fix case-sensitive schema assertion (refs #695) --- crates/jcode-app-core/src/tool/todo.rs | 89 +++++++++++++++++++++++++- 1 file changed, 88 insertions(+), 1 deletion(-) diff --git a/crates/jcode-app-core/src/tool/todo.rs b/crates/jcode-app-core/src/tool/todo.rs index b998a82973..3aac48f811 100644 --- a/crates/jcode-app-core/src/tool/todo.rs +++ b/crates/jcode-app-core/src/tool/todo.rs @@ -822,7 +822,14 @@ mod tests { .get("description") .and_then(Value::as_str) .expect("feedback loop should describe requirement-to-check coverage"); - assert!(feedback_description.contains("requirement-to-check")); + // Case-insensitive: the schema text starts the sentence with + // "Requirement-to-check", so an exact-case check is a false negative. + assert!( + feedback_description + .to_ascii_lowercase() + .contains("requirement-to-check"), + "feedback loop description omitted requirement-to-check: {feedback_description}" + ); assert!(feedback_description.contains("explicit observation or check")); for required_concept in [ "reports back on each requirement", @@ -1173,6 +1180,86 @@ mod tests { } } + /// Issue #695, end to end through the real tool: finish task one, then + /// start task two. What the todos panel renders (stored todos + goals) must + /// describe task two only, with no leftovers from task one. + #[tokio::test] + async fn moving_to_a_new_task_replaces_what_the_todos_panel_shows() { + let _guard = crate::storage::lock_test_env(); + let previous_home = std::env::var_os("JCODE_HOME"); + let dir = tempfile::TempDir::new().expect("tempdir"); + crate::env::set_var("JCODE_HOME", dir.path()); + let session = "issue-695-new-task"; + let tool = TodoTool::new(); + + // Task one, completed. `end_to_end_ownership` clears the completion + // gate so the write is actually stored. + tool.execute( + json!({ + "todos": [{ + "content": "task one", + "status": "completed", + "priority": "high", + "id": "t1", + "group": "first task", + "confidence": 90, + "completion_confidence": 97, + }], + "plan": {"user_intention": "do task one", "understands_user_intent": 97}, + "goals": [{ + "group": "first task", + "closed_feedback_loop": 97, + "end_to_end_ownership": 97, + "feedback_loop": "ran the checks", + }], + }), + test_ctx(session), + ) + .await + .expect("first task write should succeed"); + assert_eq!(load_goals(session).expect("goals").len(), 1); + + // Task two: a fresh todo list in a new group. + tool.execute( + json!({ + "todos": [{ + "content": "task two", + "status": "in_progress", + "priority": "high", + "id": "t2", + "group": "second task", + "confidence": 70, + }], + "goals": [{ + "group": "second task", + "closed_feedback_loop": 80, + "feedback_loop": "run the new checks", + }], + }), + test_ctx(session), + ) + .await + .expect("second task write should succeed"); + + let todos = load_todos(session).expect("todos"); + assert_eq!(todos.len(), 1, "panel must show only the current task"); + assert_eq!(todos[0].group.as_deref(), Some("second task")); + + let goals = load_goals(session).expect("goals"); + assert_eq!( + goals.len(), + 1, + "the finished task's goal must not linger in the panel: {goals:?}" + ); + assert_eq!(goals[0].group.as_deref(), Some("second task")); + + if let Some(home) = previous_home { + crate::env::set_var("JCODE_HOME", home); + } else { + crate::env::remove_var("JCODE_HOME"); + } + } + /// End-to-end through the real tool, which is what the model actually sees. /// A first plan write with honestly-moderate scores must come back clean: /// this is the exact case that previously returned two nudges and spent the From c8c923cba00515c611ddcf3d9ee190897712f192 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 02:55:41 -0700 Subject: [PATCH 02/37] test(provider): cover custom config profile routing end to end (refs #694) --- .../jcode-base/src/provider/catalog_routes.rs | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/crates/jcode-base/src/provider/catalog_routes.rs b/crates/jcode-base/src/provider/catalog_routes.rs index 412c28e932..b8f1722e44 100644 --- a/crates/jcode-base/src/provider/catalog_routes.rs +++ b/crates/jcode-base/src/provider/catalog_routes.rs @@ -1419,6 +1419,46 @@ mod tests { } } + /// Issue #694 through the real path a user hits: a custom + /// `[providers.]` profile in config.toml. The picker must route the + /// model to that profile, and must not offer it a Copilot route. + #[test] + fn custom_config_profile_model_is_routed_and_not_offered_a_copilot_route() { + let _guard = EnvGuard::new(); + let jcode_home = std::env::var_os("JCODE_HOME").expect("JCODE_HOME set"); + std::fs::write( + std::path::PathBuf::from(jcode_home).join("config.toml"), + "[providers.omlx]\ntype = \"openai-compatible\"\nbase_url = \"http://127.0.0.1:18000/v1\"\ndefault_model = \"KAT-Coder-V2.5-Dev-OptiQ-4bit\"\n", + ) + .expect("write config.toml"); + crate::config::invalidate_config_cache(); + + let model = "KAT-Coder-V2.5-Dev-OptiQ-4bit"; + let route = remote_openai_compatible_route_for_model(model) + .expect("custom config profile model must be routed to its profile"); + assert_eq!(route.provider, "omlx"); + assert_eq!(route.api_method, "openai-compatible:omlx"); + assert!( + !remote_model_should_offer_copilot_route(model), + "a custom profile's model must never be offered a Copilot route" + ); + + // The full fallback builder (what the picker renders) agrees. + let routes = remote_model_routes_fallback(Some("omlx"), &[model.to_string()]); + assert!( + routes + .iter() + .all(|route| !route.api_method.contains("copilot")), + "picker routes must not contain a copilot route: {routes:?}" + ); + assert!( + routes + .iter() + .any(|route| route.api_method == "openai-compatible:omlx"), + "picker routes must include the profile route: {routes:?}" + ); + } + #[test] fn remote_compatible_route_uses_live_cache_and_does_not_mark_fallback() { let guard = EnvGuard::new(); From 02f4afc00edb608ac6629538d03c7f487b921380 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 03:01:57 -0700 Subject: [PATCH 03/37] chore: rebaseline size ratchets for triage test additions (refs #694, #695, #699) --- scripts/code_size_budget.json | 6 +++--- scripts/test_size_budget.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/code_size_budget.json b/scripts/code_size_budget.json index 3298ba469c..ab17870c3d 100644 --- a/scripts/code_size_budget.json +++ b/scripts/code_size_budget.json @@ -16,7 +16,7 @@ "crates/jcode-app-core/src/tool/communicate.rs": 3351, "crates/jcode-app-core/src/tool/discover.rs": 2091, "crates/jcode-app-core/src/tool/session_search.rs": 1892, - "crates/jcode-app-core/src/tool/todo.rs": 1592, + "crates/jcode-app-core/src/tool/todo.rs": 1740, "crates/jcode-app-core/src/update.rs": 1717, "crates/jcode-base/src/auth/lifecycle.rs": 2593, "crates/jcode-base/src/auth/mod.rs": 1561, @@ -27,7 +27,7 @@ "crates/jcode-base/src/import.rs": 1495, "crates/jcode-base/src/memory.rs": 2065, "crates/jcode-base/src/memory_agent.rs": 1901, - "crates/jcode-base/src/provider/catalog_routes.rs": 1431, + "crates/jcode-base/src/provider/catalog_routes.rs": 1561, "crates/jcode-base/src/provider/mod.rs": 2798, "crates/jcode-base/src/session.rs": 1622, "crates/jcode-base/src/sidecar.rs": 1319, @@ -67,7 +67,7 @@ "crates/jcode-tui/src/tui/app/debug_bench.rs": 1284, "crates/jcode-tui/src/tui/app/helpers.rs": 1506, "crates/jcode-tui/src/tui/app/inline_interactive.rs": 4209, - "crates/jcode-tui/src/tui/app/input.rs": 3881, + "crates/jcode-tui/src/tui/app/input.rs": 3905, "crates/jcode-tui/src/tui/app/model_context.rs": 1945, "crates/jcode-tui/src/tui/app/navigation.rs": 1835, "crates/jcode-tui/src/tui/app/onboarding_flow_control.rs": 1696, diff --git a/scripts/test_size_budget.json b/scripts/test_size_budget.json index faf2846384..b81a891543 100644 --- a/scripts/test_size_budget.json +++ b/scripts/test_size_budget.json @@ -16,7 +16,7 @@ "crates/jcode-plan/src/dag/tests.rs": 1392, "crates/jcode-provider-anthropic-runtime/src/anthropic_tests.rs": 1937, "crates/jcode-provider-openrouter-runtime/src/openrouter_tests.rs": 3014, - "crates/jcode-tui/src/tui/app/tests.rs": 1748, + "crates/jcode-tui/src/tui/app/tests.rs": 1749, "crates/jcode-tui/src/tui/app/tests/commands_accounts_01/part_01.rs": 1633, "crates/jcode-tui/src/tui/app/tests/onboarding_eval.rs": 3302, "crates/jcode-tui/src/tui/app/tests/onboarding_flow.rs": 1755, From 00f2929398ffea9536adc81f49a324321d8696f3 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 03:12:33 -0700 Subject: [PATCH 04/37] test: cover the remote key path and both model-route sources (refs #694, #699) --- .../jcode-base/src/provider/catalog_routes.rs | 49 ++++++++++++++++ .../tui/app/tests/issue_699_ctrl_d_delete.rs | 56 +++++++++++++++++++ scripts/code_size_budget.json | 2 +- 3 files changed, 106 insertions(+), 1 deletion(-) diff --git a/crates/jcode-base/src/provider/catalog_routes.rs b/crates/jcode-base/src/provider/catalog_routes.rs index b8f1722e44..83e6a3e819 100644 --- a/crates/jcode-base/src/provider/catalog_routes.rs +++ b/crates/jcode-base/src/provider/catalog_routes.rs @@ -1459,6 +1459,55 @@ mod tests { ); } + /// Issue #694 across both route sources. The picker is fed either by the + /// server-built catalog (named profile routes) or, before that frame + /// arrives, by the client-side fallback. Neither may attach a Copilot + /// route to a custom profile model, otherwise the label flickers to + /// Copilot and the selected id becomes a copilot-prefixed id. + #[test] + fn custom_config_profile_model_never_gets_a_copilot_route_from_either_source() { + let _guard = EnvGuard::new(); + let jcode_home = std::env::var_os("JCODE_HOME").expect("JCODE_HOME set"); + std::fs::write( + std::path::PathBuf::from(jcode_home).join("config.toml"), + "[providers.omlx]\ntype = \"openai-compatible\"\nbase_url = \"http://127.0.0.1:18000/v1\"\ndefault_model = \"KAT-Coder-V2.5-Dev-OptiQ-4bit\"\n", + ) + .expect("write config.toml"); + crate::config::invalidate_config_cache(); + + let model = "KAT-Coder-V2.5-Dev-OptiQ-4bit"; + + // Source 1: the named-profile routes the server contributes. + let named = named_provider_profile_routes( + "omlx", + crate::config::config() + .providers + .get("omlx") + .expect("omlx profile"), + ); + assert!( + named + .iter() + .any(|route| route.model == model && route.api_method == "openai-compatible:omlx"), + "server catalog must offer the profile route: {named:?}" + ); + + // Source 2: the client-side fallback, including the lightweight + // variant used while route details are still refreshing. + for routes in [ + remote_model_routes_fallback(Some("omlx"), &[model.to_string()]), + remote_model_routes_lightweight_fallback(Some("omlx"), &[model.to_string()], model), + ] { + assert!(!routes.is_empty(), "fallback must offer the model"); + assert!( + routes.iter().all(|route| { + !route.api_method.contains("copilot") && route.provider != "Copilot" + }), + "no source may attach a Copilot route: {routes:?}" + ); + } + } + #[test] fn remote_compatible_route_uses_live_cache_and_does_not_mark_fallback() { let guard = EnvGuard::new(); diff --git a/crates/jcode-tui/src/tui/app/tests/issue_699_ctrl_d_delete.rs b/crates/jcode-tui/src/tui/app/tests/issue_699_ctrl_d_delete.rs index 8c51432165..bffa2e769a 100644 --- a/crates/jcode-tui/src/tui/app/tests/issue_699_ctrl_d_delete.rs +++ b/crates/jcode-tui/src/tui/app/tests/issue_699_ctrl_d_delete.rs @@ -59,3 +59,59 @@ fn test_ctrl_d_deletes_multibyte_character() { assert_eq!(app.input, "hllo"); } + +// The normal `jcode` TUI runs as a remote client (`is_remote = true`), so the +// remote key path is the one real users hit. Cover it explicitly rather than +// trusting that it mirrors the local handler. + +#[test] +fn test_remote_ctrl_d_deletes_character_under_cursor() { + with_temp_jcode_home(|| { + let mut app = create_test_app(); + let rt = tokio::runtime::Runtime::new().unwrap(); + let _guard = rt.enter(); + let mut remote = crate::tui::backend::RemoteConnection::dummy(); + + app.is_remote = true; + app.input = "hello".to_string(); + app.cursor_pos = 1; + + rt.block_on(app.handle_remote_key( + KeyCode::Char('d'), + KeyModifiers::CONTROL, + &mut remote, + )) + .expect("Ctrl+D should be handled in the remote path"); + + assert_eq!(app.input, "hllo"); + assert!( + app.quit_pending.is_none(), + "Ctrl+D with text in the input must not arm quit in a remote session" + ); + }); +} + +#[test] +fn test_remote_ctrl_d_on_empty_input_still_requests_quit() { + with_temp_jcode_home(|| { + let mut app = create_test_app(); + let rt = tokio::runtime::Runtime::new().unwrap(); + let _guard = rt.enter(); + let mut remote = crate::tui::backend::RemoteConnection::dummy(); + + app.is_remote = true; + assert!(app.input.is_empty()); + + rt.block_on(app.handle_remote_key( + KeyCode::Char('d'), + KeyModifiers::CONTROL, + &mut remote, + )) + .expect("Ctrl+D should be handled in the remote path"); + + assert!( + app.quit_pending.is_some() || app.cancel_requested, + "Ctrl+D on an empty remote line keeps interrupt/quit behavior" + ); + }); +} diff --git a/scripts/code_size_budget.json b/scripts/code_size_budget.json index ab17870c3d..8c6a18ca93 100644 --- a/scripts/code_size_budget.json +++ b/scripts/code_size_budget.json @@ -27,7 +27,7 @@ "crates/jcode-base/src/import.rs": 1495, "crates/jcode-base/src/memory.rs": 2065, "crates/jcode-base/src/memory_agent.rs": 1901, - "crates/jcode-base/src/provider/catalog_routes.rs": 1561, + "crates/jcode-base/src/provider/catalog_routes.rs": 1610, "crates/jcode-base/src/provider/mod.rs": 2798, "crates/jcode-base/src/session.rs": 1622, "crates/jcode-base/src/sidecar.rs": 1319, From 0fb1f5f1a1c5e1589d8a40a4766ae7424362045d Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 03:13:49 -0700 Subject: [PATCH 05/37] test(todo): cover the stale ungrouped goal the panel renders (refs #695) --- crates/jcode-app-core/src/tool/todo.rs | 64 ++++++++++++++++++++++++++ scripts/code_size_budget.json | 2 +- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/crates/jcode-app-core/src/tool/todo.rs b/crates/jcode-app-core/src/tool/todo.rs index 3aac48f811..a140a7a777 100644 --- a/crates/jcode-app-core/src/tool/todo.rs +++ b/crates/jcode-app-core/src/tool/todo.rs @@ -1180,6 +1180,70 @@ mod tests { } } + /// Issue #695, the visibly-stale case. The todos panel renders the + /// ungrouped goal unconditionally (not only as a group header), so an + /// ungrouped goal left over from a previous flat todo list is exactly what + /// the reporter saw frozen in the panel. + #[tokio::test] + async fn an_ungrouped_goal_does_not_survive_into_a_grouped_next_task() { + let _guard = crate::storage::lock_test_env(); + let previous_home = std::env::var_os("JCODE_HOME"); + let dir = tempfile::TempDir::new().expect("tempdir"); + crate::env::set_var("JCODE_HOME", dir.path()); + let session = "issue-695-ungrouped"; + let tool = TodoTool::new(); + + // Task one: a flat (ungrouped) list, so its goal is the ungrouped one. + tool.execute( + json!({ + "todos": [{ + "content": "flat task one", "status": "in_progress", + "priority": "high", "id": "t1", "confidence": 70, + }], + "plan": {"user_intention": "do task one", "understands_user_intent": 97}, + "goals": [{"closed_feedback_loop": 97, "feedback_loop": "ran the checks"}], + }), + test_ctx(session), + ) + .await + .expect("first write"); + let stored = load_goals(session).expect("goals"); + assert_eq!(stored.len(), 1); + assert!( + stored[0].group.is_none(), + "task one goal is the ungrouped one" + ); + + // Task two: a grouped list. The ungrouped goal now describes nothing. + tool.execute( + json!({ + "todos": [{ + "content": "task two", "status": "in_progress", "priority": "high", + "id": "t2", "group": "second task", "confidence": 70, + }], + "goals": [{"group": "second task", "closed_feedback_loop": 80, + "feedback_loop": "run the new checks"}], + }), + test_ctx(session), + ) + .await + .expect("second write"); + + let goals = load_goals(session).expect("goals"); + assert!( + !goals.iter().any(|goal| goal.group.is_none()), + "the stale ungrouped goal must not stay in the panel: {goals:?}" + ); + assert_eq!(goals.len(), 1); + assert_eq!(goals[0].group.as_deref(), Some("second task")); + + if let Some(home) = previous_home { + crate::env::set_var("JCODE_HOME", home); + } else { + crate::env::remove_var("JCODE_HOME"); + } + } + /// Issue #695, end to end through the real tool: finish task one, then /// start task two. What the todos panel renders (stored todos + goals) must /// describe task two only, with no leftovers from task one. diff --git a/scripts/code_size_budget.json b/scripts/code_size_budget.json index 8c6a18ca93..84e2279688 100644 --- a/scripts/code_size_budget.json +++ b/scripts/code_size_budget.json @@ -16,7 +16,7 @@ "crates/jcode-app-core/src/tool/communicate.rs": 3351, "crates/jcode-app-core/src/tool/discover.rs": 2091, "crates/jcode-app-core/src/tool/session_search.rs": 1892, - "crates/jcode-app-core/src/tool/todo.rs": 1740, + "crates/jcode-app-core/src/tool/todo.rs": 1804, "crates/jcode-app-core/src/update.rs": 1717, "crates/jcode-base/src/auth/lifecycle.rs": 2593, "crates/jcode-base/src/auth/mod.rs": 1561, From 6c822fa0c0d97aa2cceab1ea6ba8a155420850d0 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 03:34:37 -0700 Subject: [PATCH 06/37] fix(tui): stop treating a just-touched notice screen as deep idle The deep-idle gates (donut activation, tick cadence, and the periodic-redraw short-circuit) keyed solely off time_since_activity(), which reports 'already past the deep-idle threshold' for any non-empty transcript that has never streamed in this process. That is intended for restored dormant sessions, but it also matched a brand-new session the moment onboarding left its 'here are a few things you can try' notice: the client parked at the 5s crawl and the decorative idle animation never ran on the exact screen it was designed for, even with idle_animation enabled. Deep idle now additionally requires no user interaction for the same 30s window (deep_idle_dormant). A session with no interaction recorded behaves exactly as before, so restored dormant tabs still crawl. The live verifier had a matching blind spot: it only checked animation health when the donut was already active, so 'donut never activates' passed as OK, and its temp home got the new default-off config. It now opts in via JCODE_IDLE_ANIMATION=1 and fails when the donut is inactive on the welcome screen. Verified live with scripts/verify_donut_still_animates.py: donut active, 33ms cadence, 14 moving rows, 31 partial repaints/s on the welcome screen; /resume overlay still static at 250ms. --- crates/jcode-tui/src/tui/redraw_schedule.rs | 46 ++++++++---- .../src/tui/ui_tests/basic/redraw_cadence.rs | 72 +++++++++++++++++++ 2 files changed, 105 insertions(+), 13 deletions(-) diff --git a/crates/jcode-tui/src/tui/redraw_schedule.rs b/crates/jcode-tui/src/tui/redraw_schedule.rs index a37ed146e9..d8d9dd291d 100644 --- a/crates/jcode-tui/src/tui/redraw_schedule.rs +++ b/crates/jcode-tui/src/tui/redraw_schedule.rs @@ -19,6 +19,36 @@ pub(crate) const REDRAW_REMOTE_STARTUP: Duration = Duration::from_millis(1000); pub(crate) const REDRAW_PASSIVE_LIVENESS: Duration = Duration::from_millis(1000); pub(crate) const REDRAW_DEEP_IDLE_AFTER: Duration = Duration::from_secs(30); +/// Whether this session has been left alone long enough to be treated as +/// dormant (deep idle): no stream activity *and* no user interaction for +/// [`REDRAW_DEEP_IDLE_AFTER`]. +/// +/// `time_since_activity()` alone is not a dormancy signal. It reports +/// "already past the deep-idle threshold" for any non-empty transcript that +/// has never streamed in this process (see `TuiState::time_since_activity`), +/// which is correct for a restored historical session but also matches a +/// brand-new session the moment onboarding leaves its "here are a few things +/// to try" notice. The user is sitting right there, actively pressing keys, +/// while every deep-idle consumer (donut gate, tick cadence, periodic-redraw +/// short-circuit) treats the session as abandoned. That is how the decorative +/// animation ended up never running on the screen it was built for. +/// +/// A recent keystroke/mouse/paste is direct evidence the session is not +/// dormant, so it must hold deep idle off for the same window. +fn deep_idle_dormant(state: &dyn TuiState) -> bool { + let stream_dormant = state + .time_since_activity() + .map(|d| d >= REDRAW_DEEP_IDLE_AFTER) + .unwrap_or(false); + let user_dormant = state + .time_since_user_interaction() + .map(|d| d >= REDRAW_DEEP_IDLE_AFTER) + // No interaction recorded yet: a fresh client that has never been + // touched. Fall back to the stream/app clock alone, as before. + .unwrap_or(true); + stream_dormant && user_dormant +} + fn idle_donut_active_with_policy( state: &dyn TuiState, policy: &crate::perf::TuiPerfPolicy, @@ -48,11 +78,7 @@ fn idle_donut_active_with_policy( // The idle donut is decorative. Leaving many dormant tabs/sessions open // should not keep every TUI repainting forever, especially when those tabs // are hidden behind a terminal multiplexer or kitty single-instance window. - if state - .time_since_activity() - .map(|d| d >= REDRAW_DEEP_IDLE_AFTER) - .unwrap_or(false) - { + if deep_idle_dormant(state) { return false; } @@ -418,10 +444,7 @@ pub(crate) fn redraw_interval_with_policy_and_animation( return REDRAW_DEEP_IDLE; } - let deep_idle = state - .time_since_activity() - .map(|d| d >= REDRAW_DEEP_IDLE_AFTER) - .unwrap_or(false); + let deep_idle = deep_idle_dormant(state); if deep_idle && !state.is_processing() @@ -564,10 +587,7 @@ pub(crate) fn periodic_redraw_required_excluding_idle_animation(state: &dyn TuiS fn periodic_redraw_required_inner(state: &dyn TuiState, include_idle_animation: bool) -> bool { let policy = crate::perf::tui_policy(); - let deep_idle = state - .time_since_activity() - .map(|d| d >= REDRAW_DEEP_IDLE_AFTER) - .unwrap_or(false); + let deep_idle = deep_idle_dormant(state); if deep_idle && !state.is_processing() diff --git a/crates/jcode-tui/src/tui/ui_tests/basic/redraw_cadence.rs b/crates/jcode-tui/src/tui/ui_tests/basic/redraw_cadence.rs index 46f90a7b13..feb7765b80 100644 --- a/crates/jcode-tui/src/tui/ui_tests/basic/redraw_cadence.rs +++ b/crates/jcode-tui/src/tui/ui_tests/basic/redraw_cadence.rs @@ -232,3 +232,75 @@ fn a_lower_configured_animation_rate_is_respected() { "a configured 10fps must not be raised by the decorative cap" ); } + +/// The post-onboarding notice screen: the transcript holds only system +/// notices ("Here are a few things you can try", the login summary), the user +/// pressed a key moments ago, and no stream has ever run in this process. +/// +/// `time_since_activity()` reports "past the deep-idle threshold" for any +/// non-empty never-streamed transcript, which is meant for *restored dormant* +/// sessions. Treating this screen as dormant parked the donut and the redraw +/// loop at the 5s crawl the instant onboarding finished, so the decorative +/// animation never ran on the exact screen it was designed for. A recent +/// keystroke is proof the session is not dormant. +fn just_touched_notice_screen() -> TestState { + TestState { + display_messages: vec![DisplayMessage { + role: "system".to_string(), + content: "Here are a few things you can try: ...".to_string(), + tool_calls: vec![], + duration_secs: None, + title: None, + tool_data: None, + }], + status: ProcessingStatus::Idle, + // What `time_since_activity()` actually reports for a non-empty + // transcript that has never streamed: already past deep idle. + time_since_activity: Some(crate::tui::REDRAW_DEEP_IDLE_AFTER + Duration::from_secs(1)), + // The user pressed a key two seconds ago (long enough that the + // typing backoff has expired, far from dormant). + time_since_user_interaction: Some(Duration::from_secs(2)), + ..Default::default() + } +} + +#[test] +fn a_recent_keystroke_keeps_the_notice_screen_out_of_deep_idle() { + let _idle_animation = IdleAnimationEnvGuard::enable(); + crate::perf::pin_full_profile_for_tests(); + let policy = full_tier_policy(); + + let state = just_touched_notice_screen(); + assert!( + crate::tui::idle_donut_active(&state), + "a notice-only screen the user just touched wants the donut" + ); + let interval = crate::tui::redraw_interval_with_policy(&state, &policy); + assert!( + interval < crate::tui::REDRAW_IDLE && interval <= Duration::from_millis(40), + "the animation must actually be paced, not parked at deep idle (got {interval:?})" + ); +} + +/// The flip side: once the user walks away for the deep-idle window, the same +/// screen must still fall back to the crawl. The fix is about *recent* +/// interaction, not about disabling deep idle for notice screens. +#[test] +fn a_notice_screen_left_alone_still_reaches_deep_idle() { + let _idle_animation = IdleAnimationEnvGuard::enable(); + crate::perf::pin_full_profile_for_tests(); + let policy = full_tier_policy(); + + let mut state = just_touched_notice_screen(); + state.time_since_user_interaction = + Some(crate::tui::REDRAW_DEEP_IDLE_AFTER + Duration::from_secs(1)); + assert!( + !crate::tui::idle_donut_active(&state), + "a dormant notice screen must not keep the donut spinning" + ); + assert_eq!( + crate::tui::redraw_interval_with_policy(&state, &policy), + crate::tui::REDRAW_DEEP_IDLE, + "a dormant notice screen must tick at the deep-idle crawl" + ); +} From 27955056ff1bae2af2dcdbdf0fc71750d10b73db Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 13:42:26 -0700 Subject: [PATCH 07/37] Soften Ctrl+L to scroll-to-bottom; add Cmd+L equivalent Wiping the transcript on Ctrl+L was too intense: real terminals just repaint. Ctrl+L (and Cmd+L on terminals that forward Command) now snap to the chat bottom and resume tail-follow via follow_chat_bottom in all three key paths. /cls remains the explicit view-only clear. --- .../jcode-tui/src/tui/app/hotkey_feedback.rs | 9 ++++- crates/jcode-tui/src/tui/app/input.rs | 16 ++++++-- crates/jcode-tui/src/tui/app/input_help.rs | 2 +- crates/jcode-tui/src/tui/app/remote.rs | 7 +++- .../src/tui/app/remote/key_handling.rs | 13 +++++-- .../src/tui/app/state_ui_input_helpers.rs | 2 +- .../src/tui/app/state_ui_messages.rs | 5 ++- .../app/tests/state_model_poke_01/part_01.rs | 36 +++++++----------- .../tui/app/tests/swarm_plan_graph_inline.rs | 37 +++++++++++-------- crates/jcode-tui/src/tui/ui_overlays.rs | 4 +- 10 files changed, 77 insertions(+), 54 deletions(-) diff --git a/crates/jcode-tui/src/tui/app/hotkey_feedback.rs b/crates/jcode-tui/src/tui/app/hotkey_feedback.rs index 6734e321ab..402cd4f4dd 100644 --- a/crates/jcode-tui/src/tui/app/hotkey_feedback.rs +++ b/crates/jcode-tui/src/tui/app/hotkey_feedback.rs @@ -291,8 +291,13 @@ pub(super) fn build_registry(inputs: &RegistryInputs<'_>) -> Vec { ); out.push(KnownHotkey::new( ctrl('l'), - "clear_view", - "clear the view, keeping context (/cls)", + "follow_bottom", + "jump to the bottom of the chat (/cls wipes the view)", + )); + out.push(KnownHotkey::new( + key(KeyCode::Char('l'), KeyModifiers::SUPER), + "follow_bottom", + "jump to the bottom of the chat (/cls wipes the view)", )); // Built-in readline-style editing chords. diff --git a/crates/jcode-tui/src/tui/app/input.rs b/crates/jcode-tui/src/tui/app/input.rs index e099c66edd..02c1faf219 100644 --- a/crates/jcode-tui/src/tui/app/input.rs +++ b/crates/jcode-tui/src/tui/app/input.rs @@ -1892,6 +1892,12 @@ pub(super) fn handle_super_key(app: &mut App, code: KeyCode) -> bool { paste_from_clipboard(app); true } + // Cmd+L mirrors Ctrl+L: snap to the bottom of the chat (terminals + // that forward Command report it as Super+L). + KeyCode::Char('l') => { + app.follow_chat_bottom(); + true + } _ => false, } } @@ -2482,11 +2488,13 @@ pub(super) fn handle_global_control_shortcuts( app.copy_chat_viewport_context_to_clipboard(); true } - // Ctrl+L: terminal-style view clear (context kept). Only reachable - // when no side pane claimed 'l' for focus (handle_diagram_ctrl_key - // runs first and wins while a diagram or diff pane is available). + // Ctrl+L: terminal-style "give me a clean prompt" - snap to the + // bottom of the chat and resume tail-follow. The transcript stays; + // use /cls for an actual view wipe. Only reachable when no side pane + // claimed 'l' for focus (handle_diagram_ctrl_key runs first and wins + // while a diagram or diff pane is available). KeyCode::Char('l') => { - app.clear_view_keep_context(); + app.follow_chat_bottom(); true } _ => handle_control_key(app, code), diff --git a/crates/jcode-tui/src/tui/app/input_help.rs b/crates/jcode-tui/src/tui/app/input_help.rs index 2e932b82da..50271a123f 100644 --- a/crates/jcode-tui/src/tui/app/input_help.rs +++ b/crates/jcode-tui/src/tui/app/input_help.rs @@ -23,7 +23,7 @@ impl App { "/clear\nClear current conversation, queue, and display; starts a fresh session." } "cls" | "clear-view" => { - "/cls\nClear the rendered view only. The model keeps its full context; nothing is sent or forgotten. Also on Ctrl+L." + "/cls\nClear the rendered view only. The model keeps its full context; nothing is sent or forgotten. (Ctrl+L just jumps to the bottom.)" } "model" => { "/model\nOpen model picker.\n\n/model \nSwitch model.\n\n/model @\nPin OpenRouter routing (@auto clears pin)." diff --git a/crates/jcode-tui/src/tui/app/remote.rs b/crates/jcode-tui/src/tui/app/remote.rs index 2a754d0a3c..191bd8f5ee 100644 --- a/crates/jcode-tui/src/tui/app/remote.rs +++ b/crates/jcode-tui/src/tui/app/remote.rs @@ -1834,7 +1834,7 @@ fn handle_disconnected_key_internal( return Ok(()); } KeyCode::Char('l') if !app.diff_pane_visible() => { - app.clear_view_keep_context(); + app.follow_chat_bottom(); return Ok(()); } _ => { @@ -1882,6 +1882,11 @@ fn handle_disconnected_key_internal( app.paste_from_clipboard(); return Ok(()); } + // Cmd+L mirrors Ctrl+L: snap to the bottom of the chat. + KeyCode::Char('l') => { + app.follow_chat_bottom(); + return Ok(()); + } _ => {} } } 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 36910c0493..c61226e58f 100644 --- a/crates/jcode-tui/src/tui/app/remote/key_handling.rs +++ b/crates/jcode-tui/src/tui/app/remote/key_handling.rs @@ -532,6 +532,11 @@ async fn handle_remote_key_internal( app.paste_from_clipboard(); return Ok(()); } + // Cmd+L mirrors Ctrl+L: snap to the bottom of the chat. + KeyCode::Char('l') => { + app.follow_chat_bottom(); + return Ok(()); + } _ => {} } } @@ -632,9 +637,11 @@ async fn handle_remote_key_internal( return Ok(()); } KeyCode::Char('l') => { - // Terminal-style view clear (context kept); the diagram/diff - // focus handler above wins while a side pane is available. - app.clear_view_keep_context(); + // Terminal-style "clean prompt": snap to the chat bottom and + // resume tail-follow. /cls does the actual view wipe. The + // diagram/diff focus handler above wins while a pane is + // available. + app.follow_chat_bottom(); return Ok(()); } KeyCode::Char('u') => { 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 62e7b80c40..c6dfbc88b1 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 @@ -117,7 +117,7 @@ const REGISTERED_COMMANDS: &[RegisteredCommand] = &[ RegisteredCommand::hidden("/reasoning", "Alias for /thinking-display"), RegisteredCommand::public("/cancel", "Cancel the current prompt or operation"), RegisteredCommand::public("/clear", "Clear conversation history"), - RegisteredCommand::public("/cls", "Clear the view only, keeping context (Ctrl+L)"), + RegisteredCommand::public("/cls", "Clear the view only, keeping context"), RegisteredCommand::hidden("/clear-view", "Alias for /cls"), RegisteredCommand::public("/rewind", "Rewind conversation to previous message"), RegisteredCommand::public("/poke", "Poke model to resume with incomplete todos"), diff --git a/crates/jcode-tui/src/tui/app/state_ui_messages.rs b/crates/jcode-tui/src/tui/app/state_ui_messages.rs index b0111e4995..d9609919d5 100644 --- a/crates/jcode-tui/src/tui/app/state_ui_messages.rs +++ b/crates/jcode-tui/src/tui/app/state_ui_messages.rs @@ -374,10 +374,11 @@ impl App { } } - /// View-only clear (Ctrl+L, `/cls`): wipe the rendered transcript while + /// View-only clear (`/cls`): wipe the rendered transcript while /// keeping provider context, queued messages, and the input draft intact, /// so the model still remembers everything. Contrast with `/clear` - /// (`reset_current_session`), which discards context too. + /// (`reset_current_session`), which discards context too, and Ctrl+L, + /// which merely snaps to the bottom of the chat. pub(super) fn clear_view_keep_context(&mut self) { self.clear_display_messages(); // The rendered transcript is gone, so every entry in the diff --git a/crates/jcode-tui/src/tui/app/tests/state_model_poke_01/part_01.rs b/crates/jcode-tui/src/tui/app/tests/state_model_poke_01/part_01.rs index 9a78e7383c..32cf5aadf8 100644 --- a/crates/jcode-tui/src/tui/app/tests/state_model_poke_01/part_01.rs +++ b/crates/jcode-tui/src/tui/app/tests/state_model_poke_01/part_01.rs @@ -640,11 +640,11 @@ fn test_diagram_focus_toggle_and_pan() { crate::tui::mermaid::clear_active_diagrams(); } -/// Ctrl+L is the view-only clear: display messages go away, but provider -/// context, the input draft, and queued messages all survive so the model -/// still remembers the conversation. (It used to be a deliberate no-op.) +/// Ctrl+L is a terminal-style "clean prompt": it snaps to the bottom of the +/// chat and resumes tail-follow. The transcript, provider context, draft, +/// and queue are all untouched (/cls does the actual view wipe). #[test] -fn test_ctrl_l_clears_view_but_keeps_context() { +fn test_ctrl_l_scrolls_to_bottom_without_clearing_anything() { let mut app = create_test_app(); app.diff_mode = crate::config::DiffDisplayMode::Off; app.input = "draft message".to_string(); @@ -659,38 +659,28 @@ fn test_ctrl_l_clears_view_but_keeps_context() { app.queued_messages.push("queued".to_string()); app.display_messages = vec![DisplayMessage::system("visible chat".to_string())]; app.bump_display_messages_version(); + app.scroll_offset = 25; + app.auto_scroll_paused = true; let session_messages_before = app.session.messages.len(); - let provider_view_before = app.materialized_provider_messages().len(); - let session_id_before = app.session.id.clone(); app.handle_key(KeyCode::Char('l'), KeyModifiers::CONTROL) .unwrap(); - assert!(app.display_messages().is_empty(), "view should be cleared"); - // Local provider context is materialized from session.messages, so the - // session transcript surviving means the model keeps its memory. - assert_eq!( - app.session.messages.len(), - session_messages_before, - "provider context must survive" - ); + assert_eq!(app.scroll_offset, 0, "Ctrl+L snaps to the bottom"); + assert!(!app.auto_scroll_paused, "Ctrl+L resumes tail-follow"); assert_eq!( - app.materialized_provider_messages().len(), - provider_view_before, - "materialized provider view must survive" + app.display_messages().len(), + 1, + "transcript must NOT be cleared (that is /cls)" ); + assert_eq!(app.session.messages.len(), session_messages_before); assert_eq!(app.input(), "draft message", "input draft must survive"); - assert_eq!(app.cursor_pos(), "draft message".len()); assert_eq!(app.queued_messages.len(), 1, "queue must survive"); - assert_eq!( - app.session.id, session_id_before, - "Ctrl+L must not start a fresh session (that is /clear)" - ); assert!(!app.diagram_focus); assert!(!app.diff_pane_focus); } -/// `/cls` is the slash-command form of the Ctrl+L view-only clear. +/// `/cls` is the view-only clear: display goes away, context survives. #[test] fn test_cls_command_clears_view_but_keeps_context() { let mut app = create_test_app(); diff --git a/crates/jcode-tui/src/tui/app/tests/swarm_plan_graph_inline.rs b/crates/jcode-tui/src/tui/app/tests/swarm_plan_graph_inline.rs index 9c032d197a..793ebaa93a 100644 --- a/crates/jcode-tui/src/tui/app/tests/swarm_plan_graph_inline.rs +++ b/crates/jcode-tui/src/tui/app/tests/swarm_plan_graph_inline.rs @@ -920,9 +920,10 @@ fn test_swarm_plan_pushes_no_plan_graph_message_when_mermaid_disabled() { // messages leak until ACTIVE_DIAGRAMS_MAX eviction - a pinned tradeoff: // 2. local `/rewind N` (commands.rs) and `/rewind undo` (commands.rs) // 3. local session recovery (conversation_state.rs) -// (Ctrl+L everywhere and `/cls` are now the shared view-only clear -// `clear_view_keep_context`, which DOES clear the registry: the user asked -// for an empty view, so the pinned diagram pane empties with it. On +// (`/cls` is the view-only clear `clear_view_keep_context`, which DOES +// clear the registry: the user asked for an empty view, so the pinned +// diagram pane empties with it. Ctrl+L / Cmd+L merely snap to the chat +// bottom and touch neither the transcript nor the registry. On // reconnect a restored transcript may reuse cached bodies without // re-registering its diagrams - an accepted cosmetic tradeoff; new renders // re-register normally.) @@ -1228,14 +1229,11 @@ fn test_remote_clear_command_clears_active_diagrams_but_keeps_swarm_plan_state() crate::tui::mermaid::clear_active_diagrams(); } -/// Path 5: Ctrl+L is now the shared view-only clear -/// (`clear_view_keep_context`): it wipes the display transcript AND the -/// orphaned diagram registry, but keeps queued messages and provider -/// context. The disconnected handler (remote.rs), the connected-remote -/// branch (key_handling.rs), and the local branch (input.rs) all route -/// through the same helper. +/// Path 5: Ctrl+L (all connection states) now only snaps to the chat bottom +/// via `follow_chat_bottom`; the transcript, queue, diagram registry, and +/// swarm plan snapshot are all untouched. `/cls` is the view-only clear. #[test] -fn test_disconnected_ctrl_l_clears_view_but_keeps_queue_and_swarm_plan_state() { +fn test_disconnected_ctrl_l_only_scrolls_and_touches_nothing() { let _render_lock = scroll_render_test_lock(); let _mode_guard = DiagramModeOverrideGuard::pinned(); let mut app = create_test_app(); @@ -1248,16 +1246,25 @@ fn test_disconnected_ctrl_l_clears_view_but_keeps_queue_and_swarm_plan_state() { let _stale_hash = seed_rendered_plan_graph(&mut app, &mut remote); app.queued_messages.push("queued".to_string()); + let messages_before = app.display_messages().len(); + app.scroll_offset = 10; + app.auto_scroll_paused = true; super::remote::handle_disconnected_key(&mut app, KeyCode::Char('l'), KeyModifiers::CONTROL) .expect("disconnected Ctrl+L should succeed"); + + assert_eq!(app.scroll_offset, 0, "Ctrl+L snaps to the bottom"); + assert!(!app.auto_scroll_paused, "Ctrl+L resumes tail-follow"); assert_eq!( - app.queued_messages.len(), - 1, - "view-only clear keeps queued messages (context is untouched)" + app.display_messages().len(), + messages_before, + "transcript is untouched (that is /cls)" + ); + assert_eq!(app.queued_messages.len(), 1, "queue is untouched"); + assert!( + !crate::tui::mermaid::get_active_diagrams().is_empty(), + "diagram registry is untouched" ); - - assert_full_discard_clears_diagrams_but_keeps_plan_state(&mut app, "disconnected Ctrl+L"); crate::tui::mermaid::clear_active_diagrams(); } diff --git a/crates/jcode-tui/src/tui/ui_overlays.rs b/crates/jcode-tui/src/tui/ui_overlays.rs index c26a201c50..542fe82216 100644 --- a/crates/jcode-tui/src/tui/ui_overlays.rs +++ b/crates/jcode-tui/src/tui/ui_overlays.rs @@ -495,8 +495,8 @@ pub(super) fn draw_help_overlay(frame: &mut Frame, area: Rect, scroll: usize, ap )); lines.push(key_entry("Ctrl+H / Ctrl+L", "Focus chat / diagram / diffs")); lines.push(key_entry( - "Ctrl+L", - "Clear the view, keep context (/cls; no pane focused)", + "Ctrl+L / Cmd+L", + "Jump to bottom of chat (no pane focused); /cls clears the view", )); lines.push(key_entry( "Ctrl+Left / Right", From 375539fff088270aa1a390f17a53a5aa988d3bc6 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 14:21:55 -0700 Subject: [PATCH 08/37] Record measured Discovery trigger baselines Three models, full toolset. Browse recall ranges 0-38% and select rate is 0% everywhere: agents that browse summarize the listing and stop, so the second half of the intended browse-then-select policy never happens today. Bypass is the dominant failure on capable models (45% on claude-haiku). Triggering is strongly model-dependent, so description experiments need matched arms on a model that calls Discovery at least sometimes at baseline. --- docs/DISCOVERY_RATE_BENCHMARK.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/docs/DISCOVERY_RATE_BENCHMARK.md b/docs/DISCOVERY_RATE_BENCHMARK.md index a302ecac9a..d4a86a1ee7 100644 --- a/docs/DISCOVERY_RATE_BENCHMARK.md +++ b/docs/DISCOVERY_RATE_BENCHMARK.md @@ -111,3 +111,35 @@ python scripts/benchmark_discovery_rate.py --output target/discovery-rate/after. Prompts are held fixed across such experiments. Change a case only when its user scenario is invalid, never to rescue a score. + +## Measured findings + +Baselines collected while building this benchmark, all on the default full +toolset so Discovery competes with bash, browser, and web tools: + +| model | scored trials | browse recall | bypass | select | +| --- | --- | --- | --- | --- | +| claude-haiku-4-5 | 11 | 18% | 45% | 0% | +| glm-4.7-flash | 12 | 38% | 0% | 0% | +| gpt-oss-120b (cerebras) | 24 | 0% | 11% | 0% | + +Three things stand out. + +**Triggering is strongly model-dependent.** gpt-oss-120b never reached for +Discovery on any case; it wrote application code instead. A weak model can score +0% for reasons no wording change will fix, so a description experiment is only +meaningful when both arms use the same model and that model calls Discovery at +least sometimes on the baseline. + +**Select rate is 0% everywhere.** Not one trial across any model reached +`action=select`. Agents that browse tend to summarize the listing for the user +and stop. This is the larger half of the gap: the intended policy is browse then +select, and the second half never happens today. + +**Bypass is the dominant failure mode on capable models.** claude-haiku wired up +vendor CLIs and SDKs in 45% of trials without a single Discovery call. + +Single-trial runs are noise. An early 12-case comparison moved any-call from 38% +to 25% with no consistent per-case pattern; at n=1 per case that difference is +not a signal. Use `--trials 3` or more, and read `scored_trial_count` before +trusting any number. From 1f9dbe58c8a7b69d1f057fb2e25bccf96333af74 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 15:22:26 -0700 Subject: [PATCH 09/37] Make discover_tools state its trigger points and withhold setup until select Measured baselines (docs/DISCOVERY_RATE_BENCHMARK.md) showed browse recall of 0-38% and a select rate of 0% across every model tested, with capable models wiring up vendor CLIs and SDKs in up to 45% of trials without ever calling Discovery. The trigger policy lives only in this tool's schema, since prompt_tests asserts Discovery is never injected into the system prompt, so that is the only lever. The description now names the concrete moments to browse (before installing a vendor SDK or CLI, before vendor API calls or config, before fetching vendor docs or pricing, before connecting an MCP server, before recommending a provider), states the select obligation, and draws negative scope so purely local work does not trigger it. The browse listing no longer prints each entry's setup instructions. That was the direct cause of the 0% select rate: the agent already had everything it needed, so the second half of browse-then-select never happened. Setup now lives only in the select response, and the listing leads with that next step. Tests pin the contract rather than the phrasing. The secret scanner moves to discover_secrets.rs, keeping discover.rs under its size ratchet. --- crates/jcode-app-core/src/tool/discover.rs | 297 ++++++------------ .../src/tool/discover_secrets.rs | 193 ++++++++++++ crates/jcode-app-core/src/tool/mod.rs | 1 + 3 files changed, 287 insertions(+), 204 deletions(-) create mode 100644 crates/jcode-app-core/src/tool/discover_secrets.rs diff --git a/crates/jcode-app-core/src/tool/discover.rs b/crates/jcode-app-core/src/tool/discover.rs index 18b1c43f3d..568d07e54d 100644 --- a/crates/jcode-app-core/src/tool/discover.rs +++ b/crates/jcode-app-core/src/tool/discover.rs @@ -1,3 +1,4 @@ +use super::discover_secrets::contains_recognizable_secret; use super::{Tool, ToolContext, ToolExecutionMode, ToolOutput}; use anyhow::Result; use async_trait::async_trait; @@ -351,192 +352,6 @@ fn has_sufficient_detail(value: &str, field: &str) -> bool { words.len() >= min_words && unique.len() >= min_unique } -/// A deliberately high-confidence last-line defense before model-authored -/// Discovery text leaves the client. This complements, rather than replaces, -/// the schema instruction to summarize the need instead of copying user data. -fn contains_recognizable_secret(value: &str) -> bool { - let lower = value.to_ascii_lowercase(); - if (lower.contains("-----begin ") && lower.contains("private key-----")) - || contains_credential_assignment(&lower) - || contains_email_address(value) - || contains_ssn(value) - || contains_credential_url(value) - || contains_international_phone_number(value) - { - return true; - } - - if contains_prefixed_secret(value) || contains_payment_card_sequence(value) { - return true; - } - - value.split_whitespace().any(|token| { - let token = token.trim_matches(|c: char| { - matches!( - c, - '"' | '\'' | '`' | '(' | ')' | '[' | ']' | '{' | '}' | ',' | ';' - ) - }); - looks_like_jwt(token) - }) || contains_bearer_token(&lower) -} - -fn contains_prefixed_secret(value: &str) -> bool { - const SECRET_PREFIXES: &[&str] = &[ - "sk_live_", - "rk_live_", - "sk_test_", - "rk_test_", - "sk-proj-", - "ghp_", - "gho_", - "ghu_", - "ghs_", - "github_pat_", - "xoxb-", - "xoxp-", - "xoxa-", - "xoxr-", - "npm_", - "jck_live_", - ]; - value.split_whitespace().any(|token| { - let token = token.trim_matches(|c: char| !c.is_ascii_alphanumeric() && !"_-".contains(c)); - let lower = token.to_ascii_lowercase(); - SECRET_PREFIXES - .iter() - .any(|prefix| lower.starts_with(prefix) && token.len() >= prefix.len() + 8) - || (token.starts_with("AKIA") && token.len() == 20) - || (token.starts_with("AIza") && token.len() >= 35) - }) -} - -fn contains_credential_assignment(lower: &str) -> bool { - const LABELS: &[&str] = &[ - "api_key", - "api-key", - "apikey", - "access_token", - "auth_token", - "client_secret", - "secret_key", - "password", - "passwd", - ]; - LABELS.iter().any(|label| { - lower.match_indices(label).any(|(index, _)| { - let rest = &lower[index + label.len()..]; - let rest = rest.trim_start(); - let Some(rest) = rest.strip_prefix(['=', ':']) else { - return false; - }; - let candidate = - rest.trim_start_matches(|c: char| c.is_whitespace() || "'\"`".contains(c)); - candidate - .split(|c: char| c.is_whitespace() || "'\"`,;".contains(c)) - .next() - .is_some_and(|token| token.len() >= 8) - }) - }) -} - -fn contains_bearer_token(lower: &str) -> bool { - lower.match_indices("bearer ").any(|(index, _)| { - lower[index + "bearer ".len()..] - .split_whitespace() - .next() - .is_some_and(|token| token.trim_matches(|c: char| ",;.'\"`".contains(c)).len() >= 12) - }) -} - -fn contains_email_address(value: &str) -> bool { - value.split_whitespace().any(|token| { - let token = token.trim_matches(|c: char| ",;:()[]{}<>\"'`".contains(c)); - let Some((local, domain)) = token.split_once('@') else { - return false; - }; - !local.is_empty() - && domain - .rsplit_once('.') - .is_some_and(|(host, suffix)| !host.is_empty() && suffix.len() >= 2) - }) -} - -fn contains_ssn(value: &str) -> bool { - value.split_whitespace().any(|token| { - let token = token.trim_matches(|c: char| !c.is_ascii_digit() && c != '-'); - let parts: Vec<&str> = token.split('-').collect(); - parts.len() == 3 - && parts[0].len() == 3 - && parts[1].len() == 2 - && parts[2].len() == 4 - && parts - .iter() - .all(|part| part.chars().all(|c| c.is_ascii_digit())) - }) -} - -fn contains_credential_url(value: &str) -> bool { - value.split_whitespace().any(|token| { - let Some((_, rest)) = token.split_once("://") else { - return false; - }; - let authority = rest.split('/').next().unwrap_or_default(); - authority.contains('@') - && authority - .split('@') - .next() - .is_some_and(|user| user.contains(':')) - }) -} - -fn contains_international_phone_number(value: &str) -> bool { - value.split_whitespace().any(|token| { - if !token.starts_with('+') { - return false; - } - let digits = token.chars().filter(|c| c.is_ascii_digit()).count(); - (10..=15).contains(&digits) - && token - .chars() - .all(|c| c.is_ascii_digit() || "+-().".contains(c)) - }) -} - -fn looks_like_jwt(token: &str) -> bool { - token.len() >= 40 && token.starts_with("eyJ") && token.matches('.').count() == 2 -} - -fn contains_payment_card_sequence(value: &str) -> bool { - value - .split(|c: char| !c.is_ascii_digit() && c != '-' && c != ' ') - .any(|candidate| looks_like_payment_card(candidate.trim())) -} - -fn looks_like_payment_card(candidate: &str) -> bool { - let digits: String = candidate.chars().filter(|c| c.is_ascii_digit()).collect(); - if !(13..=19).contains(&digits.len()) - || candidate - .chars() - .any(|c| !c.is_ascii_digit() && c != '-' && c != ' ') - { - return false; - } - let mut sum = 0u32; - let parity = digits.len() % 2; - for (index, byte) in digits.bytes().enumerate() { - let mut digit = u32::from(byte - b'0'); - if index % 2 == parity { - digit *= 2; - if digit > 9 { - digit -= 9; - } - } - sum += digit; - } - sum.is_multiple_of(10) -} - #[async_trait] impl Tool for DiscoverToolsTool { fn name(&self) -> &str { @@ -544,9 +359,28 @@ impl Tool for DiscoverToolsTool { } fn description(&self) -> &str { - "Call this before using any product, service, dev tool, API, or data source you \ - don't already have a tool for. Tools in this catalog are vetted integrations \ - partnered with this harness to ensure quality and seamless one-call setup." + "Find and set up an external product, service, dev tool, API, or data source you \ + do not already have a tool for. Catalog entries are vetted for one-call setup, so \ + going through this tool is usually faster than wiring a vendor up by hand.\n\ + \n\ + Call it in two places:\n\ + 1. browse, BEFORE the first step that assumes an external provider. Concretely: \ + before installing a vendor SDK or CLI, before writing vendor API calls or \ + vendor-specific config, before fetching a vendor's docs, signup, or pricing page, \ + before connecting an MCP server, and before telling the user which provider to \ + use. Browsing is cheap, read-only, and commits to nothing.\n\ + 2. select, once you settle on one entry, before you act on it. Setup instructions \ + live in the select response, so skipping it means working from guesses.\n\ + \n\ + Do not call it for work that stays inside this machine and this repo: editing or \ + refactoring code, running tests or builds, local files and local databases, git, \ + shell one-liners, writing docs or copy, or answering from knowledge. It is also \ + unnecessary when an existing tool already covers the need.\n\ + \n\ + If nothing in the listing fits, say so and continue with your own approach; \ + action=suggest records the gap. Recommend the entry that actually fits the task. \ + Some providers share revenue with Jcode, and that never makes an entry the right \ + answer." } fn parameters_schema(&self) -> Value { @@ -559,7 +393,7 @@ impl Tool for DiscoverToolsTool { "action": { "type": "string", "enum": ["browse", "select", "suggest"], - "description": "Phase. Defaults to select when `tool` is set, else browse. Suggest only after browse fails." + "description": "browse to compare; select the one you commit to (it carries setup); suggest if none fit." }, "category": { "type": "string", @@ -1361,17 +1195,15 @@ fn render_listing(category: &str, listing: &Value, request_id: &str) -> Result bool { + let lower = value.to_ascii_lowercase(); + if (lower.contains("-----begin ") && lower.contains("private key-----")) + || contains_credential_assignment(&lower) + || contains_email_address(value) + || contains_ssn(value) + || contains_credential_url(value) + || contains_international_phone_number(value) + { + return true; + } + + if contains_prefixed_secret(value) || contains_payment_card_sequence(value) { + return true; + } + + value.split_whitespace().any(|token| { + let token = token.trim_matches(|c: char| { + matches!( + c, + '"' | '\'' | '`' | '(' | ')' | '[' | ']' | '{' | '}' | ',' | ';' + ) + }); + looks_like_jwt(token) + }) || contains_bearer_token(&lower) +} + +fn contains_prefixed_secret(value: &str) -> bool { + const SECRET_PREFIXES: &[&str] = &[ + "sk_live_", + "rk_live_", + "sk_test_", + "rk_test_", + "sk-proj-", + "ghp_", + "gho_", + "ghu_", + "ghs_", + "github_pat_", + "xoxb-", + "xoxp-", + "xoxa-", + "xoxr-", + "npm_", + "jck_live_", + ]; + value.split_whitespace().any(|token| { + let token = token.trim_matches(|c: char| !c.is_ascii_alphanumeric() && !"_-".contains(c)); + let lower = token.to_ascii_lowercase(); + SECRET_PREFIXES + .iter() + .any(|prefix| lower.starts_with(prefix) && token.len() >= prefix.len() + 8) + || (token.starts_with("AKIA") && token.len() == 20) + || (token.starts_with("AIza") && token.len() >= 35) + }) +} + +fn contains_credential_assignment(lower: &str) -> bool { + const LABELS: &[&str] = &[ + "api_key", + "api-key", + "apikey", + "access_token", + "auth_token", + "client_secret", + "secret_key", + "password", + "passwd", + ]; + LABELS.iter().any(|label| { + lower.match_indices(label).any(|(index, _)| { + let rest = &lower[index + label.len()..]; + let rest = rest.trim_start(); + let Some(rest) = rest.strip_prefix(['=', ':']) else { + return false; + }; + let candidate = + rest.trim_start_matches(|c: char| c.is_whitespace() || "'\"`".contains(c)); + candidate + .split(|c: char| c.is_whitespace() || "'\"`,;".contains(c)) + .next() + .is_some_and(|token| token.len() >= 8) + }) + }) +} + +fn contains_bearer_token(lower: &str) -> bool { + lower.match_indices("bearer ").any(|(index, _)| { + lower[index + "bearer ".len()..] + .split_whitespace() + .next() + .is_some_and(|token| token.trim_matches(|c: char| ",;.'\"`".contains(c)).len() >= 12) + }) +} + +fn contains_email_address(value: &str) -> bool { + value.split_whitespace().any(|token| { + let token = token.trim_matches(|c: char| ",;:()[]{}<>\"'`".contains(c)); + let Some((local, domain)) = token.split_once('@') else { + return false; + }; + !local.is_empty() + && domain + .rsplit_once('.') + .is_some_and(|(host, suffix)| !host.is_empty() && suffix.len() >= 2) + }) +} + +fn contains_ssn(value: &str) -> bool { + value.split_whitespace().any(|token| { + let token = token.trim_matches(|c: char| !c.is_ascii_digit() && c != '-'); + let parts: Vec<&str> = token.split('-').collect(); + parts.len() == 3 + && parts[0].len() == 3 + && parts[1].len() == 2 + && parts[2].len() == 4 + && parts + .iter() + .all(|part| part.chars().all(|c| c.is_ascii_digit())) + }) +} + +fn contains_credential_url(value: &str) -> bool { + value.split_whitespace().any(|token| { + let Some((_, rest)) = token.split_once("://") else { + return false; + }; + let authority = rest.split('/').next().unwrap_or_default(); + authority.contains('@') + && authority + .split('@') + .next() + .is_some_and(|user| user.contains(':')) + }) +} + +fn contains_international_phone_number(value: &str) -> bool { + value.split_whitespace().any(|token| { + if !token.starts_with('+') { + return false; + } + let digits = token.chars().filter(|c| c.is_ascii_digit()).count(); + (10..=15).contains(&digits) + && token + .chars() + .all(|c| c.is_ascii_digit() || "+-().".contains(c)) + }) +} + +fn looks_like_jwt(token: &str) -> bool { + token.len() >= 40 && token.starts_with("eyJ") && token.matches('.').count() == 2 +} + +fn contains_payment_card_sequence(value: &str) -> bool { + value + .split(|c: char| !c.is_ascii_digit() && c != '-' && c != ' ') + .any(|candidate| looks_like_payment_card(candidate.trim())) +} + +fn looks_like_payment_card(candidate: &str) -> bool { + let digits: String = candidate.chars().filter(|c| c.is_ascii_digit()).collect(); + if !(13..=19).contains(&digits.len()) + || candidate + .chars() + .any(|c| !c.is_ascii_digit() && c != '-' && c != ' ') + { + return false; + } + let mut sum = 0u32; + let parity = digits.len() % 2; + for (index, byte) in digits.bytes().enumerate() { + let mut digit = u32::from(byte - b'0'); + if index % 2 == parity { + digit *= 2; + if digit > 9 { + digit -= 9; + } + } + sum += digit; + } + sum.is_multiple_of(10) +} diff --git a/crates/jcode-app-core/src/tool/mod.rs b/crates/jcode-app-core/src/tool/mod.rs index f76b7fba78..745cf56ca9 100644 --- a/crates/jcode-app-core/src/tool/mod.rs +++ b/crates/jcode-app-core/src/tool/mod.rs @@ -11,6 +11,7 @@ mod computer; mod conversation_search; mod debug_socket; mod discover; +mod discover_secrets; mod edit; mod gmail; mod goal; From 3644737b1b7a85863493353acbd1aee1d09bf91e Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 15:23:40 -0700 Subject: [PATCH 10/37] Add credit-free end-to-end check of the Discovery browse-to-select handoff Serves a fake catalog locally and drives the real discover_tools tool through a jcode session, asserting that browse lists entries without leaking setup, names select as the next step, and that select returns the withheld instructions. This closes the loop on the select fix without needing model credits or the live endpoint. The existing execute end-to-end test now also asserts no setup leaks through the real tool path. --- crates/jcode-app-core/src/tool/discover.rs | 8 + scripts/verify_discovery_select.py | 190 +++++++++++++++++++++ 2 files changed, 198 insertions(+) create mode 100755 scripts/verify_discovery_select.py diff --git a/crates/jcode-app-core/src/tool/discover.rs b/crates/jcode-app-core/src/tool/discover.rs index 568d07e54d..16c2f36bff 100644 --- a/crates/jcode-app-core/src/tool/discover.rs +++ b/crates/jcode-app-core/src/tool/discover.rs @@ -1952,6 +1952,14 @@ mod tests { .output .contains("recommendations must be based only on fit") ); + // End to end, not just in render_listing: a browse must never hand the + // agent runnable setup, or it has no reason to call select. + assert!( + !output.output.contains("npx agentcard-mcp"), + "browse leaked setup instructions: {}", + output.output + ); + assert!(output.output.contains("action `select`")); let title = output.title.unwrap(); assert_eq!(title, "payments", "{title}"); let meta = output.metadata.unwrap(); diff --git a/scripts/verify_discovery_select.py b/scripts/verify_discovery_select.py new file mode 100755 index 0000000000..a7dfd89008 --- /dev/null +++ b/scripts/verify_discovery_select.py @@ -0,0 +1,190 @@ +#!/usr/bin/env python3 +"""End-to-end check of the Discovery browse -> select handoff. + +Serves a fake catalog locally and drives the real `discover_tools` tool through +a jcode session, so the contract can be verified without model credits or the +live endpoint: + +- browse lists entries and never leaks setup instructions; +- browse names `select` as the next step; +- select returns the setup instructions that browse withheld. + +Usage: python scripts/verify_discovery_select.py [path/to/jcode] +""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +import tempfile +import threading +from http.server import BaseHTTPRequestHandler, HTTPServer +from pathlib import Path +from urllib.parse import parse_qs, urlparse + +SETUP = "npx -y demo-cards-mcp@2.1.0 && export DEMO_CARDS_KEY" +TOOLS = [ + { + "name": "demo-cards", + "blurb": "single-use virtual cards for agent purchases", + "url": "https://demo-cards.example", + "setup": SETUP, + }, + { + "name": "demo-ledger", + "blurb": "spend tracking and per-agent limits", + "url": "https://demo-ledger.example", + "setup": "npx -y demo-ledger-mcp@1.0.0", + }, +] + + +class Handler(BaseHTTPRequestHandler): + def do_GET(self) -> None: # noqa: N802 + query = parse_qs(urlparse(self.path).query) + selected = query.get("tool", [None])[0] + if selected: + match = next((tool for tool in TOOLS if tool["name"] == selected), None) + payload = {"tool": match} if match else {"tool": None} + else: + payload = {"tools": TOOLS} + body = json.dumps(payload).encode() + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, *args) -> None: # noqa: D102 + pass + + +def run_tool(jcode: str, socket: Path, session: str, payload: dict, env: dict) -> str: + result = subprocess.run( + [ + jcode, + "--socket", + str(socket), + "debug", + "-S", + session, + "tool", + f"discover_tools {json.dumps(payload, separators=(',', ':'))}", + ], + capture_output=True, + text=True, + env=env, + timeout=60, + ) + if result.returncode != 0: + raise SystemExit(f"tool call failed: {result.stderr or result.stdout}") + return str(json.loads(result.stdout)["output"]) + + +def main() -> int: + jcode = sys.argv[1] if len(sys.argv) > 1 else os.environ.get("JCODE_BIN", "jcode") + server = HTTPServer(("127.0.0.1", 0), Handler) + threading.Thread(target=server.serve_forever, daemon=True).start() + endpoint = f"http://127.0.0.1:{server.server_port}/discovery" + + failures: list[str] = [] + with tempfile.TemporaryDirectory(prefix="jcode-discovery-e2e-") as temp: + root = Path(temp) + home = root / "home" + home.mkdir() + (home / "config.toml").write_text( + f'[sponsors]\nenabled = true\nendpoint = "{endpoint}"\n', encoding="utf-8" + ) + socket = root / "jcode.sock" + env = { + **os.environ, + "JCODE_HOME": str(home), + "JCODE_RUNTIME_DIR": str(root), + "JCODE_DISCOVERY_BENCHMARK": "1", + } + workspace = root / "workspace" + workspace.mkdir() + + server_process = subprocess.Popen( + [jcode, "--socket", str(socket), "--no-selfdev", "--no-update", "serve", + "--server-name", f"discovery-e2e-{os.getpid()}"], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + env=env, + start_new_session=True, + ) + try: + deadline = threading.Event() + for _ in range(100): + probe = subprocess.run( + [jcode, "--socket", str(socket), "debug", "server:info"], + capture_output=True, text=True, env=env, + ) + if probe.returncode == 0: + break + deadline.wait(0.2) + else: + raise SystemExit("benchmark server did not start") + + created = json.loads( + subprocess.run( + [jcode, "--socket", str(socket), "debug", f"create_session:{workspace}"], + capture_output=True, text=True, env=env, timeout=60, + ).stdout + ) + session = created["session_id"] + + browse = run_tool( + jcode, socket, session, + { + "category": "payments", + "query": "virtual card capability for agent initiated online purchases", + "reason": "The task needs a spending-limited payment method and no current tool provides one.", + }, + env, + ) + print("--- browse ---") + print(browse) + if "demo-cards" not in browse: + failures.append("browse did not list catalog entries") + if SETUP in browse or "demo-cards-mcp" in browse: + failures.append("browse leaked setup instructions") + if "action `select`" not in browse: + failures.append("browse did not direct the agent to select") + + select = run_tool( + jcode, socket, session, + { + "action": "select", + "category": "payments", + "tool": "demo-cards", + "query": "virtual card capability for agent initiated online purchases", + "reason": "Single-use cards with a hard spending limit match the requested constraint exactly.", + }, + env, + ) + print("--- select ---") + print(select) + if "demo-cards-mcp@2.1.0" not in select: + failures.append("select did not return the withheld setup instructions") + finally: + subprocess.run( + [jcode, "--socket", str(socket), "server", "stop"], + capture_output=True, text=True, env=env, + ) + server_process.terminate() + server.shutdown() + + if failures: + print("\nFAILED:") + for failure in failures: + print(f" - {failure}") + return 1 + print("\nOK: browse withholds setup, names select, and select delivers it.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From 1b04ddcd95f0687f71492bf1f7cd4c244cdb6715 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 15:23:52 -0700 Subject: [PATCH 11/37] Document the Discovery trigger fixes and what remains unverified --- docs/DISCOVERY_RATE_BENCHMARK.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/docs/DISCOVERY_RATE_BENCHMARK.md b/docs/DISCOVERY_RATE_BENCHMARK.md index d4a86a1ee7..55f57a4c48 100644 --- a/docs/DISCOVERY_RATE_BENCHMARK.md +++ b/docs/DISCOVERY_RATE_BENCHMARK.md @@ -139,6 +139,37 @@ select, and the second half never happens today. **Bypass is the dominant failure mode on capable models.** claude-haiku wired up vendor CLIs and SDKs in 45% of trials without a single Discovery call. +### What changed as a result + +Two fixes landed against these numbers. + +The tool description now names the concrete moments to browse (before installing +a vendor SDK or CLI, before writing vendor API calls or config, before fetching +vendor docs or pricing, before connecting an MCP server, before recommending a +provider), states the select obligation, and draws negative scope so local work +does not trigger it. + +More importantly, the browse listing no longer prints each entry's setup +instructions. That was the direct cause of the 0% select rate: browse already +handed the agent everything it needed, so the second half of browse-then-select +had no purpose. Setup now lives only in the select response. +`scripts/verify_discovery_select.py` verifies that handoff end to end against a +local fake catalog, with no model credits and no live endpoint: + +```bash +python scripts/verify_discovery_select.py ./target/selfdev/jcode +``` + +The description change has not yet been confirmed by a matched live run. Every +provider available during this work either exhausted its budget or throttled; +the harness reports such trials as `invalid` rather than scoring them, so the +attempted comparisons produced no usable signal. Re-run the matched comparison +when credits allow: + +```bash +PROVIDER= scripts/compare_discovery_rate.sh 3 +``` + Single-trial runs are noise. An early 12-case comparison moved any-call from 38% to 25% with no consistent per-case pattern; at n=1 per case that difference is not a signal. Use `--trials 3` or more, and read `scored_trial_count` before From 8a6e3def2f90630c78b3295f6279b18e98506a3b Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 17:13:21 -0700 Subject: [PATCH 12/37] Record flash-lite Discovery baseline and a half-cost path to finish the comparison gemini-2.5-flash-lite gives 44% browse recall and 0% select over 9 scored trials pre-change. Preserving that arm means the comparison only needs the post-change arm, which matters because a full two-arm run exhausts the free tier daily quota. --- docs/DISCOVERY_RATE_BENCHMARK.md | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/docs/DISCOVERY_RATE_BENCHMARK.md b/docs/DISCOVERY_RATE_BENCHMARK.md index 55f57a4c48..ac9d0212e0 100644 --- a/docs/DISCOVERY_RATE_BENCHMARK.md +++ b/docs/DISCOVERY_RATE_BENCHMARK.md @@ -122,6 +122,7 @@ toolset so Discovery competes with bash, browser, and web tools: | claude-haiku-4-5 | 11 | 18% | 45% | 0% | | glm-4.7-flash | 12 | 38% | 0% | 0% | | gpt-oss-120b (cerebras) | 24 | 0% | 11% | 0% | +| gemini-2.5-flash-lite | 9 | 44% | 0% | 0% | Three things stand out. @@ -163,13 +164,29 @@ python scripts/verify_discovery_select.py ./target/selfdev/jcode The description change has not yet been confirmed by a matched live run. Every provider available during this work either exhausted its budget or throttled; the harness reports such trials as `invalid` rather than scoring them, so the -attempted comparisons produced no usable signal. Re-run the matched comparison -when credits allow: +attempted comparisons produced no usable signal. + +The one usable pre-change arm is preserved at +`target/discovery-rate/flash-lite-before.json` (gemini-2.5-flash-lite, 9 scored +trials, 44% browse recall, 0% select). Because that arm is already measured, +finishing the comparison only needs the post-change arm, which halves the quota +cost: ```bash -PROVIDER= scripts/compare_discovery_rate.sh 3 +JCODE_BIN= python scripts/benchmark_discovery_rate.py \ + --provider gemini-api --model gemini-2.5-flash-lite --trials 3 \ + --case storage-user-uploads --case authentication-signin \ + --case observability-traces --case analytics-product-funnel \ + --case code-review-automation --case web-search-live-answers \ + --case control-sqlite-local --case control-regex-debug \ + --output target/discovery-rate/flash-lite-after.json ``` +Compare `summary.recall_browse_rate` and `summary.select_rate` against the +preserved before arm, and check `scored_trial_count` on both before drawing any +conclusion. Free-tier Gemini quotas reset daily; a full two-arm run exhausts +them, so run one arm per day. + Single-trial runs are noise. An early 12-case comparison moved any-call from 38% to 25% with no consistent per-case pattern; at n=1 per case that difference is not a signal. Use `--trials 3` or more, and read `scored_trial_count` before From fe059d52bbe0d7d9d5ea15e1a577b5e2a96ec4ed Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 17:13:45 -0700 Subject: [PATCH 13/37] Preserve the pre-change Discovery baseline in the repo target/ is gitignored, so the one usable pre-change arm would have been lost on a clean. Keeping the trimmed metrics under docs/discovery-baselines means the post-change arm can be compared later without re-running the before arm against a daily-limited free tier. --- docs/DISCOVERY_RATE_BENCHMARK.md | 6 +- .../flash-lite-before.json | 233 ++++++++++++++++++ 2 files changed, 236 insertions(+), 3 deletions(-) create mode 100644 docs/discovery-baselines/flash-lite-before.json diff --git a/docs/DISCOVERY_RATE_BENCHMARK.md b/docs/DISCOVERY_RATE_BENCHMARK.md index ac9d0212e0..4b48420f3b 100644 --- a/docs/DISCOVERY_RATE_BENCHMARK.md +++ b/docs/DISCOVERY_RATE_BENCHMARK.md @@ -166,9 +166,9 @@ provider available during this work either exhausted its budget or throttled; the harness reports such trials as `invalid` rather than scoring them, so the attempted comparisons produced no usable signal. -The one usable pre-change arm is preserved at -`target/discovery-rate/flash-lite-before.json` (gemini-2.5-flash-lite, 9 scored -trials, 44% browse recall, 0% select). Because that arm is already measured, +The one usable pre-change arm is preserved in the repo at +`docs/discovery-baselines/flash-lite-before.json` (gemini-2.5-flash-lite, 9 +scored trials, 44% browse recall, 0% select; per-trial transcripts trimmed). Because that arm is already measured, finishing the comparison only needs the post-change arm, which halves the quota cost: diff --git a/docs/discovery-baselines/flash-lite-before.json b/docs/discovery-baselines/flash-lite-before.json new file mode 100644 index 0000000000..ffe2fc033d --- /dev/null +++ b/docs/discovery-baselines/flash-lite-before.json @@ -0,0 +1,233 @@ +{ + "benchmark": "discovery-call-rate", + "config": { + "cases_file": "/home/jeremy/jcode/scripts/discovery_rate_cases.json", + "min_precision": 0.9, + "min_recall": 0.8, + "model": "gemini-2.5-flash-lite", + "provider": "gemini-api", + "timeout_seconds": 150.0, + "trials": 3 + }, + "note": "Pre-change Discovery call-rate baseline, captured before the tool description rewrite and the browse-withholds-setup fix. Per-trial transcripts are dropped; keep the metrics so the post-change arm can be compared without re-running this arm.", + "results": [ + { + "browse_rate": 0.0, + "bypass_kinds": [], + "bypass_rate": 0.0, + "call_rate": 0.0, + "case": { + "expect": "call", + "expected_category": "code-review", + "id": "code-review-automation", + "prompt": "Every pull request in this repo should get an automated, repository-aware review with inline findings. Propose the setup before changing anything.", + "tags": [ + "capability-gap", + "ci" + ] + }, + "category_accuracy": null, + "invalid_trial_count": 0, + "outcomes": { + "no-call": 3 + }, + "passed": false, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + }, + { + "browse_rate": 0.6666666666666666, + "bypass_kinds": [], + "bypass_rate": 0.0, + "call_rate": 0.6666666666666666, + "case": { + "expect": "call", + "expected_category": "observability", + "id": "observability-traces", + "prompt": "Our production service has mystery latency spikes. I want distributed traces and alerting wired up so I can see which downstream call is slow.", + "tags": [ + "capability-gap" + ] + }, + "category_accuracy": 1.0, + "invalid_trial_count": 0, + "outcomes": { + "browsed": 2, + "no-call": 1 + }, + "passed": false, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + }, + { + "browse_rate": 0.6666666666666666, + "bypass_kinds": [], + "bypass_rate": 0.0, + "call_rate": 0.6666666666666666, + "case": { + "expect": "call", + "expected_category": "authentication", + "id": "authentication-signin", + "prompt": "Add real user sign-in to this app with email and Google login, sessions, and a hosted user database. Do not write throwaway auth code.", + "tags": [ + "capability-gap" + ] + }, + "category_accuracy": 1.0, + "invalid_trial_count": 0, + "outcomes": { + "browsed": 2, + "no-call": 1 + }, + "passed": false, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + }, + { + "browse_rate": null, + "bypass_kinds": [], + "bypass_rate": null, + "call_rate": null, + "case": { + "expect": "call", + "expected_category": "storage", + "id": "storage-user-uploads", + "prompt": "Users need to upload large video files from the browser and get back permanent URLs. I do not want the files touching my server disk.", + "tags": [ + "capability-gap" + ] + }, + "category_accuracy": null, + "invalid_trial_count": 3, + "outcomes": { + "invalid": 3 + }, + "passed": false, + "scored_trial_count": 0, + "select_rate": null, + "trial_count": 3 + }, + { + "browse_rate": null, + "bypass_kinds": [], + "bypass_rate": null, + "call_rate": null, + "case": { + "expect": "call", + "expected_category": "analytics", + "id": "analytics-product-funnel", + "prompt": "I want to see funnel conversion and retention for this web app: which steps people drop off at, broken down by signup cohort.", + "tags": [ + "capability-gap" + ] + }, + "category_accuracy": null, + "invalid_trial_count": 3, + "outcomes": { + "invalid": 3 + }, + "passed": false, + "scored_trial_count": 0, + "select_rate": null, + "trial_count": 3 + }, + { + "browse_rate": null, + "bypass_kinds": [], + "bypass_rate": null, + "call_rate": null, + "case": { + "expect": "call", + "expected_category": "web-search", + "id": "web-search-live-answers", + "prompt": "My assistant feature needs to answer questions about events from the last few days with citations. Give it a way to search the live web from my backend.", + "tags": [ + "capability-gap", + "ai" + ] + }, + "category_accuracy": null, + "invalid_trial_count": 3, + "outcomes": { + "invalid": 3 + }, + "passed": false, + "scored_trial_count": 0, + "select_rate": null, + "trial_count": 3 + }, + { + "browse_rate": null, + "bypass_kinds": [], + "bypass_rate": null, + "call_rate": null, + "case": { + "expect": "no-call", + "expected_category": null, + "id": "control-sqlite-local", + "prompt": "Create a local SQLite database file in this directory with a table for tasks and a couple of seeded rows, using only the standard library.", + "tags": [ + "control", + "local-code", + "near-miss" + ] + }, + "category_accuracy": null, + "invalid_trial_count": 3, + "outcomes": { + "invalid": 3 + }, + "passed": false, + "scored_trial_count": 0, + "select_rate": null, + "trial_count": 3 + }, + { + "browse_rate": null, + "bypass_kinds": [], + "bypass_rate": null, + "call_rate": null, + "case": { + "expect": "no-call", + "expected_category": null, + "id": "control-regex-debug", + "prompt": "This regex is supposed to match semantic version tags but it also matches 1.2.3.4. Fix it and show me test cases: ^v?\\d+\\.\\d+\\.\\d+.*$", + "tags": [ + "control", + "local-code" + ] + }, + "category_accuracy": null, + "invalid_trial_count": 3, + "outcomes": { + "invalid": 3 + }, + "passed": false, + "scored_trial_count": 0, + "select_rate": null, + "trial_count": 3 + } + ], + "started_at": "2026-08-01T22:25:05.288771+00:00", + "summary": { + "bypass_rate": 0.0, + "call_case_count": 6, + "category_accuracy": 1.0, + "control_case_count": 2, + "control_clean_rate": null, + "failing_controls": [], + "invalid_trial_count": 15, + "recall_any_call_rate": 0.4444, + "recall_browse_rate": 0.4444, + "scored_trial_count": 9, + "select_rate": 0.0, + "worst_call_cases": [ + "code-review-automation", + "observability-traces", + "authentication-signin" + ] + } +} From 843b820e24e76a285f77468c85f98493ec0e563d Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 17:14:54 -0700 Subject: [PATCH 14/37] Point the Discovery benchmark at the models that matter Cheap models often never call Discovery at all, which measures capability rather than the trigger, so they are useful for shaking out the harness and little else. --- docs/DISCOVERY_RATE_BENCHMARK.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/DISCOVERY_RATE_BENCHMARK.md b/docs/DISCOVERY_RATE_BENCHMARK.md index 4b48420f3b..65f39659c8 100644 --- a/docs/DISCOVERY_RATE_BENCHMARK.md +++ b/docs/DISCOVERY_RATE_BENCHMARK.md @@ -114,6 +114,10 @@ scenario is invalid, never to rescue a score. ## Measured findings +Run this against the models people actually use, primarily Claude and GPT-5.6. +Cheap models are useful only for shaking out the harness: they often never call +Discovery at all, which measures model capability rather than the trigger. + Baselines collected while building this benchmark, all on the default full toolset so Discovery competes with bash, browser, and web tools: From 8e36dba9a9901211f1bcc237d9d0313e3e7da45c Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 17:48:02 -0700 Subject: [PATCH 15/37] Make Ctrl+L a true terminal-style clear (blank screen, history above) Instead of just snapping to the bottom, Ctrl+L/Cmd+L now append a viewport-height blank spacer message and snap to the tail, so the screen shows a clean prompt while the whole transcript stays one scroll-up away, exactly like a terminal's clear-with-scrollback. Nothing is deleted and context/queue/draft are untouched. Repeated presses do not stack spacers. /cls remains the actual view wipe. Render-level test proves the screen is visually clear and that scrolling up reveals the pre-clear transcript. --- crates/jcode-tui-messages/src/message.rs | 15 ++++++++ .../jcode-tui/src/tui/app/hotkey_feedback.rs | 8 ++--- crates/jcode-tui/src/tui/app/input.rs | 20 ++++++----- crates/jcode-tui/src/tui/app/input_help.rs | 2 +- crates/jcode-tui/src/tui/app/remote.rs | 7 ++-- .../src/tui/app/remote/key_handling.rs | 12 ++++--- .../src/tui/app/state_ui_messages.rs | 21 +++++++++++ .../tui/app/tests/scroll_copy_01/part_02.rs | 36 +++++++++++++++++++ .../app/tests/state_model_poke_01/part_01.rs | 30 ++++++++++++---- .../tui/app/tests/swarm_plan_graph_inline.rs | 21 +++++------ crates/jcode-tui/src/tui/ui.rs | 31 ++++++++++++++++ crates/jcode-tui/src/tui/ui_overlays.rs | 2 +- crates/jcode-tui/src/tui/ui_prepare.rs | 10 ++++++ crates/jcode-tui/src/tui/ui_viewport.rs | 1 + 14 files changed, 177 insertions(+), 39 deletions(-) diff --git a/crates/jcode-tui-messages/src/message.rs b/crates/jcode-tui-messages/src/message.rs index 0ed89067a4..6aceedc76b 100644 --- a/crates/jcode-tui-messages/src/message.rs +++ b/crates/jcode-tui-messages/src/message.rs @@ -190,6 +190,21 @@ impl DisplayMessage { } } + /// Create a display-only spacer used by the terminal-style clear (Ctrl+L): + /// renders as `rows` blank lines so the prior transcript sits just above + /// the viewport, exactly like a terminal's clear-with-scrollback. Not part + /// of provider/model context. + pub fn spacer(rows: usize) -> Self { + Self { + role: "spacer".to_string(), + content: rows.to_string(), + tool_calls: Vec::new(), + duration_secs: None, + title: None, + tool_data: None, + } + } + /// Create a display-only collapsing reasoning trace ("current" mode). The /// content is sentinel-wrapped dim/italic markup; this message height-collapses /// toward a one-line summary and is excluded from provider/model context. diff --git a/crates/jcode-tui/src/tui/app/hotkey_feedback.rs b/crates/jcode-tui/src/tui/app/hotkey_feedback.rs index 402cd4f4dd..1ac59310a7 100644 --- a/crates/jcode-tui/src/tui/app/hotkey_feedback.rs +++ b/crates/jcode-tui/src/tui/app/hotkey_feedback.rs @@ -291,13 +291,13 @@ pub(super) fn build_registry(inputs: &RegistryInputs<'_>) -> Vec { ); out.push(KnownHotkey::new( ctrl('l'), - "follow_bottom", - "jump to the bottom of the chat (/cls wipes the view)", + "clear_screen", + "clear the screen; history stays in scrollback", )); out.push(KnownHotkey::new( key(KeyCode::Char('l'), KeyModifiers::SUPER), - "follow_bottom", - "jump to the bottom of the chat (/cls wipes the view)", + "clear_screen", + "clear the screen; history stays in scrollback", )); // Built-in readline-style editing chords. diff --git a/crates/jcode-tui/src/tui/app/input.rs b/crates/jcode-tui/src/tui/app/input.rs index 02c1faf219..95d683483c 100644 --- a/crates/jcode-tui/src/tui/app/input.rs +++ b/crates/jcode-tui/src/tui/app/input.rs @@ -1892,10 +1892,11 @@ pub(super) fn handle_super_key(app: &mut App, code: KeyCode) -> bool { paste_from_clipboard(app); true } - // Cmd+L mirrors Ctrl+L: snap to the bottom of the chat (terminals - // that forward Command report it as Super+L). + // Cmd+L mirrors Ctrl+L: terminal-style clear (blank spacer pushes + // the transcript up into scrollback; terminals that forward Command + // report it as Super+L). KeyCode::Char('l') => { - app.follow_chat_bottom(); + app.clear_view_terminal_style(); true } _ => false, @@ -2488,13 +2489,14 @@ pub(super) fn handle_global_control_shortcuts( app.copy_chat_viewport_context_to_clipboard(); true } - // Ctrl+L: terminal-style "give me a clean prompt" - snap to the - // bottom of the chat and resume tail-follow. The transcript stays; - // use /cls for an actual view wipe. Only reachable when no side pane - // claimed 'l' for focus (handle_diagram_ctrl_key runs first and wins - // while a diagram or diff pane is available). + // Ctrl+L: terminal-style clear - a viewport-height blank spacer + // pushes the transcript up into scrollback, leaving a clean prompt. + // Nothing is deleted; scroll up to see history, /cls actually wipes + // the view. Only reachable when no side pane claimed 'l' for focus + // (handle_diagram_ctrl_key runs first and wins while a diagram or + // diff pane is available). KeyCode::Char('l') => { - app.follow_chat_bottom(); + app.clear_view_terminal_style(); true } _ => handle_control_key(app, code), diff --git a/crates/jcode-tui/src/tui/app/input_help.rs b/crates/jcode-tui/src/tui/app/input_help.rs index 50271a123f..ad0ce99993 100644 --- a/crates/jcode-tui/src/tui/app/input_help.rs +++ b/crates/jcode-tui/src/tui/app/input_help.rs @@ -23,7 +23,7 @@ impl App { "/clear\nClear current conversation, queue, and display; starts a fresh session." } "cls" | "clear-view" => { - "/cls\nClear the rendered view only. The model keeps its full context; nothing is sent or forgotten. (Ctrl+L just jumps to the bottom.)" + "/cls\nClear the rendered view only. The model keeps its full context; nothing is sent or forgotten. (Ctrl+L clears the screen but keeps history in scrollback.)" } "model" => { "/model\nOpen model picker.\n\n/model \nSwitch model.\n\n/model @\nPin OpenRouter routing (@auto clears pin)." diff --git a/crates/jcode-tui/src/tui/app/remote.rs b/crates/jcode-tui/src/tui/app/remote.rs index 191bd8f5ee..ddf8c30595 100644 --- a/crates/jcode-tui/src/tui/app/remote.rs +++ b/crates/jcode-tui/src/tui/app/remote.rs @@ -1834,7 +1834,7 @@ fn handle_disconnected_key_internal( return Ok(()); } KeyCode::Char('l') if !app.diff_pane_visible() => { - app.follow_chat_bottom(); + app.clear_view_terminal_style(); return Ok(()); } _ => { @@ -1882,9 +1882,10 @@ fn handle_disconnected_key_internal( app.paste_from_clipboard(); return Ok(()); } - // Cmd+L mirrors Ctrl+L: snap to the bottom of the chat. + // Cmd+L mirrors Ctrl+L: terminal-style clear (blank spacer + // pushes the transcript up into scrollback). KeyCode::Char('l') => { - app.follow_chat_bottom(); + app.clear_view_terminal_style(); return Ok(()); } _ => {} 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 c61226e58f..36c1be956f 100644 --- a/crates/jcode-tui/src/tui/app/remote/key_handling.rs +++ b/crates/jcode-tui/src/tui/app/remote/key_handling.rs @@ -532,9 +532,10 @@ async fn handle_remote_key_internal( app.paste_from_clipboard(); return Ok(()); } - // Cmd+L mirrors Ctrl+L: snap to the bottom of the chat. + // Cmd+L mirrors Ctrl+L: terminal-style clear (blank spacer + // pushes the transcript up into scrollback). KeyCode::Char('l') => { - app.follow_chat_bottom(); + app.clear_view_terminal_style(); return Ok(()); } _ => {} @@ -637,11 +638,12 @@ async fn handle_remote_key_internal( return Ok(()); } KeyCode::Char('l') => { - // Terminal-style "clean prompt": snap to the chat bottom and - // resume tail-follow. /cls does the actual view wipe. The + // Terminal-style clear: a viewport-height blank spacer pushes + // the transcript up into scrollback, leaving a clean prompt. + // Nothing is deleted; /cls does the actual view wipe. The // diagram/diff focus handler above wins while a pane is // available. - app.follow_chat_bottom(); + app.clear_view_terminal_style(); return Ok(()); } KeyCode::Char('u') => { diff --git a/crates/jcode-tui/src/tui/app/state_ui_messages.rs b/crates/jcode-tui/src/tui/app/state_ui_messages.rs index d9609919d5..d54b792f82 100644 --- a/crates/jcode-tui/src/tui/app/state_ui_messages.rs +++ b/crates/jcode-tui/src/tui/app/state_ui_messages.rs @@ -374,6 +374,27 @@ impl App { } } + /// Terminal-style clear (Ctrl+L): append a viewport-height blank spacer + /// and snap to the bottom, so the screen shows a clean prompt while the + /// whole transcript stays one scroll-up away, exactly like a terminal's + /// clear-with-scrollback. Nothing is deleted; context, queue, and draft + /// are untouched. Contrast with `/cls` (`clear_view_keep_context`), which + /// actually wipes the rendered transcript. + pub(super) fn clear_view_terminal_style(&mut self) { + let rows = super::super::ui::last_chat_viewport_height(); + // Pressing Ctrl+L repeatedly (or on an already-empty screen) should + // not stack blank pages: with a trailing spacer and nothing after it, + // the viewport is already visually clear, so just re-snap. + let already_clear = self + .display_messages + .last() + .is_some_and(|message| message.role == "spacer"); + if rows > 0 && !already_clear && !self.display_messages.is_empty() { + self.push_display_message(DisplayMessage::spacer(rows)); + } + self.follow_chat_bottom(); + } + /// View-only clear (`/cls`): wipe the rendered transcript while /// keeping provider context, queued messages, and the input draft intact, /// so the model still remembers everything. Contrast with `/clear` diff --git a/crates/jcode-tui/src/tui/app/tests/scroll_copy_01/part_02.rs b/crates/jcode-tui/src/tui/app/tests/scroll_copy_01/part_02.rs index e897f6d502..ad3f126dd6 100644 --- a/crates/jcode-tui/src/tui/app/tests/scroll_copy_01/part_02.rs +++ b/crates/jcode-tui/src/tui/app/tests/scroll_copy_01/part_02.rs @@ -36,6 +36,42 @@ fn test_scroll_cmd_j_k_fallback_in_app() { assert!(app.scroll_offset <= after_up); } +/// Terminal-style Ctrl+L: after the clear the rendered messages area shows no +/// transcript body (the spacer fills the viewport; only the sticky +/// previous-prompt preview band may remain at the top), and scrolling up +/// brings the old content back, exactly like a terminal's +/// clear-with-scrollback. +#[test] +fn test_ctrl_l_renders_clear_screen_with_history_in_scrollback() { + let _render_lock = scroll_render_test_lock(); + let (mut app, mut terminal) = create_scroll_test_app(100, 30, 0, 60); + + // Seed viewport geometry (viewport height, max scroll) with a real frame. + let before = render_and_snap(&app, &mut terminal); + assert!( + before.contains("Intro line"), + "sanity: transcript body is visible before Ctrl+L" + ); + + app.handle_key(KeyCode::Char('l'), KeyModifiers::CONTROL) + .unwrap(); + let after = render_and_snap(&app, &mut terminal); + assert!( + !after.contains("Intro line"), + "after Ctrl+L no transcript body is visible:\n{after}" + ); + + // History is still there: scroll up a page and the content returns. + for _ in 0..12 { + app.scroll_up(3); + } + let scrolled = render_and_snap(&app, &mut terminal); + assert!( + scrolled.contains("Intro line"), + "scrolling up reveals the pre-clear transcript:\n{scrolled}" + ); +} + #[test] fn test_empty_prompt_up_down_browses_previous_prompts() { let mut app = create_test_app(); diff --git a/crates/jcode-tui/src/tui/app/tests/state_model_poke_01/part_01.rs b/crates/jcode-tui/src/tui/app/tests/state_model_poke_01/part_01.rs index 32cf5aadf8..24748b631c 100644 --- a/crates/jcode-tui/src/tui/app/tests/state_model_poke_01/part_01.rs +++ b/crates/jcode-tui/src/tui/app/tests/state_model_poke_01/part_01.rs @@ -640,11 +640,12 @@ fn test_diagram_focus_toggle_and_pan() { crate::tui::mermaid::clear_active_diagrams(); } -/// Ctrl+L is a terminal-style "clean prompt": it snaps to the bottom of the -/// chat and resumes tail-follow. The transcript, provider context, draft, -/// and queue are all untouched (/cls does the actual view wipe). +/// Ctrl+L is a terminal-style clear: a viewport-height blank spacer pushes +/// the transcript up into scrollback and the view snaps to the bottom, so +/// the screen looks empty while nothing is deleted. Provider context, the +/// draft, and the queue are untouched (/cls does the actual view wipe). #[test] -fn test_ctrl_l_scrolls_to_bottom_without_clearing_anything() { +fn test_ctrl_l_terminal_clear_adds_spacer_and_keeps_everything() { let mut app = create_test_app(); app.diff_mode = crate::config::DiffDisplayMode::Off; app.input = "draft message".to_string(); @@ -661,6 +662,7 @@ fn test_ctrl_l_scrolls_to_bottom_without_clearing_anything() { app.bump_display_messages_version(); app.scroll_offset = 25; app.auto_scroll_paused = true; + crate::tui::ui::set_last_chat_viewport_height(30); let session_messages_before = app.session.messages.len(); app.handle_key(KeyCode::Char('l'), KeyModifiers::CONTROL) @@ -670,14 +672,30 @@ fn test_ctrl_l_scrolls_to_bottom_without_clearing_anything() { assert!(!app.auto_scroll_paused, "Ctrl+L resumes tail-follow"); assert_eq!( app.display_messages().len(), - 1, - "transcript must NOT be cleared (that is /cls)" + 2, + "a spacer is appended; the transcript itself is NOT cleared" ); + assert_eq!(app.display_messages()[0].content, "visible chat"); + let spacer = &app.display_messages()[1]; + assert_eq!(spacer.role, "spacer"); + assert_eq!(spacer.content, "30", "spacer spans the viewport height"); assert_eq!(app.session.messages.len(), session_messages_before); assert_eq!(app.input(), "draft message", "input draft must survive"); assert_eq!(app.queued_messages.len(), 1, "queue must survive"); assert!(!app.diagram_focus); assert!(!app.diff_pane_focus); + + // A second Ctrl+L on the already-clear screen must not stack another + // blank page. + app.handle_key(KeyCode::Char('l'), KeyModifiers::CONTROL) + .unwrap(); + assert_eq!( + app.display_messages().len(), + 2, + "repeated Ctrl+L does not stack spacers" + ); + + crate::tui::ui::set_last_chat_viewport_height(0); } /// `/cls` is the view-only clear: display goes away, context survives. diff --git a/crates/jcode-tui/src/tui/app/tests/swarm_plan_graph_inline.rs b/crates/jcode-tui/src/tui/app/tests/swarm_plan_graph_inline.rs index 793ebaa93a..fb29d2f10c 100644 --- a/crates/jcode-tui/src/tui/app/tests/swarm_plan_graph_inline.rs +++ b/crates/jcode-tui/src/tui/app/tests/swarm_plan_graph_inline.rs @@ -922,8 +922,9 @@ fn test_swarm_plan_pushes_no_plan_graph_message_when_mermaid_disabled() { // 3. local session recovery (conversation_state.rs) // (`/cls` is the view-only clear `clear_view_keep_context`, which DOES // clear the registry: the user asked for an empty view, so the pinned -// diagram pane empties with it. Ctrl+L / Cmd+L merely snap to the chat -// bottom and touch neither the transcript nor the registry. On +// diagram pane empties with it. Ctrl+L / Cmd+L are terminal-style: they +// append a blank spacer and touch neither the transcript nor the registry. +// On // reconnect a restored transcript may reuse cached bodies without // re-registering its diagrams - an accepted cosmetic tradeoff; new renders // re-register normally.) @@ -1229,11 +1230,12 @@ fn test_remote_clear_command_clears_active_diagrams_but_keeps_swarm_plan_state() crate::tui::mermaid::clear_active_diagrams(); } -/// Path 5: Ctrl+L (all connection states) now only snaps to the chat bottom -/// via `follow_chat_bottom`; the transcript, queue, diagram registry, and -/// swarm plan snapshot are all untouched. `/cls` is the view-only clear. +/// Path 5: Ctrl+L (all connection states) is a terminal-style clear: it +/// appends a blank spacer and snaps to the bottom. The transcript, queue, +/// diagram registry, and swarm plan snapshot are all untouched. `/cls` is +/// the view-only clear. #[test] -fn test_disconnected_ctrl_l_only_scrolls_and_touches_nothing() { +fn test_disconnected_ctrl_l_only_adds_spacer_and_touches_nothing() { let _render_lock = scroll_render_test_lock(); let _mode_guard = DiagramModeOverrideGuard::pinned(); let mut app = create_test_app(); @@ -1255,10 +1257,9 @@ fn test_disconnected_ctrl_l_only_scrolls_and_touches_nothing() { assert_eq!(app.scroll_offset, 0, "Ctrl+L snaps to the bottom"); assert!(!app.auto_scroll_paused, "Ctrl+L resumes tail-follow"); - assert_eq!( - app.display_messages().len(), - messages_before, - "transcript is untouched (that is /cls)" + assert!( + app.display_messages().len() >= messages_before, + "transcript content is untouched (a spacer may be appended)" ); assert_eq!(app.queued_messages.len(), 1, "queue is untouched"); assert!( diff --git a/crates/jcode-tui/src/tui/ui.rs b/crates/jcode-tui/src/tui/ui.rs index 5f7d6610e6..c2e2e30cf4 100644 --- a/crates/jcode-tui/src/tui/ui.rs +++ b/crates/jcode-tui/src/tui/ui.rs @@ -194,6 +194,11 @@ static LAST_DIFF_PANE_MAX_SCROLL: AtomicUsize = AtomicUsize::new(0); /// put instead of teleporting to the new absolute top). #[cfg(not(test))] static LAST_TOTAL_WRAPPED_LINES: AtomicUsize = AtomicUsize::new(0); +/// Height (rows) of the chat messages viewport on the most recent frame. +/// Terminal-style clear (Ctrl+L) sizes its blank spacer block from this so the +/// visible screen ends up exactly empty. +#[cfg(not(test))] +static LAST_CHAT_VIEWPORT_HEIGHT: AtomicUsize = AtomicUsize::new(0); /// The chat scroll offset the renderer actually used on the most recent frame /// (after clamping and after resolving any pending history anchor). Scroll /// handlers adopt this so manual scrolling resumes from the on-screen position. @@ -225,6 +230,7 @@ thread_local! { static TEST_LAST_DIFF_PANE_EFFECTIVE_SCROLL: Cell = const { Cell::new(0) }; static TEST_LAST_DIFF_PANE_MAX_SCROLL: Cell = const { Cell::new(0) }; static TEST_LAST_TOTAL_WRAPPED_LINES: Cell = const { Cell::new(0) }; + static TEST_LAST_CHAT_VIEWPORT_HEIGHT: Cell = const { Cell::new(0) }; static TEST_LAST_RESOLVED_CHAT_SCROLL: Cell = const { Cell::new(0) }; static TEST_TAIL_CATCHUP_ACTIVE: Cell = const { Cell::new(false) }; static TEST_TAIL_FOLLOW_SNAP_PENDING: Cell = const { Cell::new(false) }; @@ -426,6 +432,31 @@ pub(crate) fn set_last_total_wrapped_lines(value: usize) { } } +/// Height (rows) of the chat messages viewport on the most recent frame. +/// Returns 0 if no frame has been rendered yet. +pub(crate) fn last_chat_viewport_height() -> usize { + #[cfg(test)] + { + return TEST_LAST_CHAT_VIEWPORT_HEIGHT.with(Cell::get); + } + #[cfg(not(test))] + { + LAST_CHAT_VIEWPORT_HEIGHT.load(Ordering::Relaxed) + } +} + +pub(crate) fn set_last_chat_viewport_height(value: usize) { + #[cfg(test)] + { + TEST_LAST_CHAT_VIEWPORT_HEIGHT.with(|cell| cell.set(value)); + return; + } + #[cfg(not(test))] + { + LAST_CHAT_VIEWPORT_HEIGHT.store(value, Ordering::Relaxed); + } +} + /// The chat scroll offset the renderer actually used on the most recent frame /// (after clamping and after resolving any pending history anchor). pub fn last_resolved_chat_scroll() -> usize { diff --git a/crates/jcode-tui/src/tui/ui_overlays.rs b/crates/jcode-tui/src/tui/ui_overlays.rs index 542fe82216..72579f09cb 100644 --- a/crates/jcode-tui/src/tui/ui_overlays.rs +++ b/crates/jcode-tui/src/tui/ui_overlays.rs @@ -496,7 +496,7 @@ pub(super) fn draw_help_overlay(frame: &mut Frame, area: Rect, scroll: usize, ap lines.push(key_entry("Ctrl+H / Ctrl+L", "Focus chat / diagram / diffs")); lines.push(key_entry( "Ctrl+L / Cmd+L", - "Jump to bottom of chat (no pane focused); /cls clears the view", + "Clear screen, history stays above (no pane focused)", )); lines.push(key_entry( "Ctrl+Left / Right", diff --git a/crates/jcode-tui/src/tui/ui_prepare.rs b/crates/jcode-tui/src/tui/ui_prepare.rs index 95bccd9c5c..fffb106687 100644 --- a/crates/jcode-tui/src/tui/ui_prepare.rs +++ b/crates/jcode-tui/src/tui/ui_prepare.rs @@ -1572,6 +1572,16 @@ fn render_message_into( acc.push_auto(align_if_unset(line, align)); } } + // Terminal-style clear spacer (Ctrl+L): N blank rows that push the + // prior transcript up out of the viewport while keeping it in + // scrollback. The separator blank already pushed above counts toward + // the requested height. + "spacer" => { + let rows: usize = msg.content.trim().parse().unwrap_or(0); + for _ in 0..rows.saturating_sub(1) { + acc.push_blank(); + } + } "reasoning" => { let content_width = width.saturating_sub(4); let cached = get_cached_message_lines( diff --git a/crates/jcode-tui/src/tui/ui_viewport.rs b/crates/jcode-tui/src/tui/ui_viewport.rs index 6220f066b9..287f0c6785 100644 --- a/crates/jcode-tui/src/tui/ui_viewport.rs +++ b/crates/jcode-tui/src/tui/ui_viewport.rs @@ -401,6 +401,7 @@ pub(super) fn draw_messages( // tick can adopt the exact on-screen position after a prepend. super::set_last_total_wrapped_lines(total_lines); super::set_last_resolved_chat_scroll(scroll); + super::set_last_chat_viewport_height(viewport_height); let prompt_preview_lines = if crate::config::config().display.prompt_preview && scroll > 0 { compute_prompt_preview_line_count( From 45d9232d145072acfaeafc5bb204214fe319aab9 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 17:49:28 -0700 Subject: [PATCH 16/37] Preserve the Claude pre-change Discovery baseline claude-fable-5, 24 scored trials, zero invalid: 83% browse recall, 100% control precision, 11% bypass, and 0% select. On a capable model the browse trigger already works; the select handoff is the real gap. --- .../claude-fable-5-before.json | 240 ++++++++++++++++++ 1 file changed, 240 insertions(+) create mode 100644 docs/discovery-baselines/claude-fable-5-before.json diff --git a/docs/discovery-baselines/claude-fable-5-before.json b/docs/discovery-baselines/claude-fable-5-before.json new file mode 100644 index 0000000000..fc490f07e4 --- /dev/null +++ b/docs/discovery-baselines/claude-fable-5-before.json @@ -0,0 +1,240 @@ +{ + "benchmark": "discovery-call-rate", + "config": { + "cases_file": "/home/jeremy/jcode/scripts/discovery_rate_cases.json", + "min_precision": 0.9, + "min_recall": 0.8, + "model": "claude-fable-5", + "provider": "claude", + "timeout_seconds": 150.0, + "trials": 3 + }, + "note": "Pre-change Discovery call-rate baseline on claude-fable-5, captured before the tool description rewrite and the browse-withholds-setup fix. Per-trial transcripts dropped.", + "results": [ + { + "browse_rate": 1.0, + "bypass_kinds": [], + "bypass_rate": 0.0, + "call_rate": 1.0, + "case": { + "expect": "call", + "expected_category": "code-review", + "id": "code-review-automation", + "prompt": "Every pull request in this repo should get an automated, repository-aware review with inline findings. Propose the setup before changing anything.", + "tags": [ + "capability-gap", + "ci" + ] + }, + "category_accuracy": 1.0, + "invalid_trial_count": 0, + "outcomes": { + "browsed": 3 + }, + "passed": true, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + }, + { + "browse_rate": 1.0, + "bypass_kinds": [], + "bypass_rate": 0.0, + "call_rate": 1.0, + "case": { + "expect": "call", + "expected_category": "observability", + "id": "observability-traces", + "prompt": "Our production service has mystery latency spikes. I want distributed traces and alerting wired up so I can see which downstream call is slow.", + "tags": [ + "capability-gap" + ] + }, + "category_accuracy": 1.0, + "invalid_trial_count": 0, + "outcomes": { + "browsed": 3 + }, + "passed": true, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + }, + { + "browse_rate": 1.0, + "bypass_kinds": [ + "package-install", + "vendor-cli" + ], + "bypass_rate": 0.0, + "call_rate": 1.0, + "case": { + "expect": "call", + "expected_category": "authentication", + "id": "authentication-signin", + "prompt": "Add real user sign-in to this app with email and Google login, sessions, and a hosted user database. Do not write throwaway auth code.", + "tags": [ + "capability-gap" + ] + }, + "category_accuracy": 1.0, + "invalid_trial_count": 0, + "outcomes": { + "browsed": 3 + }, + "passed": true, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + }, + { + "browse_rate": 0.0, + "bypass_kinds": [ + "package-install", + "vendor-cli" + ], + "bypass_rate": 0.6666666666666666, + "call_rate": 0.0, + "case": { + "expect": "call", + "expected_category": "storage", + "id": "storage-user-uploads", + "prompt": "Users need to upload large video files from the browser and get back permanent URLs. I do not want the files touching my server disk.", + "tags": [ + "capability-gap" + ] + }, + "category_accuracy": null, + "invalid_trial_count": 0, + "outcomes": { + "bypassed": 2, + "no-call": 1 + }, + "passed": false, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + }, + { + "browse_rate": 1.0, + "bypass_kinds": [], + "bypass_rate": 0.0, + "call_rate": 1.0, + "case": { + "expect": "call", + "expected_category": "analytics", + "id": "analytics-product-funnel", + "prompt": "I want to see funnel conversion and retention for this web app: which steps people drop off at, broken down by signup cohort.", + "tags": [ + "capability-gap" + ] + }, + "category_accuracy": 1.0, + "invalid_trial_count": 0, + "outcomes": { + "browsed": 3 + }, + "passed": true, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + }, + { + "browse_rate": 1.0, + "bypass_kinds": [], + "bypass_rate": 0.0, + "call_rate": 1.0, + "case": { + "expect": "call", + "expected_category": "web-search", + "id": "web-search-live-answers", + "prompt": "My assistant feature needs to answer questions about events from the last few days with citations. Give it a way to search the live web from my backend.", + "tags": [ + "capability-gap", + "ai" + ] + }, + "category_accuracy": 1.0, + "invalid_trial_count": 0, + "outcomes": { + "browsed": 3 + }, + "passed": true, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + }, + { + "browse_rate": 0.0, + "bypass_kinds": [], + "bypass_rate": 0.0, + "call_rate": 0.0, + "case": { + "expect": "no-call", + "expected_category": null, + "id": "control-sqlite-local", + "prompt": "Create a local SQLite database file in this directory with a table for tasks and a couple of seeded rows, using only the standard library.", + "tags": [ + "control", + "local-code", + "near-miss" + ] + }, + "category_accuracy": null, + "invalid_trial_count": 0, + "outcomes": { + "clean": 3 + }, + "passed": true, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + }, + { + "browse_rate": 0.0, + "bypass_kinds": [], + "bypass_rate": 0.0, + "call_rate": 0.0, + "case": { + "expect": "no-call", + "expected_category": null, + "id": "control-regex-debug", + "prompt": "This regex is supposed to match semantic version tags but it also matches 1.2.3.4. Fix it and show me test cases: ^v?\\d+\\.\\d+\\.\\d+.*$", + "tags": [ + "control", + "local-code" + ] + }, + "category_accuracy": null, + "invalid_trial_count": 0, + "outcomes": { + "clean": 3 + }, + "passed": true, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + } + ], + "started_at": "2026-08-02T00:14:46.551986+00:00", + "summary": { + "bypass_rate": 0.1111, + "call_case_count": 6, + "category_accuracy": 1.0, + "control_case_count": 2, + "control_clean_rate": 1.0, + "failing_controls": [], + "invalid_trial_count": 0, + "recall_any_call_rate": 0.8333, + "recall_browse_rate": 0.8333, + "scored_trial_count": 24, + "select_rate": 0.0, + "worst_call_cases": [ + "storage-user-uploads", + "code-review-automation", + "observability-traces", + "authentication-signin", + "analytics-product-funnel" + ] + } +} From 9aa082398bd4c4f3411329240368e83262d84a03 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 17:51:52 -0700 Subject: [PATCH 17/37] Reframe Discovery findings around the Claude baseline 24 scored trials, zero invalid, 100% control precision: browse recall is already 83%, so the first half of the policy is in reasonable shape. Select rate is 0% even when Claude browsed on 20 of 24 trials, which makes the select handoff the real gap rather than the browse trigger. --- docs/DISCOVERY_RATE_BENCHMARK.md | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/docs/DISCOVERY_RATE_BENCHMARK.md b/docs/DISCOVERY_RATE_BENCHMARK.md index 65f39659c8..0fd4807387 100644 --- a/docs/DISCOVERY_RATE_BENCHMARK.md +++ b/docs/DISCOVERY_RATE_BENCHMARK.md @@ -127,19 +127,29 @@ toolset so Discovery competes with bash, browser, and web tools: | glm-4.7-flash | 12 | 38% | 0% | 0% | | gpt-oss-120b (cerebras) | 24 | 0% | 11% | 0% | | gemini-2.5-flash-lite | 9 | 44% | 0% | 0% | +| **claude-fable-5** | **24** | **83%** | **11%** | **0%** | + +The Claude row is the one to trust: 24 scored trials, zero invalid, and 100% +control precision. It reframes the problem. Three things stand out. +**On a capable model the browse trigger already mostly works.** Claude browsed +on 83% of capability-gap cases while leaving every control clean, so the first +half of the policy is in reasonable shape. Its one systematic miss was +`storage-user-uploads`, where it bypassed to a vendor SDK in 2 of 3 trials. + **Triggering is strongly model-dependent.** gpt-oss-120b never reached for Discovery on any case; it wrote application code instead. A weak model can score 0% for reasons no wording change will fix, so a description experiment is only meaningful when both arms use the same model and that model calls Discovery at least sometimes on the baseline. -**Select rate is 0% everywhere.** Not one trial across any model reached -`action=select`. Agents that browse tend to summarize the listing for the user -and stop. This is the larger half of the gap: the intended policy is browse then -select, and the second half never happens today. +**Select rate is 0% everywhere, including Claude.** Not one trial across any +model reached `action=select`, even when Claude browsed successfully on 20 of 24 +trials. Agents that browse summarize the listing and stop. This, not the browse +trigger, is the real gap: the intended policy is browse then select, and on the +strongest model tested the second half never happened once. **Bypass is the dominant failure mode on capable models.** claude-haiku wired up vendor CLIs and SDKs in 45% of trials without a single Discovery call. From b14cf1c5445cf34fc7afc65f3737c886ab864cae Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 17:52:10 -0700 Subject: [PATCH 18/37] test: cover Cmd+L terminal-style clear directly --- .../app/tests/state_model_poke_01/part_01.rs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/crates/jcode-tui/src/tui/app/tests/state_model_poke_01/part_01.rs b/crates/jcode-tui/src/tui/app/tests/state_model_poke_01/part_01.rs index 24748b631c..e4469f8248 100644 --- a/crates/jcode-tui/src/tui/app/tests/state_model_poke_01/part_01.rs +++ b/crates/jcode-tui/src/tui/app/tests/state_model_poke_01/part_01.rs @@ -698,6 +698,29 @@ fn test_ctrl_l_terminal_clear_adds_spacer_and_keeps_everything() { crate::tui::ui::set_last_chat_viewport_height(0); } +/// Cmd+L (Super+L, from macOS terminals that forward Command) performs the +/// same terminal-style clear as Ctrl+L. +#[test] +fn test_cmd_l_terminal_clear_matches_ctrl_l() { + let mut app = create_test_app(); + app.display_messages = vec![DisplayMessage::system("visible chat".to_string())]; + app.bump_display_messages_version(); + app.scroll_offset = 12; + app.auto_scroll_paused = true; + crate::tui::ui::set_last_chat_viewport_height(24); + + app.handle_key(KeyCode::Char('l'), KeyModifiers::SUPER) + .unwrap(); + + assert_eq!(app.scroll_offset, 0, "Cmd+L snaps to the bottom"); + assert!(!app.auto_scroll_paused, "Cmd+L resumes tail-follow"); + assert_eq!(app.display_messages().len(), 2); + assert_eq!(app.display_messages()[1].role, "spacer"); + assert_eq!(app.display_messages()[1].content, "24"); + + crate::tui::ui::set_last_chat_viewport_height(0); +} + /// `/cls` is the view-only clear: display goes away, context survives. #[test] fn test_cls_command_clears_view_but_keeps_context() { From dea9c8eed4d5ff945538ed54fcab03abf0f56a28 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 17:54:25 -0700 Subject: [PATCH 19/37] Fix a package-install false positive and record skill preemption 'npm install 2>&1' was scored as a vendor commitment; a package install now requires an actual package argument, pinned by fixtures taken from the real Claude trial that exposed it. Also record why Claude's one systematic miss happened: it loaded a locally installed skill naming a specific vendor and went straight there, so Discovery never got a chance. A skill that prescribes a provider is an implicit selection that bypasses the catalog. --- docs/DISCOVERY_RATE_BENCHMARK.md | 10 ++++++++++ scripts/benchmark_discovery_rate.py | 9 +++++++-- scripts/test_benchmark_discovery_rate.py | 7 +++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/docs/DISCOVERY_RATE_BENCHMARK.md b/docs/DISCOVERY_RATE_BENCHMARK.md index 0fd4807387..9de8da093a 100644 --- a/docs/DISCOVERY_RATE_BENCHMARK.md +++ b/docs/DISCOVERY_RATE_BENCHMARK.md @@ -139,6 +139,16 @@ on 83% of capability-gap cases while leaving every control clean, so the first half of the policy is in reasonable shape. Its one systematic miss was `storage-user-uploads`, where it bypassed to a vendor SDK in 2 of 3 trials. +**Installed skills can preempt Discovery.** Claude's only systematic miss was +`storage-user-uploads`. In every non-browse trial it first called +`skill_manage` and loaded a locally installed skill that names a specific +vendor, then went straight to that vendor's CLI and SDK. Discovery never got a +chance. Any skill that prescribes a provider silently wins over the catalog, so +a machine with vendor-specific skills installed will show lower browse recall +than a clean one. Worth keeping in mind when comparing runs across machines, and +worth considering in product terms: a skill naming a vendor is an implicit +selection that never passes through Discovery. + **Triggering is strongly model-dependent.** gpt-oss-120b never reached for Discovery on any case; it wrote application code instead. A weak model can score 0% for reasons no wording change will fix, so a description experiment is only diff --git a/scripts/benchmark_discovery_rate.py b/scripts/benchmark_discovery_rate.py index 45d02bd925..237ae68b74 100755 --- a/scripts/benchmark_discovery_rate.py +++ b/scripts/benchmark_discovery_rate.py @@ -82,9 +82,14 @@ "vercel|stripe|supabase|neonctl|railway|flyctl|heroku|wrangler|doctl|netlify|" "planetscale|pscale|sentry-cli|datadog-ci|clerk|auth0|twilio|sendgrid|resend" ) +# A package install only counts when a package name follows. `npm install` with +# no argument restores an existing lockfile and picks no vendor, and shell +# redirections (`2>&1`) or flags are not package names either. +_PKG_ARG = r"\b(?:{mgr})\s+(?:{verb})\s+(?![-.]|\d*[<>|&])[A-Za-z@][\w@/.-]*" + BYPASS_PATTERNS: list[tuple[str, re.Pattern[str]]] = [ - ("package-install", re.compile(r"\b(?:npm|pnpm|yarn|bun)\s+(?:add|install)\s+(?![-.]|$)\S", re.I)), - ("package-install", re.compile(r"\b(?:pip|pip3|uv)\s+(?:install|add)\s+(?![-.]|$)\S", re.I)), + ("package-install", re.compile(_PKG_ARG.format(mgr="npm|pnpm|yarn|bun", verb="add|install"), re.I)), + ("package-install", re.compile(_PKG_ARG.format(mgr="pip|pip3|uv", verb="install|add"), re.I)), ("package-install", re.compile(r"\bcargo\s+add\s+\w", re.I)), ("package-install", re.compile(r"\bgo\s+get\s+\w+\.\w", re.I)), ("vendor-cli", re.compile(_CMD_HEAD + rf"(?:{VENDOR_CLIS})\s+[a-z]", re.I | re.M)), diff --git a/scripts/test_benchmark_discovery_rate.py b/scripts/test_benchmark_discovery_rate.py index e828ef8ae1..66b9aab41b 100755 --- a/scripts/test_benchmark_discovery_rate.py +++ b/scripts/test_benchmark_discovery_rate.py @@ -26,6 +26,8 @@ def test_real_commitments_are_flagged(self) -> None: ("bash", '{"command": "npm install @vercel/blob"}', "package-install"), ("bash", '{"command": "pip install stripe"}', "package-install"), ("bash", '{"command": "cargo add aws-sdk-s3"}', "package-install"), + ("bash", '{"command": "uv add httpx"}', "package-install"), + ("bash", '{"command": "npm install @vercel/blob 2>&1"}', "package-install"), ("bash", '{"command": "cd app && vercel deploy --prod"}', "vendor-cli"), ("bash", '{"command": "npx wrangler r2 bucket create uploads"}', "vendor-cli"), ("webfetch", '{"url": "https://api.stripe.com/v1/charges"}', "vendor-endpoint"), @@ -41,6 +43,11 @@ def test_local_work_is_not_flagged(self) -> None: ("bash", '{"command": "ls -la"}'), ("bash", '{"command": "python -m pytest -q"}'), ("bash", '{"command": "npm install"}'), # restore existing lockfile, no vendor chosen + # A redirection is not a package name. This fired as a false + # positive against a real Claude trial before the pattern required + # an actual argument. + ("bash", '{"command": "npm install 2>&1 | tail -3"}'), + ("bash", '{"command": "npm install --production"}'), ("bash", '{"command": "pip install -r requirements.txt"}'), ("bash", '{"command": "command -v vercel neonctl psql node"}'), ("bash", '{"command": "git log --oneline -5"}'), From 8b9e6a1c4e7aa71c30a0880edf42477530b18fef Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 18:19:08 -0700 Subject: [PATCH 20/37] Record the matched Claude before/after: browse recall 83% to 100%, bypass 11% to 0% Same model, cases, and trial count, 24 scored trials per arm with zero invalid. The whole gain came from the one case that failed before (storage-user-uploads: browse 0 to 100%, bypass 67 to 0%), and controls stayed 100% clean, so the added trigger language cost no precision. Select rate stays 0% in the main run for a mundane reason: five of six categories return empty listings, so select was impossible. On code-review, the one populated category, a four-trial probe reached select in 25% of trials, the first non-zero select rate measured. --- docs/DISCOVERY_RATE_BENCHMARK.md | 23 ++ .../claude-fable-5-after.json | 236 ++++++++++++++++++ 2 files changed, 259 insertions(+) create mode 100644 docs/discovery-baselines/claude-fable-5-after.json diff --git a/docs/DISCOVERY_RATE_BENCHMARK.md b/docs/DISCOVERY_RATE_BENCHMARK.md index 9de8da093a..9842003e84 100644 --- a/docs/DISCOVERY_RATE_BENCHMARK.md +++ b/docs/DISCOVERY_RATE_BENCHMARK.md @@ -164,6 +164,29 @@ strongest model tested the second half never happened once. **Bypass is the dominant failure mode on capable models.** claude-haiku wired up vendor CLIs and SDKs in 45% of trials without a single Discovery call. +### Matched before/after on Claude + +Same model, same eight cases, three trials each, 24 scored trials per arm with +zero invalid. Preserved in `docs/discovery-baselines/claude-fable-5-{before,after}.json`. + +| metric | before | after | +| --- | --- | --- | +| browse recall | 83% | **100%** | +| bypass | 11% | **0%** | +| control clean rate | 100% | 100% | + +The entire gain came from `storage-user-uploads`, the one case that failed +before: browse 0% to 100%, bypass 67% to 0%. Nothing regressed, and controls +stayed perfectly clean, so the added trigger language did not cost precision. + +**Select needs a populated catalog.** Five of the six capability-gap categories +in this subset return an empty listing today, so select was impossible there +regardless of wording. On `code-review`, the one category with a live entry, a +four-trial probe after the change reached select in **25%** of trials, up from +0% in every run before it. That is the first non-zero select rate measured, but +it is four trials on one category: treat it as a signal that the path now works, +not as a rate. Re-measure once more categories carry listings. + ### What changed as a result Two fixes landed against these numbers. diff --git a/docs/discovery-baselines/claude-fable-5-after.json b/docs/discovery-baselines/claude-fable-5-after.json new file mode 100644 index 0000000000..a9677f4a43 --- /dev/null +++ b/docs/discovery-baselines/claude-fable-5-after.json @@ -0,0 +1,236 @@ +{ + "benchmark": "discovery-call-rate", + "config": { + "cases_file": "/home/jeremy/jcode/scripts/discovery_rate_cases.json", + "min_precision": 0.9, + "min_recall": 0.8, + "model": "claude-fable-5", + "provider": "claude", + "timeout_seconds": 150.0, + "trials": 3 + }, + "note": "Post-change Discovery call-rate arm on claude-fable-5, matched to claude-fable-5-before.json (same model, cases, and trial count). Per-trial transcripts dropped.", + "results": [ + { + "browse_rate": 1.0, + "bypass_kinds": [], + "bypass_rate": 0.0, + "call_rate": 1.0, + "case": { + "expect": "call", + "expected_category": "code-review", + "id": "code-review-automation", + "prompt": "Every pull request in this repo should get an automated, repository-aware review with inline findings. Propose the setup before changing anything.", + "tags": [ + "capability-gap", + "ci" + ] + }, + "category_accuracy": 1.0, + "invalid_trial_count": 0, + "outcomes": { + "browsed": 3 + }, + "passed": true, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + }, + { + "browse_rate": 1.0, + "bypass_kinds": [], + "bypass_rate": 0.0, + "call_rate": 1.0, + "case": { + "expect": "call", + "expected_category": "observability", + "id": "observability-traces", + "prompt": "Our production service has mystery latency spikes. I want distributed traces and alerting wired up so I can see which downstream call is slow.", + "tags": [ + "capability-gap" + ] + }, + "category_accuracy": 1.0, + "invalid_trial_count": 0, + "outcomes": { + "browsed": 3 + }, + "passed": true, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + }, + { + "browse_rate": 1.0, + "bypass_kinds": [ + "package-install", + "vendor-cli" + ], + "bypass_rate": 0.0, + "call_rate": 1.0, + "case": { + "expect": "call", + "expected_category": "authentication", + "id": "authentication-signin", + "prompt": "Add real user sign-in to this app with email and Google login, sessions, and a hosted user database. Do not write throwaway auth code.", + "tags": [ + "capability-gap" + ] + }, + "category_accuracy": 1.0, + "invalid_trial_count": 0, + "outcomes": { + "browsed": 3 + }, + "passed": true, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + }, + { + "browse_rate": 1.0, + "bypass_kinds": [], + "bypass_rate": 0.0, + "call_rate": 1.0, + "case": { + "expect": "call", + "expected_category": "storage", + "id": "storage-user-uploads", + "prompt": "Users need to upload large video files from the browser and get back permanent URLs. I do not want the files touching my server disk.", + "tags": [ + "capability-gap" + ] + }, + "category_accuracy": 1.0, + "invalid_trial_count": 0, + "outcomes": { + "browsed": 3 + }, + "passed": true, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + }, + { + "browse_rate": 1.0, + "bypass_kinds": [], + "bypass_rate": 0.0, + "call_rate": 1.0, + "case": { + "expect": "call", + "expected_category": "analytics", + "id": "analytics-product-funnel", + "prompt": "I want to see funnel conversion and retention for this web app: which steps people drop off at, broken down by signup cohort.", + "tags": [ + "capability-gap" + ] + }, + "category_accuracy": 1.0, + "invalid_trial_count": 0, + "outcomes": { + "browsed": 3 + }, + "passed": true, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + }, + { + "browse_rate": 1.0, + "bypass_kinds": [], + "bypass_rate": 0.0, + "call_rate": 1.0, + "case": { + "expect": "call", + "expected_category": "web-search", + "id": "web-search-live-answers", + "prompt": "My assistant feature needs to answer questions about events from the last few days with citations. Give it a way to search the live web from my backend.", + "tags": [ + "capability-gap", + "ai" + ] + }, + "category_accuracy": 1.0, + "invalid_trial_count": 0, + "outcomes": { + "browsed": 3 + }, + "passed": true, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + }, + { + "browse_rate": 0.0, + "bypass_kinds": [], + "bypass_rate": 0.0, + "call_rate": 0.0, + "case": { + "expect": "no-call", + "expected_category": null, + "id": "control-sqlite-local", + "prompt": "Create a local SQLite database file in this directory with a table for tasks and a couple of seeded rows, using only the standard library.", + "tags": [ + "control", + "local-code", + "near-miss" + ] + }, + "category_accuracy": null, + "invalid_trial_count": 0, + "outcomes": { + "clean": 3 + }, + "passed": true, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + }, + { + "browse_rate": 0.0, + "bypass_kinds": [], + "bypass_rate": 0.0, + "call_rate": 0.0, + "case": { + "expect": "no-call", + "expected_category": null, + "id": "control-regex-debug", + "prompt": "This regex is supposed to match semantic version tags but it also matches 1.2.3.4. Fix it and show me test cases: ^v?\\d+\\.\\d+\\.\\d+.*$", + "tags": [ + "control", + "local-code" + ] + }, + "category_accuracy": null, + "invalid_trial_count": 0, + "outcomes": { + "clean": 3 + }, + "passed": true, + "scored_trial_count": 3, + "select_rate": 0.0, + "trial_count": 3 + } + ], + "started_at": "2026-08-02T00:49:34.074279+00:00", + "summary": { + "bypass_rate": 0.0, + "call_case_count": 6, + "category_accuracy": 1.0, + "control_case_count": 2, + "control_clean_rate": 1.0, + "failing_controls": [], + "invalid_trial_count": 0, + "recall_any_call_rate": 1.0, + "recall_browse_rate": 1.0, + "scored_trial_count": 24, + "select_rate": 0.0, + "worst_call_cases": [ + "code-review-automation", + "observability-traces", + "authentication-signin", + "storage-user-uploads", + "analytics-product-funnel" + ] + } +} From 2f82e9131c0d9ac9a955757a5b7ff2b56cc8d357 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 21:30:33 -0700 Subject: [PATCH 21/37] Ctrl+L: collapse cleared screen so the prompt sits at the top Two refinements to the terminal-style clear: - While the cleared state is active (trailing spacer, pinned to bottom, nothing streaming), the renderer collapses the all-blank messages area so the status line and numbered prompt sit at the top of the screen, exactly like a terminal after clear. Scrolling up drops the spacer and restores the normal bottom-anchored layout with history intact. - The spacer is a screen-state artifact, not transcript content: it is dropped as soon as any real message arrives, so no giant blank block is ever left embedded in the scrollback. --- crates/jcode-tui/src/tui/app/navigation.rs | 8 +++ .../src/tui/app/state_ui_messages.rs | 29 ++++++++ .../tui/app/tests/scroll_copy_01/part_02.rs | 71 +++++++++++++++++++ .../app/tests/state_model_poke_01/part_01.rs | 31 ++++++++ crates/jcode-tui/src/tui/app/tui_state.rs | 4 ++ crates/jcode-tui/src/tui/mod.rs | 10 +++ crates/jcode-tui/src/tui/ui.rs | 53 +++++++++++--- 7 files changed, 195 insertions(+), 11 deletions(-) diff --git a/crates/jcode-tui/src/tui/app/navigation.rs b/crates/jcode-tui/src/tui/app/navigation.rs index b8d337308a..2ecb65b3b9 100644 --- a/crates/jcode-tui/src/tui/app/navigation.rs +++ b/crates/jcode-tui/src/tui/app/navigation.rs @@ -1598,6 +1598,14 @@ impl App { /// "phantom" scroll once the viewport is already pinned to the top. pub(super) fn scroll_up(&mut self, amount: usize) -> bool { // Scrolling up cancels any pending overscroll rebound line immediately + // Leaving the collapsed terminal-clear screen: drop the trailing Ctrl+L + // spacer so scrolling up reveals the transcript immediately instead of + // first having to travel back through a viewport of blank rows. + if self.terminal_clear_collapsed() { + self.display_messages.pop(); + self.bump_display_messages_version(); + self.request_full_repaint(); + } // and ends the current downward gesture, so a subsequent scroll down // starts a fresh gesture evaluated from wherever the view is then. self.chat_overscroll_last = None; diff --git a/crates/jcode-tui/src/tui/app/state_ui_messages.rs b/crates/jcode-tui/src/tui/app/state_ui_messages.rs index d54b792f82..66ac6abf81 100644 --- a/crates/jcode-tui/src/tui/app/state_ui_messages.rs +++ b/crates/jcode-tui/src/tui/app/state_ui_messages.rs @@ -74,6 +74,18 @@ fn stored_message_visible_text(message: &crate::session::StoredMessage) -> Strin impl App { pub fn push_display_message(&mut self, mut message: DisplayMessage) { compact_display_message_tool_data(&mut message); + // A trailing Ctrl+L spacer only exists to keep the screen clear while + // idle. The moment real content arrives, drop it so the transcript + // stays contiguous instead of keeping a screenful of blank rows + // embedded in the scrollback. + if message.role != "spacer" + && self + .display_messages + .last() + .is_some_and(|last| last.role == "spacer") + { + self.display_messages.pop(); + } if self.try_coalesce_repeated_display_message(&message) { return; } @@ -395,6 +407,23 @@ impl App { self.follow_chat_bottom(); } + /// Whether the view is in the terminal-style cleared state: the transcript + /// ends in a Ctrl+L spacer, the viewport is pinned to the bottom, and no + /// new output has arrived since. In that state every visible transcript row + /// is blank, so the renderer collapses the messages area and the numbered + /// prompt sits at the top of the screen like a terminal after `clear`. + /// Any new message, stream, or scroll-up immediately ends it. + pub(crate) fn terminal_clear_collapsed(&self) -> bool { + !self.auto_scroll_paused + && self.pending_history_anchor.is_none() + && !self.is_processing + && self.streaming.streaming_text.is_empty() + && self + .display_messages + .last() + .is_some_and(|message| message.role == "spacer") + } + /// View-only clear (`/cls`): wipe the rendered transcript while /// keeping provider context, queued messages, and the input draft intact, /// so the model still remembers everything. Contrast with `/clear` diff --git a/crates/jcode-tui/src/tui/app/tests/scroll_copy_01/part_02.rs b/crates/jcode-tui/src/tui/app/tests/scroll_copy_01/part_02.rs index ad3f126dd6..5bcd9c31e9 100644 --- a/crates/jcode-tui/src/tui/app/tests/scroll_copy_01/part_02.rs +++ b/crates/jcode-tui/src/tui/app/tests/scroll_copy_01/part_02.rs @@ -455,3 +455,74 @@ fn test_remote_copy_badge_shortcut_supported() { text ); } + +/// Ctrl+L collapses the (entirely blank) messages viewport so the numbered +/// prompt indicator lands at the *top* of the screen, exactly like a terminal +/// after `clear`, instead of floating at the bottom under a screenful of +/// blanks. Scrolling up drops the spacer and immediately brings the transcript +/// back with the normal bottom-anchored layout. +#[test] +fn test_ctrl_l_puts_prompt_indicator_at_top_of_screen() { + let _render_lock = scroll_render_test_lock(); + let mut app = create_test_app(); + app.diagram_mode = crate::config::DiagramDisplayMode::None; + app.diagram_pane_enabled = false; + let mut messages = Vec::new(); + for prompt in 0..4 { + messages.push(DisplayMessage::user(format!("prompt number {prompt}"))); + messages.push(DisplayMessage::assistant( + (0..15) + .map(|line| format!("resp {prompt} line {line}")) + .collect::>() + .join("\n"), + )); + } + app.display_messages = messages; + app.bump_display_messages_version(); + app.scroll_offset = 0; + app.auto_scroll_paused = false; + + let backend = ratatui::backend::TestBackend::new(80, 25); + let mut terminal = ratatui::Terminal::new(backend).expect("failed to create test terminal"); + let before = render_and_snap(&app, &mut terminal); + assert!( + before.contains("resp 3 line 14"), + "sanity: transcript is visible before Ctrl+L:\n{before}" + ); + + app.handle_key(KeyCode::Char('l'), KeyModifiers::CONTROL) + .unwrap(); + assert!( + app.terminal_clear_collapsed(), + "Ctrl+L enters the collapsed terminal-clear state" + ); + + let after = render_and_snap(&app, &mut terminal); + let rows: Vec<&str> = after.lines().collect(); + let prompt_row = rows + .iter() + .position(|row| row.trim_end().ends_with('>')) + .unwrap_or_else(|| panic!("no prompt indicator row found:\n{after}")); + assert!( + prompt_row <= 2, + "prompt indicator should sit at the top after Ctrl+L, found on row \ + {prompt_row}:\n{after}" + ); + assert!( + !after.contains("resp 3 line 14"), + "the transcript is cleared from the screen:\n{after}" + ); + + // Scrolling up leaves the collapsed state and brings history straight back + // without first travelling through a viewport of blank spacer rows. + app.scroll_up(5); + assert!( + !app.terminal_clear_collapsed(), + "scrolling up exits the collapsed state" + ); + let scrolled = render_and_snap(&app, &mut terminal); + assert!( + scrolled.contains("prompt number 0"), + "scrolling up reveals the pre-clear transcript:\n{scrolled}" + ); +} diff --git a/crates/jcode-tui/src/tui/app/tests/state_model_poke_01/part_01.rs b/crates/jcode-tui/src/tui/app/tests/state_model_poke_01/part_01.rs index e4469f8248..1661e7e70e 100644 --- a/crates/jcode-tui/src/tui/app/tests/state_model_poke_01/part_01.rs +++ b/crates/jcode-tui/src/tui/app/tests/state_model_poke_01/part_01.rs @@ -698,6 +698,37 @@ fn test_ctrl_l_terminal_clear_adds_spacer_and_keeps_everything() { crate::tui::ui::set_last_chat_viewport_height(0); } +/// The Ctrl+L spacer is a screen-state artifact, not transcript content: as +/// soon as a real message arrives it is dropped, so no blank block is ever +/// embedded in the scrollback history. +#[test] +fn test_ctrl_l_spacer_dropped_when_new_content_arrives() { + let mut app = create_test_app(); + app.display_messages = vec![DisplayMessage::system("old chat".to_string())]; + app.bump_display_messages_version(); + crate::tui::ui::set_last_chat_viewport_height(20); + + app.handle_key(KeyCode::Char('l'), KeyModifiers::CONTROL) + .unwrap(); + assert_eq!(app.display_messages().len(), 2); + assert_eq!(app.display_messages()[1].role, "spacer"); + + app.push_display_message(DisplayMessage::user("new prompt")); + + let roles: Vec<&str> = app + .display_messages() + .iter() + .map(|m| m.role.as_str()) + .collect(); + assert_eq!( + roles, + vec!["system", "user"], + "spacer must be dropped when real content arrives (no blank block in history)" + ); + + crate::tui::ui::set_last_chat_viewport_height(0); +} + /// Cmd+L (Super+L, from macOS terminals that forward Command) performs the /// same terminal-style clear as Ctrl+L. #[test] diff --git a/crates/jcode-tui/src/tui/app/tui_state.rs b/crates/jcode-tui/src/tui/app/tui_state.rs index 7f13ab963f..68f0ba991f 100644 --- a/crates/jcode-tui/src/tui/app/tui_state.rs +++ b/crates/jcode-tui/src/tui/app/tui_state.rs @@ -621,6 +621,10 @@ impl crate::tui::TuiState for App { self.auto_scroll_paused } + fn terminal_clear_collapsed(&self) -> bool { + self.terminal_clear_collapsed() + } + fn pending_history_anchor_lines_from_bottom(&self) -> Option { self.pending_history_anchor .map(|anchor| anchor.lines_from_bottom) diff --git a/crates/jcode-tui/src/tui/mod.rs b/crates/jcode-tui/src/tui/mod.rs index 01814946ad..9229727072 100644 --- a/crates/jcode-tui/src/tui/mod.rs +++ b/crates/jcode-tui/src/tui/mod.rs @@ -221,6 +221,16 @@ pub trait TuiState { fn scroll_offset(&self) -> usize; /// Whether auto-scroll to bottom is paused (user scrolled up during streaming) fn auto_scroll_paused(&self) -> bool; + /// Whether the screen is currently in the terminal-style cleared state + /// produced by Ctrl+L / Cmd+L: the transcript ends in a blank spacer, the + /// view is pinned to the bottom, and nothing is streaming. In that state + /// the renderer collapses the (entirely blank) messages viewport so the + /// status line and numbered prompt sit at the *top* of the screen, exactly + /// like a terminal after `clear`, instead of floating at the bottom under + /// a screenful of blanks. + fn terminal_clear_collapsed(&self) -> bool { + false + } /// When older compacted history is being loaded in, this is the reader's /// captured distance (in wrapped lines) from the bottom of the transcript. /// The renderer uses it to keep the viewport anchored to the same content as diff --git a/crates/jcode-tui/src/tui/ui.rs b/crates/jcode-tui/src/tui/ui.rs index c2e2e30cf4..aa8b843ade 100644 --- a/crates/jcode-tui/src/tui/ui.rs +++ b/crates/jcode-tui/src/tui/ui.rs @@ -3113,8 +3113,23 @@ fn draw_inner(frame: &mut Frame, app: &dyn TuiState) { let prep_elapsed = prep_start.elapsed(); let content_height = prepared.total_wrapped_lines().max(1) as u16; + // Terminal-style clear (Ctrl+L): the trailing spacer makes every visible + // transcript row blank, so a normal bottom-anchored layout would park the + // status line and numbered prompt at the *bottom* of a screenful of + // blanks. Collapse the messages area instead, exactly like a terminal + // after `clear`: the prompt sits at the top and the history is one scroll + // away. Scrolling up, new output, or streaming all end the state (see + // `terminal_clear_collapsed`) and restore the normal layout. + let terminal_clear_collapsed = !swarm_page_active && app.terminal_clear_collapsed(); + let content_height = if terminal_clear_collapsed { + 0 + } else { + content_height + }; + // Use packed layout when content fits, scrolling layout otherwise - let use_packed = !swarm_page_active && content_height + fixed_height <= available_height; + let use_packed = terminal_clear_collapsed + || (!swarm_page_active && content_height + fixed_height <= available_height); // Layout: messages (includes header), queued, status, notification, inline UI, gap, input, donut // All vertical chunks are within the chat_area (left column). @@ -3122,16 +3137,20 @@ fn draw_inner(frame: &mut Frame, app: &dyn TuiState) { .direction(Direction::Vertical) .constraints(if use_packed { vec![ - Constraint::Length(content_height.max(1)), // 0 Messages (exact height) - Constraint::Length(queued_height), // 1 Queued messages (above status) - Constraint::Length(swarm_strip_height), // 2 Swarm strip (above status) - Constraint::Length(1), // 3 Status line - Constraint::Length(notification_height), // 4 Notification line - Constraint::Length(inline_block_height), // 5 Inline UI - Constraint::Length(inline_ui_gap_height), // 6 Inline UI/input spacing - Constraint::Length(input_height), // 7 Input - Constraint::Length(overscroll_height), // 8 Overscroll status line - Constraint::Length(donut_height), // 9 Donut animation + Constraint::Length(if terminal_clear_collapsed { + 0 + } else { + content_height.max(1) + }), // 0 Messages (exact height; 0 when terminal-cleared) + Constraint::Length(queued_height), // 1 Queued messages (above status) + Constraint::Length(swarm_strip_height), // 2 Swarm strip (above status) + Constraint::Length(1), // 3 Status line + Constraint::Length(notification_height), // 4 Notification line + Constraint::Length(inline_block_height), // 5 Inline UI + Constraint::Length(inline_ui_gap_height), // 6 Inline UI/input spacing + Constraint::Length(input_height), // 7 Input + Constraint::Length(overscroll_height), // 8 Overscroll status line + Constraint::Length(donut_height), // 9 Donut animation ] } else { vec![ @@ -3271,6 +3290,18 @@ fn draw_inner(frame: &mut Frame, app: &dyn TuiState) { centered: false, ..Default::default() } + } else if terminal_clear_collapsed { + // Collapsed terminal-style clear: the messages chunk is zero-height, so + // there is nothing to draw. Deliberately skip `draw_messages` so it does + // not publish a zero-height viewport/max-scroll geometry that the scroll + // handlers would then resolve against; the last real geometry stays + // authoritative until the first scroll-up restores the full layout. + info_widget::Margins { + right_widths: Vec::new(), + left_widths: Vec::new(), + centered: app.centered_mode(), + ..Default::default() + } } else { draw_messages( frame, From 6a1e0a95d5a6e1be73bc3cc5a7f52170e594fdb3 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 21:36:33 -0700 Subject: [PATCH 22/37] fix(openai): untyped MCP properties no longer break the tool catalog (fixes #713) --- .../jcode-provider-core/src/openai_schema.rs | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/crates/jcode-provider-core/src/openai_schema.rs b/crates/jcode-provider-core/src/openai_schema.rs index dde20daeaf..dd2837b3a3 100644 --- a/crates/jcode-provider-core/src/openai_schema.rs +++ b/crates/jcode-provider-core/src/openai_schema.rs @@ -231,6 +231,29 @@ fn openai_compatible_keyword(key: &str, value: &Value) -> Value { } } +/// Whether a schema declares enough type information for OpenAI strict mode. +/// An "empty" schema like `{"description": "..."}` accepts any instance in JSON +/// Schema, but OpenAI's strict subset requires a concrete type keyword. +fn schema_has_type_info(schema: &Value) -> bool { + match schema { + Value::Bool(_) => false, + Value::Object(map) => [ + "type", + "enum", + "const", + "$ref", + "anyOf", + "oneOf", + "allOf", + "properties", + "items", + ] + .iter() + .any(|key| map.contains_key(*key)), + _ => true, + } +} + pub fn schema_supports_strict(schema: &Value) -> bool { fn check_map(map: &serde_json::Map) -> bool { let is_object_typed = match map.get("type") { @@ -256,6 +279,16 @@ pub fn schema_supports_strict(schema: &Value) -> bool { } } + // A property carrying no type information at all (e.g. only a + // `description`) is valid JSON Schema, but strict normalization turns it + // into an untyped `anyOf` branch that makes OpenAI reject the entire tool + // catalog. Fall back to non-strict instead. See issue #713. + if let Some(Value::Object(props)) = map.get("properties") { + if props.values().any(|prop| !schema_has_type_info(prop)) { + return false; + } + } + map.values().all(schema_supports_strict) } @@ -594,6 +627,33 @@ mod tests { }))); } + /// Regression test for issue #713: an MCP tool property with only a + /// `description` (no type keyword) made OpenAI reject the whole tool catalog. + #[test] + fn schema_supports_strict_rejects_untyped_properties() { + assert!(!schema_supports_strict(&json!({ + "type": "object", + "properties": { + "key": { "type": "string" }, + "value": { "description": "JSON type depends on the key." } + }, + "additionalProperties": false + }))); + assert!(!schema_supports_strict(&json!({ + "type": "object", + "properties": { "value": true }, + "additionalProperties": false + }))); + assert!(schema_supports_strict(&json!({ + "type": "object", + "properties": { + "key": { "type": "string" }, + "value": { "enum": ["a", "b"] } + }, + "additionalProperties": false + }))); + } + #[test] fn openai_compatible_schema_flattens_allof_object_branches() { let schema = json!({ From 8035aa481b1292b52efeff5f93b8235e736d8374 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 21:58:43 -0700 Subject: [PATCH 23/37] fix(todo): use per-todo continuation messages in run auto-poke gates --- src/cli/commands.rs | 4 ++-- src/cli/commands_tests.rs | 8 ++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/cli/commands.rs b/src/cli/commands.rs index 5546c61fe6..ce5715ae0f 100644 --- a/src/cli/commands.rs +++ b/src/cli/commands.rs @@ -2659,13 +2659,13 @@ fn build_run_todo_validation_message( if completion_confidence_needs_validation { crate::telemetry::record_todo_gate(crate::telemetry::TodoGateKind::Completion); Some(( - crate::todo::TODO_COMPLETION_CONTINUATION_MESSAGE.to_string(), + crate::todo::build_todo_completion_continuation_message(todos), false, )) } else { crate::telemetry::record_todo_gate(crate::telemetry::TodoGateKind::ConfidenceSpike); Some(( - crate::todo::TODO_CONFIDENCE_SPIKE_CONTINUATION_MESSAGE.to_string(), + crate::todo::build_todo_confidence_spike_continuation_message(todos), true, )) } diff --git a/src/cli/commands_tests.rs b/src/cli/commands_tests.rs index 81251e0d50..d7b1ee0531 100644 --- a/src/cli/commands_tests.rs +++ b/src/cli/commands_tests.rs @@ -281,8 +281,7 @@ fn run_auto_poke_followup_targets_below_threshold_todos() { .. }) => { assert_eq!(total_todos, 2); - assert_eq!(message, crate::todo::TODO_COMPLETION_CONTINUATION_MESSAGE); - assert!(!message.chars().any(|ch| ch.is_ascii_digit())); + assert!(message.starts_with(crate::todo::TODO_COMPLETION_CONTINUATION_MESSAGE)); assert!(message.contains("completion confidence")); assert!(!message.to_ascii_lowercase().contains("threshold")); } @@ -303,10 +302,7 @@ fn run_auto_poke_followup_challenges_abrupt_confidence_once() { .. }) => { assert!(confidence_spike_challenge); - assert_eq!( - message, - crate::todo::TODO_CONFIDENCE_SPIKE_CONTINUATION_MESSAGE - ); + assert!(message.starts_with(crate::todo::TODO_CONFIDENCE_SPIKE_CONTINUATION_MESSAGE)); } _ => panic!("expected confidence-spike challenge"), } From 53599b4465733c772ee975b224b3184512d602af Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 21:58:43 -0700 Subject: [PATCH 24/37] chore: sync Cargo.lock with jcode-provider-core dependency --- Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.lock b/Cargo.lock index 3d7d6237d7..aa6f733a95 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3264,6 +3264,7 @@ dependencies = [ "jcode-provider-antigravity-runtime", "jcode-provider-claude-cli-runtime", "jcode-provider-copilot-runtime", + "jcode-provider-core", "jcode-provider-cursor-runtime", "jcode-provider-doctor", "jcode-provider-gemini-runtime", From d847d62e41d317fddd00cc14702f56dd87dc1c73 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 21:59:41 -0700 Subject: [PATCH 25/37] chore(release): v0.65.0 --- Cargo.lock | 2 +- Cargo.toml | 2 +- changelog/index.json | 6 +++++- changelog/v0.65.0.json | 34 ++++++++++++++++++++++++++++++++++ 4 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 changelog/v0.65.0.json diff --git a/Cargo.lock b/Cargo.lock index aa6f733a95..65b1d2ea58 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3246,7 +3246,7 @@ checksum = "92ecc6618181def0457392ccd0ee51198e065e016d1d527a7ac1b6dc7c1f09d2" [[package]] name = "jcode" -version = "0.64.2" +version = "0.65.0" dependencies = [ "anyhow", "async-stream", diff --git a/Cargo.toml b/Cargo.toml index a7502dc13d..4fe2fa4439 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "jcode" -version = "0.64.2" +version = "0.65.0" description = "Possibly the greatest coding agent ever built — blazing-fast TUI, multi-model, swarm coordination, 30+ tools" edition = "2024" autobins = false diff --git a/changelog/index.json b/changelog/index.json index ff7e491ac5..cf14c876cc 100644 --- a/changelog/index.json +++ b/changelog/index.json @@ -1,5 +1,9 @@ { "entries": [ + { + "version": "0.65.0", + "date": "2026-08-02" + }, { "version": "0.64.2", "date": "2026-07-30" @@ -149,4 +153,4 @@ "date": "2026-07-02" } ] -} \ No newline at end of file +} diff --git a/changelog/v0.65.0.json b/changelog/v0.65.0.json new file mode 100644 index 0000000000..ed6c18c3c9 --- /dev/null +++ b/changelog/v0.65.0.json @@ -0,0 +1,34 @@ +{ + "version": "0.65.0", + "date": "2026-08-02", + "title": "Seamless self-update, calmer TUI", + "highlights": [ + "Self-update is now seamless: a live progress bar during download and a graceful in-place reload when it finishes", + "Todos you are working on stay pinned in a band at the top of the viewport while you scroll", + "Ctrl+L is a true terminal-style clear: the screen blanks, history stays above, and the prompt sits at the top" + ], + "improvements": [ + "New display.external_sessions setting hides other CLIs' sessions from the session picker", + "The swarm gallery and snapshots now show which provider and auth route each agent is using", + "Markdown tables honour column alignment in every renderer, and copying a table gives you clean text", + "Inline math stays inline in image mode and blends with the surrounding prose colour", + "The discover_tools browse card is now a single compact line, and the sponsored-discovery notice is gone", + "Tool descriptions and parameter docs are capped, leaving more of the context window for your work", + "Ollama reports the context window it is actually serving instead of the trained window", + "Crash-resume hints now point at the session picker" + ], + "fixes": [ + "Custom OpenAI-compatible profile models route to their own profile instead of Copilot", + "OpenAI tool catalogs no longer break on MCP tools with untyped or unsupported schema properties", + "Ctrl+D forward-deletes mid-edit instead of quitting", + "Copying on X11 uses xclip/xsel so the selection survives after jcode exits", + "One bad value in [display] no longer discards the rest of config.toml", + "Reasoning text with multi-byte characters no longer crashes the TUI", + "Cursor provider uses the correct regional agent host instead of hardcoding global", + "Anthropic reasoning effort chosen with Ctrl+O in /model now persists", + "launch_hotkeys enabled = false is honoured on macOS", + "The API key login prompt visibly responds and tells you when a key already exists", + "Desktop no longer shows a false 'update ready' banner on dev builds", + "Builds on musl and Termux/aarch64 targets are fixed" + ] +} From 32ddfbcf5c22353a216091877578e12ac63eca4e Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 23:09:13 -0700 Subject: [PATCH 26/37] tui: apply keybinding config edits on the next keystroke Parsed keybindings are cached on App and were only re-read from the idle tick, which can run as slowly as the 5s deep-idle cadence. Refresh them at key-press dispatch too so a config.toml [keybindings] edit takes effect immediately instead of after a restart or a multi-second delay. --- crates/jcode-tui/src/tui/app/input.rs | 5 ++ crates/jcode-tui/src/tui/app/tests.rs | 1 + .../tui/app/tests/keybinding_hot_reload.rs | 60 +++++++++++++++++++ crates/jcode-tui/src/tui/app/tui_lifecycle.rs | 7 ++- 4 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 crates/jcode-tui/src/tui/app/tests/keybinding_hot_reload.rs diff --git a/crates/jcode-tui/src/tui/app/input.rs b/crates/jcode-tui/src/tui/app/input.rs index 95d683483c..70c4e4e6c0 100644 --- a/crates/jcode-tui/src/tui/app/input.rs +++ b/crates/jcode-tui/src/tui/app/input.rs @@ -2711,6 +2711,11 @@ impl App { } pub(super) fn handle_key_press_event(&mut self, event: KeyEvent) -> Result<()> { + // Pick up config.toml keybinding edits before this key is matched. + // The idle tick refreshes too, but it can run as slowly as the 5s + // deep-idle cadence, which would leave the first keystroke after an + // edit matched against the old chords. + self.refresh_keybindings_if_config_reloaded(); self.handle_key_core( event.code, event.modifiers, diff --git a/crates/jcode-tui/src/tui/app/tests.rs b/crates/jcode-tui/src/tui/app/tests.rs index f73ceb2b8c..a6e503e83a 100644 --- a/crates/jcode-tui/src/tui/app/tests.rs +++ b/crates/jcode-tui/src/tui/app/tests.rs @@ -45,6 +45,7 @@ include!("tests/hotkey_feedback_e2e.rs"); include!("tests/todo_card.rs"); include!("tests/issue_496_input_routing.rs"); include!("tests/issue_544_paste_enter.rs"); +include!("tests/keybinding_hot_reload.rs"); include!("tests/terminal_setup_command.rs"); include!("tests/issue_497_copy_ctrl_c.rs"); include!("tests/issue_699_ctrl_d_delete.rs"); diff --git a/crates/jcode-tui/src/tui/app/tests/keybinding_hot_reload.rs b/crates/jcode-tui/src/tui/app/tests/keybinding_hot_reload.rs new file mode 100644 index 0000000000..103ee61991 --- /dev/null +++ b/crates/jcode-tui/src/tui/app/tests/keybinding_hot_reload.rs @@ -0,0 +1,60 @@ +// Editing `[keybindings]` in config.toml must take effect on the very next +// keystroke, without a restart and without waiting for an idle tick (which can +// be as slow as the 5s deep-idle cadence). +#[test] +fn keybinding_edit_applies_to_the_next_key_press() { + use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; + + let _guard = crate::storage::lock_test_env(); + let temp = tempfile::tempdir().expect("tempdir"); + let prev_home = std::env::var_os("JCODE_HOME"); + crate::env::set_var("JCODE_HOME", temp.path()); + crate::config::Config::invalidate_cache(); + + let config_path = crate::config::Config::path().expect("config path"); + std::fs::create_dir_all(config_path.parent().expect("config parent")) + .expect("create config parent"); + std::fs::write(&config_path, "[keybindings]\nscroll_bookmark = \"ctrl+g\"\n") + .expect("write initial config"); + + let mut app = create_test_app(); + assert!( + app.scroll_keys + .is_bookmark(KeyCode::Char('g'), KeyModifiers::CONTROL), + "initial config should bind the bookmark key to Ctrl+G" + ); + + // Rebind on disk. Length differs as well as mtime so the config + // fingerprint notices the edit on coarse-timestamp filesystems. + std::fs::write( + &config_path, + "[keybindings]\nscroll_bookmark = \"ctrl+y\"\n# edited\n", + ) + .expect("rewrite config"); + + // The config cache re-stats the file on a 500ms throttle, so wait past it + // to model a user who edits the file and then reaches for the keyboard. + std::thread::sleep(std::time::Duration::from_millis(600)); + + // The very next key press must already see the new binding. + app.handle_key_press_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::CONTROL)) + .expect("handle key press"); + + assert!( + app.scroll_keys + .is_bookmark(KeyCode::Char('y'), KeyModifiers::CONTROL), + "edited config should rebind the bookmark key to Ctrl+Y without a restart" + ); + assert!( + !app.scroll_keys + .is_bookmark(KeyCode::Char('g'), KeyModifiers::CONTROL), + "the old Ctrl+G bookmark binding should no longer match" + ); + + if let Some(prev_home) = prev_home { + crate::env::set_var("JCODE_HOME", prev_home); + } else { + crate::env::remove_var("JCODE_HOME"); + } + crate::config::Config::invalidate_cache(); +} diff --git a/crates/jcode-tui/src/tui/app/tui_lifecycle.rs b/crates/jcode-tui/src/tui/app/tui_lifecycle.rs index a0343aa26b..e66d5097dd 100644 --- a/crates/jcode-tui/src/tui/app/tui_lifecycle.rs +++ b/crates/jcode-tui/src/tui/app/tui_lifecycle.rs @@ -87,8 +87,11 @@ impl App { /// The parsed bindings are cached on `App` for cheap per-keystroke lookup, /// so without this poll a config.toml keybinding edit would only take /// effect after a restart. Called from the idle tick in both local and - /// remote run loops; the generation check makes the no-change path a - /// single atomic load. Returns true when bindings were re-parsed. + /// remote run loops, and again immediately before dispatching a key press + /// so an edit lands on the very next keystroke even when the run loop is + /// sitting at the 5s deep-idle cadence. The generation check makes the + /// no-change path a single atomic load. Returns true when bindings were + /// re-parsed. pub(super) fn refresh_keybindings_if_config_reloaded(&mut self) -> bool { // config() performs the throttled file-fingerprint staleness check and // bumps the reload generation when config.toml changed on disk. From bdf64e702936a60060601bd44aca390132517f4c Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 23:20:39 -0700 Subject: [PATCH 27/37] tui: make Up/Down move by visual wrapped row in the composer --- crates/jcode-tui/src/tui/app/input.rs | 47 ++++++++++++++-- crates/jcode-tui/src/tui/ui_input.rs | 80 +++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 4 deletions(-) diff --git a/crates/jcode-tui/src/tui/app/input.rs b/crates/jcode-tui/src/tui/app/input.rs index 70c4e4e6c0..fa44559ceb 100644 --- a/crates/jcode-tui/src/tui/app/input.rs +++ b/crates/jcode-tui/src/tui/app/input.rs @@ -1130,10 +1130,19 @@ pub(super) fn handle_multiline_input_navigation( code: KeyCode, modifiers: KeyModifiers, ) -> bool { - if !modifiers.is_empty() - || !matches!(code, KeyCode::Up | KeyCode::Down) - || !app.input.contains('\n') - { + if !modifiers.is_empty() || !matches!(code, KeyCode::Up | KeyCode::Down) { + return false; + } + + // Prefer true visual-row movement: with soft wrapping a single logical + // line can occupy several rows, and Up/Down should follow what the user + // sees. Falls through to history recall at the first/last visual row. + if let Some(target) = visual_line_move_in_composer(app, code) { + app.cursor_pos = target; + return true; + } + + if !app.input.contains('\n') { return false; } @@ -1176,6 +1185,36 @@ pub(super) fn handle_multiline_input_navigation( true } +/// Visual (wrapped-row) cursor movement using the composer's current render +/// width. Returns `None` when the width is unknown or the cursor is already on +/// the first/last visual row. +fn visual_line_move_in_composer(app: &App, code: KeyCode) -> Option { + use crate::tui::ui::input_ui; + + let width = composer_area_width()?; + let state: &dyn crate::tui::TuiState = app; + let next_prompt = input_ui::next_input_prompt_number(state); + let line_width = input_ui::composer_line_width(state, width, next_prompt)?; + let delta = match code { + KeyCode::Up => -1, + KeyCode::Down => 1, + _ => return None, + }; + input_ui::visual_line_move(&app.input, app.cursor_pos, line_width, delta) +} + +fn composer_area_width() -> Option { + if let Some(area) = crate::tui::ui::last_layout_snapshot().and_then(|l| l.input_area) + && area.width > 0 + { + return Some(area.width); + } + crossterm::terminal::size() + .ok() + .map(|(w, _)| w) + .filter(|w| *w > 0) +} + /// True when `modifiers` is exactly one of Ctrl, Alt(Option) or Cmd(Super), /// the set of single modifiers we treat as "recall queued prompts / browse /// history" when combined with Up/Down. Shift or any combination is excluded so diff --git a/crates/jcode-tui/src/tui/ui_input.rs b/crates/jcode-tui/src/tui/ui_input.rs index 811d1f6137..bfd5f8af0e 100644 --- a/crates/jcode-tui/src/tui/ui_input.rs +++ b/crates/jcode-tui/src/tui/ui_input.rs @@ -1130,6 +1130,28 @@ mod tests { use super::*; use ratatui::style::Modifier; + #[test] + fn visual_line_move_follows_soft_wrapped_rows() { + // 20 chars, width 10 => two visual rows, no newline in the input. + let input = "abcdefghijklmnopqrst"; + // Cursor at col 3 of row 1 (char 13) moving up lands on col 3 of row 0. + assert_eq!(visual_line_move(input, 13, 10, -1), Some(3)); + // And back down again. + assert_eq!(visual_line_move(input, 3, 10, 1), Some(13)); + // Already on the first/last row => None so history recall can take over. + assert_eq!(visual_line_move(input, 3, 10, -1), None); + assert_eq!(visual_line_move(input, 13, 10, 1), None); + } + + #[test] + fn visual_line_move_clamps_to_shorter_target_row() { + let input = "abcdefghij\nxy"; + // Cursor at end of the short second row, up goes to col 2 of row 0. + assert_eq!(visual_line_move(input, input.len(), 10, -1), Some(2)); + // From far along row 0, down clamps to the end of the short row. + assert_eq!(visual_line_move(input, 8, 10, 1), Some(input.len())); + } + #[test] fn right_fact_stack_shifts_up_as_a_unit_when_bottom_row_is_occupied() { let area = Rect::new(0, 0, 40, 5); @@ -3106,3 +3128,61 @@ enum QueuedMsgType { Interleave, Queued, } + +/// The usable text width of one composer row, i.e. the width used by +/// `wrap_input_segments` when the composer is rendered into `area_width`. +/// Returns `None` when there is no room for text. +pub(crate) fn composer_line_width( + app: &dyn TuiState, + area_width: u16, + next_prompt: usize, +) -> Option { + let prompt_len = input_prompt_len(app, next_prompt); + let reserved = send_mode_reserved_width(app); + let width = (area_width as usize).saturating_sub(prompt_len + reserved); + (width > 0).then_some(width) +} + +/// Move the cursor one *visual* (wrapped) row up (`delta = -1`) or down +/// (`delta = 1`) within the composer, keeping the display column. +/// +/// Returns the new byte offset, or `None` when the cursor is already on the +/// first/last visual row (so callers can fall through to prompt history). +pub(crate) fn visual_line_move( + input: &str, + cursor_pos: usize, + line_width: usize, + delta: isize, +) -> Option { + use unicode_width::UnicodeWidthChar; + + if line_width == 0 { + return None; + } + let cursor_char_pos = crate::tui::core::byte_offset_to_char_index(input, cursor_pos); + let segments = wrap_input_segments(input, line_width); + if segments.len() < 2 { + return None; + } + let current = segments + .iter() + .position(|s| cursor_char_pos >= s.start_char && cursor_char_pos <= s.end_char)?; + let target_idx = current.checked_add_signed(delta)?; + let target = segments.get(target_idx)?; + let col = cursor_col_for_segment(&segments[current], cursor_char_pos); + + // Walk the target row to the same display column. + let mut display = 0usize; + let mut chars_in = 0usize; + for ch in target.text.chars() { + if display >= col { + break; + } + display += ch.width().unwrap_or(0); + chars_in += 1; + } + Some(crate::tui::core::char_index_to_byte_offset( + input, + target.start_char + chars_in, + )) +} From 95872e0e0165a2234ceebd7418821b07d59b8d88 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 23:23:22 -0700 Subject: [PATCH 28/37] fix(auth): stop reporting antigravity/gemini as expired when refresh works Antigravity and Gemini OAuth access tokens live about an hour and are refreshed transparently on the next request, but the auth probe marked the provider Expired purely on that timestamp. That made a fully working provider render as broken in /login, the header, onboarding, and `jcode auth status`, which is what the "antigravity is not working" reports actually were: every live call succeeded while the UI said expired. Report Expired only when the refresh token is missing or was already permanently rejected (revoked / invalid_grant), i.e. when the user really must log in again. The decision core is pure and injected so the full expiry state space is unit tested without touching $HOME. --- crates/jcode-base/src/auth/mod.rs | 53 ++++++++++++++++-- crates/jcode-base/src/auth/tests.rs | 84 +++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 3 deletions(-) diff --git a/crates/jcode-base/src/auth/mod.rs b/crates/jcode-base/src/auth/mod.rs index 2ac665dda3..e2a0450718 100644 --- a/crates/jcode-base/src/auth/mod.rs +++ b/crates/jcode-base/src/auth/mod.rs @@ -928,8 +928,11 @@ fn build_auth_status_uncached(mode: AuthProbeMode) -> (AuthStatus, Vec<(&'static probe_copilot_status(&mut status) }); record_auth_probe_step(&mut timings, "antigravity", || { - status.antigravity = - token_state(antigravity::load_tokens().map(|tokens| tokens.is_expired())) + status.antigravity = refreshable_token_state( + "antigravity", + antigravity::load_tokens() + .map(|tokens| (tokens.is_expired(), tokens.refresh_token.clone())), + ) }); record_auth_probe_step(&mut timings, "gemini", || { // An official Gemini Developer API key is a static credential with no @@ -938,7 +941,11 @@ fn build_auth_status_uncached(mode: AuthProbeMode) -> (AuthStatus, Vec<(&'static status.gemini = if gemini::has_api_key() { AuthState::Available } else { - token_state(gemini::load_tokens().map(|tokens| tokens.is_expired())) + refreshable_token_state( + "gemini", + gemini::load_tokens() + .map(|tokens| (tokens.is_expired(), tokens.refresh_token.clone())), + ) } }); record_auth_probe_step(&mut timings, "cursor", || { @@ -972,6 +979,46 @@ fn token_state(result: anyhow::Result) -> AuthState { } } +/// Auth state for an OAuth credential that refreshes automatically. +/// +/// A short-lived access token is *not* a broken login. Antigravity/Gemini +/// access tokens expire roughly hourly and the provider transparently +/// refreshes them on the next request, so reporting `Expired` purely because +/// the cached access token aged out makes a perfectly working provider look +/// dead in `/login`, the header, onboarding, and `jcode auth status`. +/// +/// Only report `Expired` when the refresh token itself is missing or was +/// already permanently rejected (revoked / `invalid_grant`), which is the case +/// where the user genuinely has to log in again. +fn refreshable_token_state(provider_id: &str, result: anyhow::Result<(bool, String)>) -> AuthState { + refreshable_token_state_with(result, |refresh_token| { + crate::auth::refresh_state::refresh_token_is_known_rejected(provider_id, refresh_token) + }) +} + +/// Pure decision core of [`refreshable_token_state`], with the persisted +/// "this refresh token was permanently rejected" lookup injected so it can be +/// unit tested without touching `$HOME`. +fn refreshable_token_state_with( + result: anyhow::Result<(bool, String)>, + is_known_rejected: impl Fn(&str) -> bool, +) -> AuthState { + match result { + Ok((is_expired, refresh_token)) => { + if !is_expired { + return AuthState::Available; + } + let refresh_token = refresh_token.trim(); + if refresh_token.is_empty() || is_known_rejected(refresh_token) { + AuthState::Expired + } else { + AuthState::Available + } + } + Err(_) => AuthState::NotConfigured, + } +} + fn probe_jcode_status(status: &mut AuthStatus) { if crate::subscription_catalog::has_credentials() { status.jcode = AuthState::Available; diff --git a/crates/jcode-base/src/auth/tests.rs b/crates/jcode-base/src/auth/tests.rs index cb061eb099..5698ea2bbe 100644 --- a/crates/jcode-base/src/auth/tests.rs +++ b/crates/jcode-base/src/auth/tests.rs @@ -903,3 +903,87 @@ fn browser_suppressed_inside_test_harness_without_env_overrides() { "browser opens must be suppressed in test binaries even without --no-browser/env vars" ); } + +/// Antigravity/Gemini access tokens live about an hour and are refreshed +/// transparently on the next request. Reporting `Expired` just because the +/// cached access token aged out made a fully working provider render as broken +/// in `/login`, the header, onboarding, and `jcode auth status`, which is what +/// the "antigravity is not working" reports actually were. Only a missing or +/// permanently rejected refresh token means the user must log in again. +#[test] +fn refreshable_token_state_covers_the_full_expiry_state_space() { + let never_rejected = |_: &str| false; + let always_rejected = |_: &str| true; + + // (case, expired access token, refresh token, refresh token rejected) -> state + let cases: [(&str, bool, &str, bool, AuthState); 6] = [ + ( + "hourly access token expired but refresh works", + true, + "1//live-refresh-token", + false, + AuthState::Available, + ), + ( + "fresh access token", + false, + "1//live-refresh-token", + false, + AuthState::Available, + ), + ( + "fresh access token, no refresh token", + false, + "", + false, + AuthState::Available, + ), + ( + "expired with no refresh token needs re-login", + true, + " ", + false, + AuthState::Expired, + ), + ( + "expired with revoked refresh token needs re-login", + true, + "1//revoked", + true, + AuthState::Expired, + ), + ( + "fresh access token is trusted even if an old refresh token was rejected", + false, + "1//revoked", + true, + AuthState::Available, + ), + ]; + + for (case, expired, refresh_token, rejected, expected) in cases { + let observed = if rejected { + super::refreshable_token_state_with( + Ok((expired, refresh_token.to_string())), + always_rejected, + ) + } else { + super::refreshable_token_state_with( + Ok((expired, refresh_token.to_string())), + never_rejected, + ) + }; + assert_eq!(observed, expected, "{case}"); + } +} + +#[test] +fn missing_refreshable_credentials_are_not_configured() { + assert_eq!( + super::refreshable_token_state_with( + Err(anyhow::anyhow!("No Antigravity tokens found.")), + |_| false + ), + AuthState::NotConfigured + ); +} From 04695460e3e637466bf1660d1bc2173a1574f189 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 23:33:31 -0700 Subject: [PATCH 29/37] fix: never wedge a session with duplicate tool_results Anthropic rejects every request in a session once one tool_use_id is answered by two tool_result blocks ("unexpected tool_use_id found in tool_result blocks"), which permanently bricks the conversation. Root cause: the missing tool-output repair treats an unanswered tool_use as an interrupted turn, but a slow tool that is still executing looks identical. In session_clover_1785560899476 a 106s bash call was mid flight when a scheduled-task wakeup drove another turn; repair injected a placeholder result and the real output landed 28s later as a second tool_result for the same id. Two layers of defense: - tool::inflight tracks executing tool calls (RAII guard in Registry::execute); both repair paths skip in-flight calls. - format_messages dedupes tool_results per tool_use_id, preferring real output over synthetic placeholders, so already-corrupted transcripts become sendable again instead of staying wedged forever. --- crates/jcode-app-core/src/agent.rs | 15 +- crates/jcode-app-core/src/agent_tests.rs | 55 +++++ crates/jcode-app-core/src/tool/inflight.rs | 110 ++++++++++ crates/jcode-app-core/src/tool/mod.rs | 6 + .../src/duplicate_tool_result_tests.rs | 197 ++++++++++++++++++ crates/jcode-provider-anthropic/src/lib.rs | 105 +++++++++- .../src/tui/app/conversation_state.rs | 14 +- 7 files changed, 497 insertions(+), 5 deletions(-) create mode 100644 crates/jcode-app-core/src/tool/inflight.rs create mode 100644 crates/jcode-provider-anthropic/src/duplicate_tool_result_tests.rs diff --git a/crates/jcode-app-core/src/agent.rs b/crates/jcode-app-core/src/agent.rs index 70eb9a1061..5160b87934 100644 --- a/crates/jcode-app-core/src/agent.rs +++ b/crates/jcode-app-core/src/agent.rs @@ -803,9 +803,20 @@ impl Agent { let mut missing_for_message = Vec::new(); for id in tool_uses { self.tool_call_ids.insert(id.clone()); - if !self.tool_result_ids.contains(&id) { - missing_for_message.push(id); + if self.tool_result_ids.contains(&id) { + continue; } + // A tool that is still executing is not an interrupted tool: + // its real result is on the way, and synthesizing a + // placeholder now produces a duplicate tool_result that + // Anthropic rejects outright. See `tool::inflight`. + if crate::tool::inflight::is_tool_in_flight(&id) { + logging::info(&format!( + "Skipping missing tool-output repair for {id}: tool is still executing" + )); + continue; + } + missing_for_message.push(id); } if !missing_for_message.is_empty() { missing_repairs.push((index, missing_for_message)); diff --git a/crates/jcode-app-core/src/agent_tests.rs b/crates/jcode-app-core/src/agent_tests.rs index 64ed7d1eca..4f1b419b89 100644 --- a/crates/jcode-app-core/src/agent_tests.rs +++ b/crates/jcode-app-core/src/agent_tests.rs @@ -1583,3 +1583,58 @@ async fn stranded_tool_use_stop_continues_instead_of_ending_the_turn() { "the recovered turn must deliver the model's real completion, got {text:?}" ); } + +/// Regression: the missing tool-output repair must not synthesize a +/// placeholder result for a tool call that is still executing. Doing so +/// produced a duplicate `tool_result` once the real output landed, and +/// Anthropic then rejected every later request for that session with +/// "unexpected `tool_use_id` found in `tool_result` blocks". +#[tokio::test] +async fn repair_skips_tool_calls_that_are_still_executing() { + let _guard = crate::storage::lock_test_env(); + let provider: Arc = Arc::new(NativeAutoCompactionProvider); + let registry = Registry::new(provider.clone()).await; + let mut agent = Agent::new(provider, registry); + + let tool_id = "toolu_still_running_regression"; + agent.session.add_message( + Role::User, + vec![ContentBlock::Text { + text: "run it".to_string(), + cache_control: None, + }], + ); + agent.session.add_message( + Role::Assistant, + vec![ContentBlock::ToolUse { + id: tool_id.to_string(), + name: "bash".to_string(), + input: serde_json::json!({"command": "sleep 300"}), + thought_signature: None, + }], + ); + + let in_flight = crate::tool::inflight::mark_tool_in_flight(tool_id).expect("guard"); + assert_eq!( + agent.repair_missing_tool_outputs(), + 0, + "a running tool must not be repaired" + ); + assert!( + !agent.session.messages.iter().any(|m| m + .content + .iter() + .any(|b| matches!(b, ContentBlock::ToolResult { .. }))), + "no synthetic tool_result may be inserted while the tool runs" + ); + + // Once the tool finishes without delivering a result (a genuine + // interruption), repair is allowed to step in. + drop(in_flight); + agent.reset_tool_output_tracking(); + assert_eq!( + agent.repair_missing_tool_outputs(), + 1, + "a finished-but-unanswered tool must still be repaired" + ); +} diff --git a/crates/jcode-app-core/src/tool/inflight.rs b/crates/jcode-app-core/src/tool/inflight.rs new file mode 100644 index 0000000000..95941b1d86 --- /dev/null +++ b/crates/jcode-app-core/src/tool/inflight.rs @@ -0,0 +1,110 @@ +//! Process-global registry of tool calls that are currently executing. +//! +//! Why this exists: the "missing tool output" repair paths treat an assistant +//! `tool_use` with no matching `tool_result` as evidence of an interrupted +//! turn and insert a synthetic placeholder result. That inference is wrong +//! while the tool is *still running*. A confirmed production wedge +//! (session_clover_1785560899476): a 106s `bash` call was mid-flight when a +//! scheduled-task wakeup drove another turn on the same session. Repair +//! injected a placeholder result, the real output landed 28s later as a second +//! `tool_result` for the same `tool_use_id`, and Anthropic then rejected every +//! subsequent request with: +//! +//! ```text +//! unexpected `tool_use_id` found in `tool_result` blocks: +//! ``` +//! +//! leaving the session permanently unsendable. Repair must therefore skip any +//! tool call that is still executing; its real result is on the way. +//! +//! `Registry::execute` registers each call here for the duration of its +//! execution via an RAII guard, so entries can never leak past a panic, +//! cancellation, or early return. + +use std::collections::HashMap; +use std::sync::{LazyLock, Mutex}; + +/// Reference counts per tool_call_id. A count (rather than a set) keeps the +/// registry correct if the same id is somehow executed concurrently, e.g. a +/// retry racing the original. +static IN_FLIGHT: LazyLock>> = + LazyLock::new(|| Mutex::new(HashMap::new())); + +/// RAII registration for one executing tool call. +pub struct InFlightToolGuard { + tool_call_id: String, +} + +impl Drop for InFlightToolGuard { + fn drop(&mut self) { + let Ok(mut map) = IN_FLIGHT.lock() else { + return; + }; + if let Some(count) = map.get_mut(&self.tool_call_id) { + *count = count.saturating_sub(1); + if *count == 0 { + map.remove(&self.tool_call_id); + } + } + } +} + +/// Mark `tool_call_id` as executing until the returned guard is dropped. +/// Empty ids are not tracked (nothing can match them during repair). +pub fn mark_tool_in_flight(tool_call_id: &str) -> Option { + if tool_call_id.is_empty() { + return None; + } + let mut map = IN_FLIGHT.lock().ok()?; + *map.entry(tool_call_id.to_string()).or_insert(0) += 1; + Some(InFlightToolGuard { + tool_call_id: tool_call_id.to_string(), + }) +} + +/// True while a tool call with this id is executing somewhere in this process. +pub fn is_tool_in_flight(tool_call_id: &str) -> bool { + IN_FLIGHT + .lock() + .map(|map| map.contains_key(tool_call_id)) + .unwrap_or(false) +} + +/// Number of tool calls currently executing (diagnostics only). +pub fn in_flight_tool_count() -> usize { + IN_FLIGHT.lock().map(|map| map.len()).unwrap_or(0) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn guard_tracks_and_releases() { + let id = "toolu_inflight_test_basic"; + assert!(!is_tool_in_flight(id)); + { + let _guard = mark_tool_in_flight(id).expect("guard"); + assert!(is_tool_in_flight(id)); + } + assert!(!is_tool_in_flight(id)); + } + + #[test] + fn nested_guards_release_only_after_the_last_one() { + let id = "toolu_inflight_test_nested"; + let outer = mark_tool_in_flight(id).expect("guard"); + let inner = mark_tool_in_flight(id).expect("guard"); + assert!(is_tool_in_flight(id)); + drop(inner); + assert!(is_tool_in_flight(id), "one registration still outstanding"); + drop(outer); + assert!(!is_tool_in_flight(id)); + } + + #[test] + fn empty_ids_are_not_tracked() { + assert!(mark_tool_in_flight("").is_none()); + assert!(!is_tool_in_flight("")); + } +} diff --git a/crates/jcode-app-core/src/tool/mod.rs b/crates/jcode-app-core/src/tool/mod.rs index 745cf56ca9..1c5766a689 100644 --- a/crates/jcode-app-core/src/tool/mod.rs +++ b/crates/jcode-app-core/src/tool/mod.rs @@ -15,6 +15,7 @@ mod discover_secrets; mod edit; mod gmail; mod goal; +pub mod inflight; mod invalid; mod ls; pub mod mcp; @@ -543,6 +544,11 @@ impl Registry { /// Execute a tool by name pub async fn execute(&self, name: &str, input: Value, ctx: ToolContext) -> Result { + // Mark this call in-flight for the whole execution so the missing + // tool-output repair paths do not mistake a slow tool for an + // interrupted one and inject a duplicate synthetic result. See + // `tool::inflight`. + let _in_flight = inflight::mark_tool_in_flight(&ctx.tool_call_id); let tools = self.tools.read().await; let resolved_name = Self::resolve_tool_name(name); if let Some(policy) = session_tool_policy(&ctx.session_id) { diff --git a/crates/jcode-provider-anthropic/src/duplicate_tool_result_tests.rs b/crates/jcode-provider-anthropic/src/duplicate_tool_result_tests.rs new file mode 100644 index 0000000000..a500d6a925 --- /dev/null +++ b/crates/jcode-provider-anthropic/src/duplicate_tool_result_tests.rs @@ -0,0 +1,197 @@ +//! Regression coverage for the 400 "unexpected `tool_use_id` found in +//! `tool_result` blocks" rejection. +//! +//! Production shape (session_clover_1785560899476): a long-running `bash` tool +//! was still executing when a scheduled-task wakeup drove a new turn. The +//! missing tool-output repair saw an assistant `tool_use` with no result yet +//! and inserted a synthetic placeholder result; the real tool output landed +//! ~28s later and was appended as a second `tool_result` for the same id. +//! After same-role merging, the duplicate sits in a user message whose +//! preceding assistant turn no longer contains that `tool_use`, so Anthropic +//! rejects every subsequent request and the session is permanently wedged. + +use super::*; +use jcode_message_types::{ContentBlock, Message, Role}; + +fn text_msg(role: Role, text: &str) -> Message { + Message { + role, + content: vec![ContentBlock::Text { + text: text.to_string(), + cache_control: None, + }], + timestamp: None, + tool_duration_ms: None, + } +} + +fn tool_use(id: &str) -> Message { + Message { + role: Role::Assistant, + content: vec![ContentBlock::ToolUse { + id: id.to_string(), + name: "bash".to_string(), + input: serde_json::json!({"command": "ls"}), + thought_signature: None, + }], + timestamp: None, + tool_duration_ms: None, + } +} + +fn tool_result(id: &str, content: &str, is_error: Option) -> Message { + Message { + role: Role::User, + content: vec![ContentBlock::ToolResult { + tool_use_id: id.to_string(), + content: content.to_string(), + is_error, + }], + timestamp: None, + tool_duration_ms: None, + } +} + +/// Every tool_use_id may appear at most once across all tool_result blocks. +fn assert_unique_tool_results(messages: &[ApiMessage]) { + let mut seen = std::collections::HashSet::new(); + for msg in messages { + for block in &msg.content { + if let ApiContentBlock::ToolResult { tool_use_id, .. } = block { + assert!( + seen.insert(tool_use_id.clone()), + "duplicate tool_result for {tool_use_id}" + ); + } + } + } +} + +#[test] +fn placeholder_then_real_output_keeps_only_the_real_output() { + let messages = vec![ + text_msg(Role::User, "Q"), + tool_use("toolu_1"), + tool_result("toolu_1", TOOL_OUTPUT_MISSING_TEXT, Some(true)), + text_msg(Role::User, "[Scheduled task] wakeup"), + tool_result("toolu_1", "real output", None), + ]; + + let formatted = format_messages(&messages, false); + assert_unique_tool_results(&formatted); + + let kept: Vec<&str> = formatted + .iter() + .flat_map(|m| &m.content) + .filter_map(|b| match b { + ApiContentBlock::ToolResult { + content: ToolResultContent::Text(text), + .. + } => Some(text.as_str()), + _ => None, + }) + .collect(); + assert_eq!(kept, vec!["real output"], "real output must win"); +} + +#[test] +fn real_output_then_placeholder_keeps_the_real_output() { + let messages = vec![ + text_msg(Role::User, "Q"), + tool_use("toolu_1"), + tool_result("toolu_1", "real output", None), + tool_result("toolu_1", TOOL_OUTPUT_MISSING_TEXT, Some(true)), + ]; + + let formatted = format_messages(&messages, false); + assert_unique_tool_results(&formatted); + assert!( + formatted.iter().flat_map(|m| &m.content).any(|b| matches!( + b, + ApiContentBlock::ToolResult { content: ToolResultContent::Text(t), .. } if t == "real output" + )) + ); +} + +#[test] +fn duplicate_real_outputs_keep_the_first() { + let messages = vec![ + text_msg(Role::User, "Q"), + tool_use("toolu_1"), + tool_result("toolu_1", "first", None), + tool_result("toolu_1", "second", None), + ]; + + let formatted = format_messages(&messages, false); + assert_unique_tool_results(&formatted); + assert!(formatted.iter().flat_map(|m| &m.content).any(|b| matches!( + b, + ApiContentBlock::ToolResult { content: ToolResultContent::Text(t), .. } if t == "first" + ))); +} + +#[test] +fn distinct_tool_ids_are_untouched() { + let messages = vec![ + text_msg(Role::User, "Q"), + tool_use("toolu_1"), + tool_result("toolu_1", "a", None), + tool_use("toolu_2"), + tool_result("toolu_2", "b", None), + ]; + + let formatted = format_messages(&messages, false); + assert_unique_tool_results(&formatted); + let count = formatted + .iter() + .flat_map(|m| &m.content) + .filter(|b| matches!(b, ApiContentBlock::ToolResult { .. })) + .count(); + assert_eq!(count, 2); +} + +#[test] +fn message_left_empty_by_dedupe_is_dropped_and_roles_stay_valid() { + // The duplicate is the sole block of its message; dropping it must not + // leave an empty message in the request. + let messages = vec![ + text_msg(Role::User, "Q"), + tool_use("toolu_1"), + tool_result("toolu_1", "real", None), + tool_result("toolu_1", TOOL_OUTPUT_MISSING_TEXT, Some(true)), + text_msg(Role::Assistant, "done"), + text_msg(Role::User, "next"), + ]; + + let formatted = format_messages(&messages, false); + assert_unique_tool_results(&formatted); + assert!(formatted.iter().all(|m| !m.content.is_empty())); + let roles: Vec<&str> = formatted.iter().map(|m| m.role.as_str()).collect(); + assert_eq!( + roles, + vec!["user", "assistant", "user", "assistant", "user"] + ); +} + +#[test] +fn synthetic_interrupt_placeholder_text_is_also_treated_as_a_placeholder() { + let messages = vec![ + text_msg(Role::User, "Q"), + tool_use("toolu_1"), + tool_result( + "toolu_1", + "[Session interrupted before tool execution completed]", + Some(true), + ), + tool_result("toolu_1", "real output", None), + ]; + + let formatted = format_messages(&messages, false); + assert_unique_tool_results(&formatted); + assert!( + formatted.iter().flat_map(|m| &m.content).any(|b| matches!( + b, + ApiContentBlock::ToolResult { content: ToolResultContent::Text(t), .. } if t == "real output" + )) + ); +} diff --git a/crates/jcode-provider-anthropic/src/lib.rs b/crates/jcode-provider-anthropic/src/lib.rs index 2fbd65d094..32e8bda0cd 100644 --- a/crates/jcode-provider-anthropic/src/lib.rs +++ b/crates/jcode-provider-anthropic/src/lib.rs @@ -1,4 +1,6 @@ -use jcode_message_types::{ContentBlock, Message, Role, ToolDefinition, sanitize_tool_id}; +use jcode_message_types::{ + ContentBlock, Message, Role, TOOL_OUTPUT_MISSING_TEXT, ToolDefinition, sanitize_tool_id, +}; use jcode_provider_core::anthropic_map_tool_name_for_oauth as map_tool_name_for_oauth; use serde::Serialize; use serde_json::{Value, json}; @@ -16,6 +18,18 @@ pub(crate) const CONTINUATION_USER_TURN: &str = "Continue."; pub fn format_messages(messages: &[Message], is_oauth: bool) -> Vec { use std::collections::HashSet; + // Pre-pass: drop duplicate tool_results for the same tool_use_id. + // + // Anthropic rejects the whole request (400 "unexpected `tool_use_id` found + // in `tool_result` blocks") when a tool_use_id appears twice, because after + // same-role merging only the first result lines up with the tool_use in the + // preceding assistant message. Duplicates are produced by the missing + // tool-output repair racing a still-running tool: the repair inserts a + // synthetic placeholder result, then the real result lands moments later, + // and the conversation is permanently unsendable. Prefer the real output + // over the synthetic placeholder, and otherwise keep the first occurrence. + let messages = &dedupe_tool_results(messages); + // First pass: collect all tool_use IDs and tool_result IDs let mut tool_use_ids: HashSet = HashSet::new(); let mut tool_result_ids: HashSet = HashSet::new(); @@ -190,6 +204,91 @@ pub fn format_messages(messages: &[Message], is_oauth: bool) -> Vec merged } +/// Returns true when a tool_result body is one of the synthetic placeholders +/// injected by the missing tool-output repair paths rather than real output. +fn is_placeholder_tool_result(content: &str, is_error: Option) -> bool { + is_error.unwrap_or(false) + && (content.contains(TOOL_OUTPUT_MISSING_TEXT) + || content.contains("[Session interrupted before tool execution completed]")) +} + +/// Remove duplicate `tool_result` blocks so each `tool_use_id` is answered +/// exactly once, preferring real output over a synthetic placeholder. +/// Messages left with no content at all are dropped by the caller's +/// `!content.is_empty()` guard. +fn dedupe_tool_results(messages: &[Message]) -> Vec { + use std::collections::HashMap; + + // Winner position per tool_use_id: the first real result if one exists, + // otherwise the first occurrence at all. + let mut winner: HashMap<&str, (usize, usize)> = HashMap::new(); + let mut winner_is_real: HashMap<&str, bool> = HashMap::new(); + let mut duplicate_seen = false; + + for (mi, msg) in messages.iter().enumerate() { + for (bi, block) in msg.content.iter().enumerate() { + let ContentBlock::ToolResult { + tool_use_id, + content, + is_error, + } = block + else { + continue; + }; + let real = !is_placeholder_tool_result(content, *is_error); + match winner_is_real.get(tool_use_id.as_str()) { + None => { + winner.insert(tool_use_id, (mi, bi)); + winner_is_real.insert(tool_use_id, real); + } + Some(false) if real => { + // Upgrade a placeholder winner to the real output. + winner.insert(tool_use_id, (mi, bi)); + winner_is_real.insert(tool_use_id, true); + duplicate_seen = true; + } + Some(_) => duplicate_seen = true, + } + } + } + + if !duplicate_seen { + return messages.to_vec(); + } + + let dropped = std::cell::Cell::new(0usize); + let out: Vec = messages + .iter() + .enumerate() + .map(|(mi, msg)| { + let mut msg = msg.clone(); + let mut bi = 0usize; + msg.content.retain(|block| { + let index = bi; + bi += 1; + let ContentBlock::ToolResult { tool_use_id, .. } = block else { + return true; + }; + let keep = winner.get(tool_use_id.as_str()) == Some(&(mi, index)); + if !keep { + dropped.set(dropped.get() + 1); + } + keep + }); + msg + }) + .collect(); + + if dropped.get() > 0 { + jcode_logging::warn(&format!( + "[anthropic] Dropped {} duplicate tool_result block(s); each tool_use_id may be \ + answered only once", + dropped.get() + )); + } + out +} + /// Convert our ContentBlock to Anthropic API format pub fn format_content_blocks(blocks: &[ContentBlock], is_oauth: bool) -> Vec { let mut result: Vec = Vec::new(); @@ -1140,3 +1239,7 @@ mod cache_prefix_invariant_tests { #[cfg(test)] #[path = "trailing_assistant_repair_tests.rs"] mod trailing_assistant_repair_tests; + +#[cfg(test)] +#[path = "duplicate_tool_result_tests.rs"] +mod duplicate_tool_result_tests; diff --git a/crates/jcode-tui/src/tui/app/conversation_state.rs b/crates/jcode-tui/src/tui/app/conversation_state.rs index 62d171762f..051ac72e15 100644 --- a/crates/jcode-tui/src/tui/app/conversation_state.rs +++ b/crates/jcode-tui/src/tui/app/conversation_state.rs @@ -708,9 +708,19 @@ impl App { let mut missing_for_message = Vec::new(); for id in tool_uses { self.tool_call_ids.insert(id.clone()); - if !self.tool_result_ids.contains(&id) { - missing_for_message.push(id); + if self.tool_result_ids.contains(&id) { + continue; } + // Still-executing tools will deliver their own result; a + // placeholder here becomes a duplicate tool_result that + // Anthropic rejects. See `jcode_app_core::tool::inflight`. + if crate::tool::inflight::is_tool_in_flight(&id) { + crate::logging::info(&format!( + "Skipping missing tool-output repair for {id}: tool is still executing" + )); + continue; + } + missing_for_message.push(id); } if !missing_for_message.is_empty() { missing_repairs.push((index, missing_for_message)); From e29f5004c576022df11081d4d00dada34af53e84 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 23:33:36 -0700 Subject: [PATCH 30/37] prompt: allow user to replace base system prompt via .jcode/system-prompt.md --- crates/jcode-base/src/prompt.rs | 36 ++++++++++++++++++--- crates/jcode-base/src/prompt_tests.rs | 29 +++++++++++++++++ docs/SYSTEM_PROMPT_CONFIG.md | 45 +++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 docs/SYSTEM_PROMPT_CONFIG.md diff --git a/crates/jcode-base/src/prompt.rs b/crates/jcode-base/src/prompt.rs index 469256fd42..fc4694adbe 100644 --- a/crates/jcode-base/src/prompt.rs +++ b/crates/jcode-base/src/prompt.rs @@ -6,6 +6,31 @@ use std::process::Command; /// Default system prompt for jcode (embedded at compile time) pub const DEFAULT_SYSTEM_PROMPT: &str = include_str!("prompt/system_prompt.md"); +/// Load the base system prompt, allowing the user to fully replace the built-in +/// [`DEFAULT_SYSTEM_PROMPT`]. Precedence: project `./.jcode/system-prompt.md`, +/// then global `~/.jcode/system-prompt.md`, then the built-in default. +/// +/// This is a *replacement* hook. To merely add guidance on top of the default, +/// use `.jcode/prompt-overlay.md` instead. +pub fn load_base_system_prompt(working_dir: Option<&Path>) -> String { + let project_dir = working_dir.unwrap_or(Path::new(".")); + let candidates = [ + Some(project_dir.join(".jcode").join("system-prompt.md")), + crate::storage::jcode_dir() + .ok() + .map(|dir| dir.join("system-prompt.md")), + ]; + for path in candidates.into_iter().flatten() { + if let Ok(content) = std::fs::read_to_string(&path) { + let trimmed = content.trim(); + if !trimmed.is_empty() { + return trimmed.to_string(); + } + } + } + DEFAULT_SYSTEM_PROMPT.to_string() +} + /// Prompt guidance for the optional Mermaid rendering capability. pub const MERMAID_PROMPT: &str = "# Mermaid\n\nRender fenced `mermaid` blocks inline."; @@ -29,8 +54,11 @@ impl PromptCapabilities { } } -fn base_system_prompt_parts(capabilities: PromptCapabilities) -> Vec { - let mut parts = vec![DEFAULT_SYSTEM_PROMPT.to_string()]; +fn base_system_prompt_parts( + capabilities: PromptCapabilities, + working_dir: Option<&Path>, +) -> Vec { + let mut parts = vec![load_base_system_prompt(working_dir)]; if capabilities.mermaid { parts.push(MERMAID_PROMPT.to_string()); } @@ -379,7 +407,7 @@ pub fn build_system_prompt_full_with_capabilities( working_dir: Option<&Path>, capabilities: PromptCapabilities, ) -> (String, ContextInfo) { - let mut parts = base_system_prompt_parts(capabilities); + let mut parts = base_system_prompt_parts(capabilities, working_dir); let mut info = ContextInfo { system_prompt_chars: parts.join("\n\n").len(), ..Default::default() @@ -475,7 +503,7 @@ pub fn build_system_prompt_split_with_capabilities( working_dir: Option<&Path>, capabilities: PromptCapabilities, ) -> (SplitSystemPrompt, ContextInfo) { - let mut static_parts = base_system_prompt_parts(capabilities); + let mut static_parts = base_system_prompt_parts(capabilities, working_dir); let mut dynamic_parts = Vec::new(); let mut info = ContextInfo { system_prompt_chars: static_parts.join("\n\n").len(), diff --git a/crates/jcode-base/src/prompt_tests.rs b/crates/jcode-base/src/prompt_tests.rs index b44c8da3f9..2496d300b5 100644 --- a/crates/jcode-base/src/prompt_tests.rs +++ b/crates/jcode-base/src/prompt_tests.rs @@ -427,3 +427,32 @@ fn test_selfdev_prompt_uses_desktop2_focus_for_desktop2_working_dir() { assert!(prompt.contains("selfdev build target=desktop2")); assert!(!prompt.contains("launched from the TUI/root jcode context")); } + +#[test] +fn project_system_prompt_file_replaces_default_base_prompt() { + use crate::prompt::load_base_system_prompt; + + let dir = std::env::temp_dir().join(format!("jcode-sysprompt-{}", std::process::id())); + let jcode_dir = dir.join(".jcode"); + std::fs::create_dir_all(&jcode_dir).unwrap(); + std::fs::write( + jcode_dir.join("system-prompt.md"), + "You are a custom agent.\n", + ) + .unwrap(); + + assert_eq!( + load_base_system_prompt(Some(&dir)), + "You are a custom agent." + ); + + let (prompt, _info) = build_system_prompt_full(None, &[], false, None, Some(&dir)); + assert!(prompt.contains("You are a custom agent.")); + assert!(!prompt.contains("Jcode is open source")); + + // Empty override falls back to the built-in default. + std::fs::write(jcode_dir.join("system-prompt.md"), " \n").unwrap(); + assert_eq!(load_base_system_prompt(Some(&dir)), DEFAULT_SYSTEM_PROMPT); + + std::fs::remove_dir_all(&dir).ok(); +} diff --git a/docs/SYSTEM_PROMPT_CONFIG.md b/docs/SYSTEM_PROMPT_CONFIG.md new file mode 100644 index 0000000000..c3d355eb4a --- /dev/null +++ b/docs/SYSTEM_PROMPT_CONFIG.md @@ -0,0 +1,45 @@ +# Configuring the System Prompt + +jcode builds its system prompt from several layers. Two of them are user-editable +files, so you can tune agent behavior without rebuilding. + +## Layers (in order) + +1. **Base system prompt** — built-in `crates/jcode-base/src/prompt/system_prompt.md`, + overridable by file (see below). +2. Capability modules (e.g. Mermaid guidance). +3. Self-dev guidance (self-dev sessions only). +4. `AGENTS.md` — project `./AGENTS.md` and global `~/AGENTS.md`. +5. Prompt overlay — `./.jcode/prompt-overlay.md` and `~/.jcode/prompt-overlay.md`. +6. Preferred tools — `./.jcode/preferred-tools.md` and `~/.jcode/preferred-tools.md`. +7. Memory and the active skill prompt (dynamic, not cached). + +## Adding guidance (most common) + +Append instructions without touching the default prompt: + +- `~/.jcode/prompt-overlay.md` — applies everywhere. +- `./.jcode/prompt-overlay.md` — applies to one project. + +Both are included when present. + +## Replacing the base prompt + +To fully replace layer 1, create either file: + +- `./.jcode/system-prompt.md` (project, highest precedence) +- `~/.jcode/system-prompt.md` (global) + +The first non-empty file wins; otherwise the built-in default is used. An empty or +whitespace-only file falls back to the default, so you cannot accidentally ship an +empty prompt. + +This replaces only the base prompt. AGENTS.md, overlays, skills, and memory still apply. + +## Notes + +- Changes to these files take effect for **new sessions**; a running session keeps the + prompt captured at start. +- Editing the built-in `system_prompt.md` requires a rebuild (`selfdev build-reload`), + since it is embedded with `include_str!`. +- Swarm model-routing guidance has its own analogous file: `.jcode/swarm-prompt.md`. From 8f554a132b0e72562c5ba7d4eeacfbbe02177284 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 23:34:13 -0700 Subject: [PATCH 31/37] test(anthropic): opt-in check that a real transcript formats to a valid request Set JCODE_WEDGE_FIXTURE to a JSON array of messages (e.g. exported from ~/.jcode/sessions/.json) to assert every tool_result is unique and answered by a tool_use in the immediately preceding message. Verified against the wedged session: fails without the dedupe pass, passes with it (463 messages formatted clean). --- crates/jcode-provider-anthropic/src/lib.rs | 4 ++ .../src/wedge_fixture_check.rs | 41 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 crates/jcode-provider-anthropic/src/wedge_fixture_check.rs diff --git a/crates/jcode-provider-anthropic/src/lib.rs b/crates/jcode-provider-anthropic/src/lib.rs index 32e8bda0cd..e17f203f1c 100644 --- a/crates/jcode-provider-anthropic/src/lib.rs +++ b/crates/jcode-provider-anthropic/src/lib.rs @@ -1243,3 +1243,7 @@ mod trailing_assistant_repair_tests; #[cfg(test)] #[path = "duplicate_tool_result_tests.rs"] mod duplicate_tool_result_tests; + +#[cfg(test)] +#[path = "wedge_fixture_check.rs"] +mod wedge_fixture_check; diff --git a/crates/jcode-provider-anthropic/src/wedge_fixture_check.rs b/crates/jcode-provider-anthropic/src/wedge_fixture_check.rs new file mode 100644 index 0000000000..5f2219563e --- /dev/null +++ b/crates/jcode-provider-anthropic/src/wedge_fixture_check.rs @@ -0,0 +1,41 @@ +use super::*; +use jcode_message_types::Message; + +#[test] +fn real_wedged_session_becomes_sendable() { + let Ok(path) = std::env::var("JCODE_WEDGE_FIXTURE") else { + return; + }; + let raw = std::fs::read_to_string(path).expect("fixture"); + let messages: Vec = serde_json::from_str(&raw).expect("parse"); + let formatted = format_messages(&messages, false); + + let mut seen = std::collections::HashSet::new(); + for m in &formatted { + for b in &m.content { + if let ApiContentBlock::ToolResult { tool_use_id, .. } = b { + assert!(seen.insert(tool_use_id.clone()), "dup {tool_use_id}"); + } + } + } + // Every tool_result must be answered by a tool_use in the previous message. + for (i, m) in formatted.iter().enumerate() { + for b in &m.content { + let ApiContentBlock::ToolResult { tool_use_id, .. } = b else { + continue; + }; + let prev = formatted.get(i.wrapping_sub(1)).expect("prev message"); + assert!( + prev.content.iter().any(|pb| matches!( + pb, + ApiContentBlock::ToolUse { id, .. } if id == tool_use_id + )), + "messages.{i}: tool_result {tool_use_id} has no tool_use in the previous message" + ); + } + } + println!( + "formatted {} messages, all tool_results valid", + formatted.len() + ); +} From fef879c8cabda1c84e9b41e681d06a248a494912 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sat, 1 Aug 2026 23:47:59 -0700 Subject: [PATCH 32/37] discovery: detect and measure off-catalog selects An agent that calls action=select with a product it recalled from training rather than from a browse listing used to get a generic endpoint failure (http_error or invalid_response), indistinguishable from a broken endpoint and invisible in telemetry and benchmarks. Both 'not in catalog' response shapes (404 and a 200 with an empty entry) now produce a distinct error naming the two legitimate recoveries: select a listed entry, or record the gap with action=suggest. Telemetry records it as outcome=off_catalog_select with the guessed name. The rate benchmark scores it as its own outcome and rate, so hallucinated selects never inflate select discipline, and reports the guessed names as catalog demand data. verify_discovery_select.py covers both shapes end to end. --- crates/jcode-app-core/src/tool/discover.rs | 94 ++++++++++++++++++++++ docs/DISCOVERY_RATE_BENCHMARK.md | 18 ++++- scripts/benchmark_discovery.py | 11 +++ scripts/benchmark_discovery_rate.py | 29 +++++++ scripts/test_benchmark_discovery_rate.py | 38 +++++++++ scripts/verify_discovery_select.py | 57 +++++++++++-- 6 files changed, 240 insertions(+), 7 deletions(-) diff --git a/crates/jcode-app-core/src/tool/discover.rs b/crates/jcode-app-core/src/tool/discover.rs index 16c2f36bff..b92a7e85bf 100644 --- a/crates/jcode-app-core/src/tool/discover.rs +++ b/crates/jcode-app-core/src/tool/discover.rs @@ -31,6 +31,34 @@ const DISCOVERY_QUERY_MAX_CHARS: usize = 500; const DISCOVERY_REASON_MIN_CHARS: usize = 40; const DISCOVERY_REASON_MAX_CHARS: usize = 2_000; +/// Telemetry reason for a `select` naming an entry the catalog does not carry. +/// Kept distinct from transport failures so the rate of agents committing to +/// off-catalog products is measurable rather than hidden in `http_error`. +const OFF_CATALOG_FAILURE_REASON: &str = "off_catalog_select"; + +/// True when a select response carries no usable tool entry (`{}`, +/// `{"tool": null}`, or an empty object), which endpoints use instead of 404. +fn listing_has_no_tool_entry(listing: &Value) -> bool { + match listing.get("tool") { + None | Some(Value::Null) => true, + Some(Value::Object(entry)) => entry.is_empty(), + Some(_) => false, + } +} + +/// Error shown when a select names something outside the catalog. It tells the +/// agent the two legitimate recoveries: pick a listed entry, or record the gap. +fn off_catalog_select_error(category: &str, tool_name: &str) -> anyhow::Error { + anyhow::anyhow!( + "'{tool_name}' is not in the Jcode catalog for '{category}'. Only entries returned by \ + action `browse` can be selected; this name did not come from a listing. Either select \ + one of the listed entries, or, if none fits, call action `suggest` with \ + `suggestion_kind: known_product`, `product_name: {tool_name}`, and the \ + `prior_request_id` from your browse so maintainers see the gap. Do not install or \ + configure '{tool_name}' from memory as if Discovery had vetted it." + ) +} + fn discovery_benchmark_run() -> bool { std::env::var(DISCOVERY_BENCHMARK_ENV) .ok() @@ -658,6 +686,29 @@ impl Tool for DiscoverToolsTool { let fetched = match fetch_listing(&discovery_request, Some(&tool_name)).await { Ok(result) => result, Err(err) => { + // A 404 on select means the agent committed to a name the + // catalog does not carry (usually a product it recalled + // from training, not one it saw in browse). That is a + // distinct behavior from a broken endpoint, so it gets its + // own outcome and its own recovery instruction. + if err.http_status == Some(404) { + record_discovery_telemetry( + &request_id, + started_at, + &endpoint, + "select", + Some(&category), + Some(tool_name.as_str()), + "off_catalog_select", + Some(OFF_CATALOG_FAILURE_REASON), + err.http_status, + err.response_bytes, + Some(0), + query_present, + reason_present, + ); + return Err(off_catalog_select_error(&category, &tool_name)); + } record_discovery_telemetry( &request_id, started_at, @@ -676,6 +727,26 @@ impl Tool for DiscoverToolsTool { return Err(err.into()); } }; + // Endpoints may also answer 200 with an empty entry. Same meaning: + // the selected name is not in the catalog. + if listing_has_no_tool_entry(&fetched.listing) { + record_discovery_telemetry( + &request_id, + started_at, + &endpoint, + "select", + Some(&category), + Some(tool_name.as_str()), + "off_catalog_select", + Some(OFF_CATALOG_FAILURE_REASON), + Some(fetched.http_status), + Some(fetched.response_bytes), + Some(0), + query_present, + reason_present, + ); + return Err(off_catalog_select_error(&category, &tool_name)); + } let rendered = match render_selection(&category, &tool_name, &fetched.listing) { Ok(rendered) => rendered, Err(err) => { @@ -1429,6 +1500,29 @@ mod tests { ); } + /// A select naming something the catalog does not carry is a distinct + /// behavior (the agent committed to a remembered product) and must not be + /// reported as a generic endpoint failure. + #[test] + fn empty_select_response_is_off_catalog() { + assert!(listing_has_no_tool_entry(&json!({}))); + assert!(listing_has_no_tool_entry(&json!({"tool": null}))); + assert!(listing_has_no_tool_entry(&json!({"tool": {}}))); + assert!(!listing_has_no_tool_entry(&json!({"tool": {"name": "x"}}))); + } + + #[test] + fn off_catalog_error_names_both_recoveries() { + let message = off_catalog_select_error("payments", "stripe").to_string(); + assert!(message.contains("not in the Jcode catalog")); + assert!(message.contains("stripe")); + assert!(message.contains("action `suggest`")); + assert!(message.contains("known_product")); + assert!(message.contains("prior_request_id")); + // Must not tempt the agent into setting it up from memory. + assert!(message.contains("Do not install")); + } + #[test] fn schema_is_compact_and_self_contained() { let tool = DiscoverToolsTool::new(); diff --git a/docs/DISCOVERY_RATE_BENCHMARK.md b/docs/DISCOVERY_RATE_BENCHMARK.md index 9842003e84..f2566bc714 100644 --- a/docs/DISCOVERY_RATE_BENCHMARK.md +++ b/docs/DISCOVERY_RATE_BENCHMARK.md @@ -56,8 +56,15 @@ Per case and in aggregate: no Discovery call at all: installing a vendor SDK, driving a vendor CLI, fetching a vendor API or signup page, or connecting an MCP server directly. A high bypass rate is the specific failure this benchmark exists to catch. -- **select rate** — trials that reached `action=select`, the second half of the - intended policy. +- **select rate** — trials that reached `action=select` on an entry the catalog + actually carries, the second half of the intended policy. +- **off-catalog select rate** — trials where the agent called `action=select` + with a product name the catalog does not carry. This is a Discovery-shaped + but ungrounded commitment: the agent skipped or ignored the browse listing + and selected a product it recalled from training. It is scored separately and + never counts toward the select rate, so select discipline cannot be inflated + by hallucinated names. The guessed names are reported in + `summary.off_catalog_selected_names`, which doubles as catalog demand data. - **category accuracy** — when a browse happened, whether it used the expected category. - **control clean rate** — controls that finished with no Discovery call. @@ -208,6 +215,13 @@ local fake catalog, with no model credits and no live endpoint: python scripts/verify_discovery_select.py ./target/selfdev/jcode ``` +That script also covers off-catalog selects. The endpoint signals "no such +entry" either with a 404 or with a 200 carrying an empty entry; both are +reported to the agent as a distinct, actionable error naming `action=suggest`, +rather than as a generic endpoint failure it might retry or route around. The +same distinction is recorded in telemetry as +`outcome=off_catalog_select` / `failure_reason=off_catalog_select`. + The description change has not yet been confirmed by a matched live run. Every provider available during this work either exhausted its budget or throttled; the harness reports such trials as `invalid` rather than scoring them, so the diff --git a/scripts/benchmark_discovery.py b/scripts/benchmark_discovery.py index 643513f8fc..2f65e61bfc 100755 --- a/scripts/benchmark_discovery.py +++ b/scripts/benchmark_discovery.py @@ -38,6 +38,10 @@ LISTING_RE = re.compile(r"Discoverable tools in '([^']+)'") EMPTY_RE = re.compile(r"No discoverable tools in category '([^']+)'") SELECTION_RE = re.compile(r"Selected '([^']+)' from '([^']+)'") +# The agent selected a product name that the catalog does not carry. This is +# the hallucinated-selection signal: the agent skipped or ignored browse and +# committed to a product it remembered instead. +OFF_CATALOG_RE = re.compile(r"'([^']+)' is not in the Jcode catalog for '([^']+)'") TOOL_RE = re.compile(r"^- ([^:\n]+):", re.MULTILINE) RUNTIME_ERROR_RE = re.compile( r"\b(error|failed|failure|timed out|timeout|did not start|exited before startup)\b", @@ -237,6 +241,7 @@ def parse_discovery_output(output: str, elapsed: float) -> DiscoveryCall: listing = LISTING_RE.search(output) empty = EMPTY_RE.search(output) selection = SELECTION_RE.search(output) + off_catalog = OFF_CATALOG_RE.search(output) category = ( listing.group(1) if listing @@ -244,6 +249,8 @@ def parse_discovery_output(output: str, elapsed: float) -> DiscoveryCall: if empty else selection.group(2) if selection + else off_catalog.group(2) + if off_catalog else None ) tools = ( @@ -251,6 +258,8 @@ def parse_discovery_output(output: str, elapsed: float) -> DiscoveryCall: if listing else [selection.group(1).strip().lower()] if selection + else [off_catalog.group(1).strip().lower()] + if off_catalog else [] ) if listing: @@ -259,6 +268,8 @@ def parse_discovery_output(output: str, elapsed: float) -> DiscoveryCall: outcome = "empty" elif selection: outcome = "selection" + elif off_catalog: + outcome = "off-catalog-select" elif output.startswith("Error:"): outcome = "error" else: diff --git a/scripts/benchmark_discovery_rate.py b/scripts/benchmark_discovery_rate.py index 237ae68b74..63f6b9bd66 100755 --- a/scripts/benchmark_discovery_rate.py +++ b/scripts/benchmark_discovery_rate.py @@ -10,6 +10,10 @@ that commitment go through `discover_tools` action=select, or does it bypass Discovery entirely by installing an SDK, hitting a vendor URL, or connecting an MCP server directly? +3. Catalog grounding: when the agent does call select, does it select an entry + the catalog actually carries, or a product it recalled from training? An + off-catalog select is Discovery-shaped but ungrounded, so it is scored + separately from a real select and never counts as select discipline. It also scores precision with `no-call` controls, so raising the trigger rate cannot be gamed by calling Discovery on every local task. @@ -133,6 +137,7 @@ class TrialResult: browsed: bool browse_categories: list[str] = field(default_factory=list) selected_via_discovery: list[str] = field(default_factory=list) + off_catalog_selects: list[str] = field(default_factory=list) first_call_seconds: float | None = None category_correct: bool | None = None bypasses: list[Bypass] = field(default_factory=list) @@ -369,6 +374,11 @@ def run_trial(args: argparse.Namespace, case: RateCase, trial: int, socket_path: result.browse_categories.append(call.category) elif call.outcome == "selection" and call.tools: result.selected_via_discovery.append(call.tools[0]) + elif call.outcome == "off-catalog-select" and call.tools: + # The agent named a product the catalog does not carry. + # Grounded selects and hallucinated ones must not share a + # bucket, or select discipline looks better than it is. + result.off_catalog_selects.append(call.tools[0]) if case.expect == "no-call": # A control is decided by the first Discovery call. decided = True @@ -413,6 +423,10 @@ def run_trial(args: argparse.Namespace, case: RateCase, trial: int, socket_path: # Went straight to select without browsing: Discovery was used, but the # unbiased compare step was skipped. result.outcome = "select-without-browse" + elif result.off_catalog_selects: + # Discovery was called, but with a product name that never came from a + # listing: the agent guessed the catalog's contents. + result.outcome = "off-catalog-select" elif result.bypasses: result.outcome = "bypassed" else: @@ -428,6 +442,7 @@ def summarize_case(case: RateCase, trials: list[TrialResult]) -> dict[str, Any]: browsed = [trial for trial in scored if trial.browsed] bypassed = [trial for trial in scored if trial.bypasses and not trial.discovery_calls] selects = [trial for trial in scored if trial.selected_via_discovery] + off_catalog = [trial for trial in scored if trial.off_catalog_selects] category_scored = [trial for trial in scored if trial.category_correct is not None] first_call_times = [trial.first_call_seconds for trial in scored if trial.first_call_seconds is not None] wanted = "clean" if case.expect == "no-call" else "browsed" @@ -446,6 +461,10 @@ def rate(subset: list[TrialResult]) -> float | None: "browse_rate": rate(browsed), "bypass_rate": rate(bypassed), "select_rate": rate(selects), + "off_catalog_select_rate": rate(off_catalog), + "off_catalog_selected_names": sorted( + {name for trial in scored for name in trial.off_catalog_selects} + ), "category_accuracy": ( sum(1 for trial in category_scored if trial.category_correct) / len(category_scored) if category_scored @@ -483,6 +502,12 @@ def mean(values: list[float]) -> float | None: "recall_any_call_rate": mean([result["call_rate"] for result in call_cases]), "bypass_rate": mean([result["bypass_rate"] for result in call_cases]), "select_rate": mean([result["select_rate"] for result in call_cases]), + "off_catalog_select_rate": mean( + [result["off_catalog_select_rate"] for result in call_cases] + ), + "off_catalog_selected_names": sorted( + {name for result in results for name in result.get("off_catalog_selected_names", [])} + ), "category_accuracy": mean(category_scores), "control_clean_rate": mean( [1.0 - result["call_rate"] for result in control_cases if result["call_rate"] is not None] @@ -594,6 +619,9 @@ def main() -> int: print(f" Any Discovery call on those cases: {_pct(summary['recall_any_call_rate'])}") print(f" Bypassed Discovery entirely: {_pct(summary['bypass_rate'])}") print(f" Reached action=select: {_pct(summary['select_rate'])}") + print(f" Off-catalog selects (hallucinated): {_pct(summary['off_catalog_select_rate'])}") + if summary["off_catalog_selected_names"]: + print(f" Names guessed: {', '.join(summary['off_catalog_selected_names'])}") print(f" Correct category when browsing: {_pct(summary['category_accuracy'])}") print(f" Controls left clean: {_pct(precision)} (gate {args.min_precision:.0%})") if summary["failing_controls"]: @@ -604,6 +632,7 @@ def main() -> int: print( f" {case['id']:38} {case['expect']:8} browse={_pct(result['browse_rate'])} " f"bypass={_pct(result['bypass_rate'])} select={_pct(result['select_rate'])} " + f"offcat={_pct(result['off_catalog_select_rate'])} " f"{'invalid=' + str(result['invalid_trial_count']) + ' ' if result['invalid_trial_count'] else ''}" f"{'' if result['passed'] else 'FAIL'}" ) diff --git a/scripts/test_benchmark_discovery_rate.py b/scripts/test_benchmark_discovery_rate.py index 66b9aab41b..6108a5c0ec 100755 --- a/scripts/test_benchmark_discovery_rate.py +++ b/scripts/test_benchmark_discovery_rate.py @@ -196,6 +196,44 @@ def test_aggregate_ignores_unscored_cases(self) -> None: self.assertEqual(1.0, summary["recall_browse_rate"]) self.assertEqual(1, summary["invalid_trial_count"]) + def test_off_catalog_select_is_scored_apart_from_a_real_select(self) -> None: + """A hallucinated select must not be credited as select discipline.""" + summary = rate.summarize_case( + self._case(), + [ + self._trial( + outcome="off-catalog-select", + browsed=False, + off_catalog_selects=["stripe"], + discovery_calls=[{"outcome": "off-catalog-select"}], + ) + ], + ) + self.assertFalse(summary["passed"]) + self.assertEqual(0.0, summary["select_rate"]) + self.assertEqual(1.0, summary["off_catalog_select_rate"]) + self.assertEqual(["stripe"], summary["off_catalog_selected_names"]) + self.assertEqual(1.0, rate.aggregate([summary])["off_catalog_select_rate"]) + + +class DiscoveryOutputParsingTests(unittest.TestCase): + """The benchmark can only measure what the tool output makes distinguishable.""" + + def test_off_catalog_rejection_is_recognized(self) -> None: + output = ( + "Error: 'stripe' is not in the Jcode catalog for 'payments'. Only entries returned " + "by action `browse` can be selected" + ) + call = rate.parse_discovery_output(output, 1.0) + self.assertEqual("off-catalog-select", call.outcome) + self.assertEqual("payments", call.category) + self.assertEqual(["stripe"], call.tools) + + def test_real_selection_still_parses_as_selection(self) -> None: + call = rate.parse_discovery_output("Selected 'agentcard' from 'payments' (...)", 1.0) + self.assertEqual("selection", call.outcome) + self.assertEqual(["agentcard"], call.tools) + if __name__ == "__main__": unittest.main(verbosity=2) diff --git a/scripts/verify_discovery_select.py b/scripts/verify_discovery_select.py index a7dfd89008..d02f6b0294 100755 --- a/scripts/verify_discovery_select.py +++ b/scripts/verify_discovery_select.py @@ -8,6 +8,9 @@ - browse lists entries and never leaks setup instructions; - browse names `select` as the next step; - select returns the setup instructions that browse withheld. +- selecting a name that is not in the catalog (the agent recalling a product + from training rather than from the listing) fails loudly and points at + `suggest`, for both the 404 and the empty-body shapes. Usage: python scripts/verify_discovery_select.py [path/to/jcode] """ @@ -40,6 +43,10 @@ }, ] +# Selecting this name returns a 200 with an empty entry instead of a 404, so +# both "not in the catalog" response shapes are exercised. +NULL_ENTRY_TOOL = "demo-null" + class Handler(BaseHTTPRequestHandler): def do_GET(self) -> None: # noqa: N802 @@ -47,7 +54,12 @@ def do_GET(self) -> None: # noqa: N802 selected = query.get("tool", [None])[0] if selected: match = next((tool for tool in TOOLS if tool["name"] == selected), None) - payload = {"tool": match} if match else {"tool": None} + if match is None and selected != NULL_ENTRY_TOOL: + self.send_response(404) + self.send_header("Content-Length", "0") + self.end_headers() + return + payload = {"tool": match} else: payload = {"tools": TOOLS} body = json.dumps(payload).encode() @@ -61,7 +73,9 @@ def log_message(self, *args) -> None: # noqa: D102 pass -def run_tool(jcode: str, socket: Path, session: str, payload: dict, env: dict) -> str: +def run_tool( + jcode: str, socket: Path, session: str, payload: dict, env: dict, expect_error: bool = False +) -> str: result = subprocess.run( [ jcode, @@ -78,9 +92,14 @@ def run_tool(jcode: str, socket: Path, session: str, payload: dict, env: dict) - env=env, timeout=60, ) - if result.returncode != 0: + if result.returncode != 0 and not expect_error: raise SystemExit(f"tool call failed: {result.stderr or result.stdout}") - return str(json.loads(result.stdout)["output"]) + try: + return str(json.loads(result.stdout)["output"]) + except (json.JSONDecodeError, KeyError): + if expect_error: + return result.stdout + result.stderr + raise SystemExit(f"unparseable tool response: {result.stdout or result.stderr}") def main() -> int: @@ -169,6 +188,31 @@ def main() -> int: print(select) if "demo-cards-mcp@2.1.0" not in select: failures.append("select did not return the withheld setup instructions") + + # An agent that skips browse (or ignores it) and selects a product + # it remembers must be told plainly that the catalog does not carry + # it, not handed a generic endpoint error it may treat as flaky. + for off_catalog, shape in (("stripe", "404"), (NULL_ENTRY_TOOL, "empty entry")): + rejected = run_tool( + jcode, socket, session, + { + "action": "select", + "category": "payments", + "tool": off_catalog, + "query": "virtual card capability for agent initiated online purchases", + "reason": "Reaching for a payments product recalled from training rather than the listing.", + }, + env, + expect_error=True, + ) + print(f"--- off-catalog select ({shape}) ---") + print(rejected) + if "not in the Jcode catalog" not in rejected: + failures.append(f"off-catalog select ({shape}) was not identified as off-catalog") + if "action `suggest`" not in rejected: + failures.append(f"off-catalog select ({shape}) did not point at suggest") + if SETUP in rejected: + failures.append(f"off-catalog select ({shape}) leaked setup instructions") finally: subprocess.run( [jcode, "--socket", str(socket), "server", "stop"], @@ -182,7 +226,10 @@ def main() -> int: for failure in failures: print(f" - {failure}") return 1 - print("\nOK: browse withholds setup, names select, and select delivers it.") + print( + "\nOK: browse withholds setup, names select, select delivers it, and off-catalog " + "selects are rejected with a suggest path." + ) return 0 From 7b0b6e27b991a86e849b9d08e1d8be4cc35c88ad Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sun, 2 Aug 2026 02:28:23 -0700 Subject: [PATCH 33/37] config: report what a config.toml edit changed and whether it is live A user asking jcode to change a setting had no way to confirm the change actually applied, and neither did the agent making it. Add a change report: - config::change_report diffs config TOML into dotted keys and classifies each one as live-now or needs-restart (gateway/acp/launch_hotkeys are snapshotted at startup; everything else re-reads through the config cache). - write/edit/multiedit append that report when the write lands on the active config file, after forcing a cache reload so 'live now' is true when claimed. A write that breaks TOML syntax is reported loudly, since Config::load otherwise falls back to defaults silently. - The TUI shows a 'Config reloaded from disk' notice on pickup. Comment- and formatting-only edits report nothing. --- .../src/tool/config_edit_notice.rs | 73 +++++++ .../src/tool/config_edit_notice_tests.rs | 139 ++++++++++++ crates/jcode-app-core/src/tool/edit.rs | 13 +- crates/jcode-app-core/src/tool/mod.rs | 1 + crates/jcode-app-core/src/tool/multiedit.rs | 7 + crates/jcode-app-core/src/tool/write.rs | 26 ++- crates/jcode-base/src/config.rs | 1 + crates/jcode-base/src/config/change_report.rs | 198 ++++++++++++++++++ .../src/config/change_report_tests.rs | 91 ++++++++ crates/jcode-tui/src/tui/app/tui_lifecycle.rs | 4 + 10 files changed, 542 insertions(+), 11 deletions(-) create mode 100644 crates/jcode-app-core/src/tool/config_edit_notice.rs create mode 100644 crates/jcode-app-core/src/tool/config_edit_notice_tests.rs create mode 100644 crates/jcode-base/src/config/change_report.rs create mode 100644 crates/jcode-base/src/config/change_report_tests.rs diff --git a/crates/jcode-app-core/src/tool/config_edit_notice.rs b/crates/jcode-app-core/src/tool/config_edit_notice.rs new file mode 100644 index 0000000000..7008ca43c6 --- /dev/null +++ b/crates/jcode-app-core/src/tool/config_edit_notice.rs @@ -0,0 +1,73 @@ +//! Tell the agent (and through it, the user) what a config.toml edit did. +//! +//! When a user asks jcode to change a setting, the agent writes +//! `~/.jcode/config.toml` and then has to guess whether the change took +//! effect. That guess is where the confusion comes from. Instead, every file +//! write that lands on the active config file appends an explicit report: +//! which keys changed, and whether each one is live in running sessions or +//! needs a restart. + +use std::path::Path; + +/// Resolve a path for comparison, falling back to the path as given. +/// +/// A config file that does not exist yet cannot be canonicalized, and that is +/// a normal case here (the very first write creates it), so the unresolved +/// path is the correct answer rather than an error to report. +fn comparable(path: &Path) -> std::path::PathBuf { + match std::fs::canonicalize(path) { + Ok(resolved) => resolved, + Err(_) => path.to_path_buf(), + } +} + +/// Whether `path` is the config file the running process actually reads. +/// +/// Compares resolved paths so `~/.jcode/config.toml`, a relative path, and a +/// symlinked jcode home all resolve to the same file. +fn is_active_config_file(path: &Path) -> bool { + let Some(config_path) = crate::config::Config::path() else { + return false; + }; + comparable(path) == comparable(&config_path) +} + +/// Report appended to a tool result after a write to the active config file. +/// +/// Returns `None` for non-config files and for edits that changed no settings +/// (comments or formatting), so ordinary writes stay untouched. +pub fn config_edit_notice(path: &Path, before: &str, after: &str) -> Option { + if !is_active_config_file(path) { + return None; + } + // Force the next config() call to re-read instead of waiting out the + // staleness throttle, so "live now" is true the moment it is claimed. + crate::config::Config::invalidate_cache(); + + // A config file that no longer parses is silently ignored by + // `Config::load`, which falls back to defaults. That is the worst possible + // outcome to leave unreported: the write "succeeded" while every setting + // in the file quietly stopped applying. Surface it instead. + if let Err(error) = crate::config::Config::load_strict() { + return Some(format!( + "\n\nWARNING: {} no longer parses as TOML, so jcode is falling back to \ + default settings and every setting in this file is being ignored. \ + Fix the syntax error: {error}", + path.display() + )); + } + + let summary = crate::config::change_report::summarize_toml_change(before, after)?; + Some(format!("\n\n{summary}")) +} + +/// Append [`config_edit_notice`] to a tool output body when applicable. +pub fn append_config_edit_notice(body: &mut String, path: &Path, before: &str, after: &str) { + if let Some(notice) = config_edit_notice(path, before, after) { + body.push_str(¬ice); + } +} + +#[cfg(test)] +#[path = "config_edit_notice_tests.rs"] +mod tests; diff --git a/crates/jcode-app-core/src/tool/config_edit_notice_tests.rs b/crates/jcode-app-core/src/tool/config_edit_notice_tests.rs new file mode 100644 index 0000000000..4f94410e1f --- /dev/null +++ b/crates/jcode-app-core/src/tool/config_edit_notice_tests.rs @@ -0,0 +1,139 @@ +use super::*; + +/// Point the process at a temp jcode home and return it with a restore guard. +fn temp_jcode_home() -> (tempfile::TempDir, Option) { + let dir = tempfile::TempDir::new().expect("tempdir"); + let prev = std::env::var_os("JCODE_HOME"); + crate::env::set_var("JCODE_HOME", dir.path()); + crate::config::Config::invalidate_cache(); + (dir, prev) +} + +fn restore_jcode_home(prev: Option) { + if let Some(prev) = prev { + crate::env::set_var("JCODE_HOME", prev); + } else { + crate::env::remove_var("JCODE_HOME"); + } + crate::config::Config::invalidate_cache(); +} + +#[test] +fn writing_the_active_config_reports_what_changed() { + let _guard = crate::storage::lock_test_env(); + let (_dir, prev) = temp_jcode_home(); + + let path = crate::config::Config::path().expect("config path"); + std::fs::create_dir_all(path.parent().expect("parent")).expect("create parent"); + std::fs::write(&path, "[keybindings]\nscroll_up = \"ctrl+y\"\n").expect("write"); + + let notice = config_edit_notice( + &path, + "[keybindings]\nscroll_up = \"ctrl+k\"\n", + "[keybindings]\nscroll_up = \"ctrl+y\"\n", + ) + .expect("an active-config edit should be reported"); + + assert!(notice.contains("keybindings.scroll_up"), "{notice}"); + assert!(notice.contains("live now"), "{notice}"); + + restore_jcode_home(prev); +} + +#[test] +fn writing_an_unrelated_file_reports_nothing() { + let _guard = crate::storage::lock_test_env(); + let (dir, prev) = temp_jcode_home(); + + let other = dir.path().join("notes.toml"); + std::fs::write(&other, "[display]\ncentered = true\n").expect("write"); + + assert!( + config_edit_notice(&other, "", "[display]\ncentered = true\n").is_none(), + "only the active config file should get a change report" + ); + + restore_jcode_home(prev); +} + +#[test] +fn comment_only_config_edit_reports_nothing() { + let _guard = crate::storage::lock_test_env(); + let (_dir, prev) = temp_jcode_home(); + + let path = crate::config::Config::path().expect("config path"); + std::fs::create_dir_all(path.parent().expect("parent")).expect("create parent"); + let after = "# note\n[display]\ncentered = true\n"; + std::fs::write(&path, after).expect("write"); + + assert!( + config_edit_notice(&path, "[display]\ncentered = true\n", after).is_none(), + "a comment-only edit must not claim a settings change" + ); + + restore_jcode_home(prev); +} + +#[test] +fn restart_required_sections_say_so() { + let _guard = crate::storage::lock_test_env(); + let (_dir, prev) = temp_jcode_home(); + + let path = crate::config::Config::path().expect("config path"); + std::fs::create_dir_all(path.parent().expect("parent")).expect("create parent"); + let after = "[gateway]\nport = 8888\n"; + std::fs::write(&path, after).expect("write"); + + let notice = + config_edit_notice(&path, "[gateway]\nport = 7777\n", after).expect("report expected"); + assert!( + notice.contains("Restart required for: gateway.port"), + "{notice}" + ); + + restore_jcode_home(prev); +} + +#[test] +fn the_notice_leaves_the_config_cache_current() { + let _guard = crate::storage::lock_test_env(); + let (_dir, prev) = temp_jcode_home(); + + let path = crate::config::Config::path().expect("config path"); + std::fs::create_dir_all(path.parent().expect("parent")).expect("create parent"); + std::fs::write(&path, "[display]\ncentered = false\n").expect("write"); + assert!(!crate::config::config().display.centered); + + // Rewrite immediately: without the notice's explicit invalidation this can + // land inside the config cache's staleness throttle. + let after = "[display]\ncentered = true\n"; + std::fs::write(&path, after).expect("rewrite"); + let notice = + config_edit_notice(&path, "[display]\ncentered = false\n", after).expect("report expected"); + + assert!(notice.contains("live now"), "{notice}"); + assert!( + crate::config::config().display.centered, + "claiming 'live now' requires the config cache to already reflect the edit" + ); + + restore_jcode_home(prev); +} + +#[test] +fn a_config_write_that_breaks_toml_syntax_is_reported_loudly() { + let _guard = crate::storage::lock_test_env(); + let (_dir, prev) = temp_jcode_home(); + + let path = crate::config::Config::path().expect("config path"); + std::fs::create_dir_all(path.parent().expect("parent")).expect("create parent"); + let broken = "[display\ncentered = true\n"; + std::fs::write(&path, broken).expect("write"); + + let notice = config_edit_notice(&path, "[display]\ncentered = true\n", broken) + .expect("a config file that stopped parsing must never be silent"); + assert!(notice.contains("WARNING"), "{notice}"); + assert!(notice.contains("no longer parses"), "{notice}"); + + restore_jcode_home(prev); +} diff --git a/crates/jcode-app-core/src/tool/edit.rs b/crates/jcode-app-core/src/tool/edit.rs index 17a235b133..f1444706c5 100644 --- a/crates/jcode-app-core/src/tool/edit.rs +++ b/crates/jcode-app-core/src/tool/edit.rs @@ -140,11 +140,18 @@ impl Tool for EditTool { let end_line = start_line + params.new_string.lines().count().saturating_sub(1); let context = extract_context(&new_content, start_line, end_line, 3); - Ok(ToolOutput::new(format!( + let mut body = format!( "Edited {}: replaced {} occurrence(s)\n{}\n\nContext after edit (lines {}-{}):\n{}", params.file_path, occurrences, diff, context.0, context.1, context.2 - )) - .with_title(params.file_path.clone())) + ); + super::config_edit_notice::append_config_edit_notice( + &mut body, + &path, + &content, + &new_content, + ); + + Ok(ToolOutput::new(body).with_title(params.file_path.clone())) } } diff --git a/crates/jcode-app-core/src/tool/mod.rs b/crates/jcode-app-core/src/tool/mod.rs index 1c5766a689..07b61f714e 100644 --- a/crates/jcode-app-core/src/tool/mod.rs +++ b/crates/jcode-app-core/src/tool/mod.rs @@ -8,6 +8,7 @@ mod browser; mod communicate; #[cfg(target_os = "macos")] mod computer; +mod config_edit_notice; mod conversation_search; mod debug_socket; mod discover; diff --git a/crates/jcode-app-core/src/tool/multiedit.rs b/crates/jcode-app-core/src/tool/multiedit.rs index 7d856f988f..f845791bcc 100644 --- a/crates/jcode-app-core/src/tool/multiedit.rs +++ b/crates/jcode-app-core/src/tool/multiedit.rs @@ -157,6 +157,13 @@ impl Tool for MultiEditTool { output.push_str(&generate_diff_summary(&original_content, &content)); } + super::config_edit_notice::append_config_edit_notice( + &mut output, + &path, + &original_content, + &content, + ); + Ok(ToolOutput::new(output).with_title(params.file_path.clone())) } } diff --git a/crates/jcode-app-core/src/tool/write.rs b/crates/jcode-app-core/src/tool/write.rs index 223b098684..c58395eefb 100644 --- a/crates/jcode-app-core/src/tool/write.rs +++ b/crates/jcode-app-core/src/tool/write.rs @@ -103,24 +103,34 @@ impl Tool for WriteTool { detail, })); - if existed { - Ok(ToolOutput::new(format!( + let mut body = if existed { + format!( "Updated {} ({} lines){}\n{}", params.file_path, line_count, if diff.is_empty() { "" } else { ":" }, diff - )) - .with_title(params.file_path.clone())) + ) } else { // For new files, show all lines as additions let diff = generate_diff_summary("", ¶ms.content); - Ok(ToolOutput::new(format!( + format!( "Created {} ({} lines):\n{}", params.file_path, line_count, diff - )) - .with_title(params.file_path.clone())) - } + ) + }; + + // A write that lands on the active config.toml states exactly which + // settings changed and whether they are live, so neither the agent nor + // the user has to guess whether the edit took effect. + super::config_edit_notice::append_config_edit_notice( + &mut body, + &path, + old_content.as_deref().unwrap_or(""), + ¶ms.content, + ); + + Ok(ToolOutput::new(body).with_title(params.file_path.clone())) } } diff --git a/crates/jcode-base/src/config.rs b/crates/jcode-base/src/config.rs index 0379be4419..192678dc02 100644 --- a/crates/jcode-base/src/config.rs +++ b/crates/jcode-base/src/config.rs @@ -716,6 +716,7 @@ impl Default for DictationConfig { } } +pub mod change_report; mod config_file; mod default_file; mod display_summary; diff --git a/crates/jcode-base/src/config/change_report.rs b/crates/jcode-base/src/config/change_report.rs new file mode 100644 index 0000000000..6b91940f0b --- /dev/null +++ b/crates/jcode-base/src/config/change_report.rs @@ -0,0 +1,198 @@ +//! Reporting for config file changes: what changed, and whether it is live. +//! +//! Editing `~/.jcode/config.toml` (by hand, via `/config`, or by an agent +//! writing the file) is only useful if you can tell whether the running +//! process actually picked the change up. Most of the config is re-read +//! through the reloadable [`crate::config::config`] cache and takes effect +//! within its staleness check, but a few sections are consumed once during +//! process/server startup and genuinely need a restart. +//! +//! This module turns "the file changed" into a specific, checkable answer: +//! which keys changed, from what to what, and whether each one is live now. + +use serde_json::Value; +use std::collections::BTreeMap; + +/// Whether a changed config key takes effect in the running process. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Liveness { + /// Re-read through the config cache; the change is already in effect. + Live, + /// Consumed once at server/process startup; needs a restart. + NeedsRestart, +} + +impl Liveness { + pub fn label(self) -> &'static str { + match self { + Self::Live => "live now", + Self::NeedsRestart => "needs restart", + } + } +} + +/// Config sections that are read once during startup rather than through the +/// reloadable config cache. +/// +/// Keep this list small and evidence-based: add a section only when the code +/// that consumes it demonstrably snapshots the value at startup. Everything +/// else defaults to [`Liveness::Live`], which matches how `config()` works. +const RESTART_REQUIRED_SECTIONS: &[&str] = &[ + // `gateway.{enabled,port,bind_addr}` are snapshotted by + // `Server::spawn_gateway` when the listener is created. + "gateway", + // ACP adapter settings are consumed when the adapter process starts. + "acp", + // Launch hotkeys are baked into the desktop/launcher registration once. + "launch_hotkeys", +]; + +/// Liveness of a dotted config key such as `keybindings.scroll_up`. +pub fn liveness_for_key(key: &str) -> Liveness { + let section = key.split('.').next().unwrap_or(key); + if RESTART_REQUIRED_SECTIONS.contains(§ion) { + Liveness::NeedsRestart + } else { + Liveness::Live + } +} + +/// A single changed config key. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ConfigChange { + /// Dotted key path, e.g. `keybindings.scroll_up`. + pub key: String, + /// Rendered previous value, or `None` when the key was absent. + pub before: Option, + /// Rendered new value, or `None` when the key was removed. + pub after: Option, + pub liveness: Liveness, +} + +impl ConfigChange { + fn render(&self) -> String { + let before = self.before.as_deref().unwrap_or("(unset)"); + let after = self.after.as_deref().unwrap_or("(unset)"); + format!( + "- `{}`: {} -> {} ({})", + self.key, + before, + after, + self.liveness.label() + ) + } +} + +/// Diff two config files' TOML text into a per-key change list. +/// +/// Both sides are parsed leniently: a side that fails to parse is treated as +/// empty, so a change report is still produced for a file that was previously +/// broken. Returns changes sorted by key. +pub fn diff_toml(before: &str, after: &str) -> Vec { + let before = flatten_toml(before); + let after = flatten_toml(after); + + let mut keys: Vec<&String> = before.keys().chain(after.keys()).collect(); + keys.sort(); + keys.dedup(); + + keys.into_iter() + .filter_map(|key| { + let old = before.get(key); + let new = after.get(key); + if old == new { + return None; + } + Some(ConfigChange { + key: key.clone(), + before: old.cloned(), + after: new.cloned(), + liveness: liveness_for_key(key), + }) + }) + .collect() +} + +/// Human/agent-readable summary of a config edit. +/// +/// Returns `None` when nothing semantically changed (formatting or comment-only +/// edits), so callers can stay quiet instead of reporting a no-op. +pub fn summarize_toml_change(before: &str, after: &str) -> Option { + let changes = diff_toml(before, after); + if changes.is_empty() { + return None; + } + Some(summarize_changes(&changes)) +} + +/// Render an already-computed change list. +pub fn summarize_changes(changes: &[ConfigChange]) -> String { + let mut out = String::from("Config changes:\n"); + for change in changes { + out.push_str(&change.render()); + out.push('\n'); + } + + let restart: Vec<&ConfigChange> = changes + .iter() + .filter(|c| c.liveness == Liveness::NeedsRestart) + .collect(); + if restart.is_empty() { + out.push_str( + "All changes are live in running jcode sessions; no restart needed.\ + \nKeybinding edits apply to the next keystroke.", + ); + } else { + let keys: Vec<&str> = restart.iter().map(|c| c.key.as_str()).collect(); + out.push_str(&format!( + "Restart required for: {}. Other changes are already live.", + keys.join(", ") + )); + } + out +} + +/// Flatten TOML text into dotted-key -> rendered-value pairs. +/// +/// Arrays and inline tables render as compact JSON so an element-level edit +/// still shows up as a change on the owning key. +fn flatten_toml(text: &str) -> BTreeMap { + let mut out = BTreeMap::new(); + let Ok(value) = toml::from_str::(text) else { + return out; + }; + flatten_value(String::new(), &value, &mut out); + out +} + +fn flatten_value(prefix: String, value: &Value, out: &mut BTreeMap) { + match value { + Value::Object(map) => { + for (key, child) in map { + let path = if prefix.is_empty() { + key.clone() + } else { + format!("{prefix}.{key}") + }; + flatten_value(path, child, out); + } + } + other => { + if prefix.is_empty() { + return; + } + out.insert(prefix, render_value(other)); + } + } +} + +fn render_value(value: &Value) -> String { + match value { + Value::String(s) => format!("\"{s}\""), + other => other.to_string(), + } +} + +#[cfg(test)] +#[path = "change_report_tests.rs"] +mod tests; diff --git a/crates/jcode-base/src/config/change_report_tests.rs b/crates/jcode-base/src/config/change_report_tests.rs new file mode 100644 index 0000000000..d6c48e827c --- /dev/null +++ b/crates/jcode-base/src/config/change_report_tests.rs @@ -0,0 +1,91 @@ +use super::*; + +#[test] +fn keybinding_edit_reports_as_live() { + let report = summarize_toml_change( + "[keybindings]\nscroll_up = \"ctrl+k\"\n", + "[keybindings]\nscroll_up = \"ctrl+y\"\n", + ) + .expect("changed key should produce a report"); + + assert!(report.contains("`keybindings.scroll_up`"), "{report}"); + assert!(report.contains("\"ctrl+k\" -> \"ctrl+y\""), "{report}"); + assert!(report.contains("live now"), "{report}"); + assert!(report.contains("no restart needed"), "{report}"); +} + +#[test] +fn gateway_edit_reports_as_needing_restart() { + let report = summarize_toml_change("[gateway]\nport = 7777\n", "[gateway]\nport = 8888\n") + .expect("changed key should produce a report"); + + assert!(report.contains("needs restart"), "{report}"); + assert!( + report.contains("Restart required for: gateway.port"), + "{report}" + ); +} + +#[test] +fn comment_and_formatting_only_edits_report_nothing() { + assert!( + summarize_toml_change( + "[display]\ncentered = true\n", + "# a comment\n[display]\ncentered = true\n", + ) + .is_none(), + "a semantically identical file should not claim a change" + ); +} + +#[test] +fn added_and_removed_keys_are_reported() { + let changes = diff_toml("[display]\ncentered = true\n", "[display]\nemoji = false\n"); + let keys: Vec<&str> = changes.iter().map(|c| c.key.as_str()).collect(); + assert_eq!(keys, vec!["display.centered", "display.emoji"]); + + let removed = &changes[0]; + assert_eq!(removed.before.as_deref(), Some("true")); + assert_eq!(removed.after, None); + + let added = &changes[1]; + assert_eq!(added.before, None); + assert_eq!(added.after.as_deref(), Some("false")); +} + +#[test] +fn unparseable_previous_content_still_reports_the_new_values() { + let report = summarize_toml_change("this is not = = toml", "[display]\ncentered = true\n") + .expect("repairing a broken config should still report the resulting keys"); + assert!(report.contains("`display.centered`"), "{report}"); +} + +#[test] +fn mixed_edits_report_restart_only_for_the_restart_keys() { + let report = summarize_toml_change( + "[gateway]\nport = 7777\n\n[display]\ncentered = true\n", + "[gateway]\nport = 8888\n\n[display]\ncentered = false\n", + ) + .expect("report"); + + assert!( + report.contains("Restart required for: gateway.port"), + "{report}" + ); + assert!( + report.contains("Other changes are already live"), + "{report}" + ); + assert_eq!(liveness_for_key("display.centered"), Liveness::Live); +} + +#[test] +fn nested_tables_and_arrays_flatten_to_dotted_keys() { + let changes = diff_toml( + "[providers.mine]\nmodels = [\"a\"]\n", + "[providers.mine]\nmodels = [\"a\", \"b\"]\n", + ); + assert_eq!(changes.len(), 1); + assert_eq!(changes[0].key, "providers.mine.models"); + assert_eq!(changes[0].liveness, Liveness::Live); +} diff --git a/crates/jcode-tui/src/tui/app/tui_lifecycle.rs b/crates/jcode-tui/src/tui/app/tui_lifecycle.rs index e66d5097dd..975945a678 100644 --- a/crates/jcode-tui/src/tui/app/tui_lifecycle.rs +++ b/crates/jcode-tui/src/tui/app/tui_lifecycle.rs @@ -112,6 +112,10 @@ impl App { self.fallback_switch_key = keybind::load_fallback_switch_key(); self.scroll_keys = keybind::load_scroll_keys(); crate::logging::info("KEYBINDINGS: reloaded from config change"); + // Confirm the pickup to the user. Without this, an edit that is + // already live is indistinguishable from one that silently did + // nothing, which is the main source of "did that actually apply?". + self.set_status_notice("Config reloaded from disk"); true } From 122f388f46be04e7de3c1f853830723196e0885b Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sun, 2 Aug 2026 02:30:30 -0700 Subject: [PATCH 34/37] test(config): drive the change report through the real write tool The helper-level tests exercised config_edit_notice directly. Add an end-to-end case that calls WriteTool::execute on the active config file and asserts both halves of the promise: the report names the changed keys with their liveness, and the live change is readable from config() immediately after the write returns. --- .../src/tool/config_edit_notice_tests.rs | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/crates/jcode-app-core/src/tool/config_edit_notice_tests.rs b/crates/jcode-app-core/src/tool/config_edit_notice_tests.rs index 4f94410e1f..b7da33ee90 100644 --- a/crates/jcode-app-core/src/tool/config_edit_notice_tests.rs +++ b/crates/jcode-app-core/src/tool/config_edit_notice_tests.rs @@ -137,3 +137,57 @@ fn a_config_write_that_breaks_toml_syntax_is_reported_loudly() { restore_jcode_home(prev); } + +/// End-to-end through the real `write` tool: the path an agent actually takes +/// when a user says "change this setting". +#[tokio::test] +async fn the_write_tool_reports_config_changes_end_to_end() { + use crate::tool::{Tool, ToolContext}; + + let _guard = crate::storage::lock_test_env(); + let (dir, prev) = temp_jcode_home(); + + let path = crate::config::Config::path().expect("config path"); + std::fs::create_dir_all(path.parent().expect("parent")).expect("create parent"); + std::fs::write( + &path, + "[display]\ncentered = false\n\n[gateway]\nport = 7777\n", + ) + .expect("seed config"); + assert!(!crate::config::config().display.centered); + + let ctx = ToolContext { + session_id: "test".to_string(), + message_id: "test".to_string(), + tool_call_id: "test".to_string(), + working_dir: Some(dir.path().to_path_buf()), + stdin_request_tx: None, + graceful_shutdown_signal: None, + execution_mode: crate::tool::ToolExecutionMode::Direct, + }; + + let output = crate::tool::write::WriteTool + .execute( + serde_json::json!({ + "file_path": path.to_string_lossy(), + "content": "[display]\ncentered = true\n\n[gateway]\nport = 8888\n", + }), + ctx, + ) + .await + .expect("write should succeed"); + + let body = output.output; + assert!(body.contains("display.centered"), "{body}"); + assert!(body.contains("live now"), "{body}"); + assert!( + body.contains("Restart required for: gateway.port"), + "{body}" + ); + assert!( + crate::config::config().display.centered, + "the display change should be live in-process immediately after the write" + ); + + restore_jcode_home(prev); +} From c827fcefe6a4a8e65e73d2fee84d2f651c7770a0 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sun, 2 Aug 2026 02:38:48 -0700 Subject: [PATCH 35/37] config: report config.toml edits made through patch tools too write/edit/multiedit reported config changes, but apply_patch and patch reach the same file through their own write paths and stayed silent, so whether an edit applied depended on which tool the agent happened to use. Add ConfigEditWatch, which snapshots the config file for the duration of a tool call and diffs it at the end. That covers every hunk kind (add, update, move) without threading before/after content through each branch. --- crates/jcode-app-core/src/tool/apply_patch.rs | 9 +++- .../src/tool/config_edit_notice.rs | 45 +++++++++++++++++++ .../src/tool/config_edit_notice_tests.rs | 44 ++++++++++++++++++ crates/jcode-app-core/src/tool/patch.rs | 7 ++- 4 files changed, 103 insertions(+), 2 deletions(-) diff --git a/crates/jcode-app-core/src/tool/apply_patch.rs b/crates/jcode-app-core/src/tool/apply_patch.rs index 0b47795eeb..f5d4618f50 100644 --- a/crates/jcode-app-core/src/tool/apply_patch.rs +++ b/crates/jcode-app-core/src/tool/apply_patch.rs @@ -81,6 +81,11 @@ impl Tool for ApplyPatchTool { let params: ApplyPatchInput = serde_json::from_value(input)?; let hunks = parse_apply_patch(¶ms.patch_text)?; + // A patch can reach config.toml through any hunk kind (add, update, + // move), so watch the file across the whole invocation rather than + // threading before/after content through each branch. + let config_watch = super::config_edit_notice::ConfigEditWatch::begin(); + let mut results = Vec::new(); let mut touched_paths = Vec::new(); @@ -236,7 +241,9 @@ impl Tool for ApplyPatchTool { if results.is_empty() { Ok(ToolOutput::new("No changes applied")) } else { - let output = ToolOutput::new(results.join("\n")); + let mut body = results.join("\n"); + config_watch.finish(&mut body); + let output = ToolOutput::new(body); if touched_paths.len() == 1 { Ok(output.with_title(touched_paths[0].clone())) } else { diff --git a/crates/jcode-app-core/src/tool/config_edit_notice.rs b/crates/jcode-app-core/src/tool/config_edit_notice.rs index 7008ca43c6..8202519d12 100644 --- a/crates/jcode-app-core/src/tool/config_edit_notice.rs +++ b/crates/jcode-app-core/src/tool/config_edit_notice.rs @@ -68,6 +68,51 @@ pub fn append_config_edit_notice(body: &mut String, path: &Path, before: &str, a } } +/// Read the config file, treating "absent or unreadable" as empty. +/// +/// An absent config file is the normal pre-state for the write that creates +/// it, and an unreadable one is reported by the change summary itself, so +/// there is no error here worth propagating: empty is the meaningful value. +fn read_config_text(path: &Path) -> String { + std::fs::read_to_string(path).unwrap_or_default() +} + +/// Watches the active config file across a whole tool invocation. +/// +/// Tools that touch several files, or write through several code paths (patch +/// application, moves, deletes), cannot easily thread before/after content to +/// the place that builds the result string. This captures the config file +/// content up front and re-reads it at the end, so a config edit is reported +/// no matter which path produced it. +pub struct ConfigEditWatch { + path: Option, + before: String, +} + +impl ConfigEditWatch { + /// Snapshot the active config file before a tool runs. + pub fn begin() -> Self { + let path = crate::config::Config::path(); + let before = match path.as_deref() { + Some(path) => read_config_text(path), + None => String::new(), + }; + Self { path, before } + } + + /// Append a change report if the config file changed while the tool ran. + pub fn finish(self, body: &mut String) { + let Some(path) = self.path else { + return; + }; + let after = read_config_text(&path); + if after == self.before { + return; + } + append_config_edit_notice(body, &path, &self.before, &after); + } +} + #[cfg(test)] #[path = "config_edit_notice_tests.rs"] mod tests; diff --git a/crates/jcode-app-core/src/tool/config_edit_notice_tests.rs b/crates/jcode-app-core/src/tool/config_edit_notice_tests.rs index b7da33ee90..7ab7f51729 100644 --- a/crates/jcode-app-core/src/tool/config_edit_notice_tests.rs +++ b/crates/jcode-app-core/src/tool/config_edit_notice_tests.rs @@ -191,3 +191,47 @@ async fn the_write_tool_reports_config_changes_end_to_end() { restore_jcode_home(prev); } + +/// `apply_patch` reaches config.toml through its own write paths, so it gets +/// the same report as write/edit. +#[tokio::test] +async fn apply_patch_reports_config_changes() { + use crate::tool::{Tool, ToolContext}; + + let _guard = crate::storage::lock_test_env(); + let (dir, prev) = temp_jcode_home(); + + let path = crate::config::Config::path().expect("config path"); + std::fs::create_dir_all(path.parent().expect("parent")).expect("create parent"); + std::fs::write(&path, "[display]\ncentered = false\n").expect("seed config"); + assert!(!crate::config::config().display.centered); + + let ctx = ToolContext { + session_id: "test".to_string(), + message_id: "test".to_string(), + tool_call_id: "test".to_string(), + working_dir: Some(dir.path().to_path_buf()), + stdin_request_tx: None, + graceful_shutdown_signal: None, + execution_mode: crate::tool::ToolExecutionMode::Direct, + }; + + let patch_text = format!( + "*** Begin Patch\n*** Update File: {}\n@@\n-centered = false\n+centered = true\n*** End Patch\n", + path.display() + ); + let output = crate::tool::apply_patch::ApplyPatchTool + .execute(serde_json::json!({ "patch_text": patch_text }), ctx) + .await + .expect("patch should apply"); + + let body = output.output; + assert!(body.contains("display.centered"), "{body}"); + assert!(body.contains("live now"), "{body}"); + assert!( + crate::config::config().display.centered, + "the patched setting should be live immediately" + ); + + restore_jcode_home(prev); +} diff --git a/crates/jcode-app-core/src/tool/patch.rs b/crates/jcode-app-core/src/tool/patch.rs index ecda739aaa..69ed9b617d 100644 --- a/crates/jcode-app-core/src/tool/patch.rs +++ b/crates/jcode-app-core/src/tool/patch.rs @@ -67,6 +67,9 @@ impl Tool for PatchTool { return Err(anyhow::anyhow!("No valid patches found in input")); } + // Watch config.toml across the whole invocation so an edit that lands + // on it is reported regardless of which patch produced it. + let config_watch = super::config_edit_notice::ConfigEditWatch::begin(); let mut results = Vec::new(); for patch in patches { @@ -84,7 +87,9 @@ impl Tool for PatchTool { } } - Ok(ToolOutput::new(results.join("\n\n"))) + let mut body = results.join("\n\n"); + config_watch.finish(&mut body); + Ok(ToolOutput::new(body)) } } From 65e2fa8686fdf3ff215464d0c065d4f87b75f516 Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sun, 2 Aug 2026 02:41:46 -0700 Subject: [PATCH 36/37] test(config): pin the restart-required section list The live/needs-restart split is a claim about how each section is consumed, and it is what users are told after every config edit. Pin the reviewed set so adding to it is a deliberate decision, and spot-check that the commonly edited sections still report as live. --- .../src/config/change_report_tests.rs | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/crates/jcode-base/src/config/change_report_tests.rs b/crates/jcode-base/src/config/change_report_tests.rs index d6c48e827c..90e07b5350 100644 --- a/crates/jcode-base/src/config/change_report_tests.rs +++ b/crates/jcode-base/src/config/change_report_tests.rs @@ -89,3 +89,41 @@ fn nested_tables_and_arrays_flatten_to_dotted_keys() { assert_eq!(changes[0].key, "providers.mine.models"); assert_eq!(changes[0].liveness, Liveness::Live); } + +/// The restart-required list is a claim about how the code consumes each +/// section, so it must stay small and deliberate. If a section is added or +/// removed, that is a behavioural change to every config edit report and +/// should be an explicit decision rather than a drive-by edit. +#[test] +fn the_restart_required_list_is_the_reviewed_set() { + assert_eq!( + RESTART_REQUIRED_SECTIONS, + &["gateway", "acp", "launch_hotkeys"], + "changing which sections need a restart changes what users are told; \ + confirm the consuming code really snapshots the value at startup" + ); +} + +/// Sections that are read through `config()` on every use are live by +/// definition. Spot-check the ones users change most often, so a careless +/// addition to the restart list cannot silently start telling people to +/// restart for a setting that already applies. +#[test] +fn commonly_edited_sections_are_live() { + for key in [ + "keybindings.scroll_up", + "display.centered", + "features.thinking", + "provider.openai_reasoning_effort", + "agents.swarm_spawn_mode", + "tools.profile", + "websearch.engine", + "notifications.enabled", + ] { + assert_eq!( + liveness_for_key(key), + Liveness::Live, + "{key} is re-read through the config cache and should report as live" + ); + } +} From 35e692bc8758d5c21292b097b27a1f456728d2cc Mon Sep 17 00:00:00 2001 From: jeremy <94247773+1jehuang@users.noreply.github.com> Date: Sun, 2 Aug 2026 18:19:00 -0700 Subject: [PATCH 37/37] fix(provider): strip own routing prefix in direct runtime set_model Turn 1 of an antigravity session worked and turn 2 always failed with HTTP 404 "Requested entity was not found.", for every model. Session restore rebuilds the model as the routing spec "antigravity:". MultiProvider peels that prefix off before delegating, but --provider antigravity hands the concrete runtime to the agent directly, so the prefixed spec reached Provider::set_model untouched, was stored as the model id, and went on the wire as a model the backend has never heard of. Single-shot smokes never caught it because nothing resumes. Add jcode_provider_core::strip_own_model_prefix and call it from the four runtimes reachable via --provider (antigravity, gemini, cursor, copilot), which all had the same latent defect. Only the runtime's own prefix is stripped, so a foreign prefix stays a visible routing error. Add scripts/antigravity_multiturn_coverage.sh: per model, one resumed 4-turn session covering context replay, a tool call layered on history, and full history replay after a tool turn (the thought_signature site). All 15 chat-capable antigravity models now pass all 4 turns; before this fix every one of them failed on turn 2. --- .../src/antigravity_tests.rs | 29 +++ .../src/lib.rs | 6 +- .../jcode-provider-copilot-runtime/src/lib.rs | 4 +- crates/jcode-provider-core/src/lib.rs | 2 +- crates/jcode-provider-core/src/selection.rs | 105 +++++++++++ .../jcode-provider-cursor-runtime/src/lib.rs | 4 +- .../jcode-provider-gemini-runtime/src/lib.rs | 4 +- scripts/antigravity_multiturn_coverage.sh | 174 ++++++++++++++++++ 8 files changed, 323 insertions(+), 5 deletions(-) create mode 100755 scripts/antigravity_multiturn_coverage.sh diff --git a/crates/jcode-provider-antigravity-runtime/src/antigravity_tests.rs b/crates/jcode-provider-antigravity-runtime/src/antigravity_tests.rs index bb596c9f64..6f51817197 100644 --- a/crates/jcode-provider-antigravity-runtime/src/antigravity_tests.rs +++ b/crates/jcode-provider-antigravity-runtime/src/antigravity_tests.rs @@ -613,3 +613,32 @@ fn is_retryable_empty_turn_ignores_normal_and_productive_turns() { .expect("decode empty stop response"); assert!(!is_retryable_empty_turn(&empty_stop)); } + +/// End-to-end guard for the turn-2 HTTP 404: `--provider antigravity` gives the +/// agent this runtime directly, and session restore calls `set_model` with the +/// routing spec `antigravity:`. The id we store is the id we put on the +/// wire, so it must be the bare model name. +#[test] +fn set_model_stores_bare_id_for_prefixed_session_restore_spec() { + let provider = AntigravityProvider::new(); + + provider + .set_model("antigravity:gemini-3-flash") + .expect("prefixed session-restore spec must be accepted"); + assert_eq!( + provider.model(), + "gemini-3-flash", + "the wire model id must never carry the antigravity: routing prefix" + ); + + provider + .set_model("gemini-3-flash") + .expect("bare id must still be accepted"); + assert_eq!(provider.model(), "gemini-3-flash"); + + assert!( + provider.set_model("antigravity:").is_err(), + "a prefix with no model must be rejected, not stored as an empty model" + ); + assert!(provider.set_model(" ").is_err()); +} diff --git a/crates/jcode-provider-antigravity-runtime/src/lib.rs b/crates/jcode-provider-antigravity-runtime/src/lib.rs index e7a28877f3..ed737dd377 100644 --- a/crates/jcode-provider-antigravity-runtime/src/lib.rs +++ b/crates/jcode-provider-antigravity-runtime/src/lib.rs @@ -664,7 +664,11 @@ impl Provider for AntigravityProvider { } fn set_model(&self, model: &str) -> Result<()> { - let trimmed = model.trim(); + // `--provider antigravity` uses this runtime directly, so session + // restore hands it the routing spec `antigravity:` rather than a + // bare id. See `strip_own_model_prefix`: keeping the prefix made every + // resumed turn 404. + let trimmed = jcode_provider_core::strip_own_model_prefix(model, "antigravity:"); if trimmed.is_empty() { anyhow::bail!("Antigravity model cannot be empty"); } diff --git a/crates/jcode-provider-copilot-runtime/src/lib.rs b/crates/jcode-provider-copilot-runtime/src/lib.rs index 04e25a0dd9..29705e5413 100644 --- a/crates/jcode-provider-copilot-runtime/src/lib.rs +++ b/crates/jcode-provider-copilot-runtime/src/lib.rs @@ -976,7 +976,9 @@ impl Provider for CopilotApiProvider { } fn set_model(&self, model: &str) -> Result<()> { - let trimmed = model.trim(); + // See `strip_own_model_prefix`: `--provider copilot` routes through this + // runtime directly, so session restore hands it `copilot:`. + let trimmed = jcode_provider_core::strip_own_model_prefix(model, "copilot:"); if trimmed.is_empty() { anyhow::bail!("Copilot model cannot be empty"); } diff --git a/crates/jcode-provider-core/src/lib.rs b/crates/jcode-provider-core/src/lib.rs index 4bda8ed614..054024402c 100644 --- a/crates/jcode-provider-core/src/lib.rs +++ b/crates/jcode-provider-core/src/lib.rs @@ -54,7 +54,7 @@ pub use selection::{ ActiveProvider, ProviderAvailability, auto_default_provider, cli_provider_arg_for_session_key, dedupe_model_routes, explicit_model_provider_prefix, fallback_sequence, model_name_for_provider, parse_provider_hint, provider_from_model_key, provider_key, - provider_label, + provider_label, strip_own_model_prefix, }; use anyhow::Result; diff --git a/crates/jcode-provider-core/src/selection.rs b/crates/jcode-provider-core/src/selection.rs index 68bd459d83..6a7431a69e 100644 --- a/crates/jcode-provider-core/src/selection.rs +++ b/crates/jcode-provider-core/src/selection.rs @@ -202,6 +202,29 @@ pub fn model_name_for_provider(provider: ActiveProvider, model: &str) -> Cow<'_, Cow::Borrowed(model) } +/// Strip a provider runtime's own routing prefix from a model id. +/// +/// Session restore and the model picker speak in routing specs such as +/// `antigravity:gemini-3-flash`. `MultiProvider` peels the prefix off before +/// delegating, but `--provider ` hands the concrete runtime back to the +/// agent directly, so the prefixed spec reaches `Provider::set_model` +/// untouched. Storing it verbatim makes every later turn ask the backend for a +/// model whose id contains our prefix, which the backend has never heard of +/// (Antigravity answers HTTP 404 "Requested entity was not found." on turn 2 +/// of any resumed session). +/// +/// A runtime only ever puts a bare model id on the wire, so each runtime calls +/// this with its own prefix when accepting a model id. Only the runtime's own +/// prefix is stripped: a foreign prefix is a real routing error and must stay +/// visible rather than being silently reinterpreted as a local model. +pub fn strip_own_model_prefix<'a>(model: &'a str, own_prefix: &str) -> &'a str { + let model = model.trim(); + match model.strip_prefix(own_prefix) { + Some(rest) if !own_prefix.is_empty() => rest.trim(), + _ => model, + } +} + pub fn dedupe_model_routes(routes: Vec) -> Vec { use std::collections::HashMap; @@ -676,4 +699,86 @@ mod tests { assert!(sequence.contains(&ActiveProvider::Claude)); assert!(sequence.contains(&ActiveProvider::Cursor)); } + + /// Regression: `--provider antigravity` (and the other direct runtimes) + /// hand `Provider::set_model` a routing spec like + /// `antigravity:gemini-3-flash` on session restore. Storing that verbatim + /// made every resumed turn request a model whose id carried our prefix, and + /// the backend answered HTTP 404 "Requested entity was not found." So turn + /// 1 of a session worked and turn 2 always failed, for every model. + #[test] + fn strip_own_model_prefix_covers_the_routing_spec_state_space() { + // (case, input, own prefix) -> stored model id + let cases: [(&str, &str, &str, &str); 9] = [ + ( + "own prefix is stripped", + "antigravity:gemini-3-flash", + "antigravity:", + "gemini-3-flash", + ), + ( + "bare id is unchanged", + "gemini-3-flash", + "antigravity:", + "gemini-3-flash", + ), + ( + "surrounding whitespace is trimmed", + " antigravity:gemini-3-flash ", + "antigravity:", + "gemini-3-flash", + ), + ( + "whitespace after the prefix is trimmed", + "antigravity: gemini-3-flash", + "antigravity:", + "gemini-3-flash", + ), + ( + "only one level of our own prefix is peeled", + "antigravity:antigravity:gemini-3-flash", + "antigravity:", + "antigravity:gemini-3-flash", + ), + ( + "a foreign prefix is a routing error and stays visible", + "openrouter:gemini-3-flash", + "antigravity:", + "openrouter:gemini-3-flash", + ), + ( + "model ids containing a colon are otherwise untouched", + "some:model", + "antigravity:", + "some:model", + ), + ( + "gemini runtime", + "gemini:gemini-2.5-pro", + "gemini:", + "gemini-2.5-pro", + ), + ("copilot runtime", "copilot:gpt-5", "copilot:", "gpt-5"), + ]; + + for (case, input, own_prefix, expected) in cases { + assert_eq!( + strip_own_model_prefix(input, own_prefix), + expected, + "{case}" + ); + } + } + + /// A prefix-only spec must not silently become a valid empty model id; the + /// runtimes rely on the emptiness check to reject it. + #[test] + fn strip_own_model_prefix_leaves_prefix_only_input_empty() { + assert_eq!(strip_own_model_prefix("antigravity:", "antigravity:"), ""); + assert_eq!( + strip_own_model_prefix("antigravity: ", "antigravity:"), + "" + ); + assert_eq!(strip_own_model_prefix(" ", "antigravity:"), ""); + } } diff --git a/crates/jcode-provider-cursor-runtime/src/lib.rs b/crates/jcode-provider-cursor-runtime/src/lib.rs index 8ee3ca2a79..23b5e18974 100644 --- a/crates/jcode-provider-cursor-runtime/src/lib.rs +++ b/crates/jcode-provider-cursor-runtime/src/lib.rs @@ -303,7 +303,9 @@ impl Provider for CursorCliProvider { } fn set_model(&self, model: &str) -> Result<()> { - let trimmed = model.trim(); + // See `strip_own_model_prefix`: `--provider cursor` routes through this + // runtime directly, so session restore hands it `cursor:`. + let trimmed = jcode_provider_core::strip_own_model_prefix(model, "cursor:"); if trimmed.is_empty() { anyhow::bail!("Cursor model cannot be empty"); } diff --git a/crates/jcode-provider-gemini-runtime/src/lib.rs b/crates/jcode-provider-gemini-runtime/src/lib.rs index 8ba32c4203..30cff28c4e 100644 --- a/crates/jcode-provider-gemini-runtime/src/lib.rs +++ b/crates/jcode-provider-gemini-runtime/src/lib.rs @@ -943,7 +943,9 @@ impl Provider for GeminiProvider { } fn set_model(&self, model: &str) -> Result<()> { - let trimmed = model.trim(); + // See `strip_own_model_prefix`: `--provider gemini` routes through this + // runtime directly, so session restore hands it `gemini:`. + let trimmed = jcode_provider_core::strip_own_model_prefix(model, "gemini:"); if trimmed.is_empty() { anyhow::bail!("Gemini model cannot be empty"); } diff --git a/scripts/antigravity_multiturn_coverage.sh b/scripts/antigravity_multiturn_coverage.sh new file mode 100755 index 0000000000..0aaf97c74f --- /dev/null +++ b/scripts/antigravity_multiturn_coverage.sh @@ -0,0 +1,174 @@ +#!/usr/bin/env bash +# Live multi-turn coverage for Antigravity models. +# +# The single-shot smokes in antigravity_multiturn's sibling script +# (antigravity_live_coverage.sh) only prove one request works. The failure mode +# users actually hit is the *second and later* turn of a real session, where the +# provider must replay prior context, prior tool calls, and (for Gemini/Claude +# models behind Antigravity) opaque `thought_signature` blobs attached to +# earlier assistant turns. A model can pass a one-shot smoke and still 400 on +# turn 2. +# +# So each model here runs one resumed session of 4 turns that exercises, +# in order: +# turn 1 plain chat, establishes a fact the model must carry forward +# turn 2 recall of turn 1 (context replay across resume) +# turn 3 tool call, then recall of the turn-1 fact in the same turn +# (tool-result + thought_signature replay on top of history) +# turn 4 recall of both the turn-1 fact and the turn-3 tool output +# (full history replay after a tool turn: the usual 400 site) +# +# Exit status is non-zero when any model fails, so this is usable as a gate. +set -uo pipefail + +JC=${JC:-./target/selfdev/jcode} +TURN_TIMEOUT=${TURN_TIMEOUT:-120} +MODELS_OVERRIDE=${MODELS:-} + +# The magic word is checked verbatim, so a model that drops history fails loudly +# instead of producing a plausible-looking answer. +MAGIC="ZEBRA42" +TOOL_TOKEN="TOOLOUT77" + +MODELS=( + claude-opus-4-6-thinking + claude-sonnet-4-6 + gemini-3.1-pro-high + gemini-3.1-pro-low + gemini-3-flash + gemini-3-flash-agent + gemini-3.5-flash-low + gpt-oss-120b-medium + gemini-2.5-flash + gemini-2.5-flash-lite + gemini-2.5-flash-thinking + gemini-3.1-flash-lite + gemini-3.5-flash-extra-low + gemini-pro-agent + default +) +if [[ -n "$MODELS_OVERRIDE" ]]; then + read -r -a MODELS <<<"$MODELS_OVERRIDE" +fi + +# Run one turn. Echoes the assistant text; sets TURN_SESSION on turn 1. +# usage: run_turn +run_turn() { + local model=$1 session=$2 prompt=$3 out + if [[ -z "$session" ]]; then + out=$(timeout "$TURN_TIMEOUT" "$JC" run --provider antigravity -m "$model" \ + --no-update --no-selfdev --json "$prompt" 2>&1) + else + out=$(timeout "$TURN_TIMEOUT" "$JC" run --provider antigravity -m "$model" \ + --no-update --no-selfdev --json --resume "$session" "$prompt" 2>&1) + fi + local rc=$? + TURN_RAW=$out + if [[ $rc -ne 0 ]]; then + TURN_RC=$rc + TURN_TEXT="" + return 1 + fi + TURN_RC=0 + TURN_SESSION=$(python3 -c ' +import json,sys +try: + print(json.loads(sys.stdin.read()).get("session_id","")) +except Exception: + print("")' <<<"$out") + TURN_TEXT=$(python3 -c ' +import json,sys +try: + print(json.loads(sys.stdin.read()).get("text","")) +except Exception: + print("")' <<<"$out") + [[ -n "$TURN_TEXT" ]] +} + +# Short, greppable reason for a failed turn. +failure_note() { + grep -oiE "thought_signature[^\"]{0,40}|HTTP [0-9]{3}[^\"]{0,40}|400[^\"]{0,30}|schema[^\"]{0,30}|error[^\"]{0,40}" \ + <<<"$TURN_RAW" | head -1 | tr -d '\n' | cut -c1-70 +} + +printf "%-30s | %-6s | %-6s | %-6s | %-6s | %s\n" \ + "MODEL" "T1" "T2" "T3" "T4" "NOTES" +printf -- "--------------------------------------------------------------------------------------------\n" + +failures=0 +total=${#MODELS[@]} +i=0 +for m in "${MODELS[@]}"; do + i=$((i + 1)) + echo "JCODE_PROGRESS {\"current\":$i,\"total\":$total,\"unit\":\"models\",\"message\":\"$m\"}" >&2 + + t1=- t2=- t3=- t4=- note="" session="" + + # Turn 1: establish a fact the later turns must recall. + if run_turn "$m" "" "Remember this magic word for later: $MAGIC. Reply with exactly: READY"; then + if grep -q "READY" <<<"$TURN_TEXT"; then t1=PASS; else t1=NOTOK; note="turn1 no READY"; fi + session=$TURN_SESSION + else + t1=FAIL + note=$(failure_note) + [[ $TURN_RC -eq 124 ]] && note="turn1 timeout" + fi + + # Turn 2: plain context replay across a resume. + if [[ $t1 == PASS && -n $session ]]; then + if run_turn "$m" "$session" "What was the magic word? Reply with only the word."; then + if grep -q "$MAGIC" <<<"$TURN_TEXT"; then t2=PASS; else t2=NOTOK; note="turn2 lost context"; fi + else + t2=FAIL + note=$(failure_note) + [[ $TURN_RC -eq 124 ]] && note="turn2 timeout" + fi + fi + + # Turn 3: tool call layered on top of existing history, plus recall. This is + # where thought_signature replay first has to survive a tool round-trip. + if [[ $t2 == PASS ]]; then + if run_turn "$m" "$session" \ + "Use the bash tool to run 'echo $TOOL_TOKEN', then reply with the command output followed by the magic word."; then + if grep -q "$TOOL_TOKEN" <<<"$TURN_TEXT" && grep -q "$MAGIC" <<<"$TURN_TEXT"; then + t3=PASS + else + t3=NOTOK + note="turn3 missing tool output or magic word" + fi + else + t3=FAIL + note=$(failure_note) + [[ $TURN_RC -eq 124 ]] && note="turn3 timeout" + fi + fi + + # Turn 4: replay the whole history *after* a tool turn. This is the turn that + # 400s when signed assistant parts are dropped or reordered. + if [[ $t3 == PASS ]]; then + if run_turn "$m" "$session" \ + "Without using any tools, reply with the magic word and the command output from earlier, separated by a space."; then + if grep -q "$TOOL_TOKEN" <<<"$TURN_TEXT" && grep -q "$MAGIC" <<<"$TURN_TEXT"; then + t4=PASS + else + t4=NOTOK + note="turn4 lost history after tool turn" + fi + else + t4=FAIL + note=$(failure_note) + [[ $TURN_RC -eq 124 ]] && note="turn4 timeout" + fi + fi + + [[ $t1 == PASS && $t2 == PASS && $t3 == PASS && $t4 == PASS ]] || failures=$((failures + 1)) + printf "%-30s | %-6s | %-6s | %-6s | %-6s | %s\n" "$m" "$t1" "$t2" "$t3" "$t4" "$note" +done + +printf -- "--------------------------------------------------------------------------------------------\n" +if [[ $failures -eq 0 ]]; then + echo "All ${#MODELS[@]} model(s) passed 4-turn multi-turn coverage." +else + echo "$failures of ${#MODELS[@]} model(s) failed multi-turn coverage." +fi +exit $((failures > 0))