Skip to content

fix(opencode): keep the session model on injected prompts - #38959

Open
iceteaSA wants to merge 1 commit into
anomalyco:devfrom
iceteaSA:injected-message-model
Open

fix(opencode): keep the session model on injected prompts#38959
iceteaSA wants to merge 1 commit into
anomalyco:devfrom
iceteaSA:injected-message-model

Conversation

@iceteaSA

@iceteaSA iceteaSA commented Jul 26, 2026

Copy link
Copy Markdown

Issue for this PR

Closes #38770

Also relevant to #28735 and #23369, which describe the same symptom through paths this does not fully cover.

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

A background subagent finishing can silently switch the parent session's model to the agent's configured default. Since Anthropic prompt caching keys on the model, the next turn misses the cache prefix and re-uploads the entire context — on a long session that is the expensive kind of silent. #38770 reports being billed on both models sequentially, which is the same thing seen from the invoice side.

createUserMessage resolves an injected prompt's model as:

const model = input.model ?? ag.model ?? (yield* currentModel(input.sessionID))

So any injected prompt that omits model lands on ag.model — the configured default — regardless of what the session was actually using. Two call sites on dev do exactly that:

TaskTool.injectBackgroundResult (packages/opencode/src/tool/task.ts) passes agent and variant but no model. This is #38770: the completion notification for a background child re-prompts the parent, and the parent's model changes underneath them. It now reads the parent's latest user message and forwards that model explicitly, with the existing variant retained as a fallback.

That also explains the detail in #38770 that looks strange at first — the agent kept reporting the old model as its identity. The system prompt was assembled before the switch; nothing re-runs it when an injected message changes the resolved model, so the prompt and the billed model disagree until the next assembly.

The shell-execution message (packages/opencode/src/session/prompt.ts) carries provider and model but drops the variant, so a session running xhigh quietly falls back to default reasoning effort. The variant is now preserved when the resolved model carries one. Separate bug, same shape, found while fixing the first.

Both fixes only supply what was already implied — neither changes agent resolution, so a genuine agent switch still adopts the new agent's model as before.

I found this from the other end: a live session moved claude-fable-5claude-opus-5 at a user message with no text (the injected notification), timestamp-matched to its child session completing, landing on exactly that agent's configured default.

How did you verify your code works?

Red-first, one test per site, each failing for its own distinct reason against unmodified dev:

task injection:  19 pass, 1 fail — expected parent model, received undefined
shell execution: 56 pass, 1 skip, 1 fail — expected variant "xhigh", received none

Mutation-checked separately, so neither fix is riding on the other's coverage:

task.ts reverted    → RED 19 pass / 1 fail   restored → GREEN 20 pass / 0 fail
prompt.ts reverted  → RED 56 pass / 1 fail   restored → GREEN 57 pass / 0 fail

Full suite in packages/opencode: 3208 pass / 0 fail. bun typecheck exit 0.

Note on scope: #35195 fixes this class more broadly at the shared resolution point, by consulting the durable session row before the agent default — these two sites would be covered by it. This PR fixes the call sites directly, which is independently useful and does not conflict with that approach.

Screenshots / recordings

Not a UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@iceteaSA

iceteaSA commented Aug 5, 2026

Copy link
Copy Markdown
Author

Adding field measurements and a scope note, since this has been sitting a while and I've since learned the fix here is narrower than the bug.

Measured exposure. On one long-running box (build agent pinned to anthropic/claude-opus-5 in config), from the local session store:

  • 75 of 89 build sessions (84.3%) have a session-row model that differs from the agent's configured model.
  • 59,289 of 69,382 build-agent assistant turns (85.5%) ran on something other than the configured model.

So for a config that pins an agent model, the resolution order input.model ?? ag.model ?? currentModel(...) picks the wrong model roughly six times out of seven whenever an injected prompt omits model. currentModel() itself is fine — it reads the session row — it just never gets consulted, because ag.model preempts it. That ordering is the whole bug, and it only bites when an agent config sets a model (which is why it's easy to miss in a default setup).

Scope note against this PR. What I did here fixes the caller: task.ts now resolves the model itself and passes it explicitly. That closes #38770's reported path, but it leaves the resolution order untouched, so any other injector that omits model still lands on ag.model. In current dev that includes session/compaction.ts, and it will include any future injector, since omitting model looks like the natural thing to do.

I've since been running a resolver-side variant instead, which changes createUserMessage to prefer the durable session row ahead of ag.model:

const switched = !!current.agent && ag.name !== current.agent
const sessionModel = !switched && current.model ? { /* from session row */ } : undefined
const model = input.model ?? sessionModel ?? prev?.info.model ?? ag.model ?? (yield* currentModel(input.sessionID))

The switched guard is the load-bearing part: a genuine agent switch must still adopt the new agent's configured model, while an injected prompt for the current agent keeps the session model. It also carries the variant through, which is the same thing this PR's prompt.ts hunk is reaching for.

That approach covers every injector at once rather than one call site at a time. I'm not pushing it into this PR unprompted — it's a broader change and the ordering question is really a maintainer call. Happy to do either:

  1. keep this PR as the narrow Background subagent notification silently reverts manually-selected model to config default #38770 fix, and open a separate one for the resolution order; or
  2. replace this PR's contents with the resolver fix and let it close the class.

Say which you'd prefer and I'll turn it around. I have the resolver version already running with tests, though they're written against a diverged local tree, so I'd rewrite them against dev's harness before submitting.

One caveat on my numbers, stated plainly: they come from a single heavily-used installation, so treat them as an existence proof for the exposure rather than a population estimate. Anyone whose agents don't pin a model won't see this at all.

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.

Background subagent notification silently reverts manually-selected model to config default

1 participant