diff --git a/CHANGELOG.md b/CHANGELOG.md index 26b5e39..0de4880 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,45 @@ All notable changes to bare-agent are documented here. Format: [Keep a Changelog](https://keepachangelog.com/). Versioning: [SemVer](https://semver.org/). +## [0.35.0] - 2026-07-28 + +### Added + +- **BA-19 — a total call-duration deadline (`deadlineMs`) on the http(s) providers, beside the + BA-18 idle timeout.** BA-18's `timeoutMs` bounds socket **inactivity** (`req.setTimeout`), and by + design it resets on *any* socket activity — so a response that trickles a byte forever (a "zombie + stream") never trips it. Bytes keep arriving while the response never completes, and the call + hangs until the OS TCP timeout. An adopter observed one `generate()` run **274 minutes** and end + in `read ECONNRESET` (not a `TimeoutError`) — the reset is the proof of mechanism: bytes *were* + arriving for 4.5h, so the idle timer never fired. `timeoutMs` is the right bound for the quantity + it watches; nothing bounded *total* call duration. + - New `deadlineMs` option on all four providers — an absolute, **non-resetting** wall-clock + ceiling on the whole request. **Disabled by default** (a deliberately long single call — large + `maxTokens`, slow model — is legitimate; a default here would kill it). Overridable per call via + `generate(..., { deadlineMs })`; same disable idiom as `timeoutMs` (`0`/`Infinity`), and a + per-call `null`/`undefined` inherits the instance value. **Disable-edge is fail-loud, not + fail-silent:** an *unset* deadline resolves to disabled, but an *explicitly-set* garbage value + (`NaN`, a non-numeric string like `'30s'`) throws a `ValidationError` at resolve time rather than + silently disabling the deadline and running unbounded for hours — the idle bound can fall back to + its 10-min default on garbage because that default *is* a real bound, but the deadline has no safe + default to fall back to, so a config mistake must surface (review finding 1). + - On trip, `generate()` rejects with a **terminal `TimeoutError`** distinguishable from the idle + trip: `code: 'EDEADLINE'`, `context.bound: 'deadline'`, and `retryable: false` — a deadline is a + hard ceiling the caller set to **stop**, so it is not auto-retried (retrying would re-spend up to + another full `deadlineMs` of tokens/budget). The idle trip now also carries `context.bound: + 'idle'`, so a consumer can switch on one uniform field to tell which timer fired. A consumer that + *wants* to retry a deadline can still opt in via `retryOn`. + - When both bounds are armed and `timeoutMs < deadlineMs`, a silent socket trips the idle bound + first; only a still-active-but-never-completing stream reaches the deadline. + - Implemented once in `src/provider-http.js` (`applyRequestDeadline`; `resolveTimeoutMs` gained a + `defaultMs` parameter so the deadline resolves to `0`/disabled by default while the idle bound + keeps its 10-min default). Each provider's `_request` wires all bounds through one + `applyRequestBounds(req, { timeoutMs, deadlineMs }, name)` seam, so the idle (BA-18) + deadline + (BA-19) wiring is not copy-pasted at four call sites and a future third bound is added in one + place (review finding 2). CLIPipe already bounds its child process on wall-clock and is unchanged. + - **Filed with its own limiting evidence:** n=1 (one 274-minute hang). This is a defense-in-depth + knob for consumers who bound total work no other way — not a new default. + ## [0.34.0] - 2026-07-26 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index 2054e35..2e6ed71 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -31,7 +31,7 @@ Node.js >= 18, pure JS + JSDoc, `node:test` for testing. Flat `src/` layout with | Spawn | tools/spawn.js | Fork child bareagent (`bin/cli.js --config`). LLM-callable blocks; library `spawnChild` returns handle. One JSONL channel per child (stderr re-emitted as `child:stderr`). Threads BAREGUARD env vars; bareguard 0.2+ caps per-family via `spawn.ratePerMinute` + `limits.maxDepth`. `timeoutMs` = wall-clock ceiling; opt-in `idleTimeoutMs` = heartbeat watchdog (kills a child silent on both stdio for N ms; resets per line, so slow-but-working children survive — result carries `idleKilled`) | | Defer | tools/defer.js | Append `{id, action, when}` to JSONL queue for an external waker (cron + `examples/wake.sh`) to fire later. Two-phase gov: emit-time gate.check on `defer` action; fire-time gate.check on inner action. bareguard 0.2+ caps via `defer.ratePerMinute` | -Providers: OpenAI, Anthropic, Gemini, Ollama, CLIPipe, Fallback -- each in `src/provider-*.js`. All normalize `stopReason` (BA-6) to one neutral vocabulary via `src/provider-stop-reason.js` (`end_turn`/`max_tokens`/`tool_use`/`stop_sequence`/`refusal`/`pause_turn`/`context_exceeded`; unknown-or-absent ⇒ `null` ⇒ pre-BA-6 behavior, so an unmapped provider degrades to the status quo, never to a false truncation). All 5 verified live. `tool_use` is DERIVED where the API can't express it: Gemini/Ollama return `STOP`/`stop` on a complete tool call (no tool-call finish reason), so `normalizeStopReason(raw, provider, {hasToolCalls})` promotes `end_turn`→`tool_use` when the round carried a call — narrow by design (only `end_turn`; a truncated `max_tokens` round stays truncated, preserving BA-4's refusal; never touches refusal/pause/context/passthrough). All normalize `usage` to one neutral shape incl. prompt-cache tiers (`cacheReadTokens`/`cacheCreationTokens`; `inputTokens` = uncached remainder). Gemini is native `generateContent` (its OpenAI-compat endpoint drops the cache tier). Anthropic has opt-in `cacheSystem` (cache_control on the system prompt — weak on its own: the 1024–4096-tok model-dependent cache minimum means a short persona silently never caches) + **opt-in `cacheMessages` (BA-1)** = a rolling `cache_control` breakpoint on the LAST content block of the LAST message, so a tool loop stops re-buying its whole transcript at full price every round (measured: $0.0753→$0.0110/round, 6.8× steady-state; the 1.25× write is paid once). It MUST live in the provider — the transcript ends on a `tool_result` that `_toAnthropicMessage` rebuilds, so no caller seam (`assemble` included) can reach it. Copy-on-write (never mutate the caller's array — a stale mark accumulates; Anthropic allows ≤4). Caching pays for RE-SENDING, not GROWING; a destructive `trim` fold that rewrites the PREFIX invalidates it (fold the middle, keep the head). **Provider-native block passthrough (BA-7):** `Message.providerBlocks`/`GenerateResult.providerBlocks` = `{provider, model, blocks}` — the transcript field for content blocks the normalized `{text, toolCalls}` shape CANNOT express (Anthropic `thinking`/`redacted_thinking`), which the OpenAI-shaped `Message` had nowhere to put and so silently DROPPED (the API returns 200 either way — the loss never surfaced). Anthropic echoes thinking back unchanged (signature included) on a tool-use continuation; the provider now replays them at the FRONT of the assistant turn (thinking must lead). Three constraints: OPAQUE (keep bytes, never re-serialize from parsed fields — a `redacted_thinking` block can't survive it, and a `type==='thinking'` check would silently drop the next new block type = the same bug again); the provider+model TAG is enforced on replay (a signature is model-bound; mismatch ⇒ drop, degrade to the lossy-but-valid pre-BA-7 request, never a 400); and normalized `content`/`tool_calls` stay the SOURCE OF TRUTH (only unexpressible blocks ride along, so an `assemble`/`trim` seam that rewrites a message isn't silently undone by a stale cached copy of its text). Adaptive thinking is ALREADY DEFAULT on sonnet-5/Opus 4.7+ (measured: blocks on ~3/10 rounds with the param OMITTED; sending `thinking:{type:'adaptive'}` changed the rate not at all) — so the opt-in `thinking` option (forwarded to `body.thinking` VERBATIM/unvalidated, since `budget_tokens` already died and a reshaping library would need a release per API move) does NOT enable thinking; it pins the mode / reaches `display`/`effort`. **PROTOCOL fix, NOT a capability fix — the adopter's head-to-head found NO outcome difference; never re-sell it as one.** **Request/idle timeout (BA-18):** the four http(s) providers wired only `req.on('error')` — no socket timeout — so a silently-dropped or never-answering socket hung `generate()` until the OS TCP timeout (~2h), a HANG not an error (so every retry policy above it was inert; the BA-4/5/6/13 under-modeled-boundary family, rounding toward "still working"). One shared `src/provider-http.js` (`resolveTimeoutMs`/`applyRequestTimeout`) gives each provider a `timeoutMs` option (default 600000, per-call overridable, `0`/`Infinity` disable) that bounds on socket INACTIVITY (`req.setTimeout` — a slow-but-streaming response is not killed; the 10-min default clears any single non-streaming completion where TTFB≈generation time) and on trip rejects with a retryable `TimeoutError` (`code:'ETIMEDOUT'`). The RETRY seam is caller-side and already exists — `Loop({retry})` wraps `provider.generate` with `Retry.call`, `run-plan`'s `stepRetry` is a second consumer, and `DEFAULT_RETRY_ON` classifies ETIMEDOUT transient — so a wired Retry retries a timed-out request with ZERO new wiring (the ask's "`withRetry` has zero call sites" was a name mismatch: the primitive is `Retry.call`, not `withRetry`). CLIPipe already bounded its child, unchanged. + `baseUrl`. Loop returns `result.metrics` (the meter: cumulative 4-tier tokens, byTool, costUsd null-not-zero, unpricedRounds, spawned, context.{compactions,summaries,tokensTrimmed}, memory.{stashed,episodes,recalls,stored,facts}) — `estimateCost` prices the 4 tiers separately. The §3.6 CE rollup: `context.tokensTrimmed` is an APPROXIMATE (~4 chars/token) estimate over the evicted span (no exact provider count); `memory.*` is reported via the loop-lent `ctx.recordMemoryOp` hook (channel A — the originating module announces, the loop counts + emits `loop:memory`), bounded PER RUN — `stashed`/`episodes` flow through the stash fold; the Memory wrapper is metered symmetrically opt-in via ctx: `recalls` (`Memory.search` reads) + `stored` (`Memory.store` writes; ctx in a trailing opts arg, never in persisted metadata). `memory.facts` is written by `remember` (the F5 consolidation pass) via the Store socket — disjoint from `stored`; litectx's own episode→fact promotion is a separate, litectx-internal op (§3.7) +Providers: OpenAI, Anthropic, Gemini, Ollama, CLIPipe, Fallback -- each in `src/provider-*.js`. All normalize `stopReason` (BA-6) to one neutral vocabulary via `src/provider-stop-reason.js` (`end_turn`/`max_tokens`/`tool_use`/`stop_sequence`/`refusal`/`pause_turn`/`context_exceeded`; unknown-or-absent ⇒ `null` ⇒ pre-BA-6 behavior, so an unmapped provider degrades to the status quo, never to a false truncation). All 5 verified live. `tool_use` is DERIVED where the API can't express it: Gemini/Ollama return `STOP`/`stop` on a complete tool call (no tool-call finish reason), so `normalizeStopReason(raw, provider, {hasToolCalls})` promotes `end_turn`→`tool_use` when the round carried a call — narrow by design (only `end_turn`; a truncated `max_tokens` round stays truncated, preserving BA-4's refusal; never touches refusal/pause/context/passthrough). All normalize `usage` to one neutral shape incl. prompt-cache tiers (`cacheReadTokens`/`cacheCreationTokens`; `inputTokens` = uncached remainder). Gemini is native `generateContent` (its OpenAI-compat endpoint drops the cache tier). Anthropic has opt-in `cacheSystem` (cache_control on the system prompt — weak on its own: the 1024–4096-tok model-dependent cache minimum means a short persona silently never caches) + **opt-in `cacheMessages` (BA-1)** = a rolling `cache_control` breakpoint on the LAST content block of the LAST message, so a tool loop stops re-buying its whole transcript at full price every round (measured: $0.0753→$0.0110/round, 6.8× steady-state; the 1.25× write is paid once). It MUST live in the provider — the transcript ends on a `tool_result` that `_toAnthropicMessage` rebuilds, so no caller seam (`assemble` included) can reach it. Copy-on-write (never mutate the caller's array — a stale mark accumulates; Anthropic allows ≤4). Caching pays for RE-SENDING, not GROWING; a destructive `trim` fold that rewrites the PREFIX invalidates it (fold the middle, keep the head). **Provider-native block passthrough (BA-7):** `Message.providerBlocks`/`GenerateResult.providerBlocks` = `{provider, model, blocks}` — the transcript field for content blocks the normalized `{text, toolCalls}` shape CANNOT express (Anthropic `thinking`/`redacted_thinking`), which the OpenAI-shaped `Message` had nowhere to put and so silently DROPPED (the API returns 200 either way — the loss never surfaced). Anthropic echoes thinking back unchanged (signature included) on a tool-use continuation; the provider now replays them at the FRONT of the assistant turn (thinking must lead). Three constraints: OPAQUE (keep bytes, never re-serialize from parsed fields — a `redacted_thinking` block can't survive it, and a `type==='thinking'` check would silently drop the next new block type = the same bug again); the provider+model TAG is enforced on replay (a signature is model-bound; mismatch ⇒ drop, degrade to the lossy-but-valid pre-BA-7 request, never a 400); and normalized `content`/`tool_calls` stay the SOURCE OF TRUTH (only unexpressible blocks ride along, so an `assemble`/`trim` seam that rewrites a message isn't silently undone by a stale cached copy of its text). Adaptive thinking is ALREADY DEFAULT on sonnet-5/Opus 4.7+ (measured: blocks on ~3/10 rounds with the param OMITTED; sending `thinking:{type:'adaptive'}` changed the rate not at all) — so the opt-in `thinking` option (forwarded to `body.thinking` VERBATIM/unvalidated, since `budget_tokens` already died and a reshaping library would need a release per API move) does NOT enable thinking; it pins the mode / reaches `display`/`effort`. **PROTOCOL fix, NOT a capability fix — the adopter's head-to-head found NO outcome difference; never re-sell it as one.** **Request/idle timeout (BA-18):** the four http(s) providers wired only `req.on('error')` — no socket timeout — so a silently-dropped or never-answering socket hung `generate()` until the OS TCP timeout (~2h), a HANG not an error (so every retry policy above it was inert; the BA-4/5/6/13 under-modeled-boundary family, rounding toward "still working"). One shared `src/provider-http.js` (`resolveTimeoutMs`/`applyRequestTimeout`) gives each provider a `timeoutMs` option (default 600000, per-call overridable, `0`/`Infinity` disable) that bounds on socket INACTIVITY (`req.setTimeout` — a slow-but-streaming response is not killed; the 10-min default clears any single non-streaming completion where TTFB≈generation time) and on trip rejects with a retryable `TimeoutError` (`code:'ETIMEDOUT'`). The RETRY seam is caller-side and already exists — `Loop({retry})` wraps `provider.generate` with `Retry.call`, `run-plan`'s `stepRetry` is a second consumer, and `DEFAULT_RETRY_ON` classifies ETIMEDOUT transient — so a wired Retry retries a timed-out request with ZERO new wiring (the ask's "`withRetry` has zero call sites" was a name mismatch: the primitive is `Retry.call`, not `withRetry`). CLIPipe already bounded its child, unchanged. **Total call-duration deadline (BA-19):** `timeoutMs` bounds socket INACTIVITY, and `req.setTimeout` RESETS on any activity by design — so a "zombie stream" that trickles a byte forever (bytes arriving, response never completing) is invisible to it and hung a caller 274 min→`ECONNRESET` (the reset PROVES bytes were flowing, so the idle timer never fired). `applyRequestDeadline` (same `src/provider-http.js`) adds an absolute NON-resetting wall-clock ceiling via a plain `setTimeout`+`req.destroy` — opt-in `deadlineMs` (DISABLED by default: a deliberately long single call is legitimate; `0`/`Infinity` disable; per-call overridable; same fail-safe resolution as `timeoutMs` via `resolveTimeoutMs(..., defaultMs)`). On trip: a TERMINAL `TimeoutError` DISTINGUISHABLE from the idle trip — `code:'EDEADLINE'`, `context.bound:'deadline'`, `retryable:false` (a hard ceiling is meant to STOP, not re-spend a full `deadlineMs` on retry; the idle trip now carries `context.bound:'idle'` so a consumer switches on ONE uniform field). With both armed and `timeoutMs < deadlineMs`, a silent socket trips idle FIRST; only a still-active-but-never-completing stream reaches the deadline. Two INDEPENDENT failure modes — a silent socket (BA-18) and a zombie stream (BA-19). + `baseUrl`. Loop returns `result.metrics` (the meter: cumulative 4-tier tokens, byTool, costUsd null-not-zero, unpricedRounds, spawned, context.{compactions,summaries,tokensTrimmed}, memory.{stashed,episodes,recalls,stored,facts}) — `estimateCost` prices the 4 tiers separately. The §3.6 CE rollup: `context.tokensTrimmed` is an APPROXIMATE (~4 chars/token) estimate over the evicted span (no exact provider count); `memory.*` is reported via the loop-lent `ctx.recordMemoryOp` hook (channel A — the originating module announces, the loop counts + emits `loop:memory`), bounded PER RUN — `stashed`/`episodes` flow through the stash fold; the Memory wrapper is metered symmetrically opt-in via ctx: `recalls` (`Memory.search` reads) + `stored` (`Memory.store` writes; ctx in a trailing opts arg, never in persisted metadata). `memory.facts` is written by `remember` (the F5 consolidation pass) via the Store socket — disjoint from `stored`; litectx's own episode→fact promotion is a separate, litectx-internal op (§3.7) CLIPipe tool mode has TWO shapes, and the choice between them is COST, not capability. **EMULATION (v0.32.0, `src/provider-clipipe-tools.js`, `toolProtocol:'claude'`):** the caller's tools are described in the system prompt, the CLI is constrained to a JSON envelope (`--json-schema`), the envelope is parsed back into `toolCalls`, and **the Loop keeps ownership of the cycle** (round accounting, spin guards, stop-reason classification apply per round). It re-sends the whole transcript every round — right for a CLI with NO MCP channel, WRONG for the claude CLI on cost. CLI reduced to a bare brain via `--tools '' --strict-mcp-config` + `--setting-sources ''` (MEASURED ~18× cut). Loud-failure by design: a weak model answering in prose is caught UPFRONT by a capability probe (behaviour-based, mirrors real-task shape, per BA-10) → model floor (sonnet-class+ for tools); a malformed envelope is a loud `ProviderError`; tools with no `toolProtocol` stay plain-text with a one-time warn. **NATIVE (BA-16, `src/provider-clipipe-mcp.js` + `src/mcp-bridge-stub.js`, `toolProtocol:'claude-mcp'` — the default for the claude CLI):** the CLI runs its OWN multi-turn session per `generate()` and executes the caller's tools natively over a unix-socket MCP bridge that calls back into the caller's in-process closures. The CLI owns the inner cycle — so the Loop's per-round machinery cannot run and the governance moves to the PROVIDER at the `tools/call` bridge (the one seam every call crosses): the SAME `policy` chokepoint (identical audit-row shape, zero gate changes), BA-11 deny-streak + BA-12 identical-error guards (same 3/3 narrowest triggers), and the turn bound (NAMED stop `error:'max_turns'` + `stopReason:'max_turns'`, work preserved per BA-5). **A TURN IS ONE ASSISTANT MESSAGE, NOT ONE STREAM EVENT (BA-17)** — the CLI emits a separate `assistant` event per content BLOCK, each repeating that message's `usage`, so counting events inflated a caller's turn axis 5–7× (guillotining a bounded worker at half its allowance) and its token axis 5.04×; a run of consecutive events sharing `message.id` is ONE turn (adjacent-run dedup, no-id ⇒ degrade to per-event). `maxTurns` is an LLM-TURN bound — the same unit as the Loop path, never a tool-call count — enforced BOTH by the CLI's `--max-turns` (which stops cleanly and emits the cost-bearing result event) and by a parent-side counter that kills only on an OVERRUN, because the flag is undocumented; the flag itself was measured to enforce correctly and in the right unit, so the ask that reported it broken was a mis-count, not a flag defect. `onTurn` STREAMS once per TURN with all four cache tiers (never sum-at-end; a mid-run death has already surfaced its spend) then one closing `session` event carrying the authoritative cost AND the token RESIDUAL — a turn's `usage` is an unrevised first-block snapshot, so the residual is what makes a gate's tiers add up to the CLI's own total; when wired the Loop skips its own forward (billed once, never starved). **Provider owns its cycle (`Provider.ownsCycle`):** the Loop THROWS at construction on any option it could never honor — `assemble`/`trim`/`cacheMessages` and a Loop-level `policy` (which would be a fence silently not there) — no silently-dead knobs. `GenerateResult.session` + `metrics.sessionTurns` report the REAL turn/tool count in MESSAGES (a 14-turn session never reads as one round, and a 2-turn session never reads as 14); an internal terminal (bound, guard, or a **broken bridge** — a dead bridge still ends `subtype:'success'`, so it is caught parent-side by attempted-vs-served tool calls) surfaces as the run's `error`, never a laundered clean finish. **Cost is the claim (measured $0.25–0.55/round emulation vs ~$0.006/turn native); output-quality parity is deliberately UNMINTED (n=2).** Claude-only for now; both shapes isolated so a second CLI slots in behind the same seams. Stores: SQLiteStore (peer dep: better-sqlite3), JsonFileStore (zero deps) -- each in `src/store-*.js` diff --git a/bareagent.context.md b/bareagent.context.md index 538e8ae..d4efa4c 100644 --- a/bareagent.context.md +++ b/bareagent.context.md @@ -1,7 +1,7 @@ # bareagent — Integration Guide > For AI assistants and developers wiring bareagent into a project. -> v0.34.0 | Node.js >= 18 | zero required deps (`bareguard >=0.9.0 <0.13.0` optional peer for governance) | Apache 2.0 +> v0.35.0 | Node.js >= 18 | zero required deps (`bareguard >=0.9.0 <0.13.0` optional peer for governance) | Apache 2.0 > > Full human guide with composition examples, design philosophy, and recipes: [Usage Guide](docs/02-features/usage-guide.md) @@ -829,7 +829,9 @@ All return `{ text, toolCalls, usage: { inputTokens, outputTokens }, model?, cos **Error body (v0.11.0):** on an HTTP error the OpenAI/Anthropic/Ollama providers throw a `ProviderError` whose `message` carries the upstream error string. The full parsed response is **not** attached to `err.body` by default (so an unexpected field can't leak through logs that dump the error object). Pass `{ exposeErrorBody: true }` to attach it for debugging. -**Request/idle timeout (BA-18, v0.34.0):** the four http(s) providers (Anthropic, OpenAI, Gemini, Ollama) accept a `timeoutMs` option — constructor default **600000 (10 min)**, overridable per call via `generate(..., { timeoutMs })`, and `0`/`Infinity` disables it. Before this they wired only `req.on('error')`, so a socket the server silently dropped — or a response that never starts — hung `generate()` until the OS TCP timeout (~2h): a hang, not an error, so retry/casualty policy above it never fired. `timeoutMs` bounds on socket **inactivity** (`req.setTimeout`), so a slow-but-streaming response is not killed — only a silent/never-answering socket trips it; the 10-min default clears any single non-streaming completion (TTFB ≈ generation time). On trip, `generate()` rejects with a retryable `TimeoutError` (`code: 'ETIMEDOUT'`, `retryable: true`). **Retry is caller-side and already wired:** `new Loop({ provider, retry: new Retry() })` wraps `provider.generate`, and `DEFAULT_RETRY_ON` classifies `ETIMEDOUT` (and `ECONNRESET`/`ENOTFOUND`/429/5xx) as transient — so a wired `Retry` retries a timed-out request and rethrows under `retryOn: () => false`, with no extra wiring (`run-plan`'s `stepRetry` is a second consumer of the same seam). CLIPipe already bounded its child process (`timeout`, default 30000 for one-shot) and is unchanged. +**Request/idle timeout (BA-18, v0.34.0):** the four http(s) providers (Anthropic, OpenAI, Gemini, Ollama) accept a `timeoutMs` option — constructor default **600000 (10 min)**, overridable per call via `generate(..., { timeoutMs })`, and `0`/`Infinity` disables it. Before this they wired only `req.on('error')`, so a socket the server silently dropped — or a response that never starts — hung `generate()` until the OS TCP timeout (~2h): a hang, not an error, so retry/casualty policy above it never fired. `timeoutMs` bounds on socket **inactivity** (`req.setTimeout`), so a slow-but-streaming response is not killed — only a silent/never-answering socket trips it; the 10-min default clears any single non-streaming completion (TTFB ≈ generation time). On trip, `generate()` rejects with a retryable `TimeoutError` (`code: 'ETIMEDOUT'`, `context.bound: 'idle'`, `retryable: true`). **Retry is caller-side and already wired:** `new Loop({ provider, retry: new Retry() })` wraps `provider.generate`, and `DEFAULT_RETRY_ON` classifies `ETIMEDOUT` (and `ECONNRESET`/`ENOTFOUND`/429/5xx) as transient — so a wired `Retry` retries a timed-out request and rethrows under `retryOn: () => false`, with no extra wiring (`run-plan`'s `stepRetry` is a second consumer of the same seam). CLIPipe already bounded its child process (`timeout`, default 30000 for one-shot) and is unchanged. + +**Total call-duration deadline (BA-19, v0.35.0):** `timeoutMs` bounds socket *inactivity*, and `req.setTimeout` resets on any activity by design — so a "zombie stream" that trickles a byte forever (bytes arriving, the response never completing) never trips it and hangs the caller for hours (an adopter saw one `generate()` run **274 min** and end in `ECONNRESET`, not a `TimeoutError`: the reset proves bytes *were* flowing, so the idle timer never fired). The four http(s) providers now also accept a `deadlineMs` option — an absolute, **non-resetting** wall-clock ceiling on the whole request. **Disabled by default** (a deliberately long single call — large `maxTokens`, slow model — is legitimate; a default here would kill it), overridable per call via `generate(..., { deadlineMs })`, `0`/`Infinity` disable. An *unset* deadline resolves to disabled, but an *explicitly-set* garbage value (`NaN`, a non-numeric string) throws a `ValidationError` at resolve time rather than silently disabling the bound and running unbounded — unlike `timeoutMs`, the deadline has no safe default to fall back to, so a config mistake must surface loudly. On trip, `generate()` rejects with a **terminal** `TimeoutError` distinguishable from the idle trip: `code: 'EDEADLINE'`, `context.bound: 'deadline'`, `retryable: false` — a deadline is a hard ceiling meant to STOP, so it is *not* auto-retried (retrying would re-spend up to another full `deadlineMs`); a consumer that wants retry opts in via `retryOn`. When both are armed and `timeoutMs < deadlineMs`, a silent socket trips the idle bound first; only a still-active-but-never-completing stream reaches the deadline. The idle bound (BA-18) and the deadline (BA-19) are two independent failure modes — a silent socket vs a zombie stream. **Plaintext-key warning (Unreleased):** the OpenAI provider's `baseUrl` accepts `http://` (for local/OpenAI-compatible endpoints), but a `Bearer` key sent over plaintext http to a **non-loopback** host is exposed on the wire. The provider now warns once when that happens. Loopback hosts (`localhost`/`127.0.0.0/8`/`::1` — local proxies, Ollama-style endpoints) stay silent, since that's the legitimate keyless-local case. The header is **not** stripped (some local proxies want a key), so use `https` for any remote endpoint, or drop `apiKey` when the local endpoint needs none. diff --git a/docs/01-product/prd.md b/docs/01-product/prd.md index ae46915..5672244 100644 --- a/docs/01-product/prd.md +++ b/docs/01-product/prd.md @@ -809,6 +809,50 @@ re-litigated unless the user explicitly asks. > back into this main PRD; granular evidence tables for each live in the > CHANGELOG and git history. +### v0.35.0 / provider total-duration deadline (bareloop BA-19) (2026-07-28) + +> **Numbering note.** This is **bareloop's** BA-19 (`docs/UPSTREAM-ASKS.md`), not +> a bareagent-internal BA-number — cross-repo ask numbers can collide. + +- **The gap was real (unlike BA-18 part 2).** BA-18's `timeoutMs` bounds socket + *inactivity* via `req.setTimeout`, which resets on any activity by design — so a + "zombie stream" that trickles a byte forever (bytes arriving, response never + completing) is invisible to it. An adopter saw one `generate()` run 274 min and + end in `ECONNRESET`, not a `TimeoutError`: the reset is the proof of mechanism — + bytes *were* flowing, so the idle timer never fired. Nothing bounded *total* + call duration. Confirmed in source before building. +- **Opt-in `deadlineMs`, DISABLED by default (per the ask, and the deliberate + contrast with BA-18's finite default).** An absolute, non-resetting wall-clock + ceiling (a plain `setTimeout` + `req.destroy`, unref'd + cleared on request + close). A deliberately long single call (large `maxTokens`, slow model) is a + legitimate multi-minute stream a default deadline would kill — so this is a + knob, not a new default. `0`/`Infinity` disable; per-call overridable; + `resolveTimeoutMs` gained a `defaultMs` param so the deadline resolves to + `0`/disabled by default while the idle bound keeps its 10-min default. +- **Disable-edge is fail-LOUD for the deadline (review finding 1).** An unset + deadline resolves to disabled, but an explicitly-set garbage value + (`NaN`/non-numeric) throws a `ValidationError` — the idle bound can fall back to + its 10-min default on garbage because that default is a real bound, but the + deadline has no safe default, so a silent disable would reintroduce the very hang + it prevents. Both bounds now wire through one `applyRequestBounds` seam per + `_request` (review finding 2), so a future third bound is added in one place. +- **Terminal, and distinguishable from the idle trip (user decision: + `retryable:false`).** On trip: `code:'EDEADLINE'`, `context.bound:'deadline'`, + `retryable:false` — a deadline is a hard ceiling meant to STOP, so it is not + auto-retried (retrying would re-spend up to another full `deadlineMs`). The idle + trip now also carries `context.bound:'idle'`, so a consumer switches on one + uniform field to route which timer fired. A consumer that *wants* retry opts in + via `retryOn`. Criterion 3 holds: with `timeoutMs < deadlineMs`, a silent socket + trips the idle bound first; only a still-active-but-never-completing stream + reaches the deadline. +- **Same BA-4/5/6/13/17/18 family** — an under-modeled boundary (a zombie stream + has no slot in the neutral result shape) rounding optimistically toward + "still working." POC + loopback trickling-server tests reproduce it faithfully + (a byte every idle/2 ms, never completing) and were mutation-proven, incl. a + deadline-OFF negative control that hangs. Filed n=1 (one 274-min hang) — a + defense-in-depth knob, not load-bearing for the adopter (their harness-layer + watchdog self-heals this class). + ### v0.34.0 / provider request-idle timeout (bareloop BA-18) (2026-07-26) > **Numbering note.** This is **bareloop's** BA-18 (`docs/UPSTREAM-ASKS.md`), not diff --git a/docs/02-features/errors.md b/docs/02-features/errors.md index 7635a47..4069bbb 100644 --- a/docs/02-features/errors.md +++ b/docs/02-features/errors.md @@ -117,12 +117,15 @@ The circuit breaker tracks failures per key. After `threshold` failures, calls a | Error | When | Fix | |-------|------|-----| | `[Retry] Timeout` (`TimeoutError`) | A single attempt exceeded the configured `timeout` (default 60s) | Increase `timeout` in constructor or per-call options. `instanceof TimeoutError`, `code: 'ETIMEDOUT'`, `retryable: true` | -| `[] request timed out after Nms of socket inactivity` (`TimeoutError`, BA-18) | An http(s) provider's socket went idle past its `timeoutMs` (default 600000; a silently-dropped or never-answering socket). Applies to Anthropic/OpenAI/Gemini/Ollama. | Expected on a dead/hung connection — it's now an error instead of a ~2h hang. `code: 'ETIMEDOUT'`, `retryable: true`. Tune per provider via `timeoutMs` (`0`/`Infinity` disables); wrap the Loop in `Retry` to auto-retry it. | +| `[] request timed out after Nms of socket inactivity` (`TimeoutError`, BA-18) | An http(s) provider's socket went idle past its `timeoutMs` (default 600000; a silently-dropped or never-answering socket). Applies to Anthropic/OpenAI/Gemini/Ollama. | Expected on a dead/hung connection — it's now an error instead of a ~2h hang. `code: 'ETIMEDOUT'`, `context.bound: 'idle'`, `retryable: true`. Tune per provider via `timeoutMs` (`0`/`Infinity` disables); wrap the Loop in `Retry` to auto-retry it. | +| `[] request exceeded its total deadline of Nms` (`TimeoutError`, BA-19) | An http(s) provider's whole request ran past its opt-in `deadlineMs` (default disabled). Catches a "zombie stream" that trickles bytes forever — which the idle `timeoutMs` cannot, since activity keeps resetting it. Applies to Anthropic/OpenAI/Gemini/Ollama. | A hard wall-clock ceiling; on trip the request is destroyed. `code: 'EDEADLINE'`, `context.bound: 'deadline'`, **`retryable: false`** (a deadline is meant to stop, not re-spend — it is *not* auto-retried by `DEFAULT_RETRY_ON`; opt in via `retryOn` if you want it). Set `deadlineMs` per provider or per call; `0`/`Infinity` disable. | **Note:** After exhausting `maxAttempts`, Retry rethrows the last error from the wrapped function — it does not add its own prefix. Errors with `err.retryable === true` are automatically retried; `err.retryable === false` bail immediately without consuming remaining attempts. **Provider request timeout + Retry (BA-18).** The Loop wires `Retry` around `provider.generate` — `new Loop({ provider, retry: new Retry() })` — and the default retry predicate (`DEFAULT_RETRY_ON`) classifies `ETIMEDOUT`/`ECONNRESET`/`ENOTFOUND`/429/5xx as transient. So a provider request that times out (or drops its socket) is retried automatically under a wired `Retry`, and rethrows under `retryOn: () => false`. `run-plan`'s `stepRetry` is the same seam at the step level. Using a provider **without** a Loop? Its own `timeoutMs` still bounds the hang — you just handle the rejection yourself. +**Idle vs total-duration bound (BA-18 vs BA-19).** `timeoutMs` (BA-18, default 10 min) bounds socket *inactivity* — it catches a silent or never-answering socket, but by design resets on any activity, so it cannot catch a "zombie stream" that keeps trickling bytes without ever completing. `deadlineMs` (BA-19, opt-in, disabled by default) is an absolute wall-clock ceiling on the *whole* request that catches exactly that. They compose: with both set and `timeoutMs < deadlineMs`, a silent socket trips the idle bound (`ETIMEDOUT`, retryable) first, and only a still-active-but-never-completing stream reaches the deadline (`EDEADLINE`, terminal). Switch on `err.context.bound` (`'idle'` vs `'deadline'`) to tell which fired. Leave `deadlineMs` unset unless you need to bound total call duration and have no other watchdog — a legitimate long completion (large `maxTokens`, slow model) is a multi-minute stream a deadline would kill. An *unset* `deadlineMs` is disabled, but an *explicitly-set* garbage value (`NaN`, a non-numeric string) throws a `ValidationError` at resolve time rather than silently disabling the bound — the deadline has no safe default to fall back to, so a config mistake surfaces loudly instead of running unbounded. + ## CLIPipeProvider | Error | When | Fix | diff --git a/package-lock.json b/package-lock.json index d0a0a55..78e6034 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "bare-agent", - "version": "0.34.0", + "version": "0.35.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "bare-agent", - "version": "0.34.0", + "version": "0.35.0", "license": "Apache-2.0", "bin": { "bare-agent": "bin/cli.js" diff --git a/package.json b/package.json index a0a20f9..b433216 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "bare-agent", - "version": "0.34.0", + "version": "0.35.0", "files": [ "index.js", "index.d.ts", diff --git a/poc/ba19-deadline.mjs b/poc/ba19-deadline.mjs new file mode 100644 index 0000000..5689787 --- /dev/null +++ b/poc/ba19-deadline.mjs @@ -0,0 +1,163 @@ +// BA-19 POC — a per-call TOTAL-DURATION deadline beside the BA-18 idle bound. +// +// Riskiest assumption: the BA-18 idle timer (`req.setTimeout`) resets on ANY socket +// activity, so a response that trickles a byte forever never trips it. A SEPARATE plain +// setTimeout that `req.destroy(err)`s — NOT reset on activity — must fire at ~deadlineMs +// on exactly that zombie stream. Prove it against real local http servers, faithful to +// src/provider-http.js's mechanism. +// +// Prove, don't assert: every case observes real timing + the real error object. + +import http from 'node:http'; + +// ---- faithful copies of the shipped mechanism ------------------------------------------- +class TimeoutError extends Error { + constructor(message, { code = 'ETIMEDOUT', retryable = true, context = {} } = {}) { + super(message); + this.name = 'TimeoutError'; + this.code = code; + this.retryable = retryable; + this.context = context; + } +} + +// BA-18 shipped helper (idle bound) — verbatim mechanism +function applyRequestTimeout(req, timeoutMs, providerName) { + if (!(timeoutMs > 0)) return; + req.setTimeout(timeoutMs, () => { + req.destroy(new TimeoutError(`[${providerName}] request timed out after ${timeoutMs}ms of socket inactivity`)); + }); +} + +// BA-19 proposed helper (total-duration bound) — the thing under test +function applyRequestDeadline(req, deadlineMs, providerName) { + if (!(deadlineMs > 0)) return; + const timer = setTimeout(() => { + req.destroy(new TimeoutError( + `[${providerName}] request exceeded total deadline of ${deadlineMs}ms`, + { code: 'EDEADLINE', context: { bound: 'deadline' } } + )); + }, deadlineMs); + if (timer.unref) timer.unref(); + req.once('close', () => clearTimeout(timer)); +} + +// a faithful _request skeleton mirroring provider-anthropic.js:_request +function request(port, { timeoutMs = 0, deadlineMs = 0 } = {}) { + return new Promise((resolve, reject) => { + const req = http.request({ host: '127.0.0.1', port, method: 'POST', path: '/' }, (res) => { + let chunks = ''; + res.on('data', d => chunks += d); + res.on('end', () => resolve({ status: res.statusCode, body: chunks })); + }); + applyRequestTimeout(req, timeoutMs, 'PocProvider'); + applyRequestDeadline(req, deadlineMs, 'PocProvider'); + req.on('error', reject); + req.end(); + }); +} + +const listen = (handler) => new Promise((res) => { + const srv = http.createServer(handler); + srv.listen(0, '127.0.0.1', () => res(srv)); +}); +const ms = (t0) => Math.round(Number(process.hrtime.bigint() - t0) / 1e6); + +let pass = 0, fail = 0; +const ok = (name, cond, detail = '') => { (cond ? (pass++, console.log(` ✓ ${name} ${detail}`)) : (fail++, console.log(` ✗ ${name} ${detail}`))); }; + +// ---- Case 1 (LOAD-BEARING): zombie stream — trickles a byte every idle/2, never ends ----- +// idle timer never trips (constant activity); the deadline MUST fire. +{ + const IDLE = 300, DEADLINE = 800; + const srv = await listen((req, res) => { + res.writeHead(200, { 'Content-Type': 'text/plain' }); + // write a byte every IDLE/2 ms, forever — never res.end() + const iv = setInterval(() => { try { res.write('x'); } catch { clearInterval(iv); } }, IDLE / 2); + req.on('close', () => clearInterval(iv)); + }); + const port = srv.address().port; + const t0 = process.hrtime.bigint(); + try { + await request(port, { timeoutMs: IDLE, deadlineMs: DEADLINE }); + ok('C1 zombie stream rejects', false, '(resolved — BUG: hung/completed)'); + } catch (e) { + const dt = ms(t0); + ok('C1 zombie stream trips the DEADLINE (idle could not catch it)', + e.code === 'EDEADLINE' && dt >= DEADLINE - 50 && dt < DEADLINE + 400, + `code=${e.code} bound=${e.context?.bound} at ${dt}ms (deadline=${DEADLINE}, idle=${IDLE} never fired)`); + } + srv.close(); +} + +// ---- Case 2: silent socket, idle < deadline → IDLE must trip FIRST (criterion 3) --------- +{ + const IDLE = 250, DEADLINE = 2000; + const srv = await listen(() => { /* accept, never respond, never write */ }); + const port = srv.address().port; + const t0 = process.hrtime.bigint(); + try { + await request(port, { timeoutMs: IDLE, deadlineMs: DEADLINE }); + ok('C2 silent socket rejects', false, '(resolved — BUG)'); + } catch (e) { + const dt = ms(t0); + ok('C2 silent socket trips IDLE first (not deadline)', + e.code === 'ETIMEDOUT' && dt >= IDLE - 50 && dt < IDLE + 400, + `code=${e.code} at ${dt}ms (idle=${IDLE} < deadline=${DEADLINE})`); + } + srv.close(); +} + +// ---- Case 3: normal fast response with a deadline set → no false trip, real body --------- +{ + const srv = await listen((req, res) => { + res.writeHead(200); res.end('{"ok":true}'); + }); + const port = srv.address().port; + try { + const r = await request(port, { timeoutMs: 5000, deadlineMs: 5000 }); + ok('C3 fast response: no false trip, real body', r.body === '{"ok":true}', `body=${r.body}`); + } catch (e) { + ok('C3 fast response', false, `(rejected ${e.code})`); + } + srv.close(); +} + +// ---- Case 4: deadline disabled (0) on a slow-but-completing response → completes ---------- +// response takes 400ms (single delayed body) with deadline OFF → must complete, not trip. +{ + const srv = await listen((req, res) => { + setTimeout(() => { res.writeHead(200); res.end('done'); }, 400); + }); + const port = srv.address().port; + try { + const r = await request(port, { timeoutMs: 5000, deadlineMs: 0 }); + ok('C4 deadline=0 disables the total bound', r.body === 'done', `body=${r.body}`); + } catch (e) { + ok('C4 deadline=0 disables', false, `(rejected ${e.code})`); + } + srv.close(); +} + +// ---- Case 5 (negative control): SAME zombie stream, deadline OFF → the BA-19 bug repro ---- +// with only the idle bound (deadline disabled), the zombie stream must HANG past a generous +// deadline — proving the deadline is load-bearing and the idle bound genuinely cannot catch it. +{ + const IDLE = 300; + const srv = await listen((req, res) => { + res.writeHead(200); + const iv = setInterval(() => { try { res.write('x'); } catch { clearInterval(iv); } }, IDLE / 2); + req.on('close', () => clearInterval(iv)); + }); + const port = srv.address().port; + const t0 = process.hrtime.bigint(); + let settled = false; + const p = request(port, { timeoutMs: IDLE, deadlineMs: 0 }).then(() => { settled = 'resolved'; }, () => { settled = 'rejected'; }); + await Promise.race([p, new Promise(r => setTimeout(r, 1500))]); + ok('C5 negative control: zombie hangs with deadline OFF (idle cannot catch it)', + settled === false, `still pending at ${ms(t0)}ms (idle=${IDLE} kept resetting)`); + srv.close(); +} + +console.log(`\nBA-19 POC: ${pass} pass, ${fail} fail`); +process.exit(fail ? 1 : 0); diff --git a/src/provider-anthropic.js b/src/provider-anthropic.js index 5338022..2e78e6b 100644 --- a/src/provider-anthropic.js +++ b/src/provider-anthropic.js @@ -5,7 +5,7 @@ const http = require('http'); const { ProviderError } = require('./errors'); const { requestWithTemperatureFallback } = require('./provider-temperature'); const { normalizeStopReason } = require('./provider-stop-reason'); -const { resolveTimeoutMs, applyRequestTimeout } = require('./provider-http'); +const { resolveTimeoutMs, applyRequestBounds } = require('./provider-http'); /** @param {string} hostname @returns {boolean} */ function isLoopbackHost(hostname) { @@ -28,7 +28,8 @@ function isLoopbackHost(hostname) { * * **MEASURED CAVEAT — this option does not "turn thinking on".** On `claude-sonnet-5` adaptive thinking is ALREADY the default: sending this changed the observed thinking rate not at all (2/10 rounds with it vs 3/10 without — `poc/ba7-adaptive-default.mjs`). Its real use is pinning the mode and reaching `display`/`effort`. The change that mattered is that thinking blocks are now PRESERVED and replayed (see `Message.providerBlocks`), which happens whether or not you ever set this. * @property {boolean} [exposeErrorBody=false] - Attach the full upstream response to `err.body` on HTTP errors (off by default to avoid leaking unexpected fields through error logs; `err.message` still carries the API error). - * @property {number} [timeoutMs=600000] - BA-18: request/idle timeout in ms. A silent or never-answering socket (dropped by the server, or a response that never starts) was otherwise bounded only by the OS TCP timeout (~2h) — a hang, not an error, so every retry policy above it was inert. Bounds on socket INACTIVITY (the timer resets on activity, so a slow-but-streaming response is not killed); on trip, `generate()` rejects with a retryable `TimeoutError` (`code: 'ETIMEDOUT'`) that a wired `Retry` (`Loop({ retry })`) retries. Default 10 min sits above any single non-streaming completion; `0` or `Infinity` disables it (pre-BA-18 behaviour). Overridable per call via `generate(..., { timeoutMs })`. + * @property {number} [timeoutMs=600000] - BA-18: request/idle timeout in ms. A silent or never-answering socket (dropped by the server, or a response that never starts) was otherwise bounded only by the OS TCP timeout (~2h) — a hang, not an error, so every retry policy above it was inert. Bounds on socket INACTIVITY (the timer resets on activity, so a slow-but-streaming response is not killed); on trip, `generate()` rejects with a retryable `TimeoutError` (`code: 'ETIMEDOUT'`, `context.bound: 'idle'`) that a wired `Retry` (`Loop({ retry })`) retries. Default 10 min sits above any single non-streaming completion; `0` or `Infinity` disables it (pre-BA-18 behaviour). Overridable per call via `generate(..., { timeoutMs })`. + * @property {number} [deadlineMs=0] - BA-19: TOTAL call-duration deadline in ms, beside `timeoutMs`. The idle bound resets on any socket activity, so a response that trickles a byte forever (a "zombie stream") never trips it and hangs the caller for hours. This is an absolute, non-resetting wall-clock ceiling; on trip, `generate()` rejects with a TERMINAL `TimeoutError` (`code: 'EDEADLINE'`, `context.bound: 'deadline'`, `retryable: false` — a hard ceiling meant to STOP, not re-spend). DISABLED by default (a deliberately long single call is legitimate); `0`/`Infinity` disable. When both are set with `timeoutMs < deadlineMs`, a silent socket trips the idle bound first. Overridable per call via `generate(..., { deadlineMs })`. */ class AnthropicProvider { @@ -56,13 +57,15 @@ class AnthropicProvider { this.exposeErrorBody = options.exposeErrorBody === true; // BA-18: request/idle timeout (ms). Resolved at call time (default 600000; 0/Infinity disable). this.timeoutMs = options.timeoutMs; + // BA-19: total call-duration deadline (ms). Resolved at call time (default 0 = disabled). + this.deadlineMs = options.deadlineMs; } /** * Generate a response from the Anthropic API. * @param {Message[]} messages - Conversation messages (OpenAI format, auto-converted). * @param {ToolDef[]} [tools=[]] - Tool definitions. - * @param {Record} [options={}] - Options (temperature, maxTokens, system, timeoutMs — a per-call override of the constructor's `timeoutMs`; see BA-18). + * @param {Record} [options={}] - Options (temperature, maxTokens, system, timeoutMs — a per-call override of the constructor's `timeoutMs`, see BA-18; deadlineMs — a per-call override of the constructor's `deadlineMs`, see BA-19). * @returns {Promise} * @throws {Error} `[AnthropicProvider] ...` — on HTTP errors (4xx/5xx) or invalid JSON response. */ @@ -147,8 +150,9 @@ class AnthropicProvider { // and retry once rather than let the whole call fail. `temperatureDropped` flows back so an upstream // receipt (recurse's refineLeaf) can report the effective temperature, not the one the model ignored. const timeoutMs = resolveTimeoutMs(this.timeoutMs, options.timeoutMs); + const deadlineMs = resolveTimeoutMs(this.deadlineMs, options.deadlineMs, 0, 'deadlineMs'); const { data, temperatureDropped } = await requestWithTemperatureFallback({ - request: () => this._request(body, timeoutMs), + request: () => this._request(body, timeoutMs, deadlineMs), hadTemperature: () => body.temperature != null, stripTemperature: () => { delete body.temperature; }, warnOnce: () => this._warnTemperatureDropped(), @@ -281,9 +285,10 @@ class AnthropicProvider { /** * @param {Record} body * @param {number} [timeoutMs=0] - Idle-socket timeout (ms); 0 disables. See BA-18 / provider-http. + * @param {number} [deadlineMs=0] - Total call-duration deadline (ms); 0 disables. See BA-19 / provider-http. * @returns {Promise} */ - _request(body, timeoutMs = 0) { + _request(body, timeoutMs = 0, deadlineMs = 0) { return new Promise((resolve, reject) => { const payload = JSON.stringify(body); const url = new URL(this.baseUrl + '/messages'); @@ -320,7 +325,7 @@ class AnthropicProvider { } }); }); - applyRequestTimeout(req, timeoutMs, 'AnthropicProvider'); + applyRequestBounds(req, { timeoutMs, deadlineMs }, 'AnthropicProvider'); req.on('error', reject); req.write(payload); req.end(); diff --git a/src/provider-gemini.js b/src/provider-gemini.js index 8648c54..a082f95 100644 --- a/src/provider-gemini.js +++ b/src/provider-gemini.js @@ -5,7 +5,7 @@ const http = require('http'); const { ProviderError } = require('./errors'); const { requestWithTemperatureFallback } = require('./provider-temperature'); const { normalizeStopReason } = require('./provider-stop-reason'); -const { resolveTimeoutMs, applyRequestTimeout } = require('./provider-http'); +const { resolveTimeoutMs, applyRequestBounds } = require('./provider-http'); /** @typedef {import('../types').Message} Message */ /** @typedef {import('../types').ToolDef} ToolDef */ @@ -24,7 +24,8 @@ function isLoopbackHost(hostname) { * @property {string} [model='gemini-2.5-flash'] - Model ID. * @property {string} [baseUrl='https://generativelanguage.googleapis.com/v1beta'] - API base (override for proxies; posts to `${baseUrl}/models/${model}:generateContent`). * @property {boolean} [exposeErrorBody=false] - Attach the full upstream response to `err.body` on HTTP errors (off by default; `err.message` still carries the API error). - * @property {number} [timeoutMs=600000] - BA-18: request/idle timeout in ms. Bounds a silent or never-answering socket on inactivity so `generate()` rejects with a retryable `TimeoutError` (`code: 'ETIMEDOUT'`) instead of hanging until the OS TCP timeout (~2h). `0`/`Infinity` disables it (pre-BA-18 behaviour). Overridable per call via `generate(..., { timeoutMs })`. + * @property {number} [timeoutMs=600000] - BA-18: request/idle timeout in ms. Bounds a silent or never-answering socket on inactivity so `generate()` rejects with a retryable `TimeoutError` (`code: 'ETIMEDOUT'`, `context.bound: 'idle'`) instead of hanging until the OS TCP timeout (~2h). `0`/`Infinity` disables it (pre-BA-18 behaviour). Overridable per call via `generate(..., { timeoutMs })`. + * @property {number} [deadlineMs=0] - BA-19: TOTAL call-duration deadline in ms, beside `timeoutMs`. The idle bound resets on any socket activity, so a response that trickles a byte forever never trips it and hangs for hours. This is an absolute, non-resetting ceiling; on trip, `generate()` rejects with a TERMINAL `TimeoutError` (`code: 'EDEADLINE'`, `context.bound: 'deadline'`, `retryable: false`). DISABLED by default; `0`/`Infinity` disable. Overridable per call via `generate(..., { deadlineMs })`. */ /** @@ -43,13 +44,15 @@ class GeminiProvider { this.exposeErrorBody = options.exposeErrorBody === true; // BA-18: request/idle timeout (ms). Resolved at call time (default 600000; 0/Infinity disable). this.timeoutMs = options.timeoutMs; + // BA-19: total call-duration deadline (ms). Resolved at call time (default 0 = disabled). + this.deadlineMs = options.deadlineMs; } /** * Generate a response from the Gemini API. * @param {Message[]} messages - Conversation messages (OpenAI format, auto-converted). * @param {ToolDef[]} [tools=[]] - Tool definitions. - * @param {Record} [options={}] - Options (temperature, maxTokens, timeoutMs — a per-call override of the constructor's `timeoutMs`; see BA-18). + * @param {Record} [options={}] - Options (temperature, maxTokens, timeoutMs — a per-call override of the constructor's `timeoutMs`, see BA-18; deadlineMs — a per-call override of the constructor's `deadlineMs`, see BA-19). * @returns {Promise} * @throws {Error} `[GeminiProvider] ...` — on HTTP errors (4xx/5xx) or invalid JSON response. */ @@ -112,8 +115,9 @@ class GeminiProvider { // BA-10: graceful degrade if a model rejects a non-default `temperature` (Gemini nests it under // generationConfig). Keyed off the API error text, so dormant on models that accept temperature. const timeoutMs = resolveTimeoutMs(this.timeoutMs, options.timeoutMs); + const deadlineMs = resolveTimeoutMs(this.deadlineMs, options.deadlineMs, 0, 'deadlineMs'); const { data, temperatureDropped } = await requestWithTemperatureFallback({ - request: () => this._request(`/models/${this.model}:generateContent`, body, timeoutMs), + request: () => this._request(`/models/${this.model}:generateContent`, body, timeoutMs, deadlineMs), hadTemperature: () => body.generationConfig?.temperature != null, stripTemperature: () => { if (body.generationConfig) delete body.generationConfig.temperature; }, warnOnce: () => this._warnTemperatureDropped(), @@ -180,9 +184,10 @@ class GeminiProvider { * @param {string} path * @param {Record} body * @param {number} [timeoutMs=0] - Idle-socket timeout (ms); 0 disables. See BA-18 / provider-http. + * @param {number} [deadlineMs=0] - Total call-duration deadline (ms); 0 disables. See BA-19 / provider-http. * @returns {Promise} */ - _request(path, body, timeoutMs = 0) { + _request(path, body, timeoutMs = 0, deadlineMs = 0) { return new Promise((resolve, reject) => { const url = new URL(this.baseUrl + path); const transport = url.protocol === 'https:' ? https : http; @@ -219,7 +224,7 @@ class GeminiProvider { } }); }); - applyRequestTimeout(req, timeoutMs, 'GeminiProvider'); + applyRequestBounds(req, { timeoutMs, deadlineMs }, 'GeminiProvider'); req.on('error', reject); req.write(payload); req.end(); diff --git a/src/provider-http.js b/src/provider-http.js index 15dae75..51d19f0 100644 --- a/src/provider-http.js +++ b/src/provider-http.js @@ -1,6 +1,6 @@ 'use strict'; -const { TimeoutError } = require('./errors'); +const { TimeoutError, ValidationError } = require('./errors'); /** * BA-18 — shared request-timeout helper for the http(s)-based providers (Anthropic, OpenAI, @@ -18,24 +18,41 @@ const { TimeoutError } = require('./errors'); const DEFAULT_TIMEOUT_MS = 600000; /** - * Resolve the effective timeout in ms. A per-call value overrides the instance default, but `null` - * and `undefined` BOTH mean "inherit" — so a per-call `null` never shadows an instance-level - * disable (finding-2). The explicit opt-out idiom is `0` or `Infinity` → returns 0 (no bound, the - * pre-BA-18 behaviour). A NaN / negative / otherwise non-finite value is treated as a caller - * MISTAKE and falls back to {@link DEFAULT_TIMEOUT_MS}: it must never silently disable the safety - * bound, which would round optimistically back toward the ~2h hang BA-18 exists to prevent - * (finding-3; the disable-edge bug class). + * Resolve an effective timeout-shaped ms bound. A per-call value overrides the instance default, but + * `null` and `undefined` BOTH mean "inherit" — so a per-call `null` never shadows an instance-level + * disable. The explicit opt-out idiom is `0` or `Infinity` → returns 0 (no bound). When the knob is + * UNSET on both, returns `defaultMs` (the idle bound's 10 min, or 0/disabled for the deadline). + * + * The disable-edge (finding-3, and the BA-4/5/6 optimistic-rounding family) is the load-bearing part: + * a config MISTAKE — a NaN / negative / non-numeric value the caller EXPLICITLY set (e.g. + * `Number(process.env.X)` on an unset var, or a string `'30s'`) — must never SILENTLY remove a bound + * the caller evidently tried to set. Two cases, decided by whether the knob has a safe default: + * - `defaultMs > 0` (the BA-18 idle bound): fail SAFE to that real default (a garbage idle value + * keeps the 10-min safety net — byte-identical to the shipped BA-18 behaviour). + * - `defaultMs === 0` (the BA-19 deadline, disabled-by-design): there is NO safe bound to fall back + * to, so silently returning 0 would reintroduce the very hang the deadline exists to prevent + * (BA-19 review finding 1). Fail LOUD instead — throw a {@link ValidationError} so the config + * mistake surfaces immediately, rather than running unbounded for hours. + * NOTE: an UNSET deadline (null/undefined) is legitimate and still returns 0 — only an explicitly-set + * garbage value throws. * @param {number|undefined|null} instanceTimeout * @param {number|undefined|null} [callTimeout] + * @param {number} [defaultMs=DEFAULT_TIMEOUT_MS] - value returned when the knob is unset (or garbage, if >0) + * @param {string} [name='timeoutMs'] - knob name for the throw message * @returns {number} a finite positive ms bound, or 0 to disable + * @throws {ValidationError} when the value is explicitly set but garbage AND `defaultMs` is 0 (no safe fallback) */ -function resolveTimeoutMs(instanceTimeout, callTimeout) { +function resolveTimeoutMs(instanceTimeout, callTimeout, defaultMs = DEFAULT_TIMEOUT_MS, name = 'timeoutMs') { const raw = callTimeout != null ? callTimeout : instanceTimeout; // null/undefined per-call → inherit - if (raw == null) return DEFAULT_TIMEOUT_MS; // absent on both → finite default + if (raw == null) return defaultMs; // absent on both → the knob's own default (0 = disabled) const n = Number(raw); if (n === 0 || n === Infinity) return 0; // the explicit opt-out idiom → no bound - if (!Number.isFinite(n) || n < 0) return DEFAULT_TIMEOUT_MS; // NaN / negative / garbage → SAFE default, never a silent disable - return n; // finite positive bound + if (Number.isFinite(n) && n > 0) return n; // finite positive bound + // Explicitly set but garbage. Never SILENTLY disable a bound the caller tried to set. + if (defaultMs > 0) return defaultMs; // a real default exists → fail safe to it (BA-18) + throw new ValidationError( + `[provider-http] invalid ${name}: ${String(raw).slice(0, 40)} — expected a positive number, 0, or Infinity` + ); } /** @@ -52,8 +69,58 @@ function resolveTimeoutMs(instanceTimeout, callTimeout) { function applyRequestTimeout(req, timeoutMs, providerName) { if (!(timeoutMs > 0)) return; req.setTimeout(timeoutMs, () => { - req.destroy(new TimeoutError(`[${providerName}] request timed out after ${timeoutMs}ms of socket inactivity`)); + // `context.bound: 'idle'` mirrors the deadline trip's discriminator (BA-19) so a consumer can + // switch on one uniform field to tell which timer spoke, not just on the `code`. + req.destroy(new TimeoutError( + `[${providerName}] request timed out after ${timeoutMs}ms of socket inactivity`, + { context: { bound: 'idle' } } + )); }); } -module.exports = { DEFAULT_TIMEOUT_MS, resolveTimeoutMs, applyRequestTimeout }; +/** + * BA-19 — bound an in-flight ClientRequest on TOTAL call duration, beside {@link applyRequestTimeout}. + * The idle bound (`req.setTimeout`) resets on ANY socket activity, so a response that trickles a byte + * forever (a "zombie stream") never trips it — bytes keep arriving while the response never completes, + * and the call hangs until the OS TCP timeout (~4.5h observed). This adds an absolute, non-resetting + * wall-clock ceiling: a plain `setTimeout` that destroys the request whether or not the socket is + * active. On trip, the request is destroyed with a TERMINAL {@link TimeoutError} — distinct + * `code: 'EDEADLINE'` and `context.bound: 'deadline'` (so a consumer routing governance stops vs + * transport casualties can tell which timer fired), and `retryable: false` because a deadline is a + * HARD ceiling the caller set to STOP: auto-retrying would re-spend up to another full `deadlineMs` + * of tokens/budget. Disabled-by-design (a deliberately long single call is legitimate); a + * `deadlineMs` of 0 is a no-op. When both bounds are armed and `timeoutMs < deadlineMs`, a silent + * socket trips the idle bound first; only a still-active-but-never-completing stream reaches the + * deadline. The timer is unref'd (never keeps the event loop alive) and cleared when the request + * closes (no dangling handle, no late destroy of a settled request). + * @param {import('http').ClientRequest} req + * @param {number} deadlineMs - resolved bound; 0 disables + * @param {string} providerName - for the error message (e.g. 'AnthropicProvider') + */ +function applyRequestDeadline(req, deadlineMs, providerName) { + if (!(deadlineMs > 0)) return; + const timer = setTimeout(() => { + req.destroy(new TimeoutError( + `[${providerName}] request exceeded its total deadline of ${deadlineMs}ms`, + { code: 'EDEADLINE', retryable: false, context: { bound: 'deadline' } } + )); + }, deadlineMs); + if (timer.unref) timer.unref(); + req.once('close', () => clearTimeout(timer)); +} + +/** + * Wire ALL request bounds onto a ClientRequest in one call — the single seam each provider's + * `_request` uses, so the idle (BA-18) + deadline (BA-19) wiring is not copy-pasted at four call + * sites (BA-19 review finding 2). A future third bound is added HERE once, not at every provider. + * Each individual bound is a no-op when its ms value is 0/absent, so an unset knob costs nothing. + * @param {import('http').ClientRequest} req + * @param {{ timeoutMs?: number, deadlineMs?: number }} bounds - resolved ms bounds; 0/absent disables each + * @param {string} providerName - for error messages (e.g. 'AnthropicProvider') + */ +function applyRequestBounds(req, bounds, providerName) { + applyRequestTimeout(req, (bounds && bounds.timeoutMs) || 0, providerName); + applyRequestDeadline(req, (bounds && bounds.deadlineMs) || 0, providerName); +} + +module.exports = { DEFAULT_TIMEOUT_MS, resolveTimeoutMs, applyRequestTimeout, applyRequestDeadline, applyRequestBounds }; diff --git a/src/provider-ollama.js b/src/provider-ollama.js index 19a2049..31bccb5 100644 --- a/src/provider-ollama.js +++ b/src/provider-ollama.js @@ -4,7 +4,7 @@ const http = require('http'); const { ProviderError } = require('./errors'); const { requestWithTemperatureFallback } = require('./provider-temperature'); const { normalizeStopReason } = require('./provider-stop-reason'); -const { resolveTimeoutMs, applyRequestTimeout } = require('./provider-http'); +const { resolveTimeoutMs, applyRequestBounds } = require('./provider-http'); /** @typedef {import('../types').Message} Message */ /** @typedef {import('../types').ToolDef} ToolDef */ @@ -15,7 +15,8 @@ const { resolveTimeoutMs, applyRequestTimeout } = require('./provider-http'); * @property {string} [model='llama3.2'] * @property {string} [url='http://localhost:11434'] * @property {boolean} [exposeErrorBody=false] - * @property {number} [timeoutMs=600000] - BA-18: request/idle timeout in ms. Bounds a silent or never-answering socket on inactivity so `generate()` rejects with a retryable `TimeoutError` (`code: 'ETIMEDOUT'`) instead of hanging until the OS TCP timeout. `0`/`Infinity` disables it. Overridable per call via `generate(..., { timeoutMs })`. (A local Ollama that is loading a large model cold can be slow to first byte — raise this or disable it for very large local models.) + * @property {number} [timeoutMs=600000] - BA-18: request/idle timeout in ms. Bounds a silent or never-answering socket on inactivity so `generate()` rejects with a retryable `TimeoutError` (`code: 'ETIMEDOUT'`, `context.bound: 'idle'`) instead of hanging until the OS TCP timeout. `0`/`Infinity` disables it. Overridable per call via `generate(..., { timeoutMs })`. (A local Ollama that is loading a large model cold can be slow to first byte — raise this or disable it for very large local models.) + * @property {number} [deadlineMs=0] - BA-19: TOTAL call-duration deadline in ms, beside `timeoutMs`. The idle bound resets on any socket activity, so a response that trickles a byte forever never trips it and hangs for hours. This is an absolute, non-resetting ceiling; on trip, `generate()` rejects with a TERMINAL `TimeoutError` (`code: 'EDEADLINE'`, `context.bound: 'deadline'`, `retryable: false`). DISABLED by default; `0`/`Infinity` disable. Overridable per call via `generate(..., { deadlineMs })`. (A cold large-model load is a legitimate long single call — leave this disabled or set it generously for such models.) */ class OllamaProvider { @@ -29,13 +30,15 @@ class OllamaProvider { this.exposeErrorBody = options.exposeErrorBody === true; // BA-18: request/idle timeout (ms). Resolved at call time (default 600000; 0/Infinity disable). this.timeoutMs = options.timeoutMs; + // BA-19: total call-duration deadline (ms). Resolved at call time (default 0 = disabled). + this.deadlineMs = options.deadlineMs; } /** * Generate a response from a local Ollama instance. * @param {Message[]} messages - Conversation messages. * @param {ToolDef[]} [tools=[]] - Tool definitions. - * @param {Record} [options={}] - Options (`temperature`, `maxTokens`, `timeoutMs` — a per-call override of the constructor's `timeoutMs`; see BA-18). + * @param {Record} [options={}] - Options (`temperature`, `maxTokens`, `timeoutMs` — a per-call override of the constructor's `timeoutMs`, see BA-18; `deadlineMs` — a per-call override of the constructor's `deadlineMs`, see BA-19). * @returns {Promise} * @throws {Error} `[OllamaProvider] ...` — on HTTP errors or invalid JSON response. */ @@ -69,8 +72,9 @@ class OllamaProvider { // BA-10: graceful degrade if a model rejects a non-default `temperature` (Ollama nests it under // `options`). Keyed off the API error text, so dormant on models that accept temperature. const timeoutMs = resolveTimeoutMs(this.timeoutMs, options.timeoutMs); + const deadlineMs = resolveTimeoutMs(this.deadlineMs, options.deadlineMs, 0, 'deadlineMs'); const { data, temperatureDropped } = await requestWithTemperatureFallback({ - request: () => this._request('/api/chat', body, timeoutMs), + request: () => this._request('/api/chat', body, timeoutMs, deadlineMs), hadTemperature: () => body.options?.temperature != null, stripTemperature: () => { if (body.options) delete body.options.temperature; }, warnOnce: () => this._warnTemperatureDropped(), @@ -116,9 +120,10 @@ class OllamaProvider { * @param {string} path * @param {Record} body * @param {number} [timeoutMs=0] - Idle-socket timeout (ms); 0 disables. See BA-18 / provider-http. + * @param {number} [deadlineMs=0] - Total call-duration deadline (ms); 0 disables. See BA-19 / provider-http. * @returns {Promise} */ - _request(path, body, timeoutMs = 0) { + _request(path, body, timeoutMs = 0, deadlineMs = 0) { return new Promise((resolve, reject) => { const url = new URL(this.url + path); const payload = JSON.stringify(body); @@ -147,7 +152,7 @@ class OllamaProvider { } }); }); - applyRequestTimeout(req, timeoutMs, 'OllamaProvider'); + applyRequestBounds(req, { timeoutMs, deadlineMs }, 'OllamaProvider'); req.on('error', reject); req.write(payload); req.end(); diff --git a/src/provider-openai.js b/src/provider-openai.js index 7b802aa..4350c50 100644 --- a/src/provider-openai.js +++ b/src/provider-openai.js @@ -5,7 +5,7 @@ const http = require('http'); const { ProviderError } = require('./errors'); const { requestWithTemperatureFallback } = require('./provider-temperature'); const { normalizeStopReason } = require('./provider-stop-reason'); -const { resolveTimeoutMs, applyRequestTimeout } = require('./provider-http'); +const { resolveTimeoutMs, applyRequestBounds } = require('./provider-http'); /** @typedef {import('../types').Message} Message */ /** @typedef {import('../types').ToolDef} ToolDef */ @@ -30,8 +30,14 @@ function isLoopbackHost(hostname) { * debugging only. * @property {number} [timeoutMs=600000] - BA-18: request/idle timeout in ms. Bounds a silent or * never-answering socket on inactivity so `generate()` rejects with a retryable `TimeoutError` - * (`code: 'ETIMEDOUT'`) instead of hanging until the OS TCP timeout (~2h). `0`/`Infinity` - * disables it (pre-BA-18 behaviour). Overridable per call via `generate(..., { timeoutMs })`. + * (`code: 'ETIMEDOUT'`, `context.bound: 'idle'`) instead of hanging until the OS TCP timeout (~2h). + * `0`/`Infinity` disables it (pre-BA-18 behaviour). Overridable per call via `generate(..., { timeoutMs })`. + * @property {number} [deadlineMs=0] - BA-19: TOTAL call-duration deadline in ms, beside `timeoutMs`. + * The idle bound resets on any socket activity, so a response that trickles a byte forever never + * trips it and hangs for hours. This is an absolute, non-resetting ceiling; on trip, `generate()` + * rejects with a TERMINAL `TimeoutError` (`code: 'EDEADLINE'`, `context.bound: 'deadline'`, + * `retryable: false`). DISABLED by default; `0`/`Infinity` disable. Overridable per call via + * `generate(..., { deadlineMs })`. */ class OpenAIProvider { @@ -45,13 +51,15 @@ class OpenAIProvider { this.exposeErrorBody = options.exposeErrorBody === true; // BA-18: request/idle timeout (ms). Resolved at call time (default 600000; 0/Infinity disable). this.timeoutMs = options.timeoutMs; + // BA-19: total call-duration deadline (ms). Resolved at call time (default 0 = disabled). + this.deadlineMs = options.deadlineMs; } /** * Generate a response from the OpenAI API. * @param {Message[]} messages - Conversation messages. * @param {ToolDef[]} [tools=[]] - Tool definitions. - * @param {Record} [options={}] - Options (temperature, maxTokens, timeoutMs — a per-call override of the constructor's `timeoutMs`; see BA-18). + * @param {Record} [options={}] - Options (temperature, maxTokens, timeoutMs — a per-call override of the constructor's `timeoutMs`, see BA-18; deadlineMs — a per-call override of the constructor's `deadlineMs`, see BA-19). * @returns {Promise} * @throws {Error} `[OpenAIProvider] ...` — on HTTP errors (4xx/5xx) or invalid JSON response. */ @@ -73,8 +81,9 @@ class OpenAIProvider { // BA-10: newer models (o1/gpt-5-class) reject a non-default `temperature` with a 400 — drop it and // retry once. `temperatureDropped` flows back so an upstream receipt can report the effective value. const timeoutMs = resolveTimeoutMs(this.timeoutMs, options.timeoutMs); + const deadlineMs = resolveTimeoutMs(this.deadlineMs, options.deadlineMs, 0, 'deadlineMs'); const { data, temperatureDropped } = await requestWithTemperatureFallback({ - request: () => this._request('/chat/completions', body, timeoutMs), + request: () => this._request('/chat/completions', body, timeoutMs, deadlineMs), hadTemperature: () => body.temperature != null, stripTemperature: () => { delete body.temperature; }, warnOnce: () => this._warnTemperatureDropped(), @@ -133,9 +142,10 @@ class OpenAIProvider { * @param {string} path * @param {Record} body * @param {number} [timeoutMs=0] - Idle-socket timeout (ms); 0 disables. See BA-18 / provider-http. + * @param {number} [deadlineMs=0] - Total call-duration deadline (ms); 0 disables. See BA-19 / provider-http. * @returns {Promise} */ - _request(path, body, timeoutMs = 0) { + _request(path, body, timeoutMs = 0, deadlineMs = 0) { return new Promise((resolve, reject) => { const url = new URL(this.baseUrl + path); const transport = url.protocol === 'https:' ? https : http; @@ -177,7 +187,7 @@ class OpenAIProvider { } }); }); - applyRequestTimeout(req, timeoutMs, 'OpenAIProvider'); + applyRequestBounds(req, { timeoutMs, deadlineMs }, 'OpenAIProvider'); req.on('error', reject); req.write(payload); req.end(); diff --git a/test/provider-deadline.test.js b/test/provider-deadline.test.js new file mode 100644 index 0000000..84ac41e --- /dev/null +++ b/test/provider-deadline.test.js @@ -0,0 +1,263 @@ +'use strict'; + +// BA-19 — a TOTAL call-duration deadline on the http(s) providers, beside the BA-18 idle bound. +// The idle bound (`req.setTimeout`) resets on ANY socket activity, so a response that trickles a +// byte forever (a "zombie stream") never trips it and hangs the caller for hours (274 min observed). +// This adds an absolute, non-resetting wall-clock ceiling: on trip, generate() rejects with a +// TERMINAL TimeoutError (code:'EDEADLINE', context.bound:'deadline', retryable:false). These tests +// drive real loopback servers so they can FAIL. + +const { describe, it } = require('node:test'); +const assert = require('node:assert/strict'); +const http = require('node:http'); + +const { AnthropicProvider } = require('../src/provider-anthropic'); +const { OpenAIProvider } = require('../src/provider-openai'); +const { GeminiProvider } = require('../src/provider-gemini'); +const { OllamaProvider } = require('../src/provider-ollama'); +const { resolveTimeoutMs, applyRequestDeadline } = require('../src/provider-http'); +const { Retry } = require('../src/retry'); +const { TimeoutError, ValidationError } = require('../src/errors'); + +// A server that BEGINS a 200 response then trickles a byte every `intervalMs`, forever — never ends. +// This is the exact zombie-stream shape the idle bound cannot catch: bytes keep arriving, so +// `req.setTimeout` keeps resetting, while the response never completes. +function tricklingServer(intervalMs) { + /** @type {Array<() => void>} */ + const stops = []; + const server = http.createServer((req, res) => { + res.writeHead(200, { 'Content-Type': 'application/json' }); + const iv = setInterval(() => { try { res.write('x'); } catch { clearInterval(iv); } }, intervalMs); + const stop = () => clearInterval(iv); + stops.push(stop); + req.on('close', stop); + }); + return new Promise((resolve) => + server.listen(0, '127.0.0.1', () => + resolve({ server, stops, url: `http://127.0.0.1:${server.address().port}` }))); +} +function closeTrickling(s) { + s.stops.forEach(fn => { try { fn(); } catch { /* noop */ } }); + s.server.close(); +} + +// A server that accepts and NEVER writes a byte — a fully silent socket (BA-18's shape). +function silentServer() { + /** @type {import('http').ServerResponse[]} */ + const held = []; + const server = http.createServer((req, res) => { held.push(res); }); + return new Promise((resolve) => + server.listen(0, '127.0.0.1', () => + resolve({ server, held, url: `http://127.0.0.1:${server.address().port}` }))); +} +function closeSilent(s) { + s.held.forEach(r => { try { r.socket.destroy(); } catch { /* noop */ } }); + s.server.close(); +} + +// A server that responds after `delayMs` with a canned 200 JSON body. +function slowServer(delayMs, body) { + const server = http.createServer((req, res) => { + setTimeout(() => { + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify(body)); + }, delayMs); + }); + return new Promise((resolve) => + server.listen(0, '127.0.0.1', () => + resolve({ server, url: `http://127.0.0.1:${server.address().port}` }))); +} + +const ANTHROPIC_OK = { content: [{ type: 'text', text: 'hi' }], stop_reason: 'end_turn', usage: { input_tokens: 1, output_tokens: 1 } }; +const OPENAI_OK = { choices: [{ message: { content: 'hi', role: 'assistant' }, finish_reason: 'stop' }], usage: { prompt_tokens: 1, completion_tokens: 1 } }; +const GEMINI_OK = { candidates: [{ content: { parts: [{ text: 'hi' }] }, finishReason: 'STOP' }], usageMetadata: { promptTokenCount: 1, candidatesTokenCount: 1 } }; +const OLLAMA_OK = { message: { content: 'hi', role: 'assistant' }, done: true, done_reason: 'stop', prompt_eval_count: 1, eval_count: 1 }; +const OK_BODY = { Anthropic: ANTHROPIC_OK, OpenAI: OPENAI_OK, Gemini: GEMINI_OK, Ollama: OLLAMA_OK }; + +const MSGS = [{ role: 'user', content: 'hi' }]; + +// `opts` sets timeoutMs (idle) and/or deadlineMs (total). Ollama takes `url`, the rest `baseUrl`. +function makeProvider(kind, url, opts) { + if (kind === 'Anthropic') return new AnthropicProvider({ apiKey: 'x', baseUrl: url, ...opts }); + if (kind === 'OpenAI') return new OpenAIProvider({ apiKey: 'x', baseUrl: url, ...opts }); + if (kind === 'Gemini') return new GeminiProvider({ apiKey: 'x', baseUrl: url, ...opts }); + if (kind === 'Ollama') return new OllamaProvider({ url, ...opts }); + throw new Error('unknown kind ' + kind); +} + +const KINDS = ['Anthropic', 'OpenAI', 'Gemini', 'Ollama']; + +describe('BA-19: a zombie stream trips the total-duration deadline (all four providers)', () => { + for (const kind of KINDS) { + it(`${kind}: a trickling-but-never-completing response rejects with EDEADLINE, not a hang`, async () => { + const s = await tricklingServer(60); // a byte every 60ms → the idle bound (1000ms) keeps resetting + // timeoutMs 1000 (idle) never fires because activity resets it; deadlineMs 400 is the hard cap. + const p = makeProvider(kind, s.url, { timeoutMs: 1000, deadlineMs: 400 }); + const t0 = Date.now(); + let err; + try { await p.generate(MSGS, []); } + catch (e) { err = e; } + const dt = Date.now() - t0; + closeTrickling(s); + assert.ok(err, `${kind}: generate must reject the zombie stream, not hang`); + assert.equal(err.code, 'EDEADLINE', `${kind}: the deadline trip carries code EDEADLINE`); + assert.equal(err.context && err.context.bound, 'deadline', `${kind}: context.bound names which timer fired`); + assert.equal(err.retryable, false, `${kind}: a deadline is terminal, not retryable`); + // Fired on the deadline (~400ms), well before the idle bound and the OS TCP window. + assert.ok(dt >= 350 && dt < 2000, `${kind}: rejected on the deadline (was ${dt}ms)`); + }); + } +}); + +describe('BA-19 criterion 3: with timeoutMs < deadlineMs, a SILENT socket trips the idle bound FIRST', () => { + for (const kind of KINDS) { + it(`${kind}: idle (ETIMEDOUT/idle) fires before the deadline`, async () => { + const s = await silentServer(); + const p = makeProvider(kind, s.url, { timeoutMs: 120, deadlineMs: 3000 }); + const t0 = Date.now(); + let err; + try { await p.generate(MSGS, []); } + catch (e) { err = e; } + const dt = Date.now() - t0; + closeSilent(s); + assert.ok(err, `${kind}: must reject`); + assert.equal(err.code, 'ETIMEDOUT', `${kind}: the idle bound won (not EDEADLINE)`); + assert.equal(err.context && err.context.bound, 'idle', `${kind}: bound names the idle timer`); + assert.equal(err.retryable, true, `${kind}: an idle trip stays retryable`); + assert.ok(dt >= 90 && dt < 2000, `${kind}: fired on the idle bound (was ${dt}ms)`); + }); + } +}); + +describe('BA-19: no false trip, and disable semantics', () => { + it('a fast response with a deadline set returns the real body (no false trip)', async () => { + const s = await slowServer(0, ANTHROPIC_OK); + // Generous bounds on purpose — this asserts the deadline does NOT fire, so it must not race a + // loopback round-trip under CPU load. The "deadline fires" cases own the tight bounds. + const p = new AnthropicProvider({ apiKey: 'x', baseUrl: s.url, timeoutMs: 5000, deadlineMs: 5000 }); + const r = await p.generate(MSGS, []); + s.server.close(); + assert.equal(r.text, 'hi'); + }); + + it('deadlineMs is DISABLED by default — a slow-but-completing response is not killed', async () => { + // No deadlineMs at all (the default is 0/disabled); a 300ms single-body response must complete. + const s = await slowServer(300, ANTHROPIC_OK); + const p = new AnthropicProvider({ apiKey: 'x', baseUrl: s.url, timeoutMs: 5000 }); + const r = await p.generate(MSGS, []); + s.server.close(); + assert.equal(r.text, 'hi', 'a legitimate long call must survive with no deadline set'); + }); + + it('negative control: the SAME zombie stream with deadlineMs:0 HANGS (idle cannot catch it)', async () => { + // Proves the deadline is load-bearing: with only the idle bound (which keeps resetting on the + // trickle), the call never returns — bounded here only by an outer race. + const s = await tricklingServer(60); + const p = new AnthropicProvider({ apiKey: 'x', baseUrl: s.url, timeoutMs: 1000, deadlineMs: 0 }); + let outerFired = false; + let err; + try { + await Promise.race([ + p.generate(MSGS, []), + new Promise((_, rej) => setTimeout(() => { outerFired = true; rej(new Error('outer')); }, 800)), + ]); + } catch (e) { err = e; } + closeTrickling(s); + assert.ok(outerFired, 'with deadlineMs:0 the zombie stream is never bounded by the provider'); + assert.equal(err.message, 'outer'); + }); + + it('a per-call deadlineMs overrides an instance-level disable', async () => { + const s = await tricklingServer(60); + const p = new AnthropicProvider({ apiKey: 'x', baseUrl: s.url, timeoutMs: 1000, deadlineMs: 0 }); + let err; + try { await p.generate(MSGS, [], { deadlineMs: 400 }); } + catch (e) { err = e; } + closeTrickling(s); + assert.equal(err && err.code, 'EDEADLINE', 'a per-call deadline must bound even when the instance disabled it'); + }); + + it('a garbage deadlineMs throws ValidationError at generate() — never a silent unbounded run (review finding 1)', async () => { + // A config typo (a non-numeric/NaN deadlineMs) must surface loudly, not silently disable the + // deadline and run for hours. The bad value is caught at resolve time — the socket is never opened. + const p = new AnthropicProvider({ apiKey: 'x', baseUrl: 'http://127.0.0.1:1', deadlineMs: NaN }); + await assert.rejects(() => p.generate(MSGS, []), (e) => e instanceof ValidationError && e.code === 'VALIDATION_ERROR'); + }); +}); + +describe('BA-19: resolveTimeoutMs with defaultMs:0 (the deadline default — disabled)', () => { + it('absent instance and call → 0 (disabled by design, NOT the idle 10-min default)', () => { + assert.equal(resolveTimeoutMs(undefined, undefined, 0), 0); + }); + it('a per-call value wins over the instance value', () => { + assert.equal(resolveTimeoutMs(1000, 5000, 0), 5000); + assert.equal(resolveTimeoutMs(5000, undefined, 0), 5000); + }); + it('0 and Infinity are the explicit opt-out (return 0)', () => { + assert.equal(resolveTimeoutMs(0, undefined, 0), 0); + assert.equal(resolveTimeoutMs(Infinity, undefined, 0), 0); + assert.equal(resolveTimeoutMs(1000, 0, 0), 0); + }); + it('NaN / negative / garbage EXPLICITLY set THROWS — never a silent disable of the deadline (review finding 1)', () => { + // With defaultMs:0 (deadline) there is no safe bound to fall back to, so a garbage value that + // silently returned 0 would reintroduce the zombie-stream hang. Fail loud instead. + assert.throws(() => resolveTimeoutMs(NaN, undefined, 0, 'deadlineMs'), (e) => e instanceof ValidationError && /invalid deadlineMs/.test(e.message)); + assert.throws(() => resolveTimeoutMs(-1, undefined, 0, 'deadlineMs'), (e) => e instanceof ValidationError && /expected a positive number/.test(e.message)); + assert.throws(() => resolveTimeoutMs('30s', undefined, 0, 'deadlineMs'), (e) => e instanceof ValidationError && /30s/.test(e.message)); + // But UNSET (null/undefined) is legitimate and still resolves to 0/disabled — only garbage throws. + assert.equal(resolveTimeoutMs(undefined, undefined, 0), 0); + assert.equal(resolveTimeoutMs(null, null, 0), 0); + }); + it('the SAME garbage value falls back SAFE (not a throw) when the knob has a real default — BA-18 idle unchanged', () => { + // defaultMs>0 (idle bound): a garbage value keeps the 10-min safety net, byte-identical to BA-18. + assert.equal(resolveTimeoutMs(NaN, undefined), 600000); + assert.equal(resolveTimeoutMs(-1, undefined), 600000); + assert.equal(resolveTimeoutMs('nope', undefined), 600000); + }); + it('null and undefined per-call both INHERIT the instance — null never shadows an instance value', () => { + assert.equal(resolveTimeoutMs(1000, null, 0), 1000); + assert.equal(resolveTimeoutMs(1000, undefined, 0), 1000); + assert.equal(resolveTimeoutMs(0, null, 0), 0); + }); + it('applyRequestDeadline is a no-op when deadlineMs is 0 (never arms a timer)', () => { + let armed = false; + const fakeReq = { once: () => { armed = true; }, destroy: () => {} }; + applyRequestDeadline(/** @type {any} */ (fakeReq), 0, 'X'); + assert.equal(armed, false, 'a disabled deadline must not arm a timer or wire a close handler'); + }); +}); + +describe('BA-19: EDEADLINE is TERMINAL — a wired Retry does NOT retry it (contrast the idle trip)', () => { + it('the deadline error advertises retryable:false and a distinct code', () => { + const err = new TimeoutError('[X] exceeded its total deadline', { code: 'EDEADLINE', retryable: false, context: { bound: 'deadline' } }); + assert.equal(err.code, 'EDEADLINE'); + assert.equal(err.retryable, false); + assert.equal(err.context.bound, 'deadline'); + }); + + it('a generate that throws EDEADLINE under a default Retry is NOT retried (terminal)', async () => { + // Mirrors loop.js: `this.retry ? await this.retry.call(generate) : await generate()`. + // DEFAULT_RETRY_ON returns false for retryable:false → one attempt, rethrows. + let calls = 0; + const generate = async () => { + calls++; + throw new TimeoutError('[AnthropicProvider] request exceeded its total deadline of 400ms', { code: 'EDEADLINE', retryable: false, context: { bound: 'deadline' } }); + }; + const retry = new Retry({ maxAttempts: 3, backoff: 1 }); + await assert.rejects(() => retry.call(generate), (e) => e.code === 'EDEADLINE'); + assert.equal(calls, 1, 'a terminal deadline is not retried — one attempt only'); + }); + + it('an idle ETIMEDOUT under the SAME Retry IS retried (proves the difference is the retryable flag)', async () => { + let calls = 0; + const generate = async () => { + calls++; + if (calls === 1) throw new TimeoutError('[AnthropicProvider] idle', { context: { bound: 'idle' } }); + return { text: 'recovered' }; + }; + const retry = new Retry({ maxAttempts: 3, backoff: 1 }); + const result = await retry.call(generate); + assert.equal(calls, 2, 'the idle trip retried once'); + assert.equal(result.text, 'recovered'); + }); +});