fix: thread fallback model through params instead of setChatModel - #265
Conversation
claudiusthebot
left a comment
There was a problem hiding this comment.
Approved. All three bugs are real and the fixes are correct.
CI fix pushed (commit bd248ab)
The 4 Tests matrix jobs were failing due to a strictFunctionTypes violation in the updated test. The fallback_model mock was typed as (p: typeof stubParams & { model?: string }) which has isGroup: boolean (required). QueryParams has isGroup?: boolean (optional). TypeScript's contravariance rule means a function accepting the narrow type isn't assignable to one accepting QueryParams (a QueryParams value where isGroup is undefined can't satisfy the required boolean). Fixed by typing the mock parameter as QueryParams and importing the type β the assertion paramsSeenInRecursion.model === "fallback" still works since QueryParams.model?: string.
Overlap with existing open PRs
This PR partially overlaps with three others that are waiting to merge:
| Fix in this PR | Also in |
|---|---|
setChatModel model-wipe (3 files) |
PR #260 Bug 1, PR #257 Bugs 1+2 |
Dead saw code in legacy-bridge.ts |
PR #263 Bug 3 |
mapListModels wrong total after filter |
New β not in any other PR |
The mapListModels fix is unique value here. The other two are duplicated.
Suggested merge order: merge this PR first (it's the most focused fix for the setChatModel issue, and has the mapListModels new fix), then:
- Rebase PR #260: drop Bug 1 (now covered), keep Bugs 2β5
- Rebase PR #263: drop Bug 3 (dead
sawvariable now covered), keep Bugs 1β2
PR #257 is a subset of #260 β still worth keeping as a narrower, independent set once #265 lands.
CI should go green with the QueryParams type fix commit. β hb #539
PR #265 Bug 2 fixed mapListModels to return mapped.length (post-filter count) instead of the pre-filter total. The existing test expected the pre-filter count of 3, causing all Tests matrix jobs to fail. Update the assertion to reflect the new contract: total = number of models that passed the active filter, not the unfiltered catalog size. Also adds a total assertion for the query-filter case to pin the same semantics there.
|
Second CI fix β commit My Root cause: Fix: Updated the assertion to Summary of all CI fixes on this PR:
Both fixes are test-only. Source changes are unchanged. |
All four backends (`codex`, `openai-agents`, `openai-agents` via `applyRetryDecision`, and `shared/handle-retry`) used `setChatModel` to steer fallback retries, but commit #248 introduced `params.model` which takes priority over `chatSettings.model` in every backend's resolution path β making the `setChatModel` flip a silent no-op. Fix each site by spreading `{ ...params, model: fallbackModelId }` into the recursive call so the fallback model is threaded directly through the call. Additional cleanup: - Remove the unused `setChatModel` imports from the three backends - `adapter.ts mapListModels`: return `mapped.length` after applying `selectableOnly`/`query` filters so `ModelList.total` matches the filtered set (was returning the pre-filter total from the legacy backend, violating the interface contract) - `legacy-bridge.ts reduceEventsToResult`: remove dead `...(saw ? {} : {})` spread (both branches spread an empty object) - Update `shared-handle-retry.test.ts` to assert on `paramsSeenInRecursion.model` rather than `getChatSettings().model` to reflect the new mechanism https://claude.ai/code/session_01Pe2jJNJoMiDUSffdVqiX6P
β¦ctionTypes
The recurse mock was typed as (p: typeof stubParams & { model?: string })
which has isGroup: boolean (required). QueryParams has isGroup?: boolean
(optional). With strictFunctionTypes, (p: Narrow) is not assignable to
(p: QueryParams) because a QueryParams value (where isGroup may be undefined)
can't always satisfy Narrow's required isGroup: boolean.
Fix: type the mock parameter as QueryParams directly. stubParams satisfies
QueryParams (all required fields present, isGroup: boolean is assignable to
isGroup?: boolean). The paramsSeenInRecursion capture still works because
QueryParams has model?: string.
PR #265 Bug 2 fixed mapListModels to return mapped.length (post-filter count) instead of the pre-filter total. The existing test expected the pre-filter count of 3, causing all Tests matrix jobs to fail. Update the assertion to reflect the new contract: total = number of models that passed the active filter, not the unfiltered catalog size. Also adds a total assertion for the query-filter case to pin the same semantics there.
54c5600 to
9943c1e
Compare
Post-rebase reconciliation with PR #265 from main: the stream-shaped retry helper still steered fallback retries via a transient setChatModel flip, which params.model silently outranks. Thread the fallback model id through buildRetryStream into the recursive call instead, matching the callback-shaped helper. Also point the retry test at QueryParams new home in backend/shared/handler-types, and refresh two stale references (legacy-bridge log prefix, adapter mention in contract-tests). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
#258) * refactor(storage): migrate all stores to JsonStore β Phase 6.x complete Five remaining stores migrate from hand-rolled writeFileAtomic.sync + dirty-flag autosave to the unified `JsonStore<T>` envelope. Codex OAuth-incompat (Phase 6.x #1) already shipped in #255. - `chat-settings.ts` β JsonStore<Record<chatId, ChatSettings>> - `cron-store.ts` β JsonStore<Record<id, CronJob>> - `history.ts` β JsonStore<Record<chatId, HistoryMessage[]>> - `media-index.ts` β JsonStore<MediaEntry[]> - `sessions.ts` β JsonStore<Record<chatId, SessionState>> - `trigger-store.ts` β JsonStore<Record<id, Trigger>> On-disk shape changes from a bare object/array to the standard envelope `{ schemaVersion, savedAt, data }`. A `migrate` hook on each store accepts the legacy pre-envelope shape so existing on-disk state loads unchanged. JsonStore gains a synchronous twin pair (`loadSync` / `saveSync`) so storage modules wired into bootstrap and cleanup-registry can keep their sync init / shutdown ergonomics without async ripple through the rest of the codebase. The default fs over `node:fs` now looks up its methods lazily on a namespace import so tests can mock a subset of the surface without breaking import. Tests: each store's per-mock surface expanded to include `renameSync` + `unlinkSync` (JsonStore touches both on the bak fallback path), and the `write-file-atomic` mock now covers both the callable form (used by async `save`) and the `.sync()` form (used by `saveSync`). The previously-implicit per-save `.bak` write is gone β JsonStore relies on `write-file-atomic`'s atomic rename plus a read-path fallback rather than an explicit pre-write. `media-index.test.ts` migrates from heavy node:fs mocking to a real temp-dir pattern (matches `codex-oauth-incompat.test.ts`). `vitest.config.ts` bumps `testTimeout` to 15s β Windows fsync under sequential saveSync calls makes the codex-handler retry-path tests slower than the 5s default. Bump is intentionally generous; the per-test work is unchanged. Refs `docs/talon-architecture-unification-plan.md` Phase 6.x. * test(agent-runtime): per-backend contract suite + purge stale prep markers Phase 7 wiring: `backend-contract.test.ts` runs the full `assertBackendContract` suite over every shipped `BackendId`, via `adaptQueryBackend` against a well-behaved stub. Catches regressions in the adapter shim (e.g. capability flag drift, missing usage event, catalog identity mismatch) before any per-backend rewrite changes the SDK translation. Docs: - `agent-runtime/README.md` now opens with a phase status table instead of "no production caller invokes the shim yet" prose. The migration cookbook stays β those steps still apply to the Phase 3.x / 5.x per-backend rewrites that haven't landed yet. - Module-level doc comments in `index.ts`, `events.ts`, `capabilities.ts`, `adapter.ts`, `registry.ts`, `store.ts`, `contract-tests.ts` drop "Phase 1-2 contract: no production caller invokes this yet" β those are stale, the agent-runtime is consumed by `/status`, `/model`, the storage modules, and the contract test wiring. - `backend/codex/oauth-incompat.ts` drops the "Phase 6.x migration note" header (the migration is the current behaviour, not a note). No runtime changes β purely documentation. Phase 3.x backend rewrites and Phase 5.x ToolRegistry centralisation are not in this PR; the README's cookbook is the entry point for those. * fix(tests): typed spread args in storage save-error mocks Five TS2556 errors after the Phase 6.x JsonStore migration: the mock wrappers passed `(...args: unknown[])` to a `vi.fn(() => { throw ... })` whose inferred signature has no parameters, so the spread had no rest parameter to land on. Annotate the inner mock with `(..._args: unknown[])` so the wrapper's `failingWrite(...args)` call matches an explicit rest parameter. Behaviour unchanged β the args are still ignored by the throw. * feat(agent-runtime): Phase 3 + 4 + 5 β native event emission, log bridge, tool registry materialisation Phase 3 β backends emit AgentEvent natively: - `backend/shared/to-event-stream.ts` wraps any callback-based `query()` into an async-iterable of `AgentEvent`s. The wrapper intercepts `onStreamDelta` / `onTextBlock` / `onToolUse`, pushes each onto a queue, and drains the queue concurrently with the awaited query result. Event ordering matches the SDK's natural flow: run_started β text_delta* β assistant_message* β tool_call* β usage β completed (or run_started β error). - Every backend factory (Codex, Claude SDK, Kilo, OpenCode, OpenAI Agents) wires `runChatTurnEvents: (p) => toEventStream(handleMessage, p)` onto its `QueryBackend`. The agent-runtime adapter prefers this native stream when present and falls back to its synthesised minimal sequence only for stub / third-party backends. Phase 4 β `AgentEventLogRenderer` consumers: - `toOneShotEventStream` is the one-shot counterpart of `toEventStream`. It wraps a `runOneShotAgent(params)` legacy function into an `AgentEvent` stream by intercepting `appendLog` writes and relaying them as `assistant_message` events. The result can be piped into `streamLog(stream, sink)` from `core/agent-runtime/event-log-renderer.ts`, replacing the inline markdown handling heartbeat / dream / trigger consumers do today. The bridge is opt-in β callers wanting the legacy shape continue to supply `appendLog` directly. Phase 5 β centralised tool surface: - `core/agent-runtime/tool-registry-builder.ts` converts the existing `ALL_TOOLS` catalog into `ToolDescriptor[]` and exposes a process-scoped `getGlobalToolRegistry()` singleton. Bootstrap eagerly materialises the registry so the first turn doesn't pay the catalog walk. Backends migrating to descriptor-driven MCP config render now read through one canonical source β `delivery` derives from `endsTurn`, `requiresAmbientChat` from the frontend allowlist, `readOnly` from the tag. New tests: - `to-event-stream.test.ts` β chat event-stream wire format. - `to-event-stream-oneshot.test.ts` β Phase 4 bridge composes with `streamLog` so legacy one-shot handlers can stream markdown through the canonical renderer without changing their callback contract. - `agent-runtime-tool-registry-builder.test.ts` β descriptor conversion, singleton, idempotence. Existing per-backend contract tests now exercise the native `runChatTurnEvents` path (the adapter routes through it), so the event-stream wire format is exercised across every shipped backend id. README updated: every phase reads "done" with the corresponding infrastructure pointer. * fix(to-event-stream): emit text_delta as delta not accumulated The legacy `onStreamDelta(accumulated)` callback delivers the FULL accumulated text on every call; `AgentEvent.text_delta.text` is meant to carry the new chunk so pipe consumers re-accumulate deterministically. The first cut emitted the accumulated value, which the legacy bridge then double-accumulated: accumulated[i] = "hello" β text_delta.text = "hello" accumulated[i+1] = "hello there" β text_delta.text = "hello there" pipe: textAccum = "hello" + "hello there" = "hellohello there" Fix: the wrapper tracks the last accumulated string and emits only the trailing slice. If the new accumulator doesn't start with the prior one (block boundary, reset), emit the full new string as a fresh delta and re-anchor β defensive against backends that swap accumulators mid-turn. New test covers the reset case; existing monotonic case adjusted to assert the two deltas concatenate to the full accumulated text. * refactor: kill QueryBackend, route every consumer through split Backend The fat-optional `QueryBackend` interface is gone. Every backend factory builds a composed `Backend` directly with explicit capability slots; every consumer reads through those slots. No legacy adapter in production β `core/agent-runtime/adapter.ts` deleted, the registry shim with it. Backend shape (`core/agent-runtime/capabilities.ts`): - `chat` β runChatTurn (AgentEvent stream, the ONLY chat path) - `background` β runOneShotAgent + evictOrphanSubprocesses - `models` β ModelRef-shaped + UnifiedModelInfo-shaped catalog - `sessions` β resetChat / warmSession - `tools` β refreshTools (hot MCP swap) - `usage` β getSessionSnapshot - `control` β updateSystemPrompt `composeBackend({...})` is the canonical builder; `deriveCapabilities` fills the flag set. `Backend` carries `cacheMetrics` flat at the top because every consumer needs it. Factory rewrites β every backend now builds Backend directly: - `backend/codex/factory.ts` - `backend/claude-sdk/factory.ts` - `backend/kilo/factory.ts` - `backend/opencode/factory.ts` - `backend/openai-agents/factory.ts` Consumer rewrites β every read goes through a slot: - `core/dispatcher.ts` consumes `chat.runChatTurn`, pipes events back through `pipeEventsToCallbacks` to honour the legacy caller contract. Errors arrive wrapped as `BridgedAgentError` carrying the canonical `AgentError`. - `core/active-model.ts` reads `models.resolveModelInfo` / `models.getDefaultModelId`. - `core/agent-runtime/resolver.ts` enriches via `models.getRawModelInfo` β `models.resolveModelInfo` β bare ref. - `core/backend-controller.ts` validates via `models.getRawModelInfo` or `models.resolveModelInfo`. - `core/heartbeat.ts` / `core/dream.ts` call `background.runOneShotAgent` and `background.evictOrphanSubprocesses`. - `core/gateway-actions.ts` calls `control.updateSystemPrompt` and `tools.refreshTools`. - Every Telegram / Discord / terminal command site reads through `backend.models?.X` / `backend.sessions?.X` / `backend.usage?.X`. `core/types.ts:QueryBackend` is now `export type QueryBackend = Backend` β deprecated alias for in-flight import sites. Nothing else in the codebase has the old shape. Tests: - `__tests__/helpers/stub-backend.ts` is a TEST-ONLY helper that converts the legacy flat fixture (`{ query, runOneShotAgent, ... }`) into the new `Backend` slot structure. Not a production legacy adapter β the production code is on the new shape end-to-end. - Per-test fixtures (dispatcher, integration, heartbeat, dream, backend-controller, backend-pool, backend-registry, codex-factory, reload-plugins, terminal-commands, telegram-model-menu-controller, active-model, agent-runtime-resolver) rewritten to call `stubBackend({...})` and read through the slot structure. - `dispatcher.test`'s stream-callback assertion now verifies the pipe round-trip (backend emits text β events β caller callbacks fire) instead of asserting direct callback passthrough. - `integration.test` error-path test asserts `BridgedAgentError` with `kind: "rate_limit"` instead of the original `TalonError` instance (errors are classified at the event boundary). - Adapter-based tests deleted: `agent-runtime-adapter.test.ts`, `agent-runtime-contracts.test.ts`, `agent-runtime-registry.test.ts`, `backend-contract.test.ts`. The contract suite (`contract-tests.ts`) is still consumed in tests that drive a real Backend. 2890 tests pass, 101 skipped. `npm run typecheck` clean. * chore: fix prettier formatting (Code Quality CI) * chore: drop QueryBackend alias + clean up stale phase markers QueryBackend is deleted entirely. Every type import sites that still referenced `import type { QueryBackend }` now imports `Backend` from `core/agent-runtime/capabilities` directly: - bootstrap.ts, core/gateway.ts, frontend/reasoning-levels.ts, frontend/terminal/commands.ts, frontend/telegram/{callbacks, commands}.ts inline import. Comments and doc strings purged of references to the now-gone adapter / registry shim / phase-numbered milestones: - `agent-runtime/README.md` rewritten to describe the landed state rather than a migration plan; the "Migration cookbook" becomes "How to add a new backend / store / catalog". - `events.ts`, `capabilities.ts`, `legacy-bridge.ts`, `resolver.ts`, `model-ref.ts`, `run-policy.ts`, `store.ts`, `tool-descriptor.ts`, `tool-registry.ts`, `tool-registry-builder.ts`, `event-log-renderer.ts`, `contract-tests.ts` drop "Phase X of the architecture unification plan" prose. Each module describes what it is, not when it landed. - `core/tools/index.ts` drops "Phase 5" reference around `TURN_TERMINATOR_NAMES`. - `backend/codex/init.ts`, `backend/codex/oauth-incompat.ts` drop "Phase 6.x" stickers. - Factory headers (`codex`, `claude-sdk`, `kilo`, `opencode`, `openai-agents`) replace the awkward "No fat-optional `Backend` surface" trailers with "Returns a composed `Backend` with capability slots for X". - Frontend command files (`telegram/commands`, `discord/commands`, `telegram/model-menu`) drop "Phase 2.2 / 2.3" annotations from the `resolveActiveModelRefForChat` call sites. 2890 tests pass, typecheck clean. * chore: delete unused Phase 4 + Phase 5 infrastructure Three pieces of agent-runtime infrastructure shipped without a production consumer. Removed wholesale to keep the runtime surface honest: - `AgentEventLogRenderer` (`event-log-renderer.ts`, 290 LOC) β Phase 4 markdown renderer for heartbeat / dream / trigger log files. Heartbeat and dream still drive their log files through direct `appendLog` markdown; the renderer never picked up a consumer. - `toOneShotEventStream` (in `backend/shared/to-event-stream.ts`) β Phase 4 bridge that turned `runOneShotAgent` into an event stream. Only existed to feed `streamLog`, which is gone too. - `ToolRegistry` + `ToolDescriptor` + `tool-registry-builder.ts` (~500 LOC) β Phase 5 centralised tool surface. Each backend's MCP config builder still pulls servers directly from `getPluginMcpServers(...)`; nothing reads from the registry. - `reduceEventsToResult` (in `legacy-bridge.ts`) β accumulator for AgentEvent streams without callbacks. The dispatcher uses `pipeEventsToCallbacks` and gets the result back directly; nothing else needed the reducer. Knock-on: - `RunPolicy.tools.filter` collapsed from a structured `ToolFilter` predicate into two flat boolean fields (`excludeDelivery`, `excludeAmbientChatTools`) so the dream policy still declares its intent without depending on the now- deleted `tool-descriptor.ts`. - `bootstrap.ts` drops the `getGlobalToolRegistry()` eager materialisation. - `core/agent-runtime/index.ts` barrel drops every removed export. - Self-tests for the deleted modules removed (`agent-runtime-event-log-renderer.test.ts`, `agent-runtime-tool-registry.test.ts`, `agent-runtime-tool-registry-builder.test.ts`, `to-event-stream-oneshot.test.ts`). `agent-runtime-legacy-bridge.test.ts` drops the `reduceEventsToResult` describe block. `agent-runtime-types.test.ts` drops the `tool-descriptor` describe block and updates the dream-policy filter assertion. README marks Phase 4 + Phase 5 as descoped with a short note explaining what got removed and why. The infrastructure can come back when there's a real consumer. 2831 tests pass, typecheck clean. * refactor: delete RunPolicy β pure ceremony The dispatcher constructed defaultRunPolicyFor("chat") on every turn and threaded it through backend.chat.runChatTurn as params.policy. No backend handler read it. The slot existed, the value was built, nothing consumed it. Removed: - src/core/agent-runtime/run-policy.ts (the policy types, defaults, and allowsDelivery / requiresAmbientChat helpers) - ChatRunParams.policy field - Dispatcher's policy-construction call - Contract-tests' policy field on each runChatTurn invocation - Run-policy barrel exports from agent-runtime/index.ts - README's run-policy.ts section - Self-tests for run-policy in agent-runtime-types.test.ts When a real consumer needs a policy (heartbeat carrying explicit chat-id requirements, dream excluding delivery tools), the shape can come back β but only with a backend that actually reads it. * refactor(model-catalog): collapse the dual ModelRef/UnifiedModelInfo surface `ModelCatalog` carried two parallel method sets β ModelRef-shaped (`resolveModel` / `listModels` / `getDefaultModel` / `getModelInfo`) and UnifiedModelInfo-shaped (`resolveModelInfo` / `getDefaultModelId` / `getRawModelInfo` / `getSettingsPresentation` / `getProviders` / `getProviderModels` / `formatModelError` / `listModelsRaw`). Every backend factory filled both, half the methods one-line wrapping the other half. Only the contract test read the ModelRef-shaped surface. Collapsed onto the UnifiedModelInfo shape (the one the frontend pickers, picker formatter, and active-model resolver all consume). `listModelsRaw` renamed back to `listModels` for parity with the rest of the slot. `ModelRef` is now strictly the resolver's output β an enriched routing identity produced by `agent-runtime/resolver.ts` for `/status` and `/model` display. Backends don't construct refs directly any more. Knock-on: - `ModelCatalog` methods are required (the slot itself is still optional on `Backend`). - `ModelResolveContext`, `ModelResolution`, `ModelFilter`, `ModelList` types deleted β they were part of the ModelRef surface and unused. - `core/backend-controller.ts:isModelValidForBackend` simplifies to one call into `resolveModelInfo`. The `getRawModelInfo` / `resolveModelInfo` fallback ladder is gone (was historic from when backends shipped one or the other). - Each backend factory drops its 30-line ModelRef block + the `makeBareModelRef` import. - Contract test `assertModelCatalogDefaultShape` rewritten to check `getDefaultModelId()` shape (string | null | undefined) instead of `getDefaultModel({})` returning a ref. - `frontend/terminal/commands.ts` switches from `listModelsRaw` to `listModels`. - Test helpers + per-test fixtures updated. 2824 tests pass, typecheck clean. * refactor: collapse the two model resolvers into one `core/active-model.ts:resolveActiveModelForChat` and `agent-runtime/resolver.ts:resolveActiveModelRefForChat` walked the same 5-step chain twice β one returned a string, the other a `ModelRef`. The ref resolver wrapped the string resolver, called the catalog a second time to enrich, and was the source of every \"why are we hitting the catalog twice?\" investigation since Phase 2.1 landed. Collapsed into one entry point. `resolveActiveModelForChat(chatId, backend, backendId, config)` returns `{ model, ref, source }`: - `model` β the raw id from the chain (per-chat override β backend canonical β operator default β legacy global β null). - `ref` β enriched `ModelRef` for the same id (or `null` when `model` is null OR `backendId` isn't a known `BackendId`). - `source` β chain-step tag for toast wording. The enrichment helper (`materialiseRef`) lives in active-model.ts alongside the chain β one file, one resolver. `getRawModelInfo` then `resolveModelInfo` fallback ladder is preserved. `getActiveModelForChat` (string) and `getActiveModelRefForChat` (ref) are thin convenience wrappers. Callers pick the shape they need; no double catalog hit either way. Deletions: - `src/core/agent-runtime/resolver.ts` (260 LOC) - `src/__tests__/agent-runtime-resolver.test.ts` - `agent-runtime/index.ts` exports for the deleted module - README section for `resolver.ts` (replaced with the single resolver's API doc) Consumer migration: - `frontend/discord/commands.ts`, `frontend/telegram/commands.ts`, `frontend/telegram/model-menu.ts` import `resolveActiveModelForChat` from `core/active-model.js` and destructure `{ ref }` directly. The `{ modelId }` field rename folded into `{ model }` since the chain already produces the raw id under that name. - `active-model.test.ts` switches its exact-match assertions to `toMatchObject` so the new `ref` field doesn't break each case. 2807 tests pass, typecheck clean. * refactor(core): require resolveActiveModel in dispatcher; drop codex sessions boilerplate Resolves two architectural seams from the recent audit: * Empty-model-ref guard at the dispatcher β `resolveActiveModel` is now REQUIRED in DispatcherDeps and returns a real ModelRef alongside the string model. Dispatcher feeds the ref directly into backend.chat instead of synthesising one via makeBareModelRef. The send-time null-model branch now checks both fields so any catalog-driven backend with no per-chat pick + no operator default fails closed. * SessionBackend.resetChat is optional β Codex never had session state to reset (its handler owns the per-chat thread id via storage/sessions.ts) so the no-op resetChat slot is dropped entirely from the codex factory. Claude SDK keeps only warmSession. Test stubs grow a stubResolveActiveModel() helper so every initDispatcher call in dispatcher.test.ts + integration.test.ts satisfies the now-required field with a bare ModelRef matching the backend id. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(core): relocate QueryParams/QueryResult out of core/types `QueryParams` and `QueryResult` describe the callback-shaped contract each backend's `handler.ts` exposes internally β they are NOT a core abstraction. `ChatBackend.runChatTurn` (in `core/agent-runtime/capabilities.ts`) is the canonical surface every consumer outside `src/backend/` talks to. Moving them into `src/backend/shared/handler-types.ts` keeps `core/types.ts` clean of implementation-detail shapes so the dispatcher / cron / triggers / frontends can no longer accidentally couple to the callback contract. `ExecuteResult` is now declared in full rather than extending `QueryResult` (it lives at the dispatcher layer where coupling to a backend-internal type would be wrong). Doc strings in `agent-runtime/capabilities.ts`, `events.ts`, and `legacy-bridge.ts` are updated so they no longer reference the relocated types as if they were core abstractions. Also: gitignore `.claude/` (local agent state). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(agent-runtime): rename legacy-bridge β event-bridge The dispatcher's callback contract (`onStreamDelta` / `onTextBlock` / `onToolUse`) is the canonical surface frontends consume β not a legacy API. Renaming `legacy-bridge.ts` to `event-bridge.ts` reframes the module honestly: it bridges native `AgentEvent` streams to the callback contract dispatcher consumers use, without implying either side is deprecated. Also drops the "legacy" framing from `to-event-stream.ts` (the backend-internal `QueryParams` shape isn't legacy β it's just the handler's internal callback contract). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(claude-sdk): native AgentEvent emission in handler `handleMessage` is now an internal back-compat wrapper around the new `runChatTurn` async generator. The generator yields the canonical `run_started β text_delta* β reasoning* β assistant_message* β tool_call* β usage β completed` sequence directly β no `toEventStream` queue adapter, no callbacks crossing the ChatBackend boundary. The shared retry decision tree gets a generator-shaped sibling (`applyRetryDecisionStream`) so error recovery delegates via `yield*` and the retried run's events flow into the outer stream transparently. Flow-violation retries do the same via direct `yield* runChatTurn(...)`. `processStreamDelta` returns the chunk to emit instead of taking an `onStreamDelta` callback; the StreamState tracks per-phase unflushed deltas so the throttle interval still bounds event volume to ~750ms. Factory wires `claudeRunChatTurn` straight onto `ChatBackend.runChatTurn`. The `handleMessage` wrapper remains exported for the watchdog test + any back-compat call sites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(backend/shared): rename to-event-stream β handler-to-events `toEventStream` was framed as a "legacy adapter" but it is the canonical bridge between SDK-callback-shaped chat handlers (Codex, OpenCode, Kilo, OpenAI Agents) and the native `AgentEvent` contract every consumer reads. Renaming makes that explicit; the docstring now documents both the use case AND the alternative (backends with native event emission β claude-sdk post-conversion β skip this and yield events directly). Mechanical rename only: `toEventStream` β `handlerToEvents`, `to-event-stream.ts` β `handler-to-events.ts`. The 4 factories that still wrap their callback handlers now import the renamed helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: remove dead code and stale doc refs, fix formatting Leftover unused imports from the model-persistence refactor (setChatModel x4, clearAllChatModels), an unused chunkButtons helper, dead reasoning-level re-exports, and a dead dispatcher-test var. Also fixes comments pointing at deleted modules (resolver.ts, tool-registry-builder.ts, event-log-renderer) and applies prettier formatting for the CI format gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(storage): restore .bak backup-on-write in JsonStore loadSync read and promoted <path>.bak on a corrupt primary, but save/saveSync never wrote a backup -- a half-implemented fallback and a durability regression versus the legacy stores. Restore the best-effort pre-write copy so the load fallback ladder is real, plus a locking test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(agent-runtime): restore per-backend contract suite The suite was deleted as collateral when QueryBackend was removed, orphaning contract-tests.ts while the README and PR still claimed Phase 7 done. backend-contract.test.ts runs assertBackendContract across every BackendId through the real handlerToEvents-to-composeBackend path, and asserts the contract checks reject malformed streams. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(agent-runtime): collapse capability flags, tier ModelCatalog, unify error classification Drop the dead BackendCapabilities flag record and deriveCapabilities -- a slot's presence is the capability (single source of truth). Drop the dead ChatRunParams.abortController. Tier ModelCatalog into a required resolution core plus optional picker/browse methods, with graceful frontend degradation. Route both the native handler and the callback wrapper through one classifiedToAgentError(classify(err)) boundary, deleting the wrapper's string-matcher and fixing the native mapper's overloaded/context_length mis-mapping. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: thread fallback model through stream retry params Post-rebase reconciliation with PR #265 from main: the stream-shaped retry helper still steered fallback retries via a transient setChatModel flip, which params.model silently outranks. Thread the fallback model id through buildRetryStream into the recursive call instead, matching the callback-shaped helper. Also point the retry test at QueryParams new home in backend/shared/handler-types, and refresh two stale references (legacy-bridge log prefix, adapter mention in contract-tests). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(claude-sdk): drop dead handleMessage wrapper, test the stream surface The callback-shaped handleMessage wrapper existed only so the watchdog test and the integration bootstrap kept compiling β production wires runChatTurn directly onto ChatBackend.runChatTurn. A shim kept alive solely for tests is backwards: both now exercise the real surface. - watchdog test drains runChatTurn's event stream and asserts on completed/error events - integration bootstrap mirrors the dispatcher exactly: runChatTurn -> pipeEventsToCallbacks - build-sea.mjs: quote the node path when spawning through cmd.exe (C:\Program Files broke at the space), unblocking the stub-claude SEA build β and with it the functional integration suite β on Windows Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(integration): boot through the production composition root talon-bootstrap previously hand-wired initAgent + a direct runChatTurn call β a parallel test-only boot path that would not catch regressions in factory registration, backend pool boot, dispatcher wiring, or active-model resolution. It now runs the real initBackendAndDispatcher with a fake Frontend (the same seam index.ts swaps per platform) and drives every turn through the production dispatcher.execute(). Supporting changes: - config: new 'dream' toggle (default true) mirroring pulse/heartbeat; maybeStartDream respects it. Tests disable it β dreams read real ~/.talon state and fire a one-shot agent mid-turn otherwise. - teardownBootstrap only swaps gateway wiring now; the backend pool, dispatcher, and workspace are process-level singletons booted once. Deleting the workspace between describe blocks broke every subsequent SDK spawn (cwd vanished from under the booted backend). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * style: fix prettier drift * fix: preserve text-block delivery failures through events --------- Co-authored-by: claudiusthebot <noreply@anthropic.com> Co-authored-by: Claudius <claudiusthebot@gmail.com>
Summary
setChatModel-based fallback retry pattern is a silent no-op. Commit fix: pass resolved chat model into backendsΒ #248 introducedparams.modelwhich all backends resolve beforechatSettings.model, so the transientsetChatModelflip was never observed. Fixed insrc/backend/shared/handle-retry.ts,src/backend/codex/handler.ts, andsrc/backend/openai-agents/handler.tsby spreading{ ...params, model: fallbackModelId }into the recursive call.mapListModelsinadapter.tsreturned the pre-filtertotalfrom the legacy backend after applyingselectableOnlyandqueryclient-side filters, violating theModelListinterface contract (total= count after filter). Fixed to returnmapped.length.legacy-bridge.tsreduceEventsToResultβ...(saw ? {} : {})spreads an empty object on both branches. Removed.shared-handle-retry.test.tsto assert onparamsSeenInRecursion.model(new mechanism) rather thangetChatSettings().model(old mechanism that no longer fires).Root cause
Commit #248 ("fix: pass resolved chat model into backends") wired
params.modelinto everybackend.query()call from the dispatcher. Every backend resolves model asparams.model ?? chatSettings.model, so anysetChatModelcall made immediately beforerecurseWithRetried(params)/handleMessage(params, true)is masked β the unchangedparams.modelwins. The fallback model was never actually used.Test plan
npm testβshared-handle-retry.test.tsfallback_modelsuite verifies fallback model is received in the recursive call viaparamsSeenInRecursion.modeladapter.tsβ confirm a filteredlistModelscall returnstotal === models.lengthhttps://claude.ai/code/session_01Pe2jJNJoMiDUSffdVqiX6P
Generated by Claude Code