fix(llm): a streaming llama response is bounded by idle time, not total time

[plombeer31]

The defect

LlamaServerClient.completeStream applied localModels.requestTimeoutMs
(default 300 000 ms, ENV_DEFAULTS.REQUEST_TIMEOUT_MS) as a total
wall-clock deadline over the whole streaming generation.

createRequestController armed exactly one timer when the request was
built:

const timer = setTimeout(() => { expired = true; controller.abort(); },
                         this.requestTimeoutMs);

Nothing ever refreshed it. completeStream then handed that controller to
fetch and read the SSE body in a while (true) { await reader.read() … }
loop, with cleanup() only in the outer finally. So the abort fired 300 s
after the request was sent — mid-stream, while the server was emitting
tokens perfectly healthily.

Who this hits: a reasoning model on CPU, a large model on a LAN box, any
generation whose honest duration exceeds five minutes.

The consequence chain

1. The stream is killed at exactly 300 s. reader.read() throws,
   wrapTransportError sees timedOut === true and reports
   "llama-server request exceeded requestTimeoutMs (300000ms) — raise
   localModels.requestTimeoutMs or lower completionMaxTokens". Every
   partial token already produced is discarded.
2. isRetryableLlamaError refuses to replay a timedOut error — correctly,
   by its own doc comment — so there is no retry.
3. timedOutOf() in src/llm/fallback/should-advance.ts deliberately makes
   a self-inflicted timeout non-immediate, so the fallback chain does not
   step in either.
4. The turn simply dies.

The local-vs-cloud asymmetry

The cloud path has never behaved this way. In
src/llm/provider/openai/openai-http.ts, openAiFetch sets the same kind of
timer but clears it in finally { clearTimeout(timer) } when the fetch
promise settles — i.e. as soon as response headers arrive. For every
OpenAI-compatible provider requestTimeoutMs therefore bounds only the
connect/headers phase and the stream body is unbounded.

One config key, two incompatible meanings depending on which provider is
serving. That is the part users cannot possibly reason about.

The fix

The streaming deadline becomes an idle deadline: it bounds time since
the last byte, not total generation time. A second, never-refreshed timer
keeps an absolute ceiling on the whole streaming body, so "not a total
budget" does not mean "no budget".

• createRequestController now also returns:
  • keepAlive(kind) — clears and re-arms the idle timer and records what a
    later expiry means (first-token or idle). It is a no-op once the
    request is already aborted or a deadline has already fired, so a late
    call cannot resurrect a controller nor rewrite which deadline is
    reported.
  • startStreamDeadline() — arms the absolute streaming cap
    (streamTotalTimeoutMs, below). Idempotent, armed once at headers, never
    refreshed.
• completeStream calls keepAlive("first-token") and
  startStreamDeadline() immediately after headers, and keepAlive("idle")
  after every reader.read() that returns data.
• cleanup() clears both timers. A live stream holds exactly two pending
  timers and zero once the generator finishes, returns, or is broken out of
  early.
• complete() and fetchProps() are untouched. A unary request has exactly
  one event to wait for and no idle signal to refresh against, so a total
  budget is the only budget it can have.

Why keepAlive() at headers, before the loop. llama.cpp answers with
response headers immediately and only then evaluates the prompt. A long
CPU prompt-eval is legitimately silent for minutes before the first token.
Re-arming at headers gives the body a full budget of its own rather than
whatever the connect phase happened to leave over, and makes every
body-phase timeout report a body-phase wording.

Why not on done. done breaks straight out of the loop into
finally { cleanup() } with nothing awaited in between, so there is no
window in which the timer could fire. Re-arming there would be dead code.

The new upper bound: streamTotalTimeoutMs

An idle deadline on its own is not an upper bound. A server emitting one
byte every requestTimeoutMs - 1 refreshes it forever, and nothing else on
the turn path stops it: src/agent arms no timers, there is no
AbortSignal.timeout anywhere on the turn path (every occurrence in the
tree is a health probe, an HF/catalog fetch, embeddings, or browser spawn),
and ctx.signal is user-driven only. Before this PR, requestTimeoutMs did
at least bound one local completion at 300 s; dropping that with nothing in
its place would let a wedged or hostile llama-server pin a slot, a session
and — under headless run or a cron task — a process indefinitely. That
would be a capability regression introduced by this PR, not inherited cloud
behaviour, so it gets an explicit backstop rather than a note.

New key, following the existing pattern for local-model knobs:

config	localModels.streamTotalTimeoutMs (AtomicAgentConfig)
env override	ATOMIC_AGENT_LLAMA_STREAM_TOTAL_TIMEOUT_MS
default	ENV_DEFAULTS.STREAM_TOTAL_TIMEOUT_MS = 6 h
client override	LlamaServerClientOptions.streamTotalTimeoutMs

