From a8d088f8ce1fcbf5ccb170869f8e9275894b3a3c Mon Sep 17 00:00:00 2001 From: CDVolvik Date: Mon, 10 Aug 2026 12:21:56 -0700 Subject: [PATCH 1/3] fix(server): stop sending the local directory to a remote OpenCode server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Connecting to an OpenCode server by URL from Windows failed to load models, with the server reporting Invalid path /var/log/C:\Users\.... The client is built with the local working directory and forwards it to whichever server is on the other end, so a Windows path reached a Linux host and was joined onto its own. The message comes from OpenCode itself; what we contribute is a path that cannot mean anything there. Only skip the directory when the server is externally configured AND its URL is not loopback. External is not the same thing as remote: running OpenCode locally and pointing the setting at http://localhost:4096 is also external, and there the directory is correct and worth sending. Dropping it for those users would trade a visible failure for a silent one, so localhost, 127.0.0.0/8, ::1 and 0.0.0.0 all keep it, as does a base URL that cannot be parsed. The client config moves into a pure buildOpenCodeSdkClientConfig so the decision is testable on its own, alongside the other exported helpers in this module. Callers pass the externality they already know: the two adapter paths forward server.external, which OpenCodeAdapter already branches on for the authorization header, and the text-generation path uses the configured serverUrl it already tests for the same purpose. Follow-up: resume was still broken. After creating a session without a directory, the adopted session's server-side directory never matched the local Windows path, so the fork path reintroduced the same Invalid path failure. For remote (external non-loopback) sessions the cwd check and fork are now skipped entirely — any adopted session is reused in place. isLoopbackBaseUrl is exported for the adapter to share the same heuristic. Three of the five new tests cover the cases that must NOT change, and they pass against the previous always-send behaviour as well, so they pin the localhost case rather than the fix. Fixes #3094 Rebased onto current main. Two things followed from the rebase: the skills path in OpenCodeDriver reaches createOpenCodeSdkClient too and now forwards server.external, and checkOpenCodeProviderStatus's existing assertions record the client input verbatim, so they carry the new field. `external` is required rather than optional on the client input. An optional flag defaulting to "local" is how a caller reintroduces this silently, which already happened once: the skills path in OpenCodeDriver landed upstream after this branch and built its client without one. The version probe in connectToOpenCodeServer runs against the configured host too, so it passes `external: true` for the same reason. Loopback detection classifies the parsed hostname instead of pattern-matching the string, which the previous `startsWith("127.")` got wrong in both directions: `127.example.com` is a domain and was read as an address, so the local path was still sent to it, and `localhost.`, `name.localhost` and `[::ffff:127.0.0.1]` are all this machine and were treated as remote. URL parsing already separates literals from names and canonicalises IPv4, so the remaining work is reading what it produced -- including that an IPv4-mapped address serialises as `::ffff:7f00:1` rather than its readable spelling. Both new cases fail against the old heuristic. --- .../src/provider/Drivers/OpenCodeDriver.ts | 1 + .../provider/Layers/OpenCodeAdapter.test.ts | 59 ++++++++++ .../src/provider/Layers/OpenCodeAdapter.ts | 42 +++++-- .../provider/Layers/OpenCodeProvider.test.ts | 4 + .../src/provider/Layers/OpenCodeProvider.ts | 2 + .../opencodeRuntime.sdkClient.test.ts | 104 ++++++++++++++++++ apps/server/src/provider/opencodeRuntime.ts | 90 ++++++++++++--- .../textGeneration/OpenCodeTextGeneration.ts | 1 + 8 files changed, 276 insertions(+), 27 deletions(-) create mode 100644 apps/server/src/provider/opencodeRuntime.sdkClient.test.ts diff --git a/apps/server/src/provider/Drivers/OpenCodeDriver.ts b/apps/server/src/provider/Drivers/OpenCodeDriver.ts index 8ff01946644c..73ddda66f544 100644 --- a/apps/server/src/provider/Drivers/OpenCodeDriver.ts +++ b/apps/server/src/provider/Drivers/OpenCodeDriver.ts @@ -165,6 +165,7 @@ export const OpenCodeDriver: ProviderDriver const client = openCodeRuntime.createOpenCodeSdkClient({ baseUrl: server.url, directory: cwd, + external: server.external, ...(effectiveConfig.serverPassword ? { serverPassword: effectiveConfig.serverPassword } : {}), diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts index 7f327cae8fb3..0737451a1090 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts @@ -440,6 +440,13 @@ const openCodeAdapterTestSettings = Schema.decodeSync(OpenCodeSettings)({ serverPassword: "secret-password", }); +// A non-loopback server URL, so the adapter treats the session as remote. +const openCodeAdapterRemoteSettings = Schema.decodeSync(OpenCodeSettings)({ + binaryPath: "fake-opencode", + serverUrl: "http://10.0.0.5:4096", + serverPassword: "secret-password", +}); + const OpenCodeAdapterTestLayer = Layer.effect( OpenCodeAdapter, makeOpenCodeAdapter(openCodeAdapterTestSettings), @@ -1112,6 +1119,58 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => { }), ); + it.effect( + "reuses a remote session without forking even when the server directory differs", + () => { + const remoteLayer = Layer.effect( + OpenCodeAdapter, + makeOpenCodeAdapter(openCodeAdapterRemoteSettings), + ).pipe( + Layer.provideMerge(Layer.succeed(OpenCodeRuntime, OpenCodeRuntimeTestDouble)), + Layer.provideMerge(ServerConfig.layerTest(process.cwd(), process.cwd())), + Layer.provideMerge( + ServerSettingsService.layerTest({ + providers: { + opencode: { + binaryPath: "fake-opencode", + serverUrl: "http://10.0.0.5:4096", + serverPassword: "secret-password", + }, + }, + }), + ), + Layer.provideMerge(providerSessionDirectoryTestLayer), + Layer.provideMerge(NodeServices.layer), + ); + + return Effect.gen(function* () { + const adapter = yield* OpenCodeAdapter; + // Remote Linux directory will never equal the local Windows cwd — without + // the remote short-circuit this would fork and re-send the Windows path (#3094). + runtimeMock.state.sessionDirectoryById.set("ses_remote", "/var/log"); + + const session = yield* adapter.startSession({ + provider: ProviderDriverKind.make("opencode"), + threadId: asThreadId("thread-opencode-remote-reuse"), + runtimeMode: "full-access", + resumeCursor: { schemaVersion: 1, sessionId: "ses_remote" }, + }); + + NodeAssert.deepEqual(runtimeMock.state.sessionGetIds, ["ses_remote"]); + NodeAssert.deepEqual(runtimeMock.state.sessionCreateUrls, []); + NodeAssert.deepEqual(runtimeMock.state.forkCalls, []); + NodeAssert.deepEqual(session.resumeCursor, { + schemaVersion: 1, + sessionId: "ses_remote", + }); + NodeAssert.equal(runtimeMock.state.sessionUpdateCalls.length, 1); + NodeAssert.equal(runtimeMock.state.sessionUpdateCalls[0]?.sessionID, "ses_remote"); + + yield* adapter.stopSession(asThreadId("thread-opencode-remote-reuse")); + }).pipe(Effect.provide(remoteLayer)); + }, + ); + 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 d0b4f0de78ce..e10e8b37cd3d 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.ts @@ -44,6 +44,7 @@ import { import { type OpenCodeAdapterShape } from "../Services/OpenCodeAdapter.ts"; import { buildOpenCodePermissionRules, + isLoopbackBaseUrl, OpenCodeRuntime, OpenCodeRuntimeError, openCodeQuestionId, @@ -2407,6 +2408,7 @@ export function makeOpenCodeAdapter( const client = openCodeRuntime.createOpenCodeSdkClient({ baseUrl: server.url, directory, + external: server.external, ...(server.serverPassword ? { serverPassword: server.serverPassword } : {}), }); const mcpSession = McpProviderSession.readMcpProviderSession(input.threadId); @@ -2430,7 +2432,7 @@ export function makeOpenCodeAdapter( // 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* () { - const adopted = resumeSessionId + const adoptedRaw = resumeSessionId ? yield* runOpenCodeSdk("session.get", () => client.session.get({ sessionID: resumeSessionId }), ).pipe( @@ -2441,33 +2443,53 @@ export function makeOpenCodeAdapter( ), ) : undefined; + const adopted = (adoptedRaw ?? undefined) as + | { readonly id: string; readonly directory?: string | null } + | undefined; + + // For a remote server the local directory is meaningless — the + // client no longer sends it (#3094) and the adopted session's + // directory is a Linux path that will never equal the Windows + // one. Skip the cwd check and the fork entirely in that case + // so we don't reintroduce the Invalid path failure this PR fixes. + const isRemote = server.external && !isLoopbackBaseUrl(server.url); // 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))) - ? adopted - : undefined; + // Remote: any adopted session is reusable — directory is server-side. + const reusable = (() => { + if (!adopted) return undefined; + if (isRemote) return adopted; + if (!adopted.directory) return adopted; + return null; // need async check below + })(); + + let resolvedReusable = reusable as typeof adopted | null; + if (reusable === null) { + const same = yield* sameDirectory(adopted!.directory!, directory); + resolvedReusable = same ? adopted : undefined; + } - if (reusable) { + if (resolvedReusable) { // 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, + sessionID: resolvedReusable!.id, permission: buildOpenCodePermissionRules(input.runtimeMode), }), ); - return { openCodeSession: reusable, created: false }; + return { openCodeSession: resolvedReusable!, created: false }; } // 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) { + // Never fork a remote session — it would send the local path + // back to the server (the #3094 bug). Treat it as reusable above. + if (adopted && !isRemote) { yield* Effect.logInfo( `OpenCode session '${adopted.id}' was created under a different working directory; forking into '${directory}' to preserve conversation history.`, ); diff --git a/apps/server/src/provider/Layers/OpenCodeProvider.test.ts b/apps/server/src/provider/Layers/OpenCodeProvider.test.ts index 0c0bf0c28801..cd0f8c53e7d0 100644 --- a/apps/server/src/provider/Layers/OpenCodeProvider.test.ts +++ b/apps/server/src/provider/Layers/OpenCodeProvider.test.ts @@ -45,6 +45,7 @@ const runtimeMock = { sdkClientInputs: [] as Array<{ baseUrl: string; directory: string; + external: boolean; serverPassword?: string; }>, inventory: { @@ -372,6 +373,7 @@ it.layer(testLayer)("checkOpenCodeProviderStatus", (it) => { { baseUrl: "http://127.0.0.1:4301", directory: process.cwd(), + external: false, serverPassword: "secret-password", }, ]); @@ -390,6 +392,7 @@ it.layer(testLayer)("checkOpenCodeProviderStatus", (it) => { { baseUrl: "http://127.0.0.1:4301", directory: process.cwd(), + external: false, serverPassword: "environment-password", }, ]); @@ -438,6 +441,7 @@ it.layer(testLayer)("checkOpenCodeProviderStatus with configured server URL", (i { baseUrl: "http://127.0.0.1:9999", directory: process.cwd(), + external: true, }, ]); }), diff --git a/apps/server/src/provider/Layers/OpenCodeProvider.ts b/apps/server/src/provider/Layers/OpenCodeProvider.ts index a094fe8601a2..b19090a5e488 100644 --- a/apps/server/src/provider/Layers/OpenCodeProvider.ts +++ b/apps/server/src/provider/Layers/OpenCodeProvider.ts @@ -447,12 +447,14 @@ export const checkOpenCodeProviderStatus = Effect.fn("checkOpenCodeProviderStatu readonly url: string; readonly serverPassword?: string; readonly version: string; + readonly external?: boolean; }) => openCodeRuntime .loadOpenCodeInventory( openCodeRuntime.createOpenCodeSdkClient({ baseUrl: server.url, directory: cwd, + external: server.external ?? false, ...(server.serverPassword !== undefined ? { serverPassword: server.serverPassword } : {}), }), ) diff --git a/apps/server/src/provider/opencodeRuntime.sdkClient.test.ts b/apps/server/src/provider/opencodeRuntime.sdkClient.test.ts new file mode 100644 index 000000000000..266507dafc9c --- /dev/null +++ b/apps/server/src/provider/opencodeRuntime.sdkClient.test.ts @@ -0,0 +1,104 @@ +import * as NodeAssert from "node:assert/strict"; + +import { describe, it } from "vite-plus/test"; + +import { buildOpenCodeSdkClientConfig } from "./opencodeRuntime.ts"; + +const WINDOWS_DIRECTORY = "C:\\Users\\someone\\code\\project"; + +describe("buildOpenCodeSdkClientConfig", () => { + it("omits the directory for an external server on another host", () => { + const config = buildOpenCodeSdkClientConfig({ + baseUrl: "http://10.0.0.5:4096", + directory: WINDOWS_DIRECTORY, + external: true, + }); + + NodeAssert.equal("directory" in config, false); + NodeAssert.equal(config.baseUrl, "http://10.0.0.5:4096"); + }); + + it("keeps the directory for a managed server", () => { + const config = buildOpenCodeSdkClientConfig({ + baseUrl: "http://127.0.0.1:51234", + directory: WINDOWS_DIRECTORY, + external: false, + }); + + NodeAssert.equal(config.directory, WINDOWS_DIRECTORY); + }); + + it("keeps the directory for an external server on this machine", () => { + for (const baseUrl of [ + "http://localhost:4096", + "http://LOCALHOST:4096", + // A fully qualified `localhost` keeps its root label through URL parsing. + "http://localhost.:4096", + // RFC 6761 reserves the whole `.localhost` tree for loopback. + "http://name.localhost:4096", + "http://foo.bar.localhost:4096", + "http://127.0.0.1:4096", + // Shorthand and trailing-dot IPv4 both canonicalise to 127.0.0.1. + "http://127.1:4096", + "http://127.0.0.1.:4096", + "http://127.255.255.254:4096", + "http://[::1]:4096", + // IPv4-mapped loopback, which serialises as [::ffff:7f00:1]. + "http://[::ffff:127.0.0.1]:4096", + "http://0.0.0.0:4096", + "http://[::]:4096", + ]) { + const config = buildOpenCodeSdkClientConfig({ + baseUrl, + directory: WINDOWS_DIRECTORY, + external: true, + }); + + NodeAssert.equal(config.directory, WINDOWS_DIRECTORY, `expected directory for ${baseUrl}`); + } + }); + + it("treats a domain that merely looks like a loopback address as remote", () => { + // `127.example.com` is somebody else's server. Reading the leading `127.` as an + // address is what sends a local Windows path to it. + for (const baseUrl of [ + "http://127.example.com:4096", + "http://localhost.example.com:4096", + "http://notlocalhost:4096", + "http://127.0.0.1.example.com:4096", + ]) { + const config = buildOpenCodeSdkClientConfig({ + baseUrl, + directory: WINDOWS_DIRECTORY, + external: true, + }); + + NodeAssert.equal("directory" in config, false, `expected no directory for ${baseUrl}`); + } + }); + + it("keeps the directory when the base URL cannot be parsed", () => { + const config = buildOpenCodeSdkClientConfig({ + baseUrl: "not a url", + directory: WINDOWS_DIRECTORY, + external: true, + }); + + NodeAssert.equal(config.directory, WINDOWS_DIRECTORY); + }); + + it("still sends the authorization header when the directory is dropped", () => { + const config = buildOpenCodeSdkClientConfig({ + baseUrl: "http://build-server.internal:4096", + directory: WINDOWS_DIRECTORY, + external: true, + serverPassword: "hunter2", + }); + + NodeAssert.equal("directory" in config, false); + NodeAssert.equal( + config.headers?.Authorization, + `Basic ${Buffer.from("opencode:hunter2", "utf8").toString("base64")}`, + ); + }); +}); diff --git a/apps/server/src/provider/opencodeRuntime.ts b/apps/server/src/provider/opencodeRuntime.ts index afd806e5666e..37dfc77f497d 100644 --- a/apps/server/src/provider/opencodeRuntime.ts +++ b/apps/server/src/provider/opencodeRuntime.ts @@ -246,11 +246,7 @@ export interface OpenCodeRuntimeShape { readonly cwd?: string; readonly maxOutputBytes?: number; }) => Effect.Effect; - readonly createOpenCodeSdkClient: (input: { - readonly baseUrl: string; - readonly directory: string; - readonly serverPassword?: string; - }) => OpencodeClient; + readonly createOpenCodeSdkClient: (input: OpenCodeSdkClientInput) => OpencodeClient; readonly loadOpenCodeInventory: ( client: OpencodeClient, ) => Effect.Effect; @@ -269,6 +265,75 @@ export interface OpenCodeRuntimeShape { }) => Effect.Effect, OpenCodeRuntimeError>; } +export interface OpenCodeSdkClientInput { + readonly baseUrl: string; + readonly directory: string; + // Required, not optional: a caller that forgets it is exactly how the local + // directory reached a remote server in the first place, and an optional flag + // defaulting to "local" would let the next caller reintroduce it silently. + readonly external: boolean; + readonly serverPassword?: string; +} + +// An externally-configured server can live on another machine, where a local path means +// nothing: a Windows directory reaching a Linux host produced "Invalid path +// /var/log/C:\Users\...". A loopback URL is still this machine, so its directory stays +// meaningful — "external" is not the same thing as "remote". An unparseable URL keeps the +// directory too, so a malformed setting cannot quietly change what the server receives. +export function isLoopbackBaseUrl(baseUrl: string): boolean { + let hostname: string; + try { + hostname = new URL(baseUrl).hostname; + } catch { + return true; + } + + return isLoopbackHostname(hostname.toLowerCase()); +} + +// Classify the parsed hostname rather than pattern-match the string. `URL` has already +// done the hard part: it brackets IPv6 literals, canonicalises IPv4 ones (`127.1` and +// `127.0.0.1.` both arrive as `127.0.0.1`), and leaves domains alone — so `127.example.com` +// stays a name and must not be read as a `127.` address. Getting that backwards sends the +// local path to someone else's server, which is the bug this module exists to fix. +function isLoopbackHostname(hostname: string): boolean { + if (hostname.startsWith("[") && hostname.endsWith("]")) { + const address = hostname.slice(1, -1); + // An IPv4-mapped address is serialised in hex, so `::ffff:127.0.0.1` arrives as + // `::ffff:7f00:1`. Match the mapped 127.0.0.0/8 range, not the readable spelling. + return ( + address === "::1" || address === "::" || /^::ffff:7f[\da-f]{2}:[\da-f]{1,4}$/u.test(address) + ); + } + + const octets = /^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/u.exec(hostname); + if (octets) { + // Only a real IPv4 literal reaches here, so the first octet can be trusted. + return octets[1] === "127" || hostname === "0.0.0.0"; + } + + // RFC 6761 reserves `localhost` and everything under it for loopback. `URL` keeps the + // root label on a domain, so `localhost.` arrives with its trailing dot still attached. + const name = hostname.endsWith(".") ? hostname.slice(0, -1) : hostname; + return name === "localhost" || name.endsWith(".localhost"); +} + +export function buildOpenCodeSdkClientConfig(input: OpenCodeSdkClientInput) { + const sendDirectory = !input.external || isLoopbackBaseUrl(input.baseUrl); + return { + baseUrl: input.baseUrl, + ...(sendDirectory ? { directory: input.directory } : {}), + ...(input.serverPassword + ? { + headers: { + Authorization: `Basic ${Buffer.from(`opencode:${input.serverPassword}`, "utf8").toString("base64")}`, + }, + } + : {}), + throwOnError: true as const, + }; +} + function parseServerUrlFromOutput(output: string): string | null { for (const line of output.split("\n")) { if (!line.startsWith(OPENCODE_SERVER_READY_PREFIX)) { @@ -614,18 +679,7 @@ const makeOpenCodeRuntime = Effect.gen(function* () { ); const createOpenCodeSdkClient: OpenCodeRuntimeShape["createOpenCodeSdkClient"] = (input) => - createOpencodeClient({ - baseUrl: input.baseUrl, - directory: input.directory, - ...(input.serverPassword - ? { - headers: { - Authorization: `Basic ${Buffer.from(`opencode:${input.serverPassword}`, "utf8").toString("base64")}`, - }, - } - : {}), - throwOnError: true, - }); + createOpencodeClient(buildOpenCodeSdkClientConfig(input)); const startOpenCodeServerProcess: OpenCodeRuntimeShape["startOpenCodeServerProcess"] = (input) => Effect.gen(function* () { @@ -794,6 +848,7 @@ const makeOpenCodeRuntime = Effect.gen(function* () { createOpenCodeSdkClient({ baseUrl: url, directory: input.directory, + external: false, ...(serverPassword !== undefined ? { serverPassword } : {}), }), ); @@ -821,6 +876,7 @@ const makeOpenCodeRuntime = Effect.gen(function* () { createOpenCodeSdkClient({ baseUrl: serverUrl, directory: input.directory, + external: true, ...(serverPassword !== undefined ? { serverPassword } : {}), }), ).pipe( diff --git a/apps/server/src/textGeneration/OpenCodeTextGeneration.ts b/apps/server/src/textGeneration/OpenCodeTextGeneration.ts index e0e960422b18..162a7d11131d 100644 --- a/apps/server/src/textGeneration/OpenCodeTextGeneration.ts +++ b/apps/server/src/textGeneration/OpenCodeTextGeneration.ts @@ -209,6 +209,7 @@ export const makeOpenCodeTextGeneration = Effect.fn("makeOpenCodeTextGeneration" const client = openCodeRuntime.createOpenCodeSdkClient({ baseUrl: server.url, directory: input.cwd, + external: openCodeSettings.serverUrl.length > 0, ...(server.serverPassword !== undefined ? { serverPassword: server.serverPassword } : {}), }); const session = yield* Effect.tryPromise({ From 142afbfacc8fa98b78ebac1851e4c862e2371eaa Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Sat, 5 Sep 2026 01:49:16 -0700 Subject: [PATCH 2/3] fix(server): verify remote OpenCode routing across callers --- .../src/provider/Drivers/OpenCodeDriver.ts | 1 + .../src/provider/Layers/OpenCodeAdapter.ts | 33 +- .../provider/opencodeRuntime.requests.test.ts | 322 ++++++++++++++++++ 3 files changed, 335 insertions(+), 21 deletions(-) create mode 100644 apps/server/src/provider/opencodeRuntime.requests.test.ts diff --git a/apps/server/src/provider/Drivers/OpenCodeDriver.ts b/apps/server/src/provider/Drivers/OpenCodeDriver.ts index 02bb13235084..f973044f38da 100644 --- a/apps/server/src/provider/Drivers/OpenCodeDriver.ts +++ b/apps/server/src/provider/Drivers/OpenCodeDriver.ts @@ -193,6 +193,7 @@ export const OpenCodeDriver: ProviderDriver openCodeRuntime.createOpenCodeSdkClient({ baseUrl: server.url, directory: cwd, + external: false, ...(server.serverPassword !== undefined ? { serverPassword: server.serverPassword } : {}), diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.ts index 5d775086d3b1..c1ee16b139ed 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.ts @@ -2836,7 +2836,7 @@ export function makeOpenCodeAdapter( // 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* () { - const adoptedRaw = resumeSessionId + const adopted = resumeSessionId ? yield* runOpenCodeSdk("session.get", () => client.session.get({ sessionID: resumeSessionId }), ).pipe( @@ -2847,10 +2847,6 @@ export function makeOpenCodeAdapter( ), ) : undefined; - const adopted = (adoptedRaw ?? undefined) as - | { readonly id: string; readonly directory?: string | null } - | undefined; - // For a remote server the local directory is meaningless — the // client no longer sends it (#3094) and the adopted session's // directory is a Linux path that will never equal the Windows @@ -2861,30 +2857,25 @@ export function makeOpenCodeAdapter( // Reuse in place only when the session still matches the // requested cwd; on a cwd change it is forked below instead. // Remote: any adopted session is reusable — directory is server-side. - const reusable = (() => { - if (!adopted) return undefined; - if (isRemote) return adopted; - if (!adopted.directory) return adopted; - return null; // need async check below - })(); - - let resolvedReusable = reusable as typeof adopted | null; - if (reusable === null) { - const same = yield* sameDirectory(adopted!.directory!, directory); - resolvedReusable = same ? adopted : undefined; - } - - if (resolvedReusable) { + const reusable = + adopted && + (isRemote || + !adopted.directory || + (yield* sameDirectory(adopted.directory, directory))) + ? adopted + : undefined; + + if (reusable) { // 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: resolvedReusable!.id, + sessionID: reusable.id, permission: buildOpenCodePermissionRules(input.runtimeMode), }), ); - return { openCodeSession: resolvedReusable!, created: false }; + return { openCodeSession: reusable, created: false }; } // The session lives under a different cwd (e.g. the thread diff --git a/apps/server/src/provider/opencodeRuntime.requests.test.ts b/apps/server/src/provider/opencodeRuntime.requests.test.ts new file mode 100644 index 000000000000..f1fb9971376f --- /dev/null +++ b/apps/server/src/provider/opencodeRuntime.requests.test.ts @@ -0,0 +1,322 @@ +import * as NodeAssert from "node:assert/strict"; +import * as NodeServices from "@effect/platform-node/NodeServices"; +import { it } from "@effect/vitest"; +import { + OpenCodeSettings, + ProviderDriverKind, + ProviderInstanceId, + ThreadId, +} from "@t3tools/contracts"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; +import * as Schema from "effect/Schema"; +import { ChildProcessSpawner } from "effect/unstable/process"; +import { HttpClient } from "effect/unstable/http"; +import { afterEach, beforeEach, vi } from "vite-plus/test"; + +import { checkOpenCodeProviderStatus } from "./Layers/OpenCodeProvider.ts"; +import { makeOpenCodeAdapter } from "./Layers/OpenCodeAdapter.ts"; +import * as BackgroundPolicy from "../background/BackgroundPolicy.ts"; +import { ServerConfig } from "../config.ts"; +import { ServerSettingsService } from "../serverSettings.ts"; +import { OpenCodeDriver } from "./Drivers/OpenCodeDriver.ts"; +import { NoOpProviderEventLoggers, ProviderEventLoggers } from "./Layers/ProviderEventLoggers.ts"; +import { OpenCodeServerOwner } from "./OpenCodeServerOwner.ts"; +import { OpenCodeRuntime, OpenCodeRuntimeLive } from "./opencodeRuntime.ts"; + +const directory = "C:\\Users\\example\\project"; +const decodeOpenCodeSettings = Schema.decodeEffect(OpenCodeSettings); +const requests: Request[] = []; +function assertRequestDirectories(expectedDirectory: string | null) { + for (const request of requests) { + const readRequest = request.method === "GET" || request.method === "HEAD"; + const url = new URL(request.url); + const queryDirectory = readRequest || url.pathname.endsWith("/fork"); + NodeAssert.equal( + url.searchParams.get("directory"), + queryDirectory ? expectedDirectory : null, + request.url, + ); + NodeAssert.equal( + request.headers.get("x-opencode-directory"), + !readRequest && expectedDirectory !== null ? encodeURIComponent(expectedDirectory) : null, + request.url, + ); + NodeAssert.equal( + request.headers.get("authorization"), + `Basic ${btoa("opencode:audit-only-password")}`, + ); + } +} +const noSpawn = ChildProcessSpawner.make(() => Effect.die("This request test must not spawn")); +const testLayer = OpenCodeRuntimeLive.pipe( + Layer.provide(Layer.succeed(ChildProcessSpawner.ChildProcessSpawner, noSpawn)), + Layer.provideMerge(NodeServices.layer), + Layer.provideMerge( + Layer.succeed(OpenCodeServerOwner, { + withServer: () => Effect.die("Configured external server must not acquire a local server"), + }), + ), +); + +const driverLayer = ServerConfig.layerTest(process.cwd(), { + prefix: "opencode-request-test-", +}).pipe( + Layer.provideMerge(testLayer), + Layer.provideMerge(ServerSettingsService.layerTest({ enableProviderUpdateChecks: false })), + Layer.provideMerge(Layer.succeed(ProviderEventLoggers, NoOpProviderEventLoggers)), + Layer.provideMerge( + Layer.mock(BackgroundPolicy.BackgroundPolicy)({ + shouldRunScopeWork: () => Effect.succeed(false), + }), + ), + Layer.provideMerge( + Layer.succeed( + HttpClient.HttpClient, + HttpClient.make(() => Effect.die("No external metadata requests are allowed")), + ), + ), +); + +beforeEach(() => { + requests.length = 0; + vi.stubGlobal("fetch", async (input: string | URL | Request) => { + const request = input instanceof Request ? input : new Request(input.toString()); + requests.push(request); + const pathname = new URL(request.url).pathname; + switch (pathname) { + case "/proxy/global/health": + return Response.json({ healthy: true, version: "1.14.19" }); + case "/proxy/provider": + return Response.json({ connected: [], all: [], default: {} }); + case "/proxy/agent": + return Response.json([]); + case "/proxy/skill": + return Response.json([ + { + name: "audit-skill", + location: "/server/skills/audit/SKILL.md", + description: "Synthetic skill", + }, + ]); + case "/proxy/session": + return Response.json({ id: "ses_audit_commit" }); + case "/proxy/session/ses_audit_commit/message": + return Response.json({ + parts: [ + { + type: "text", + text: JSON.stringify({ subject: "Audit request routing", body: "Synthetic result." }), + }, + ], + }); + case "/proxy/session/ses_remote": + return Response.json({ id: "ses_remote", directory: "/var/log" }); + case "/proxy/session/ses_forked": + case "/proxy/session/ses_remote/fork": + return Response.json({ id: "ses_forked", directory }); + case "/proxy/permission": + case "/proxy/question": + case "/proxy/session/ses_remote/children": + case "/proxy/session/ses_forked/children": + return Response.json([]); + case "/proxy/session/ses_remote/abort": + case "/proxy/session/ses_forked/abort": + return Response.json(true); + case "/proxy/event": { + let closed = false; + return new Response( + new ReadableStream({ + start(controller) { + controller.enqueue( + new TextEncoder().encode('data: {"type":"server.connected","properties":{}}\n\n'), + ); + request.signal.addEventListener( + "abort", + () => { + if (!closed) { + closed = true; + controller.close(); + } + }, + { once: true }, + ); + }, + cancel() { + closed = true; + }, + }), + { headers: { "Content-Type": "text/event-stream" } }, + ); + } + default: + throw new Error(`Unexpected intercepted request ${request.method} ${pathname}`); + } + }); +}); + +it.layer(driverLayer)("OpenCode driver SDK requests", (it) => { + for (const [label, serverUrl, expectedDirectory] of [ + ["remote", "http://opencode.example.test/proxy", null], + ["external loopback", "http://localhost:4096/proxy", directory], + ["managed", "", directory], + ] as const) { + for (const operation of ["skills", "text generation"] as const) { + it.effect(`routes ${operation} requests for ${label}`, () => + Effect.gen(function* () { + const runtime = yield* OpenCodeRuntime; + const instance = yield* OpenCodeDriver.create({ + instanceId: ProviderInstanceId.make("opencode-directory-test"), + displayName: "OpenCode fixture", + enabled: true, + environment: [], + config: yield* decodeOpenCodeSettings({ + enabled: true, + binaryPath: "/nonexistent/opencode-request-fixture", + serverUrl, + serverPassword: "audit-only-password", + }), + }).pipe( + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, noSpawn), + Effect.provideService(OpenCodeRuntime, { + ...runtime, + runOpenCodeCommand: () => + Effect.succeed({ stdout: "opencode 1.14.19\n", stderr: "", code: 0 }), + startOpenCodeServerProcess: () => + Effect.succeed({ + url: "http://127.0.0.1:4096/proxy", + serverPassword: "audit-only-password", + version: "1.14.19", + isRunning: Effect.succeed(true), + exitCode: Effect.never, + }), + }), + ); + // Wait for the snapshot's refresh semaphore before observing workspace requests. + yield* instance.snapshot.refresh; + requests.length = 0; + if (operation === "skills") { + NodeAssert.ok(instance.snapshotForCwd); + const snapshot = yield* instance.snapshotForCwd(directory); + NodeAssert.deepEqual( + snapshot.skills.map((skill) => skill.name), + ["audit-skill"], + ); + NodeAssert.ok( + requests.some((request) => new URL(request.url).pathname === "/proxy/skill"), + ); + } else { + const result = yield* instance.textGeneration.generateCommitMessage({ + cwd: directory, + branch: "audit/request-routing", + stagedSummary: "M README.md", + stagedPatch: "synthetic fixture patch", + modelSelection: { instanceId: instance.instanceId, model: "openai/audit-model" }, + }); + NodeAssert.equal(result.subject, "Audit request routing"); + NodeAssert.ok( + requests.some( + (request) => + new URL(request.url).pathname === "/proxy/session/ses_audit_commit/message", + ), + ); + } + assertRequestDirectories(expectedDirectory); + }).pipe(Effect.scoped), + ); + } + } +}); + +it.layer(driverLayer)("OpenCode resume request routing", (it) => { + for (const [serverUrl, expectedSession, expectedDirectory] of [ + ["http://10.0.0.5:4096/proxy", "ses_remote", null], + ["http://localhost:4096/proxy", "ses_forked", directory], + ] as const) { + it.effect(`preserves remote adoption or local cwd fork at ${serverUrl}`, () => + Effect.gen(function* () { + const settings = yield* decodeOpenCodeSettings({ + enabled: true, + serverUrl, + serverPassword: "audit-only-password", + }); + const adapter = yield* makeOpenCodeAdapter(settings); + const threadId = ThreadId.make("remote-directory-resume-test"); + const session = yield* adapter.startSession({ + provider: ProviderDriverKind.make("opencode"), + threadId, + runtimeMode: "full-access", + cwd: directory, + resumeCursor: { schemaVersion: 1, sessionId: "ses_remote" }, + }); + NodeAssert.deepEqual(session.resumeCursor, { + schemaVersion: 1, + sessionId: expectedSession, + }); + const forkRequests = requests.filter((request) => + new URL(request.url).pathname.endsWith("/fork"), + ); + NodeAssert.equal(forkRequests.length, expectedSession === "ses_forked" ? 1 : 0); + NodeAssert.equal( + requests.some( + (request) => + request.method === "POST" && new URL(request.url).pathname === "/proxy/session", + ), + false, + ); + const update = requests.find( + (request) => + request.method === "PATCH" && + new URL(request.url).pathname === `/proxy/session/${expectedSession}`, + ); + NodeAssert.ok(update); + const updateBody = yield* Effect.promise(() => update.clone().json()); + NodeAssert.deepEqual(updateBody, { + permission: [ + { permission: "*", pattern: "*", action: "allow" }, + { permission: "external_directory", pattern: "*", action: "allow" }, + ], + }); + assertRequestDirectories(expectedDirectory); + const eventRequest = requests.find( + (request) => new URL(request.url).pathname === "/proxy/event", + ); + NodeAssert.ok(eventRequest); + yield* adapter.stopSession(threadId); + NodeAssert.equal(eventRequest.signal.aborted, true); + assertRequestDirectories(expectedDirectory); + }).pipe(Effect.scoped), + ); + } +}); + +afterEach(() => vi.unstubAllGlobals()); + +it.layer(testLayer)("OpenCode remote directory requests", (it) => { + for (const serverUrl of [ + "http://opencode.example.test/proxy", + "http://10.0.0.5:4096/proxy", + "http://localhost:4096/proxy", + "http://127.0.0.1:4096/proxy", + ]) { + const isLoopback = serverUrl.includes("localhost") || serverUrl.includes("127.0.0.1"); + it.effect(`routes actual initial health and inventory requests at ${serverUrl}`, () => + Effect.gen(function* () { + const settings = yield* decodeOpenCodeSettings({ + enabled: true, + serverUrl, + serverPassword: "audit-only-password", + }); + const snapshot = yield* checkOpenCodeProviderStatus(settings, directory); + NodeAssert.equal(snapshot.version, "1.14.19", snapshot.message ?? undefined); + NodeAssert.deepEqual(requests.map((request) => new URL(request.url).pathname).toSorted(), [ + "/proxy/agent", + "/proxy/global/health", + "/proxy/provider", + "/proxy/skill", + ]); + assertRequestDirectories(isLoopback ? directory : null); + }), + ); + } +}); From 1b223d3cf867720484aa6ea7fba8f9c8e39ec491 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Sat, 5 Sep 2026 01:57:45 -0700 Subject: [PATCH 3/3] docs(server): clarify OpenCode directory routing policy --- .../provider/Layers/OpenCodeAdapter.test.ts | 3 +-- .../src/provider/Layers/OpenCodeAdapter.ts | 13 +++---------- apps/server/src/provider/opencodeRuntime.ts | 19 ++++++------------- 3 files changed, 10 insertions(+), 25 deletions(-) diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts index 564567b8ba2d..57dfd0d70eea 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.test.ts @@ -1273,8 +1273,7 @@ it.layer(OpenCodeAdapterTestLayer)("OpenCodeAdapterLive", (it) => { return Effect.gen(function* () { const adapter = yield* OpenCodeAdapter; - // Remote Linux directory will never equal the local Windows cwd — without - // the remote short-circuit this would fork and re-send the Windows path (#3094). + // The server-side directory differs from this test's local cwd. runtimeMock.state.sessionDirectoryById.set("ses_remote", "/var/log"); const session = yield* adapter.startSession({ diff --git a/apps/server/src/provider/Layers/OpenCodeAdapter.ts b/apps/server/src/provider/Layers/OpenCodeAdapter.ts index c1ee16b139ed..ed50864560f2 100644 --- a/apps/server/src/provider/Layers/OpenCodeAdapter.ts +++ b/apps/server/src/provider/Layers/OpenCodeAdapter.ts @@ -2847,16 +2847,11 @@ export function makeOpenCodeAdapter( ), ) : undefined; - // For a remote server the local directory is meaningless — the - // client no longer sends it (#3094) and the adopted session's - // directory is a Linux path that will never equal the Windows - // one. Skip the cwd check and the fork entirely in that case - // so we don't reintroduce the Invalid path failure this PR fixes. + // Non-loopback external servers retain their own session directory. const isRemote = server.external && !isLoopbackBaseUrl(server.url); - // Reuse in place only when the session still matches the - // requested cwd; on a cwd change it is forked below instead. - // Remote: any adopted session is reusable — directory is server-side. + // Remote sessions keep their server-side cwd. Local sessions need a + // matching cwd to be reused; otherwise they are forked below. const reusable = adopted && (isRemote || @@ -2882,8 +2877,6 @@ export function makeOpenCodeAdapter( // 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). - // Never fork a remote session — it would send the local path - // back to the server (the #3094 bug). Treat it as reusable above. if (adopted && !isRemote) { yield* Effect.logInfo( `OpenCode session '${adopted.id}' was created under a different working directory; forking into '${directory}' to preserve conversation history.`, diff --git a/apps/server/src/provider/opencodeRuntime.ts b/apps/server/src/provider/opencodeRuntime.ts index 66f6f9a90b99..d7b2b7481f4a 100644 --- a/apps/server/src/provider/opencodeRuntime.ts +++ b/apps/server/src/provider/opencodeRuntime.ts @@ -269,18 +269,14 @@ export interface OpenCodeRuntimeShape { export interface OpenCodeSdkClientInput { readonly baseUrl: string; readonly directory: string; - // Required, not optional: a caller that forgets it is exactly how the local - // directory reached a remote server in the first place, and an optional flag - // defaulting to "local" would let the next caller reintroduce it silently. + // Callers must explicitly identify managed or externally configured servers. readonly external: boolean; readonly serverPassword?: string; } -// An externally-configured server can live on another machine, where a local path means -// nothing: a Windows directory reaching a Linux host produced "Invalid path -// /var/log/C:\Users\...". A loopback URL is still this machine, so its directory stays -// meaningful — "external" is not the same thing as "remote". An unparseable URL keeps the -// directory too, so a malformed setting cannot quietly change what the server receives. +// Directory routing treats non-loopback external URLs as remote. This hostname policy +// cannot distinguish SSH tunnels or same-host LAN URLs. Malformed URLs retain the +// local-directory policy. export function isLoopbackBaseUrl(baseUrl: string): boolean { let hostname: string; try { @@ -292,11 +288,8 @@ export function isLoopbackBaseUrl(baseUrl: string): boolean { return isLoopbackHostname(hostname.toLowerCase()); } -// Classify the parsed hostname rather than pattern-match the string. `URL` has already -// done the hard part: it brackets IPv6 literals, canonicalises IPv4 ones (`127.1` and -// `127.0.0.1.` both arrive as `127.0.0.1`), and leaves domains alone — so `127.example.com` -// stays a name and must not be read as a `127.` address. Getting that backwards sends the -// local path to someone else's server, which is the bug this module exists to fix. +// URL brackets IPv6 literals and normalizes IPv4 spellings such as `127.1`. +// Domains such as `127.example.com` remain names, not IPv4 literals. function isLoopbackHostname(hostname: string): boolean { if (hostname.startsWith("[") && hostname.endsWith("]")) { const address = hostname.slice(1, -1);