fix(opencode): resume the OpenCode session on follow-ups instead of starting an empty one#3617
fix(opencode): resume the OpenCode session on follow-ups instead of starting an empty one#3617vdmkotai wants to merge 6 commits into
Conversation
…tarting an empty one The OpenCode adapter always called session.create and never read or emitted a resume cursor, so the upstream ses_… id lived only in memory. When that in-memory binding was lost — the ProviderSessionReaper stopping an idle session (~30 min) or an app/server restart — the next follow-up in the same visible thread was sent to a brand-new, empty OpenCode session. t3code kept rendering its own projection DB, so the user still saw the full history while the model had no context (issue pingdotgg#3604). Mirror the Grok/Cursor/Codex resume pattern, entirely within the adapter: - Emit resumeCursor { schemaVersion, sessionId } on the started ProviderSession (and echo it from sendTurn) so ProviderService persists it into provider_session_runtime.resume_cursor_json. - On startSession, when a cursor is present, validate the id with session.get and re-adopt that session instead of creating a new one. OpenCode scopes history by session id, so prompting the same id restores the full prior conversation. A missing/closed session (or any get failure) falls back to a fresh session so a stale cursor can't wedge the thread. - Only abort the upstream session in the start race-cleanup when we actually created it; never abort a session we merely resumed. The persistence/recovery plumbing is provider-agnostic and already feeds a persisted cursor back into startSession on a reaped/restarted follow-up (ProviderService falls back to the stored binding cursor when the reactor passes none), so no changes outside the adapter are needed. Adds regression tests: fresh-session cursor emission, resume re-adopting the persisted id (no create), follow-up turns targeting the resumed id, stale-cursor fallback to create, and malformed-cursor rejection. Fixes pingdotgg#3604
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ApprovabilityVerdict: Needs human review This bug fix introduces significant new session management behavior (resumption logic, error classification, permission re-application) that changes runtime control flow. The author has no prior commits to this repository, and the behavioral scope warrants human verification despite comprehensive test coverage. You can customize Macroscope's approvability policy. Learn more. |
…ass) Addresses review feedback on pingdotgg#3617 (macroscope + Cursor Bugbot + a deep multi-agent review) without widening scope beyond the adapter: - Re-apply permissions on resume. `session.create` is the only place the runtimeMode permission ruleset is set, so re-adopting a session skipped it; the reactor restarts with the persisted cursor on a runtime-mode change, which would leave a resumed OpenCode session on stale (e.g. full-access) permissions. Resume now calls session.update with buildOpenCodePermissionRules(runtimeMode). - Don't resume into the wrong directory. OpenCode routes a prompt to the session's own stored directory, so reusing a session created under a different cwd would silently run there. Resume now starts a fresh session when the re-adopted session's directory differs from the requested cwd. - Distinguish "not found" from transient failures. The SDK client uses throwOnError:true, so session.get rejects on any non-2xx. Only a confirmed 404 / NotFoundError now falls back to creating a fresh session; transport/auth/server errors propagate instead of silently resetting a live thread to an empty session. Tests model throwOnError:true (session.get rejects) and add coverage for the permission re-application, cwd-mismatch fallback, and transient-error propagation paths.
|
Thanks for the reviews — addressed in 7be6629. Three hardening changes, all kept within the adapter:
Tests updated to model |
Address review (Cursor Bugbot, high): isOpenCodeNotFound only walked the `cause` chain checking a numeric `status`, so it relied on the 404 sitting at one specific nesting and ignored `response.status` and the OpenCodeRuntimeError `detail` string. Reworked it into a bounded BFS that also checks `statusCode`, nested `response.status`, the NotFoundError `name`/`body`, and `message`/`detail` text, descending cause/body/error/data. Export it and add direct unit tests across every shape (incl. the real wrapped-Error production shape and a response.status-only 404), plus the transient/auth/network cases that must still propagate.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b984684. Configure here.
…e text Address review (Cursor Bugbot): isOpenCodeNotFound matched the free-text message/detail, so a non-404 error whose text merely contains 'not found' (a 500 saying 'upstream X not found', an auth error, or a serialized body from openCodeRuntimeErrorDetail) was misclassified as a missing session and silently started a fresh one. Decide only on structured signals — a numeric 404 (status/statusCode/nested response.status) or an explicit NotFoundError name — which already cover the real throwOnError:true production shape (cause.status=404 + body.name). Update unit tests to assert free-text-only inputs (incl. a 500 whose message contains 'not found') now propagate.

