Skip to content

feat(agent): bind the model to the conversation, not the process - #284

Open
arelchan wants to merge 8 commits into
mainfrom
feat/session_scoped_model
Open

feat(agent): bind the model to the conversation, not the process#284
arelchan wants to merge 8 commits into
mainfrom
feat/session_scoped_model

Conversation

@arelchan

@arelchan arelchan commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Stacked on #282 -- this branch contains that PR's commits plus one. Review #282 first; the diff here is the last commit, 6882916.

The model was two attributes on a process-wide AgentLoop, so there was one answer for everyone. Two sessions could not run different models, a switch in one moved all of them, and the "default" was whatever the last switch happened to write to agents.defaults. This makes the model a property of the conversation.

A ModelBinding is a model id and the credential that serves it, as one value -- the pairing that everything below depends on. ProviderPool is the single place that decides which credential a model id pairs with, cached per (vendor, model); constructing a binding from an already-resolved pair happens in several places. run_turn resolves the binding for the turn's session and holds it in a ContextVar for the whole turn tree; the loop's provider/model, the context engine's LLM-backed segments, the skill gate and rewriter, and the consolidator all read that instead of a reference of their own.

The rules, and what each cost

Rule How it lands
Different sessions, different models A dict of overrides, read once at turn entry
A switch moves only the session that asked Nothing else reads that session's entry
A new session starts on the configured default A session-scoped switch does not write agents.defaults, so nothing accumulates
A configured subsystem uses its own model+credentials; otherwise it follows the conversation The factory resolves each pin through the pool, so a holder has either a complete pair or nothing
A switch mid-turn takes effect next turn Free -- the turn holds the binding it entered on

The last one is worth spelling out: it needs no parking and no counter, and this branch deletes the ones #282 added -- _turns_in_flight, _pending_provider and the adopt-on-entry hook are gone, so #282's "In-flight work" rationale describes a mechanism that no longer exists once both land. The binding is captured before the turn's first read, so a switch landing mid-turn is simply not visible to that turn. The client-side refusal that used to pre-empt it (interrupt the current turn before trying to change models) is gone, and with it ModelSwitchInTurnError across the Python errors module, the TypeScript client, the OpenRPC schema and the code-table test -- nothing raises it, and leaving it would tell the next reader the server still can.

Detached work inherits the context copy asyncio makes at task creation, so a spawned subagent finishes on the model it was spawned under. spawn also passes the pair explicitly, because a subagent outlives its turn and that is worth being able to read in the code.

User-facing

config.set model takes a scope: session (what the picker sends) or default. /model <name> --default is the counterpart in the TUI, since a session-scoped switch alone would leave no way to change the persisted default at all. A session's choice is stored on its own record and restored on resume, so it outlives the process without becoming everyone's default. The picker and the session info bundle (session.create / session.resume) now report the session's own model -- reading agents.defaults there showed two different models for one conversation.

Config defaults

No subsystem ships a vendor default any more. context.curator_model, token_wise.tool_result_lifecycle.summary_model and skill_forge.detect_model all hardcoded the same Gemini id, and token_wise.smart_routing.tiers shipped six models across three vendors -- for users who may hold no key for any of them. All four are unset now, which is what "not configured" has to mean for the subsystem rule to be expressible. Only curator_model has readers today; the other three are dead config, emptied for consistency rather than for effect. The media tools keep their defaults, which live in the tool code rather than the schema: they are capability-bound, and no conversation model generates images or speech.

A subsystem pin is a pair too

context.curator_model and skill_forge.llm_gate_model took a model id and nothing else, so the pool had to guess which credential served it -- the same shape as the mis-pairing this whole line of work is about, one layer up. The guess is unanswerable once a gateway is configured: openrouter serving anthropic/claude-haiku-4-5 and anthropic serving claude-haiku-4-5 are both valid, name different credentials and different bills, and the id does not distinguish them. Guessing "the configured gateway serves everything" then handed the gateway an id it has no route for, and the 404 was swallowed by the subsystem's own fallback -- a pin that looked configured and never ran.

Each pin now takes a provider alongside the model (curator_provider, llm_gate_provider). Set, nothing is derived. Unset, the vendor is still derived from the id, which is what every existing config gets and what keeps them working. Either way a pin that cannot be paired is logged and dropped rather than silently borrowing the conversation's key. bind_pin's guard is also broadened from the credential exceptions to anything: it runs in the context-engine factory at construction and building a provider imports a vendor module, so a misconfigured pin could stop the agent from starting -- the opposite of what its docstring promised.

