Release candidate 2026-09-03: PRs #309-#324 - #325
Merged
Conversation
OpenAI-compatible providers that emit one whole tool call per SSE event, with an `id` but no `index`, had every call folded onto slot 0: the parser substituted the call's position within its own event for the missing index, and the stream consumer keyed its accumulator on that number alone. Two parallel calls arrived as one, with the second call's name and arguments concatenated onto the first — a tool the registry has never heard of, or worse, the right tool run with spliced arguments. The parser now reports `index` only when the provider actually sent one, and the consumer resolves slot identity across events: provider index first, then call id, then position among the slots already open (which is what an id-less continuation delta means). Provider indexes still decide the output order, so a stream that opens slot 1 before slot 0 comes out in the provider's order; unindexed slots queue up behind in arrival order. Fixes #103 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every non-SEA way of starting the CLI leaves NODE_ENV unset, so react-reconciler resolves to its development build — the one carrying React 19's Component Performance Track, eight performance.measure() call sites fired on every commit. A TUI commits continuously and nothing drains Node's global performance entry buffer, so entries accumulate for the life of the process: Node warns at one million, V8 aborts at its ~4 GB ceiling around the eleven-hour mark, and an unattended agent goes silent with no JS stack. scripts/bundle-sea.ts already defines the constant away for the SEA binary. This gives the source/dist path the same footing via a side-effect import that must stay first in cli/index.ts — ES module bodies run after their dependency graph is evaluated, so setting the variable anywhere inside index.ts would land after ink had already required the reconciler and chosen a build. An explicit NODE_ENV is still honoured. Measured over 50 Ink rerenders: 206 measure entries before, 0 after. Fixes #307 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
darwin-x64 is commented out of the release matrix, so an Intel Mac resolved a slug the installer knows how to name but the release does not carry, then died on "download failed: <url>" with a 404 behind it and no hint that the binary had never existed. Probe the asset before downloading and, on a literal 404, explain: no build for this slug, where to see what is published, and how to build from source. Intel Macs additionally get told the Apple Silicon build will not run for them, with the tracking issue. Only a 404 counts. Offline, a proxy, a 5xx, or no curl at all falls through to the real download, which reports failures as before. curl's own exit code is unusable here — a 404 that arrives after -L follows the release redirect surfaces as 56, not 22 — so the status is read directly. Whether to publish the darwin-x64 build is left as the maintainers' call. Fixes #300 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Models repeatedly picked os.fs.read_document for `.py`/`.ts` sources, got a generic "unsupported extension (override with `format`)" error, guessed the invalid `format: "text"`, and only then found os.fs.read. Nothing in the prompt or the error told them which reader owns source code, so the recovery was left to chance. Two deterministic nudges: - The stable-prefix summaries now split the two readers explicitly: os.fs.read is "the default for source code and text files", read_document is document extraction and "NOT for source code or text files: use os.fs.read". Both stay one line — the frequent-tool block ships on every turn, so the added wording is ~25 tokens. - detectFormat keeps a set of source-like extensions. They still are not mapped to `plain` (os.fs.read is the better tool: offset/limit pagination, no extractor in the way), but they now reject with a message that names os.fs.read first and spells out the accepted override `format: "plain"` instead of the bare word `format`. Unknown extensions carry no such signal, so their message offers both paths — and also names `format: "plain"`. Document formats and the plain-text extension set are untouched.
A cloud provider that answers 2xx, thinks for a long time (reasoning models, cold routes) and then drops the SSE socket before emitting a single delta failed the whole turn instantly with "Turn failed [transport]: terminated". Nobody owned that window. `openAiStartStream` retries the open, but its budget is spent the moment headers arrive; failures raised while reading the body are undici's bare `Error: terminated` and never pass through the HTTP client at all; and `createFallbackStreamer` only helps when a second provider is configured, which the reporter had not. `OpenAiProvider.completeStream` now reopens the stream while nothing has been yielded to its caller. That is the same argument `openAiStartStream` already makes for retrying the open: with zero chunks emitted, a replay is unobservable, so it cannot duplicate output. `canReopenStream` spells out the guards and their order — committed (any yielded chunk, reasoning or a bare role preamble, ends retrying for good), cancellation checked before the error is inspected, `OpenAiHttpError` rejected so the open's budget is not squared to 9 requests, `isNetworkError` so a consumer bug is not replayed, and the shared `OPENAI_MAX_ATTEMPTS` budget with `openAiRetryBackoff`'s existing pacing. The dead body is cancelled before reopening so a retry does not leak a socket. Resuming a stream that has already emitted output is deliberately left out: restarting would duplicate text or need prefill continuation, and non-deterministic sampling rules out a prefix dedupe. That is a maintainer design call, not a defect fix. Reported in Discord #feedback-and-bugs by thegreatteacher.
`ToolLoopTracker` hashes canonical arguments, so two reads of the same unchanged file with shifted `offset`/`limit` produce different signatures and neither the repeat counter nor the no-progress streak ever fires -- even when the second read returns only lines the first one already showed. `os.fs.read` is also (correctly) excluded from the wandering detector, because scanning many files is legitimate work. The result is that an overlapping re-read loop burns steps until the exact-repeat protection eventually catches a verbatim duplicate. Add a read-coverage detector beside the argument-hashing machinery rather than another way to hash arguments, because the question it answers is about the RESULT: did this read show the model a line it had not already seen? - `os.fs.read` now publishes `details.readCoverage`: the canonical (symlink-resolved) path, a hash of the bytes it actually read, and the line range it actually returned. Requested offset/limit cannot answer any of the three -- they are clamped, may be negative, and byte-mode reads carry no range at all. The hash is over content, so a same-size in-place replacement with an untouched mtime is caught. - `ToolLoopTracker.checkReadRepeat` / `recordRead` keep a per-file merged set of the lines read at the current content hash. A returned range fully inside that set (or an empty return) is no progress; new lines reset the streak; a changed hash discards the coverage entirely. - The batch gate raises a warn-only `read_repeat` signal after the call, since the read has already executed and there is nothing to veto. The notice, the `loop_detected` event and the log line carry the path, the range and the fingerprint transition -- line numbers only, never file content. Failed reads record nothing, a truncated read banks only the prefix it returned, and a scan over many distinct files never produces a signal. Fixes #114
…contradicting itself Review follow-up on the issue #113 routing fix. Three gaps in the first commit, none of which made it wrong, all of which blunted it: - `format` was still an open `string` in both the stable-prefix argsSchema and the cloud JSON Schema, so the exact bad guess the issue is about — `format: "text"` — stayed fully representable, and the runtime rejection named neither the valid values nor os.fs.read. The repo convention (see the header of default-tool-args-schemas.ts, "enums match the runtime validators verbatim", and os.fs.archive.*) is to enumerate closed sets in both places. The set now lives once, as `DOCUMENT_FORMATS` in extractor-types.ts, with `DocumentFormat` derived from it; the validator, the rejection message and both prompt surfaces all read that declaration instead of copying it. - The new prompt strings said read_document is "NOT for source code or text files" while the same sentence advertised plain-text support — and the tool really does extract .txt/.md/.csv/.json/.yaml/.html as `plain`. For a change whose whole point is removing ambiguity, that is the same failure mode one level up. Both strings now say "NOT for source code" and stop there, which is true and equally pointed. - The `description` — the text the model reads in `### loaded-tools` after tool.view, i.e. at the moment it picks the tool — was the one of the three surfaces with no test. It has one now, on the same rationale as the stable-prefix pin. Two smaller inconsistencies the review turned up in the source-like set: - `.tsv` claimed to be a "source/text file" while its twin `.csv` extracted happily. `.tsv` now joins `.csv` on the `plain` arm; the error message no longer asserts a classification the switch next to it contradicts. The rejection wording moves to "source or config file", accurate for every remaining member of the set. - `env` only ever fires for the `name.env` shape — `extname("/x/.env")` is `""`, so bare dotfiles land on the extensionless arm and extract as `plain`. That is inherited from `extname` and not worth changing, but it is now written down next to the set. The document-format guard test only covered 11 of the switch's arms; it missed legacy `.doc` (whose extractor was not even in the injected bag), `.html`, `.xml`, `.log`, `.yml` and the extensionless case, so removing the `.doc` arm would not have failed it. All of them are in the loop now.
A live API key proves the account, not the route. `/v1/models` proves
reachability, and the pre-save key check proves one non-streaming
completion with no tools — neither exercises what an Atomic turn
actually is: a streamed chat completion carrying a `tools` payload from
which a native tool call must come back whole. Routes that pass both and
then fail a turn are common (STREAM_EARLY_EOF, HTTP 400 only once
`tools` is in the body, forced tool choice refused, incomplete tool-call
deltas), and until now the operator's first message was the test.
Adds a conformance probe next to the existing key check, built as a
three-rung ladder so a healthy route costs exactly one request:
1. forced named tool, streaming, tools payload — a complete tool call
here is the whole answer;
2. `tool_choice: auto` with the same payload, reached only when rung 1
was refused for a reason that is not the key, the quota or the
model — a tool call now means it was the forcing the route refused;
3. the same streamed completion with no tools at all, reached only
when both tool requests were refused — if this answers, the route
works and it is specifically `tools` it rejects.
Rung 3 is an experiment rather than a regex over error wording, which is
not stable across providers; "refuses with tools, answers without them"
is. A text answer under `auto` stays inconclusive and is never reported
as "tools unsupported".
The synthetic function is a diagnostic fixture: nothing registers it,
nothing dispatches it, and its name sits outside the dotted namespace
every built-in tool uses so it cannot shadow one. Details are bounded
and credential-scrubbed — the exact key by match, key-shaped strings by
pattern — and that redaction is now shared with the key check.
Wired into the two setup paths (Providers wizard save, first-run cloud
onboarding) as advisory only: it can never refuse a save, so a custom
endpoint that blocks synthetic probes stays configurable, but a route it
could not prove no longer reports as a working backend. `/llm check`
runs it on demand against a provider that is already saved. It never
runs on a turn path.
The previous commit claimed, in three places, that one streaming completion gets `OPENAI_MAX_ATTEMPTS` requests "shared with the open, not a fresh budget per layer". It did not. Every reopen called `openAiStartStream`, which starts a fresh `runOpenAiWithRetry` with its own three attempts, so only the outer counter was shared. A provider that answers two 500s before each successful open and then drops the body before its first chunk issued nine requests for one turn — exactly the "3 x 3 = 9" outcome the `OpenAiHttpError` guard was justified by preventing. Nine billable prompt submissions, and on a reasoning model that thinks for a minute before dropping, minutes of frozen UI. `OpenAiAttemptBudget` is a mutable counter created once per `completeStream` and passed into every `openAiStartStream`, so opens and reopens add instead of multiplying. `runOpenAiWithRetry` spends it and indexes the backoff by requests already made, so the delay keeps growing across the open/reopen seam. Callers that pass no budget (every non-streaming path) get a private full one, unchanged. Four further corrections, each now pinned by a test that fails without the line it describes: - Esc during the inter-attempt backoff walked into the next open, because `sleep()` resolves on abort rather than throwing. The turn then failed as `transport` — `classifyFailure` files every `OpenAiHttpError` that way before it looks for an abort — and `shouldAdvance` read that as an immediate provider-down signal, switching links and restarting the completion the user had just stopped. Cancellation is now checked before the guards and after the backoff, and throws `signal.reason`, which classifies as `cancelled`. - The guard-3 doc justified `OpenAiHttpError` with budget squaring and "a bad API key would be tried nine times". A 401 is stopped by the shape guard, not that one. The guard's real job is narrower and is now described and tested as what it is: a non-retryable open failure whose message happens to look transport-shaped (a 400 whose body mentions a socket) must not have the HTTP client's verdict overridden here. - `discardResponseBody` was a no-op on every reachable path. The reopen is only reached when the body reader raised, so the stream is already errored, `cancel()` rejects with the stored error into an empty catch, and undici has already destroyed the socket. Deleted, with the reason written down where the call used to be. - The reopen loop was an unbounded `for (;;)` whose only exit was a helper's return value. Bounded by `OPENAI_MAX_ATTEMPTS`, with an explicit throw if the bound is ever what stops it — an empty completion would otherwise look to the user like a model that said nothing. Tool-call arguments never set `committed`, because the consumer yields on a `function.arguments` delta only once the arguments contain reply text. That widens the retry window past "any byte the provider sent" and was undocumented; it is safe (nothing reached the user, tools are dispatched only after the completion returns) and is now stated and pinned rather than left to be rediscovered. AGENTS.md also now records the two costs the layer does carry: no observability event, and up to a 3x wall-clock multiplier on a provider that hangs before dropping.
Review of the read-coverage detector turned up two cases where it either fired on a read that was not redundant, or fired correctly but gave advice the model could not act on. Rendering is part of the file's version. Coverage was keyed on (canonical path, content hash) alone, so re-reading an already-covered range with `lineNumbers: true` — the ordinary last step before a precise edit — counted as no progress. It is not: the numbered text carries line numbers the plain read never showed, and the notice's "re-reading a covered range returns the same text" was simply untrue there. Two plain reads followed by two numbered ones reached the warn floor. `os.fs.read` now publishes `readCoverage.numbered`, and the tracker treats a rendering switch exactly like a content change: fresh coverage, no warning. The cost is a missed detection when a model alternates renderings forever, which is the direction this detector is allowed to fail in. An empty return is not a covered-range re-read. On a file larger than `maxBytes` only the byte-capped prefix is reachable, so `offset: 3000` on a 1905-line window returns nothing and counts as no progress — correctly, it is pure waste — but the notice then told the model to "read a range you have not covered", which is the request that had just come back empty, and claimed a covered range had been re-read, which had not happened. `readCoverage.truncated` now records whether content sits behind the byte cap (the byte cap specifically, not the top-level `truncated` detail, which is also true when a line window merely ended early and another offset can still reach the rest), and the notice picks one of three remediations: name the byte cap and tell the model to raise `maxBytes`; or name the reachable window for an offset past the end of a fully readable file; or keep the original covered-range advice when the read really did return lines the turn already had. Both flags parse leniently (`=== true`, default false) so a replayed trace or an older session degrades to "plain, un-capped read" instead of switching the detector off. Also pins two pieces of the range algebra that had no test: `mergeRange` merging a span adjacent to the interval AFTER it (losing that branch leaves a one-line seam and breaks the module's own non-adjacent invariant), and `newlyCoveredCount` clamping at zero for overlapping input — callers test its result both with `> 0` and with `=== 0`, so an unclamped negative would answer "no" to both and silently reset the no-progress streak.
The detector's core was tested; everything downstream of the batch signal was not. The new tests asserted `signal.read` at the batch-executor layer and stopped there, so deleting the event payload, the recorder passthrough or the trace-formatter interpolation left the suite green — and acceptance criterion 8 (events identify the detector, path, range and fingerprint transition) rested on code nothing executed. - `src/agent/agent-loop.test.ts`: one turn through the production path — real `os.fs.read`, real batch gate, real agent-loop wiring. Three reads with three different argument hashes, only the coverage detector able to see that the last two returned nothing new. It pins the detector's own floor of 2 (the generic threshold of 3 would never fire inside the turn), the read-specific notice reaching the next prompt (the generic one talks about repeated arguments, which was never true here), the `read` payload on the emitted event, and that no line of the file appears in it. - `src/tracing/trace/trace-recorder.test.ts`: the `read` payload survives into the recorded trace event, and is absent for detectors that have none. - `src/cli/trace-formatter.test.ts`: new file. `formatTraceChronology` had no test at all, so the `loop_detected` line — the only place a human sees why the detector fired — was unpinned, including the ` detector=` field this branch adds to EVERY loop_detected line and the back-compat path for older NDJSON traces that carry neither `detector` nor `read`. Verified by mutation: with these tests in place, deleting the threshold branch, the notice call, the event payload, the recorder passthrough or either half of the formatter interpolation each fails the suite.
Review of the contract probe found several verdicts that describe something Atomic did, not something the route did. Each one reaches an operator as a sentence about their provider, so each is a bug. * **Our deadline was reported as `stream_early_eof`.** The timeout flag was discarded the moment any byte arrived, so a queued OpenRouter request (it sends `: OPENROUTER PROCESSING` while it waits) or a slow first token was classified as "closed the stream before finishing the answer. Turns will end mid-tool-call on this route" — and blocked from `reportModelConfigured`. The flag now survives: only a stream that announced its own end before the timer fired is classified at all, anything else is `timeout`, which is what it always was. * **An abort mid-stream was reported the same way.** Once headers had arrived the reader's rejection was swallowed and the partial body classified, so cancelling produced an invented route defect. The read now races the caller's signal, which also means Esc gets the screen back at once instead of at the deadline. * **Forced-tool-choice verdicts warned about a request Atomic never makes.** `step-executor` sends `tool_choice: "auto"` on every turn, deliberately, with production-observed reasons written down beside it. A route that refuses a forced choice and streams a complete call under `auto` therefore runs every real turn — it is now `proven`, not a warning that suppresses "this install has a working backend". A route that *accepts* forcing and ignores it no longer stops the ladder either: rung 2 asks the way a turn asks, and only if that also produces nothing is `forced_tool_choice_ignored` reported. * **The probe sent no `max_tokens`, which every turn sends.** `buildOpenAiChatBody` sets it unconditionally and has no `max_completion_tokens` fallback, so a model that rejects the field fails every turn — exactly the class of failure this probe exists to catch, and one it passed. The cap is now in the body, generous enough (1024) that it cannot truncate a one-field tool call, a `retry_token_field` refusal is reported as the new `token_cap_rejected` instead of being retried in a shape Atomic never uses, and `finish_reason: "length"` is read as inconclusive so our own cap can never be reported as a route defect either. Smaller corrections from the same review: * `parallel_tool_calls` is no longer hardcoded: the wizard resolves what a turn would send (`agent.maxParallelToolCalls > 1`). * Keyless-listing presets (Nous, Novita, Ollama Cloud, SambaNova, Sarvam) are skipped rather than probed with an empty credential and told their absent key "was rejected" — that flag is about listing models, not about completions. * `contractProbeFoundDefect` is gone. It had no caller and its doc described a warning policy the shipped code contradicts. * One timeout constant instead of two: the 20s default no caller passed described a budget that never existed. * The unreachable `MAX_PROBE_REQUESTS` guard is gone; the ladder is three fixed rungs and the deadline is the real bound. * `/llm check` is now in the `/llm` menu description, and prints the redacted provider detail when the route did not pass — until now that string was computed, scrubbed and never shown to anyone.
Three claims the feature is sold on had no test that could fail. Both save paths could be deleted wholesale and every test still passed: the wizard's `completeWizard` tests all stop at a refused key, so nothing reached the probe call, the gate, or the note the operator gets. `completeWizard` now runs end to end with only the disk writes stubbed — real key check, real probe, real gate — in two shapes: a route that streams a native tool call is saved *and* reported as a working backend, and a route whose stream ends early is saved, warned about, and not reported. First-run onboarding gets the same pair. Deleting either call site, or weakening the gate to `gate.warning === null`, now fails four tests. The redaction test was vacuous: 400 filler characters sat in front of the credentials, so the 300-character cap satisfied both assertions on its own and the test passed with `redactProviderDetail` reduced to a plain slice. The fixture now puts both keys inside the cap, and `redact-provider-detail.test.ts` pins each rule on strings short enough that truncation cannot do the work — including the pattern pass, the one capability this adds over the key check's exact-key removal, which nothing in the repo detected the loss of. Verified by mutation, each of these now fails at least one test: redaction disabled; the `KEY_SHAPED` loop deleted; the probe removed from either save path; the gate weakened; the old "timed out with zero bytes" rule restored; the abort dropped from the read race; `max_tokens` removed from the probe body; `forced_tool_choice_rejected` dropped from the proven set.
Esc, Ctrl+C and /abort all stop the agent, but they are keyboard lore - nothing on screen says a running turn can be stopped at all. Put a clickable stop chip inside the input field, next to Send, rendered only while status is running; a press takes exactly the path Esc does (onAbort + abort_requested), so there is one abort path however it was asked for. The chip paints on the palette's error ground with a measured ink (readableOn), the same trick the composer buffer uses, so it stays legible across all eleven themes.
The startup update modal is skippable, and skipping it left no trace on screen — an operator who pressed n once stayed on the old version with nothing reminding them a newer one exists. The offer now also raises a banner pinned to the right edge of the status bar: 'new version vX.Y.Z available [ Update ]'. It renders inverse-video, so it is the opposite of whatever ground the palette and terminal use — distinguishable by construction, and static, so it never pulls the eye away from the work. Clicking Update runs the same path as the modal's y (including the refusal while a turn is in flight); the modal itself is untouched and remains the keyboard route. The banner survives update_dismissed (the modal is the question, the banner is the memory), yields while an update runs or has finished, and returns on a failed install so there is still a way to retry. On narrow rows it degrades — full sentence, bare version, button alone, nothing — instead of wrapping the one-row bar. Testing ground: 'atomic-agent tui --fake-update 9.9.9' pretends that version is released — the real check and the real installer are both bypassed, so the modal, the banner and its degradations can be eyeballed on a dev build.
…cluded llama-server's prompt_n / tokens_evaluated count only the tokens evaluated on this request; the prefix reused from the KV cache sits in tokens_cached and was dropped. Every consumer treats promptTokens as "how big was the prompt" (their fallback is prompt.tokens.total, and the TUI overwrites the context readout with it after each completion), so on a warm cache the occupied-context figure collapsed to the newly-evaluated slice — and then leapt ~4x back to the estimator's full figure the moment the context panel's task selector reprojected it. Report evaluated + cached from the llama client so the measured readout and the selector projection agree. Cloud adapters already include cached tokens in their prompt counts and are untouched.
Two gaps between the context panel and the chip under it: - Working the panel's task selector reprojected every figure inside the panel and left the composer chip on the last built prompt, so the one readout that survives closing the panel never showed the number just chosen. The chip now renders the same projection the panel does (selectComposerContextUsage): stepping the dial moves the minibar gauge, token pair and task count on the same render, and the draft keeps driving the chip until a prompt is actually built against it. - Switching the active chat model (or provider) left the chip gauging against the window the *previous* model's prompt was built with, because resolveWindow prefers the prompt-derived window over every live source. A providers_refresh that changes the active text route now drops that stale window, so the chip re-gauges from the health poller / catalogue for the newly chosen model immediately instead of one prompt build later.
The composer's context chip was computed only from the live turn's prompt_built event, so it vanished on relaunch and — worse — switching threads kept the previous thread's numbers on screen until the next turn ran. - Move the ContextUsageState shape + contextUsageFromPrompt projection from the TUI into src/session/context-usage.ts (TUI paths re-export) so the runtime can use them without a TUI dependency. - executeTurn now stamps the turn's window occupancy (prompt_built estimate, refined by the provider's real promptTokens) onto SessionState before the post-turn save — every origin funnels through it. - session_switched carries the stored snapshot; the reducer restores the chip from it and resets to empty when the target session has none, fixing the stale-gauge-on-switch bug.
The active text provider + chat model are one global config setting, so
every session silently followed whatever the operator last picked —
switch from an OpenRouter/glm thread into a local-llama thread and the
OpenRouter model kept serving it.
- executeTurn stamps metadata.llm = { providerId, chatModel } (resolved
from the live config at turn start, not the fallback chain's
substitute) onto the session with its post-turn save.
- The TUI also stamps the open session the moment the operator picks a
model, so a choice made between turns survives switching away.
- switchSession re-applies the target session's stamp when it differs
from the active model, through the LLM panel's own bus actions
(providers_select_chat_model / providers_set_active_text) so config
persistence + provider reload + panel refresh stay in the one place
that owns them. A stamped provider that was removed changes nothing
and says so; sessions without a stamp keep the current model.
- planModelRestore/readSessionLlmStamp are pure and unit-tested;
malformed metadata degrades to "no stamp", never a crash.
Aborting via the stop chip (or Esc / Ctrl+C / /abort) used to land a warn-styled 'Turn failed [cancelled]: This operation was aborted' in the chat — an error wall for something the operator did on purpose. Worse, the abort races the LLM stream, so it sometimes surfaced as 'Turn failed [model]: model returned empty content', dressing the user's own stop as a provider failure. The reducer now treats loop_failed as stopped-by-user when the category is 'cancelled' OR an abort_requested is on the books (state.aborting — every abort entry point sets it, finishRun clears it). It leaves a calm system notice — 'Agent stopped by user.' — that carries the aborted turn's prompt as retryText, and chat-log hangs the existing [try again] beside [copy] on exactly that notice, resending the user's prompt: a mistaken stop is one click to undo.
Two field reports from the first demo round: The obvious first click a fresh launch invites is the banner's Update button — while the startup offer modal is still on screen. The modal raises the mouse floor to the modal rung, so that click was silently swallowed. The button now registers on the modal rung itself: a click there is exactly the modal's y, so answering the offer from the corner is the same decision, not a bypass. And once an update is accepted, the corner went quiet at the very moment it had something worth saying. The strip now narrates the lifecycle instead of hiding: 'updating to vX — do not close' while the installer runs (degrading to 'updating — do not close', then 'updating…'), 'updated to vX — restart to apply' once it lands, and back to the offer — the retry path — after a failure. --fake-update's accept now walks the same event sequence the real installer emits (update_started, feed lines, update_finished ok) on a watchable timeline instead of a one-line notice, so the whole arc — modal, click-through, do-not-close strip, restart prompt — is on show; the restart re-execs the same dev command, so nothing is ever installed.
Two kinds of "this provider is unusable" failure were filed under categories that shouldAdvance refuses to advance on, so a permanently broken link pinned the whole fallback chain and the user got a diagnosis for a problem they did not have. - llama-server 4xx: every non-null status under 500 mapped to grammar. A 404 (the configured localModels.url does not serve completions) or a 405 read as "Turn failed [grammar]" and stopped the chain. The endpoint/auth/availability statuses (401 402 403 404 405 408 409 429) now classify as transport; the request-shape statuses (400 413 422 and any other unlisted 4xx) stay grammar. - SubscriptionCliNotInstalledError / SubscriptionCliAuthError were plain Errors, so they fell through to the catch-all tool arm. A missing or signed-out claude/codex CLI now classifies as transport. Routing the llama 404 to transport also unblocks the llama-unreachable hint in format-agent-error-for-chat, which is gated on the transport category — the one failure where "check your llama URL" is the right advice was the one that never got it. Doc comments in failure-category.ts, classify-failure.ts and should-advance.ts updated to the new rule.
…al time `localModels.requestTimeoutMs` (default 300s) was applied as a wall-clock deadline over an entire streaming generation. `createRequestController` armed one `setTimeout` when the request was sent and nothing refreshed it, so `completeStream` aborted its own healthy stream 300s in — a reasoning model on CPU, or a llama-server across a LAN, is killed mid-token with every byte already produced discarded. Nothing downstream recovers it: `isRetryableLlamaError` refuses to replay a `timedOut` error, and `timedOutOf` in `should-advance` deliberately keeps a self-inflicted timeout off the immediate-fallover path. The turn just dies. The cloud path never behaved this way. `openAiFetch` clears its identical timer in `finally` when the fetch promise settles, i.e. at response headers, so for OpenAI-compatible providers the same knob bounds only connect. Local and cloud read one config key two incompatible ways. `createRequestController` now returns `keepAlive()`, which re-arms the deadline and marks it an idle budget; `completeStream` calls it once at headers and again on every chunk. `complete()` is 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 one it can have. `timedOut()` now reports which deadline fired so the two carry honest, opposite advice: "lower completionMaxTokens" is meaningless for a stall where nothing arrived at all. An idle stall still sets `LlamaServerError.timedOut`, leaving retry and fallover exactly where they were — llama.cpp sends headers before it evaluates the prompt, so minutes of silence during a long CPU prompt-eval is not evidence the provider is dead.
…ider is active A cloud-backed session opened with `/health` + `/props` against `http://127.0.0.1:8080`, warned that nothing answered, kept a 3 s footer poller running against it, and refreshed a local `ModelProfileManager` on every turn — all for a backend the session never talks to. The warnings read as an active-backend failure on a run whose real provider was healthy the whole time (issue #112, Yabloko Labs §9). `activeTextProviderIsLlamaServer` moves from `src/tui/local-turn-gate.ts` to `src/llm/provider/registry/active-text-provider.ts` (re-exported from its old home, so its callers and tests are untouched). It is a pure function of `resolveLlmConfig`, which does no I/O, so the answer is available at the top of `buildRuntime` — hundreds of lines before `ProviderRegistry.fromConfig` resolves the active provider. Detection stays KIND-based and conservative: an id resolving to no entry counts as local. Gated on that predicate: - bootstrap's `/health` line, the `/props` profile probe, and the context-window advice that only names llama-server flags; - the sidecar's `start_session` health probe and its `llm_unavailable` event; - the agent loop's turn-start `refresh()` and between-steps `refreshIfStale()`; - the TUI footer poller's `/health` tick and `/props` label fetch, and `select-context-usage`'s use of the poller's `n_ctx` — a stale local reading must not scale a cloud model's gauge. The `ModelProfileManager` is still constructed on a cloud boot (construction is pure field assignment): deleting it would leave a mid-turn fallover to a `llama-server` link running on a frozen `plain-instruct` profile with no way back. Only its probing is deferred, into `DeferredLocalBackendProbes`, which replays health + `/props` + slot discovery + the context advice exactly once for whichever path reaches local inference first — a provider switch (the loop's turn-start gate) or a cloud→local fallover (the fallback seam's new `prepareLink` hook, called with the chosen link before its completion is sent). Local embeddings are untouched: they hang off their own flags and their own port, and are still probed under a cloud text provider. Fixes #112
toLlmFailure carried a second, hardcoded copy of the llama-server taxonomy (status null or >= 500 => transport, everything else => grammar). That copy, not classifyFailure, decided the category the user actually reads: executeStep wraps every escaping error through toLlmFailure and rethrows the wrapper, and classifyFailure short-circuits on `err instanceof LlmFailure`, so the new endpoint/availability status set was never consulted on the path that produces the chat message. Concretely, a raw LlamaServerError(404) still surfaced as `Turn failed [grammar]: llama-server returned http 404` with no unreachable hint, and the Sentry clusters CLI-B7 / CLI-BE (category=grammar, cause_type=LlamaServerError) are exactly the signature of the GrammarError constructed here — they would have kept firing at the same rate. Delete the duplicate and ask classifyFailure instead: transport keeps TransportError(message, status, url), anything else keeps the historical GrammarError(message, ""). The cause chain is unchanged in both arms, so the scrubber's causeType still resolves. classifyFailure cannot answer cancelled/model/tool for a LlamaServerError, and an aborted step is already claimed by the ctx.signal.aborted check above this arm, so no other category is laundered into a TransportError. The fallover half was already correct: runWithFallback catches the raw LlamaServerError before executeStep's wrapper, so only the user-facing category was stuck on the old taxonomy.
…line
The block added in this PR claimed to mirror production but composed
formatAgentErrorForChat(classifyFailure(err), err.message, local) by
hand, omitting the toLlmFailure link that was exactly what was broken.
It was green while a 404 still reached the user as a grammar failure.
Rewritten to run the whole path: AgentLoop executes a step whose
llmComplete throws a raw LlamaServerError, executeStep normalises it
through toLlmFailure, the loop's catch classifies that wrapper and emits
loop_failed { category, error }, and the assertion formats exactly those
fields the way agent-event-reducer's loop_failed case does.
With the toLlmFailure change reverted, the 404 and 405 cases fail with
`Turn failed [grammar]: llama-server returned http 404`, reproducing the
production defect. The 400 case is the regression guard for the half that
is intentionally unchanged.
The AdvanceDecision doc has long said an immediate signal "should switch
on the FIRST occurrence, bypassing the consecutive-failure threshold",
and the test names this PR added inherited that wording ("advances via
threshold on a local llama 404", "advances immediately on a local llama
429").
ProviderFallbackChain.advanceFrom returns the next link on the FIRST
fallover-worthy failure whenever decision.advance is true, immediate or
not. What immediate changes is registerFailure: it arms the breaker
cooldown right away instead of waiting for failureThreshold consecutive
failures. The threshold governs how long a failed link stays quarantined
across later turns, not the in-turn switch.
Assertions are unchanged — only the doc comment and the six new test
names/comments that misstated the mechanism.
Making `requestTimeoutMs` an idle deadline removed the last cap on one local completion: a server emitting one byte every (budget - 1)ms refreshes the deadline forever, and nothing else on the turn path stops it — `src/agent` arms no timers, there is no `AbortSignal.timeout` anywhere on the path, and `ctx.signal` is user-driven only. A wedged or hostile llama-server could pin a slot, a session and, under headless `run`, the process indefinitely. Adds `localModels.streamTotalTimeoutMs` (env `ATOMIC_AGENT_LLAMA_STREAM_TOTAL_TIMEOUT_MS`, default 6 h) alongside the existing knob. It is a backstop, not a budget: 72x `REQUEST_TIMEOUT_MS`, and well clear of the worst honest local generation — the default `completionMaxTokens` of 8192 decoded at 0.4 tok/s is ~5.7 h.
Three follow-ups to the idle deadline, in one file:
1. Enforce `streamTotalTimeoutMs`. `createRequestController` now also
returns `startStreamDeadline()`, a second timer armed once at
response headers and never refreshed, reported as its own
`stream-total` kind with its own wording ("data kept arriving, so
this is the absolute cap … not a stall"). A live stream therefore
holds two pending timers; `cleanup()` clears both.
2. A stall *before the first token* no longer claims the server
"stopped responding after starting the reply". llama.cpp sends
headers and only then evaluates the prompt, so that silence is the
ordinary look of a long CPU prompt eval — exactly the population this
change exists to protect. `keepAlive()` now takes the kind to record:
`first-token` at headers, `idle` once a byte has actually arrived.
The new wording says the request was accepted but no first token
came, and points at `requestTimeoutMs` or the prompt/context size.
3. Tests for both, plus the two behaviours that were load-bearing but
unpinned: the `keepAlive()` at headers (deleting it now fails two
tests instead of none) and its no-op-once-aborted guard (a byte still
in the decode pipe when the abort lands must not re-arm the timer and
rewrite which deadline gets reported).
…ved local link Two review findings on the same class. F3. `ensureProbed()` assigned `this.inFlight = this.deps.restore()` outside the `try`, so a SYNCHRONOUS throw from `restore` escaped before the assignment: `restored` stayed `false`, `inFlight` stayed `null`, and the probes re-armed on every later call — three `ensureProbed()` calls ran `restore()` three times, contradicting the class's own "latched even on failure" contract. The existing test throws from an `async` function, whose rejection arrives after the assignment, so it passed either way. Moving the call inside the `try` makes the `finally` latch both shapes. Bootstrap's `restore` is `async` with a catch-all so this was latent there, but the class is exported. F1 (groundwork). `noteLinkServed()` / `takeLinkServed()` carry the fact that a `llama-server` link served an attempt while the active text provider was something else — a cloud->local fallover. Take-and-clear, so a recovered cloud primary quiets the local probes again after one turn. The two members are optional on the interface, so legacy and test wiring that implements only `isActive` + `ensureProbed` still type-checks.
…cal fallover
F1, the one confirmed regression against `main`.
`fallback.appendLocal` defaults to `true`, so a rate-limited or down
cloud primary falls over to the llama-server link on every turn under the
DEFAULT config shape — this is not an opt-in path. The loop's turn-start
gate reads `activeTextProvider`, which stays cloud for the whole outage,
so after the first fallover latched `ensureProbed()` nothing refreshed
the profile again: profile and GBNF grammar stayed pinned to whatever the
first fallover probed, and `observeCompletionModelId`'s staleness flag had
no consumer. `main` refreshed unconditionally at every turn start and did
not have this hole. Measured over three turns with the model hot-swapped
behind llama-server between turns 2 and 3:
before: turn1 props=1 | turn2 props=0 | turn3 props=0 (pinned)
main: turn1 props=1 | turn2 props=1 | turn3 props=1
after: turn1 props=1 | turn2 props=1 | turn3 props=1
Two arms, neither of which probes on a turn that never touches a local
link:
- bootstrap's `prepareLink` calls `noteLinkServed()` for a
`llama-server` link, and the loop's turn-start refresh gains a second
arm that fires on `takeLinkServed()` even while the active provider
is cloud. Take-and-clear: one trailing refresh after the outage ends,
then silence.
- `prepareLink` no longer returns empty-handed once the gate has
latched. It falls through to `profileManager.refreshIfStale()`, which
on a cloud-active turn is the only surviving consumer of the
staleness flag, and sits strictly closer to the request than the
loop's between-steps refresh it replaces.
The zero-request criterion is unchanged: both arms are reached only via
a `llama-server` link, so a cloud bootstrap, a cloud turn and a sidecar
cloud start still make zero local requests.
Pinned by a test that drives the REAL `createFallbackCompleter` seam and
the REAL `DeferredLocalBackendProbes` through a real `AgentLoop`, with
`prepareLink` wired verbatim from `bootstrap.ts`, a cloud primary that
429s and a fake llama-server whose model is swapped between turns 2 and
3. It asserts the cumulative `/props` counts (1, 2, 3), that `/health` is
still replayed only once, and — the load-bearing one — the profile id
each local completion was actually built with:
["gemma4-think", "gemma4-think", "qwen-think"]. Reverting either arm
reproduces the measured [1, 1, 1].
F2. The `localActive &&` clause in `resolveWindow` had zero coverage: dropping it survived all 244 files / 2638 tests of `src/tui`, and the PR body credited it with an acceptance criterion on the strength of that run. The existing poller-fallback test uses a state with no provider rows, where `active === undefined` reads as local, so it exercises the other side of the branch. Two tests. The killer: a cloud row is the active text route, `llmHealth.contextWindow` is 4096 (a llama-server `n_ctx` left in state after a local->cloud switch — `agent-event-reducer` deliberately PRESERVES `contextWindow` when an `llm_model_updated` omits it, which is exactly the shape `notifyCatalogModel` emits, so the stale value is reachable by design), and the row's model is in no catalogue. Guard present: `null`. Guard removed: 4096, a local slot size drawn as a cloud model's context gauge. Plus the control: the guard must not cost the local route its window.
F4. `updateUrl()` emitted `llm_model_updated {model: null,
contextWindow: null}` unconditionally. No request is made — the follow-up
`tick()` returns early on a cloud route — so the zero-request criterion
was never at risk, but "on a cloud route it emits nothing at all" was
inaccurate: `/llama <url>` on a cloud session blanked the tray label and
window that the active provider had put there. The new poller test
asserted `actions == []` for `start()` only.
The bookkeeping reset still runs unconditionally (so a later switch back
to local re-discovers the model); only the emit is gated, behind the same
`localTextActive()` predicate as `tick` and `refreshModelLabel`. Test
covers `updateUrl` + `refreshModelLabel` on a cloud route, with a local
control asserting the URL change is still announced.
F6. `prepareLink`'s docstring implied per-link warming. Bootstrap's implementation warms the one local backend the runtime owns — the `ModelProfileManager` over the shared `LlamaServerClient`, which reads `localModels.url` per request — so a second `llama-server` entry pointed at a different host is announced through the hook but probed against the configured URL. That is a pre-existing `ModelProfileManager` limitation (a singleton over one client, not a per-link cache), now stated where the hook is defined. F5. `activeTextProviderIsLlamaServer` is KIND-based while `LocalModelsOrchestrator.autoStartIfReady` is id-based (`!== "local-llama"`). Calling that "the same conservative default" overstated it: for a `llama-server` entry under a custom id the two disagree and the managed daemon is not auto-started. Left as-is on purpose — it errs toward less local activity and leaves no acceptance criterion unmet — but recorded as a divergence rather than an equivalence.
F1's fix opens a second arm on the loop's turn-start refresh — a `llama-server` link that SERVED the previous turn reopens it even while the active provider is cloud — so the criterion the whole PR rests on had to be re-proved past boot. Boots the real runtime on the default fallover shape (`appendLocal` defaults to true and a `llama-server` text entry is configured, so the chain really is `[cloudy, local-llama]`), runs three turns, and asserts `127.0.0.1:8080` sees zero requests after each one. The counting `fetchImpl` now answers the cloud provider with a real chat completion, because a cloud link that fails would never reach the chain's local tail and the assertion would be vacuous; the run is checked for exactly three cloud completions. Measured outside the suite as well: those three completions are the ONLY outbound requests the process makes.
…nd pin the fallover end to end Follow-up to F1's fix. Two mutations of the new `prepareLink` body — deleting `noteLinkServed()`, and deleting the `refreshIfStale()` fall-through — survived the whole suite, because the body was an inline closure inside `buildRuntime` that only a booted runtime driving a real fallover could reach, and no such test existed. A third (bootstrap not wiring `prepareLink` into its seam deps at all) survived too. `createLocalLinkPreparer` lifts the three decisions out of `buildRuntime` into `local-backend-gate.ts`, next to the gate they drive, with unit tests for each: the non-local early return, the served mark, restore without a double refresh, the fall-through on every later attempt, and the local-from-boot run where there is nothing to restore but the staleness flag still needs a consumer. The agent-loop fallover test now drives that same function instead of a verbatim copy of it. And the end-to-end case the PR body had listed as tested only at the seam: a booted runtime, a cloud primary flipped to 429, and the fallback chain routing three turns onto the llama-server link. Asserts `/health` = 1 and `/props` = 1 with `/props` landing BEFORE the local `/completion`, then `/props` = 2 and 3 on the following turns with `/health` still 1. All four mutations are now killed.
Conflict resolutions against the context-usage work already on the rc branch: - runtime/bootstrap.ts: one save at the end of a turn now stamps both the turn's context usage (#320) and the provider/model it ran on (#321). Two PRs rewrote the same choke point; keeping either alone silently dropped the other's stamp. - session/index.ts: both barrels re-exported. - bootstrap.test.ts: the two near-identical turn-stamp tests folded into one that asserts both stamps, returned and reloaded. Also fixes merge-caused breakage between #309 and #315: #309 made `OpenAiToolCallDelta.index` optional, so the contract probe's accumulator no longer type-checks and would have keyed every index-less call into slot 0. It now resolves call identity the way the stream consumer does — index > id > the call currently open.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Integration branch for the 2026-09-03 release candidate:
main@ v0.5.4 (0ed39a7) plus the sixteen open PRs, each merged with--no-ffin numeric order.Merging this instead of the sixteen PRs one by one keeps the two conflict resolutions below, which merging them individually would make someone redo.
Included
Conflict resolutions
#320 vs #321 —
src/runtime/bootstrap.ts. Both rewrote the post-turn save. The merge keeps both stamps in one save: the turn's context usage and the provider/model it ran on. Keeping either side alone silently dropped the other's stamp.src/session/index.tsre-exports both barrels; the two near-identical turn-stamp tests inbootstrap.test.tsfold into one asserting both stamps, returned and reloaded.#309 vs #315 — merge-caused compile break. #309 made
OpenAiToolCallDelta.indexoptional; #315's probe accumulator keyed its map ondelta.index, so it stopped type-checking and would have collapsed every index-less call into slot 0 — the exact bug the probe exists to catch. It now resolves call identity the way the stream consumer does: index > id > the call currently open.Validation
npm run lintclean.src/sidecar/send-message-concurrency.test.ts, fails identically on plainmain— pre-existing.publish=false: run 33749850747, all four platforms built.