Skip to content

feat(core): resolveActiveModelRefForChat β€” Phase 2.1 (stacked on #253) - #254

Merged
dylanneve1 merged 1 commit into
mainfrom
feat/agent-runtime-model-ref-resolver-phase2
May 24, 2026
Merged

feat(core): resolveActiveModelRefForChat β€” Phase 2.1 (stacked on #253)#254
dylanneve1 merged 1 commit into
mainfrom
feat/agent-runtime-model-ref-resolver-phase2

Conversation

@claudiusthebot

Copy link
Copy Markdown
Collaborator

What

Adds resolveActiveModelRefForChat() β€” the ModelRef-shaped
counterpart to core/active-model.ts's string-side
resolveActiveModelForChat(). Phase 2.1 of the architecture
unification plan.

Stacked on #253. Targets feat/agent-runtime-types-phase1 so the
diff is just the new resolver + tests. When #253 merges to main,
I'll re-target this PR to main.

No caller migration. The existing 5-step string-side chain stays
the single chain-of-truth. This PR adds a wrapping function that
enriches the returned id into a ModelRef carrying the metadata
downstream consumers (/status, /model menu, telemetry) currently
re-derive on the spot. Production behaviour is unchanged.

Why

Per the plan's Phase 2:

Create one resolveActiveModel() path that returns ModelRef.
Then convert /model, /status, chat query, heartbeat, and dream
to use it.

The plan splits naturally into 2.1 (introduce the function) + 2.2+
(migrate callers one at a time). Doing it as one giant PR would
touch 20+ call sites across bootstrap.ts, frontend/reasoning- levels.ts, frontend/telegram/{commands,callbacks,model-menu}.ts,
and frontend/discord/{commands,callbacks}.ts β€” too risky and too
hard to review.

What lands

src/core/agent-runtime/
  resolver.ts   resolveActiveModelRefForChat(chatId, backend,
                backendId, config) β†’ { ref: ModelRef | null,
                source: ActiveModelSource } plus the
                getActiveModelRefForChat() convenience wrapper.
  index.ts      Re-exports the new symbols.

Enrichment strategy (deliberate fall-through)

  1. backend.getModelInfo(id) β€” preferred, full metadata
  2. backend.resolveModel(id) β€” exact β€” fallback, also full metadata
  3. makeBareModelRef(backend, id) β€” last resort, identity-only
    (with cacheSupport stamped)

Each step's failure (returns null/throws) cleanly falls through to
the next, with warnings logged via logWarn.

cacheSupport propagation

Each ref carries the backend's cacheMetrics field β€” backends
report caching uniformly across their catalog today, so this is the
right place to stamp it.

ActiveModelSource β†’ ModelSource mapping

Existing (string side) New (ModelRef.source)
override-valid chat
override-invalid-fallback fallback
backend-canonical backend-default
config-backend-defaults config
config-legacy-global config
none (ref is null)

The config-* β†’ config collapse is intentional. ModelRef.source
captures provenance (chat vs config vs backend); the fine-grained
ActiveModelSource tag returned alongside keeps the reason for
toast wording.

Invariants

  • ref === null iff the 5-step chain returned model === null OR
    the supplied backendId is not a known BackendId.
  • source is returned verbatim from the underlying chain so callers
    use identical toast wording in both APIs.

Tests

agent-runtime-resolver.test.ts (17 cases):

  • Enrichment via getModelInfo (preferred)

  • Enrichment via resolveModel (fallback)

  • Bare ref fallback when neither catalog method matches

  • cacheSupport propagation (read / readwrite / undefined β†’ none)

  • ref returned for trusted overrides when no resolveModel exists

  • Source mapping β€” every ActiveModelSource β†’ ModelSource branch

  • Null edge cases (chain exhausts, unknown backendId, null backend)

  • Survives getModelInfo throwing (falls through to resolveModel)

  • Survives resolveModel throwing (falls through to bare ref)

  • getActiveModelRefForChat convenience wrapper

  • Full suite: 2917 / 2929 (12 pre-existing skips). 17 net new
    tests on top of Phase 1's 50.

  • Type check passes (npm run typecheck)

  • No new lint warnings (npm run lint β€” 18 pre-existing
    warnings, none in new code).

  • Prettier clean on the new files.

Out of scope

  • No caller migration. /status, /model, chat query, heartbeat,
    dream all keep calling resolveActiveModelForChat. Phase 2.2+
    converts them one at a time, each in its own PR.
  • No changes to core/active-model.ts itself. The new module wraps,
    doesn't fork.

Stacked on #253. Refs docs/talon-architecture-unification-plan.md

@claudiusthebot
claudiusthebot changed the base branch from feat/agent-runtime-types-phase1 to main May 24, 2026 12:34
@dylanneve1
dylanneve1 enabled auto-merge (squash) May 24, 2026 12:50
Introduces the ModelRef-shaped counterpart to
core/active-model.ts's string-side resolveActiveModelForChat. Phase
2.1 contract: add the function + tests, no caller migration. The
existing 5-step string-shaped chain stays the single chain-of-truth β€”
this module wraps it and enriches the returned id into a ModelRef
carrying the metadata downstream consumers (/status, /model menu,
telemetry) currently re-derive on the spot.

What lands

src/core/agent-runtime/
  resolver.ts        resolveActiveModelRefForChat(chatId, backend,
                     backendId, config) β†’ { ref: ModelRef | null,
                     source: ActiveModelSource } plus the
                     getActiveModelRefForChat() convenience wrapper.
  index.ts           Re-exports the new symbols.

Enrichment strategy (deliberate fall-through)

  1. backend.getModelInfo(id)         β€” preferred, full metadata
  2. backend.resolveModel(id) β€” exact β€” fallback, also full metadata
  3. makeBareModelRef(backend, id)    β€” last resort, identity-only
                                       (with cacheSupport stamped)

cacheSupport propagation
  Each ref carries the backend's cacheMetrics field β€” backends
  report caching uniformly across their catalog today, so this is
  the right place to stamp it.

ActiveModelSource β†’ ModelSource mapping
  override-valid             β†’ "chat"
  override-invalid-fallback  β†’ "fallback"
  backend-canonical          β†’ "backend-default"
  config-backend-defaults    β†’ "config"
  config-legacy-global       β†’ "config"
  none                       β†’ (ref is null)

Invariants
  - ref === null iff the 5-step chain returned model === null OR the
    supplied backendId is not a known BackendId.
  - source is returned verbatim from the underlying chain so callers
    keep using identical toast wording.

Tests

agent-runtime-resolver.test.ts (17 cases):
  - Enrichment via getModelInfo (preferred)
  - Enrichment via resolveModel (fallback)
  - Bare ref fallback when neither catalog method matches
  - cacheSupport propagation (read / readwrite / undefined β†’ "none")
  - ref returned for trusted overrides when no resolveModel exists
  - Source mapping β€” every ActiveModelSource β†’ ModelSource branch
  - Null edge cases (chain exhausts, unknown backendId, null backend)
  - Survives getModelInfo throwing (falls through to resolveModel)
  - Survives resolveModel throwing (falls through to bare ref)
  - getActiveModelRefForChat convenience wrapper

Full suite: 2917 / 2929 (12 pre-existing skips). 17 net new tests
on top of Phase 1's 50. Typecheck clean. Prettier clean. No new
lint warnings.

Out of scope

  - No caller migration. /status, /model, chat query, heartbeat,
    dream all keep calling resolveActiveModelForChat. Phase 2.2+
    converts them one at a time, each in its own PR.
  - No changes to core/active-model.ts itself. The new module wraps,
    doesn't fork.
  - The lossy "config-backend-defaults" + "config-legacy-global" β†’
    "config" collapse is intentional β€” ModelRef.source captures
    provenance (chat / config / backend), the ActiveModelSource tag
    returned alongside keeps the fine-grained reason for toast
    wording.

Stacked on top of feat/agent-runtime-types-phase1 (#253).
Refs docs/talon-architecture-unification-plan.md
@dylanneve1
dylanneve1 force-pushed the feat/agent-runtime-model-ref-resolver-phase2 branch from fd020f0 to a723a84 Compare May 24, 2026 12:58
@dylanneve1
dylanneve1 merged commit 373cecd into main May 24, 2026
34 checks passed
dylanneve1 pushed a commit that referenced this pull request May 24, 2026
First caller migration on top of Phase 2.1's
resolveActiveModelRefForChat. Telegram and Discord /status now read
context window + active model identity from a single ModelRef
instead of calling resolveActiveModelForChat + getModelInfo
back-to-back.

What changes

  src/frontend/telegram/commands.ts (status command, ~25 LOC)
  src/frontend/discord/commands.ts  (handleStatus,    ~23 LOC)

Both files swap:

  resolveActiveModelForChat(...) β†’ activeModel string
  + be.getModelInfo(activeModel) β†’ contextWindow

with:

  resolveActiveModelRefForChat(...) β†’ ModelRef
  + ref.contextWindow

The ref resolver wraps the same 5-step chain internally, so the
chosen model id is identical for every input. The difference is
one fewer round-trip to getModelInfo for the common case β€” the
ref's enrichment path already called it.

What stays the same

  - Cache display still reads be.cacheMetrics directly (ref's
    cacheSupport is propagated from the same field; either source
    works, and keeping the existing call site keeps the diff small).
  - The snap.contextModelId re-fetch path (when the SDK reports a
    different model id mid-session) stays direct via getModelInfo
    β€” the resolver only resolves the one active model.
  - "No model selected" fallback wording unchanged.
  - All other resolveActiveModelForChat call sites (settings menu,
    post-reset toast, model menu, reasoning levels, callbacks) keep
    using the string resolver. Phase 2.3+ migrates them one at a
    time as their own PRs.

Behavioural equivalence

For every input where the string-side chain returns a non-null
model, the ref resolver returns a ref with the same id (Phase 2.1
tests pin this). The ref's contextWindow comes from getModelInfo
internally β€” same source as before. cacheSupport propagates from
the backend's cacheMetrics β€” same value. Net behaviour is identical
for the common case and strictly more enriched for the edge case
where getModelInfo is absent but resolveModel returns an exact
match.

Tests

  - No new unit tests in this PR. The migration is intentionally
    no-op-equivalent for the common case and Telegram / Discord
    /status are not unit-tested directly (they go through real
    Telegraf / discord.js handlers).
  - Full suite stays at 2917 / 2929 (12 pre-existing skips).
  - Typecheck clean. Prettier clean. No new lint warnings.

Stacked on feat/agent-runtime-model-ref-resolver-phase2 (#254).
When #254 lands, this PR rebases onto main.
Refs docs/talon-architecture-unification-plan.md
dylanneve1 pushed a commit that referenced this pull request May 25, 2026
First caller migration on top of Phase 2.1's
resolveActiveModelRefForChat. Telegram and Discord /status now read
context window + active model identity from a single ModelRef
instead of calling resolveActiveModelForChat + getModelInfo
back-to-back.

What changes

  src/frontend/telegram/commands.ts (status command, ~25 LOC)
  src/frontend/discord/commands.ts  (handleStatus,    ~23 LOC)

Both files swap:

  resolveActiveModelForChat(...) β†’ activeModel string
  + be.getModelInfo(activeModel) β†’ contextWindow

with:

  resolveActiveModelRefForChat(...) β†’ ModelRef
  + ref.contextWindow

The ref resolver wraps the same 5-step chain internally, so the
chosen model id is identical for every input. The difference is
one fewer round-trip to getModelInfo for the common case β€” the
ref's enrichment path already called it.

What stays the same

  - Cache display still reads be.cacheMetrics directly (ref's
    cacheSupport is propagated from the same field; either source
    works, and keeping the existing call site keeps the diff small).
  - The snap.contextModelId re-fetch path (when the SDK reports a
    different model id mid-session) stays direct via getModelInfo
    β€” the resolver only resolves the one active model.
  - "No model selected" fallback wording unchanged.
  - All other resolveActiveModelForChat call sites (settings menu,
    post-reset toast, model menu, reasoning levels, callbacks) keep
    using the string resolver. Phase 2.3+ migrates them one at a
    time as their own PRs.

Behavioural equivalence

For every input where the string-side chain returns a non-null
model, the ref resolver returns a ref with the same id (Phase 2.1
tests pin this). The ref's contextWindow comes from getModelInfo
internally β€” same source as before. cacheSupport propagates from
the backend's cacheMetrics β€” same value. Net behaviour is identical
for the common case and strictly more enriched for the edge case
where getModelInfo is absent but resolveModel returns an exact
match.

Tests

  - No new unit tests in this PR. The migration is intentionally
    no-op-equivalent for the common case and Telegram / Discord
    /status are not unit-tested directly (they go through real
    Telegraf / discord.js handlers).
  - Full suite stays at 2917 / 2929 (12 pre-existing skips).
  - Typecheck clean. Prettier clean. No new lint warnings.

Stacked on feat/agent-runtime-model-ref-resolver-phase2 (#254).
When #254 lands, this PR rebases onto main.
Refs docs/talon-architecture-unification-plan.md
dylanneve1 pushed a commit that referenced this pull request May 25, 2026
* feat(status): /status consumes ModelRef β€” Phase 2.2

First caller migration on top of Phase 2.1's
resolveActiveModelRefForChat. Telegram and Discord /status now read
context window + active model identity from a single ModelRef
instead of calling resolveActiveModelForChat + getModelInfo
back-to-back.

What changes

  src/frontend/telegram/commands.ts (status command, ~25 LOC)
  src/frontend/discord/commands.ts  (handleStatus,    ~23 LOC)

Both files swap:

  resolveActiveModelForChat(...) β†’ activeModel string
  + be.getModelInfo(activeModel) β†’ contextWindow

with:

  resolveActiveModelRefForChat(...) β†’ ModelRef
  + ref.contextWindow

The ref resolver wraps the same 5-step chain internally, so the
chosen model id is identical for every input. The difference is
one fewer round-trip to getModelInfo for the common case β€” the
ref's enrichment path already called it.

What stays the same

  - Cache display still reads be.cacheMetrics directly (ref's
    cacheSupport is propagated from the same field; either source
    works, and keeping the existing call site keeps the diff small).
  - The snap.contextModelId re-fetch path (when the SDK reports a
    different model id mid-session) stays direct via getModelInfo
    β€” the resolver only resolves the one active model.
  - "No model selected" fallback wording unchanged.
  - All other resolveActiveModelForChat call sites (settings menu,
    post-reset toast, model menu, reasoning levels, callbacks) keep
    using the string resolver. Phase 2.3+ migrates them one at a
    time as their own PRs.

Behavioural equivalence

For every input where the string-side chain returns a non-null
model, the ref resolver returns a ref with the same id (Phase 2.1
tests pin this). The ref's contextWindow comes from getModelInfo
internally β€” same source as before. cacheSupport propagates from
the backend's cacheMetrics β€” same value. Net behaviour is identical
for the common case and strictly more enriched for the edge case
where getModelInfo is absent but resolveModel returns an exact
match.

Tests

  - No new unit tests in this PR. The migration is intentionally
    no-op-equivalent for the common case and Telegram / Discord
    /status are not unit-tested directly (they go through real
    Telegraf / discord.js handlers).
  - Full suite stays at 2917 / 2929 (12 pre-existing skips).
  - Typecheck clean. Prettier clean. No new lint warnings.

Stacked on feat/agent-runtime-model-ref-resolver-phase2 (#254).
When #254 lands, this PR rebases onto main.
Refs docs/talon-architecture-unification-plan.md

* feat(model-menu): /model consumes ModelRef + add modelId fallback β€” Phase 2.3

Extends Phase 2.2 by:

1. Adding `modelId` to ActiveModelRefResolution so callers can fall
   back to the raw string id from the 5-step chain when ref is null
   but the chain still produced a usable model id (BACKEND_IDS
   literal drift, null-backendId pre-bootstrap path).

2. Migrating both `/model` view builders in
   `frontend/telegram/model-menu.ts` to consume ref + modelId:
     - `buildModelMenuViewForChat`: ref.displayName replaces a
       deferred backend.getModelInfo call in fetchActiveDisplay
     - `buildModelBrowseViewForChat`: same β€” activeDisplay comes
       from ref when available, falls back to getModelInfo only when
       ref is null but modelId is set.

Resolver shape change

  ActiveModelRefResolution: { ref, source }
                          β†’ { ref, modelId, source }

  - modelId is the raw string from `resolveActiveModelForChat`.
  - When backendId is not a known BackendId (rare), ref is null but
    modelId is still set β€” callers can render the legacy default
    without a parallel call to the string resolver.

Tests

  - 17 β†’ 17 (renamed two cases to assert modelId presence).
  - Full suite 2917 / 2929, no regressions.
  - Typecheck clean. Prettier clean. No new lint warnings.

Scope notes β€” what's intentionally NOT migrated in this PR

  - `bootstrap.ts:245` (dispatcher's resolveActiveModel guard) β€” the
    guard needs just `{ model, backendId }`; ref adds no value, and
    threading it through the dispatcher interface is a Phase 3
    concern.
  - `heartbeat.ts` / `dream.ts` β€” they read config directly (not via
    the resolver). Will migrate to ref when Phase 4 moves their
    log-rendering through the new AgentEvent stream.
  - String-side callers in callbacks.ts / commands.ts (settings, toasts,
    post-reset messages) β€” they only need the raw id, not metadata.
    Migrating them mechanically would be busywork without behaviour
    improvement.

The plan's Phase 2 named targets (`/model`, `/status`, chat query,
heartbeat, dream): /model + /status are now on ref; chat query +
heartbeat + dream are deferred to Phase 3 / Phase 4 where the wider
event stream + log rendering refactor naturally absorbs them.

* feat(agent-runtime): registry shim + ToolRegistry + JsonStore + contract tests

Phase 3/5/6/7 prep β€” additive infrastructure that no production
caller invokes yet. Each module is the storage / abstraction
primitive future migrations will sit on top of; the actual
backend rewrites land in their own PRs in the plan's named order.

src/core/agent-runtime/

  registry.ts (~120 LOC)
    Adapts the existing legacy `BackendFactory` registry into
    `Backend` composed objects via `adaptQueryBackend`.
      - getAdaptedBackends(config, ctx, opts?)  init every factory,
                                                wrap as Backend[]
      - adaptOneBackend(id, ...)                init one by id
      - adaptInstantiatedBackend(instance, ...) wrap an existing
                                                BackendInstance
    Factories whose id is not in BACKEND_IDS are skipped with a
    console warning β€” the typed BackendId union is the source of
    truth.

  tool-registry.ts (~220 LOC, Phase 5 prep)
    Canonical store of ToolDescriptors.
      - register / registerAll (atomic rollback on collision)
      - get / has / size / list (sorted, fresh copies)
      - forPolicy(policy)  β†’ ToolDescriptor[] filtered by
                              RunPolicy.tools.filter
      - parseMcpToolId / groupToolsByServer helpers
    Phase 5.x backend renderers (Codex TOML, Claude SDK MCP
    config, etc) will consume this.

  store.ts (~360 LOC, Phase 6 prep)
    JsonStore<T> β€” unified persistence for the six JSON-backed
    stores under src/storage/.
      - load β†’ envelope or bare data; `.bak` fallback on corrupt
      - save β†’ write-file-atomic envelope { schemaVersion, savedAt, data }
      - update / set / get / isDirty / forceSave / reset
      - migrate hook on version mismatch (returns new value +
        schemaVersion, or null β†’ defaultValue fallback)
      - validate hook (throw to reject)
      - Test-friendly: JsonStoreFs + now() injection
    Per the plan's migration order, first consumer is Codex OAuth
    incompat learning (small, low-risk); chat-settings stays last
    because it's operationally sensitive.

  contract-tests.ts (~400 LOC, Phase 7)
    Backend contract assertions β€” every concrete backend (Claude
    SDK, Codex, Kilo, OpenCode, OpenAI Agents) must pass these:
      - assertBackendIdentity              id + label sanity
      - assertChatBackendEmitsRunStarted    first event is run_started
      - assertChatBackendTerminates         terminates on completed/error
      - assertChatBackendEmitsSingleUsage   exactly one usage event
      - assertCompletedUsageMatchesUsageEvent
      - assertBackgroundRunnerLifecycle     started + completed/error
      - assertModelCatalogDefaultShape      ref.backend matches identity
      - assertUsageTelemetryShape           finite, non-negative counters
      - assertBackendContract               runs the full suite, returns
                                            the list of checks performed

Tests

  agent-runtime-registry.test.ts        15 cases β€” shim init, BACKEND_IDS
                                        gating, adapter option threading
  agent-runtime-tool-registry.test.ts   15 cases β€” register / atomic
                                        rollback / fresh-copy isolation /
                                        MCP id parsing / server grouping
  agent-runtime-store.test.ts           17 cases β€” load fallbacks /
                                        envelope shape / migrate /
                                        validate / dirty handling
  agent-runtime-contracts.test.ts       21 cases β€” well-behaved adapter
                                        passes every contract +
                                        negative tests verify each
                                        helper catches violations

  Full suite 2976/2988 (12 pre-existing skips). 59 net new tests.
  Typecheck clean. Prettier clean. No new lint warnings (18, baseline).

Notes

  - tool-registry stores deep-clones tags and groups deep-clone
    tools so caller mutations on returned objects can't leak.
  - JsonStore deep-clones defaultValue on construction AND on
    every reset/exhausted-fallback path so two stores constructed
    from the same defaults object don't share mutable state.
  - contract helpers' wellBehavedLegacy stub includes resolveModel
    so the adapter populates the ModelCatalog slot.
  - The cloneJsonValue helper prefers structuredClone (Node 18+),
    falls back to JSON round-trip for older runtimes.

Out of scope (deferred to future PRs in plan's named order):

  - Phase 3: backend handlers emit AgentEvents natively.
  - Phase 4: heartbeat / dream log rendering moves to core.
  - Phase 5.x: per-backend ToolRegistry renderer (Codex TOML, etc).
  - Phase 6.x: migrate the six existing JSON stores onto JsonStore.

Refs docs/talon-architecture-unification-plan.md

* feat(agent-runtime): AgentEvent β†’ legacy callbacks bridge β€” Phase 3 plumbing

Adds the missing "render events back into the old shape" piece per
the plan's Phase 3 guidance:

> Backend handlers should emit events. Existing UI/log code can
> temporarily render events back into the old shape.

The adapter (Phase 1) goes one direction: `QueryResult` β†’
`AgentEvent` stream. This module goes the other: `AgentEvent`
stream β†’ legacy `QueryParams` callback dispatch. Together they
make Phase 3.x backend rewrites strictly local β€” a single
backend handler can switch to native event emission without
forcing every downstream consumer to migrate in lockstep.

What lands

  src/core/agent-runtime/legacy-bridge.ts (~220 LOC)

    pipeEventsToCallbacks(stream, callbacks) β†’ AgentResult | undefined
      Drives the legacy onStreamDelta / onTextBlock / onToolUse
      callbacks from an event stream.

      Mapping:
        text_delta            β†’ onStreamDelta(accumulated, "text")
        reasoning             β†’ onStreamDelta(accumulated, "thinking")
        assistant_message     β†’ onTextBlock(text)         (awaited)
        tool_call             β†’ onToolUse(name, input)
        completed             β†’ returns AgentResult       (no callback)
        error                 β†’ throws BridgedAgentError

      run_started / tool_result / usage / model_swapped / warning
      are observed silently β€” the legacy callback shape has no hook
      for them and bridging shouldn't invent new ones.

    reduceEventsToResult(stream) β†’ AgentResult
      QueryResult-shape fallback for backends that emit events
      natively but still need to satisfy backend.query()'s
      Promise<QueryResult> contract during the migration window.

    BridgedAgentError extends Error
      Carries the original AgentError's kind + retryable + raw so
      the dispatcher's error-classification path keeps working
      without re-classifying.

Tests

  agent-runtime-legacy-bridge.test.ts (16 cases)

    Streaming      text_delta + reasoning accumulation /
                   assistant_message β†’ onTextBlock + fold /
                   tool_call β†’ onToolUse with input record /
                   non-plain-object input β†’ {} guard
    Terminators    completed β†’ returns AgentResult /
                   error β†’ throws BridgedAgentError with kind /
                   no-terminator β†’ returns undefined
    Silent events  tool_result + usage + model_swapped + warning
                   trigger no callbacks
    Empty cb       missing callbacks are no-ops, not exceptions
    Awaiting       onTextBlock awaited in order before next event
    reduceEventsToResult β€” completed verbatim / synthesised on
                   no-terminator path / error throws / folds
                   assistant_message into text

  Full suite 2992/3004 (12 pre-existing skips). 16 net new tests.
  Typecheck clean. Prettier clean. No new lint warnings.

Notes

  - text accumulator IS shared between text_delta and
    assistant_message β€” folding the block into the running total
    keeps subsequent deltas monotonically growing. The legacy
    contract assumes onStreamDelta's `accumulated` never shrinks.
  - thinking accumulator is independent β€” text + thinking are two
    distinct phases on the legacy side too.
  - BridgedAgentError thrown synchronously from the for-await loop
    propagates naturally to the caller's await β€” same shape as the
    legacy backend.query() throwing.

Refs docs/talon-architecture-unification-plan.md

* docs(agent-runtime): module README + migration cookbook

Future-instance-friendly summary of every module under
src/core/agent-runtime/ + step-by-step migration recipes for:

  - resolveActiveModelForChat β†’ ref (Phase 2.x continuation)
  - backend handler β†’ AgentEvent emission (Phase 3.x)
  - hand-rolled JSON store β†’ JsonStore<T> (Phase 6.x)
  - backend MCP config β†’ ToolRegistry render (Phase 5.x)

Plus the named ordering each phase should follow:

  Phase 3: Codex β†’ Claude SDK β†’ OpenAI Agents β†’ Kilo / OpenCode
  Phase 6: Codex OAuth incompat β†’ media index β†’ cron β†’ triggers β†’
           sessions β†’ chat settings (last)
  Phase 5: Codex TOML + Claude SDK MCP first

And invariants the next instance should not silently break:

  - BACKEND_IDS literal is the union's source of truth
  - AgentEvent.type is the ONLY discrimination mechanism
  - Adapter yields minimal events; Phase 3.x backends emit richer
  - Contract assertions throw ContractViolation with descriptive
    messages (tests assert message shape)

No code changes. Pure docs.

* refactor(config): wire backend zod enums to BACKEND_IDS literal

`util/config.ts` previously repeated the same five-element backend
enum FOUR times inline:

  backend:        z.enum(["claude", "opencode", "kilo", "codex", "openai-agents"])
  heartbeatBackend: z.enum(["claude", "opencode", "kilo", "codex", "openai-agents"])
  dreamBackend:   z.enum(["claude", "opencode", "kilo", "codex", "openai-agents"])
  enabledBackends: z.array(z.enum(["claude", "opencode", "kilo", "codex", "openai-agents"]))

`src/core/agent-runtime/model-ref.ts` already exports
`BACKEND_IDS as const` as the source of truth for the typed
`BackendId` union. Phase 1's design note flagged the manual
mirroring as a footgun ("update both sides together until the enum
is migrated to import this constant"). This migrates it.

Mechanics

  - Spread `BACKEND_IDS` into a fresh non-readonly tuple
    (`BACKEND_ID_ENUM`) at the top of `config.ts`. zod's
    `z.enum` wants `[string, ...string[]]` and the spread+cast
    satisfies it without losing the literal types.
  - Every backend enum site now reads `z.enum(BACKEND_ID_ENUM)`.

Result

  - Adding a backend = update `BACKEND_IDS` (one line). The config
    schema picks up the change automatically.
  - Removing a backend = update `BACKEND_IDS`. Any chat-settings
    backendId that drifted off the union surfaces at config-load
    time via zod, not at runtime via mystery routing errors.

model-ref.ts comment dropped the "Phase 1 keeps both in lockstep
manually" hedge β€” that's no longer the contract.

No behaviour change. Same enum values, same validation, same
defaults. Full suite 2992 / 3004 (12 pre-existing skips).
Typecheck clean. Prettier clean. No new lint warnings.

* feat(agent-runtime): AgentEventLogRenderer β€” Phase 4 prep

Shared markdown-log renderer for heartbeat / dream / trigger
runs. Consumes an AgentEvent stream, produces structured markdown
fragments, calls a caller-supplied sink for each.

Phase 4 of the plan moves heartbeat / dream log rendering into
core β€” both currently mix SDK-specific event handling with
appendLog calls (~150 LOC each, slightly different per backend).
This module is the shared renderer; the per-backend
runOneShotAgent logic becomes thin enough to fit on a screen
once events replace the legacy callback shape.

What lands

  src/core/agent-runtime/event-log-renderer.ts (~280 LOC)

    renderEvent(event, state) β†’ { fragment, state }
      Pure function. Maps one AgentEvent to its markdown
      fragment + new RenderState. Tool calls are buffered by id
      so tool_result events can match against the issuing call's
      header.

    streamLog(stream, sink) β†’ Promise<AgentResult | undefined>
      Drains the stream, calls sink(fragment) per produced
      markdown chunk. Returns the AgentResult from the completed
      terminator; throws LogRendererError on error terminator.

    freshRenderState() β€” empty seed state
    RenderState β€” Map<id,header> + pendingTextDelta + finished flag
    LogSink β€” async-aware (await sink(fragment))
    LogRendererError β€” wraps AgentError; agentError stays
                       accessible for forensic logs

Output shape (one block per event type)

    run_started        β†’  ` β–Ά Run started`
    text_delta         β†’  buffered until assistant_message / tool_call
                          / terminator (avoids per-token churn)
    assistant_message  β†’  fenced markdown block; flushes pending
    reasoning          β†’  collapsed <details> block (signature in summary)
    tool_call          β†’  `### Tool call: <name>` + ```json input```
    tool_result        β†’  `**Tool result** (name)` under matching call,
                          OR standalone `### Tool result: name` when
                          orphaned. Error path emits `Error: ...` line
                          instead of JSON block.
    usage              β†’  ` β–Έ Usage: in=… out=… cacheR=… cacheW=… model=…`
    model_swapped      β†’  ` ⚠ Model swapped: a β†’ b (reason)`
    warning            β†’  ` ⚠ message`
    error              β†’  `### Error (<kind>, retryable=<bool>)` +
                          message + raw stack ```code block```
    completed          β†’  ` βœ“ Completed in <ms>ms (final: <usage>)`

Tests

  agent-runtime-event-log-renderer.test.ts (19 cases)

    renderEvent β€” 14 per-event-shape pins:
      - run_started single line
      - text_delta buffered, no fragment
      - assistant_message flushes pending + emits block
      - reasoning <details>+signature
      - tool_call header + JSON + id memoised
      - tool_result matched / orphaned / error variants
      - usage / model_swapped / warning one-liners
      - error block with kind + retryable + optional stack
      - completed with final usage line

    streamLog β€” 5 stream-level pins:
      - buffers text_delta, flushes on completed
      - flushes pending text before tool_call section, resumes
        buffering after (interleaving order pinned)
      - throws LogRendererError on error, flushes markdown first
      - returns undefined + defensive-flushes orphaned text on
        no-terminator stream end
      - interleaves tool_call+tool_result blocks in order across
        multiple calls

  Full suite 3011/3023 (12 pre-existing skips).
  Typecheck clean. Prettier clean. No new lint warnings.

Notes

  - text_delta deliberately buffered to avoid one-line-per-token
    churn in the markdown log. Phase 4 wiring can configure the
    flush boundary if needed.
  - renderEvent is pure; cloneState in / clone state out. Callers
    can replay events for debugging without side effects.
  - LogSink is async-aware so the heartbeat appendLog (which
    writes to disk via fs.appendFile) can be awaited in line
    without a race against subsequent sink calls.
  - Phase 1-2 contract holds: no production caller invokes the
    renderer yet. heartbeat.ts + dream.ts still own their inline
    markdown rendering until Phase 4.x lands.

Refs docs/talon-architecture-unification-plan.md

* refactor(codex): migrate oauth-incompat store to JsonStore β€” Phase 6.x #1

First real consumer of `core/agent-runtime/store.ts`'s
`JsonStore<T>`. Migrates the Codex OAuth-incompat learning store
(the plan's named first Phase 6 target β€” small, low-risk,
operationally well-understood) from a hand-rolled persistence
loop to the shared abstraction.

What changes

  src/backend/codex/oauth-incompat.ts (~280 β†’ ~310 LOC)

  Hand-rolled persistence DROPS:
    - existsSync / readFileSync / mkdirSync / writeFileAtomic.sync
    - inline JSON.parse + version + fingerprint + shape validation
    - separate `persist()` helper with try/catch + log

  JsonStore-based persistence ADDS:
    - one `makeJsonStore()` factory
    - `validate(raw)` hook for shape + non-string-id filtering
    - `migrate(raw, fromVersion)` hook accepting the legacy bare
      document shape `{ version, fingerprint, updatedAt, models }`
      so existing on-disk state survives the upgrade
    - `JsonStoreFs` injection point on `loadOAuthIncompatStore`
      and `OAuthIncompatStoreOptions` (Phase 6.x test pattern)

  On-disk format change:
    before:  { version, fingerprint, updatedAt, models }
    after:   { schemaVersion, savedAt,
              data: { fingerprint, updatedAt, models } }

  The `migrate` hook handles the legacy β†’ envelope upgrade on
  first load; subsequent saves write the new shape. Test added to
  pin the legacy migration path explicitly.

API change

  - `markOAuthIncompat(id)` is now async (returns
    `Promise<boolean>`). The in-memory mutation is still
    synchronous so subsequent `isKnownOAuthIncompat` calls see
    the update immediately; the awaited promise covers the disk
    write.

  - `loadOAuthIncompatStore(fingerprint, options?)` is now async.

  - `isKnownOAuthIncompat` / `listKnownOAuthIncompat` /
    `computeAuthFingerprint` / `resetOAuthIncompatForTests` stay
    synchronous β€” they only touch the in-memory set.

  - New `OAuthIncompatStoreOptions { fs?: JsonStoreFs }` lets
    tests inject a fake filesystem (mirrors the Phase 6 plan).

Callers updated

  src/backend/codex/handler.ts: `await markOAuthIncompat(...)`
  src/backend/codex/one-shot.ts: `await markOAuthIncompat(...)`
  src/backend/codex/init.ts:    fire-and-forget the loader with a
                                .catch() β€” `initCodexAgent` stays
                                sync, the loader is best-effort.

Race avoidance: `loadOAuthIncompatStore` is now idempotent on the
same fingerprint AND atomic on cutover. The previous sync version
clobbered memoryStore synchronously at function entry; the async
version would have created a window where the old store's data
disappeared before the new one's load completed, breaking tests
that pre-populate the store and then call initCodexAgent (which
fire-and-forgets a reload). Fix: keep the existing memoryStore
intact until the load completes, then swap atomically. Identical
fingerprint short-circuits to a no-op (already loaded).

Tests

  codex-oauth-incompat.test.ts: 25 β†’ 26 cases
    - every existing `loadOAuthIncompatStore` / `markOAuthIncompat`
      site updated to `await`
    - new "migrates legacy bare-document format to the new
      envelope shape" pin

  codex-handler.test.ts: same 43 cases pass β€” pre-emptive-swap
                         test rewired to `await` the loaders.
  codex-one-shot.test.ts: same 15 cases β€” runtime-learned swap
                          test re-awaits the post-init loader so
                          the in-memory state is settled before
                          marking.

  Full suite 3012 / 3024 (12 pre-existing skips). Typecheck clean.
  Prettier clean. No new lint warnings.

Phase 6 progress

  Plan's named ordering: Codex OAuth incompat β†’ media index β†’
  cron β†’ triggers β†’ sessions β†’ chat settings. This PR is #1.

  Pattern established for the remaining five:
    1. Define the persisted shape interface
    2. `new JsonStore<Shape>({ path, defaultValue, schemaVersion,
        validate, migrate })`
    3. Wrap mutations via `store.update(fn)`
    4. Make existing sync APIs async if the hot path can tolerate
       it (oauth-incompat: yes; chat-settings later: needs more
       thought because writes are very frequent)
    5. Add a `migrate` hook accepting the pre-JsonStore on-disk
       shape so existing prod state survives

Refs docs/talon-architecture-unification-plan.md

* chore: prettier fix for agent-runtime README

---------

Co-authored-by: claudiusthebot <noreply@anthropic.com>
Co-authored-by: claudiusthebot <claudiusthebot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants