From 5dc511b4e91939d998378bba2328d8223c015c30 Mon Sep 17 00:00:00 2001 From: Marius Date: Tue, 23 Jun 2026 12:39:39 +0200 Subject: [PATCH] fix(vscode): silence auto-approved permissions (#11573) --- .changeset/quiet-auto-approved-permissions.md | 5 ++ .../src/commands/toggle-auto-approve.ts | 41 +++++++----- packages/kilo-vscode/src/extension.ts | 11 ++-- .../src/services/attention/service.ts | 36 +++++++--- .../kilo-vscode/tests/unit/attention.test.ts | 34 +++++++++- .../tests/unit/auto-approve.test.ts | 66 +++++++++++-------- 6 files changed, 137 insertions(+), 56 deletions(-) create mode 100644 .changeset/quiet-auto-approved-permissions.md diff --git a/.changeset/quiet-auto-approved-permissions.md b/.changeset/quiet-auto-approved-permissions.md new file mode 100644 index 0000000000..c56014978c --- /dev/null +++ b/.changeset/quiet-auto-approved-permissions.md @@ -0,0 +1,5 @@ +--- +"kilo-code": patch +--- + +Keep attention sounds silent for permission requests handled by auto-approve. diff --git a/packages/kilo-vscode/src/commands/toggle-auto-approve.ts b/packages/kilo-vscode/src/commands/toggle-auto-approve.ts index 21eca5f8ce..a94873fc21 100644 --- a/packages/kilo-vscode/src/commands/toggle-auto-approve.ts +++ b/packages/kilo-vscode/src/commands/toggle-auto-approve.ts @@ -1,5 +1,5 @@ import * as vscode from "vscode" -import type { KiloClient } from "@kilocode/sdk/v2/client" +import type { Event, KiloClient } from "@kilocode/sdk/v2/client" import type { KiloConnectionService } from "../services/cli-backend/connection-service" /** @@ -13,9 +13,11 @@ export type DirectoryResolver = (sessionId?: string) => string * (workspace root + all registered worktree paths). */ export type AllDirectories = () => string[] +type Asked = Extract export interface AutoApproveController { active(): boolean + approve(event: Asked, directory?: string): Promise toggle(): Promise onChange(listener: (active: boolean) => void): { dispose(): void } } @@ -26,9 +28,9 @@ const KEY = "enabled" /** * Runtime auto-accept toggle for permissions. * - * Instead of writing to the config file, we intercept `permission.asked` SSE - * events and auto-reply "once" to each. This avoids config-layer issues - * (merged vs global, sparse defaults) and works even when the sidebar is closed. + * Instead of writing to the CLI config, the attention coordinator delegates + * `permission.asked` events here and auto-replies "once". This avoids config-layer + * issues (merged vs global, sparse defaults) and works even when the sidebar is closed. */ export function registerToggleAutoApprove( context: vscode.ExtensionContext, @@ -71,9 +73,11 @@ export function registerToggleAutoApprove( const { data: pending } = await client.permission.list({ directory: dir }, { throwOnError: true }) for (const req of pending) { if (generation !== snapshot) break - await client.permission.reply({ requestID: req.id, directory: dir, reply: "once" }).catch((err) => { - console.error("[Kilo New] toggleAutoApprove: failed to drain pending:", err) - }) + await client.permission + .reply({ requestID: req.id, directory: dir, reply: "once" }, { throwOnError: true }) + .catch((err) => { + console.error("[Kilo New] toggleAutoApprove: failed to drain pending:", err) + }) } } catch (err) { console.error("[Kilo New] toggleAutoApprove: failed to list pending permissions:", err) @@ -83,19 +87,23 @@ export function registerToggleAutoApprove( return active } - const unsubscribe = connectionService.onEvent((event, directory) => { - if (!active) return - if (event.type !== "permission.asked") return + const approve = async (event: Asked, directory?: string) => { + if (!active) return false const client = tryGetClient(connectionService) - if (!client) return + if (!client) return false const dir = directory ?? connectionService.getPermissionDirectory(event.properties.id) ?? resolve(event.properties.sessionID) - client.permission.reply({ requestID: event.properties.id, directory: dir, reply: "once" }).catch((err) => { - console.error("[Kilo New] toggleAutoApprove: failed to auto-reply:", err) - }) - }) + return client.permission + .reply({ requestID: event.properties.id, directory: dir, reply: "once" }, { throwOnError: true }) + .then( + () => true, + (err) => { + console.error("[Kilo New] toggleAutoApprove: failed to auto-reply:", err) + return false + }, + ) + } - context.subscriptions.push({ dispose: unsubscribe }) context.subscriptions.push( vscode.workspace.onDidChangeConfiguration((event) => { if (!event.affectsConfiguration(`${CONFIG}.${KEY}`)) return @@ -111,6 +119,7 @@ export function registerToggleAutoApprove( return { active: () => active, + approve, toggle, onChange(listener) { listeners.add(listener) diff --git a/packages/kilo-vscode/src/extension.ts b/packages/kilo-vscode/src/extension.ts index 5c99e181cd..55cd1480d9 100644 --- a/packages/kilo-vscode/src/extension.ts +++ b/packages/kilo-vscode/src/extension.ts @@ -49,7 +49,6 @@ export function activate(context: vscode.ExtensionContext) { // Create shared connection service (one server for all webviews) const connectionService = new KiloConnectionService(context) - const attention = new AttentionService(connectionService) let restore = context.workspaceState.get(RESTORE_KEY) ?? {} const remember = (patch: RestoreState) => { const next = { ...restore, ...patch } @@ -103,9 +102,6 @@ export function activate(context: vscode.ExtensionContext) { }), ) - // Prewarm the CLI backend early so autocomplete is ready before first editor use. - ensureBackendForAutocomplete(connectionService) - for (const folder of vscode.workspace.workspaceFolders ?? []) { void markWorkspace(folder.uri.fsPath, (msg) => console.warn(`[Kilo New] ${msg}`)) } @@ -181,6 +177,13 @@ export function activate(context: vscode.ExtensionContext) { return [...dirs] }, ) + const attention = new AttentionService(connectionService, { + approve: (event, directory) => autoApprove.approve(event, directory), + }) + + // Prewarm only after all global event consumers are ready. + ensureBackendForAutocomplete(connectionService) + provider.setAutoApproveController(autoApprove) agentManagerHost.setAutoApproveController(autoApprove) diff --git a/packages/kilo-vscode/src/services/attention/service.ts b/packages/kilo-vscode/src/services/attention/service.ts index 19bbe7bd7d..e70c1310dc 100644 --- a/packages/kilo-vscode/src/services/attention/service.ts +++ b/packages/kilo-vscode/src/services/attention/service.ts @@ -7,10 +7,15 @@ import { playSound, resolveSoundID } from "./sound" type Sync = Extract type Question = Extract type Permission = Extract +type Asked = Extract type Status = Extract type Close = Extract type Error = Extract +type Options = { + approve?: (event: Asked, directory?: string) => boolean | Promise +} + export function previewSound(value: string) { void playSound("default", resolveSoundID(value)) } @@ -23,8 +28,11 @@ export class AttentionService implements vscode.Disposable { private readonly unsubscribeEvent: () => void private readonly unsubscribeState: () => void - constructor(connection: KiloConnectionService) { - this.unsubscribeEvent = connection.onEvent((event) => this.handle(event)) + constructor( + connection: KiloConnectionService, + private readonly opts: Options = {}, + ) { + this.unsubscribeEvent = connection.onEvent((event, directory) => this.handle(event, directory)) this.unsubscribeState = connection.onStateChange((state) => { if (state === "error" || state === "disconnected") this.reset() }) @@ -36,12 +44,14 @@ export class AttentionService implements vscode.Disposable { this.reset() } - private handle(event: SSEPayload) { + private handle(event: SSEPayload, directory?: string) { if (event.type === "sync") return this.sync(event) if (event.type === "question.asked" || event.type === "question.replied" || event.type === "question.rejected") { return this.question(event) } - if (event.type === "permission.asked" || event.type === "permission.replied") return this.permission(event) + if (event.type === "permission.asked" || event.type === "permission.replied") { + return this.permission(event, directory) + } if (event.type === "session.deleted") return this.remove(event.properties.sessionID) if (event.type === "session.status") return this.status(event) if (event.type === "session.turn.close") return this.close(event) @@ -68,14 +78,24 @@ export class AttentionService implements vscode.Disposable { this.notify("question") } - private permission(event: Permission) { + private permission(event: Permission, directory?: string) { if (event.type !== "permission.asked") { this.permissions.delete(event.properties.requestID) return } - if (this.permissions.has(event.properties.id)) return - this.permissions.add(event.properties.id) - this.notify("permission") + const id = event.properties.id + if (this.permissions.has(id)) return + this.permissions.add(id) + const alert = () => { + if (!this.permissions.has(id)) return + this.notify("permission") + } + const approval = this.opts.approve?.(event, directory) + if (approval === true) return + if (approval === false || approval === undefined) return alert() + void approval.then((handled) => { + if (!handled) alert() + }, alert) } private status(event: Status) { diff --git a/packages/kilo-vscode/tests/unit/attention.test.ts b/packages/kilo-vscode/tests/unit/attention.test.ts index b25eb2a92c..2e3acb935d 100644 --- a/packages/kilo-vscode/tests/unit/attention.test.ts +++ b/packages/kilo-vscode/tests/unit/attention.test.ts @@ -5,7 +5,7 @@ import type { KiloConnectionService } from "../../src/services/cli-backend/conne import type { SSEPayload } from "../../src/services/cli-backend/sdk-sse-adapter" import { CustomSoundIDs, resolveSoundID } from "../../src/services/attention/sound" -function setup() { +function setup(opts: { approve?: () => boolean | Promise } = {}) { const sounds: TuiAttentionSoundName[] = [] const events: Array<(event: SSEPayload) => void> = [] const states: Array<(state: "connecting" | "connected" | "disconnected" | "error") => void> = [] @@ -19,7 +19,7 @@ function setup() { return () => undefined }, } as unknown as KiloConnectionService - const service = new AttentionService(connection) + const service = new AttentionService(connection, opts) ;(service as unknown as { notify: (sound: TuiAttentionSoundName) => void }).notify = (sound) => sounds.push(sound) return { sounds, @@ -75,6 +75,36 @@ describe("AttentionService", () => { test.service.dispose() }) + it("stays silent for auto-approved permission requests", () => { + const test = setup({ approve: () => true }) + test.event(event({ type: "permission.asked", properties: { id: "p1", sessionID: "s1" } })) + test.event(event({ type: "permission.replied", properties: { requestID: "p1", sessionID: "s1" } })) + + expect(test.sounds).toEqual([]) + test.service.dispose() + }) + + it("plays attention when auto-approval fails and the request remains pending", async () => { + const test = setup({ approve: async () => false }) + test.event(event({ type: "permission.asked", properties: { id: "p1", sessionID: "s1" } })) + await Bun.sleep(0) + + expect(test.sounds).toEqual(["permission"]) + test.service.dispose() + }) + + it("stays silent when a permission resolves before auto-approval failure settles", async () => { + const approval = Promise.withResolvers() + const test = setup({ approve: () => approval.promise }) + test.event(event({ type: "permission.asked", properties: { id: "p1", sessionID: "s1" } })) + test.event(event({ type: "permission.replied", properties: { requestID: "p1", sessionID: "s1" } })) + approval.resolve(false) + await Bun.sleep(0) + + expect(test.sounds).toEqual([]) + test.service.dispose() + }) + it("plays the error sound and suppresses the following completion", () => { const test = setup() test.event(event({ type: "session.status", properties: { sessionID: "s1", status: { type: "busy" } } })) diff --git a/packages/kilo-vscode/tests/unit/auto-approve.test.ts b/packages/kilo-vscode/tests/unit/auto-approve.test.ts index a10542b9ff..2f66b71c71 100644 --- a/packages/kilo-vscode/tests/unit/auto-approve.test.ts +++ b/packages/kilo-vscode/tests/unit/auto-approve.test.ts @@ -80,44 +80,37 @@ function context() { } function connection(client: KiloClient | null, dirs = new Map()) { - const listeners: Array<(event: Event, directory?: string) => void> = [] const svc = { getClient: () => { if (!client) throw new Error("not connected") return client }, - onEvent: (listener: (event: Event, directory?: string) => void) => { - listeners.push(listener) - return () => { - const index = listeners.indexOf(listener) - if (index >= 0) listeners.splice(index, 1) - } - }, getPermissionDirectory: (id: string) => dirs.get(id), } as unknown as KiloConnectionService - return { - svc, - emit(event: Event, directory?: string) { - for (const listener of listeners) listener(event, directory) - }, - } + return { svc } } function client(opts: { list?: (dir: string) => Promise<{ data: Permission[] }> - reply?: (args: { requestID: string; directory: string; reply: "once" }) => Promise + reply?: ( + args: { requestID: string; directory: string; reply: "once" }, + options?: { throwOnError?: boolean }, + ) => Promise }) { return { permission: { list: async (args: { directory: string }) => opts.list?.(args.directory) ?? { data: [] }, - reply: async (args: { requestID: string; directory: string; reply: "once" }) => opts.reply?.(args), + reply: async ( + args: { requestID: string; directory: string; reply: "once" }, + options?: { throwOnError?: boolean }, + ) => opts.reply?.(args, options), }, } as unknown as KiloClient } function asked(id: string, sessionID = "ses_1") { - return { type: "permission.asked", properties: { id, sessionID } } as Event + return { type: "permission.asked", properties: { id, sessionID } } as Extract } describe("registerToggleAutoApprove", () => { @@ -135,7 +128,7 @@ describe("registerToggleAutoApprove", () => { ctrl.onChange((active) => changes.push(active)) expect(ctrl.active()).toBe(true) - conn.emit(asked("perm_1")) + expect(await ctrl.approve(asked("perm_1"))).toBe(true) expect(replies).toEqual([{ requestID: "perm_1", directory: "/repo/ses_1", reply: "once" }]) env.active = false @@ -143,7 +136,7 @@ describe("registerToggleAutoApprove", () => { expect(ctrl.active()).toBe(false) expect(changes).toEqual([false]) - conn.emit(asked("perm_2")) + expect(await ctrl.approve(asked("perm_2"))).toBe(false) expect(replies).toHaveLength(1) await ctrl.toggle() @@ -153,19 +146,19 @@ describe("registerToggleAutoApprove", () => { expect(env.messages).toContain("Auto-approve enabled") }) - it("uses the SSE directory for worktree permissions before session mappings are available", () => { + it("uses the SSE directory for worktree permissions before session mappings are available", async () => { config(true) const replies: unknown[] = [] const conn = connection(client({ reply: async (args) => replies.push(args) })) - registerToggleAutoApprove( + const ctrl = registerToggleAutoApprove( context(), conn.svc, () => "/workspace", () => ["/workspace"], ) - conn.emit(asked("perm_worktree", "ses_worktree"), "/workspace/.kilo/worktrees/feature") - conn.emit(asked("perm_child", "ses_child"), "/workspace/.kilo/worktrees/feature") + await ctrl.approve(asked("perm_worktree", "ses_worktree"), "/workspace/.kilo/worktrees/feature") + await ctrl.approve(asked("perm_child", "ses_child"), "/workspace/.kilo/worktrees/feature") expect(replies).toEqual([ { requestID: "perm_worktree", directory: "/workspace/.kilo/worktrees/feature", reply: "once" }, @@ -173,27 +166,47 @@ describe("registerToggleAutoApprove", () => { ]) }) - it("uses the shared permission directory before falling back to session mappings", () => { + it("uses the shared permission directory before falling back to session mappings", async () => { config(true) const replies: unknown[] = [] const conn = connection( client({ reply: async (args) => replies.push(args) }), new Map([["perm_shared", "/workspace/.kilo/worktrees/shared"]]), ) - registerToggleAutoApprove( + const ctrl = registerToggleAutoApprove( context(), conn.svc, () => "/workspace", () => ["/workspace"], ) - conn.emit(asked("perm_shared", "ses_child")) + await ctrl.approve(asked("perm_shared", "ses_child")) expect(replies).toEqual([ { requestID: "perm_shared", directory: "/workspace/.kilo/worktrees/shared", reply: "once" }, ]) }) + it("returns unhandled when an automatic reply fails", async () => { + config(true) + const conn = connection( + client({ + reply: async (_args, options) => { + expect(options).toEqual({ throwOnError: true }) + throw new Error("offline") + }, + }), + ) + const ctrl = registerToggleAutoApprove( + context(), + conn.svc, + () => "/workspace", + () => ["/workspace"], + ) + + expect(await ctrl.approve(asked("perm_1"))).toBe(false) + }) + it("cancels pending permission drains when disabled during an enable generation", async () => { config(false) const gate = defer<{ data: Permission[] }>() @@ -240,6 +253,7 @@ describe("createAutoApproveBridge", () => { const state = { active: false } const ctrl: AutoApproveController = { active: () => state.active, + approve: async () => false, toggle: async () => { state.active = !state.active for (const listener of listeners) listener(state.active)