fix(session): recover from stale encrypted reasoning on provider rejection

[marwanvx]

Issue for this PR

Closes #48741

Type of change

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

What does this PR do?

When resuming sessions or continuing tool execution with Responses API models (e.g. Muse Spark / OpenAI models on Zen/Console gateways), requests fail when replaying expired or caller-mismatched reasoning state:

• reasoning 'encrypted_content' was not issued to this caller
• invalid_encrypted_content
• Referenced reasoning item '...' was not found or has expired

This handles the rejection transparently in the session runner:

1. Classifies these provider errors as stale-reasoning in @opencode/ai.
2. When rejected before output streaming begins, strips ephemeral crypto tokens (reasoningEncryptedContent and itemId) from assistant messages while preserving visible text.
3. Publishes SessionEvent.MessageContentUpdated on the bus so sanitized state is durable in EventTable and survives projection rebuilds (replaySessionProjection).
4. Retries the turn once with clean history.

Related: #48773, #48805

How did you verify your code works?

• bun test test/provider-error.test.ts in packages/ai (all passed)
• bun test test/session-runner.test.ts in packages/core (passes, including multi-turn & tool continuation recovery tests, and asserting state is undefined after replaySessionProjection)
• bun run typecheck passes with 0 errors
• Tested 15-turn live session with tool calls on muse-spark-1.3-contributor-free

Screenshots / recordings

N/A (session runner fix)

Checklist

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

[github-actions]

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.

[github-actions]

The following comment was made by an LLM, it may be inaccurate:

Potential Duplicate Found:

• PR fix(opencode): recover stale encrypted reasoning state #43595: fix(opencode): recover stale encrypted reasoning state — fix(opencode): recover stale encrypted reasoning state #43595

Why it's related: Both PRs address the same core issue of recovering from stale encrypted reasoning tokens that expire or become invalid when replayed. PR #43595 appears to be an earlier attempt at handling this problem, while the current PR #48908 implements a more comprehensive solution with reactive auto-recovery in the Session Runner, error classification, and extensive test coverage.

[holny]

Read through the sanitize→resend chain and it holds up on our side — to-llm-message.ts:168 lifts item.state into providerMetadata, and lowerReasoning (open-responses.ts:529) drops the part entirely once state is undefined, so the stale item really does leave the request.

One durability concern: RecoverStaleReasoning updates SessionMessageTable rows directly without publishing anything on the Bus. That table is a projection — replaySessionProjection (session-runner.test.ts:715) deletes the rows and re-folds everything from EventTable, so any later rebuild would resurrect the old state with reasoningEncryptedContent/itemId and the stale-reasoning error can come back after a replay. Would it make sense to go through the event path instead, the way normal assistant updates reach the projector (projector.ts:232)?

Related: the post-replay assertion only matches {type, text}, so it can't tell the state came back — worth asserting state is gone there.

Minor: the update loop rewrites every assistant message in the window, even untouched ones. Filtering to the messages sanitizeReasoning actually changed would keep it cheap.

[github-actions]

Thanks for updating your PR! It now meets our contributing guidelines. 👍

[marwanvx]

Good catch on the projection replay. Pushed an update:

• Routed updates through the bus with SessionEvent.MessageContentUpdated so changes fold into EventTable and survive projection rebuilds.
• Added expect(reasoning?.state).toBeUndefined() to the post-replay assertions so regressions there get caught.
• Filtered sanitizeReasoning to only emit events for messages that actually had tokens stripped.

[holny]

Checked the update — all three points land correctly. Routing through SessionEvent.MessageContentUpdated works because the projection replaces the whole content array (message-updater.ts:83-88), so a replay folds the sanitized version back in, and the post-replay assertions on state catch any regression there. One subtle thing I appreciated: the retry loop reuses the same in-memory loaded instance (initial ?? prepareContext in llm.ts), so the in-place sanitize means the retried request goes out clean immediately without depending on projection refresh timing. LGTM from my side.

[Mirrowel]

@Mirrobot-Agent Check this PR, is this a good fix?

[Mirrobot-Agent]

