diff --git a/packages/coding-agent/src/ipy/executor.ts b/packages/coding-agent/src/ipy/executor.ts index 0ef299e07..a4b928273 100644 --- a/packages/coding-agent/src/ipy/executor.ts +++ b/packages/coding-agent/src/ipy/executor.ts @@ -227,6 +227,7 @@ export async function warmPythonEnvironment( useSharedGateway?: boolean, sessionFile?: string, ): Promise<{ ok: boolean; reason?: string; docs: PreludeHelper[] }> { + const isTestEnv = process.env.BUN_ENV === "test" || process.env.NODE_ENV === "test"; let cacheState: PreludeCacheState | null = null; try { debugStartup("warmPython:ensureKernel:start"); @@ -238,16 +239,18 @@ export async function warmPythonEnvironment( cachedPreludeDocs = []; return { ok: false, reason, docs: [] }; } - try { - cacheState = await buildPreludeCacheState(cwd); - const cached = await readPreludeCache(cacheState); - if (cached) { - cachedPreludeDocs = cached; - return { ok: true, docs: cached }; + if (!isTestEnv) { + try { + cacheState = await buildPreludeCacheState(cwd); + const cached = await readPreludeCache(cacheState); + if (cached) { + cachedPreludeDocs = cached; + return { ok: true, docs: cached }; + } + } catch (err) { + logger.warn("Failed to resolve Python prelude cache", { error: String(err) }); + cacheState = null; } - } catch (err) { - logger.warn("Failed to resolve Python prelude cache", { error: String(err) }); - cacheState = null; } if (cachedPreludeDocs && cachedPreludeDocs.length > 0) { return { ok: true, docs: cachedPreludeDocs }; @@ -265,7 +268,7 @@ export async function warmPythonEnvironment( debugStartup("warmPython:withKernelSession:done"); time("warmPython:withKernelSession"); cachedPreludeDocs = docs; - if (docs.length > 0) { + if (!isTestEnv && docs.length > 0) { const state = cacheState ?? (await buildPreludeCacheState(cwd)); await writePreludeCache(state, docs); } @@ -526,8 +529,7 @@ export async function executePython(code: string, options?: PythonExecutorOption await ensureKernelAvailable(cwd); const kernelMode = options?.kernelMode ?? "session"; - const isTestEnv = process.env.BUN_ENV === "test" || process.env.NODE_ENV === "test"; - const useSharedGateway = isTestEnv ? false : options?.useSharedGateway; + const useSharedGateway = options?.useSharedGateway; const sessionFile = options?.sessionFile; const artifactsDir = options?.artifactsDir; diff --git a/packages/coding-agent/src/ipy/gateway-coordinator.ts b/packages/coding-agent/src/ipy/gateway-coordinator.ts index 104321a1d..54740825f 100644 --- a/packages/coding-agent/src/ipy/gateway-coordinator.ts +++ b/packages/coding-agent/src/ipy/gateway-coordinator.ts @@ -313,10 +313,6 @@ async function killGateway(pid: number, context: string): Promise { } export async function acquireSharedGateway(cwd: string): Promise { - if (process.env.BUN_ENV === "test" || process.env.NODE_ENV === "test") { - return null; - } - try { return await withGatewayLock(async () => { time("acquireSharedGateway:lockAcquired"); diff --git a/packages/coding-agent/test/core/python-kernel-env.test.ts b/packages/coding-agent/test/core/python-kernel-env.test.ts index 1a682295a..596558ece 100644 --- a/packages/coding-agent/test/core/python-kernel-env.test.ts +++ b/packages/coding-agent/test/core/python-kernel-env.test.ts @@ -1,7 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; import { _resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; -import { PythonKernel } from "@oh-my-pi/pi-coding-agent/ipy/kernel"; -import { PYTHON_PRELUDE } from "@oh-my-pi/pi-coding-agent/ipy/prelude"; +import { acquireSharedGateway } from "@oh-my-pi/pi-coding-agent/ipy/gateway-coordinator"; import * as shellSnapshot from "@oh-my-pi/pi-coding-agent/utils/shell-snapshot"; import { TempDir } from "@oh-my-pi/pi-utils"; @@ -31,7 +30,7 @@ class FakeWebSocket { } } -describe("PythonKernel.start (local gateway)", () => { +describe("Shared Python gateway environment", () => { const originalEnv = { ...process.env }; const originalFetch = globalThis.fetch; const originalWebSocket = globalThis.WebSocket; @@ -58,15 +57,12 @@ describe("PythonKernel.start (local gateway)", () => { vi.restoreAllMocks(); }); - it("filters environment variables before spawning gateway", async () => { - const fetchSpy = vi.fn(async (input: string | URL, init?: RequestInit) => { + it("filters environment variables before spawning shared gateway", async () => { + const fetchSpy = vi.fn(async (input: string | URL) => { const url = typeof input === "string" ? input : input.toString(); if (url.endsWith("/api/kernelspecs")) { return new Response(JSON.stringify({}), { status: 200 }); } - if (url.endsWith("/api/kernels") && init?.method === "POST") { - return new Response(JSON.stringify({ id: "kernel-1" }), { status: 201 }); - } return new Response("", { status: 200 }); }); globalThis.fetch = fetchSpy as unknown as typeof fetch; @@ -96,46 +92,21 @@ describe("PythonKernel.start (local gateway)", () => { return { pid: 1234, exited: Promise.resolve(0) } as unknown as Bun.Subprocess; }) as unknown as typeof Bun.spawn); - const executeSpy = vi - .spyOn(PythonKernel.prototype, "execute") - .mockResolvedValue({ status: "ok", cancelled: false, timedOut: false, stdinRequested: false }); - using tempDir = TempDir.createSync("@python-kernel-env-"); - const kernel = await PythonKernel.start({ cwd: tempDir.path(), env: { CUSTOM_VAR: "ok" } }); - - const createCall = fetchSpy.mock.calls.find(([input, init]: [string | URL, RequestInit?]) => { - const url = typeof input === "string" ? input : input.toString(); - return url.endsWith("/api/kernels") && init?.method === "POST"; - }); - expect(createCall).toBeDefined(); - if (createCall) { - expect(JSON.parse(String(createCall[1]?.body ?? "{}"))).toEqual({ name: "python3" }); - } + process.env.OMP_CODING_AGENT_DIR = tempDir.path(); + await acquireSharedGateway(tempDir.path()); expect(spawnArgs).toContain("kernel_gateway"); expect(spawnEnv?.PATH).toBe("/bin"); expect(spawnEnv?.HOME).toBe("/home/test"); expect(spawnEnv?.OMP_CUSTOM).toBe("1"); expect(spawnEnv?.LC_ALL).toBe("en_US.UTF-8"); - expect(spawnEnv?.CUSTOM_VAR).toBe("ok"); expect(spawnEnv?.OPENAI_API_KEY).toBeUndefined(); expect(spawnEnv?.UNSAFE_TOKEN).toBeUndefined(); - expect(spawnEnv?.PYTHONPATH).toBe(tempDir.path()); - - expect(executeSpy).toHaveBeenCalledWith( - PYTHON_PRELUDE, - expect.objectContaining({ - silent: true, - storeHistory: false, - }), - ); - - await kernel.shutdown(); vi.restoreAllMocks(); snapshotSpy.mockRestore(); whichSpy.mockRestore(); spawnSpy.mockRestore(); - executeSpy.mockRestore(); }); }); diff --git a/packages/coding-agent/test/core/python-kernel.lifecycle.test.ts b/packages/coding-agent/test/core/python-kernel.lifecycle.test.ts index 257132737..4ee8e2618 100644 --- a/packages/coding-agent/test/core/python-kernel.lifecycle.test.ts +++ b/packages/coding-agent/test/core/python-kernel.lifecycle.test.ts @@ -1,5 +1,6 @@ -import { afterEach, beforeEach, describe, expect, it } from "bun:test"; +import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; import { PythonKernel } from "@oh-my-pi/pi-coding-agent/ipy/kernel"; +import * as gatewayCoordinator from "@oh-my-pi/pi-coding-agent/ipy/gateway-coordinator"; import { TempDir } from "@oh-my-pi/pi-utils"; import type { Subprocess } from "bun"; @@ -148,18 +149,16 @@ describe("PythonKernel gateway lifecycle", () => { Object.defineProperty(PythonKernel.prototype, "execute", { value: originalExecute, configurable: true }); }); - it("starts local gateway, polls readiness, interrupts, and shuts down", async () => { - let kernelspecAttempts = 0; + it("starts shared gateway, interrupts, and shuts down", async () => { + vi.spyOn(gatewayCoordinator, "acquireSharedGateway").mockResolvedValue({ + url: "http://127.0.0.1:9999", + isShared: true, + }); + globalThis.fetch = (async (input: string | URL, init?: RequestInit) => { const url = String(input); env.fetchCalls.push({ url, init }); - if (url.endsWith("/api/kernelspecs")) { - kernelspecAttempts += 1; - const ok = kernelspecAttempts >= 2; - return createResponse({ ok }) as unknown as Response; - } - if (url.endsWith("/api/kernels") && init?.method === "POST") { return createResponse({ ok: true, json: { id: "kernel-123" } }) as unknown as Response; } @@ -167,66 +166,47 @@ describe("PythonKernel gateway lifecycle", () => { return createResponse({ ok: true }) as unknown as Response; }) as typeof fetch; - const kernel = await PythonKernel.start({ cwd: tempDir.path(), useSharedGateway: false }); + const kernel = await PythonKernel.start({ cwd: tempDir.path() }); - expect(env.spawnCalls).toHaveLength(1); - expect(env.spawnCalls[0].cmd).toEqual( - expect.arrayContaining([ - "-m", - "kernel_gateway", - "--KernelGatewayApp.allow_origin=*", - "--JupyterApp.answer_yes=true", - ]), - ); - expect(env.fetchCalls.filter(call => call.url.endsWith("/api/kernelspecs"))).toHaveLength(2); expect(env.fetchCalls.some(call => call.url.endsWith("/api/kernels") && call.init?.method === "POST")).toBe(true); await kernel.interrupt(); expect(env.fetchCalls.some(call => call.url.includes("/interrupt") && call.init?.method === "POST")).toBe(true); - expect(FakeWebSocket.instances[0]?.sent.length).toBe(1); await kernel.shutdown(); expect(env.fetchCalls.some(call => call.init?.method === "DELETE")).toBe(true); expect(kernel.isAlive()).toBe(false); }); - it("throws when gateway readiness never succeeds", async () => { - const originalNow = Date.now; - let now = 0; - Date.now = () => { - now += 1000; - return now; - }; + it("throws when shared gateway kernel creation never succeeds", async () => { + vi.spyOn(gatewayCoordinator, "acquireSharedGateway").mockResolvedValue({ + url: "http://127.0.0.1:9999", + isShared: true, + }); - try { - globalThis.fetch = (async (input: string | URL, init?: RequestInit) => { - const url = String(input); - env.fetchCalls.push({ url, init }); - if (url.endsWith("/api/kernelspecs")) { - return createResponse({ ok: false, status: 503 }) as unknown as Response; - } - return createResponse({ ok: true }) as unknown as Response; - }) as typeof fetch; - - await expect(PythonKernel.start({ cwd: tempDir.path(), useSharedGateway: false })).rejects.toThrow( - "Kernel gateway failed to start", - ); - expect(env.spawnCalls).toHaveLength(3); - } finally { - Date.now = originalNow; - } - }); - - it("does not throw when shutdown API fails", async () => { - let kernelspecAttempts = 0; globalThis.fetch = (async (input: string | URL, init?: RequestInit) => { const url = String(input); env.fetchCalls.push({ url, init }); - if (url.endsWith("/api/kernelspecs")) { - kernelspecAttempts += 1; - const ok = kernelspecAttempts >= 1; - return createResponse({ ok }) as unknown as Response; + if (url.endsWith("/api/kernels") && init?.method === "POST") { + return createResponse({ ok: false, status: 503, text: "oops" }) as unknown as Response; } + return createResponse({ ok: true }) as unknown as Response; + }) as typeof fetch; + + await expect(PythonKernel.start({ cwd: tempDir.path() })).rejects.toThrow( + "Failed to create kernel on shared gateway", + ); + }); + + it("does not throw when shutdown API fails", async () => { + vi.spyOn(gatewayCoordinator, "acquireSharedGateway").mockResolvedValue({ + url: "http://127.0.0.1:9999", + isShared: true, + }); + + globalThis.fetch = (async (input: string | URL, init?: RequestInit) => { + const url = String(input); + env.fetchCalls.push({ url, init }); if (url.endsWith("/api/kernels") && init?.method === "POST") { return createResponse({ ok: true, json: { id: "kernel-456" } }) as unknown as Response; } diff --git a/packages/coding-agent/test/core/python-prelude.test.ts b/packages/coding-agent/test/core/python-prelude.test.ts index ea9ad7a2d..43b5f2214 100644 --- a/packages/coding-agent/test/core/python-prelude.test.ts +++ b/packages/coding-agent/test/core/python-prelude.test.ts @@ -50,6 +50,7 @@ describe.skipIf(!shouldRun)("PYTHON_PRELUDE integration", () => { "lsp.diagnosticsOnWrite": false, "python.toolMode": "ipy-only", "python.kernelMode": "per-call", + "python.sharedGateway": true, }), }; @@ -74,7 +75,7 @@ describe.skipIf(!shouldRun)("PYTHON_PRELUDE integration", () => { it("exposes prelude docs via warmup", async () => { resetPreludeDocsCache(); - const result = await warmPythonEnvironment(process.cwd(), undefined, false); + const result = await warmPythonEnvironment(process.cwd()); expect(result.ok).toBe(true); const names = result.docs.map(doc => doc.name); expect(names).toContain("read"); @@ -82,7 +83,7 @@ describe.skipIf(!shouldRun)("PYTHON_PRELUDE integration", () => { it("renders prelude docs in python tool description", async () => { resetPreludeDocsCache(); - const result = await warmPythonEnvironment(process.cwd(), undefined, false); + const result = await warmPythonEnvironment(process.cwd()); expect(result.ok).toBe(true); const description = getPythonToolDescription(); expect(description).toContain("read");