diff --git a/plugins/opencode/scripts/lib/opencode-server.mjs b/plugins/opencode/scripts/lib/opencode-server.mjs index 786f492..182f68f 100644 --- a/plugins/opencode/scripts/lib/opencode-server.mjs +++ b/plugins/opencode/scripts/lib/opencode-server.mjs @@ -225,6 +225,7 @@ export class OpencodeServerClient { this.baseUrl = trimBaseUrl(baseUrl); this.fetch = options.fetch ?? globalThis.fetch; this.authorization = buildBasicAuthHeader(options); + this.directory = typeof options.directory === "string" && options.directory ? options.directory : null; if (!this.baseUrl) { throw new Error("OpenCode server URL is required."); } @@ -235,7 +236,16 @@ export class OpencodeServerClient { url(path) { const suffix = String(path ?? "").startsWith("/") ? path : `/${path}`; - return `${this.baseUrl}${suffix}`; + const base = `${this.baseUrl}${suffix}`; + // Bind project-scoped requests to the invoking workspace. Without the + // `directory` query an external server resolves them against its own + // launch directory (issue #29). Every non-/global/ route the client uses + // accepts it (verified against the 1.17.15 OpenAPI document). + if (!this.directory || suffix.startsWith("/global/")) { + return base; + } + const separator = suffix.includes("?") ? "&" : "?"; + return `${base}${separator}directory=${encodeURIComponent(this.directory)}`; } authHeaders() { diff --git a/plugins/opencode/scripts/lib/opencode.mjs b/plugins/opencode/scripts/lib/opencode.mjs index a43b0de..93a8066 100644 --- a/plugins/opencode/scripts/lib/opencode.mjs +++ b/plugins/opencode/scripts/lib/opencode.mjs @@ -1040,12 +1040,29 @@ async function captureTurn(client, sessionID, startRequest, options = {}) { } } +// The canonical (symlink-resolved) workspace path is what OpenCode records as +// a session's directory, so use it for the client's directory scope. +function canonicalWorkspaceDirectory(cwd) { + try { + return fs.realpathSync(cwd); + } catch { + return cwd; + } +} + +function buildServerClient(cwd, server) { + return new OpencodeServerClient(server.url, { + ...serverSessionCredentials(server), + directory: canonicalWorkspaceDirectory(cwd) + }); +} + async function withServer(cwd, fn) { const server = await ensureServer(cwd); if (!server?.url) { throw new Error("OpenCode server did not become ready."); } - return fn(new OpencodeServerClient(server.url, serverSessionCredentials(server)), server); + return fn(buildServerClient(cwd, server), server); } // A job record only stores the server URL, so a later cancel process must @@ -1066,11 +1083,14 @@ function resolveCredentialsForServerUrl(cwd, serverUrl) { return { password: null, username: undefined }; } -async function abortSessionAtUrl(serverUrl, threadId, timeoutMs = 1000, credentials = {}) { +async function abortSessionAtUrl(serverUrl, threadId, timeoutMs = 1000, credentials = {}, directory = null) { const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), timeoutMs); try { - const client = new OpencodeServerClient(serverUrl, credentials); + const client = new OpencodeServerClient(serverUrl, { + ...credentials, + ...(directory ? { directory } : {}) + }); await client.abort(threadId, { signal: controller.signal }); } finally { clearTimeout(timeout); @@ -1142,14 +1162,18 @@ export function getAvailability(cwd) { } export function getSessionRuntimeStatus(env = process.env, cwd = process.cwd()) { - const url = env?.[SERVER_URL_ENV] ?? loadServerSession(cwd)?.url ?? null; + const envUrl = env?.[SERVER_URL_ENV] ?? null; + const url = envUrl ?? loadServerSession(cwd)?.url ?? null; if (url) { return { mode: "shared", label: "shared OpenCode server", detail: "This Claude session is configured to reuse one shared OpenCode server.", endpoint: url, - url + url, + // Env-configured servers are user-managed; a persisted server session is + // one the plugin spawned and owns. + external: Boolean(envUrl) }; } @@ -1158,7 +1182,8 @@ export function getSessionRuntimeStatus(env = process.env, cwd = process.cwd()) label: "lazy OpenCode server startup", detail: "No shared OpenCode server is active yet. The first review or task command will start one on demand.", endpoint: null, - url: null + url: null, + external: false }; } @@ -1205,41 +1230,48 @@ export async function getAuthStatus(cwd) { } } -export async function interruptServerTurn(cwd, { threadId, serverUrl = null }) { +// `serverExternal` is the ownership flag persisted in the job record when the +// job started. Historical ownership must never be inferred from the current +// process environment — a cancel process without the job-start env would +// otherwise mistake a user-managed server for a plugin-owned one (issue #29). +export async function interruptServerTurn(cwd, { threadId, serverUrl = null, serverExternal = null }) { + const ownership = typeof serverExternal === "boolean" ? { serverExternal } : {}; if (!threadId) { - const serverExternal = - serverUrl && normalizeServerUrlForCompare(serverUrl) === normalizeServerUrlForCompare(process.env[SERVER_URL_ENV]); return { attempted: false, interrupted: false, transport: null, detail: "missing OpenCode session id", ...(serverUrl ? { serverUrl } : {}), - ...(serverExternal ? { serverExternal: true } : {}) + ...ownership }; } if (serverUrl) { try { - await abortSessionAtUrl(serverUrl, threadId, 1000, resolveCredentialsForServerUrl(cwd, serverUrl)); - const serverExternal = normalizeServerUrlForCompare(serverUrl) === normalizeServerUrlForCompare(process.env[SERVER_URL_ENV]); + await abortSessionAtUrl( + serverUrl, + threadId, + 1000, + resolveCredentialsForServerUrl(cwd, serverUrl), + canonicalWorkspaceDirectory(cwd) + ); return { attempted: true, interrupted: true, transport: "server", detail: `Aborted OpenCode session ${threadId}.`, serverUrl, - ...(serverExternal ? { serverExternal: true } : {}) + ...ownership }; } catch (error) { - const serverExternal = normalizeServerUrlForCompare(serverUrl) === normalizeServerUrlForCompare(process.env[SERVER_URL_ENV]); return { attempted: true, interrupted: false, transport: "server", detail: error instanceof Error ? error.message : String(error), serverUrl, - ...(serverExternal ? { serverExternal: true } : {}) + ...ownership }; } } @@ -1263,7 +1295,7 @@ export async function interruptServerTurn(cwd, { threadId, serverUrl = null }) { if (!usedServerUrl) { throw new Error("OpenCode server did not become ready."); } - const client = new OpencodeServerClient(usedServerUrl, serverSessionCredentials(server)); + const client = buildServerClient(cwd, server); await client.abort(threadId); return { attempted: true, @@ -1300,7 +1332,10 @@ export async function runServerTurn(cwd, options = {}) { return withServer(cwd, async (client, server) => { emitProgress(options.onProgress, "Using shared OpenCode server.", "starting", { - serverUrl: server.url + serverUrl: server.url, + // Recorded into the job so a later cancel process knows whether this + // server is plugin-owned without consulting its own environment. + serverExternal: Boolean(server.external) }); let sessionID = options.resumeThreadId ?? options.resumeSessionId ?? null; diff --git a/plugins/opencode/scripts/lib/tracked-jobs.mjs b/plugins/opencode/scripts/lib/tracked-jobs.mjs index 065ead3..3b2ea8c 100644 --- a/plugins/opencode/scripts/lib/tracked-jobs.mjs +++ b/plugins/opencode/scripts/lib/tracked-jobs.mjs @@ -24,6 +24,8 @@ function normalizeProgressEvent(value) { threadId: typeof value.threadId === "string" && value.threadId.trim() ? value.threadId.trim() : null, turnId: typeof value.turnId === "string" && value.turnId.trim() ? value.turnId.trim() : null, serverUrl: typeof value.serverUrl === "string" && value.serverUrl.trim() ? value.serverUrl.trim() : null, + // Tri-state: only an explicit boolean is persisted into the job record. + serverExternal: typeof value.serverExternal === "boolean" ? value.serverExternal : null, stderrMessage: value.stderrMessage == null ? null : String(value.stderrMessage).trim(), logTitle: typeof value.logTitle === "string" && value.logTitle.trim() ? value.logTitle.trim() : null, logBody: value.logBody == null ? null : String(value.logBody).trimEnd() @@ -36,6 +38,7 @@ function normalizeProgressEvent(value) { threadId: null, turnId: null, serverUrl: null, + serverExternal: null, stderrMessage: String(value ?? "").trim(), logTitle: null, logBody: null @@ -81,6 +84,7 @@ export function createJobProgressUpdater(workspaceRoot, jobId) { let lastThreadId = null; let lastTurnId = null; let lastServerUrl = null; + let lastServerExternal = null; return (event) => { const normalized = normalizeProgressEvent(event); @@ -111,6 +115,12 @@ export function createJobProgressUpdater(workspaceRoot, jobId) { changed = true; } + if (normalized.serverExternal != null && normalized.serverExternal !== lastServerExternal) { + lastServerExternal = normalized.serverExternal; + patch.serverExternal = normalized.serverExternal; + changed = true; + } + if (!changed) { return; } diff --git a/plugins/opencode/scripts/opencode-companion.mjs b/plugins/opencode/scripts/opencode-companion.mjs index 8067227..fb4be68 100644 --- a/plugins/opencode/scripts/opencode-companion.mjs +++ b/plugins/opencode/scripts/opencode-companion.mjs @@ -790,10 +790,12 @@ async function handleTask(argv) { ensureOpenCodeAvailable(cwd); requireTaskRequest(prompt, resumeLast); - const currentServerUrl = getSessionRuntimeStatus(process.env, workspaceRoot).url ?? null; + const runtime = getSessionRuntimeStatus(process.env, workspaceRoot); const job = { ...buildTaskJob(workspaceRoot, taskMetadata, write), - ...(currentServerUrl ? { serverUrl: currentServerUrl } : {}) + // Persist ownership with the URL at job start; a later cancel process + // must not re-derive it from its own environment (issue #29). + ...(runtime.url ? { serverUrl: runtime.url, serverExternal: Boolean(runtime.external) } : {}) }; const request = buildTaskRequest({ cwd, @@ -1086,8 +1088,9 @@ async function handleCancel(argv) { const threadId = currentJob.threadId ?? null; const serverUrl = currentJob.serverUrl ?? null; + const serverExternal = typeof currentJob.serverExternal === "boolean" ? currentJob.serverExternal : null; - const interrupt = await interruptServerTurn(cwd, { threadId, serverUrl }); + const interrupt = await interruptServerTurn(cwd, { threadId, serverUrl, serverExternal }); const completedAt = nowIso(); const cancelResult = await cancelJobIfStillActive(workspaceRoot, currentJob, completedAt); if (cancelResult.cancelled) { @@ -1109,7 +1112,16 @@ async function handleCancel(argv) { } try { const teardownServerUrl = interrupt.serverUrl ?? serverUrl; - if (!interrupt.serverExternal) { + // Tear down only a server the job explicitly recorded as plugin-owned + // (serverExternal === false). When the job recorded a URL but no + // ownership (legacy records), fail safe and leave the server running — + // it may be a shared, user-managed service (issue #29). Jobs without a + // recorded URL fall back to the workspace's own persisted server unless + // the interrupt just connected to an env-configured external one. + const ownedServerTeardown = serverUrl + ? serverExternal === false + : interrupt.serverExternal !== true; + if (ownedServerTeardown) { await teardownServerSession({ cwd: workspaceRoot, ignoreCurrentProcessLease: Boolean(interrupt.serverUrl && !serverUrl), diff --git a/tests/fake-opencode-fixture.mjs b/tests/fake-opencode-fixture.mjs index f13fd94..17ea743 100644 --- a/tests/fake-opencode-fixture.mjs +++ b/tests/fake-opencode-fixture.mjs @@ -513,6 +513,12 @@ const server = http.createServer(async (req, res) => { } if (req.method === "GET" && url.pathname === "/event") { + // Record the workspace scope of each event subscription so tests can + // assert the client binds its stream to the invoking directory. + const eventState = loadState(); + eventState.eventDirectories = eventState.eventDirectories || []; + eventState.eventDirectories.push(url.searchParams.get("directory")); + saveState(eventState); res.writeHead(200, { "content-type": "text/event-stream", "cache-control": "no-cache", @@ -529,14 +535,22 @@ const server = http.createServer(async (req, res) => { } if (req.method === "GET" && url.pathname === "/session") { - sendJson(res, loadState().sessions); + // Mirror the real server: a directory query scopes the listing to that + // workspace. + const directory = url.searchParams.get("directory"); + const sessions = loadState().sessions.filter( + (session) => !directory || session.directory === directory + ); + sendJson(res, sessions); return; } if (req.method === "POST" && url.pathname === "/session") { const body = await readJson(req); // Mirror the real server: create-session rejects \`directory\` and \`model\` - // (additionalProperties:false / model belongs on the message) with 400. + // in the BODY (additionalProperties:false / model belongs on the message) + // with 400; the workspace is scoped via the \`directory\` QUERY parameter + // and otherwise inherits the server process launch directory. if ("directory" in body || "model" in body) { sendJson(res, { _tag: "BadRequest" }, 400); return; @@ -544,7 +558,7 @@ const server = http.createServer(async (req, res) => { const state = loadState(); const session = { id: "ses_" + state.nextSessionId++, - directory: body.directory || process.cwd(), + directory: url.searchParams.get("directory") || process.cwd(), title: body.title || null, agent: body.agent || null, model: body.model || null, diff --git a/tests/opencode-server.test.mjs b/tests/opencode-server.test.mjs index 29693d6..ce8a6c2 100644 --- a/tests/opencode-server.test.mjs +++ b/tests/opencode-server.test.mjs @@ -41,6 +41,41 @@ test("buildBasicAuthHeader follows OpenCode's basic-auth convention", () => { assert.equal(buildBasicAuthHeader({ password: "" }), null); }); +test("client scopes project routes to the configured directory but never /global routes", async () => { + const seen = []; + const client = new OpencodeServerClient("http://opencode.test", { + directory: "/work/repo a", + fetch: async (requestUrl) => { + const url = new URL(String(requestUrl)); + seen.push(url); + if (url.pathname === "/event") { + return new Response(":ok\n\n", { + status: 200, + headers: { "content-type": "text/event-stream" } + }); + } + return new Response("{}", { status: 200, headers: { "content-type": "application/json" } }); + } + }); + + await client.listSessions(); + await client.createSession({ agent: "plan" }); + await client.subscribeEvents(() => {}); + await client.health(); + await client.dispose(); + + assert.equal(seen[0].pathname, "/session"); + assert.equal(seen[0].searchParams.get("directory"), "/work/repo a"); + assert.equal(seen[1].pathname, "/session"); + assert.equal(seen[1].searchParams.get("directory"), "/work/repo a"); + assert.equal(seen[2].pathname, "/event"); + assert.equal(seen[2].searchParams.get("directory"), "/work/repo a"); + assert.equal(seen[3].pathname, "/global/health"); + assert.equal(seen[3].searchParams.get("directory"), null); + assert.equal(seen[4].pathname, "/global/dispose"); + assert.equal(seen[4].searchParams.get("directory"), null); +}); + test("client sends Basic auth on requests, fresh connections, and the event stream", { skip: LOCAL_LISTEN_SKIP }, async () => { const authorization = `Basic ${Buffer.from("opencode:secret").toString("base64")}`; const seen = []; diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index f3a9606..770d4b3 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -124,6 +124,36 @@ function jsonResponse(value, status = 200) { }); } +// Spawns the fake fixture directly as a user-managed external server (no +// plugin lifecycle, no password) and resolves its base URL from stdout. +function startExternalFixtureServer(binDir) { + const child = spawn("node", [path.join(binDir, "opencode"), "serve", "--hostname", "127.0.0.1", "--port", "0"], { + env: { + ...process.env, + FAKE_OPENCODE_STATE_PATH: path.join(binDir, "fake-opencode-state.json"), + OPENCODE_SERVER_PASSWORD: "" + }, + windowsHide: true + }); + + const url = new Promise((resolve, reject) => { + let output = ""; + child.stdout.setEncoding("utf8"); + child.stdout.on("data", (chunk) => { + output += chunk; + const match = output.match(/listening on (http:\/\/[^\s]+)/); + if (match) { + resolve(match[1]); + } + }); + child.once("error", reject); + child.once("exit", () => reject(new Error("external fixture server exited before listening"))); + setTimeout(() => reject(new Error("external fixture server did not start")), 5000).unref(); + }); + + return { child, url }; +} + test("interruptServerTurn marks env-provided server urls as external", async () => { const workspace = makeTempDir(); const binDir = makeTempDir(); @@ -175,6 +205,8 @@ process.exit(1); { method: "POST", pathname: "/session/ses_external/abort" } ]); + // Ownership comes from the job record, never from the current environment + // (issue #29): without a persisted flag the result must not claim one. const missingThreadResult = await withProcessEnv( { [SERVER_URL_ENV]: "http://opencode.test/" @@ -182,7 +214,15 @@ process.exit(1); () => interruptServerTurn(workspace, { threadId: null, serverUrl: "http://opencode.test" }) ); assert.equal(missingThreadResult.attempted, false); - assert.equal(missingThreadResult.serverExternal, true); + assert.equal("serverExternal" in missingThreadResult, false); + + const persistedExternalResult = await interruptServerTurn(workspace, { + threadId: null, + serverUrl: "http://opencode.test", + serverExternal: true + }); + assert.equal(persistedExternalResult.attempted, false); + assert.equal(persistedExternalResult.serverExternal, true); } finally { globalThis.fetch = previousFetch; } @@ -426,6 +466,107 @@ test("cancel tears down a server it starts to abort a job without a recorded ser }); }); +test("external server requests are bound to each invoking workspace (issue #29)", { skip: LOCAL_LISTEN_SKIP }, async () => { + const repoA = makeTempDir(); + const repoB = makeTempDir(); + const binDir = makeTempDir(); + installFakeOpencode(binDir); + initGitRepo(repoA); + initGitRepo(repoB); + const { child, url } = startExternalFixtureServer(binDir); + + try { + const serverUrl = await url; + const resultA = run("node", [SCRIPT, "task", "--json", "task in workspace A"], { + cwd: repoA, + env: buildTestEnv(binDir, { [SERVER_URL_ENV]: serverUrl }) + }); + assert.equal(resultA.status, 0, resultA.stderr); + const resultB = run("node", [SCRIPT, "task", "--json", "task in workspace B"], { + cwd: repoB, + env: buildTestEnv(binDir, { [SERVER_URL_ENV]: serverUrl }) + }); + assert.equal(resultB.status, 0, resultB.stderr); + + // One shared external server, two workspaces: each session must be scoped + // to the invoking repo, not the server process's launch directory. + const fakeState = readFakeState(binDir); + const directories = fakeState.sessions.map((session) => session.directory).sort(); + assert.deepEqual(directories, [fs.realpathSync(repoA), fs.realpathSync(repoB)].sort()); + + // The event subscriptions carried the workspace scope too. + const eventDirectories = (fakeState.eventDirectories || []).filter(Boolean); + assert.ok(eventDirectories.includes(fs.realpathSync(repoA)), "event stream scoped to workspace A"); + assert.ok(eventDirectories.includes(fs.realpathSync(repoB)), "event stream scoped to workspace B"); + } finally { + child.kill(); + } +}); + +test("cancel without the job-start environment leaves an external server running (issue #29)", { skip: LOCAL_LISTEN_SKIP }, async () => { + const workspace = makeTempDir(); + const binDir = makeTempDir(); + installFakeOpencode(binDir); + const pluginDataDir = makeTempDir("opencode-plugin-data-"); + const { child, url } = startExternalFixtureServer(binDir); + + try { + const serverUrl = await url; + await withProcessEnv({ CLAUDE_PLUGIN_DATA: pluginDataDir }, async () => { + const jobId = "job-cancel-env-divergent"; + const timestamp = new Date().toISOString(); + const logFile = resolveJobLogFile(workspace, jobId); + const runningJob = { + id: jobId, + workspaceRoot: workspace, + jobClass: "task", + kind: "task", + status: "running", + phase: "running", + pid: null, + title: "Running external task", + threadId: "ses_env_divergent", + // Persisted at job start; the cancel environment below deliberately + // lacks OPENCODE_COMPANION_SERVER_URL (the review's H-04 scenario). + serverUrl, + serverExternal: true, + logFile, + createdAt: timestamp, + updatedAt: timestamp + }; + saveState(workspace, { + version: 1, + config: { stopReviewGate: false }, + jobs: [runningJob] + }); + writeJobFile(workspace, jobId, runningJob); + + const result = run("node", [SCRIPT, "cancel", jobId, "--cwd", workspace, "--json"], { + cwd: workspace, + env: buildEnv(binDir, { + CLAUDE_PLUGIN_DATA: pluginDataDir, + OPENCODE_COMPANION_SESSION_ID: "sess-current" + }) + }); + + assert.equal(result.status, 0, result.stderr); + const payload = JSON.parse(result.stdout); + assert.equal(payload.cancelled, true); + assert.equal(payload.turnInterruptAttempted, true); + assert.equal(payload.turnInterrupted, true); + assert.equal(readFakeState(binDir).lastAbort, "ses_env_divergent"); + + // The user-managed server must survive the cancel: no dispose (the + // fixture exits on dispose), no process kill. + const health = await fetch(`${serverUrl}/global/health`); + assert.equal(health.status, 200); + await health.text(); + }); + } finally { + child.kill(); + } +}); + test("cancel aborts but does not dispose an env-provided external server", async () => { const workspace = makeTempDir(); const binDir = makeTempDir();