Fixes #3604.
What
The OpenCode adapter never read or emitted a resume cursor, so the upstream
ses_…id lived only in process memory. When that in-memory binding was lost —ProviderSessionReaperstopping an idle session (~30 min), or an app/server restart — the next follow-up in the same visible thread was sent to a brand-new, empty OpenCode session. t3code kept rendering its own projection DB, so the user still saw the full history while the model had no context.This makes the OpenCode adapter resumable, mirroring the existing Grok/Cursor/Codex pattern, entirely within
apps/server/src/provider/Layers/OpenCodeAdapter.ts:startSessionnow emitsresumeCursor: { schemaVersion, sessionId }on the returnedProviderSession(andsendTurnechoes it), soProviderServicepersists it intoprovider_session_runtime.resume_cursor_json.startSessionvalidates the id withsession.getand re-adopts that session instead of callingsession.create. OpenCode scopes history by session id, so prompting the same id restores the full prior conversation.session.getfailure) falls back to a fresh session, so a stale cursor can never wedge the thread.Why
This is the documented root cause in #3604 (with DB-level evidence of two OpenCode sessions per visible thread). The persistence/recovery plumbing is already provider-agnostic:
ProviderService.startSessionfalls back to the stored binding cursor when the reactor passes none, so no changes are needed outside the adapter — it just needed the adapter to start producing and consuming a cursor like every other provider already does.Scope
Intentionally small and focused — 2 files, no contract / persistence / orchestration changes:
apps/server/src/provider/Layers/OpenCodeAdapter.ts(+118/-20)apps/server/src/provider/Layers/OpenCodeAdapter.test.ts(+147) — regression tests for: fresh-session cursor emission, resume re-adopting the persisted id (nocreate), follow-up turns targeting the resumed id, stale-cursor fallback tocreate, and malformed/foreign-cursor rejection.Validation
tsgo --noEmitclean;OpenCodeAdapter.test.ts(21 tests) plus the fullsrc/provider+src/orchestrationsuites (531 tests) pass.Note
Medium Risk
Changes session lifecycle and error handling in the provider adapter; misclassified errors could still drop context or wedge threads, but scope is limited to OpenCode and is heavily regression-tested.
Overview
Fixes silent loss of model context (#3604) when an in-memory OpenCode binding was dropped (idle reaper, restart) while the UI still showed full thread history.
OpenCode adapter now emits and consumes a durable
resumeCursor(schemaVersion+sessionId) onstartSessionand echoes it fromsendTurn, aligned with other providers. On start, a valid cursor triggerssession.getto re-adopt the upstreamses_…id, reapplies permissions withsession.updatefor the currentruntimeMode, and only callssession.createwhen there is no cursor, the session is missing (structured 404 /NotFoundErrorviaisOpenCodeNotFound), or the session’sdirectorydoes not match the requested cwd. Transient or auth errors from the probe propagate instead of falling back to a new session. ConcurrentstartSessionrace cleanup aborts the remote session only when this call created it, not when it resumed a shared session.Tests extend the OpenCode runtime mock (
session.get/session.update, 404 vs 500) and add coverage for cursor emission, resume, stale/malformed cursors, directory mismatch, andisOpenCodeNotFoundclassification.Reviewed by Cursor Bugbot for commit ed5fadf. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Resume existing OpenCode sessions on follow-up turns instead of starting new ones
startSessionin OpenCodeAdapter.ts now parses a resume cursor and probes the upstream session viasession.getbefore creating a new one.session.update; on a 404 or directory mismatch, a fresh session is created instead.sendTurnnow includes theresumeCursor(containingschemaVersionandsessionId) in its result so callers can persist it across turns.startSessiononly aborts the remote session if it created it, not if it merely resumed one.Macroscope summarized ed5fadf.