From 4ffedad69d51de219f973fdcb9b71ddcf3ff6e36 Mon Sep 17 00:00:00 2001 From: Justin Carper Date: Thu, 30 Jul 2026 10:27:58 -0500 Subject: [PATCH 1/2] fix(provider): use a larger stall budget while a Cursor tool is in flight The stream watchdog re-armed only from push(), which fires for the ten mapped update types. A long shell command, build, or test suite streams nothing between tool-call-started and tool-call-completed, so a healthy run was cancelled at the 60s budget and the turn lost. Split the budget in two: an idle budget (OPENCODE_CURSOR_STALL_MS, raised 60000 -> 120000) and a tool-phase budget applied while at least one tool call is open (OPENCODE_CURSOR_TOOL_STALL_MS, default 600000, 0 disables just that bound). A tool-phase stall stays terminal and now names the tool still in flight. Also re-arm on any raw SDK update rather than only mapped ones, so progress/heartbeat types the plugin does not model still count as liveness. The re-arm runs after the switch so it observes post-mutation openTools state and always selects the correct budget. Reconcile openTools on turn-ended and on the forced resend: a dropped or differently-keyed completion would otherwise pin the turn to the 10-minute budget and name a tool that had already finished. Harden env parsing: Number("abc") is NaN and NaN <= 0 is false, so the old guard passed and setTimeout(fn, NaN) fired immediately, stalling every turn. Non-finite values now fall back to the default; an empty string still disables, preserving the existing escape hatch. --- CHANGELOG.md | 14 ++ README.md | 21 ++- src/provider/agent-events.ts | 65 +++++++- test/agent-events.test.ts | 310 +++++++++++++++++++++++++++++++++++ 4 files changed, 400 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f29682d..37e43fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,20 @@ All notable changes to this project will be documented in this file. own interceptor forwarding over the existing JSONL protocol) and re-emitted as structured opencode logs instead of raw terminal noise. Every other `console.log` call passes through unchanged. +- **Fixed: the stream watchdog killed healthy runs during long tool execution.** The watchdog + re-armed only on mapped event types, so a long shell command, build, or test suite that streamed + nothing for 60s was cancelled and the turn lost. It now uses two budgets — an idle budget + (`OPENCODE_CURSOR_STALL_MS`, default raised to `120000`) and a larger tool-phase budget + (`OPENCODE_CURSOR_TOOL_STALL_MS`, default `600000`) applied while a tool call is in flight — and + re-arms on **any** SDK update, including types the plugin doesn't model (progress/heartbeats). A + tool-phase stall is terminal and names the in-flight tool. `OPENCODE_CURSOR_STALL_MS=0` still + disables the whole watchdog; the tool-phase bound is independently disabled with + `OPENCODE_CURSOR_TOOL_STALL_MS=0`. Open tool calls are reconciled on `turn-ended` and on a forced + resend, so a dropped completion can't pin a turn to the 10-minute budget. +- **Fixed: a non-numeric `OPENCODE_CURSOR_STALL_MS` stalled every turn immediately.** + `Number("abc")` is `NaN`; `NaN <= 0` is `false`, so the guard passed and `setTimeout(fn, NaN)` + fired at once. Env parsing now falls back to the default for non-finite values (an empty string + still disables, preserving the historical escape hatch). ## [0.6.2] — 2026-07-28 diff --git a/README.md b/README.md index 57554ed..97c7527 100644 --- a/README.md +++ b/README.md @@ -192,7 +192,8 @@ See [SECURITY.md](./SECURITY.md) for the full threat model. | `OPENCODE_CURSOR_MODEL_CACHE_TTL_MS` | `86400000` | Model-list cache lifetime (ms) | | `OPENCODE_CURSOR_DEBUG` | — | Set to `1` for trace logging on stderr | | `OPENCODE_CURSOR_TRANSPORT` | — | Force a transport: `http1` \| `http2-direct` \| `sidecar` — see [Transport](#transport) | -| `OPENCODE_CURSOR_STALL_MS` | `60000` | Stream watchdog timeout (ms); `0` disables — see [Reliability](#reliability) | +| `OPENCODE_CURSOR_STALL_MS` | `120000` | Idle stream-watchdog timeout in ms (no tool call open). `0` disables the whole watchdog; an empty string also disables — see [Reliability](#reliability) | +| `OPENCODE_CURSOR_TOOL_STALL_MS` | `600000` | Stream-watchdog timeout in ms while a tool call is in flight (e.g. a long build or test suite). `0` disables the bound during tool execution only — see [Reliability](#reliability) | | `OPENCODE_CURSOR_SIDECAR` | — | Legacy: `1` maps to `sidecar`, `0` maps to `http2-direct` (superseded by `OPENCODE_CURSOR_TRANSPORT`) | | `OPENCODE_CURSOR_TOOL_INPUT_STREAM` | on | Set to `0` to disable live tool-input streaming (`tool-input-start`/`-delta`/`-end` parts) | @@ -378,10 +379,20 @@ The provider classifies Cursor SDK errors into typed kinds (`agent-not-found`, ` Sends carry an idempotency key so a retry is a server-side dedupe, not a duplicate turn. -A **stream watchdog** guards against a wedged run that streams nothing: if no event arrives within -`OPENCODE_CURSOR_STALL_MS` (default `60000`), a pre-first-event stall cancels and force-resends -once; a stall after partial output is surfaced as a terminal error rather than re-emitting the -already-yielded prefix. Set `OPENCODE_CURSOR_STALL_MS=0` to disable. +A **stream watchdog** guards against a wedged run that streams nothing. It uses two budgets: + +- **Idle** (`OPENCODE_CURSOR_STALL_MS`, default `120000`): when no tool call is open. A + pre-first-event stall cancels and force-resends once; a stall after partial output is surfaced + as a terminal error rather than re-emitting the already-yielded prefix. +- **Tool-phase** (`OPENCODE_CURSOR_TOOL_STALL_MS`, default `600000`): while at least one Cursor + tool call is in flight. A long shell command, build, or test suite legitimately streams nothing + for minutes; the larger budget stops a healthy run from being killed mid-tool. A tool-phase + stall is terminal and names the in-flight tool. Set `0` to disable the bound during tool + execution only. + +The watchdog re-arms on **any** SDK update — including types the plugin doesn't model — so +progress/heartbeat updates count as liveness. Set `OPENCODE_CURSOR_STALL_MS=0` to disable the whole +watchdog (an empty string also disables, for backward compatibility). ## Troubleshooting diff --git a/src/provider/agent-events.ts b/src/provider/agent-events.ts index 6099124..12c53eb 100644 --- a/src/provider/agent-events.ts +++ b/src/provider/agent-events.ts @@ -65,6 +65,20 @@ function toolDisplayName(toolCall: ({ type?: string } & Record) | u return toolCall.type ?? "tool"; } +/** + * Parse a millisecond env var, falling back when unset. An empty string is + * treated as `0` (preserving the historical "set to empty to disable" behavior + * of `OPENCODE_CURSOR_STALL_MS`); any other non-finite value falls back to the + * default so a typo can't arm `setTimeout(fn, NaN)` (which fires immediately). + */ +function envMs(name: string, fallback: number): number { + const raw = process.env[name]; + if (raw === undefined) return fallback; + if (raw === "") return 0; + const n = Number(raw); + return Number.isFinite(n) ? n : fallback; +} + /** * Stream a single turn on an already-acquired Cursor agent and yield normalized * events. The agent's lifecycle (create/resume/close) is owned by the caller @@ -86,13 +100,23 @@ export async function* streamAgentTurn( const debug = process.env.OPENCODE_CURSOR_DEBUG === "1"; const counts: Record = {}; - // Stall watchdog: if no event arrives within stallMs, cancel the wedged run - // and force-resend once (pre-first-event only). `0` disables. - const stallMs = Number(process.env.OPENCODE_CURSOR_STALL_MS ?? 60_000); + // Stall watchdog. Two budgets: + // - stallMs: idle budget (no tool call open). `0` disables the whole + // watchdog, matching the historical single-knob behavior. + // - toolStallMs: budget while at least one tool call is in flight. A long + // shell command or test suite legitimately streams nothing for minutes; + // killing it at the idle budget was a real-work-destroying false stall. + // `0` disables the bound during tool execution only. Default 10 min. + const stallMs = envMs("OPENCODE_CURSOR_STALL_MS", 120_000); + const toolStallMs = envMs("OPENCODE_CURSOR_TOOL_STALL_MS", 600_000); let stallTimer: ReturnType | undefined; let forced = false; let anyEvent = false; + // Open tool calls: callId -> display name. Lets the stall message name the + // culprit and lets armWatchdog pick the larger budget while a tool runs. + const openTools = new Map(); + const push = (event: CursorEvent) => { anyEvent = true; queue.push(event); @@ -103,10 +127,15 @@ export async function* streamAgentTurn( const armWatchdog = () => { if (stallMs <= 0 || finished) return; + const budget = openTools.size > 0 ? toolStallMs : stallMs; if (stallTimer) clearTimeout(stallTimer); + if (budget <= 0) { + stallTimer = undefined; + return; + } stallTimer = setTimeout(() => { void onStall(); - }, stallMs); + }, budget); stallTimer.unref?.(); }; @@ -136,6 +165,9 @@ export async function* streamAgentTurn( }); break; case "tool-call-started": + // Track the open call BEFORE push() re-arms the watchdog with the + // larger tool budget. + openTools.set(String(update.callId), toolDisplayName(update.toolCall)); push({ type: "tool-call", id: String(update.callId), @@ -144,6 +176,7 @@ export async function* streamAgentTurn( }); break; case "tool-call-completed": { + openTools.delete(String(update.callId)); const tool = update.toolCall ?? {}; const result = tool.result; // MCP failures often arrive as {status:"success", value:{isError:true}} @@ -159,12 +192,22 @@ export async function* streamAgentTurn( break; } case "turn-ended": + // Reconcile: a dropped or differently-keyed `tool-call-completed` + // would otherwise leave an entry pinned here, holding the turn on the + // 10-minute tool budget and naming a tool that already finished. + openTools.clear(); if (update.usage) { const summed = addUsage(options.usageBase, update.usage as CursorUsage); if (summed) push({ type: "usage", usage: summed }); } break; } + // Any SDK update proves the stream is alive — including types we don't map + // (progress, heartbeats, future types), which never reach `push()`. Armed + // AFTER the switch so it observes the post-mutation `openTools` state and + // therefore always selects the correct budget (a `turn-ended` that cleared + // the map must fall back to the idle budget immediately). + armWatchdog(); }; const runHolder: { run?: AgentRunLike } = {}; @@ -264,7 +307,16 @@ export async function* streamAgentTurn( // A stall AFTER partial output is terminal: force-resending would // re-emit the already-yielded prefix. Cancel the wedged run and surface // the stall instead. - await failTerminal(`Cursor run stalled (no events for ${stallMs}ms)`); + const budget = openTools.size > 0 ? toolStallMs : stallMs; + const inFlight = [...openTools.values()]; + const toolHint = + inFlight.length > 0 + ? `; tool${inFlight.length > 1 ? "s" : ""} ${inFlight.map((n) => `"${n}"`).join(", ")} still in flight` + : ""; + const knob = openTools.size > 0 ? "OPENCODE_CURSOR_TOOL_STALL_MS" : "OPENCODE_CURSOR_STALL_MS"; + await failTerminal( + `Cursor run stalled (no events for ${budget}ms${toolHint}). Raise ${knob} (or set 0 to disable) if this legitimately runs longer.`, + ); return; } if (forced) { @@ -278,6 +330,9 @@ export async function* streamAgentTurn( } catch { /* best effort */ } + // The abandoned run's tool calls will never complete; don't let them hold + // the resend on the tool budget. + openTools.clear(); armWatchdog(); startRun(true); }; diff --git a/test/agent-events.test.ts b/test/agent-events.test.ts index 5849b0e..12da299 100644 --- a/test/agent-events.test.ts +++ b/test/agent-events.test.ts @@ -512,4 +512,314 @@ describe("stream watchdog", () => { vi.useRealTimers(); } }); + + /** + * Drive a turn against an agent whose run only settles on cancel, tracking + * the outcome without blocking so a test can assert "still pending" partway + * through. `emit` receives the SDK `onDelta` hook, so updates can be + * scheduled at t>0 with `setTimeout` under fake timers. + */ + function watchdogTurn(opts: { + emit?: (onDelta: OnDelta) => void; + abortSignal?: AbortSignal; + }): { outcome: { state: "pending" | "stalled" | "done"; error?: unknown }; promise: Promise } { + const outcome: { state: "pending" | "stalled" | "done"; error?: unknown } = { + state: "pending", + }; + const agent: AgentLike = { + agentId: "agent-wd-harness", + send: async (_m: unknown, sendOptions?: Record) => { + const onDelta = sendOptions?.["onDelta"] as OnDelta | undefined; + if (onDelta) opts.emit?.(onDelta); + let cancelled = false; + return { + wait: () => + new Promise((resolve) => { + const t = setInterval(() => { + if (cancelled) { + clearInterval(t); + resolve({ status: "cancelled" }); + } + }, 10); + }), + cancel: async () => { + cancelled = true; + }, + } as never; + }, + close: () => {}, + } as unknown as AgentLike; + + const promise = (async () => { + try { + for await (const _e of streamAgentTurn(agent, MESSAGE, { + mode: "agent", + ...(opts.abortSignal ? { abortSignal: opts.abortSignal } : {}), + })) { + /* drain */ + } + outcome.state = "done"; + } catch (err) { + outcome.state = "stalled"; + outcome.error = err; + } + })(); + promise.catch(() => {}); + return { outcome, promise }; + } + + /** An update that opens a tool call under `callId`. */ + function toolStarted(callId: string, type = "shell") { + return { update: { type: "tool-call-started", callId, toolCall: { type } } }; + } + + it("uses the larger tool budget while a tool call is in flight", async () => { + vi.useFakeTimers(); + try { + process.env.OPENCODE_CURSOR_STALL_MS = "1000"; + process.env.OPENCODE_CURSOR_TOOL_STALL_MS = "5000"; + const { outcome, promise } = watchdogTurn({ + emit: (onDelta) => onDelta(toolStarted("c1")), + }); + // Well past the idle budget (1000). This assertion is what fails if + // budget selection is reverted to always using `stallMs`. + await vi.advanceTimersByTimeAsync(2_000); + expect(outcome.state).toBe("pending"); + // Past the tool budget (5000): terminal, and it names the tool. + await vi.advanceTimersByTimeAsync(3_500); + await promise; + expect(outcome.state).toBe("stalled"); + expect(String(outcome.error)).toMatch(/tool "shell" still in flight/); + } finally { + delete process.env.OPENCODE_CURSOR_STALL_MS; + delete process.env.OPENCODE_CURSOR_TOOL_STALL_MS; + vi.useRealTimers(); + } + }); + + it("negative control: with no tool open the idle budget governs", async () => { + vi.useFakeTimers(); + try { + process.env.OPENCODE_CURSOR_STALL_MS = "1000"; + process.env.OPENCODE_CURSOR_TOOL_STALL_MS = "5000"; + // A text-delta sets `anyEvent` so the stall is terminal rather than a + // pre-first-event force-resend, but opens no tool. + const { outcome, promise } = watchdogTurn({ + emit: (onDelta) => onDelta({ update: { type: "text-delta", text: "x" } }), + }); + await vi.advanceTimersByTimeAsync(2_000); + await promise; + // Same 2000ms window that stayed pending above now stalls, proving the + // larger budget is applied only while a tool is open. + expect(outcome.state).toBe("stalled"); + expect(String(outcome.error)).not.toMatch(/in flight/); + } finally { + delete process.env.OPENCODE_CURSOR_STALL_MS; + delete process.env.OPENCODE_CURSOR_TOOL_STALL_MS; + vi.useRealTimers(); + } + }); + + it("restores the idle budget once the tool call completes", async () => { + vi.useFakeTimers(); + try { + process.env.OPENCODE_CURSOR_STALL_MS = "1000"; + process.env.OPENCODE_CURSOR_TOOL_STALL_MS = "5000"; + const { outcome, promise } = watchdogTurn({ + emit: (onDelta) => { + onDelta(toolStarted("c1")); + onDelta({ + update: { + type: "tool-call-completed", + callId: "c1", + toolCall: { type: "shell", result: { status: "success" } }, + }, + }); + }, + }); + // Tool closed, so the idle budget (1000) applies again: stalls inside + // the 2000ms window. Fails if `openTools.delete` is removed (the turn + // would stay pending on the 5000ms tool budget). + await vi.advanceTimersByTimeAsync(2_000); + await promise; + expect(outcome.state).toBe("stalled"); + expect(String(outcome.error)).not.toMatch(/in flight/); + } finally { + delete process.env.OPENCODE_CURSOR_STALL_MS; + delete process.env.OPENCODE_CURSOR_TOOL_STALL_MS; + vi.useRealTimers(); + } + }); + + it("clears open tool calls on turn-ended so a dropped completion can't pin the tool budget", async () => { + vi.useFakeTimers(); + try { + process.env.OPENCODE_CURSOR_STALL_MS = "1000"; + process.env.OPENCODE_CURSOR_TOOL_STALL_MS = "5000"; + const { outcome, promise } = watchdogTurn({ + emit: (onDelta) => { + onDelta(toolStarted("c1")); + // Note: no `tool-call-completed` — simulates a dropped or + // differently-keyed completion. + onDelta({ update: { type: "turn-ended" } }); + }, + }); + await vi.advanceTimersByTimeAsync(2_000); + await promise; + expect(outcome.state).toBe("stalled"); + expect(String(outcome.error)).not.toMatch(/in flight/); + } finally { + delete process.env.OPENCODE_CURSOR_STALL_MS; + delete process.env.OPENCODE_CURSOR_TOOL_STALL_MS; + vi.useRealTimers(); + } + }); + + it("re-arms on an unmapped update type (raw SDK activity counts as liveness)", async () => { + vi.useFakeTimers(); + try { + process.env.OPENCODE_CURSOR_STALL_MS = "1000"; + const { outcome, promise } = watchdogTurn({ + emit: (onDelta) => { + // t=0: sets anyEvent, arms deadline t=1000. + onDelta({ update: { type: "text-delta", text: "x" } }); + // t=800: a type the plugin does not map, so it never reaches + // push(). With the raw re-arm it pushes the deadline to t=1800. + setTimeout(() => onDelta({ update: { type: "some-future-type" } }), 800); + }, + }); + // Past the original t=1000 deadline but inside the re-armed t=1800 one. + // Fails if the post-switch armWatchdog() is removed. + await vi.advanceTimersByTimeAsync(1_500); + expect(outcome.state).toBe("pending"); + // Past the re-armed deadline. + await vi.advanceTimersByTimeAsync(600); + await promise; + expect(outcome.state).toBe("stalled"); + } finally { + delete process.env.OPENCODE_CURSOR_STALL_MS; + vi.useRealTimers(); + } + }); + + it("negative control: without the intervening update the same schedule stalls", async () => { + vi.useFakeTimers(); + try { + process.env.OPENCODE_CURSOR_STALL_MS = "1000"; + const { outcome, promise } = watchdogTurn({ + emit: (onDelta) => onDelta({ update: { type: "text-delta", text: "x" } }), + }); + // Identical timeline to the test above, minus the t=800 update: the + // t=1000 deadline stands, so 1500ms is enough to stall. This proves the + // preceding test's "pending" result is caused by the re-arm. + await vi.advanceTimersByTimeAsync(1_500); + await promise; + expect(outcome.state).toBe("stalled"); + } finally { + delete process.env.OPENCODE_CURSOR_STALL_MS; + vi.useRealTimers(); + } + }); + + it("falls back to the default when STALL_MS is not a number", async () => { + vi.useFakeTimers(); + try { + // Number("abc") is NaN. NaN <= 0 is false, so the old code armed + // setTimeout(fn, NaN), which fires immediately and stalled every turn. + process.env.OPENCODE_CURSOR_STALL_MS = "abc"; + const ac = new AbortController(); + const { outcome, promise } = watchdogTurn({ abortSignal: ac.signal }); + await vi.advanceTimersByTimeAsync(500); + expect(outcome.state).toBe("pending"); + ac.abort(); + await vi.advanceTimersByTimeAsync(50); + await promise; + } finally { + delete process.env.OPENCODE_CURSOR_STALL_MS; + vi.useRealTimers(); + } + }); + + it("negative control: a valid small STALL_MS does stall inside that window", async () => { + vi.useFakeTimers(); + try { + process.env.OPENCODE_CURSOR_STALL_MS = "50"; + const { outcome, promise } = watchdogTurn({}); + // Proves the harness can observe a stall at this timescale, so the + // "pending" result above is the fallback and not an artifact. + await vi.advanceTimersByTimeAsync(500); + await promise; + expect(outcome.state).toBe("stalled"); + } finally { + delete process.env.OPENCODE_CURSOR_STALL_MS; + vi.useRealTimers(); + } + }); + + it("treats an empty STALL_MS as fully disabled (historical escape hatch)", async () => { + vi.useFakeTimers(); + try { + process.env.OPENCODE_CURSOR_STALL_MS = ""; + const ac = new AbortController(); + // The text-delta sets `anyEvent`, so any stall would be TERMINAL rather + // than a pre-first-event force-resend (which would leave the turn + // "pending" and make this assertion vacuous). + const { outcome, promise } = watchdogTurn({ + abortSignal: ac.signal, + emit: (onDelta) => onDelta({ update: { type: "text-delta", text: "x" } }), + }); + // Past the 120000 fallback: had empty fallen back to the default + // instead of disabling, this would have stalled. + await vi.advanceTimersByTimeAsync(130_000); + expect(outcome.state).toBe("pending"); + ac.abort(); + await vi.advanceTimersByTimeAsync(50); + await promise; + } finally { + delete process.env.OPENCODE_CURSOR_STALL_MS; + vi.useRealTimers(); + } + }); + + it("applies the raised 120000 idle default when STALL_MS is unset", async () => { + vi.useFakeTimers(); + try { + delete process.env.OPENCODE_CURSOR_STALL_MS; + const { outcome, promise } = watchdogTurn({ + emit: (onDelta) => onDelta({ update: { type: "text-delta", text: "x" } }), + }); + // Past the old 60000 default, inside the new 120000 one. + await vi.advanceTimersByTimeAsync(90_000); + expect(outcome.state).toBe("pending"); + // Past the new default. + await vi.advanceTimersByTimeAsync(35_000); + await promise; + expect(outcome.state).toBe("stalled"); + } finally { + vi.useRealTimers(); + } + }); + + it("TOOL_STALL_MS=0 disables the bound during tool execution", async () => { + vi.useFakeTimers(); + try { + process.env.OPENCODE_CURSOR_STALL_MS = "1000"; + process.env.OPENCODE_CURSOR_TOOL_STALL_MS = "0"; + const ac = new AbortController(); + const { outcome, promise } = watchdogTurn({ + emit: (onDelta) => onDelta(toolStarted("c1")), + abortSignal: ac.signal, + }); + // Far past the idle budget with a tool open and the tool bound off. + await vi.advanceTimersByTimeAsync(4_000); + expect(outcome.state).toBe("pending"); + ac.abort(); + await vi.advanceTimersByTimeAsync(50); + await promise; + } finally { + delete process.env.OPENCODE_CURSOR_STALL_MS; + delete process.env.OPENCODE_CURSOR_TOOL_STALL_MS; + vi.useRealTimers(); + } + }); }); From 0556e8cb7f0821f2c132296775fbef3168365d0c Mon Sep 17 00:00:00 2001 From: Justin Carper Date: Thu, 30 Jul 2026 12:54:17 -0500 Subject: [PATCH 2/2] fix(provider): cap stall budgets at setTimeout's 32-bit ceiling A setTimeout delay is stored as a signed 32-bit int, so a value above 2147483647 overflows and Node silently clamps it to 1ms. envMs rejected NaN and Infinity but passed every other finite value straight through, so OPENCODE_CURSOR_TOOL_STALL_MS=999999999999 stalled every tool-bearing turn within milliseconds while reporting "no events for 999999999999ms". The tool-phase stall message tells operators to raise that same variable, so the trap was reachable by following the plugin's own advice. Both budgets are now capped at 2147483647. Verified against a live agent: with the over-large budget set, a 150s shell call died after 5.6s before this change and completes normally after it. --- CHANGELOG.md | 6 +++ src/provider/agent-events.ts | 14 ++++++- test/agent-events.test.ts | 71 ++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37e43fa..4c91ae3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,12 @@ All notable changes to this project will be documented in this file. `Number("abc")` is `NaN`; `NaN <= 0` is `false`, so the guard passed and `setTimeout(fn, NaN)` fired at once. Env parsing now falls back to the default for non-finite values (an empty string still disables, preserving the historical escape hatch). +- **Fixed: an over-large stall budget overflowed to a ~1 ms deadline.** A `setTimeout` delay is + stored as a signed 32-bit int, so anything above `2147483647` is silently clamped to `1` — and the + tool-phase stall message tells operators to *raise* `OPENCODE_CURSOR_TOOL_STALL_MS`, making the + trap reachable by following the plugin's own advice. Setting it to e.g. `999999999999` stalled + every tool-bearing turn within milliseconds while reporting `no events for 999999999999ms`. Both + budgets are now capped at `2147483647`. ## [0.6.2] — 2026-07-28 diff --git a/src/provider/agent-events.ts b/src/provider/agent-events.ts index 12c53eb..e6d9f05 100644 --- a/src/provider/agent-events.ts +++ b/src/provider/agent-events.ts @@ -65,18 +65,30 @@ function toolDisplayName(toolCall: ({ type?: string } & Record) | u return toolCall.type ?? "tool"; } +/** + * Node stores a timer delay in a signed 32-bit int; anything larger overflows + * and is silently clamped to `1`. An operator following the tool-phase stall + * message's own advice to "raise OPENCODE_CURSOR_TOOL_STALL_MS" could therefore + * pick a number so large that every tool-bearing turn stalls within a + * millisecond — the exact failure the budget is meant to prevent. Cap instead. + */ +const MAX_TIMEOUT_MS = 2_147_483_647; + /** * Parse a millisecond env var, falling back when unset. An empty string is * treated as `0` (preserving the historical "set to empty to disable" behavior * of `OPENCODE_CURSOR_STALL_MS`); any other non-finite value falls back to the * default so a typo can't arm `setTimeout(fn, NaN)` (which fires immediately). + * Finite values are capped at {@link MAX_TIMEOUT_MS} so an over-large budget + * degrades to "as long as a timer can express" rather than to ~instant. */ function envMs(name: string, fallback: number): number { const raw = process.env[name]; if (raw === undefined) return fallback; if (raw === "") return 0; const n = Number(raw); - return Number.isFinite(n) ? n : fallback; + if (!Number.isFinite(n)) return fallback; + return Math.min(n, MAX_TIMEOUT_MS); } /** diff --git a/test/agent-events.test.ts b/test/agent-events.test.ts index 12da299..185cd48 100644 --- a/test/agent-events.test.ts +++ b/test/agent-events.test.ts @@ -822,4 +822,75 @@ describe("stream watchdog", () => { vi.useRealTimers(); } }); + + it("caps an over-large TOOL_STALL_MS instead of overflowing to ~instant", async () => { + vi.useFakeTimers(); + try { + // A timer delay above 2^31-1 overflows and Node clamps it to 1ms, so + // without the cap this budget stalls the turn almost immediately — + // while reporting "no events for 999999999999ms". The stall message + // tells operators to raise this very variable, so the trap is reachable + // by following the plugin's own advice. + process.env.OPENCODE_CURSOR_STALL_MS = "1000"; + process.env.OPENCODE_CURSOR_TOOL_STALL_MS = "999999999999"; + const ac = new AbortController(); + const { outcome, promise } = watchdogTurn({ + emit: (onDelta) => onDelta(toolStarted("c1")), + abortSignal: ac.signal, + }); + // Past the idle budget AND past the 600000 tool default, so the result + // can be neither the idle budget nor a silent fall back to the default. + // (It cannot be the fallback anyway — that needs a non-finite value — + // but this rules it out observationally rather than by argument.) + await vi.advanceTimersByTimeAsync(601_000); + expect(outcome.state).toBe("pending"); + ac.abort(); + await vi.advanceTimersByTimeAsync(50); + await promise; + } finally { + delete process.env.OPENCODE_CURSOR_STALL_MS; + delete process.env.OPENCODE_CURSOR_TOOL_STALL_MS; + vi.useRealTimers(); + } + }); + + it("negative control: a large tool budget still stalls once exceeded", async () => { + vi.useFakeTimers(); + try { + // Same timescale as the cap test, with a budget the harness can outrun. + // Proves that test's "pending" reflects a deadline further out and not + // a watchdog that stopped arming, and that a stall is observable here. + process.env.OPENCODE_CURSOR_STALL_MS = "1000"; + process.env.OPENCODE_CURSOR_TOOL_STALL_MS = "600000"; + const { outcome, promise } = watchdogTurn({ + emit: (onDelta) => onDelta(toolStarted("c1")), + }); + await vi.advanceTimersByTimeAsync(601_000); + await promise; + expect(outcome.state).toBe("stalled"); + expect(String(outcome.error)).toMatch(/tool "shell" still in flight/); + } finally { + delete process.env.OPENCODE_CURSOR_STALL_MS; + delete process.env.OPENCODE_CURSOR_TOOL_STALL_MS; + vi.useRealTimers(); + } + }); + + it("caps an over-large idle STALL_MS as well", async () => { + vi.useFakeTimers(); + try { + process.env.OPENCODE_CURSOR_STALL_MS = "1e30"; + const ac = new AbortController(); + const { outcome, promise } = watchdogTurn({ abortSignal: ac.signal }); + // Without the cap this is a 1ms deadline and every turn dies here. + await vi.advanceTimersByTimeAsync(1_000); + expect(outcome.state).toBe("pending"); + ac.abort(); + await vi.advanceTimersByTimeAsync(50); + await promise; + } finally { + delete process.env.OPENCODE_CURSOR_STALL_MS; + vi.useRealTimers(); + } + }); });