Release note. With context.curator_model unset, the Curator's slow path now runs on the conversation's model instead of failing on a Gemini id nobody had a key for and dropping to the deterministic plan. That is the rule working as asked, but on a long conversation it is up to 12 tool-calling requests per turn of context housekeeping that previously cost nothing. Set context.curator_model to a small model, and configure that vendor's key, to keep it cheap.

Type

  • Feature

Verification

uv run pytest tests/ -q
5362 passed, 1 failed (tests/test_cli_theme.py::test_bold_accent_renders_styled_not_bare)

uv run ruff check raven/ tests/     # All checks passed
cd ui-tui && npm test               # 973 passed (85 files)
cd ui-tui && npx tsc --noEmit       # clean
cd ui-tui && npm run lint:rpc       # generated.ts in sync
cd ui-tui && npm run lint           # 0 errors (22 pre-existing warnings, untouched files)

tests/test_default_context_engine.py::TestTwoTrackConcurrency::test_skill_and_memory_run_concurrently also fails in some full-suite runs -- a load-dependent timing assertion that flakes on the unmodified branch too, and passes alone and with its own file. That one failure fails identically on an unmodified main at 53aeb0c when the whole file runs, and passes when the single test runs alone -- machine-dependent order pollution in that file, and CI is green on it. Same note as #282.

This change was reviewed by seven independent agents across two rounds, each in its own worktree, which is where most of its shape came from. The second round found the persistence half-built -- the write to the session record had no reader, so a switch died with the process while the code and the description both said otherwise -- and the provider pool handed a config snapshot at all three construction sites, so the freshness it documents never fired and a key added through the picker then selected in the same session failed where it used to work. It also found the picker overriding its selection for every session (session_model falls back to the default and so never answers None), the gateway pin escaping the factory's guard, a deleted session leaving its override behind, a fork dropping its parent's model, and -- on the TUI side -- two existing tests that would have gone red in CI on the new scope key, plus a default-scoped switch painting the new default into the status bar while the session kept its own model. All fixed here.

The first round found two features that existed on the server and were unreachable from the product: the client still refused a mid-turn switch, and a session-scoped /model had no counterpart for changing the persisted default.

A third round, run against the pushed branch, found three defects the earlier rounds had missed and three claims this description got wrong. The defects: config.set treated an explicit scope="session" carrying no session id as a request to change the default, so it wrote agents.defaults.model and moved every session that never switched -- reachable from the TUI, whose session_id is null until the first session.create resolves; the fork fix above re-pointed the live binding but never wrote the child's record, so a branched session kept its parent's model until the first restart and then silently dropped to the default; and /model <name> --default refused to repaint the status bar on the theory that a default-scoped switch cannot move the asking session, which is true only of sessions that already chose their own model -- for a fresh conversation, the common case for that command, the switch moved it and the bar then disagreed for the life of the session. The scope of a switch is now the server's answer (applies_to_session) rather than something the client infers.

The wrong claims were all in the mutation table, and the same class of error as the round-two finding above: three rows asserted a test that no longer discriminated. _set_model ignores scope and model.options loses its loop factory both left the suite green -- the conjunct that routes --default and the production registration that makes the picker session-aware were each exercised only by callers that could not tell the difference; both now have tests, verified red. The spawn row was true at #282's head and false at this one: asyncio.create_task copies the context, so moving the read out of spawn returns the same binding and no test can separate them. That row is dropped rather than reworded -- the explicit snapshot stays for readability, which is what the prose above already claimed for it. ProviderPool.default had no production caller at all: what a new session starts on is AgentLoop._default_binding, built in the constructor, so the method and the two table rows protecting it are deleted.

Mutation-tested rather than assumed. Each of these turns a test red:

Mutation
binding_for_session ignores the session dict 7 failed
set_session_binding -> pass 7 failed
run_turn drops use_binding 5 failed
run_turn keys on a hardcoded session 5 failed
run_turn drops the channel:chat_id key fallback 1 failed
set_default_binding stops fanning out 2 failed
use_binding never resets its token 7 failed
resolve ignores the pin 2 failed
spawn passes its fallback instead of the active binding 1 failed
_set_model ignores scope 2 failed
a session scope with no session id is widened to the default 1 failed
applies_to_session always claims the asking session moved 1 failed
ProviderPool.bind drops the cache 1 failed
bind_pin skips the credentials check 1 failed
bind_pin loses the gateway branch 1 failed
the pool holds a config snapshot 1 failed
the gate sends its raw pin out of turn 1 failed
the session info bundle drops the per-session model 1 failed
session.resume stops restoring the stored model 1 failed
_remember_session_model -> pass 1 failed
model.options ignores the session 1 failed
model.options loses its loop factory 1 failed
_build_binding never uses the pool 1 failed
scope validation removed 1 failed
the factory stops building the skill-gate pin 1 failed
bind_pin ignores the configured provider and guesses 4 failed
the factory drops the curator's configured provider 1 failed
the factory drops the gate's configured provider 1 failed
fork stops carrying the parent's model record 2 failed
session.branch stops handing the child the live binding 1 failed
session.delete stops releasing the binding 1 failed
the client guesses from the scope instead of asking the server (TS) 1 failed
  • Relevant tests pass locally
  • Relevant lint / type checks pass locally
  • User-facing docs or screenshots are updated when needed

Risk

  • Security impact considered
  • Backward compatibility considered
  • Rollback path is clear for risky changes

Existing config files load unchanged and still pin: a file containing curatorModel (or the snake_case spelling) keeps its value, and smartRouting.tiers round-trips verbatim. The new curatorProvider / llmGateProvider fields are additive and optional -- unset reproduces the previous derivation exactly. The scope key on config.set is additive and the response's new scope / session_id / applies_to_session keys are ignored by older clients. AgentLoop.provider / model became read-only properties, so anything assigning them must move to set_provider or a binding -- a deliberate break, since assigning one half of a pair is the bug this whole line of work started from. Rollback is a revert.

CONTEXT.md defines the new vocabulary (model binding, session binding, default binding, provider pool, subsystem pin) and ui-tui/CONTEXT.md defines model scope for the TUI, per AGENTS.md 6. config.set's params and result are declared in the OpenRPC schema and the Pydantic models -- both omitted session_id / provider before this, so the schema-match guardrail could not see the drift; generated.ts is regenerated.

Related Issues

N/A

arelchan and others added 4 commits August 7, 2026 22:24
config.set key="model" built a fresh provider and then assigned
loop.provider and loop.model. The loop is not the only holder: AgentLoop
hands the provider it was built with to the subagent manager, to the
context engine's LLM-backed segments (skill rewriter, skill gate, curator
and its history trimmer) and to the memory consolidator, and each keeps
its own reference. A switch that stopped at the loop left all of them
calling the provider built at process start for the rest of the run.

What that looks like in practice: switching away from an unusable
credential fixes the main loop, while subagent spawns and the skill
rewriter/gate keep failing to authenticate against the abandoned
endpoint. The auth error is classified non-retryable, so each one fails
on the first attempt and is swallowed by its caller's fallback, which is
why this stayed invisible apart from a warning line.

AgentLoop.set_provider now fans the new provider out to every holder, and
the RPC handler calls it instead of assigning the two attributes. The
context engine walks its builders and forwards to the ones implementing
set_provider, so a purely textual segment needs no override. A pinned
gate model and an explicit config.curator_model survive the switch; both
follow the agent's model only when they were already following it.
In-flight turns and subagents keep the provider they started with, so no
single conversation spans two endpoints.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Review of #282 found the fan-out landed but its promise did not. Both
docstrings claimed a running turn or subagent keeps the provider it
started with; nothing provided that. Every LLM call site reads the
provider off self at call time, so before this the subagent manager's
reference simply never changed -- the fan-out is what made a running
subagent able to span two vendors, and that was documented as a
deliberate non-change.

Two mechanisms, because the two lifetimes differ. AgentLoop parks a
switch that arrives mid-turn and adopts it at the next run_turn entry:
one boundary covers the dozen self.provider reads plus the context
engine and consolidator underneath them, where a snapshot would have to
be threaded through each. A subagent is a detached task that outlives
its turn, so the park cannot reach it; _run_subagent_inner reads the
provider and model once before its iteration loop instead.

Also from the review:

- curator: drop the branch on config.curator_model. It is declared str
  with a non-empty default, so it is never falsy and the branch never
  ran; curator_model is always a pin, at construction too.