It is read in src/config/load-config.ts next to requestTimeoutMs with the
same readInt helper. It is deliberately not added to
src/config/llm-config.ts: that file validates per-provider user config
file entries, and requestTimeoutMs there is a provider-registry key —
localModels.requestTimeoutMs itself has never been a user-config-file
field, only an env one, so a per-provider streamTotalTimeoutMs would be a
new surface with no sibling to be consistent with.

Why 6 h. It has to be a backstop, not a budget — it must never fire for
the long-CPU-prompt-eval population this PR exists to protect. The worst
honest local generation I can construct with default settings is
completionMaxTokens = 8 192 tokens decoded at 0.4 tok/s, slower than any
CPU setup people actually sit through, which is ≈ 5.7 h. 6 h clears that. It
is also 72× REQUEST_TIMEOUT_MS, so the idle deadline gets dozens of
chances to fire first and this one only fires when the server was genuinely
streaming continuously for six hours without finishing. Anyone actually
running a 131 072-token completion on a slow box raises the env var; the
error message names it.

Honest messages

timedOut() now reports which deadline fired
("total" | "first-token" | "idle" | "stream-total" | null) so each case
keeps accurate, non-contradictory advice. The union stays private:
LlamaServerError.timedOut is still a boolean and no consumer sees a string.

• total (unary complete()) — unchanged wording: llama-server request exceeded requestTimeoutMs (300000ms) — raise localModels.requestTimeoutMs or lower completionMaxTokens
• first-token (stream, headers in, no body byte yet):
  llama-server accepted the request but sent no first token within 300000ms — it may still be evaluating the prompt; raise localModels.requestTimeoutMs, or shorten the prompt/context if it is too large for this machine to evaluate in time
• idle (stream, at least one byte, then silence): llama-server sent no data for 300000ms mid-stream — the server stopped responding after starting the reply; check that llama-server is still running, or raise localModels.requestTimeoutMs
• stream-total: llama-server streamed for longer than streamTotalTimeoutMs (21600000ms) without finishing — data kept arriving, so this is the absolute cap on one streaming reply, not a stall; raise localModels.streamTotalTimeoutMs (ATOMIC_AGENT_LLAMA_STREAM_TOTAL_TIMEOUT_MS) or lower completionMaxTokens

The split between first-token and idle matters as much as the
original fix. "The server stopped responding after starting the reply" is
false when the server never started the reply, and a silence before the
first token is precisely what a healthy llama-server looks like while
grinding a long context on CPU. Telling that user their server is dead would
have replaced one piece of bad advice with another for the same population.
"Lower completionMaxTokens" is likewise wrong for a stall: the answer was
not too long, it never came.

Known limitation: the idle deadline also measures consumer backpressure

keepAlive() runs only when the generator's read loop runs; between yields
the generator is suspended and the body applies backpressure, so a consumer
stalled for longer than requestTimeoutMs trips the idle timer and the user
is told the server went quiet when the stall was ours. This is not a
regression — main's total budget had the same exposure, worse, since it
counted consumer time against a non-refreshing clock — and it needs a >300 s
stall inside the consuming loop to reach. It is called out because the new
wording asserts a specific server fault where the old one only asserted
that time had passed.

Decision: an idle stall still sets timedOut = true

This was the one genuinely arguable call, so spelling it out.

The tempting read is that silence is stronger evidence of a dead provider
than slowness, which would argue for timedOut = false — that would make
isImmediateSignal in should-advance.ts treat the null status as an
unambiguous provider-down signal and fall over to the next chain link on the
first occurrence.

I did not do that, for one concrete reason: llama.cpp streams headers
before it evaluates the prompt. A 300 s silence is exactly what a healthy
llama-server looks like while grinding a long context on CPU — the very
population this PR is trying to stop breaking. Making that an immediate
fallover would trade one bad failure mode for another.

So the behaviour is unchanged on both axes:

• isRetryableLlamaError still refuses to replay it. (Note this is
  academic for the stall itself: mid-body errors are thrown outside
  runWithRetry, which only wraps the initial fetch. It matters for the
  connect-phase timeout, which is still "total" and still non-retryable.)
• shouldAdvance still returns { advance: true, immediate: false } — the
  stall counts toward the consecutive-failure threshold and never triggers a
  first-occurrence switch.

The field's doc comment now records this reasoning so the next reader does
not "fix" it.

Sentry evidence

