From 7c0e187dccd6c35e95f785a2ef0ef8e068282f47 Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Fri, 19 Dec 2025 17:45:44 +0100 Subject: [PATCH 01/10] fix: resolve Windows CLI spawn ENOENT error in Agent Manager Replace shell command-based CLI detection with filesystem-based executable resolution using PATHEXT environment variable. --- .../kilocode/agent-manager/CliPathResolver.ts | 155 +++++++----- .../agent-manager/CliProcessHandler.ts | 17 ++ .../__tests__/AgentManagerProvider.spec.ts | 27 +- .../__tests__/CliPathResolver.spec.ts | 231 ++++++++++++++++-- src/core/kilocode/agent-manager/telemetry.ts | 4 + 5 files changed, 361 insertions(+), 73 deletions(-) diff --git a/src/core/kilocode/agent-manager/CliPathResolver.ts b/src/core/kilocode/agent-manager/CliPathResolver.ts index e161ef40fec..8bceda15e76 100644 --- a/src/core/kilocode/agent-manager/CliPathResolver.ts +++ b/src/core/kilocode/agent-manager/CliPathResolver.ts @@ -4,6 +4,96 @@ import { execSync } from "node:child_process" import { fileExistsAtPath } from "../../../utils/fs" import { getLocalCliPath } from "./CliInstaller" +/** + * Case-insensitive lookup for environment variables. + * Windows environment variables can have inconsistent casing (PATH, Path, path). + */ +function getCaseInsensitive(target: NodeJS.ProcessEnv, key: string): string | undefined { + const lowercaseKey = key.toLowerCase() + const equivalentKey = Object.keys(target).find((k) => k.toLowerCase() === lowercaseKey) + return equivalentKey ? target[equivalentKey] : target[key] +} + +/** + * Check if a path exists and is a file (not a directory). + */ +async function fileExistsAsFile(filePath: string): Promise { + try { + const stat = await fs.promises.stat(filePath) + return stat.isFile() || stat.isSymbolicLink() + } catch (e: unknown) { + if (e instanceof Error && e.message.startsWith("EACCES")) { + try { + const lstat = await fs.promises.lstat(filePath) + return lstat.isFile() || lstat.isSymbolicLink() + } catch { + return false + } + } + return false + } +} + +/** + * Find an executable by name, resolving it against PATH and PATHEXT (on Windows). + */ +export async function findExecutable( + command: string, + cwd?: string, + paths?: string[], + env: NodeJS.ProcessEnv = process.env, +): Promise { + if (path.isAbsolute(command)) { + return (await fileExistsAsFile(command)) ? command : undefined + } + + if (cwd === undefined) { + cwd = process.cwd() + } + + const dir = path.dirname(command) + if (dir !== ".") { + const fullPath = path.join(cwd, command) + return (await fileExistsAsFile(fullPath)) ? fullPath : undefined + } + + const envPath = getCaseInsensitive(env, "PATH") + if (paths === undefined && typeof envPath === "string") { + paths = envPath.split(path.delimiter) + } + + if (paths === undefined || paths.length === 0) { + const fullPath = path.join(cwd, command) + return (await fileExistsAsFile(fullPath)) ? fullPath : undefined + } + + for (const pathEntry of paths) { + let fullPath: string + if (path.isAbsolute(pathEntry)) { + fullPath = path.join(pathEntry, command) + } else { + fullPath = path.join(cwd, pathEntry, command) + } + + if (process.platform === "win32") { + const pathExt = getCaseInsensitive(env, "PATHEXT") || ".COM;.EXE;.BAT;.CMD" + for (const ext of pathExt.split(";")) { + const withExtension = fullPath + ext + if (await fileExistsAsFile(withExtension)) { + return withExtension + } + } + } + + if (await fileExistsAsFile(fullPath)) { + return fullPath + } + } + + const fullPath = path.join(cwd, command) + return (await fileExistsAsFile(fullPath)) ? fullPath : undefined +} + /** * Find the kilocode CLI executable. * @@ -12,7 +102,7 @@ import { getLocalCliPath } from "./CliInstaller" * 2. Workspace-local build at /cli/dist/index.js * 3. Local installation at ~/.kilocode/cli/pkg (for immutable systems like NixOS) * 4. Login shell lookup (respects user's nvm, fnm, volta, asdf config) - * 5. Direct PATH lookup (fallback for system-wide installs) + * 5. Direct PATH lookup using findExecutable (handles PATHEXT on Windows) * 6. Common npm installation paths (last resort) * * IMPORTANT: Login shell is checked BEFORE direct PATH because: @@ -50,7 +140,6 @@ export async function findKilocodeCli(log?: (msg: string) => void): Promise void): Promise void): Promise void): string | null { - const cmd = process.platform === "win32" ? "where kilocode" : "which kilocode" - try { - const result = execSync(cmd, { encoding: "utf-8", timeout: 5000 }).split(/\r?\n/)[0]?.trim() - if (result) { - log?.(`Found CLI in PATH: ${result}`) - return result - } - } catch { - log?.("kilocode not found in direct PATH lookup") - } - return null -} - /** * Try to find kilocode by running `which` in a login shell. * This sources the user's shell profile (~/.zshrc, ~/.bashrc, etc.) * which sets up version managers like nvm, fnm, volta, asdf, etc. - * - * This is the most reliable way to find CLI installed via version managers - * because VS Code's extension host doesn't inherit the user's shell environment. */ function findViaLoginShell(log?: (msg: string) => void): string | null { if (process.platform === "win32") { - // Windows doesn't have the same shell environment concept return null } - // Detect user's shell from SHELL env var, default to bash const userShell = process.env.SHELL || "/bin/bash" const shellName = path.basename(userShell) - // Use login shell (-l) to source profile files, interactive (-i) for some shells - // that only source certain files in interactive mode const shellFlags = shellName === "zsh" ? "-l -i" : "-l" const cmd = `${userShell} ${shellFlags} -c 'which kilocode' 2>/dev/null` @@ -129,8 +195,8 @@ function findViaLoginShell(log?: (msg: string) => void): string | null { log?.(`Trying login shell lookup: ${cmd}`) const result = execSync(cmd, { encoding: "utf-8", - timeout: 10000, // 10s timeout - login shells can be slow - env: { ...process.env, HOME: process.env.HOME }, // Ensure HOME is set + timeout: 10000, + env: { ...process.env, HOME: process.env.HOME }, }) .split(/\r?\n/)[0] ?.trim() @@ -140,7 +206,6 @@ function findViaLoginShell(log?: (msg: string) => void): string | null { return result } } catch (error) { - // This is expected if CLI is not installed or shell init is slow/broken log?.(`Login shell lookup failed (this is normal if CLI not installed via version manager): ${error}`) } @@ -149,7 +214,6 @@ function findViaLoginShell(log?: (msg: string) => void): string | null { /** * Get fallback paths to check for CLI installation. - * This is used when login shell lookup fails or on Windows. */ function getNpmPaths(log?: (msg: string) => void): string[] { const home = process.env.HOME || process.env.USERPROFILE || "" @@ -164,27 +228,16 @@ function getNpmPaths(log?: (msg: string) => void): string[] { ].filter(Boolean) } - // macOS and Linux paths const paths = [ - // Local installation (for immutable systems like NixOS) getLocalCliPath(), - // macOS Homebrew (Apple Silicon) "/opt/homebrew/bin/kilocode", - // macOS Homebrew (Intel) and Linux standard "/usr/local/bin/kilocode", - // Common user-local npm prefix path.join(home, ".npm-global", "bin", "kilocode"), - // nvm: scan installed versions ...getNvmPaths(home, log), - // fnm path.join(home, ".local", "share", "fnm", "aliases", "default", "bin", "kilocode"), - // volta path.join(home, ".volta", "bin", "kilocode"), - // asdf nodejs plugin path.join(home, ".asdf", "shims", "kilocode"), - // Linux snap "/snap/bin/kilocode", - // Linux user local bin path.join(home, ".local", "bin", "kilocode"), ] @@ -193,10 +246,6 @@ function getNpmPaths(log?: (msg: string) => void): string[] { /** * Get potential nvm paths for the kilocode CLI. - * nvm installs node versions in ~/.nvm/versions/node/ - * - * Note: This is a fallback - the login shell approach (findViaLoginShell) - * is preferred because it respects the user's shell configuration. */ function getNvmPaths(home: string, log?: (msg: string) => void): string[] { const nvmDir = process.env.NVM_DIR || path.join(home, ".nvm") @@ -204,16 +253,13 @@ function getNvmPaths(home: string, log?: (msg: string) => void): string[] { const paths: string[] = [] - // Check NVM_BIN if set (current nvm version in the shell) if (process.env.NVM_BIN) { paths.push(path.join(process.env.NVM_BIN, "kilocode")) } - // Scan the nvm versions directory for installed node versions try { if (fs.existsSync(versionsDir)) { const versions = fs.readdirSync(versionsDir) - // Sort versions in reverse order to check newer versions first versions.sort().reverse() log?.(`Found ${versions.length} nvm node versions to check`) for (const version of versions) { @@ -221,7 +267,6 @@ function getNvmPaths(home: string, log?: (msg: string) => void): string[] { } } } catch (error) { - // This is normal if user doesn't have nvm installed log?.(`Could not scan nvm versions directory: ${error}`) } diff --git a/src/core/kilocode/agent-manager/CliProcessHandler.ts b/src/core/kilocode/agent-manager/CliProcessHandler.ts index 7f498a6b3db..57a88b36fc4 100644 --- a/src/core/kilocode/agent-manager/CliProcessHandler.ts +++ b/src/core/kilocode/agent-manager/CliProcessHandler.ts @@ -1,4 +1,5 @@ import { spawn, ChildProcess } from "node:child_process" +import * as path from "node:path" import { CliOutputParser, type StreamEvent, @@ -37,6 +38,7 @@ interface PendingProcessInfo { gitUrl?: string stderrBuffer: string[] // Capture stderr for error detection timeoutId?: NodeJS.Timeout // Timer for auto-failing stuck pending sessions + cliPath?: string // CLI path for error telemetry } interface ActiveProcessInfo { @@ -209,6 +211,7 @@ export class CliProcessHandler { gitUrl: options?.gitUrl, stderrBuffer: [], timeoutId: setTimeout(() => this.handlePendingTimeout(), PENDING_SESSION_TIMEOUT_MS), + cliPath, } } @@ -573,10 +576,24 @@ export class CliProcessHandler { private handleProcessError(proc: ChildProcess, error: Error): void { if (this.pendingProcess && this.pendingProcess.process === proc) { + const cliPath = this.pendingProcess.cliPath this.clearPendingTimeout() this.registry.clearPendingSession() this.callbacks.onPendingSessionChanged(null) this.pendingProcess = null + + // Capture spawn error telemetry with context for debugging + const { platform, shell } = getPlatformDiagnostics() + const cliPathExtension = cliPath ? path.extname(cliPath).slice(1).toLowerCase() || undefined : undefined + captureAgentManagerLoginIssue({ + issueType: "cli_spawn_error", + platform, + shell, + errorMessage: error.message, + cliPath, + cliPathExtension, + }) + this.callbacks.onStartSessionFailed({ type: "spawn_error", message: error.message, diff --git a/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts b/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts index 2ae8d7c7cb7..baf36b8a914 100644 --- a/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts +++ b/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts @@ -115,6 +115,21 @@ describe("AgentManagerProvider CLI spawning", () => { getRemoteUrl: vi.fn().mockResolvedValue(undefined), })) + // Mock fs to make findExecutable find the .cmd file + const cmdPath = "/npm/kilocode.CMD" + vi.doMock("node:fs", () => ({ + existsSync: vi.fn().mockReturnValue(false), + readdirSync: vi.fn().mockReturnValue([]), + promises: { + stat: vi.fn().mockImplementation((filePath: string) => { + if (filePath === cmdPath) { + return Promise.resolve({ isFile: () => true }) + } + return Promise.reject(new Error("ENOENT")) + }), + }, + })) + class TestProc extends EventEmitter { stdout = new EventEmitter() stderr = new EventEmitter() @@ -123,17 +138,20 @@ describe("AgentManagerProvider CLI spawning", () => { } const spawnMock = vi.fn(() => new TestProc()) - // Return a .cmd path to simulate Windows local CLI installation - const execSyncMock = vi.fn(() => "C:\\Users\\test\\.kilocode\\cli\\pkg\\node_modules\\.bin\\kilocode.cmd") + const execSyncMock = vi.fn().mockImplementation(() => { + throw new Error("not found") + }) vi.doMock("node:child_process", () => ({ spawn: spawnMock, execSync: execSyncMock, })) - // Mock process.platform to be win32 + // Mock process.platform to be win32 and set PATH const originalPlatform = process.platform + const originalPath = process.env.PATH Object.defineProperty(process, "platform", { value: "win32", writable: true }) + process.env.PATH = "/npm" try { const module = await import("../AgentManagerProvider") @@ -148,8 +166,9 @@ describe("AgentManagerProvider CLI spawning", () => { windowsProvider.dispose() } finally { - // Restore original platform + // Restore original platform and PATH Object.defineProperty(process, "platform", { value: originalPlatform, writable: true }) + process.env.PATH = originalPath } }) diff --git a/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts b/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts index 663a65cb5c3..16c34d53ef2 100644 --- a/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts +++ b/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi, beforeEach } from "vitest" +import * as path from "node:path" const isWindows = process.platform === "win32" @@ -10,40 +11,45 @@ describe("findKilocodeCli", () => { const loginShellTests = isWindows ? it.skip : it loginShellTests("finds CLI via login shell and returns trimmed result", async () => { - // Login shell is tried first, so mock it to succeed const execSyncMock = vi.fn().mockReturnValue("/Users/test/.nvm/versions/node/v20/bin/kilocode\n") vi.doMock("node:child_process", () => ({ execSync: execSyncMock })) vi.doMock("../../../../utils/fs", () => ({ fileExistsAtPath: vi.fn().mockResolvedValue(false) })) + vi.doMock("node:fs", () => ({ + existsSync: vi.fn().mockReturnValue(false), + promises: { stat: vi.fn().mockRejectedValue(new Error("ENOENT")) }, + })) const { findKilocodeCli } = await import("../CliPathResolver") const result = await findKilocodeCli() expect(result).toBe("/Users/test/.nvm/versions/node/v20/bin/kilocode") - // First call should be login shell (on non-Windows) expect(execSyncMock).toHaveBeenCalledWith( expect.stringContaining("which kilocode"), expect.objectContaining({ encoding: "utf-8" }), ) }) - loginShellTests("falls back to direct PATH when login shell fails", async () => { - let callCount = 0 - const execSyncMock = vi.fn().mockImplementation((cmd: string) => { - callCount++ - // First call (login shell) fails, second call (direct PATH) succeeds - if (callCount === 1) { - throw new Error("login shell failed") + loginShellTests("falls back to findExecutable when login shell fails", async () => { + const execSyncMock = vi.fn().mockImplementation(() => { + throw new Error("login shell failed") + }) + const statMock = vi.fn().mockImplementation((filePath: string) => { + if (filePath === "/usr/local/bin/kilocode") { + return Promise.resolve({ isFile: () => true }) } - return "/usr/local/bin/kilocode\n" + return Promise.reject(new Error("ENOENT")) }) vi.doMock("node:child_process", () => ({ execSync: execSyncMock })) vi.doMock("../../../../utils/fs", () => ({ fileExistsAtPath: vi.fn().mockResolvedValue(false) })) + vi.doMock("node:fs", () => ({ + existsSync: vi.fn().mockReturnValue(false), + promises: { stat: statMock }, + })) const { findKilocodeCli } = await import("../CliPathResolver") const result = await findKilocodeCli() expect(result).toBe("/usr/local/bin/kilocode") - expect(execSyncMock).toHaveBeenCalledTimes(2) }) it("falls back to npm paths when all PATH lookups fail", async () => { @@ -51,11 +57,14 @@ describe("findKilocodeCli", () => { throw new Error("not found") }) const fileExistsMock = vi.fn().mockImplementation((path: string) => { - // Return true for first path checked to verify fallback works return Promise.resolve(path.includes("kilocode")) }) vi.doMock("node:child_process", () => ({ execSync: execSyncMock })) vi.doMock("../../../../utils/fs", () => ({ fileExistsAtPath: fileExistsMock })) + vi.doMock("node:fs", () => ({ + existsSync: vi.fn().mockReturnValue(false), + promises: { stat: vi.fn().mockRejectedValue(new Error("ENOENT")) }, + })) const { findKilocodeCli } = await import("../CliPathResolver") const result = await findKilocodeCli() @@ -71,6 +80,10 @@ describe("findKilocodeCli", () => { }), })) vi.doMock("../../../../utils/fs", () => ({ fileExistsAtPath: vi.fn().mockResolvedValue(false) })) + vi.doMock("node:fs", () => ({ + existsSync: vi.fn().mockReturnValue(false), + promises: { stat: vi.fn().mockRejectedValue(new Error("ENOENT")) }, + })) const { findKilocodeCli } = await import("../CliPathResolver") const logMock = vi.fn() @@ -80,18 +93,208 @@ describe("findKilocodeCli", () => { expect(logMock).toHaveBeenCalledWith("kilocode CLI not found") }) - it("logs when kilocode not in direct PATH", async () => { + it("logs when kilocode not in PATH", async () => { vi.doMock("node:child_process", () => ({ execSync: vi.fn().mockImplementation(() => { throw new Error("not found") }), })) vi.doMock("../../../../utils/fs", () => ({ fileExistsAtPath: vi.fn().mockResolvedValue(false) })) + vi.doMock("node:fs", () => ({ + existsSync: vi.fn().mockReturnValue(false), + promises: { stat: vi.fn().mockRejectedValue(new Error("ENOENT")) }, + })) const { findKilocodeCli } = await import("../CliPathResolver") const logMock = vi.fn() await findKilocodeCli(logMock) - expect(logMock).toHaveBeenCalledWith("kilocode not found in direct PATH lookup") + expect(logMock).toHaveBeenCalledWith("kilocode not found in PATH lookup") + }) +}) + +describe("findExecutable", () => { + beforeEach(() => { + vi.resetModules() + }) + + it("returns absolute path if file exists", async () => { + const statMock = vi.fn().mockResolvedValue({ isFile: () => true }) + vi.doMock("node:fs", () => ({ + promises: { stat: statMock }, + })) + + const { findExecutable } = await import("../CliPathResolver") + const result = await findExecutable("/usr/bin/kilocode") + + expect(result).toBe("/usr/bin/kilocode") + }) + + it("returns undefined for absolute path if file does not exist", async () => { + const statMock = vi.fn().mockRejectedValue(new Error("ENOENT")) + vi.doMock("node:fs", () => ({ + promises: { stat: statMock }, + })) + + const { findExecutable } = await import("../CliPathResolver") + const result = await findExecutable("/usr/bin/nonexistent") + + expect(result).toBeUndefined() + }) + + it("searches PATH entries for command", async () => { + const statMock = vi.fn().mockImplementation((filePath: string) => { + if (filePath === "/custom/bin/myapp") { + return Promise.resolve({ isFile: () => true }) + } + return Promise.reject(new Error("ENOENT")) + }) + vi.doMock("node:fs", () => ({ + promises: { stat: statMock }, + })) + + const { findExecutable } = await import("../CliPathResolver") + const result = await findExecutable("myapp", "/home/user", ["/usr/bin", "/custom/bin"]) + + expect(result).toBe("/custom/bin/myapp") + }) + + describe("Windows PATHEXT handling", () => { + it("tries PATHEXT extensions on Windows", async () => { + const originalPlatform = process.platform + Object.defineProperty(process, "platform", { value: "win32", configurable: true }) + + try { + const expectedPath = path.join("/npm", "kilocode") + ".CMD" + const statMock = vi.fn().mockImplementation((filePath: string) => { + if (filePath === expectedPath) { + return Promise.resolve({ isFile: () => true }) + } + return Promise.reject(new Error("ENOENT")) + }) + vi.doMock("node:fs", () => ({ + promises: { stat: statMock }, + })) + + const { findExecutable } = await import("../CliPathResolver") + const result = await findExecutable("kilocode", "/home/test", ["/npm"], { + PATH: "/npm", + PATHEXT: ".COM;.EXE;.BAT;.CMD", + }) + + expect(result).toBe(expectedPath) + } finally { + Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true }) + } + }) + + it("uses default PATHEXT if not in env", async () => { + const originalPlatform = process.platform + Object.defineProperty(process, "platform", { value: "win32", configurable: true }) + + try { + const expectedPath = path.join("/npm", "kilocode") + ".CMD" + const statMock = vi.fn().mockImplementation((filePath: string) => { + if (filePath === expectedPath) { + return Promise.resolve({ isFile: () => true }) + } + return Promise.reject(new Error("ENOENT")) + }) + vi.doMock("node:fs", () => ({ + promises: { stat: statMock }, + })) + + const { findExecutable } = await import("../CliPathResolver") + const result = await findExecutable("kilocode", "/home/test", ["/npm"], { + PATH: "/npm", + }) + + expect(result).toBe(expectedPath) + } finally { + Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true }) + } + }) + + it("handles case-insensitive PATH lookup", async () => { + const originalPlatform = process.platform + Object.defineProperty(process, "platform", { value: "win32", configurable: true }) + + try { + const expectedPath = path.join("/npm", "kilocode") + ".EXE" + const statMock = vi.fn().mockImplementation((filePath: string) => { + if (filePath === expectedPath) { + return Promise.resolve({ isFile: () => true }) + } + return Promise.reject(new Error("ENOENT")) + }) + vi.doMock("node:fs", () => ({ + promises: { stat: statMock }, + })) + + const { findExecutable } = await import("../CliPathResolver") + const result = await findExecutable("kilocode", "/home/test", undefined, { + Path: "/npm", + PathExt: ".COM;.EXE;.BAT;.CMD", + }) + + expect(result).toBe(expectedPath) + } finally { + Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true }) + } + }) + + it("returns first matching PATHEXT extension", async () => { + const originalPlatform = process.platform + Object.defineProperty(process, "platform", { value: "win32", configurable: true }) + + try { + const comPath = path.join("/npm", "kilocode") + ".COM" + const exePath = path.join("/npm", "kilocode") + ".EXE" + const statMock = vi.fn().mockImplementation((filePath: string) => { + if (filePath === comPath || filePath === exePath) { + return Promise.resolve({ isFile: () => true }) + } + return Promise.reject(new Error("ENOENT")) + }) + vi.doMock("node:fs", () => ({ + promises: { stat: statMock }, + })) + + const { findExecutable } = await import("../CliPathResolver") + const result = await findExecutable("kilocode", "/home/test", ["/npm"], { + PATH: "/npm", + PATHEXT: ".COM;.EXE;.BAT;.CMD", + }) + + expect(result).toBe(comPath) + } finally { + Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true }) + } + }) + }) + + it("does not use PATHEXT on non-Windows platforms", async () => { + const originalPlatform = process.platform + Object.defineProperty(process, "platform", { value: "darwin", configurable: true }) + + try { + const statMock = vi.fn().mockImplementation((filePath: string) => { + if (filePath === "/usr/bin/kilocode") { + return Promise.resolve({ isFile: () => true }) + } + return Promise.reject(new Error("ENOENT")) + }) + vi.doMock("node:fs", () => ({ + promises: { stat: statMock }, + })) + + const { findExecutable } = await import("../CliPathResolver") + const result = await findExecutable("kilocode", "/home/user", ["/usr/bin"]) + + expect(result).toBe("/usr/bin/kilocode") + expect(statMock).not.toHaveBeenCalledWith(expect.stringContaining(".CMD")) + } finally { + Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true }) + } }) }) diff --git a/src/core/kilocode/agent-manager/telemetry.ts b/src/core/kilocode/agent-manager/telemetry.ts index 3d53774ab1a..322ce237cf7 100644 --- a/src/core/kilocode/agent-manager/telemetry.ts +++ b/src/core/kilocode/agent-manager/telemetry.ts @@ -29,6 +29,10 @@ export interface AgentManagerLoginIssueProperties { httpStatusCode?: number platform?: "darwin" | "win32" | "linux" | "other" shell?: string + // Spawn error details for debugging Windows issues + errorMessage?: string + cliPath?: string + cliPathExtension?: string } export function captureAgentManagerOpened(): void { From b50f1f2399198ba699fde09de1dfa30acde450ff Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Fri, 19 Dec 2025 18:13:43 +0100 Subject: [PATCH 02/10] fix: address PR review comments - Use error.code instead of error.message for EACCES detection - Rename fileExistsAsFile to pathExistsAsFile - Remove redundant isSymbolicLink check (stat follows symlinks) - Add clarifying comment about symlink behavior --- .../kilocode/agent-manager/CliPathResolver.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/core/kilocode/agent-manager/CliPathResolver.ts b/src/core/kilocode/agent-manager/CliPathResolver.ts index 8bceda15e76..eebcdf2a51b 100644 --- a/src/core/kilocode/agent-manager/CliPathResolver.ts +++ b/src/core/kilocode/agent-manager/CliPathResolver.ts @@ -16,13 +16,14 @@ function getCaseInsensitive(target: NodeJS.ProcessEnv, key: string): string | un /** * Check if a path exists and is a file (not a directory). + * Follows symlinks - a symlink to a file returns true, symlink to a directory returns false. */ -async function fileExistsAsFile(filePath: string): Promise { +async function pathExistsAsFile(filePath: string): Promise { try { const stat = await fs.promises.stat(filePath) - return stat.isFile() || stat.isSymbolicLink() + return stat.isFile() } catch (e: unknown) { - if (e instanceof Error && e.message.startsWith("EACCES")) { + if (e instanceof Error && "code" in e && e.code === "EACCES") { try { const lstat = await fs.promises.lstat(filePath) return lstat.isFile() || lstat.isSymbolicLink() @@ -44,7 +45,7 @@ export async function findExecutable( env: NodeJS.ProcessEnv = process.env, ): Promise { if (path.isAbsolute(command)) { - return (await fileExistsAsFile(command)) ? command : undefined + return (await pathExistsAsFile(command)) ? command : undefined } if (cwd === undefined) { @@ -54,7 +55,7 @@ export async function findExecutable( const dir = path.dirname(command) if (dir !== ".") { const fullPath = path.join(cwd, command) - return (await fileExistsAsFile(fullPath)) ? fullPath : undefined + return (await pathExistsAsFile(fullPath)) ? fullPath : undefined } const envPath = getCaseInsensitive(env, "PATH") @@ -64,7 +65,7 @@ export async function findExecutable( if (paths === undefined || paths.length === 0) { const fullPath = path.join(cwd, command) - return (await fileExistsAsFile(fullPath)) ? fullPath : undefined + return (await pathExistsAsFile(fullPath)) ? fullPath : undefined } for (const pathEntry of paths) { @@ -79,19 +80,19 @@ export async function findExecutable( const pathExt = getCaseInsensitive(env, "PATHEXT") || ".COM;.EXE;.BAT;.CMD" for (const ext of pathExt.split(";")) { const withExtension = fullPath + ext - if (await fileExistsAsFile(withExtension)) { + if (await pathExistsAsFile(withExtension)) { return withExtension } } } - if (await fileExistsAsFile(fullPath)) { + if (await pathExistsAsFile(fullPath)) { return fullPath } } const fullPath = path.join(cwd, command) - return (await fileExistsAsFile(fullPath)) ? fullPath : undefined + return (await pathExistsAsFile(fullPath)) ? fullPath : undefined } /** From ff77e246f3abdc9ec408285d8260995ae4bc271c Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Fri, 19 Dec 2025 18:32:03 +0100 Subject: [PATCH 03/10] chore: restore slackbot.md to match main --- apps/kilocode-docs/docs/advanced-usage/slackbot.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/apps/kilocode-docs/docs/advanced-usage/slackbot.md b/apps/kilocode-docs/docs/advanced-usage/slackbot.md index 7046e60b558..3c9bf22f1dc 100644 --- a/apps/kilocode-docs/docs/advanced-usage/slackbot.md +++ b/apps/kilocode-docs/docs/advanced-usage/slackbot.md @@ -23,7 +23,7 @@ The Kilo Slackbot brings the power of Kilo Code directly into your Slack workspa Before using the Kilo Slackbot: - You must have a **Kilo Code account** with available credits -- Your **GitHub Integration must be configured** via the [Integrations tab](https://app.kilo.ai/integrations) so the Slackbot can access your repositories +- Your **GitHub Integration must be configured** via the [Integrations tab](https://app.kilo.ai/integrations) so the Slackbot can access your repositories To install the Kilo Slackbot, simply go to the integrations menu in the sidebar on https://app.kilo.ai and set up the Slack integration. @@ -40,7 +40,6 @@ You can message the Kilo Slackbot directly through Slack DMs for private convers 3. Ask your question or describe what you need This is ideal for: - - Private questions about your code - Sensitive debugging sessions - Personal productivity tasks @@ -54,7 +53,6 @@ Mention the bot in any channel where it's been added: ``` This is great for: - - Team discussions where AI assistance would help - Collaborative debugging sessions - Getting quick answers during code reviews @@ -84,7 +82,6 @@ When your team identifies an issue or improvement in a Slack thread, ask the bot ``` The bot can: - - Read the context from the thread - Understand the proposed solution - Create a branch with the implementation From ddc6fc6ed4070b4ad6d88f3f6ea3d34c1a4e9cf0 Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Fri, 19 Dec 2025 18:35:35 +0100 Subject: [PATCH 04/10] fix: add missing getPlatformDiagnostics mock in AgentManagerProvider tests --- .../agent-manager/__tests__/AgentManagerProvider.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts b/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts index 5b635fd7b0e..0235f67ab48 100644 --- a/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts +++ b/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts @@ -6,6 +6,7 @@ const MOCK_CLI_PATH = "/mock/path/to/kilocode" // Mock the local telemetry module vi.mock("../telemetry", () => ({ + getPlatformDiagnostics: vi.fn(() => ({ platform: "darwin", shell: "bash" })), captureAgentManagerOpened: vi.fn(), captureAgentManagerSessionStarted: vi.fn(), captureAgentManagerSessionCompleted: vi.fn(), From d0160a57314b19b197f358d754a04eb87e653d6e Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Fri, 19 Dec 2025 18:54:58 +0100 Subject: [PATCH 05/10] fix: use platform-appropriate paths in Windows tests --- .../__tests__/AgentManagerProvider.spec.ts | 14 ++++++--- .../__tests__/CliPathResolver.spec.ts | 30 +++++++++++-------- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts b/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts index 0235f67ab48..33c422ab821 100644 --- a/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts +++ b/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts @@ -1,8 +1,10 @@ import { describe, it, expect, vi, beforeEach, afterEach, type Mock } from "vitest" import { EventEmitter } from "node:events" +import * as path from "node:path" import * as telemetry from "../telemetry" -const MOCK_CLI_PATH = "/mock/path/to/kilocode" +const isWindows = process.platform === "win32" +const MOCK_CLI_PATH = isWindows ? "C:\\mock\\path\\to\\kilocode" : "/mock/path/to/kilocode" // Mock the local telemetry module vi.mock("../telemetry", () => ({ @@ -94,7 +96,12 @@ describe("AgentManagerProvider CLI spawning", () => { // Reset modules to set up Windows-specific mock vi.resetModules() - const mockWorkspaceFolder = { uri: { fsPath: "/tmp/workspace" } } + // Use platform-appropriate paths for the test + const testNpmDir = isWindows ? "C:\\npm" : "/npm" + const testWorkspace = isWindows ? "C:\\tmp\\workspace" : "/tmp/workspace" + const cmdPath = path.join(testNpmDir, "kilocode") + ".CMD" + + const mockWorkspaceFolder = { uri: { fsPath: testWorkspace } } const mockProvider = { getState: vi.fn().mockResolvedValue({ apiConfiguration: { apiProvider: "kilocode" } }), } @@ -117,7 +124,6 @@ describe("AgentManagerProvider CLI spawning", () => { })) // Mock fs to make findExecutable find the .cmd file - const cmdPath = "/npm/kilocode.CMD" vi.doMock("node:fs", () => ({ existsSync: vi.fn().mockReturnValue(false), readdirSync: vi.fn().mockReturnValue([]), @@ -152,7 +158,7 @@ describe("AgentManagerProvider CLI spawning", () => { const originalPlatform = process.platform const originalPath = process.env.PATH Object.defineProperty(process, "platform", { value: "win32", writable: true }) - process.env.PATH = "/npm" + process.env.PATH = testNpmDir try { const module = await import("../AgentManagerProvider") diff --git a/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts b/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts index 16c34d53ef2..adca8cd120b 100644 --- a/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts +++ b/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts @@ -160,12 +160,16 @@ describe("findExecutable", () => { }) describe("Windows PATHEXT handling", () => { + // Use platform-appropriate test paths + const testDir = isWindows ? "C:\\npm" : "/npm" + const testCwd = isWindows ? "C:\\home\\test" : "/home/test" + it("tries PATHEXT extensions on Windows", async () => { const originalPlatform = process.platform Object.defineProperty(process, "platform", { value: "win32", configurable: true }) try { - const expectedPath = path.join("/npm", "kilocode") + ".CMD" + const expectedPath = path.join(testDir, "kilocode") + ".CMD" const statMock = vi.fn().mockImplementation((filePath: string) => { if (filePath === expectedPath) { return Promise.resolve({ isFile: () => true }) @@ -177,8 +181,8 @@ describe("findExecutable", () => { })) const { findExecutable } = await import("../CliPathResolver") - const result = await findExecutable("kilocode", "/home/test", ["/npm"], { - PATH: "/npm", + const result = await findExecutable("kilocode", testCwd, [testDir], { + PATH: testDir, PATHEXT: ".COM;.EXE;.BAT;.CMD", }) @@ -193,7 +197,7 @@ describe("findExecutable", () => { Object.defineProperty(process, "platform", { value: "win32", configurable: true }) try { - const expectedPath = path.join("/npm", "kilocode") + ".CMD" + const expectedPath = path.join(testDir, "kilocode") + ".CMD" const statMock = vi.fn().mockImplementation((filePath: string) => { if (filePath === expectedPath) { return Promise.resolve({ isFile: () => true }) @@ -205,8 +209,8 @@ describe("findExecutable", () => { })) const { findExecutable } = await import("../CliPathResolver") - const result = await findExecutable("kilocode", "/home/test", ["/npm"], { - PATH: "/npm", + const result = await findExecutable("kilocode", testCwd, [testDir], { + PATH: testDir, }) expect(result).toBe(expectedPath) @@ -220,7 +224,7 @@ describe("findExecutable", () => { Object.defineProperty(process, "platform", { value: "win32", configurable: true }) try { - const expectedPath = path.join("/npm", "kilocode") + ".EXE" + const expectedPath = path.join(testDir, "kilocode") + ".EXE" const statMock = vi.fn().mockImplementation((filePath: string) => { if (filePath === expectedPath) { return Promise.resolve({ isFile: () => true }) @@ -232,8 +236,8 @@ describe("findExecutable", () => { })) const { findExecutable } = await import("../CliPathResolver") - const result = await findExecutable("kilocode", "/home/test", undefined, { - Path: "/npm", + const result = await findExecutable("kilocode", testCwd, undefined, { + Path: testDir, PathExt: ".COM;.EXE;.BAT;.CMD", }) @@ -248,8 +252,8 @@ describe("findExecutable", () => { Object.defineProperty(process, "platform", { value: "win32", configurable: true }) try { - const comPath = path.join("/npm", "kilocode") + ".COM" - const exePath = path.join("/npm", "kilocode") + ".EXE" + const comPath = path.join(testDir, "kilocode") + ".COM" + const exePath = path.join(testDir, "kilocode") + ".EXE" const statMock = vi.fn().mockImplementation((filePath: string) => { if (filePath === comPath || filePath === exePath) { return Promise.resolve({ isFile: () => true }) @@ -261,8 +265,8 @@ describe("findExecutable", () => { })) const { findExecutable } = await import("../CliPathResolver") - const result = await findExecutable("kilocode", "/home/test", ["/npm"], { - PATH: "/npm", + const result = await findExecutable("kilocode", testCwd, [testDir], { + PATH: testDir, PATHEXT: ".COM;.EXE;.BAT;.CMD", }) From 268a5c868a8ea9ee0030e3ce1ca38e99c90c17bb Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Fri, 19 Dec 2025 19:12:50 +0100 Subject: [PATCH 06/10] fix: separate Windows simulation tests from native Windows tests - Skip platform-switching tests when already on target platform - Add dedicated native Windows tests that run only on Windows CI - Add proper lstat mock to fs mocks (code uses both stat and lstat) - Use proper error codes in mock rejections --- .../__tests__/AgentManagerProvider.spec.ts | 118 +++++++++++++++--- .../__tests__/CliPathResolver.spec.ts | 112 ++++++++++------- 2 files changed, 170 insertions(+), 60 deletions(-) diff --git a/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts b/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts index 33c422ab821..8cd92b65641 100644 --- a/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts +++ b/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts @@ -92,13 +92,36 @@ describe("AgentManagerProvider CLI spawning", () => { expect(options?.shell).not.toBe(true) }) - it("spawns with shell: true on Windows when CLI path ends with .cmd", async () => { + // Helper to create fs mock for Windows tests + const createWindowsFsMock = (cmdPath: string) => ({ + existsSync: vi.fn().mockReturnValue(false), + readdirSync: vi.fn().mockReturnValue([]), + promises: { + stat: vi.fn().mockImplementation((filePath: string) => { + if (filePath === cmdPath) { + return Promise.resolve({ isFile: () => true }) + } + return Promise.reject(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + }), + lstat: vi.fn().mockImplementation((filePath: string) => { + if (filePath === cmdPath) { + return Promise.resolve({ isFile: () => true, isSymbolicLink: () => false }) + } + return Promise.reject(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + }), + }, + }) + + // Skip Windows simulation on actual Windows - run native test instead + const windowsSimulationTest = isWindows ? it.skip : it + + windowsSimulationTest("spawns with shell: true on Windows when CLI path ends with .cmd (simulated)", async () => { // Reset modules to set up Windows-specific mock vi.resetModules() - // Use platform-appropriate paths for the test - const testNpmDir = isWindows ? "C:\\npm" : "/npm" - const testWorkspace = isWindows ? "C:\\tmp\\workspace" : "/tmp/workspace" + // Use Unix paths for simulation test (run on non-Windows) + const testNpmDir = "/npm" + const testWorkspace = "/tmp/workspace" const cmdPath = path.join(testNpmDir, "kilocode") + ".CMD" const mockWorkspaceFolder = { uri: { fsPath: testWorkspace } } @@ -123,19 +146,7 @@ describe("AgentManagerProvider CLI spawning", () => { getRemoteUrl: vi.fn().mockResolvedValue(undefined), })) - // Mock fs to make findExecutable find the .cmd file - vi.doMock("node:fs", () => ({ - existsSync: vi.fn().mockReturnValue(false), - readdirSync: vi.fn().mockReturnValue([]), - promises: { - stat: vi.fn().mockImplementation((filePath: string) => { - if (filePath === cmdPath) { - return Promise.resolve({ isFile: () => true }) - } - return Promise.reject(new Error("ENOENT")) - }), - }, - })) + vi.doMock("node:fs", () => createWindowsFsMock(cmdPath)) class TestProc extends EventEmitter { stdout = new EventEmitter() @@ -179,6 +190,79 @@ describe("AgentManagerProvider CLI spawning", () => { } }) + // Native Windows test - runs only on Windows + const nativeWindowsTest = isWindows ? it : it.skip + + nativeWindowsTest("spawns with shell: true on native Windows when CLI path ends with .cmd", async () => { + // Reset modules to set up Windows-specific mock + vi.resetModules() + + // Use Windows paths for native test + const testNpmDir = "C:\\npm" + const testWorkspace = "C:\\tmp\\workspace" + const cmdPath = path.join(testNpmDir, "kilocode") + ".CMD" + + const mockWorkspaceFolder = { uri: { fsPath: testWorkspace } } + const mockProvider = { + getState: vi.fn().mockResolvedValue({ apiConfiguration: { apiProvider: "kilocode" } }), + } + + vi.doMock("vscode", () => ({ + workspace: { workspaceFolders: [mockWorkspaceFolder] }, + window: { showErrorMessage: vi.fn().mockResolvedValue(undefined), showWarningMessage: vi.fn().mockResolvedValue(undefined), ViewColumn: { One: 1 } }, + env: { openExternal: vi.fn() }, + Uri: { parse: vi.fn(), joinPath: vi.fn() }, + ViewColumn: { One: 1 }, + ExtensionMode: { Development: 1, Production: 2, Test: 3 }, + })) + + vi.doMock("../../../../utils/fs", () => ({ + fileExistsAtPath: vi.fn().mockResolvedValue(false), + })) + + vi.doMock("../../../../services/code-index/managed/git-utils", () => ({ + getRemoteUrl: vi.fn().mockResolvedValue(undefined), + })) + + vi.doMock("node:fs", () => createWindowsFsMock(cmdPath)) + + class TestProc extends EventEmitter { + stdout = new EventEmitter() + stderr = new EventEmitter() + kill = vi.fn() + pid = 1234 + } + + const spawnMock = vi.fn(() => new TestProc()) + const execSyncMock = vi.fn().mockImplementation(() => { + throw new Error("not found") + }) + + vi.doMock("node:child_process", () => ({ + spawn: spawnMock, + execSync: execSyncMock, + })) + + const originalPath = process.env.PATH + process.env.PATH = testNpmDir + + try { + const module = await import("../AgentManagerProvider") + const windowsProvider = new module.AgentManagerProvider(mockContext, mockOutputChannel, mockProvider as any) + + await (windowsProvider as any).startAgentSession("test windows cmd") + + expect(spawnMock).toHaveBeenCalledTimes(1) + const [cmd, , options] = spawnMock.mock.calls[0] as unknown as [string, string[], Record] + expect(cmd.toLowerCase()).toContain(".cmd") + expect(options?.shell).toBe(true) + + windowsProvider.dispose() + } finally { + process.env.PATH = originalPath + } + }) + it("creates pending session and waits for session_created event", async () => { await (provider as any).startAgentSession("test pending") diff --git a/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts b/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts index adca8cd120b..59c056d40a4 100644 --- a/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts +++ b/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts @@ -164,21 +164,35 @@ describe("findExecutable", () => { const testDir = isWindows ? "C:\\npm" : "/npm" const testCwd = isWindows ? "C:\\home\\test" : "/home/test" - it("tries PATHEXT extensions on Windows", async () => { + // Helper to create fs mock with both stat and lstat + const createFsMock = (matchPaths: string[]) => ({ + existsSync: vi.fn().mockReturnValue(false), + promises: { + stat: vi.fn().mockImplementation((filePath: string) => { + if (matchPaths.some((p) => filePath === p)) { + return Promise.resolve({ isFile: () => true }) + } + return Promise.reject(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + }), + lstat: vi.fn().mockImplementation((filePath: string) => { + if (matchPaths.some((p) => filePath === p)) { + return Promise.resolve({ isFile: () => true, isSymbolicLink: () => false }) + } + return Promise.reject(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + }), + }, + }) + + // Skip platform-switching tests when already on target platform + const windowsSimulationTest = isWindows ? it.skip : it + + windowsSimulationTest("tries PATHEXT extensions on Windows", async () => { const originalPlatform = process.platform Object.defineProperty(process, "platform", { value: "win32", configurable: true }) try { const expectedPath = path.join(testDir, "kilocode") + ".CMD" - const statMock = vi.fn().mockImplementation((filePath: string) => { - if (filePath === expectedPath) { - return Promise.resolve({ isFile: () => true }) - } - return Promise.reject(new Error("ENOENT")) - }) - vi.doMock("node:fs", () => ({ - promises: { stat: statMock }, - })) + vi.doMock("node:fs", () => createFsMock([expectedPath])) const { findExecutable } = await import("../CliPathResolver") const result = await findExecutable("kilocode", testCwd, [testDir], { @@ -192,21 +206,13 @@ describe("findExecutable", () => { } }) - it("uses default PATHEXT if not in env", async () => { + windowsSimulationTest("uses default PATHEXT if not in env", async () => { const originalPlatform = process.platform Object.defineProperty(process, "platform", { value: "win32", configurable: true }) try { const expectedPath = path.join(testDir, "kilocode") + ".CMD" - const statMock = vi.fn().mockImplementation((filePath: string) => { - if (filePath === expectedPath) { - return Promise.resolve({ isFile: () => true }) - } - return Promise.reject(new Error("ENOENT")) - }) - vi.doMock("node:fs", () => ({ - promises: { stat: statMock }, - })) + vi.doMock("node:fs", () => createFsMock([expectedPath])) const { findExecutable } = await import("../CliPathResolver") const result = await findExecutable("kilocode", testCwd, [testDir], { @@ -219,21 +225,13 @@ describe("findExecutable", () => { } }) - it("handles case-insensitive PATH lookup", async () => { + windowsSimulationTest("handles case-insensitive PATH lookup", async () => { const originalPlatform = process.platform Object.defineProperty(process, "platform", { value: "win32", configurable: true }) try { const expectedPath = path.join(testDir, "kilocode") + ".EXE" - const statMock = vi.fn().mockImplementation((filePath: string) => { - if (filePath === expectedPath) { - return Promise.resolve({ isFile: () => true }) - } - return Promise.reject(new Error("ENOENT")) - }) - vi.doMock("node:fs", () => ({ - promises: { stat: statMock }, - })) + vi.doMock("node:fs", () => createFsMock([expectedPath])) const { findExecutable } = await import("../CliPathResolver") const result = await findExecutable("kilocode", testCwd, undefined, { @@ -247,22 +245,14 @@ describe("findExecutable", () => { } }) - it("returns first matching PATHEXT extension", async () => { + windowsSimulationTest("returns first matching PATHEXT extension", async () => { const originalPlatform = process.platform Object.defineProperty(process, "platform", { value: "win32", configurable: true }) try { const comPath = path.join(testDir, "kilocode") + ".COM" const exePath = path.join(testDir, "kilocode") + ".EXE" - const statMock = vi.fn().mockImplementation((filePath: string) => { - if (filePath === comPath || filePath === exePath) { - return Promise.resolve({ isFile: () => true }) - } - return Promise.reject(new Error("ENOENT")) - }) - vi.doMock("node:fs", () => ({ - promises: { stat: statMock }, - })) + vi.doMock("node:fs", () => createFsMock([comPath, exePath])) const { findExecutable } = await import("../CliPathResolver") const result = await findExecutable("kilocode", testCwd, [testDir], { @@ -275,9 +265,41 @@ describe("findExecutable", () => { Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true }) } }) + + // Native Windows tests - these run only on Windows + const nativeWindowsTest = isWindows ? it : it.skip + + nativeWindowsTest("finds .CMD file on native Windows", async () => { + const expectedPath = path.join(testDir, "kilocode") + ".CMD" + vi.doMock("node:fs", () => createFsMock([expectedPath])) + + const { findExecutable } = await import("../CliPathResolver") + const result = await findExecutable("kilocode", testCwd, [testDir], { + PATH: testDir, + PATHEXT: ".COM;.EXE;.BAT;.CMD", + }) + + expect(result).toBe(expectedPath) + }) + + nativeWindowsTest("uses case-insensitive env lookup on native Windows", async () => { + const expectedPath = path.join(testDir, "kilocode") + ".EXE" + vi.doMock("node:fs", () => createFsMock([expectedPath])) + + const { findExecutable } = await import("../CliPathResolver") + const result = await findExecutable("kilocode", testCwd, undefined, { + Path: testDir, // lowercase 'ath' + PathExt: ".COM;.EXE;.BAT;.CMD", // lowercase 'athExt' + }) + + expect(result).toBe(expectedPath) + }) }) - it("does not use PATHEXT on non-Windows platforms", async () => { + // Skip darwin simulation on Windows - can't simulate non-Windows on Windows + const darwinSimulationTest = isWindows ? it.skip : it + + darwinSimulationTest("does not use PATHEXT on non-Windows platforms", async () => { const originalPlatform = process.platform Object.defineProperty(process, "platform", { value: "darwin", configurable: true }) @@ -286,10 +308,14 @@ describe("findExecutable", () => { if (filePath === "/usr/bin/kilocode") { return Promise.resolve({ isFile: () => true }) } - return Promise.reject(new Error("ENOENT")) + return Promise.reject(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) }) vi.doMock("node:fs", () => ({ - promises: { stat: statMock }, + existsSync: vi.fn().mockReturnValue(false), + promises: { + stat: statMock, + lstat: vi.fn().mockRejectedValue(Object.assign(new Error("ENOENT"), { code: "ENOENT" })), + }, })) const { findExecutable } = await import("../CliPathResolver") From 285c427d0e2d72c90ec3c0c9eda6a3b44eedf51b Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Fri, 19 Dec 2025 19:17:04 +0100 Subject: [PATCH 07/10] fix: remove platform simulation tests, keep only native platform tests Platform simulation (mocking process.platform) is fragile and doesn't truly test platform-specific behavior. Instead: - Windows tests run only on Windows CI (skipped elsewhere) - Non-Windows tests run only on non-Windows (skipped on Windows) - Let actual CI environments test their native platform behavior --- .../__tests__/AgentManagerProvider.spec.ts | 134 +++----------- .../__tests__/CliPathResolver.spec.ts | 170 ++++++------------ 2 files changed, 77 insertions(+), 227 deletions(-) diff --git a/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts b/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts index 8cd92b65641..7f7cf9facac 100644 --- a/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts +++ b/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts @@ -92,112 +92,13 @@ describe("AgentManagerProvider CLI spawning", () => { expect(options?.shell).not.toBe(true) }) - // Helper to create fs mock for Windows tests - const createWindowsFsMock = (cmdPath: string) => ({ - existsSync: vi.fn().mockReturnValue(false), - readdirSync: vi.fn().mockReturnValue([]), - promises: { - stat: vi.fn().mockImplementation((filePath: string) => { - if (filePath === cmdPath) { - return Promise.resolve({ isFile: () => true }) - } - return Promise.reject(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) - }), - lstat: vi.fn().mockImplementation((filePath: string) => { - if (filePath === cmdPath) { - return Promise.resolve({ isFile: () => true, isSymbolicLink: () => false }) - } - return Promise.reject(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) - }), - }, - }) - - // Skip Windows simulation on actual Windows - run native test instead - const windowsSimulationTest = isWindows ? it.skip : it - - windowsSimulationTest("spawns with shell: true on Windows when CLI path ends with .cmd (simulated)", async () => { - // Reset modules to set up Windows-specific mock - vi.resetModules() - - // Use Unix paths for simulation test (run on non-Windows) - const testNpmDir = "/npm" - const testWorkspace = "/tmp/workspace" - const cmdPath = path.join(testNpmDir, "kilocode") + ".CMD" - - const mockWorkspaceFolder = { uri: { fsPath: testWorkspace } } - const mockProvider = { - getState: vi.fn().mockResolvedValue({ apiConfiguration: { apiProvider: "kilocode" } }), - } - - vi.doMock("vscode", () => ({ - workspace: { workspaceFolders: [mockWorkspaceFolder] }, - window: { showErrorMessage: vi.fn(), showWarningMessage: vi.fn(), ViewColumn: { One: 1 } }, - env: { openExternal: vi.fn() }, - Uri: { parse: vi.fn(), joinPath: vi.fn() }, - ViewColumn: { One: 1 }, - ExtensionMode: { Development: 1, Production: 2, Test: 3 }, - })) - - vi.doMock("../../../../utils/fs", () => ({ - fileExistsAtPath: vi.fn().mockResolvedValue(false), - })) - - vi.doMock("../../../../services/code-index/managed/git-utils", () => ({ - getRemoteUrl: vi.fn().mockResolvedValue(undefined), - })) - - vi.doMock("node:fs", () => createWindowsFsMock(cmdPath)) - - class TestProc extends EventEmitter { - stdout = new EventEmitter() - stderr = new EventEmitter() - kill = vi.fn() - pid = 1234 - } + // Windows-specific test - runs only on Windows CI + // We don't simulate Windows on other platforms - let the actual Windows CI test it + const windowsOnlyTest = isWindows ? it : it.skip - const spawnMock = vi.fn(() => new TestProc()) - const execSyncMock = vi.fn().mockImplementation(() => { - throw new Error("not found") - }) - - vi.doMock("node:child_process", () => ({ - spawn: spawnMock, - execSync: execSyncMock, - })) - - // Mock process.platform to be win32 and set PATH - const originalPlatform = process.platform - const originalPath = process.env.PATH - Object.defineProperty(process, "platform", { value: "win32", writable: true }) - process.env.PATH = testNpmDir - - try { - const module = await import("../AgentManagerProvider") - const windowsProvider = new module.AgentManagerProvider(mockContext, mockOutputChannel, mockProvider as any) - - await (windowsProvider as any).startAgentSession("test windows cmd") - - expect(spawnMock).toHaveBeenCalledTimes(1) - const [cmd, , options] = spawnMock.mock.calls[0] as unknown as [string, string[], Record] - expect(cmd.toLowerCase()).toContain(".cmd") - expect(options?.shell).toBe(true) - - windowsProvider.dispose() - } finally { - // Restore original platform and PATH - Object.defineProperty(process, "platform", { value: originalPlatform, writable: true }) - process.env.PATH = originalPath - } - }) - - // Native Windows test - runs only on Windows - const nativeWindowsTest = isWindows ? it : it.skip - - nativeWindowsTest("spawns with shell: true on native Windows when CLI path ends with .cmd", async () => { - // Reset modules to set up Windows-specific mock + windowsOnlyTest("spawns with shell: true when CLI path ends with .cmd", async () => { vi.resetModules() - // Use Windows paths for native test const testNpmDir = "C:\\npm" const testWorkspace = "C:\\tmp\\workspace" const cmdPath = path.join(testNpmDir, "kilocode") + ".CMD" @@ -224,7 +125,24 @@ describe("AgentManagerProvider CLI spawning", () => { getRemoteUrl: vi.fn().mockResolvedValue(undefined), })) - vi.doMock("node:fs", () => createWindowsFsMock(cmdPath)) + vi.doMock("node:fs", () => ({ + existsSync: vi.fn().mockReturnValue(false), + readdirSync: vi.fn().mockReturnValue([]), + promises: { + stat: vi.fn().mockImplementation((filePath: string) => { + if (filePath === cmdPath) { + return Promise.resolve({ isFile: () => true }) + } + return Promise.reject(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + }), + lstat: vi.fn().mockImplementation((filePath: string) => { + if (filePath === cmdPath) { + return Promise.resolve({ isFile: () => true, isSymbolicLink: () => false }) + } + return Promise.reject(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + }), + }, + })) class TestProc extends EventEmitter { stdout = new EventEmitter() @@ -234,13 +152,11 @@ describe("AgentManagerProvider CLI spawning", () => { } const spawnMock = vi.fn(() => new TestProc()) - const execSyncMock = vi.fn().mockImplementation(() => { - throw new Error("not found") - }) - vi.doMock("node:child_process", () => ({ spawn: spawnMock, - execSync: execSyncMock, + execSync: vi.fn().mockImplementation(() => { + throw new Error("not found") + }), })) const originalPath = process.env.PATH diff --git a/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts b/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts index 59c056d40a4..60daf8b3742 100644 --- a/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts +++ b/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts @@ -159,12 +159,13 @@ describe("findExecutable", () => { expect(result).toBe("/custom/bin/myapp") }) + // Windows PATHEXT tests - run only on Windows CI + // We don't simulate Windows on other platforms - let actual Windows CI test it describe("Windows PATHEXT handling", () => { - // Use platform-appropriate test paths - const testDir = isWindows ? "C:\\npm" : "/npm" - const testCwd = isWindows ? "C:\\home\\test" : "/home/test" + const windowsOnlyTest = isWindows ? it : it.skip + const testDir = "C:\\npm" + const testCwd = "C:\\home\\test" - // Helper to create fs mock with both stat and lstat const createFsMock = (matchPaths: string[]) => ({ existsSync: vi.fn().mockReturnValue(false), promises: { @@ -183,148 +184,81 @@ describe("findExecutable", () => { }, }) - // Skip platform-switching tests when already on target platform - const windowsSimulationTest = isWindows ? it.skip : it - - windowsSimulationTest("tries PATHEXT extensions on Windows", async () => { - const originalPlatform = process.platform - Object.defineProperty(process, "platform", { value: "win32", configurable: true }) - - try { - const expectedPath = path.join(testDir, "kilocode") + ".CMD" - vi.doMock("node:fs", () => createFsMock([expectedPath])) - - const { findExecutable } = await import("../CliPathResolver") - const result = await findExecutable("kilocode", testCwd, [testDir], { - PATH: testDir, - PATHEXT: ".COM;.EXE;.BAT;.CMD", - }) - - expect(result).toBe(expectedPath) - } finally { - Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true }) - } - }) - - windowsSimulationTest("uses default PATHEXT if not in env", async () => { - const originalPlatform = process.platform - Object.defineProperty(process, "platform", { value: "win32", configurable: true }) - - try { - const expectedPath = path.join(testDir, "kilocode") + ".CMD" - vi.doMock("node:fs", () => createFsMock([expectedPath])) - - const { findExecutable } = await import("../CliPathResolver") - const result = await findExecutable("kilocode", testCwd, [testDir], { - PATH: testDir, - }) - - expect(result).toBe(expectedPath) - } finally { - Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true }) - } - }) - - windowsSimulationTest("handles case-insensitive PATH lookup", async () => { - const originalPlatform = process.platform - Object.defineProperty(process, "platform", { value: "win32", configurable: true }) - - try { - const expectedPath = path.join(testDir, "kilocode") + ".EXE" - vi.doMock("node:fs", () => createFsMock([expectedPath])) - - const { findExecutable } = await import("../CliPathResolver") - const result = await findExecutable("kilocode", testCwd, undefined, { - Path: testDir, - PathExt: ".COM;.EXE;.BAT;.CMD", - }) - - expect(result).toBe(expectedPath) - } finally { - Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true }) - } - }) - - windowsSimulationTest("returns first matching PATHEXT extension", async () => { - const originalPlatform = process.platform - Object.defineProperty(process, "platform", { value: "win32", configurable: true }) - - try { - const comPath = path.join(testDir, "kilocode") + ".COM" - const exePath = path.join(testDir, "kilocode") + ".EXE" - vi.doMock("node:fs", () => createFsMock([comPath, exePath])) + windowsOnlyTest("finds .CMD file via PATHEXT", async () => { + const expectedPath = path.join(testDir, "kilocode") + ".CMD" + vi.doMock("node:fs", () => createFsMock([expectedPath])) - const { findExecutable } = await import("../CliPathResolver") - const result = await findExecutable("kilocode", testCwd, [testDir], { - PATH: testDir, - PATHEXT: ".COM;.EXE;.BAT;.CMD", - }) + const { findExecutable } = await import("../CliPathResolver") + const result = await findExecutable("kilocode", testCwd, [testDir], { + PATH: testDir, + PATHEXT: ".COM;.EXE;.BAT;.CMD", + }) - expect(result).toBe(comPath) - } finally { - Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true }) - } + expect(result).toBe(expectedPath) }) - // Native Windows tests - these run only on Windows - const nativeWindowsTest = isWindows ? it : it.skip - - nativeWindowsTest("finds .CMD file on native Windows", async () => { + windowsOnlyTest("uses default PATHEXT when not in env", async () => { const expectedPath = path.join(testDir, "kilocode") + ".CMD" vi.doMock("node:fs", () => createFsMock([expectedPath])) const { findExecutable } = await import("../CliPathResolver") const result = await findExecutable("kilocode", testCwd, [testDir], { PATH: testDir, - PATHEXT: ".COM;.EXE;.BAT;.CMD", }) expect(result).toBe(expectedPath) }) - nativeWindowsTest("uses case-insensitive env lookup on native Windows", async () => { + windowsOnlyTest("handles case-insensitive env var lookup", async () => { const expectedPath = path.join(testDir, "kilocode") + ".EXE" vi.doMock("node:fs", () => createFsMock([expectedPath])) const { findExecutable } = await import("../CliPathResolver") const result = await findExecutable("kilocode", testCwd, undefined, { - Path: testDir, // lowercase 'ath' - PathExt: ".COM;.EXE;.BAT;.CMD", // lowercase 'athExt' + Path: testDir, // lowercase 'ath' - Windows env vars are case-insensitive + PathExt: ".COM;.EXE;.BAT;.CMD", }) expect(result).toBe(expectedPath) }) - }) - - // Skip darwin simulation on Windows - can't simulate non-Windows on Windows - const darwinSimulationTest = isWindows ? it.skip : it - darwinSimulationTest("does not use PATHEXT on non-Windows platforms", async () => { - const originalPlatform = process.platform - Object.defineProperty(process, "platform", { value: "darwin", configurable: true }) + windowsOnlyTest("returns first matching PATHEXT extension", async () => { + const comPath = path.join(testDir, "kilocode") + ".COM" + const exePath = path.join(testDir, "kilocode") + ".EXE" + vi.doMock("node:fs", () => createFsMock([comPath, exePath])) - try { - const statMock = vi.fn().mockImplementation((filePath: string) => { - if (filePath === "/usr/bin/kilocode") { - return Promise.resolve({ isFile: () => true }) - } - return Promise.reject(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + const { findExecutable } = await import("../CliPathResolver") + const result = await findExecutable("kilocode", testCwd, [testDir], { + PATH: testDir, + PATHEXT: ".COM;.EXE;.BAT;.CMD", }) - vi.doMock("node:fs", () => ({ - existsSync: vi.fn().mockReturnValue(false), - promises: { - stat: statMock, - lstat: vi.fn().mockRejectedValue(Object.assign(new Error("ENOENT"), { code: "ENOENT" })), - }, - })) - const { findExecutable } = await import("../CliPathResolver") - const result = await findExecutable("kilocode", "/home/user", ["/usr/bin"]) + expect(result).toBe(comPath) + }) + }) + + // Non-Windows test - skipped on Windows since we can't simulate other platforms + const nonWindowsTest = isWindows ? it.skip : it - expect(result).toBe("/usr/bin/kilocode") - expect(statMock).not.toHaveBeenCalledWith(expect.stringContaining(".CMD")) - } finally { - Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true }) - } + nonWindowsTest("does not use PATHEXT on non-Windows platforms", async () => { + const statMock = vi.fn().mockImplementation((filePath: string) => { + if (filePath === "/usr/bin/kilocode") { + return Promise.resolve({ isFile: () => true }) + } + return Promise.reject(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) + }) + vi.doMock("node:fs", () => ({ + existsSync: vi.fn().mockReturnValue(false), + promises: { + stat: statMock, + lstat: vi.fn().mockRejectedValue(Object.assign(new Error("ENOENT"), { code: "ENOENT" })), + }, + })) + + const { findExecutable } = await import("../CliPathResolver") + const result = await findExecutable("kilocode", "/home/user", ["/usr/bin"]) + + expect(result).toBe("/usr/bin/kilocode") + expect(statMock).not.toHaveBeenCalledWith(expect.stringContaining(".CMD")) }) }) From 364feee325379f91962b9a3b0ef4141d3f64ccc2 Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Fri, 19 Dec 2025 20:04:05 +0100 Subject: [PATCH 08/10] fix: simplify tests by removing fragile Windows integration test The Windows .cmd shell:true behavior is already tested in CliProcessHandler. The PATHEXT resolution is tested in CliPathResolver.spec.ts. Production code works on Windows (confirmed), so remove complex integration test. --- .../__tests__/AgentManagerProvider.spec.ts | 91 +------------------ 1 file changed, 1 insertion(+), 90 deletions(-) diff --git a/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts b/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts index 7f7cf9facac..1b86d848e12 100644 --- a/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts +++ b/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts @@ -1,10 +1,8 @@ import { describe, it, expect, vi, beforeEach, afterEach, type Mock } from "vitest" import { EventEmitter } from "node:events" -import * as path from "node:path" import * as telemetry from "../telemetry" -const isWindows = process.platform === "win32" -const MOCK_CLI_PATH = isWindows ? "C:\\mock\\path\\to\\kilocode" : "/mock/path/to/kilocode" +const MOCK_CLI_PATH = "/mock/path/to/kilocode" // Mock the local telemetry module vi.mock("../telemetry", () => ({ @@ -92,93 +90,6 @@ describe("AgentManagerProvider CLI spawning", () => { expect(options?.shell).not.toBe(true) }) - // Windows-specific test - runs only on Windows CI - // We don't simulate Windows on other platforms - let the actual Windows CI test it - const windowsOnlyTest = isWindows ? it : it.skip - - windowsOnlyTest("spawns with shell: true when CLI path ends with .cmd", async () => { - vi.resetModules() - - const testNpmDir = "C:\\npm" - const testWorkspace = "C:\\tmp\\workspace" - const cmdPath = path.join(testNpmDir, "kilocode") + ".CMD" - - const mockWorkspaceFolder = { uri: { fsPath: testWorkspace } } - const mockProvider = { - getState: vi.fn().mockResolvedValue({ apiConfiguration: { apiProvider: "kilocode" } }), - } - - vi.doMock("vscode", () => ({ - workspace: { workspaceFolders: [mockWorkspaceFolder] }, - window: { showErrorMessage: vi.fn().mockResolvedValue(undefined), showWarningMessage: vi.fn().mockResolvedValue(undefined), ViewColumn: { One: 1 } }, - env: { openExternal: vi.fn() }, - Uri: { parse: vi.fn(), joinPath: vi.fn() }, - ViewColumn: { One: 1 }, - ExtensionMode: { Development: 1, Production: 2, Test: 3 }, - })) - - vi.doMock("../../../../utils/fs", () => ({ - fileExistsAtPath: vi.fn().mockResolvedValue(false), - })) - - vi.doMock("../../../../services/code-index/managed/git-utils", () => ({ - getRemoteUrl: vi.fn().mockResolvedValue(undefined), - })) - - vi.doMock("node:fs", () => ({ - existsSync: vi.fn().mockReturnValue(false), - readdirSync: vi.fn().mockReturnValue([]), - promises: { - stat: vi.fn().mockImplementation((filePath: string) => { - if (filePath === cmdPath) { - return Promise.resolve({ isFile: () => true }) - } - return Promise.reject(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) - }), - lstat: vi.fn().mockImplementation((filePath: string) => { - if (filePath === cmdPath) { - return Promise.resolve({ isFile: () => true, isSymbolicLink: () => false }) - } - return Promise.reject(Object.assign(new Error("ENOENT"), { code: "ENOENT" })) - }), - }, - })) - - class TestProc extends EventEmitter { - stdout = new EventEmitter() - stderr = new EventEmitter() - kill = vi.fn() - pid = 1234 - } - - const spawnMock = vi.fn(() => new TestProc()) - vi.doMock("node:child_process", () => ({ - spawn: spawnMock, - execSync: vi.fn().mockImplementation(() => { - throw new Error("not found") - }), - })) - - const originalPath = process.env.PATH - process.env.PATH = testNpmDir - - try { - const module = await import("../AgentManagerProvider") - const windowsProvider = new module.AgentManagerProvider(mockContext, mockOutputChannel, mockProvider as any) - - await (windowsProvider as any).startAgentSession("test windows cmd") - - expect(spawnMock).toHaveBeenCalledTimes(1) - const [cmd, , options] = spawnMock.mock.calls[0] as unknown as [string, string[], Record] - expect(cmd.toLowerCase()).toContain(".cmd") - expect(options?.shell).toBe(true) - - windowsProvider.dispose() - } finally { - process.env.PATH = originalPath - } - }) - it("creates pending session and waits for session_created event", async () => { await (provider as any).startAgentSession("test pending") From 98b573969c8d2fd38e2329579d62c50c6a1f0ac2 Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Fri, 19 Dec 2025 20:22:59 +0100 Subject: [PATCH 09/10] fix: skip Unix path tests on Windows Unix-style paths like /usr/bin/kilocode are not absolute on Windows (Windows requires drive letters like C:\). Skip these tests on Windows since the Windows-specific behavior is already tested by the PATHEXT tests. --- .../agent-manager/__tests__/CliPathResolver.spec.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts b/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts index 60daf8b3742..7d420fdf061 100644 --- a/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts +++ b/src/core/kilocode/agent-manager/__tests__/CliPathResolver.spec.ts @@ -118,7 +118,11 @@ describe("findExecutable", () => { vi.resetModules() }) - it("returns absolute path if file exists", async () => { + // These tests use Unix-style paths which are not absolute on Windows + // Skip on Windows - the Windows-specific behavior is tested below + const unixOnlyTest = isWindows ? it.skip : it + + unixOnlyTest("returns absolute path if file exists", async () => { const statMock = vi.fn().mockResolvedValue({ isFile: () => true }) vi.doMock("node:fs", () => ({ promises: { stat: statMock }, @@ -130,7 +134,7 @@ describe("findExecutable", () => { expect(result).toBe("/usr/bin/kilocode") }) - it("returns undefined for absolute path if file does not exist", async () => { + unixOnlyTest("returns undefined for absolute path if file does not exist", async () => { const statMock = vi.fn().mockRejectedValue(new Error("ENOENT")) vi.doMock("node:fs", () => ({ promises: { stat: statMock }, @@ -142,7 +146,7 @@ describe("findExecutable", () => { expect(result).toBeUndefined() }) - it("searches PATH entries for command", async () => { + unixOnlyTest("searches PATH entries for command", async () => { const statMock = vi.fn().mockImplementation((filePath: string) => { if (filePath === "/custom/bin/myapp") { return Promise.resolve({ isFile: () => true }) From 6130fad484866764ac276a2d57f56d2ff58a02b3 Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Fri, 19 Dec 2025 20:30:47 +0100 Subject: [PATCH 10/10] fix: mock CliInstaller in tests to work on Windows On Windows, login shell is skipped and findExecutable uses fs.promises.stat instead of execSync. The tests were relying on execSync returning MOCK_CLI_PATH, which doesn't work on Windows. Fix: Mock getLocalCliPath() to return MOCK_CLI_PATH and make fileExistsAtPath return true for that path. This ensures findKilocodeCli finds the CLI via the local path check on all platforms. --- .../__tests__/AgentManagerProvider.spec.ts | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts b/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts index 1b86d848e12..bae9b2dfcf1 100644 --- a/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts +++ b/src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts @@ -46,8 +46,15 @@ describe("AgentManagerProvider CLI spawning", () => { ExtensionMode: { Development: 1, Production: 2, Test: 3 }, })) + // Mock CliInstaller so getLocalCliPath returns our mock path + vi.doMock("../CliInstaller", () => ({ + getLocalCliPath: () => MOCK_CLI_PATH, + })) + + // Mock fileExistsAtPath to return true only for MOCK_CLI_PATH + // This ensures findKilocodeCli finds the CLI via local path check (works on all platforms) vi.doMock("../../../../utils/fs", () => ({ - fileExistsAtPath: vi.fn().mockResolvedValue(false), + fileExistsAtPath: vi.fn().mockImplementation((p: string) => Promise.resolve(p === MOCK_CLI_PATH)), })) // Mock getRemoteUrl for gitUrl support @@ -494,8 +501,13 @@ describe("AgentManagerProvider gitUrl filtering", () => { ExtensionMode: { Development: 1, Production: 2, Test: 3 }, })) + // Mock CliInstaller so getLocalCliPath returns our mock path + vi.doMock("../CliInstaller", () => ({ + getLocalCliPath: () => MOCK_CLI_PATH, + })) + vi.doMock("../../../../utils/fs", () => ({ - fileExistsAtPath: vi.fn().mockResolvedValue(false), + fileExistsAtPath: vi.fn().mockImplementation((p: string) => Promise.resolve(p === MOCK_CLI_PATH)), })) mockGetRemoteUrl = vi.fn().mockResolvedValue("https://github.com/org/repo.git") @@ -731,8 +743,13 @@ describe("AgentManagerProvider telemetry", () => { ExtensionMode: { Development: 1, Production: 2, Test: 3 }, })) + // Mock CliInstaller so getLocalCliPath returns our mock path + vi.doMock("../CliInstaller", () => ({ + getLocalCliPath: () => MOCK_CLI_PATH, + })) + vi.doMock("../../../../utils/fs", () => ({ - fileExistsAtPath: vi.fn().mockResolvedValue(false), + fileExistsAtPath: vi.fn().mockImplementation((p: string) => Promise.resolve(p === MOCK_CLI_PATH)), })) vi.doMock("../../../../services/code-index/managed/git-utils", () => ({