- gate: stop describing a kept pin as safe. A pin is only a model id
  while the credential comes from the provider, so a pin naming a vendor
  the provider does not serve was already broken at boot. Fixing that
  pairing is a separate change.
- context_engine.base: the concrete no-op exists because AgentLoop calls
  through the ABC unconditionally, not because an engine without
  LLM-backed segments exists. There is only one implementation.
- main.py: the fan-out comment pointed the wrong way. All four receivers
  are above it, and the list below it names the one attribute not in the
  fan-out.

Tests: the previous file only exercised the dispatcher, so replacing any
receiver with pass left it green. It now builds a real AgentLoop and
asserts the gate, rewriter, curator, curator assembler, trimmer,
subagent manager and consolidator all moved; guards the attribute names
the fan-out walks against a rename; and drives the real
_run_subagent_inner across a switch. Each of those five mutations now
fails something.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Panel review of the two commits above found the park did nothing in the
one configuration it exists for. OriginPools gates USER and system
origins on independent semaphores with no global cap
(spine/scheduler.py), and the TUI defaults to one slot each, so a user
turn and a cron turn run concurrently on one AgentLoop. With a bool:
the shorter turn's finally cleared the flag under the longer one, and a
correctly parked switch was adopted by an unrelated turn entering
run_turn. Both land the switch mid-flight, which is what the park exists
to prevent. Now a depth counter, with both ends gated on zero, and the
last turn out adopts so a park cannot outlive the turns it waited on.

The subagent snapshot moved from _run_subagent_inner to spawn. A spawn
queues behind the concurrency gate and a sandbox boot before the inner
method runs, and a switch landing in that window handed the task an
endpoint the user chose after asking for it -- so "only spawns started
after this call are affected" was not true of the window that matters.

Three prose corrections, all cases of describing a property the code
does not have:

- "LiteLLM drops the shapes the new vendor rejects instead of failing"
  named the wrong mechanism. drop_params filters request kwargs, not
  message content. The silence comes from the provider turning a
  rejected request into finish_reason="error" content.
- "curator_model is always a pin, at construction either" was false for
  an explicitly empty context.curator_model, which the constructor's own
  `or model` still follows. set_provider now re-derives with the
  constructor's expression instead of asserting.
- "a dozen call sites" was eight.

The park's relationship to the RPC guard is now stated: is_turn_active
rejects a same-session switch first, the park covers what that cannot
see, and a parked switch is on disk while the loop still reports the old
model.

Tests: the run_turn wrapper had no coverage at all -- deleting its
finally left the suite green -- because the park test hand-set the flag
and hand-called the adopt. It now drives the real run_turn: adopt on
entry, slot released on return and on exception, and a second concurrent
turn that must not unpark a switch held for the first. Plus a spawn-time
snapshot test. Signature change to _run_subagent/_run_subagent_inner
updated in the two suites that stub them.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Review found two docstrings of the kind this PR was already rejected for
once. The park docstring said a mid-turn split "does not raise"; that
holds for the chat_with_retry sites, but _llm_call_stream -- the path a
TUI turn takes -- catches only TimeoutError, so there the rejection
propagates. And the context-engine ABC justified its concrete no-op by
the loop calling it unconditionally, which an abstract method would
satisfy equally; what concrete buys is not forcing a future
implementation to write an empty override.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
@arelchan
arelchan force-pushed the feat/session_scoped_model branch from 9648724 to e9176cd Compare August 8, 2026 15:02
arelchan and others added 2 commits August 9, 2026 12:13
The only mutation the review could not kill: moving the snapshot from
spawn into _run_subagent_inner left the suite green, which is exactly the
state the commit before it was written to fix. Neither existing test could
see it -- one stubbed _run_subagent wholesale, so it proved spawn passes a
pair but not when the pair is read; the other called _run_subagent_inner
directly, bypassing spawn, the concurrency gate and the sandbox boot, so
it proved the iteration loop does not re-read but not where the read
happens.

This drives the real _run_subagent with the gate held shut, switches the
provider while the task sits in that window, then releases it and asserts
which provider actually served the call.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
The model was two attributes on a process-wide loop, so there was one
answer for everyone: two sessions could not run different models, a
switch in one moved all of them, and the "default" was whatever the last
switch happened to write. This makes the model a property of the
conversation.

