mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-09-19 10:02:04 +08:00
test(memory): remove MCP-specific docs and lifecycle tests
This commit is contained in:
@@ -1,91 +0,0 @@
|
||||
// Minimal JSON-RPC 2.0 MCP server over stdio for testing
|
||||
// Uses newline-delimited JSON (NDJSON) format matching the MCP SDK
|
||||
// Exits on stdin close, SIGTERM, or SIGINT
|
||||
|
||||
let readBuffer = ""
|
||||
|
||||
process.stdin.setEncoding("utf8")
|
||||
process.stdin.on("data", (chunk) => {
|
||||
readBuffer += chunk
|
||||
processBuffer()
|
||||
})
|
||||
|
||||
process.stdin.on("end", () => {
|
||||
process.exit(0)
|
||||
})
|
||||
|
||||
process.on("SIGTERM", () => process.exit(0))
|
||||
process.on("SIGINT", () => process.exit(0))
|
||||
|
||||
function processBuffer() {
|
||||
while (true) {
|
||||
const index = readBuffer.indexOf("\n")
|
||||
if (index === -1) break
|
||||
const line = readBuffer.slice(0, index).replace(/\r$/, "")
|
||||
readBuffer = readBuffer.slice(index + 1)
|
||||
if (line.length === 0) continue
|
||||
handle(line)
|
||||
}
|
||||
}
|
||||
|
||||
function send(msg) {
|
||||
process.stdout.write(JSON.stringify(msg) + "\n")
|
||||
}
|
||||
|
||||
function handle(raw) {
|
||||
let data
|
||||
try {
|
||||
data = JSON.parse(raw)
|
||||
} catch {
|
||||
return
|
||||
}
|
||||
|
||||
if (data.method === "initialize") {
|
||||
send({
|
||||
jsonrpc: "2.0",
|
||||
id: data.id,
|
||||
result: {
|
||||
protocolVersion: "2024-11-05",
|
||||
capabilities: { tools: {} },
|
||||
serverInfo: { name: "fake-mcp-server", version: "1.0.0" },
|
||||
},
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
if (data.method === "notifications/initialized") {
|
||||
return
|
||||
}
|
||||
|
||||
if (data.method === "tools/list") {
|
||||
send({
|
||||
jsonrpc: "2.0",
|
||||
id: data.id,
|
||||
result: {
|
||||
tools: [
|
||||
{
|
||||
name: "fake_tool",
|
||||
description: "A fake tool for testing",
|
||||
inputSchema: { type: "object", properties: {}, additionalProperties: false },
|
||||
},
|
||||
],
|
||||
},
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
if (data.method === "tools/call") {
|
||||
send({
|
||||
jsonrpc: "2.0",
|
||||
id: data.id,
|
||||
result: { content: [{ type: "text", text: "ok" }] },
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
// Respond to any other request to keep transport flowing
|
||||
if (typeof data.id !== "undefined") {
|
||||
send({ jsonrpc: "2.0", id: data.id, result: null })
|
||||
return
|
||||
}
|
||||
}
|
||||
@@ -1,78 +0,0 @@
|
||||
# Memory Leak & Orphan Process Detection Tests
|
||||
|
||||
## Overview
|
||||
|
||||
8 test files and 1 fixture file in `packages/opencode/test/` to detect memory leaks and orphan processes in Kilo CLI. These tests target the specific leak sources identified in upstream [opencode#3013](https://github.com/anomalyco/opencode/issues/3013).
|
||||
|
||||
## Files Created
|
||||
|
||||
| File | Lines | Purpose |
|
||||
|------|-------|---------|
|
||||
| `test/fixture/mcp/fake-mcp-server.js` | ~90 | Minimal JSON-RPC 2.0 MCP server over stdio |
|
||||
| `test/memory/helper.ts` | ~130 | Shared utilities for all memory/process tests |
|
||||
| `test/memory/orphan-process.test.ts` | ~175 | Orphan process detection after MCP/LSP lifecycle |
|
||||
| `test/memory/heap-growth.test.ts` | ~125 | Unbounded heap growth across subsystems |
|
||||
| `test/memory/disposal.test.ts` | ~145 | Disposal chain correctness verification |
|
||||
| `test/memory/mcp-lifecycle.test.ts` | ~130 | MCP-specific leak scenarios |
|
||||
| `test/memory/lsp-lifecycle.test.ts` | ~130 | LSP server process lifecycle |
|
||||
| `test/memory/state-leak.test.ts` | ~175 | State/Bus/GlobalBus/subscription leaks |
|
||||
| `test/memory/session-heap-growth.test.ts` | ~145 | Full session DB + event lifecycle |
|
||||
|
||||
## Test Results: 28 tests total
|
||||
|
||||
### 23 PASS — no leaks detected in these areas
|
||||
|
||||
| Suite | Tests | Key Findings |
|
||||
|-------|-------|-------------|
|
||||
| `disposal.test.ts` | 5/5 | Disposal chain works: `Instance.dispose` -> `State.dispose`, idempotent `disposeAll`, slow disposal completes |
|
||||
| `state-leak.test.ts` | 5/5 | `recordsByKey` cleared on dispose, Instance cache cleared on `disposeAll`, GlobalBus listeners bounded, Bus subscriptions per-instance and cleared |
|
||||
| `lsp-lifecycle.test.ts` | 3/3 | `LSPClient.shutdown()` correctly kills server process, no orphans after 5 cycles, 0.22 MB growth over 10 cycles |
|
||||
| `heap-growth.test.ts` | 4/4 | Instance provide/dispose: 0.57 MB/20 cycles. Bus subscriptions: 0.50 MB/100 cycles. State entries: -0.40 MB/100 cycles. MCP add/disconnect heap: 0.00 MB/3 cycles |
|
||||
| `session-heap-growth.test.ts` | 3/3 | Session create/remove: 0.32 MB/10 cycles. Message CRUD: 0.15 MB/50 ops. Full lifecycle with events: 0.27 MB/25 sessions |
|
||||
| `orphan-process.test.ts` | 1/4 | LSP shutdown: no orphans |
|
||||
| `mcp-lifecycle.test.ts` | 1/3 | `MCP.tools()` 50x: 0.05 MB growth |
|
||||
|
||||
### 5 FAIL — correctly detecting real MCP orphan process bugs
|
||||
|
||||
| Test | Error |
|
||||
|------|-------|
|
||||
| MCP: no orphans after dispose | `Found 1 orphan process: bun fake-mcp-server.js` |
|
||||
| MCP: no orphans after disconnect | `Found 1 orphan process: bun fake-mcp-server.js` |
|
||||
| disposeAll cleans up all instances | `Found 1 orphan process: bun fake-mcp-server.js` |
|
||||
| MCP.add closes existing before overwriting | `Found 1 orphan process: bun fake-mcp-server.js` |
|
||||
| MCP.connect closes existing before reconnecting | `Found 1 orphan process: bun fake-mcp-server.js` |
|
||||
|
||||
## Root Cause Confirmed
|
||||
|
||||
Every MCP test that spawns a server and then calls `client.close()` (via `StdioClientTransport`) leaves the child process alive. The MCP SDK's `StdioClientTransport.close()` closes the transport pipes but does **not** kill the spawned child process. This is the exact orphan process bug described in upstream #3013.
|
||||
|
||||
In contrast, `LSPClient.shutdown()` explicitly calls `process.kill()` on the server process, which is why all LSP tests pass cleanly.
|
||||
|
||||
## Architecture Decisions
|
||||
|
||||
- **Real processes, mocked nothing for process tests**: MCP/LSP servers are real `child_process.spawn` instances running `fake-mcp-server.js` / `fake-lsp-server.js`. This ensures process lifecycle is tested authentically.
|
||||
- **`pgrep -P` for process tree snapshots**: Works on macOS and Linux CI. Each test takes a before/after snapshot and asserts no new orphan descendants.
|
||||
- **`afterEach` safety net**: Force-kills any orphans to prevent cascading test failures.
|
||||
- **Empty config for MCP tests**: The MCP `state()` init connects to ALL configured servers on first access. Putting servers in config doubles connections and causes timeouts. Tests use `MCP.add()` directly.
|
||||
- **Session tests use real DB, no LLM mock**: Instead of mocking `streamText`'s complex async iterable, session tests exercise `Session.create/updateMessage/updatePart/remove` with Bus subscriptions — covering the real Database + Bus + State code paths.
|
||||
- **Generous thresholds**: `< 10 MB` for most growth tests, `< 5 MB` for targeted ones. Multiple GC passes (`Bun.gc(true)` x 3) with 50ms sleeps for stable measurement.
|
||||
|
||||
## Running the Tests
|
||||
|
||||
```bash
|
||||
cd packages/opencode
|
||||
|
||||
# All memory tests (non-MCP-process tests complete in ~150s)
|
||||
bun test test/memory/disposal.test.ts test/memory/state-leak.test.ts test/memory/lsp-lifecycle.test.ts test/memory/heap-growth.test.ts test/memory/session-heap-growth.test.ts
|
||||
|
||||
# MCP process tests (detect real orphan bugs, ~300s)
|
||||
bun test test/memory/orphan-process.test.ts test/memory/mcp-lifecycle.test.ts
|
||||
|
||||
# Individual suites
|
||||
bun test test/memory/disposal.test.ts
|
||||
bun test test/memory/session-heap-growth.test.ts
|
||||
```
|
||||
|
||||
## What Happens When The Bug Is Fixed
|
||||
|
||||
Once `StdioClientTransport.close()` is patched to kill the child process (or a wrapper is added in `src/mcp/index.ts`), all 5 currently-failing orphan tests will pass — providing regression protection.
|
||||
@@ -1,155 +0,0 @@
|
||||
import { describe, test, expect } from "bun:test"
|
||||
import path from "path"
|
||||
import { Instance } from "../../src/project/instance"
|
||||
import { MCP } from "../../src/mcp"
|
||||
import { tmpdir } from "../fixture/fixture"
|
||||
import { PROJECT_ROOT, snapshotDescendants, assertNoOrphans, forceKillAll, stableHeapMB, waitForExit } from "./helper"
|
||||
|
||||
const FAKE_MCP_SERVER = path.join(PROJECT_ROOT, "test/fixture/mcp/fake-mcp-server.js")
|
||||
|
||||
describe("memory: MCP lifecycle", () => {
|
||||
const cleanup = async (baseline: Set<number>, name: string) => {
|
||||
try {
|
||||
const afterPids = await snapshotDescendants(process.pid)
|
||||
const orphans = [...afterPids].filter((pid) => !baseline.has(pid))
|
||||
forceKillAll(orphans)
|
||||
} catch (err) {
|
||||
console.log(`[${name}] cleanup failed: ${err}`)
|
||||
}
|
||||
}
|
||||
|
||||
test("MCP.add closes existing client before overwriting", async () => {
|
||||
await using tmp = await tmpdir({ git: true })
|
||||
const baseline = await snapshotDescendants(process.pid)
|
||||
|
||||
try {
|
||||
await Instance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
try {
|
||||
// Add first server
|
||||
await MCP.add("lifecycle-test", {
|
||||
type: "local",
|
||||
command: ["bun", FAKE_MCP_SERVER],
|
||||
})
|
||||
|
||||
const afterFirstPids = await snapshotDescendants(process.pid)
|
||||
const firstOnlyPids = [...afterFirstPids].filter((p) => !baseline.has(p))
|
||||
const firstCount = firstOnlyPids.length
|
||||
|
||||
// Add second server with same key — should close first
|
||||
await MCP.add("lifecycle-test", {
|
||||
type: "local",
|
||||
command: ["bun", FAKE_MCP_SERVER],
|
||||
})
|
||||
|
||||
expect(await waitForExit(firstOnlyPids)).toBe(true)
|
||||
const afterSecondPids = await snapshotDescendants(process.pid)
|
||||
const secondCount = [...afterSecondPids].filter((p) => !baseline.has(p)).length
|
||||
|
||||
// Process count should stay roughly constant (old killed, new spawned)
|
||||
// Allow +1 for timing of process teardown
|
||||
expect(secondCount).toBeLessThanOrEqual(firstCount + 1)
|
||||
} finally {
|
||||
await Instance.dispose()
|
||||
}
|
||||
},
|
||||
})
|
||||
|
||||
const beforeAssertPids = await snapshotDescendants(process.pid)
|
||||
expect(await waitForExit([...beforeAssertPids].filter((p) => !baseline.has(p)))).toBe(true)
|
||||
const afterPids = await snapshotDescendants(process.pid)
|
||||
await assertNoOrphans(baseline, afterPids)
|
||||
} finally {
|
||||
await cleanup(baseline, "MCP.add closes existing client before overwriting")
|
||||
}
|
||||
}, 120_000)
|
||||
|
||||
test("MCP.connect closes existing client before reconnecting", async () => {
|
||||
await using tmp = await tmpdir({
|
||||
git: true,
|
||||
config: {
|
||||
mcp: {
|
||||
"connect-test": {
|
||||
type: "local",
|
||||
command: ["bun", FAKE_MCP_SERVER],
|
||||
},
|
||||
},
|
||||
},
|
||||
})
|
||||
const baseline = await snapshotDescendants(process.pid)
|
||||
|
||||
try {
|
||||
await Instance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
try {
|
||||
// First connect (state init may already connect from config)
|
||||
await MCP.connect("connect-test")
|
||||
|
||||
const afterFirstPids = await snapshotDescendants(process.pid)
|
||||
const firstOnlyPids = [...afterFirstPids].filter((p) => !baseline.has(p))
|
||||
const firstCount = firstOnlyPids.length
|
||||
|
||||
// Second connect — should close existing before reconnecting
|
||||
await MCP.connect("connect-test")
|
||||
|
||||
expect(await waitForExit(firstOnlyPids)).toBe(true)
|
||||
const afterSecondPids = await snapshotDescendants(process.pid)
|
||||
const secondCount = [...afterSecondPids].filter((p) => !baseline.has(p)).length
|
||||
|
||||
// Should not accumulate processes
|
||||
expect(secondCount).toBeLessThanOrEqual(firstCount + 1)
|
||||
} finally {
|
||||
await Instance.dispose()
|
||||
}
|
||||
},
|
||||
})
|
||||
|
||||
const beforeAssertPids = await snapshotDescendants(process.pid)
|
||||
expect(await waitForExit([...beforeAssertPids].filter((p) => !baseline.has(p)))).toBe(true)
|
||||
const afterPids = await snapshotDescendants(process.pid)
|
||||
await assertNoOrphans(baseline, afterPids)
|
||||
} finally {
|
||||
await cleanup(baseline, "MCP.connect closes existing client before reconnecting")
|
||||
}
|
||||
}, 120_000)
|
||||
|
||||
test("MCP.tools() does not leak memory", async () => {
|
||||
await using tmp = await tmpdir({ git: true })
|
||||
const baseline = await snapshotDescendants(process.pid)
|
||||
|
||||
try {
|
||||
await Instance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
try {
|
||||
await MCP.add("tools-test", {
|
||||
type: "local",
|
||||
command: ["bun", FAKE_MCP_SERVER],
|
||||
})
|
||||
|
||||
// Warm up
|
||||
await MCP.tools()
|
||||
|
||||
const baseline = await stableHeapMB()
|
||||
|
||||
for (let i = 0; i < 50; i++) {
|
||||
await MCP.tools()
|
||||
}
|
||||
|
||||
const after = await stableHeapMB()
|
||||
const growth = after - baseline
|
||||
|
||||
console.log(` MCP.tools() 50x growth: ${growth.toFixed(2)} MB`)
|
||||
expect(growth).toBeLessThan(5)
|
||||
} finally {
|
||||
await Instance.dispose()
|
||||
}
|
||||
},
|
||||
})
|
||||
} finally {
|
||||
await cleanup(baseline, "MCP.tools() does not leak memory")
|
||||
}
|
||||
}, 60_000)
|
||||
})
|
||||
@@ -1,179 +0,0 @@
|
||||
import { describe, test, expect } from "bun:test"
|
||||
import path from "path"
|
||||
import { Instance } from "../../src/project/instance"
|
||||
import { MCP } from "../../src/mcp"
|
||||
import { LSPClient } from "../../src/lsp/client"
|
||||
import { spawn } from "child_process"
|
||||
import { tmpdir } from "../fixture/fixture"
|
||||
import { PROJECT_ROOT, snapshotDescendants, assertNoOrphans, forceKillAll, waitForExit } from "./helper"
|
||||
|
||||
const FAKE_MCP_SERVER = path.join(PROJECT_ROOT, "test/fixture/mcp/fake-mcp-server.js")
|
||||
const FAKE_LSP_SERVER = path.join(PROJECT_ROOT, "test/fixture/lsp/fake-lsp-server.js")
|
||||
|
||||
// Don't put MCP in config — the MCP state() init function connects to ALL
|
||||
// configured servers on first access, which doubles connections and causes timeouts.
|
||||
// Instead, use empty config and add servers via MCP.add().
|
||||
|
||||
describe("memory: orphan process detection", () => {
|
||||
const cleanup = async (beforePids: Set<number>) => {
|
||||
// Safety net: kill any orphaned processes to prevent cascading failures
|
||||
try {
|
||||
const afterPids = await snapshotDescendants(process.pid)
|
||||
const orphans: number[] = []
|
||||
for (const pid of afterPids) {
|
||||
if (!beforePids.has(pid)) orphans.push(pid)
|
||||
}
|
||||
forceKillAll(orphans)
|
||||
} catch (error) {
|
||||
// Best-effort cleanup
|
||||
console.warn("orphan cleanup failed", error)
|
||||
}
|
||||
}
|
||||
|
||||
const baseline = async (run: (beforePids: Set<number>) => Promise<void>) => {
|
||||
const beforePids = await snapshotDescendants(process.pid)
|
||||
try {
|
||||
await run(beforePids)
|
||||
} finally {
|
||||
await cleanup(beforePids)
|
||||
}
|
||||
}
|
||||
|
||||
test("MCP local server: no orphans after dispose", async () => {
|
||||
await using tmp = await tmpdir({ git: true })
|
||||
|
||||
await baseline(async (beforePids) => {
|
||||
const spawned = new Set<number>()
|
||||
|
||||
await Instance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
// MCP.add triggers spawn of the fake server via StdioClientTransport
|
||||
await MCP.add("test-server", {
|
||||
type: "local",
|
||||
command: ["bun", FAKE_MCP_SERVER],
|
||||
})
|
||||
|
||||
// Verify server is running (new descendants exist)
|
||||
const duringPids = await snapshotDescendants(process.pid)
|
||||
const newProcesses = [...duringPids].filter((p) => !beforePids.has(p))
|
||||
expect(newProcesses.length).toBeGreaterThan(0)
|
||||
for (const pid of newProcesses) {
|
||||
spawned.add(pid)
|
||||
}
|
||||
|
||||
// Dispose the instance — should close all MCP clients and kill processes
|
||||
await Instance.dispose()
|
||||
},
|
||||
})
|
||||
|
||||
expect(await waitForExit([...spawned])).toBe(true)
|
||||
const afterPids = await snapshotDescendants(process.pid)
|
||||
await assertNoOrphans(beforePids, afterPids)
|
||||
})
|
||||
}, 60_000)
|
||||
|
||||
test("MCP local server: no orphans after disconnect", async () => {
|
||||
await using tmp = await tmpdir({ git: true })
|
||||
|
||||
await baseline(async (beforePids) => {
|
||||
await Instance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
await MCP.add("test-server", {
|
||||
type: "local",
|
||||
command: ["bun", FAKE_MCP_SERVER],
|
||||
})
|
||||
|
||||
const duringPids = await snapshotDescendants(process.pid)
|
||||
const newProcesses = [...duringPids].filter((p) => !beforePids.has(p))
|
||||
expect(newProcesses.length).toBeGreaterThan(0)
|
||||
|
||||
// Disconnect should close the client (which closes the transport/process)
|
||||
await MCP.disconnect("test-server")
|
||||
expect(await waitForExit(newProcesses)).toBe(true)
|
||||
|
||||
const afterPids = await snapshotDescendants(process.pid)
|
||||
await assertNoOrphans(beforePids, afterPids)
|
||||
|
||||
await Instance.dispose()
|
||||
},
|
||||
})
|
||||
})
|
||||
}, 60_000)
|
||||
|
||||
test("disposeAll cleans up all instances", async () => {
|
||||
await using tmp1 = await tmpdir({ git: true })
|
||||
await using tmp2 = await tmpdir({ git: true })
|
||||
|
||||
await baseline(async (beforePids) => {
|
||||
// Create two instances with MCP servers
|
||||
await Instance.provide({
|
||||
directory: tmp1.path,
|
||||
fn: async () => {
|
||||
await MCP.add("test-server", {
|
||||
type: "local",
|
||||
command: ["bun", FAKE_MCP_SERVER],
|
||||
})
|
||||
},
|
||||
})
|
||||
|
||||
await Instance.provide({
|
||||
directory: tmp2.path,
|
||||
fn: async () => {
|
||||
await MCP.add("test-server", {
|
||||
type: "local",
|
||||
command: ["bun", FAKE_MCP_SERVER],
|
||||
})
|
||||
},
|
||||
})
|
||||
|
||||
// Verify servers are running
|
||||
const duringPids = await snapshotDescendants(process.pid)
|
||||
const newProcesses = [...duringPids].filter((p) => !beforePids.has(p))
|
||||
expect(newProcesses.length).toBeGreaterThan(0)
|
||||
|
||||
// Dispose all
|
||||
await Instance.disposeAll()
|
||||
expect(await waitForExit(newProcesses)).toBe(true)
|
||||
|
||||
const afterPids = await snapshotDescendants(process.pid)
|
||||
await assertNoOrphans(beforePids, afterPids)
|
||||
})
|
||||
}, 120_000)
|
||||
|
||||
test("LSP server: no orphans after shutdown", async () => {
|
||||
await using tmp = await tmpdir({ git: true })
|
||||
|
||||
await baseline(async (beforePids) => {
|
||||
await Instance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
const serverProcess = spawn("bun", [FAKE_LSP_SERVER], {
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
cwd: tmp.path,
|
||||
})
|
||||
|
||||
const client = await LSPClient.create({
|
||||
serverID: "test-lsp",
|
||||
server: { process: serverProcess as any },
|
||||
root: tmp.path,
|
||||
})
|
||||
|
||||
expect(client).toBeTruthy()
|
||||
|
||||
// Shutdown should kill the process
|
||||
await client!.shutdown()
|
||||
if (serverProcess.pid) {
|
||||
expect(await waitForExit([serverProcess.pid])).toBe(true)
|
||||
}
|
||||
|
||||
const afterPids = await snapshotDescendants(process.pid)
|
||||
await assertNoOrphans(beforePids, afterPids)
|
||||
|
||||
await Instance.dispose()
|
||||
},
|
||||
})
|
||||
})
|
||||
}, 30_000)
|
||||
})
|
||||
Reference in New Issue
Block a user