From 52559098528be0bdd5ce5a2d46626cad6a15d413 Mon Sep 17 00:00:00 2001 From: Justin Carper Date: Thu, 23 Jul 2026 16:09:23 -0500 Subject: [PATCH] fix(provider): recycle sidecar child after idle and terminal errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cursor's SDK memoizes its streaming transport with no reconnect. After an idle gap (15-60min+), the backend drops the session and every later run ends `status:"error"` until the process restarts — /new doesn't help because the state lives in the long-lived Node sidecar child. Recycle the child after 10 minutes idle (below the shortest reported onset) and after any terminal run error. Pooled agents resume from Cursor's checkpoint store on the next turn, same path as an opencode restart. Complements #52's per-agent resume retry, which cannot heal a dead transport shared by every subsequent agent in the same child. --- README.md | 9 +++ src/provider/sidecar-client.ts | 81 +++++++++++++++++++++++++- test/fixtures/fake-cursor-sdk.mjs | 19 +++++++ test/sidecar.test.ts | 94 ++++++++++++++++++++++++++++++- 4 files changed, 198 insertions(+), 5 deletions(-) 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); + }); + }); });