[pull] main from Kilo-Org:main

.changeset/agent-manager-telemetry-org-filter.md

@@ -0,0 +1,5 @@
+---
+"kilo-code": patch
+---
+
+Improve Agent Manager telemetry

.changeset/quick-otters-sleep.md

@@ -0,0 +1,5 @@
+---
+"kilo-code": patch
+---
+
+Fix Agent Manager CLI detection and Windows spawn by sanitizing shell output and running .cmd via cmd.exe.

packages/telemetry/src/BaseTelemetryClient.ts

@@ -28,8 +28,13 @@ export abstract class BaseTelemetryClient implements TelemetryClient {
 	/**
 	 * Determines if a specific property should be included in telemetry events
 	 * Override in subclasses to filter specific properties
+	 * @param _propertyName The name of the property to check
+	 * @param _allProperties All properties for context (e.g., to check organization membership) // kilocode_change
 	 */
-	protected isPropertyCapturable(_propertyName: string): boolean {
+	protected isPropertyCapturable(
+		_propertyName: string,
+		_allProperties: Record<string, unknown>, // kilocode_change
+	): boolean {
 		return true
 	}
 
@@ -63,7 +68,9 @@ export abstract class BaseTelemetryClient implements TelemetryClient {
 		// kilocode_change end
 
 		// Filter out properties that shouldn't be captured by this client
-		return Object.fromEntries(Object.entries(mergedProperties).filter(([key]) => this.isPropertyCapturable(key)))
+		return Object.fromEntries(
+			Object.entries(mergedProperties).filter(([key]) => this.isPropertyCapturable(key, mergedProperties)), // kilocode_change: pass mergedProperties for org filtering
+		)
 	}
 
 	public abstract capture(event: TelemetryEvent): Promise<void>

packages/telemetry/src/PostHogTelemetryClient.ts

@@ -13,8 +13,11 @@ import { BaseTelemetryClient } from "./BaseTelemetryClient"
 export class PostHogTelemetryClient extends BaseTelemetryClient {
 	private client: PostHog
 	private distinctId: string = vscode.env.machineId
-	// Git repository properties that should be filtered out
+	// Git repository properties that should be filtered out for all users
 	private readonly gitPropertyNames = ["repositoryUrl", "repositoryName", "defaultBranch"]
+	// kilocode_change start: filter sensitive error properties for organization users
+	private readonly orgFilteredProperties = ["errorMessage", "cliPath", "stderrPreview"]
+	// kilocode_change end
 
 	constructor(debug = false) {
 		super(
@@ -35,17 +38,25 @@ export class PostHogTelemetryClient extends BaseTelemetryClient {
 	}
 
 	/**
-	 * Filter out git repository properties for PostHog telemetry
+	 * Filter out properties based on privacy rules
+	 * - Git repository properties are filtered for all users
+	 * - Error details (paths, messages) are filtered for organization users
 	 * @param propertyName The property name to check
+	 * @param allProperties All properties for context (to check organization membership)
 	 * @returns Whether the property should be included in telemetry events
 	 */
-	protected override isPropertyCapturable(propertyName: string): boolean {
-		// Filter out git repository properties
+	// kilocode_change start: add allProperties parameter for org-based filtering
+	protected override isPropertyCapturable(propertyName: string, allProperties: Record<string, unknown>): boolean {
+		// Filter out git repository properties for all users
 		if (this.gitPropertyNames.includes(propertyName)) {
 			return false
 		}
+		if (allProperties.kilocodeOrganizationId && this.orgFilteredProperties.includes(propertyName)) {
+			return false
+		}
 		return true
 	}
+	// kilocode_change end
 
 	public override async capture(event: TelemetryEvent): Promise<void> {
 		if (!this.isTelemetryEnabled() || !this.isEventCapturable(event.event)) {

packages/telemetry/src/__tests__/PostHogTelemetryClient.test.ts

@@ -72,25 +72,63 @@ describe("PostHogTelemetryClient", () => {
 	})
 
 	describe("isPropertyCapturable", () => {
-		it("should filter out git repository properties", () => {
+		it("should filter out git repository properties for all users", () => {
 			const client = new PostHogTelemetryClient()
 
-			const isPropertyCapturable = getPrivateProperty<(propertyName: string) => boolean>(
-				client,
-				"isPropertyCapturable",
-			).bind(client)
+			const isPropertyCapturable = getPrivateProperty<
+				(propertyName: string, allProperties: Record<string, unknown>) => boolean
+			>(client, "isPropertyCapturable").bind(client)
+
+			const noOrgProperties = { appVersion: "1.0.0" }
 
 			// Git properties should be filtered out
-			expect(isPropertyCapturable("repositoryUrl")).toBe(false)
-			expect(isPropertyCapturable("repositoryName")).toBe(false)
-			expect(isPropertyCapturable("defaultBranch")).toBe(false)
+			expect(isPropertyCapturable("repositoryUrl", noOrgProperties)).toBe(false)
+			expect(isPropertyCapturable("repositoryName", noOrgProperties)).toBe(false)
+			expect(isPropertyCapturable("defaultBranch", noOrgProperties)).toBe(false)
+
+			// Other properties should be included
+			expect(isPropertyCapturable("appVersion", noOrgProperties)).toBe(true)
+			expect(isPropertyCapturable("vscodeVersion", noOrgProperties)).toBe(true)
+			expect(isPropertyCapturable("platform", noOrgProperties)).toBe(true)
+			expect(isPropertyCapturable("mode", noOrgProperties)).toBe(true)
+			expect(isPropertyCapturable("customProperty", noOrgProperties)).toBe(true)
+		})
+
+		it("should filter out error properties for organization users", () => {
+			const client = new PostHogTelemetryClient()
+
+			const isPropertyCapturable = getPrivateProperty<
+				(propertyName: string, allProperties: Record<string, unknown>) => boolean
+			>(client, "isPropertyCapturable").bind(client)
+
+			const orgProperties = { appVersion: "1.0.0", kilocodeOrganizationId: "org-123" }
+
+			// Error properties should be filtered out for org users
+			expect(isPropertyCapturable("errorMessage", orgProperties)).toBe(false)
+			expect(isPropertyCapturable("cliPath", orgProperties)).toBe(false)
+			expect(isPropertyCapturable("stderrPreview", orgProperties)).toBe(false)
+
+			// Git properties should still be filtered
+			expect(isPropertyCapturable("repositoryUrl", orgProperties)).toBe(false)
 
 			// Other properties should be included
-			expect(isPropertyCapturable("appVersion")).toBe(true)
-			expect(isPropertyCapturable("vscodeVersion")).toBe(true)
-			expect(isPropertyCapturable("platform")).toBe(true)
-			expect(isPropertyCapturable("mode")).toBe(true)
-			expect(isPropertyCapturable("customProperty")).toBe(true)
+			expect(isPropertyCapturable("appVersion", orgProperties)).toBe(true)
+			expect(isPropertyCapturable("platform", orgProperties)).toBe(true)
+		})
+
+		it("should allow error properties for non-organization users", () => {
+			const client = new PostHogTelemetryClient()
+
+			const isPropertyCapturable = getPrivateProperty<
+				(propertyName: string, allProperties: Record<string, unknown>) => boolean
+			>(client, "isPropertyCapturable").bind(client)
+
+			const noOrgProperties = { appVersion: "1.0.0" }
+
+			// Error properties should be included for non-org users
+			expect(isPropertyCapturable("errorMessage", noOrgProperties)).toBe(true)
+			expect(isPropertyCapturable("cliPath", noOrgProperties)).toBe(true)
+			expect(isPropertyCapturable("stderrPreview", noOrgProperties)).toBe(true)
 		})
 	})
 

src/core/kilocode/agent-manager/AgentManagerProvider.ts

@@ -1484,6 +1484,7 @@ export class AgentManagerProvider implements vscode.Disposable {
 				hasNpm,
 				platform,
 				shell,
+				errorMessage: error.message,
 			})
 		} else if (error?.type === "cli_configuration_error") {
 			captureAgentManagerLoginIssue({

src/core/kilocode/agent-manager/CliPathResolver.ts

@@ -3,6 +3,7 @@ import * as fs from "node:fs"
 import { execSync, spawnSync } from "node:child_process"
 import { fileExistsAtPath } from "../../../utils/fs"
 import { getLocalCliPath } from "./CliInstaller"
+import { stripShellControlCodes } from "./ShellOutput"
 
 /**
  * Result of CLI discovery including optional shell environment.
@@ -26,6 +27,28 @@ function getCaseInsensitive(target: NodeJS.ProcessEnv, key: string): string | un
 	return equivalentKey ? target[equivalentKey] : target[key]
 }
 
+function extractAbsolutePath(line: string): string | null {
+	const trimmed = line.trim()
+	if (!trimmed) {
+		return null
+	}
+	if (path.isAbsolute(trimmed)) {
+		return trimmed
+	}
+	const parts = trimmed.split(/\s+/)
+	const last = parts[parts.length - 1]
+	return path.isAbsolute(last) ? last : null
+}
+
+function isExecutablePath(candidate: string): boolean {
+	try {
+		const stat = fs.statSync(candidate)
+		return stat.isFile()
+	} catch {
+		return false
+	}
+}
+
 /**
  * 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.
@@ -276,17 +299,25 @@ function findViaLoginShell(log?: (msg: string) => void): string | null {
 
 	try {
 		log?.(`Trying login shell lookup: ${cmd}`)
-		const result = execSync(cmd, {
+		const rawOutput = execSync(cmd, {
 			encoding: "utf-8",
 			timeout: 10000,
 			env: { ...process.env, HOME: process.env.HOME },
 		})
-			.split(/\r?\n/)[0]
-			?.trim()
-
-		if (result && !result.includes("not found")) {
-			log?.(`Found CLI via login shell: ${result}`)
-			return result
+		const lines = stripShellControlCodes(rawOutput)
+			.split(/\r?\n/)
+			.map((line) => line.trim())
+			.filter(Boolean)
+
+		for (const line of lines) {
+			if (line.includes("not found")) {
+				continue
+			}
+			const candidate = extractAbsolutePath(line)
+			if (candidate && isExecutablePath(candidate)) {
+				log?.(`Found CLI via login shell: ${candidate}`)
+				return candidate
+			}
 		}
 	} catch (error) {
 		log?.(`Login shell lookup failed (this is normal if CLI not installed via version manager): ${error}`)
@@ -304,11 +335,10 @@ function getNpmPaths(log?: (msg: string) => void): string[] {
 	if (process.platform === "win32") {
 		const appData = process.env.APPDATA || ""
 		const localAppData = process.env.LOCALAPPDATA || ""
-		return [
-			appData ? path.join(appData, "npm", "kilocode.cmd") : "",
-			appData ? path.join(appData, "npm", "kilocode") : "",
-			localAppData ? path.join(localAppData, "npm", "kilocode.cmd") : "",
-		].filter(Boolean)
+		const basePaths = [appData, localAppData].filter(Boolean).map((base) => path.join(base, "npm", "kilocode"))
+		const pathExt = getCaseInsensitive(process.env, "PATHEXT") || ".COM;.EXE;.BAT;.CMD"
+		const extensions = pathExt.split(";").filter(Boolean)
+		return basePaths.flatMap((basePath) => extensions.map((ext) => `${basePath}${ext}`))
 	}
 
 	const paths = [

src/core/kilocode/agent-manager/CliProcessHandler.ts

@@ -186,21 +186,23 @@ export class CliProcessHandler {
 			sessionId: options?.sessionId,
 			existingBranch: options?.existingBranch,
 		})
-		this.debugLog(`Command: ${cliPath} ${cliArgs.join(" ")}`)
-		this.debugLog(`Working dir: ${workspace}`)
-
 		const env = this.buildEnvWithApiConfiguration(options?.apiConfiguration, 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
-		const needsShell = process.platform === "win32" && cliPath.toLowerCase().endsWith(".cmd")
+		// On Windows, batch files must be launched via cmd.exe to handle paths with spaces reliably.
+		const isWindowsBatch =
+			process.platform === "win32" && [".cmd", ".bat"].includes(path.extname(cliPath).toLowerCase())
+		const spawnCommand = isWindowsBatch ? process.env.ComSpec || "cmd.exe" : cliPath
+		const spawnArgs = isWindowsBatch ? ["/d", "/s", "/c", cliPath, ...cliArgs] : cliArgs
+
+		this.debugLog(`Command: ${spawnCommand} ${spawnArgs.join(" ")}`)
+		this.debugLog(`Working dir: ${workspace}`)
 
 		// Spawn CLI process
-		const proc = spawn(cliPath, cliArgs, {
+		const proc = spawn(spawnCommand, spawnArgs, {
 			cwd: workspace,
 			stdio: ["pipe", "pipe", "pipe"],
 			env,
-			shell: needsShell,
+			shell: false,
 		})
 
 		if (proc.pid) {

src/core/kilocode/agent-manager/ShellOutput.ts

@@ -0,0 +1,34 @@
+import stripAnsi from "strip-ansi"
+
+// this strips any form os OSC sequences from a value coming from a terminal command execution
+export function stripOscSequences(value: string): string {
+	let result = ""
+	let index = 0
+
+	while (index < value.length) {
+		if (value[index] === "\x1b" && value[index + 1] === "]") {
+			index += 2
+			while (index < value.length) {
+				const ch = value[index]
+				if (ch === "\x07") {
+					index += 1
+					break
+				}
+				if (ch === "\x1b" && value[index + 1] === "\\") {
+					index += 2
+					break
+				}
+				index += 1
+			}
+			continue
+		}
+		result += value[index]
+		index += 1
+	}
+
+	return result
+}
+
+export function stripShellControlCodes(value: string): string {
+	return stripAnsi(stripOscSequences(value))
+}

src/core/kilocode/agent-manager/__tests__/AgentManagerProvider.spec.ts

@@ -121,7 +121,7 @@ describe("AgentManagerProvider CLI spawning", () => {
 	// 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 () => {
+	windowsOnlyTest("spawns via cmd.exe when CLI path ends with .cmd", async () => {
 		vi.resetModules()
 
 		const testNpmDir = "C:\\npm"
@@ -207,9 +207,16 @@ describe("AgentManagerProvider CLI spawning", () => {
 			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<string, unknown>]
-			expect(cmd.toLowerCase()).toContain(".cmd")
-			expect(options?.shell).toBe(true)
+			const [cmd, args, options] = spawnMock.mock.calls[0] as unknown as [
+				string,
+				string[],
+				Record<string, unknown>,
+			]
+			const expectedCommand = process.env.ComSpec ?? "cmd.exe"
+			expect(cmd.toLowerCase()).toBe(expectedCommand.toLowerCase())
+			expect(args.slice(0, 3)).toEqual(["/d", "/s", "/c"])
+			expect(args).toEqual(expect.arrayContaining([cmdPath]))
+			expect(options?.shell).toBe(false)
 
 			windowsProvider.dispose()
 		} finally {