Stated carefully: this data is consistent with the diagnosis, not proof
of it. This project scrubs error messages by design, so the timeout string
itself is not visible in the events — the case rests on the code path plus
the shape of the tags.

CLI-BX — 38 events / 3 users, first seen 2026-09-01, exclusively on
release 0.5.4, category=transport, cause_type=LlamaServerError.

• transport_host: 192.168.1.18:8080 (25), 127.0.0.1:8080 (10),
  127.0.0.1:19091 (3). The LAN host dominating the sample is the tell — a
  remote, slower llama-server is exactly the population that blows a total
  wall-clock budget.
• No code tag at all. An abort leaves no errno, unlike a socket
  failure. This is our own controller firing, not the network dying.
• Frames: async meterStream ← async run ← async stampServedTransport ←
  async replayPrimedStream ← async completeStream. The failure happens
  while consuming an already-primed stream body — precisely the window
  this timer covered, and the one window nothing retries.

CLI-BK — 6 events / 3 users, code = UND_ERR_BODY_TIMEOUT (3),
ECONNRESET (2), UND_ERR_SOCKET (1), culprit Fetch.onAborted, releases
0.4.2 and 0.5.4. UND_ERR_BODY_TIMEOUT is undici's own inter-chunk body
timeout — a second, independent idle deadline living in the same window,
which this PR does not touch. Worth knowing it exists: if stalls persist
after this change, undici's bodyTimeout (300 s default) is the next knob,
and it is already an idle deadline, so it is the correct shape.

Tests

Seven tests in src/llm/llama-server-client.test.ts, all on fake timers,
driving a hand-pushed SSE body whose abort signal errors the stream
mid-read the way undici does.

test	on main
keeps streaming past requestTimeoutMs while chunks keep arriving	fails
aborts a stream that goes silent for longer than requestTimeoutMs	fails
reports a stall before the first token as a prompt eval, not a dead server	fails
caps one streaming response with streamTotalTimeoutMs even while chunks keep arriving	fails
does not let a byte still in flight rewrite which deadline fired	fails
still enforces a total deadline on the unary complete() path	passes (pin)
lets an external abort cancel mid-stream without reporting a timeout	passes (pin)

Verified by restoring src/llm/llama-server-client.ts to origin/main and
re-running with the new tests in place: Tests 5 failed | 27 passed (32),
failing exactly the five rows above. The first is the defect reproducing
verbatim: six chunks 999 ms apart under a 1 000 ms budget — 5 994 ms of
healthy streaming, every gap comfortably inside the budget — and main
still kills it.

The two pins pass on main by construction; they exist so nobody widens the
unary deadline or launders a user cancellation into a timeout later. Calling
them "regression tests" would be dishonest, so they are labelled as pins.

Three of the five are there to pin things that were previously asserted in
prose only:

• the keepAlive() at headers. Deleting that one line used to pass the
  whole file, because the existing stall test pushes a chunk first and the
  loop's keepAlive() masked the deletion. It now fails two tests.
• the no-op-once-aborted guard. Removing
  if (expired !== null || controller.signal.aborted) return; used to pass
  the whole file too. The test drives the narrow window it actually
  defends: the abort has landed but a byte already in the decode pipe is
  still delivered, so the read loop calls keepAlive() after the deadline
  fired. Without the guard that re-arms the timer, which fires a second
  time and overwrites the reason — the user is told the server stalled
  mid-reply when it never produced a first token at all.
• the streaming cap, above.

On the external-abort test: the cancelled category is decided upstream
in toLlmFailure from ctx.signal.aborted, not from the error's shape, and
this PR does not touch that. What the test pins at the client boundary is
the thing my change could plausibly have broken — that an Esc-abort is not
laundered into timedOut: true with an idle-timeout message.

Verification

• npm run lint (tsc --noEmit) — clean.
• npx vitest run src/llm/llama-server-client.test.ts src/llm/fallback src/llm/reliability src/llm/provider/openai — 20 files, 215 tests, all passing.
• npx vitest run src/config — 12 files, 271 tests, all passing.
• Widened to src/llm src/local-llm src/agent — 92 files, 1082 tests, all passing. No flakes in that slice.
• Mutation checks on the final tree: deleting the headers keepAlive() → 2
  failures; deleting the loop keepAlive("idle") → 3 failures; deleting
  startStreamDeadline() → 1 failure; removing the aborted/expired guard →
  1 failure.
• grep for requestTimeoutMs / exceeded requestTimeoutMs across tests: the
  string was asserted nowhere outside this file, and no other test stubs
  createRequestController (it is private and has no external callers).
• No conflict with fix(llm): provider-availability failures no longer block fallover #322: git merge-tree is clean and the two PRs share no
  files.