From 6dafaf94d755b4d5388aeb0fdff08e2e53dd6f52 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Mon, 20 Jul 2026 12:07:45 -0400 Subject: [PATCH 1/2] fix(cli): prevent competing detached owners --- .changeset/concurrent-start-ownership.md | 5 + packages/drive/src/cli/start.ts | 13 +- packages/drive/src/instance/registry.ts | 27 ++- packages/drive/test/cli/integration.test.ts | 189 ++++++++++++++++-- packages/drive/test/fixtures/fake-opencode.ts | 3 + 5 files changed, 217 insertions(+), 20 deletions(-) create mode 100644 .changeset/concurrent-start-ownership.md diff --git a/.changeset/concurrent-start-ownership.md b/.changeset/concurrent-start-ownership.md new file mode 100644 index 0000000..d0d17a0 --- /dev/null +++ b/.changeset/concurrent-start-ownership.md @@ -0,0 +1,5 @@ +--- +"opencode-drive": patch +--- + +Prevent concurrent detached launchers from stealing prepared instance ownership and spawning competing daemon processes. diff --git a/packages/drive/src/cli/start.ts b/packages/drive/src/cli/start.ts index 11f16a0..744e06c 100644 --- a/packages/drive/src/cli/start.ts +++ b/packages/drive/src/cli/start.ts @@ -33,12 +33,22 @@ export const start = Effect.fn("DriveCli.start")((options: StartOptions) => ) const startScoped = Effect.fn("DriveCli.startScoped")(function* (options: StartOptions) { + const initializerPid = Number.parseInt( + options.daemon ? process.env.OPENCODE_DRIVE_INITIALIZER_PID ?? "" : "", + 10, + ) + if (options.daemon) delete process.env.OPENCODE_DRIVE_INITIALIZER_PID const initialized = yield* fromPromise(() => initializeManifest( options.name, process.cwd(), () => initializeInstance(options.name), - { temporary: true }, + { + temporary: true, + ...(Number.isInteger(initializerPid) && initializerPid > 0 + ? { adoptPid: initializerPid } + : {}), + }, ), ) configureLogFile(initialized.artifacts) @@ -419,6 +429,7 @@ const startDetached = Effect.fn("DriveCli.startDetached")(function* ( ...process.env, OPENCODE_DRIVE_LOG: configureLogFile(artifacts), OPENCODE_DRIVE_OWNER_LOG: ownerLog, + OPENCODE_DRIVE_INITIALIZER_PID: String(process.pid), }, stdin: "ignore", stdout: "ignore", diff --git a/packages/drive/src/instance/registry.ts b/packages/drive/src/instance/registry.ts index 08a0067..f7239a3 100644 --- a/packages/drive/src/instance/registry.ts +++ b/packages/drive/src/instance/registry.ts @@ -51,7 +51,10 @@ export async function initializeManifest( name: string, cwd: string, create: () => Promise, - options: { readonly temporary?: boolean } = {}, + options: { + readonly temporary?: boolean + readonly adoptPid?: number + } = {}, ) { let initialized: InitializedManifest | undefined await withLock(name, false, async () => { @@ -64,10 +67,24 @@ export async function initializeManifest( ]) existing = undefined } else { - initialized = - options.temporary && existing.temporary - ? { ...existing, pid: process.pid } - : existing + if ( + !existing.temporary && + existing.pid !== undefined && + !isProcessAlive(existing.pid) + ) { + const { pid: _, ...released } = existing + existing = released + await write(existing) + } + if (existing.pid !== undefined && existing.pid !== process.pid) { + if (!options.temporary || options.adoptPid !== existing.pid) + throw new Error(`drive instance "${name}" is already starting`) + initialized = { ...existing, pid: process.pid } + } else if (options.temporary && existing.pid === undefined) { + initialized = { ...existing, pid: process.pid } + } else { + initialized = existing + } if (initialized !== existing) await write(initialized) return } diff --git a/packages/drive/test/cli/integration.test.ts b/packages/drive/test/cli/integration.test.ts index 03c4987..08f72c5 100644 --- a/packages/drive/test/cli/integration.test.ts +++ b/packages/drive/test/cli/integration.test.ts @@ -1,9 +1,11 @@ import { afterEach, describe, expect, setDefaultTimeout, test } from "bun:test" import { mkdir, mkdtemp, readdir, realpath, rm } from "node:fs/promises" import { tmpdir } from "node:os" -import { basename, dirname, join, resolve } from "node:path" +import { basename, dirname, isAbsolute, join, relative, resolve } from "node:path" import { controlPath, + initializeManifest, + isProcessAlive, listInstances, manifestPath, register, @@ -27,7 +29,14 @@ afterEach(async () => { await spawn(["stop", "--name", name], root).exited }), ) - await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) + const cleanup = [...new Set(roots.splice(0).map((root) => resolve(root)))].filter( + (root, _, all) => + !all.some((parent) => { + const nested = relative(parent, root) + return nested !== "" && !nested.startsWith("..") && !isAbsolute(nested) + }), + ) + await Promise.all(cleanup.map((root) => rm(root, { recursive: true, force: true }))) }) describe("opencode-drive", () => { @@ -255,11 +264,20 @@ describe("opencode-drive", () => { const root = await temporary() const name = "exited-recording-test" const started = spawn( - ["start", "--name", name, "--record", "--", process.execPath, fixture("fake-opencode.ts"), "500"], + [ + "start", + "--name", + name, + "--record", + "--", + process.execPath, + fixture("fake-opencode.ts"), + "exit-after-attach", + ], root, ) const [status, stderr] = await Promise.all([started.exited, new Response(started.stderr).text()]) - expect(status).toBe(0) + expect(status, stderr).toBe(0) const artifacts = stderr.match(/opencode-drive: using artifacts (.+)/)?.[1] expect(artifacts).toBeDefined() roots.push(artifacts!) @@ -308,16 +326,11 @@ describe("opencode-drive", () => { expect(stderr).toContain(`drive instance "${name}" is already running`) }) - test("only the owning detached launcher reports concurrent startup success", async () => { - const root = await temporary() - const name = "concurrent-start" - const args = ["start", "--name", name, "--", process.execPath, fixture("fake-opencode.ts")] - const children = [spawn(args, root), spawn(args, root)] - expect((await Promise.all(children.map((child) => child.exited))).sort()).toEqual([0, 1]) - const manifest = await Bun.file(join(root, "registry", `${name}.json`)).json() - roots.push(manifest.artifacts) - instances.push({ root, name }) - }) + test("only the owning detached launcher starts an automatic instance", () => + expectSingleConcurrentStart(false)) + + test("only the owning detached launcher starts a prepared instance", () => + expectSingleConcurrentStart(true)) test("registers only one concurrent owner for a name", async () => { const root = await temporary() @@ -331,6 +344,83 @@ describe("opencode-drive", () => { }) }) + test("transfers temporary initialization only to its declared daemon", async () => { + const root = await temporary() + const owner = Bun.spawn([ + process.execPath, + "-e", + "await Bun.sleep(60_000)", + ]) + try { + await withRegistry(root, async () => { + const name = "temporary-owner" + const artifacts = join(root, "artifacts") + await mkdir(join(root, "registry"), { recursive: true }) + await Bun.write( + manifestPath(name), + `${JSON.stringify({ + version: 1, + name, + createdAt: new Date().toISOString(), + cwd: root, + artifacts, + status: "initialized", + temporary: true, + pid: owner.pid, + })}\n`, + ) + const create = async () => { + throw new Error("existing initialization was replaced") + } + + await expect( + initializeManifest(name, root, create, { temporary: true }), + ).rejects.toThrow(`drive instance "${name}" is already starting`) + expect( + await initializeManifest(name, root, create, { + temporary: true, + adoptPid: owner.pid, + }), + ).toMatchObject({ pid: process.pid, artifacts }) + }) + } finally { + owner.kill() + await owner.exited + } + }) + + test("preserves prepared artifacts when reclaiming a dead launcher", async () => { + const root = await temporary() + await withRegistry(root, async () => { + const name = "prepared-owner" + const artifacts = join(root, "prepared-artifacts") + await mkdir(join(root, "registry"), { recursive: true }) + await Bun.write( + manifestPath(name), + `${JSON.stringify({ + version: 1, + name, + createdAt: new Date().toISOString(), + cwd: root, + artifacts, + status: "initialized", + pid: 2_000_000_000, + })}\n`, + ) + + const reclaimed = await initializeManifest( + name, + root, + async () => { + throw new Error("prepared artifacts were replaced") + }, + { temporary: true }, + ) + expect(reclaimed).toMatchObject({ pid: process.pid, artifacts }) + expect("temporary" in reclaimed).toBe(false) + }) + }) + test("registers only one visible instance", async () => { const root = await temporary() await withRegistry(root, async () => { @@ -1645,6 +1735,77 @@ async function waitForManifest(root: string, name: string) { throw new Error(`timed out waiting for ${file}`) } +async function waitForLauncher( + child: ReturnType, + stderr: Promise, +) { + let timer: ReturnType | undefined + const timeout = new Promise<"timeout">((resolve) => { + timer = setTimeout(() => resolve("timeout"), 10_000) + }) + try { + const status = await Promise.race([child.exited, timeout]) + if (status !== "timeout") return status + if (child.exitCode === null) child.kill() + await child.exited + throw new Error( + `launcher ${child.pid} did not exit after ownership resolved: ${await stderr}`, + ) + } finally { + if (timer !== undefined) clearTimeout(timer) + } +} + +async function expectSingleConcurrentStart(prepared: boolean) { + const root = await temporary() + const name = `concurrent-start-${prepared ? "prepared" : "automatic"}` + if (prepared) expect(await spawn(["init", "--name", name], root).exited).toBe(0) + const args = ["start", "--name", name, "--", process.execPath, fixture("fake-opencode.ts")] + const children = [spawn(args, root), spawn(args, root)] + const stderr = children.map((child) => new Response(child.stderr).text()) + let managed = false + try { + const manifest = await waitForManifest(root, name) + instances.push({ root, name }) + managed = true + const statuses = await Promise.all( + children.map((child, index) => waitForLauncher(child, stderr[index]!)), + ) + const errors = await Promise.all(stderr) + expect(statuses.sort()).toEqual([0, 1]) + expect(errors.filter((error) => error.includes("is already starting"))).toHaveLength(1) + expect(errors.some((error) => error.includes("detached instance exited"))).toBe(false) + expect(manifest.pid).toBeGreaterThan(0) + } finally { + await Promise.all( + children.map(async (child) => { + if (child.exitCode === null) child.kill() + await child.exited + }), + ) + await Promise.allSettled(stderr) + if (!managed) await terminateUnmanagedInstance(root, name) + } +} + +async function terminateUnmanagedInstance(root: string, name: string) { + const manifest: unknown = await Bun.file(join(root, "registry", `${name}.json`)) + .json() + .catch(() => undefined) + if ( + typeof manifest !== "object" || + manifest === null || + !("pid" in manifest) || + typeof manifest.pid !== "number" || + !isProcessAlive(manifest.pid) + ) + return + process.kill(manifest.pid, "SIGTERM") + const deadline = Date.now() + 2_000 + while (isProcessAlive(manifest.pid) && Date.now() < deadline) await Bun.sleep(25) + if (isProcessAlive(manifest.pid)) process.kill(manifest.pid, "SIGKILL") +} + async function waitForMissing(file: string) { const deadline = Date.now() + 10_000 while (Date.now() < deadline) { diff --git a/packages/drive/test/fixtures/fake-opencode.ts b/packages/drive/test/fixtures/fake-opencode.ts index cbd9e02..3aa1cff 100644 --- a/packages/drive/test/fixtures/fake-opencode.ts +++ b/packages/drive/test/fixtures/fake-opencode.ts @@ -24,6 +24,7 @@ const recordingStarted = performance.now() const endpoints = drive.endpoints let toolAttachments = 0 let backendHandshakes = 0 +const exitAfterAttach = Promise.withResolvers() const servicePassword = "drive-test-password" const api = role === "service" ? Bun.serve({ @@ -277,6 +278,7 @@ const backend = role === "client" ? undefined : Bun.serve({ }, }), ) + if (process.argv.includes("exit-after-attach")) exitAfterAttach.resolve() } }, }, @@ -294,6 +296,7 @@ await new Promise((resolve) => { else process.once("SIGTERM", resolve) const lifetime = role === "service" ? Number.NaN : Number(process.argv[2]) if (Number.isFinite(lifetime)) setTimeout(resolve, lifetime) + if (process.argv.includes("exit-after-attach")) void exitAfterAttach.promise.then(resolve) }) await Promise.all([ui?.stop(true), backend?.stop(true)]) From 835376eb51ca8e36f88eb3d7812bbadce06b4267 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Mon, 20 Jul 2026 12:36:19 -0400 Subject: [PATCH 2/2] fix(cli): harden detached ownership handoff --- packages/drive/src/cli/start.ts | 4 - packages/drive/src/instance/registry.ts | 37 ++++++---- packages/drive/test/cli/integration.test.ts | 82 +++++++++------------ 3 files changed, 58 insertions(+), 65 deletions(-) diff --git a/packages/drive/src/cli/start.ts b/packages/drive/src/cli/start.ts index 744e06c..6b1cbf9 100644 --- a/packages/drive/src/cli/start.ts +++ b/packages/drive/src/cli/start.ts @@ -404,10 +404,6 @@ const startDetached = Effect.fn("DriveCli.startDetached")(function* ( options: StartOptions, artifacts: string, ) { - const existing = yield* fromPromise(() => - resolveInstance(options.name, { ready: false }).catch(() => undefined), - ) - if (existing) throw new Error(`drive instance "${options.name}" is already running`) const ownerLog = join(registryDirectory(), `${options.name}.log`) yield* fromPromise(() => mkdir(registryDirectory(), { recursive: true })) yield* fromPromise(() => rm(ownerLog, { force: true })) diff --git a/packages/drive/src/instance/registry.ts b/packages/drive/src/instance/registry.ts index f7239a3..317b527 100644 --- a/packages/drive/src/instance/registry.ts +++ b/packages/drive/src/instance/registry.ts @@ -60,6 +60,15 @@ export async function initializeManifest( await withLock(name, false, async () => { let existing = await read(manifestPath(name)) if (existing?.status === "initialized") { + if ( + options.temporary && + options.adoptPid !== undefined && + existing.pid === options.adoptPid + ) { + initialized = { ...existing, pid: process.pid } + await write(initialized) + return + } if (!keepInitialized(existing)) { await Promise.all([ rm(manifestPath(name), { force: true }), @@ -67,23 +76,17 @@ export async function initializeManifest( ]) existing = undefined } else { - if ( - !existing.temporary && - existing.pid !== undefined && - !isProcessAlive(existing.pid) - ) { - const { pid: _, ...released } = existing - existing = released - await write(existing) - } + let available = existing if (existing.pid !== undefined && existing.pid !== process.pid) { - if (!options.temporary || options.adoptPid !== existing.pid) + if (isProcessAlive(existing.pid)) throw new Error(`drive instance "${name}" is already starting`) - initialized = { ...existing, pid: process.pid } - } else if (options.temporary && existing.pid === undefined) { - initialized = { ...existing, pid: process.pid } + const { pid: _, ...released } = existing + available = released + } + if (options.temporary && available.pid === undefined) { + initialized = { ...available, pid: process.pid } } else { - initialized = existing + initialized = available } if (initialized !== existing) await write(initialized) return @@ -118,6 +121,12 @@ export async function register(manifest: InstanceManifest) { } await withLock(manifest.name, false, async () => { const existing = await read(manifestPath(manifest.name)) + if ( + existing?.status === "initialized" && + existing.pid !== undefined && + existing.pid !== manifest.pid + ) + throw new Error(`drive instance "${manifest.name}" changed ownership`) if (existing && existing.status !== "initialized" && isProcessAlive(existing.pid)) throw new Error(`drive instance "${manifest.name}" is already running`) await Promise.all([ diff --git a/packages/drive/test/cli/integration.test.ts b/packages/drive/test/cli/integration.test.ts index 08f72c5..366afca 100644 --- a/packages/drive/test/cli/integration.test.ts +++ b/packages/drive/test/cli/integration.test.ts @@ -1,11 +1,10 @@ import { afterEach, describe, expect, setDefaultTimeout, test } from "bun:test" import { mkdir, mkdtemp, readdir, realpath, rm } from "node:fs/promises" import { tmpdir } from "node:os" -import { basename, dirname, isAbsolute, join, relative, resolve } from "node:path" +import { basename, dirname, join, resolve } from "node:path" import { controlPath, initializeManifest, - isProcessAlive, listInstances, manifestPath, register, @@ -29,14 +28,7 @@ afterEach(async () => { await spawn(["stop", "--name", name], root).exited }), ) - const cleanup = [...new Set(roots.splice(0).map((root) => resolve(root)))].filter( - (root, _, all) => - !all.some((parent) => { - const nested = relative(parent, root) - return nested !== "" && !nested.startsWith("..") && !isAbsolute(nested) - }), - ) - await Promise.all(cleanup.map((root) => rm(root, { recursive: true, force: true }))) + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) }) describe("opencode-drive", () => { @@ -122,7 +114,6 @@ describe("opencode-drive", () => { const [initStatus, initOutput] = await Promise.all([initialized.exited, new Response(initialized.stdout).text()]) expect(initStatus).toBe(0) const artifacts = initOutput.trim() - roots.push(artifacts) expect(await Bun.file(join(artifacts, "files", ".opencode", "opencode.jsonc")).exists()).toBe(true) expect(await Bun.file(join(artifacts, "files", ".opencode", "opencode.jsonc")).json()).toMatchObject({ model: "simulation/gpt-sim-model", @@ -183,7 +174,6 @@ describe("opencode-drive", () => { instances.push({ root, name }) const manifest = await Bun.file(join(root, "registry", `${name}.json`)).json() - roots.push(manifest.artifacts) expect(manifest.visible).toBe(false) expect(manifest.endpoints.ui).toMatch(/^ws:\/\/127\.0\.0\.1:\d+$/) expect(manifest.endpoints.backend).toMatch(/^ws:\/\/127\.0\.0\.1:\d+$/) @@ -280,7 +270,6 @@ describe("opencode-drive", () => { expect(status, stderr).toBe(0) const artifacts = stderr.match(/opencode-drive: using artifacts (.+)/)?.[1] expect(artifacts).toBeDefined() - roots.push(artifacts!) const deadline = Date.now() + 10_000 while (await Bun.file(join(root, "registry", `${name}.json`)).exists()) { @@ -302,7 +291,6 @@ describe("opencode-drive", () => { ).toBe(0) instances.push({ root, name }) const manifest = await Bun.file(join(root, "registry", `${name}.json`)).json() - roots.push(manifest.artifacts) const drive = await Bun.file(join(manifest.artifacts, "drive", `${name}.json`)).json() expect(drive.recording).toBeUndefined() @@ -327,10 +315,10 @@ describe("opencode-drive", () => { }) test("only the owning detached launcher starts an automatic instance", () => - expectSingleConcurrentStart(false)) + expectSingleConcurrentStart(false), 75_000) test("only the owning detached launcher starts a prepared instance", () => - expectSingleConcurrentStart(true)) + expectSingleConcurrentStart(true), 75_000) test("registers only one concurrent owner for a name", async () => { const root = await temporary() @@ -376,15 +364,20 @@ describe("opencode-drive", () => { await expect( initializeManifest(name, root, create, { temporary: true }), ).rejects.toThrow(`drive instance "${name}" is already starting`) + owner.kill() + await owner.exited expect( await initializeManifest(name, root, create, { temporary: true, adoptPid: owner.pid, }), ).toMatchObject({ pid: process.pid, artifacts }) + await expect(register(testManifest(name, owner.pid))).rejects.toThrow( + `drive instance "${name}" changed ownership`, + ) }) } finally { - owner.kill() + if (owner.exitCode === null) owner.kill() await owner.exited } }) @@ -408,12 +401,18 @@ describe("opencode-drive", () => { })}\n`, ) + const create = async () => { + throw new Error("prepared artifacts were replaced") + } + const released = await initializeManifest(name, root, create) + expect(released).toMatchObject({ artifacts }) + expect("pid" in released).toBe(false) + expect("pid" in await Bun.file(manifestPath(name)).json()).toBe(false) + const reclaimed = await initializeManifest( name, root, - async () => { - throw new Error("prepared artifacts were replaced") - }, + create, { temporary: true }, ) expect(reclaimed).toMatchObject({ pid: process.pid, artifacts }) @@ -680,7 +679,6 @@ describe("opencode-drive", () => { instances.push(...names.map((name) => ({ root, name }))) const first = await Bun.file(join(root, "registry", "first.json")).json() const second = await Bun.file(join(root, "registry", "second.json")).json() - roots.push(first.artifacts, second.artifacts) expect(first.endpoints.ui).not.toBe(second.endpoints.ui) expect(await spawn(["send", "--name", "first", "--command.ui.state"], root).exited).toBe(0) expect(await spawn(["send", "--name", "second", "--command.ui.state"], root).exited).toBe(0) @@ -740,13 +738,11 @@ describe("opencode-drive", () => { new Response(backgroundStart.stderr).text(), ]) expect(backgroundStatus).toBe(0) - roots.push(artifactPath(backgroundError)) instances.push({ root, name: background }) const running = spawn(["start", "--visible", "--", process.execPath, fixture("fake-opencode.ts"), "30000"], root) const name = `visible-${running.pid}` const manifest = await waitForManifest(root, name) - roots.push(manifest.artifacts) instances.push({ root, name: manifest.name }) expect(manifest.visible).toBe(true) @@ -780,7 +776,6 @@ describe("opencode-drive", () => { expect(status).toBe(0) expect(Date.now() - started).toBeLessThan(10_000) const artifacts = artifactPath(stderr) - roots.push(artifacts) expect(await Bun.file(join(artifacts, "script-result.json")).json()).toMatchObject({ frame: { cols: 80, rows: 24 }, gitWriteError: expect.stringContaining("must not modify Git metadata"), @@ -837,7 +832,6 @@ describe("opencode-drive", () => { expect(await initialized.exited).toBe(0) const artifacts = (await new Response(initialized.stdout).text()).trim() const files = join(artifacts, "files") - roots.push(artifacts) await rm(join(files, ".git"), { recursive: true }) await Bun.$`git init --quiet --initial-branch=main`.cwd(files) await Bun.$`git add --all`.cwd(files) @@ -888,7 +882,6 @@ describe("opencode-drive", () => { const [status, stderr] = await Promise.all([child.exited, new Response(child.stderr).text()]) expect(status).toBe(1) const artifacts = artifactPath(stderr) - roots.push(artifacts) expect(stderr).toContain('timed out waiting for the UI to match "this text never appears"') expect(await Bun.file(join(root, "registry", `${name}.json`)).exists()).toBe(false) }, 10_000) @@ -912,7 +905,6 @@ describe("opencode-drive", () => { const [status, stderr] = await Promise.all([child.exited, new Response(child.stderr).text()]) expect(status).toBe(1) const artifacts = artifactPath(stderr) - roots.push(artifacts) expect(stderr).toContain("script crashed") expect(await Bun.file(join(root, "registry", `${name}.json`)).exists()).toBe(false) const pid = Number(await Bun.file(join(artifacts, "child.pid")).text()) @@ -1007,7 +999,6 @@ describe("opencode-drive", () => { expect(await child.exited).toBe(0) const error = await stderr const artifacts = artifactPath(error) - roots.push(artifacts) expect(await Bun.file(join(artifacts, "manual-clients.json")).json()).toEqual({ apiHealthy: true, aliceFrame: { cols: 80, rows: 24 }, @@ -1039,7 +1030,6 @@ describe("opencode-drive", () => { const stderr = new Response(child.stderr).text() expect(await child.exited).toBe(0) const artifacts = artifactPath(await stderr) - roots.push(artifacts) const events = (await Bun.file(join(artifacts, "tool-control-events.jsonl")).text()) .trim() .split("\n") @@ -1067,7 +1057,6 @@ describe("opencode-drive", () => { ) const stderr = new Response(child.stderr).text() expect(await child.exited).toBe(0) - roots.push(artifactPath(await stderr)) }, 60_000) test("kills and relaunches the scripted server and TUIs", async () => { @@ -1089,7 +1078,6 @@ describe("opencode-drive", () => { expect(await child.exited).toBe(0) const error = await stderr const artifacts = artifactPath(error) - roots.push(artifacts) const result = await Bun.file(join(artifacts, "kill-server-result.json")).json() expect(Number.isInteger(result.firstServer)).toBe(true) expect(Number.isInteger(result.secondServer)).toBe(true) @@ -1122,7 +1110,6 @@ describe("opencode-drive", () => { const stderr = new Response(child.stderr).text() expect(await child.exited).toBe(0) const artifacts = artifactPath(await stderr) - roots.push(artifacts) const result = await Bun.file(join(artifacts, "failed-launch-retry.json")).json() expect(Number.isInteger(result.firstPid)).toBe(true) expect(Number.isInteger(result.secondPid)).toBe(true) @@ -1156,7 +1143,6 @@ describe("opencode-drive", () => { const stderr = new Response(child.stderr).text() expect(await child.exited).toBe(0) const artifacts = artifactPath(await stderr) - roots.push(artifacts) expect(await Bun.file(join(artifacts, "tui-options.json")).json()).toEqual({ primaryRecording: true, secondaryRecording: false, @@ -1295,7 +1281,6 @@ describe("opencode-drive", () => { const stderr = new Response(child.stderr).text() expect(await child.exited).toBe(0) const artifacts = artifactPath(await stderr) - roots.push(artifacts) const events = (await Bun.file(join(artifacts, "backend-events.jsonl")).text()) .trim() .split("\n") @@ -1330,7 +1315,6 @@ describe("opencode-drive", () => { const stderr = new Response(child.stderr).text() expect(await child.exited).toBe(0) const artifacts = artifactPath(await stderr) - roots.push(artifacts) const events = (await Bun.file(join(artifacts, "backend-events.jsonl")).text()) .trim() .split("\n") @@ -1393,7 +1377,6 @@ describe("opencode-drive", () => { root, ) const manifest = await waitForManifest(root, name) - roots.push(manifest.artifacts) await waitForLines(join(manifest.artifacts, "script-runs.txt"), 1) expect(await spawn(["restart", "--name", name], root).exited).toBe(0) @@ -1454,7 +1437,6 @@ describe("opencode-drive", () => { root, ) const manifest = await waitForManifest(root, name) - roots.push(manifest.artifacts) const openCodePid = Number(await Bun.file(join(manifest.artifacts, "child.pid")).text()) process.kill(child.pid, "SIGTERM") @@ -1485,7 +1467,6 @@ describe("opencode-drive", () => { root, ) const manifest = await waitForManifest(root, name) - roots.push(manifest.artifacts) process.kill(child.pid, "SIGTERM") await child.exited @@ -1722,9 +1703,9 @@ async function waitForLines(file: string, count: number) { throw new Error(`timed out waiting for ${count} lines in ${file}`) } -async function waitForManifest(root: string, name: string) { +async function waitForManifest(root: string, name: string, timeout = 10_000) { const file = join(root, "registry", `${name}.json`) - const deadline = Date.now() + 10_000 + const deadline = Date.now() + timeout while (Date.now() < deadline) { const manifest = await Bun.file(file) .json() @@ -1765,7 +1746,7 @@ async function expectSingleConcurrentStart(prepared: boolean) { const stderr = children.map((child) => new Response(child.stderr).text()) let managed = false try { - const manifest = await waitForManifest(root, name) + const manifest = await waitForManifest(root, name, 65_000) instances.push({ root, name }) managed = true const statuses = await Promise.all( @@ -1796,14 +1777,21 @@ async function terminateUnmanagedInstance(root: string, name: string) { typeof manifest !== "object" || manifest === null || !("pid" in manifest) || - typeof manifest.pid !== "number" || - !isProcessAlive(manifest.pid) + typeof manifest.pid !== "number" ) return - process.kill(manifest.pid, "SIGTERM") - const deadline = Date.now() + 2_000 - while (isProcessAlive(manifest.pid) && Date.now() < deadline) await Bun.sleep(25) - if (isProcessAlive(manifest.pid)) process.kill(manifest.pid, "SIGKILL") + for (const signal of ["SIGTERM", "SIGKILL"] as const) { + if (!running(manifest.pid)) return + try { + process.kill(manifest.pid, signal) + } catch (error) { + if (!running(manifest.pid)) return + throw error + } + const deadline = Date.now() + 2_000 + while (running(manifest.pid) && Date.now() < deadline) await Bun.sleep(25) + } + if (running(manifest.pid)) throw new Error(`failed to terminate drive owner ${manifest.pid}`) } async function waitForMissing(file: string) {