From 95d536803e214007baf260118ab1e2bd534f3d1f Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sun, 19 Jul 2026 14:49:54 -0400 Subject: [PATCH] fix(media): isolate outputs by run generation --- .changeset/run-scoped-media.md | 5 ++ packages/drive/README.md | 2 +- packages/drive/src/instance/instance.ts | 2 +- packages/drive/src/instance/media.ts | 18 ++--- packages/drive/src/instance/runtime.ts | 16 +++-- packages/drive/test/cli/integration.test.ts | 70 ++++++++++++++----- .../drive/test/fixtures/restart-script.ts | 10 ++- 7 files changed, 87 insertions(+), 36 deletions(-) create mode 100644 .changeset/run-scoped-media.md diff --git a/.changeset/run-scoped-media.md b/.changeset/run-scoped-media.md new file mode 100644 index 0000000..48a1a2b --- /dev/null +++ b/.changeset/run-scoped-media.md @@ -0,0 +1,5 @@ +--- +"opencode-drive": minor +--- + +Write screenshots and recordings beneath run- and restart-scoped media directories so named outputs cannot overwrite earlier runs. diff --git a/packages/drive/README.md b/packages/drive/README.md index 357efd9..aff6c7e 100644 --- a/packages/drive/README.md +++ b/packages/drive/README.md @@ -182,7 +182,7 @@ If you installed the skill file, OpenCode will be able to see and interact with Install the skill file above and ask the agent to test various flows with the app. Start with `--record` when you want a video; `opencode-drive stop` then exports the complete session and prints its path. -Screenshots and videos are written to `/opencode-drive/output` with unique filenames. Set `OPENCODE_DRIVE_MEDIA_DIR` to use a different directory. +Screenshots and videos are written beneath `/opencode-drive/output//`, so named outputs cannot overwrite media from earlier runs or restarts. Set `OPENCODE_DRIVE_MEDIA_DIR` to use a different media root. Captured frames use the official full Commit Mono v1.143 faces at 16px with bundled Noto Symbols, Symbols 2, and Math fallbacks in a fixed 10x20 cell grid. Set `OPENCODE_DRIVE_FONT` to a comma-separated list of font files (for example regular, bold, italic, and bold-italic faces) to use a different primary capture font without changing the symbol fallback or cell geometry. diff --git a/packages/drive/src/instance/instance.ts b/packages/drive/src/instance/instance.ts index ffd32f6..77a5978 100644 --- a/packages/drive/src/instance/instance.ts +++ b/packages/drive/src/instance/instance.ts @@ -21,7 +21,7 @@ export function artifactDirectory() { export async function initializeInstance(name?: string) { const artifacts = resolve( - join(artifactDirectory(), `run-${crypto.randomUUID().slice(0, 6)}`), + join(artifactDirectory(), `run-${crypto.randomUUID()}`), ) const logs = join(artifacts, "logs") const drive = join(artifacts, "drive") diff --git a/packages/drive/src/instance/media.ts b/packages/drive/src/instance/media.ts index de8f4d8..22abaf4 100644 --- a/packages/drive/src/instance/media.ts +++ b/packages/drive/src/instance/media.ts @@ -1,16 +1,16 @@ -import { mkdir } from "node:fs/promises" -import { tmpdir } from "node:os" -import { join, resolve } from "node:path" +import { basename, join, resolve } from "node:path" +import { artifactDirectory } from "./instance.js" export function mediaDirectory() { return resolve( process.env.OPENCODE_DRIVE_MEDIA_DIR ?? - join(tmpdir(), "opencode-drive", "output"), + join(artifactDirectory(), "output"), ) } -export async function ensureMediaDirectory() { - const directory = mediaDirectory() - await mkdir(directory, { recursive: true }) - return directory -} +export const runMediaDirectory = (artifacts: string, generation: number) => + join( + mediaDirectory(), + basename(resolve(artifacts)), + `generation-${generation}`, + ) diff --git a/packages/drive/src/instance/runtime.ts b/packages/drive/src/instance/runtime.ts index 82b2a98..4951531 100644 --- a/packages/drive/src/instance/runtime.ts +++ b/packages/drive/src/instance/runtime.ts @@ -10,7 +10,7 @@ import * as Scope from "effect/Scope" import { ChildProcessSpawner } from "effect/unstable/process" import { prepareDev } from "./dev.js" import { instanceError, OpenCodeInstanceError } from "./error.js" -import { ensureMediaDirectory } from "./media.js" +import { runMediaDirectory } from "./media.js" import { prepareInstanceProject } from "./instance.js" import * as Process from "./process.js" import { freePort, waitForWebSocket } from "./readiness.js" @@ -111,10 +111,8 @@ export const make = Effect.fn("OpenCodeInstance.make")(function* ( const logs = join(artifacts, "logs") const drive = join(artifacts, "drive") const files = join(artifacts, "files") - const media = yield* Effect.tryPromise({ - try: () => ensureMediaDirectory(), - catch: (cause) => instanceError("prepare media", cause), - }) + let mediaGeneration = 0 + let media = runMediaDirectory(artifacts, mediaGeneration) const endpoints = { ui: `ws://127.0.0.1:${yield* freePort}`, backend: `ws://127.0.0.1:${yield* freePort}`, @@ -148,7 +146,6 @@ export const make = Effect.fn("OpenCodeInstance.make")(function* ( OPENCODE_DRIVE_SCRIPTED: options.scripted ? "1" : undefined, DRIVE_REGISTRY_DIR: drive, OPENCODE_DRIVE_RENDERER: options.visible ? "visible" : "headless", - OPENCODE_DRIVE_MEDIA_DIR: media, OPENCODE_CONFIG_DIR: join(files, ".opencode"), OPENCODE_DB: database, OPENCODE_LOG_LEVEL: !options.visible ? "DEBUG" : process.env.OPENCODE_LOG_LEVEL, @@ -209,7 +206,11 @@ export const make = Effect.fn("OpenCodeInstance.make")(function* ( options.log?.(`launching ${logName}`) return yield* Process.spawn(appCommand, { cwd: files, - env: { ...environment, OPENCODE_DRIVE: driveName }, + env: { + ...environment, + OPENCODE_DRIVE: driveName, + OPENCODE_DRIVE_MEDIA_DIR: media, + }, stdin: visible ? "inherit" : "ignore", stdout: visible ? "inherit" : { path: join(logs, `${logName}.stdout.log`) }, stderr: visible ? "inherit" : { path: join(logs, `${logName}.stderr.log`) }, @@ -484,6 +485,7 @@ export const make = Effect.fn("OpenCodeInstance.make")(function* ( return yield* Effect.failCause(combined.cause).pipe( Effect.mapError((cause) => instanceError("restart", cause)), ) + media = runMediaDirectory(artifacts, ++mediaGeneration) const recording = options.record ? recordingPaths(media) : undefined yield* Ref.set(state, State.Running({ recording, diff --git a/packages/drive/test/cli/integration.test.ts b/packages/drive/test/cli/integration.test.ts index 6a619f6..3719a2c 100644 --- a/packages/drive/test/cli/integration.test.ts +++ b/packages/drive/test/cli/integration.test.ts @@ -1,7 +1,7 @@ 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 { join, resolve } from "node:path" +import { basename, dirname, join, resolve } from "node:path" import { controlPath, listInstances, @@ -126,7 +126,8 @@ describe("opencode-drive", () => { const screenshot = spawn(["send", "--name", name, "--command.ui.screenshot"], root) expect(await screenshot.exited).toBe(0) const screenshotPath = (await new Response(screenshot.stdout).text()).trim() - expect(screenshotPath.startsWith(`${join(root, "output")}/screenshot-`)).toBe(true) + expect(dirname(screenshotPath)).toBe(runOutputDirectory(root, manifest.artifacts, 0)) + expect(basename(screenshotPath).startsWith("screenshot-")).toBe(true) expect(screenshotPath.endsWith(".png")).toBe(true) const listed = spawn(["dir", "--name", name], root) @@ -145,7 +146,8 @@ describe("opencode-drive", () => { const restarted = spawn(["restart", "--name", name], root) expect(await restarted.exited).toBe(0) const restartedRecording = (await new Response(restarted.stdout).text()).trim() - expect(restartedRecording).toMatch(/\/output\/recording-.*\.mp4$/) + expect(dirname(restartedRecording)).toBe(runOutputDirectory(root, manifest.artifacts, 0)) + expect(basename(restartedRecording)).toMatch(/^recording-.*\.mp4$/) expect(await Bun.file(restartedRecording).exists()).toBe(true) await waitForLines(join(manifest.artifacts, "launches.txt"), 2) expect(await spawn(["send", "--name", name, "--command.ui.state"], root).exited).toBe(0) @@ -160,7 +162,8 @@ describe("opencode-drive", () => { expect(stoppedOutput).toBe("") const stoppedRecording = stoppedError.match(/Video successfully created: (.+\.mp4)/)?.[1] expect(stoppedRecording).toBeDefined() - expect(stoppedRecording).toMatch(/\/output\/recording-.*\.mp4$/) + expect(dirname(stoppedRecording!)).toBe(runOutputDirectory(root, manifest.artifacts, 1)) + expect(basename(stoppedRecording!)).toMatch(/^recording-.*\.mp4$/) expect(await Bun.file(stoppedRecording!).exists()).toBe(true) expect(stoppedError).toContain("Rendering video: 10%") expect(stoppedError).toContain("Rendering video: 100%") @@ -191,10 +194,11 @@ describe("opencode-drive", () => { if (Date.now() >= deadline) throw new Error("timed out waiting for exited instance cleanup") await Bun.sleep(25) } - const files = await readdir(join(root, "output")) + const directory = runOutputDirectory(root, artifacts!, 0) + const files = await readdir(directory) const video = files.find((file) => file.endsWith(".mp4")) expect(video).toBeDefined() - expect(await Bun.file(join(root, "output", video!)).size).toBeGreaterThan(500) + expect(await Bun.file(join(directory, video!)).size).toBeGreaterThan(500) }, 15_000) test("does not record unless start receives --record", async () => { @@ -212,7 +216,7 @@ describe("opencode-drive", () => { const stopped = spawn(["stop", "--name", name], root) expect(await stopped.exited).toBe(0) expect(await new Response(stopped.stdout).text()).toBe("success\n") - expect(await readdir(join(root, "output"))).toEqual([]) + expect(await Bun.file(join(root, "output")).exists()).toBe(false) expect(await Bun.file(manifest.artifacts).exists()).toBe(false) instances.pop() }) @@ -503,18 +507,36 @@ describe("opencode-drive", () => { test("runs multiple named instances concurrently", async () => { const root = await temporary() - for (const name of ["first", "second"]) { - expect( - await spawn(["start", "--name", name, "--", process.execPath, fixture("fake-opencode.ts")], root).exited, - ).toBe(0) - instances.push({ root, name }) - } + const names = ["first", "second"] as const + const starts = names.map((name) => + spawn(["start", "--name", name, "--", process.execPath, fixture("fake-opencode.ts")], root), + ) + expect(await Promise.all(starts.map((child) => child.exited))).toEqual([0, 0]) + 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) + const screenshots = await Promise.all( + [first, second].map(async (manifest, index) => { + const name = index === 0 ? "first" : "second" + const screenshot = spawn([ + "send", + "--name", + name, + "--command.ui.screenshot", + '{"name":"shared"}', + ], root) + expect(await screenshot.exited).toBe(0) + return (await new Response(screenshot.stdout).text()).trim() + }), + ) + expect(screenshots).toEqual([ + join(runOutputDirectory(root, first.artifacts, 0), "shared.png"), + join(runOutputDirectory(root, second.artifacts, 0), "shared.png"), + ]) const unnamed = spawn(["dir"], root) const [status, stderr] = await Promise.all([unnamed.exited, new Response(unnamed.stderr).text()]) @@ -828,8 +850,8 @@ describe("opencode-drive", () => { bobMatches: true, tuiBeforeServer: "launch the script server before launching TUIs", duplicateServer: "the script server has already been launched", - aliceScreenshot: join(root, "output", "alice.png"), - bobScreenshot: join(root, "output", "bob.png"), + aliceScreenshot: join(runOutputDirectory(root, artifacts, 0), "alice.png"), + bobScreenshot: join(runOutputDirectory(root, artifacts, 0), "bob.png"), }) expect((await Bun.file(join(artifacts, "launches.txt")).text()).trim().split("\n")).toHaveLength(2) }, 60_000) @@ -907,7 +929,8 @@ describe("opencode-drive", () => { expect(Number.isInteger(result.firstServer)).toBe(true) expect(Number.isInteger(result.secondServer)).toBe(true) expect(result.secondServer).not.toBe(result.firstServer) - expect(result.aliceRecording).toMatch(/\/output\/recording-.*\.mp4$/) + expect(dirname(result.aliceRecording)).toBe(runOutputDirectory(root, artifacts, 0)) + expect(basename(result.aliceRecording)).toMatch(/^recording-.*\.mp4$/) expect(await Bun.file(result.aliceRecording).exists()).toBe(true) const recordings = [...error.matchAll(/opencode-drive: recording (.+\.mp4)/g)].map((match) => match[1]!) expect(recordings).toHaveLength(2) @@ -1184,6 +1207,13 @@ describe("opencode-drive", () => { expect(await spawn(["restart", "--name", name], root).exited).toBe(0) await waitForLines(join(manifest.artifacts, "script-runs.txt"), 2) + const screenshots = (await Bun.file(join(manifest.artifacts, "script-screenshots.txt")).text()) + .trim() + .split("\n") + expect(screenshots).toEqual([ + join(runOutputDirectory(root, manifest.artifacts, 0), "restart-shared.png"), + join(runOutputDirectory(root, manifest.artifacts, 1), "restart-shared.png"), + ]) expect(await spawn(["stop", "--name", name], root).exited).toBe(0) expect(await owner.exited).toBe(0) }) @@ -1269,7 +1299,9 @@ describe("opencode-drive", () => { process.kill(child.pid, "SIGTERM") await child.exited - expect((await readdir(join(root, "output"))).some((file) => file.endsWith(".mp4"))).toBe(true) + expect( + (await readdir(runOutputDirectory(root, manifest.artifacts, 0))).some((file) => file.endsWith(".mp4")), + ).toBe(true) expect(await Bun.file(join(root, "registry", `${name}.json`)).exists()).toBe(false) expect(await Bun.file(join(root, "registry", `${name}.sock`)).exists()).toBe(false) }, 60_000) @@ -1476,6 +1508,10 @@ function fixture(name: string) { return resolve("test", "fixtures", name) } +function runOutputDirectory(root: string, artifacts: string, generation: number) { + return join(root, "output", basename(artifacts), `generation-${generation}`) +} + async function temporary() { const root = await mkdtemp(join(tmpdir(), "opencode-drive-test-")) roots.push(root) diff --git a/packages/drive/test/fixtures/restart-script.ts b/packages/drive/test/fixtures/restart-script.ts index a45f9e8..dd2e86b 100644 --- a/packages/drive/test/fixtures/restart-script.ts +++ b/packages/drive/test/fixtures/restart-script.ts @@ -3,7 +3,7 @@ import * as Effect from "effect/Effect" import * as Stream from "effect/Stream" export default defineScript({ - run: ({ artifacts, llm }) => + run: ({ artifacts, llm, ui }) => Effect.gen(function* () { yield* llm.serve(() => Stream.fromEffect( @@ -11,9 +11,17 @@ export default defineScript({ ), ) const file = `${artifacts}/script-runs.txt` + const screenshotFile = `${artifacts}/script-screenshots.txt` const previous = yield* Effect.promise(() => Bun.file(file).text().catch(() => ""), ) + const screenshots = yield* Effect.promise(() => + Bun.file(screenshotFile).text().catch(() => ""), + ) + const screenshot = yield* ui.screenshot("restart-shared") + yield* Effect.tryPromise(() => + Bun.write(screenshotFile, `${screenshots}${screenshot}\n`), + ) yield* Effect.tryPromise(() => Bun.write(file, `${previous}run\n`)) yield* Effect.never }),