From 0024505ae7b444b09a24282a5d1a968063ed233c Mon Sep 17 00:00:00 2001 From: Vadym Kotai Date: Tue, 30 Jun 2026 14:21:03 +0400 Subject: [PATCH 01/10] fix(opencode): resume the OpenCode session on follow-ups instead of starting an empty one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #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 #3604 Co-authored-by: codex --- .../provider/Layers/OpenCodeAdapter.test.ts | 147 ++++++++++++++++++ .../src/provider/Layers/OpenCodeAdapter.ts | 117 +++++++++++--- 2 files changed, 245 insertions(+), 19 deletions(-) diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts index 4358cf305b0..8601d94966d 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts @@ -64,6 +64,8 @@ const runtimeMock = { closeError: null as Error | null, messages: [] as MessageEntry[], subscribedEvents: [] as unknown[], + sessionGetIds: [] as string[], + missingSessionIds: new Set(), }, reset() { this.state.startCalls.length = 0; @@ -78,6 +80,8 @@ const runtimeMock = { this.state.closeError = null; this.state.messages = []; this.state.subscribedEvents = []; + this.state.sessionGetIds.length = 0; + this.state.missingSessionIds.clear(); }, }; @@ -132,6 +136,15 @@ const OpenCodeRuntimeTestDouble: OpenCodeRuntimeShape = { ); return { data: { id: `${baseUrl}/session` } }; }, + get: async ({ sessionID }: { sessionID: string }) => { + runtimeMock.state.sessionGetIds.push(sessionID); + // Model OpenCode's `session.get`: a known id returns the session, + // an unknown/closed id resolves with no `data` (the SDK surfaces the + // 404 as `{ data: undefined, error }` under ThrowOnError=false). + return runtimeMock.state.missingSessionIds.has(sessionID) + ? { data: undefined } + : { data: { id: sessionID } }; + }, abort: async ({ sessionID }: { sessionID: string }) => { runtimeMock.state.abortCalls.push(sessionID); }, @@ -251,6 +264,140 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => { }), ); + it.effect("returns a durable resume cursor for a freshly created session", () => + Effect.gen(function* () { + const adapter = yield* OpenCodeAdapter; + const threadId = asThreadId("thread-opencode-cursor"); + + const session = yield* adapter.startSession({ + provider: ProviderDriverKind.make("opencode"), + threadId, + runtimeMode: "full-access", + }); + + // Without a persisted cursor, a session is created and its id is + // surfaced as a resume cursor so the upper layer can persist it. + NodeAssert.deepEqual(runtimeMock.state.sessionGetIds, []); + NodeAssert.deepEqual(session.resumeCursor, { + schemaVersion: 1, + sessionId: "http://127.0.0.1:9999/session", + }); + + yield* adapter.stopSession(threadId); + }), + ); + + it.effect("resumes the persisted OpenCode session instead of creating a new one", () => + Effect.gen(function* () { + const adapter = yield* OpenCodeAdapter; + const threadId = asThreadId("thread-opencode-resume"); + + const session = yield* adapter.startSession({ + provider: ProviderDriverKind.make("opencode"), + threadId, + runtimeMode: "full-access", + resumeCursor: { schemaVersion: 1, sessionId: "ses_persisted" }, + }); + + // The adapter validates the persisted id with session.get and re-adopts + // it — no new session is minted (issue #3604). + NodeAssert.deepEqual(runtimeMock.state.sessionGetIds, ["ses_persisted"]); + NodeAssert.deepEqual(runtimeMock.state.sessionCreateUrls, []); + NodeAssert.deepEqual(session.resumeCursor, { + schemaVersion: 1, + sessionId: "ses_persisted", + }); + + yield* adapter.stopSession(threadId); + }), + ); + + it.effect("sends follow-up turns to the resumed session id", () => + Effect.gen(function* () { + const adapter = yield* OpenCodeAdapter; + const threadId = asThreadId("thread-opencode-resume-turn"); + + yield* adapter.startSession({ + provider: ProviderDriverKind.make("opencode"), + threadId, + runtimeMode: "full-access", + resumeCursor: { schemaVersion: 1, sessionId: "ses_persisted" }, + }); + + const result = yield* adapter.sendTurn({ + threadId, + input: "continue where we left off", + modelSelection: createModelSelection( + ProviderInstanceId.make("opencode"), + "anthropic/sonnet", + ), + }); + + // The prompt targets the resumed id, and the turn re-surfaces the cursor. + NodeAssert.deepEqual( + (runtimeMock.state.promptCalls[0] as { sessionID: string }).sessionID, + "ses_persisted", + ); + NodeAssert.deepEqual(result.resumeCursor, { + schemaVersion: 1, + sessionId: "ses_persisted", + }); + + yield* adapter.stopSession(threadId); + }), + ); + + it.effect("falls back to a fresh session when the persisted session is gone", () => + Effect.gen(function* () { + const adapter = yield* OpenCodeAdapter; + const threadId = asThreadId("thread-opencode-stale"); + runtimeMock.state.missingSessionIds.add("ses_stale"); + + const session = yield* adapter.startSession({ + provider: ProviderDriverKind.make("opencode"), + threadId, + runtimeMode: "full-access", + resumeCursor: { schemaVersion: 1, sessionId: "ses_stale" }, + }); + + // get probed the stale id, found nothing, then created a new session and + // emitted a fresh cursor rather than wedging the thread. + NodeAssert.deepEqual(runtimeMock.state.sessionGetIds, ["ses_stale"]); + NodeAssert.deepEqual(runtimeMock.state.sessionCreateUrls, ["http://127.0.0.1:9999"]); + NodeAssert.deepEqual(session.resumeCursor, { + schemaVersion: 1, + sessionId: "http://127.0.0.1:9999/session", + }); + + yield* adapter.stopSession(threadId); + }), + ); + + it.effect("ignores a malformed or wrong-version resume cursor", () => + Effect.gen(function* () { + const adapter = yield* OpenCodeAdapter; + const threadId = asThreadId("thread-opencode-badcursor"); + + const session = yield* adapter.startSession({ + provider: ProviderDriverKind.make("opencode"), + threadId, + runtimeMode: "full-access", + resumeCursor: { schemaVersion: 99, sessionId: "ses_persisted" }, + }); + + // A foreign/stale-shaped cursor is treated as "no resume": never probed, + // a fresh session is created. + NodeAssert.deepEqual(runtimeMock.state.sessionGetIds, []); + NodeAssert.deepEqual(runtimeMock.state.sessionCreateUrls, ["http://127.0.0.1:9999"]); + NodeAssert.deepEqual(session.resumeCursor, { + schemaVersion: 1, + sessionId: "http://127.0.0.1:9999/session", + }); + + yield* adapter.stopSession(threadId); + }), + ); + it.effect("fails sendTurn for missing sessions through the typed error channel", () => Effect.gen(function* () { const adapter = yield* OpenCodeAdapter; diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.ts index e7622e6c705..20203cb943d 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.ts @@ -53,6 +53,35 @@ import * as Option from "effect/Option"; const PROVIDER = ProviderDriverKind.make("opencode"); +/** + * Version tag stamped into the OpenCode resume cursor. Bump if the cursor + * shape changes so stale-shaped cursors written by older builds are ignored + * rather than misread (mirrors GROK_RESUME_VERSION / CURSOR_RESUME_VERSION). + */ +const OPENCODE_RESUME_VERSION = 1 as const; + +/** + * Decode a persisted OpenCode resume cursor back into the upstream `ses_…` + * id. Returns `undefined` for anything that isn't a current-version cursor + * carrying a non-empty session id, so a malformed or foreign cursor simply + * means "no resume" instead of throwing. OpenCode has no dedicated resume + * RPC — re-adopting the same session id *is* the resume mechanism, because + * the server scopes a conversation's history by session id. + */ +function parseOpenCodeResume(raw: unknown): { readonly sessionId: string } | undefined { + if (typeof raw !== "object" || raw === null || Array.isArray(raw)) { + return undefined; + } + const record = raw as Record; + if (record.schemaVersion !== OPENCODE_RESUME_VERSION) { + return undefined; + } + if (typeof record.sessionId !== "string" || record.sessionId.trim().length === 0) { + return undefined; + } + return { sessionId: record.sessionId.trim() }; +} + interface OpenCodeTurnSnapshot { readonly id: TurnId; readonly items: Array; @@ -1076,6 +1105,7 @@ export function makeOpenCodeAdapter( const serverUrl = openCodeSettings.serverUrl; const serverPassword = openCodeSettings.serverPassword; const directory = input.cwd ?? serverConfig.cwd; + const resumeSessionId = parseOpenCodeResume(input.resumeCursor)?.sessionId; const existing = sessions.get(input.threadId); if (existing) { yield* stopOpenCodeContext(existing); @@ -1115,22 +1145,50 @@ export function makeOpenCodeAdapter( }), ); } - const openCodeSession = yield* runOpenCodeSdk("session.create", () => - client.session.create({ - permission: buildOpenCodePermissionRules(input.runtimeMode), - }), - ); - if (!openCodeSession.data) { - return yield* new OpenCodeRuntimeError({ - operation: "session.create", - detail: "OpenCode session.create returned no session payload.", - }); - } + // Resume path: when a durable cursor names a prior OpenCode + // session, re-adopt that `ses_…` instead of minting a new one. + // OpenCode scopes history by session id, so prompting the same + // id automatically restores the full prior conversation. We + // validate with `session.get` first; a missing/closed session + // (or any get failure) falls back to creating a fresh one so a + // stale cursor can never wedge the thread. + const resumedSession = resumeSessionId + ? yield* runOpenCodeSdk("session.get", () => + client.session.get({ sessionID: resumeSessionId }), + ).pipe( + Effect.map((response) => response.data), + Effect.orElseSucceed(() => undefined), + ) + : undefined; + const created = resumedSession === undefined; + const openCodeSession = yield* resumedSession + ? Effect.succeed(resumedSession) + : Effect.gen(function* () { + if (resumeSessionId) { + yield* Effect.logWarning( + `OpenCode session '${resumeSessionId}' could not be resumed; starting a fresh session.`, + ); + } + const createdSession = yield* runOpenCodeSdk("session.create", () => + client.session.create({ + title: `T3 Code ${input.threadId}`, + permission: buildOpenCodePermissionRules(input.runtimeMode), + }), + ); + if (!createdSession.data) { + return yield* new OpenCodeRuntimeError({ + operation: "session.create", + detail: "OpenCode session.create returned no session payload.", + }); + } + return createdSession.data; + }); return { sessionScope, server, client, - openCodeSession: openCodeSession.data, + openCodeSession, + created, }; }).pipe(Effect.provideService(Scope.Scope, sessionScope)), ); @@ -1145,13 +1203,19 @@ export function makeOpenCodeAdapter( // and already inserted a session while we were awaiting async work. const raceWinner = sessions.get(input.threadId); if (raceWinner) { - // Another call won the race – clean up the session we just created - // (including the remote SDK session) and return the existing one. - yield* runOpenCodeSdk("session.abort", () => - started.client.session.abort({ - sessionID: started.openCodeSession.id, - }), - ).pipe(Effect.ignore); + // Another call won the race – clean up the session we just started + // and return the existing one. Only abort the remote SDK session if + // we *created* it here; a session we merely resumed is shared upstream + // state the race winner is now using, so aborting it would interrupt + // them (and `session.abort` is otherwise the wrong tool — it cancels + // an in-flight turn rather than disowning a session). + if (started.created) { + yield* runOpenCodeSdk("session.abort", () => + started.client.session.abort({ + sessionID: started.openCodeSession.id, + }), + ).pipe(Effect.ignore); + } yield* Scope.close(started.sessionScope, Exit.void).pipe(Effect.ignore); return raceWinner.session; } @@ -1165,6 +1229,16 @@ export function makeOpenCodeAdapter( cwd: directory, ...(input.modelSelection ? { model: input.modelSelection.model } : {}), threadId: input.threadId, + // Durable binding to the upstream OpenCode session. ProviderService + // persists this into provider_session_runtime.resume_cursor_json and + // feeds it back into `startSession` on the next turn after the + // in-memory session is lost (reaper / app or server restart), which + // is what lets a follow-up continue the same conversation instead of + // silently landing in a new, empty session (issue #3604). + resumeCursor: { + schemaVersion: OPENCODE_RESUME_VERSION, + sessionId: started.openCodeSession.id, + }, createdAt, updatedAt: createdAt, }; @@ -1330,6 +1404,11 @@ export function makeOpenCodeAdapter( return { threadId: input.threadId, turnId, + // Re-surface the durable cursor on every turn so the persisted binding + // is refreshed alongside last-seen/runtime state (mirrors Grok/Codex). + ...(context.session.resumeCursor !== undefined + ? { resumeCursor: context.session.resumeCursor } + : {}), }; }); From 482fd9642ea5e788699bb6db85e8819e54aa299b Mon Sep 17 00:00:00 2001 From: Vadym Kotai Date: Tue, 30 Jun 2026 20:03:22 +0400 Subject: [PATCH 02/10] fix(opencode): harden session resume per review (perms, cwd, error class) Addresses review feedback on #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. Co-authored-by: codex --- .../provider/Layers/OpenCodeAdapter.test.ts | 110 ++++++++++++- .../src/provider/Layers/OpenCodeAdapter.ts | 146 +++++++++++++----- 2 files changed, 213 insertions(+), 43 deletions(-) diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts index 8601d94966d..173deaae828 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts @@ -66,6 +66,9 @@ const runtimeMock = { subscribedEvents: [] as unknown[], sessionGetIds: [] as string[], missingSessionIds: new Set(), + transientErrorSessionIds: new Set(), + sessionDirectoryById: new Map(), + sessionUpdateCalls: [] as Array<{ sessionID: string; permission: unknown }>, }, reset() { this.state.startCalls.length = 0; @@ -82,6 +85,9 @@ const runtimeMock = { this.state.subscribedEvents = []; this.state.sessionGetIds.length = 0; this.state.missingSessionIds.clear(); + this.state.transientErrorSessionIds.clear(); + this.state.sessionDirectoryById.clear(); + this.state.sessionUpdateCalls.length = 0; }, }; @@ -138,12 +144,24 @@ const OpenCodeRuntimeTestDouble: OpenCodeRuntimeShape = { }, get: async ({ sessionID }: { sessionID: string }) => { runtimeMock.state.sessionGetIds.push(sessionID); - // Model OpenCode's `session.get`: a known id returns the session, - // an unknown/closed id resolves with no `data` (the SDK surfaces the - // 404 as `{ data: undefined, error }` under ThrowOnError=false). - return runtimeMock.state.missingSessionIds.has(sessionID) - ? { data: undefined } - : { data: { id: sessionID } }; + // The real client is created with `throwOnError: true`, so a non-2xx + // response REJECTS (it does not resolve to a tuple). Model that: a + // transient error throws a non-404, a missing session throws a 404, + // and success resolves with the session payload. + if (runtimeMock.state.transientErrorSessionIds.has(sessionID)) { + throw new Error("opencode server error", { cause: { status: 500 } }); + } + if (runtimeMock.state.missingSessionIds.has(sessionID)) { + throw new Error(`Session not found: ${sessionID}`, { + cause: { status: 404, body: { name: "NotFoundError" } }, + }); + } + const directory = runtimeMock.state.sessionDirectoryById.get(sessionID); + return { data: { id: sessionID, ...(directory ? { directory } : {}) } }; + }, + update: async ({ sessionID, permission }: { sessionID: string; permission: unknown }) => { + runtimeMock.state.sessionUpdateCalls.push({ sessionID, permission }); + return { data: { id: sessionID } }; }, abort: async ({ sessionID }: { sessionID: string }) => { runtimeMock.state.abortCalls.push(sessionID); @@ -307,6 +325,10 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => { schemaVersion: 1, sessionId: "ses_persisted", }); + // Resume re-asserts the permission ruleset for the current runtimeMode. + NodeAssert.equal(runtimeMock.state.sessionUpdateCalls.length, 1); + NodeAssert.equal(runtimeMock.state.sessionUpdateCalls[0]?.sessionID, "ses_persisted"); + NodeAssert.equal(runtimeMock.state.sessionUpdateCalls[0]?.permission != null, true); yield* adapter.stopSession(threadId); }), @@ -398,6 +420,82 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => { }), ); + it.effect("surfaces a non-not-found resume probe error instead of silently starting fresh", () => + Effect.gen(function* () { + const adapter = yield* OpenCodeAdapter; + const threadId = asThreadId("thread-opencode-transient"); + // session.get returns a 500 (not a 404) for this id. + runtimeMock.state.transientErrorSessionIds.add("ses_transient"); + + const exit = yield* Effect.exit( + adapter.startSession({ + provider: ProviderDriverKind.make("opencode"), + threadId, + runtimeMode: "full-access", + resumeCursor: { schemaVersion: 1, sessionId: "ses_transient" }, + }), + ); + + // A transient/transport/auth failure must propagate — NOT be masked as a + // brand-new empty session (the #3604 class of silent context loss). + NodeAssert.equal(Exit.isFailure(exit), true); + NodeAssert.deepEqual(runtimeMock.state.sessionGetIds, ["ses_transient"]); + NodeAssert.deepEqual(runtimeMock.state.sessionCreateUrls, []); + }), + ); + + it.effect("re-applies the current runtimeMode permissions when resuming", () => + Effect.gen(function* () { + const adapter = yield* OpenCodeAdapter; + const threadId = asThreadId("thread-opencode-perms"); + + yield* adapter.startSession({ + provider: ProviderDriverKind.make("opencode"), + // A different runtimeMode than the original create — resume must not + // leave the upstream session on stale permissions. + runtimeMode: "approval-required", + threadId, + resumeCursor: { schemaVersion: 1, sessionId: "ses_perms" }, + }); + + NodeAssert.deepEqual(runtimeMock.state.sessionGetIds, ["ses_perms"]); + NodeAssert.deepEqual(runtimeMock.state.sessionCreateUrls, []); + NodeAssert.equal(runtimeMock.state.sessionUpdateCalls.length, 1); + NodeAssert.equal(runtimeMock.state.sessionUpdateCalls[0]?.sessionID, "ses_perms"); + NodeAssert.equal(runtimeMock.state.sessionUpdateCalls[0]?.permission != null, true); + + yield* adapter.stopSession(threadId); + }), + ); + + it.effect("starts fresh when the resumed session is bound to a different directory", () => + Effect.gen(function* () { + const adapter = yield* OpenCodeAdapter; + const threadId = asThreadId("thread-opencode-cwd"); + // The persisted session still exists but lives in another working dir. + runtimeMock.state.sessionDirectoryById.set("ses_otherdir", "/some/other/worktree"); + + const session = yield* adapter.startSession({ + provider: ProviderDriverKind.make("opencode"), + threadId, + runtimeMode: "full-access", + resumeCursor: { schemaVersion: 1, sessionId: "ses_otherdir" }, + }); + + // OpenCode routes prompts to the session's stored directory, so a cwd + // change must NOT silently reuse the old-directory session — create fresh. + NodeAssert.deepEqual(runtimeMock.state.sessionGetIds, ["ses_otherdir"]); + NodeAssert.deepEqual(runtimeMock.state.sessionCreateUrls, ["http://127.0.0.1:9999"]); + NodeAssert.deepEqual(runtimeMock.state.sessionUpdateCalls, []); + NodeAssert.deepEqual(session.resumeCursor, { + schemaVersion: 1, + sessionId: "http://127.0.0.1:9999/session", + }); + + yield* adapter.stopSession(threadId); + }), + ); + it.effect("fails sendTurn for missing sessions through the typed error channel", () => Effect.gen(function* () { const adapter = yield* OpenCodeAdapter; diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.ts index 20203cb943d..96ef872604c 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.ts @@ -82,6 +82,40 @@ function parseOpenCodeResume(raw: unknown): { readonly sessionId: string } | und return { sessionId: record.sessionId.trim() }; } +/** + * Walk an error's `cause` chain for a definitive "session not found" signal — + * an HTTP 404 or an OpenCode `NotFoundError`. The SDK client is configured + * `throwOnError: true` (see `createOpenCodeSdkClient`), so `session.get` on a + * missing/closed session rejects rather than resolving; `runOpenCodeSdk` then + * surfaces it as a failed Effect whose cause wraps the original error (message + * plus `{ body, status }`). Only a confirmed miss justifies silently starting a + * fresh session — any other failure (transport, auth, server error) must + * propagate, so a momentary blip can't quietly reset a live thread to an empty + * session (the #3604 class of silent context loss). + */ +function isOpenCodeNotFound(cause: unknown): boolean { + let current: unknown = cause; + for (let depth = 0; depth < 6 && current !== null && typeof current === "object"; depth += 1) { + const record = current as Record; + if (record.status === 404 || record.statusCode === 404) { + return true; + } + const name = record.name; + if (typeof name === "string" && name.toLowerCase().includes("notfound")) { + return true; + } + const message = record.message; + if ( + typeof message === "string" && + /\bnot found\b|no such session|unknown session|does not exist/i.test(message) + ) { + return true; + } + current = record.cause; + } + return false; +} + interface OpenCodeTurnSnapshot { readonly id: TurnId; readonly items: Array; @@ -1147,48 +1181,86 @@ export function makeOpenCodeAdapter( } // Resume path: when a durable cursor names a prior OpenCode // session, re-adopt that `ses_…` instead of minting a new one. - // OpenCode scopes history by session id, so prompting the same - // id automatically restores the full prior conversation. We - // validate with `session.get` first; a missing/closed session - // (or any get failure) falls back to creating a fresh one so a - // stale cursor can never wedge the thread. - const resumedSession = resumeSessionId - ? yield* runOpenCodeSdk("session.get", () => - client.session.get({ sessionID: resumeSessionId }), - ).pipe( - Effect.map((response) => response.data), - Effect.orElseSucceed(() => undefined), - ) - : undefined; - const created = resumedSession === undefined; - const openCodeSession = yield* resumedSession - ? Effect.succeed(resumedSession) - : Effect.gen(function* () { - if (resumeSessionId) { - yield* Effect.logWarning( - `OpenCode session '${resumeSessionId}' could not be resumed; starting a fresh session.`, - ); - } - const createdSession = yield* runOpenCodeSdk("session.create", () => - client.session.create({ - title: `T3 Code ${input.threadId}`, - permission: buildOpenCodePermissionRules(input.runtimeMode), - }), - ); - if (!createdSession.data) { - return yield* new OpenCodeRuntimeError({ - operation: "session.create", - detail: "OpenCode session.create returned no session payload.", - }); - } - return createdSession.data; + // OpenCode scopes history by session id, so prompting the same id + // restores the full prior conversation. + // + // The probe distinguishes three outcomes. A request that throws + // (transport/network) fails the Effect in `runOpenCodeSdk` and + // propagates — we never silently reset a live thread on a blip. + // A confirmed "not found" falls back to a fresh session. Any + // other error response (auth, bad request, server error) is + // surfaced rather than masked as a brand-new empty session. + const resolved = yield* Effect.gen(function* () { + // Probe the persisted session. With `throwOnError: true`, + // `session.get` rejects on any non-2xx; we recover ONLY a + // confirmed "not found" (the session is genuinely gone -> start + // fresh). A transport/auth/server error propagates instead of + // masquerading as a brand-new empty session (issue #3604). + const adopted = resumeSessionId + ? yield* runOpenCodeSdk("session.get", () => + client.session.get({ sessionID: resumeSessionId }), + ).pipe( + Effect.map((response) => response.data), + Effect.catchIf( + (cause) => isOpenCodeNotFound(cause), + () => Effect.succeed(undefined), + ), + ) + : undefined; + + // Reuse the upstream session only when it still matches the + // requested working directory. OpenCode routes a prompt to the + // session's OWN stored directory, so resuming a session created + // under a different cwd would silently run there — in that case + // start fresh in the requested directory instead. + const reusable = + adopted && (!adopted.directory || adopted.directory === directory) + ? adopted + : undefined; + + if (reusable) { + // `session.create` is skipped on resume, so re-assert the + // permission ruleset for the CURRENT runtimeMode — otherwise a + // runtime-mode change (the reactor restarts with the persisted + // cursor) would leave the re-adopted session on the + // permissions it was originally created with. + yield* runOpenCodeSdk("session.update", () => + client.session.update({ + sessionID: reusable.id, + permission: buildOpenCodePermissionRules(input.runtimeMode), + }), + ); + return { openCodeSession: reusable, created: false }; + } + + if (resumeSessionId) { + yield* Effect.logWarning( + adopted !== undefined + ? `OpenCode session '${resumeSessionId}' is bound to a different working directory; starting a fresh session.` + : `OpenCode session '${resumeSessionId}' no longer exists; starting a fresh session.`, + ); + } + const createdSession = yield* runOpenCodeSdk("session.create", () => + client.session.create({ + title: `T3 Code ${input.threadId}`, + permission: buildOpenCodePermissionRules(input.runtimeMode), + }), + ); + if (!createdSession.data) { + return yield* new OpenCodeRuntimeError({ + operation: "session.create", + detail: "OpenCode session.create returned no session payload.", }); + } + return { openCodeSession: createdSession.data, created: true }; + }); + return { sessionScope, server, client, - openCodeSession, - created, + openCodeSession: resolved.openCodeSession, + created: resolved.created, }; }).pipe(Effect.provideService(Scope.Scope, sessionScope)), ); From e731bb186030f2c3a66e09f8e4d1ae19c0c01449 Mon Sep 17 00:00:00 2001 From: Vadym Kotai Date: Tue, 30 Jun 2026 20:13:12 +0400 Subject: [PATCH 03/10] fix(opencode): make not-found detection robust to all SDK error shapes 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. Co-authored-by: codex --- .../provider/Layers/OpenCodeAdapter.test.ts | 36 ++++++++++ .../src/provider/Layers/OpenCodeAdapter.ts | 68 +++++++++++++------ 2 files changed, 84 insertions(+), 20 deletions(-) diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts index 173deaae828..a713df0c24c 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts @@ -31,6 +31,7 @@ import { } from "../opencodeRuntime.ts"; import { appendOpenCodeAssistantTextDelta, + isOpenCodeNotFound, makeOpenCodeAdapter, mergeOpenCodeAssistantText, } from "./OpenCodeAdapter.ts"; @@ -918,6 +919,41 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => { }), ); + it.effect("classifies a confirmed not-found across the shapes the SDK/runtime can produce", () => + Effect.sync(() => { + // The real production shape: runOpenCodeSdk wraps the thrown Error + // (cause = { body, status }) under OpenCodeRuntimeError. + const wrappedError = new Error("Session not found: ses_x", { + cause: { body: { name: "NotFoundError" }, status: 404 }, + }); + NodeAssert.equal( + isOpenCodeNotFound({ + _tag: "OpenCodeRuntimeError", + operation: "session.get", + detail: "Session not found: ses_x", + cause: wrappedError, + }), + true, + ); + + // 404 expressed only via response.status (the bot's flagged shape). + NodeAssert.equal(isOpenCodeNotFound({ cause: { response: { status: 404 } } }), true); + // 404 via a bare numeric status / statusCode. + NodeAssert.equal(isOpenCodeNotFound(new Error("x", { cause: { status: 404 } })), true); + NodeAssert.equal(isOpenCodeNotFound({ statusCode: 404 }), true); + // OpenCode NotFoundError body name with no status. + NodeAssert.equal(isOpenCodeNotFound({ body: { name: "NotFoundError" } }), true); + // Only the detail string carries the signal. + NodeAssert.equal(isOpenCodeNotFound({ detail: "no such session" }), true); + + // NOT a miss: transient/server/auth errors must propagate. + NodeAssert.equal(isOpenCodeNotFound(new Error("boom", { cause: { status: 500 } })), false); + NodeAssert.equal(isOpenCodeNotFound({ cause: { response: { status: 401 } } }), false); + NodeAssert.equal(isOpenCodeNotFound(new Error("network error (no response)")), false); + NodeAssert.equal(isOpenCodeNotFound(undefined), false); + }), + ); + it.effect("appends raw assistant text deltas and reconciles part update snapshots", () => Effect.sync(() => { const firstUpdate = mergeOpenCodeAssistantText(undefined, "Hello"); diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.ts index 96ef872604c..949075bf723 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.ts @@ -82,36 +82,64 @@ function parseOpenCodeResume(raw: unknown): { readonly sessionId: string } | und return { sessionId: record.sessionId.trim() }; } +const OPENCODE_NOT_FOUND_MESSAGE = /\bnot found\b|no such session|unknown session|does not exist/i; + /** - * Walk an error's `cause` chain for a definitive "session not found" signal — - * an HTTP 404 or an OpenCode `NotFoundError`. The SDK client is configured - * `throwOnError: true` (see `createOpenCodeSdkClient`), so `session.get` on a - * missing/closed session rejects rather than resolving; `runOpenCodeSdk` then - * surfaces it as a failed Effect whose cause wraps the original error (message - * plus `{ body, status }`). Only a confirmed miss justifies silently starting a - * fresh session — any other failure (transport, auth, server error) must - * propagate, so a momentary blip can't quietly reset a live thread to an empty - * session (the #3604 class of silent context loss). + * Whether an error definitively reports a "session not found" — an HTTP 404 or + * an OpenCode `NotFoundError`. The SDK client is configured `throwOnError: true` + * (see `createOpenCodeSdkClient`), so `session.get` on a missing/closed session + * REJECTS rather than resolving; `runOpenCodeSdk` then surfaces it as a failed + * Effect whose `cause` wraps the original thrown `Error` (which carries the + * parsed body and HTTP status under its own `cause`), alongside the + * `OpenCodeRuntimeError.detail` string. Only a confirmed miss justifies silently + * starting a fresh session — any other failure (transport, auth, server error) + * must propagate, so a momentary blip can't quietly reset a live thread to an + * empty session (the #3604 class of silent context loss). + * + * Implemented as a bounded breadth-first walk so it is robust to the several + * shapes the SDK/runtime can produce: a 404 may surface as a numeric + * `status`/`statusCode`, a nested `response.status`, an OpenCode `NotFoundError` + * `name`/`body`, or text in `message`/`detail`. Exported for unit testing. */ -function isOpenCodeNotFound(cause: unknown): boolean { - let current: unknown = cause; - for (let depth = 0; depth < 6 && current !== null && typeof current === "object"; depth += 1) { - const record = current as Record; +export function isOpenCodeNotFound(cause: unknown): boolean { + const seen = new Set(); + const queue: Array = [cause]; + for (let steps = 0; queue.length > 0 && steps < 32; steps += 1) { + const node = queue.shift(); + if (node === null || typeof node !== "object" || seen.has(node)) { + continue; + } + seen.add(node); + const record = node as Record; + if (record.status === 404 || record.statusCode === 404) { return true; } + const response = record.response; + if ( + response !== null && + typeof response === "object" && + (response as { readonly status?: unknown }).status === 404 + ) { + return true; + } + const name = record.name; if (typeof name === "string" && name.toLowerCase().includes("notfound")) { return true; } - const message = record.message; - if ( - typeof message === "string" && - /\bnot found\b|no such session|unknown session|does not exist/i.test(message) - ) { - return true; + for (const key of ["message", "detail"] as const) { + const value = record[key]; + if (typeof value === "string" && OPENCODE_NOT_FOUND_MESSAGE.test(value)) { + return true; + } + } + + for (const key of ["cause", "body", "error", "data"] as const) { + if (record[key] !== undefined) { + queue.push(record[key]); + } } - current = record.cause; } return false; } From fa26ab342dd9165732aeb5888d183c49a742b438 Mon Sep 17 00:00:00 2001 From: Vadym Kotai Date: Tue, 30 Jun 2026 20:43:20 +0400 Subject: [PATCH 04/10] fix(opencode): detect not-found by structured signals only, never free text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Co-authored-by: codex --- .../provider/Layers/OpenCodeAdapter.test.ts | 12 ++++++-- .../src/provider/Layers/OpenCodeAdapter.ts | 30 ++++++++----------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts index a713df0c24c..5291ff4f599 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts @@ -943,10 +943,16 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => { NodeAssert.equal(isOpenCodeNotFound({ statusCode: 404 }), true); // OpenCode NotFoundError body name with no status. NodeAssert.equal(isOpenCodeNotFound({ body: { name: "NotFoundError" } }), true); - // Only the detail string carries the signal. - NodeAssert.equal(isOpenCodeNotFound({ detail: "no such session" }), true); - // NOT a miss: transient/server/auth errors must propagate. + // NOT a miss: only structured signals count, never free text. A non-404 + // error whose message/detail merely contains "not found" must propagate, + // not be misread as a missing session and silently start fresh. + NodeAssert.equal( + isOpenCodeNotFound(new Error("upstream provider not found", { cause: { status: 500 } })), + false, + ); + NodeAssert.equal(isOpenCodeNotFound({ detail: "status=500 body={...not found...}" }), false); + // Other transient/auth/network failures must propagate too. NodeAssert.equal(isOpenCodeNotFound(new Error("boom", { cause: { status: 500 } })), false); NodeAssert.equal(isOpenCodeNotFound({ cause: { response: { status: 401 } } }), false); NodeAssert.equal(isOpenCodeNotFound(new Error("network error (no response)")), false); diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.ts index 949075bf723..7972e4f1257 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.ts @@ -82,24 +82,24 @@ function parseOpenCodeResume(raw: unknown): { readonly sessionId: string } | und return { sessionId: record.sessionId.trim() }; } -const OPENCODE_NOT_FOUND_MESSAGE = /\bnot found\b|no such session|unknown session|does not exist/i; - /** * Whether an error definitively reports a "session not found" — an HTTP 404 or * an OpenCode `NotFoundError`. The SDK client is configured `throwOnError: true` * (see `createOpenCodeSdkClient`), so `session.get` on a missing/closed session * REJECTS rather than resolving; `runOpenCodeSdk` then surfaces it as a failed - * Effect whose `cause` wraps the original thrown `Error` (which carries the - * parsed body and HTTP status under its own `cause`), alongside the - * `OpenCodeRuntimeError.detail` string. Only a confirmed miss justifies silently - * starting a fresh session — any other failure (transport, auth, server error) - * must propagate, so a momentary blip can't quietly reset a live thread to an - * empty session (the #3604 class of silent context loss). + * Effect whose `cause` wraps the original thrown `Error`, which carries the + * parsed body and HTTP status under its own `cause`. Only a confirmed miss + * justifies silently starting a fresh session — any other failure (transport, + * auth, server error) must propagate, so a momentary blip can't quietly reset a + * live thread to an empty session (the #3604 class of silent context loss). * - * Implemented as a bounded breadth-first walk so it is robust to the several - * shapes the SDK/runtime can produce: a 404 may surface as a numeric - * `status`/`statusCode`, a nested `response.status`, an OpenCode `NotFoundError` - * `name`/`body`, or text in `message`/`detail`. Exported for unit testing. + * Decided only on STRUCTURED signals — a numeric 404 (`status`/`statusCode`/ + * nested `response.status`) or an explicit `NotFoundError` `name` — found via a + * bounded breadth-first walk over the error's `cause`/`body`/`error`/`data`. We + * deliberately do NOT match free text (`message`/`detail`): those can carry a + * serialized non-404 body or an unrelated "not found" phrase (e.g. a 500 whose + * message says "upstream X not found"), which would misclassify a real failure + * as a missing session and silently drop context. Exported for unit testing. */ export function isOpenCodeNotFound(cause: unknown): boolean { const seen = new Set(); @@ -128,12 +128,6 @@ export function isOpenCodeNotFound(cause: unknown): boolean { if (typeof name === "string" && name.toLowerCase().includes("notfound")) { return true; } - for (const key of ["message", "detail"] as const) { - const value = record[key]; - if (typeof value === "string" && OPENCODE_NOT_FOUND_MESSAGE.test(value)) { - return true; - } - } for (const key of ["cause", "body", "error", "data"] as const) { if (record[key] !== undefined) { From a2fef89f326f5a26c54b72e7eaf288aecf6c10f3 Mon Sep 17 00:00:00 2001 From: Vadym Kotai Date: Wed, 1 Jul 2026 19:10:49 +0400 Subject: [PATCH 05/10] fix(opencode): fork the session on a cwd change instead of dropping context The directory-mismatch guard added while hardening this PR started a fresh, EMPTY session whenever the resumed session's stored directory differed from the requested cwd. Its stated rationale -- "OpenCode routes a prompt to the session's own stored directory, so resuming under a different cwd would run in the wrong tree" -- does not hold: OpenCode resolves tool execution, snapshots and file ops from the per-request directory param (instance context), not from session.info.directory. So the guard solved a non-problem and introduced a real one: any time a thread's cwd changes (most commonly when it moves from the project root into a git worktree between turns) the whole conversation was stranded in the old session and the follow-up landed in an empty one -- the exact #3604 symptom this PR set out to fix. Fix: when the persisted session exists but was created under a different directory, fork it INTO the requested directory (client.session.fork({ sessionID, directory })) instead of creating an empty session. OpenCode clones the full message history into a new session bound to the requested worktree, so the follow-up keeps its context and tools still run on the correct tree. The fork id becomes the durable resume cursor. A genuinely missing (404) session still starts fresh, unchanged. Verified against the OpenCode source that fork copies all messages/parts with fresh ids and stamps the new session's directory/path from the request context. Test: the former "starts fresh on directory mismatch" case now asserts the session is forked with history (fork called, no session.create, cursor -> fork id) via a new fork mock; the not-found path still starts fresh. Co-authored-by: codex --- .../provider/Layers/OpenCodeAdapter.test.ts | 68 ++++++++++++------- .../src/provider/Layers/OpenCodeAdapter.ts | 44 +++++++++--- 2 files changed, 81 insertions(+), 31 deletions(-) diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts index 5291ff4f599..e822b4a4647 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts @@ -70,6 +70,7 @@ const runtimeMock = { transientErrorSessionIds: new Set(), sessionDirectoryById: new Map(), sessionUpdateCalls: [] as Array<{ sessionID: string; permission: unknown }>, + forkCalls: [] as Array<{ sessionID: string; directory?: string }>, }, reset() { this.state.startCalls.length = 0; @@ -89,6 +90,7 @@ const runtimeMock = { this.state.transientErrorSessionIds.clear(); this.state.sessionDirectoryById.clear(); this.state.sessionUpdateCalls.length = 0; + this.state.forkCalls.length = 0; }, }; @@ -164,6 +166,16 @@ const OpenCodeRuntimeTestDouble: OpenCodeRuntimeShape = { runtimeMock.state.sessionUpdateCalls.push({ sessionID, permission }); return { data: { id: sessionID } }; }, + fork: async ({ sessionID, directory }: { sessionID: string; directory?: string }) => { + // Model OpenCode fork: clones history into a NEW session bound to the + // requested directory (all prior messages carried over upstream). + const forkedId = `${sessionID}_fork`; + runtimeMock.state.forkCalls.push({ sessionID, ...(directory ? { directory } : {}) }); + if (directory) { + runtimeMock.state.sessionDirectoryById.set(forkedId, directory); + } + return { data: { id: forkedId, ...(directory ? { directory } : {}) } }; + }, abort: async ({ sessionID }: { sessionID: string }) => { runtimeMock.state.abortCalls.push(sessionID); }, @@ -469,32 +481,42 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => { }), ); - it.effect("starts fresh when the resumed session is bound to a different directory", () => - Effect.gen(function* () { - const adapter = yield* OpenCodeAdapter; - const threadId = asThreadId("thread-opencode-cwd"); - // The persisted session still exists but lives in another working dir. - runtimeMock.state.sessionDirectoryById.set("ses_otherdir", "/some/other/worktree"); + it.effect( + "forks the resumed session into the requested directory instead of losing context", + () => + Effect.gen(function* () { + const adapter = yield* OpenCodeAdapter; + const threadId = asThreadId("thread-opencode-cwd"); + // The persisted session still exists but was created in another working dir + // (e.g. the thread moved from the project root into a git worktree). + runtimeMock.state.sessionDirectoryById.set("ses_otherdir", "/some/other/worktree"); - const session = yield* adapter.startSession({ - provider: ProviderDriverKind.make("opencode"), - threadId, - runtimeMode: "full-access", - resumeCursor: { schemaVersion: 1, sessionId: "ses_otherdir" }, - }); + const session = yield* adapter.startSession({ + provider: ProviderDriverKind.make("opencode"), + threadId, + runtimeMode: "full-access", + resumeCursor: { schemaVersion: 1, sessionId: "ses_otherdir" }, + }); - // OpenCode routes prompts to the session's stored directory, so a cwd - // change must NOT silently reuse the old-directory session — create fresh. - NodeAssert.deepEqual(runtimeMock.state.sessionGetIds, ["ses_otherdir"]); - NodeAssert.deepEqual(runtimeMock.state.sessionCreateUrls, ["http://127.0.0.1:9999"]); - NodeAssert.deepEqual(runtimeMock.state.sessionUpdateCalls, []); - NodeAssert.deepEqual(session.resumeCursor, { - schemaVersion: 1, - sessionId: "http://127.0.0.1:9999/session", - }); + // A cwd change must NOT mint an empty session and drop context. OpenCode routes + // tools by the request directory, so the adapter FORKS the persisted session into + // the requested cwd — carrying all prior messages forward — instead of session.create. + NodeAssert.deepEqual(runtimeMock.state.sessionGetIds, ["ses_otherdir"]); + NodeAssert.deepEqual(runtimeMock.state.sessionCreateUrls, []); + NodeAssert.equal(runtimeMock.state.forkCalls.length, 1); + NodeAssert.equal(runtimeMock.state.forkCalls[0]?.sessionID, "ses_otherdir"); + NodeAssert.equal(typeof runtimeMock.state.forkCalls[0]?.directory, "string"); + // Permission ruleset re-asserted on the fork for the current runtimeMode. + NodeAssert.equal(runtimeMock.state.sessionUpdateCalls.length, 1); + NodeAssert.equal(runtimeMock.state.sessionUpdateCalls[0]?.sessionID, "ses_otherdir_fork"); + // Durable cursor now points at the history-complete fork in the new directory. + NodeAssert.deepEqual(session.resumeCursor, { + schemaVersion: 1, + sessionId: "ses_otherdir_fork", + }); - yield* adapter.stopSession(threadId); - }), + yield* adapter.stopSession(threadId); + }), ); it.effect("fails sendTurn for missing sessions through the typed error channel", () => diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.ts index 7972e4f1257..a82d5ca88b4 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.ts @@ -1230,11 +1230,10 @@ export function makeOpenCodeAdapter( ) : undefined; - // Reuse the upstream session only when it still matches the - // requested working directory. OpenCode routes a prompt to the - // session's OWN stored directory, so resuming a session created - // under a different cwd would silently run there — in that case - // start fresh in the requested directory instead. + // Reuse the upstream session as-is only when it still matches the + // requested working directory. When the cwd changed (e.g. the thread + // moved from the project root into a git worktree), the session is + // forked into the new directory below rather than reused in place. const reusable = adopted && (!adopted.directory || adopted.directory === directory) ? adopted @@ -1255,11 +1254,40 @@ export function makeOpenCodeAdapter( return { openCodeSession: reusable, created: false }; } + // The persisted session exists but was created under a DIFFERENT working + // directory (e.g. the thread moved from the project root into a git worktree + // between turns). OpenCode routes tool execution by the per-request `directory`, + // NOT the session's stored directory, so resuming under a new cwd does not run + // in the wrong tree. Fork the session INTO the requested directory instead of + // minting an empty one: this carries the full message history forward and binds + // the fork to the correct worktree, so the follow-up keeps its context (issue + // #3604, worktree cwd-change facet). Only a genuinely missing session starts fresh. + if (adopted) { + yield* Effect.logInfo( + `OpenCode session '${adopted.id}' was created under a different working directory; forking into '${directory}' to preserve conversation history.`, + ); + const forkedSession = yield* runOpenCodeSdk("session.fork", () => + client.session.fork({ sessionID: adopted.id, directory }), + ); + const forked = forkedSession.data; + if (!forked) { + return yield* new OpenCodeRuntimeError({ + operation: "session.fork", + detail: "OpenCode session.fork returned no session payload.", + }); + } + yield* runOpenCodeSdk("session.update", () => + client.session.update({ + sessionID: forked.id, + permission: buildOpenCodePermissionRules(input.runtimeMode), + }), + ); + return { openCodeSession: forked, created: true }; + } + if (resumeSessionId) { yield* Effect.logWarning( - adopted !== undefined - ? `OpenCode session '${resumeSessionId}' is bound to a different working directory; starting a fresh session.` - : `OpenCode session '${resumeSessionId}' no longer exists; starting a fresh session.`, + `OpenCode session '${resumeSessionId}' no longer exists; starting a fresh session.`, ); } const createdSession = yield* runOpenCodeSdk("session.create", () => From 98b1e00784a7188cf07952a7eaf4cd3987ad272e Mon Sep 17 00:00:00 2001 From: Vadym Kotai Date: Fri, 17 Jul 2026 16:12:22 +0400 Subject: [PATCH 06/10] fix(opencode): never treat a non-404 error as a missing session via its name A node carrying an explicit non-404 numeric HTTP status now seals its subtree in isOpenCodeNotFound: a 500 whose serialized body is named NotFoundError (or that is itself named UpstreamNotFoundError) propagates instead of silently falling back to session.create and dropping context. Co-authored-by: codex --- .../provider/Layers/OpenCodeAdapter.test.ts | 17 +++++++++++++ .../src/provider/Layers/OpenCodeAdapter.ts | 25 ++++++++++++------- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts index e822b4a4647..e3656f7d8b9 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts @@ -974,6 +974,23 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => { false, ); NodeAssert.equal(isOpenCodeNotFound({ detail: "status=500 body={...not found...}" }), false); + // An explicit non-404 status seals its subtree: a 500 whose serialized + // body echoes a NotFoundError name — or that is itself named + // *NotFound* — is a real failure, never a miss. + NodeAssert.equal( + isOpenCodeNotFound({ status: 500, body: { name: "NotFoundError" } }), + false, + ); + NodeAssert.equal( + isOpenCodeNotFound({ name: "UpstreamNotFoundError", status: 500 }), + false, + ); + NodeAssert.equal( + isOpenCodeNotFound( + new Error("x", { cause: { status: 502, body: { name: "NotFoundError" } } }), + ), + false, + ); // Other transient/auth/network failures must propagate too. NodeAssert.equal(isOpenCodeNotFound(new Error("boom", { cause: { status: 500 } })), false); NodeAssert.equal(isOpenCodeNotFound({ cause: { response: { status: 401 } } }), false); diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.ts index a82d5ca88b4..437c43090e0 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.ts @@ -99,7 +99,11 @@ function parseOpenCodeResume(raw: unknown): { readonly sessionId: string } | und * deliberately do NOT match free text (`message`/`detail`): those can carry a * serialized non-404 body or an unrelated "not found" phrase (e.g. a 500 whose * message says "upstream X not found"), which would misclassify a real failure - * as a missing session and silently drop context. Exported for unit testing. + * as a missing session and silently drop context. A node carrying an explicit + * non-404 numeric status seals its entire subtree: a 500 whose serialized body + * happens to be named `NotFoundError` (or that is itself named + * `UpstreamNotFoundError`) is a real failure, and neither its `name` nor + * anything it wraps may reclassify it as a miss. Exported for unit testing. */ export function isOpenCodeNotFound(cause: unknown): boolean { const seen = new Set(); @@ -112,17 +116,20 @@ export function isOpenCodeNotFound(cause: unknown): boolean { seen.add(node); const record = node as Record; - if (record.status === 404 || record.statusCode === 404) { - return true; - } const response = record.response; - if ( - response !== null && - typeof response === "object" && - (response as { readonly status?: unknown }).status === 404 - ) { + const statuses = [ + record.status, + record.statusCode, + response !== null && typeof response === "object" + ? (response as { readonly status?: unknown }).status + : undefined, + ].filter((status): status is number => typeof status === "number"); + if (statuses.includes(404)) { return true; } + if (statuses.length > 0) { + continue; + } const name = record.name; if (typeof name === "string" && name.toLowerCase().includes("notfound")) { From 269a411c8f9cac4039cf95637ba75016fccb308c Mon Sep 17 00:00:00 2001 From: Vadym Kotai Date: Sun, 19 Jul 2026 19:26:35 +0400 Subject: [PATCH 07/10] fix(opencode): compare resume directories canonically, not by raw string A trailing slash, an unnormalized segment, or a symlinked cwd (macOS /tmp -> /private/tmp) made the resume path misread the same working tree as a cwd change, forking the session and repointing the durable cursor at the clone on every resume. Compare lexically resolved forms first, then realpath both sides, each degrading to its lexical form when resolution fails (deleted directory, external-server path). Co-authored-by: codex --- .../provider/Layers/OpenCodeAdapter.test.ts | 62 +++++++++++++++++++ .../src/provider/Layers/OpenCodeAdapter.ts | 34 +++++++++- 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts index e3656f7d8b9..66b9c649da5 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts @@ -1,4 +1,8 @@ +// @effect-diagnostics nodeBuiltinImport:off import * as NodeAssert from "node:assert/strict"; +import * as NodeFSP from "node:fs/promises"; +import * as NodeOS from "node:os"; +import * as NodePath from "node:path"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { it } from "@effect/vitest"; import * as Context from "effect/Context"; @@ -32,6 +36,7 @@ import { import { appendOpenCodeAssistantTextDelta, isOpenCodeNotFound, + isSameOpenCodeDirectory, makeOpenCodeAdapter, mergeOpenCodeAssistantText, } from "./OpenCodeAdapter.ts"; @@ -519,6 +524,34 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => { }), ); + it.effect("reuses the resumed session when the stored directory differs only lexically", () => + Effect.gen(function* () { + const adapter = yield* OpenCodeAdapter; + const threadId = asThreadId("thread-opencode-samedir"); + // Same working tree, different spelling (trailing slash). A raw string + // comparison would misread this as a cwd change and fork the session, + // churning upstream sessions and the durable cursor on every resume. + runtimeMock.state.sessionDirectoryById.set("ses_samedir", `${process.cwd()}/`); + + const session = yield* adapter.startSession({ + provider: ProviderDriverKind.make("opencode"), + threadId, + runtimeMode: "full-access", + resumeCursor: { schemaVersion: 1, sessionId: "ses_samedir" }, + }); + + NodeAssert.deepEqual(runtimeMock.state.sessionGetIds, ["ses_samedir"]); + NodeAssert.deepEqual(runtimeMock.state.sessionCreateUrls, []); + NodeAssert.deepEqual(runtimeMock.state.forkCalls, []); + NodeAssert.deepEqual(session.resumeCursor, { + schemaVersion: 1, + sessionId: "ses_samedir", + }); + + yield* adapter.stopSession(threadId); + }), + ); + it.effect("fails sendTurn for missing sessions through the typed error channel", () => Effect.gen(function* () { const adapter = yield* OpenCodeAdapter; @@ -999,6 +1032,35 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => { }), ); + it.effect("treats lexically or physically identical directories as the same", () => + Effect.gen(function* () { + // Lexical-only differences (trailing slash, dot segments) short-circuit + // without touching the filesystem — the paths need not exist. + NodeAssert.equal(yield* isSameOpenCodeDirectory("/repo/project/", "/repo/project"), true); + NodeAssert.equal( + yield* isSameOpenCodeDirectory("/repo/nested/../project", "/repo/project"), + true, + ); + // Distinct nonexistent paths degrade to the lexical comparison instead + // of failing (covers directories recorded by an external OpenCode server + // that don't exist on this machine, or since-deleted worktrees). + NodeAssert.equal(yield* isSameOpenCodeDirectory("/repo/project", "/repo/other"), false); + + // A symlinked cwd (the macOS `/tmp` → `/private/tmp` shape) resolves to + // the directory it points at, so the two spellings compare equal. + const base = yield* Effect.acquireRelease( + Effect.promise(() => NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "t3-opencode-dir-"))), + (dir) => Effect.promise(() => NodeFSP.rm(dir, { recursive: true, force: true })), + ); + const real = NodePath.join(base, "real"); + const link = NodePath.join(base, "link"); + yield* Effect.promise(() => NodeFSP.mkdir(real)); + yield* Effect.promise(() => NodeFSP.symlink(real, link)); + NodeAssert.equal(yield* isSameOpenCodeDirectory(link, real), true); + NodeAssert.equal(yield* isSameOpenCodeDirectory(link, NodePath.join(base, "other")), false); + }).pipe(Effect.scoped), + ); + it.effect("appends raw assistant text deltas and reconciles part update snapshots", () => Effect.sync(() => { const firstUpdate = mergeOpenCodeAssistantText(undefined, "Hello"); diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.ts index 437c43090e0..3615c99d52b 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.ts @@ -1,3 +1,7 @@ +// @effect-diagnostics nodeBuiltinImport:off +import * as NodeFSP from "node:fs/promises"; +import * as NodePath from "node:path"; + import { EventId, type OpenCodeSettings, @@ -145,6 +149,32 @@ export function isOpenCodeNotFound(cause: unknown): boolean { return false; } +/** + * Whether the directory stored on an upstream OpenCode session and the + * requested working directory name the same location. Raw string equality + * produces false mismatches — a trailing slash, an unnormalized `.`/`..` + * segment, or a symlinked cwd (macOS `/tmp` → `/private/tmp`) — and every + * false mismatch needlessly forks the session and repoints the durable + * cursor at the clone. Lexically equal paths short-circuit without touching + * the filesystem; otherwise both sides are compared through `realpath`, each + * falling back to its lexical form when resolution fails (directory since + * deleted, or a path recorded by an external OpenCode server that doesn't + * exist on this machine). Because the lexical check runs first, the realpath + * pass can only ever turn a spurious mismatch into a match — it can never + * split paths that compare equal today. Exported for unit testing. + */ +export function isSameOpenCodeDirectory(left: string, right: string): Effect.Effect { + return Effect.promise(async () => { + const lexicalLeft = NodePath.resolve(left); + const lexicalRight = NodePath.resolve(right); + if (lexicalLeft === lexicalRight) { + return true; + } + const canonicalize = (lexical: string) => NodeFSP.realpath(lexical).catch(() => lexical); + return (await canonicalize(lexicalLeft)) === (await canonicalize(lexicalRight)); + }); +} + interface OpenCodeTurnSnapshot { readonly id: TurnId; readonly items: Array; @@ -1242,7 +1272,9 @@ export function makeOpenCodeAdapter( // moved from the project root into a git worktree), the session is // forked into the new directory below rather than reused in place. const reusable = - adopted && (!adopted.directory || adopted.directory === directory) + adopted && + (!adopted.directory || + (yield* isSameOpenCodeDirectory(adopted.directory, directory))) ? adopted : undefined; From 9d38bbd3f2c8eee8e30a97fcfa7616b28afa6cba Mon Sep 17 00:00:00 2001 From: Vadym Kotai Date: Sun, 19 Jul 2026 19:45:53 +0400 Subject: [PATCH 08/10] refactor(opencode): resolve directory comparison via Effect FileSystem/Path services Drops the nodeBuiltinImport pragmas: isSameOpenCodeDirectory now takes the FileSystem and Path services (resolved once in makeOpenCodeAdapter, already present in OpenCodeDriverEnv) instead of importing node:fs and node:path directly, and the symlink test fixture moves to makeTempDirectoryScoped/symlink on the FileSystem service. Co-authored-by: codex --- .../provider/Layers/OpenCodeAdapter.test.ts | 37 +++++++------- .../src/provider/Layers/OpenCodeAdapter.ts | 50 ++++++++++++------- 2 files changed, 48 insertions(+), 39 deletions(-) diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts index 66b9c649da5..9b3f671d70b 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts @@ -1,16 +1,14 @@ -// @effect-diagnostics nodeBuiltinImport:off import * as NodeAssert from "node:assert/strict"; -import * as NodeFSP from "node:fs/promises"; -import * as NodeOS from "node:os"; -import * as NodePath from "node:path"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { it } from "@effect/vitest"; import * as Context from "effect/Context"; import * as Effect from "effect/Effect"; import * as Exit from "effect/Exit"; import * as Fiber from "effect/Fiber"; +import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; +import * as Path from "effect/Path"; import * as Schema from "effect/Schema"; import * as Scope from "effect/Scope"; import * as Stream from "effect/Stream"; @@ -1034,30 +1032,29 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => { it.effect("treats lexically or physically identical directories as the same", () => Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const sameDirectory = (left: string, right: string) => + isSameOpenCodeDirectory(fileSystem, path, left, right); + // Lexical-only differences (trailing slash, dot segments) short-circuit // without touching the filesystem — the paths need not exist. - NodeAssert.equal(yield* isSameOpenCodeDirectory("/repo/project/", "/repo/project"), true); - NodeAssert.equal( - yield* isSameOpenCodeDirectory("/repo/nested/../project", "/repo/project"), - true, - ); + NodeAssert.equal(yield* sameDirectory("/repo/project/", "/repo/project"), true); + NodeAssert.equal(yield* sameDirectory("/repo/nested/../project", "/repo/project"), true); // Distinct nonexistent paths degrade to the lexical comparison instead // of failing (covers directories recorded by an external OpenCode server // that don't exist on this machine, or since-deleted worktrees). - NodeAssert.equal(yield* isSameOpenCodeDirectory("/repo/project", "/repo/other"), false); + NodeAssert.equal(yield* sameDirectory("/repo/project", "/repo/other"), false); // A symlinked cwd (the macOS `/tmp` → `/private/tmp` shape) resolves to // the directory it points at, so the two spellings compare equal. - const base = yield* Effect.acquireRelease( - Effect.promise(() => NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "t3-opencode-dir-"))), - (dir) => Effect.promise(() => NodeFSP.rm(dir, { recursive: true, force: true })), - ); - const real = NodePath.join(base, "real"); - const link = NodePath.join(base, "link"); - yield* Effect.promise(() => NodeFSP.mkdir(real)); - yield* Effect.promise(() => NodeFSP.symlink(real, link)); - NodeAssert.equal(yield* isSameOpenCodeDirectory(link, real), true); - NodeAssert.equal(yield* isSameOpenCodeDirectory(link, NodePath.join(base, "other")), false); + const base = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-opencode-dir-" }); + const real = path.join(base, "real"); + const link = path.join(base, "link"); + yield* fileSystem.makeDirectory(real); + yield* fileSystem.symlink(real, link); + NodeAssert.equal(yield* sameDirectory(link, real), true); + NodeAssert.equal(yield* sameDirectory(link, path.join(base, "other")), false); }).pipe(Effect.scoped), ); diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.ts index 3615c99d52b..0a8e99ecf8c 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.ts @@ -1,7 +1,3 @@ -// @effect-diagnostics nodeBuiltinImport:off -import * as NodeFSP from "node:fs/promises"; -import * as NodePath from "node:path"; - import { EventId, type OpenCodeSettings, @@ -21,6 +17,8 @@ import * as Crypto from "effect/Crypto"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as Exit from "effect/Exit"; +import * as FileSystem from "effect/FileSystem"; +import * as Path from "effect/Path"; import * as Queue from "effect/Queue"; import * as Ref from "effect/Ref"; import * as Scope from "effect/Scope"; @@ -156,23 +154,34 @@ export function isOpenCodeNotFound(cause: unknown): boolean { * segment, or a symlinked cwd (macOS `/tmp` → `/private/tmp`) — and every * false mismatch needlessly forks the session and repoints the durable * cursor at the clone. Lexically equal paths short-circuit without touching - * the filesystem; otherwise both sides are compared through `realpath`, each + * the filesystem; otherwise both sides are compared through `realPath`, each * falling back to its lexical form when resolution fails (directory since * deleted, or a path recorded by an external OpenCode server that doesn't - * exist on this machine). Because the lexical check runs first, the realpath + * exist on this machine). Because the lexical check runs first, the realPath * pass can only ever turn a spurious mismatch into a match — it can never - * split paths that compare equal today. Exported for unit testing. + * split paths that compare equal today. Takes the platform services as plain + * arguments so the adapter's methods keep their service-free signatures — the + * services are resolved once in `makeOpenCodeAdapter`. Exported for unit + * testing. */ -export function isSameOpenCodeDirectory(left: string, right: string): Effect.Effect { - return Effect.promise(async () => { - const lexicalLeft = NodePath.resolve(left); - const lexicalRight = NodePath.resolve(right); - if (lexicalLeft === lexicalRight) { - return true; - } - const canonicalize = (lexical: string) => NodeFSP.realpath(lexical).catch(() => lexical); - return (await canonicalize(lexicalLeft)) === (await canonicalize(lexicalRight)); - }); +export function isSameOpenCodeDirectory( + fileSystem: FileSystem.FileSystem, + path: Path.Path, + left: string, + right: string, +): Effect.Effect { + const lexicalLeft = path.resolve(left); + const lexicalRight = path.resolve(right); + if (lexicalLeft === lexicalRight) { + return Effect.succeed(true); + } + const canonicalize = (lexical: string) => + fileSystem.realPath(lexical).pipe(Effect.orElseSucceed(() => lexical)); + return Effect.zipWith( + canonicalize(lexicalLeft), + canonicalize(lexicalRight), + (canonicalLeft, canonicalRight) => canonicalLeft === canonicalRight, + ); } interface OpenCodeTurnSnapshot { @@ -581,6 +590,10 @@ export function makeOpenCodeAdapter( const serverConfig = yield* ServerConfig; const openCodeRuntime = yield* OpenCodeRuntime; const crypto = yield* Crypto.Crypto; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const sameDirectory = (left: string, right: string) => + isSameOpenCodeDirectory(fileSystem, path, left, right); const nativeEventLogger = options?.nativeEventLogger ?? (options?.nativeEventLogPath !== undefined @@ -1273,8 +1286,7 @@ export function makeOpenCodeAdapter( // forked into the new directory below rather than reused in place. const reusable = adopted && - (!adopted.directory || - (yield* isSameOpenCodeDirectory(adopted.directory, directory))) + (!adopted.directory || (yield* sameDirectory(adopted.directory, directory))) ? adopted : undefined; From d14ca6eb547f0ab91bc250d1ba66591f55556579 Mon Sep 17 00:00:00 2001 From: Vadym Kotai Date: Sun, 19 Jul 2026 20:00:10 +0400 Subject: [PATCH 09/10] docs(opencode): tighten resume-path comments to repo norm Comment-only: the resume doc blocks had grown to 17-23 lines while sibling adapters cap around 11; keep the constraints (structured-404-only classification, subtree sealing, fork-preserves-history, race cleanup scope) and drop the narration. Co-authored-by: codex --- .../provider/Layers/OpenCodeAdapter.test.ts | 29 ++-- .../src/provider/Layers/OpenCodeAdapter.ts | 127 ++++++------------ 2 files changed, 50 insertions(+), 106 deletions(-) diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts index 9b3f671d70b..ac87c08b0e7 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts @@ -118,10 +118,8 @@ const OpenCodeRuntimeTestDouble: OpenCodeRuntimeShape = { connectToOpenCodeServer: ({ serverUrl }) => Effect.gen(function* () { const url = serverUrl ?? "http://127.0.0.1:4301"; - // Unconditionally register a scope finalizer for test observability — - // preserves the `closeCalls` / `closeError` probes that the existing - // suites rely on. Production code never attaches a finalizer to an - // external server (it simply returns `Effect.succeed(...)`). + // Always register a finalizer so the closeCalls/closeError probes fire; + // production attaches none for external servers. yield* Effect.addFinalizer(() => Effect.sync(() => { runtimeMock.state.closeCalls.push(url); @@ -150,10 +148,8 @@ const OpenCodeRuntimeTestDouble: OpenCodeRuntimeShape = { }, get: async ({ sessionID }: { sessionID: string }) => { runtimeMock.state.sessionGetIds.push(sessionID); - // The real client is created with `throwOnError: true`, so a non-2xx - // response REJECTS (it does not resolve to a tuple). Model that: a - // transient error throws a non-404, a missing session throws a 404, - // and success resolves with the session payload. + // The real client is `throwOnError: true`: non-2xx rejects rather + // than resolving, so missing → 404 throw, transient → 500 throw. if (runtimeMock.state.transientErrorSessionIds.has(sessionID)) { throw new Error("opencode server error", { cause: { status: 500 } }); } @@ -170,8 +166,7 @@ const OpenCodeRuntimeTestDouble: OpenCodeRuntimeShape = { return { data: { id: sessionID } }; }, fork: async ({ sessionID, directory }: { sessionID: string; directory?: string }) => { - // Model OpenCode fork: clones history into a NEW session bound to the - // requested directory (all prior messages carried over upstream). + // Fork clones history into a new session bound to the directory. const forkedId = `${sessionID}_fork`; runtimeMock.state.forkCalls.push({ sessionID, ...(directory ? { directory } : {}) }); if (directory) { @@ -501,9 +496,8 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => { resumeCursor: { schemaVersion: 1, sessionId: "ses_otherdir" }, }); - // A cwd change must NOT mint an empty session and drop context. OpenCode routes - // tools by the request directory, so the adapter FORKS the persisted session into - // the requested cwd — carrying all prior messages forward — instead of session.create. + // A cwd change must not mint an empty session: the adapter forks the + // persisted session into the requested cwd, carrying history forward. NodeAssert.deepEqual(runtimeMock.state.sessionGetIds, ["ses_otherdir"]); NodeAssert.deepEqual(runtimeMock.state.sessionCreateUrls, []); NodeAssert.equal(runtimeMock.state.forkCalls.length, 1); @@ -526,9 +520,8 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => { Effect.gen(function* () { const adapter = yield* OpenCodeAdapter; const threadId = asThreadId("thread-opencode-samedir"); - // Same working tree, different spelling (trailing slash). A raw string - // comparison would misread this as a cwd change and fork the session, - // churning upstream sessions and the durable cursor on every resume. + // Same working tree, different spelling (trailing slash) — must reuse, + // not fork. runtimeMock.state.sessionDirectoryById.set("ses_samedir", `${process.cwd()}/`); const session = yield* adapter.startSession({ @@ -1041,9 +1034,7 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => { // without touching the filesystem — the paths need not exist. NodeAssert.equal(yield* sameDirectory("/repo/project/", "/repo/project"), true); NodeAssert.equal(yield* sameDirectory("/repo/nested/../project", "/repo/project"), true); - // Distinct nonexistent paths degrade to the lexical comparison instead - // of failing (covers directories recorded by an external OpenCode server - // that don't exist on this machine, or since-deleted worktrees). + // Nonexistent paths degrade to the lexical comparison instead of failing. NodeAssert.equal(yield* sameDirectory("/repo/project", "/repo/other"), false); // A symlinked cwd (the macOS `/tmp` → `/private/tmp` shape) resolves to diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.ts index 0a8e99ecf8c..9a7fad90fa0 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.ts @@ -63,12 +63,10 @@ const PROVIDER = ProviderDriverKind.make("opencode"); const OPENCODE_RESUME_VERSION = 1 as const; /** - * Decode a persisted OpenCode resume cursor back into the upstream `ses_…` - * id. Returns `undefined` for anything that isn't a current-version cursor - * carrying a non-empty session id, so a malformed or foreign cursor simply - * means "no resume" instead of throwing. OpenCode has no dedicated resume - * RPC — re-adopting the same session id *is* the resume mechanism, because - * the server scopes a conversation's history by session id. + * Decode a persisted resume cursor into the upstream `ses_…` id. Anything + * that isn't a current-version cursor with a non-empty id means "no resume" + * rather than an error. Re-adopting the session id IS the resume mechanism — + * OpenCode scopes a conversation's history by session id. */ function parseOpenCodeResume(raw: unknown): { readonly sessionId: string } | undefined { if (typeof raw !== "object" || raw === null || Array.isArray(raw)) { @@ -85,27 +83,15 @@ function parseOpenCodeResume(raw: unknown): { readonly sessionId: string } | und } /** - * Whether an error definitively reports a "session not found" — an HTTP 404 or - * an OpenCode `NotFoundError`. The SDK client is configured `throwOnError: true` - * (see `createOpenCodeSdkClient`), so `session.get` on a missing/closed session - * REJECTS rather than resolving; `runOpenCodeSdk` then surfaces it as a failed - * Effect whose `cause` wraps the original thrown `Error`, which carries the - * parsed body and HTTP status under its own `cause`. Only a confirmed miss - * justifies silently starting a fresh session — any other failure (transport, - * auth, server error) must propagate, so a momentary blip can't quietly reset a - * live thread to an empty session (the #3604 class of silent context loss). - * - * Decided only on STRUCTURED signals — a numeric 404 (`status`/`statusCode`/ - * nested `response.status`) or an explicit `NotFoundError` `name` — found via a - * bounded breadth-first walk over the error's `cause`/`body`/`error`/`data`. We - * deliberately do NOT match free text (`message`/`detail`): those can carry a - * serialized non-404 body or an unrelated "not found" phrase (e.g. a 500 whose - * message says "upstream X not found"), which would misclassify a real failure - * as a missing session and silently drop context. A node carrying an explicit - * non-404 numeric status seals its entire subtree: a 500 whose serialized body - * happens to be named `NotFoundError` (or that is itself named - * `UpstreamNotFoundError`) is a real failure, and neither its `name` nor - * anything it wraps may reclassify it as a miss. Exported for unit testing. + * Whether an error definitively reports a missing session. Only a confirmed + * miss may silently start a fresh session; any other failure (the SDK client + * is `throwOnError: true`, so `session.get` rejects on every non-2xx) must + * propagate, or a transient blip resets a live thread to an empty one — the + * #3604 silent context loss. Decides on structured signals only, never free + * text: a numeric 404 or a `NotFoundError` name, found via a bounded walk + * over `cause`/`body`/`error`/`data`. An explicit non-404 status seals its + * subtree so a wrapped "NotFound" name can't reclassify a real failure. + * Exported for unit testing. */ export function isOpenCodeNotFound(cause: unknown): boolean { const seen = new Set(); @@ -148,21 +134,14 @@ export function isOpenCodeNotFound(cause: unknown): boolean { } /** - * Whether the directory stored on an upstream OpenCode session and the - * requested working directory name the same location. Raw string equality - * produces false mismatches — a trailing slash, an unnormalized `.`/`..` - * segment, or a symlinked cwd (macOS `/tmp` → `/private/tmp`) — and every - * false mismatch needlessly forks the session and repoints the durable - * cursor at the clone. Lexically equal paths short-circuit without touching - * the filesystem; otherwise both sides are compared through `realPath`, each - * falling back to its lexical form when resolution fails (directory since - * deleted, or a path recorded by an external OpenCode server that doesn't - * exist on this machine). Because the lexical check runs first, the realPath - * pass can only ever turn a spurious mismatch into a match — it can never - * split paths that compare equal today. Takes the platform services as plain - * arguments so the adapter's methods keep their service-free signatures — the - * services are resolved once in `makeOpenCodeAdapter`. Exported for unit - * testing. + * Whether two directory spellings name the same location. Raw string + * equality misreads a trailing slash, `.`/`..` segment, or symlinked cwd + * (macOS `/tmp` → `/private/tmp`) as a cwd change, needlessly forking the + * session on every resume. Lexically equal paths short-circuit; otherwise + * both sides go through `realPath`, each falling back to its lexical form + * on failure (deleted directory, external-server path) — so the probe can + * only widen matches, never split them. Takes the services as arguments so + * adapter methods stay service-free. Exported for unit testing. */ export function isSameOpenCodeDirectory( fileSystem: FileSystem.FileSystem, @@ -1251,23 +1230,11 @@ export function makeOpenCodeAdapter( }), ); } - // Resume path: when a durable cursor names a prior OpenCode - // session, re-adopt that `ses_…` instead of minting a new one. - // OpenCode scopes history by session id, so prompting the same id - // restores the full prior conversation. - // - // The probe distinguishes three outcomes. A request that throws - // (transport/network) fails the Effect in `runOpenCodeSdk` and - // propagates — we never silently reset a live thread on a blip. - // A confirmed "not found" falls back to a fresh session. Any - // other error response (auth, bad request, server error) is - // surfaced rather than masked as a brand-new empty session. + // Resume: re-adopt the session named by the durable cursor — + // OpenCode scopes history by session id. The probe recovers only + // a confirmed not-found (start fresh); transport/auth/server + // errors propagate instead of masking as a new empty session. const resolved = yield* Effect.gen(function* () { - // Probe the persisted session. With `throwOnError: true`, - // `session.get` rejects on any non-2xx; we recover ONLY a - // confirmed "not found" (the session is genuinely gone -> start - // fresh). A transport/auth/server error propagates instead of - // masquerading as a brand-new empty session (issue #3604). const adopted = resumeSessionId ? yield* runOpenCodeSdk("session.get", () => client.session.get({ sessionID: resumeSessionId }), @@ -1280,10 +1247,8 @@ export function makeOpenCodeAdapter( ) : undefined; - // Reuse the upstream session as-is only when it still matches the - // requested working directory. When the cwd changed (e.g. the thread - // moved from the project root into a git worktree), the session is - // forked into the new directory below rather than reused in place. + // Reuse in place only when the session still matches the + // requested cwd; on a cwd change it is forked below instead. const reusable = adopted && (!adopted.directory || (yield* sameDirectory(adopted.directory, directory))) @@ -1291,11 +1256,9 @@ export function makeOpenCodeAdapter( : undefined; if (reusable) { - // `session.create` is skipped on resume, so re-assert the - // permission ruleset for the CURRENT runtimeMode — otherwise a - // runtime-mode change (the reactor restarts with the persisted - // cursor) would leave the re-adopted session on the - // permissions it was originally created with. + // Resume skips `session.create`, so re-assert the ruleset — + // a runtime-mode change would otherwise leave the session on + // its original permissions. yield* runOpenCodeSdk("session.update", () => client.session.update({ sessionID: reusable.id, @@ -1305,14 +1268,10 @@ export function makeOpenCodeAdapter( return { openCodeSession: reusable, created: false }; } - // The persisted session exists but was created under a DIFFERENT working - // directory (e.g. the thread moved from the project root into a git worktree - // between turns). OpenCode routes tool execution by the per-request `directory`, - // NOT the session's stored directory, so resuming under a new cwd does not run - // in the wrong tree. Fork the session INTO the requested directory instead of - // minting an empty one: this carries the full message history forward and binds - // the fork to the correct worktree, so the follow-up keeps its context (issue - // #3604, worktree cwd-change facet). Only a genuinely missing session starts fresh. + // The session lives under a different cwd (e.g. the thread + // moved into a git worktree). Fork it into the requested + // directory instead of minting an empty one — the fork carries + // the full history, so the follow-up keeps its context (#3604). if (adopted) { yield* Effect.logInfo( `OpenCode session '${adopted.id}' was created under a different working directory; forking into '${directory}' to preserve conversation history.`, @@ -1376,12 +1335,9 @@ export function makeOpenCodeAdapter( // and already inserted a session while we were awaiting async work. const raceWinner = sessions.get(input.threadId); if (raceWinner) { - // Another call won the race – clean up the session we just started - // and return the existing one. Only abort the remote SDK session if - // we *created* it here; a session we merely resumed is shared upstream - // state the race winner is now using, so aborting it would interrupt - // them (and `session.abort` is otherwise the wrong tool — it cancels - // an in-flight turn rather than disowning a session). + // Another call won the race — clean up. Only abort the remote + // session if we created it here; a resumed one is shared upstream + // state the winner is now using. if (started.created) { yield* runOpenCodeSdk("session.abort", () => started.client.session.abort({ @@ -1402,12 +1358,9 @@ export function makeOpenCodeAdapter( cwd: directory, ...(input.modelSelection ? { model: input.modelSelection.model } : {}), threadId: input.threadId, - // Durable binding to the upstream OpenCode session. ProviderService - // persists this into provider_session_runtime.resume_cursor_json and - // feeds it back into `startSession` on the next turn after the - // in-memory session is lost (reaper / app or server restart), which - // is what lets a follow-up continue the same conversation instead of - // silently landing in a new, empty session (issue #3604). + // ProviderService persists this cursor and feeds it back into + // `startSession` after the in-memory session is lost (reaper / + // restart), so follow-ups continue the same conversation (#3604). resumeCursor: { schemaVersion: OPENCODE_RESUME_VERSION, sessionId: started.openCodeSession.id, From 8bfeff69cb992a9a915a9140e73ba28c54f4ea7f Mon Sep 17 00:00:00 2001 From: Vadym Kotai Date: Sun, 19 Jul 2026 23:26:11 +0400 Subject: [PATCH 10/10] fix(opencode): require the exact NotFoundError name for a confirmed miss A substring match let any status-less error named *NotFound* (UpstreamNotFoundError, ProviderNotFoundError) pass as a missing session and silently start an empty one. OpenCode's API generates exactly name "NotFoundError" for every 404, so match it exactly. Co-authored-by: codex --- .../src/provider/Layers/OpenCodeAdapter.test.ts | 14 ++++++-------- apps/server/src/provider/Layers/OpenCodeAdapter.ts | 7 +++---- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts index ac87c08b0e7..9e7211dc3e8 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts @@ -1001,14 +1001,12 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => { // An explicit non-404 status seals its subtree: a 500 whose serialized // body echoes a NotFoundError name — or that is itself named // *NotFound* — is a real failure, never a miss. - NodeAssert.equal( - isOpenCodeNotFound({ status: 500, body: { name: "NotFoundError" } }), - false, - ); - NodeAssert.equal( - isOpenCodeNotFound({ name: "UpstreamNotFoundError", status: 500 }), - false, - ); + NodeAssert.equal(isOpenCodeNotFound({ status: 500, body: { name: "NotFoundError" } }), false); + NodeAssert.equal(isOpenCodeNotFound({ name: "UpstreamNotFoundError", status: 500 }), false); + // A "NotFound"-flavored name that isn't OpenCode's exact `NotFoundError` + // is not a confirmed miss even without a sealing status. + NodeAssert.equal(isOpenCodeNotFound({ name: "UpstreamNotFoundError" }), false); + NodeAssert.equal(isOpenCodeNotFound({ cause: { name: "ProviderNotFoundError" } }), false); NodeAssert.equal( isOpenCodeNotFound( new Error("x", { cause: { status: 502, body: { name: "NotFoundError" } } }), diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.ts index 9a7fad90fa0..73c23b77e68 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.ts @@ -88,7 +88,7 @@ function parseOpenCodeResume(raw: unknown): { readonly sessionId: string } | und * is `throwOnError: true`, so `session.get` rejects on every non-2xx) must * propagate, or a transient blip resets a live thread to an empty one — the * #3604 silent context loss. Decides on structured signals only, never free - * text: a numeric 404 or a `NotFoundError` name, found via a bounded walk + * text: a numeric 404 or the exact `NotFoundError` name, found via a bounded walk * over `cause`/`body`/`error`/`data`. An explicit non-404 status seals its * subtree so a wrapped "NotFound" name can't reclassify a real failure. * Exported for unit testing. @@ -120,7 +120,7 @@ export function isOpenCodeNotFound(cause: unknown): boolean { } const name = record.name; - if (typeof name === "string" && name.toLowerCase().includes("notfound")) { + if (typeof name === "string" && name.toLowerCase() === "notfounderror") { return true; } @@ -1242,7 +1242,7 @@ export function makeOpenCodeAdapter( Effect.map((response) => response.data), Effect.catchIf( (cause) => isOpenCodeNotFound(cause), - () => Effect.succeed(undefined), + () => Effect.void, ), ) : undefined; @@ -1302,7 +1302,6 @@ export function makeOpenCodeAdapter( } const createdSession = yield* runOpenCodeSdk("session.create", () => client.session.create({ - title: `T3 Code ${input.threadId}`, permission: buildOpenCodePermissionRules(input.runtimeMode), }), );