fix(fallback): converge main-chat and workflow model fallback - #2201
Merged
Conversation
Contributor
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
flora131
added a commit
that referenced
this pull request
Aug 5, 2026
… walk (#2170) Prompt delivery and session creation authorized same-model retries with the broad fallback-eligibility predicate, so a 401/400/model-unavailable failure burned the whole settings.retry budget on a candidate that had already definitively rejected the request before advancing. Both call sites now use the shared isRetryableSameModelFailure() classifier, keeping the broad predicate for fallback advancement and the unresolved-context-overflow exclusion. Addresses Greptile review on #2201. Assistant-model: Claude Fable 5
Main chat and workflow stages resolved fallback candidates through the same helper but disagreed on when a chain advances, how long a candidate is retried first, and how long a switch lasts. Converge them in three directions. Share the failure classifier. The single implementation now lives in packages/coding-agent/src/core/model-fallback-failures.ts; the workflows and subagents modules re-export it. Main chat splits the one predicate in two: _isRetryableError answers "request this model again", _isFallbackableError answers "spend the next candidate". Auth, model-unavailable, and request-incompatible failures now advance the chain, and openai-codex token invalidation is terminal for the current model while remaining fallbackable. Scope a main-chat switch to the failing turn. A fallback records the user-selected origin model and reasoning level, and restores them at the next turn boundary — before the next idle prompt, before queued follow-ups, and after a compaction continuation settles. Restores are recorded in model history and in the model_fallback_* lifecycle. An explicit /model, model cycle, or thinking-level choice cancels the pending restore so the user's selection wins. Give the workflow candidate walk retry-then-advance timing. Thrown auth, model-resolution, and transport failures now get a bounded same-model retry with exponential backoff from settings.retry before handleCandidateFailure advances, and retry.enabled: false keeps immediate advancement. The retry honours pause/resume and abort, and keeps the admitted stage prompt when it resumes the existing turn with _runAgentContinue(), which the agent rejects on a transcript that does not end in the prompt being resumed. Context overflow keeps compaction first. Only once compaction is disabled, fails, or reports the overflow unresolved does the chain advance, so a compactable first overflow still costs no candidate but an unrecoverable one can reach a larger-context model. The retry decision and its backoff curve now live once, in packages/coding-agent/src/core/retry-policy.ts, used by both the main-chat retry loop and the workflow prompt and session-creation retries. Refs: #2170 Assistant-model: Claude Opus 5
Captures the fallback seams, file references, contracts, and risks that guided the convergence work, alongside the state of the inherited implementation it was handed. Refs: #2170 Assistant-model: Claude Opus 5
…ail (#2170) restoreSessionMessages() retains non-error assistant messages, so an attempt that streamed a completed assistant before throwing left an assistant tail. pi-agent-core's Agent.continue() rejects that with "Cannot continue from message role: assistant", which turned a recoverable retry into a hard stop. Gate the continuation path on the contract continue() actually states: the restored transcript must be non-empty and must not end in an assistant. When the tail is ineligible the retry drops the retained prompt and re-prompts instead, so the input is re-sent exactly once. Cover the four admitted orderings on a real-AgentSession probe — tool-result tail, non-error assistant tail, pause during backoff, and abort during backoff — and add a main-chat case that drives the production compaction path from a failed overflow compaction through to a real fallback switch, rather than injecting the unresolved-overflow flag. Mark the research ticket superseded and correct the claims that no longer describe the repository. Refs: #2170 Assistant-model: Claude Opus 5
…2170) setThinkingLevel() cleared the fallback scope on every call, before the isChanging check. That cancelled the pending primary-model restore both when a user changed reasoning level during a fallback turn and when the registry refresh re-applied the current level unchanged, leaving the session stranded on the fallback model. The contract's only stated exception is an explicit /model choice. Drop the clear. When the level actually changes during an open fallback scope, carry it into the scope so the restore returns the primary model without overwriting the reasoning level the user picked — the same rule the model already follows. setModel() and both model cycles still clear the scope, so an explicit /model still cancels the restore. Also add ResourceExhausted to the shared retryable pattern list. Upstream pi-ai retries it, but the shared classifier did not, so a transient gRPC provider error stopped a session with no fallback chain instead of spending its same-model retry budget. It goes in the one shared list rather than a main-chat-only pattern, and the shared conformance corpus covers it so both companion packages assert the same answer. Refs: #2170 Assistant-model: Claude Opus 5
…2170) Four defects in the stage fallback path, all reproduced before fixing. createInitialSession() created only candidates[0] and handed failures to a retry helper that rethrew after exhausting that one candidate. A session created before any prompt — an eager stage call, a control attach, or ctx.__ensureSession() — therefore failed the stage with its configured fallbacks untried, and ensureSession() cached the rejected promise so every later caller replayed the same failure. Add a creation-only version of the prompt candidate walk, and clear the cached promise by identity after a terminal rejection. The explicit-candidate branch of promptWithFallback() never consumed pendingCreationResumeMessage, so a paused eager creation resumed with a replacement objective sent the stale original instead. Consume and clear it before tryResumeCurrentSession() and before the loop. canContinueFromTranscript() checked the raw tail role, but pi-agent-core requires the converted tail to be user or toolResult. Atomic's converter drops a custom, bash-execution, or branch-summary message that is excluded from context or empty, exposing the assistant beneath it. Evaluate the same convertToLlm() result Atomic sends. An already-unresolved context overflow was fed to the shared retry policy as an ordinary retryable failure, so it burned the whole settings.retry budget on a model whose compaction had already failed. Gate same-candidate retry on !isUnresolvedContextOverflowFailure while leaving the shared classifier and the wrapper untouched, so the candidate walk still advances. Refs: #2170 Assistant-model: Claude Opus 5
_trySwitchToFallbackModel() skipped a candidate only when it matched the failed model AND its reasoning level, so openai-codex/gpt-5.5:high failing with an invalidated OAuth token fell back to openai-codex/gpt-5.5:low — the same dead credential, with model_fallback_start reporting an identical from and to. The boundary is not "auth"; it is the distinction the shared classifier already draws. A failure that may spend a candidate but cannot be repaired by another request to the same model — rejected credential, unavailable model, request the model cannot serve — is unaffected by reasoning level, so every reasoning variant of that model is out for the rest of the turn. Track those models in a turn-scoped blocked list and skip matching candidates before the fallback scope opens, so a skipped variant emits no event and starts no lifecycle. The list clears with the existing fallback-attempt state on a new user turn, a successful turn, and scope close. Transient rate-limit and transport failures stay same-model retryable and keep their reasoning variants. The pre-existing same-provider reasoning test used the fixture's default "Not Found" message, which is fallbackable but not same-model retryable, so it was asserting the defect. It now uses a real transient failure and covers the behavior that must survive. Refs: #2170 Assistant-model: Claude Opus 5
Three races in the stage runner, each reproduced before fixing. disposeCurrentSession() cleared the shared creation promise unconditionally, including while a candidate walk was still creating the next session. A concurrent ctx.__ensureSession(), or a first ctx.prompt() whose explicit-candidate branch bypassed ensureSession() entirely, could then start a second walk: duplicate provider work, with the losing session left live and undisposed. Mark the promise a walk owns, clear it on dispose only when it is not that promise, and let the explicit-candidate prompt join a creation already in flight. createSession() was awaited with no pause observer, so a controlled pause that both started and finished while the adapter was in flight was invisible; completing a resume clears the pause request, so a post-await currentResume() check sees nothing and the stale objective is prompted. Register an observer before creating and latch any replacement objective onto the resume itself. The observer set is separate from the thrown-retry states and the creation promise is returned untouched, so abort semantics, backoff timing, and attached-stream observability are all unchanged — an earlier attempt that added one microtask hop broke the executor pause and abort suites, which is what caught it. _runAgentContinue() was called directly, so the abort a controlled pause causes was classified as a terminal model failure and the stage became unrecoverable by resume. Apply the prompt's pause rules to the continuation: settle the pause, await its delivery settlement, then re-prompt with the replacement objective instead of spending a fallback candidate. Refs: #2170 Assistant-model: Claude Opus 5
Assistant-model: GPT-5.4
Assistant-model: GPT-5.4
Assistant-model: GPT-5.4
… walk (#2170) Prompt delivery and session creation authorized same-model retries with the broad fallback-eligibility predicate, so a 401/400/model-unavailable failure burned the whole settings.retry budget on a candidate that had already definitively rejected the request before advancing. Both call sites now use the shared isRetryableSameModelFailure() classifier, keeping the broad predicate for fallback advancement and the unresolved-context-overflow exclusion. Addresses Greptile review on #2201. Assistant-model: Claude Fable 5
flora131
force-pushed
the
fix/2170-model-fallback-convergence
branch
from
August 5, 2026 04:25
c3175a4 to
71728a5
Compare
flora131
added a commit
that referenced
this pull request
Aug 5, 2026
…ane (#2205) * docs(specs): spec for Codex-aligned in-process subagent runner with Rust control plane (#2188) Full Codex alignment resolved with the requester: zero OS processes (async runner deleted, survival traded for cold-identity reload), turn-scoped execution guards, LRU residency with transparent cold reload, persistent canonical child identities, 100ms interrupt grace, no kill-capable timers. Control plane implemented in Rust (crates/atomic-natives, NAPI-RS). Atomic keeps per-agent tool allowlists, depth<=5, model fallback, and its request/response tool surface. Clean break: env bridge, watchdog, exit codes, and the file-claim delivery pipeline are deleted. Fixes #2191 via unified background continuation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(natives): add Rust in-process subagent control plane (#2188) * fix(natives): emit consumable string union types * feat(subagents): add in-process runner doors * refactor(subagents): route foreground attempts in process * docs(subagents): note in-process foreground semantics * docs(natives): note consumable status unions * docs(subagents): record in-process slices report * docs: track issue 2188 validation flake * fix(subagents): route termination to live in-process attempts * feat(subagents): drive foreground candidates, structured output, and detach in process Adds an explicit test-session seam to the in-process runner so the foreground path can be exercised without an OS child, moves pre-admission model-candidate filtering and cwd validation onto the in-process door, runs the structured-output corrective-retry loop inside the runner, and routes intercom detach through continue_in_background instead of the process-era placeholder. * style(subagents): sort in-process run-sync imports * fix(subagents): persist the child input artifact and surface failed progress errors The in-process foreground path stopped writing _input.md, which spec 5.5 keeps as user-facing artifact naming, and never populated AgentProgress.error on a failed attempt. Rewrites the acceptance and structured-output suites against the typed status contract instead of a spawned fake CLI. * test(subagents): migrate intercom detach tests in-process * docs(research): add issue 2188 TS migration blueprint * feat(subagents): add in-process background continuation seam * feat(subagents): run async single children in process Replaces the detached-runner single path with the in-process continuation seam. async-execution-single.ts is deleted per spec section 10 and its behavior moves to runs/inprocess/background-single.ts, which admits the child, continues it in the background, and returns the canonical child path immediately. Refs #2188 * chore(gitignore): ignore subagent test scratch dir * feat(subagents): collapse async chain and parallel onto the foreground executor (#2188) async: true is now a don't-wait request over the same in-process foreground executor. runAsyncPath admits the child, settles the call with a typed continued status and canonical child path, then runs the work on the existing runChainPath / runParallelPath un-awaited. Chain substitution, dynamic fanout, worktrees, structured output, skills, progress files, fail-fast, and per-step model candidates stay on the one code path that already implements and tests them, rather than a second serialized runner. This removes the last two OS spawn sites, satisfying spec G1 (zero OS child processes): async-execution-common.ts spawnRunner and the detached runner in subagent-runner-streaming.ts. Deletes per spec section 10 "Modules removed": async-execution-{chain,common,types}.ts and the async-execution.ts barrel, the 12-file subagent-runner*.ts family, async-event-journal.ts, and top-level-async.ts. Their exclusively-process-era tests (subagents-async-config, subagents-async-event-journal) are removed with their subjects; tests covering kept behavior are re-pointed at the in-process path with typed-status assertions. * refactor(subagents): delete the process-era spawn and watchdog modules (#2188) With the last OS spawn site gone, these spec section 10 "Modules removed" entries have no importers left: - runs/shared/attempt-watchdog.ts - the idle/wall watchdog; zero importers - runs/shared/final-drain.ts - the stdout drain grace - runs/shared/pi-spawn.ts - CLI-child spawn resolution - runs/shared/spawn-env.ts - the env bridge builder - shared/post-exit-stdio-guard.ts - process-only; sole importer was the watchdog subagents-final-drain.test.ts and subagents-pi-spawn.test.ts are removed with their subjects. interactive-engine-env-scrub.test.ts keeps its three scrubInteractiveEngineEnv tests, which cover kept coding-agent behavior; only the two buildSubagentSpawnEnv cases are removed with spawn-env.ts. * docs(subagents): record the in-process async break in the changelog (#2188) * feat(subagents): resolve child mode policy at admission * docs(subagents): record typed child policy move * chore: drop stray agent report from the repository * feat(subagents): move child prompt behavior into session construction * feat(subagents): route nested controls in process * feat(mcp): consume typed child direct-tool policy * feat(intercom): bind child identity at client connect * feat(workflows): pass typed stage policy without env isolation * refactor(natives): modularize subagent control domains * refactor(subagents): move nested runtime support behind in-process boundary * refactor(subagents): remove legacy prompt runtime adapter * feat(subagents): route management actions through control registry * feat(subagents): route cold resume through control plane * fix(subagents): preserve in-process child results * fix(subagents): persist async dispatch results * fix(natives): await child termination grace and preserve causes * refactor(subagents): delete process-era result pipeline * refactor(subagents): back async jobs by in-process state * test(mcp): remove process-era env setup * test(subagents): retain extension lifecycle coverage * fix(subagents): route list action through control registry * test(subagents): guard in-process clean break and document async ownership * docs(subagents): remove legacy bridge references * docs(mcp): record direct-tool bridge removal * test(ci): include untracked clean-break paths in guard * refactor(subagents): make typed status the only outcome discriminator (#2188) Removes exitCode from SingleResult/ModelAttempt/AsyncStatus, deletes the numeric synthesis on the in-process path, and migrates every TUI, intercom, notification, chain, and workflow-graph consumer onto the typed status union. deliverChildResult now distinguishes artifacts-disabled from an unresolved artifacts directory and throws for the latter. Native terminateChildAttempt calls are awaited. * test(ci): guard removed nested process modules * test(ci): scan unreleased clean-break contracts * docs(subagents): record typed result status contract * fix(subagents): restore agent listing and trim async status text * fix(subagents): retry capacity and normalize child ids * fix(subagents): show live progress and ctrl+o hint for parallel and chain runs The in-process runner only surfaced onUpdate at terminal moments, so a foreground parallel/chain run rendered a bare header with no rows, no activity, and no 'Press ctrl+o for live detail' hint while children ran. - runner: publish throttled AgentProgress from session events (tool start/end, assistant message_end, agent_start) via ChildSpec.onProgress - run-sync: forward progress as live tool-result updates with a synthetic running result, feeding the existing parallel/chain merge paths - renderers: stop bailing to text when a multi run has progress but no results yet; span rows across progress/totalSteps; render running rows with activity and the live-detail hint; add a group-level hint fallback - test: regression coverage for progress-only parallel/chain rendering Verified live against the built CLI under tmux (captures in atomic-e2e-evidence-2188: tour-06/07/08). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(subagents): align child model fallback with the shared SDK session fallback In-process children now pass their pre-filtered candidate ladder to createAgentSession's fallbackModels, so child fallback uses the exact classification, same-model retry, and candidate-advancement behavior main chat and workflow stages converged on in #2170/#2201, instead of a subagents-local single-candidate path. Results report the effective model, attemptedModels (from model_fallback events), and pre-spawn skipped attempts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(subagents): survive stale extension ctx in deferred job hydration hydrateActiveJobsDeferred captured the extension ctx into a timer; after session replacement or reload the ctx.cwd getter throws, crashing the host (caught by the installed-package Node smoke test). Capture cwd while the ctx is live and never touch ctx from the timer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(natives): track subagent control exports in the binding contract The Rust in-process subagent control plane added five NAPI exports (AdmissionRefusalKind, AgentStatus, NapiSubagentControl, SubagentControl, TerminationCause) that the export contract test never learned about, so the coding-agent suite failed on both linux-x64 and windows-x64. Add the five exports to EXPECTED_NATIVE_EXPORTS. The assertion stays an exact whole-surface equality check. * ci(test): build native bindings for the root suites The bundled subagent extension now reaches the Rust control plane in crates/atomic-natives through a static import, so the extension throws at import time when no binding is present. The suites job carried no Rust toolchain, so on a clean Windows checkout 21 unit suites died during collection and 6 more tests failed on the empty tool list that followed. Build the binding before the unit and integration steps, as agent-suite already does. Measured at 42s Linux and 86s Windows, both inside the existing 8/12 minute caps. The topology contract asserted that suites needed no Rust. Replace that stale claim with the ordering assertions the other native-consuming jobs carry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(test): add missing statement separator in Windows child-process probe The PowerShell one-liner in subagents-zero-process joined the $parent assignment and Get-CimInstance without a ';', producing a parser error (Unexpected token 'Get-CimInstance') and a nonzero probe exit code on the Windows suites job. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
sina85
added a commit
to sina85/atomic
that referenced
this pull request
Aug 6, 2026
Bring node-card model/thinking display + durable thinkingLevel persistence up to date with upstream 0.9.13-alpha.1. Only CHANGELOG conflicted; the stage-runner thinkingLevel additions auto-merged cleanly against upstream's model-fallback convergence (bastani-inc#2201).
MarkAronov
added a commit
to MarkAronov/atomic
that referenced
this pull request
Aug 7, 2026
…-summaries Two conflicts, both in packages/coding-agent. `agent-session-events.ts`: the session-summary launch landed on the same line as the context-overflow fallback block and the turn-scoped model restore added by bastani-inc#2201. Both sides are kept, with the launch last, for two reasons. A successful `_trySwitchToFallbackModel()` returns above it, so a turn that continues on another model is not summarized mid-flight. And `_restoreFallbackModel()` has already run by then, which matters because the launch reads `this.model` synchronously before it parks on `waitForIdle()`; launching any earlier would send the summary request on the fallback model rather than the one the user selected. `CHANGELOG.md`: both sides appended to `[Unreleased]`. Kept both, `### Added` first per the section order in AGENTS.md, with upstream's text and its new `0.9.13-alpha.1` section unchanged.
MarkAronov
added a commit
to MarkAronov/atomic
that referenced
this pull request
Aug 7, 2026
… one request Every turn schedules a summary launch, and the previous turn's can still be in flight when the next one wakes. The newcomer aborted its predecessor and issued its own request, so both spent a provider call and only the second could persist. The ordering that hid this was incidental: bastani-inc#2201 added awaits to the turn path, and the wasted request became reliable rather than rare. Publish the in-flight request as `_sessionSummaryRun`, carrying the `throughId` it describes. A launch that wakes to find a run covering the same conversation state now awaits it instead of replacing it; a launch describing a newer state still supersedes, exactly as before. Once a run is published, ownership of that slot rather than the token is what licenses a write. A joiner claims the token on its way in, so a token check after the request would have the joiner invalidate the very run it is waiting for. The token still guards the parked phase, where a launch holds no AbortController and nothing else can reach it. `abortSessionSummary()` clears the run as well, so a provider that ignores its signal still fails the ownership check, and a later launch cannot join a cancelled run. The deferred behind that promise is hand-rolled rather than `Promise.withResolvers`. coding-agent is the one compiled package here and its lib target predates ES2024, so the shipped sources cannot use it even though the raw-TypeScript packages do. Only `test:integration`, which compiles the package the way the build does, catches that; the root `tsc --noEmit` runs against a newer lib and passes. Guard the launch itself with `typeof this._maybeGenerateSessionSummary === "function"`, matching every other optional method in that block. The main-chat fallback suites drive `_processAgentEvent` on a synthetic session object, so an unguarded call threw and took `a compactable context overflow does not spend a fallback candidate` with it. Two tests budgeted no response for the turn-2 launch, which reaches the provider before disposal or the next prompt lands and is cancelled mid-request. That request is spent either way and cannot be recalled, so both now budget it. No assertion changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
flora131
added a commit
that referenced
this pull request
Aug 12, 2026
…cker (#2155) * feat(coding-agent): persist per-session resume summaries Add a `session_summary` session entry recording a generated one-line summary alongside the id of the last user/assistant message it describes. The resume picker reads it during its existing single pass over the session file and treats it as fresh only while that id is still the newest conversation message, so a summary retires the moment the conversation moves on. A later `branch_summary` retires it too, since the branch it described was abandoned. The anchor is the last message id rather than the leaf id. Appends move the leaf -- including the summary's own append, and every model or thinking-level change -- so a leaf-based check would never match and would regenerate on every idle. Summary usage counts toward session usage totals and the footer, matching branch summaries. It is deliberately excluded from cache-continuity stats, where compaction and branch summaries reset the chain: a summary request is a separate call and does not break the main prompt prefix. Generation and display follow in later commits. Refs: #1033 * feat(coding-agent): generate resume summaries when the agent goes idle Add a one-line session summarizer that runs after `agent_end` and persists a `session_summary` entry for the resume picker. It reuses the existing summarization path -- entry selection under a token budget, transcript serialization, credential resolution, stream function and retry policy -- with a new one-line prompt, no reasoning request, and a hard clamp on the stored line so a chatty model cannot break picker rendering. Generation is fire-and-forget and silent by design: nothing awaits it, failures and aborts return without surfacing anything, and retries carry no lifecycle callbacks, since the picker already falls back to the session name or first message. It declines to run when disabled, in print or json mode, while streaming or compacting, without a model, on workflow-stage sessions, on very short sessions, or when the last conversation message has not moved since the stored summary -- which is the main cost control. Concurrency is handled by a monotonic token plus a session-scoped AbortController. After the request returns, the run re-reads both the token and the last conversation message id and discards its own result if either moved, so a slow call cannot persist a summary the conversation has already outrun. The controller is cleared only by the run that still owns it. Adds `sessionSummary.enabled` (default true) as a kill switch. Cancellation wiring from the prompt and shutdown paths, and seeding the in-memory anchor on resume, follow in a later commit. Refs: #1033 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(coding-agent): show session summaries in the resume picker Surface the generated summary in `/resume` and `atomic -r`. The selector now displays `summary ?? name ?? firstMessage`, so a fresh summary takes the row and today's display becomes the fallback. Staleness needs no handling here: a missing or outdated summary is simply absent from `SessionInfo`, so the chain falls through on its own. Session id, message count, age, and cwd stay on the row, and search matches against the summary as well. Carry the field across the interactive-engine boundary too -- row type, protocol parser, and the row-to-SessionInfo mapper -- otherwise summaries would be silently dropped for engine-hosted pickers. The workflows package mirrors the row type by hand rather than importing it, so it gets the field on both sides; nothing enforces that mirror, and omitting either half would drop summaries from `/workflow resume` with no type error. Refs: #1033 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(coding-agent): cancel, reseed and document session summaries Wire up the two lifecycle gaps and cover the feature with tests. Cancellation: `abortSessionSummary()` now runs when the next prompt starts and during session disposal. A background summary can no longer outlive the conversation it describes or hold the process open at shutdown. Resume: the "nothing new to summarize" check falls back to the persisted `summarizedThroughId` when the in-memory anchor is empty, so the first idle after resuming a session no longer regenerates a summary that is already current. Tests cover the listing rules (fresh, stale after a newer message, retired by a later branch summary, unaffected by tool results, absent when never generated) and the generation rules (anchored to the newest conversation message, no regeneration while the conversation has not moved, disabled by setting, skipped in non-interactive modes, and no persistence once the conversation has outrun the request). Documents the entry type and its staleness rule in the session format, the picker behaviour and its fallbacks in the sessions guide, and the new setting. Refs: #1033 * fix(coding-agent): stop a failed session summary from rejecting Credential resolution throws outright when no API key is configured, which is an ordinary state for a session that never prompts. Because the summary runs as `void this._maybeGenerateSessionSummary()`, that throw escaped as an unhandled rejection and could take the process down mid-run; the workflow tool-node quit integration test hit it through a real CLI child and timed out. Background work now swallows every failure -- credential resolution, generation, and persistence alike -- which is what fire-and-forget has to mean here. The returned `error`/`aborted` cases were already silent; thrown ones were not. Refs: #1033 * fix(coding-agent): cancel the session summary at the right point in prompt() Cancellation ran at the top of `prompt()`, ahead of the workflow-delivery authorization boundary and the slash-command path, both of which must observe an untouched session. It now runs once real user input is admitted, which is also when a summary of the previous turn actually becomes stale. Two prompt tests drive `prompt.call()` against hand-built stub sessions, so a new method call on `this` surfaced there as `abortSessionSummary is not a function` rather than as an assertion failure. Their fixtures gain the method. Refs: #1033 * fix(coding-agent): apply one retirement rule to summaries, and honour abort Two P1 findings from the automated review, both verified by running code. A retired summary suppressed its own replacement. The resume fallback read the latest persisted `session_summary` as its freshness anchor without the later-`branch_summary` retirement rule the picker applies, so a session whose summary was retired by a branch would match the anchor, skip generation on every idle, and show fallback text in `/resume` indefinitely. `getLatestSessionSummary` is now retirement-aware and the picker calls it too, instead of tracking the same rule inline -- one lookup, both sides, which is what the split had broken. An aborted request could still persist. `abortSessionSummary()` cancels the signal without bumping the ordering token, so a provider that ignores the signal and returns an ordinary result would pass the token and anchor checks and write a summary that was explicitly cancelled. The signal is now checked directly before persisting. Adds listing coverage for a summary generated *after* a branch summary, which must survive: retirement is positional, not permanent. Refs: #1033 * fix(coding-agent): cancel an in-flight session summary on tree navigation Third P1 from the automated review. Moving the leaf invalidates a summary still being generated, but the anchor check could not see it: a `branch_summary` is not a conversation message, and navigating to an existing assistant message leaves the last conversation message id unchanged. A request that returned after the move therefore passed the token and anchor checks and was persisted against a branch that was no longer active. `navigateTree` now cancels the in-flight summary the same way `prompt()` does, which the signal check added for the previous finding turns into a silent discard -- including for a provider that ignores cancellation and returns an ordinary result. The regression test holds the summary request open with a faux response factory, moves the leaf past the last assistant with a `session_info` entry, then navigates back to that message, so the anchor genuinely survives the move. Verified to fail without the fix. Refs: #1033 * fix(coding-agent): generate session summaries at a real idle boundary `agent_end` fires while the agent still reports `isStreaming`, and that flag survives the entire microtask queue, clearing only on a later macrotask. The summary launch read the flag immediately and returned, with nothing to retry it, so generation only ever happened when `_checkCompaction` on the preceding line happened to cross a macrotask boundary. It does under the unit harness and does not in the real TUI, so the suite stayed green while the feature produced nothing. Wait for `agent.waitForIdle()` instead, and claim the supersession token before the wait so a later turn abandons a launch that is still parked. Deferring to idle widens the window in which a launch exists with no AbortController, which `abortSessionSummary()` cannot reach, so disposal is now tracked as terminal state. `_disposed` is checked on entry, after the wait, before auth, before the provider call, and before persisting. `abortSessionSummary()` bumps the token as well as aborting, and `dispose()` is idempotent. Reported by @flora131 from a real tmux TUI run against the PR head. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(coding-agent): collapse overlapping session-summary launches into one request Every turn schedules a summary launch, and the previous turn's can still be in flight when the next one wakes. The newcomer aborted its predecessor and issued its own request, so both spent a provider call and only the second could persist. The ordering that hid this was incidental: #2201 added awaits to the turn path, and the wasted request became reliable rather than rare. Publish the in-flight request as `_sessionSummaryRun`, carrying the `throughId` it describes. A launch that wakes to find a run covering the same conversation state now awaits it instead of replacing it; a launch describing a newer state still supersedes, exactly as before. Once a run is published, ownership of that slot rather than the token is what licenses a write. A joiner claims the token on its way in, so a token check after the request would have the joiner invalidate the very run it is waiting for. The token still guards the parked phase, where a launch holds no AbortController and nothing else can reach it. `abortSessionSummary()` clears the run as well, so a provider that ignores its signal still fails the ownership check, and a later launch cannot join a cancelled run. The deferred behind that promise is hand-rolled rather than `Promise.withResolvers`. coding-agent is the one compiled package here and its lib target predates ES2024, so the shipped sources cannot use it even though the raw-TypeScript packages do. Only `test:integration`, which compiles the package the way the build does, catches that; the root `tsc --noEmit` runs against a newer lib and passes. Guard the launch itself with `typeof this._maybeGenerateSessionSummary === "function"`, matching every other optional method in that block. The main-chat fallback suites drive `_processAgentEvent` on a synthetic session object, so an unguarded call threw and took `a compactable context overflow does not spend a fallback candidate` with it. Two tests budgeted no response for the turn-2 launch, which reaches the provider before disposal or the next prompt lands and is cancelled mid-request. That request is spent either way and cannot be recalled, so both now budget it. No assertion changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(test): keep the cycle-fallback request count to the prompt turn sessionSummary generation issues a background provider request once the agent goes idle, which the fake server in this test counts alongside the cycled prompt's turn, so requests reached 2. Turn summaries off in the fixture settings. The counter exists to prove the fallback path does not fire a second turn for one prompt, and that claim is only readable when the prompt is the sole caller. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(coding-agent): render session summaries in a dedicated picker column - keep the session name/first message as the row identity; the generated summary renders beside it and never displaces it - show a "No summary available." placeholder when a summary is missing, stale, or failed; omit the column entirely on narrow terminals - gate the disposal test on request start so turn 2s launch provably spends a response before dispose(), fixing the linux CI failure where the pending-response count depended on scheduler timing - update docs/sessions.md and the changelog wording to match Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * fix(coding-agent): regenerate session summary after branch retirement abortSessionSummary() now drops the in-memory anchor cache. The cache is not retirement-aware: after branchWithSummary() retired the stored summary without moving the last conversation message id, a cached anchor still matching that id skipped regeneration and left the picker on fallback text until the next real turn. The persisted lookup already handles retirement, so the cache defers to it after any cancellation. Addresses the one substantiated Greptile P1 on #2155. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * fix(coding-agent): restore the released 0.9.13-alpha.2 changelog section The branch's merge of main dropped the immutable [0.9.13-alpha.2] section and left its entries duplicated under [Unreleased], which the changelog immutability test rejects against the release tag. CHANGELOG.md is now origin/main's content plus only this PR's own entry under [Unreleased]. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * chore: retrigger CI --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Norin Lavaee <nlavaee@umich.edu>
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
2 participants
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.
Summary
This PR converges main-chat and workflow fallback behavior so both paths classify failures the same way, retry and advance candidates on the same rules, and keep fallback model switches scoped to the failing turn.
Changes
/modelchoices from being overwritten by that restore.settings.retrybefore advancing throughfallbackModels, while preserving the admitted stage prompt on continue paths.origin/mainand keep the package changelog entries under## [Unreleased].Notes
npm run check,npm run test:unit, andnpm run test --workspace=@bastani/atomic.Closes #2170
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Greptile Summary
This change unifies model fallback behavior across main chat, workflow execution, and subagents. It distinguishes terminal model failures from transient failures, retries transient provider errors on the current model within the configured limit, and advances immediately to an available fallback for authentication and incompatible-request errors. Main chat restores the user-selected model before the next independent request.
Focused execution contradicted the tested failure hypotheses: authentication and incompatible-request failures advanced directly without retrying the failed model; transient failures retried before fallback; the selected main-chat model was restored at the next request boundary; and workflow fallback continued after retry-eligible errors. The focused suite passed 61 tests, with follow-up main-chat and workflow coverage also passing.
Confidence Score: 5/5
The change is safe to merge; no blocking failure remains in the validated fallback and retry behavior.
Focused execution confirmed direct fallback for terminal authentication and request-compatibility failures, bounded same-model recovery for transient provider failures, restoration of the selected main-chat model, and continued workflow recovery.
What T-Rex did
Reviews (6): Last reviewed commit: "chore: re-trigger CI for 7500aa72b (Test..." | Re-trigger Greptile