diff --git a/README.md b/README.md index 6b31029..4231789 100644 --- a/README.md +++ b/README.md @@ -315,6 +315,11 @@ sidecar remains as a rollback fallback. | `http2-direct` | in-process, SDK's default HTTP/2 | under Node (tests, scripts, non-Bun hosts) | | `sidecar` | spawned Node child hosting the SDK | never (rollback only) | +When the `sidecar` transport is in use, its child is recycled after 10 minutes idle and after any +turn that ends with a terminal error: the SDK's cached streaming connection does not recover once +Cursor drops an idle session, so a fresh child (fresh connection) is the only reliable cure. Pooled +agents resume from their checkpoint on the next turn, exactly as they do across an opencode restart. + Resolution order: the `transport` provider option → `OPENCODE_CURSOR_TRANSPORT` → legacy `OPENCODE_CURSOR_SIDECAR` (`1`→`sidecar`, `0`→`http2-direct`) → the per-runtime default above. @@ -351,6 +356,10 @@ already-yielded prefix. Set `OPENCODE_CURSOR_STALL_MS=0` to disable. `http2-direct` transport was forced under Bun. Unset `OPENCODE_CURSOR_TRANSPORT` (defaults to the Bun-safe `http1`), or roll back with `OPENCODE_CURSOR_TRANSPORT=sidecar` (needs Node.js 22.13+ on `PATH`). +- **On `OPENCODE_CURSOR_TRANSPORT=sidecar`, every message fails with `Cursor run ended with status + "error"` after a long idle.** Cursor dropped the idle session and the sidecar's cached connection + couldn't recover. The sidecar now recycles its child after idle periods and terminal errors, so + this self-heals on the next turn; update to a release including that fix. - **Plugin enabled but no `cursor` provider/models appear, or you see a stale-version warning.** opencode caches the `@latest` plugin install on first use and never refreshes it. Exit opencode, delete `~/.cache/opencode/packages/@stablekernel/opencode-cursor@latest` diff --git a/src/provider/sidecar-client.ts b/src/provider/sidecar-client.ts index 2a4687a..4706e65 100644 --- a/src/provider/sidecar-client.ts +++ b/src/provider/sidecar-client.ts @@ -39,8 +39,25 @@ export interface SidecarClientOptions { env?: Record; /** Mirror child stderr to this process (debug aid). */ debug?: boolean; + /** + * Recycle the child after this much idle time (default + * {@link DEFAULT_IDLE_RECYCLE_MS}). See the field docs on `stale` for why. + */ + idleRecycleMs?: number; } +/** + * Idle lifetime after which the child is recycled. The SDK inside the child + * memoizes its streaming transport (and auth token) at module scope with no + * reconnect logic, so a backend session dropped while idle leaves every later + * run ending `status:"error"` until the process restarts. 10 minutes sits + * comfortably below the shortest reported failure onset (15-30min); the + * respawn cost is a cheap Node spawn paid only after an idle gap, and pooled + * agents resume from Cursor's checkpoint store exactly as they do across an + * opencode restart. + */ +const DEFAULT_IDLE_RECYCLE_MS = 10 * 60 * 1000; + interface Pending { resolve: (msg: Record) => void; reject: (err: Error) => void; @@ -75,6 +92,14 @@ export class SidecarClient { private readonly pending = new Map(); private nextId = 1; private disposed = false; + /** + * Set when a run ends terminally bad (status:"error" or a stream error). + * The SDK's memoized transport does not recover from a dead backend + * session, so the child is recycled before the next request instead of + * failing every turn until the whole process is restarted. + */ + private stale = false; + private idleTimer: ReturnType | undefined; constructor(options: SidecarClientOptions) { this.options = options; @@ -83,7 +108,9 @@ export class SidecarClient { /** Spawn (or reuse) the child process. */ private ensureChild(): ChildProcessByStdio { if (this.disposed) throw new Error("cursor sidecar client disposed"); + if (this.child && this.stale && this.pending.size === 0) this.recycleChild(); if (this.child) return this.child; + this.stale = false; const child = spawn(this.options.nodePath ?? "node", [this.options.scriptPath], { stdio: ["pipe", "pipe", "pipe"], @@ -99,14 +126,20 @@ export class SidecarClient { } }); child.on("exit", (code) => { - this.failAll(new Error(`cursor sidecar exited (code ${code ?? "unknown"})`)); + // A recycled child's exit arrives after its replacement spawned; it + // must not clobber the new child or reject its in-flight requests. + if (this.child !== child) return; + // Clear before failAll so updateRefs doesn't arm the idle timer (or + // re-unref pipes) against a child that's already gone. this.child = undefined; this.reader?.close(); this.reader = undefined; + this.failAll(new Error(`cursor sidecar exited (code ${code ?? "unknown"})`)); }); child.on("error", (err) => { - this.failAll(new Error(`cursor sidecar failed to start: ${err.message}`)); + if (this.child !== child) return; this.child = undefined; + this.failAll(new Error(`cursor sidecar failed to start: ${err.message}`)); }); this.updateRefs(); return child; @@ -129,9 +162,44 @@ export class SidecarClient { for (const target of refable) target.ref?.(); } else { for (const target of refable) target.unref?.(); + this.armIdleTimer(); } } + /** + * Kill the child so the next request spawns a fresh one (fresh SDK module + * state). Only called with nothing in flight; pooled agents are resumable, + * so nothing is lost. The exit handler's failAll no-ops on an empty pending + * map. + */ + private recycleChild(): void { + this.clearIdleTimer(); + this.stale = false; + const child = this.child; + this.child = undefined; + this.reader?.close(); + this.reader = undefined; + child?.kill(); + } + + /** + * Arm the idle-recycle timer. Unref'd like the child pipes so it can never + * hold the parent's event loop open (see updateRefs). + */ + private armIdleTimer(): void { + this.clearIdleTimer(); + if (this.disposed) return; + this.idleTimer = setTimeout(() => { + if (this.pending.size === 0) this.recycleChild(); + }, this.options.idleRecycleMs ?? DEFAULT_IDLE_RECYCLE_MS); + this.idleTimer.unref?.(); + } + + private clearIdleTimer(): void { + if (this.idleTimer) clearTimeout(this.idleTimer); + this.idleTimer = undefined; + } + private failAll(err: Error): void { for (const pending of this.pending.values()) { pending.onStreamError?.(err); @@ -162,12 +230,17 @@ export class SidecarClient { if (ev === "result") { this.pending.delete(id); this.updateRefs(); - pending.onResult?.(msg["result"] as { status: string; result?: string }); + const result = msg["result"] as { status: string; result?: string }; + // A terminally errored run marks the child stale: the SDK's memoized + // transport can't be trusted after this, so recycle before next use. + if (result.status === "error") this.stale = true; + pending.onResult?.(result); return; } if (ev === "error") { this.pending.delete(id); this.updateRefs(); + this.stale = true; pending.onStreamError?.(reviveError(msg["error"])); return; } @@ -190,6 +263,7 @@ export class SidecarClient { payload: Record, hooks?: Pick, ): Promise> { + this.clearIdleTimer(); const child = this.ensureChild(); const id = this.nextId++; return new Promise>((resolve, reject) => { @@ -275,6 +349,7 @@ export class SidecarClient { /** Kill the child and reject anything in flight. */ dispose(): void { this.disposed = true; + this.clearIdleTimer(); this.failAll(new Error("cursor sidecar client disposed")); this.reader?.close(); this.reader = undefined; diff --git a/test/fixtures/fake-cursor-sdk.mjs b/test/fixtures/fake-cursor-sdk.mjs index e5e9665..5cb965e 100644 --- a/test/fixtures/fake-cursor-sdk.mjs +++ b/test/fixtures/fake-cursor-sdk.mjs @@ -7,8 +7,21 @@ * "busy" -> send() rejects with AgentBusyError unless local.force is set * "rich" -> send() rejects with an error carrying status/code/isRetryable/helpUrl * "hang" -> run.wait() never resolves (until cancel(), which resolves cancelled) + * "error" -> run.wait() resolves {status:"error"} (mimics a dead backend session) * other -> emits one text-delta "echo:" update, wait() -> done: + * + * When FAKE_SDK_LOAD_LOG is set, each loading child appends its pid so tests + * can detect client-side child recycles (a new pid = a respawned child). */ +import { appendFileSync } from "node:fs"; + +if (process.env.FAKE_SDK_LOAD_LOG) { + try { + appendFileSync(process.env.FAKE_SDK_LOAD_LOG, `${Date.now()} ${process.pid}\n`); + } catch { + // best effort + } +} function makeAgent(agentId, options) { return { @@ -31,6 +44,12 @@ function makeAgent(agentId, options) { throw err; } sendOptions?.onDelta?.({ update: { type: "text-delta", text: `echo:${text}` } }); + if (text === "error") { + return { + wait: async () => ({ status: "error" }), + cancel: () => {}, + }; + } if (text === "hang") { let resolveWait; const waited = new Promise((resolve) => { diff --git a/test/sidecar.test.ts b/test/sidecar.test.ts index eebeb6e..316bf0e 100644 --- a/test/sidecar.test.ts +++ b/test/sidecar.test.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, it } from "vitest"; +import { readFileSync, rmSync } from "node:fs"; import { fileURLToPath } from "node:url"; import { SidecarClient } from "../src/provider/sidecar-client.js"; @@ -7,10 +8,16 @@ const FAKE_SDK = fileURLToPath(new URL("./fixtures/fake-cursor-sdk.mjs", import. const clients: SidecarClient[] = []; -function makeClient(): SidecarClient { +function makeClient(options?: { idleRecycleMs?: number; loadLog?: string }): SidecarClient { const client = new SidecarClient({ scriptPath: SCRIPT, - env: { OPENCODE_CURSOR_SDK_PATH: FAKE_SDK }, + ...(options?.idleRecycleMs !== undefined + ? { idleRecycleMs: options.idleRecycleMs } + : {}), + env: { + OPENCODE_CURSOR_SDK_PATH: FAKE_SDK, + ...(options?.loadLog ? { FAKE_SDK_LOAD_LOG: options.loadLog } : {}), + }, }); clients.push(client); return client; @@ -111,4 +118,87 @@ describe("SidecarClient", () => { client.dispose(); await expect(waited).rejects.toThrow(/sidecar/i); }); + + describe("child recycling", () => { + let loadLog: string; + let logSeq = 0; + + afterEach(() => { + rmSync(loadLog, { force: true }); + }); + + /** Fresh per-test log path (avoids cross-test timing bleed). */ + const nextLoadLog = (): string => { + logSeq += 1; + loadLog = fileURLToPath( + new URL(`./fixtures/.load-log-${process.pid}-${logSeq}`, import.meta.url), + ); + rmSync(loadLog, { force: true }); + return loadLog; + }; + + /** Distinct pids that loaded the fake SDK (one per spawned child). */ + const spawnedPids = (): string[] => [ + ...new Set( + readFileSync(loadLog, "utf8") + .split("\n") + .filter(Boolean) + .map((line) => line.split(" ")[1]!), + ), + ]; + + const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + + it("recycles the child after a run ends with status error", async () => { + const client = makeClient({ loadLog: nextLoadLog() }); + const agent = await client.createAgent(CREATE_OPTIONS); + const run = await agent.send({ type: "user", text: "error" }, { mode: "agent" }); + await expect(run.wait()).resolves.toMatchObject({ status: "error" }); + + // The next turn must run in a fresh child (fresh SDK transport state). + const next = await client.createAgent(CREATE_OPTIONS); + const run2 = await next.send({ type: "user", text: "ok" }, { mode: "agent" }); + await expect(run2.wait()).resolves.toMatchObject({ status: "finished" }); + + expect(spawnedPids()).toHaveLength(2); + }); + + it("recycles the child after the idle timeout", async () => { + const client = makeClient({ loadLog: nextLoadLog(), idleRecycleMs: 50 }); + await client.createAgent(CREATE_OPTIONS); + await sleep(150); // let the idle timer fire + await client.createAgent(CREATE_OPTIONS); + expect(spawnedPids()).toHaveLength(2); + }); + + it("keeps one child across healthy turns", async () => { + const client = makeClient({ loadLog: nextLoadLog(), idleRecycleMs: 60_000 }); + const agent = await client.createAgent(CREATE_OPTIONS); + const run = await agent.send({ type: "user", text: "ok" }, { mode: "agent" }); + await run.wait(); + await client.createAgent(CREATE_OPTIONS); + expect(spawnedPids()).toHaveLength(1); + }); + + it("never recycles while a sibling request is still in flight", async () => { + const client = makeClient({ loadLog: nextLoadLog() }); + const agent = await client.createAgent(CREATE_OPTIONS); + // A hung send keeps the child busy while another turn errors (stale). + const hung = await agent.send({ type: "user", text: "hang" }, { mode: "agent" }); + const errored = await agent.send({ type: "user", text: "error" }, { mode: "agent" }); + await expect(errored.wait()).resolves.toMatchObject({ status: "error" }); + + // Stale, but the hung send is still pending: no recycle yet. + await client.createAgent(CREATE_OPTIONS); + expect(spawnedPids()).toHaveLength(1); + + // Once it settles, the next request lands on a fresh child. + await hung.cancel(); + // Awaiting wait() guarantees the terminal event (and pending cleanup) + // has been processed before we assert the recycle. + await expect(hung.wait()).resolves.toMatchObject({ status: "cancelled" }); + await client.createAgent(CREATE_OPTIONS); + expect(spawnedPids()).toHaveLength(2); + }); + }); });