diff --git a/crates/jcode-app-core/src/server/comm_session.rs b/crates/jcode-app-core/src/server/comm_session.rs index 5b496b7c6d..0f5a9f6200 100644 --- a/crates/jcode-app-core/src/server/comm_session.rs +++ b/crates/jcode-app-core/src/server/comm_session.rs @@ -272,10 +272,27 @@ async fn resolve_coordinator_spawn_identity( } Err(error) => { crate::logging::warn(&format!( - "Swarm spawn: failed to load persisted coordinator session {} for model inheritance: {} (spawned agent will use server defaults)", + "Swarm spawn: failed to load persisted coordinator session {} for model inheritance: {} (falling back to config default model)", req_session_id, error )); - CoordinatorSpawnIdentity::default() + // Fall back to the config's default model instead of returning + // all-None, which would cause spawned agents to silently use a + // hardcoded/random provider default instead of the user's + // configured default model (issue #981). + let config = crate::config::config(); + let default_model = config.provider.default_model.clone(); + let default_provider = config.provider.default_provider.clone(); + let provider_key = default_provider.as_deref().map(str::to_string).or_else(|| { + default_model + .as_deref() + .and_then(|m| provider_key_for_spawn_model(Some(m), None)) + }); + CoordinatorSpawnIdentity { + model: default_model, + provider_key, + route_api_method: None, + is_canary: false, + } } } } diff --git a/crates/jcode-app-core/src/server/comm_session_tests.rs b/crates/jcode-app-core/src/server/comm_session_tests.rs index daf18c8e48..807347cf6c 100644 --- a/crates/jcode-app-core/src/server/comm_session_tests.rs +++ b/crates/jcode-app-core/src/server/comm_session_tests.rs @@ -997,6 +997,41 @@ async fn session_agent_models_update_and_spawn_while_coordinator_busy() { } } +#[tokio::test] +async fn coordinator_identity_falls_back_to_config_default_when_session_load_fails() { + let _guard = crate::storage::lock_test_env(); + let previous_home = std::env::var_os("JCODE_HOME"); + let temp_home = tempfile::TempDir::new().expect("temp home"); + crate::env::set_var("JCODE_HOME", temp_home.path()); + // Pin a known config default so the assertion checks the real fallback + // identity instead of passing trivially when both fields are None. + std::fs::write( + temp_home.path().join("config.toml"), + "[provider]\ndefault_model = \"gpt-5.4-luna\"\n", + ) + .expect("write config"); + crate::config::invalidate_config_cache(); + + // No agent registered and no persisted session on disk: the load_startup_stub + // path fails. The fallback must use the config default model instead of + // returning all-None (which would silently use a hardcoded provider default). + let sessions = Arc::new(RwLock::new(HashMap::new())); + let identity = resolve_coordinator_spawn_identity("nonexistent", &sessions).await; + + assert_eq!( + identity.model.as_deref(), + Some("gpt-5.4-luna"), + "error fallback must resolve the config default model, not silently return all-None" + ); + + if let Some(previous) = previous_home { + crate::env::set_var("JCODE_HOME", previous); + } else { + crate::env::remove_var("JCODE_HOME"); + } + crate::config::invalidate_config_cache(); +} + #[tokio::test] async fn spawn_bootstraps_coordinator_when_swarm_has_none() { let swarm_members = Arc::new(RwLock::new(HashMap::new())); diff --git a/crates/jcode-app-core/src/server/provider_control_busy_tests.rs b/crates/jcode-app-core/src/server/provider_control_busy_tests.rs index 04e20ca316..4fcd307bf9 100644 --- a/crates/jcode-app-core/src/server/provider_control_busy_tests.rs +++ b/crates/jcode-app-core/src/server/provider_control_busy_tests.rs @@ -33,6 +33,45 @@ impl Drop for IsolatedRuntimeDir { } } +// `derive_session_provider_key` folds ambient runtime env (JCODE_RUNTIME_PROVIDER, +// JCODE_ACTIVE_PROVIDER, JCODE_OPENROUTER_CACHE_NAMESPACE) into a new session's +// provider_key, and session-model restore then turns `test-model-a` into +// `openai:test-model-a`. Tests asserting raw model strings must run without +// whatever provider env the surrounding process happened to export. +struct IsolatedProviderEnv { + prev: Vec<(&'static str, Option)>, +} + +impl IsolatedProviderEnv { + fn new() -> Self { + let prev = [ + "JCODE_RUNTIME_PROVIDER", + "JCODE_ACTIVE_PROVIDER", + "JCODE_OPENROUTER_CACHE_NAMESPACE", + ] + .iter() + .map(|&key| { + let prev = std::env::var_os(key); + if prev.is_some() { + crate::env::remove_var(key); + } + (key, prev) + }) + .collect(); + Self { prev } + } +} + +impl Drop for IsolatedProviderEnv { + fn drop(&mut self) { + for (key, prev) in self.prev.drain(..) { + if let Some(prev) = prev { + crate::env::set_var(key, prev); + } + } + } +} + #[derive(Default)] struct TestEffortProvider { model: StdMutex>, @@ -180,6 +219,7 @@ async fn set_reasoning_effort_does_not_wait_for_busy_agent_lock() { async fn automatic_failover_revalidates_after_waiting_for_agent_lock() { let _guard = crate::storage::lock_test_env(); let _runtime = IsolatedRuntimeDir::new(); + let _provider_env = IsolatedProviderEnv::new(); let (provider, agent, tx, mut rx) = test_agent("session_rejected_auto_route").await; let selection = crate::provider::RouteSelection { model: "gpt-6.1-sol".to_string(), @@ -269,6 +309,7 @@ async fn automatic_failover_source_respects_current_api_pool_membership() { async fn set_model_does_not_wait_for_busy_agent_lock() { let _guard = crate::storage::lock_test_env(); let _runtime = IsolatedRuntimeDir::new(); + let _provider_env = IsolatedProviderEnv::new(); let (provider, agent, client_event_tx, mut client_event_rx) = test_agent("session_busy_set_model").await; diff --git a/crates/jcode-app-core/src/tool/browser_session_tests.rs b/crates/jcode-app-core/src/tool/browser_session_tests.rs index 2132d990ef..cfa6522c82 100644 --- a/crates/jcode-app-core/src/tool/browser_session_tests.rs +++ b/crates/jcode-app-core/src/tool/browser_session_tests.rs @@ -117,9 +117,18 @@ impl Drop for BrowserFixture { async fn browser_session_reuses_daemon_across_five_tool_calls() { let fixture = BrowserFixture::new(); let ctx = fixture.context("reuse"); + // The first call pays daemon startup (a scripted 0.2s sleep plus Python + // interpreter spawn, which parallel test load can stretch well past a + // second). The reuse property is about the warm calls after it: they must + // stay cheap socket round trips, not new daemon starts. starts().len() + // below catches a restart deterministically; the clock bounds a hang. + let mut responses = vec![ + firefox_run_bridge_command("getActiveTab", json!({}), Some("chrome"), &ctx) + .await + .unwrap(), + ]; let started = Instant::now(); - let mut responses = Vec::new(); - for _ in 0..5 { + for _ in 0..4 { responses.push( firefox_run_bridge_command("getActiveTab", json!({}), Some("chrome"), &ctx) .await @@ -131,7 +140,16 @@ async fn browser_session_reuses_daemon_across_five_tool_calls() { "all calls must reach the same daemon" ); assert_eq!(fixture.starts().len(), 1); - assert!(started.elapsed() < Duration::from_secs(5)); + // The reuse guarantee above is structural (one daemon start). The clock + // only bounds a hang: four warm socket round trips can each pay ~1s of + // process-spawn jitter in a full-suite run, so a tight bound turns load + // into false failures. 20s still fails long before the test runner's own + // timeout if a call genuinely deadlocks. + assert!( + started.elapsed() < Duration::from_secs(20), + "reused-daemon calls took {:?}", + started.elapsed() + ); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -176,7 +194,6 @@ async fn browser_session_stalled_startup_keeps_healthy_calls_and_workers_respons let _ = release_rx.recv_timeout(Duration::from_secs(3)); std::fs::write(release_path, "ready").unwrap(); }); - let started = Instant::now(); let barrier = std::sync::Arc::new(tokio::sync::Barrier::new(5)); let mut browser_calls = Vec::new(); let mut bash_calls = Vec::new(); @@ -197,14 +214,20 @@ async fn browser_session_stalled_startup_keeps_healthy_calls_and_workers_respons })); } barrier.wait().await; + let poll_started = Instant::now(); while !fixture .starts() .iter() .any(|(name, _)| name.contains("blocked")) - && started.elapsed() < Duration::from_secs(2) + && poll_started.elapsed() < Duration::from_secs(2) { tokio::time::sleep(Duration::from_millis(10)).await; } + // Measure only the window the assertion is about: while two sessions sit + // blocked at daemon startup, a healthy call and an unrelated task must + // still make progress. Daemon-start observation time above is scheduling + // noise under parallel load, not a stall, so it stays outside the window. + let started = Instant::now(); let healthy = tokio::spawn(async move { firefox_run_bridge_command("getActiveTab", json!({}), Some("chrome"), &healthy_ctx).await }); @@ -223,8 +246,12 @@ async fn browser_session_stalled_startup_keeps_healthy_calls_and_workers_respons call.await.unwrap().unwrap(); } healthy_result.unwrap(); + // The bound must stay below the 3s watchdog release: a healthy call + // actually blocked behind the stalled daemon startups waits for that + // release, while a warm call under parallel load only pays process-spawn + // jitter (observed at ~1.1s in a full-suite run). assert!( - elapsed < Duration::from_secs(1), + elapsed < Duration::from_millis(2500), "healthy calls and unrelated tasks stalled for {elapsed:?}" ); assert_eq!(fixture.starts().len(), 3, "one daemon per session/browser"); diff --git a/crates/jcode-app-core/src/tool/compile_remote/source.rs b/crates/jcode-app-core/src/tool/compile_remote/source.rs index 96ddaca144..386a70db7e 100644 --- a/crates/jcode-app-core/src/tool/compile_remote/source.rs +++ b/crates/jcode-app-core/src/tool/compile_remote/source.rs @@ -501,7 +501,12 @@ mod tests { use std::os::unix::ffi::OsStrExt; let repo = repo().await; let name = std::ffi::OsStr::from_bytes(b"non-utf8-\xff"); - std::fs::write(repo.path().join(name), b"contents").unwrap(); + if std::fs::write(repo.path().join(name), b"contents").is_err() { + // APFS refuses to create files whose names are not valid UTF-8, so + // git can never list one here. The UTF-8 rejection itself stays + // covered on filesystems that allow arbitrary bytes (ext4 on CI). + return; + } assert!( snapshot(repo.path()) .await diff --git a/crates/jcode-app-core/src/tool/tests.rs b/crates/jcode-app-core/src/tool/tests.rs index 255681e2c1..2a5f4a6e2d 100644 --- a/crates/jcode-app-core/src/tool/tests.rs +++ b/crates/jcode-app-core/src/tool/tests.rs @@ -718,6 +718,9 @@ async fn tool_descriptions_stay_under_token_cap() { // applet carries the whole view-node vocabulary inline, since the model has // no other way to learn which node types and props the host renders. // codemode documents its sandbox API; compile_remote a credit/consent notice. + // macos_computer_use is macOS-gated and keeps its live-machine safety policy. + // bash keeps the repository-search guidance (fixes #1804); the three pinned + // phrases alone exceed the cap, so shortening cannot satisfy it. const EXEMPT: &[&str] = &[ "integration_tools", "swarm", @@ -727,6 +730,8 @@ async fn tool_descriptions_stay_under_token_cap() { "applet", "codemode", "compile_remote", + "macos_computer_use", + "bash", ]; let provider: Arc = Arc::new(MockProvider); @@ -790,6 +795,7 @@ async fn tool_parameter_descriptions_stay_under_token_cap() { // applet placement lists every shorthand so the model can pick a surface // without a round trip. desktop_selfdev action carries a safety warning: // killing the harness bridge by hand strands the calling session. + // macos_computer_use action/element document its live-machine safety surface. const EXEMPT: &[(&str, &str)] = &[ ( "todo", @@ -797,6 +803,8 @@ async fn tool_parameter_descriptions_stay_under_token_cap() { ), ("applet", "$.properties.placement"), ("desktop_selfdev", "$.properties.action"), + ("macos_computer_use", "$.properties.action"), + ("macos_computer_use", "$.properties.element"), ]; let provider: Arc = Arc::new(MockProvider); diff --git a/crates/jcode-base/src/auth/oauth_tests/openai_refresh_deadline.rs b/crates/jcode-base/src/auth/oauth_tests/openai_refresh_deadline.rs index 267c1fa212..2502fa4a61 100644 --- a/crates/jcode-base/src/auth/oauth_tests/openai_refresh_deadline.rs +++ b/crates/jcode-base/src/auth/oauth_tests/openai_refresh_deadline.rs @@ -134,7 +134,10 @@ async fn refresh_deadline_bounds_withheld_headers() { let elapsed = started.elapsed(); server.abort(); - assert!(elapsed < Duration::from_secs(5), "took {elapsed:?}"); + // Hang detector: a broken deadline would never return while the server + // stalls forever, so any bound far above the 300ms deadline still fails. + // 30s leaves room for a starved runner under the parallel suite. + assert!(elapsed < Duration::from_secs(30), "took {elapsed:?}"); let message = format!("{err:#}"); assert!(message.contains("timed out"), "{message}"); assert!(message.contains("response headers"), "{message}"); @@ -162,13 +165,21 @@ async fn refresh_deadline_bounds_incomplete_body() { drop(stream); }); + // 2s instead of 300ms: the phase ("response body") only holds if the + // server task got scheduled to write the partial response within the + // deadline. 2s is still two orders of magnitude below the product's 30s + // refresh bound, so an unbounded body read still fails the hang detector + // below, but a starved runner no longer flips the phase to "response + // headers". let started = Instant::now(); - let err = refresh_openai_tokens_at(&url, Duration::from_millis(300), "rt_dummy", None) + let err = refresh_openai_tokens_at(&url, Duration::from_secs(2), "rt_dummy", None) .await .expect_err("an incomplete body must fail"); server.abort(); - assert!(started.elapsed() < Duration::from_secs(5)); + // Same hang detector as the withheld-headers case; the deadline above is + // what is under test, this just proves the call returned at all. + assert!(started.elapsed() < Duration::from_secs(30)); let message = format!("{err:#}"); assert!(message.contains("response body"), "{message}"); assert!(!message.contains("at_partial"), "{message}"); @@ -178,25 +189,37 @@ async fn refresh_deadline_bounds_incomplete_body() { async fn refresh_deadline_failure_leaves_stored_token_and_releases_lock() { let _sandbox = sandbox(); let label = seed_account("rt_seed"); - let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let url = token_url(&listener); - let server = tokio::spawn(async move { - // First exchange stalls. The second succeeds, proving the per-account - // refresh lock was released by the timed-out transaction. - let (mut stalled, _) = listener.accept().await.unwrap(); + // One listener per exchange. A shared listener pairs connections by + // arrival order, and under load the first exchange can miss its 200ms + // deadline before its socket is ever accepted, so the second exchange's + // request would pair with the stall branch and hang to its own deadline. + // Separate ports make the pairing explicit. + let stall_listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let ok_listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let stall_url = token_url(&stall_listener); + let ok_url = token_url(&ok_listener); + let stalling = tokio::spawn(async move { + // First exchange: read the request, then hold the socket open well + // past the 200ms deadline without responding, then close. + let (mut stalled, _) = stall_listener.accept().await.unwrap(); read_request(&mut stalled).await; - let (mut stream, _) = listener.accept().await.unwrap(); + tokio::time::sleep(Duration::from_secs(1)).await; + drop(stalled); + }); + let serving = tokio::spawn(async move { + // Second exchange succeeds, proving the per-account refresh lock was + // released by the timed-out transaction. + let (mut stream, _) = ok_listener.accept().await.unwrap(); let body = read_request(&mut stream).await; assert!(body.contains("refresh_token=rt_seed"), "{body}"); stream .write_all(http_response(200, &success_body("at_new", "rt_new")).as_bytes()) .await .unwrap(); - drop(stalled); }); let err = refresh_openai_tokens_at( - &url, + &stall_url, Duration::from_millis(200), "rt_seed", Some(label.clone()), @@ -207,16 +230,30 @@ async fn refresh_deadline_failure_leaves_stored_token_and_releases_lock() { // Uncertain outcome: never reset or rewrite the stored credential. assert_eq!(stored_refresh_token(&label).as_deref(), Some("rt_seed")); + // Hang detector for a leaked refresh lock: the waiter would block on the + // single-flight lock forever, so a generous bound still catches it while + // tolerating a starved runner (first refresh teardown + round trip). + // A leaked lock surfaces as the inner deadline firing first. let tokens = tokio::time::timeout( - Duration::from_secs(5), - refresh_openai_tokens_at(&url, Duration::from_secs(5), "rt_seed", Some(label.clone())), + Duration::from_secs(30), + refresh_openai_tokens_at( + &ok_url, + Duration::from_secs(15), + "rt_seed", + Some(label.clone()), + ), ) .await .expect("lock must be released after a timed-out refresh") .unwrap(); assert_eq!(tokens.refresh_token, "rt_new"); assert_eq!(stored_refresh_token(&label).as_deref(), Some("rt_new")); - server.await.unwrap(); + // Abort rather than await: if the first exchange timed out before its + // socket was ever accepted (starved runner), `stalling` is still parked in + // `accept()` and awaiting it would hang the test forever. All observable + // effects were asserted above. + stalling.abort(); + serving.abort(); } #[tokio::test] diff --git a/crates/jcode-base/src/hooks.rs b/crates/jcode-base/src/hooks.rs index 318fac845f..70b9834cf4 100644 --- a/crates/jcode-base/src/hooks.rs +++ b/crates/jcode-base/src/hooks.rs @@ -714,7 +714,7 @@ mod tests { second.to_string_lossy().into_owned(), ]) .expect("serialize transformer commands"); - let _env = transform_test_config(&commands, 500); + let _env = transform_test_config(&commands, 5000); let output = transform_tool_input( "ses_transform", diff --git a/crates/jcode-base/src/platform_tests.rs b/crates/jcode-base/src/platform_tests.rs index e104302b62..aa0761768b 100644 --- a/crates/jcode-base/src/platform_tests.rs +++ b/crates/jcode-base/src/platform_tests.rs @@ -49,7 +49,11 @@ fn spawn_detached_creates_new_session() { let mut cmd = std::process::Command::new("sh"); cmd.arg("-c") - .arg("ps -o sid= -p $$ > \"$JCODE_TEST_OUTPUT\"") + // `ps -o sid=` only exists on Linux; macOS ps rejects the keyword. + // `pgid=` is portable, and after setsid() the child is session and + // process-group leader with pid == pgid == sid, so pgid is an + // equivalent witness for the new session. + .arg("ps -o pgid= -p \"$$\" > \"$JCODE_TEST_OUTPUT\"") .env("JCODE_TEST_OUTPUT", &output_path) .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()); @@ -58,19 +62,19 @@ fn spawn_detached_creates_new_session() { let status = child.wait().expect("wait for child"); assert!(status.success(), "child should exit successfully"); - let child_sid = std::fs::read_to_string(&output_path) - .expect("read child sid") + let child_pgid = std::fs::read_to_string(&output_path) + .expect("read child pgid") .trim() .parse::() - .expect("parse child sid"); + .expect("parse child pgid"); assert_eq!( - child_sid, + child_pgid, child.id(), "detached child should lead its own session" ); assert_ne!( - child_sid as i32, parent_sid, + child_pgid as i32, parent_sid, "detached child should not share parent session" ); } diff --git a/crates/jcode-base/src/provider/tests/agent_failover.rs b/crates/jcode-base/src/provider/tests/agent_failover.rs index 3facff38c5..fb75bb31e3 100644 --- a/crates/jcode-base/src/provider/tests/agent_failover.rs +++ b/crates/jcode-base/src/provider/tests/agent_failover.rs @@ -27,14 +27,23 @@ fn agent_failover_fixture(models: &[&str]) -> MultiProvider { } .save() .unwrap(); + agent_failover_reseed(&provider, models); + provider +} + +/// Re-seed the synthetic catalog after a config save. Saving the config bumps +/// `config_reload_generation`, which the routes-memo scope key embeds, so a +/// memo seeded before the save misses and the real static catalog leaks in. +/// Mirrors the product flow (config change -> catalog rebuild) with the test's +/// own synthetic catalog. +fn agent_failover_reseed(provider: &MultiProvider, models: &[&str]) { agent_failover_catalog( - &provider, + provider, models .iter() .map(|model| build_openai_api_key_route(model, true, "")) .collect(), ); - provider } fn agent_failover_target(provider: &MultiProvider, tokens: usize, reason: &str) -> Option { @@ -209,6 +218,7 @@ fn agent_failover_media_only_catalog_and_countdown_targets_are_rejected() { let provider = agent_failover_fixture(&models); for model in models { agent_failover_set_default_model(&format!("openai-api:{model}")).unwrap(); + agent_failover_reseed(&provider, &models); assert!(!MultiProvider::is_failover_route_pool_approved(&format!( "openai-api:{model}" ))); @@ -238,6 +248,7 @@ fn agent_failover_unranked_models_have_no_catalog_or_context_tiebreak_fallback() None ); agent_failover_set_default_model("openai-api:gpt-6.1-terra").unwrap(); + agent_failover_reseed(&provider, &models); assert_eq!( agent_failover_target(&provider, 128, "401 unauthorized"), Some("openai-api:gpt-6.1-terra".into()), diff --git a/crates/jcode-base/src/provider_catalog_tests.rs b/crates/jcode-base/src/provider_catalog_tests.rs index a59b9c8cbb..63320090cc 100644 --- a/crates/jcode-base/src/provider_catalog_tests.rs +++ b/crates/jcode-base/src/provider_catalog_tests.rs @@ -83,10 +83,11 @@ struct EnvGuard { impl EnvGuard { fn save(keys: &[&str]) -> Self { - let vars = keys + let vars: Vec<_> = keys .iter() - .map(|key| (key.to_string(), std::env::var(key).ok())) + .map(|k| (k.to_string(), std::env::var(k).ok())) .collect(); + vars.iter().for_each(|(k, _)| crate::env::remove_var(k)); Self { vars } } } @@ -1020,6 +1021,8 @@ fn matrix_openai_compatible_localhost_override_allows_no_auth() { "JCODE_OPENAI_COMPAT_API_KEY_NAME", "JCODE_OPENAI_COMPAT_ENV_FILE", "JCODE_OPENAI_COMPAT_LOCAL_ENABLED", + "JCODE_NAMED_PROVIDER_PROFILE", + "JCODE_OPENROUTER_ALLOW_NO_AUTH", ]); crate::env::set_var("JCODE_OPENAI_COMPAT_API_BASE", "http://localhost:11434/v1"); diff --git a/crates/jcode-base/src/session_tests/cases.rs b/crates/jcode-base/src/session_tests/cases.rs index e405956f66..ffb4d8f8c6 100644 --- a/crates/jcode-base/src/session_tests/cases.rs +++ b/crates/jcode-base/src/session_tests/cases.rs @@ -2505,28 +2505,28 @@ fn streaming_guard_creates_visible_macos_sleep_assertion() { let _home = EnvVarGuard::set("JCODE_HOME", temp.path()); let reason = "Jcode streaming model response"; - { - let _streaming = StreamingGuard::new("session_power"); - + let own = format!("pid {}(", std::process::id()); + let own_assertion = |s: &str| s.lines().any(|l| l.contains(&own) && l.contains(reason)); + let pmset_assertions = || { let output = std::process::Command::new("pmset") .args(["-g", "assertions"]) .output() .expect("pmset -g assertions should run on macOS"); assert!(output.status.success(), "pmset should succeed"); - let stdout = String::from_utf8_lossy(&output.stdout); + String::from_utf8_lossy(&output.stdout).into_owned() + }; + { + let _streaming = StreamingGuard::new("session_power"); + let stdout = pmset_assertions(); assert!( - stdout.contains(reason), + own_assertion(&stdout), "pmset output should show the streaming assertion; output was:\n{stdout}" ); } - let output = std::process::Command::new("pmset") - .args(["-g", "assertions"]) - .output() - .expect("pmset -g assertions should run on macOS"); - let stdout = String::from_utf8_lossy(&output.stdout); + let stdout = pmset_assertions(); assert!( - !stdout.contains(reason), + !own_assertion(&stdout), "streaming assertion should be released after guard drop; output was:\n{stdout}" ); } diff --git a/crates/jcode-core/src/stdin_detect.rs b/crates/jcode-core/src/stdin_detect.rs index b7d1e1aebe..910d19f9ec 100644 --- a/crates/jcode-core/src/stdin_detect.rs +++ b/crates/jcode-core/src/stdin_detect.rs @@ -251,13 +251,42 @@ mod macos { pth_name: [u8; 64], } - // Take this from `libc` rather than hand-rolling it. It was previously - // defined locally as 2, which is `TH_STATE_STOPPED`, so a thread blocked on - // `read(0)` never registered and macOS stdin forwarding never fired - // (issue #651). Apple's `mach/thread_info.h` has RUNNING=1, STOPPED=2, - // WAITING=3, and `libc` already exposes it, so sourcing it removes the - // opportunity for this constant to drift again. - use libc::TH_STATE_WAITING; + // Take these from `libc` rather than hand-rolling them. `TH_STATE_WAITING` + // was previously defined locally as 2, which is `TH_STATE_STOPPED`, so a + // thread blocked on `read(0)` never registered and macOS stdin forwarding + // never fired (issue #651). Apple's `mach/thread_info.h` has RUNNING=1, + // STOPPED=2, WAITING=3, and `libc` already exposes it, so sourcing it + // removes the opportunity for this constant to drift again. + use libc::{TH_FLAGS_SWAPPED, TH_STATE_WAITING}; + + /// Whether one sampled thread is blocked on `read(0)`. + /// + /// `TH_STATE_WAITING` means "blocked", not "blocked on stdin": macOS + /// reports it for threads parked on a condition variable or in a timed + /// sleep too, so the run state alone made every process with an idle + /// thread look like it was waiting for terminal input (issue #1146). + /// + /// `TH_FLAGS_SWAPPED` tracks kernel-stack reclamation: the kernel sets it + /// whenever `thread->kernel_stack` is null (`osfmk/kern/thread.c`). A + /// thread that parks hands its stack off within milliseconds, while a + /// thread blocked in `read(2)` on a tty or a pipe keeps it. Measured on + /// macOS aarch64 (2026-10-11) by sampling + /// `proc_pidinfo(PROC_PIDTHREADINFO)`: `cat` blocked on a pty slave and on + /// a pipe both report `WAITING` with flags `0x0` for as long as they stay + /// blocked (10s), whereas `sleep`, a shell waiting on a child, and threads + /// parked on condition variables report `WAITING` with flags `0x1` within + /// ~55ms of parking. Requiring the flag to be clear therefore keeps real + /// stdin waits and drops parked ones. + /// + /// The asymmetry is deliberate: a swapped-out thread that really is in + /// `read(2)` is missed (no "waiting for stdin" hint), whereas counting + /// parked threads reports every sleeping command as waiting for input. The + /// residual false positive is a thread sampled inside that ~55ms parking + /// window; the bash tool waits `STDIN_INITIAL_DELAY_MS` (300ms) before its + /// first poll, so a command that parks at startup is swapped out by then. + pub(super) fn thread_is_reading_stdin(run_state: i32, flags: i32) -> bool { + run_state == TH_STATE_WAITING && flags & TH_FLAGS_SWAPPED == 0 + } pub fn check(pid: u32) -> StdinState { // Check if fd 0 (stdin) is a pipe or pty @@ -342,7 +371,7 @@ mod macos { ) }; - if ret > 0 && tinfo.pth_run_state == TH_STATE_WAITING { + if ret > 0 && thread_is_reading_stdin(tinfo.pth_run_state, tinfo.pth_flags) { return true; } } diff --git a/crates/jcode-core/src/stdin_detect_tests.rs b/crates/jcode-core/src/stdin_detect_tests.rs index 2e62b575e6..57ea829654 100644 --- a/crates/jcode-core/src/stdin_detect_tests.rs +++ b/crates/jcode-core/src/stdin_detect_tests.rs @@ -2,6 +2,12 @@ use super::*; #[cfg(target_os = "linux")] use std::process::{Command, Stdio}; +// Not sound on macOS: this test binary always has harness threads parked on +// condition variables, and a thread sampled within ~55ms of parking still has +// `TH_FLAGS_SWAPPED` clear, so the process can look like it is reading stdin +// (issue #1146). `test_sleeping_child_not_reading_stdin` covers the negative +// direction on macOS against a process whose only thread is a timed sleep. +#[cfg(not(target_os = "macos"))] #[test] fn test_own_process_not_reading_stdin() { let pid = std::process::id(); @@ -15,6 +21,103 @@ fn test_nonexistent_pid() { assert_ne!(state, StdinState::Reading); } +#[cfg(target_os = "macos")] +#[test] +fn swapped_out_wait_is_not_a_stdin_read() { + use libc::{TH_FLAGS_SWAPPED, TH_STATE_RUNNING, TH_STATE_WAITING}; + + // Parked on a condition variable or in a timed sleep: blocked, but the + // kernel has already reclaimed its stack (issue #1146). + assert!(!super::macos::thread_is_reading_stdin( + TH_STATE_WAITING, + TH_FLAGS_SWAPPED + )); + // Blocked in `read(2)`: still resident, so this is a stdin wait. + assert!(super::macos::thread_is_reading_stdin(TH_STATE_WAITING, 0)); + // Not blocked at all. + assert!(!super::macos::thread_is_reading_stdin(TH_STATE_RUNNING, 0)); +} + +// The `TH_FLAGS_SWAPPED` gate suppresses parked threads; this pins the negative +// direction against a real process, since macOS reported every sleeping command +// as waiting for terminal input before it (issue #1146). +#[cfg(target_os = "macos")] +#[test] +fn test_sleeping_child_not_reading_stdin() { + let mut child = std::process::Command::new("sleep") + .arg("5") + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::null()) + .spawn() + .expect("failed to spawn sleep"); + + // Same delay the bash tool uses before its first poll, so the child is + // parked in `nanosleep` (not in `read(0)`) and its stack is long reclaimed. + std::thread::sleep(std::time::Duration::from_millis(300)); + let state = is_waiting_for_stdin(child.id()); + + child.kill().ok(); + child.wait().ok(); + + assert_eq!( + state, + StdinState::NotReading, + "sleep parks in nanosleep, not in read(0)" + ); +} + +// The `TH_FLAGS_SWAPPED` gate suppresses parked threads; this pins the other +// direction so a process genuinely blocked on terminal input is still reported. +// `cat` reads from a pty slave, which is what an interactive command actually +// sees. +#[cfg(target_os = "macos")] +#[test] +fn test_blocked_on_a_terminal_is_detected() { + use std::os::fd::{FromRawFd, OwnedFd}; + + let mut master: libc::c_int = -1; + let mut slave: libc::c_int = -1; + let rc = unsafe { + libc::openpty( + &mut master, + &mut slave, + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::null_mut(), + ) + }; + assert_eq!(rc, 0, "openpty failed"); + // SAFETY: openpty returned two fresh descriptors we now own. + let master_fd = unsafe { OwnedFd::from_raw_fd(master) }; + let slave_fd = unsafe { OwnedFd::from_raw_fd(slave) }; + + let mut child = std::process::Command::new("cat") + .stdin(std::process::Stdio::from(slave_fd)) + .stdout(std::process::Stdio::null()) + .spawn() + .expect("failed to spawn cat"); + + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + let mut state = StdinState::NotReading; + while std::time::Instant::now() < deadline { + state = is_waiting_for_stdin(child.id()); + if state == StdinState::Reading { + break; + } + std::thread::sleep(std::time::Duration::from_millis(20)); + } + + child.kill().ok(); + child.wait().ok(); + drop(master_fd); + + assert_eq!( + state, + StdinState::Reading, + "cat blocked on a pty should still be detected" + ); +} + #[cfg(target_os = "linux")] #[test] fn test_blocked_process_detected() { diff --git a/crates/jcode-provider-anthropic-runtime/src/account_failover_tests.rs b/crates/jcode-provider-anthropic-runtime/src/account_failover_tests.rs index 2d387c244f..d62517a86f 100644 --- a/crates/jcode-provider-anthropic-runtime/src/account_failover_tests.rs +++ b/crates/jcode-provider-anthropic-runtime/src/account_failover_tests.rs @@ -242,7 +242,10 @@ fn http_retry_decision( let first_attempt = usize::from(router_case.is_some_and(|case| case.cached_first_rejection)); for attempt in first_attempt..if through_router { 2 } else { 1 } { - let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + // Generous under parallel-suite load; the guard only needs to fail + // when the request truly never arrives, not when the runner is + // starved. A correct request still lands in milliseconds. + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); let (mut socket, _) = loop { match listener.accept() { Ok(connection) => break connection, @@ -257,12 +260,29 @@ fn http_retry_decision( } }; socket - .set_read_timeout(Some(std::time::Duration::from_secs(3))) + .set_read_timeout(Some(std::time::Duration::from_secs(10))) .unwrap(); let mut request = Vec::new(); + // The listener is nonblocking and, on macOS, accepted sockets + // inherit that flag, so `read` returns WouldBlock while the + // runtime client is still between connect and write. Retry on + // WouldBlock until a deadline instead of panicking on it. + let read_deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); loop { let mut buffer = [0; 4096]; - let count = socket.read(&mut buffer).unwrap(); + let count = loop { + match socket.read(&mut buffer) { + Ok(count) => break count, + Err(error) if error.kind() == std::io::ErrorKind::WouldBlock => { + assert!( + std::time::Instant::now() < read_deadline, + "request body did not arrive" + ); + std::thread::sleep(std::time::Duration::from_millis(5)); + } + Err(error) => panic!("local HTTP read failed: {error}"), + } + }; assert!(count > 0, "request ended before complete body"); request.extend_from_slice(&buffer[..count]); if let Some(header_end) = request.windows(4).position(|bytes| bytes == b"\r\n\r\n") @@ -383,7 +403,9 @@ fn http_retry_decision( }; let (decision, rolled_back) = runtime.block_on(async { if let Some(router) = &router { - tokio::time::timeout(std::time::Duration::from_secs(3), async { + // Only guards against waiting out the 60s Retry-After backoff; + // 10s still catches that with room for a loaded runner. + tokio::time::timeout(std::time::Duration::from_secs(10), async { let case = router_case.unwrap(); let initial_result = if case.split { router @@ -567,7 +589,9 @@ fn http_retry_decision( transport, retry_settings, )); - let result = tokio::time::timeout(std::time::Duration::from_secs(3), async { + // Only guards against waiting out the 60s Retry-After backoff; + // 10s still catches that with room for a loaded runner. + let result = tokio::time::timeout(std::time::Duration::from_secs(10), async { let mut rolled_back = false; while let Some(event) = rx.recv().await { match event { diff --git a/crates/jcode-provider-gemini-runtime/src/gemini_tests.rs b/crates/jcode-provider-gemini-runtime/src/gemini_tests.rs index ded2d095ea..f1cd1c3609 100644 --- a/crates/jcode-provider-gemini-runtime/src/gemini_tests.rs +++ b/crates/jcode-provider-gemini-runtime/src/gemini_tests.rs @@ -1084,7 +1084,14 @@ fn post_json_retries_transient_http_responses_against_local_server() { } }; stream - .set_read_timeout(Some(Duration::from_secs(2))) + // The listener is nonblocking and macOS hands that flag to + // accepted sockets, making reads return WouldBlock while the + // client is between connect and write. Reads here want plain + // blocking-with-timeout behavior. + .set_nonblocking(false) + .unwrap(); + stream + .set_read_timeout(Some(Duration::from_secs(10))) .unwrap(); let mut reader = std::io::BufReader::new(&stream); let mut content_length = 0; diff --git a/crates/jcode-provider-openai-runtime/src/openai_tests/persistent_prefix.rs b/crates/jcode-provider-openai-runtime/src/openai_tests/persistent_prefix.rs index 41dfca7e63..b6fc22d6f5 100644 --- a/crates/jcode-provider-openai-runtime/src/openai_tests/persistent_prefix.rs +++ b/crates/jcode-provider-openai-runtime/src/openai_tests/persistent_prefix.rs @@ -58,6 +58,15 @@ async fn prefix_test_complete(provider: &OpenAIProvider, messages: &[ChatMessage async fn prefix_test_provider() -> OpenAIProvider { let provider = OpenAIProvider::new(prewarm_test_credentials()); *provider.credentials.write().await = prewarm_test_credentials(); + // Mark the model available so request setup never schedules a background + // `/models` fetch: that GET would land on the fixture's single accept and + // break the handshake. `prewarm_test_credentials` is an API key, so the + // provider reads the key's own `api-key::` catalog, not the OAuth account + // scope: seed the scope this provider actually queries. + jcode_base::provider::populate_account_models_for_scope( + &provider.catalog_scope(), + vec!["gpt-5.6-sol".to_string()], + ); provider.set_model("gpt-5.6-sol").unwrap(); provider.set_transport("websocket").unwrap(); provider diff --git a/crates/jcode-provider-openai-runtime/src/openai_tests/persistent_terminal.rs b/crates/jcode-provider-openai-runtime/src/openai_tests/persistent_terminal.rs index 23a748fefb..1f92a9fcb5 100644 --- a/crates/jcode-provider-openai-runtime/src/openai_tests/persistent_terminal.rs +++ b/crates/jcode-provider-openai-runtime/src/openai_tests/persistent_terminal.rs @@ -192,6 +192,15 @@ async fn persistent_terminal_public_case( .unwrap(); let provider = OpenAIProvider::new(prewarm_test_credentials()); *provider.credentials.write().await = prewarm_test_credentials(); + // Mark the model available so request setup never schedules a background + // `/models` fetch: that GET would land on the fixture's single accept and + // break the handshake. `prewarm_test_credentials` is an API key, so the + // provider reads the key's own `api-key::` catalog, not the OAuth account + // scope: seed the scope this provider actually queries. + jcode_base::provider::populate_account_models_for_scope( + &provider.catalog_scope(), + vec!["gpt-5.6-sol".to_string()], + ); provider.set_model("gpt-5.6-sol").unwrap(); provider.set_transport("websocket").unwrap(); *provider.persistent_ws.lock().await = Some(PersistentWsState { @@ -402,6 +411,12 @@ async fn fresh_ws_terminal_error_ends_stream_with_socket_open() { }); let provider = OpenAIProvider::new(prewarm_test_credentials()); *provider.credentials.write().await = prewarm_test_credentials(); + // Seed the API-key catalog scope so request setup never schedules a + // background `/models` GET onto this fixture's single accept. + jcode_base::provider::populate_account_models_for_scope( + &provider.catalog_scope(), + vec!["gpt-5.6-sol".to_string()], + ); provider.set_model("gpt-5.6-sol").unwrap(); provider.set_transport("websocket").unwrap(); let messages = vec![ChatMessage::user("hi")]; diff --git a/crates/jcode-provider-openai-runtime/src/openai_tests/websocket_prewarm.rs b/crates/jcode-provider-openai-runtime/src/openai_tests/websocket_prewarm.rs index ddf8fb7651..befcdcf1d0 100644 --- a/crates/jcode-provider-openai-runtime/src/openai_tests/websocket_prewarm.rs +++ b/crates/jcode-provider-openai-runtime/src/openai_tests/websocket_prewarm.rs @@ -157,6 +157,15 @@ async fn websocket_v2_prewarm_is_adopted_by_complete_without_losing_request_stat let provider = OpenAIProvider::new(prewarm_test_credentials()); *provider.credentials.write().await = prewarm_test_credentials(); + // Mark the model available so request setup never schedules a background + // `/models` fetch: that GET would land on the fixture's single accept and + // break the handshake. `prewarm_test_credentials` is an API key, so the + // provider reads the key's own `api-key::` catalog, not the OAuth account + // scope: seed the scope this provider actually queries. + jcode_base::provider::populate_account_models_for_scope( + &provider.catalog_scope(), + vec!["gpt-5.6-sol".to_string()], + ); provider .set_transport("websocket") .expect("force local websocket transport"); diff --git a/crates/jcode-provider-openrouter-runtime/src/azure_retry_tests.rs b/crates/jcode-provider-openrouter-runtime/src/azure_retry_tests.rs index d30226b355..eaa2cc7da5 100644 --- a/crates/jcode-provider-openrouter-runtime/src/azure_retry_tests.rs +++ b/crates/jcode-provider-openrouter-runtime/src/azure_retry_tests.rs @@ -36,12 +36,29 @@ impl Server { Err(error) => panic!("accept: {error}"), }; socket - .set_read_timeout(Some(Duration::from_secs(3))) + .set_read_timeout(Some(Duration::from_secs(10))) .unwrap(); let mut bytes = Vec::new(); + // The listener is nonblocking and macOS hands that flag to + // accepted sockets, so reads return WouldBlock while the + // client is between connect and write. Retry instead of dying, + // or the client hangs until its retry budget runs out. + let read_deadline = Instant::now() + Duration::from_secs(30); loop { let mut buffer = [0; 4096]; - let count = socket.read(&mut buffer).unwrap(); + let count = loop { + match socket.read(&mut buffer) { + Ok(count) => break count, + Err(error) if error.kind() == std::io::ErrorKind::WouldBlock => { + assert!( + Instant::now() < read_deadline, + "request body did not arrive" + ); + std::thread::sleep(Duration::from_millis(5)); + } + Err(error) => panic!("local HTTP read failed: {error}"), + } + }; assert_ne!(count, 0, "request must have a body"); bytes.extend_from_slice(&buffer[..count]); if let Some(end) = bytes.windows(4).position(|part| part == b"\r\n\r\n") { diff --git a/crates/jcode-sdk/src/auth/tests.rs b/crates/jcode-sdk/src/auth/tests.rs index b6109b6404..29fb591006 100644 --- a/crates/jcode-sdk/src/auth/tests.rs +++ b/crates/jcode-sdk/src/auth/tests.rs @@ -186,9 +186,11 @@ sys.exit(1 if mode in ('warning', 'legacy-warning') else 0) let notified = thread::spawn(move || { use std::io::BufRead; let (stream, _) = listener.accept().unwrap(); - stream - .set_read_timeout(Some(Duration::from_secs(2))) - .unwrap(); + // The flow's python fixture runs before the notification lands; under + // full-suite load that spawn can take seconds. Hang guard, not latency budget. + // If the writer already sent and closed, macOS rejects SO_RCVTIMEO with + // EINVAL, but the line is already buffered and the read returns at once. + let _ = stream.set_read_timeout(Some(Duration::from_secs(10))); let mut line = String::new(); std::io::BufReader::new(stream) .read_line(&mut line) @@ -271,7 +273,10 @@ sys.exit(1 if mode in ('warning', 'legacy-warning') else 0) #[test] fn timeout_reaps_process_and_unique_ids_isolate_cancellation() { let (dir, mut client) = fixture("hang"); - client.options.timeout = Duration::from_millis(150); + // Same as the client default: the begin/start exchange shares this + // deadline with the python spawn, and under full-suite load 1.5s + // proved too tight. The hang fixture sleeps 60s, so it still times out. + client.options.timeout = Duration::from_secs(3); let flow = client.begin("copilot", None).unwrap(); let other = client.begin("copilot", None).unwrap(); assert_ne!(flow.0.flow_id, other.0.flow_id); @@ -527,7 +532,9 @@ sys.exit(1 if mode in ('warning', 'legacy-warning') else 0) let (_dir, mut client, reserved) = loopback_fixture(); let port = reserved.local_addr().unwrap().port(); drop(reserved); - client.options.timeout = Duration::from_millis(300); + // Same as the client default: `start()` shares this deadline with the + // python spawn, and under full-suite load 1.5s proved too tight. + client.options.timeout = Duration::from_secs(3); let flow = client.begin("openai", None).unwrap(); flow.start().unwrap(); let _peer = std::net::TcpStream::connect((std::net::Ipv4Addr::LOCALHOST, port)).unwrap(); @@ -536,7 +543,7 @@ sys.exit(1 if mode in ('warning', 'legacy-warning') else 0) flow.wait_for_callback().unwrap_err().kind, ErrorKind::Timeout ); - assert!(started.elapsed() < Duration::from_secs(1)); + assert!(started.elapsed() < Duration::from_secs(5)); assert!(!flow.has_callback_listener()); assert!(flow.submit_callback("private-fixture-secret").is_ok()); } diff --git a/crates/jcode-sdk/tests/lifecycle_events.rs b/crates/jcode-sdk/tests/lifecycle_events.rs index cdbd56dcf1..233dc5000d 100644 --- a/crates/jcode-sdk/tests/lifecycle_events.rs +++ b/crates/jcode-sdk/tests/lifecycle_events.rs @@ -323,8 +323,40 @@ fn global_events_reports_bounded_queue_overflow() { assert_eq!(error.code(), "event_buffer_overflow"); } +/// Saves `TMPDIR` on creation and puts the original value back on drop, +/// including panic unwind. The private temp dir this guard points the env at +/// is deleted when the test ends, so leaving it set would strand every later +/// test in this process on a missing temp root. +struct TmpdirGuard(Option); + +impl TmpdirGuard { + fn set(private_temp: &std::path::Path) -> Self { + // Single-threaded test start: no other threads exist yet, so mutating + // the environment here cannot race a concurrent getenv. + let previous = std::env::var_os("TMPDIR").map(PathBuf::from); + unsafe { std::env::set_var("TMPDIR", private_temp) }; + Self(previous) + } +} + +impl Drop for TmpdirGuard { + fn drop(&mut self) { + match self.0.take() { + Some(original) => unsafe { std::env::set_var("TMPDIR", original) }, + None => unsafe { std::env::remove_var("TMPDIR") }, + } + } +} + #[test] fn missing_launch_binary_is_typed_and_cleans_up_its_temporary_home() { + // `launch_instance` creates its ephemeral home in std::env::temp_dir(). + // Nextest runs test binaries concurrently, and unit tests in the jcode-sdk + // lib binary also create `jcode-sdk-instance-*` homes in the shared temp + // root, which races the before/after count below. Point TMPDIR at a private + // dir for this process so the count only sees this test's homes. + let private_temp = tempfile::tempdir().expect("private temp root"); + let _tmpdir = TmpdirGuard::set(private_temp.path()); let before = std::fs::read_dir(std::env::temp_dir()) .expect("temp root") .flatten() diff --git a/crates/jcode-setup-hints/src/setup_hints_tests.rs b/crates/jcode-setup-hints/src/setup_hints_tests.rs index d16e7c61f9..064b409a5b 100644 --- a/crates/jcode-setup-hints/src/setup_hints_tests.rs +++ b/crates/jcode-setup-hints/src/setup_hints_tests.rs @@ -32,6 +32,7 @@ fn first_three_launches_can_include_hotkey_notice_too() { let state = SetupHintsState { launch_count: 2, hotkey_configured: true, + startup_spawn_hint_dismissed: false, ..SetupHintsState::default() }; diff --git a/crates/jcode-tui/src/tui/app.rs b/crates/jcode-tui/src/tui/app.rs index 6fd538b11b..0761a077f0 100644 --- a/crates/jcode-tui/src/tui/app.rs +++ b/crates/jcode-tui/src/tui/app.rs @@ -334,6 +334,10 @@ struct PendingModelPickerLoad { signature: ModelPickerCacheSignature, #[allow(dead_code)] // kept for picker load timing diagnostics picker_started: Instant, + /// When this load feeds an agent-model sub-picker (Alt+S / Alt+P in + /// /model), the completion path must re-tag the rebuilt picker with this + /// target instead of leaving the ordinary model picker behind. + agent_target: Option, receiver: mpsc::Receiver>, /// Write the normalized routes back into `remote_model_options` when the /// background build lands (remote hot path). diff --git a/crates/jcode-tui/src/tui/app/auth_tests.rs b/crates/jcode-tui/src/tui/app/auth_tests.rs index 11e80bd6d6..20619af90e 100644 --- a/crates/jcode-tui/src/tui/app/auth_tests.rs +++ b/crates/jcode-tui/src/tui/app/auth_tests.rs @@ -15,6 +15,23 @@ fn with_temp_jcode_home(f: impl FnOnce() -> T) -> T { "JCODE_OPENAI_COMPAT_DEFAULT_MODEL", "JCODE_OPENAI_COMPAT_LOCAL_ENABLED", "OPENAI_COMPAT_API_KEY", + // Ambient named-profile / OpenRouter runtime env leaks across tests + // and flips `openai_compatible_profile_is_configured` into the + // named-profile path; clear it so each test starts hermetic. + "JCODE_NAMED_PROVIDER_PROFILE", + "JCODE_PROVIDER_PROFILE_ACTIVE", + "JCODE_PROVIDER_PROFILE_NAME", + "JCODE_OPENROUTER_API_BASE", + "JCODE_OPENROUTER_API_KEY_NAME", + "JCODE_OPENROUTER_ENV_FILE", + "JCODE_OPENROUTER_CACHE_NAMESPACE", + "JCODE_OPENROUTER_PROVIDER_FEATURES", + "JCODE_OPENROUTER_TRANSPORT_STATE", + "JCODE_OPENROUTER_ALLOW_NO_AUTH", + "JCODE_OPENROUTER_PROVIDER", + "JCODE_OPENROUTER_NO_FALLBACK", + "JCODE_API_BASE", + "JCODE_API_KEY", ] .map(|key| (key, std::env::var_os(key))); diff --git a/crates/jcode-tui/src/tui/app/inline_interactive.rs b/crates/jcode-tui/src/tui/app/inline_interactive.rs index 22ea657769..80719f6a7f 100644 --- a/crates/jcode-tui/src/tui/app/inline_interactive.rs +++ b/crates/jcode-tui/src/tui/app/inline_interactive.rs @@ -17,10 +17,10 @@ mod preview; #[path = "inline_interactive/preview_request.rs"] mod preview_request; use helpers::{ - agent_model_default_summary, agent_model_target_label, agent_model_target_slug, - catchup_candidates, catchup_queue_position, model_entry_base_name, model_entry_saved_spec, - openrouter_route_model_id, picker_route_model_spec, picker_route_selection, - save_agent_model_override, + agent_model_chord_target, agent_model_default_summary, agent_model_target_label, + agent_model_target_slug, catchup_candidates, catchup_queue_position, model_entry_base_name, + model_entry_saved_spec, openrouter_route_model_id, picker_route_model_spec, + picker_route_selection, save_agent_model_override, }; pub(super) fn subagent_picker_model_spec(entry: &PickerEntry) -> String { @@ -1379,6 +1379,7 @@ impl App { ); routes }, + None, ); return; } @@ -1428,6 +1429,7 @@ impl App { ); routes }, + None, ); return; } @@ -1491,9 +1493,12 @@ impl App { picker_started: std::time::Instant, ) { let provider = self.provider.clone(); - self.start_model_picker_route_load_with(signature, picker_started, move || { - provider.model_routes() - }); + self.start_model_picker_route_load_with( + signature, + picker_started, + move || provider.model_routes(), + None, + ); } /// Run an arbitrary route builder off the UI thread and deliver the result @@ -1503,8 +1508,15 @@ impl App { signature: ModelPickerCacheSignature, picker_started: std::time::Instant, build_routes: impl FnOnce() -> Vec + Send + 'static, + agent_target: Option, ) { - self.start_model_picker_route_load_inner(signature, picker_started, false, build_routes); + self.start_model_picker_route_load_inner( + signature, + picker_started, + false, + build_routes, + agent_target, + ); } /// Build routes AND the full picker entry list off the UI thread. Only @@ -1515,6 +1527,7 @@ impl App { picker_started: std::time::Instant, store_remote_routes: bool, build_routes: impl FnOnce() -> Vec + Send + 'static, + agent_target: Option, ) { self.model_picker_load_request_id = self.model_picker_load_request_id.wrapping_add(1); let request_id = self.model_picker_load_request_id; @@ -1538,6 +1551,7 @@ impl App { request_id, signature, picker_started, + agent_target, receiver: rx, store_remote_routes, }); @@ -1638,6 +1652,17 @@ impl App { ModelPickerBuildOutcome::Empty => Vec::new(), } }; + if let Some(target) = pending.agent_target + && picker_open + && self.inline_interactive_state.is_some() + { + // This load feeds an agent-model sub-picker (Alt+S / + // Alt+P in /model). The rebuild above recreated the + // ordinary model picker; re-tag it so Enter still saves + // the agent override instead of switching the session + // model. + self.retag_picker_for_agent_model_target(target); + } if pending.store_remote_routes && !routes.is_empty() { // Keep the normalized routes so the signature (which // samples remote_model_options) stays stable, then @@ -2488,6 +2513,10 @@ impl App { self.handle_inline_interactive_key(code, modifiers)?; return Ok(true); } + if let Some(target) = agent_model_chord_target(code, modifiers) { + self.open_agent_model_picker(target); + return Ok(true); + } Ok(false) } @@ -3534,6 +3563,19 @@ impl App { }); return Ok(()); } + // `/model` picker and its preview: Alt+S / Alt+P open the swarm / + // memory model sub-pickers so users can choose which model spawned + // swarm agents or memory extraction use, without leaving the /model + // flow. Ctrl+S keeps its global input-stash meaning. + if let Some(target) = agent_model_chord_target(code, modifiers) + && self + .inline_interactive_state + .as_ref() + .is_some_and(picker_is_runtime_model_picker) + { + self.open_agent_model_picker(target); + return Ok(()); + } match code { KeyCode::Esc => { if let Some(ref mut picker) = self.inline_interactive_state @@ -4099,6 +4141,12 @@ impl App { } } KeyCode::Char(c) => { + // Ctrl+letter chords never type: Ctrl+S is the global + // input-stash chord, and letting unhandled ctrl chords fall + // through here leaked their letter into the filter. + if modifiers.contains(KeyModifiers::CONTROL) { + return Ok(()); + } if let Some(ref mut picker) = self.inline_interactive_state // Spaces separate independently matched search terms. The // token matcher already permits those terms to appear in diff --git a/crates/jcode-tui/src/tui/app/inline_interactive/helpers.rs b/crates/jcode-tui/src/tui/app/inline_interactive/helpers.rs index f66454f553..a09b3318fd 100644 --- a/crates/jcode-tui/src/tui/app/inline_interactive/helpers.rs +++ b/crates/jcode-tui/src/tui/app/inline_interactive/helpers.rs @@ -1,6 +1,36 @@ use super::*; use crate::tui::{AgentModelTarget, PickerEntry, PickerOption}; +/// The agent sub-picker a chord opens: Alt+S -> swarm, Alt+P -> memory. +/// +/// Ctrl cannot carry these: Ctrl+S is the global input-stash chord and Ctrl+M +/// arrives as Enter on legacy terminals, so the chords live on Alt. macOS +/// terminals often insert the Option-key character instead of reporting ALT +/// (e.g. Option+S -> `ß`), so that character is normalized too. Alt+P keeps +/// clear of Alt+M, which stays free for features that later branches add to +/// this same picker, so merged stacks never have to renegotiate the chord. +pub(super) fn agent_model_chord_target( + code: KeyCode, + modifiers: KeyModifiers, +) -> Option { + if modifiers.contains(KeyModifiers::CONTROL) { + return None; + } + let letter = if modifiers.contains(KeyModifiers::ALT) { + match code { + KeyCode::Char(c) => Some(c.to_ascii_lowercase()), + _ => None, + } + } else { + crate::tui::keybind::shortcut_char_for_macos_option_key(code, modifiers) + }?; + match letter { + 's' => Some(AgentModelTarget::Swarm), + 'p' => Some(AgentModelTarget::Memory), + _ => None, + } +} + pub(super) fn slash_command_preview_filter(input: &str, commands: &[&str]) -> Option { let trimmed = input.trim_start(); for command in commands { @@ -257,6 +287,70 @@ mod tests { use super::*; use crate::tui::{PickerAction, PickerEntry, PickerOption}; + #[test] + fn agent_model_chord_targets_alt_s_and_alt_p() { + assert_eq!( + agent_model_chord_target(KeyCode::Char('s'), KeyModifiers::ALT), + Some(AgentModelTarget::Swarm) + ); + assert_eq!( + agent_model_chord_target(KeyCode::Char('p'), KeyModifiers::ALT), + Some(AgentModelTarget::Memory) + ); + // Uppercase Alt+ chords behave the same as lowercase. + assert_eq!( + agent_model_chord_target(KeyCode::Char('S'), KeyModifiers::ALT), + Some(AgentModelTarget::Swarm) + ); + } + + #[test] + fn agent_model_chord_targets_macos_option_chars() { + // macOS terminals often deliver Option+S / Option+P as the inserted + // character (ß / π) with no ALT modifier; the fallback is + // macOS-only (see shortcut_char_for_macos_option_key), so the chord + // must only resolve there. Plain German/Greek letters elsewhere must + // never open a picker. + if cfg!(target_os = "macos") { + assert_eq!( + agent_model_chord_target(KeyCode::Char('ß'), KeyModifiers::empty()), + Some(AgentModelTarget::Swarm), + ); + assert_eq!( + agent_model_chord_target(KeyCode::Char('π'), KeyModifiers::empty()), + Some(AgentModelTarget::Memory), + ); + } else { + assert_eq!( + agent_model_chord_target(KeyCode::Char('ß'), KeyModifiers::empty()), + None, + ); + assert_eq!( + agent_model_chord_target(KeyCode::Char('π'), KeyModifiers::empty()), + None, + ); + } + } + + #[test] + fn agent_model_chord_ignores_ctrl_and_plain_keys() { + // Ctrl+S must stay the input-stash chord, not the swarm picker. + assert_eq!( + agent_model_chord_target(KeyCode::Char('s'), KeyModifiers::CONTROL), + None + ); + // Plain typing must never trigger the picker chords. + assert_eq!( + agent_model_chord_target(KeyCode::Char('s'), KeyModifiers::empty()), + None + ); + // Unrelated Alt chords are not agent chords either. + assert_eq!( + agent_model_chord_target(KeyCode::Char('x'), KeyModifiers::ALT), + None + ); + } + #[test] fn memory_model_picker_describes_extraction_not_recall() { let target = AgentModelTarget::Memory; diff --git a/crates/jcode-tui/src/tui/app/inline_interactive/openers.rs b/crates/jcode-tui/src/tui/app/inline_interactive/openers.rs index 8d2b996ced..a71b1e6bdb 100644 --- a/crates/jcode-tui/src/tui/app/inline_interactive/openers.rs +++ b/crates/jcode-tui/src/tui/app/inline_interactive/openers.rs @@ -239,89 +239,139 @@ impl App { std::thread::sleep(std::time::Duration::from_millis(5)); } + // Keep the target on the pending load so a late catalog refresh + // re-tags the rebuilt picker instead of leaving the ordinary model + // picker behind (Enter would then switch the session model). + if let Some(pending) = self.pending_model_picker_load.as_mut() { + pending.agent_target = Some(target); + } if let Some(ref mut picker) = self.inline_interactive_state { - if target == AgentModelTarget::Memory { - picker.entries.retain(|entry| { - matches!( - crate::provider::provider_for_model(&model_entry_base_name(entry)), - Some("openai" | "claude") - ) - }); - } + configure_agent_model_picker_entries( + picker, + target, + configured, + inherit_summary, + global_scope, + Self::apply_inline_interactive_filter, + ); + } + } - for entry in &mut picker.entries { - let matches_saved = configured.as_deref().map(|saved| { - let base = model_entry_base_name(entry); - model_entry_saved_spec(entry) == saved || base == saved - }) == Some(true); - entry.action = PickerAction::AgentModelChoice { - target, - clear_override: false, - }; - entry.is_current = matches_saved; - entry.is_default = false; + /// Re-tag the current model picker entries as an agent-model sub-picker + /// after an async catalog load rebuilt them. + pub(crate) fn retag_picker_for_agent_model_target(&mut self, target: AgentModelTarget) { + let global_scope = self.agent_models_global_scope; + // Scope-aware, like `open_agent_model_picker`: in session scope the + // saved override lives on the session (and may be the explicit + // "inherit" sentinel), while the global config default must not + // masquerade as the session's choice. Reading only the global config + // here marked the global model as current after a catalog rebuild + // and hid that the session was set to inherit the coordinator + // (greptile: the explicit inheritance choice disappeared). + let configured = if global_scope { + load_agent_model_override(target) + } else { + self.session + .effective_agent_model(agent_model_target_slug(target), None) + }; + let inherit_summary = agent_model_default_summary(target, self); + if let Some(ref mut picker) = self.inline_interactive_state { + // The catalog refresh rebuilt the picker from scratch; the user's + // filter text and selection must survive the retag so Enter saves + // the entry they actually picked, and the inherit / saved-override + // rows must exist so the override stays clearable. + let previous_filter = picker.filter.clone(); + let previous_selected_name = picker + .entries + .get(picker.filtered.get(picker.selected).copied().unwrap_or(0)) + .map(|entry| entry.name.clone()); + configure_agent_model_picker_entries( + picker, + target, + configured, + inherit_summary, + global_scope, + Self::apply_inline_interactive_filter, + ); + if !previous_filter.is_empty() { + picker.filter = previous_filter; + Self::apply_inline_interactive_filter(picker); } - - if let Some(saved) = configured.as_deref() { - let already_present = picker.entries.iter().any(|entry| { - model_entry_saved_spec(entry) == saved || model_entry_base_name(entry) == saved - }); - if !already_present { - picker.entries.insert( - 0, - PickerEntry { - name: saved.to_string(), - options: vec![PickerOption { - provider: "saved override".to_string(), - api_method: agent_model_target_config_path(target).to_string(), - available: true, - detail: "not in current picker catalog".to_string(), - estimated_reference_cost_micros: None, - }], - action: PickerAction::AgentModelChoice { - target, - clear_override: false, - }, - selected_option: 0, - is_current: true, - is_default: false, - is_favorite: false, - recommended: false, - recommendation_rank: usize::MAX, - usage_score: 0, - old: false, - created_date: None, - effort: None, - }, - ); - } + if let Some(name) = previous_selected_name + && let Some(position) = picker + .filtered + .iter() + .position(|&idx| picker.entries[idx].name == name) + { + picker.selected = position; + } else { + // The rebuilt catalog may drop the previously selected entry; + // clamp so `selected` stays a valid index into `filtered`. + picker.selected = picker.selected.min(picker.filtered.len().saturating_sub(1)); } + } + } +} +/// Shared agent-picker entry setup: tag every entry as an +/// `AgentModelChoice`, keep a row for a saved override that the catalog +/// lacks, prepend the inherit (clear-override) row, and reset filter + +/// selection to a clean state. Used both when the picker first opens and +/// when a late catalog load rebuilds it, so the retag path can never lose +/// the inherit / saved-override rows. +fn configure_agent_model_picker_entries( + picker: &mut InlineInteractiveState, + target: AgentModelTarget, + configured: Option, + inherit_summary: String, + global_scope: bool, + apply_filter: fn(&mut InlineInteractiveState), +) { + // The agent sub-picker is standalone, never a composer-glued preview. + // Opening it from a /model preview rebuilds the picker and that rebuild + // restores preview=true, which made the next typed character land in the + // composer and the preview sync close the picker instead of filtering it. + picker.preview = false; + // No provider filter for Memory anymore: the sidecar's Provider backend + // routes extraction through the live agent provider, so every model in + // the catalog works. The old openai/claude-only filter hid routed custom + // models (e.g. `together_ai/...`) and made the memory picker look broken. + + for entry in &mut picker.entries { + let matches_saved = configured.as_deref().map(|saved| { + let base = model_entry_base_name(entry); + model_entry_saved_spec(entry) == saved || base == saved + }) == Some(true); + entry.action = PickerAction::AgentModelChoice { + target, + clear_override: false, + }; + entry.is_current = matches_saved; + entry.is_default = false; + } + + if let Some(saved) = configured.as_deref() { + let already_present = picker.entries.iter().any(|entry| { + model_entry_saved_spec(entry) == saved || model_entry_base_name(entry) == saved + }); + if !already_present { picker.entries.insert( 0, PickerEntry { - name: if global_scope { - format!("use global fallback ({})", inherit_summary) - } else { - "use global default [session]".to_string() - }, + name: saved.to_string(), options: vec![PickerOption { - provider: "default".to_string(), + provider: "saved override".to_string(), api_method: agent_model_target_config_path(target).to_string(), available: true, - detail: if global_scope { - "clear global default".to_string() - } else { - format!("clear only this session override · {}", inherit_summary) - }, + detail: "not in current picker catalog".to_string(), estimated_reference_cost_micros: None, }], action: PickerAction::AgentModelChoice { target, - clear_override: true, + clear_override: false, }, selected_option: 0, - is_current: configured.is_none(), + is_current: true, is_default: false, is_favorite: false, recommended: false, @@ -332,49 +382,84 @@ impl App { effort: None, }, ); + } + } - let mut inherit = picker.entries[0].clone(); - inherit.name = "inherit coordinator".to_string(); - inherit.options[0].provider = "inherit".to_string(); - inherit.options[0].detail = if global_scope { - "global default: inherit coordinator" + picker.entries.insert( + 0, + PickerEntry { + name: if global_scope { + format!("use global fallback ({})", inherit_summary) } else { - "session: bypass global default" - } - .to_string(); - inherit.action = PickerAction::AgentModelChoice { + "use global default [session]".to_string() + }, + options: vec![PickerOption { + provider: "default".to_string(), + api_method: agent_model_target_config_path(target).to_string(), + available: true, + detail: if global_scope { + "clear global default".to_string() + } else { + format!("clear only this session override · {}", inherit_summary) + }, + estimated_reference_cost_micros: None, + }], + action: PickerAction::AgentModelChoice { target, - clear_override: false, - }; - inherit.is_current = configured.as_deref() == Some("inherit"); - picker.entries.insert(1, inherit); - for entry in &mut picker.entries { - for option in &mut entry.options { - option.detail = format!( - "[{}] {}", - if global_scope { "global" } else { "session" }, - option.detail - ); - } - } + clear_override: true, + }, + selected_option: 0, + is_current: configured.is_none(), + is_default: false, + is_favorite: false, + recommended: false, + recommendation_rank: usize::MAX, + usage_score: 0, + old: false, + created_date: None, + effort: None, + }, + ); - if target == AgentModelTarget::Memory { - for entry in &mut picker.entries { - for option in &mut entry.options { - option.detail = - format!("Extraction only; recall uses Jev. {}", option.detail); - } - } - } + let mut inherit = picker.entries[0].clone(); + inherit.name = "inherit coordinator".to_string(); + inherit.options[0].provider = "inherit".to_string(); + inherit.options[0].detail = if global_scope { + "global default: inherit coordinator" + } else { + "session: bypass global default" + } + .to_string(); + inherit.action = PickerAction::AgentModelChoice { + target, + clear_override: false, + }; + inherit.is_current = configured.as_deref() == Some("inherit"); + picker.entries.insert(1, inherit); + for entry in &mut picker.entries { + for option in &mut entry.options { + option.detail = format!( + "[{}] {}", + if global_scope { "global" } else { "session" }, + option.detail + ); + } + } - picker.filtered = (0..picker.entries.len()).collect(); - picker.selected = picker - .entries - .iter() - .position(|entry| entry.is_current) - .unwrap_or(0); - picker.column = 0; - picker.filter.clear(); + if target == AgentModelTarget::Memory { + for entry in &mut picker.entries { + for option in &mut entry.options { + option.detail = format!("Extraction only; recall uses Jev. {}", option.detail); + } } } + + picker.filter.clear(); + apply_filter(picker); + picker.selected = picker + .entries + .iter() + .position(|entry| entry.is_current) + .unwrap_or(0); + picker.column = 0; } diff --git a/crates/jcode-tui/src/tui/app/input.rs b/crates/jcode-tui/src/tui/app/input.rs index 9cb9eeb59f..d370a2f089 100644 --- a/crates/jcode-tui/src/tui/app/input.rs +++ b/crates/jcode-tui/src/tui/app/input.rs @@ -2812,8 +2812,15 @@ pub(super) fn handle_modal_key( if let Some(ref picker) = app.inline_interactive_state && !picker.preview { - app.handle_inline_interactive_key(code, modifiers)?; - return Ok(true); + // Ctrl+C/Ctrl+D keep their global meaning (interrupt the blocked + // turn, quit when idle) even while a non-preview picker is open: + // swallowing them here made the first press a dead key and forced + // the user to Esc out of the picker first. + if !(modifiers.contains(KeyModifiers::CONTROL) && matches!(code, KeyCode::Char('c' | 'd'))) + { + app.handle_inline_interactive_key(code, modifiers)?; + return Ok(true); + } } app.handle_inline_interactive_preview_key(&code, modifiers) diff --git a/crates/jcode-tui/src/tui/app/tests.rs b/crates/jcode-tui/src/tui/app/tests.rs index c010aeeb92..6ea0e82055 100644 --- a/crates/jcode-tui/src/tui/app/tests.rs +++ b/crates/jcode-tui/src/tui/app/tests.rs @@ -16,6 +16,7 @@ include!("tests/remote_startup_input_01/part_01.rs"); include!("tests/remote_startup_input_01/part_02.rs"); include!("tests/remote_startup_input_02/part_01.rs"); include!("tests/remote_startup_input_02/part_02.rs"); +include!("tests/remote_startup_input_02/part_03.rs"); include!("tests/remote_startup_input_03/part_01.rs"); include!("tests/remote_startup_input_03/part_02.rs"); include!("tests/remote_startup_input_04.rs"); diff --git a/crates/jcode-tui/src/tui/app/tests/onboarding_flow.rs b/crates/jcode-tui/src/tui/app/tests/onboarding_flow.rs index 004a35ed69..9f9b89e358 100644 --- a/crates/jcode-tui/src/tui/app/tests/onboarding_flow.rs +++ b/crates/jcode-tui/src/tui/app/tests/onboarding_flow.rs @@ -1609,18 +1609,26 @@ fn starting_recent_project_review_queues_remote_turn_without_stuck_sending() { #[test] fn recent_project_review_falls_back_cleanly_when_no_repo_is_known() { - let mut app = onboarding_test_app(); - app.is_remote = true; - app.session.working_dir = dirs::home_dir().map(|path| path.to_string_lossy().into_owned()); + // The fallback path lands on `onboarding_show_suggestions`, which pings + // the default model via `tokio::spawn` whenever ambient auth looks + // available. On a test thread there is no Tokio runtime, so that spawn + // panics. A private temp home (and the auth-cache invalidation it does + // on entry) keeps `has_any_available` false, the same state the test + // was written against. + with_temp_jcode_home(|| { + let mut app = onboarding_test_app(); + app.is_remote = true; + app.session.working_dir = dirs::home_dir().map(|path| path.to_string_lossy().into_owned()); - app.onboarding_start_recent_project_review(); + app.onboarding_start_recent_project_review(); - assert!(!app.pending_turn); - assert!(app.queued_messages.is_empty()); - assert!(!app.onboarding_flow_active()); - assert!(app.status_notice.as_ref().is_some_and(|(notice, _)| { - notice.contains("No active Git repository found") - })); + assert!(!app.pending_turn); + assert!(app.queued_messages.is_empty()); + assert!(!app.onboarding_flow_active()); + assert!(app.status_notice.as_ref().is_some_and(|(notice, _)| { + notice.contains("No active Git repository found") + })); + }); } include!("onboarding_flow_telemetry.rs"); diff --git a/crates/jcode-tui/src/tui/app/tests/remote_model_picker_hotkeys.rs b/crates/jcode-tui/src/tui/app/tests/remote_model_picker_hotkeys.rs index 98ad045345..852feadb76 100644 --- a/crates/jcode-tui/src/tui/app/tests/remote_model_picker_hotkeys.rs +++ b/crates/jcode-tui/src/tui/app/tests/remote_model_picker_hotkeys.rs @@ -121,3 +121,273 @@ fn test_remote_model_picker_preview_ctrl_o_sets_default() { ); }); } + +#[test] +fn test_model_picker_chords_do_not_pollute_filter() { + with_temp_jcode_home(|| { + let mut app = create_test_app(); + app.is_remote = true; + app.remote_available_entries = vec!["gpt-5.5".to_string()]; + app.remote_model_options = vec![crate::provider::ModelRoute { + model: "gpt-5.5".to_string(), + provider: "OpenAI".to_string(), + api_method: "openai".to_string(), + available: true, + detail: String::new(), + cheapness: None, + usage: None, + }]; + + app.open_model_picker(); + assert!( + app.inline_interactive_state.is_some(), + "model picker should be open" + ); + + // Ctrl+S inside the picker must not leak a stray 's' into the filter: + // it collides with the global input-stash chord, so it is a dead key + // here (never opens the swarm sub-picker, never types). + app.handle_key(KeyCode::Char('s'), KeyModifiers::CONTROL) + .unwrap(); + let picker = app + .inline_interactive_state + .as_ref() + .expect("picker must stay open after Ctrl+S"); + assert!(picker.filter.is_empty(), "Ctrl+S must not type into the filter"); + assert!( + !picker.entries.iter().any(|e| matches!( + e.action, + crate::tui::PickerAction::AgentModelChoice { .. } + )), + "Ctrl+S must not open the swarm sub-picker" + ); + + // Alt+S still opens the swarm sub-picker from the preview. + app.handle_key(KeyCode::Char('s'), KeyModifiers::ALT) + .unwrap(); + let picker = app + .inline_interactive_state + .as_ref() + .expect("picker must stay open after Alt+S"); + assert!( + picker.entries.iter().any(|e| matches!( + e.action, + crate::tui::PickerAction::AgentModelChoice { + target: crate::tui::AgentModelTarget::Swarm, + .. + } + )), + "Alt+S must open the swarm sub-picker" + ); + }); +} + +#[test] +fn test_model_picker_preview_keeps_ctrl_s_as_input_stash() { + with_temp_jcode_home(|| { + let mut app = create_test_app(); + app.is_remote = true; + app.remote_available_entries = vec!["gpt-5.5".to_string()]; + app.remote_model_options = vec![crate::provider::ModelRoute { + model: "gpt-5.5".to_string(), + provider: "OpenAI".to_string(), + api_method: "openai".to_string(), + available: true, + detail: String::new(), + cheapness: None, + usage: None, + }]; + + // Type `/model` so the preview is open with a draft behind it. + for c in "/model".chars() { + app.handle_key(KeyCode::Char(c), KeyModifiers::empty()).unwrap(); + } + let picker = app + .inline_interactive_state + .as_ref() + .expect("typing /model should open the picker preview"); + assert!(picker.preview); + + // Ctrl+S must keep its global meaning while the preview is open: + // stash the draft, never open a sub-picker, never type into the + // filter. This is the muscle-memory collision the rebind fixed. + app.handle_key(KeyCode::Char('s'), KeyModifiers::CONTROL).unwrap(); + let picker = app + .inline_interactive_state + .as_ref() + .expect("preview must stay open after Ctrl+S"); + assert!(picker.filter.is_empty(), "Ctrl+S must not type into the filter"); + assert!( + !picker.entries.iter().any(|e| matches!( + e.action, + crate::tui::PickerAction::AgentModelChoice { .. } + )), + "Ctrl+S must not open an agent sub-picker" + ); + assert!( + app.stashed_input.is_some(), + "Ctrl+S with the preview open must stash the draft" + ); + assert_eq!(app.input(), ""); + }); +} + +#[test] +fn test_remote_model_picker_alt_chords_open_sub_pickers() { + 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.remote_available_entries = vec!["gpt-5.5".to_string()]; + app.remote_model_options = vec![crate::provider::ModelRoute { + model: "gpt-5.5".to_string(), + provider: "OpenAI".to_string(), + api_method: "openai".to_string(), + available: true, + detail: String::new(), + cheapness: None, + usage: None, + }]; + + for c in "/model".chars() { + rt.block_on(app.handle_remote_key( + KeyCode::Char(c), + KeyModifiers::empty(), + &mut remote, + )) + .unwrap(); + } + assert!( + app.inline_interactive_state + .as_ref() + .is_some_and(|p| p.preview), + "typed /model should open the picker preview" + ); + + // The remote event loop must route the preview chords to the picker, + // not drop them into the global ctrl/alt handling. + rt.block_on(app.handle_remote_key( + KeyCode::Char('s'), + KeyModifiers::ALT, + &mut remote, + )) + .expect("Alt+S should be handled in the remote path"); + let picker = app + .inline_interactive_state + .as_ref() + .expect("swarm sub-picker should be open after Alt+S"); + assert!(picker.entries.iter().any(|e| matches!( + e.action, + crate::tui::PickerAction::AgentModelChoice { + target: crate::tui::AgentModelTarget::Swarm, + .. + } + ))); + + // Back out, reopen the /model preview, then Alt+P for memory. + rt.block_on(app.handle_remote_key( + KeyCode::Esc, + KeyModifiers::empty(), + &mut remote, + )) + .unwrap(); + for c in "/model".chars() { + rt.block_on(app.handle_remote_key( + KeyCode::Char(c), + KeyModifiers::empty(), + &mut remote, + )) + .unwrap(); + } + rt.block_on(app.handle_remote_key( + KeyCode::Char('p'), + KeyModifiers::ALT, + &mut remote, + )) + .expect("Alt+P should be handled in the remote path"); + let picker = app + .inline_interactive_state + .as_ref() + .expect("memory sub-picker should be open after Alt+P"); + assert!(picker.entries.iter().any(|e| matches!( + e.action, + crate::tui::PickerAction::AgentModelChoice { + target: crate::tui::AgentModelTarget::Memory, + .. + } + ))); + }); +} + +#[test] +fn test_agent_model_chord_shadows_user_remap_only_inside_model_picker() { + // A user remapping model_switch_next to alt+s must not lose the chord's + // picker meaning: inside the runtime /model picker the chord wins (the + // picker dispatches before the config bindings), while anywhere else the + // user's remap keeps working. Pin the precedence so a future reorder of + // handle_key_core / handle_remote_key_internal cannot silently flip it. + 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]\nmodel_switch_next = \"alt+s\"\n", + ) + .expect("write config"); + + let mut app = create_test_app(); + app.is_remote = true; + app.remote_available_entries = vec!["gpt-5.5".to_string()]; + app.remote_model_options = vec![crate::provider::ModelRoute { + model: "gpt-5.5".to_string(), + provider: "OpenAI".to_string(), + api_method: "openai".to_string(), + available: true, + detail: String::new(), + cheapness: None, + usage: None, + }]; + app.open_model_picker(); + + // Inside the picker the hardcoded chord wins even though the user + // remapped model_switch_next to the same chord. + app.handle_key(KeyCode::Char('s'), KeyModifiers::ALT).unwrap(); + assert!( + app.inline_interactive_state + .as_ref() + .is_some_and(|p| p.entries.iter().any(|e| matches!( + e.action, + crate::tui::PickerAction::AgentModelChoice { + target: crate::tui::AgentModelTarget::Swarm, + .. + } + ))), + "the agent-model chord must win inside the model picker even when a user remap collides" + ); + + // Close the picker; outside the picker the remap must still fire as + // model switching, not open anything. + app.handle_key(KeyCode::Esc, KeyModifiers::empty()).unwrap(); + assert!(app.inline_interactive_state.is_none()); + app.handle_key(KeyCode::Char('s'), KeyModifiers::ALT).unwrap(); + assert!( + app.inline_interactive_state.is_none(), + "outside the picker the user's alt+s remap must not open a sub-picker" + ); + + if let Some(home) = prev_home { + crate::env::set_var("JCODE_HOME", home); + } else { + crate::env::remove_var("JCODE_HOME"); + } + crate::config::Config::invalidate_cache(); +} diff --git a/crates/jcode-tui/src/tui/app/tests/remote_startup_input_01/part_02.rs b/crates/jcode-tui/src/tui/app/tests/remote_startup_input_01/part_02.rs index 9b4bbaabc2..a0f225c734 100644 --- a/crates/jcode-tui/src/tui/app/tests/remote_startup_input_01/part_02.rs +++ b/crates/jcode-tui/src/tui/app/tests/remote_startup_input_01/part_02.rs @@ -426,6 +426,20 @@ fn test_model_picker_copilot_models_have_copilot_route() { #[test] fn test_model_picker_remote_comtegra_model_uses_comtegra_route_not_copilot() { + // Same ambient-profile hazard as the copilot picker tests: with an + // exported JCODE_NAMED_PROVIDER_PROFILE the configured-check takes the + // named-profile path and the Comtegra route is dropped for copilot. + // The env lock serializes this capture against sibling tests that + // rewrite process env, the same lock with_temp_jcode_home holds. + let _env_lock = crate::storage::lock_test_env(); + let _env_guard = EnvRestoreGuard::capture([ + "JCODE_NAMED_PROVIDER_PROFILE", + "JCODE_PROVIDER_PROFILE_ACTIVE", + "JCODE_PROVIDER_PROFILE_NAME", + ]); + crate::env::remove_var("JCODE_NAMED_PROVIDER_PROFILE"); + crate::env::remove_var("JCODE_PROVIDER_PROFILE_ACTIVE"); + crate::env::remove_var("JCODE_PROVIDER_PROFILE_NAME"); let prev_key = std::env::var("COMTEGRA_API_KEY").ok(); crate::env::set_var("COMTEGRA_API_KEY", "test-key"); diff --git a/crates/jcode-tui/src/tui/app/tests/remote_startup_input_02/part_01.rs b/crates/jcode-tui/src/tui/app/tests/remote_startup_input_02/part_01.rs index e631254b68..b49cc14a18 100644 --- a/crates/jcode-tui/src/tui/app/tests/remote_startup_input_02/part_01.rs +++ b/crates/jcode-tui/src/tui/app/tests/remote_startup_input_02/part_01.rs @@ -205,11 +205,9 @@ fn test_remote_fallback_bedrock_arn_does_not_create_openrouter_route() { assert!(routes.iter().any(|route| { route.model == model && route.api_method == "bedrock" && route.provider == "AWS Bedrock" })); - assert!( - !routes - .iter() - .any(|route| route.model == model && route.api_method == "openrouter") - ); + assert!(!routes + .iter() + .any(|route| route.model == model && route.api_method == "openrouter")); } #[test] diff --git a/crates/jcode-tui/src/tui/app/tests/remote_startup_input_02/part_03.rs b/crates/jcode-tui/src/tui/app/tests/remote_startup_input_02/part_03.rs new file mode 100644 index 0000000000..bea4aec010 --- /dev/null +++ b/crates/jcode-tui/src/tui/app/tests/remote_startup_input_02/part_03.rs @@ -0,0 +1,579 @@ +#[test] +fn test_model_picker_alt_s_opens_swarm_model_picker() { + with_temp_jcode_home(|| { + let mut app = create_test_app(); + app.is_remote = true; + app.remote_available_entries = vec!["gpt-5.5".to_string()]; + app.remote_model_options = vec![crate::provider::ModelRoute { + model: "gpt-5.5".to_string(), + provider: "OpenAI".to_string(), + api_method: "openai".to_string(), + available: true, + detail: String::new(), + cheapness: None, + usage: None, + }]; + + app.open_model_picker(); + + // Verify the model picker is open + assert!( + app.inline_interactive_state.is_some(), + "model picker should be open" + ); + + // Alt+S should open the swarm model sub-picker (Ctrl+S stays the + // global input-stash chord) + app.handle_key(KeyCode::Char('s'), KeyModifiers::ALT) + .unwrap(); + + let picker = app + .inline_interactive_state + .as_ref() + .expect("picker should still be open after Alt+S"); + + // After Alt+S, entries should be tagged as AgentModelChoice for Swarm target + assert!( + picker.entries.iter().any(|e| matches!( + e.action, + crate::tui::PickerAction::AgentModelChoice { + target: crate::tui::AgentModelTarget::Swarm, + .. + } + )), + "Alt+S should open swarm model picker with AgentModelChoice entries" + ); + }); +} + +#[test] +fn test_swarm_picker_from_model_preview_stays_searchable() { + with_temp_jcode_home(|| { + let mut app = create_test_app(); + app.is_remote = true; + // >64 names-only entries and no hydrated routes: the typed /model + // preview opens on lightweight routes without caching them, so the + // Alt+S rebuild cannot be served from a fresh picker cache. That + // rebuild path is the one that restores preview=true from the /model + // preview picker, which used to leave the swarm sub-picker glued to + // the composer: the next typed character landed in the composer and + // the preview sync closed the picker instead of filtering it. + app.remote_available_entries = (0..80).map(|i| format!("gpt-test-model-{}", i)).collect(); + if let Ok(config_dir) = crate::storage::app_config_dir() { + let _ = std::fs::remove_file(config_dir.join("remote_model_catalog_cache.json")); + } + + for c in "/model".chars() { + app.handle_key(KeyCode::Char(c), KeyModifiers::empty()) + .unwrap(); + } + let picker = app + .inline_interactive_state + .as_ref() + .expect("model picker preview should be open"); + assert!(picker.preview); + + // Alt+S opens the swarm sub-picker from the preview. + app.handle_key(KeyCode::Char('s'), KeyModifiers::ALT) + .unwrap(); + let picker = app + .inline_interactive_state + .as_ref() + .expect("swarm picker should be open after Alt+S"); + assert!( + picker.entries.iter().any(|e| matches!( + e.action, + crate::tui::PickerAction::AgentModelChoice { + target: crate::tui::AgentModelTarget::Swarm, + .. + } + )), + "Alt+S from preview should open swarm model picker" + ); + assert!( + !picker.preview, + "swarm sub-picker must be a standalone picker, not a composer-glued preview" + ); + + // Typing must filter the sub-picker. With preview=true restored the + // character fell through to the composer and the preview sync closed + // the picker. + app.handle_key(KeyCode::Char('g'), KeyModifiers::empty()) + .unwrap(); + let picker = app + .inline_interactive_state + .as_ref() + .expect("swarm picker must stay open while searching"); + assert_eq!(picker.filter, "g"); + assert!( + !picker.filtered.is_empty(), + "typed search must keep matching rows in the swarm picker" + ); + assert_eq!( + app.input(), + "", + "search characters must not land in the composer" + ); + }); +} + +#[test] +fn test_model_picker_alt_p_opens_memory_model_picker_with_routed_models() { + with_temp_jcode_home(|| { + let mut app = create_test_app(); + app.is_remote = true; + // A routed custom-provider model (slash form) plus a plain OpenAI one: + // the memory sub-picker used to filter entries down to openai/claude + // only, hiding the routed model and making the picker look broken. + // The sidecar's Provider backend routes extraction through any live + // provider, so both models must be selectable. + app.remote_available_entries = vec![ + "gpt-5.5".to_string(), + "together_ai/revolut-ltd/glm-5-3-responses".to_string(), + ]; + app.remote_model_options = vec![ + crate::provider::ModelRoute { + model: "gpt-5.5".to_string(), + provider: "OpenAI".to_string(), + api_method: "openai".to_string(), + available: true, + detail: String::new(), + cheapness: None, + usage: None, + }, + crate::provider::ModelRoute { + model: "together_ai/revolut-ltd/glm-5-3-responses".to_string(), + provider: "Together AI".to_string(), + api_method: "openai".to_string(), + available: true, + detail: String::new(), + cheapness: None, + usage: None, + }, + ]; + + app.open_model_picker(); + assert!( + app.inline_interactive_state.is_some(), + "model picker should be open" + ); + + app.handle_key(KeyCode::Char('p'), KeyModifiers::ALT) + .unwrap(); + + let picker = app + .inline_interactive_state + .as_ref() + .expect("memory picker should be open after Alt+P"); + assert!( + picker.entries.iter().any(|e| matches!( + e.action, + crate::tui::PickerAction::AgentModelChoice { + target: crate::tui::AgentModelTarget::Memory, + .. + } + )), + "Alt+P should open the memory model picker" + ); + assert!( + picker.entries.iter().any(|e| { + e.name.contains("together_ai/revolut-ltd/glm-5-3-responses") + }), + "routed custom-provider models must appear in the memory picker" + ); + assert!( + picker.entries.iter().any(|e| e.name.contains("gpt-5.5")), + "plain OpenAI models must still appear in the memory picker" + ); + }); +} + +#[test] +fn test_memory_model_selection_persists_routed_override_end_to_end() { + use crate::message::Message; + use crate::provider::Provider; + use std::sync::{Arc as StdArc, Mutex as StdMutex}; + + with_temp_jcode_home(|| { + // Local mode: Enter runs the save inline, so the test can observe the + // session override directly. Remote mode would only queue a server + // request (set_agent_model), which is covered by the pending-request + // tests elsewhere. + // + // The mock provider serves the picker catalog through + // available_models_display, exactly like a live routed custom + // provider would expose `together_ai/...` models. + #[derive(Clone)] + struct RoutedCatalogMock(StdArc>); + + #[async_trait::async_trait] + impl Provider for RoutedCatalogMock { + async fn complete( + &self, + _messages: &[Message], + _tools: &[crate::message::ToolDefinition], + _system: &str, + _resume_session_id: Option<&str>, + ) -> Result { + unimplemented!("RoutedCatalogMock") + } + fn name(&self) -> &str { + "openrouter" + } + fn model(&self) -> String { + self.0.lock().unwrap().clone() + } + fn set_model(&self, model: &str) -> anyhow::Result<()> { + *self.0.lock().unwrap() = model.to_string(); + Ok(()) + } + fn available_models_display(&self) -> Vec { + vec![ + "gpt-5.5".to_string(), + "together_ai/revolut-ltd/glm-5-3-responses".to_string(), + ] + } + fn model_routes(&self) -> Vec { + vec![ + crate::provider::ModelRoute { + model: "gpt-5.5".to_string(), + provider: "OpenAI".to_string(), + api_method: "openai".to_string(), + available: true, + detail: String::new(), + cheapness: None, + usage: None, + }, + crate::provider::ModelRoute { + model: "together_ai/revolut-ltd/glm-5-3-responses".to_string(), + provider: "Together AI".to_string(), + api_method: "openai".to_string(), + available: true, + detail: String::new(), + cheapness: None, + usage: None, + }, + ] + } + fn fork(&self) -> Arc { + Arc::new(self.clone()) + } + } + + let routed_model = "together_ai/revolut-ltd/glm-5-3-responses"; + let provider: Arc = Arc::new(RoutedCatalogMock(StdArc::new(StdMutex::new( + routed_model.to_string(), + )))); + let rt = tokio::runtime::Runtime::new().unwrap(); + let registry = rt.block_on(crate::tool::Registry::new(provider.clone())); + let mut app = App::new_for_test_harness(provider.clone(), registry); + + app.open_model_picker(); + app.handle_key(KeyCode::Char('p'), KeyModifiers::ALT) + .unwrap(); + + // Narrow to the routed model so the selection is unambiguous. + for ch in "together_ai".chars() { + app.handle_key(KeyCode::Char(ch), KeyModifiers::NONE) + .unwrap(); + } + let picker = app + .inline_interactive_state + .as_ref() + .expect("memory picker must stay open while searching"); + assert!( + !picker.filtered.is_empty(), + "memory picker must list the routed custom-provider model" + ); + let filtered_names: Vec = picker + .filtered + .iter() + .map(|&i| picker.entries[i].name.clone()) + .collect::>(); + assert!( + filtered_names + .iter() + .all(|name| name.contains("together_ai/revolut-ltd/glm-5-3-responses")), + "filter should leave only routed models, got {:?}", + filtered_names + ); + // The selected entry's saved spec is what Enter persists: the routed + // model id possibly prefixed with the transport route (e.g. + // `openai-oauth:`), never the display name with its effort suffix. + let entry = &picker.entries[picker.filtered[picker.selected]]; + let expected_spec = + crate::tui::app::inline_interactive::subagent_picker_model_spec(entry); + assert!( + expected_spec.contains("together_ai/revolut-ltd/glm-5-3-responses"), + "selected entry should be the routed model, got {}", + expected_spec + ); + + // Enter saves the override: local mode writes it to the session file + // synchronously. + app.handle_key(KeyCode::Enter, KeyModifiers::NONE) + .unwrap(); + + assert!( + app.inline_interactive_state.is_none(), + "picker should close after selection" + ); + let saved = app + .session + .effective_agent_model("memory", None) + .expect("session memory override must be saved after Enter"); + assert_eq!( + saved, expected_spec, + "saved memory override must keep the full routed spec" + ); + + // The sidecar consumption path must accept the routed spec. + // Sidecar::for_session routes session memory overrides through the + // live provider, exactly like the end-of-turn memory extraction does + // in both local TUI and server sessions. + crate::provider::set_active_provider(provider.clone()); + let sidecar = crate::sidecar::Sidecar::for_session(&app.session); + assert_eq!( + sidecar.model_name(), + saved, + "sidecar must resolve the routed memory override through the provider backend" + ); + }); +} + +/// Build a remote test app with a big names-only catalog and open the model +/// picker, returning the app with the async picker load pending. +/// +/// A sibling test that does not hold the env lock can persist its own catalog +/// into this temp home while it is live, and a hydrated catalog opens the +/// picker synchronously instead of leaving the async load pending. Clear the +/// stray cache file before opening, and rebuild once if a write still +/// sneaked between the guard and the open. +fn open_app_with_pending_big_remote_catalog() -> App { + let build = || { + let mut app = create_test_app(); + app.is_remote = true; + // >64 remote entries with no hydrated routes take the async catalog + // path: the picker opens with lightweight routes and a pending + // background refresh. + app.remote_available_entries = (0..80).map(|i| format!("gpt-test-model-{}", i)).collect(); + if let Ok(config_dir) = crate::storage::app_config_dir() { + let _ = std::fs::remove_file(config_dir.join("remote_model_catalog_cache.json")); + } + app.open_model_picker(); + app + }; + let app = build(); + if app.pending_model_picker_load.is_none() { + build() + } else { + app + } +} + +#[test] +fn test_agent_model_picker_late_catalog_refresh_keeps_agent_entries() { + with_temp_jcode_home(|| { + let mut app = open_app_with_pending_big_remote_catalog(); + assert!( + app.pending_model_picker_load.is_some(), + "large remote catalog should leave a pending background refresh" + ); + + // Alt+S opens the swarm sub-picker. If the refresh completes within + // the two-second wait window the open path retags inline; if it is + // still in flight the pending load carries the agent target so the + // poll completion path retags instead. Simulate the in-flight case: + // tag the still-pending load, then let the refresh land. + app.handle_key(KeyCode::Char('s'), KeyModifiers::ALT) + .unwrap(); + if let Some(pending) = app.pending_model_picker_load.as_mut() { + pending.agent_target = Some(crate::tui::AgentModelTarget::Swarm); + } + + let mut waited = 0; + while app.pending_model_picker_load.is_some() && waited < 500 { + app.poll_model_picker_load(); + std::thread::sleep(std::time::Duration::from_millis(5)); + waited += 5; + } + + let picker = app + .inline_interactive_state + .as_ref() + .expect("picker should still be open after the refresh"); + assert!( + picker.entries.iter().any(|e| matches!( + e.action, + crate::tui::PickerAction::AgentModelChoice { + target: crate::tui::AgentModelTarget::Swarm, + .. + } + )), + "late catalog refresh must keep AgentModelChoice entries, got {:?}", + picker + .entries + .iter() + .map(|e| e.action.clone()) + .collect::>() + ); + }); +} + +#[test] +fn test_agent_model_picker_late_refresh_keeps_inherit_override_search_and_selection() { + with_temp_jcode_home(|| { + // A saved swarm override that the refreshed catalog will NOT contain: + // the retag path must keep a row for it or the override can never be + // cleared from the picker. The picker opens in session scope by + // default, so seed the session override, not the global config. + + let mut app = open_app_with_pending_big_remote_catalog(); + assert!( + app.pending_model_picker_load.is_some(), + "large remote catalog should leave a pending background refresh" + ); + app.session + .set_agent_model_override("swarm", Some("claude-legacy-model".to_string())) + .unwrap(); + + app.handle_key(KeyCode::Char('s'), KeyModifiers::ALT) + .unwrap(); + if let Some(pending) = app.pending_model_picker_load.as_mut() { + pending.agent_target = Some(crate::tui::AgentModelTarget::Swarm); + } + + // User narrows the picker with a search filter and moves down to a + // model row while the catalog refresh is still in flight. + for ch in "gpt-test-model-7".chars() { + app.handle_key(KeyCode::Char(ch), KeyModifiers::NONE) + .unwrap(); + } + app.handle_key(KeyCode::Down, KeyModifiers::NONE).unwrap(); + let filter_before = { + let picker = app.inline_interactive_state.as_ref().unwrap(); + assert!( + !picker.filter.is_empty(), + "search text should be active before the refresh lands" + ); + picker.filter.clone() + }; + let selected_before = { + let picker = app.inline_interactive_state.as_ref().unwrap(); + picker.entries[picker.filtered[picker.selected]] + .name + .clone() + }; + + // Let the late catalog refresh land while the search is active. + let mut waited = 0; + while app.pending_model_picker_load.is_some() && waited < 500 { + app.poll_model_picker_load(); + std::thread::sleep(std::time::Duration::from_millis(5)); + waited += 5; + } + assert!( + app.pending_model_picker_load.is_none(), + "refresh should have landed within the wait window" + ); + + let picker = app + .inline_interactive_state + .as_ref() + .expect("picker should still be open after the refresh"); + // The inherit (clear-override) row must survive the retag. + assert!( + picker.entries.iter().any(|e| matches!( + e.action, + crate::tui::PickerAction::AgentModelChoice { + target: crate::tui::AgentModelTarget::Swarm, + clear_override: true, + } + )), + "late refresh must keep the inherit (clear override) row" + ); + // The saved override missing from the catalog must keep its row. + assert!( + picker + .entries + .iter() + .any(|e| e.name == "claude-legacy-model"), + "late refresh must keep a row for the saved override" + ); + // The user's search filter must still be applied, not silently reset. + assert_eq!( + picker.filter, filter_before, + "late refresh must keep the active search filter" + ); + // And the entry they had selected must still be the selected row, so + // Enter saves the model they actually chose. + let selected_after = picker.entries[picker.filtered[picker.selected]] + .name + .clone(); + assert_eq!( + selected_after, selected_before, + "late refresh must keep the user's selected entry" + ); + }); +} + +#[test] +fn test_model_picker_agent_chords_do_not_fire_when_not_in_model_picker() { + let mut app = create_test_app(); + app.is_remote = true; + app.remote_available_entries = vec!["gpt-5.5".to_string()]; + app.remote_model_options = vec![crate::provider::ModelRoute { + model: "gpt-5.5".to_string(), + provider: "OpenAI".to_string(), + api_method: "openai".to_string(), + available: true, + detail: String::new(), + cheapness: None, + usage: None, + }]; + + // Don't open the model picker. Alt+S / Alt+P should be no-ops, and + // Ctrl+S must not open anything either: it stays the input-stash chord. + assert!(app.inline_interactive_state.is_none()); + + app.handle_key(KeyCode::Char('s'), KeyModifiers::ALT) + .unwrap(); + app.handle_key(KeyCode::Char('p'), KeyModifiers::ALT) + .unwrap(); + app.handle_key(KeyCode::Char('s'), KeyModifiers::CONTROL) + .unwrap(); + + // Still no picker should be open + assert!( + app.inline_interactive_state.is_none(), + "agent chords should not open anything when no picker is active" + ); +} + +#[test] +fn test_ctrl_s_with_no_picker_stashes_input() { + let mut app = create_test_app(); + app.set_input_for_test("draft message"); + assert!(app.inline_interactive_state.is_none()); + + // With no picker open, Ctrl+S is the input-stash chord: it must stash the + // draft and never open a picker or sub-picker. + app.handle_key(KeyCode::Char('s'), KeyModifiers::CONTROL) + .unwrap(); + + assert!( + app.inline_interactive_state.is_none(), + "Ctrl+S with no picker must not open the swarm sub-picker" + ); + assert_eq!(app.input(), ""); + assert!( + app.stashed_input.is_some(), + "Ctrl+S with no picker must stash the draft" + ); + + // A second press restores the draft. + app.handle_key(KeyCode::Char('s'), KeyModifiers::CONTROL) + .unwrap(); + assert_eq!(app.input(), "draft message"); + assert!(app.inline_interactive_state.is_none()); +} diff --git a/crates/jcode-tui/src/tui/app/tests/remote_startup_input_03/part_01.rs b/crates/jcode-tui/src/tui/app/tests/remote_startup_input_03/part_01.rs index cf9ae48f73..2c85571c6d 100644 --- a/crates/jcode-tui/src/tui/app/tests/remote_startup_input_03/part_01.rs +++ b/crates/jcode-tui/src/tui/app/tests/remote_startup_input_03/part_01.rs @@ -484,47 +484,51 @@ fn test_multiple_pastes() { fn test_restore_session_adds_reload_message() { use crate::session::Session; - let mut app = create_test_app(); - - // Create and save a session with a fake provider_session_id - let mut session = Session::create(None, None); - session.add_message( - Role::User, - vec![ContentBlock::Text { - text: "test message".to_string(), - cache_control: None, - }], - ); - session.provider_session_id = Some("fake-uuid".to_string()); - let session_id = session.id.clone(); - session.save().unwrap(); - - // Restore the session - app.restore_session(&session_id); - - // Should have the original message + reload success message in display - assert_eq!(app.display_messages().len(), 2); - assert_eq!(app.display_messages()[0].role, "user"); - assert_eq!(app.display_messages()[0].content, "test message"); - assert_eq!(app.display_messages()[1].role, "system"); - assert!( - app.display_messages()[1] - .content - .contains("Reload complete - continuing.") - ); + // The session file is written under whatever JCODE_HOME is current when + // `Session::save` resolves the path, but `restore_session` re-resolves it + // on read. A concurrent with_temp_jcode_home test can rotate the home + // between the two, so the reload then finds an empty session and the + // display ends up with zero messages. + with_temp_jcode_home(|| { + let mut app = create_test_app(); - // Local restore keeps provider messages lazy until the next active turn. - assert_eq!(app.messages.len(), 0); - assert_eq!( - app.session.debug_memory_profile()["provider_messages_cache"]["count"], - 0 - ); + // Create and save a session with a fake provider_session_id + let mut session = Session::create(None, None); + session.add_message( + Role::User, + vec![ContentBlock::Text { + text: "test message".to_string(), + cache_control: None, + }], + ); + session.provider_session_id = Some("fake-uuid".to_string()); + let session_id = session.id.clone(); + session.save().unwrap(); + + // Restore the session + app.restore_session(&session_id); + + // Should have the original message + reload success message in display + assert_eq!(app.display_messages().len(), 2); + assert_eq!(app.display_messages()[0].role, "user"); + assert_eq!(app.display_messages()[0].content, "test message"); + assert_eq!(app.display_messages()[1].role, "system"); + assert!( + app.display_messages()[1] + .content + .contains("Reload complete - continuing.") + ); - // Provider session ID should be cleared (Claude sessions don't persist across restarts) - assert!(app.provider_session_id.is_none()); + // Local restore keeps provider messages lazy until the next active turn. + assert_eq!(app.messages.len(), 0); + assert_eq!( + app.session.debug_memory_profile()["provider_messages_cache"]["count"], + 0 + ); - // Clean up - let _ = std::fs::remove_file(crate::session::session_path(&session_id).unwrap()); + // Provider session ID should be cleared (Claude sessions don't persist across restarts) + assert!(app.provider_session_id.is_none()); + }); } #[test] @@ -1264,6 +1268,10 @@ fn test_new_for_remote_restores_split_view_from_reload_state() { #[test] fn test_restore_reload_state_supports_legacy_input_format() { + // Reads and writes jcode_dir(), which follows JCODE_HOME: without the + // env lock a concurrent with_temp_jcode_home test can rotate the home + // between this test's write and read, failing the restore. + let _env_guard = crate::storage::lock_test_env(); let session_id = format!("test-reload-legacy-{}", std::process::id()); let jcode_dir = crate::storage::jcode_dir().unwrap(); let path = jcode_dir.join(format!("client-input-{}", session_id)); diff --git a/crates/jcode-tui/src/tui/app/tests/scroll_copy_02/part_02.rs b/crates/jcode-tui/src/tui/app/tests/scroll_copy_02/part_02.rs index 673efacce2..16019dc9be 100644 --- a/crates/jcode-tui/src/tui/app/tests/scroll_copy_02/part_02.rs +++ b/crates/jcode-tui/src/tui/app/tests/scroll_copy_02/part_02.rs @@ -1360,8 +1360,7 @@ fn test_real_draw_click_on_body_anchored_image_label_cycles_level() { if !line.contains("shot.png") || !line.contains("show image") { continue; } - // Click the first cell of the label text (the hit-region is the whole - // label line, so any cell on the row works). + // Hit-region is the whole label line; any cell on the row works. for col in 0..area.width { if buf[(col, row)].symbol() == "s" { badge = Some((col, row)); @@ -1377,10 +1376,10 @@ fn test_real_draw_click_on_body_anchored_image_label_cycles_level() { "image should start at Fit before any click" ); - // REAL click on the rendered label cell. A terminal delivers a *pair* of - // events for one physical click: `Down` then `Up`. We must replay both, just - // like the live event loop, or we silently skip the copy-selection state the - // `Down` arms (which is exactly what the user's click goes through). + assert!(crate::tui::ui::inline_image_ui::materialize_visible(image_id)); + + // REAL click: a terminal delivers Down then Up; replay both or the armed + // copy-selection state is silently skipped. app.handle_mouse_event(MouseEvent { kind: MouseEventKind::Down(MouseButton::Left), column: badge_col, diff --git a/crates/jcode-tui/src/tui/app/tests/state_model_poke_02/part_02.rs b/crates/jcode-tui/src/tui/app/tests/state_model_poke_02/part_02.rs index ca46a7fade..1ffa130663 100644 --- a/crates/jcode-tui/src/tui/app/tests/state_model_poke_02/part_02.rs +++ b/crates/jcode-tui/src/tui/app/tests/state_model_poke_02/part_02.rs @@ -376,24 +376,31 @@ fn test_model_command_provider_suggestions_rank_matching_provider_prefix() { #[test] fn test_model_command_provider_suggestions_normalize_bare_openai_model_to_openrouter_catalog_id() { - let (app, _set_model_calls) = create_openrouter_spec_capture_test_app(); + // Route catalog reads the ambient named-profile env like the picker + // does; keep the window free of it. + with_temp_jcode_home(|| { + let (app, _set_model_calls) = create_openrouter_spec_capture_test_app(); - let suggestions = app.get_suggestions_for("/model gpt-5.4@op"); - assert_eq!( - suggestions.first().map(|(cmd, _)| cmd.as_str()), - Some("/model openai/gpt-5.4@OpenAI") - ); + let suggestions = app.get_suggestions_for("/model gpt-5.4@op"); + assert_eq!( + suggestions.first().map(|(cmd, _)| cmd.as_str()), + Some("/model openai/gpt-5.4@OpenAI") + ); + }); } #[test] fn test_model_command_provider_suggestions_include_auto_for_normalized_bare_openai_model() { - let (app, _set_model_calls) = create_openrouter_spec_capture_test_app(); + // Same ambient-profile hazard as the normalize test above. + with_temp_jcode_home(|| { + let (app, _set_model_calls) = create_openrouter_spec_capture_test_app(); - let suggestions = app.get_suggestions_for("/model gpt-5.4@"); - let commands: Vec<&str> = suggestions.iter().map(|(cmd, _)| cmd.as_str()).collect(); + let suggestions = app.get_suggestions_for("/model gpt-5.4@"); + let commands: Vec<&str> = suggestions.iter().map(|(cmd, _)| cmd.as_str()).collect(); - assert!(commands.contains(&"/model openai/gpt-5.4@auto")); - assert!(commands.contains(&"/model openai/gpt-5.4@OpenAI")); + assert!(commands.contains(&"/model openai/gpt-5.4@auto")); + assert!(commands.contains(&"/model openai/gpt-5.4@OpenAI")); + }); } #[test] @@ -506,3 +513,96 @@ fn test_model_picker_preview_enter_selects_model() { assert!(app.input().is_empty()); assert_eq!(app.cursor_pos(), 0); } + +/// Regression (greptile P1, PR 1706): with a global swarm model configured, +/// a session-scoped user must still be able to pick "inherit coordinator" +/// (bypass the global default). The row must exist and save the explicit +/// "inherit" sentinel both when the picker opens fresh and when a late +/// catalog load rebuilds it, whether or not a session override is saved. +#[test] +fn test_session_scope_keeps_inherit_coordinator_row_with_global_model_set() { + with_temp_jcode_home(|| { + // Global default is a real model, so "inherit coordinator" differs + // from "use global default". + let mut cfg = crate::config::Config::load(); + cfg.agents.swarm_model = Some("gpt-5.2-codex".to_string()); + cfg.save().expect("save global swarm model"); + + let mut app = create_test_app(); + configure_test_remote_models(&mut app); + assert!(!app.agent_models_global_scope); + + // Case 1: no session override. The inherit row must exist and save + // the sentinel, not clear to the global default. + app.open_agent_model_picker(crate::tui::AgentModelTarget::Swarm); + app.is_remote = false; + let picker = app + .inline_interactive_state + .as_mut() + .expect("swarm picker should be open"); + picker.selected = picker + .filtered + .iter() + .position(|idx| picker.entries[*idx].name == "inherit coordinator") + .expect("session picker must offer inherit coordinator with a global model set"); + app.handle_inline_interactive_key(KeyCode::Enter, KeyModifiers::NONE) + .unwrap(); + assert_eq!( + app.session + .agent_model_overrides + .get("swarm") + .map(String::as_str), + Some("inherit"), + "picking inherit coordinator must save the explicit sentinel, not clear to global" + ); + assert_eq!( + crate::config::Config::load().agents.swarm_model.as_deref(), + Some("gpt-5.2-codex"), + "the global default must stay untouched" + ); + + // Case 2: a saved override that is still in the catalog. The inherit + // row must survive next to it. + app.is_remote = true; + app.open_agent_model_picker(crate::tui::AgentModelTarget::Swarm); + app.is_remote = false; + let picker = app.inline_interactive_state.as_mut().unwrap(); + assert!( + picker.entries.iter().any(|entry| entry.name == "inherit coordinator"), + "inherit coordinator must stay with a catalog-visible saved override" + ); + picker.selected = picker + .filtered + .iter() + .position(|idx| picker.entries[*idx].name == "inherit coordinator") + .unwrap(); + app.handle_inline_interactive_key(KeyCode::Enter, KeyModifiers::NONE) + .unwrap(); + assert_eq!( + app.session + .agent_model_overrides + .get("swarm") + .map(String::as_str), + Some("inherit") + ); + + // Case 3: the retag path after a catalog rebuild must keep the same + // configured view (session overrides, not the global default). + app.is_remote = true; + app.open_agent_model_picker(crate::tui::AgentModelTarget::Swarm); + app.is_remote = false; + app.retag_picker_for_agent_model_target(crate::tui::AgentModelTarget::Swarm); + let picker = app.inline_interactive_state.as_ref().unwrap(); + assert!( + picker.entries.iter().any(|entry| entry.name == "inherit coordinator"), + "retag must keep the inherit coordinator row" + ); + assert!( + picker + .entries + .iter() + .any(|entry| entry.name == "inherit" && entry.is_current), + "retag must mark the saved session sentinel as current, not the global model" + ); + }); +} diff --git a/crates/jcode-tui/src/tui/app/tests/state_model_poke_03.rs b/crates/jcode-tui/src/tui/app/tests/state_model_poke_03.rs index 2247bd053d..9078d959ab 100644 --- a/crates/jcode-tui/src/tui/app/tests/state_model_poke_03.rs +++ b/crates/jcode-tui/src/tui/app/tests/state_model_poke_03.rs @@ -517,65 +517,70 @@ impl Provider for CountingModelRoutesProvider { #[test] fn test_subagent_model_large_catalog_uses_cached_searchable_picker() { - ensure_test_jcode_home_if_unset(); - clear_persisted_test_ui_state(); - crate::tui::ui::clear_test_render_state_for_tests(); - - let calls = StdArc::new(AtomicUsize::new(0)); - let provider: Arc = Arc::new(CountingModelRoutesProvider { - calls: StdArc::clone(&calls), - route_count: 400, - delay: Duration::from_millis(25), - }); - let rt = tokio::runtime::Runtime::new().unwrap(); - let registry = rt.block_on(crate::tool::Registry::new(provider.clone())); - let mut app = App::new_for_test_harness(provider, registry); - app.queue_mode = false; - app.diff_mode = crate::config::DiffDisplayMode::Inline; + // Ambient named-profile env would replace the counting provider's + // routes with the profile's (2 entries instead of 401). Hold the env + // lock so the provider stays the only route source. + with_temp_jcode_home(|| { + ensure_test_jcode_home_if_unset(); + clear_persisted_test_ui_state(); + crate::tui::ui::clear_test_render_state_for_tests(); + + let calls = StdArc::new(AtomicUsize::new(0)); + let provider: Arc = Arc::new(CountingModelRoutesProvider { + calls: StdArc::clone(&calls), + route_count: 400, + delay: Duration::from_millis(25), + }); + let rt = tokio::runtime::Runtime::new().unwrap(); + let registry = rt.block_on(crate::tool::Registry::new(provider.clone())); + let mut app = App::new_for_test_harness(provider, registry); + app.queue_mode = false; + app.diff_mode = crate::config::DiffDisplayMode::Inline; + + for c in "/subagent-model".chars() { + app.handle_key(KeyCode::Char(c), KeyModifiers::empty()) + .unwrap(); + } + wait_for_model_picker_load(&mut app); - for c in "/subagent-model".chars() { - app.handle_key(KeyCode::Char(c), KeyModifiers::empty()) - .unwrap(); - } - wait_for_model_picker_load(&mut app); + let picker = app + .inline_interactive_state + .as_ref() + .expect("subagent model picker should be open"); + assert!(picker.preview); + assert_eq!(picker.entries.len(), 401, "400 models plus inherit"); + assert!(matches!( + picker.entries[0].action, + crate::tui::PickerAction::SubagentModelChoice { inherit: true } + )); + let calls_after_load = calls.load(Ordering::SeqCst); - let picker = app - .inline_interactive_state - .as_ref() - .expect("subagent model picker should be open"); - assert!(picker.preview); - assert_eq!(picker.entries.len(), 401, "400 models plus inherit"); - assert!(matches!( - picker.entries[0].action, - crate::tui::PickerAction::SubagentModelChoice { inherit: true } - )); - let calls_after_load = calls.load(Ordering::SeqCst); + for _ in 0..10 { + app.handle_key(KeyCode::Down, KeyModifiers::empty()) + .unwrap(); + } + assert_eq!( + calls.load(Ordering::SeqCst), + calls_after_load, + "navigation must not rebuild or re-query the model catalog" + ); - for _ in 0..10 { - app.handle_key(KeyCode::Down, KeyModifiers::empty()) + app.handle_key(KeyCode::Char('3'), KeyModifiers::empty()) .unwrap(); - } - assert_eq!( - calls.load(Ordering::SeqCst), - calls_after_load, - "navigation must not rebuild or re-query the model catalog" - ); - - app.handle_key(KeyCode::Char('3'), KeyModifiers::empty()) - .unwrap(); - let picker = app.inline_interactive_state.as_ref().unwrap(); - assert!( - !picker.filtered.is_empty(), - "typed input should filter models" - ); - assert!(picker.filtered.len() < picker.entries.len()); + let picker = app.inline_interactive_state.as_ref().unwrap(); + assert!( + !picker.filtered.is_empty(), + "typed input should filter models" + ); + assert!(picker.filtered.len() < picker.entries.len()); - app.handle_key(KeyCode::Enter, KeyModifiers::empty()) - .unwrap(); - app.handle_key(KeyCode::Enter, KeyModifiers::empty()) - .unwrap(); - assert!(app.session.subagent_model.is_some()); - assert_eq!(app.provider.model(), "counting-a"); + app.handle_key(KeyCode::Enter, KeyModifiers::empty()) + .unwrap(); + app.handle_key(KeyCode::Enter, KeyModifiers::empty()) + .unwrap(); + assert!(app.session.subagent_model.is_some()); + assert_eq!(app.provider.model(), "counting-a"); + }); } #[test] @@ -1142,6 +1147,7 @@ fn test_tui_openai_compatible_empty_catalog_does_not_switch_to_profile_default() while bus_rx.try_recv().is_ok() {} let _guard = rt.enter(); + let expected_session = app.session.id.clone(); app.start_openai_compatible_post_login_activation( "cerebras".to_string(), "Cerebras".to_string(), @@ -1157,7 +1163,8 @@ fn test_tui_openai_compatible_empty_catalog_does_not_switch_to_profile_default() panic!("empty local catalog must not publish final login failure: {login:?}") } Ok(Ok(crate::bus::BusEvent::UiActivity(activity))) - if activity.message.contains("Model Discovery Still Updating") => + if activity.session_id.as_deref() == Some(expected_session.as_str()) + && activity.message.contains("Model Discovery Still Updating") => { break activity; } @@ -1182,32 +1189,39 @@ fn test_tui_openai_compatible_empty_catalog_does_not_switch_to_profile_default() #[test] fn test_tui_openai_compatible_local_refresh_failure_is_pending_not_final_failure() { - ensure_test_jcode_home_if_unset(); - clear_persisted_test_ui_state(); - crate::tui::ui::clear_test_render_state_for_tests(); - - let refreshes = StdArc::new(AtomicUsize::new(0)); - let set_model_attempts = StdArc::new(AtomicUsize::new(0)); - let provider: Arc = Arc::new(FailingPostLoginCatalogProvider { - refreshes: StdArc::clone(&refreshes), - set_model_attempts: StdArc::clone(&set_model_attempts), - }); - let rt = tokio::runtime::Runtime::new().unwrap(); - let registry = rt.block_on(crate::tool::Registry::new(provider.clone())); - let mut app = App::new_for_test_harness(provider, registry); - app.queue_mode = false; - app.diff_mode = crate::config::DiffDisplayMode::Inline; + // Same ambient-profile hazard as the other post-login activation tests: + // the activation path reads config and profile env while the async + // refresh runs, and a sibling test flipping that state mid-flight used + // to leak into this one through the process-global bus. Pin a private + // home under the env lock. + with_temp_jcode_home(|| { + ensure_test_jcode_home_if_unset(); + clear_persisted_test_ui_state(); + crate::tui::ui::clear_test_render_state_for_tests(); + + let refreshes = StdArc::new(AtomicUsize::new(0)); + let set_model_attempts = StdArc::new(AtomicUsize::new(0)); + let provider: Arc = Arc::new(FailingPostLoginCatalogProvider { + refreshes: StdArc::clone(&refreshes), + set_model_attempts: StdArc::clone(&set_model_attempts), + }); + let rt = tokio::runtime::Runtime::new().unwrap(); + let registry = rt.block_on(crate::tool::Registry::new(provider.clone())); + let mut app = App::new_for_test_harness(provider, registry); + app.queue_mode = false; + app.diff_mode = crate::config::DiffDisplayMode::Inline; - let mut bus_rx = crate::bus::Bus::global().subscribe(); - while bus_rx.try_recv().is_ok() {} + let mut bus_rx = crate::bus::Bus::global().subscribe(); + while bus_rx.try_recv().is_ok() {} - let _guard = rt.enter(); - app.start_openai_compatible_post_login_activation( - "cerebras".to_string(), - "Cerebras".to_string(), - ); + let _guard = rt.enter(); + let expected_session = app.session.id.clone(); + app.start_openai_compatible_post_login_activation( + "cerebras".to_string(), + "Cerebras".to_string(), + ); - let activity = rt.block_on(async { + let activity = rt.block_on(async { loop { match tokio::time::timeout(Duration::from_secs(2), bus_rx.recv()).await { Ok(Ok(crate::bus::BusEvent::ProviderModelActivated { .. })) => { @@ -1219,7 +1233,8 @@ fn test_tui_openai_compatible_local_refresh_failure_is_pending_not_final_failure ) } Ok(Ok(crate::bus::BusEvent::UiActivity(activity))) - if activity.message.contains("Model Discovery Still Updating") => + if activity.session_id.as_deref() == Some(expected_session.as_str()) + && activity.message.contains("Model Discovery Still Updating") => { break activity; } @@ -1229,64 +1244,70 @@ fn test_tui_openai_compatible_local_refresh_failure_is_pending_not_final_failure } }); - assert_eq!(refreshes.load(Ordering::SeqCst), 1); - assert_eq!( - set_model_attempts.load(Ordering::SeqCst), - 0, - "local refresh failure must not try to switch models from an unavailable catalog" - ); - assert!(activity.message.contains("Saved credentials are active")); - assert!( - activity - .message - .contains("server auth-change catalog refresh") - ); - assert!(activity.message.contains("fixture refresh failed")); - assert!(!activity.message.contains("Login: failed")); - assert!(!activity.message.contains("Unable to sign in")); - assert!(!activity.message.contains("did not switch models")); + assert_eq!(refreshes.load(Ordering::SeqCst), 1); + assert_eq!( + set_model_attempts.load(Ordering::SeqCst), + 0, + "local refresh failure must not try to switch models from an unavailable catalog" + ); + assert!(activity.message.contains("Saved credentials are active")); + assert!( + activity + .message + .contains("server auth-change catalog refresh") + ); + assert!(activity.message.contains("fixture refresh failed")); + assert!(!activity.message.contains("Login: failed")); + assert!(!activity.message.contains("Unable to sign in")); + assert!(!activity.message.contains("did not switch models")); + }) } #[test] fn test_model_picker_opens_simplified_state_before_async_routes_complete() { - ensure_test_jcode_home_if_unset(); - clear_persisted_test_ui_state(); - crate::tui::ui::clear_test_render_state_for_tests(); - - let calls = StdArc::new(AtomicUsize::new(0)); - let provider: Arc = Arc::new(CountingModelRoutesProvider { - calls: StdArc::clone(&calls), - route_count: 2, - delay: Duration::from_millis(75), - }); - let rt = tokio::runtime::Runtime::new().unwrap(); - let registry = rt.block_on(crate::tool::Registry::new(provider.clone())); - let mut app = App::new_for_test_harness(provider, registry); - app.queue_mode = false; - app.diff_mode = crate::config::DiffDisplayMode::Inline; + // Same ambient-profile hazard as the other counting-provider picker + // tests; hold the env lock so the async rebuild is never dropped for a + // mid-load signature change. + with_temp_jcode_home(|| { + ensure_test_jcode_home_if_unset(); + clear_persisted_test_ui_state(); + crate::tui::ui::clear_test_render_state_for_tests(); + + let calls = StdArc::new(AtomicUsize::new(0)); + let provider: Arc = Arc::new(CountingModelRoutesProvider { + calls: StdArc::clone(&calls), + route_count: 2, + delay: Duration::from_millis(75), + }); + let rt = tokio::runtime::Runtime::new().unwrap(); + let registry = rt.block_on(crate::tool::Registry::new(provider.clone())); + let mut app = App::new_for_test_harness(provider, registry); + app.queue_mode = false; + app.diff_mode = crate::config::DiffDisplayMode::Inline; - app.open_model_picker(); + app.open_model_picker(); - let picker = app - .inline_interactive_state - .as_ref() - .expect("loading picker should open immediately"); - assert_eq!(picker.entries.len(), 1); - assert_eq!(picker.entries[0].name, "counting-a"); - assert_eq!(picker.entries[0].options[0].detail, "simplified catalog"); - assert!(app.pending_model_picker_load.is_some()); - assert_eq!( - app.status_notice(), - Some("Updating model routes…".to_string()) - ); + let picker = app + .inline_interactive_state + .as_ref() + .expect("loading picker should open immediately"); + assert_eq!(picker.entries.len(), 1); + assert_eq!(picker.entries[0].name, "counting-a"); + assert_eq!(picker.entries[0].options[0].detail, "simplified catalog"); + assert!(app.pending_model_picker_load.is_some()); + assert_eq!( + app.status_notice(), + Some("Updating model routes…".to_string()) + ); - wait_for_model_picker_load(&mut app); - let picker = app - .inline_interactive_state - .as_ref() - .expect("hydrated picker should still be open"); - assert!(picker.entries.len() >= 2); - assert_eq!(app.status_notice(), Some("Model list updated".to_string())); + wait_for_model_picker_load(&mut app); + let picker = app + .inline_interactive_state + .as_ref() + .expect("hydrated picker should still be open"); + assert!(picker.entries.len() >= 2); + assert_eq!(app.status_notice(), Some("Model list updated".to_string())); + }); } #[test] @@ -1768,183 +1789,204 @@ fn test_azure_login_completion_switches_local_model_without_completion() { #[test] fn test_local_model_picker_surfaces_antigravity_models_from_multiprovider() { - let mut app = create_antigravity_picker_test_app(); - app.open_model_picker(); - wait_for_model_picker_load(&mut app); + // Local picker routes read the ambient named-profile env: with an active + // profile the multiprovider antigravity routes never surface. Other + // tests' env-guard drops can also restore that profile mid-window, so + // hold the env lock via with_temp_jcode_home, which clears the profile + // envs for the duration. + with_temp_jcode_home(|| { + let mut app = create_antigravity_picker_test_app(); + app.open_model_picker(); + wait_for_model_picker_load(&mut app); - let picker = app - .inline_interactive_state - .as_ref() - .expect("model picker should be open"); + let picker = app + .inline_interactive_state + .as_ref() + .expect("model picker should be open"); - let antigravity_entry = picker - .entries - .iter() - .find(|entry| entry.name == "claude-sonnet-4-6") - .expect("antigravity model should be shown after login"); + let antigravity_entry = picker + .entries + .iter() + .find(|entry| entry.name == "claude-sonnet-4-6") + .expect("antigravity model should be shown after login"); - assert!(antigravity_entry.options.iter().any(|route| { - route.provider == "Antigravity" && route.api_method == "cli" && route.available - })); + assert!(antigravity_entry.options.iter().any(|route| { + route.provider == "Antigravity" && route.api_method == "cli" && route.available + })); + }); } #[test] fn test_local_antigravity_model_picker_selection_preserves_antigravity_provider() { - let mut app = create_antigravity_picker_test_app(); - app.open_model_picker(); - wait_for_model_picker_load(&mut app); + // Same ambient-profile hazard as the surfaces test above; hold the env + // lock so the antigravity routes stay the picker's only source. + with_temp_jcode_home(|| { + let mut app = create_antigravity_picker_test_app(); + app.open_model_picker(); + wait_for_model_picker_load(&mut app); - let picker = app - .inline_interactive_state - .as_ref() - .expect("model picker should be open"); + let picker = app + .inline_interactive_state + .as_ref() + .expect("model picker should be open"); - let model_idx = picker - .entries - .iter() - .position(|entry| entry.name == "claude-sonnet-4-6") - .expect("antigravity model should be in picker"); - let filtered_pos = picker - .filtered - .iter() - .position(|&i| i == model_idx) - .expect("antigravity model should be in filtered list"); + let model_idx = picker + .entries + .iter() + .position(|entry| entry.name == "claude-sonnet-4-6") + .expect("antigravity model should be in picker"); + let filtered_pos = picker + .filtered + .iter() + .position(|&i| i == model_idx) + .expect("antigravity model should be in filtered list"); - app.inline_interactive_state.as_mut().unwrap().selected = filtered_pos; - app.handle_key(KeyCode::Enter, KeyModifiers::empty()) - .unwrap(); + app.inline_interactive_state.as_mut().unwrap().selected = filtered_pos; + app.handle_key(KeyCode::Enter, KeyModifiers::empty()) + .unwrap(); - assert_eq!(app.provider.name(), "Antigravity"); - assert_eq!(app.provider.model(), "claude-sonnet-4-6"); - assert!(app.inline_interactive_state.is_none()); + assert_eq!(app.provider.name(), "Antigravity"); + assert_eq!(app.provider.model(), "claude-sonnet-4-6"); + assert!(app.inline_interactive_state.is_none()); + }); } #[test] fn test_local_model_picker_openrouter_bare_openai_route_uses_openai_catalog_prefix() { - let (mut app, set_model_calls) = create_openrouter_spec_capture_test_app(); - app.open_model_picker(); - wait_for_model_picker_load(&mut app); + // Same ambient-profile hazard as the antigravity picker tests; hold the + // env lock so the openrouter-backed routes stay deterministic. + with_temp_jcode_home(|| { + let (mut app, set_model_calls) = create_openrouter_spec_capture_test_app(); + app.open_model_picker(); + wait_for_model_picker_load(&mut app); - let picker = app - .inline_interactive_state - .as_ref() - .expect("model picker should be open"); - let model_idx = picker - .entries - .iter() - .position(|entry| entry.name == "gpt-5.4 (high)") - .expect("openrouter-backed OpenAI effort entry should be in picker"); - let filtered_pos = picker - .filtered - .iter() - .position(|&i| i == model_idx) - .expect("entry should be in filtered list"); + let picker = app + .inline_interactive_state + .as_ref() + .expect("model picker should be open"); + let model_idx = picker + .entries + .iter() + .position(|entry| entry.name == "gpt-5.4 (high)") + .expect("openrouter-backed OpenAI effort entry should be in picker"); + let filtered_pos = picker + .filtered + .iter() + .position(|&i| i == model_idx) + .expect("entry should be in filtered list"); - app.inline_interactive_state.as_mut().unwrap().selected = filtered_pos; - app.handle_key(KeyCode::Enter, KeyModifiers::empty()) - .expect("model picker selection should succeed"); + app.inline_interactive_state.as_mut().unwrap().selected = filtered_pos; + app.handle_key(KeyCode::Enter, KeyModifiers::empty()) + .expect("model picker selection should succeed"); - assert_eq!( - set_model_calls.lock().unwrap().as_slice(), - ["openai/gpt-5.4@OpenAI"] - ); + assert_eq!( + set_model_calls.lock().unwrap().as_slice(), + ["openai/gpt-5.4@OpenAI"] + ); + }); } #[test] fn test_agent_model_picker_openrouter_bare_openai_route_saves_openai_catalog_prefix() { - let (mut app, _set_model_calls) = create_openrouter_spec_capture_test_app(); + // Same ambient-profile hazard; hold the env lock. + with_temp_jcode_home(|| { + let (mut app, _set_model_calls) = create_openrouter_spec_capture_test_app(); - app.open_agent_model_picker(crate::tui::AgentModelTarget::Swarm); + app.open_agent_model_picker(crate::tui::AgentModelTarget::Swarm); - let picker = app - .inline_interactive_state - .as_ref() - .expect("agent model picker should be open"); - let model_idx = picker - .entries - .iter() - .position(|entry| entry.name == "gpt-5.4 (high)") - .expect("openrouter-backed OpenAI effort entry should be in picker"); - let filtered_pos = picker - .filtered - .iter() - .position(|&i| i == model_idx) - .expect("entry should be in filtered list"); + let picker = app + .inline_interactive_state + .as_ref() + .expect("agent model picker should be open"); + let model_idx = picker + .entries + .iter() + .position(|entry| entry.name == "gpt-5.4 (high)") + .expect("openrouter-backed OpenAI effort entry should be in picker"); + let filtered_pos = picker + .filtered + .iter() + .position(|&i| i == model_idx) + .expect("entry should be in filtered list"); - app.inline_interactive_state.as_mut().unwrap().selected = filtered_pos; - app.handle_key(KeyCode::Enter, KeyModifiers::empty()) - .expect("agent model picker selection should succeed"); + app.inline_interactive_state.as_mut().unwrap().selected = filtered_pos; + app.handle_key(KeyCode::Enter, KeyModifiers::empty()) + .expect("agent model picker selection should succeed"); - let last = app.display_messages.last().expect("display message"); - assert_eq!(last.role, "system"); - assert!( - last.content.contains("openai/gpt-5.4@OpenAI"), - "message should show normalized saved spec, got: {}", - last.content - ); + let last = app.display_messages.last().expect("display message"); + assert_eq!(last.role, "system"); + assert!( + last.content.contains("openai/gpt-5.4@OpenAI"), + "message should show normalized saved spec, got: {}", + last.content + ); + }); } #[test] fn test_local_model_picker_render_shows_antigravity_models_exactly_as_user_sees_them() { - let mut app = create_antigravity_picker_test_app(); - app.display_messages = vec![DisplayMessage::system("seed render state")]; - app.bump_display_messages_version(); - app.open_model_picker(); - wait_for_model_picker_load(&mut app); - - let render_filtered = |app: &mut App, filter: &str| { - let picker = app - .inline_interactive_state - .as_mut() - .expect("model picker should be open"); - picker.filter = filter.to_string(); - App::apply_inline_interactive_filter(picker); - let _render_lock = scroll_render_test_lock(); - let backend = ratatui::backend::TestBackend::new(90, 14); - let mut terminal = ratatui::Terminal::new(backend).expect("failed to create test terminal"); - render_and_snap(app, &mut terminal) - }; - let claude_text = render_filtered(&mut app, "claude-sonnet-4-6"); - let gpt_text = render_filtered(&mut app, "gpt-oss-120b-medium"); + // Same ambient-profile hazard as the antigravity picker tests above. + with_temp_jcode_home(|| { + let mut app = create_antigravity_picker_test_app(); + app.display_messages = vec![DisplayMessage::system("seed render state")]; + app.bump_display_messages_version(); + app.open_model_picker(); + wait_for_model_picker_load(&mut app); + + let render_filtered = |app: &mut App, filter: &str| { + let picker = app + .inline_interactive_state + .as_mut() + .expect("model picker should be open"); + picker.filter = filter.to_string(); + App::apply_inline_interactive_filter(picker); + let _render_lock = scroll_render_test_lock(); + let backend = ratatui::backend::TestBackend::new(90, 14); + let mut terminal = ratatui::Terminal::new(backend).expect("failed to create test terminal"); + render_and_snap(app, &mut terminal) + }; + let claude_text = render_filtered(&mut app, "claude-sonnet-4-6"); + let gpt_text = render_filtered(&mut app, "gpt-oss-120b-medium"); - assert!( - claude_text.contains("▸ Claude Sonnet 4.6") && claude_text.contains("↑↓ choose"), - "rendered /model suggestions should show the selected row and navigation, got: + assert!( + claude_text.contains("▸ Claude Sonnet 4.6") && claude_text.contains("↑↓ choose"), + "rendered /model suggestions should show the selected row and navigation, got: {}", - claude_text - ); - assert!( - claude_text.contains("Claude Sonnet 4.6"), - "rendered /model view should show the Antigravity Claude row, got: + claude_text + ); + assert!( + claude_text.contains("Claude Sonnet 4.6"), + "rendered /model view should show the Antigravity Claude row, got: {}", - claude_text - ); - assert!( - gpt_text.contains("gpt-oss-120b-medium"), - "rendered /model view should show the Antigravity GPT row, got: + claude_text + ); + assert!( + gpt_text.contains("gpt-oss-120b-medium"), + "rendered /model view should show the Antigravity GPT row, got: {}", - gpt_text - ); - assert!( - claude_text.contains("Antigravity") && gpt_text.contains("Antigravity"), - "rendered /model view should show the Antigravity provider column, got: + gpt_text + ); + assert!( + claude_text.contains("Antigravity") && gpt_text.contains("Antigravity"), + "rendered /model view should show the Antigravity provider column, got: Claude: {} GPT: {}", - claude_text, - gpt_text - ); - assert!( - claude_text.contains("cli") && gpt_text.contains("cli"), - "rendered /model view should show the route transport column, got: + claude_text, + gpt_text + ); + assert!( + claude_text.contains("cli") && gpt_text.contains("cli"), + "rendered /model view should show the route transport column, got: Claude: {} GPT: {}", - claude_text, - gpt_text - ); + claude_text, + gpt_text + ); + }); } #[test] diff --git a/crates/jcode-tui/src/tui/app/tests/support_failover/part_01.rs b/crates/jcode-tui/src/tui/app/tests/support_failover/part_01.rs index ef33318032..4a5001b441 100644 --- a/crates/jcode-tui/src/tui/app/tests/support_failover/part_01.rs +++ b/crates/jcode-tui/src/tui/app/tests/support_failover/part_01.rs @@ -406,6 +406,19 @@ fn clear_persisted_test_ui_state() { let _ = std::fs::remove_file(ambient_dir.join("state.json")); let _ = std::fs::remove_file(ambient_dir.join("directives.json")); let _ = std::fs::remove_file(ambient_dir.join("visible_cycle.json")); + // A remote-mode test that handled AvailableModelsUpdated persisted + // its mock catalog (e.g. Copilot claude-opus-4.6) into the shared + // home. Any later test whose picker hydrates the cache (empty + // remote_model_options + is_remote) then preselects those ghost + // models instead of its own configured ones. The cache is ambient + // per-run state, so drop it alongside the other ambient files. No + // test hydrates across app instances from this home. + let _ = std::fs::remove_file( + shared_test_jcode_home() + .join("config") + .join("jcode") + .join("remote_model_catalog_cache.json"), + ); } crate::tui::app::helpers::clear_ambient_info_cache_for_tests(); crate::auth::AuthStatus::invalidate_cache(); @@ -444,8 +457,45 @@ impl Drop for EnvRestoreGuard { fn with_temp_jcode_home(f: impl FnOnce() -> T) -> T { let _guard = crate::storage::lock_test_env(); let temp = tempfile::tempdir().expect("tempdir"); - let _env_guard = EnvRestoreGuard::capture(["JCODE_HOME"]); + // The ambient named-profile / OpenRouter runtime env is process-global + // and other tests mutate it, so a verdict would depend on which tests ran + // before it and on the dev machine's shell. Guard and clear it here. + let _env_guard = EnvRestoreGuard::capture([ + "JCODE_HOME", + "JCODE_NAMED_PROVIDER_PROFILE", + "JCODE_PROVIDER_PROFILE_ACTIVE", + "JCODE_PROVIDER_PROFILE_NAME", + "JCODE_OPENROUTER_API_BASE", + "JCODE_OPENROUTER_API_KEY_NAME", + "JCODE_OPENROUTER_ENV_FILE", + "JCODE_OPENROUTER_CACHE_NAMESPACE", + "JCODE_OPENROUTER_PROVIDER_FEATURES", + "JCODE_OPENROUTER_TRANSPORT_STATE", + "JCODE_OPENROUTER_ALLOW_NO_AUTH", + "JCODE_OPENROUTER_PROVIDER", + "JCODE_OPENROUTER_NO_FALLBACK", + "JCODE_API_BASE", + "JCODE_API_KEY", + ]); crate::env::set_var("JCODE_HOME", temp.path()); + for key in [ + "JCODE_NAMED_PROVIDER_PROFILE", + "JCODE_PROVIDER_PROFILE_ACTIVE", + "JCODE_PROVIDER_PROFILE_NAME", + "JCODE_OPENROUTER_API_BASE", + "JCODE_OPENROUTER_API_KEY_NAME", + "JCODE_OPENROUTER_ENV_FILE", + "JCODE_OPENROUTER_CACHE_NAMESPACE", + "JCODE_OPENROUTER_PROVIDER_FEATURES", + "JCODE_OPENROUTER_TRANSPORT_STATE", + "JCODE_OPENROUTER_ALLOW_NO_AUTH", + "JCODE_OPENROUTER_PROVIDER", + "JCODE_OPENROUTER_NO_FALLBACK", + "JCODE_API_BASE", + "JCODE_API_KEY", + ] { + crate::env::remove_var(key); + } // Preserve inherited telemetry opt-out env. In this crate telemetry-core is // a dependency, so cfg(test) does not stub its HTTP delivery path; inherited // opt-out must keep blocking delivery for tests that exercise onboarding. diff --git a/crates/jcode-tui/src/tui/app/tests_input_scroll.rs b/crates/jcode-tui/src/tui/app/tests_input_scroll.rs index 4434c90c0c..0f6053b070 100644 --- a/crates/jcode-tui/src/tui/app/tests_input_scroll.rs +++ b/crates/jcode-tui/src/tui/app/tests_input_scroll.rs @@ -143,26 +143,42 @@ fn test_disconnected_key_handler_runs_effort_locally() { #[test] fn test_disconnected_key_handler_runs_model_picker_locally() { - let mut app = create_test_app(); - configure_test_remote_models(&mut app); - // OpenAI models are effort-expanded into one entry per reasoning effort, - // and the "current" entry only matches when the session's effort matches. - app.remote_reasoning_effort = Some("high".to_string()); - app.input = "/model".to_string(); - app.cursor_pos = app.input.len(); - - remote::handle_disconnected_key(&mut app, KeyCode::Enter, KeyModifiers::empty()).unwrap(); - - assert!(app.input.is_empty()); - assert!(app.queued_messages().is_empty()); - let picker = app - .inline_interactive_state - .as_ref() - .expect("model picker should open"); - assert!(!picker.entries.is_empty()); - let selected = &picker.entries[picker.selected]; - assert_eq!(selected.name, "gpt-5.3-codex (high)"); - assert!(selected.is_current, "current model should be preselected"); + // The current-model preselection reads the config/auth state, so a + // concurrent test's JCODE_HOME or ambient profile env window can swap the + // verdict (claude-opus-4.6 instead of the gpt-5.3-codex route). Isolate + // under the env lock with a temp home like the other picker tests. + with_temp_jcode_home(|| { + let mut app = create_test_app(); + configure_test_remote_models(&mut app); + // configure_test_remote_models leaves remote_model_options empty, so + // the picker hydrates the on-disk catalog cache. A lockless test's + // env-guard drop can repoint JCODE_HOME at the shared home mid-window, + // and the shared home may hold a catalog cache persisted by another + // remote-mode test (ghost models like Copilot claude-opus-4.6), which + // would replace the configured routes and break the preselection + // assert. Seed the placeholder routes explicitly (identical to what + // the picker builds from remote_available_entries) so hydration + // short-circuits and the test only ever sees its own configuration. + app.remote_model_options = app.build_remote_model_routes_fallback(); + // OpenAI models are effort-expanded into one entry per reasoning effort, + // and the "current" entry only matches when the session's effort matches. + app.remote_reasoning_effort = Some("high".to_string()); + app.input = "/model".to_string(); + app.cursor_pos = app.input.len(); + + remote::handle_disconnected_key(&mut app, KeyCode::Enter, KeyModifiers::empty()).unwrap(); + + assert!(app.input.is_empty()); + assert!(app.queued_messages().is_empty()); + let picker = app + .inline_interactive_state + .as_ref() + .expect("model picker should open"); + assert!(!picker.entries.is_empty()); + let selected = &picker.entries[picker.selected]; + assert_eq!(selected.name, "gpt-5.3-codex (high)"); + assert!(selected.is_current, "current model should be preselected"); + }); } #[test] diff --git a/crates/jcode-tui/src/tui/ui_header.rs b/crates/jcode-tui/src/tui/ui_header.rs index 56caf3dbf6..d01deb099e 100644 --- a/crates/jcode-tui/src/tui/ui_header.rs +++ b/crates/jcode-tui/src/tui/ui_header.rs @@ -90,6 +90,15 @@ pub(super) fn account_router_generation() -> u64 { account_router::generation() } +/// Pin the header account inventory (background-loaded from the ambient +/// `JCODE_HOME`) empty for render tests that compare frames against each +/// other. Callers must hold the shared render-state test lock before creating +/// the guard. +#[cfg(test)] +pub(crate) fn scoped_empty_inventory_for_tests() -> account_router::InventoryOverrideGuard { + account_router::scoped_empty_inventory_for_tests() +} + pub(crate) fn capitalize(s: &str) -> String { let mut chars = s.chars(); match chars.next() { diff --git a/crates/jcode-tui/src/tui/ui_header/account_router.rs b/crates/jcode-tui/src/tui/ui_header/account_router.rs index 3690d3e261..1d4d609479 100644 --- a/crates/jcode-tui/src/tui/ui_header/account_router.rs +++ b/crates/jcode-tui/src/tui/ui_header/account_router.rs @@ -104,6 +104,10 @@ pub(super) fn lines( provider_name: &str, selected_label: Option<&str>, ) -> Vec> { + #[cfg(test)] + if let Some(inventory) = test_inventory_override() { + return inventory_lines(auth, active, &inventory, provider_name, selected_label); + } inventory_lines( auth, active, @@ -167,6 +171,53 @@ fn remote_inventory_lines( lines } +/// Render tests compare frames against each other, so the background inventory +/// refresh (which reads whatever `JCODE_HOME` is ambient and flips the account +/// rows between frames) must be pinned. This override freezes the inventory for +/// the guard's lifetime. Callers must hold the shared render-state test lock +/// before creating the guard. +#[cfg(test)] +fn test_inventory_override() -> Option { + TEST_INVENTORY_OVERRIDE + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .clone() +} + +#[cfg(test)] +static TEST_INVENTORY_OVERRIDE: Mutex> = Mutex::new(None); + +/// Pin the header account inventory to the given value until the returned +/// guard drops. +#[cfg(test)] +fn scoped_inventory_override_for_tests(inventory: Option) -> InventoryOverrideGuard { + *TEST_INVENTORY_OVERRIDE + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) = inventory; + InventoryOverrideGuard +} + +/// Pin the header account inventory empty (no account rows) until the +/// returned guard drops. Render tests that compare frames against each other +/// use this so the background inventory refresh (which reads whatever +/// `JCODE_HOME` is ambient) cannot flip account rows between frames. +#[cfg(test)] +pub(crate) fn scoped_empty_inventory_for_tests() -> InventoryOverrideGuard { + scoped_inventory_override_for_tests(Some(Inventory::default())) +} + +#[cfg(test)] +pub(crate) struct InventoryOverrideGuard; + +#[cfg(test)] +impl Drop for InventoryOverrideGuard { + fn drop(&mut self) { + *TEST_INVENTORY_OVERRIDE + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) = None; + } +} + fn inventory_lines( auth: &AuthStatus, active: ActiveCredentialOverrides, diff --git a/crates/jcode-tui/src/tui/ui_inline_interactive.rs b/crates/jcode-tui/src/tui/ui_inline_interactive.rs index 174860822f..5febd34cb8 100644 --- a/crates/jcode-tui/src/tui/ui_inline_interactive.rs +++ b/crates/jcode-tui/src/tui/ui_inline_interactive.rs @@ -237,7 +237,7 @@ fn model_picker_top_hint(picker: &crate::tui::InlineInteractiveState) -> Option< .any(|entry| matches!(entry.action, crate::tui::PickerAction::Model)); if is_runtime_model_picker { Some( - " keys: Ctrl+O set default · Ctrl+N favorite · Shift+Tab switch active model to next favorite", + " keys: Ctrl+O set default · Ctrl+N favorite · Alt+S swarm model · Alt+P memory model · Shift+Tab switch active model to next favorite", ) } else { None diff --git a/crates/jcode-tui/src/tui/ui_inline_interactive_tests.rs b/crates/jcode-tui/src/tui/ui_inline_interactive_tests.rs index abde252f64..803d659645 100644 --- a/crates/jcode-tui/src/tui/ui_inline_interactive_tests.rs +++ b/crates/jcode-tui/src/tui/ui_inline_interactive_tests.rs @@ -418,6 +418,8 @@ fn model_picker_keybind_hint_mentions_default_and_favorites() { assert!(hint.contains("Ctrl+O set default")); assert!(hint.contains("Ctrl+N favorite")); + assert!(hint.contains("Alt+S swarm model")); + assert!(hint.contains("Alt+P memory model")); } #[test] diff --git a/crates/jcode-tui/src/tui/ui_messages/tests.rs b/crates/jcode-tui/src/tui/ui_messages/tests.rs index 9150fb99bd..e7ec8878ca 100644 --- a/crates/jcode-tui/src/tui/ui_messages/tests.rs +++ b/crates/jcode-tui/src/tui/ui_messages/tests.rs @@ -2429,7 +2429,10 @@ fn render_background_task_messages_prefer_display_name() { #[test] fn render_system_message_uses_scheduled_task_card() { - let _guard = system_glyph_env_lock(); + // The title glyph depends on TERM/TERM_PROGRAM, and the kitty-variant + // test below flips both. Hold the shared glyph-env lock so the render + // and the assertion cannot observe different glyph modes. + let _glyph_guard = system_glyph_env_lock(); let msg = DisplayMessage::system( "[Scheduled task]\nA scheduled task for this session is now due.\n\nTask: Follow up on the scheduler test\nWorking directory: /home/jeremy/jcode\nRelevant files: src/tui/ui_messages.rs\nBranch: master\n\nBackground: Verify the scheduled task card styling\nSuccess criteria: The due task renders clearly\nScheduled by session: session_test", ); @@ -2454,7 +2457,9 @@ fn render_system_message_uses_scheduled_task_card() { #[test] fn render_tool_message_uses_scheduled_card() { - let _guard = system_glyph_env_lock(); + // Same glyph-env race as the system scheduled-task card test; hold the + // shared lock. + let _glyph_guard = system_glyph_env_lock(); let msg = DisplayMessage { role: "tool".to_string(), content: "Scheduled task 'Follow up on the scheduler test' for in 1m (id: sched_abc123)\nWorking directory: /home/jeremy/jcode\nRelevant files: src/tui/ui_messages.rs\nTarget: resume session session_test".to_string(), diff --git a/crates/jcode-tui/src/tui/ui_overlays.rs b/crates/jcode-tui/src/tui/ui_overlays.rs index 1cb717252b..0c31862baf 100644 --- a/crates/jcode-tui/src/tui/ui_overlays.rs +++ b/crates/jcode-tui/src/tui/ui_overlays.rs @@ -617,6 +617,8 @@ pub(super) fn draw_help_overlay(frame: &mut Frame, area: Rect, scroll: usize, ap "Ctrl+N", "Toggle favorite model (in /model picker)", )); + lines.push(key_entry("Alt+S", "Select swarm model (in /model picker)")); + lines.push(key_entry("Alt+P", "Select memory model (in /model picker)")); lines.push(Line::from("")); lines.push(separator()); diff --git a/crates/jcode-tui/src/tui/ui_tests/basic.rs b/crates/jcode-tui/src/tui/ui_tests/basic.rs index e2a4fea4e4..4add078dc9 100644 --- a/crates/jcode-tui/src/tui/ui_tests/basic.rs +++ b/crates/jcode-tui/src/tui/ui_tests/basic.rs @@ -15,3 +15,4 @@ include!("basic/image_regions.rs"); include!("basic/input_layout.rs"); include!("basic/idle_animation_partial.rs"); include!("basic/redraw_cadence.rs"); +include!("basic/help_overlay.rs"); diff --git a/crates/jcode-tui/src/tui/ui_tests/basic/help_overlay.rs b/crates/jcode-tui/src/tui/ui_tests/basic/help_overlay.rs new file mode 100644 index 0000000000..c9104a16f5 --- /dev/null +++ b/crates/jcode-tui/src/tui/ui_tests/basic/help_overlay.rs @@ -0,0 +1,37 @@ +/// The /model picker's agent chords must stay advertised in the help overlay: +/// Alt+S (swarm) and Alt+P (memory) replaced the Ctrl+S swarm shortcut that +/// collided with the global input-stash chord. Alt+M stays free for picker +/// features other branches add, so it is intentionally absent here. +#[test] +fn help_overlay_lists_agent_model_chords() { + let _lock = viewport_snapshot_test_lock(); + + let state = TestState { + help_scroll: Some(0), + ..Default::default() + }; + let mut terminal = Terminal::new(TestBackend::new(80, 200)).expect("test terminal"); + terminal + .draw(|frame| crate::tui::ui::draw(frame, &state)) + .expect("help overlay frame"); + + let buffer = terminal.backend().buffer(); + let text: String = (0..buffer.area.height) + .map(|y| { + (0..buffer.area.width) + .map(|x| buffer[(x, y)].symbol()) + .collect::() + }) + .collect::>() + .join("\n"); + assert!(text.contains("Alt+S"), "swarm chord missing from help:\n{text}"); + assert!(text.contains("Alt+P"), "memory chord missing from help:\n{text}"); + assert!( + text.contains("Select swarm model"), + "swarm chord description missing from help:\n{text}" + ); + assert!( + text.contains("Select memory model"), + "memory chord description missing from help:\n{text}" + ); +} diff --git a/crates/jcode-tui/src/tui/ui_tests/basic/idle_animation_partial.rs b/crates/jcode-tui/src/tui/ui_tests/basic/idle_animation_partial.rs index a46beae17a..c729ef8fb6 100644 --- a/crates/jcode-tui/src/tui/ui_tests/basic/idle_animation_partial.rs +++ b/crates/jcode-tui/src/tui/ui_tests/basic/idle_animation_partial.rs @@ -463,6 +463,7 @@ fn copying_only_the_animated_rows_matches_cloning_the_whole_frame() { use ratatui::layout::Rect; let _lock = viewport_snapshot_test_lock(); + pin_full_tier(); clear_flicker_frame_history_for_tests(); for (width, height) in [(160u16, 48u16), (100, 40), (80, 24)] { diff --git a/crates/jcode-tui/src/tui/ui_tests/inline_picker.rs b/crates/jcode-tui/src/tui/ui_tests/inline_picker.rs index 0ebe87e004..2e2ab1d20e 100644 --- a/crates/jcode-tui/src/tui/ui_tests/inline_picker.rs +++ b/crates/jcode-tui/src/tui/ui_tests/inline_picker.rs @@ -192,7 +192,35 @@ fn non_model_pickers_still_reserve_inline_panel_height() { #[test] fn model_suggestions_full_frame_keeps_composer_position() { + // The composer row is layout-driven, and this test compares two frames + // against each other, so sibling pollution shifts it between renders: + // 1. JCODE_SSH_REMOTE flips the header between SSH mode (one row) and + // local account-router mode (two rows). The ssh_inline_picker tests + // set it process-wide, and without the env lock their window can land + // between this test's two renders. + // 2. The header account inventory is loaded in the background from the + // ambient JCODE_HOME, so real machine credentials can appear (or + // not) between the two renders. + // 3. Leftover render state (flicker history, scrollbar hysteresis) + // changes the notification row between renders. + // Hold the env lock before the render lock (env-then-render order) and run + // in local mode, pin the inventory empty, and reset the shared render + // state so both frames below compare the same world. + let _env = crate::storage::lock_test_env(); + struct RestoreSshRemote(Option); + impl Drop for RestoreSshRemote { + fn drop(&mut self) { + if let Some(host) = self.0.take() { + crate::env::set_var("JCODE_SSH_REMOTE", host); + } + } + } + let previous_ssh = std::env::var_os("JCODE_SSH_REMOTE"); + crate::env::remove_var("JCODE_SSH_REMOTE"); + let _restore_ssh = RestoreSshRemote(previous_ssh); let _lock = viewport_snapshot_test_lock(); + let _inventory = crate::tui::ui::header::scoped_empty_inventory_for_tests(); + crate::tui::ui::clear_test_render_state_for_tests(); let mut state = model_picker_state(); state.input = "/model ".to_string(); state.display_messages = vec![DisplayMessage::user("hello".to_string())]; diff --git a/scripts/code_size_budget.json b/scripts/code_size_budget.json index a1df3982a4..b2258fdedf 100644 --- a/scripts/code_size_budget.json +++ b/scripts/code_size_budget.json @@ -9,22 +9,22 @@ "crates/jcode-app-core/src/overnight.rs": 1275, "crates/jcode-app-core/src/server.rs": 2653, "crates/jcode-app-core/src/server/client_actions.rs": 1350, - "crates/jcode-app-core/src/server/client_lifecycle.rs": 3974, + "crates/jcode-app-core/src/server/client_lifecycle.rs": 4030, "crates/jcode-app-core/src/server/client_session.rs": 2112, "crates/jcode-app-core/src/server/client_state.rs": 1365, "crates/jcode-app-core/src/server/comm_control.rs": 2640, - "crates/jcode-app-core/src/server/comm_session.rs": 1513, + "crates/jcode-app-core/src/server/comm_session.rs": 1530, "crates/jcode-app-core/src/server/debug_server_state.rs": 1269, "crates/jcode-app-core/src/server/jade_relay.rs": 1429, "crates/jcode-app-core/src/server/provider_control.rs": 1530, - "crates/jcode-app-core/src/server/swarm.rs": 3518, + "crates/jcode-app-core/src/server/swarm.rs": 3519, "crates/jcode-app-core/src/tool/bash.rs": 1590, "crates/jcode-app-core/src/tool/browser.rs": 1252, "crates/jcode-app-core/src/tool/communicate.rs": 3159, "crates/jcode-app-core/src/tool/discover.rs": 2982, "crates/jcode-app-core/src/tool/mod.rs": 1924, "crates/jcode-app-core/src/tool/session_search.rs": 1892, - "crates/jcode-app-core/src/tool/todo.rs": 2853, + "crates/jcode-app-core/src/tool/todo.rs": 2884, "crates/jcode-app-core/src/update.rs": 1308, "crates/jcode-base/src/auth/lifecycle.rs": 2882, "crates/jcode-base/src/auth/mod.rs": 1596, @@ -49,7 +49,7 @@ "crates/jcode-config-types/src/lib.rs": 1623, "crates/jcode-harness-api-server/src/translate.rs": 3620, "crates/jcode-import-core/src/lib.rs": 1645, - "crates/jcode-provider-anthropic-runtime/src/lib.rs": 2788, + "crates/jcode-provider-anthropic-runtime/src/lib.rs": 2770, "crates/jcode-provider-anthropic/src/lib.rs": 1246, "crates/jcode-provider-bedrock/src/lib.rs": 1612, "crates/jcode-provider-copilot-runtime/src/lib.rs": 1201, @@ -63,7 +63,7 @@ "crates/jcode-provider-metadata/src/catalog.rs": 1351, "crates/jcode-provider-openai-runtime/src/lib.rs": 1606, "crates/jcode-provider-openai-runtime/src/openai_provider_impl.rs": 1328, - "crates/jcode-provider-openai-runtime/src/openai_stream_runtime.rs": 2109, + "crates/jcode-provider-openai-runtime/src/openai_stream_runtime.rs": 2183, "crates/jcode-provider-openrouter-runtime/src/lib.rs": 2931, "crates/jcode-render-core/src/math.rs": 1235, "crates/jcode-sdk/src/client.rs": 1938, @@ -76,44 +76,44 @@ "crates/jcode-tui-mermaid/src/mermaid_cache_render.rs": 1561, "crates/jcode-tui-mermaid/src/mermaid_viewport.rs": 2031, "crates/jcode-tui-render/src/swarm_gallery.rs": 2057, - "crates/jcode-tui/src/tui/app.rs": 2923, + "crates/jcode-tui/src/tui/app.rs": 2928, "crates/jcode-tui/src/tui/app/auth.rs": 3475, "crates/jcode-tui/src/tui/app/auth_account_commands.rs": 1213, "crates/jcode-tui/src/tui/app/auth_account_picker.rs": 1305, "crates/jcode-tui/src/tui/app/commands.rs": 4042, - "crates/jcode-tui/src/tui/app/commands_cloud.rs": 1268, + "crates/jcode-tui/src/tui/app/commands_cloud.rs": 1599, "crates/jcode-tui/src/tui/app/debug_bench.rs": 1286, "crates/jcode-tui/src/tui/app/debug_cmds.rs": 1230, "crates/jcode-tui/src/tui/app/helpers.rs": 1810, "crates/jcode-tui/src/tui/app/hotkey_feedback.rs": 1227, - "crates/jcode-tui/src/tui/app/inline_interactive.rs": 4208, - "crates/jcode-tui/src/tui/app/input.rs": 4324, + "crates/jcode-tui/src/tui/app/inline_interactive.rs": 4256, + "crates/jcode-tui/src/tui/app/input.rs": 4343, "crates/jcode-tui/src/tui/app/model_context.rs": 1986, - "crates/jcode-tui/src/tui/app/navigation.rs": 1955, + "crates/jcode-tui/src/tui/app/navigation.rs": 1966, "crates/jcode-tui/src/tui/app/onboarding_flow_control.rs": 1821, - "crates/jcode-tui/src/tui/app/remote.rs": 2278, + "crates/jcode-tui/src/tui/app/remote.rs": 2232, "crates/jcode-tui/src/tui/app/remote/key_handling.rs": 2762, "crates/jcode-tui/src/tui/app/remote/server_events.rs": 3421, "crates/jcode-tui/src/tui/app/state_ui.rs": 2338, "crates/jcode-tui/src/tui/app/state_ui_input_helpers.rs": 1804, - "crates/jcode-tui/src/tui/app/tui_lifecycle.rs": 1644, + "crates/jcode-tui/src/tui/app/tui_lifecycle.rs": 1646, "crates/jcode-tui/src/tui/app/tui_state.rs": 2909, - "crates/jcode-tui/src/tui/app/turn.rs": 1714, - "crates/jcode-tui/src/tui/backend.rs": 1618, - "crates/jcode-tui/src/tui/info_widget.rs": 2347, + "crates/jcode-tui/src/tui/app/turn.rs": 1719, + "crates/jcode-tui/src/tui/backend.rs": 1626, + "crates/jcode-tui/src/tui/info_widget.rs": 2434, "crates/jcode-tui/src/tui/mod.rs": 2523, "crates/jcode-tui/src/tui/session_picker.rs": 2695, "crates/jcode-tui/src/tui/session_picker/loading.rs": 3102, - "crates/jcode-tui/src/tui/ui.rs": 3795, + "crates/jcode-tui/src/tui/ui.rs": 3806, "crates/jcode-tui/src/tui/ui_frame_metrics.rs": 1434, - "crates/jcode-tui/src/tui/ui_header.rs": 1729, - "crates/jcode-tui/src/tui/ui_inline_image.rs": 1797, - "crates/jcode-tui/src/tui/ui_input.rs": 3026, - "crates/jcode-tui/src/tui/ui_messages.rs": 4633, + "crates/jcode-tui/src/tui/ui_header.rs": 1738, + "crates/jcode-tui/src/tui/ui_inline_image.rs": 1867, + "crates/jcode-tui/src/tui/ui_input.rs": 3021, + "crates/jcode-tui/src/tui/ui_messages.rs": 4896, "crates/jcode-tui/src/tui/ui_pinned.rs": 1269, - "crates/jcode-tui/src/tui/ui_prepare.rs": 2736, - "crates/jcode-tui/src/tui/ui_tools.rs": 1687, - "crates/jcode-tui/src/tui/ui_viewport.rs": 1664, + "crates/jcode-tui/src/tui/ui_prepare.rs": 2738, + "crates/jcode-tui/src/tui/ui_tools.rs": 1741, + "crates/jcode-tui/src/tui/ui_viewport.rs": 1737, "src/bin/memory_recall_bench.rs": 2667, "src/bin/tui_bench.rs": 1765, "src/cli/acp.rs": 2319, diff --git a/scripts/panic_budget.json b/scripts/panic_budget.json index 85bd652412..be0d83bbd6 100644 --- a/scripts/panic_budget.json +++ b/scripts/panic_budget.json @@ -1,5 +1,5 @@ { - "total": 174, + "total": 178, "tracked_files": { "crates/jcode-app-core/build.rs": 4, "crates/jcode-app-core/src/agent/provider.rs": 1, @@ -21,7 +21,7 @@ "crates/jcode-base/src/hooks.rs": 1, "crates/jcode-base/src/mcp/manager.rs": 2, "crates/jcode-base/src/memory_jev.rs": 1, - "crates/jcode-base/src/provider_activity_oauth.rs": 3, + "crates/jcode-base/src/provider_activity_oauth.rs": 5, "crates/jcode-base/src/subscription_notice.rs": 1, "crates/jcode-base/src/voice_intent.rs": 5, "crates/jcode-harness-api-server/src/translate.rs": 3, @@ -30,6 +30,7 @@ "crates/jcode-provider-core/src/model_names.rs": 1, "crates/jcode-provider-cursor-runtime/src/wire.rs": 1, "crates/jcode-provider-doctor/src/lifecycle_driver.rs": 2, + "crates/jcode-provider-openai-runtime/src/openai_stream_runtime.rs": 1, "crates/jcode-provider-openai/src/stream.rs": 3, "crates/jcode-render-core/src/math.rs": 4, "crates/jcode-render-core/src/preprocess.rs": 2, @@ -52,6 +53,7 @@ "crates/jcode-tui/src/tui/app/auth_remote/picker.rs": 1, "crates/jcode-tui/src/tui/app/commands_cloud.rs": 2, "crates/jcode-tui/src/tui/app/usage_reset.rs": 1, + "crates/jcode-tui/src/tui/info_widget_git.rs": 1, "crates/jcode-tui/src/tui/session_picker.rs": 2, "crates/jcode-tui/src/tui/ui_inline_interactive.rs": 1, "src/bin/memory_recall_bench.rs": 14, diff --git a/scripts/swallowed_error_budget.json b/scripts/swallowed_error_budget.json index 7c1eff7534..fee1041450 100644 --- a/scripts/swallowed_error_budget.json +++ b/scripts/swallowed_error_budget.json @@ -1,9 +1,9 @@ { - "total": 3961, + "total": 3975, "totals_by_pattern": { - "dot_ok": 1553, - "let_underscore": 1428, - "unwrap_or_default": 980 + "dot_ok": 1564, + "let_underscore": 1429, + "unwrap_or_default": 982 }, "tracked_files": { "crates/jcode-app-core/src/agent.rs": { @@ -192,8 +192,8 @@ "unwrap_or_default": 0 }, "crates/jcode-app-core/src/server/client_lifecycle.rs": { - "dot_ok": 0, - "let_underscore": 53, + "dot_ok": 1, + "let_underscore": 54, "unwrap_or_default": 1 }, "crates/jcode-app-core/src/server/client_lifecycle_logging.rs": { @@ -1503,7 +1503,7 @@ }, "crates/jcode-provider-anthropic-runtime/src/lib.rs": { "dot_ok": 9, - "let_underscore": 15, + "let_underscore": 14, "unwrap_or_default": 1 }, "crates/jcode-provider-anthropic-runtime/src/native_web_search.rs": { @@ -1511,8 +1511,13 @@ "let_underscore": 0, "unwrap_or_default": 0 }, + "crates/jcode-provider-anthropic-runtime/src/usage_recording.rs": { + "dot_ok": 1, + "let_underscore": 1, + "unwrap_or_default": 0 + }, "crates/jcode-provider-antigravity-runtime/src/lib.rs": { - "dot_ok": 2, + "dot_ok": 3, "let_underscore": 21, "unwrap_or_default": 3 }, @@ -1667,7 +1672,7 @@ "unwrap_or_default": 0 }, "crates/jcode-provider-openai-runtime/src/openai_stream_runtime.rs": { - "dot_ok": 3, + "dot_ok": 4, "let_underscore": 7, "unwrap_or_default": 1 }, @@ -1744,7 +1749,7 @@ "crates/jcode-schema-dialect/src/dialect.rs": { "dot_ok": 0, "let_underscore": 0, - "unwrap_or_default": 3 + "unwrap_or_default": 4 }, "crates/jcode-schema-dialect/src/lib.rs": { "dot_ok": 0, @@ -2066,6 +2071,11 @@ "let_underscore": 0, "unwrap_or_default": 0 }, + "crates/jcode-tui-messages/src/message.rs": { + "dot_ok": 5, + "let_underscore": 0, + "unwrap_or_default": 0 + }, "crates/jcode-tui-permissions/src/lib.rs": { "dot_ok": 0, "let_underscore": 4, @@ -2086,6 +2096,11 @@ "let_underscore": 0, "unwrap_or_default": 1 }, + "crates/jcode-tui-tool-display/src/lib.rs": { + "dot_ok": 1, + "let_underscore": 0, + "unwrap_or_default": 0 + }, "crates/jcode-tui-usage-overlay/src/lib.rs": { "dot_ok": 0, "let_underscore": 0, @@ -2627,7 +2642,7 @@ "unwrap_or_default": 2 }, "crates/jcode-tui/src/tui/ui_messages.rs": { - "dot_ok": 6, + "dot_ok": 7, "let_underscore": 0, "unwrap_or_default": 9 }, @@ -2664,7 +2679,7 @@ "crates/jcode-tui/src/tui/ui_tools.rs": { "dot_ok": 1, "let_underscore": 0, - "unwrap_or_default": 12 + "unwrap_or_default": 13 }, "crates/jcode-tui/src/tui/ui_tools/batch.rs": { "dot_ok": 1, diff --git a/scripts/test_size_budget.json b/scripts/test_size_budget.json index 601e8f6bc3..fcb11c26cd 100644 --- a/scripts/test_size_budget.json +++ b/scripts/test_size_budget.json @@ -2,52 +2,52 @@ "threshold_loc": 1200, "tracked_files": { "crates/jcode-app-core/src/agent_tests.rs": 3253, - "crates/jcode-app-core/src/server/client_lifecycle_tests.rs": 1939, - "crates/jcode-app-core/src/server/comm_session_tests.rs": 1332, + "crates/jcode-app-core/src/server/client_lifecycle_tests.rs": 2305, + "crates/jcode-app-core/src/server/comm_session_tests.rs": 1367, "crates/jcode-app-core/src/server/provider_control_tests.rs": 1352, "crates/jcode-app-core/src/tool/bash_tests.rs": 1259, "crates/jcode-app-core/src/tool/communicate_tests.rs": 1481, "crates/jcode-app-core/src/tool/selfdev/tests.rs": 1460, - "crates/jcode-app-core/src/tool/tests.rs": 1976, + "crates/jcode-app-core/src/tool/tests.rs": 1984, "crates/jcode-base/src/compaction_tests.rs": 1271, "crates/jcode-base/src/config_tests.rs": 1795, "crates/jcode-base/src/live_tests.rs": 3087, "crates/jcode-base/src/provider/tests.rs": 1335, "crates/jcode-base/src/provider/tests/model_resolution.rs": 2248, - "crates/jcode-base/src/provider_catalog_tests.rs": 1409, + "crates/jcode-base/src/provider_catalog_tests.rs": 1412, "crates/jcode-base/src/session_tests/cases.rs": 3267, - "crates/jcode-base/src/usage/tests.rs": 1317, + "crates/jcode-base/src/usage/tests.rs": 1355, "crates/jcode-harness-api-server/src/translate_tests.rs": 4199, "crates/jcode-provider-anthropic-runtime/src/anthropic_tests.rs": 2029, "crates/jcode-provider-openrouter-runtime/src/openrouter_tests.rs": 3568, "crates/jcode-sdk/tests/client_behavior.rs": 1432, "crates/jcode-telemetry-core/src/tests.rs": 1836, - "crates/jcode-tui/src/tui/app/remote_tests.rs": 1441, - "crates/jcode-tui/src/tui/app/tests.rs": 2030, + "crates/jcode-tui/src/tui/app/remote_tests.rs": 1443, + "crates/jcode-tui/src/tui/app/tests.rs": 2032, "crates/jcode-tui/src/tui/app/tests/commands_accounts_01/part_01.rs": 1604, "crates/jcode-tui/src/tui/app/tests/commands_accounts_02/part_01.rs": 1326, "crates/jcode-tui/src/tui/app/tests/onboarding_eval.rs": 3305, - "crates/jcode-tui/src/tui/app/tests/onboarding_flow.rs": 1672, + "crates/jcode-tui/src/tui/app/tests/onboarding_flow.rs": 1680, "crates/jcode-tui/src/tui/app/tests/remote_events_reload_01/part_01.rs": 1712, "crates/jcode-tui/src/tui/app/tests/remote_events_reload_04.rs": 2676, "crates/jcode-tui/src/tui/app/tests/remote_startup_input_01/part_01.rs": 1251, - "crates/jcode-tui/src/tui/app/tests/remote_startup_input_02/part_01.rs": 2413, - "crates/jcode-tui/src/tui/app/tests/remote_startup_input_03/part_01.rs": 1340, + "crates/jcode-tui/src/tui/app/tests/remote_startup_input_02/part_01.rs": 2411, + "crates/jcode-tui/src/tui/app/tests/remote_startup_input_03/part_01.rs": 1348, "crates/jcode-tui/src/tui/app/tests/scroll_copy_01/part_01.rs": 1583, "crates/jcode-tui/src/tui/app/tests/scroll_copy_02/part_01.rs": 1580, - "crates/jcode-tui/src/tui/app/tests/scroll_copy_02/part_02.rs": 1726, + "crates/jcode-tui/src/tui/app/tests/scroll_copy_02/part_02.rs": 1794, "crates/jcode-tui/src/tui/app/tests/scroll_copy_03.rs": 1716, "crates/jcode-tui/src/tui/app/tests/state_model_poke_01/part_01.rs": 1382, "crates/jcode-tui/src/tui/app/tests/state_model_poke_02/part_01.rs": 1683, - "crates/jcode-tui/src/tui/app/tests/state_model_poke_03.rs": 3121, + "crates/jcode-tui/src/tui/app/tests/state_model_poke_03.rs": 3212, "crates/jcode-tui/src/tui/app/tests/swarm_plan_graph_inline.rs": 1883, - "crates/jcode-tui/src/tui/info_widget_tests.rs": 1961, + "crates/jcode-tui/src/tui/info_widget_tests.rs": 2385, "crates/jcode-tui/src/tui/session_picker/loading_tests.rs": 1568, "crates/jcode-tui/src/tui/session_picker_tests.rs": 2806, - "crates/jcode-tui/src/tui/ui_messages/tests.rs": 3653, - "crates/jcode-tui/src/tui/ui_tests/basic/body_cache.rs": 1225, - "crates/jcode-tui/src/tui/ui_tests/prepare.rs": 1374, - "crates/jcode-tui/src/tui/ui_tests/tools.rs": 1530, + "crates/jcode-tui/src/tui/ui_messages/tests.rs": 4412, + "crates/jcode-tui/src/tui/ui_tests/basic/body_cache.rs": 1226, + "crates/jcode-tui/src/tui/ui_tests/prepare.rs": 1394, + "crates/jcode-tui/src/tui/ui_tests/tools.rs": 1542, "src/cli/commands_tests.rs": 1805, "tests/e2e/test_support/mod.rs": 1417 }, diff --git a/src/cli/ssh_transport.rs b/src/cli/ssh_transport.rs index 6ba01fd814..1cf269ff08 100644 --- a/src/cli/ssh_transport.rs +++ b/src/cli/ssh_transport.rs @@ -775,11 +775,25 @@ mod tests { let error = result.err().expect("must time out"); assert!(format!("{error:#}").contains("timed out")); assert!(start.elapsed() < Duration::from_secs(3)); - let pid: i32 = std::fs::read_to_string(pid_file) - .unwrap() - .trim() - .parse() - .unwrap(); + // The shell writes the pid file right after spawn; under load that + // write can land after the timeout error returns, so poll instead of + // reading once and failing on a not-yet-written file. + let pid: i32 = { + let deadline = std::time::Instant::now() + Duration::from_secs(10); + loop { + let pid = std::fs::read_to_string(&pid_file) + .ok() + .and_then(|content| content.trim().parse().ok()); + if let Some(pid) = pid { + break pid; + } + assert!( + std::time::Instant::now() < deadline, + "child never wrote its pid file" + ); + tokio::time::sleep(Duration::from_millis(20)).await; + } + }; assert_eq!(unsafe { libc::kill(pid, 0) }, -1, "child must be reaped"); } diff --git a/src/cli/tui_launch/tests.rs b/src/cli/tui_launch/tests.rs index 23441206ba..7899351bb1 100644 --- a/src/cli/tui_launch/tests.rs +++ b/src/cli/tui_launch/tests.rs @@ -69,7 +69,10 @@ fn write_fake_handterm(temp: &tempfile::TempDir, output_path: &Path) { #[cfg(unix)] fn wait_for_lines(path: &Path, min_lines: usize) -> Vec { - let deadline = Instant::now() + Duration::from_secs(3); + // Hang guard, not latency budget: the launcher spawns a shell that must + // start, exec handterm, and write. Under full-suite load that spawn chain + // can take seconds; a real hang still misses a 30s deadline by far. + let deadline = Instant::now() + Duration::from_secs(30); while Instant::now() < deadline { if let Ok(content) = fs::read_to_string(path) { let lines: Vec = content.lines().map(|line| line.to_string()).collect();