diff --git a/src-tauri/src/acp/connection.rs b/src-tauri/src/acp/connection.rs index 052609a34a..a527beea6c 100644 --- a/src-tauri/src/acp/connection.rs +++ b/src-tauri/src/acp/connection.rs @@ -4556,6 +4556,11 @@ async fn apply_and_emit_session_config_options( // session birth, and on a catalog broadcast). `None` when empty // keeps the switch path on the flat-fallback branch. state.write().await.grok_model_specs = (!specs.is_empty()).then(|| specs.clone()); + // Grok's picker comes from its handshake, not a config-option list: + // its values are what the agent picked on its own, read before the + // replay below rewrites them (see + // `SessionState::agent_chosen_config_values`). + state.write().await.agent_chosen_config_values = current_config_option_values(&opts); let session_id = session.session_id().clone(); apply_grok_preferred_options( cx, @@ -4571,6 +4576,10 @@ async fn apply_and_emit_session_config_options( // No x.ai/sessionConfig (unexpected): fall through to the standard path, // which for Grok emits an empty list (no selectors) — same as before. } + // What the agent picked on its own, read before the replay below rewrites + // any of it (see `SessionState::agent_chosen_config_values`). + state.write().await.agent_chosen_config_values = + current_config_option_values(&map_session_config_options(&initial_config_options)); let updated = apply_preferred_session_options( cx, session, @@ -30864,6 +30873,144 @@ mod tests { assert_eq!(ordered, vec!["a_thing", "z_thing"]); } + /// Establishment keeps what the agent picked on its own, read BEFORE the + /// saved preferences replay over it. The options probe reports that for + /// each option it applied (`report_agent_chosen_values` in the manager), so + /// the delegation panel's "Default" names the agent's model rather than the + /// user's. Shaped like a live opencode 2.0.24: switching the model re-lists + /// `effort` for it. + #[tokio::test] + async fn establishment_keeps_the_agents_own_picks_from_before_the_preference_replay() { + use agent_client_protocol::schema::v1::SetSessionConfigOptionResponse; + + fn options(model: &str, effort: &str, efforts: &[&str]) -> Vec { + let efforts: Vec = efforts + .iter() + .map(|value| serde_json::json!({"value": value, "name": value})) + .collect(); + serde_json::from_value(serde_json::json!([ + { + "type": "select", + "id": "model", + "name": "Model", + "category": "model", + "currentValue": model, + "options": [ + {"value": "opencode/exo-free", "name": "Exo"}, + {"value": "opencode/step-5-preview-free", "name": "Step 5"} + ] + }, + { + "type": "select", + "id": "effort", + "name": "Effort", + "category": "thought_level", + "currentValue": effort, + "options": efforts + }, + ])) + .expect("parses") + } + + let (client_end, agent_end) = agent_client_protocol::Channel::duplex(); + let agent = tokio::spawn(async move { + let _ = Agent + .builder() + .on_receive_request( + async |_req: NewSessionRequest, + responder: Responder, + _cx: ConnectionTo| { + let opened = options("opencode/exo-free", "high", &["high", "default"]); + responder.respond( + NewSessionResponse::new(SessionId::new("s1")).config_options(opened), + ) + }, + on_receive_request!(), + ) + .on_receive_request( + async |_req: SetSessionConfigOptionRequest, + responder: Responder, + _cx: ConnectionTo| { + // Only the model is replayed: the answer re-lists effort. + let switched = options( + "opencode/step-5-preview-free", + "default", + &["low", "medium", "high", "default"], + ); + responder.respond(SetSessionConfigOptionResponse::new(switched)) + }, + on_receive_request!(), + ) + .connect_with(agent_end, async |_cx: ConnectionTo| { + std::future::pending::>().await + }) + .await; + }); + + let state = Arc::new(RwLock::new(SessionState::new( + "conn-probe".to_string(), + AgentType::OpenCode, + None, + "delegation-probe".to_string(), + None, + ))); + let establishment_state = Arc::clone(&state); + Client + .builder() + .connect_with(client_end, async move |cx: ConnectionTo| { + let raw = cx + .send_request_to( + Agent, + UntypedMessage::new("session/new", NewSessionRequest::new("/tmp"))?, + ) + .block_task() + .await?; + let response: NewSessionResponse = serde_json::from_value(raw) + .map_err(agent_client_protocol::Error::into_internal_error)?; + let initial = response.config_options.clone().unwrap_or_default(); + let mut session = AgentSession::attach(&cx, response)?; + let preferred = BTreeMap::from([( + "model".to_string(), + "opencode/step-5-preview-free".to_string(), + )]); + apply_and_emit_session_config_options( + &cx, + &mut session, + &establishment_state, + &EventEmitter::Noop, + AgentType::OpenCode, + None, + None, + None, + &preferred, + initial, + ) + .await; + Ok(()) + }) + .await + .expect("the establishment runs"); + agent.abort(); + + let state = state.read().await; + assert_eq!( + state.agent_chosen_config_values, + BTreeMap::from([ + ("model".to_string(), "opencode/exo-free".to_string()), + ("effort".to_string(), "high".to_string()), + ]), + "the agent's own picks, from before the replay" + ); + // …while the session itself runs the preference, effort re-listed for it. + let current = + current_config_option_values(state.config_options.as_deref().unwrap_or_default()); + assert_eq!( + current.get("model").map(String::as_str), + Some("opencode/step-5-preview-free") + ); + assert_eq!(current.get("effort").map(String::as_str), Some("default")); + } + /// The claude shape: a model select plus the effort option that hangs off it. fn asserted_drift_options(model: &str, effort: &str) -> Vec { serde_json::from_value(serde_json::json!([ diff --git a/src-tauri/src/acp/manager.rs b/src-tauri/src/acp/manager.rs index fb4a87b49f..c38c665e03 100644 --- a/src-tauri/src/acp/manager.rs +++ b/src-tauri/src/acp/manager.rs @@ -2419,6 +2419,15 @@ impl ConnectionManager { /// what `codeg-mcp` will pass through to `session/set_config_option` /// when a delegation actually fires. /// + /// `preferred_config_values` are applied on the probe session before the + /// snapshot is read, exactly as a real launch would apply them: an agent + /// that derives one option's choices from another's value (opencode + /// re-lists `effort` per selected model) then answers for the user's + /// selection instead of its own default. A value the agent rejects is + /// logged and skipped — the snapshot still comes back. Each applied + /// option itself still reports the agent's own pick as its current value + /// (see [`report_agent_chosen_values`]). + /// /// Returns `Ok(snapshot)` even when the agent advertises no options /// (empty `config_options`, `None` modes) — that's a valid outcome the /// UI can render as "this agent has nothing to configure." @@ -2427,6 +2436,7 @@ impl ConnectionManager { agent_type: AgentType, working_dir: Option, runtime_env: BTreeMap, + preferred_config_values: BTreeMap, ) -> Result { // Owner window label is informational only (used for // disconnect_by_owner_window), but worth being explicit so a probe @@ -2453,6 +2463,7 @@ impl ConnectionManager { .clone() }; let _probe_guard = per_agent_lock.lock_owned().await; + let applied_ids: Vec = preferred_config_values.keys().cloned().collect(); let conn_id = self .spawn_agent( agent_type, @@ -2462,7 +2473,7 @@ impl ConnectionManager { owner_window, EventEmitter::Noop, None, - BTreeMap::new(), + preferred_config_values, ) .await?; @@ -2487,7 +2498,19 @@ impl ConnectionManager { // message over the generic ProbeTimedOut / ConnectionNotFound — // an agent that died on Initialize already explained why. let snapshot = match raw_snapshot { - Ok(s) => Ok(s), + Ok(mut s) => { + if !applied_ids.is_empty() { + if let Some(state) = state_arc.as_ref() { + let agent_chosen = state.read().await.agent_chosen_config_values.clone(); + report_agent_chosen_values( + &mut s.config_options, + &applied_ids, + &agent_chosen, + ); + } + } + Ok(s) + } Err(wait_err) => { let captured = if let Some(state) = state_arc.as_ref() { state.read().await.last_error.clone() @@ -4194,11 +4217,114 @@ impl SessionPlanApprovalAccess for ConnectionManagerPlanApprovalLookup { } } +/// Make every option the probe applied a caller's selection to report the value +/// the agent picked on its own, instead of that selection. +/// +/// Every consumer of the probe's snapshot reads an option's current value as +/// what it runs when the caller leaves it unset: the delegation settings panel +/// names its "Default" choice after it, and the task and automation editors +/// show and pin it for an option nobody touched. For an applied option that is +/// the agent's own pick, which the session no longer holds once the selection +/// is in — reporting the selection would label the user's model as the agent's +/// default. The options the agent derived from the selection (opencode's +/// `effort`, re-listed per model) keep the value it answered, because that is +/// what they run when unset given the selection. +/// +/// `agent_chosen` is the establishment's own answer +/// (`SessionState::agent_chosen_config_values`). An applied id the agent never +/// answered with, or the snapshot does not carry, is left alone. +fn report_agent_chosen_values( + options: &mut [crate::acp::types::SessionConfigOptionInfo], + applied_ids: &[String], + agent_chosen: &BTreeMap, +) { + use crate::acp::types::SessionConfigKindInfo; + for option in options + .iter_mut() + .filter(|option| applied_ids.contains(&option.id)) + { + let Some(chosen) = agent_chosen.get(&option.id) else { + continue; + }; + match &mut option.kind { + SessionConfigKindInfo::Select(select) => select.current_value = chosen.clone(), + SessionConfigKindInfo::Boolean(boolean) => boolean.current_value = chosen == "true", + } + } +} + #[cfg(test)] mod tests { use super::*; use crate::acp::connection::AgentConnection; + /// The probe applies the caller's model, so the session it reads holds that + /// model; the delegation panel names its "Default" choice after the + /// snapshot's current value. Model ids from a live opencode 2.0.24 probe: + /// the agent opened on `exo-free` and the caller asked for + /// `step-5-preview-free`. The agent's own effort differs from the one it + /// answered after the switch, so the test can tell which one survives. + #[test] + fn the_probe_reports_the_agents_own_pick_for_the_options_it_applied() { + use crate::acp::types::{ + SessionConfigBooleanInfo, SessionConfigKindInfo, SessionConfigOptionInfo, + SessionConfigSelectInfo, + }; + let option = |id: &str, kind: SessionConfigKindInfo| SessionConfigOptionInfo { + id: id.to_string(), + name: id.to_string(), + description: None, + category: None, + kind, + recommended_value: None, + }; + let select = |current: &str| { + SessionConfigKindInfo::Select(SessionConfigSelectInfo { + current_value: current.to_string(), + options: Vec::new(), + groups: Vec::new(), + }) + }; + let current = |options: &[SessionConfigOptionInfo], id: &str| { + let option = options.iter().find(|o| o.id == id).expect("present"); + match &option.kind { + SessionConfigKindInfo::Select(select) => select.current_value.clone(), + SessionConfigKindInfo::Boolean(boolean) => boolean.current_value.to_string(), + } + }; + let mut options = vec![ + option("model", select("opencode/step-5-preview-free")), + option("effort", select("default")), + option("mode", select("plan")), + option( + "auto_approve", + SessionConfigKindInfo::Boolean(SessionConfigBooleanInfo { + current_value: true, + }), + ), + ]; + let agent_chosen: BTreeMap = [ + ("model", "opencode/exo-free"), + ("effort", "high"), + ("mode", "build"), + ("auto_approve", "false"), + ] + .into_iter() + .map(|(id, value)| (id.to_string(), value.to_string())) + .collect(); + // `provider` was applied but neither answered nor in the snapshot. + let applied = ["model", "auto_approve", "provider"].map(String::from); + + report_agent_chosen_values(&mut options, &applied, &agent_chosen); + + assert_eq!(current(&options, "model"), "opencode/exo-free"); + assert_eq!(current(&options, "auto_approve"), "false"); + // Not applied: what the agent answered GIVEN the selection stands — the + // default effort of the selected model, and a mode nobody asked for. + assert_eq!(current(&options, "effort"), "default"); + assert_eq!(current(&options, "mode"), "plan"); + } + /// An agent that has left the connection map but not yet exited can still /// be appending to the transcript a restore is about to unlink, so the /// gate has to see it. `disconnect` drops the map entry immediately, which diff --git a/src-tauri/src/acp/session_state.rs b/src-tauri/src/acp/session_state.rs index 94b60d0191..e3c1173919 100644 --- a/src-tauri/src/acp/session_state.rs +++ b/src-tauri/src/acp/session_state.rs @@ -382,6 +382,21 @@ pub struct SessionState { /// /// Backend-internal — not serialized, not carried on `to_snapshot()`. pub asserted_config_values: BTreeMap, + /// Each config option's value as the agent itself picked it for this + /// session: the establishment's own answer (or the picker Grok's handshake + /// yields), read BEFORE codeg replays any saved preference over it. + /// Rewritten by every establishment, whether or not it had preferences to + /// replay. + /// + /// Only the options probe reads it (`ConnectionManager::probe_agent_options`). + /// The probe applies the caller's model so that options the agent derives + /// from it — opencode re-lists `effort` per model — answer for that model. + /// `config_options` then holds the caller's own model as the current one, + /// while what the probe must report for an applied option is what it runs + /// when left unset: the agent's default, kept here. + /// + /// Backend-internal — not serialized, not carried on `to_snapshot()`. + pub agent_chosen_config_values: BTreeMap, /// Config-option ids this launch pinned through the environment, which the /// agent will therefore refuse to change for as long as the process lives. /// @@ -708,6 +723,7 @@ impl SessionState { grok_catalog_broadcast: None, pi_startup_banner: None, asserted_config_values: BTreeMap::new(), + agent_chosen_config_values: BTreeMap::new(), env_pinned_config_option_ids: Vec::new(), prompt_capabilities: None, fork_supported: false, diff --git a/src-tauri/src/acp/types.rs b/src-tauri/src/acp/types.rs index d5b182e63f..1e37892856 100644 --- a/src-tauri/src/acp/types.rs +++ b/src-tauri/src/acp/types.rs @@ -1436,6 +1436,11 @@ pub struct GrokModelCatalog { /// to give the delegation settings UI an authoritative view of what an /// agent will accept (no reliance on chat-side caches). /// +/// The caller's selections (the model) are applied first, as a real launch +/// applies them, so an option the agent derives from one answers for it. Each +/// option's current value is what it runs when the caller leaves it unset — +/// for an applied option that is the agent's own pick, not the selection. +/// /// Both fields mirror `SessionState`: `modes` is `None` when the agent /// reports no mode catalog (e.g. some thin wrappers); `config_options` is /// empty when the agent advertises no configurable options. diff --git a/src-tauri/src/commands/acp.rs b/src-tauri/src/commands/acp.rs index bb6fc62bfa..400abc9ab3 100644 --- a/src-tauri/src/commands/acp.rs +++ b/src-tauri/src/commands/acp.rs @@ -11238,6 +11238,7 @@ pub async fn acp_describe_agent_options_core( data_dir: &Path, agent_type: AgentType, working_dir: Option, + preferred_config_values: BTreeMap, ) -> Result { verify_agent_installed(agent_type).await?; // Build the same runtime env delegation/acp_connect would build so @@ -11247,7 +11248,12 @@ pub async fn acp_describe_agent_options_core( // model_provider injects a different model list, etc.). let runtime_env = build_session_runtime_env(db, agent_type, None, data_dir).await?; manager - .probe_agent_options(agent_type, working_dir, runtime_env) + .probe_agent_options( + agent_type, + working_dir, + runtime_env, + preferred_config_values, + ) .await } @@ -11256,6 +11262,9 @@ pub async fn acp_describe_agent_options_core( pub async fn acp_describe_agent_options( agent_type: AgentType, working_dir: Option, + // Config selections to apply on the probe session before reading the + // snapshot — callers pass the model so per-model option lists match. + config_values: Option>, manager: State<'_, ConnectionManager>, db: State<'_, AppDatabase>, app_handle: tauri::AppHandle, @@ -11265,7 +11274,15 @@ pub async fn acp_describe_agent_options( .app_data_dir() .map(|p| crate::paths::resolve_effective_data_dir(&p)) .unwrap_or_else(|_| PathBuf::from(".")); - acp_describe_agent_options_core(&manager, &db, &app_data_dir, agent_type, working_dir).await + acp_describe_agent_options_core( + &manager, + &db, + &app_data_dir, + agent_type, + working_dir, + config_values.unwrap_or_default(), + ) + .await } #[cfg(feature = "tauri-runtime")] diff --git a/src-tauri/src/web/handlers/acp.rs b/src-tauri/src/web/handlers/acp.rs index 758e44031b..93c61c5c2f 100644 --- a/src-tauri/src/web/handlers/acp.rs +++ b/src-tauri/src/web/handlers/acp.rs @@ -420,6 +420,10 @@ pub struct AcpDescribeAgentOptionsParams { pub agent_type: crate::models::AgentType, #[serde(default)] pub working_dir: Option, + /// Config selections to apply on the probe session before reading the + /// snapshot — callers pass the model so per-model option lists match. + #[serde(default)] + pub config_values: Option>, } pub async fn acp_describe_agent_options( @@ -432,6 +436,7 @@ pub async fn acp_describe_agent_options( &state.data_dir, params.agent_type, params.working_dir, + params.config_values.unwrap_or_default(), ) .await .map_err(|e| AppCommandError::task_execution_failed(e.to_string()))?; diff --git a/src/components/automations/automation-editor.tsx b/src/components/automations/automation-editor.tsx index 2e8e4ea27c..f5879d7d18 100644 --- a/src/components/automations/automation-editor.tsx +++ b/src/components/automations/automation-editor.tsx @@ -163,7 +163,12 @@ export function AutomationEditor({ // One transient probe feeds both the config selectors and the `/` command menu // (the snapshot carries available_commands). `$` Codex skills load separately // (filesystem scan) inside the invocations hook. - const agentOptions = useAgentOptions(agentType, folderPath) + const agentOptions = useAgentOptions( + agentType, + folderPath, + true, + configValues + ) const invocations = useComposerInvocations({ editorRef, agentType, diff --git a/src/components/automations/use-agent-options.test.ts b/src/components/automations/use-agent-options.test.ts index e00e7a3395..1c4bfbf0a9 100644 --- a/src/components/automations/use-agent-options.test.ts +++ b/src/components/automations/use-agent-options.test.ts @@ -75,3 +75,79 @@ describe("useAgentOptions snapshot ownership", () => { expect(result.current.snapshotAgentType).toBe("codex") }) }) + +describe("useAgentOptions model-scoped probes", () => { + beforeEach(() => { + vi.useFakeTimers() + describeAgentOptions.mockReset() + describeAgentOptions.mockImplementation((agent: AgentType) => + Promise.resolve(snapshotFor(agent)) + ) + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it("applies the selected model and re-probes when it changes", async () => { + const folder = `/tmp/use-agent-options-model-${Math.random()}` + const { rerender } = renderHook( + ({ model }: { model: string }) => + useAgentOptions("deepseek" as AgentType, folder, true, { model }), + { initialProps: { model: "opencode-go/deepseek-v4.1-flash" } } + ) + + await act(async () => { + await vi.advanceTimersByTimeAsync(300) + }) + expect(describeAgentOptions).toHaveBeenCalledTimes(1) + expect(describeAgentOptions).toHaveBeenLastCalledWith("deepseek", folder, { + model: "opencode-go/deepseek-v4.1-flash", + }) + + // A different model derives different option lists — the (agent, folder) + // cache must not serve the previous model's snapshot. + rerender({ model: "vercel/callstack/apex" }) + await act(async () => { + await vi.advanceTimersByTimeAsync(300) + }) + expect(describeAgentOptions).toHaveBeenCalledTimes(2) + expect(describeAgentOptions).toHaveBeenLastCalledWith("deepseek", folder, { + model: "vercel/callstack/apex", + }) + }) + + /** The task editor's config bar and its brief composer each run this hook + * for the same agent and folder. Only the model keys the probe, so both + * read one probe as long as they pass the same model — a host that left it + * out would spawn the agent a second time. */ + it("shares one probe between hosts passing the same model", async () => { + const folder = `/tmp/use-agent-options-shared-${Math.random()}` + const selections = { model: "opencode/step-5-preview-free", effort: "high" } + // Held open, so the second host arrives while the first probe is still + // running rather than after it filled the cache. + let answer: (snapshot: AgentOptionsSnapshot) => void = () => {} + describeAgentOptions.mockImplementation( + () => + new Promise((resolve) => { + answer = resolve + }) + ) + const { result } = renderHook(() => [ + useAgentOptions("deepseek" as AgentType, folder, true, selections), + useAgentOptions("deepseek" as AgentType, folder, true, selections), + ]) + + await act(async () => { + await vi.advanceTimersByTimeAsync(300) + }) + expect(describeAgentOptions).toHaveBeenCalledTimes(1) + + await act(async () => { + answer(snapshotFor("deepseek")) + await vi.advanceTimersByTimeAsync(0) + }) + expect(result.current[0].snapshot).not.toBeNull() + expect(result.current[1].snapshot).toBe(result.current[0].snapshot) + }) +}) diff --git a/src/components/automations/use-agent-options.ts b/src/components/automations/use-agent-options.ts index 4f8be4a81f..f765dfe855 100644 --- a/src/components/automations/use-agent-options.ts +++ b/src/components/automations/use-agent-options.ts @@ -16,22 +16,30 @@ interface CachedSnapshot { ts: number } -// Keyed by (agent, folderPath): the same agent probed in two target folders can -// surface different folder/project-scoped slash commands or options, so a folder -// switch must not return another folder's cached snapshot. JSON.stringify is a -// collision-free composite key (and avoids a literal NUL separator). +// Keyed by (agent, folderPath, model): the same agent probed in two target +// folders can surface different folder/project-scoped slash commands or +// options, so a folder switch must not return another folder's cached +// snapshot — and an agent that derives one option's choices from another's +// value (opencode lists `effort` per model) answers differently per model. +// JSON.stringify is a collision-free composite key (and avoids a literal NUL +// separator). const snapshotCache = new Map() const inflight = new Map>() -function cacheKey(agent: AgentType, folderPath: string | null): string { - return JSON.stringify([agent, folderPath ?? null]) +function cacheKey( + agent: AgentType, + folderPath: string | null, + model: string | null +): string { + return JSON.stringify([agent, folderPath ?? null, model]) } function readCache( agent: AgentType, - folderPath: string | null + folderPath: string | null, + model: string | null ): AgentOptionsSnapshot | null { - const key = cacheKey(agent, folderPath) + const key = cacheKey(agent, folderPath, model) const entry = snapshotCache.get(key) if (!entry) return null if (Date.now() - entry.ts > CACHE_TTL_MS) { @@ -43,12 +51,13 @@ function readCache( function fetchOptions( agent: AgentType, - folderPath: string | null + folderPath: string | null, + model: string | null ): Promise { - const key = cacheKey(agent, folderPath) + const key = cacheKey(agent, folderPath, model) let promise = inflight.get(key) if (!promise) { - promise = describeAgentOptions(agent, folderPath) + promise = describeAgentOptions(agent, folderPath, model ? { model } : null) .then((snapshot) => { snapshotCache.set(key, { snapshot, ts: Date.now() }) inflight.delete(key) @@ -95,8 +104,14 @@ export function useAgentOptions( /** When false, the automatic probe is suppressed (no transient CLI spawn) — * for editors whose agent-override section is collapsed. `ensure()` still * probes on demand at save time. */ - enabled: boolean = true + enabled: boolean = true, + /** The host's current config selections — only `model` is consumed: the + * probe applies it before snapshotting so agent-derived option lists + * (opencode's per-model `effort`) match the selection. Changing it + * re-probes; the snapshot cache is keyed by it. */ + preferredConfigValues?: Record | null ): AgentOptionsState { + const preferredModel = preferredConfigValues?.model ?? null // Snapshot and its producer live in ONE state value so they can never be // rendered out of step — see `snapshotAgentType`. const [loaded, setLoaded] = useState<{ @@ -108,17 +123,22 @@ export function useAgentOptions( const reqRef = useRef(0) const load = useCallback( - (agent: AgentType, folder: string | null, force: boolean) => { + ( + agent: AgentType, + folder: string | null, + model: string | null, + force: boolean + ) => { // Bump FIRST so a cache hit also invalidates any still-in-flight probe for a - // previously-selected (agent, folder) — otherwise that slow probe's late - // result would overwrite the snapshot for the now-current one. + // previously-selected (agent, folder, model) — otherwise that slow probe's + // late result would overwrite the snapshot for the now-current one. const id = ++reqRef.current - const key = cacheKey(agent, folder) + const key = cacheKey(agent, folder, model) if (force) { snapshotCache.delete(key) inflight.delete(key) } else { - const cached = readCache(agent, folder) + const cached = readCache(agent, folder, model) if (cached) { setLoaded({ agent, snapshot: cached }) setError(null) @@ -129,7 +149,7 @@ export function useAgentOptions( setLoading(true) setError(null) setLoaded(null) - fetchOptions(agent, folder) + fetchOptions(agent, folder, model) .then((fresh) => { if (reqRef.current !== id) return setLoaded({ agent, snapshot: fresh }) @@ -146,27 +166,27 @@ export function useAgentOptions( useEffect(() => { if (!enabled) return - // Debounce so switching agents/folders quickly doesn't fire a probe (CLI - // spawn) per click; the last (agent, folder) landed on wins. + // Debounce so switching agents/folders/models quickly doesn't fire a probe + // (CLI spawn) per click; the last (agent, folder, model) landed on wins. const handle = window.setTimeout(() => { - void load(agentType, folderPath, false) + void load(agentType, folderPath, preferredModel, false) }, 250) return () => window.clearTimeout(handle) - }, [agentType, folderPath, load, enabled]) + }, [agentType, folderPath, preferredModel, load, enabled]) const reload = useCallback( - () => load(agentType, folderPath, true), - [agentType, folderPath, load] + () => load(agentType, folderPath, preferredModel, true), + [agentType, folderPath, preferredModel, load] ) const ensure = useCallback(async (): Promise => { - // Resolve against the CURRENT (agent, folder), not the retained React - // `snapshot`: after an agent/folder switch the previous snapshot lingers until - // the debounced re-probe lands, and returning it here would pin the wrong - // agent's/folder's defaults into the save. The module cache + inflight map are - // keyed by (agent, folder), so a hit is instant and a switch rides the - // effect's in-flight probe (no double spawn). - const cached = readCache(agentType, folderPath) + // Resolve against the CURRENT (agent, folder, model), not the retained + // React `snapshot`: after an agent/folder/model switch the previous snapshot + // lingers until the debounced re-probe lands, and returning it here would + // pin the wrong agent's/folder's/model's defaults into the save. The module + // cache + inflight map are keyed by (agent, folder, model), so a hit is + // instant and a switch rides the effect's in-flight probe (no double spawn). + const cached = readCache(agentType, folderPath, preferredModel) if (cached) return cached // Bound the wait so a wedged probe degrades to "save with raw overrides" // rather than hanging the save. @@ -176,13 +196,13 @@ export function useAgentOptions( }) try { return await Promise.race([ - fetchOptions(agentType, folderPath).catch(() => null), + fetchOptions(agentType, folderPath, preferredModel).catch(() => null), timeout, ]) } finally { if (timer !== undefined) window.clearTimeout(timer) } - }, [agentType, folderPath]) + }, [agentType, folderPath, preferredModel]) return { snapshot: loaded?.snapshot ?? null, diff --git a/src/components/settings/delegation-agent-defaults.test.tsx b/src/components/settings/delegation-agent-defaults.test.tsx new file mode 100644 index 0000000000..dbf46d2ae1 --- /dev/null +++ b/src/components/settings/delegation-agent-defaults.test.tsx @@ -0,0 +1,161 @@ +import { act, fireEvent, render, screen } from "@testing-library/react" +import { NextIntlClientProvider } from "next-intl" +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" + +import type { + AgentDelegationDefaults, + AgentOptionsSnapshot, + AgentType, +} from "@/lib/types" + +const describeAgentOptions = vi.hoisted(() => vi.fn()) + +vi.mock("@/lib/api", () => ({ describeAgentOptions })) +vi.mock("@/hooks/use-acp-agents", () => ({ + useAcpAgents: () => ({ agents: [], fresh: true, refresh: () => {} }), +})) + +import { DelegationAgentDefaultsPanel } from "./delegation-agent-defaults" +import enMessages from "@/i18n/messages/en.json" + +type Defaults = Partial> + +/** A probe answer whose effort default is named after `tag`, so the + * "Agent default: …" hint on screen says which answer is showing. */ +function answer(tag: string): AgentOptionsSnapshot { + return { + modes: null, + config_options: [ + { + id: "effort", + name: "Effort", + description: null, + category: "thought_level", + kind: { + type: "select", + current_value: `${tag}-effort`, + options: [ + { + value: `${tag}-effort`, + name: `${tag} effort`, + description: null, + }, + ], + groups: [], + }, + }, + ], + available_commands: [], + } +} + +function panel(value: Defaults) { + return ( + + {}} /> + + ) +} + +/** Past the panel's 250ms debounce, settling the probe's promise chain. */ +async function settle() { + await act(async () => { + await vi.advanceTimersByTimeAsync(300) + }) +} + +// The panel's snapshot cache lives in module scope for 30s, so every test +// probes ids no other test uses. +const unique = () => Math.random().toString(36).slice(2) + +describe("DelegationAgentDefaultsPanel probes", () => { + beforeEach(() => { + vi.useFakeTimers() + describeAgentOptions.mockReset() + }) + + afterEach(() => { + vi.useRealTimers() + }) + + /** opencode lists `effort` per model: the options shown beside a saved model + * have to be that model's, so changing it re-probes with it applied. */ + it("probes with the saved model and re-probes when it changes", async () => { + describeAgentOptions.mockImplementation( + (_agent: AgentType, _dir: string | null, cfg: { model: string } | null) => + Promise.resolve(answer(cfg?.model ?? "no-model")) + ) + const first = `model-a-${unique()}` + const second = `model-b-${unique()}` + const saved = (model: string): Defaults => ({ + claude_code: { config_values: { model } }, + }) + + const { rerender } = render(panel(saved(first))) + await settle() + expect(describeAgentOptions).toHaveBeenLastCalledWith("claude_code", null, { + model: first, + }) + expect( + screen.getByText(`Agent default: ${first} effort`) + ).toBeInTheDocument() + + rerender(panel(saved(second))) + await settle() + expect(describeAgentOptions).toHaveBeenCalledTimes(2) + expect(describeAgentOptions).toHaveBeenLastCalledWith("claude_code", null, { + model: second, + }) + expect( + screen.getByText(`Agent default: ${second} effort`) + ).toBeInTheDocument() + + // Back to a model probed moments ago: served from the cache, no new probe. + rerender(panel(saved(first))) + await settle() + expect(describeAgentOptions).toHaveBeenCalledTimes(2) + expect( + screen.getByText(`Agent default: ${first} effort`) + ).toBeInTheDocument() + }) + + /** A tab served from the cache must not be overwritten by the answer of a + * probe the user already left — that answer is another agent's options. */ + it("drops a probe answer that lands after a cache hit replaced it", async () => { + const claudeModel = `claude-${unique()}` + let answerCodex: (snapshot: AgentOptionsSnapshot) => void = () => {} + describeAgentOptions.mockImplementation((agent: AgentType) => + agent === "codex" + ? new Promise((resolve) => { + answerCodex = resolve + }) + : Promise.resolve(answer(claudeModel)) + ) + + render(panel({ claude_code: { config_values: { model: claudeModel } } })) + await settle() + expect( + screen.getByText(`Agent default: ${claudeModel} effort`) + ).toBeInTheDocument() + + // Codex's probe is still running when the user goes back to Claude Code, + // whose snapshot is cached. + fireEvent.click(screen.getByRole("tab", { name: "Codex" })) + await settle() + expect(describeAgentOptions).toHaveBeenLastCalledWith("codex", null, null) + fireEvent.click(screen.getByRole("tab", { name: "Claude Code" })) + await settle() + expect( + screen.getByText(`Agent default: ${claudeModel} effort`) + ).toBeInTheDocument() + + await act(async () => { + answerCodex(answer("codex")) + await vi.advanceTimersByTimeAsync(0) + }) + expect( + screen.getByText(`Agent default: ${claudeModel} effort`) + ).toBeInTheDocument() + expect(screen.queryByText("Agent default: codex effort")).toBeNull() + }) +}) diff --git a/src/components/settings/delegation-agent-defaults.tsx b/src/components/settings/delegation-agent-defaults.tsx index 7775f29f2d..9ac2b1f9d0 100644 --- a/src/components/settings/delegation-agent-defaults.tsx +++ b/src/components/settings/delegation-agent-defaults.tsx @@ -79,20 +79,35 @@ interface CachedSnapshot { ts: number } const SNAPSHOT_TTL_MS = 30_000 -const snapshotCache = new Map() +// Keyed by (agent, model): an agent that derives one option's choices from +// another's value (opencode lists `effort` per model) answers differently per +// model, so a cached snapshot for one model must not serve another. +const snapshotCache = new Map() -function readCache(agent: AgentType): AgentOptionsSnapshot | null { - const entry = snapshotCache.get(agent) +function snapshotKey(agent: AgentType, model: string | null): string { + return JSON.stringify([agent, model]) +} + +function readCache( + agent: AgentType, + model: string | null +): AgentOptionsSnapshot | null { + const key = snapshotKey(agent, model) + const entry = snapshotCache.get(key) if (!entry) return null if (Date.now() - entry.ts > SNAPSHOT_TTL_MS) { - snapshotCache.delete(agent) + snapshotCache.delete(key) return null } return entry.snapshot } -function writeCache(agent: AgentType, snapshot: AgentOptionsSnapshot): void { - snapshotCache.set(agent, { snapshot, ts: Date.now() }) +function writeCache( + agent: AgentType, + model: string | null, + snapshot: AgentOptionsSnapshot +): void { + snapshotCache.set(snapshotKey(agent, model), { snapshot, ts: Date.now() }) } export interface DelegationAgentDefaultsPanelProps { @@ -132,43 +147,58 @@ export function DelegationAgentDefaultsPanel({ const [error, setError] = useState(null) const reqIdRef = useRef(0) - const loadSnapshot = useCallback(async (agent: AgentType, force: boolean) => { - if (!force) { - const cached = readCache(agent) - if (cached) { - setLoaded({ agent, snapshot: cached }) - setError(null) - setLoading(false) - return + const loadSnapshot = useCallback( + async (agent: AgentType, model: string | null, force: boolean) => { + // Bump FIRST so a cache hit also invalidates a probe still in flight for + // the previous (agent, model) — otherwise that probe's late answer would + // replace the snapshot just shown for the current one. + const reqId = ++reqIdRef.current + if (!force) { + const cached = readCache(agent, model) + if (cached) { + setLoaded({ agent, snapshot: cached }) + setError(null) + setLoading(false) + return + } } - } - const reqId = ++reqIdRef.current - setLoading(true) - setError(null) - setLoaded(null) - try { - const fresh = await describeAgentOptions(agent) - if (reqIdRef.current !== reqId) return - writeCache(agent, fresh) - setLoaded({ agent, snapshot: fresh }) - } catch (err: unknown) { - if (reqIdRef.current !== reqId) return - setError(toErrorMessage(err)) - } finally { - if (reqIdRef.current === reqId) setLoading(false) - } - }, []) + setLoading(true) + setError(null) + setLoaded(null) + try { + const fresh = await describeAgentOptions( + agent, + null, + model ? { model } : null + ) + if (reqIdRef.current !== reqId) return + writeCache(agent, model, fresh) + setLoaded({ agent, snapshot: fresh }) + } catch (err: unknown) { + if (reqIdRef.current !== reqId) return + setError(toErrorMessage(err)) + } finally { + if (reqIdRef.current === reqId) setLoading(false) + } + }, + [] + ) + + // The model currently selected for this agent (its saved override). The + // probe applies it so option lists an agent derives per model — opencode's + // `effort` — answer for THAT model instead of the agent's own default. + const probeModel = value[selectedAgent]?.config_values?.model ?? null useEffect(() => { - // Debounce so rapid tab clicks (which would each fire a real probe - // — even with backend serialization, each one still spawns the CLI) - // collapse into a single load. Cancelling on cleanup means the - // *last* tab the user lands on wins, not the first. + // Debounce so rapid tab clicks / model changes (each would fire a real + // probe — even with backend serialization, each one still spawns the + // CLI) collapse into a single load. Cancelling on cleanup means the + // *last* (agent, model) the user lands on wins, not the first. const handle = window.setTimeout(() => { - void loadSnapshot(selectedAgent, false) + void loadSnapshot(selectedAgent, probeModel, false) }, TAB_SWITCH_DEBOUNCE_MS) return () => window.clearTimeout(handle) - }, [selectedAgent, loadSnapshot]) + }, [selectedAgent, probeModel, loadSnapshot]) const updateAgentDefaults = useCallback( (agent: AgentType, next: AgentDelegationDefaults | null) => { @@ -263,7 +293,9 @@ export function DelegationAgentDefaultsPanel({ diff --git a/src/components/tasks/task-editor-dialog.agent.test.tsx b/src/components/tasks/task-editor-dialog.agent.test.tsx index 3463876dd8..d37f7a5a68 100644 --- a/src/components/tasks/task-editor-dialog.agent.test.tsx +++ b/src/components/tasks/task-editor-dialog.agent.test.tsx @@ -21,6 +21,12 @@ const inheritance = vi.hoisted(() => ({ folderDefault: null as string | null, })) const templateSave = vi.hoisted(() => vi.fn()) +// The config selections each probe host passed on its latest render: the +// editor's own options hook, and the brief's composer. +const probeSelections = vi.hoisted(() => ({ + bar: [] as unknown[], + composer: [] as unknown[], +})) vi.mock("@/lib/api", () => ({ gitListAllBranches: () => @@ -64,14 +70,22 @@ vi.mock("@/components/automations/agent-config-section", () => ({ snapshotLabels: () => ({}), })) vi.mock("@/components/automations/use-agent-options", () => ({ - useAgentOptions: (agentType: string) => ({ - snapshot: null, - snapshotAgentType: agentType, - loading: false, - error: null, - reload: vi.fn(), - ensure: () => Promise.resolve(null), - }), + useAgentOptions: ( + agentType: string, + _folderPath: string | null, + _enabled: boolean, + configValues: unknown + ) => { + probeSelections.bar.push(configValues) + return { + snapshot: null, + snapshotAgentType: agentType, + loading: false, + error: null, + reload: vi.fn(), + ensure: () => Promise.resolve(null), + } + }, })) // The real composer is a Tiptap editor; the editor dialog only reads text and @@ -82,12 +96,14 @@ vi.mock("./task-message-composer", async () => { defaultText?: string ariaLabel?: string onChange?: (text: string) => void + probeConfigValues?: Record | null } return { TaskMessageComposer: forwardRef(function Stub( props: StubProps, ref: React.Ref ) { + probeSelections.composer.push(props.probeConfigValues) const [text, setText] = useState(props.defaultText ?? "") useImperativeHandle( ref, @@ -279,9 +295,29 @@ beforeEach(() => { inheritance.settingsConfig = {} inheritance.folderDefault = null templateSave.mockReset().mockResolvedValue(undefined) + probeSelections.bar = [] + probeSelections.composer = [] }) describe("TaskEditorDialog agent", () => { + it("the brief's composer probes with the bar's selections, so they share one probe", async () => { + // Probes are keyed by the selected model: the composer has to pass the + // same selections as the mode/model bar, or opening the editor on a saved + // model would spawn the agent twice. + inheritance.settingsAgent = "claude_code" + inheritance.settingsConfig = { model: "opus", effort: "high" } + registry(agent("claude_code")) + renderEditor() + const latest = (renders: unknown[]) => renders[renders.length - 1] + await waitFor(() => + expect(latest(probeSelections.bar)).toEqual({ + model: "opus", + effort: "high", + }) + ) + expect(latest(probeSelections.composer)).toBe(latest(probeSelections.bar)) + }) + it("nothing to inherit: saves the agent the selector substituted and shows", async () => { // #864: no task settings, no folder default, and the placeholder agent is // disabled — so the selector highlights the first usable one on its own. diff --git a/src/components/tasks/task-editor-dialog.tsx b/src/components/tasks/task-editor-dialog.tsx index b5d19f0a62..7707645f43 100644 --- a/src/components/tasks/task-editor-dialog.tsx +++ b/src/components/tasks/task-editor-dialog.tsx @@ -232,7 +232,12 @@ function TaskEditorBody({ // quietly ignored. const choosesBranch = task?.source_kind !== "forge_pr" - const agentOptions = useAgentOptions(agentType, folderPath, true) + const agentOptions = useAgentOptions( + agentType, + folderPath, + true, + configValues + ) // An untouched pill is saved as "inherit" only while it shows the agent that // inheriting launches. It can show another: the placeholder when nothing is @@ -465,6 +470,9 @@ function TaskEditorBody({ onChange={setPrompt} onAttachmentsChange={setAttachmentCount} editorClassName="max-h-[14rem] min-h-[6rem]" + // The bar below probes with these; so must the composer, or the two + // stop sharing one probe and the agent is spawned twice. + probeConfigValues={configValues} bottomBarExtra={ | null /** Sizing for the editor surface itself. */ editorClassName?: string } @@ -139,6 +145,7 @@ export function TaskMessageComposer({ onSubmit, onAttachmentsChange, bottomBarExtra, + probeConfigValues, editorClassName = "max-h-[12rem] min-h-[4.5rem]", }: TaskMessageComposerProps) { const t = useTranslations("Folder.chat.messageInput") @@ -156,7 +163,12 @@ export function TaskMessageComposer({ // One transient probe backs the `/` menu (the snapshot carries // available_commands) and the image encoding (prompt_capabilities); `$` // skills are a filesystem scan inside `useComposerInvocations`. - const agentOptions = useAgentOptions(agentType, folderPath) + const agentOptions = useAgentOptions( + agentType, + folderPath, + true, + probeConfigValues + ) const availableCommands: AvailableCommandInfo[] = agentOptions.snapshot?.available_commands ?? [] const promptCapabilities = diff --git a/src/components/tasks/task-settings-dialog.tsx b/src/components/tasks/task-settings-dialog.tsx index 43161ad144..3df53336d8 100644 --- a/src/components/tasks/task-settings-dialog.tsx +++ b/src/components/tasks/task-settings-dialog.tsx @@ -292,7 +292,8 @@ function TaskSettingsBody({ const agentOptions = useAgentOptions( agentType, folder?.path ?? null, - loaded != null && editing + loaded != null && editing, + configValues ) const save = async () => { diff --git a/src/lib/api.ts b/src/lib/api.ts index ba46ccaa3b..f0fbd52f76 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -5390,7 +5390,15 @@ export async function setChatAuthoringSettings( * Does NOT touch chat-side `selectorsCache` or `localStorage` preferences. */ export async function describeAgentOptions( agentType: AgentType, - workingDir?: string | null + workingDir?: string | null, + /** Config selections to apply on the probe session before reading the + * snapshot. Callers pass the model: an agent that derives one option's + * choices from another's value (opencode lists `effort` per model) then + * answers for the user's selection instead of its own default model. An + * applied option still reports the agent's own pick as its `current_value` + * (what it runs when left unset), so a "Default" label keeps naming the + * agent's model rather than the selection. */ + configValues?: Record | null ): Promise { // The backend probe has its own 60s timeout (`ConnectionManager:: // probe_agent_options`) plus 500ms grace + poll/serialization @@ -5403,6 +5411,7 @@ export async function describeAgentOptions( { agentType, workingDir: workingDir ?? null, + configValues: configValues ?? null, }, { timeoutMs: 70_000 } )