release: v6.2.6#87
Conversation
When VS Code is launched from macOS Dock/Finder, the extension inherits launchd's restricted PATH (/usr/bin:/bin) instead of the user's full shell PATH configured in .zshrc/.bashrc. This causes failures for CLI tools installed via Homebrew, nvm, cargo, and other package managers. Introduce ShellEnvironment.ts which captures the user's login shell env by running 'shell -l -c env' and caching the result. This mirrors the ShellSnapshot pattern used by ClaudeCode. Changes: - New ShellEnvironment.ts: captures login-shell env at startup, caches result, exposes getShellEnvironment()/getCapturedShell()/invalidateShellEnvironment() - ExecaTerminalProcess: use captured shell path (bash/zsh) instead of generic shell:true; use captured env instead of process.env so CLI tools on user PATH are reachable - extension.ts: eagerly capture shell environment during activation before any terminal processes are spawned - Updated tests to use invalidateShellEnvironment() in beforeEach/afterEach to ensure clean state, and loosen assertions for cross-platform safety
When the extension's built-in model list changes (models added/removed), a user's saved kilocodeModel config may reference a model no longer in the list. This caused either empty model selections or 404 errors from the API. Add a defense-in-depth validation at two layers: 1. ClineProvider (startup validation): On initialization, check whether the stored kilocodeModel is in the static KILO_CODE_MODELS map via new isValidKilocodeModel(). If the model was removed, reset the config to default (undefined kilocodeModel) and log the stale model name. 2. KilocodeOpenrouterHandler (runtime safety net): In getModel(), if the selected model is not found in the fetched (live) model list, fall back to the default model. This catches edge cases where initialization validation hasn't run yet or the model was removed from the remote list. New export: - isValidKilocodeModel(modelId): simple lookup in KILO_CODE_MODELS map
Patch version bump for the shell environment capture feature and stale model validation fix.
…patibility Relax required fields across all native tool parameter schemas to accommodate older/stale models that may omit optional parameters: - ask_followup_question: make follow_up optional - browser_action: make url, coordinate, size, text optional (only action required) - codebase_search: make path optional - execute_command: make cwd optional, add message field for approval context - generate_image: make image optional - list_files: make recursive optional - new_task: make todos optional - run_slash_command: make args optional - switch_mode: make reason optional Also handle "null" string values in codebaseSearchTool to prevent validation failures when stale models pass null as a literal string.
…d navigation Add an interactive approval mode to CommandExecution that presents users with three numbered options before running a command: - Yes: approve the command - Yes, and dont ask again: approve and remember the command pattern - No, with feedback: reject and provide guidance Support keyboard navigation (ArrowUp/Down, Enter, Escape) for quick approvals. Also add a custom message field in executeCommand so the model can describe the command purpose to the user before approval. Update parseCommandAndOutput to extract a MESSAGE: prefix from the command text, enabling the approval header display.
ContextSummary By MatterAI
🔄 What Changed
🔍 Impact of the Change
📁 Total Files ChangedClick to Expand
🧪 Test Added/RecommendedAdded
🔒 Security Vulnerabilities
ImplementationThe PR refactors the shell environment scraping mechanism to use synchronous, array-based execution for safety. It implements a Screenshots
How to Test
Get in TouchDiscord: @matterai_agent |
![matterai-app[bot]](https://avatars.githubusercontent.com/in/474549?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧪 PR Review is completed: Shell environment capture introduces command injection risk via unsanitized shell path; approval-mode keyboard handler leaks global listeners and has stale-closure race conditions.
⬇️ Low Priority Suggestions (5)
src/integrations/terminal/ShellEnvironment.ts (1 suggestion)
Location:
src/integrations/terminal/ShellEnvironment.ts(Lines 60-62)🟡 Code Quality
Issue:
console.logat L60 andconsole.warnat L65 useconsoledirectly instead of the extension's structured logging infrastructure (outputChannel/createDualLogger). This bypasses log levels and makes debugging harder in production.Fix: Inject or import the proper logger and use it instead of
console.Impact: Consistent log routing and level control
- console.log( - `[ShellEnvironment] Captured ${Object.keys(env).length} env vars from ${shell} -l (PATH=${env.PATH?.slice(0, 80)}...)`, - ) + logger.info( + `[ShellEnvironment] Captured ${Object.keys(env).length} env vars from ${shell} -l (PATH=${env.PATH?.slice(0, 80)}...)`, + ) +
webview-ui/src/components/chat/CommandExecution.tsx (2 suggestions)
Location:
webview-ui/src/components/chat/CommandExecution.tsx(Lines 248-280)🔴 Concurrency / Memory Leak
Issue: The keyboard
useEffectat L248 registers a globalwindowkeydown listener. It does not check if the component is still mounted before callingsetSelectedOption,handleSubmit, orhandleSkipin the async gap aftere.preventDefault(). Rapid unmounting (e.g. message stream clearing) can leave a dangling listener that fires on unrelated UI elements, and the state updaters may operate on a destroyed component.Fix: Use a mounted ref and remove the listener on cleanup; also gate handler execution with the ref.
Impact: Prevents memory leaks and stale-state updates after unmount
- useEffect(() => { - if (!isApprovalMode) return - - const handleKeyDown = (e: KeyboardEvent) => { - const currentIndex = APPROVAL_OPTIONS.findIndex((o) => o.key === selectedOption) - - switch (e.key) { - case "ArrowUp": - e.preventDefault() - if (currentIndex > 0) { - setSelectedOption(APPROVAL_OPTIONS[currentIndex - 1].key) - } - break - case "ArrowDown": - e.preventDefault() - if (currentIndex < APPROVAL_OPTIONS.length - 1) { - setSelectedOption(APPROVAL_OPTIONS[currentIndex + 1].key) - } - break - case "Enter": - e.preventDefault() - handleSubmit() - break - case "Escape": - e.preventDefault() - handleSkip() - break - } - } - - window.addEventListener("keydown", handleKeyDown) - return () => window.removeEventListener("keydown", handleKeyDown) - }, [isApprovalMode, selectedOption, handleSubmit, handleSkip]) + // Keyboard navigation for approval options + useEffect(() => { + if (!isApprovalMode) return + + const mounted = { current: true } + + const handleKeyDown = (e: KeyboardEvent) => { + if (!mounted.current) return + + const currentIndex = APPROVAL_OPTIONS.findIndex((o) => o.key === selectedOption) + + switch (e.key) { + case "ArrowUp": + e.preventDefault() + if (currentIndex > 0) { + setSelectedOption(APPROVAL_OPTIONS[currentIndex - 1].key) + } + break + case "ArrowDown": + e.preventDefault() + if (currentIndex < APPROVAL_OPTIONS.length - 1) { + setSelectedOption(APPROVAL_OPTIONS[currentIndex + 1].key) + } + break + case "Enter": + e.preventDefault() + handleSubmit() + break + case "Escape": + e.preventDefault() + handleSkip() + break + } + } + + window.addEventListener("keydown", handleKeyDown) + return () => { + mounted.current = false + window.removeEventListener("keydown", handleKeyDown) + } + }, [isApprovalMode, selectedOption, handleSubmit, handleSkip]) +Location:
webview-ui/src/components/chat/CommandExecution.tsx(Lines 206-238)🟠 Logic Error
Issue:
handleSubmitat L207 capturescommandin its dependency array, butcommandcomes fromparseCommandAndOutput(text)which is memoized. Iftextchanges while the approval UI is visible (e.g. streaming update), the memoizedcommandstays stale, yethandleSubmitwill use the oldcommandto compute the allow-list pattern. This can whitelist the wrong command.Fix: Ensure the pattern added to
allowedCommandsis derived from the latestcommandat submit time, not the closure-captured value. Use a functional update or ref for the latest command value.Impact: Prevents incorrect command patterns from being permanently allowed
- // Handle Submit button click based on selected option - const handleSubmit = useCallback(() => { - if (!onPrimaryButtonClick || !onSecondaryButtonClick) return - - switch (selectedOption) { - case "yes": - onPrimaryButtonClick() - break - case "yes_always": - // Add the command pattern to allowedCommands before approving - if (command && command.trim()) { - const pattern = command.trim() - if (!allowedCommands.includes(pattern)) { - const newAllowed = [...allowedCommands, pattern] - setAllowedCommands(newAllowed) - vscode.postMessage({ type: "allowedCommands", commands: newAllowed }) - } - } - onPrimaryButtonClick() - break - case "no_feedback": - onSecondaryButtonClick(feedbackText || undefined) - break - } - }, [ - selectedOption, - onPrimaryButtonClick, - onSecondaryButtonClick, - command, - allowedCommands, - setAllowedCommands, - feedbackText, - ]) + // Handle Submit button click based on selected option + const handleSubmit = useCallback(() => { + if (!onPrimaryButtonClick || !onSecondaryButtonClick) return + + const currentCommand = commandRef.current + + switch (selectedOption) { + case "yes": + onPrimaryButtonClick() + break + case "yes_always": + // Add the command pattern to allowedCommands before approving + if (currentCommand && currentCommand.trim()) { + const pattern = currentCommand.trim() + if (!allowedCommands.includes(pattern)) { + const newAllowed = [...allowedCommands, pattern] + setAllowedCommands(newAllowed) + vscode.postMessage({ type: "allowedCommands", commands: newAllowed }) + } + } + onPrimaryButtonClick() + break + case "no_feedback": + onSecondaryButtonClick(feedbackText || undefined) + break + } + }, [ + selectedOption, + onPrimaryButtonClick, + onSecondaryButtonClick, + allowedCommands, + setAllowedCommands, + feedbackText, + ]) +
src/core/webview/ClineProvider.ts (1 suggestion)
Location:
src/core/webview/ClineProvider.ts(Lines 2294-2307)🟠 Logic Error
Issue: The stale-model reset at L2297-L2307 sets
kilocodeModel: undefinedin the persisted config, but does not await or verify thatgetKilocodeDefaultModel()will actually resolve to a valid model before the rest of the initialization proceeds. If the default model fetch also fails, the system may end up with no model selected silently.Fix: After clearing the stale model, explicitly fall back to the default and validate it, or surface a warning to the user if no valid model can be determined.
Impact: Prevents silent failure into an unconfigured model state
- // Validate global kilocodeModel against available models. - // If the stored model ID was removed from the extension's model list - // (e.g. after an update), force-reset to the default model. - if (apiConfiguration?.apiProvider === "kilocode" && apiConfiguration?.kilocodeModel) { - if (!isValidKilocodeModel(apiConfiguration.kilocodeModel)) { - const staleModel = apiConfiguration.kilocodeModel - const updatedConfig = { ...apiConfiguration, kilocodeModel: undefined } - await this.contextProxy.setProviderSettings(updatedConfig) - mergedApiConfiguration = { ...mergedApiConfiguration, kilocodeModel: undefined } - this.log( - `[ModelValidation] Reset stale kilocodeModel "${staleModel}" to default — model no longer available`, - ) - } - } + if (apiConfiguration?.apiProvider === "kilocode" && apiConfiguration?.kilocodeModel) { + if (!isValidKilocodeModel(apiConfiguration.kilocodeModel)) { + const staleModel = apiConfiguration.kilocodeModel + const defaultModel = getKilocodeDefaultModel() + const updatedConfig = { ...apiConfiguration, kilocodeModel: defaultModel } + await this.contextProxy.setProviderSettings(updatedConfig) + mergedApiConfiguration = { ...mergedApiConfiguration, kilocodeModel: defaultModel } + this.log( + `[ModelValidation] Reset stale kilocodeModel "${staleModel}" to default "${defaultModel}" — model no longer available`, + ) + } + } +
src/api/providers/kilocode-openrouter.ts (1 suggestion)
Location:
src/api/providers/kilocode-openrouter.ts(Lines 92-94)🟡 Code Quality
Issue: The fallback logic at L92 checks
Object.keys(this.models).length > 0before verifying!this.models[id]. This is redundant because!this.models[id]is already falsy when the model map is empty; the extra length check adds noise and a tiny performance cost.Fix: Simplify the condition to just
if (id && !this.models[id]).Impact: Cleaner, more idiomatic code
- if (id && Object.keys(this.models).length > 0 && !this.models[id]) { - id = this.defaultModel - } + if (id && !this.models[id]) { + id = this.defaultModel + } +
| const output = execSync(`${shell} -l -c 'env'`, { | ||
| encoding: "utf8", | ||
| timeout: 5000, | ||
| env: process.env as Record<string, string>, | ||
| stdio: ["ignore", "pipe", "pipe"], | ||
| }) |
There was a problem hiding this comment.
🔴 Security
Issue: execSync at L37 concatenates shell (from getShell()) directly into a command string without validation or escaping. If getShell() returns a path containing shell metacharacters (e.g. from a malicious VS Code setting), this is command injection.
Fix: Pass the shell and command as arguments using execFileSync or execSync with an array-form command, avoiding string concatenation.
Impact: Prevents arbitrary command execution from a compromised shell path
| const output = execSync(`${shell} -l -c 'env'`, { | |
| encoding: "utf8", | |
| timeout: 5000, | |
| env: process.env as Record<string, string>, | |
| stdio: ["ignore", "pipe", "pipe"], | |
| }) | |
| const output = execSync("env", { | |
| shell, | |
| encoding: "utf8", | |
| timeout: 5000, | |
| env: process.env as Record<string, string>, | |
| stdio: ["ignore", "pipe", "pipe"], | |
| }) | |
| const rawCwd: string | undefined = block.params.cwd | ||
| const customCwd: string | undefined = rawCwd && rawCwd !== "null" ? rawCwd : undefined | ||
| const customMessage: string | undefined = block.params.message |
There was a problem hiding this comment.
🟠 Logic Error
Issue: customMessage at L34 is extracted from block.params.message, but there is no validation that block.params.message is actually a string. If the LLM emits a non-string value (e.g. object, number), the downstream askText construction at L59 will produce [object Object] or similar garbage in the approval prompt.
Fix: Validate or coerce block.params.message to string before using it.
Impact: Prevents malformed approval prompts from reaching the user
| const rawCwd: string | undefined = block.params.cwd | |
| const customCwd: string | undefined = rawCwd && rawCwd !== "null" ? rawCwd : undefined | |
| const customMessage: string | undefined = block.params.message | |
| const rawMessage: unknown = block.params.message | |
| const customMessage: string | undefined = | |
| typeof rawMessage === "string" && rawMessage !== "null" ? rawMessage : undefined | |
- ShellEnvironment: use execFileSync instead of execSync for command injection prevention - ShellEnvironment: add setShellLogger() for diagnostic output via outputChannel - ShellEnvironment: fall back to process.env when login shell produces empty value - CommandExecution: add commandRef to prevent stale closure on streaming updates - CommandExecution: add mounted ref guard in keyboard useEffect cleanup - ClineProvider: await getKilocodeDefaultModel() for stale model reset - kilocode-openrouter: remove redundant Object.keys check in model lookup - executeCommandTool: guard against non-string customMessage values - extension.ts: wire ShellEnvironment logger to outputChannel - Add tests for ShellEnvironment, CommandExecution approval mode, ClineProvider model validation
There was a problem hiding this comment.
🧪 PR Review is completed: Reviewed 8 files. Found 1 new issue in ShellEnvironment.ts (unhandled execFileSync exception edge case) and verified previous issues are resolved. No issues in remaining files.
⬇️ Low Priority Suggestions (1)
src/integrations/terminal/ShellEnvironment.ts (1 suggestion)
Location:
src/integrations/terminal/ShellEnvironment.ts(Lines 82-83)🟠 Error Handling
Issue:
execFileSyncat L50 can throw exceptions that bypass the try-catch if the shell path doesn't exist or lacks execute permissions. The current catch block at L81 handles general errors, butexecFileSyncthrowsErrorobjects withstatus,signal,stdout,stderrproperties that may contain partial output. The current error logging only captureserror.message, potentially losing diagnostic stderr data that could help debug shell environment capture failures.Fix: Capture and log
stderrfrom the error object when available to improve debuggability of shell environment capture failures.Impact: Better diagnostic information when shell environment capture fails due to shell misconfiguration or permission issues
- logger( - `[ShellEnvironment] Failed to capture shell environment: ${error instanceof Error ? error.message : String(error)}`, + logger( + `[ShellEnvironment] Failed to capture shell environment: ${error instanceof Error ? error.message : String(error)}${error && typeof error === "object" && "stderr" in error ? ` (stderr: ${String((error as any).stderr).slice(0, 200)})` : ""}`, + ) +
1 participant
Changes
fix(tools): relax native tool schema requirements for stale model compatibility
codebaseSearchToolfor stale model responsesfeat(ui): add command approval mode with option selection and keyboard navigation