diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 747d7e2..80541e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,10 +1,11 @@ name: CI # Adapted from openai/codex-plugin-cc's pull-request-ci.yml. -# Modified by JohnnyVicious (2026): drops the Codex CLI install step (this -# fork wraps OpenCode, not Codex) and runs only on a single Node version -# matching the package.json `engines.node` floor. (Apache License 2.0 -# §4(b) modification notice — see NOTICE.) +# Modified by JohnnyVicious (2026): wraps OpenCode (not Codex), so installs +# `opencode-ai` instead of `@openai/codex` for the integration tests, and +# runs only on a single Node version matching the package.json +# `engines.node` floor. (Apache License 2.0 §4(b) modification notice — +# see NOTICE.) on: pull_request: @@ -19,7 +20,7 @@ jobs: test: name: Test runs-on: ubuntu-latest - timeout-minutes: 5 + timeout-minutes: 10 steps: - name: Check out repository @@ -34,6 +35,9 @@ jobs: - name: Install dependencies run: npm ci + - name: Install OpenCode CLI + run: npm install -g opencode-ai + - name: Syntax-check companion scripts run: | node --check plugins/opencode/scripts/opencode-companion.mjs diff --git a/plugins/opencode/scripts/lib/opencode-server.mjs b/plugins/opencode/scripts/lib/opencode-server.mjs index f4192a8..14614bd 100644 --- a/plugins/opencode/scripts/lib/opencode-server.mjs +++ b/plugins/opencode/scripts/lib/opencode-server.mjs @@ -1,6 +1,15 @@ // OpenCode HTTP API client. // Unlike codex-plugin-cc which uses JSON-RPC over stdin/stdout, // OpenCode exposes a REST API + SSE. This module wraps that API. +// +// Modified by JohnnyVicious (2026): `ensureServer` now spawns opencode +// with `stdio: "ignore"` instead of piping stdout/stderr that nothing +// reads. The piped streams were ref'd handles on the parent event loop, +// which deadlocked any long-lived parent (e.g. `node:test`) once +// opencode wrote enough log output to fill the pipe buffer. In normal +// CLI usage the deadlock was masked because the companion script exited +// before the buffer filled. (Apache License 2.0 §4(b) modification +// notice — see NOTICE.) import { spawn } from "node:child_process"; @@ -42,9 +51,13 @@ export async function ensureServer(opts = {}) { return { url, alreadyRunning: true }; } - // Start the server + // Start the server. + // `stdio: "ignore"` is critical: piping stdout/stderr without draining + // them creates ref'd file descriptors on the parent that prevent any + // long-lived parent (notably `node:test`) from exiting cleanly once + // opencode writes enough output to fill the pipe buffer. const proc = spawn("opencode", ["serve", "--port", String(port)], { - stdio: ["ignore", "pipe", "pipe"], + stdio: "ignore", detached: true, cwd: opts.cwd, }); diff --git a/tests/opencode-server.test.mjs b/tests/opencode-server.test.mjs new file mode 100644 index 0000000..ea1eccf --- /dev/null +++ b/tests/opencode-server.test.mjs @@ -0,0 +1,133 @@ +// Integration tests for the OpenCode HTTP server wrapper. +// +// These tests start a real `opencode serve` process on a high test port +// (so they don't collide with a user's default-port server) and exercise +// the protocol surfaces our companion script depends on: +// +// - server lifecycle (`isServerRunning`, `ensureServer`) +// - health endpoint +// - session create / get / list / delete +// +// They intentionally do NOT call `sendPrompt` because that requires a +// configured AI provider and would burn paid API credits in CI. +// +// Locally, this suite is skipped if the `opencode` binary is not on PATH, +// so developers without OpenCode installed can still run `npm test`. + +import { describe, it, before, after } from "node:test"; +import assert from "node:assert/strict"; +import { + isOpencodeInstalled, +} from "../plugins/opencode/scripts/lib/process.mjs"; +import { + isServerRunning, + ensureServer, + createClient, +} from "../plugins/opencode/scripts/lib/opencode-server.mjs"; + +const TEST_HOST = "127.0.0.1"; +const TEST_PORT = Number(process.env.OPENCODE_TEST_PORT ?? 14096); + +const opencodeAvailable = await isOpencodeInstalled(); +const describeOrSkip = opencodeAvailable ? describe : describe.skip; + +describeOrSkip("opencode HTTP server (integration)", () => { + let serverInfo; + let client; + + before(async () => { + // If something is already squatting our test port, fail fast with a + // useful message instead of silently sharing state with a foreign + // server. + if (await isServerRunning(TEST_HOST, TEST_PORT)) { + throw new Error( + `Test port ${TEST_PORT} is already in use. Set OPENCODE_TEST_PORT to a free port.` + ); + } + + serverInfo = await ensureServer({ host: TEST_HOST, port: TEST_PORT }); + client = createClient(serverInfo.url); + }); + + after(async () => { + // Tear down the server we spawned. `ensureServer` only sets `pid` + // when it actually started a new process, so this is a no-op when + // the server was already running (which our `before` rejects, but + // be defensive). + if (!serverInfo?.pid || serverInfo.alreadyRunning) return; + + // opencode is spawned with `detached: true`, which puts it in its + // own process group. SIGTERM the negative pid to take down the + // whole group (any children opencode forked included). + const pgid = -serverInfo.pid; + try { + process.kill(pgid, "SIGTERM"); + } catch { + // group may already be gone — that's fine + } + + // Wait briefly for graceful shutdown, then SIGKILL the group if + // anything is still alive. We poll instead of sleeping a fixed + // interval so a fast exit doesn't waste time. + const deadline = Date.now() + 3_000; + while (Date.now() < deadline) { + try { + process.kill(pgid, 0); // signal 0 = existence check + } catch { + return; // group is gone + } + await new Promise((r) => setTimeout(r, 100)); + } + + try { + process.kill(pgid, "SIGKILL"); + } catch { + // already exited between checks + } + }); + + it("isServerRunning detects the spawned server", async () => { + assert.equal(await isServerRunning(TEST_HOST, TEST_PORT), true); + }); + + it("health() reports healthy with a version string", async () => { + const h = await client.health(); + assert.equal(h.healthy, true); + assert.ok(typeof h.version === "string" && h.version.length > 0, + `expected version string, got ${JSON.stringify(h.version)}`); + }); + + it("createSession returns an id", async () => { + const session = await client.createSession({ title: "test session" }); + assert.ok(typeof session.id === "string" && session.id.length > 0); + await client.deleteSession(session.id); + }); + + it("listSessions returns an array containing a freshly-created session", async () => { + const created = await client.createSession({ title: "list test" }); + try { + const sessions = await client.listSessions(); + assert.ok(Array.isArray(sessions)); + assert.ok( + sessions.some((s) => s.id === created.id), + "freshly-created session was not present in listSessions output" + ); + } finally { + await client.deleteSession(created.id); + } + }); + + it("createSession -> getSession -> deleteSession roundtrip", async () => { + const created = await client.createSession({ title: "roundtrip" }); + const fetched = await client.getSession(created.id); + assert.equal(fetched.id, created.id); + + await client.deleteSession(created.id); + + // After deletion, getSession should reject with a non-2xx status. + await assert.rejects( + () => client.getSession(created.id), + /OpenCode API GET .* returned 4\d\d/ + ); + }); +});