From d676150f9ca304a88253eb28faf94c41a4db81ed Mon Sep 17 00:00:00 2001 From: can1357 Date: Fri, 23 Jan 2026 09:17:24 +0100 Subject: [PATCH] refactor(coding-agent/core): simplified Python gateway coordination by removing reference counting - Simplified Python gateway coordination by removing reference counting and idle timeout functionality. - Updated Python kernel initialization to support environment variable setup and working directory configuration. - Modified interactive status display to show Python and venv paths instead of client counts. - Updated system prompt to clarify CHECKPOINT step 0 timing after user request completion. --- packages/coding-agent/CHANGELOG.md | 9 + .../src/core/python-gateway-coordinator.ts | 362 ++---------------- .../coding-agent/src/core/python-kernel.ts | 36 +- .../controllers/command-controller.ts | 10 +- .../src/prompts/system/system-prompt.md | 21 +- .../coding-agent/src/prompts/tools/task.md | 2 + .../test/core/python-kernel.test.ts | 25 ++ .../test/core/python-prelude.test.ts | 1 - .../test/core/system-prompt.python.test.ts | 2 +- .../coding-agent/test/tools/python.test.ts | 2 +- 10 files changed, 122 insertions(+), 348 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index e0ff21e29..508cf7676 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## [Unreleased] + ### Added - Added artifact storage system for truncated tool outputs with artifact:// URL protocol @@ -18,6 +19,11 @@ ### Changed +- Simplified Python gateway coordination by removing reference counting and client tracking +- Updated Python gateway to use global shared instance instead of per-process coordination +- Modified Python kernel initialization to set working directory and environment per kernel +- Updated interactive status display to show Python and venv paths instead of client count +- Changed system prompt to clarify CHECKPOINT step 0 timing - Updated Python environment warming to use await instead of void for proper error handling - Updated interactive mode shutdown to use postmortem.quit instead of process.exit - Updated bash tool documentation to clarify specialized tool usage @@ -53,11 +59,14 @@ ### Fixed +- Fixed Python kernel environment initialization for external and shared gateways +- Fixed gateway status reporting to include Python and virtual environment paths - Fixed inconsistent error formatting across tools by standardizing on ToolError types - Fixed timeout parameter handling to auto-convert milliseconds to seconds and clamp to reasonable ranges - Fixed whitespace formatting in json-query.ts comment - Fixed interactive shutdown to await postmortem cleanup so Python kernel gateways are terminated - Fixed shared Python gateway reuse across working directories by initializing kernel cwd and env per kernel +- Fixed Python gateway coordination to use a single global gateway without ref counting ## [7.0.0] - 2026-01-21 ### Added diff --git a/packages/coding-agent/src/core/python-gateway-coordinator.ts b/packages/coding-agent/src/core/python-gateway-coordinator.ts index af991d675..86139637d 100644 --- a/packages/coding-agent/src/core/python-gateway-coordinator.ts +++ b/packages/coding-agent/src/core/python-gateway-coordinator.ts @@ -3,7 +3,6 @@ import { existsSync, mkdirSync, openSync, - readdirSync, readFileSync, renameSync, statSync, @@ -13,7 +12,7 @@ import { } from "node:fs"; import { createServer } from "node:net"; import { delimiter, join } from "node:path"; -import { logger, postmortem } from "@oh-my-pi/pi-utils"; +import { logger } from "@oh-my-pi/pi-utils"; import type { Subprocess } from "bun"; import { getAgentDir } from "../config"; import { getShellConfig, killProcessTree } from "../utils/shell"; @@ -22,9 +21,7 @@ import { getOrCreateSnapshot } from "../utils/shell-snapshot"; 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 = GATEWAY_STARTUP_TIMEOUT_MS + 5000; const GATEWAY_LOCK_RETRY_MS = 50; const GATEWAY_LOCK_STALE_MS = GATEWAY_STARTUP_TIMEOUT_MS * 2; @@ -140,8 +137,6 @@ export interface GatewayInfo { url: string; pid: number; startedAt: number; - refCount: number; - cwd: string; pythonPath?: string; venvPath?: string | null; } @@ -158,61 +153,7 @@ interface AcquireResult { let localGatewayProcess: Subprocess | null = null; let localGatewayUrl: string | null = null; -let idleShutdownTimer: ReturnType | null = null; let isCoordinatorInitialized = false; -let localClientFile: string | null = null; -let postmortemRegistered = false; - -/** - * Register cleanup handler for process exit. Called lazily on first gateway acquisition. - * Ensures the gateway process we spawned is killed when omp exits, preventing orphaned processes. - */ -function ensurePostmortemCleanup(): void { - if (postmortemRegistered) return; - postmortemRegistered = true; - - postmortem.register("shared-gateway", async () => { - cancelIdleShutdown(); - - // Clean up our client file first so refcount is accurate - if (localClientFile) { - try { - unlinkSync(localClientFile); - } catch { - // Ignore cleanup errors - } - localClientFile = null; - } - - // If we spawned the gateway, kill it only if no other clients remain - if (localGatewayProcess) { - const clients = pruneStaleClientInfos(listClientInfos()); - const remainingRefs = clients.reduce((sum, c) => sum + c.info.refCount, 0); - - if (remainingRefs === 0) { - logger.debug("Cleaning up shared gateway on process exit", { pid: localGatewayProcess.pid }); - try { - await killProcessTree(localGatewayProcess.pid); - } catch (err) { - logger.warn("Failed to kill shared gateway on exit", { - error: err instanceof Error ? err.message : String(err), - }); - } - clearGatewayInfo(); - } else { - logger.debug("Leaving shared gateway running for other clients", { - pid: localGatewayProcess.pid, - remainingRefs, - }); - } - - localGatewayProcess = null; - localGatewayUrl = null; - } - - isCoordinatorInitialized = false; - }); -} function filterEnv(env: Record): Record { const filtered: Record = {}; @@ -398,6 +339,7 @@ async function withGatewayLock(handler: () => Promise): Promise { function readGatewayInfo(): GatewayInfo | null { const infoPath = getGatewayInfoPath(); if (!existsSync(infoPath)) return null; + try { const content = readFileSync(infoPath, "utf-8"); const parsed = JSON.parse(content) as Partial; @@ -405,16 +347,10 @@ function readGatewayInfo(): GatewayInfo | 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, }; @@ -450,103 +386,6 @@ 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(); @@ -585,11 +424,6 @@ async function startGatewayProcess( OMP_SHELL_SNAPSHOT: snapshotPath ?? undefined, }; - const pythonPathParts = [cwd, kernelEnv.PYTHONPATH].filter(Boolean).join(delimiter); - if (pythonPathParts) { - kernelEnv.PYTHONPATH = pythonPathParts; - } - const gatewayPort = await allocatePort(); const gatewayUrl = `http://127.0.0.1:${gatewayPort}`; @@ -622,7 +456,6 @@ async function startGatewayProcess( exited = true; }); - // Wait for gateway to become healthy const startTime = Date.now(); while (Date.now() - startTime < GATEWAY_STARTUP_TIMEOUT_MS) { if (exited) { @@ -645,70 +478,15 @@ async function startGatewayProcess( throw new Error("Gateway startup timeout"); } -function scheduleIdleShutdown(): void { - if (idleShutdownTimer) { - clearTimeout(idleShutdownTimer); - } - idleShutdownTimer = setTimeout(async () => { - try { - await withGatewayLock(async () => { - const info = readGatewayInfo(); - 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) { - await shutdownLocalGateway(); - } else if (isPidRunning(info.pid)) { - try { - await 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", { - error: err instanceof Error ? err.message : String(err), - }); - } finally { - idleShutdownTimer = null; - } - }, GATEWAY_IDLE_TIMEOUT_MS); -} - -function cancelIdleShutdown(): void { - if (idleShutdownTimer) { - clearTimeout(idleShutdownTimer); - idleShutdownTimer = null; - } -} - -async function shutdownLocalGateway(): Promise { - if (localGatewayProcess) { - try { - await killProcessTree(localGatewayProcess.pid); - } catch (err) { - logger.warn("Failed to kill shared gateway process", { - error: err instanceof Error ? err.message : String(err), - }); - } - localGatewayProcess = null; - localGatewayUrl = null; +async function killGateway(pid: number, context: string): Promise { + try { + await killProcessTree(pid); + } catch (err) { + logger.warn("Failed to kill shared gateway process", { + error: err instanceof Error ? err.message : String(err), + pid, + context, + }); } } @@ -717,72 +495,35 @@ export async function acquireSharedGateway(cwd: string): Promise { const existingInfo = readGatewayInfo(); - if (existingInfo && (await isGatewayAlive(existingInfo))) { - const { env } = await getShellConfig(); - const filteredEnv = filterEnv(env); - const runtime = await resolvePythonRuntime(cwd, filteredEnv); - const existingVenv = existingInfo.venvPath ?? null; - const runtimeVenv = runtime.venvPath ?? null; - if ( - existingInfo.cwd !== cwd || - !existingInfo.pythonPath || - existingInfo.pythonPath !== runtime.pythonPath || - existingVenv !== runtimeVenv - ) { - logger.debug("Shared gateway metadata mismatch", { - existingCwd: existingInfo.cwd, - requestedCwd: cwd, - existingPython: existingInfo.pythonPath, - runtimePython: runtime.pythonPath, - existingVenv, - runtimeVenv, - }); - return null; - } - const { totalRefCount } = updateLocalClientRefCount(1); - const updatedInfo = { ...existingInfo, refCount: totalRefCount }; - writeGatewayInfo(updatedInfo); - cancelIdleShutdown(); - logger.debug("Reusing shared gateway", { url: existingInfo.url, refCount: updatedInfo.refCount }); - isCoordinatorInitialized = true; - return { url: existingInfo.url, isShared: true }; - } - if (existingInfo) { + if (await isGatewayAlive(existingInfo)) { + localGatewayUrl = existingInfo.url; + isCoordinatorInitialized = true; + logger.debug("Reusing global Python gateway", { url: existingInfo.url }); + return { url: existingInfo.url, isShared: true }; + } + logger.debug("Cleaning up stale gateway info", { pid: existingInfo.pid }); if (isPidRunning(existingInfo.pid)) { - try { - await killProcessTree(existingInfo.pid); - } catch (err) { - logger.warn("Failed to kill stale shared gateway process", { - error: err instanceof Error ? err.message : String(err), - pid: existingInfo.pid, - }); - } + await killGateway(existingInfo.pid, "stale"); } clearGatewayInfo(); - clearClientFiles(); } const { url, pid, pythonPath, venvPath } = await startGatewayProcess(cwd); - const { totalRefCount } = updateLocalClientRefCount(1); const info: GatewayInfo = { url, pid, startedAt: Date.now(), - refCount: totalRefCount, - cwd, pythonPath, venvPath, }; writeGatewayInfo(info); isCoordinatorInitialized = true; - logger.debug("Started shared gateway", { url, pid }); + logger.debug("Started global Python gateway", { url, pid }); return { url, isShared: true }; }); } catch (err) { @@ -795,48 +536,24 @@ export async function acquireSharedGateway(cwd: string): Promise { if (!isCoordinatorInitialized) return; - - try { - await withGatewayLock(async () => { - const { totalRefCount } = updateLocalClientRefCount(-1); - const info = readGatewayInfo(); - if (!info) return; - - const newRefCount = Math.max(0, totalRefCount); - if (newRefCount === 0) { - const updatedInfo = { ...info, refCount: 0 }; - writeGatewayInfo(updatedInfo); - scheduleIdleShutdown(); - logger.debug("Scheduled idle shutdown for shared gateway", { pid: info.pid }); - return; - } - const updatedInfo = { ...info, refCount: newRefCount }; - writeGatewayInfo(updatedInfo); - logger.debug("Released shared gateway reference", { url: info.url, refCount: newRefCount }); - }); - } catch (err) { - logger.warn("Failed to release shared gateway", { - error: err instanceof Error ? err.message : String(err), - }); - } } export function getSharedGatewayUrl(): string | null { - return localGatewayUrl; + if (localGatewayUrl) return localGatewayUrl; + return readGatewayInfo()?.url ?? null; } export function isSharedGatewayActive(): boolean { - return localGatewayProcess !== null && localGatewayUrl !== null; + return getGatewayStatus().active; } export interface GatewayStatus { active: boolean; - shared: boolean; url: string | null; pid: number | null; - refCount: number; - cwd: string | null; uptime: number | null; + pythonPath: string | null; + venvPath: string | null; } export function getGatewayStatus(): GatewayStatus { @@ -844,45 +561,44 @@ export function getGatewayStatus(): GatewayStatus { if (!info) { return { active: false, - shared: false, url: null, pid: null, - refCount: 0, - cwd: null, uptime: null, + pythonPath: null, + venvPath: null, }; } 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 && refCount > 1, url: info.url, pid: info.pid, - refCount, - cwd: info.cwd, - uptime: Date.now() - info.startedAt, + uptime: active ? Date.now() - info.startedAt : null, + pythonPath: info.pythonPath ?? null, + venvPath: info.venvPath ?? null, }; } export async function shutdownSharedGateway(): Promise { - cancelIdleShutdown(); try { await withGatewayLock(async () => { const info = readGatewayInfo(); - if (info) { - clearGatewayInfo(); + if (!info) return; + if (isPidRunning(info.pid)) { + await killGateway(info.pid, "shutdown"); } - clearClientFiles(); + clearGatewayInfo(); }); } catch (err) { logger.warn("Failed to shutdown shared gateway", { error: err instanceof Error ? err.message : String(err), }); } finally { - await shutdownLocalGateway(); + if (localGatewayProcess) { + await killGateway(localGatewayProcess.pid, "shutdown-local"); + } + localGatewayProcess = null; + localGatewayUrl = null; isCoordinatorInitialized = false; } } diff --git a/packages/coding-agent/src/core/python-kernel.ts b/packages/coding-agent/src/core/python-kernel.ts index 3fc3160d4..ed6244796 100644 --- a/packages/coding-agent/src/core/python-kernel.ts +++ b/packages/coding-agent/src/core/python-kernel.ts @@ -511,7 +511,7 @@ export class PythonKernel { const externalConfig = getExternalGatewayConfig(); if (externalConfig) { - return PythonKernel.startWithExternalGateway(externalConfig, options.cwd); + return PythonKernel.startWithExternalGateway(externalConfig, options.cwd, options.env); } // Try shared gateway first (unless explicitly disabled) @@ -519,7 +519,7 @@ export class PythonKernel { try { const sharedResult = await acquireSharedGateway(options.cwd); if (sharedResult) { - return PythonKernel.startWithSharedGateway(sharedResult.url, options.cwd); + return PythonKernel.startWithSharedGateway(sharedResult.url, options.cwd, options.env); } } catch (err) { logger.warn("Failed to acquire shared gateway, falling back to local", { @@ -531,7 +531,11 @@ export class PythonKernel { return PythonKernel.startWithLocalGateway(options); } - private static async startWithExternalGateway(config: ExternalGatewayConfig, cwd: string): Promise { + private static async startWithExternalGateway( + config: ExternalGatewayConfig, + cwd: string, + env?: Record, + ): Promise { const headers: Record = { "Content-Type": "application/json" }; if (config.token) { headers.Authorization = `token ${config.token}`; @@ -554,6 +558,7 @@ export class PythonKernel { try { await kernel.connectWebSocket(); + await kernel.initializeKernelEnvironment(cwd, env); kernel.startHeartbeat(); const preludeResult = await kernel.execute(PYTHON_PRELUDE, { silent: true, storeHistory: false }); if (preludeResult.cancelled || preludeResult.status === "error") { @@ -567,7 +572,11 @@ export class PythonKernel { } } - private static async startWithSharedGateway(gatewayUrl: string, cwd: string): Promise { + private static async startWithSharedGateway( + gatewayUrl: string, + cwd: string, + env?: Record, + ): Promise { const createResponse = await fetch(`${gatewayUrl}/api/kernels`, { method: "POST", headers: { "Content-Type": "application/json" }, @@ -586,6 +595,7 @@ export class PythonKernel { try { await kernel.connectWebSocket(); + await kernel.initializeKernelEnvironment(cwd, env); kernel.startHeartbeat(); const preludeResult = await kernel.execute(PYTHON_PRELUDE, { silent: true, storeHistory: false }); if (preludeResult.cancelled || preludeResult.status === "error") { @@ -702,6 +712,7 @@ export class PythonKernel { try { await kernel.connectWebSocket(); + await kernel.initializeKernelEnvironment(options.cwd, options.env); kernel.startHeartbeat(); const preludeResult = await kernel.execute(PYTHON_PRELUDE, { silent: true, storeHistory: false }); if (preludeResult.cancelled || preludeResult.status === "error") { @@ -802,6 +813,23 @@ export class PythonKernel { return promise; } + private async initializeKernelEnvironment(cwd: string, env?: Record): Promise { + const envEntries = Object.entries(env ?? {}).filter(([, value]) => value !== undefined); + const envPayload = Object.fromEntries(envEntries); + const initScript = [ + "import os, sys", + `__omp_cwd = ${JSON.stringify(cwd)}`, + "os.chdir(__omp_cwd)", + `__omp_env = ${JSON.stringify(envPayload)}`, + "for __omp_key, __omp_val in __omp_env.items():\n os.environ[__omp_key] = __omp_val", + "if __omp_cwd not in sys.path:\n sys.path.insert(0, __omp_cwd)", + ].join("\n"); + const result = await this.execute(initScript, { silent: true, storeHistory: false }); + if (result.cancelled || result.status === "error") { + throw new Error("Failed to initialize Python kernel environment"); + } + } + private abortPendingExecutions(reason: string): void { if (this.#pendingExecutions.size === 0) return; for (const cancel of this.#pendingExecutions.values()) { 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 f8dca6c1b..d09b36fe7 100644 --- a/packages/coding-agent/src/modes/interactive/controllers/command-controller.ts +++ b/packages/coding-agent/src/modes/interactive/controllers/command-controller.ts @@ -243,11 +243,15 @@ export class CommandController { const gateway = getGatewayStatus(); info += `\n${theme.bold("Python Gateway")}\n`; if (gateway.active) { - const mode = gateway.shared ? "Shared" : "Local"; - info += `${theme.fg("dim", "Status:")} ${theme.fg("success", `Active (${mode})`)}\n`; + info += `${theme.fg("dim", "Status:")} ${theme.fg("success", "Active (Global)")}\n`; info += `${theme.fg("dim", "URL:")} ${gateway.url}\n`; info += `${theme.fg("dim", "PID:")} ${gateway.pid}\n`; - info += `${theme.fg("dim", "Clients:")} ${gateway.refCount}\n`; + if (gateway.pythonPath) { + info += `${theme.fg("dim", "Python:")} ${gateway.pythonPath}\n`; + } + if (gateway.venvPath) { + info += `${theme.fg("dim", "Venv:")} ${gateway.venvPath}\n`; + } if (gateway.uptime !== null) { const uptimeSec = Math.floor(gateway.uptime / 1000); const mins = Math.floor(uptimeSec / 60); diff --git a/packages/coding-agent/src/prompts/system/system-prompt.md b/packages/coding-agent/src/prompts/system/system-prompt.md index 7e9141069..635910360 100644 --- a/packages/coding-agent/src/prompts/system/system-prompt.md +++ b/packages/coding-agent/src/prompts/system/system-prompt.md @@ -201,26 +201,17 @@ Do not carry the whole problem in one skull. Split the load. Bring back facts. T ## Before action -0. **CHECKPOINT** — For complex tasks, plan before acting: - ``` - TASK DECOMPOSITION - - Distinct work streams: [list files, subsystems, or questions] - - Dependencies: [which streams need results from others?] +0. **CHECKPOINT** — For complex tasks, mentally evaluate before acting (do not output this evaluation): + - What distinct work streams exist? Which depend on others? {{#has tools "task"}} - - Parallelizable: [yes → Task tool / no → because X] + - Can these run in parallel via Task tool, or must they be sequential? {{/has}} {{#if skills.length}} - SKILL CHECK - - Task domain: [what am I producing? code, prompt, UI, document, design...] - - Skill scan: [match from skills list, or "none"] - - Action: [read skill first / no match] + - Does any skill match this task domain? If so, read it first. {{/if}} {{#if rules.length}} - RULE CHECK - - Matching rule: [from rules list, or "none"] - - Action: [read rule first / no match] + - Does any rule apply? If so, read it first. {{/if}} - ``` Skip for trivial tasks (single file, obvious action). Use judgment. 1. Plan if the task has weight. Three to seven bullets. 2. Before each tool call: state intent in one sentence. @@ -337,7 +328,7 @@ Keep going until finished. - Quote only what is needed. The rest is noise. - Do not write code before stating assumptions. - Do not claim correctness you haven't verified. -- CHECKPOINT step 0 is not optional. Complete it before your first tool call. +- CHECKPOINT step 0 is not optional. Complete it right after the responding to the user's request. {{#has tools "ask"}}- If files differ from expectations, ask before discarding uncommitted work.{{/has}} - Cutting corners, stopping at happy path alone, or worse, incomplete work, means you've failed your partner. - Your hard work is of no value if it will be thrown away once you yield. diff --git a/packages/coding-agent/src/prompts/tools/task.md b/packages/coding-agent/src/prompts/tools/task.md index 799f97ecc..4102dbb51 100644 --- a/packages/coding-agent/src/prompts/tools/task.md +++ b/packages/coding-agent/src/prompts/tools/task.md @@ -57,6 +57,8 @@ Results are keyed by task `id` (e.g., "AuthProvider", "AuthApi"). 3. The `task` string you provide If you discussed requirements, plans, schemas, or decisions with the user, you MUST include that information in `context`. Subagents cannot see prior messages—they start fresh with only what you explicitly pass them. + +**Never call Task multiple times in parallel.** Use a single Task call with multiple entries in the `tasks` array. Parallel Task calls waste resources and bypass coordination. diff --git a/packages/coding-agent/test/core/python-kernel.test.ts b/packages/coding-agent/test/core/python-kernel.test.ts index bad4982c7..249480132 100644 --- a/packages/coding-agent/test/core/python-kernel.test.ts +++ b/packages/coding-agent/test/core/python-kernel.test.ts @@ -182,6 +182,7 @@ describe("PythonKernel (external gateway)", () => { }); globalThis.fetch = fetchMock as unknown as typeof fetch; + let initSeen = false; let preludeSeen = false; const kernelPromise = PythonKernel.start({ cwd: "/" }); @@ -191,6 +192,12 @@ describe("PythonKernel (external gateway)", () => { ws.setSendHandler((data) => { const msg = typeof data === "string" ? (JSON.parse(data) as JupyterMessage) : decodeMessage(data); const code = String(msg.content.code ?? ""); + if (!initSeen) { + // First execution is kernel environment init + initSeen = true; + sendOkExecution(ws, msg.header.msg_id); + return; + } if (!preludeSeen) { expect(code).toBe(PYTHON_PRELUDE); preludeSeen = true; @@ -307,6 +314,7 @@ describe("PythonKernel (external gateway)", () => { }); globalThis.fetch = fetchMock as unknown as typeof fetch; + let initSeen = false; let preludeSeen = false; const kernelPromise = PythonKernel.start({ cwd: "/" }); @@ -316,6 +324,11 @@ describe("PythonKernel (external gateway)", () => { ws.setSendHandler((data) => { const msg = typeof data === "string" ? (JSON.parse(data) as JupyterMessage) : decodeMessage(data); const code = String(msg.content.code ?? ""); + if (!initSeen) { + initSeen = true; + sendOkExecution(ws, msg.header.msg_id); + return; + } if (!preludeSeen) { expect(code).toBe(PYTHON_PRELUDE); preludeSeen = true; @@ -343,6 +356,7 @@ describe("PythonKernel (external gateway)", () => { }); globalThis.fetch = fetchMock as unknown as typeof fetch; + let initSeen = false; let preludeSeen = false; const kernelPromise = PythonKernel.start({ cwd: "/" }); @@ -352,6 +366,11 @@ describe("PythonKernel (external gateway)", () => { ws.setSendHandler((data) => { const msg = typeof data === "string" ? (JSON.parse(data) as JupyterMessage) : decodeMessage(data); const code = String(msg.content.code ?? ""); + if (!initSeen) { + initSeen = true; + sendOkExecution(ws, msg.header.msg_id); + return; + } if (!preludeSeen) { expect(code).toBe(PYTHON_PRELUDE); preludeSeen = true; @@ -383,6 +402,7 @@ describe("PythonKernel (external gateway)", () => { ]; const payload = JSON.stringify(docs); + let initSeen = false; let preludeSeen = false; const kernelPromise = PythonKernel.start({ cwd: "/" }); @@ -392,6 +412,11 @@ describe("PythonKernel (external gateway)", () => { ws.setSendHandler((data) => { const msg = typeof data === "string" ? (JSON.parse(data) as JupyterMessage) : decodeMessage(data); const code = String(msg.content.code ?? ""); + if (!initSeen) { + initSeen = true; + sendOkExecution(ws, msg.header.msg_id); + return; + } if (!preludeSeen) { expect(code).toBe(PYTHON_PRELUDE); preludeSeen = true; diff --git a/packages/coding-agent/test/core/python-prelude.test.ts b/packages/coding-agent/test/core/python-prelude.test.ts index b41923a7b..87468d697 100644 --- a/packages/coding-agent/test/core/python-prelude.test.ts +++ b/packages/coding-agent/test/core/python-prelude.test.ts @@ -38,7 +38,6 @@ describe.skipIf(!shouldRun)("PYTHON_PRELUDE integration", () => { it("exposes prelude helpers via python tool", async () => { const helpers = [ "pwd", - "cd", "env", "read", "write", diff --git a/packages/coding-agent/test/core/system-prompt.python.test.ts b/packages/coding-agent/test/core/system-prompt.python.test.ts index 92875d951..5c8d304a4 100644 --- a/packages/coding-agent/test/core/system-prompt.python.test.ts +++ b/packages/coding-agent/test/core/system-prompt.python.test.ts @@ -11,7 +11,7 @@ describe("buildSystemPrompt", () => { rules: [], }); - expect(prompt).toContain("python: Execute Python code via a session-backed IPython kernel"); + expect(prompt).toContain("**python:** stateful scripting and REPL work"); expect(prompt).toContain("What python IS for"); }); }); diff --git a/packages/coding-agent/test/tools/python.test.ts b/packages/coding-agent/test/tools/python.test.ts index b0a7e63cf..b95386988 100644 --- a/packages/coding-agent/test/tools/python.test.ts +++ b/packages/coding-agent/test/tools/python.test.ts @@ -57,7 +57,7 @@ describe("python tool schema", () => { expect(schema.type).toBe("object"); expect(schema.properties.cells.type).toBe("array"); - expect(schema.properties.timeout_ms.type).toBe("number"); + expect(schema.properties.timeout.type).toBe("number"); expect(schema.properties.cwd.type).toBe("string"); expect(schema.properties.reset.type).toBe("boolean"); expect(schema.required).toEqual(["cells"]);