fix(ai): stop leaking opencode serve processes (#1445)

* fix(ai): stop leaking opencode serve processes

Every server boot with the opencode CLI on PATH eagerly called the
provider's fetchModels() to fill the Ask AI dropdown, which spawned (or
attached to) an 'opencode serve' on the shared default port 4096. Dispose
only ran on the clean decision path, so Ctrl-C orphaned the child, and
every later session attached to the orphan and piled unevictable
per-directory instances into it (multi-GB over a day of normal use).

Three changes:
- Lazy start: opencode model discovery moves onto the same deferred
  provider initializer Codex uses. Nothing spawns until the user
  activates opencode in Ask AI (?activate= from the model picker, or the
  first opencode session). The picker still lists the provider with an
  empty model list pre-activation, exactly like Codex.
- Own server per process: spawn with port 0 (OS-assigned; the SDK reads
  the real URL from the child's listening line) and never attach to a
  server we did not spawn. An explicitly configured port is honored.
- Exit cleanup: a process 'exit' handler closes the spawned server
  (SIGINT/SIGTERM are routed through process.exit by the CLI), removed
  again on dispose. No SIGHUP listener, preserving nohup.

Both runtimes; regression tests mock the SDK so no real server spawns.

* fix(ai): close review findings on the opencode lifecycle

Independent review of the leak fix found two holes, both now closed and
regression-tested against the mocked SDK:

- A failure after the spawn (client construction) left the child running
  and its exit handler registered, and because the handler read
  this.server late instead of capturing its own server, a retry's second
  spawn made the first unreachable by any cleanup. doStart now captures
  the server in its handler closure and reaps child + handler on any
  post-spawn failure.
- dispose() during an in-flight spawn was a no-op the completing spawn
  then undid, resurrecting a disposed provider with a live child and a
  fresh exit handler. dispose() now bumps a start epoch; a spawn that
  completes past its epoch reaps its own server and rejects, and the
  provider remains restartable afterwards.

Also documents the OpenCode transport (per-process server, deferred
discovery) beside the Codex note in AGENTS.md.
This commit is contained in:
Michael Ramos
2026-09-01 10:55:17 -07:00
committed by GitHub
parent 9682a05556
commit a79e6b0efe
6 changed files with 299 additions and 57 deletions
+2
View File
@@ -262,6 +262,8 @@ Automatic resolution is session-only and never writes a preference. Explicit per
> **Codex transport note:** the `codex-sdk` provider id is a stable identifier only — it no longer uses `@openai/codex-sdk` / `codex exec`. It drives a long-lived `codex app-server` process over JSON-RPC (`packages/ai/providers/codex-app-server.ts`), which respects the user's/enterprise-managed approval policy and supports interactive Allow/Deny approvals. The id stays `codex-sdk` to preserve saved cookie preferences, the `agents.ts` mapping, and the UI reasoning-effort gate.
> **OpenCode transport note:** the `opencode-sdk` provider spawns its own `opencode serve` per process on an OS-assigned port (`port: 0`) and never attaches to a server it did not spawn (an attached server can't be cleaned up by us, and opencode's per-directory instances accumulate in it without eviction). The spawned server is closed on dispose and on process exit. Model discovery is deferred behind the provider initializer (`?activate=` from the model picker, or the first opencode session) exactly like Codex — nothing spawns at server boot, so the picker lists opencode with an empty model list until first activation. Regression-pinned by `packages/ai/providers/opencode-sdk.test.ts`.
## Annotate Flow
```
+12 -5
View File
@@ -104,14 +104,21 @@ export async function createPiAIRuntime(options: CreatePiAIRuntimeOptions = {}):
type: "opencode-sdk",
cwd,
});
const providerId = registry.register(provider);
// Deferred like Codex: fetchModels spawns `opencode serve`, so it
// must NOT run eagerly at startup — that spawned a server on every
// session for every user with opencode installed, and interrupted
// sessions orphaned it. The initializer runs on first explicit
// activation (?activate= from the model picker) or first opencode
// session.
if (provider && "fetchModels" in provider) {
modelDiscovery.push(
(provider as { fetchModels: () => Promise<void> })
.fetchModels()
.catch(() => {}),
providerInitializers.set(
providerId,
ai.createBestEffortOnce(
() => (provider as { fetchModels: () => Promise<void> }).fetchModels(),
),
);
}
registry.register(provider);
}
} catch {
// OpenCode not available.
+199
View File
@@ -0,0 +1,199 @@
/**
* Guards for the opencode-serve orphan leak:
*
* 1. The provider must spawn its OWN server on an OS-assigned port (port 0),
* never share a fixed default port. A shared port made every Plannotator
* process attach to the first server spawned; interrupted sessions orphaned
* it and later sessions piled unevictable per-directory instances into it.
* 2. A process "exit" handler must close the spawned server (the CLI routes
* SIGINT/SIGTERM through process.exit, so this is what covers Ctrl-C), and
* dispose must both close the server and remove that handler.
* 3. A failure AFTER the spawn (client construction) must reap the child and
* remove the handler so a retry cannot strand the first server.
* 4. dispose() during an in-flight spawn must not resurrect the provider: the
* completing spawn reaps its own server and the start rejects.
* 5. Neither runtime's AI setup may call the provider's fetchModels eagerly at
* startup it spawns `opencode serve`, so it must stay behind the deferred
* provider initializer (?activate= / first session), like Codex.
*
* The SDK is mocked before the provider import so no real `opencode serve` is
* ever spawned. bun runs all test files in one process, so the module mock
* leaks process-wide; that is safe (and desirable) here because this provider
* is the only consumer of @opencode-ai/sdk and no test may spawn a real
* server.
*/
import { beforeEach, describe, expect, mock, test } from "bun:test";
import { readFileSync } from "node:fs";
import { resolve } from "node:path";
interface FakeSpawn {
hostname?: string;
port?: number;
timeout?: number;
closed: boolean;
}
let spawned: FakeSpawn[] = [];
let clientFactory: () => unknown = () => ({});
let spawnGate: Promise<void> | null = null;
mock.module("@opencode-ai/sdk", () => ({
createOpencodeServer: async (opts: {
hostname?: string;
port?: number;
timeout?: number;
}) => {
const record: FakeSpawn = { ...opts, closed: false };
spawned.push(record);
if (spawnGate) await spawnGate;
return {
url: "http://127.0.0.1:54321",
close: () => {
record.closed = true;
},
};
},
createOpencodeClient: () => clientFactory(),
}));
const { OpenCodeProvider } = await import("./opencode-sdk.ts");
function makeProvider(port?: number) {
return new OpenCodeProvider({
type: "opencode-sdk",
...(port != null && { port }),
});
}
beforeEach(() => {
spawned = [];
clientFactory = () => ({});
spawnGate = null;
});
describe("OpenCodeProvider server lifecycle", () => {
test("spawns on an OS-assigned port and cleans up on dispose", async () => {
const provider = makeProvider();
const listenersBefore = process.listeners("exit").length;
await provider.ensureServer();
expect(spawned.length).toBe(1);
// port 0 = OS-assigned free port. A fixed default here reintroduces the
// shared-server pile-up.
expect(spawned[0]!.port).toBe(0);
expect(process.listeners("exit").length).toBe(listenersBefore + 1);
provider.dispose();
expect(spawned[0]!.closed).toBe(true);
expect(process.listeners("exit").length).toBe(listenersBefore);
});
test("the exit handler closes the spawned server", async () => {
const provider = makeProvider();
const before = new Set(process.listeners("exit"));
await provider.ensureServer();
const added = process.listeners("exit").filter((l) => !before.has(l));
expect(added.length).toBe(1);
// Simulate the process exiting without a clean dispose (Ctrl-C routes
// through process.exit, which runs "exit" listeners).
(added[0] as () => void)();
expect(spawned[0]!.closed).toBe(true);
provider.dispose();
});
test("honors an explicitly configured port verbatim", async () => {
const provider = makeProvider(5555);
await provider.ensureServer();
expect(spawned[0]!.port).toBe(5555);
provider.dispose();
});
test("a post-spawn failure reaps the child; a retry cannot strand it", async () => {
const provider = makeProvider();
const listenersBefore = process.listeners("exit").length;
let calls = 0;
clientFactory = () => {
calls++;
if (calls === 1) throw new Error("client construction failed");
return {};
};
await expect(provider.ensureServer()).rejects.toThrow(
"client construction failed",
);
// The first spawn must be closed and its exit handler removed — a
// leaked handler here would keep a dead server's closure registered
// forever, and a leaked server is the orphan class this fix exists
// to kill.
expect(spawned.length).toBe(1);
expect(spawned[0]!.closed).toBe(true);
expect(process.listeners("exit").length).toBe(listenersBefore);
// The retry starts clean: one fresh spawn, one handler.
await provider.ensureServer();
expect(spawned.length).toBe(2);
expect(spawned[1]!.closed).toBe(false);
expect(process.listeners("exit").length).toBe(listenersBefore + 1);
provider.dispose();
expect(spawned[1]!.closed).toBe(true);
expect(process.listeners("exit").length).toBe(listenersBefore);
});
test("dispose during an in-flight spawn reaps instead of resurrecting", async () => {
const provider = makeProvider();
const listenersBefore = process.listeners("exit").length;
let openGate: () => void = () => {};
spawnGate = new Promise<void>((r) => {
openGate = r;
});
const inFlight = provider.ensureServer();
// Let doStart enter createOpencodeServer before disposing.
await Bun.sleep(0);
expect(spawned.length).toBe(1);
provider.dispose();
openGate();
await expect(inFlight).rejects.toThrow("disposed during startup");
// The late-completing spawn must reap its own server and leave no
// handler behind — a disposed provider must never hold a live child.
expect(spawned[0]!.closed).toBe(true);
expect(process.listeners("exit").length).toBe(listenersBefore);
// The provider is still usable afterwards: a fresh start respawns.
spawnGate = null;
await provider.ensureServer();
expect(spawned.length).toBe(2);
expect(spawned[1]!.closed).toBe(false);
provider.dispose();
expect(spawned[1]!.closed).toBe(true);
});
});
describe("no eager opencode spawn at runtime startup", () => {
const repoRoot = resolve(import.meta.dir, "../../..");
// Both runtimes must keep opencode model discovery behind the deferred
// provider initializer. An eager fetchModels() at startup spawns
// `opencode serve` on every session for every user with opencode on PATH.
for (const relPath of [
"packages/server/ai-runtime.ts",
"apps/pi-extension/server/ai-runtime.ts",
]) {
test(`${relPath} defers opencode model discovery`, () => {
const src = readFileSync(resolve(repoRoot, relPath), "utf8");
const start = src.indexOf('"opencode-sdk"');
expect(start).toBeGreaterThan(-1);
const end = src.indexOf("OpenCode not available", start);
expect(end).toBeGreaterThan(start);
const block = src.slice(start, end);
expect(block).toContain("providerInitializers.set");
expect(block).not.toContain("modelDiscovery.push");
});
}
});
+71 -47
View File
@@ -1,10 +1,13 @@
/**
* OpenCode provider bridges Plannotator's AI layer with OpenCode's agent server.
*
* Uses @opencode-ai/sdk to connect to an existing `opencode serve` first and
* only spawns a new server when nothing is reachable. One server is shared
* across all sessions. The user must have the `opencode` CLI installed and
* authenticated.
* Uses @opencode-ai/sdk to spawn a dedicated `opencode serve` on an
* OS-assigned port. One server is shared across all sessions of this process,
* closed on dispose and on process exit. This provider deliberately never
* attaches to a server it did not spawn: an attached server cannot be cleaned
* up by us, and opencode's per-directory instances accumulate in it without
* eviction, so a shared long-lived server grows without bound. The user must
* have the `opencode` CLI installed and authenticated.
*/
import type { OpencodeClient } from "@opencode-ai/sdk";
@@ -58,13 +61,19 @@ export class OpenCodeProvider implements AIProvider {
private server: { url: string; close: () => void } | null = null;
private client: OpencodeClient | null = null;
private startPromise: Promise<void> | null = null;
private lastAttachError: string | null = null;
private exitHandler: (() => void) | null = null;
/**
* Bumped by dispose() to invalidate an in-flight doStart: a spawn that
* completes after its epoch has passed reaps its own server instead of
* resurrecting a provider the runtime already considers disposed.
*/
private startEpoch = 0;
constructor(config: OpenCodeConfig) {
this.config = config;
}
/** Attach to an existing OpenCode server or spawn one if needed. */
/** Spawn this process's OpenCode server if it is not already running. */
async ensureServer(): Promise<void> {
if (this.client) return;
this.startPromise ??= this.doStart().catch((err) => {
@@ -75,55 +84,63 @@ export class OpenCodeProvider implements AIProvider {
}
private async doStart(): Promise<void> {
this.lastAttachError = null;
const epoch = this.startEpoch;
const { createOpencodeServer, createOpencodeClient } = await getSDK();
const attachedClient = await this.tryAttachExistingServer(createOpencodeClient);
if (attachedClient) {
this.client = attachedClient;
return;
// port 0 asks opencode for an OS-assigned free port (the SDK reads the
// real URL back from the child's "listening" line), so every Plannotator
// process gets its own server instead of piling onto a shared default
// port. An explicitly configured port is still honored verbatim.
const server: { url: string; close: () => void } = await createOpencodeServer({
hostname: this.config.hostname ?? "127.0.0.1",
port: this.config.port ?? 0,
timeout: 15_000,
});
// A SIGINT/SIGTERM death is routed through process.exit() by the CLI, so
// an "exit" handler is what keeps Ctrl-C from orphaning the spawned
// `opencode serve` child (server.close() kills it synchronously). The
// closure captures ITS server — never `this.server`, which a failed
// retry could have replaced, leaving the first child unreachable by any
// cleanup. No SIGHUP listener: that would override the ignored
// disposition `nohup` depends on.
const exitHandler = () => {
try {
server.close();
} catch {
// Best effort — the process is exiting either way.
}
};
process.once("exit", exitHandler);
const reap = () => {
process.removeListener("exit", exitHandler);
try {
server.close();
} catch {
// Best effort — the child may already be gone.
}
};
if (epoch !== this.startEpoch) {
// dispose() ran while the spawn was in flight: the runtime no longer
// wants this provider, so reap the just-spawned server instead of
// resurrecting a disposed provider with a live child.
reap();
throw new Error("OpenCode provider was disposed during startup.");
}
try {
this.server = await createOpencodeServer({
hostname: this.config.hostname ?? "127.0.0.1",
...(this.config.port != null && { port: this.config.port }),
timeout: 15_000,
this.client = createOpencodeClient({
baseUrl: server.url,
directory: this.config.cwd ?? process.cwd(),
});
} catch (err) {
const spawnMessage = err instanceof Error ? err.message : String(err);
if (this.lastAttachError) {
throw new Error(`${this.lastAttachError}\nFallback startup also failed: ${spawnMessage}`);
}
// A post-spawn failure must not leak the child: close it and drop the
// handler so a retry starts from a clean slate.
reap();
throw err;
}
this.client = createOpencodeClient({
baseUrl: this.server!.url,
directory: this.config.cwd ?? process.cwd(),
});
}
private async tryAttachExistingServer(
createOpencodeClient: (config?: { baseUrl?: string; directory?: string }) => OpencodeClient,
): Promise<OpencodeClient | null> {
const cwd = this.config.cwd ?? process.cwd();
const baseUrl = `http://${this.config.hostname ?? "127.0.0.1"}:${this.config.port ?? 4096}`;
const client = createOpencodeClient({
baseUrl,
directory: cwd,
});
try {
await client.config.get({
throwOnError: true,
signal: AbortSignal.timeout(1_000),
});
return client;
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
this.lastAttachError = `Failed to attach to existing OpenCode server at ${baseUrl}: ${message}`;
return null;
}
this.server = server;
this.exitHandler = exitHandler;
}
private getClient(): OpencodeClient {
@@ -198,6 +215,13 @@ export class OpenCodeProvider implements AIProvider {
}
dispose(): void {
// Invalidate any in-flight doStart so a spawn completing after this
// point reaps itself instead of resurrecting the provider.
this.startEpoch++;
if (this.exitHandler) {
process.removeListener("exit", this.exitHandler);
this.exitHandler = null;
}
if (this.server) {
this.server.close();
this.server = null;
+1 -1
View File
@@ -315,6 +315,6 @@ export interface OpenCodeConfig extends AIProviderConfig {
type: "opencode-sdk";
/** Hostname for the OpenCode server. Default: "127.0.0.1". */
hostname?: string;
/** Port for the OpenCode server. Default: 4096. */
/** Port for the spawned OpenCode server. Default: 0 (OS-assigned free port). */
port?: number;
}
+14 -4
View File
@@ -84,17 +84,27 @@ export async function createAIRuntime(options: CreateAIRuntimeOptions = {}): Pro
}
try {
const { OpenCodeProvider } = await import("@plannotator/ai/providers/opencode-sdk");
await import("@plannotator/ai/providers/opencode-sdk");
const opencodePath = Bun.which("opencode");
if (opencodePath) {
const provider = await createProvider({
type: "opencode-sdk",
cwd,
});
if (provider instanceof OpenCodeProvider) {
modelDiscovery.push(provider.fetchModels().catch(() => {}));
const providerId = registry.register(provider);
// Deferred like Codex: fetchModels spawns `opencode serve`, so it must
// NOT run eagerly at startup — that spawned a server on every session
// for every user with opencode installed, and interrupted sessions
// orphaned it. The initializer runs on first explicit activation
// (?activate= from the model picker) or first opencode session.
if ("fetchModels" in provider) {
providerInitializers.set(
providerId,
createBestEffortOnce(
() => (provider as { fetchModels: () => Promise<void> }).fetchModels(),
),
);
}
registry.register(provider);
}
} catch {
// OpenCode not available.