From 6722ea600ef30eac7149bd49eb9b3b64dd207e75 Mon Sep 17 00:00:00 2001 From: webreflection Date: Wed, 26 Aug 2026 10:56:40 +0200 Subject: [PATCH] fix(vscode) Removed top level Auto-Approve permission on onboarding --- .changeset/onboarding-tool-permissions.md | 5 ++++ .../src/shared/work-style-presets.ts | 1 - .../tests/unit/permission-editor.test.ts | 30 +++++++++++++++++++ .../tests/unit/work-style-apply.test.ts | 18 ++++++++++- .../tests/unit/work-style-presets.test.ts | 8 ++++- .../components/settings/PermissionEditor.tsx | 6 ++-- .../components/settings/permission-utils.ts | 8 +++++ 7 files changed, 69 insertions(+), 7 deletions(-) create mode 100644 .changeset/onboarding-tool-permissions.md diff --git a/.changeset/onboarding-tool-permissions.md b/.changeset/onboarding-tool-permissions.md new file mode 100644 index 000000000000..90715f0641af --- /dev/null +++ b/.changeset/onboarding-tool-permissions.md @@ -0,0 +1,5 @@ +--- +"kilo-code": patch +--- + +Keep Review-first onboarding permissions editable in Auto-Approve settings. diff --git a/packages/kilo-vscode/src/shared/work-style-presets.ts b/packages/kilo-vscode/src/shared/work-style-presets.ts index 4bcfecc6748a..2c4d64918ca8 100644 --- a/packages/kilo-vscode/src/shared/work-style-presets.ts +++ b/packages/kilo-vscode/src/shared/work-style-presets.ts @@ -67,7 +67,6 @@ export const WORK_STYLE_PRESETS: Record = { terminal_command_display: "expanded", auto_collapse_reasoning: false, permission: { - "*": "ask", read: { "*": "allow", "*.env": "ask", diff --git a/packages/kilo-vscode/tests/unit/permission-editor.test.ts b/packages/kilo-vscode/tests/unit/permission-editor.test.ts index ce32cbd190b1..e4af397df918 100644 --- a/packages/kilo-vscode/tests/unit/permission-editor.test.ts +++ b/packages/kilo-vscode/tests/unit/permission-editor.test.ts @@ -4,6 +4,7 @@ import { clearGroupedPatch, clearWildcardPatch, DEFAULT_RULES, + effectiveConfigLevel, inheritedWildcard, mostRestrictive, permissionExceptions, @@ -15,6 +16,8 @@ import { wildcardAction, effectiveRuleLevel, } from "../../webview-ui/src/components/settings/permission-utils" +import { ConfigState } from "../../webview-ui/src/utils/config-utils" +import { WORK_STYLE_PRESETS } from "../../src/shared/work-style-presets" import type { PermissionRule, PermissionRuleItem } from "../../webview-ui/src/types/messages" describe("effectiveRuleLevel", () => { @@ -47,6 +50,33 @@ describe("effectiveRuleLevel", () => { }) }) +describe("Auto-Approve onboarding state", () => { + const permission = WORK_STYLE_PRESETS["human-in-the-loop"].config.permission ?? {} + + it("reflects onboarding permissions in Auto-Approve", () => { + expect(effectiveConfigLevel(permission, "edit")).toBe("ask") + expect(effectiveConfigLevel(permission, "bash")).toBe("ask") + expect(effectiveConfigLevel(permission, "glob")).toBe("allow") + expect(effectiveConfigLevel(permission, "grep")).toBe("allow") + }) + + it("keeps an Auto-Approve change selected after saving", () => { + const state = new ConfigState() + state.handleConfigLoaded({ permission }) + + expect(effectiveConfigLevel(state.config.permission ?? {}, "edit")).toBe("ask") + + state.updateConfig({ permission: { edit: "allow" } }) + expect(effectiveConfigLevel(state.config.permission ?? {}, "edit")).toBe("allow") + expect(state.draft).toEqual({ permission: { edit: "allow" } }) + + state.saveConfig() + state.handleConfigUpdated(state.config) + expect(effectiveConfigLevel(state.config.permission ?? {}, "edit")).toBe("allow") + expect(state.dirty).toBe(false) + }) +}) + describe("ruleset", () => { it("preserves backend defaults when config only customizes bash", () => { const rules = [...DEFAULT_RULES, ...ruleset({ bash: { "*": "ask", "git status *": "allow" } })] diff --git a/packages/kilo-vscode/tests/unit/work-style-apply.test.ts b/packages/kilo-vscode/tests/unit/work-style-apply.test.ts index 2822494cc3aa..e958f5878c86 100644 --- a/packages/kilo-vscode/tests/unit/work-style-apply.test.ts +++ b/packages/kilo-vscode/tests/unit/work-style-apply.test.ts @@ -10,6 +10,7 @@ function setup(input?: { }) { const settings = new Map([["agentWorkStyle", "unset"]]) const events: string[] = [] + const patches: WorkStyleConfig[] = [] const store: WorkStyleStore = { read: async () => input?.config ?? {}, inspect: (key) => ({ @@ -23,10 +24,11 @@ function setup(input?: { }, patch: async (config) => { events.push(`patch:${Object.keys(config).sort().join(",")}`) + patches.push(config) if (input?.failPatch) throw new Error("Failed to patch config") }, } - return { store, settings, events } + return { store, settings, events, patches } } describe("applyWorkStyle", () => { @@ -45,6 +47,20 @@ describe("applyWorkStyle", () => { ]) }) + it("does not write a top-level permission choice when creating global config", async () => { + const state = setup({ config: {} }) + + expect(await applyWorkStyle("human-in-the-loop", state.store)).toEqual({ ok: true }) + expect(state.patches).toHaveLength(1) + expect(["ask", "allow", "deny"]).not.toContain(state.patches[0].permission?.["*"]) + expect(state.patches[0].permission).toMatchObject({ + edit: "ask", + glob: "allow", + grep: "allow", + bash: { "*": "ask" }, + }) + }) + it("rolls extension settings back when the CLI config update fails", async () => { const state = setup({ failPatch: true }) diff --git a/packages/kilo-vscode/tests/unit/work-style-presets.test.ts b/packages/kilo-vscode/tests/unit/work-style-presets.test.ts index f34df1f9eddc..c0ef7bc0798a 100644 --- a/packages/kilo-vscode/tests/unit/work-style-presets.test.ts +++ b/packages/kilo-vscode/tests/unit/work-style-presets.test.ts @@ -15,7 +15,7 @@ describe("work style presets", () => { const bash = cfg.permission?.bash as Record expect(cfg.terminal_command_display).toBe("expanded") expect(cfg.auto_collapse_reasoning).toBe(false) - expect(cfg.permission?.["*"]).toBe("ask") + expect(cfg.permission?.["*"]).toBeUndefined() expect(cfg.permission?.edit).toBe("ask") expect(bash).toMatchObject({ "*": "ask", "rg *": "allow", "*>*": "ask" }) expect(Object.keys(bash).at(-1)).toBe("*>*") @@ -50,6 +50,12 @@ describe("work style presets", () => { }) }) + it("never defines a top-level permission choice in onboarding presets", () => { + for (const preset of Object.values(WORK_STYLE_PRESETS)) { + expect(["ask", "allow", "deny"]).not.toContain(preset.config.permission?.["*"]) + } + }) + it("does not overwrite existing new-user settings", () => { const plan = buildWorkStyleApplyPlan({ style: "human-in-the-loop", diff --git a/packages/kilo-vscode/webview-ui/src/components/settings/PermissionEditor.tsx b/packages/kilo-vscode/webview-ui/src/components/settings/PermissionEditor.tsx index 97e6f3d0328d..0d401a0e7aae 100644 --- a/packages/kilo-vscode/webview-ui/src/components/settings/PermissionEditor.tsx +++ b/packages/kilo-vscode/webview-ui/src/components/settings/PermissionEditor.tsx @@ -9,12 +9,11 @@ import { addExceptionPatch, clearGroupedPatch, clearWildcardPatch, - effectiveRuleLevel, + effectiveConfigLevel, inheritedWildcard, mostRestrictive, permissionExceptions, removeExceptionPatch, - ruleset, setExceptionPatch, setGroupedPatch, setWildcardPatch, @@ -140,9 +139,8 @@ const PermissionEditor: Component<{ onChange: (patch: PermissionPatch) => void }> = (props) => { const perms = createMemo(() => props.permissions ?? {}) - const rules = createMemo(() => [...(props.rules ?? []), ...ruleset(perms())]) - const levelFor = (tool: string): PermissionLevel => effectiveRuleLevel(rules(), tool) + const levelFor = (tool: string): PermissionLevel => effectiveConfigLevel(perms(), tool, props.rules) const ruleFor = (tool: string): PermissionRule | undefined => perms()[tool] diff --git a/packages/kilo-vscode/webview-ui/src/components/settings/permission-utils.ts b/packages/kilo-vscode/webview-ui/src/components/settings/permission-utils.ts index ae8807b5e856..cf23ec3e9686 100644 --- a/packages/kilo-vscode/webview-ui/src/components/settings/permission-utils.ts +++ b/packages/kilo-vscode/webview-ui/src/components/settings/permission-utils.ts @@ -28,6 +28,14 @@ export function effectiveRuleLevel(rules: PermissionRuleItem[] | undefined, tool return "ask" } +export function effectiveConfigLevel( + config: PermissionConfig, + tool: string, + defaults: PermissionRuleItem[] = DEFAULT_RULES, +): PermissionLevel { + return effectiveRuleLevel([...defaults, ...ruleset(config)], tool) +} + export function ruleset(config: PermissionConfig): PermissionRuleItem[] { const result: PermissionRuleItem[] = [] for (const [permission, rule] of Object.entries(config)) {