On it — tracing the full recovery chain before I call it: the new classification in @opencode/ai, the runner-side strip + republish, and what the durable projection actually does on replay. Back with a real review shortly.

[Mirrobot-Agent]

Verdict: commented — the fix is the right shape and well-tested; one design sign-off plus minor notes stand between it and approval.
cc @marwanvx

Overall Assessment

This is a careful, correctly-placed fix. It mirrors the existing overflow/continuation recovery — classify in @opencode/ai, recover in the runner only while nothing was streamed, retry once with clean history — and the durability claim survives tracing:

• The new classification is gated to client-scoped failures (packages/ai/src/provider-error.ts:130-143), so 5xx/transport noise cannot earn the label.
• bus.publish runs projectors inside the event transaction (packages/core/src/bus.ts:399-402), so by the time RecoverStaleReasoning finishes, SessionMessageTable already holds the sanitized content; the retried attempt reloads history from the DB (llm.ts:233 → SessionHistory.entriesForRunner), so the second request goes out clean by construction, not by timing luck.
• The wire effect is real: lowerReasoning (packages/ai/src/protocols/open-responses.ts:529) drops the reasoning part once state is gone, while visible text and every other part survive.
• The replay test is not cosmetic — replaySessionProjection deletes SessionMessageTable and re-folds EventTable, and the added state assertion proves the sanitized form survives a rebuild, which is exactly the durability concern raised in the thread.
• The event never reaches clients: event-feed.ts:49 forwards only isOpenCodeEvent, and session.message.content.updated is not in ServerDefinitions.

I could not execute the suite in my sandbox (bun is not available in this session), so the checks above are from tracing the code paths and from the construction of the added tests rather than a local run.

🟠 Major

• packages/core/src/session/runner/llm.ts:316 — this revives live publication of an event the repo retired in #48043 and marked replay-only; needs an explicit maintainer ack before merge (details inline).

🟡 Minor

• packages/core/src/session/runner/llm.ts:318-326 — one durable publish per modified message means N transactions/lock acquisitions; bus.publishAll would commit the batch atomically.
• packages/core/test/session-runner.test.ts — no companion test for the one-shot guard: after a second stale-reasoning rejection the session should fail normally instead of stripping again or looping. The flag makes that true today; a test would keep it true.
• packages/ai/test/provider-error.test.ts — the new cases only exercise 400s with a classification present. A negative case (non-client status with the same text stays non-stale) and a direct isStaleReasoningFailure case for the regex fallback would pin the gate down.

🔵 Info

• packages/ai/src/provider-error.ts:74-78 — the fallback branch fires on unclassified errors whose text merely mentions the phrases; the strip is permanent, so it is worth a deliberate note that only affirmatively-classified failures proceed (or a comment saying why the fallback exists).
• #43595 (the duplicate bot's find) is still open and overlaps; maintainers will want to close one of the two.

This review was generated by an AI assistant.

[Mirrobot-Agent]

🟠 Major — Design sign-off needed: this resumes live publication of session.message.content.updated, which #48043 deliberately retired ("remove message content mutation API"), and which is documented as replay-only (packages/schema/src/session-event.ts:135) and excluded from the public manifest. It is mechanically sound — the projector still folds it, the event never reaches SSE clients (event-feed.ts:49 filters on isOpenCodeEvent), and UsageRecorded sets a precedent for internal durable events the runner publishes live — but it re-opens the content-mutation path that removal closed, minus the guards the old public API enforced (message completed, no unfinished tools, session idle). Before merge, get an explicit ack from the maintainer who owns #48043 that this internal revival is the intended direction; if it lands, the // Replay-only comment should be updated so the file does not misdocument itself. This is not a code defect — it is a merge decision that belongs to the maintainers, flagged so it is made consciously.

[juiceydev]

Not to complain but why are we using agents to review this simple PR, this should've been pushed since yesterday from how simple the change is.

[Mirrowel]

Not to complain but why are we using agents to review this simple PR, this should've been pushed since yesterday from how simple the change is.

There are reasons not to push this, and instead have the provider fix the problem.
This change would lobotomize the model each time it applies