A ModelBinding is a model id and the credential that serves it, as one
value. ProviderPool is the single place a model id becomes such a pair,
caching per (vendor, model) because building one imports LiteLLM and
writes its vendor's key into the environment. run_turn resolves the
binding for the turn's session and holds it in a context var for the
whole turn tree; the loop's provider/model, the context engine's
LLM-backed segments, the skill gate and rewriter and the consolidator all
read that instead of a reference of their own.

What that buys, rule by rule:

- Different sessions on different models, and a switch that moves only
  the session that asked: a dict of overrides, read at turn entry.
- A new session on the configured default: a session-scoped switch does
  not write agents.defaults, so nothing accumulates. config.set model
  takes a scope, session (what the picker sends) or default, and
  /model <name> --default is the counterpart in the TUI. The session's
  choice is stored on its own record and restored on resume, so it
  outlives the process without becoming everyone's default.
- A subsystem with a model and credentials of its own uses them,
  otherwise it follows the conversation: the factory resolves each pin
  through the pool, so a holder has either a complete pair or nothing. A
  gateway binds a pin through itself, since it serves any id under its
  own key.
- A switch mid-turn landing on the next turn: free. The turn holds the
  binding it entered on, so a later switch is not visible to it. The
  client-side refusal that used to pre-empt this is gone, and so is
  ModelSwitchInTurnError across the Python errors module, the TypeScript
  client, the OpenRPC schema and the code-table test.

Detached work inherits the context copy asyncio makes at task creation,
so a spawned subagent finishes on the model it was spawned under; spawn
also passes the pair explicitly, because a subagent outlives its turn and
that is worth being able to read in the code.

The picker and session.info now report the session's own model rather
than agents.defaults, which otherwise showed two models for one
conversation.

No subsystem ships a vendor default any more. context.curator_model,
token_wise.tool_result_lifecycle.summary_model and skill_forge.detect_model
all hardcoded the same Gemini id and token_wise.smart_routing.tiers
shipped six models across three vendors, for users who may hold no key
for any of them. All four are unset now, which is what "not configured"
has to mean for the rule above to be expressible. Only curator_model has
readers today; the other three are dead config, emptied for consistency.
The media tools keep their defaults, in the tool code rather than the
schema: they are capability-bound, and no conversation model generates
images or speech.

Note for the release: with context.curator_model unset, the Curator's
slow path runs on the conversation's model instead of failing on a
Gemini id nobody had a key for and dropping to the deterministic plan.
That is the rule working as asked, but on a long conversation it is up
to 12 tool-calling requests per turn of context housekeeping that
previously cost nothing. Set context.curator_model to a small model, and
configure that vendor's key, to keep it cheap.

Review of this branch by four agents in isolated worktrees found the
persistence half-built (the write to the session record had no reader, so
a switch died with the process while the code said otherwise), the
provider pool handed a config snapshot at all three construction sites so
the freshness it documents never fired, and the picker overriding its
selection for every session because session_model falls back to the
default and so never answers None. All are fixed here, along with the
gateway pin escaping the factory's guard, a deleted session leaving its
override behind, and a fork dropping its parent's model.

On the TUI side two existing tests asserted the config.set params by
exact equality and would have gone red in CI on the new scope key; a
default-scoped switch painted the new default into the status bar while
the session kept its own model; and --default is now stripped in any
position and however many times, with /model --default alone opening the
picker instead of sending an empty model id.

The mutation pass left seven holes green, all the same shape: a helper
tested directly while the handler or registration calling it was not.
Each now fails a test when broken, including the window the spawn
snapshot exists for -- the concurrency gate and sandbox boot a spawn
waits through before its task starts running.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
@arelchan
arelchan force-pushed the feat/session_scoped_model branch from e9176cd to 6882916 Compare August 9, 2026 04:19
arelchan and others added 2 commits August 10, 2026 00:14
An explicit scope="session" carrying no session id fell through to the
default branch, so it wrote agents.defaults.model and moved every session
that never switched -- reachable from the TUI, whose session_id is null
until the first session.create resolves and after a failed one. It is
refused now rather than widened.

A fork re-pointed its parent's live binding but never copied the record,
so a branched session kept its model until the first restart and then
dropped to the default. SessionManager.fork carries model and provider,
which covers every caller rather than only the RPC handler.

