diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 5f6157998..ddf9cd872 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -1,7 +1,6 @@ # Changelog ## [Unreleased] - ### Added - Added automatic retry logic for OpenAI Codex responses with configurable delay and max retries @@ -16,6 +15,8 @@ ### Fixed +- Fixed orphaned tool call handling to ensure proper tool_use/tool_result pairing for all assistant messages +- Fixed message transformation to insert synthetic tool results for errored/aborted assistant messages with tool calls - Fixed tool prefix handling in Claude provider to use case-insensitive comparison - Fixed Gemini 3 model handling to treat unsigned tool calls as context-only with anti-mimicry context - Fixed message transformation to filter out empty error messages from conversation history diff --git a/packages/ai/src/providers/transform-messages.ts b/packages/ai/src/providers/transform-messages.ts index 2e71d272d..af4656b53 100644 --- a/packages/ai/src/providers/transform-messages.ts +++ b/packages/ai/src/providers/transform-messages.ts @@ -119,19 +119,30 @@ export function transformMessages(messages: Message[], model: existingToolResultIds = new Set(); } - // Track tool calls from this assistant message unless it errored const assistantMsg = msg as AssistantMessage; const isErroredAssistant = assistantMsg.stopReason === "error" || assistantMsg.stopReason === "aborted"; const toolCalls = assistantMsg.content.filter((b) => b.type === "toolCall") as ToolCall[]; - if (!isErroredAssistant && toolCalls.length > 0) { - pendingToolCalls = toolCalls; - existingToolResultIds = new Set(); - } else if (isErroredAssistant) { - pendingToolCalls = []; - existingToolResultIds = new Set(); - } result.push(msg); + + // For errored/aborted messages with tool calls, insert synthetic results immediately + // to maintain tool_use/tool_result pairing required by the API + if (isErroredAssistant && toolCalls.length > 0) { + for (const tc of toolCalls) { + result.push({ + role: "toolResult", + toolCallId: tc.id, + toolName: tc.name, + content: [{ type: "text", text: "Tool execution was aborted" }], + isError: true, + timestamp: Date.now(), + } as ToolResultMessage); + } + } else if (!isErroredAssistant && toolCalls.length > 0) { + // Track tool calls to check for orphaned calls later + pendingToolCalls = toolCalls; + existingToolResultIds = new Set(); + } } else if (msg.role === "toolResult") { existingToolResultIds.add(msg.toolCallId); result.push(msg); @@ -159,5 +170,22 @@ export function transformMessages(messages: Message[], model: } } + // Handle orphaned tool calls at the end of the message array + // This can happen if the last message is an assistant with tool calls that never got results + if (pendingToolCalls.length > 0) { + for (const tc of pendingToolCalls) { + if (!existingToolResultIds.has(tc.id)) { + result.push({ + role: "toolResult", + toolCallId: tc.id, + toolName: tc.name, + content: [{ type: "text", text: "No result provided" }], + isError: true, + timestamp: Date.now(), + } as ToolResultMessage); + } + } + } + return result; } diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 8c141a93c..38854b0d0 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,8 +1,14 @@ # Changelog ## [Unreleased] + ### Added +- Added Python shared gateway setting to enable resource-efficient kernel reuse across sessions +- Added Python tool cancellation support with proper timeout and cleanup handling +- Added enhanced Python prelude helpers including file operations, text processing, and Git utilities +- Added Python tool documentation rendering with categorized helper functions +- Added session-scoped Python kernel isolation with workdir-aware session IDs - Added structured status events for Python prelude functions with TUI rendering - Added status event display system with operation icons and formatted descriptions - Added support for rich output using IPython.display.display() in Python tool @@ -42,6 +48,12 @@ ### Changed +- Updated Python tool description to display categorized helper functions with improved formatting +- Enhanced Python kernel startup to use shared gateway by default for better resource utilization +- Improved Python prelude functions to emit structured status events instead of text output +- Updated agent prompts to use bash tool instead of exec for git operations +- Changed default Python tool mode from ipy-only to both to enable shell execution +- Enhanced Python gateway coordination with Windows environment support and stale process cleanup - Updated Python prelude functions to emit structured status events instead of text output - Enhanced Python tool renderer to display status events alongside output - Improved Python tool output formatting with status event integration @@ -67,18 +79,24 @@ ### Fixed -- Fixed Python prelude `sh()` function failing with exit code 2 due to double-shell invocation from `shell=True` -- Fixed signal event listener handling in worker processes to prevent crashes when signal is not an EventTarget -- Fixed Python tool proxy execution in worker processes to use proper tool creation pattern -- Fixed Python tool session requirement when using proxy executor in worker processes -- Fixed WebSocket message handling to support both binary and JSON message formats -- Fixed Python gateway process cleanup and reference counting for shared instances -- Fixed duplicate custom message rendering in event controller -- Fixed --no-extensions flag handling -- Fixed model selection to respect remembered model roles -- Fixed auto-print mode detection when piped input is present -- Fixed shutdown flow to wait for pending renders -- Fixed auto-compaction failure handling to surface warnings instead of crashing +- Fixed Python kernel cancellation handling and WebSocket cleanup for in-flight executions +- Fixed Python tool session scoping to include workdir and honor sharedGateway settings +- Fixed gist sharing output draining to avoid truncated URLs +- Fixed streaming output byte accounting and UTF-8 decoder flushing +- Fixed Python prelude integration tests to detect virtual environments and cover helper exports +- Fixed Python kernel cancellation/timeout handling and WebSocket close cleanup for in-flight executions +- Fixed Python output byte accounting and UTF-8 decoder flushing in streaming output +- Fixed shared Python gateway coordination (Windows env allowlist, lock staleness, refcount recovery) +- Fixed Python tool session scoping to include workdir and honor sharedGateway settings +- Fixed subagent Python proxy session isolation and cancellation/timeout propagation +- Fixed print-mode cleanup to dispose Python sessions before exit +- Fixed gist share output draining to avoid truncated URLs +- Fixed explore agent tool list to use bash for git operations +- Fixed Python prelude integration tests to detect venv-only Python and cover helper exports + +### Security + +- Enhanced Python gateway environment filtering to exclude sensitive API keys and Windows system paths ## [5.5.0] - 2026-01-18 ### Changed diff --git a/packages/coding-agent/src/core/python-executor.test.ts b/packages/coding-agent/src/core/python-executor.test.ts index 13425be9e..557b29f7e 100644 --- a/packages/coding-agent/src/core/python-executor.test.ts +++ b/packages/coding-agent/src/core/python-executor.test.ts @@ -5,6 +5,7 @@ import { executePythonWithKernel, getPreludeDocs, type PythonKernelExecutor, + resetPreludeDocsCache, warmPythonEnvironment, } from "./python-executor"; import { type KernelExecuteOptions, type KernelExecuteResult, type PreludeHelper, PythonKernel } from "./python-kernel"; @@ -138,6 +139,7 @@ describe("executePythonWithKernel", () => { afterEach(async () => { await disposeAllKernelSessions(); + resetPreludeDocsCache(); vi.restoreAllMocks(); }); diff --git a/packages/coding-agent/src/core/python-executor.ts b/packages/coding-agent/src/core/python-executor.ts index 866229b9e..83a637179 100644 --- a/packages/coding-agent/src/core/python-executor.ts +++ b/packages/coding-agent/src/core/python-executor.ts @@ -119,6 +119,10 @@ export function getPreludeDocs(): PreludeHelper[] { return cachedPreludeDocs ?? []; } +export function resetPreludeDocsCache(): void { + cachedPreludeDocs = null; +} + async function createKernelSession(sessionId: string, cwd: string, useSharedGateway?: boolean): Promise { const kernel = await PythonKernel.start({ cwd, useSharedGateway }); const session: KernelSession = { diff --git a/packages/coding-agent/src/core/python-gateway-coordinator.ts b/packages/coding-agent/src/core/python-gateway-coordinator.ts index 0066cabcc..4b26ad7fe 100644 --- a/packages/coding-agent/src/core/python-gateway-coordinator.ts +++ b/packages/coding-agent/src/core/python-gateway-coordinator.ts @@ -1,4 +1,16 @@ -import { closeSync, existsSync, mkdirSync, openSync, readFileSync, statSync, unlinkSync, writeFileSync } from "node:fs"; +import { + closeSync, + existsSync, + mkdirSync, + openSync, + readdirSync, + readFileSync, + renameSync, + statSync, + unlinkSync, + utimesSync, + writeFileSync, +} from "node:fs"; import { createServer } from "node:net"; import { delimiter, join } from "node:path"; import type { Subprocess } from "bun"; @@ -10,11 +22,13 @@ import { logger } from "./logger"; const GATEWAY_DIR_NAME = "python-gateway"; const GATEWAY_INFO_FILE = "gateway.json"; const GATEWAY_LOCK_FILE = "gateway.lock"; +const GATEWAY_CLIENT_PREFIX = "client-"; const GATEWAY_STARTUP_TIMEOUT_MS = 30000; const GATEWAY_IDLE_TIMEOUT_MS = 30000; -const GATEWAY_LOCK_TIMEOUT_MS = 5000; +const GATEWAY_LOCK_TIMEOUT_MS = GATEWAY_STARTUP_TIMEOUT_MS + 5000; const GATEWAY_LOCK_RETRY_MS = 50; -const GATEWAY_LOCK_STALE_MS = 30000; +const GATEWAY_LOCK_STALE_MS = GATEWAY_STARTUP_TIMEOUT_MS * 2; +const GATEWAY_LOCK_HEARTBEAT_MS = 5000; const HEALTH_CHECK_TIMEOUT_MS = 3000; const DEFAULT_ENV_ALLOWLIST = new Set([ @@ -43,6 +57,46 @@ const DEFAULT_ENV_ALLOWLIST = new Set([ "CONDA_DEFAULT_ENV", "VIRTUAL_ENV", "PYTHONPATH", + "SYSTEMROOT", + "COMSPEC", + "WINDIR", + "USERPROFILE", + "LOCALAPPDATA", + "APPDATA", + "PROGRAMDATA", + "PATHEXT", + "USERNAME", + "HOMEDRIVE", + "HOMEPATH", +]); + +const WINDOWS_ENV_ALLOWLIST = new Set([ + "APPDATA", + "COMPUTERNAME", + "COMSPEC", + "HOMEDRIVE", + "HOMEPATH", + "LOCALAPPDATA", + "NUMBER_OF_PROCESSORS", + "OS", + "PATH", + "PATHEXT", + "PROCESSOR_ARCHITECTURE", + "PROCESSOR_IDENTIFIER", + "PROGRAMDATA", + "PROGRAMFILES", + "PROGRAMFILES(X86)", + "PROGRAMW6432", + "SESSIONNAME", + "SYSTEMDRIVE", + "SYSTEMROOT", + "TEMP", + "TMP", + "USERDOMAIN", + "USERDOMAIN_ROAMINGPROFILE", + "USERPROFILE", + "USERNAME", + "WINDIR", ]); const DEFAULT_ENV_ALLOW_PREFIXES = ["LC_", "XDG_", "OMP_"]; @@ -59,6 +113,29 @@ const DEFAULT_ENV_DENYLIST = new Set([ "MISTRAL_API_KEY", ]); +const CASE_INSENSITIVE_ENV = process.platform === "win32"; +const ACTIVE_ENV_ALLOWLIST = CASE_INSENSITIVE_ENV ? WINDOWS_ENV_ALLOWLIST : DEFAULT_ENV_ALLOWLIST; + +const NORMALIZED_ALLOWLIST = new Set( + Array.from(ACTIVE_ENV_ALLOWLIST, (key) => (CASE_INSENSITIVE_ENV ? key.toUpperCase() : key)), +); +const NORMALIZED_DENYLIST = new Set( + Array.from(DEFAULT_ENV_DENYLIST, (key) => (CASE_INSENSITIVE_ENV ? key.toUpperCase() : key)), +); +const NORMALIZED_ALLOW_PREFIXES = CASE_INSENSITIVE_ENV + ? DEFAULT_ENV_ALLOW_PREFIXES.map((prefix) => prefix.toUpperCase()) + : DEFAULT_ENV_ALLOW_PREFIXES; + +function normalizeEnvKey(key: string): string { + return CASE_INSENSITIVE_ENV ? key.toUpperCase() : key; +} + +function resolvePathKey(env: Record): string { + if (!CASE_INSENSITIVE_ENV) return "PATH"; + const match = Object.keys(env).find((candidate) => candidate.toLowerCase() === "path"); + return match ?? "PATH"; +} + export interface GatewayInfo { url: string; pid: number; @@ -69,6 +146,11 @@ export interface GatewayInfo { venvPath?: string | null; } +interface GatewayLockInfo { + pid: number; + startedAt: number; +} + interface AcquireResult { url: string; isShared: boolean; @@ -78,17 +160,19 @@ let localGatewayProcess: Subprocess | null = null; let localGatewayUrl: string | null = null; let idleShutdownTimer: ReturnType | null = null; let isCoordinatorInitialized = false; +let localClientFile: string | null = null; function filterEnv(env: Record): Record { const filtered: Record = {}; for (const [key, value] of Object.entries(env)) { if (value === undefined) continue; - if (DEFAULT_ENV_DENYLIST.has(key)) continue; - if (DEFAULT_ENV_ALLOWLIST.has(key)) { + const normalizedKey = normalizeEnvKey(key); + if (NORMALIZED_DENYLIST.has(normalizedKey)) continue; + if (NORMALIZED_ALLOWLIST.has(normalizedKey)) { filtered[key] = value; continue; } - if (DEFAULT_ENV_ALLOW_PREFIXES.some((prefix) => key.startsWith(prefix))) { + if (NORMALIZED_ALLOW_PREFIXES.some((prefix) => normalizedKey.startsWith(prefix))) { filtered[key] = value; } } @@ -114,7 +198,9 @@ async function resolvePythonRuntime(cwd: string, baseEnv: Record; + if (typeof parsed.pid === "number" && Number.isFinite(parsed.pid)) { + return { pid: parsed.pid, startedAt: typeof parsed.startedAt === "number" ? parsed.startedAt : 0 }; + } + } catch { + // Ignore parse errors + } + return null; +} + function ensureGatewayDir(): void { const dir = getGatewayDir(); if (!existsSync(dir)) { @@ -176,9 +288,19 @@ async function withGatewayLock(handler: () => Promise): Promise { while (true) { try { const fd = openSync(lockPath, "wx"); + const heartbeat = setInterval(() => { + try { + const now = new Date(); + utimesSync(lockPath, now, now); + } catch { + // Ignore heartbeat errors + } + }, GATEWAY_LOCK_HEARTBEAT_MS); try { + writeLockInfo(lockPath, fd); return await handler(); } finally { + clearInterval(heartbeat); try { closeSync(fd); unlinkSync(lockPath); @@ -192,10 +314,16 @@ async function withGatewayLock(handler: () => Promise): Promise { let removedStale = false; try { const stat = statSync(lockPath); - if (Date.now() - stat.mtimeMs > GATEWAY_LOCK_STALE_MS) { + const lockInfo = readLockInfo(lockPath); + const lockPid = lockInfo?.pid; + const lockAgeMs = lockInfo?.startedAt ? Date.now() - lockInfo.startedAt : Date.now() - stat.mtimeMs; + const staleByTime = lockAgeMs > GATEWAY_LOCK_STALE_MS; + const staleByPid = lockPid !== undefined && !isPidRunning(lockPid); + const staleByMissingPid = lockPid === undefined && staleByTime; + if (staleByPid || staleByMissingPid) { unlinkSync(lockPath); removedStale = true; - logger.warn("Removed stale shared gateway lock", { path: lockPath }); + logger.warn("Removed stale shared gateway lock", { path: lockPath, pid: lockPid }); } } catch { // Ignore stat errors; keep waiting @@ -218,7 +346,24 @@ function readGatewayInfo(): GatewayInfo | null { if (!existsSync(infoPath)) return null; try { const content = readFileSync(infoPath, "utf-8"); - return JSON.parse(content) as GatewayInfo; + const parsed = JSON.parse(content) as Partial; + if (!parsed || typeof parsed !== "object") return null; + if (typeof parsed.url !== "string" || typeof parsed.pid !== "number" || typeof parsed.startedAt !== "number") { + return null; + } + if (typeof parsed.cwd !== "string") return null; + const clients = pruneStaleClientInfos(listClientInfos()); + const totalRefCount = clients.reduce((sum, client) => sum + client.info.refCount, 0); + const recoveredRefCount = clients.length > 0 ? totalRefCount : 0; + return { + url: parsed.url, + pid: parsed.pid, + startedAt: parsed.startedAt, + refCount: recoveredRefCount, + cwd: parsed.cwd, + pythonPath: typeof parsed.pythonPath === "string" ? parsed.pythonPath : undefined, + venvPath: typeof parsed.venvPath === "string" || parsed.venvPath === null ? parsed.venvPath : undefined, + }; } catch { return null; } @@ -226,7 +371,9 @@ function readGatewayInfo(): GatewayInfo | null { function writeGatewayInfo(info: GatewayInfo): void { const infoPath = getGatewayInfoPath(); - writeFileSync(infoPath, JSON.stringify(info, null, 2)); + const tempPath = `${infoPath}.tmp`; + writeFileSync(tempPath, JSON.stringify(info, null, 2)); + renameSync(tempPath, infoPath); } function clearGatewayInfo(): void { @@ -249,6 +396,103 @@ function isPidRunning(pid: number): boolean { } } +interface GatewayClientInfo { + pid: number; + refCount: number; + updatedAt?: number; +} + +function getClientFilePath(pid: number): string { + return join(getGatewayDir(), `${GATEWAY_CLIENT_PREFIX}${pid}.json`); +} + +function readClientInfo(path: string): GatewayClientInfo | null { + try { + const raw = readFileSync(path, "utf-8"); + const parsed = JSON.parse(raw) as GatewayClientInfo; + if (typeof parsed.pid !== "number" || typeof parsed.refCount !== "number") return null; + return parsed; + } catch { + return null; + } +} + +function listClientInfos(): Array<{ path: string; info: GatewayClientInfo }> { + const dir = getGatewayDir(); + if (!existsSync(dir)) return []; + const entries = readdirSync(dir); + const results: Array<{ path: string; info: GatewayClientInfo }> = []; + for (const entry of entries) { + if (!entry.startsWith(GATEWAY_CLIENT_PREFIX)) continue; + const path = join(dir, entry); + const info = readClientInfo(path); + if (!info) continue; + results.push({ path, info }); + } + return results; +} + +function pruneStaleClientInfos( + clients: Array<{ path: string; info: GatewayClientInfo }>, +): Array<{ path: string; info: GatewayClientInfo }> { + const active: Array<{ path: string; info: GatewayClientInfo }> = []; + for (const client of clients) { + if (!isPidRunning(client.info.pid)) { + try { + unlinkSync(client.path); + } catch { + // Ignore cleanup errors + } + continue; + } + active.push(client); + } + return active; +} + +function updateLocalClientRefCount(delta: number): { totalRefCount: number; localRefCount: number } { + ensureGatewayDir(); + const clients = pruneStaleClientInfos(listClientInfos()); + const localPath = localClientFile ?? getClientFilePath(process.pid); + const localEntry = clients.find((client) => client.info.pid === process.pid); + const baseCount = localEntry?.info.refCount ?? 0; + const nextCount = Math.max(0, baseCount + delta); + const otherClients = clients.filter((client) => client.info.pid !== process.pid); + + if (nextCount <= 0) { + if (localEntry) { + try { + unlinkSync(localEntry.path); + } catch { + // Ignore cleanup errors + } + } + if (localClientFile === localPath) { + localClientFile = null; + } + } else { + const payload: GatewayClientInfo = { pid: process.pid, refCount: nextCount, updatedAt: Date.now() }; + writeFileSync(localPath, JSON.stringify(payload, null, 2)); + localClientFile = localPath; + } + + const totalRefCount = + otherClients.reduce((sum, client) => sum + client.info.refCount, 0) + (nextCount > 0 ? nextCount : 0); + return { totalRefCount, localRefCount: nextCount }; +} + +function clearClientFiles(): void { + const clients = listClientInfos(); + for (const client of clients) { + try { + unlinkSync(client.path); + } catch { + // Ignore cleanup errors + } + } + localClientFile = null; +} + async function isGatewayHealthy(url: string): Promise { try { const controller = new AbortController(); @@ -355,22 +599,33 @@ function scheduleIdleShutdown(): void { try { await withGatewayLock(async () => { const info = readGatewayInfo(); - if (info && info.refCount === 0) { - logger.debug("Shutting down idle shared gateway", { pid: info.pid }); - if (localGatewayProcess) { - shutdownLocalGateway(); - } else if (isPidRunning(info.pid)) { - try { - killProcessTree(info.pid); - } catch (err) { - logger.warn("Failed to kill idle shared gateway", { - error: err instanceof Error ? err.message : String(err), - pid: info.pid, - }); - } - } - clearGatewayInfo(); + if (!info) { + clearClientFiles(); + return; } + const clients = pruneStaleClientInfos(listClientInfos()); + const totalRefCount = clients.reduce((sum, client) => sum + client.info.refCount, 0); + if (totalRefCount > 0) { + if (info.refCount !== totalRefCount) { + writeGatewayInfo({ ...info, refCount: totalRefCount }); + } + return; + } + logger.debug("Shutting down idle shared gateway", { pid: info.pid }); + if (localGatewayProcess) { + shutdownLocalGateway(); + } else if (isPidRunning(info.pid)) { + try { + killProcessTree(info.pid); + } catch (err) { + logger.warn("Failed to kill idle shared gateway", { + error: err instanceof Error ? err.message : String(err), + pid: info.pid, + }); + } + } + clearGatewayInfo(); + clearClientFiles(); }); } catch (err) { logger.warn("Failed to shutdown idle shared gateway", { @@ -433,7 +688,8 @@ export async function acquireSharedGateway(cwd: string): Promise { try { await withGatewayLock(async () => { + const { totalRefCount } = updateLocalClientRefCount(-1); const info = readGatewayInfo(); if (!info) return; - const newRefCount = Math.max(0, info.refCount - 1); + const newRefCount = Math.max(0, totalRefCount); if (newRefCount === 0) { const updatedInfo = { ...info, refCount: 0 }; writeGatewayInfo(updatedInfo); @@ -538,12 +797,15 @@ export function getGatewayStatus(): GatewayStatus { }; } const active = isPidRunning(info.pid); + const clients = pruneStaleClientInfos(listClientInfos()); + const clientRefCount = clients.reduce((sum, client) => sum + client.info.refCount, 0); + const refCount = clientRefCount > 0 ? clientRefCount : info.refCount; return { active, - shared: active && info.refCount > 1, + shared: active && refCount > 1, url: info.url, pid: info.pid, - refCount: info.refCount, + refCount, cwd: info.cwd, uptime: Date.now() - info.startedAt, }; @@ -551,10 +813,20 @@ export function getGatewayStatus(): GatewayStatus { export async function shutdownSharedGateway(): Promise { cancelIdleShutdown(); - const info = readGatewayInfo(); - if (info) { - clearGatewayInfo(); + try { + await withGatewayLock(async () => { + const info = readGatewayInfo(); + if (info) { + clearGatewayInfo(); + } + clearClientFiles(); + }); + } catch (err) { + logger.warn("Failed to shutdown shared gateway", { + error: err instanceof Error ? err.message : String(err), + }); + } finally { + shutdownLocalGateway(); + isCoordinatorInitialized = false; } - shutdownLocalGateway(); - isCoordinatorInitialized = false; } diff --git a/packages/coding-agent/src/core/python-kernel.ts b/packages/coding-agent/src/core/python-kernel.ts index f0e199686..b4b160f55 100644 --- a/packages/coding-agent/src/core/python-kernel.ts +++ b/packages/coding-agent/src/core/python-kernel.ts @@ -60,6 +60,56 @@ const DEFAULT_ENV_ALLOWLIST = new Set([ "CONDA_DEFAULT_ENV", "VIRTUAL_ENV", "PYTHONPATH", + "APPDATA", + "COMSPEC", + "COMPUTERNAME", + "HOMEDRIVE", + "HOMEPATH", + "LOCALAPPDATA", + "NUMBER_OF_PROCESSORS", + "OS", + "PATHEXT", + "PROCESSOR_ARCHITECTURE", + "PROCESSOR_IDENTIFIER", + "PROGRAMDATA", + "PROGRAMFILES", + "PROGRAMFILES(X86)", + "PROGRAMW6432", + "SYSTEMDRIVE", + "SYSTEMROOT", + "USERDOMAIN", + "USERPROFILE", + "USERNAME", + "WINDIR", +]); + +const WINDOWS_ENV_ALLOWLIST = new Set([ + "APPDATA", + "COMPUTERNAME", + "COMSPEC", + "HOMEDRIVE", + "HOMEPATH", + "LOCALAPPDATA", + "NUMBER_OF_PROCESSORS", + "OS", + "PATH", + "PATHEXT", + "PROCESSOR_ARCHITECTURE", + "PROCESSOR_IDENTIFIER", + "PROGRAMDATA", + "PROGRAMFILES", + "PROGRAMFILES(X86)", + "PROGRAMW6432", + "SESSIONNAME", + "SYSTEMDRIVE", + "SYSTEMROOT", + "TEMP", + "TMP", + "USERDOMAIN", + "USERDOMAIN_ROAMINGPROFILE", + "USERPROFILE", + "USERNAME", + "WINDIR", ]); const DEFAULT_ENV_ALLOW_PREFIXES = ["LC_", "XDG_", "OMP_"]; @@ -76,6 +126,30 @@ const DEFAULT_ENV_DENYLIST = new Set([ "MISTRAL_API_KEY", ]); +const CASE_INSENSITIVE_ENV = process.platform === "win32"; +const BASE_ENV_ALLOWLIST = CASE_INSENSITIVE_ENV + ? new Set([...DEFAULT_ENV_ALLOWLIST, ...WINDOWS_ENV_ALLOWLIST]) + : DEFAULT_ENV_ALLOWLIST; +const NORMALIZED_ALLOWLIST = new Set( + Array.from(BASE_ENV_ALLOWLIST, (key) => (CASE_INSENSITIVE_ENV ? key.toUpperCase() : key)), +); +const NORMALIZED_DENYLIST = new Set( + Array.from(DEFAULT_ENV_DENYLIST, (key) => (CASE_INSENSITIVE_ENV ? key.toUpperCase() : key)), +); +const NORMALIZED_ALLOW_PREFIXES = CASE_INSENSITIVE_ENV + ? DEFAULT_ENV_ALLOW_PREFIXES.map((prefix) => prefix.toUpperCase()) + : DEFAULT_ENV_ALLOW_PREFIXES; + +function normalizeEnvKey(key: string): string { + return CASE_INSENSITIVE_ENV ? key.toUpperCase() : key; +} + +function resolvePathKey(env: Record): string { + if (!CASE_INSENSITIVE_ENV) return "PATH"; + const match = Object.keys(env).find((candidate) => candidate.toLowerCase() === "path"); + return match ?? "PATH"; +} + export interface JupyterHeader { msg_id: string; session: string; @@ -149,12 +223,14 @@ function filterEnv(env: Record): Record = {}; for (const [key, value] of Object.entries(env)) { if (value === undefined) continue; - if (DEFAULT_ENV_DENYLIST.has(key)) continue; - if (DEFAULT_ENV_ALLOWLIST.has(key)) { - filtered[key] = value; + const normalizedKey = normalizeEnvKey(key); + if (NORMALIZED_DENYLIST.has(normalizedKey)) continue; + if (NORMALIZED_ALLOWLIST.has(normalizedKey)) { + const destKey = normalizedKey === "PATH" ? "PATH" : key; + filtered[destKey] = value; continue; } - if (DEFAULT_ENV_ALLOW_PREFIXES.some((prefix) => key.startsWith(prefix))) { + if (NORMALIZED_ALLOW_PREFIXES.some((prefix) => normalizedKey.startsWith(prefix))) { filtered[key] = value; } } @@ -180,7 +256,9 @@ async function resolvePythonRuntime(cwd: string, baseEnv: Record void>(); #channelHandlers = new Map void>>(); + #pendingExecutions = new Map void>(); private constructor( id: string, @@ -647,25 +726,47 @@ export class PythonKernel { return new Promise((resolve, reject) => { const ws = new WebSocket(wsUrl); ws.binaryType = "arraybuffer"; + let settled = false; const timeout = setTimeout(() => { ws.close(); - reject(new Error("WebSocket connection timeout")); + if (!settled) { + settled = true; + reject(new Error("WebSocket connection timeout")); + } }, 10000); ws.onopen = () => { + if (settled) return; + settled = true; clearTimeout(timeout); this.#ws = ws; resolve(); }; ws.onerror = (event) => { - clearTimeout(timeout); - reject(new Error(`WebSocket error: ${event}`)); + const error = new Error(`WebSocket error: ${event}`); + if (!settled) { + settled = true; + clearTimeout(timeout); + reject(error); + return; + } + this.#alive = false; + this.#ws = null; + this.abortPendingExecutions(error.message); }; ws.onclose = () => { this.#alive = false; + this.#ws = null; + if (!settled) { + settled = true; + clearTimeout(timeout); + reject(new Error("WebSocket closed before connection")); + return; + } + this.abortPendingExecutions("WebSocket closed"); }; ws.onmessage = (event) => { @@ -701,6 +802,16 @@ export class PythonKernel { }); } + private abortPendingExecutions(reason: string): void { + if (this.#pendingExecutions.size === 0) return; + for (const cancel of this.#pendingExecutions.values()) { + cancel(reason); + } + this.#pendingExecutions.clear(); + this.#messageHandlers.clear(); + logger.warn("Aborted pending Python executions", { reason }); + } + isAlive(): boolean { return this.#alive && !this.#disposed && this.#ws?.readyState === WebSocket.OPEN; } @@ -742,27 +853,52 @@ export class PythonKernel { let cancelled = false; let timedOut = false; - using executionSignal = new ScopeSignal({ signal: options?.signal, timeout: options?.timeoutMs }); + const executionSignal = new ScopeSignal({ signal: options?.signal, timeout: options?.timeoutMs }); return new Promise((resolve) => { - const cleanup = () => { + let resolved = false; + const finalize = () => { + if (resolved) return; + resolved = true; this.#messageHandlers.delete(msgId); + this.#pendingExecutions.delete(msgId); + executionSignal[Symbol.dispose](); resolve({ status, executionCount, error, cancelled, timedOut, stdinRequested }); }; const checkDone = () => { if (replyReceived && idleReceived) { - cleanup(); + finalize(); } }; + const cancelFromClose = (reason: string) => { + if (resolved) return; + cancelled = true; + timedOut = false; + if (options?.onChunk) { + void options.onChunk(`[kernel] ${reason}\n`); + } + finalize(); + }; + + this.#pendingExecutions.set(msgId, cancelFromClose); + executionSignal.catch(async () => { cancelled = true; timedOut = executionSignal.timedOut(); - await this.interrupt(); - cleanup(); + try { + await this.interrupt(); + } finally { + finalize(); + } }); + if (executionSignal.aborted) { + cancelFromClose("Execution aborted"); + return; + } + this.#messageHandlers.set(msgId, async (response) => { switch (response.header.msg_type) { case "execute_reply": { @@ -844,7 +980,12 @@ export class PythonKernel { } }); - this.sendMessage(msg); + try { + this.sendMessage(msg); + } catch { + cancelled = true; + finalize(); + } }); } @@ -909,6 +1050,7 @@ export class PythonKernel { if (this.#disposed) return; this.#disposed = true; this.#alive = false; + this.abortPendingExecutions("Kernel shutdown"); if (this.#heartbeatTimer) { clearInterval(this.#heartbeatTimer); diff --git a/packages/coding-agent/src/core/python-prelude.py b/packages/coding-agent/src/core/python-prelude.py index 0a7c7f2ad..0e03a823e 100644 --- a/packages/coding-agent/src/core/python-prelude.py +++ b/packages/coding-agent/src/core/python-prelude.py @@ -19,14 +19,14 @@ if "__omp_prelude_loaded__" not in globals(): @_category("Navigation") def pwd() -> Path: - """Print and return current working directory.""" + """Return current working directory.""" p = Path.cwd() _emit_status("pwd", path=str(p)) return p @_category("Navigation") def cd(path: str | Path) -> Path: - """Change directory and print the new cwd.""" + """Change directory.""" p = Path(path).expanduser().resolve() os.chdir(p) _emit_status("cd", path=str(p)) @@ -37,9 +37,7 @@ if "__omp_prelude_loaded__" not in globals(): """Get/set environment variables.""" if key is None: items = dict(sorted(os.environ.items())) - for k, v in items.items(): - print(f"{k}={v}") - _emit_status("env", count=len(items)) + _emit_status("env", count=len(items), keys=list(items.keys())[:20]) return items if value is not None: os.environ[key] = value @@ -50,21 +48,23 @@ if "__omp_prelude_loaded__" not in globals(): return val @_category("File I/O") - def read(path: str | Path, *, limit: int | None = None) -> str: - """Read file contents. Prints a short preview + length.""" + def read(path: str | Path, *, offset: int = 1, limit: int | None = None) -> str: + """Read file contents. offset/limit are 1-indexed line numbers.""" p = Path(path) data = p.read_text(encoding="utf-8") - if limit is not None: - preview = data[:limit] - print(preview) - else: - print(data) - _emit_status("read", path=str(p), chars=len(data)) + lines = data.splitlines(keepends=True) + if offset > 1 or limit is not None: + start = max(0, offset - 1) + end = start + limit if limit else len(lines) + lines = lines[start:end] + data = "".join(lines) + preview = data[:500] + _emit_status("read", path=str(p), chars=len(data), preview=preview) return data @_category("File I/O") def write(path: str | Path, content: str) -> Path: - """Write file contents (create parents). Prints bytes written.""" + """Write file contents (create parents).""" p = Path(path) p.parent.mkdir(parents=True, exist_ok=True) p.write_text(content, encoding="utf-8") @@ -73,7 +73,7 @@ if "__omp_prelude_loaded__" not in globals(): @_category("File I/O") def append(path: str | Path, content: str) -> Path: - """Append to file. Prints bytes appended.""" + """Append to file.""" p = Path(path) p.parent.mkdir(parents=True, exist_ok=True) with p.open("a", encoding="utf-8") as f: @@ -134,68 +134,177 @@ if "__omp_prelude_loaded__" not in globals(): """List directory contents.""" p = Path(path) items = sorted(p.iterdir()) - for item in items: - suffix = "/" if item.is_dir() else "" - print(f"{item.name}{suffix}") - _emit_status("ls", path=str(p), count=len(items)) + _emit_status("ls", path=str(p), count=len(items), items=[i.name + ("/" if i.is_dir() else "") for i in items[:20]]) return items + def _load_gitignore_patterns(base: Path) -> list[str]: + """Load .gitignore patterns from base directory and parents.""" + patterns: list[str] = [] + # Always exclude these + patterns.extend(["**/.git", "**/.git/**", "**/node_modules", "**/node_modules/**"]) + # Walk up to find .gitignore files + current = base.resolve() + for _ in range(20): # Limit depth + gitignore = current / ".gitignore" + if gitignore.exists(): + try: + for line in gitignore.read_text().splitlines(): + line = line.strip() + if line and not line.startswith("#"): + # Normalize pattern for fnmatch + if line.startswith("/"): + patterns.append(str(current / line[1:])) + else: + patterns.append(f"**/{line}") + except Exception: + pass + parent = current.parent + if parent == current: + break + current = parent + return patterns + + def _match_gitignore(path: Path, patterns: list[str], base: Path) -> bool: + """Check if path matches any gitignore pattern.""" + import fnmatch + rel = str(path.relative_to(base)) if path.is_relative_to(base) else str(path) + abs_path = str(path.resolve()) + for pat in patterns: + if pat.startswith("**/"): + # Match against any part of the path + if fnmatch.fnmatch(rel, pat) or fnmatch.fnmatch(rel, pat[3:]): + return True + # Also check each path component + for part in path.parts: + if fnmatch.fnmatch(part, pat[3:]): + return True + elif fnmatch.fnmatch(abs_path, pat) or fnmatch.fnmatch(rel, pat): + return True + return False + @_category("Search") - def find(pattern: str, path: str | Path = ".", *, files_only: bool = True) -> list[Path]: - """Recursive glob find. Defaults to files only.""" + def find( + pattern: str, + path: str | Path = ".", + *, + type: str = "file", + limit: int = 1000, + hidden: bool = False, + sort_by_mtime: bool = False, + ) -> list[Path]: + """Recursive glob find. Respects .gitignore.""" p = Path(path) - matches = [] + ignore_patterns = _load_gitignore_patterns(p) + matches: list[Path] = [] for m in p.rglob(pattern): - if files_only and m.is_dir(): + if len(matches) >= limit: + break + # Skip hidden files unless requested + if not hidden and any(part.startswith(".") for part in m.parts): + continue + # Skip gitignored paths + if _match_gitignore(m, ignore_patterns, p): + continue + # Filter by type + if type == "file" and m.is_dir(): + continue + if type == "dir" and not m.is_dir(): continue matches.append(m) - matches = sorted(matches) - for m in matches: - print(str(m)) - _emit_status("find", pattern=pattern, path=str(p), count=len(matches)) + if sort_by_mtime: + matches.sort(key=lambda x: x.stat().st_mtime, reverse=True) + else: + matches.sort() + _emit_status("find", pattern=pattern, path=str(p), count=len(matches), matches=[str(m) for m in matches[:20]]) return matches @_category("Search") - def grep(pattern: str, path: str | Path, *, ignore_case: bool = False, context: int = 0) -> list[tuple[int, str]]: - """Grep a single file.""" - flags = re.IGNORECASE if ignore_case else 0 - rx = re.compile(pattern, flags) + def grep( + pattern: str, + path: str | Path, + *, + ignore_case: bool = False, + literal: bool = False, + context: int = 0, + ) -> list[tuple[int, str]]: + """Grep a single file. Returns (line_number, text) tuples.""" p = Path(path) lines = p.read_text(encoding="utf-8").splitlines() - hits: list[tuple[int, str]] = [] + if literal: + if ignore_case: + match_fn = lambda line: pattern.lower() in line.lower() + else: + match_fn = lambda line: pattern in line + else: + flags = re.IGNORECASE if ignore_case else 0 + rx = re.compile(pattern, flags) + match_fn = lambda line: rx.search(line) is not None + + match_lines: set[int] = set() for i, line in enumerate(lines, 1): - if rx.search(line): - hits.append((i, line)) - print(f"{i}: {line}") - if context: - start = max(0, i - 1 - context) - end = min(len(lines), i - 1 + context + 1) - for j in range(start, end): - if j + 1 == i: - continue - print(f"{j+1}- {lines[j]}") - _emit_status("grep", pattern=pattern, path=str(p), count=len(hits)) + if match_fn(line): + match_lines.add(i) + + # Expand with context + if context > 0: + expanded: set[int] = set() + for ln in match_lines: + for offset in range(-context, context + 1): + expanded.add(ln + offset) + output_lines = sorted(ln for ln in expanded if 1 <= ln <= len(lines)) + else: + output_lines = sorted(match_lines) + + hits = [(ln, lines[ln - 1]) for ln in output_lines] + _emit_status("grep", pattern=pattern, path=str(p), count=len(match_lines), hits=[{"line": h[0], "text": h[1][:100]} for h in hits[:10]]) return hits @_category("Search") - def rgrep(pattern: str, path: str | Path = ".", *, glob_pattern: str = "*", ignore_case: bool = False) -> list[tuple[Path, int, str]]: - """Recursive grep across files matching glob_pattern.""" - flags = re.IGNORECASE if ignore_case else 0 - rx = re.compile(pattern, flags) + def rgrep( + pattern: str, + path: str | Path = ".", + *, + glob_pattern: str = "*", + ignore_case: bool = False, + literal: bool = False, + limit: int = 100, + hidden: bool = False, + ) -> list[tuple[Path, int, str]]: + """Recursive grep across files matching glob_pattern. Respects .gitignore.""" + if literal: + if ignore_case: + match_fn = lambda line: pattern.lower() in line.lower() + else: + match_fn = lambda line: pattern in line + else: + flags = re.IGNORECASE if ignore_case else 0 + rx = re.compile(pattern, flags) + match_fn = lambda line: rx.search(line) is not None + base = Path(path) + ignore_patterns = _load_gitignore_patterns(base) hits: list[tuple[Path, int, str]] = [] for file_path in base.rglob(glob_pattern): + if len(hits) >= limit: + break if file_path.is_dir(): continue + # Skip hidden files unless requested + if not hidden and any(part.startswith(".") for part in file_path.parts): + continue + # Skip gitignored paths + if _match_gitignore(file_path, ignore_patterns, base): + continue try: lines = file_path.read_text(encoding="utf-8").splitlines() except Exception: continue for i, line in enumerate(lines, 1): - if rx.search(line): + if len(hits) >= limit: + break + if match_fn(line): hits.append((file_path, i, line)) - print(f"{file_path}:{i}: {line}") - _emit_status("rgrep", pattern=pattern, path=str(base), count=len(hits)) + _emit_status("rgrep", pattern=pattern, path=str(base), count=len(hits), hits=[{"file": str(h[0]), "line": h[1], "text": h[2][:80]} for h in hits[:10]]) return hits @_category("Text") @@ -203,8 +312,7 @@ if "__omp_prelude_loaded__" not in globals(): """Return the first n lines of text.""" lines = text.splitlines()[:n] out = "\n".join(lines) - print(out) - _emit_status("head", lines=len(lines)) + _emit_status("head", lines=len(lines), preview=out[:500]) return out @_category("Text") @@ -212,8 +320,7 @@ if "__omp_prelude_loaded__" not in globals(): """Return the last n lines of text.""" lines = text.splitlines()[-n:] out = "\n".join(lines) - print(out) - _emit_status("tail", lines=len(lines)) + _emit_status("tail", lines=len(lines), preview=out[:500]) return out @_category("Find/Replace") @@ -244,26 +351,49 @@ if "__omp_prelude_loaded__" not in globals(): def __bool__(self): return self.code == 0 - def _make_shell_result(proc: subprocess.CompletedProcess[str]) -> ShellResult: - """Create ShellResult and print output.""" - if proc.stdout: - print(proc.stdout, end="" if proc.stdout.endswith("\n") else "\n") - if proc.stderr: - print(proc.stderr, end="" if proc.stderr.endswith("\n") else "\n") + def _make_shell_result(proc: subprocess.CompletedProcess[str], cmd: str) -> ShellResult: + """Create ShellResult and emit status.""" + output = proc.stdout + proc.stderr if proc.stderr else proc.stdout + _emit_status("sh", cmd=cmd[:80], code=proc.returncode, output=output[:500]) return ShellResult(proc.stdout, proc.stderr, proc.returncode) + import signal as _signal + + def _run_with_interrupt(args: list[str], cwd: str | None, timeout: int | None, cmd: str) -> ShellResult: + """Run subprocess with proper interrupt handling.""" + proc = subprocess.Popen( + args, + cwd=cwd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + start_new_session=True, + ) + try: + stdout, stderr = proc.communicate(timeout=timeout) + except KeyboardInterrupt: + os.killpg(proc.pid, _signal.SIGINT) + try: + stdout, stderr = proc.communicate(timeout=2) + except subprocess.TimeoutExpired: + os.killpg(proc.pid, _signal.SIGKILL) + stdout, stderr = proc.communicate() + result = subprocess.CompletedProcess(args, -_signal.SIGINT, stdout, stderr) + return _make_shell_result(result, cmd) + except subprocess.TimeoutExpired: + os.killpg(proc.pid, _signal.SIGKILL) + stdout, stderr = proc.communicate() + result = subprocess.CompletedProcess(args, -_signal.SIGKILL, stdout, stderr) + return _make_shell_result(result, cmd) + result = subprocess.CompletedProcess(args, proc.returncode, stdout, stderr) + return _make_shell_result(result, cmd) + @_category("Shell") def run(cmd: str, *, cwd: str | Path | None = None, timeout: int | None = None) -> ShellResult: - """Run a shell command and print stdout/stderr.""" - proc = subprocess.run( - cmd, - cwd=str(cwd) if cwd else None, - shell=True, - capture_output=True, - text=True, - timeout=timeout, - ) - return _make_shell_result(proc) + """Run a shell command.""" + shell_path = shutil.which("bash") or shutil.which("sh") or "/bin/sh" + args = [shell_path, "-c", cmd] + return _run_with_interrupt(args, str(cwd) if cwd else None, timeout, cmd) @_category("Shell") def sh(cmd: str, *, cwd: str | Path | None = None, timeout: int | None = None) -> ShellResult: @@ -272,7 +402,6 @@ if "__omp_prelude_loaded__" not in globals(): prefix = f"source '{snapshot}' 2>/dev/null && " if snapshot else "" final = f"{prefix}{cmd}" - # Determine shell and args (mirroring shell.ts logic) shell_path = os.environ.get("SHELL") if not shell_path or not shutil.which(shell_path): shell_path = shutil.which("bash") or shutil.which("zsh") or shutil.which("sh") @@ -286,32 +415,22 @@ if "__omp_prelude_loaded__" not in globals(): text=True, timeout=timeout, ) - return _make_shell_result(proc) + return _make_shell_result(proc, cmd) raise RuntimeError("No suitable shell found") no_login = os.environ.get("OMP_BASH_NO_LOGIN") or os.environ.get("CLAUDE_BASH_NO_LOGIN") args = [shell_path, "-c", final] if no_login else [shell_path, "-l", "-c", final] - proc = subprocess.run( - args, - cwd=str(cwd) if cwd else None, - capture_output=True, - text=True, - timeout=timeout, - ) - return _make_shell_result(proc) - - # --- Extended shell-like utilities --- + return _run_with_interrupt(args, str(cwd) if cwd else None, timeout, cmd) @_category("File I/O") def cat(*paths: str | Path, separator: str = "\n") -> str: - """Concatenate multiple files and print. Like shell cat.""" + """Concatenate multiple files. Like shell cat.""" parts = [] for p in paths: parts.append(Path(p).read_text(encoding="utf-8")) out = separator.join(parts) - print(out) - _emit_status("cat", files=len(paths), chars=len(out)) + _emit_status("cat", files=len(paths), chars=len(out), preview=out[:500]) return out @_category("File I/O") @@ -340,7 +459,6 @@ if "__omp_prelude_loaded__" not in globals(): lines = list(dict.fromkeys(lines)) lines = sorted(lines, reverse=reverse) out = "\n".join(lines) - print(out) _emit_status("sort_lines", lines=len(lines), unique=unique, reverse=reverse) return out @@ -362,15 +480,10 @@ if "__omp_prelude_loaded__" not in globals(): current = line current_count = 1 groups.append((current_count, current)) + _emit_status("uniq", groups=len(groups), count_mode=count) if count: - for c, l in groups: - print(f"{c:>4} {l}") - _emit_status("uniq", groups=len(groups), count_mode=True) return groups - out = "\n".join(line for _, line in groups) - print(out) - _emit_status("uniq", groups=len(groups)) - return out + return "\n".join(line for _, line in groups) @_category("Text") def cols(text: str, *indices: int, sep: str | None = None) -> str: @@ -381,13 +494,12 @@ if "__omp_prelude_loaded__" not in globals(): selected = [parts[i] for i in indices if i < len(parts)] result_lines.append(" ".join(selected)) out = "\n".join(result_lines) - print(out) _emit_status("cols", lines=len(result_lines), columns=list(indices)) return out @_category("Navigation") def tree(path: str | Path = ".", *, max_depth: int = 3, show_hidden: bool = False) -> str: - """Print directory tree.""" + """Return directory tree.""" base = Path(path) lines = [] def walk(p: Path, prefix: str, depth: int): @@ -406,8 +518,7 @@ if "__omp_prelude_loaded__" not in globals(): lines.append(str(base) + "/") walk(base, "", 1) out = "\n".join(lines) - print(out) - _emit_status("tree", path=str(base), entries=len(lines) - 1) + _emit_status("tree", path=str(base), entries=len(lines) - 1, preview=out[:1000]) return out @_category("Navigation") @@ -423,35 +534,37 @@ if "__omp_prelude_loaded__" not in globals(): "mtime": datetime.fromtimestamp(s.st_mtime).isoformat(), "mode": oct(s.st_mode), } - for k, v in info.items(): - print(f"{k}: {v}") - _emit_status("stat", path=str(p), size=s.st_size, is_dir=p.is_dir()) + _emit_status("stat", path=str(p), size=s.st_size, is_dir=p.is_dir(), mtime=info["mtime"]) return info @_category("Batch") def diff(a: str | Path, b: str | Path) -> str: - """Compare two files, print unified diff.""" + """Compare two files, return unified diff.""" import difflib path_a, path_b = Path(a), Path(b) lines_a = path_a.read_text(encoding="utf-8").splitlines(keepends=True) lines_b = path_b.read_text(encoding="utf-8").splitlines(keepends=True) result = difflib.unified_diff(lines_a, lines_b, fromfile=str(path_a), tofile=str(path_b)) out = "".join(result) - if out: - print(out) - _emit_status("diff", file_a=str(path_a), file_b=str(path_b), identical=False) - else: - _emit_status("diff", file_a=str(path_a), file_b=str(path_b), identical=True) + _emit_status("diff", file_a=str(path_a), file_b=str(path_b), identical=not out, preview=out[:500]) return out @_category("Search") - def glob_files(pattern: str, path: str | Path = ".") -> list[Path]: - """Non-recursive glob (use find() for recursive).""" + def glob_files(pattern: str, path: str | Path = ".", *, hidden: bool = False) -> list[Path]: + """Non-recursive glob (use find() for recursive). Respects .gitignore.""" p = Path(path) - matches = sorted(p.glob(pattern)) - for m in matches: - print(str(m)) - _emit_status("glob", pattern=pattern, path=str(p), count=len(matches)) + ignore_patterns = _load_gitignore_patterns(p) + matches: list[Path] = [] + for m in p.glob(pattern): + # Skip hidden files unless requested + if not hidden and m.name.startswith("."): + continue + # Skip gitignored paths + if _match_gitignore(m, ignore_patterns, p): + continue + matches.append(m) + matches = sorted(matches) + _emit_status("glob", pattern=pattern, path=str(p), count=len(matches), matches=[str(m) for m in matches[:20]]) return matches @_category("Batch") @@ -475,29 +588,44 @@ if "__omp_prelude_loaded__" not in globals(): return count @_category("Find/Replace") - def rsed(pattern: str, repl: str, path: str | Path = ".", *, glob_pattern: str = "*", flags: int = 0) -> int: - """Recursive sed across files matching glob_pattern.""" + def rsed( + pattern: str, + repl: str, + path: str | Path = ".", + *, + glob_pattern: str = "*", + flags: int = 0, + hidden: bool = False, + ) -> int: + """Recursive sed across files matching glob_pattern. Respects .gitignore.""" base = Path(path) + ignore_patterns = _load_gitignore_patterns(base) total = 0 files_changed = 0 + changed_files = [] for file_path in base.rglob(glob_pattern): if file_path.is_dir(): continue + # Skip hidden files unless requested + if not hidden and any(part.startswith(".") for part in file_path.parts): + continue + # Skip gitignored paths + if _match_gitignore(file_path, ignore_patterns, base): + continue try: data = file_path.read_text(encoding="utf-8") new, count = re.subn(pattern, repl, data, flags=flags) if count > 0: file_path.write_text(new, encoding="utf-8") - print(f"{file_path}: {count} replacements") total += count files_changed += 1 + if len(changed_files) < 10: + changed_files.append({"file": str(file_path), "count": count}) except Exception: continue - _emit_status("rsed", path=str(base), count=total, files=files_changed) + _emit_status("rsed", path=str(base), count=total, files=files_changed, changed=changed_files) return total - # --- Line-based operations (sed-like) --- - @_category("Line ops") def lines(path: str | Path, start: int = 1, end: int | None = None) -> str: """Extract line range from file (1-indexed, inclusive). Like sed -n 'N,Mp'.""" @@ -508,10 +636,9 @@ if "__omp_prelude_loaded__" not in globals(): start = max(1, start) end = min(len(all_lines), end) selected = all_lines[start - 1 : end] - out = "\n".join(f"{start + i}: {line}" for i, line in enumerate(selected)) - print(out) - _emit_status("lines", path=str(p), start=start, end=end, count=len(selected)) - return "\n".join(selected) + out = "\n".join(selected) + _emit_status("lines", path=str(p), start=start, end=end, count=len(selected), preview=out[:500]) + return out @_category("Line ops") def delete_lines(path: str | Path, start: int, end: int | None = None) -> int: @@ -562,8 +689,6 @@ if "__omp_prelude_loaded__" not in globals(): _emit_status("insert_at", path=str(p), line=line_num, lines_inserted=len(new_lines), position=pos) return p - # --- Git helpers --- - def _git(*args: str, cwd: str | Path | None = None) -> tuple[int, str, str]: """Run git command, return (returncode, stdout, stderr).""" result = subprocess.run( @@ -604,33 +729,8 @@ if "__omp_prelude_loaded__" not in globals(): elif line.startswith("? "): result["untracked"].append(line[2:]) - # Pretty print - print(f"branch: {result['branch']}", end="") - if result["ahead"] or result["behind"]: - print(f" (+{result['ahead']}/-{result['behind']})", end="") - print() - if result["staged"]: - print(f"staged ({len(result['staged'])}):") - for f in result["staged"][:10]: - print(f" + {f}") - if len(result["staged"]) > 10: - print(f" ... and {len(result['staged']) - 10} more") - if result["modified"]: - print(f"modified ({len(result['modified'])}):") - for f in result["modified"][:10]: - print(f" M {f}") - if len(result["modified"]) > 10: - print(f" ... and {len(result['modified']) - 10} more") - if result["untracked"]: - print(f"untracked ({len(result['untracked'])}):") - for f in result["untracked"][:5]: - print(f" ? {f}") - if len(result["untracked"]) > 5: - print(f" ... and {len(result['untracked']) - 5} more") clean = not any([result["staged"], result["modified"], result["untracked"]]) - if clean: - print("working tree clean") - _emit_status("git_status", branch=result["branch"], staged=len(result["staged"]), modified=len(result["modified"]), untracked=len(result["untracked"]), clean=clean) + _emit_status("git_status", branch=result["branch"], staged=len(result["staged"]), modified=len(result["modified"]), untracked=len(result["untracked"]), clean=clean, files=result["staged"][:5] + result["modified"][:5]) return result @_category("Git") @@ -656,9 +756,8 @@ if "__omp_prelude_loaded__" not in globals(): if code != 0: _emit_status("git_diff", error=err.strip()) return "" - print(out) lines_count = len(out.splitlines()) if out else 0 - _emit_status("git_diff", staged=staged, ref=ref, lines=lines_count) + _emit_status("git_diff", staged=staged, ref=ref, lines=lines_count, preview=out[:500]) return out @_category("Git") @@ -689,11 +788,7 @@ if "__omp_prelude_loaded__" not in globals(): if len(parts) >= 4: commits.append({"sha": parts[0], "subject": parts[1], "author": parts[2], "date": parts[3]}) - # Pretty print - for c in commits: - date_short = c["date"][:10] - print(f"{c['sha'][:8]} {date_short} {c['subject'][:60]}") - _emit_status("git_log", commits=len(commits)) + _emit_status("git_log", commits=len(commits), entries=[{"sha": c["sha"][:8], "subject": c["subject"][:50]} for c in commits[:5]]) return commits @_category("Git") @@ -719,19 +814,7 @@ if "__omp_prelude_loaded__" not in globals(): _, stat_out, _ = _git("show", ref, "--stat", "--format=", cwd=cwd) result["files"] = [l.strip() for l in stat_out.strip().splitlines() if l.strip()] - # Pretty print - print(f"commit {result['sha'][:12]}") - print(f"Author: {result['author']}") - print(f"Date: {result['date']}") - print(f"\n {result['subject']}") - if result["body"]: - for line in result["body"].splitlines()[:5]: - print(f" {line}") - if result["files"]: - print() - for f in result["files"][-5:]: - print(f" {f}") - _emit_status("git_show", ref=ref, sha=result["sha"][:12], files=len(result["files"])) + _emit_status("git_show", ref=ref, sha=result["sha"][:12], subject=result["subject"][:60], files=len(result["files"])) return result @_category("Git") @@ -748,12 +831,10 @@ if "__omp_prelude_loaded__" not in globals(): start = max(1, start) end = min(len(all_lines), end) selected = all_lines[start - 1 : end] - out = "\n".join(f"{start + i}: {line}" for i, line in enumerate(selected)) - print(out) + out = "\n".join(selected) _emit_status("git_file_at", ref=ref, path=path, start=start, end=end, lines=len(selected)) - return "\n".join(selected) + return out - print(out) _emit_status("git_file_at", ref=ref, path=path, chars=len(out)) return out @@ -779,13 +860,7 @@ if "__omp_prelude_loaded__" not in globals(): if is_current: result["current"] = name - print(f"* {result['current']}") - for b in result["local"]: - if b != result["current"]: - print(f" {b}") - if result["remote"]: - print(f" ({len(result['remote'])} remote branches)") - _emit_status("git_branch", current=result["current"], local=len(result["local"]), remote=len(result["remote"])) + _emit_status("git_branch", current=result["current"], local=len(result["local"]), remote=len(result["remote"]), branches=result["local"][:10]) return result @_category("Git") diff --git a/packages/coding-agent/src/core/python-prelude.test.ts b/packages/coding-agent/src/core/python-prelude.test.ts index 7a4004ab1..bdf11df2a 100644 --- a/packages/coding-agent/src/core/python-prelude.test.ts +++ b/packages/coding-agent/src/core/python-prelude.test.ts @@ -1,7 +1,24 @@ import { describe, expect, it } from "bun:test"; -import { createPythonTool } from "./tools/python"; +import { existsSync } from "node:fs"; +import { join } from "node:path"; +import { resetPreludeDocsCache, warmPythonEnvironment } from "./python-executor"; +import { createPythonTool, getPythonToolDescription } from "./tools/python"; -const pythonPath = Bun.which("python") ?? Bun.which("python3"); +const resolvePythonPath = (): string | null => { + const venvPath = process.env.VIRTUAL_ENV; + const candidates = [venvPath, join(process.cwd(), ".venv"), join(process.cwd(), "venv")].filter(Boolean) as string[]; + for (const candidate of candidates) { + const binDir = process.platform === "win32" ? "Scripts" : "bin"; + const exeName = process.platform === "win32" ? "python.exe" : "python"; + const pythonCandidate = join(candidate, binDir, exeName); + if (existsSync(pythonCandidate)) { + return pythonCandidate; + } + } + return Bun.which("python") ?? Bun.which("python3"); +}; + +const pythonPath = resolvePythonPath(); const hasKernelDeps = (() => { if (!pythonPath) return false; const result = Bun.spawnSync( @@ -31,12 +48,36 @@ describe.skipIf(!shouldRun)("PYTHON_PRELUDE integration", () => { "mv", "cp", "ls", + "cat", + "touch", "find", "grep", "rgrep", "head", "tail", "replace", + "sed", + "rsed", + "wc", + "sort_lines", + "uniq", + "cols", + "tree", + "stat", + "diff", + "glob_files", + "batch", + "lines", + "delete_lines", + "delete_matching", + "insert_at", + "git_status", + "git_diff", + "git_log", + "git_show", + "git_file_at", + "git_branch", + "git_has_changes", "run", "sh", ]; @@ -63,15 +104,37 @@ describe.skipIf(!shouldRun)("PYTHON_PRELUDE integration", () => { const tool = createPythonTool(session); const code = ` -helpers = ${JSON.stringify(helpers)} -missing = [name for name in helpers if name not in globals() or not callable(globals()[name])] -print("HELPERS_OK=" + ("1" if not missing else "0")) -if missing: - print("MISSING=" + ",".join(missing)) -`; + helpers = ${JSON.stringify(helpers)} + missing = [name for name in helpers if name not in globals() or not callable(globals()[name])] + docs = __omp_prelude_docs__() + doc_names = [d.get("name") for d in docs] + doc_categories = [d.get("category") for d in docs] + print("HELPERS_OK=" + ("1" if not missing else "0")) + print("DOCS_OK=" + ("1" if "pwd" in doc_names and "Navigation" in doc_categories else "0")) + if missing: + print("MISSING=" + ",".join(missing)) + `; const result = await tool.execute("tool-call-1", { code }); const output = result.content.find((item) => item.type === "text")?.text ?? ""; expect(output).toContain("HELPERS_OK=1"); + expect(output).toContain("DOCS_OK=1"); + }); + + it("exposes prelude docs via warmup", async () => { + resetPreludeDocsCache(); + const result = await warmPythonEnvironment(process.cwd(), undefined, false); + expect(result.ok).toBe(true); + const names = result.docs.map((doc) => doc.name); + expect(names).toContain("pwd"); + }); + + it("renders prelude docs in python tool description", async () => { + resetPreludeDocsCache(); + const result = await warmPythonEnvironment(process.cwd(), undefined, false); + expect(result.ok).toBe(true); + const description = getPythonToolDescription(); + expect(description).toContain("pwd"); + expect(description).not.toContain("Documentation unavailable"); }); }); diff --git a/packages/coding-agent/src/core/settings-manager-python.test.ts b/packages/coding-agent/src/core/settings-manager-python.test.ts index 42a9fc022..16ea748cf 100644 --- a/packages/coding-agent/src/core/settings-manager-python.test.ts +++ b/packages/coding-agent/src/core/settings-manager-python.test.ts @@ -2,10 +2,10 @@ import { describe, expect, it } from "bun:test"; import { SettingsManager } from "./settings-manager"; describe("SettingsManager python settings", () => { - it("defaults to ipy-only and session", () => { + it("defaults to both and session", () => { const settings = SettingsManager.inMemory(); - expect(settings.getPythonToolMode()).toBe("ipy-only"); + expect(settings.getPythonToolMode()).toBe("both"); expect(settings.getPythonKernelMode()).toBe("session"); }); diff --git a/packages/coding-agent/src/core/streaming-output.ts b/packages/coding-agent/src/core/streaming-output.ts index c0923c90f..42ba6b337 100644 --- a/packages/coding-agent/src/core/streaming-output.ts +++ b/packages/coding-agent/src/core/streaming-output.ts @@ -12,10 +12,19 @@ interface OutputFileSink { export function createSanitizer(): TransformStream { const decoder = new TextDecoder(); + const sanitizeText = (text: string) => sanitizeBinaryOutput(stripAnsi(text)).replace(/\r/g, ""); return new TransformStream({ transform(chunk, controller) { - const text = sanitizeBinaryOutput(stripAnsi(decoder.decode(chunk, { stream: true }))).replace(/\r/g, ""); - controller.enqueue(text); + const text = sanitizeText(decoder.decode(chunk, { stream: true })); + if (text) { + controller.enqueue(text); + } + }, + flush(controller) { + const text = sanitizeText(decoder.decode()); + if (text) { + controller.enqueue(text); + } }, }); } @@ -40,7 +49,7 @@ export function createOutputSink( ): WritableStream & { dump: (annotation?: string) => { output: string; truncated: boolean; fullOutputPath?: string }; } { - const chunks: string[] = []; + const chunks: Array<{ text: string; bytes: number }> = []; let chunkBytes = 0; let totalBytes = 0; let fullOutputPath: string | undefined; @@ -48,22 +57,26 @@ export function createOutputSink( const sink = new WritableStream({ write(text) { - totalBytes += text.length; + const bytes = Buffer.byteLength(text, "utf-8"); + totalBytes += bytes; if (totalBytes > spillThreshold && !fullOutputPath) { fullOutputPath = join(tmpdir(), `omp-${nanoid()}.buffer`); const stream = Bun.file(fullOutputPath).writer(); - chunks.forEach((chunk) => { - stream.write(chunk); - }); + for (const chunk of chunks) { + stream.write(chunk.text); + } fullOutputStream = stream; } fullOutputStream?.write(text); - chunks.push(text); - chunkBytes += text.length; + chunks.push({ text, bytes }); + chunkBytes += bytes; while (chunkBytes > maxBuffer && chunks.length > 1) { - chunkBytes -= chunks.shift()!.length; + const removed = chunks.shift(); + if (removed) { + chunkBytes -= removed.bytes; + } } onChunk?.(text); @@ -76,9 +89,10 @@ export function createOutputSink( return Object.assign(sink, { dump(annotation?: string) { if (annotation) { - chunks.push(`\n\n${annotation}`); + const text = `\n\n${annotation}`; + chunks.push({ text, bytes: Buffer.byteLength(text, "utf-8") }); } - const full = chunks.join(""); + const full = chunks.map((chunk) => chunk.text).join(""); const { content, truncated } = truncateTail(full); return { output: truncated ? content : full, truncated, fullOutputPath }; }, diff --git a/packages/coding-agent/src/core/tools/index.ts b/packages/coding-agent/src/core/tools/index.ts index ea609d6c3..a6e0c2f93 100644 --- a/packages/coding-agent/src/core/tools/index.ts +++ b/packages/coding-agent/src/core/tools/index.ts @@ -136,6 +136,7 @@ export interface ToolSession { getBashInterceptorRules(): BashInterceptorRule[]; getPythonToolMode?(): "ipy-only" | "bash-only" | "both"; getPythonKernelMode?(): "session" | "per-call"; + getPythonSharedGateway?(): boolean; }; } @@ -221,13 +222,18 @@ export async function createTools(session: ToolSession, toolNames?: string[]): P reason: availability.reason, }); } else if (!isTestEnv && getPreludeDocs().length === 0) { - void warmPythonEnvironment(session.cwd, session.getSessionFile?.() ?? `cwd:${session.cwd}`).catch((err) => { - logger.warn("Failed to warm Python environment", { - error: err instanceof Error ? err.message : String(err), - }); - }); + const sessionFile = session.getSessionFile?.() ?? undefined; + const warmSessionId = sessionFile ? `session:${sessionFile}:workdir:${session.cwd}` : `cwd:${session.cwd}`; + void warmPythonEnvironment(session.cwd, warmSessionId, session.settings?.getPythonSharedGateway?.()).catch( + (err) => { + logger.warn("Failed to warm Python environment", { + error: err instanceof Error ? err.message : String(err), + }); + }, + ); } } + const effectiveMode = pythonAvailable ? pythonMode : "bash-only"; const allowBash = effectiveMode !== "ipy-only"; const allowPython = effectiveMode !== "bash-only"; diff --git a/packages/coding-agent/src/core/tools/python-execution.test.ts b/packages/coding-agent/src/core/tools/python-execution.test.ts index aab0255b5..b5853ee3b 100644 --- a/packages/coding-agent/src/core/tools/python-execution.test.ts +++ b/packages/coding-agent/src/core/tools/python-execution.test.ts @@ -54,7 +54,7 @@ describe("python tool execution", () => { expect.objectContaining({ cwd: tempDir, timeout: 5000, - sessionId: "session-file", + sessionId: `session:session-file:workdir:${tempDir}`, kernelMode: "per-call", reset: true, }), diff --git a/packages/coding-agent/src/core/tools/python.test.ts b/packages/coding-agent/src/core/tools/python.test.ts index 11f9e3713..33cff2c7e 100644 --- a/packages/coding-agent/src/core/tools/python.test.ts +++ b/packages/coding-agent/src/core/tools/python.test.ts @@ -77,7 +77,8 @@ describe("python tool docs template", () => { const tool = createPythonTool(createSession()); expect(tool.description).toContain("### File I/O"); - expect(tool.description).toContain("- `read(path)` — Read file contents."); + expect(tool.description).toContain("read(path)"); + expect(tool.description).toContain("Read file contents."); spy.mockRestore(); }); diff --git a/packages/coding-agent/src/core/tools/python.ts b/packages/coding-agent/src/core/tools/python.ts index 8d737c450..bf18d9efd 100644 --- a/packages/coding-agent/src/core/tools/python.ts +++ b/packages/coding-agent/src/core/tools/python.ts @@ -173,12 +173,15 @@ export function createPythonTool( const jsonOutputs: unknown[] = []; const images: ImageContent[] = []; + const sessionFile = session.getSessionFile?.() ?? undefined; + const sessionId = sessionFile ? `session:${sessionFile}:workdir:${commandCwd}` : `cwd:${commandCwd}`; const executorOptions: PythonExecutorOptions = { cwd: commandCwd, timeout: timeout ? timeout * 1000 : undefined, signal: controller.signal, - sessionId: session.getSessionFile?.() ?? `cwd:${session.cwd}`, + sessionId, kernelMode: session.settings?.getPythonKernelMode?.() ?? "session", + useSharedGateway: session.settings?.getPythonSharedGateway?.() ?? true, reset, onChunk: (chunk) => { const chunkBytes = Buffer.byteLength(chunk, "utf-8"); @@ -357,7 +360,7 @@ function formatStatusEvent(event: PythonStatusEvent, theme: Theme): string { parts.push(`${data.chars} chars`); break; case "find": - case "rgrep": + case "glob": parts.push(`${data.count} match${(data.count as number) !== 1 ? "es" : ""}`); if (data.pattern) parts.push(`for "${truncate(String(data.pattern), 20, theme.format.ellipsis)}"`); break; @@ -365,12 +368,34 @@ function formatStatusEvent(event: PythonStatusEvent, theme: Theme): string { parts.push(`${data.count} match${(data.count as number) !== 1 ? "es" : ""}`); if (data.path) parts.push(`in ${shortenPath(String(data.path))}`); break; + case "rgrep": + parts.push(`${data.count} match${(data.count as number) !== 1 ? "es" : ""}`); + if (data.pattern) parts.push(`for "${truncate(String(data.pattern), 20, theme.format.ellipsis)}"`); + break; case "ls": parts.push(`${data.count} entr${(data.count as number) !== 1 ? "ies" : "y"}`); break; + case "env": + if (data.action === "set") { + parts.push(`set ${data.key}=${truncate(String(data.value ?? ""), 30, theme.format.ellipsis)}`); + } else if (data.action === "get") { + parts.push(`${data.key}=${truncate(String(data.value ?? ""), 30, theme.format.ellipsis)}`); + } else { + parts.push(`${data.count} variable${(data.count as number) !== 1 ? "s" : ""}`); + } + break; + case "stat": + if (data.is_dir) { + parts.push("directory"); + } else { + parts.push(`${data.size} bytes`); + } + if (data.path) parts.push(shortenPath(String(data.path))); + break; case "replace": case "sed": parts.push(`${data.count} replacement${(data.count as number) !== 1 ? "s" : ""}`); + if (data.path) parts.push(`in ${shortenPath(String(data.path))}`); break; case "rsed": parts.push(`${data.count} replacement${(data.count as number) !== 1 ? "s" : ""}`); @@ -419,6 +444,18 @@ function formatStatusEvent(event: PythonStatusEvent, theme: Theme): string { case "insert_at": parts.push(`${data.lines_inserted} line${(data.lines_inserted as number) !== 1 ? "s" : ""} inserted`); break; + case "cd": + case "pwd": + case "mkdir": + case "touch": + if (data.path) parts.push(shortenPath(String(data.path))); + break; + case "rm": + case "mv": + case "cp": + if (data.src) parts.push(`${shortenPath(String(data.src))} → ${shortenPath(String(data.dst))}`); + else if (data.path) parts.push(shortenPath(String(data.path))); + break; default: // Generic formatting for other operations if (data.count !== undefined) { @@ -433,6 +470,102 @@ function formatStatusEvent(event: PythonStatusEvent, theme: Theme): string { return `${icon} ${theme.fg("muted", op)}${desc ? ` ${theme.fg("dim", desc)}` : ""}`; } +/** Format status event with expanded detail lines. */ +function formatStatusEventExpanded(event: PythonStatusEvent, theme: Theme): string[] { + const lines: string[] = []; + const { op, ...data } = event; + + // Main status line + lines.push(formatStatusEvent(event, theme)); + + // Add detail lines for operations with list data + const addItems = (items: unknown[], formatter: (item: unknown) => string, max = 5) => { + const arr = Array.isArray(items) ? items : []; + for (let i = 0; i < Math.min(arr.length, max); i++) { + lines.push(` ${theme.fg("dim", formatter(arr[i]))}`); + } + if (arr.length > max) { + lines.push(` ${theme.fg("dim", `${theme.format.ellipsis} ${arr.length - max} more`)}`); + } + }; + + // Add preview lines (truncated content) + const addPreview = (preview: string, maxLines = 3) => { + const previewLines = String(preview).split("\n").slice(0, maxLines); + for (const line of previewLines) { + lines.push(` ${theme.fg("toolOutput", truncate(line, 80, theme.format.ellipsis))}`); + } + const totalLines = String(preview).split("\n").length; + if (totalLines > maxLines) { + lines.push(` ${theme.fg("dim", `${theme.format.ellipsis} ${totalLines - maxLines} more lines`)}`); + } + }; + + switch (op) { + case "find": + case "glob": + if (data.matches) addItems(data.matches as unknown[], (m) => String(m)); + break; + case "ls": + if (data.items) addItems(data.items as unknown[], (m) => String(m)); + break; + case "grep": + if (data.hits) { + addItems(data.hits as unknown[], (h) => { + const hit = h as { line: number; text: string }; + return `${hit.line}: ${truncate(hit.text, 60, theme.format.ellipsis)}`; + }); + } + break; + case "rgrep": + if (data.hits) { + addItems(data.hits as unknown[], (h) => { + const hit = h as { file: string; line: number; text: string }; + return `${shortenPath(hit.file)}:${hit.line}: ${truncate(hit.text, 50, theme.format.ellipsis)}`; + }); + } + break; + case "rsed": + if (data.changed) { + addItems(data.changed as unknown[], (c) => { + const change = c as { file: string; count: number }; + return `${shortenPath(change.file)}: ${change.count} replacement${change.count !== 1 ? "s" : ""}`; + }); + } + break; + case "env": + if (data.keys) addItems(data.keys as unknown[], (k) => String(k), 10); + break; + case "git_log": + if (data.entries) { + addItems(data.entries as unknown[], (e) => { + const entry = e as { sha: string; subject: string }; + return `${entry.sha} ${truncate(entry.subject, 50, theme.format.ellipsis)}`; + }); + } + break; + case "git_status": + if (data.files) addItems(data.files as unknown[], (f) => String(f)); + break; + case "git_branch": + if (data.branches) addItems(data.branches as unknown[], (b) => String(b)); + break; + case "read": + case "cat": + case "head": + case "tail": + case "tree": + case "diff": + case "lines": + case "git_diff": + case "sh": + if (data.preview) addPreview(String(data.preview)); + break; + } + + return lines; +} + /** Render status events as tree lines. */ function renderStatusEvents(events: PythonStatusEvent[], theme: Theme, expanded: boolean): string[] { if (events.length === 0) return []; @@ -445,7 +578,18 @@ function renderStatusEvents(events: PythonStatusEvent[], theme: Theme, expanded: for (let i = 0; i < displayCount; i++) { const isLast = i === displayCount - 1 && (expanded || events.length <= maxCollapsed); const branch = isLast ? theme.tree.last : theme.tree.branch; - lines.push(`${theme.fg("dim", branch)} ${formatStatusEvent(events[i], theme)}`); + + if (expanded) { + // Show expanded details for each event + const eventLines = formatStatusEventExpanded(events[i], theme); + lines.push(`${theme.fg("dim", branch)} ${eventLines[0]}`); + const continueBranch = isLast ? " " : `${theme.tree.vertical} `; + for (let j = 1; j < eventLines.length; j++) { + lines.push(`${theme.fg("dim", continueBranch)}${eventLines[j]}`); + } + } else { + lines.push(`${theme.fg("dim", branch)} ${formatStatusEvent(events[i], theme)}`); + } } if (!expanded && events.length > maxCollapsed) { diff --git a/packages/coding-agent/src/core/tools/task/executor.ts b/packages/coding-agent/src/core/tools/task/executor.ts index 9f3255db8..99779100a 100644 --- a/packages/coding-agent/src/core/tools/task/executor.ts +++ b/packages/coding-agent/src/core/tools/task/executor.ts @@ -29,6 +29,7 @@ import { import type { MCPToolCallRequest, MCPToolMetadata, + PythonToolCallCancel, PythonToolCallRequest, SubagentWorkerRequest, SubagentWorkerResponse, @@ -60,6 +61,7 @@ export interface ExecutorOptions { serialize: () => import("../../settings-manager").Settings; getPythonToolMode?: () => "ipy-only" | "bash-only" | "both"; getPythonKernelMode?: () => "session" | "per-call"; + getPythonSharedGateway?: () => boolean; }; } @@ -295,8 +297,7 @@ export async function runSubprocess(options: ExecutorOptions): Promise) => void) | null = null; const listenerController = new AbortController(); const listenerSignal = listenerController.signal; + const withTimeout = async (promise: Promise, timeoutMs?: number): Promise => { + if (timeoutMs === undefined) return promise; + let timeoutId: ReturnType | undefined; + try { + return await Promise.race([ + promise, + new Promise((_resolve, reject) => { + timeoutId = setTimeout(() => { + reject(new Error(`Tool call timed out after ${timeoutMs}ms`)); + }, timeoutMs); + }), + ]); + } finally { + if (timeoutId) clearTimeout(timeoutId); + } + }; + const combineSignals = (signals: Array): AbortSignal | undefined => { + const filtered = signals.filter((value): value is AbortSignal => Boolean(value)); + if (filtered.length === 0) return undefined; + if (filtered.length === 1) return filtered[0]; + return AbortSignal.any(filtered); + }; + + const createTimeoutSignal = (timeoutMs?: number): AbortSignal | undefined => { + if (typeof timeoutMs !== "number" || !Number.isFinite(timeoutMs) || timeoutMs <= 0) { + return undefined; + } + return AbortSignal.timeout(timeoutMs); + }; + + const pythonSessionFile = sessionFile ?? `subtask:${taskId}`; const pythonToolSession: ToolSession = { cwd, hasUI: false, enableLsp: false, - getSessionFile: () => parentSessionFile, + getSessionFile: () => pythonSessionFile, getSessionSpawns: () => spawnsEnv, settings: options.settingsManager as ToolSession["settings"], settingsManager: options.settingsManager, }; const pythonTool = pythonProxyEnabled ? createPythonTool(pythonToolSession) : null; + const pythonCallControllers = new Map(); // Accumulate usage incrementally from message_end events (no memory for streaming events) const accumulatedUsage = { @@ -400,6 +433,10 @@ export async function runSubprocess(options: ExecutorOptions): Promise { + const timeout = params.timeout; + if (typeof timeout === "number" && Number.isFinite(timeout) && timeout > 0) { + return Math.max(1000, Math.round(timeout * 1000) + 1000); + } + return undefined; + }; + const handlePythonCall = async (request: PythonToolCallRequest) => { if (!pythonTool) { worker.postMessage({ @@ -707,11 +752,16 @@ export async function runSubprocess(options: ExecutorOptions): Promise { + const controller = pythonCallControllers.get(request.callId); + if (controller) { + controller.abort(); } }; @@ -738,6 +803,10 @@ export async function runSubprocess(options: ExecutorOptions): Promise; + timeoutMs?: number; } /** @@ -43,6 +44,7 @@ export interface PythonToolCallRequest { type: "python_tool_call"; callId: string; params: Record; + timeoutMs?: number; } export interface PythonToolCallResponse { @@ -56,6 +58,12 @@ export interface PythonToolCallResponse { error?: string; } +export interface PythonToolCallCancel { + type: "python_tool_cancel"; + callId: string; + reason?: string; +} + export interface SubagentWorkerStartPayload { cwd: string; task: string; @@ -78,10 +86,12 @@ export type SubagentWorkerRequest = | { type: "start"; payload: SubagentWorkerStartPayload } | { type: "abort" } | MCPToolCallResponse - | PythonToolCallResponse; + | PythonToolCallResponse + | PythonToolCallCancel; export type SubagentWorkerResponse = | { type: "event"; event: AgentEvent } | { type: "done"; exitCode: number; durationMs: number; error?: string; aborted?: boolean } | MCPToolCallRequest - | PythonToolCallRequest; + | PythonToolCallRequest + | PythonToolCallCancel; diff --git a/packages/coding-agent/src/core/tools/task/worker.ts b/packages/coding-agent/src/core/tools/task/worker.ts index e21d03310..2d0a2bb4a 100644 --- a/packages/coding-agent/src/core/tools/task/worker.ts +++ b/packages/coding-agent/src/core/tools/task/worker.ts @@ -19,6 +19,7 @@ import type { TSchema } from "@sinclair/typebox"; import type { AgentSessionEvent } from "../../agent-session"; import { AuthStorage } from "../../auth-storage"; import type { CustomTool } from "../../custom-tools/types"; +import { logger } from "../../logger"; import { ModelRegistry } from "../../model-registry"; import { parseModelPattern, parseModelString } from "../../model-resolver"; import { createAgentSession, discoverAuthStorage, discoverModels } from "../../sdk"; @@ -54,13 +55,12 @@ interface PendingMCPCall { interface PendingPythonCall { resolve: (result: PythonToolCallResponse["result"]) => void; reject: (error: Error) => void; - timeoutId: ReturnType; + timeoutId?: ReturnType; } const pendingMCPCalls = new Map(); const pendingPythonCalls = new Map(); const MCP_CALL_TIMEOUT_MS = 60_000; -const PYTHON_CALL_TIMEOUT_MS = 300_000; let mcpCallIdCounter = 0; let pythonCallIdCounter = 0; @@ -123,6 +123,7 @@ function callMCPToolViaParent( callId, toolName, params, + timeoutMs, } as SubagentWorkerResponse); }); } @@ -130,7 +131,7 @@ function callMCPToolViaParent( function callPythonToolViaParent( params: PythonToolParams, signal?: AbortSignal, - timeoutMs = PYTHON_CALL_TIMEOUT_MS, + timeoutMs?: number, ): Promise { return new Promise((resolve, reject) => { const callId = generatePythonCallId(); @@ -139,13 +140,23 @@ function callPythonToolViaParent( return; } - const timeoutId = setTimeout(() => { - pendingPythonCalls.delete(callId); - reject(new Error(`Python call timed out after ${timeoutMs}ms`)); - }, timeoutMs); + const sendCancel = (reason: string) => { + postMessageSafe({ type: "python_tool_cancel", callId, reason } as SubagentWorkerResponse); + }; + + const timeoutId = + typeof timeoutMs === "number" && Number.isFinite(timeoutMs) + ? setTimeout(() => { + pendingPythonCalls.delete(callId); + sendCancel(`Python call timed out after ${timeoutMs}ms`); + reject(new Error(`Python call timed out after ${timeoutMs}ms`)); + }, timeoutMs) + : undefined; const cleanup = () => { - clearTimeout(timeoutId); + if (timeoutId) { + clearTimeout(timeoutId); + } pendingPythonCalls.delete(callId); }; @@ -154,6 +165,7 @@ function callPythonToolViaParent( "abort", () => { cleanup(); + sendCancel("Aborted"); reject(new Error("Aborted")); }, { once: true }, @@ -176,6 +188,7 @@ function callPythonToolViaParent( type: "python_tool_call", callId, params, + timeoutMs, } as SubagentWorkerResponse); }); } @@ -200,6 +213,22 @@ function handlePythonToolResult(response: PythonToolCallResponse): void { } } +function rejectPendingCalls(reason: string): void { + const error = new Error(reason); + const mcpCalls = Array.from(pendingMCPCalls.values()); + const pythonCalls = Array.from(pendingPythonCalls.values()); + pendingMCPCalls.clear(); + pendingPythonCalls.clear(); + for (const pending of mcpCalls) { + clearTimeout(pending.timeoutId); + pending.reject(error); + } + for (const pending of pythonCalls) { + clearTimeout(pending.timeoutId); + pending.reject(error); + } +} + function createMCPProxyTool(metadata: MCPToolMetadata): CustomTool { return { name: metadata.name, @@ -237,12 +266,12 @@ function createMCPProxyTool(metadata: MCPToolMetadata): CustomTool { }; } -function getPythonCallTimeoutMs(params: PythonToolParams): number { +function getPythonCallTimeoutMs(params: PythonToolParams): number | undefined { const timeout = params.timeout; if (typeof timeout === "number" && Number.isFinite(timeout) && timeout > 0) { - return timeout * 1000 + 5000; + return Math.max(1000, Math.round(timeout * 1000) + 1000); } - return PYTHON_CALL_TIMEOUT_MS; + return undefined; } function createPythonProxyTool(): CustomTool { @@ -463,12 +492,16 @@ async function runTask(runState: RunState, payload: SubagentWorkerStartPayload): { sendMessage: (message, options) => { session.sendCustomMessage(message, options).catch((e) => { - console.error(`Extension sendMessage failed: ${e instanceof Error ? e.message : String(e)}`); + logger.error("Extension sendMessage failed", { + error: e instanceof Error ? e.message : String(e), + }); }); }, sendUserMessage: (content, options) => { session.sendUserMessage(content, options).catch((e) => { - console.error(`Extension sendUserMessage failed: ${e instanceof Error ? e.message : String(e)}`); + logger.error("Extension sendUserMessage failed", { + error: e instanceof Error ? e.message : String(e), + }); }); }, appendEntry: (customType, data) => { @@ -508,7 +541,7 @@ async function runTask(runState: RunState, payload: SubagentWorkerStartPayload): }, ); extensionRunner.onError((err) => { - console.error(`Extension error (${err.extensionPath}): ${err.error}`); + logger.error("Extension error", { path: err.extensionPath, error: err.error }); }); await extensionRunner.emit({ type: "session_start" }); } @@ -570,6 +603,7 @@ Call complete now.`; } sessionAbortController.abort(); + rejectPendingCalls("Worker finished"); if (runState.unsubscribe) { try { @@ -611,8 +645,10 @@ function handleAbort(): void { const runState = activeRun; if (!runState) { pendingAbort = true; + rejectPendingCalls("Aborted"); return; } + rejectPendingCalls("Aborted"); runState.abortController.abort(); if (runState.session) { void runState.session.abort(); diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index 135c96a44..56bb70421 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -31,6 +31,7 @@ import { resolvePromptInput } from "./core/system-prompt"; import { printTimings, time } from "./core/timings"; import { initializeWithSettings } from "./discovery"; import { runMigrations, showDeprecationWarnings } from "./migrations"; +import { runAsyncCleanup } from "./modes/cleanup"; import { InteractiveMode, installTerminalCrashHandlers, runPrintMode, runRpcMode } from "./modes/index"; import { initTheme, stopThemeWatcher } from "./modes/interactive/theme/theme"; import { getChangelogPath, getNewEntries, parseChangelog } from "./utils/changelog"; @@ -731,7 +732,9 @@ export async function main(args: string[]) { initialMessage, initialImages, }); + await session.dispose(); stopThemeWatcher(); + await runAsyncCleanup(); if (process.stdout.writableLength > 0) { await new Promise((resolve) => process.stdout.once("drain", resolve)); } diff --git a/packages/coding-agent/src/modes/interactive/components/settings-defs.ts b/packages/coding-agent/src/modes/interactive/components/settings-defs.ts index 60499e5f9..321ecba98 100644 --- a/packages/coding-agent/src/modes/interactive/components/settings-defs.ts +++ b/packages/coding-agent/src/modes/interactive/components/settings-defs.ts @@ -269,6 +269,15 @@ export const SETTINGS_DEFS: SettingDef[] = [ get: (sm) => sm.getPythonKernelMode(), set: (sm, v) => sm.setPythonKernelMode(v as PythonKernelMode), }, + { + id: "pythonSharedGateway", + tab: "tools", + type: "boolean", + label: "Python shared gateway", + description: "Share IPython kernel gateway across pi instances", + get: (sm) => sm.getPythonSharedGateway(), + set: (sm, v) => sm.setPythonSharedGateway(v), + }, { id: "editFuzzyMatch", tab: "tools", diff --git a/packages/coding-agent/src/modes/interactive/controllers/command-controller.ts b/packages/coding-agent/src/modes/interactive/controllers/command-controller.ts index 791d9f9eb..963261aac 100644 --- a/packages/coding-agent/src/modes/interactive/controllers/command-controller.ts +++ b/packages/coding-agent/src/modes/interactive/controllers/command-controller.ts @@ -174,41 +174,38 @@ export class CommandController { }; try { - const result = await new Promise<{ stdout: string; stderr: string; code: number | null }>((resolve) => { - proc = Bun.spawn(["gh", "gist", "create", "--public=false", tmpFile], { - stdout: "pipe", - stderr: "pipe", - }); - let stdout = ""; - let stderr = ""; - - const stdoutReader = (proc.stdout as ReadableStream).getReader(); - const stderrReader = (proc.stderr as ReadableStream).getReader(); - const decoder = new TextDecoder(); - - (async () => { - try { - while (true) { - const { done, value } = await stdoutReader.read(); - if (done) break; - stdout += decoder.decode(value); - } - } catch {} - })(); - - (async () => { - try { - while (true) { - const { done, value } = await stderrReader.read(); - if (done) break; - stderr += decoder.decode(value); - } - } catch {} - })(); - - proc.exited.then((code) => resolve({ stdout, stderr, code })); + proc = Bun.spawn(["gh", "gist", "create", "--public=false", tmpFile], { + stdout: "pipe", + stderr: "pipe", }); + const readStream = async (stream: ReadableStream | null): Promise => { + if (!stream) return ""; + const reader = stream.getReader(); + const decoder = new TextDecoder(); + let output = ""; + try { + while (true) { + const { done, value } = await reader.read(); + if (done) break; + output += decoder.decode(value, { stream: true }); + } + } catch { + // Ignore read errors + } finally { + output += decoder.decode(); + reader.releaseLock(); + } + return output; + }; + + const [stdout, stderr, code] = await Promise.all([ + readStream(proc.stdout as ReadableStream | null), + readStream(proc.stderr as ReadableStream | null), + proc.exited.catch(() => 1), + ]); + const result = { stdout, stderr, code }; + if (loader.signal.aborted) return; restoreEditor(); diff --git a/packages/coding-agent/src/modes/print-mode.ts b/packages/coding-agent/src/modes/print-mode.ts index a628b947d..c012dae00 100644 --- a/packages/coding-agent/src/modes/print-mode.ts +++ b/packages/coding-agent/src/modes/print-mode.ts @@ -178,4 +178,6 @@ export async function runPrintMode(session: AgentSession, options: PrintModeOpti else resolve(); }); }); + + await session.dispose(); } diff --git a/packages/coding-agent/src/prompts/agents/explore.md b/packages/coding-agent/src/prompts/agents/explore.md index 2db72ffd4..f13b0df2b 100644 --- a/packages/coding-agent/src/prompts/agents/explore.md +++ b/packages/coding-agent/src/prompts/agents/explore.md @@ -1,7 +1,7 @@ --- name: explore description: Fast read-only codebase scout that returns compressed context for handoff -tools: read, grep, find, ls, exec +tools: read, grep, find, ls, bash model: pi/smol, haiku, flash, mini --- @@ -29,7 +29,7 @@ Guidelines: - Use find for broad file pattern matching - Use grep for searching file contents with regex - Use read when you know the specific file path -- Use exec ONLY for git status/log/diff; use read/grep/find/ls tools for file and search operations +- Use bash ONLY for git status/log/diff; use read/grep/find/ls tools for file and search operations - Spawn multiple parallel tool calls wherever possible—you are meant to be fast - Return file paths as absolute paths in your final response - Communicate findings directly as a message—do NOT create output files diff --git a/packages/coding-agent/src/prompts/agents/plan.md b/packages/coding-agent/src/prompts/agents/plan.md index 7e16be0a3..000fbe8af 100644 --- a/packages/coding-agent/src/prompts/agents/plan.md +++ b/packages/coding-agent/src/prompts/agents/plan.md @@ -1,7 +1,7 @@ --- name: plan description: Software architect for complex multi-file architectural decisions. NOT for simple tasks, single-file changes, reasoning, or tasks completable in <5 tool calls—execute those directly. -tools: read, grep, find, ls, exec +tools: read, grep, find, ls, bash spawns: explore model: pi/slow, gpt-5.2-codex, gpt-5.2, codex, gpt --- @@ -14,7 +14,7 @@ You are STRICTLY PROHIBITED from: - Creating temporary files anywhere, including /tmp - Using redirect operators (>, >>, |) or heredocs to write files - Running commands that change system state (git add, git commit, npm install, pip install) -- Use exec ONLY for git status/log/diff; use read/grep/find/ls tools for file and search operations +- Use bash ONLY for git status/log/diff; use read/grep/find/ls tools for file and search operations Another engineer will execute your plan without re-exploring the codebase. Your plan must be specific enough to implement directly. diff --git a/packages/coding-agent/src/prompts/agents/reviewer.md b/packages/coding-agent/src/prompts/agents/reviewer.md index 078d91555..e2e036c41 100644 --- a/packages/coding-agent/src/prompts/agents/reviewer.md +++ b/packages/coding-agent/src/prompts/agents/reviewer.md @@ -1,7 +1,7 @@ --- name: reviewer description: Code review specialist for quality and security analysis -tools: read, grep, find, ls, exec, report_finding +tools: read, grep, find, ls, bash, report_finding spawns: explore, task model: pi/slow, gpt-5.2-codex, gpt-5.2, codex, gpt output: @@ -43,7 +43,7 @@ You are a senior engineer reviewing a proposed code change. Your goal: identify 4. Call `report_finding` for each issue 5. Call `complete` with your verdict — **review is incomplete until `complete` is called** -Exec is read-only: `git diff`, `git log`, `git show`, `gh pr diff`. No file modifications or builds. +Bash is read-only here: `git diff`, `git log`, `git show`, `gh pr diff`. No file modifications or builds. # What to Flag diff --git a/packages/coding-agent/src/prompts/agents/task.md b/packages/coding-agent/src/prompts/agents/task.md index 7d48cefdc..6a60cb57a 100644 --- a/packages/coding-agent/src/prompts/agents/task.md +++ b/packages/coding-agent/src/prompts/agents/task.md @@ -1,4 +1,4 @@ -You are a worker agent for delegated tasks. You have FULL access to all tools (edit, write, exec, grep, read, etc.) - use them as needed to complete your task. +You are a worker agent for delegated tasks. You have FULL access to all tools (edit, write, bash, grep, read, etc.) - use them as needed to complete your task. Finish only the assigned work and return the minimum useful result. diff --git a/packages/coding-agent/src/prompts/tools/python.md b/packages/coding-agent/src/prompts/tools/python.md index 36e7ceb7a..02b418839 100644 --- a/packages/coding-agent/src/prompts/tools/python.md +++ b/packages/coding-agent/src/prompts/tools/python.md @@ -1,4 +1,4 @@ -Executes Python code in a persistent IPython kernel with optional timeout. +Executes Python code in an IPython kernel (session or per-call) with optional timeout. ## When to use Python @@ -40,9 +40,12 @@ All helpers auto-print results and return values for chaining. {{#if categories.length}} {{#each categories}} ### {{name}} +``` {{#each functions}} -- `{{name}}{{signature}}` — {{docstring}} +{{name}}{{signature}} + {{docstring}} {{/each}} +``` {{/each}} {{else}} @@ -71,8 +74,18 @@ cols(read("data.tsv"), 0, 2, sep="\t") ## Notes - Code executes as IPython cells; users see the full cell output (including rendered figures, tables, etc.) -- Kernel persists for the session; use `reset: true` to clear state +- Kernel persists for the session by default; per-call mode uses a fresh kernel each call. Use `reset: true` to clear state when session mode is active - Use `workdir` parameter instead of `os.chdir()` in tool call - Use `plt.show()` to display figures - Use `display()` from IPython.display for rich output (HTML, Markdown, images, etc.) - Output streams in real time, truncated after 50KB + +## Rich output rendering + +The user sees output like a Jupyter notebook—rich displays are fully rendered: +- `display(JSON(data))` → interactive JSON tree +- `display(HTML(...))` → rendered HTML +- `display(Markdown(...))` → formatted markdown +- `plt.show()` → inline figures + +**You will see object repr** (e.g., ``) **but the user sees the rendered output.** Trust that `display()` calls work correctly—do not assume the user sees only the repr. diff --git a/packages/coding-agent/test/python-tool-settings.test.ts b/packages/coding-agent/test/python-tool-settings.test.ts index e9040dca8..df0db8151 100644 --- a/packages/coding-agent/test/python-tool-settings.test.ts +++ b/packages/coding-agent/test/python-tool-settings.test.ts @@ -20,6 +20,7 @@ function createSettings(overrides?: Partial): ToolSessi getBashInterceptorRules: () => [], getPythonToolMode: () => "ipy-only", getPythonKernelMode: () => "session", + getPythonSharedGateway: () => true, ...overrides, }; } @@ -81,7 +82,10 @@ describe("python tool settings", () => { expect(executeSpy).toHaveBeenCalledWith( "print(1)", - expect.objectContaining({ kernelMode: "per-call", sessionId: "session.json" }), + expect.objectContaining({ + kernelMode: "per-call", + sessionId: `session:session.json:workdir:${testDir}`, + }), ); }); });