diff --git a/crates/ide-agent/src/lib.rs b/crates/ide-agent/src/lib.rs index 4f3400d70..73e311b20 100644 --- a/crates/ide-agent/src/lib.rs +++ b/crates/ide-agent/src/lib.rs @@ -100,7 +100,7 @@ struct Shared { providers: RwLock>, /// Last probe of each provider. probes: Mutex>, - /// One pre-session command lookup per provider, including failed attempts. + /// One pre-session lookup of commands and settings per provider, including failed attempts. command_fetches: Mutex>>, /// [`IDLE_RELEASE`], in milliseconds (shorter in tests). idle_release_ms: AtomicU64, @@ -153,7 +153,7 @@ impl Shared { cache.get(provider).and_then(|k| k.commands.get(&self.root_key())).cloned().unwrap_or_default() } - async fn fetch_commands(self: &Arc, spec: provider::ProviderSpec) { + async fn fetch_settings(self: &Arc, spec: provider::ProviderSpec) { let mut ready = { let mut fetches = self.command_fetches.lock(); fetches @@ -163,14 +163,14 @@ impl Shared { let shared = self.clone(); // Keep the lookup alive if a composer unmounts while it is waiting. self.runtime.spawn(async move { - if let Ok(commands) = - native::claude::commands(spec.clone(), shared.paths.project_root.clone()).await + if let Ok((commands, options)) = + native::claude::discover(spec.clone(), shared.paths.project_root.clone()).await { let commands = config::commands_of(&json!({ "availableCommands": commands }), &spec.id); + let options = config::options_of(&json!({ "configOptions": options })); let _ = tokio::task::spawn_blocking(move || { - if shared.known_commands(&spec.id).is_empty() { - shared.remember(&spec.id, None, Some(&commands), None); - } + shared.remember_discovered(&spec.id, &options, &commands); + shared.emit(AgentEvent::Providers); }) .await; } @@ -187,6 +187,10 @@ impl Shared { } } + fn has_config(&self, provider: &str) -> bool { + self.known.lock().get(provider).is_some_and(|k| !k.config.is_empty()) + } + fn root_key(&self) -> String { self.paths.project_root.to_string_lossy().into_owned() } @@ -199,37 +203,52 @@ impl Shared { commands: Option<&[AgentCommand]>, model: Option<&str>, ) { - let snapshot = { - let mut cache = self.known.lock(); - let known = cache.entry(provider.to_owned()).or_default(); - let mut changed = false; - if let Some(c) = config { - if let Some(model) = model { - let levels: Vec<_> = - c.iter().filter(|o| o.category.as_deref() == Some("thought_level")).cloned().collect(); - if known.thought_levels.get(model) != Some(&levels) { - known.thought_levels.insert(model.to_owned(), levels); - changed = true; - } - } - if !c.is_empty() && known.config != c { - known.config = c.to_vec(); + self.remember_inner(provider, config, commands, model, false); + } + + fn remember_discovered(&self, provider: &str, config: &[AgentConfigOption], commands: &[AgentCommand]) { + self.remember_inner(provider, Some(config), Some(commands), model_of(config).as_deref(), true); + } + + fn remember_inner( + &self, + provider: &str, + config: Option<&[AgentConfigOption]>, + commands: Option<&[AgentCommand]>, + model: Option<&str>, + discovered: bool, + ) { + // Keep the update and its disk write ordered; a session report wins over discovery. + let mut cache = self.known.lock(); + let known = cache.entry(provider.to_owned()).or_default(); + let mut changed = false; + if let Some(c) = config.filter(|_| !discovered || known.config.is_empty()) { + if let Some(model) = model { + let levels: Vec<_> = + c.iter().filter(|o| o.category.as_deref() == Some("thought_level")).cloned().collect(); + if known.thought_levels.get(model) != Some(&levels) { + known.thought_levels.insert(model.to_owned(), levels); changed = true; } } - let root = self.root_key(); - if let Some(c) = - commands.filter(|c| !c.is_empty() && known.commands.get(&root).map(Vec::as_slice) != Some(c)) - { - known.commands.insert(root, c.to_vec()); + if !c.is_empty() && known.config != c { + known.config = c.to_vec(); changed = true; } - if !changed { - return; - } - serde_json::to_vec_pretty(&*cache) - }; - if let Ok(bytes) = snapshot + } + let root = self.root_key(); + if let Some(c) = commands.filter(|c| { + !c.is_empty() + && (!discovered || known.commands.get(&root).is_none_or(Vec::is_empty)) + && known.commands.get(&root).map(Vec::as_slice) != Some(c) + }) { + known.commands.insert(root, c.to_vec()); + changed = true; + } + if !changed { + return; + } + if let Ok(bytes) = serde_json::to_vec_pretty(&*cache) && let Err(e) = write_file(&self.paths.models_file, &bytes) { tracing::warn!("saving agent settings: {e}"); @@ -458,10 +477,10 @@ impl AgentManager { // merged without duplicating what `availableCommands` or the disk offer. let mut offered = offered; if task.is_none() - && offered.is_empty() + && (offered.is_empty() || !self.shared.has_config(&provider)) && let Some(spec) = self.shared.spec(&provider).filter(|s| s.driver == provider::Driver::Claude) { - self.shared.fetch_commands(spec).await; + self.shared.fetch_settings(spec).await; offered = self.shared.known_commands(&provider); } for &(name, description) in builtin_commands(&provider) { diff --git a/crates/ide-agent/src/native/claude/mod.rs b/crates/ide-agent/src/native/claude/mod.rs index e4e557162..39677e622 100644 --- a/crates/ide-agent/src/native/claude/mod.rs +++ b/crates/ide-agent/src/native/claude/mod.rs @@ -69,8 +69,9 @@ pub(crate) fn connect( Ok(super::serve(shim, client)) } -/// Discover commands without a prompt, ACP session, or pending session file. -pub(crate) async fn commands(spec: ProviderSpec, cwd: PathBuf) -> Result, String> { +/// Discover the slash commands and the model, mode and effort options without a prompt, +/// ACP session, or pending session file. +pub(crate) async fn discover(spec: ProviderSpec, cwd: PathBuf) -> Result<(Vec, Value), String> { tokio::time::timeout(Duration::from_secs(5), async move { let spec = tokio::task::spawn_blocking(move || (spec.resolve_for_launch(&cwd), cwd)); let (spec, cwd) = spec.await.map_err(|e| e.to_string())?; @@ -79,28 +80,52 @@ pub(crate) async fn commands(spec: ProviderSpec, cwd: PathBuf) -> Result(&line) else { continue }; - if msg["type"] == "control_response" && msg["response"]["request_id"] == "commands" { - let response = &msg["response"]; - return if response["subtype"] == "success" { - Ok(Settings::from_cli(&response["response"], &Value::Null).commands) - } else { - Err("Claude Code did not initialize".into()) - }; - } - } - Err("Claude Code exited".into()) + let init = control( + &mut io.stdin, + &mut lines, + "commands", + json!({ "subtype": "initialize", "perTaskStopAffordance": true }), + ) + .await? + .ok_or("Claude Code did not initialize")?; + // The effort and model the user configured; without them the options show the defaults. + let applied = tokio::time::timeout( + Duration::from_secs(2), + control(&mut io.stdin, &mut lines, "settings", json!({ "subtype": "get_settings" })), + ) + .await + .ok() + .and_then(Result::ok) + .flatten() + .unwrap_or_default(); + let settings = Settings::from_cli(&init, &applied); + Ok((settings.commands.clone(), settings.options())) }) .await .map_err(|_| "Claude Code command discovery timed out".to_owned())? } +/// Sends one control request and waits for its response; `None` when the CLI answered with an error. +async fn control( + stdin: &mut (impl AsyncWriteExt + Unpin), + lines: &mut tokio::io::Lines, + id: &str, + request: Value, +) -> Result, String> { + let line = json!({ "type": "control_request", "request_id": id, "request": request }); + stdin.write_all(format!("{line}\n").as_bytes()).await.map_err(|e| e.to_string())?; + stdin.flush().await.map_err(|e| e.to_string())?; + while let Some(line) = lines.next_line().await.map_err(|e| e.to_string())? { + let Ok(msg) = serde_json::from_str::(&line) else { continue }; + if msg["type"] == "control_response" && msg["response"]["request_id"] == id { + let response = &msg["response"]; + return Ok((response["subtype"] == "success").then(|| response["response"].clone())); + } + } + Err("Claude Code exited".into()) +} + struct Config { pub program: String, /// Extra arguments (a custom agent's), before the session's own. diff --git a/crates/ide-agent/src/tests.rs b/crates/ide-agent/src/tests.rs index 025bc98bb..e38114b23 100644 --- a/crates/ide-agent/src/tests.rs +++ b/crates/ide-agent/src/tests.rs @@ -371,7 +371,7 @@ fn manager_at(dir: &Path, models_file: &Path, agent: Arc) -> AgentMan async fn claude_commands_before_session_are_fetched_once_and_saved() { use std::os::unix::fs::PermissionsExt; - for outcome in ["success", "error", "exit", "timeout", "missing"] { + for outcome in ["success", "session_report", "empty", "settings_timeout", "error", "exit", "timeout", "missing"] { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); let script = root.join("fake-claude"); @@ -380,6 +380,7 @@ async fn claude_commands_before_session_are_fetched_once_and_saved() { r#"#!/usr/bin/env python3 import json, os, pathlib, sys, time root = pathlib.Path.cwd() +(root / 'pid').write_text(str(os.getpid())) with (root / 'launches').open('a') as f: f.write(json.dumps(sys.argv[1:]) + '\n') request = json.loads(sys.stdin.readline()) @@ -391,16 +392,34 @@ if outcome == 'exit': if outcome == 'timeout': time.sleep(60) response = {'subtype': outcome, 'request_id': request['request_id']} -if outcome == 'success': +if outcome in ('success', 'session_report', 'empty', 'settings_timeout'): + response['subtype'] = 'success' response['response'] = {'commands': [ {'name': 'compact', 'description': 'Free up context', 'argumentHint': '[instructions]'}, - {'name': 'clear'}, {'name': 'model'}, {'name': 'context'}]} + {'name': 'clear'}, {'name': 'model'}, {'name': 'context'}], + 'models': [ + {'value': 'default', 'displayName': 'Default (recommended)', 'description': 'Use the default model', + 'supportsEffort': True, 'supportedEffortLevels': ['low', 'high']}, + {'value': 'custom-opus', 'displayName': 'custom-opus', 'description': 'Custom Opus model'}]} + if outcome == 'empty': + response['response'] = {} else: response['error'] = 'Authentication required' +if outcome == 'session_report': + (root / 'initialized').touch() + while not (root / 'continue').exists(): + time.sleep(0.01) print(json.dumps({'type': 'control_response', 'response': response}), flush=True) for line in sys.stdin: with (root / 'requests').open('a') as f: f.write(line) + request = json.loads(line) + if request['request']['subtype'] == 'get_settings': + if outcome == 'settings_timeout': + time.sleep(60) + reply = {'subtype': 'success', 'request_id': request['request_id'], + 'response': {'applied': {'effort': 'high'}, 'effective': {}}} + print(json.dumps({'type': 'control_response', 'response': reply}), flush=True) "#, ) .unwrap(); @@ -417,19 +436,59 @@ for line in sys.stdin: } spec.env = vec![("OUTCOME".into(), outcome.into())]; } - let (first, concurrent) = tokio::join!(mgr.commands("claude", None), mgr.commands("claude", None)); + let concurrent = async { + if outcome == "session_report" { + tokio::time::timeout(Duration::from_secs(2), async { + while !root.join("initialized").exists() { + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .unwrap(); + let reported = config::options_of(&json!({ "configOptions": [ + { "id": "model", "category": "model", "type": "select", "currentValue": "default", + "options": [{ "value": "default" }, { "value": "custom-opus" }] }, + { "id": "effort", "category": "thought_level", "type": "select", "currentValue": "low", + "options": [{ "value": "default" }, { "value": "low" }, { "value": "high" }] } + ] })); + mgr.shared.remember("claude", Some(&reported), None, Some("default")); + std::fs::write(root.join("continue"), "").unwrap(); + } + mgr.commands("claude", None).await + }; + let (first, concurrent) = tokio::join!(mgr.commands("claude", None), concurrent); let first = first.unwrap(); assert_eq!(first, concurrent.unwrap()); assert_eq!(first, mgr.commands("claude", None).await.unwrap()); let names: Vec<_> = first.iter().map(|c| c.name.as_str()).collect(); - if outcome == "success" { + if matches!(outcome, "success" | "session_report" | "settings_timeout") { assert_eq!(names, ["compact", "clear", "model", "context", "triage"]); assert_eq!(first[0].hint.as_deref(), Some("[instructions]")); let restored = manager(root, Arc::new(FakeAgent::default())); assert_eq!(first, restored.commands("claude", None).await.unwrap()); + // The same handshake lists the models, so the composer has them before any task. + for mgr in [&mgr, &restored] { + let claude = mgr.providers().into_iter().find(|p| p.id == "claude").unwrap(); + let model = claude.config.iter().find(|o| o.category.as_deref() == Some("model")).unwrap(); + assert_eq!(model.current.as_deref(), Some("default")); + let values: Vec<_> = model.values.iter().map(|v| v.value.as_str()).collect(); + assert_eq!(values, ["default", "custom-opus"]); + let effort = + claude.thought_levels["default"].iter().find(|o| o.category.as_deref() == Some("thought_level")); + assert_eq!( + effort.unwrap().current.as_deref(), + Some(match outcome { + "settings_timeout" => "default", + "session_report" => "low", + _ => "high", + }) + ); + } } else { assert_eq!(names, ["triage"], "{outcome}"); - assert!(!root.join("models.json").exists()); + assert_eq!(root.join("models.json").exists(), outcome == "empty"); + let claude = mgr.providers().into_iter().find(|p| p.id == "claude").unwrap(); + assert!(!claude.config.iter().any(|o| o.category.as_deref() == Some("model")), "{outcome}"); } assert!(mgr.tasks.read().is_empty()); assert!(!root.join("store").exists()); @@ -439,10 +498,25 @@ for line in sys.stdin: let args: Vec = serde_json::from_str(launches.trim()).unwrap(); assert!(args.windows(2).any(|a| a == ["--input-format", "stream-json"])); let requests = std::fs::read_to_string(root.join("requests")).unwrap(); - assert_eq!(requests.lines().count(), 1, "only initialize, no prompt or other control request"); - let request: Value = serde_json::from_str(requests.trim()).unwrap(); - assert_eq!(request["type"], "control_request"); - assert_eq!(request["request"]["subtype"], "initialize"); + let subtypes: Vec = requests + .lines() + .map(|l| serde_json::from_str::(l).unwrap()["request"]["subtype"].as_str().unwrap().to_owned()) + .collect(); + // The handshake and the settings read: no prompt or other control request. + let expected: &[&str] = if matches!(outcome, "success" | "session_report" | "empty" | "settings_timeout") { + &["initialize", "get_settings"] + } else { + &["initialize"] + }; + assert_eq!(subtypes, expected, "{outcome}"); + let pid = std::fs::read_to_string(root.join("pid")).unwrap(); + tokio::time::timeout(Duration::from_secs(2), async { + while std::process::Command::new("kill").args(["-0", pid.trim()]).output().unwrap().status.success() { + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("discovery process must exit"); } } } @@ -2358,3 +2432,31 @@ fn mcp_servers_follow_server_state_and_agent_capabilities() { "args":["--mcp-stdio","--project","/project"],"env":[]})] ); } + +#[tokio::test(flavor = "multi_thread")] +async fn session_settings_win_over_discovery_in_memory_and_on_disk() { + for discovery_first in [false, true] { + let tmp = tempfile::tempdir().unwrap(); + let mgr = manager(tmp.path(), Arc::new(FakeAgent::default())); + let options = |model: &str, effort: &str| { + config::options_of(&json!({ "configOptions": [ + { "id": "model", "category": "model", "type": "select", "currentValue": model, + "options": [{ "value": "saved" }, { "value": "discovered" }] }, + { "id": "effort", "category": "thought_level", "type": "select", "currentValue": effort, + "options": [{ "value": "low" }, { "value": "high" }] } + ] })) + }; + let saved = options("saved", "low"); + let discovered = options("discovered", "high"); + if discovery_first { + mgr.shared.remember_discovered("claude", &discovered, &[]); + } + mgr.shared.remember("claude", Some(&saved), None, Some("saved")); + mgr.shared.remember_discovered("claude", &discovered, &[]); + let restored = manager(tmp.path(), Arc::new(FakeAgent::default())); + for mgr in [&mgr, &restored] { + assert_eq!(model_of(&mgr.shared.known_config("claude")).as_deref(), Some("saved")); + assert_eq!(mgr.shared.known.lock()["claude"].thought_levels["saved"], saved[1..]); + } + } +}