/model <name> --default refused to repaint the status bar, on the theory
that a default-scoped switch cannot move the asking session. That holds
only for sessions which already chose their own model; a fresh
conversation reads the default and does move, and it is the common case
for that command. Whether it moved is now the server's answer
(applies_to_session) rather than something the client infers from the
scope, and an unapplied switch is reported as an error instead of being
drawn as a success.

Also closes the test gaps a review round found: the conjunct that routes
--default, the fork inheritance, the binding released on session.delete
and the production registration that makes the picker session-aware were
each removable without turning anything red. One stale assertion message
named a helper that no longer exists.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
context.curator_model and skill_forge.llm_gate_model took a model id and
nothing else, so the pool had to guess which credential served it -- the
same shape as the mis-pairing this line of work is about, one layer up.
The guess is unanswerable once a gateway is configured: openrouter
serving anthropic/claude-haiku-4-5 and anthropic serving
claude-haiku-4-5 are both valid, name different credentials and
different bills, and the id does not distinguish them. Guessing "the
configured gateway serves everything" then handed the gateway an id it
has no route for, and the 404 was swallowed by the subsystem's own
fallback -- a pin that looked configured and never ran.

Each pin now takes a provider alongside the model, curator_provider and
llm_gate_provider. Set, nothing is derived. Unset, the vendor is still
derived from the id, which is what every existing config gets and what
keeps them working. Either way a pin that cannot be paired is logged and
dropped rather than silently borrowing the conversation's key.

bind_pin's guard is broadened from the credential exceptions to
anything: it runs in the context-engine factory at construction and
building a provider imports a vendor module, so a misconfigured pin
could stop the agent from starting -- the opposite of what its docstring
promised.

ProviderPool.default is deleted along with the two tests that covered
it. What a new session starts on is AgentLoop._default_binding, built in
the constructor, so the method had no production caller and the tests
protected dead code.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
arelchan added a commit that referenced this pull request Aug 10, 2026
## Summary

A live `/model` switch rebuilt the provider but only reassigned
`loop.provider` / `loop.model`. `AgentLoop.__init__` had already handed
that provider to the subagent manager, the context engine's LLM-backed
segments and the memory consolidator, and each kept its own reference.
Switching away from a dead credential fixed the main loop while
subagents and the skill rewriter/gate went on authenticating against the
endpoint the user had just abandoned. Cron was a fourth victim: its runs
failed with the same 401 and its history rendered the failures as blank
rows.

`AgentLoop.set_provider` now fans the pair out to every holder it built,
and the context engine walks its builders duck-typed so a text-only
segment is skipped rather than raising.

### In-flight work

Every LLM call site reads the provider off `self` at call time, so an
unconditional swap relays one conversation across two vendors. How that
surfaces depends on the path: the `chat_with_retry` sites turn a
rejected request into `finish_reason="error"` content, so the turn
reports a failure with no sign that its endpoint moved, while
`_llm_call_stream` -- the path a TUI turn takes -- catches only
`TimeoutError` and lets the rejection propagate. Neither is a diagnosis
the user can act on.

Two mechanisms, because the two lifetimes differ. Both are superseded by
#284, which makes the
model a property of the conversation and gets the same guarantee from
the context copy that
`asyncio` makes at task creation -- if the two land together, the park
described here exists only
between the two merges.

- **The loop parks.** A switch arriving while any turn runs is held and
adopted at the next `run_turn` entry. One boundary covers eight
`self.provider` reads in `loop/main.py` plus the context engine and
consolidator underneath them; a snapshot would have to be threaded
through each. The park is a depth counter, not a flag: `OriginPools`
gates USER and system origins on independent semaphores with no global
cap, and the TUI defaults to one slot each, so a user turn and a cron
turn overlap on one loop. Both ends gate on zero, and the last turn out
adopts so a park cannot outlive the turns it waited on.
- **Subagents snapshot.** A spawn is a detached task that outlives the
turn, so the park cannot reach it. `spawn` captures the pair it was
asked for and passes it down; capturing later would miss the window
where a spawn waits on the concurrency gate and a sandbox boot.

This is the second line of defence, not the first.
`tui_rpc.methods.config` already rejects a switch outright when the
caller's own session has a turn in flight; the park covers what that
guard cannot see -- a caller that passes no `session_id`, and proactive
turns running in their own lanes. Note the RPC still answers `applied:
True` and the config file is already written, so a parked switch is
applied on disk while the loop reports the old model until the last turn
drains.

