diff --git a/crates/ide-agent/src/config.rs b/crates/ide-agent/src/config.rs index ee9b45bfd..af9882db0 100644 --- a/crates/ide-agent/src/config.rs +++ b/crates/ide-agent/src/config.rs @@ -39,31 +39,19 @@ pub(crate) fn add_client_options(provider: &str, config: &mut Vec bool { - ["mode", "permissions", "approval"].iter().any(|category| { - config - .iter() - .find(|o| o.category.as_deref() == Some(category)) - .and_then(|o| o.current.as_deref()) - .is_some_and(|v| is_bypass(Some(category), v)) - }) + let selected = |category: &str| { + config.iter().find(|o| o.category.as_deref() == Some(category)).and_then(|o| o.current.as_deref()) + }; + selected("mode").is_some_and(is_bypass_mode) + || selected("permissions").is_some_and(|v| matches!(v.to_ascii_lowercase().as_str(), "on" | "true")) + || selected("approval").is_some_and(|v| v.eq_ignore_ascii_case("bypass")) } -fn is_bypass(category: Option<&str>, value: &str) -> bool { - match category { - Some("mode") => is_bypass_mode(value), - Some("permissions") => matches!(value.to_ascii_lowercase().as_str(), "on" | "true"), - Some("approval") => value.eq_ignore_ascii_case("bypass"), - _ => false, - } -} - -/// Provider-wide offers are not user choices; a session's bypass belongs only to that session. -pub(crate) fn clear_cached_bypass(options: &mut [AgentConfigOption]) { - for option in options { - if option.current.as_deref().is_some_and(|v| is_bypass(option.category.as_deref(), v)) { - option.current = None; - } - } +/// The mode that approves everything without asking (`bypassPermissions`, `yolo`, `full-access`…), if the agent has one. +pub(crate) fn bypass_mode(options: &[AgentConfigOption]) -> Option<(&AgentConfigOption, &AgentConfigValue)> { + let option = options.iter().find(|o| o.category.as_deref() == Some("mode"))?; + let value = option.values.iter().find(|v| is_bypass_mode(&v.value))?; + Some((option, value)) } fn is_bypass_mode(mode: &str) -> bool { @@ -307,6 +295,7 @@ mod tests { ]} })); assert!(bypass_approvals(&full_access)); + assert_eq!(bypass_mode(&full_access).map(|(_, v)| v.value.as_str()), Some("full-access")); let asking = options_of(&json!({ "modes": {"currentModeId": "ask", "availableModes": [ @@ -314,13 +303,11 @@ mod tests { ]} })); assert!(!bypass_approvals(&asking)); - let mut cached = full_access.clone(); - clear_cached_bypass(&mut cached); - assert_eq!(cached[0].current, None); - assert_eq!(cached[0].values, full_access[0].values, "bypass stays available as an explicit choice"); - let mut cached = asking.clone(); - clear_cached_bypass(&mut cached); - assert_eq!(cached, asking); + assert_eq!( + bypass_mode(&asking).map(|(_, v)| v.value.as_str()), + Some("full-access"), + "the offered bypass mode stays available" + ); let permissions = |current: &str| { options_of(&json!({"configOptions": [{ diff --git a/crates/ide-agent/src/lib.rs b/crates/ide-agent/src/lib.rs index fe1f9dbb9..73e311b20 100644 --- a/crates/ide-agent/src/lib.rs +++ b/crates/ide-agent/src/lib.rs @@ -136,7 +136,14 @@ impl Shared { // Model-specific options are offered separately, never as provider-wide defaults. config.retain(|o| o.category.as_deref() != Some("thought_level")); config::add_client_options(provider, &mut config); - config::clear_cached_bypass(&mut config); + if let Some((o, v)) = config::bypass_mode(&config).map(|(o, v)| (o.id.clone(), v.value.clone())) + && let Some(o) = config.iter_mut().find(|c| c.id == o) + { + o.current = Some(v); + } + if let Some(o) = config.iter_mut().find(|o| o.id == config::CURSOR_APPROVAL) { + o.current = Some("bypass".into()); + } config } @@ -224,10 +231,8 @@ impl Shared { changed = true; } } - let mut c = c.to_vec(); - config::clear_cached_bypass(&mut c); if !c.is_empty() && known.config != c { - known.config = c; + known.config = c.to_vec(); changed = true; } } @@ -356,11 +361,8 @@ impl AgentManager { } fn with_connector(paths: AgentPaths, runtime: tokio::runtime::Handle, connector: Connector) -> Self { - let mut known: HashMap = + let known = std::fs::read(&paths.models_file).ok().and_then(|b| serde_json::from_slice(&b).ok()).unwrap_or_default(); - for offered in known.values_mut() { - config::clear_cached_bypass(&mut offered.config); - } let providers = RwLock::new(provider::load(paths.custom_agents_dir.as_deref())); let shared = Arc::new(Shared { paths, diff --git a/crates/ide-agent/src/tests.rs b/crates/ide-agent/src/tests.rs index 668a462e8..e38114b23 100644 --- a/crates/ide-agent/src/tests.rs +++ b/crates/ide-agent/src/tests.rs @@ -1714,109 +1714,6 @@ fn usage_limits_and_cost_skip_what_is_malformed() { assert_eq!(usage.map(|u| u.cost), Some(None), "a negative cost is dropped, the context use kept"); } -#[tokio::test(flavor = "multi_thread")] -async fn cached_bypass_is_dropped_on_load_read_and_report_without_changing_task_choices() { - for (provider, category, bypass, safe) in [ - ("claude", "mode", "bypassPermissions", "default"), - ("codex", "mode", "full-access", "auto"), - ("cursor", "approval", "bypass", "ask"), - ("copilot", "permissions", "on", "off"), - ("custom", "mode", "https://example.com/modes#yolo", "ask"), - ] { - let tmp = tempfile::tempdir().unwrap(); - let agent = Arc::new(FakeAgent::default()); - let options = config::options_of(&json!({"configOptions": [{ - "id": category, "category": category, "currentValue": bypass, - "options": [{"value": safe}, {"value": bypass}] - }]})); - let models = tmp.path().join("models.json"); - std::fs::write(&models, serde_json::to_vec(&json!({provider: {"config": options}})).unwrap()).unwrap(); - let mgr = manager(tmp.path(), agent); - assert_eq!(mgr.shared.known.lock()[provider].config[0].current, None, "load: {provider}"); - // Also defend reads of in-memory data, without changing its offered values. - mgr.shared.known.lock().get_mut(provider).unwrap().config = options.clone(); - let read = mgr.shared.known_config(provider); - assert_eq!(read[0].current, None, "read: {provider}"); - assert_eq!(read[0].values, options[0].values); - mgr.shared.remember(provider, Some(&options), None, None); - assert_eq!(mgr.shared.known.lock()[provider].config[0].current, None, "report: {provider}"); - let disk: HashMap = serde_json::from_slice(&std::fs::read(models).unwrap()).unwrap(); - assert_eq!(disk[provider].config[0].current, None); - let mut safe_options = options.clone(); - safe_options[0].current = Some(safe.into()); - mgr.shared.remember(provider, Some(&safe_options), None, None); - assert_eq!(mgr.shared.known_config(provider)[0].current.as_deref(), Some(safe)); - assert_eq!(options[0].current.as_deref(), Some(bypass), "the live report stays intact"); - } -} - -#[tokio::test(flavor = "multi_thread")] -async fn new_tasks_use_reported_modes_and_explicit_bypass_survives_restart() { - for (provider, safe, bypass) in [ - ("claude", "default", "bypassPermissions"), - ("codex", "auto", "full-access"), - ("cursor", "agent", "yolo"), - ("copilot", "interactive", "yolo"), - ("claude-acp", "default", "bypassPermissions"), - ] { - let tmp = tempfile::tempdir().unwrap(); - let agent = Arc::new(FakeAgent::default()); - *agent.extra_options.lock() = vec![json!({ - "id": "mode", "category": "mode", "currentValue": safe, - "options": [{"value": safe}, {"value": bypass}] - })]; - let mgr = manager(tmp.path(), agent.clone()); - let new = |config| NewAgentTask { - project_root: None, - prompt: text("again"), - provider: provider.into(), - config, - isolation: Isolation::Local, - base: None, - }; - let fresh = mgr.create(new(vec![])).await.unwrap(); - wait_for(&mgr, &fresh.id, AgentStatus::Ready).await; - let mode = |mgr: &AgentManager, id: &str| { - mgr.task(id).unwrap().state.lock().task.config.iter().find(|o| o.id == "mode").unwrap().current.clone() - }; - assert_eq!(mode(&mgr, &fresh.id).as_deref(), Some(safe), "{provider}"); - assert!(!config::bypass_approvals(&mgr.task(&fresh.id).unwrap().state.lock().task.config)); - if provider == "cursor" { - assert_eq!( - mgr.task(&fresh.id) - .unwrap() - .state - .lock() - .task - .config - .iter() - .find(|o| o.id == config::CURSOR_APPROVAL) - .unwrap() - .current - .as_deref(), - Some("ask") - ); - } - assert!(!agent.calls.lock().iter().any(|c| c.starts_with("session/set_"))); - let explicit = - mgr.create(new(vec![AgentConfigChoice { id: "mode".into(), value: bypass.into() }])).await.unwrap(); - wait_for(&mgr, &explicit.id, AgentStatus::Ready).await; - assert_eq!(mode(&mgr, &explicit.id).as_deref(), Some(bypass)); - assert!(!config::bypass_approvals(&mgr.shared.known_config(provider))); - mgr.shutdown(); - let restored = manager(tmp.path(), agent.clone()); - assert_eq!(mode(&restored, &explicit.id).as_deref(), Some(bypass)); - restored.prompt(&explicit.id, text("again"), false).await.unwrap(); - wait_for(&restored, &explicit.id, AgentStatus::Ready).await; - assert_eq!(mode(&restored, &explicit.id).as_deref(), Some(bypass)); - assert_eq!(mode(&restored, &fresh.id).as_deref(), Some(safe)); - let next = restored.create(new(vec![])).await.unwrap(); - wait_for(&restored, &next.id, AgentStatus::Ready).await; - assert_eq!(mode(&restored, &next.id).as_deref(), Some(safe)); - restored.shutdown(); - } -} - #[tokio::test(flavor = "multi_thread")] async fn cursor_approval_switch_resolves_pending_and_survives_restart() { let tmp = tempfile::tempdir().unwrap(); @@ -1849,13 +1746,13 @@ async fn cursor_approval_switch_resolves_pending_and_survives_restart() { mgr.prompt(&task.id, text("Fix again"), false).await.unwrap(); wait_for(&mgr, &task.id, AgentStatus::Ready).await; assert!(!agent.calls.lock().iter().any(|c| c.contains("bypass"))); - assert!(!config::bypass_approvals(&mgr.shared.known_config("cursor"))); + assert!(config::bypass_approvals(&mgr.shared.known_config("cursor"))); mgr.set_config(&task.id, config::CURSOR_APPROVAL, "ask".into()).await.unwrap(); mgr.prompt(&task.id, text("Fix once more"), false).await.unwrap(); wait_for(&mgr, &task.id, AgentStatus::NeedsInput).await; mgr.cancel(&task.id, None).unwrap(); wait_for(&mgr, &task.id, AgentStatus::Ready).await; - // A task created without choosing approvals still asks. + // A task created without choosing approvals still asks; the New Task page sends the default. let other = mgr.create(new()).await.unwrap(); wait_for(&mgr, &other.id, AgentStatus::NeedsInput).await; mgr.cancel(&other.id, None).unwrap(); diff --git a/e2e/agentApproval.spec.ts b/e2e/agentApproval.spec.ts index 0ab43a684..a742cf887 100644 --- a/e2e/agentApproval.spec.ts +++ b/e2e/agentApproval.spec.ts @@ -148,66 +148,3 @@ for (const theme of ["dark", "light"] as const) { }); } } - -test("new tasks keep the displayed approval mode unless the user chooses Bypass", async ({ page }) => { - await page.goto("/"); - await page.getByRole("tree", { name: "Project" }).waitFor(); - await page.evaluate(async () => { - const path = "/src/features/agents/store.ts"; - const store = await import(path); - const ipcPath = "/src/platform/ipc.ts"; - const { commands } = await import(ipcPath); - commands.agentCreate = async (request: unknown) => { - (window as unknown as { taskRequest: unknown }).taskRequest = request; - return { status: "error", error: { kind: "invalid", message: "Verification only" } }; - }; - store.useAgents.setState({ - loaded: true, - providers: [ - { - id: "claude", - name: "Claude Code", - installed: true, - source: { kind: "builtin" }, - commands: [], - config: [ - { - id: "mode", - name: "Mode", - category: "mode", - current: "default", - via: "option", - values: [ - { value: "default", name: "Default" }, - { value: "bypassPermissions", name: "Bypass permissions" }, - ], - }, - ], - }, - ], - draft: store.savedDraft({}), - }); - store.openHome(); - }); - await expect(page.getByRole("button", { name: "Mode", exact: true })).toHaveText("Default"); - await page.getByRole("textbox").fill("Create a note"); - await page.getByRole("button", { name: "Start", exact: true }).click(); - await expect - .poll(() => page.evaluate(() => (window as unknown as { taskRequest: unknown }).taskRequest)) - .toMatchObject({ - provider: "claude", - config: [{ id: "mode", value: "default" }], - }); - await page.getByRole("button", { name: "Mode", exact: true }).click(); - await page.getByRole("option", { name: "Bypass permissions", exact: true }).click(); - await expect(page.getByRole("button", { name: "Mode", exact: true })).toHaveText("Bypass permissions"); - await page.getByRole("button", { name: "Start", exact: true }).click(); - await expect - .poll(() => page.evaluate(() => (window as unknown as { taskRequest: unknown }).taskRequest)) - .toMatchObject({ - config: [{ id: "mode", value: "bypassPermissions" }], - }); - expect(await page.evaluate(() => JSON.parse(localStorage.getItem("openidea.agents.draft.v2")!).config)).toEqual({ - mode: "bypassPermissions", - }); -}); diff --git a/src/features/agents/agents.test.ts b/src/features/agents/agents.test.ts index 3603e582a..bba042b28 100644 --- a/src/features/agents/agents.test.ts +++ b/src/features/agents/agents.test.ts @@ -3,6 +3,7 @@ import type { AgentConfigOption, AgentEntry, AgentStatus, AgentTask } from "@/bi import { applyCompletion, atEdgeLine, + bypassDefaults, canSend, completionAt, isPathQuery, @@ -202,8 +203,7 @@ describe("agent composer", () => { "https://agentclientprotocol.com/protocol/session-modes#autopilot", ].map(modeIcon), ).toEqual(["lock", "edit", "todo", "run", "unlock", "edit", "eye", "lock", "run", "run"]); - expect(permissions.current).toBe("off"); - expect(approvals.current).toBe("ask"); + expect(bypassDefaults([...config, permissions])).toEqual({ allow_all: "on" }); expect(usageLabel({ used: 24_000, size: 200_000 })).toEqual({ short: "12%", detail: "24k / 200k tokens", diff --git a/src/features/agents/composerCommands.test.tsx b/src/features/agents/composerCommands.test.tsx index 9c3d884ad..9e3113835 100644 --- a/src/features/agents/composerCommands.test.tsx +++ b/src/features/agents/composerCommands.test.tsx @@ -33,7 +33,7 @@ import { defaultTaskProject } from "./project"; import { HomeEditor } from "./HomeEditor"; import composerStyles from "./Composer.module.css"; import tagStyles from "@/ui/InlineTag.module.css"; -import { HOME, savedDraft, startTask, updateComposer, useAgents } from "./store"; +import { HOME, updateComposer, useAgents } from "./store"; import { emptyDraft } from "./composerModel"; const command = (name: string, description: string, source: "command" | "skill" = "command"): AgentCommand => ({ @@ -264,46 +264,3 @@ describe("workspace task project", () => { await vi.dynamicImportSettled(); }); }); - -describe("new task permissions", () => { - it.each([ - ["claude", "mode", "default", "bypassPermissions"], - ["codex", "mode", "auto", "full-access"], - ["cursor", "approval", "ask", "bypass"], - ["copilot", "permissions", "off", "on"], - ["custom", "mode", "ask", "yolo"], - ])("keeps %s's offered default and sends an explicit Bypass choice", async (provider, category, safe, bypass) => { - const offered = { - ...claude, - id: provider, - config: [ - { - id: category, - name: category, - description: null, - category, - current: safe, - via: "option" as const, - values: [safe, bypass].map((value) => ({ value, name: value, description: null })), - }, - ], - }; - const draft = savedDraft({ provider }); - useAgents.setState({ providers: [offered], draft }); - updateComposer(HOME, { text: "Create a note" }); - await expect(startTask()).rejects.toThrow("stop here"); - expect(agentCreate).toHaveBeenLastCalledWith(expect.objectContaining({ config: [{ id: category, value: safe }] })); - useAgents.setState({ providers: [{ ...offered, config: [{ ...offered.config[0]!, current: null }] }] }); - await expect(startTask()).rejects.toThrow("stop here"); - expect(agentCreate).toHaveBeenLastCalledWith(expect.objectContaining({ config: [] })); - // Reloading the saved draft preserves the user's explicit permission choice. - useAgents.setState({ draft: savedDraft(JSON.parse(JSON.stringify({ ...draft, config: { [category]: bypass } }))) }); - await expect(startTask("worktree")).rejects.toThrow("stop here"); - expect(agentCreate).toHaveBeenLastCalledWith( - expect.objectContaining({ - isolation: "worktree", - config: [{ id: category, value: bypass }], - }), - ); - }); -}); diff --git a/src/features/agents/composerModel.ts b/src/features/agents/composerModel.ts index 0b07e1d96..c9b9caf8e 100644 --- a/src/features/agents/composerModel.ts +++ b/src/features/agents/composerModel.ts @@ -198,6 +198,20 @@ export function modeIcon(mode: string): string { return "lock"; } +/** The settings that approve everything without asking (a bypass mode, or approvals), by option id. */ +export function bypassDefaults(config: readonly AgentConfigOption[]): Record { + const out: Record = {}; + const mode = modeOption(config); + const unlock = mode?.values.find((v) => modeIcon(v.value) === "unlock"); + if (mode && unlock) out[mode.id] = unlock.value; + const permissions = config.find((o) => o.category === "permissions"); + const allowAll = permissions?.values.find((v) => /^(on|true)$/i.test(v.value)); + if (permissions && allowAll) out[permissions.id] = allowAll.value; + const approval = config.find((o) => o.category === "approval"); + if (approval?.values.some((v) => v.value === "bypass")) out[approval.id] = "bypass"; + return out; +} + /** Shift+Tab: the mode after the current one. */ export function nextValue(option: AgentConfigOption, current = option.current): string | null { if (!option.values.length) return null; diff --git a/src/features/agents/store.ts b/src/features/agents/store.ts index 8ae912e57..934e106e6 100644 --- a/src/features/agents/store.ts +++ b/src/features/agents/store.ts @@ -20,7 +20,7 @@ import { call, commands, isTauri } from "@/platform/ipc"; import { editorService, notifications, notifyTabTitlesChanged, toolWindowService } from "@/workbench/contributions"; import { tx } from "@/platform/i18n"; import { confirm } from "@/workbench/messages"; -import { canSend, emptyDraft, toPrompt, type ComposerDraft, type FileRef } from "./composerModel"; +import { bypassDefaults, canSend, emptyDraft, toPrompt, type ComposerDraft, type FileRef } from "./composerModel"; import { withQuote } from "./conversation"; import { HOME_KEY, @@ -538,8 +538,12 @@ export async function startTask( providers.find((p) => p.id === draft.provider), draft, ); + const defaults = bypassDefaults(options); const config = options.flatMap((o) => { - const value = o.current; + const value = + o.category === "model" || o.category === "thought_level" + ? o.current + : (draft.config[o.id] ?? defaults[o.id] ?? null); return value !== null && o.values.some((v) => v.value === value) ? [{ id: o.id, value }] : []; }); const task = await call(