дзеркало
https://github.com/l0ng-ai/openidea.git
синхронізовано 2026-11-09 11:15:40 +02:00
Revert "fix(agents): stop defaulting new tasks to bypass" (#2234)
Reverts #2232: new agent tasks default to Bypass again.
Цей коміт міститься в:
1 джерело
73b3a99699
коміт
60b19b2ffb
8 files changed
+52
-254
No files matched your search
@@ -39,31 +39,19 @@ pub(crate) fn add_client_options(provider: &str, config: &mut Vec<AgentConfigOpt
|
||||
/// Whether the current settings approve tool requests without asking: a bypass mode,
|
||||
/// an allow-all permissions option, or a bypass approval policy.
|
||||
pub(crate) fn bypass_approvals(config: &[AgentConfigOption]) -> 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": [{
|
||||
|
||||
@@ -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<String, Known> =
|
||||
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,
|
||||
|
||||
@@ -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<String, Known> = 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();
|
||||
|
||||
@@ -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",
|
||||
});
|
||||
});
|
||||
@@ -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",
|
||||
|
||||
@@ -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 }],
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -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<string, string> {
|
||||
const out: Record<string, string> = {};
|
||||
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;
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in new issue
Заблокувати користувача