### Also here

- `curator_model` is re-derived on a switch with the constructor's own
expression, so the same config cannot mean one thing at build time and
another after. The default is non-empty, so in practice it is a pin; an
explicitly empty `context.curator_model` follows the agent model, and
now follows it in both places.
- The concrete no-op `set_provider` on the context-engine ABC is
concrete so a future implementation with no LLM-backed segment is not
forced to write an empty override. `ContextAssembler` is the only one
today and does override it.

### Scope

`AgentLoop` and the subsystems it builds. `HeartbeatService` and the
Sentinel stack take the same provider but are siblings on the gateway
side, which registers no tui_rpc methods, so `loop.set_provider` cannot
and does not reach them. Not reachable today; worth an issue if the two
sides ever converge. `MemoryConsolidator` is re-pointed but its detached
consolidation tasks are not snapshotted -- a single call rather than a
multi-turn conversation, so the split-conversation argument does not
apply, but it is the same shape.

## Type

- [x] Fix

## Verification

```
uv run pytest tests/ -q
5302 passed, 1 failed

uv run ruff check raven/ tests/     # All checks passed
uv run ruff format raven/ tests/    # unchanged
```

That failure is not this branch.
`tests/test_cli_theme.py::test_bold_accent_renders_styled_not_bare`
fails the same way on an unmodified `main` at `53aeb0c` when the whole
file runs (verified in a
detached worktree) and passes when the single test runs alone; it is a
`COLORTERM` artifact and CI
is green on it.
`tests/test_default_context_engine.py::TestTwoTrackConcurrency::test_skill_and_memory_run_concurrently`
also failed in some runs of this branch and of unrelated ones -- a
timing assertion that flakes
under full-suite load, passing alone and with its own file. A clean
re-run at this head has only the
theme failure.

A later review round found one of these mutations still surviving --
moving the spawn snapshot into
`_run_subagent_inner` -- because neither existing test could see the
window it exists for: one
stubbed `_run_subagent` wholesale (proving `spawn` passes a pair, not
when the pair is read) and the
other called `_run_subagent_inner` directly, bypassing `spawn`, the
concurrency gate and the sandbox
boot. There is now a test that holds the gate shut, switches the
provider while the task sits in
that window, releases it, and asserts which provider actually served the
call. The same round found
two docstrings scoped wider than the code: the mid-turn split does not
raise on the
`chat_with_retry` sites but does on `_llm_call_stream`, which is the
path a TUI turn takes; and the
context-engine ABC's concrete no-op was justified by a reason an
abstract method would satisfy
equally. Both corrected.

The tests here were rebuilt after a review found the previous set only
exercised the dispatcher -- replacing any receiver's `set_provider` with
`pass` left it green. They now build a real `AgentLoop` and assert the
gate, rewriter, curator, curator assembler, history trimmer, subagent
manager and consolidator all moved; guard the attribute names the
fan-out walks against a rename; and drive the real `run_turn` and the
real `_run_subagent_inner`. Verified by mutation -- each of these turns
something red:

| Mutation | Result |
|---|---|
| `SubagentManager.set_provider` -> `pass` | 3 failed |
| `CuratorSegmentBuilder.set_provider` -> `pass` | 1 failed |
| rename the `subagents` attribute the fan-out reaches | 2 failed |
| delete the `finally` that releases the turn slot | 3 failed |
| adopt on `run_turn` entry unconditionally | 1 failed |
| never park | 2 failed |
| spawn without the snapshot | 1 failed |
| curator `set_provider` drops the re-derive | 1 failed |
| move the spawn snapshot into `_run_subagent_inner` | 1 failed |

- [x] Relevant tests pass locally
- [x] Relevant lint / type checks pass locally
- [ ] User-facing docs or screenshots are updated when needed

## Risk

- [x] Security impact considered
- [x] Backward compatibility considered
- [x] Rollback path is clear for risky changes

`_run_subagent` / `_run_subagent_inner` take the provider and model as
parameters now; the two suites that stub them are updated. No public API
changes. Rollback is a revert -- the previous behaviour is the 401.

## Related Issues

N/A

---------

Co-authored-by: arelchan <204152633+arelchan@users.noreply.github.com>
Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.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.

1 participant