From 458d2f30930b44de1841974c003db7d153edcc14 Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Tue, 23 Dec 2025 20:12:35 +0100 Subject: [PATCH] Share permissions of extension with CLI and agent manager --- cli/src/commands/__tests__/theme.test.ts | 168 +------------ .../config/__tests__/auto-approval.test.ts | 8 +- .../config/__tests__/env-config-json.test.ts | 139 +++++++++++ cli/src/config/__tests__/persistence.test.ts | 50 ++++ cli/src/config/defaults.ts | 6 +- cli/src/config/env-config.ts | 43 ++++ cli/src/config/env-utils.ts | 8 + cli/src/config/persistence.ts | 38 +-- cli/src/host/ExtensionHost.ts | 143 +++++++---- .../ExtensionHost.raceCondition.test.ts | 97 ++++++++ cli/src/state/atoms/__tests__/config.test.ts | 12 +- .../atoms/__tests__/modelValidation.test.ts | 10 +- cli/src/state/atoms/config.ts | 23 ++ .../__tests__/useStdinJsonHandler.test.ts | 34 +++ cli/src/state/hooks/useStdinJsonHandler.ts | 18 +- .../agent-manager/AgentManagerProvider.ts | 112 ++++++++- .../kilocode/agent-manager/CliArgsBuilder.ts | 17 +- .../agent-manager/CliProcessHandler.ts | 38 ++- .../agent-manager/CliSessionLauncher.ts | 12 + .../AgentManagerProvider.ipc.spec.ts | 79 +++++- .../__tests__/CliArgsBuilder.spec.ts | 45 +++- .../__tests__/CliArgsBuilder.test.ts | 26 ++ .../__tests__/CliProcessHandler.spec.ts | 5 +- .../__tests__/autoApprovalEnv.spec.ts | 232 ++++++++++++++++++ .../kilocode/agent-manager/autoApprovalEnv.ts | 110 +++++++++ src/core/webview/ClineProvider.ts | 4 +- .../webview/__tests__/ClineProvider.spec.ts | 42 +++- .../src/i18n/locales/en/agentManager.json | 4 +- .../agent-manager/components/MessageList.tsx | 71 +++++- .../components/__tests__/MessageList.spec.tsx | 88 ++++++- 30 files changed, 1414 insertions(+), 268 deletions(-) create mode 100644 cli/src/config/__tests__/env-config-json.test.ts create mode 100644 src/core/kilocode/agent-manager/__tests__/autoApprovalEnv.spec.ts create mode 100644 src/core/kilocode/agent-manager/autoApprovalEnv.ts diff --git a/cli/src/commands/__tests__/theme.test.ts b/cli/src/commands/__tests__/theme.test.ts index 2c1c15d935e..56a58b590fc 100644 --- a/cli/src/commands/__tests__/theme.test.ts +++ b/cli/src/commands/__tests__/theme.test.ts @@ -97,162 +97,20 @@ describe("/theme command", () => { refreshTerminalMock = vi.fn().mockResolvedValue(undefined) // Create mock config - mockConfig = { - version: "1.0.0", - mode: "code", - telemetry: true, - provider: "test-provider", - providers: [], - customThemes: { - "custom-theme": mockTheme, - }, - } - - // Mock config loading - vi.doMock("../../config/persistence.js", () => ({ - loadConfig: vi.fn().mockResolvedValue({ - config: { - customThemes: { - "custom-theme": mockTheme, - }, - }, - }), - })) - - // Mock the constants/themes/index.js functions - vi.doMock("../../constants/themes/index.js", () => ({ - getAvailableThemes: vi.fn(() => [ - "alpha", - "dark", - "dracula", - "github-dark", - "light", - "github-light", - "custom-theme", - ]), - getThemeById: vi.fn((id: string) => { - const themes: Record = { - dark: { - id: "dark", - name: "Dark", - type: "dark", - brand: { primary: "#3b82f6", secondary: "#1d4ed8" }, - semantic: { - success: "#4ade80", - error: "#f87171", - warning: "#fbbf24", - info: "#60a5fa", - neutral: "#6b7280", - }, - interactive: { - prompt: "#3b82f6", - selection: "#1e40af", - hover: "#2563eb", - disabled: "#9ca3af", - focus: "#1d4ed8", - }, - messages: { user: "#10b981", assistant: "#8b5cf6", system: "#f59e0b", error: "#ef4444" }, - actions: { approve: "#10b981", reject: "#ef4444", cancel: "#6b7280", pending: "#f59e0b" }, - code: { - addition: "#10b981", - deletion: "#ef4444", - modification: "#f59e0b", - context: "#6b7280", - lineNumber: "#9ca3af", - }, - ui: { - border: { default: "#374151", active: "#3b82f6", warning: "#f59e0b", error: "#ef4444" }, - text: { primary: "#f9fafb", secondary: "#d1d5db", dimmed: "#9ca3af", highlight: "#fbbf24" }, - background: { default: "#111827", elevated: "#1f2937" }, - }, - status: { online: "#10b981", offline: "#6b7280", busy: "#f59e0b", idle: "#94a3b8" }, - }, - light: { - id: "light", - name: "Light", - type: "light", - brand: { primary: "#3b82f6", secondary: "#1d4ed8" }, - semantic: { - success: "#4ade80", - error: "#f87171", - warning: "#fbbf24", - info: "#60a5fa", - neutral: "#6b7280", - }, - interactive: { - prompt: "#3b82f6", - selection: "#1e40af", - hover: "#2563eb", - disabled: "#9ca3af", - focus: "#1d4ed8", - }, - messages: { user: "#10b981", assistant: "#8b5cf6", system: "#f59e0b", error: "#ef4444" }, - actions: { approve: "#10b981", reject: "#ef4444", cancel: "#6b7280", pending: "#f59e0b" }, - code: { - addition: "#10b981", - deletion: "#ef4444", - modification: "#f59e0b", - context: "#6b7280", - lineNumber: "#9ca3af", - }, - ui: { - border: { default: "#e5e7eb", active: "#3b82f6", warning: "#f59e0b", error: "#ef4444" }, - text: { primary: "#111827", secondary: "#6b7280", dimmed: "#9ca3af", highlight: "#fbbf24" }, - background: { default: "#ffffff", elevated: "#f9fafb" }, - }, - status: { online: "#10b981", offline: "#6b7280", busy: "#f59e0b", idle: "#94a3b8" }, - }, + mockConfig = { + version: "1.0.0", + mode: "code", + telemetry: true, + provider: "test-provider", + providers: [], + customThemes: { "custom-theme": mockTheme, - } - return ( - themes[id] || { - id: "unknown", - name: "Unknown Theme", - type: "dark", - brand: { primary: "#000000", secondary: "#000000" }, - semantic: { - success: "#000000", - error: "#000000", - warning: "#000000", - info: "#000000", - neutral: "#000000", - }, - interactive: { - prompt: "#000000", - selection: "#000000", - hover: "#000000", - disabled: "#000000", - focus: "#000000", - }, - messages: { user: "#000000", assistant: "#000000", system: "#000000", error: "#000000" }, - actions: { approve: "#000000", reject: "#000000", cancel: "#000000", pending: "#000000" }, - code: { - addition: "#000000", - deletion: "#000000", - modification: "#000000", - context: "#000000", - lineNumber: "#000000", - }, - ui: { - border: { default: "#000000", active: "#000000", warning: "#000000", error: "#000000" }, - text: { primary: "#000000", secondary: "#000000", dimmed: "#000000", highlight: "#000000" }, - background: { default: "#000000", elevated: "#000000" }, - }, - status: { online: "#000000", offline: "#000000", busy: "#000000", idle: "#000000" }, - } - ) - }), - isValidThemeId: vi.fn(() => true), - })) - - // Mock getBuiltinThemeIds - vi.doMock("../../constants/themes/custom.js", () => ({ - getBuiltinThemeIds: vi.fn(() => ["alpha", "dark", "dracula", "github-dark", "light", "github-light"]), - })) - - mockContext = createMockContext({ - input: "/theme", - config: mockConfig, + }, + } + + mockContext = createMockContext({ + input: "/theme", + config: mockConfig, addMessage: addMessageMock, setTheme: setThemeMock, refreshTerminal: refreshTerminalMock, diff --git a/cli/src/config/__tests__/auto-approval.test.ts b/cli/src/config/__tests__/auto-approval.test.ts index bb9cd4e2d9d..43a961f1c55 100644 --- a/cli/src/config/__tests__/auto-approval.test.ts +++ b/cli/src/config/__tests__/auto-approval.test.ts @@ -6,7 +6,7 @@ describe("Auto Approval Configuration", () => { describe("DEFAULT_AUTO_APPROVAL", () => { it("should have correct default values", () => { expect(DEFAULT_AUTO_APPROVAL).toEqual({ - enabled: true, + enabled: false, // Safe default: auto-approval disabled until explicitly enabled read: { enabled: true, outside: false, @@ -47,8 +47,10 @@ describe("Auto Approval Configuration", () => { }) }) - it("should have global enabled set to true by default", () => { - expect(DEFAULT_AUTO_APPROVAL.enabled).toBe(true) + it("should have global enabled set to false by default for safety", () => { + // Safe default: auto-approval is disabled until explicitly enabled + // This prevents accidental auto-approval when config isn't loaded yet + expect(DEFAULT_AUTO_APPROVAL.enabled).toBe(false) }) it("should have safe defaults for write operations", () => { diff --git a/cli/src/config/__tests__/env-config-json.test.ts b/cli/src/config/__tests__/env-config-json.test.ts new file mode 100644 index 00000000000..69fdc4696f8 --- /dev/null +++ b/cli/src/config/__tests__/env-config-json.test.ts @@ -0,0 +1,139 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest" +import { buildConfigFromEnv, applyEnvOverrides } from "../env-config.js" +import { ENV_VARS } from "../env-utils.js" +import type { CLIConfig, AutoApprovalConfig } from "../types.js" + +describe("env-config JSON auto-approval", () => { + const originalEnv = process.env + + beforeEach(() => { + // Reset environment for each test + process.env = { ...originalEnv } + }) + + afterEach(() => { + process.env = originalEnv + }) + + describe("KILO_AUTO_APPROVAL_JSON parsing", () => { + const baseConfig: CLIConfig = { + version: "1.0.0", + mode: "code", + telemetry: true, + provider: "kilocode", + providers: [{ id: "kilocode", provider: "kilocode" }], + autoApproval: { + enabled: false, + read: { enabled: false }, + write: { enabled: false }, + }, + } + + it("parses valid JSON from KILO_AUTO_APPROVAL_JSON", () => { + const jsonConfig: AutoApprovalConfig = { + enabled: true, + read: { enabled: true, outside: true }, + write: { enabled: true, outside: false, protected: true }, + browser: { enabled: true }, + mcp: { enabled: false }, + execute: { enabled: true, allowed: ["npm test"], denied: ["rm"] }, + } + + process.env[ENV_VARS.AUTO_APPROVAL_JSON] = JSON.stringify(jsonConfig) + + const result = applyEnvOverrides(baseConfig) + + expect(result.autoApproval).toEqual(jsonConfig) + }) + + it("ignores individual env vars when JSON is set", () => { + const jsonConfig: AutoApprovalConfig = { + enabled: true, + read: { enabled: true }, + } + + process.env[ENV_VARS.AUTO_APPROVAL_JSON] = JSON.stringify(jsonConfig) + // These should be ignored + process.env[ENV_VARS.AUTO_APPROVAL_ENABLED] = "false" + process.env[ENV_VARS.AUTO_APPROVAL_READ_ENABLED] = "false" + + const result = applyEnvOverrides(baseConfig) + + // Should use JSON config, not individual env vars + expect(result.autoApproval?.enabled).toBe(true) + expect(result.autoApproval?.read?.enabled).toBe(true) + }) + + it("falls back to individual env vars when JSON is not set", () => { + process.env[ENV_VARS.AUTO_APPROVAL_ENABLED] = "true" + process.env[ENV_VARS.AUTO_APPROVAL_READ_ENABLED] = "true" + + const result = applyEnvOverrides(baseConfig) + + expect(result.autoApproval?.enabled).toBe(true) + expect(result.autoApproval?.read?.enabled).toBe(true) + }) + + it("falls back to individual env vars when JSON is invalid", () => { + process.env[ENV_VARS.AUTO_APPROVAL_JSON] = "not valid json" + process.env[ENV_VARS.AUTO_APPROVAL_ENABLED] = "true" + + const result = applyEnvOverrides(baseConfig) + + // Should fall back to individual env vars + expect(result.autoApproval?.enabled).toBe(true) + }) + + it("handles empty JSON string gracefully", () => { + process.env[ENV_VARS.AUTO_APPROVAL_JSON] = "" + process.env[ENV_VARS.AUTO_APPROVAL_ENABLED] = "true" + + const result = applyEnvOverrides(baseConfig) + + // Empty string should fall back to individual env vars + expect(result.autoApproval?.enabled).toBe(true) + }) + + it("preserves all auto-approval fields from JSON", () => { + const fullConfig: AutoApprovalConfig = { + enabled: true, + read: { enabled: true, outside: true }, + write: { enabled: true, outside: true, protected: true }, + browser: { enabled: true }, + retry: { enabled: true, delay: 15 }, + mcp: { enabled: true }, + mode: { enabled: true }, + subtasks: { enabled: true }, + execute: { enabled: true, allowed: ["npm", "pnpm"], denied: ["rm -rf"] }, + question: { enabled: false, timeout: 30000 }, + todo: { enabled: true }, + } + + process.env[ENV_VARS.AUTO_APPROVAL_JSON] = JSON.stringify(fullConfig) + + const result = applyEnvOverrides(baseConfig) + + expect(result.autoApproval).toEqual(fullConfig) + }) + }) + + describe("buildConfigFromEnv with JSON", () => { + it("uses JSON config when building from env", () => { + const jsonConfig: AutoApprovalConfig = { + enabled: true, + read: { enabled: true }, + write: { enabled: false }, + } + + // Set up minimal required env vars for buildConfigFromEnv to work + process.env[ENV_VARS.PROVIDER_TYPE] = "kilocode" + process.env["KILOCODE_TOKEN"] = "test-token" + process.env[ENV_VARS.AUTO_APPROVAL_JSON] = JSON.stringify(jsonConfig) + + const result = buildConfigFromEnv() + + expect(result).not.toBeNull() + expect(result?.autoApproval).toEqual(jsonConfig) + }) + }) +}) diff --git a/cli/src/config/__tests__/persistence.test.ts b/cli/src/config/__tests__/persistence.test.ts index 335ebc157e7..12225b13bc0 100644 --- a/cli/src/config/__tests__/persistence.test.ts +++ b/cli/src/config/__tests__/persistence.test.ts @@ -13,6 +13,7 @@ import { getKiloToken, } from "../persistence.js" import { DEFAULT_CONFIG } from "../defaults.js" +import { ENV_VARS } from "../env-utils.js" // Mock the logs service vi.mock("../../services/logs.js", () => ({ @@ -190,6 +191,55 @@ describe("Config Persistence", () => { expect(result.validation.errors).toBeDefined() expect(result.validation.errors!.length).toBeGreaterThan(0) }) + + it("applies KILO_AUTO_APPROVAL_JSON at load time without persisting it", async () => { + const originalEnvValue = process.env[ENV_VARS.AUTO_APPROVAL_JSON] + + const jsonConfig = { + enabled: false, + execute: { enabled: false }, + } + + process.env[ENV_VARS.AUTO_APPROVAL_JSON] = JSON.stringify(jsonConfig) + + try { + // Write a config file with auto-approval enabled so we can verify env overrides + const fileConfig: CLIConfig = { + ...DEFAULT_CONFIG, + providers: [ + { + id: "default", + provider: "kilocode", + kilocodeToken: "valid-token-1234567890", + kilocodeModel: "anthropic/claude-sonnet-4.5", + }, + ], + autoApproval: { + ...DEFAULT_CONFIG.autoApproval, + enabled: true, + }, + theme: "dark", + customThemes: {}, + } + + await saveConfig(fileConfig) + + const result = await loadConfig() + + // Returned config should reflect env override + expect(result.config.autoApproval).toEqual(jsonConfig) + + // File should NOT persist env overrides + const persisted = JSON.parse(await fs.readFile(TEST_CONFIG_FILE, "utf-8")) + expect(persisted.autoApproval.enabled).toBe(true) + } finally { + if (originalEnvValue === undefined) { + delete process.env[ENV_VARS.AUTO_APPROVAL_JSON] + } else { + process.env[ENV_VARS.AUTO_APPROVAL_JSON] = originalEnvValue + } + } + }) }) describe("saveConfig", () => { diff --git a/cli/src/config/defaults.ts b/cli/src/config/defaults.ts index 82cac00f1e8..c398f30454b 100644 --- a/cli/src/config/defaults.ts +++ b/cli/src/config/defaults.ts @@ -2,10 +2,12 @@ import type { CLIConfig, AutoApprovalConfig } from "./types.js" /** * Default auto approval configuration - * Matches the defaults from the webview settings + * Safe defaults: auto-approval is disabled by default. + * When running via Agent Manager, the extension passes its config via KILO_AUTO_APPROVAL_JSON env var. + * When running standalone CLI, user must explicitly enable auto-approval in config. */ export const DEFAULT_AUTO_APPROVAL: AutoApprovalConfig = { - enabled: true, + enabled: false, read: { enabled: true, outside: false, diff --git a/cli/src/config/env-config.ts b/cli/src/config/env-config.ts index 833626bd218..9447d254ae2 100644 --- a/cli/src/config/env-config.ts +++ b/cli/src/config/env-config.ts @@ -17,11 +17,45 @@ import { envConfigExists, getMissingEnvVars } from "./provider-validation.js" export { envConfigExists, getMissingEnvVars } export { PROVIDER_ENV_VAR, KILOCODE_PREFIX, KILO_PREFIX } +/** + * Try to parse auto-approval config from JSON environment variable. + * This is used by Agent Manager to pass the complete config from extension state. + * Returns null if the env var is not set or parsing fails. + */ +function parseAutoApprovalFromJsonEnv(): AutoApprovalConfig | null { + const jsonValue = process.env[ENV_VARS.AUTO_APPROVAL_JSON] + if (!jsonValue) { + return null + } + + try { + const parsed = JSON.parse(jsonValue) as AutoApprovalConfig + logs.info("Loaded auto-approval config from KILO_AUTO_APPROVAL_JSON", "EnvConfig") + return parsed + } catch (error) { + logs.warn( + `Failed to parse KILO_AUTO_APPROVAL_JSON: ${error instanceof Error ? error.message : String(error)}`, + "EnvConfig", + ) + return null + } +} + /** * Build auto-approval configuration from environment variables * Used when building config from scratch (no config.json) + * + * If KILO_AUTO_APPROVAL_JSON is set, uses that (from Agent Manager). + * Otherwise, reads individual KILO_AUTO_APPROVAL_* env vars. */ function buildAutoApprovalFromEnv(): AutoApprovalConfig { + // Check for JSON config first (from Agent Manager) + const jsonConfig = parseAutoApprovalFromJsonEnv() + if (jsonConfig) { + return jsonConfig + } + + // Fall back to individual env vars return { enabled: parseBoolean(process.env[ENV_VARS.AUTO_APPROVAL_ENABLED], DEFAULT_AUTO_APPROVAL.enabled ?? true)!, read: { @@ -118,8 +152,17 @@ function buildAutoApprovalFromEnv(): AutoApprovalConfig { /** * Apply auto-approval configuration overrides from environment variables * Used when applying overrides to existing config + * + * If KILO_AUTO_APPROVAL_JSON is set, uses that (from Agent Manager). + * Otherwise, reads individual KILO_AUTO_APPROVAL_* env vars. */ function applyAutoApprovalOverrides(config: CLIConfig): CLIConfig { + // Check for JSON config first (from Agent Manager) + const jsonConfig = parseAutoApprovalFromJsonEnv() + if (jsonConfig) { + return { ...config, autoApproval: jsonConfig } + } + // Track if any overrides were applied let hasOverrides = false const overriddenConfig = { ...config } diff --git a/cli/src/config/env-utils.ts b/cli/src/config/env-utils.ts index 42eedd49adf..9e4f7cda6c8 100644 --- a/cli/src/config/env-utils.ts +++ b/cli/src/config/env-utils.ts @@ -15,6 +15,9 @@ export const ENV_VARS = { PROVIDER_TYPE: "KILO_PROVIDER_TYPE", // Auto-approval settings + // JSON format for complete config (takes precedence when set) + AUTO_APPROVAL_JSON: "KILO_AUTO_APPROVAL_JSON", + // Individual settings (used when JSON not set) AUTO_APPROVAL_ENABLED: "KILO_AUTO_APPROVAL_ENABLED", AUTO_APPROVAL_READ_ENABLED: "KILO_AUTO_APPROVAL_READ_ENABLED", AUTO_APPROVAL_READ_OUTSIDE: "KILO_AUTO_APPROVAL_READ_OUTSIDE", @@ -59,6 +62,11 @@ export const SPECIFIC_ENV_VARS = new Set([ ENV_VARS.MODE, ENV_VARS.TELEMETRY, ENV_VARS.THEME, + // Internal flags (not provider fields) + "KILO_CLI_MODE", + "KILO_EPHEMERAL_MODE", + "KILO_PLATFORM", + "KILO_TELEMETRY_DEBUG", ]) /** diff --git a/cli/src/config/persistence.ts b/cli/src/config/persistence.ts index 7e0bc5e8f0c..6ad10709520 100644 --- a/cli/src/config/persistence.ts +++ b/cli/src/config/persistence.ts @@ -5,7 +5,7 @@ import type { CLIConfig, AutoApprovalConfig } from "./types.js" import { DEFAULT_CONFIG, DEFAULT_AUTO_APPROVAL } from "./defaults.js" import { validateConfig, type ValidationResult } from "./validation.js" import { logs } from "../services/logs.js" -import { buildConfigFromEnv, isEphemeralMode } from "./env-config.js" +import { applyEnvOverrides, buildConfigFromEnv, isEphemeralMode } from "./env-config.js" /** * Result of loading config, includes both the config and validation status @@ -173,26 +173,36 @@ export async function loadConfig(): Promise { const loadedConfig = JSON.parse(content) // Merge with defaults to fill in missing keys - const config = mergeWithDefaults(loadedConfig) - - // Validate merged config - const validation = await validateConfig(config) - if (!validation.valid) { - logs.error("Invalid config file", "ConfigPersistence", { errors: validation.errors }) + const mergedConfig = mergeWithDefaults(loadedConfig) + + // Apply environment variable overrides for runtime use. + // IMPORTANT: Env overrides should NOT be persisted back to disk. + const runtimeConfig = applyEnvOverrides(mergedConfig) + + // Validate runtime config (this is what the CLI will actually use) + const runtimeValidation = await validateConfig(runtimeConfig) + if (!runtimeValidation.valid) { + logs.error("Invalid config after applying env overrides", "ConfigPersistence", { + errors: runtimeValidation.errors, + }) // Return config with validation errors instead of throwing return { - config, - validation, + config: runtimeConfig, + validation: runtimeValidation, } } - // Save the merged config back to ensure all defaults are persisted - // Only save if validation passed - await saveConfig(config) + // Validate the merged (non-env) config before persisting defaults. + // This avoids writing env-only values (like tokens) to disk. + const mergedValidation = await validateConfig(mergedConfig) + if (mergedValidation.valid) { + // Save the merged config back to ensure all defaults are persisted + await saveConfig(mergedConfig) + } return { - config: config as CLIConfig, - validation, + config: runtimeConfig, + validation: runtimeValidation, } } catch (error) { // For errors (e.g., file read errors, JSON parse errors), log and throw diff --git a/cli/src/host/ExtensionHost.ts b/cli/src/host/ExtensionHost.ts index 81ad448d97a..1343c6c1791 100644 --- a/cli/src/host/ExtensionHost.ts +++ b/cli/src/host/ExtensionHost.ts @@ -58,6 +58,8 @@ export class ExtensionHost extends EventEmitter { private webviewProviders: Map = new Map() private webviewInitialized = false private pendingMessages: WebviewMessage[] = [] + private isFlushingPendingMessages = false + private pendingFlushPromise: Promise | null = null private isInitialSetup = true private originalConsole: { log: typeof console.log @@ -298,6 +300,36 @@ export class ExtensionHost extends EventEmitter { } } + private async processWebviewMessage(message: WebviewMessage): Promise { + // Track extension message sent + getTelemetryService().trackExtensionMessageSent(message.type) + + // Handle webviewDidLaunch for CLI state synchronization + if (message.type === "webviewDidLaunch") { + // Prevent rapid-fire webviewDidLaunch messages + const now = Date.now() + if (now - this.lastWebviewLaunchTime < 1000) { + logs.debug("Ignoring webviewDidLaunch - too soon after last one", "ExtensionHost") + return + } + this.lastWebviewLaunchTime = now + await this.handleWebviewLaunch() + } + + // Forward message directly to the webview provider instead of emitting event + // This prevents duplicate handling (event listener + direct call) + const webviewProvider = this.webviewProviders.get("kilo-code.SidebarProvider") + + if (webviewProvider && typeof webviewProvider.handleCLIMessage === "function") { + await webviewProvider.handleCLIMessage(message) + } else { + logs.warn(`No webview provider found or handleCLIMessage not available for: ${message.type}`, "ExtensionHost") + } + + // Handle local state updates for CLI display after forwarding + await this.handleLocalStateUpdates(message) + } + async sendWebviewMessage(message: WebviewMessage): Promise { try { logs.debug(`Processing webview message: ${message.type}`, "ExtensionHost") @@ -307,43 +339,18 @@ export class ExtensionHost extends EventEmitter { return } - // Queue messages if webview not initialized - if (!this.webviewInitialized) { + // Queue messages if webview not initialized OR while we're flushing queued messages. + // This prevents tasks from starting before configuration sync (e.g. updateSettings) is applied. + if (!this.webviewInitialized || this.isFlushingPendingMessages) { this.pendingMessages.push(message) - logs.debug(`Queued message ${message.type} - webview not ready`, "ExtensionHost") - return - } - - // Track extension message sent - getTelemetryService().trackExtensionMessageSent(message.type) - - // Handle webviewDidLaunch for CLI state synchronization - if (message.type === "webviewDidLaunch") { - // Prevent rapid-fire webviewDidLaunch messages - const now = Date.now() - if (now - this.lastWebviewLaunchTime < 1000) { - logs.debug("Ignoring webviewDidLaunch - too soon after last one", "ExtensionHost") - return - } - this.lastWebviewLaunchTime = now - await this.handleWebviewLaunch() - } - - // Forward message directly to the webview provider instead of emitting event - // This prevents duplicate handling (event listener + direct call) - const webviewProvider = this.webviewProviders.get("kilo-code.SidebarProvider") - - if (webviewProvider && typeof webviewProvider.handleCLIMessage === "function") { - await webviewProvider.handleCLIMessage(message) - } else { - logs.warn( - `No webview provider found or handleCLIMessage not available for: ${message.type}`, + logs.debug( + `Queued message ${message.type} - ${!this.webviewInitialized ? "webview not ready" : "flushing"}`, "ExtensionHost", ) + return } - // Handle local state updates for CLI display after forwarding - await this.handleLocalStateUpdates(message) + await this.processWebviewMessage(message) } catch (error) { logs.error("Error handling webview message", "ExtensionHost", { error }) // Don't emit "error" event - emit non-fatal event instead @@ -1085,32 +1092,68 @@ export class ExtensionHost extends EventEmitter { * Called by VSCode mock after resolveWebviewView completes */ public markWebviewReady(): void { - this.webviewInitialized = true - this.isInitialSetup = false + if (!this.webviewInitialized) { + this.webviewInitialized = true + this.isInitialSetup = false + } + + // Nothing to flush - treat as ready immediately. + if (this.pendingMessages.length === 0) { + return + } + + // Avoid starting multiple concurrent flushes. + if (this.pendingFlushPromise) { + return + } + + this.isFlushingPendingMessages = true logs.info("Webview marked as ready, flushing pending messages", "ExtensionHost") - void this.flushPendingMessages() + + this.pendingFlushPromise = this.flushPendingMessages() + .catch((error) => { + logs.error("Error flushing pending messages", "ExtensionHost", { error }) + }) + .finally(() => { + this.isFlushingPendingMessages = false + this.pendingFlushPromise = null + + // If messages were enqueued right as the flush finished, start another flush. + if (this.webviewInitialized && this.pendingMessages.length > 0) { + this.markWebviewReady() + } + }) + + void this.pendingFlushPromise } /** * Flush all pending messages that were queued before webview was ready */ private async flushPendingMessages(): Promise { - const upsertMessages = this.pendingMessages.filter((m) => m.type === "upsertApiConfiguration") - const otherMessages = this.pendingMessages.filter((m) => m.type !== "upsertApiConfiguration") - this.pendingMessages = [] - - logs.info(`Flushing ${upsertMessages.length + otherMessages.length} pending messages`, "ExtensionHost") - - // Ensure the API configuration is applied before anything tries to read it - for (const message of upsertMessages) { - logs.debug(`Flushing pending message: ${message.type}`, "ExtensionHost") - // Serialize upserts so provider settings are persisted before readers run - await this.sendWebviewMessage(message) - } + // Messages can be enqueued while flushing (e.g. UI startup sending newTask), + // so drain the queue until it's empty. + while (this.pendingMessages.length > 0) { + const batch = this.pendingMessages + this.pendingMessages = [] + + const upsertMessages = batch.filter((m) => m.type === "upsertApiConfiguration") + const otherMessages = batch.filter((m) => m.type !== "upsertApiConfiguration") + + logs.info(`Flushing ${upsertMessages.length + otherMessages.length} pending messages`, "ExtensionHost") + + // Ensure the API configuration is applied before anything tries to read it. + // Serialize upserts so provider settings are persisted before readers run. + for (const message of upsertMessages) { + logs.debug(`Flushing pending message: ${message.type}`, "ExtensionHost") + await this.processWebviewMessage(message) + } - for (const message of otherMessages) { - logs.debug(`Flushing pending message: ${message.type}`, "ExtensionHost") - void this.sendWebviewMessage(message) + // Flush remaining messages in order, awaiting each to preserve ordering guarantees. + for (const message of otherMessages) { + logs.debug(`Flushing pending message: ${message.type}`, "ExtensionHost") + await this.processWebviewMessage(message) + } } } diff --git a/cli/src/host/__tests__/ExtensionHost.raceCondition.test.ts b/cli/src/host/__tests__/ExtensionHost.raceCondition.test.ts index b335d443abb..270c8ca0270 100644 --- a/cli/src/host/__tests__/ExtensionHost.raceCondition.test.ts +++ b/cli/src/host/__tests__/ExtensionHost.raceCondition.test.ts @@ -2,6 +2,28 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest" import { ExtensionHost } from "../ExtensionHost.js" import type { WebviewMessage } from "../../types/messages.js" +async function waitForCondition(condition: () => boolean, timeoutMs: number): Promise { + await new Promise((resolve, reject) => { + const start = Date.now() + + const check = () => { + if (condition()) { + resolve() + return + } + + if (Date.now() - start > timeoutMs) { + reject(new Error(`Timed out waiting for condition after ${timeoutMs}ms`)) + return + } + + setTimeout(check, 0) + } + + check() + }) +} + // Type for accessing private members in tests interface ExtensionHostInternal { initializeState: () => void @@ -271,6 +293,81 @@ describe("ExtensionHost Race Condition Fix", () => { // Verify config was sent before task expect(callOrder).toEqual(["mode", "newTask"]) }) + + it("should not process newTask until queued configuration is applied", async () => { + // This reproduces a real startup race: + // - CLI injects configuration before the webview is ready (messages are queued) + // - Webview becomes ready and flush begins + // - A newTask arrives while the flush is still applying updateSettings + // Without proper gating, newTask can be processed with default auto-approval settings. + + const configMessage: WebviewMessage = { + type: "updateSettings", + updatedSettings: { + autoApprovalEnabled: false, + alwaysAllowExecute: false, + }, + } + + const taskMessage: WebviewMessage = { + type: "newTask", + text: "Test task", + } + + // Queue the configuration message before webview is ready + await extensionHost.sendWebviewMessage(configMessage) + + let configApplied = false + let updateSettingsStarted = false + let observedConfigAppliedAtTask: boolean | undefined + + const deferred = (() => { + let resolve!: () => void + const promise = new Promise((r) => { + resolve = r + }) + return { promise, resolve } + })() + + // Mock provider - delay updateSettings application so we can race a newTask against it + const mockProvider = { + handleCLIMessage: vi.fn(async (msg: WebviewMessage) => { + if (msg.type === "updateSettings") { + updateSettingsStarted = true + await deferred.promise + configApplied = true + return + } + + if (msg.type === "newTask") { + observedConfigAppliedAtTask = configApplied + } + }), + } + ;(extensionHost as unknown as ExtensionHostInternal).webviewProviders.set( + "kilo-code.SidebarProvider", + mockProvider, + ) + + // Mark webview as ready (starts async flush) + extensionHost.markWebviewReady() + + // Wait until the queued updateSettings has started processing + await waitForCondition(() => updateSettingsStarted, 50) + expect(updateSettingsStarted).toBe(true) + + // Send a task while configuration is still being applied + await extensionHost.sendWebviewMessage(taskMessage) + const observedBeforeConfigApplied = observedConfigAppliedAtTask + + // Allow configuration to apply and flush to continue + deferred.resolve() + await new Promise((resolve) => setTimeout(resolve, 0)) + + // Correct behavior: task should not be processed until config is applied + expect(observedBeforeConfigApplied).toBeUndefined() + expect(observedConfigAppliedAtTask).toBe(true) + }) }) describe("Edge Cases", () => { diff --git a/cli/src/state/atoms/__tests__/config.test.ts b/cli/src/state/atoms/__tests__/config.test.ts index a60d67495d8..e8c095f0265 100644 --- a/cli/src/state/atoms/__tests__/config.test.ts +++ b/cli/src/state/atoms/__tests__/config.test.ts @@ -4,10 +4,14 @@ import { loadConfigAtom, configAtom, configValidationAtom } from "../config.js" import * as persistence from "../../../config/persistence.js" // Mock the persistence module -vi.mock("../../../config/persistence.js", () => ({ - loadConfig: vi.fn(), - saveConfig: vi.fn(), -})) +vi.mock("../../../config/persistence.js", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + loadConfig: vi.fn(), + saveConfig: vi.fn(), + } +}) describe("loadConfigAtom", () => { beforeEach(() => { diff --git a/cli/src/state/atoms/__tests__/modelValidation.test.ts b/cli/src/state/atoms/__tests__/modelValidation.test.ts index 3c14fb29309..41754cd1c8b 100644 --- a/cli/src/state/atoms/__tests__/modelValidation.test.ts +++ b/cli/src/state/atoms/__tests__/modelValidation.test.ts @@ -21,9 +21,13 @@ vi.mock("../../../services/logs.js", () => ({ }, })) -vi.mock("../../../config/persistence.js", () => ({ - saveConfig: vi.fn().mockResolvedValue(undefined), -})) +vi.mock("../../../config/persistence.js", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + saveConfig: vi.fn().mockResolvedValue(undefined), + } +}) vi.mock("../../../ui/utils/messages.js", () => ({ generateModelFallbackMessage: vi.fn().mockReturnValue({ diff --git a/cli/src/state/atoms/config.ts b/cli/src/state/atoms/config.ts index dfdc44abe6d..317da85b1de 100644 --- a/cli/src/state/atoms/config.ts +++ b/cli/src/state/atoms/config.ts @@ -472,6 +472,29 @@ export const updateAutoApprovalAtom = atom( }, ) +/** + * Replace the auto approval configuration (used by Agent Manager permission sync). + */ +export const setAutoApprovalConfigAtom = atom(null, async (get, set, autoApproval: AutoApprovalConfig) => { + const config = get(configAtom) + + const updatedConfig = { + ...config, + autoApproval, + } + + set(configAtom, updatedConfig) + await set(saveConfigAtom, updatedConfig) + + logs.info("Auto approval configuration replaced", "ConfigAtoms") + + // Import from config-sync to avoid circular dependency + const { syncConfigToExtensionEffectAtom } = await import("./config-sync.js") + + // Trigger sync to extension after permission update + await set(syncConfigToExtensionEffectAtom) +}) + /** * Action atom to update a specific auto approval setting */ diff --git a/cli/src/state/hooks/__tests__/useStdinJsonHandler.test.ts b/cli/src/state/hooks/__tests__/useStdinJsonHandler.test.ts index ae0779b0648..d42f74babc4 100644 --- a/cli/src/state/hooks/__tests__/useStdinJsonHandler.test.ts +++ b/cli/src/state/hooks/__tests__/useStdinJsonHandler.test.ts @@ -13,16 +13,19 @@ describe("handleStdinMessage", () => { let sendAskResponse: ReturnType let cancelTask: ReturnType let respondToTool: ReturnType + let setAutoApprovalConfig: ReturnType beforeEach(() => { sendAskResponse = vi.fn().mockResolvedValue(undefined) cancelTask = vi.fn().mockResolvedValue(undefined) respondToTool = vi.fn().mockResolvedValue(undefined) + setAutoApprovalConfig = vi.fn().mockResolvedValue(undefined) handlers = { sendAskResponse, cancelTask, respondToTool, + setAutoApprovalConfig, } }) @@ -199,6 +202,36 @@ describe("handleStdinMessage", () => { }) }) + describe("permissionConfigUpdate messages", () => { + it("should call setAutoApprovalConfig when config is provided", async () => { + const message: StdinMessage = { + type: "permissionConfigUpdate", + config: { + enabled: false, + read: { enabled: true, outside: false }, + execute: { enabled: false, allowed: [], denied: [] }, + }, + } + + const result = await handleStdinMessage(message, handlers) + + expect(result.handled).toBe(true) + expect(setAutoApprovalConfig).toHaveBeenCalledWith(message.config) + }) + + it("should return handled: false when config is missing", async () => { + const message: StdinMessage = { + type: "permissionConfigUpdate", + } + + const result = await handleStdinMessage(message, handlers) + + expect(result.handled).toBe(false) + expect(result.error).toBe("Missing config for permissionConfigUpdate") + expect(setAutoApprovalConfig).not.toHaveBeenCalled() + }) + }) + describe("unknown message types", () => { it("should return handled: false for unknown types", async () => { const message: StdinMessage = { @@ -212,6 +245,7 @@ describe("handleStdinMessage", () => { expect(sendAskResponse).not.toHaveBeenCalled() expect(cancelTask).not.toHaveBeenCalled() expect(respondToTool).not.toHaveBeenCalled() + expect(setAutoApprovalConfig).not.toHaveBeenCalled() }) }) diff --git a/cli/src/state/hooks/useStdinJsonHandler.ts b/cli/src/state/hooks/useStdinJsonHandler.ts index 6ac2fbbef27..b3299ba9b47 100644 --- a/cli/src/state/hooks/useStdinJsonHandler.ts +++ b/cli/src/state/hooks/useStdinJsonHandler.ts @@ -7,6 +7,8 @@ import { useEffect } from "react" import { useSetAtom } from "jotai" import { createInterface } from "readline" import { sendAskResponseAtom, cancelTaskAtom, respondToToolAtom } from "../atoms/actions.js" +import { setAutoApprovalConfigAtom } from "../atoms/config.js" +import type { AutoApprovalConfig } from "../../config/types.js" import { logs } from "../../services/logs.js" export interface StdinMessage { @@ -15,6 +17,7 @@ export interface StdinMessage { text?: string images?: string[] approved?: boolean + config?: AutoApprovalConfig } export interface StdinMessageHandlers { @@ -25,6 +28,7 @@ export interface StdinMessageHandlers { text?: string images?: string[] }) => Promise + setAutoApprovalConfig: (config: AutoApprovalConfig) => Promise } /** @@ -73,6 +77,14 @@ export async function handleStdinMessage( } return { handled: true } + case "permissionConfigUpdate": + if (!message.config) { + return { handled: false, error: "Missing config for permissionConfigUpdate" } + } + + await handlers.setAutoApprovalConfig(message.config) + return { handled: true } + default: return { handled: false, error: `Unknown message type: ${message.type}` } } @@ -82,6 +94,7 @@ export function useStdinJsonHandler(enabled: boolean) { const sendAskResponse = useSetAtom(sendAskResponseAtom) const cancelTask = useSetAtom(cancelTaskAtom) const respondToTool = useSetAtom(respondToToolAtom) + const setAutoApprovalConfig = useSetAtom(setAutoApprovalConfigAtom) useEffect(() => { if (!enabled) { @@ -105,6 +118,9 @@ export function useStdinJsonHandler(enabled: boolean) { respondToTool: async (params) => { await respondToTool(params) }, + setAutoApprovalConfig: async (config) => { + await setAutoApprovalConfig(config) + }, } const handleLine = async (line: string) => { @@ -142,5 +158,5 @@ export function useStdinJsonHandler(enabled: boolean) { return () => { rl.close() } - }, [enabled, sendAskResponse, cancelTask, respondToTool]) + }, [enabled, sendAskResponse, cancelTask, respondToTool, setAutoApprovalConfig]) } diff --git a/src/core/kilocode/agent-manager/AgentManagerProvider.ts b/src/core/kilocode/agent-manager/AgentManagerProvider.ts index 5a2d4ee9c38..3bc59871faa 100644 --- a/src/core/kilocode/agent-manager/AgentManagerProvider.ts +++ b/src/core/kilocode/agent-manager/AgentManagerProvider.ts @@ -19,6 +19,7 @@ import { extractRawText, tryParsePayloadJson } from "./askErrorParser" import { RemoteSessionService } from "./RemoteSessionService" import { KilocodeEventProcessor } from "./KilocodeEventProcessor" import type { RemoteSession } from "./types" +import { extractAutoApprovalConfig } from "./autoApprovalEnv" import { getUri } from "../../webview/getUri" import { getNonce } from "../../webview/getNonce" import { getViteDevServerConfig } from "../../webview/getViteDevServerConfig" @@ -68,6 +69,10 @@ export class AgentManagerProvider implements vscode.Disposable { // Track currently sending message per session (for one-at-a-time constraint) private sendingMessageMap: Map = new Map() + private permissionSyncIntervalId: NodeJS.Timeout | undefined + private permissionSyncInFlight = false + private lastAutoApprovalConfigJson: string | undefined + constructor( private readonly context: vscode.ExtensionContext, private readonly outputChannel: vscode.OutputChannel, @@ -201,6 +206,7 @@ export class AgentManagerProvider implements vscode.Disposable { this.panel.onDidDispose( () => { this.panel = undefined + this.stopPermissionSync() this.stopAllAgents() }, null, @@ -211,6 +217,77 @@ export class AgentManagerProvider implements vscode.Disposable { // Track Agent Manager panel opened captureAgentManagerOpened() + + this.startPermissionSync() + } + + private startPermissionSync(): void { + if (this.permissionSyncIntervalId) { + return + } + + // Polling is intentionally simple for the MVP and avoids wiring into internal state stores. + // We only broadcast when the derived config changes. + this.permissionSyncIntervalId = setInterval(() => { + void this.syncPermissionConfigToRunningSessions() + }, 1000) + + void this.syncPermissionConfigToRunningSessions() + } + + private stopPermissionSync(): void { + if (!this.permissionSyncIntervalId) { + return + } + + clearInterval(this.permissionSyncIntervalId) + this.permissionSyncIntervalId = undefined + this.permissionSyncInFlight = false + } + + private async syncPermissionConfigToRunningSessions(): Promise { + if (this.permissionSyncInFlight) { + return + } + + const runningSessionIds = this.registry + .getSessions() + .map((s) => s.sessionId) + .filter((sessionId) => this.processHandler.hasStdin(sessionId)) + + if (runningSessionIds.length === 0) { + return + } + + this.permissionSyncInFlight = true + + try { + const state = await this.provider.getState() + const autoApprovalConfig = extractAutoApprovalConfig(state) + const configJson = JSON.stringify(autoApprovalConfig) + + if (configJson === this.lastAutoApprovalConfigJson) { + return + } + + this.lastAutoApprovalConfigJson = configJson + + const message = { type: "permissionConfigUpdate", config: autoApprovalConfig } + + await Promise.all( + runningSessionIds.map(async (sessionId) => { + try { + // Don't fail the whole broadcast if one session is shutting down. + // safeWriteToStdin already surfaces errors to the user. + await this.safeWriteToStdin(sessionId, message, "permission-config-update") + } catch { + // Continue broadcasting to other sessions + } + }), + ) + } finally { + this.permissionSyncInFlight = false + } } /** Rename session key in all session-keyed maps when provisional session is upgraded. */ @@ -484,6 +561,16 @@ export class AgentManagerProvider implements vscode.Disposable { return apiConfiguration } + /** + * Get auto-approval configuration from extension state for CLI processes. + * This enables Agent Manager sessions to respect the same permission settings + * as the main sidebar. + */ + private async getAutoApprovalConfigForCli() { + const state = await this.provider.getState() + return extractAutoApprovalConfig(state) + } + /** * Common helper to spawn a CLI process with standard setup. * Handles CLI path lookup, workspace folder validation, API config, and event callback wiring. @@ -527,11 +614,33 @@ export class AgentManagerProvider implements vscode.Disposable { ) } + // Get permission settings from extension state + // This enables Agent Manager sessions to respect the same permission settings as the sidebar + const state = await this.provider.getState() + const autoApprovalConfig = extractAutoApprovalConfig(state) + + // Check if YOLO mode is enabled (full auto-approval of everything) + const yoloMode = (state as { yoloMode?: boolean }).yoloMode ?? false + + // Debug: Log extracted permission settings + this.outputChannel.appendLine( + `[AgentManager] Permission settings from state: autoApprovalEnabled=${state.autoApprovalEnabled}, yoloMode=${yoloMode}`, + ) + this.outputChannel.appendLine( + `[AgentManager] Extracted config: enabled=${autoApprovalConfig.enabled}, execute.enabled=${autoApprovalConfig.execute?.enabled}`, + ) + this.processHandler.spawnProcess( cliDiscovery.cliPath, workspaceFolder, prompt, - { ...options, apiConfiguration, shellPath: cliDiscovery.shellPath }, + { + ...options, + apiConfiguration, + shellPath: cliDiscovery.shellPath, + autoApprovalConfig, + yolo: yoloMode, + }, (sid, event) => { if (!this.processStartTimes.has(sid)) { this.processStartTimes.set(sid, processStartTime) @@ -1198,6 +1307,7 @@ export class AgentManagerProvider implements vscode.Disposable { } public dispose(): void { + this.stopPermissionSync() this.processHandler.dispose() this.terminalManager.dispose() this.sessionMessages.clear() diff --git a/src/core/kilocode/agent-manager/CliArgsBuilder.ts b/src/core/kilocode/agent-manager/CliArgsBuilder.ts index f37299c2ec3..96466279b75 100644 --- a/src/core/kilocode/agent-manager/CliArgsBuilder.ts +++ b/src/core/kilocode/agent-manager/CliArgsBuilder.ts @@ -2,18 +2,29 @@ export interface BuildCliArgsOptions { parallelMode?: boolean sessionId?: string existingBranch?: string + /** + * When true, adds --yolo flag for auto-approval of all tool operations. + * By default (when undefined/false), the CLI runs in permission-aware mode, + * respecting the extension's auto-approval settings. + */ + yolo?: boolean } /** * Builds CLI arguments for spawning kilocode agent processes. * Uses --json-io for bidirectional communication via stdin/stdout. - * Runs in interactive mode - approvals are handled via the JSON-IO protocol. + * Runs in permission-aware mode by default - approvals are handled via the JSON-IO protocol. */ export function buildCliArgs(workspace: string, prompt: string, options?: BuildCliArgsOptions): string[] { // --json-io: enables bidirectional JSON communication via stdin/stdout // Note: --json (without -io) exists for CI/CD read-only mode but isn't used here - // --yolo: auto-approve tool uses (file reads, writes, commands, etc.) - const args = ["--json-io", "--yolo", `--workspace=${workspace}`] + const args = ["--json-io", `--workspace=${workspace}`] + + // Only add --yolo when explicitly requested + // By default, the CLI runs in permission-aware mode, respecting extension settings + if (options?.yolo) { + args.push("--yolo") + } if (options?.parallelMode) { args.push("--parallel") diff --git a/src/core/kilocode/agent-manager/CliProcessHandler.ts b/src/core/kilocode/agent-manager/CliProcessHandler.ts index 3d5cc979a47..c684cce4aaf 100644 --- a/src/core/kilocode/agent-manager/CliProcessHandler.ts +++ b/src/core/kilocode/agent-manager/CliProcessHandler.ts @@ -15,6 +15,7 @@ import type { ClineMessage, ProviderSettings } from "@roo-code/types" import { extractApiReqFailedMessage, extractPayloadMessage } from "./askErrorParser" import { buildProviderEnvOverrides } from "./providerEnvMapper" import { captureAgentManagerLoginIssue, getPlatformDiagnostics } from "./telemetry" +import { type AutoApprovalEnvConfig, buildAutoApprovalEnv } from "./autoApprovalEnv" /** * Timeout for pending sessions (ms) - if session_created event doesn't arrive within this time, @@ -106,7 +107,11 @@ export class CliProcessHandler { } } - private buildEnvWithApiConfiguration(apiConfiguration?: ProviderSettings, shellPath?: string): NodeJS.ProcessEnv { + private buildEnvWithApiConfiguration( + apiConfiguration?: ProviderSettings, + autoApprovalConfig?: AutoApprovalEnvConfig, + shellPath?: string, + ): NodeJS.ProcessEnv { const baseEnv = { ...process.env } // On macOS/Linux, use the shell PATH to ensure CLI can access tools like git @@ -124,9 +129,22 @@ export class CliProcessHandler { (message) => this.debugLog(message), ) + // Build auto-approval env if config provided + const autoApprovalEnv = autoApprovalConfig ? buildAutoApprovalEnv(autoApprovalConfig) : {} + + // Debug: log auto-approval config being passed to CLI + if (autoApprovalConfig) { + this.callbacks.onLog( + `[AgentManager] Auto-approval config: enabled=${autoApprovalConfig.enabled}, execute.enabled=${autoApprovalConfig.execute?.enabled}`, + ) + } else { + this.callbacks.onLog(`[AgentManager] No auto-approval config provided to CLI`) + } + return { ...baseEnv, ...overrides, + ...autoApprovalEnv, NO_COLOR: "1", FORCE_COLOR: "0", KILO_PLATFORM: "agent-manager", @@ -147,6 +165,17 @@ export class CliProcessHandler { existingBranch?: string /** Shell PATH from login shell - ensures CLI can access tools like git on macOS */ shellPath?: string + /** + * Auto-approval configuration to pass to CLI. + * When provided, the CLI runs in permission-aware mode. + * When omitted, the CLI uses its default behavior. + */ + autoApprovalConfig?: AutoApprovalEnvConfig + /** + * When true, adds --yolo flag for auto-approval of ALL tool operations. + * This corresponds to the extension's "YOLO Mode" setting. + */ + yolo?: boolean } | undefined, onCliEvent: (sessionId: string, event: StreamEvent) => void, @@ -185,11 +214,16 @@ export class CliProcessHandler { parallelMode: options?.parallelMode, sessionId: options?.sessionId, existingBranch: options?.existingBranch, + yolo: options?.yolo, }) this.debugLog(`Command: ${cliPath} ${cliArgs.join(" ")}`) this.debugLog(`Working dir: ${workspace}`) - const env = this.buildEnvWithApiConfiguration(options?.apiConfiguration, options?.shellPath) + const env = this.buildEnvWithApiConfiguration( + options?.apiConfiguration, + options?.autoApprovalConfig, + options?.shellPath, + ) // On Windows, .cmd files need to be executed through cmd.exe (shell: true) // Without this, spawn() fails silently because .cmd files are batch scripts diff --git a/src/core/kilocode/agent-manager/CliSessionLauncher.ts b/src/core/kilocode/agent-manager/CliSessionLauncher.ts index 6ae8b60f220..25568bf5fde 100644 --- a/src/core/kilocode/agent-manager/CliSessionLauncher.ts +++ b/src/core/kilocode/agent-manager/CliSessionLauncher.ts @@ -6,6 +6,7 @@ import { getRemoteUrl } from "../../../services/code-index/managed/git-utils" import { normalizeGitUrl } from "./normalizeGitUrl" import { buildProviderEnvOverrides } from "./providerEnvMapper" import type { ProviderSettings } from "@roo-code/types" +import type { AutoApprovalEnvConfig } from "./autoApprovalEnv" /** * Options for spawning a CLI session @@ -16,6 +17,17 @@ export interface SpawnOptions { gitUrl?: string existingBranch?: string sessionId?: string + /** + * Auto-approval configuration to pass to CLI. + * When provided, the CLI runs in permission-aware mode. + * When omitted, the CLI uses its default behavior. + */ + autoApprovalConfig?: AutoApprovalEnvConfig + /** + * When true, adds --yolo flag for auto-approval of ALL tool operations. + * This corresponds to the extension's "YOLO Mode" setting. + */ + yolo?: boolean } /** diff --git a/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.ipc.spec.ts b/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.ipc.spec.ts index cbedc512037..07c5f1cd327 100644 --- a/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.ipc.spec.ts +++ b/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.ipc.spec.ts @@ -34,6 +34,7 @@ describe("AgentManagerProvider IPC paths", () => { let mockPanel: any let output: string[] let registry: AgentRegistry + let providerStub: { getState: ReturnType } beforeEach(() => { output = [] @@ -67,7 +68,7 @@ describe("AgentManagerProvider IPC paths", () => { asAbsolutePath: (p: string) => p, extensionMode: 1, // Development mode } as unknown as vscode.ExtensionContext - const providerStub = { + providerStub = { getState: vi.fn().mockResolvedValue({ apiConfiguration: { apiProvider: "kilocode" } }), } @@ -105,4 +106,80 @@ describe("AgentManagerProvider IPC paths", () => { expect(vscode.window.showErrorMessage).toHaveBeenLastCalledWith("Failed to send approval-yes to agent: denied") }) + + it("syncPermissionConfigToRunningSessions sends updates only when config changes", async () => { + registry.createSession("sess", "prompt") + mockProcessHandler.hasStdin.mockImplementation((id) => id === "sess") + + providerStub.getState.mockResolvedValue({ + autoApprovalEnabled: false, + alwaysAllowReadOnly: true, + alwaysAllowReadOnlyOutsideWorkspace: false, + alwaysAllowWrite: true, + alwaysAllowWriteOutsideWorkspace: false, + alwaysAllowWriteProtected: false, + alwaysAllowBrowser: false, + alwaysApproveResubmit: false, + requestDelaySeconds: 10, + alwaysAllowMcp: false, + alwaysAllowModeSwitch: false, + alwaysAllowSubtasks: false, + alwaysAllowExecute: false, + allowedCommands: ["npm test"], + deniedCommands: ["rm -rf"], + alwaysAllowFollowupQuestions: false, + followupAutoApproveTimeoutMs: 60000, + alwaysAllowUpdateTodoList: false, + }) + + await (provider as any).syncPermissionConfigToRunningSessions() + + expect(mockProcessHandler.writeToStdin).toHaveBeenCalledWith("sess", { + type: "permissionConfigUpdate", + config: { + enabled: false, + read: { enabled: true, outside: false }, + write: { enabled: true, outside: false, protected: false }, + browser: { enabled: false }, + retry: { enabled: false, delay: 10 }, + mcp: { enabled: false }, + mode: { enabled: false }, + subtasks: { enabled: false }, + execute: { enabled: false, allowed: ["npm test"], denied: ["rm -rf"] }, + question: { enabled: false, timeout: 60000 }, + todo: { enabled: false }, + }, + }) + + mockProcessHandler.writeToStdin.mockClear() + + // Same config => no-op + await (provider as any).syncPermissionConfigToRunningSessions() + expect(mockProcessHandler.writeToStdin).not.toHaveBeenCalled() + + // Change config => broadcast + providerStub.getState.mockResolvedValue({ + autoApprovalEnabled: true, + alwaysAllowReadOnly: true, + alwaysAllowReadOnlyOutsideWorkspace: true, + alwaysAllowWrite: false, + alwaysAllowWriteOutsideWorkspace: false, + alwaysAllowWriteProtected: false, + alwaysAllowBrowser: false, + alwaysApproveResubmit: false, + requestDelaySeconds: 10, + alwaysAllowMcp: false, + alwaysAllowModeSwitch: false, + alwaysAllowSubtasks: false, + alwaysAllowExecute: false, + allowedCommands: ["npm test"], + deniedCommands: ["rm -rf"], + alwaysAllowFollowupQuestions: false, + followupAutoApproveTimeoutMs: 60000, + alwaysAllowUpdateTodoList: false, + }) + + await (provider as any).syncPermissionConfigToRunningSessions() + expect(mockProcessHandler.writeToStdin).toHaveBeenCalledTimes(1) + }) }) diff --git a/src/core/kilocode/agent-manager/__tests__/CliArgsBuilder.spec.ts b/src/core/kilocode/agent-manager/__tests__/CliArgsBuilder.spec.ts index 4ef8eae8736..3eacadfcf53 100644 --- a/src/core/kilocode/agent-manager/__tests__/CliArgsBuilder.spec.ts +++ b/src/core/kilocode/agent-manager/__tests__/CliArgsBuilder.spec.ts @@ -8,24 +8,25 @@ describe("buildCliArgs", () => { expect(args).toContain("--json-io") }) - it("returns correct args for basic prompt", () => { + it("returns correct args for basic prompt (permission-aware mode by default)", () => { const args = buildCliArgs("/workspace", "hello world") - expect(args).toEqual(["--json-io", "--yolo", "--workspace=/workspace", "hello world"]) + // By default, --yolo is NOT included - runs in permission-aware mode + expect(args).toEqual(["--json-io", "--workspace=/workspace", "hello world"]) }) it("preserves prompt with special characters", () => { const prompt = 'echo "$(whoami)"' const args = buildCliArgs("/tmp", prompt) - expect(args).toHaveLength(4) - expect(args[3]).toBe(prompt) + expect(args).toHaveLength(3) + expect(args[2]).toBe(prompt) }) it("handles workspace paths with spaces", () => { const args = buildCliArgs("/path/with spaces/project", "test") - expect(args[2]).toBe("--workspace=/path/with spaces/project") + expect(args[1]).toBe("--workspace=/path/with spaces/project") }) it("omits empty prompt from args (used for resume without new prompt)", () => { @@ -33,14 +34,14 @@ describe("buildCliArgs", () => { // Empty prompt should not be added to args - this is used when resuming // a session with --session where we don't want to pass a new prompt - expect(args).toEqual(["--json-io", "--yolo", "--workspace=/workspace"]) + expect(args).toEqual(["--json-io", "--workspace=/workspace"]) }) it("handles multiline prompts", () => { const prompt = "line1\nline2\nline3" const args = buildCliArgs("/workspace", prompt) - expect(args[3]).toBe(prompt) + expect(args[2]).toBe(prompt) }) it("includes --parallel flag when parallelMode is true", () => { @@ -55,7 +56,7 @@ describe("buildCliArgs", () => { expect(args).toContain("--session=abc123") }) - it("combines all options correctly", () => { + it("combines all options correctly (without yolo by default)", () => { const args = buildCliArgs("/workspace", "prompt", { parallelMode: true, sessionId: "session-id", @@ -63,7 +64,6 @@ describe("buildCliArgs", () => { expect(args).toEqual([ "--json-io", - "--yolo", "--workspace=/workspace", "--parallel", "--session=session-id", @@ -71,10 +71,33 @@ describe("buildCliArgs", () => { ]) }) - it("uses --yolo for auto-approval of tool uses", () => { - const args = buildCliArgs("/workspace", "prompt") + it("combines all options correctly (with yolo when explicitly set)", () => { + const args = buildCliArgs("/workspace", "prompt", { + parallelMode: true, + sessionId: "session-id", + yolo: true, + }) + + expect(args).toEqual([ + "--json-io", + "--workspace=/workspace", + "--yolo", + "--parallel", + "--session=session-id", + "prompt", + ]) + }) + + it("uses --yolo for auto-approval when explicitly enabled", () => { + const args = buildCliArgs("/workspace", "prompt", { yolo: true }) expect(args).toContain("--yolo") + }) + + it("does NOT include --yolo by default (permission-aware mode)", () => { + const args = buildCliArgs("/workspace", "prompt") + + expect(args).not.toContain("--yolo") expect(args).not.toContain("--auto") }) }) diff --git a/src/core/kilocode/agent-manager/__tests__/CliArgsBuilder.test.ts b/src/core/kilocode/agent-manager/__tests__/CliArgsBuilder.test.ts index 40a056910b7..f7411e3a338 100644 --- a/src/core/kilocode/agent-manager/__tests__/CliArgsBuilder.test.ts +++ b/src/core/kilocode/agent-manager/__tests__/CliArgsBuilder.test.ts @@ -13,6 +13,32 @@ describe("CliArgsBuilder", () => { expect(args).toContain(prompt) }) + describe("yolo mode", () => { + it("does NOT include --yolo by default (permission-aware mode)", () => { + const args = buildCliArgs(workspace, prompt) + + expect(args).not.toContain("--yolo") + }) + + it("includes --yolo only when explicitly requested via options", () => { + const args = buildCliArgs(workspace, prompt, { yolo: true }) + + expect(args).toContain("--yolo") + }) + + it("does not include --yolo when yolo option is false", () => { + const args = buildCliArgs(workspace, prompt, { yolo: false }) + + expect(args).not.toContain("--yolo") + }) + + it("does not include --yolo when yolo option is undefined", () => { + const args = buildCliArgs(workspace, prompt, { parallelMode: true }) + + expect(args).not.toContain("--yolo") + }) + }) + it("adds --parallel flag when parallelMode is true", () => { const args = buildCliArgs(workspace, prompt, { parallelMode: true }) diff --git a/src/core/kilocode/agent-manager/__tests__/CliProcessHandler.spec.ts b/src/core/kilocode/agent-manager/__tests__/CliProcessHandler.spec.ts index ee078ee80d4..545133b4a9a 100644 --- a/src/core/kilocode/agent-manager/__tests__/CliProcessHandler.spec.ts +++ b/src/core/kilocode/agent-manager/__tests__/CliProcessHandler.spec.ts @@ -90,13 +90,14 @@ describe("CliProcessHandler", () => { }) describe("spawnProcess", () => { - it("spawns a CLI process with correct arguments (yolo mode for testing)", () => { + it("spawns a CLI process with correct arguments (permission-aware mode by default)", () => { const onCliEvent = vi.fn() handler.spawnProcess("/path/to/kilocode", "/workspace", "test prompt", undefined, onCliEvent) + // By default, --yolo is NOT included - runs in permission-aware mode expect(spawnMock).toHaveBeenCalledWith( "/path/to/kilocode", - ["--json-io", "--yolo", "--workspace=/workspace", "test prompt"], + ["--json-io", "--workspace=/workspace", "test prompt"], expect.objectContaining({ cwd: "/workspace", stdio: ["pipe", "pipe", "pipe"], diff --git a/src/core/kilocode/agent-manager/__tests__/autoApprovalEnv.spec.ts b/src/core/kilocode/agent-manager/__tests__/autoApprovalEnv.spec.ts new file mode 100644 index 00000000000..34b7db56666 --- /dev/null +++ b/src/core/kilocode/agent-manager/__tests__/autoApprovalEnv.spec.ts @@ -0,0 +1,232 @@ +import { describe, it, expect } from "vitest" +import { + extractAutoApprovalConfig, + buildAutoApprovalEnv, + KILO_AUTO_APPROVAL_ENV_KEY, + type AutoApprovalEnvConfig, +} from "../autoApprovalEnv" + +describe("autoApprovalEnv", () => { + describe("extractAutoApprovalConfig", () => { + it("extracts enabled state from autoApprovalEnabled", () => { + const config = extractAutoApprovalConfig({ + autoApprovalEnabled: true, + alwaysAllowReadOnly: false, + alwaysAllowReadOnlyOutsideWorkspace: false, + alwaysAllowWrite: false, + alwaysAllowWriteOutsideWorkspace: false, + alwaysAllowWriteProtected: false, + alwaysAllowBrowser: false, + alwaysApproveResubmit: false, + requestDelaySeconds: 10, + alwaysAllowMcp: false, + alwaysAllowModeSwitch: false, + alwaysAllowSubtasks: false, + alwaysAllowExecute: false, + allowedCommands: [], + deniedCommands: [], + alwaysAllowFollowupQuestions: false, + followupAutoApproveTimeoutMs: 60000, + alwaysAllowUpdateTodoList: false, + }) + + expect(config.enabled).toBe(true) + }) + + it("maps read permissions correctly", () => { + const config = extractAutoApprovalConfig({ + autoApprovalEnabled: true, + alwaysAllowReadOnly: true, + alwaysAllowReadOnlyOutsideWorkspace: true, + alwaysAllowWrite: false, + alwaysAllowWriteOutsideWorkspace: false, + alwaysAllowWriteProtected: false, + alwaysAllowBrowser: false, + alwaysApproveResubmit: false, + requestDelaySeconds: 10, + alwaysAllowMcp: false, + alwaysAllowModeSwitch: false, + alwaysAllowSubtasks: false, + alwaysAllowExecute: false, + allowedCommands: [], + deniedCommands: [], + alwaysAllowFollowupQuestions: false, + followupAutoApproveTimeoutMs: 60000, + alwaysAllowUpdateTodoList: false, + }) + + expect(config.read).toEqual({ + enabled: true, + outside: true, + }) + }) + + it("maps write permissions correctly", () => { + const config = extractAutoApprovalConfig({ + autoApprovalEnabled: true, + alwaysAllowReadOnly: false, + alwaysAllowReadOnlyOutsideWorkspace: false, + alwaysAllowWrite: true, + alwaysAllowWriteOutsideWorkspace: true, + alwaysAllowWriteProtected: true, + alwaysAllowBrowser: false, + alwaysApproveResubmit: false, + requestDelaySeconds: 10, + alwaysAllowMcp: false, + alwaysAllowModeSwitch: false, + alwaysAllowSubtasks: false, + alwaysAllowExecute: false, + allowedCommands: [], + deniedCommands: [], + alwaysAllowFollowupQuestions: false, + followupAutoApproveTimeoutMs: 60000, + alwaysAllowUpdateTodoList: false, + }) + + expect(config.write).toEqual({ + enabled: true, + outside: true, + protected: true, + }) + }) + + it("maps execute permissions with allowed/denied commands", () => { + const config = extractAutoApprovalConfig({ + autoApprovalEnabled: true, + alwaysAllowReadOnly: false, + alwaysAllowReadOnlyOutsideWorkspace: false, + alwaysAllowWrite: false, + alwaysAllowWriteOutsideWorkspace: false, + alwaysAllowWriteProtected: false, + alwaysAllowBrowser: false, + alwaysApproveResubmit: false, + requestDelaySeconds: 10, + alwaysAllowMcp: false, + alwaysAllowModeSwitch: false, + alwaysAllowSubtasks: false, + alwaysAllowExecute: true, + allowedCommands: ["npm test", "npm run build"], + deniedCommands: ["rm -rf"], + alwaysAllowFollowupQuestions: false, + followupAutoApproveTimeoutMs: 60000, + alwaysAllowUpdateTodoList: false, + }) + + expect(config.execute).toEqual({ + enabled: true, + allowed: ["npm test", "npm run build"], + denied: ["rm -rf"], + }) + }) + + it("maps retry settings correctly", () => { + const config = extractAutoApprovalConfig({ + autoApprovalEnabled: true, + alwaysAllowReadOnly: false, + alwaysAllowReadOnlyOutsideWorkspace: false, + alwaysAllowWrite: false, + alwaysAllowWriteOutsideWorkspace: false, + alwaysAllowWriteProtected: false, + alwaysAllowBrowser: false, + alwaysApproveResubmit: true, + requestDelaySeconds: 30, + alwaysAllowMcp: false, + alwaysAllowModeSwitch: false, + alwaysAllowSubtasks: false, + alwaysAllowExecute: false, + allowedCommands: [], + deniedCommands: [], + alwaysAllowFollowupQuestions: false, + followupAutoApproveTimeoutMs: 60000, + alwaysAllowUpdateTodoList: false, + }) + + expect(config.retry).toEqual({ + enabled: true, + delay: 30, + }) + }) + + it("defaults undefined values to false/empty", () => { + const config = extractAutoApprovalConfig({ + autoApprovalEnabled: undefined as unknown as boolean, + alwaysAllowReadOnly: undefined as unknown as boolean, + alwaysAllowReadOnlyOutsideWorkspace: undefined as unknown as boolean, + alwaysAllowWrite: undefined as unknown as boolean, + alwaysAllowWriteOutsideWorkspace: undefined as unknown as boolean, + alwaysAllowWriteProtected: undefined as unknown as boolean, + alwaysAllowBrowser: undefined as unknown as boolean, + alwaysApproveResubmit: undefined as unknown as boolean, + requestDelaySeconds: undefined as unknown as number, + alwaysAllowMcp: undefined as unknown as boolean, + alwaysAllowModeSwitch: undefined as unknown as boolean, + alwaysAllowSubtasks: undefined as unknown as boolean, + alwaysAllowExecute: undefined as unknown as boolean, + allowedCommands: undefined as unknown as string[], + deniedCommands: undefined as unknown as string[], + alwaysAllowFollowupQuestions: undefined as unknown as boolean, + followupAutoApproveTimeoutMs: undefined as unknown as number, + alwaysAllowUpdateTodoList: undefined as unknown as boolean, + }) + + expect(config.enabled).toBe(false) + expect(config.read?.enabled).toBe(false) + expect(config.write?.enabled).toBe(false) + expect(config.execute?.allowed).toEqual([]) + expect(config.execute?.denied).toEqual([]) + }) + }) + + describe("buildAutoApprovalEnv", () => { + it("returns object with KILO_AUTO_APPROVAL_JSON key", () => { + const config: AutoApprovalEnvConfig = { + enabled: true, + read: { enabled: true }, + } + + const env = buildAutoApprovalEnv(config) + + expect(env).toHaveProperty(KILO_AUTO_APPROVAL_ENV_KEY) + }) + + it("serializes config as JSON", () => { + const config: AutoApprovalEnvConfig = { + enabled: true, + read: { enabled: true, outside: false }, + write: { enabled: false }, + } + + const env = buildAutoApprovalEnv(config) + const parsed = JSON.parse(env[KILO_AUTO_APPROVAL_ENV_KEY]) + + expect(parsed).toEqual(config) + }) + + it("handles complete config with all options", () => { + const config: AutoApprovalEnvConfig = { + enabled: true, + read: { enabled: true, outside: true }, + write: { enabled: true, outside: true, protected: false }, + browser: { enabled: true }, + retry: { enabled: true, delay: 15 }, + mcp: { enabled: true }, + mode: { enabled: true }, + subtasks: { enabled: true }, + execute: { enabled: true, allowed: ["npm"], denied: ["rm"] }, + question: { enabled: false, timeout: 30000 }, + todo: { enabled: true }, + } + + const env = buildAutoApprovalEnv(config) + const parsed = JSON.parse(env[KILO_AUTO_APPROVAL_ENV_KEY]) + + expect(parsed).toEqual(config) + }) + }) + + describe("KILO_AUTO_APPROVAL_ENV_KEY", () => { + it("has correct value", () => { + expect(KILO_AUTO_APPROVAL_ENV_KEY).toBe("KILO_AUTO_APPROVAL_JSON") + }) + }) +}) diff --git a/src/core/kilocode/agent-manager/autoApprovalEnv.ts b/src/core/kilocode/agent-manager/autoApprovalEnv.ts new file mode 100644 index 00000000000..8ac1cb519a9 --- /dev/null +++ b/src/core/kilocode/agent-manager/autoApprovalEnv.ts @@ -0,0 +1,110 @@ +/** + * Helper functions for passing auto-approval configuration to CLI processes + * via environment variables. + * + * This enables Agent Manager sessions to respect the same permission settings + * as the main sidebar, rather than always running in YOLO mode. + */ + +import type { ExtensionState } from "../../../shared/ExtensionMessage" + +/** + * Environment variable name for passing auto-approval config to CLI + */ +export const KILO_AUTO_APPROVAL_ENV_KEY = "KILO_AUTO_APPROVAL_JSON" + +/** + * Auto-approval configuration structure that matches CLI's AutoApprovalConfig type + */ +export interface AutoApprovalEnvConfig { + enabled: boolean + read?: { enabled?: boolean; outside?: boolean } + write?: { enabled?: boolean; outside?: boolean; protected?: boolean } + browser?: { enabled?: boolean } + retry?: { enabled?: boolean; delay?: number } + mcp?: { enabled?: boolean } + mode?: { enabled?: boolean } + subtasks?: { enabled?: boolean } + execute?: { enabled?: boolean; allowed?: string[]; denied?: string[] } + question?: { enabled?: boolean; timeout?: number } + todo?: { enabled?: boolean } +} + +/** + * Extract auto-approval configuration from extension state. + * This maps the extension's state properties to the CLI's config format. + */ +export function extractAutoApprovalConfig( + state: Pick< + ExtensionState, + | "autoApprovalEnabled" + | "alwaysAllowReadOnly" + | "alwaysAllowReadOnlyOutsideWorkspace" + | "alwaysAllowWrite" + | "alwaysAllowWriteOutsideWorkspace" + | "alwaysAllowWriteProtected" + | "alwaysAllowBrowser" + | "alwaysApproveResubmit" + | "requestDelaySeconds" + | "alwaysAllowMcp" + | "alwaysAllowModeSwitch" + | "alwaysAllowSubtasks" + | "alwaysAllowExecute" + | "allowedCommands" + | "deniedCommands" + | "alwaysAllowFollowupQuestions" + | "followupAutoApproveTimeoutMs" + | "alwaysAllowUpdateTodoList" + >, +): AutoApprovalEnvConfig { + return { + enabled: state.autoApprovalEnabled ?? false, + read: { + enabled: state.alwaysAllowReadOnly ?? false, + outside: state.alwaysAllowReadOnlyOutsideWorkspace ?? false, + }, + write: { + enabled: state.alwaysAllowWrite ?? false, + outside: state.alwaysAllowWriteOutsideWorkspace ?? false, + protected: state.alwaysAllowWriteProtected ?? false, + }, + browser: { + enabled: state.alwaysAllowBrowser ?? false, + }, + retry: { + enabled: state.alwaysApproveResubmit ?? false, + delay: state.requestDelaySeconds ?? 10, + }, + mcp: { + enabled: state.alwaysAllowMcp ?? false, + }, + mode: { + enabled: state.alwaysAllowModeSwitch ?? false, + }, + subtasks: { + enabled: state.alwaysAllowSubtasks ?? false, + }, + execute: { + enabled: state.alwaysAllowExecute ?? false, + allowed: state.allowedCommands ?? [], + denied: state.deniedCommands ?? [], + }, + question: { + enabled: state.alwaysAllowFollowupQuestions ?? false, + timeout: state.followupAutoApproveTimeoutMs ?? 60000, + }, + todo: { + enabled: state.alwaysAllowUpdateTodoList ?? false, + }, + } +} + +/** + * Build environment variables for passing auto-approval config to CLI. + * Returns an object with KILO_AUTO_APPROVAL_JSON set to the JSON-encoded config. + */ +export function buildAutoApprovalEnv(config: AutoApprovalEnvConfig): Record { + return { + [KILO_AUTO_APPROVAL_ENV_KEY]: JSON.stringify(config), + } +} diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 4a6e208d4bc..83c6e5d220c 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2339,7 +2339,7 @@ ${prompt} enhancementApiConfigId, commitMessageApiConfigId, // kilocode_change terminalCommandApiConfigId, // kilocode_change - autoApprovalEnabled: autoApprovalEnabled ?? true, + autoApprovalEnabled: autoApprovalEnabled ?? (process.env.KILO_CLI_MODE === "true" ? false : true), customModes, experiments: experiments ?? experimentDefault, mcpServers: this.mcpHub?.getAllServers() ?? [], @@ -2641,7 +2641,7 @@ ${prompt} autoPurgeLastRunTimestamp: stateValues.autoPurgeLastRunTimestamp, // kilocode_change end experiments: stateValues.experiments ?? experimentDefault, - autoApprovalEnabled: stateValues.autoApprovalEnabled ?? true, + autoApprovalEnabled: stateValues.autoApprovalEnabled ?? (process.env.KILO_CLI_MODE === "true" ? false : true), customModes, maxOpenTabsContext: stateValues.maxOpenTabsContext ?? 20, maxWorkspaceFiles: stateValues.maxWorkspaceFiles ?? 200, diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index bb75ead1436..bd5259339bc 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -498,19 +498,39 @@ describe("ClineProvider", () => { }) }) - test("constructor initializes correctly", () => { - expect(provider).toBeInstanceOf(ClineProvider) - // Since getVisibleInstance returns the last instance where view.visible is true - // @ts-ignore - accessing private property for testing - provider.view = mockWebviewView - expect(ClineProvider.getVisibleInstance()).toBe(provider) - }) + test("constructor initializes correctly", () => { + expect(provider).toBeInstanceOf(ClineProvider) + // Since getVisibleInstance returns the last instance where view.visible is true + // @ts-ignore - accessing private property for testing + provider.view = mockWebviewView + expect(ClineProvider.getVisibleInstance()).toBe(provider) + }) + + test("getState defaults autoApprovalEnabled to false in CLI mode when unset", async () => { + const originalCliMode = process.env.KILO_CLI_MODE + process.env.KILO_CLI_MODE = "true" + + try { + // Create a fresh provider after setting env to ensure getState reads the correct default. + const cliProvider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext)) + const state = await cliProvider.getState() + + // Safe default: CLI mode should never auto-approve unless explicitly configured. + expect(state.autoApprovalEnabled).toBe(false) + } finally { + if (originalCliMode === undefined) { + delete process.env.KILO_CLI_MODE + } else { + process.env.KILO_CLI_MODE = originalCliMode + } + } + }) - test("resolveWebviewView sets up webview correctly", async () => { - await provider.resolveWebviewView(mockWebviewView) + test("resolveWebviewView sets up webview correctly", async () => { + await provider.resolveWebviewView(mockWebviewView) - expect(mockWebviewView.webview.options).toEqual({ - enableScripts: true, + expect(mockWebviewView.webview.options).toEqual({ + enableScripts: true, localResourceRoots: [mockContext.extensionUri], }) diff --git a/webview-ui/src/i18n/locales/en/agentManager.json b/webview-ui/src/i18n/locales/en/agentManager.json index f1d452de587..6cee9744adb 100644 --- a/webview-ui/src/i18n/locales/en/agentManager.json +++ b/webview-ui/src/i18n/locales/en/agentManager.json @@ -70,7 +70,9 @@ "usingTool": "Using tool: **{{tool}}** {{details}}", "expandOutput": "Expand output", "collapseOutput": "Collapse output", - "copyCommand": "Copy command" + "copyCommand": "Copy command", + "approve": "Approve", + "reject": "Reject" }, "chatInput": { "autoMode": "Agent running in full-auto mode - no input allowed", diff --git a/webview-ui/src/kilocode/agent-manager/components/MessageList.tsx b/webview-ui/src/kilocode/agent-manager/components/MessageList.tsx index 43b320b2d6f..2729849bcb7 100644 --- a/webview-ui/src/kilocode/agent-manager/components/MessageList.tsx +++ b/webview-ui/src/kilocode/agent-manager/components/MessageList.tsx @@ -27,6 +27,8 @@ import { User, Clock, Loader, + Check, + X, } from "lucide-react" import { cn } from "../../../lib/utils" @@ -137,6 +139,29 @@ export function MessageList({ sessionId }: MessageListProps) { [removeFromQueue], ) + // Approval handlers - send approval/rejection to extension + const handleApprove = useCallback( + (sid: string) => { + vscode.postMessage({ + type: "agentManager.respondToApproval", + sessionId: sid, + approved: true, + }) + }, + [], + ) + + const handleReject = useCallback( + (sid: string) => { + vscode.postMessage({ + type: "agentManager.respondToApproval", + sessionId: sid, + approved: false, + }) + }, + [], + ) + if (messages.length === 0 && queue.length === 0) { return (
@@ -154,9 +179,12 @@ export function MessageList({ sessionId }: MessageListProps) { key={msg.ts || idx} message={msg} isLast={idx === combinedMessages.length - 1} + sessionId={sessionId} commandExecutionByTs={commandExecutionByTs} onSuggestionClick={handleSuggestionClick} onCopyToInput={handleCopyToInput} + onApprove={handleApprove} + onReject={handleReject} /> ))} {/* Display queued messages */} @@ -194,14 +222,38 @@ function extractFollowUpData(message: ClineMessage): { question: string; suggest interface MessageItemProps { message: ClineMessage isLast: boolean + sessionId: string commandExecutionByTs: Map onSuggestionClick?: (suggestion: SuggestionItem) => void onCopyToInput?: (suggestion: SuggestionItem) => void + onApprove?: (sessionId: string) => void + onReject?: (sessionId: string) => void } -function MessageItem({ message, isLast, commandExecutionByTs, onSuggestionClick, onCopyToInput }: MessageItemProps) { +function MessageItem({ + message, + isLast, + sessionId, + commandExecutionByTs, + onSuggestionClick, + onCopyToInput, + onApprove, + onReject, +}: MessageItemProps) { const { t } = useTranslation("agentManager") + // --- 0. Determine if approval buttons should be shown --- + // Show buttons for ask messages that require approval (not answered, not partial) + // Note: We don't require isLast because the CLI waits for approval before continuing, + // so this ask message should remain actionable until answered. + const approvalAskTypes = ["tool", "command", "browser_action_launch", "use_mcp_server"] + const needsApproval = + message.type === "ask" && + message.ask && + approvalAskTypes.includes(message.ask) && + !message.isAnswered && + !message.partial + // --- 1. Determine Message Style & Content --- // Note: CLI JSON output uses "content" instead of "text" for message body const messageText = message.text || (message as any).content || "" @@ -332,6 +384,23 @@ function MessageItem({ message, isLast, commandExecutionByTs, onSuggestionClick, {extraInfo}
{content &&
{content}
} + {/* Approval buttons for ask messages that require user approval */} + {needsApproval && onApprove && onReject && ( +
+ + +
+ )} {suggestions && suggestions.length > 0 && onSuggestionClick && ( ({ @@ -32,6 +33,10 @@ vi.mock("../../../../components/ui", () => ({ describe("MessageList", () => { const sessionId = "test-session" + beforeEach(() => { + vi.clearAllMocks() + }) + describe("handleCopyToInput", () => { it("appends suggestion to empty input", () => { const store = createStore() @@ -293,4 +298,85 @@ describe("MessageList", () => { expect(screen.getByText("messages.tool")).toBeInTheDocument() }) }) + + describe("approval buttons", () => { + it("renders approve/reject for ask:command and posts approval response", () => { + const store = createStore() + store.set(sessionMessagesAtomFamily(sessionId), [ + { + ts: 1, + type: "ask", + ask: "command", + text: "echo hello", + partial: false, + isAnswered: false, + } as ClineMessage, + ]) + + render( + + + , + ) + + fireEvent.click(screen.getByText("messages.approve")) + + expect(vscode.postMessage).toHaveBeenCalledWith({ + type: "agentManager.respondToApproval", + sessionId, + approved: true, + }) + }) + + it("posts reject response when clicking reject", () => { + const store = createStore() + store.set(sessionMessagesAtomFamily(sessionId), [ + { + ts: 1, + type: "ask", + ask: "tool", + text: JSON.stringify({ tool: "readFile", path: "foo.txt" }), + partial: false, + isAnswered: false, + } as ClineMessage, + ]) + + render( + + + , + ) + + fireEvent.click(screen.getByText("messages.reject")) + + expect(vscode.postMessage).toHaveBeenCalledWith({ + type: "agentManager.respondToApproval", + sessionId, + approved: false, + }) + }) + + it("does not render buttons for answered asks", () => { + const store = createStore() + store.set(sessionMessagesAtomFamily(sessionId), [ + { + ts: 1, + type: "ask", + ask: "command", + text: "echo hello", + partial: false, + isAnswered: true, + } as ClineMessage, + ]) + + render( + + + , + ) + + expect(screen.queryByText("messages.approve")).not.toBeInTheDocument() + expect(screen.queryByText("messages.reject")).not.toBeInTheDocument() + }) + }) })