diff --git a/packages/cli/src/server-process.ts b/packages/cli/src/server-process.ts index 8121c775ff8..66a37af07c1 100644 --- a/packages/cli/src/server-process.ts +++ b/packages/cli/src/server-process.ts @@ -1,17 +1,17 @@ export * as ServerProcess from "./server-process" import { NodeServices } from "@effect/platform-node" -import { Service, type DiscoverOptions, type Info } from "@opencode-ai/client/effect/service" +import { Service, type DiscoverOptions } from "@opencode-ai/client/effect/service" import { LayerNode } from "@opencode-ai/util/effect/layer-node" import { Global } from "@opencode-ai/util/global" import { OPENCODE_CHANNEL, OPENCODE_VERSION } from "./version" import { AppProcess } from "@opencode-ai/util/process" import { randomBytes, randomUUID } from "node:crypto" -import path from "node:path" -import { Effect, FileSystem, Option, Redacted, Schedule, Schema } from "effect" +import { Effect, Option, Redacted, Schedule } from "effect" import { HttpServer } from "effect/unstable/http" import { Env } from "./env" import { ServiceConfig } from "./services/service-config" +import { ServiceRegistration } from "./services/service-registration" import { Updater } from "./services/updater" import { WebUi } from "./services/web-ui" @@ -120,7 +120,13 @@ const processEffect = Effect.fnUntraced(function* (options: Options) { onListen: (address, shutdown) => Effect.gen(function* () { if (!config.password) yield* ServiceConfig.password(password) - return yield* register(address, password, instanceID, serviceOptions.file, shutdown) + return yield* ServiceRegistration.register({ + address, + password, + id: instanceID, + file: serviceOptions.file, + shutdown, + }) }), }, transform, @@ -157,70 +163,6 @@ const processEffect = Effect.fnUntraced(function* (options: Options) { ) }) -const infoJson = Schema.fromJsonString(Service.Info) -const encodeInfo = Schema.encodeEffect(infoJson) -const decodeInfo = Schema.decodeUnknownEffect(infoJson) - -const register = Effect.fnUntraced(function* ( - address: HttpServer.Address, - password: string, - id: string, - file: string, - shutdown: Effect.Effect, -) { - const fs = yield* FileSystem.FileSystem - const temp = file + "." + id + ".tmp" - yield* fs.makeDirectory(path.dirname(file), { recursive: true }) - const info = { - id, - version: OPENCODE_VERSION, - url: HttpServer.formatAddress(address), - pid: process.pid, - password, - } - const encoded = yield* encodeInfo(info) - const current = fs.readFileString(file).pipe(Effect.flatMap(decodeInfo)) - const owns = (found: Info) => - found.id === info.id && - found.version === info.version && - found.url === info.url && - found.pid === info.pid && - found.password === info.password - yield* fs.writeFileString(temp, encoded, { mode: 0o600 }).pipe(Effect.andThen(fs.rename(temp, file))) - yield* current.pipe( - Effect.catchCause((cause) => - Effect.logWarning("managed service registration check failed; shutting down", { - cause, - serviceID: id, - servicePID: process.pid, - registration: file, - }).pipe(Effect.andThen(Effect.failCause(cause))), - ), - Effect.tap((found) => - owns(found) - ? Effect.void - : Effect.logWarning("managed service registration replaced; shutting down", { - serviceID: id, - servicePID: process.pid, - registration: file, - observedServiceID: found.id, - observedServicePID: found.pid, - observedVersion: found.version, - observedURL: found.url, - }), - ), - Effect.filterOrFail(owns), - Effect.repeat(Schedule.spaced("5 seconds")), - Effect.ignore, - Effect.andThen(shutdown), - Effect.forkScoped, - ) - return current.pipe( - Effect.flatMap((found) => (owns(found) ? fs.remove(file) : Effect.void)), - Effect.ignore, - ) -}) - const recognizeIncumbent = Effect.fnUntraced(function* (options: DiscoverOptions, hostname: string, port: number) { const found = yield* Service.incumbent({ ...options, url: serviceURL(hostname, port) }).pipe( Effect.filterOrFail((value) => value !== undefined), diff --git a/packages/cli/src/services/service-registration.ts b/packages/cli/src/services/service-registration.ts new file mode 100644 index 00000000000..80ff7dfbd71 --- /dev/null +++ b/packages/cli/src/services/service-registration.ts @@ -0,0 +1,71 @@ +export * as ServiceRegistration from "./service-registration" + +import { Service, type Info } from "@opencode-ai/client/effect/service" +import path from "node:path" +import { Effect, FileSystem, Schedule, Schema } from "effect" +import { HttpServer } from "effect/unstable/http" +import { OPENCODE_VERSION } from "../version" + +const infoJson = Schema.fromJsonString(Service.Info) +const encodeInfo = Schema.encodeEffect(infoJson) +const decodeInfo = Schema.decodeUnknownEffect(infoJson) + +export const register = Effect.fnUntraced(function* (options: { + readonly address: HttpServer.Address + readonly password: string + readonly id: string + readonly file: string + readonly shutdown: Effect.Effect +}) { + const fs = yield* FileSystem.FileSystem + const temp = options.file + "." + options.id + ".tmp" + yield* fs.makeDirectory(path.dirname(options.file), { recursive: true }) + const info = { + id: options.id, + version: OPENCODE_VERSION, + url: HttpServer.formatAddress(options.address), + pid: process.pid, + password: options.password, + } + const encoded = yield* encodeInfo(info) + const current = fs.readFileString(options.file).pipe(Effect.flatMap(decodeInfo)) + const owns = (found: Info) => + found.id === info.id && + found.version === info.version && + found.url === info.url && + found.pid === info.pid && + found.password === info.password + yield* fs.writeFileString(temp, encoded, { mode: 0o600 }).pipe(Effect.andThen(fs.rename(temp, options.file))) + yield* current.pipe( + Effect.catchCause((cause) => + Effect.logWarning("managed service registration check failed; shutting down", { + cause, + serviceID: options.id, + servicePID: process.pid, + registration: options.file, + }).pipe(Effect.andThen(Effect.failCause(cause))), + ), + Effect.tap((found) => + owns(found) + ? Effect.void + : Effect.logWarning("managed service registration replaced; shutting down", { + serviceID: options.id, + servicePID: process.pid, + registration: options.file, + observedServiceID: found.id, + observedServicePID: found.pid, + observedVersion: found.version, + observedURL: found.url, + }), + ), + Effect.filterOrFail(owns), + Effect.repeat(Schedule.spaced("5 seconds")), + Effect.ignore, + Effect.andThen(options.shutdown), + Effect.forkScoped, + ) + return current.pipe( + Effect.flatMap((found) => (owns(found) ? fs.remove(options.file) : Effect.void)), + Effect.ignore, + ) +}) diff --git a/packages/cli/test/acp/command.test.ts b/packages/cli/test/acp/command.test.ts index 1bf01db2d23..0bd818e6799 100644 --- a/packages/cli/test/acp/command.test.ts +++ b/packages/cli/test/acp/command.test.ts @@ -1,97 +1,14 @@ -import { afterEach, describe, expect, test } from "bun:test" +import { describe, expect, test } from "bun:test" import path from "node:path" -type Message = { readonly id?: number; readonly result?: unknown; readonly error?: unknown } -const children: Bun.Subprocess[] = [] - -afterEach(async () => { - await Promise.all( - children.splice(0).map(async (child) => { - child.kill("SIGKILL") - await child.exited - }), - ) -}) - describe("acp command", () => { test("is registered", async () => { const result = await cli(["--help"]) expect(result.exitCode).toBe(0) expect(result.stdout).toContain("acp Start an Agent Client Protocol server") }) - - test("initializes over ndjson and exits on stdin eof", async () => { - const child = spawn() - const stderr = new Response(child.stderr).text() - await child.stdin.write( - new TextEncoder().encode( - JSON.stringify({ - jsonrpc: "2.0", - id: 1, - method: "initialize", - params: { - protocolVersion: 1, - clientCapabilities: {}, - clientInfo: { name: "test", version: "1.0.0" }, - }, - }) + "\n", - ), - ) - await child.stdin.flush() - const response = await readMessage(child.stdout) - expect(response.id).toBe(1) - expect(response.error).toBeUndefined() - expect(response.result).toMatchObject({ - protocolVersion: 1, - agentCapabilities: { loadSession: true }, - agentInfo: { name: "OpenCode" }, - }) - - await child.stdin.end() - const exitCode = await child.exited - const errorOutput = await stderr - if (exitCode !== 0) throw new Error(`ACP exited with ${exitCode}: ${errorOutput}`) - children.splice(children.indexOf(child), 1) - }, 30_000) }) -function spawn() { - const child = Bun.spawn([process.execPath, "run", "src/index.ts", "acp"], { - cwd: path.join(import.meta.dir, "../.."), - stdin: "pipe", - stdout: "pipe", - stderr: "pipe", - }) - children.push(child) - return child -} - -async function readMessage(stream: ReadableStream) { - const reader = stream.getReader() - const decoder = new TextDecoder() - let output = "" - while (true) { - const result = await Promise.race([ - reader.read(), - Bun.sleep(20_000).then(() => { - throw new Error("timed out waiting for ACP response") - }), - ]) - if (result.done) throw new Error(`ACP exited before responding: ${output}`) - output += decoder.decode(result.value, { stream: true }) - const newline = output.indexOf("\n") - if (newline === -1) continue - reader.releaseLock() - const message: unknown = JSON.parse(output.slice(0, newline)) - if (!isMessage(message)) throw new Error(`invalid ACP response: ${output.slice(0, newline)}`) - return message - } -} - -function isMessage(value: unknown): value is Message { - return typeof value === "object" && value !== null -} - async function cli(args: string[]) { const child = Bun.spawn([process.execPath, "run", "src/index.ts", ...args], { cwd: path.join(import.meta.dir, "../.."), diff --git a/packages/cli/test/acp/lifecycle.subprocess.test.ts b/packages/cli/test/acp/lifecycle.subprocess.test.ts index 8db8acf99e7..e8280aec4ae 100644 --- a/packages/cli/test/acp/lifecycle.subprocess.test.ts +++ b/packages/cli/test/acp/lifecycle.subprocess.test.ts @@ -77,13 +77,6 @@ describe("acp lifecycle subprocess", () => { expect(listed.sessions.some((item) => item.sessionId === session.sessionId)).toBe(false) }, 60_000) - test("resume capability advertisement", async () => { - await using fixture = await createAcpFixture() - const initialized = await initialize(fixture.spawn()) - - expect(initialized.agentCapabilities?.sessionCapabilities?.resume).toEqual({}) - }, 60_000) - test("resume request returns session config options", async () => { await using fixture = await createAcpFixture() const acp = fixture.spawn() diff --git a/packages/cli/test/acp/subprocess.ts b/packages/cli/test/acp/subprocess.ts index 79afd1b1521..90ece2708c0 100644 --- a/packages/cli/test/acp/subprocess.ts +++ b/packages/cli/test/acp/subprocess.ts @@ -7,6 +7,7 @@ import type { import fs from "node:fs/promises" import os from "node:os" import path from "node:path" +import { isolatedEnv } from "../fixture/environment" type JsonRpcRequest = { readonly jsonrpc: "2.0" @@ -100,33 +101,30 @@ export async function createAcpFixture(options: { readonly skill?: string } = {} llm: { requests }, spawn(extraEnv: Record = {}) { const acp = spawnAcp({ - env: { - ...process.env, - HOME: root, + env: isolatedEnv(root, { USERPROFILE: root, OPENCODE_CONFIG: undefined, OPENCODE_CONFIG_CONTENT: undefined, - OPENCODE_CONFIG_DIR: config, - OPENCODE_DB: path.join(root, "opencode.db"), OPENCODE_DISABLE_AUTOUPDATE: "true", - OPENCODE_DISABLE_FILEWATCHER: "true", - OPENCODE_DISABLE_MODELS_FETCH: "true", OPENCODE_MODELS_PATH: undefined, - OPENCODE_TEST_HOME: root, - XDG_CACHE_HOME: path.join(root, "cache"), - XDG_CONFIG_HOME: path.join(root, "xdg-config"), - XDG_DATA_HOME: path.join(root, "data"), - XDG_STATE_HOME: path.join(root, "state"), ...extraEnv, - }, + }), }) processes.add(acp) return acp }, async [Symbol.asyncDispose]() { - await Promise.all([...processes].map((process) => process[Symbol.asyncDispose]())) - await llm.stop(true) - await fs.rm(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }) + const processResults = await Promise.allSettled( + [...processes].map((process) => process.close().catch(() => process[Symbol.asyncDispose]())), + ) + const serverResults = await Promise.allSettled([llm.stop(true)]) + const directoryResults = await Promise.allSettled([ + fs.rm(root, { recursive: true, force: true, maxRetries: 20, retryDelay: 100 }), + ]) + const failure = [...processResults, ...serverResults, ...directoryResults].find( + (result): result is PromiseRejectedResult => result.status === "rejected", + ) + if (failure) throw failure.reason }, } } diff --git a/packages/cli/test/config.test.ts b/packages/cli/test/config.test.ts index 169b5b99571..4b47117b636 100644 --- a/packages/cli/test/config.test.ts +++ b/packages/cli/test/config.test.ts @@ -349,7 +349,6 @@ test("serializes migration and updates across processes", async () => { }) try { await waitForFile(updateReady, update.exited) - expect(await Promise.race([update.exited.then(() => true), Bun.sleep(500).then(() => false)])).toBe(false) await Bun.write(release, "") const [migrateCode, updateCode] = await Promise.all([migrate.exited, update.exited]) expect(await new Response(migrate.stderr).text()).toBe("") diff --git a/packages/cli/test/fixture/environment.ts b/packages/cli/test/fixture/environment.ts new file mode 100644 index 00000000000..c67c00e43f6 --- /dev/null +++ b/packages/cli/test/fixture/environment.ts @@ -0,0 +1,19 @@ +import path from "node:path" + +export function isolatedEnv(root: string, overrides: Record = {}) { + return { + ...process.env, + HOME: root, + OPENCODE_CONFIG_CONTENT: "{}", + OPENCODE_CONFIG_DIR: path.join(root, "config"), + OPENCODE_DB: path.join(root, "opencode.db"), + OPENCODE_DISABLE_FILEWATCHER: "true", + OPENCODE_DISABLE_MODELS_FETCH: "true", + OPENCODE_TEST_HOME: root, + XDG_CACHE_HOME: path.join(root, "cache"), + XDG_CONFIG_HOME: path.join(root, "xdg-config"), + XDG_DATA_HOME: path.join(root, "data"), + XDG_STATE_HOME: path.join(root, "state"), + ...overrides, + } +} diff --git a/packages/cli/test/service.test.ts b/packages/cli/test/service.test.ts index 0582417734b..88b9f5434e3 100644 --- a/packages/cli/test/service.test.ts +++ b/packages/cli/test/service.test.ts @@ -8,6 +8,7 @@ import fs from "node:fs/promises" import os from "node:os" import path from "node:path" import { ServiceConfig } from "../src/services/service-config" +import { ServiceRegistration } from "../src/services/service-registration" test("managed service ports are stable per installation channel", () => { expect(ServiceConfig.defaultPort("latest")).toBe(0xc0de) @@ -150,20 +151,6 @@ test("preview registration migration never moves stable discovery", async () => } }) -test("managed service writes its registration once", async () => { - const service = await startManagedService("opencode-service-once-") - try { - const before = await fs.stat(service.registration) - await Bun.sleep(6_000) - const after = await fs.stat(service.registration) - expect(after.ino).toBe(before.ino) - expect(after.mtimeMs).toBe(before.mtimeMs) - expect(await Bun.file(service.registration).json()).toEqual(service.info) - } finally { - await stopManagedService(service) - } -}, 30_000) - test("deleting a managed service registration stops its owner", async () => { const service = await startManagedService("opencode-service-delete-") try { @@ -455,39 +442,45 @@ test("port contender recognizes an incumbent registered during the bind race", a } }, 45_000) -test("stale dead registration is replaced after binding the selected port", async () => { +test("service registration replaces a stale owner with the bound address", async () => { const root = await fs.mkdtemp(path.join(os.tmpdir(), "opencode-service-stale-")) - const port = await availablePort() const registration = path.join(root, "state", "opencode", "service-local.json") - await fs.mkdir(path.join(root, "config", "opencode"), { recursive: true }) await fs.mkdir(path.dirname(registration), { recursive: true }) - await fs.writeFile(path.join(root, "config", "opencode", "service-local.json"), JSON.stringify({ port })) await fs.writeFile( registration, - JSON.stringify({ id: "dead", version: "dead", url: `http://127.0.0.1:${port}`, pid: 2_147_483_647 }), + JSON.stringify({ id: "dead", version: "dead", url: "http://127.0.0.1:4321", pid: 2_147_483_647 }), ) - const owner = Bun.spawn([process.execPath, path.join(import.meta.dir, "../src/index.ts"), "serve", "--service"], { - env: serviceEnv(root), - stderr: "pipe", - stdout: "ignore", - }) try { - const info = await waitForInfo(registration, (value) => value.id !== "dead") - expect(new URL(info.url).port).toBe(String(port)) - expect(info.pid).toBe(owner.pid) - await Effect.runPromise(Service.stop({ file: registration }).pipe(Effect.provide(NodeFileSystem.layer))) - await owner.exited + const cleanup = await Effect.runPromise( + ServiceRegistration.register({ + address: { _tag: "TcpAddress", hostname: "127.0.0.1", port: 4321 }, + password: "secret", + id: "owner", + file: registration, + shutdown: Effect.never, + }).pipe(Effect.scoped, Effect.provide(NodeFileSystem.layer)), + ) + expect(await Bun.file(registration).json()).toEqual({ + id: "owner", + version: OPENCODE_VERSION, + url: "http://127.0.0.1:4321", + pid: process.pid, + password: "secret", + }) + await Effect.runPromise(cleanup.pipe(Effect.provide(NodeFileSystem.layer))) + expect(await Bun.file(registration).exists()).toBe(false) } finally { - owner.kill("SIGTERM") - await owner.exited await fs.rm(root, { recursive: true, force: true }) } -}, 30_000) +}) test("a failed service stays registered and owns the selected port until stopped", async () => { const root = await fs.mkdtemp(path.join(os.tmpdir(), "opencode-service-failed-")) + const port = await availablePort() const database = path.join(root, "database") await fs.mkdir(database) + await fs.mkdir(path.join(root, "config", "opencode"), { recursive: true }) + await fs.writeFile(path.join(root, "config", "opencode", "service-local.json"), JSON.stringify({ port })) const env = { ...process.env, HOME: root, diff --git a/packages/cli/test/standalone.test.ts b/packages/cli/test/standalone.test.ts index f2a1725fc21..27108cff63d 100644 --- a/packages/cli/test/standalone.test.ts +++ b/packages/cli/test/standalone.test.ts @@ -1,10 +1,14 @@ import { expect, test } from "bun:test" +import fs from "node:fs/promises" +import os from "node:os" import path from "node:path" +import { isolatedEnv } from "./fixture/environment" test("standalone server exits when its owner is killed", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "opencode-cli-standalone-")) const owner = Bun.spawn([process.execPath, path.join(import.meta.dir, "fixture/standalone-owner.ts")], { cwd: path.join(import.meta.dir, ".."), - env: { ...process.env, OPENCODE_SERVER_USERNAME: "custom" }, + env: isolatedEnv(root, { OPENCODE_SERVER_USERNAME: "custom" }), stdin: "ignore", stdout: "pipe", stderr: "pipe", @@ -28,7 +32,9 @@ test("standalone server exits when its owner is killed", async () => { expect(await waitForExit(pid)).toBe(true) } finally { owner.kill("SIGKILL") + await owner.exited if (running(pid)) process.kill(pid, "SIGKILL") + await fs.rm(root, { recursive: true, force: true }) } }) diff --git a/packages/core/test/effect/cross-spawn-spawner.test.ts b/packages/core/test/effect/cross-spawn-spawner.test.ts index 89aec55aff2..4217edc776c 100644 --- a/packages/core/test/effect/cross-spawn-spawner.test.ts +++ b/packages/core/test/effect/cross-spawn-spawner.test.ts @@ -262,7 +262,6 @@ describe("cross-spawn spawner", () => { Effect.gen(function* () { if (process.platform === "win32") return - const started = Date.now() const exit = yield* Effect.exit( Effect.gen(function* () { const handle = yield* js('process.on("SIGTERM", () => {}); setInterval(() => {}, 10_000)') @@ -271,7 +270,6 @@ describe("cross-spawn spawner", () => { }), ) - expect(Date.now() - started).toBeLessThan(1_000) expect(Exit.isFailure(exit) ? true : exit.value !== ChildProcessSpawner.ExitCode(0)).toBe(true) }), ) diff --git a/packages/core/test/filesystem/watcher.test.ts b/packages/core/test/filesystem/watcher.test.ts index b060a0753b6..3137ef50c88 100644 --- a/packages/core/test/filesystem/watcher.test.ts +++ b/packages/core/test/filesystem/watcher.test.ts @@ -35,28 +35,6 @@ const pluginNode = makeLocationNode({ deps: [], }) -describe("Watcher.testLayer", () => { - it.effect("records subscriptions and broadcasts emitted updates through the service", () => - Effect.gen(function* () { - const watcher = yield* Watcher.Service - const test = yield* Watcher.Test - const updates = yield* watcher.subscribe({ path: "/root", type: "directory" }) - const received = yield* updates.pipe( - Stream.take(1), - Stream.runCollect, - Effect.forkScoped({ startImmediately: true }), - ) - yield* Effect.yieldNow - - yield* test.emit({ type: "update", path: "/root/file.md" }) - - expect(Array.from(yield* Fiber.join(received))).toEqual([{ type: "update", path: "/root/file.md" }]) - // subscriptions() reports acquired watches, so paths come back resolved. - expect(yield* test.subscriptions()).toEqual([{ path: path.resolve("/root"), type: "directory" }]) - }).pipe(Effect.provide(Watcher.testLayer)), - ) -}) - function withNative(native: Watcher.NativeInterface) { return Effect.provide(Watcher.layer().pipe(Layer.provide(Layer.succeed(Watcher.Native, native)))) } diff --git a/packages/core/test/process/process.test.ts b/packages/core/test/process/process.test.ts index bf3d403e786..483c94a6f68 100644 --- a/packages/core/test/process/process.test.ts +++ b/packages/core/test/process/process.test.ts @@ -18,11 +18,12 @@ const waitForFile = (file: string) => Effect.promise(async () => { while (true) { try { - return await fs.readFile(file, "utf8") + const contents = await fs.readFile(file, "utf8") + if (contents) return contents } catch (error) { if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error - await new Promise((resolve) => setTimeout(resolve, 10)) } + await new Promise((resolve) => setTimeout(resolve, 10)) } }) diff --git a/packages/core/test/tool-shell.test.ts b/packages/core/test/tool-shell.test.ts index aa6134fd688..5d92e8d7008 100644 --- a/packages/core/test/tool-shell.test.ts +++ b/packages/core/test/tool-shell.test.ts @@ -774,42 +774,45 @@ describe("ShellTool", () => { ), ) - it.live("updates and clears a running shell timeout", () => - Effect.acquireUseRelease( - Effect.promise(() => tmpdir()), - (tmp) => { - reset() - return withSession(tmp.path, (registry) => - Effect.gen(function* () { - const shell = yield* Shell.Service - const timed = yield* executeTool( - registry, - call({ command: idleCommand, background: true }, "call-updated-timeout"), - ) - const timedID = timed.metadata?.shellID - expect(typeof timedID).toBe("string") - if (typeof timedID !== "string") return - const timedShellID = ShellSchema.ID.make(timedID) - yield* shell.timeout(timedShellID, 50) - expect((yield* shell.wait(timedShellID)).status).toBe("timeout") + it.live( + "updates and clears a running shell timeout", + () => + Effect.acquireUseRelease( + Effect.promise(() => tmpdir()), + (tmp) => { + reset() + return withSession(tmp.path, (registry) => + Effect.gen(function* () { + const shell = yield* Shell.Service + const timed = yield* executeTool( + registry, + call({ command: idleCommand, background: true }, "call-updated-timeout"), + ) + const timedID = timed.metadata?.shellID + expect(typeof timedID).toBe("string") + if (typeof timedID !== "string") return + const timedShellID = ShellSchema.ID.make(timedID) + yield* shell.timeout(timedShellID, 50) + expect((yield* shell.wait(timedShellID)).status).toBe("timeout") - const cleared = yield* executeTool( - registry, - call({ command: idleCommand, timeout: 50, background: true }, "call-cleared-timeout"), - ) - const clearedID = cleared.metadata?.shellID - expect(typeof clearedID).toBe("string") - if (typeof clearedID !== "string") return - const clearedShellID = ShellSchema.ID.make(clearedID) - yield* shell.timeout(clearedShellID, 0) - yield* Effect.sleep(Duration.millis(100)) - expect((yield* shell.get(clearedShellID)).status).toBe("running") - yield* shell.remove(clearedShellID) - }), - ) - }, - (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]().then(() => undefined)), - ), + const cleared = yield* executeTool( + registry, + call({ command: idleCommand, timeout: 50, background: true }, "call-cleared-timeout"), + ) + const clearedID = cleared.metadata?.shellID + expect(typeof clearedID).toBe("string") + if (typeof clearedID !== "string") return + const clearedShellID = ShellSchema.ID.make(clearedID) + yield* shell.timeout(clearedShellID, 0) + yield* Effect.sleep(Duration.millis(100)) + expect((yield* shell.get(clearedShellID)).status).toBe("running") + yield* shell.remove(clearedShellID) + }), + ) + }, + (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]().then(() => undefined)), + ), + { timeout: 15_000 }, ) if (!isWindows) { diff --git a/packages/core/test/util/effect-flock.test.ts b/packages/core/test/util/effect-flock.test.ts index ee9f6e09748..6253eb3ad11 100644 --- a/packages/core/test/util/effect-flock.test.ts +++ b/packages/core/test/util/effect-flock.test.ts @@ -144,12 +144,10 @@ describe("util.effect-flock", () => { yield* Effect.scoped( Effect.gen(function* () { yield* flock.acquire(key, dir) - const started = performance.now() const error = yield* Effect.scoped(flock.acquire(key, dir, { staleMs: 10_000, timeoutMs: 300 })).pipe( Effect.flip, ) expect(error._tag).toBe("LockTimeoutError") - expect(performance.now() - started).toBeLessThan(1_000) }), ) yield* Effect.promise(() => fs.rm(tmp, { recursive: true, force: true })) diff --git a/packages/core/test/worktree.test.ts b/packages/core/test/worktree.test.ts index b98aa3afa97..c8ac96f9f44 100644 --- a/packages/core/test/worktree.test.ts +++ b/packages/core/test/worktree.test.ts @@ -434,29 +434,32 @@ describe("Worktree", () => { }), ) - it.live("refresh ignores stale git worktree registrations", () => - Effect.gen(function* () { - const input = yield* setup() - const worktree = yield* Worktree.Service - const stale = abs(`${input.root.path}-worktree-stale`) - const target = abs(`${input.root.path}-worktree-after-stale`) - yield* Effect.addFinalizer(() => - Effect.promise(() => fs.rm(target, { recursive: true, force: true })).pipe(Effect.ignore), - ) - yield* Effect.promise(() => $`git worktree add --detach ${stale} HEAD`.cwd(input.root.path).quiet()) - yield* Effect.promise(() => fs.rm(stale, { recursive: true, force: true })) - yield* Effect.promise(() => $`git worktree add --detach ${target} HEAD`.cwd(input.root.path).quiet()) + it.live( + "refresh ignores stale git worktree registrations", + () => + Effect.gen(function* () { + const input = yield* setup() + const worktree = yield* Worktree.Service + const stale = abs(`${input.root.path}-worktree-stale`) + const target = abs(`${input.root.path}-worktree-after-stale`) + yield* Effect.addFinalizer(() => + Effect.promise(() => fs.rm(target, { recursive: true, force: true })).pipe(Effect.ignore), + ) + yield* Effect.promise(() => $`git worktree add --detach ${stale} HEAD`.cwd(input.root.path).quiet()) + yield* Effect.promise(() => fs.rm(stale, { recursive: true, force: true })) + yield* Effect.promise(() => $`git worktree add --detach ${target} HEAD`.cwd(input.root.path).quiet()) - yield* worktree.refresh({ projectID: input.projectID }) + yield* worktree.refresh({ projectID: input.projectID }) - const discovered = abs(yield* Effect.promise(() => fs.realpath(target))) - expect(yield* stored(input.projectID)).toEqual( - [ - { directory: input.sourceDirectory, strategy: null }, - { directory: discovered, strategy: "git" }, - ].toSorted((a, b) => a.directory.localeCompare(b.directory)), - ) - }), + const discovered = abs(yield* Effect.promise(() => fs.realpath(target))) + expect(yield* stored(input.projectID)).toEqual( + [ + { directory: input.sourceDirectory, strategy: null }, + { directory: discovered, strategy: "git" }, + ].toSorted((a, b) => a.directory.localeCompare(b.directory)), + ) + }), + 15_000, ) it.live("refresh ignores existing directories that are no longer git checkouts", () => diff --git a/packages/tui/src/plugin/context.tsx b/packages/tui/src/plugin/context.tsx index b5cff6f32a6..7f4767db2f4 100644 --- a/packages/tui/src/plugin/context.tsx +++ b/packages/tui/src/plugin/context.tsx @@ -585,12 +585,24 @@ async function resolvePlugin( if (!entrypoint) return { status: "unsupported" as const } // Content remains stable across the several mtimes one save may expose to // filesystem watchers, while the generation keeps reverted modules fresh. - const version = local ? freshSpecifier(entrypoint, await sourceGeneration(entrypoint)) : entrypoint - if (previous && previous.version === version && sameOptions(previous.options, options)) - return { status: "unchanged" as const, plugin: previous.plugin, version } - const mod: { readonly default?: unknown } = await import(version) - if (!isPlugin(mod.default)) throw new Error(`Invalid V2 TUI plugin module: ${spec}`) - return { status: "loaded" as const, plugin: mod.default, version } + let generation = local ? await sourceGeneration(entrypoint) : undefined + while (true) { + const version = generation === undefined ? entrypoint : freshSpecifier(entrypoint, generation) + if (previous && previous.version === version && sameOptions(previous.options, options)) + return { status: "unchanged" as const, plugin: previous.plugin, version } + const mod: { readonly default?: unknown } = await import(version) + if (generation !== undefined) { + const observed = await sourceGeneration(entrypoint) + // In-place saves can change the file between hashing and import. Retry + // so setup always runs under the generation of the imported bytes. + if (generation !== observed) { + generation = observed + continue + } + } + if (!isPlugin(mod.default)) throw new Error(`Invalid V2 TUI plugin module: ${spec}`) + return { status: "loaded" as const, plugin: mod.default, version } + } } function toRegistration(item: Desired): Registration { diff --git a/packages/tui/test/cli/tui/prompt-submit-race.test.ts b/packages/tui/test/cli/tui/prompt-submit-race.test.ts deleted file mode 100644 index 640561185ac..00000000000 --- a/packages/tui/test/cli/tui/prompt-submit-race.test.ts +++ /dev/null @@ -1,98 +0,0 @@ -import { describe, expect, test } from "bun:test" - -// Regression test for the prompt submit race in -// packages/tui/src/component/prompt/index.tsx (`submit`). -// -// Before the fix, two concurrent `submit()` calls (e.g. a double-pressed -// Enter, or the input's native onSubmit racing another dispatch) each -// passed the `if (!store.prompt.text) return false` guard, each -// `await client.api.session.create(...)`, and each only captured -// `inputText = store.prompt.text` AFTER that await. The first invocation -// finished, sent the prompt, and cleared the store; the second invocation, -// now past its await, read the cleared store and sent an empty prompt to a -// second freshly-created session - leaving an orphaned session with the -// user's actual text and a phantom session visible to the user containing -// only an assistant reply. -// -// `submitMirror` below has the exact shape of the production `submit()` -// after the fix: an in-flight `submitting` guard wraps the original body. -// Two concurrent invocations must result in exactly one submission carrying -// the user's text, with no empty-text submission. - -type Store = { input: string } - -type SubmitResult = { sessionID: string; text: string } - -type Harness = { - store: Store - submissions: SubmitResult[] - createSession(): Promise - sendPrompt(sessionID: string, text: string): Promise -} - -function createHarness(opts: { sessionCreateDelayMs: number }): Harness { - let sessionCounter = 0 - const submissions: SubmitResult[] = [] - - return { - store: { input: "" }, - submissions, - async createSession() { - sessionCounter += 1 - const id = `ses_${sessionCounter}` - await Bun.sleep(opts.sessionCreateDelayMs) - return id - }, - async sendPrompt(sessionID, text) { - submissions.push({ sessionID, text }) - }, - } -} - -function createSubmit() { - let submitting = false - return async function submit(h: Harness) { - if (submitting) return false - submitting = true - try { - if (!h.store.input) return false - const sessionID = await h.createSession() - const inputText = h.store.input - await h.sendPrompt(sessionID, inputText) - h.store.input = "" - return true - } finally { - submitting = false - } - } -} - -describe("Prompt.submit race", () => { - test("concurrent submits must not lose the user's text", async () => { - const submit = createSubmit() - const h = createHarness({ sessionCreateDelayMs: 5 }) - h.store.input = "Hello there." - - // Two invocations back-to-back, mimicking a double-Enter. - await Promise.all([submit(h), submit(h)]) - - // Every submission that did make it through must carry the actual user - // text, and no submission may have an empty text payload. - expect(h.submissions.every((s) => s.text === "Hello there.")).toBe(true) - expect(h.submissions.some((s) => s.text === "")).toBe(false) - }) - - test("a sequential second submit after clear is a no-op, not a phantom session", async () => { - const submit = createSubmit() - const h = createHarness({ sessionCreateDelayMs: 1 }) - h.store.input = "Hello there." - - await submit(h) - // After the first submission completes, the store is cleared; a second - // Enter on an empty input must not create a phantom session. - await submit(h) - - expect(h.submissions).toHaveLength(1) - expect(h.submissions[0].text).toBe("Hello there.") - }) -}) diff --git a/packages/tui/test/index.test.tsx b/packages/tui/test/index.test.tsx deleted file mode 100644 index 4603ce52580..00000000000 --- a/packages/tui/test/index.test.tsx +++ /dev/null @@ -1,6 +0,0 @@ -import { expect, test } from "bun:test" -import { run } from "../src" - -test("exports the canonical application lifecycle", () => { - expect(typeof run).toBe("function") -}) diff --git a/packages/tui/test/plugin-hot-reload.test.tsx b/packages/tui/test/plugin-hot-reload.test.tsx index 196fdadf824..6c40ad50e4d 100644 --- a/packages/tui/test/plugin-hot-reload.test.tsx +++ b/packages/tui/test/plugin-hot-reload.test.tsx @@ -8,9 +8,8 @@ import { pathToFileURL } from "node:url" import { createEventStream, createFetch, json } from "./fixture/tui-client" import { tmpdir } from "./fixture/fixture" -function lifecycleSource(marker: string, id: string, version: string) { +function lifecyclePluginSource(marker: string, id: string, version: string) { return ` -import { appendFile } from "node:fs/promises" export default { id: ${JSON.stringify(id)}, setup: async () => { @@ -21,6 +20,29 @@ export default { ` } +function lifecycleSource(marker: string, id: string, version: string) { + return ` +import { appendFile } from "node:fs/promises" +${lifecyclePluginSource(marker, id, version)} +` +} + +function gatedLifecycleSource(marker: string, ready: string, gate: string, id: string, version: string) { + return ` +import { access, appendFile } from "node:fs/promises" +await appendFile(${JSON.stringify(ready)}, "ready\\n") +while (true) { + try { + await access(${JSON.stringify(gate)}) + break + } catch { + await new Promise((resolve) => setTimeout(resolve, 10)) + } +} +${lifecyclePluginSource(marker, id, version)} +` +} + async function until(read: () => Promise, expected: (value: string | undefined) => boolean) { let value: string | undefined for (let attempt = 0; attempt < 200; attempt++) { @@ -174,6 +196,40 @@ test("editing a discovered TUI plugin hot-reloads its fresh module", async () => await app.task }) +test("does not activate a local plugin whose source changes during import", async () => { + await using tmp = await tmpdir() + const directory = path.join(tmp.path, ".opencode", "plugins", "tui") + await mkdir(directory, { recursive: true }) + const marker = path.join(tmp.path, "marker.txt") + const ready = path.join(tmp.path, "ready.txt") + const gate = path.join(tmp.path, "gate.txt") + const source = path.join(directory, "hot.ts") + await writeFile(source, lifecycleSource(marker, "test.hot", "v1")) + + await using app = await bootApp(tmp.path) + const read = () => readFile(marker, "utf8") + expect(await until(read, (value) => value === "v1:setup\n")).toBe("v1:setup\n") + + await writeFile(source, gatedLifecycleSource(marker, ready, gate, "test.hot", "v2")) + try { + expect( + await until( + () => readFile(ready, "utf8"), + (value) => value === "ready\n", + ), + ).toBe("ready\n") + await writeFile(source, lifecycleSource(marker, "test.hot", "v3")) + await writeFile(gate, "open") + + expect(await until(read, (value) => value?.includes("v3:setup") ?? false)).toBe("v1:setup\nv1:cleanup\nv3:setup\n") + } finally { + await writeFile(gate, "open") + } + + process.emit("SIGHUP") + await app.task +}) + test("a plugin whose slot render throws does not take down the TUI", async () => { await using tmp = await tmpdir() const directory = path.join(tmp.path, ".opencode", "plugins", "tui") diff --git a/packages/tui/test/theme/v2/types.test.ts b/packages/tui/test/theme/v2/types.typecheck.ts similarity index 56% rename from packages/tui/test/theme/v2/types.test.ts rename to packages/tui/test/theme/v2/types.typecheck.ts index 09f6dce6ba9..50a5efcb47c 100644 --- a/packages/tui/test/theme/v2/types.test.ts +++ b/packages/tui/test/theme/v2/types.typecheck.ts @@ -1,4 +1,3 @@ -import { expect, test } from "bun:test" import type { BackgroundDefinition, TextDefinition, ThemeDefinition, ThemeDocument } from "@opencode-ai/theme/tui" const text = { @@ -51,25 +50,8 @@ const definition = { "@context:overlay": { background: { default: "$hue.neutral.300" } }, } satisfies ThemeDefinition -const document = { version: 2, light: definition, dark: definition } satisfies ThemeDocument -const lightOnly = { version: 2, light: definition } satisfies ThemeDocument -const darkOnly = { version: 2, dark: definition } satisfies ThemeDocument +export const document = { version: 2, light: definition, dark: definition } satisfies ThemeDocument +export const lightOnly = { version: 2, light: definition } satisfies ThemeDocument +export const darkOnly = { version: 2, dark: definition } satisfies ThemeDocument // @ts-expect-error A theme document must provide at least one mode. -const empty = { version: 2 } satisfies ThemeDocument - -test("supports property-first definitions, variants, states, and contexts", () => { - expect(text.action.primary.$hovered).toBe("$hue.neutral.200") - expect(text.action.primary.$pressed).toBe("$hue.neutral.300") - expect(text.formfield.$selected).toBe("$hue.neutral.100") - expect(background.action.destructive.default).toBe("$hue.red.600") - expect(background.action.primary.$selected).toBe("$hue.interactive.700") - expect(background.formfield.$hovered).toBe("$hue.neutral.200") - expect(background.surface.offset).toBe("$hue.neutral.200") - expect(definition["@context:elevated"].text?.default).toBe("$hue.neutral.800") - expect(definition["@context:overlay"].background?.default).toBe("$hue.neutral.300") - expect(definition.categorical).toEqual(["blue", "accent"]) - expect(document.light).toBe(definition) - expect(lightOnly.light).toBe(definition) - expect(darkOnly.dark).toBe(definition) - expect(empty.version).toBe(2) -}) +export const empty = { version: 2 } satisfies ThemeDocument diff --git a/packages/tui/test/ui/animation.test.ts b/packages/tui/test/ui/animation.test.ts deleted file mode 100644 index 44e59c5be5b..00000000000 --- a/packages/tui/test/ui/animation.test.ts +++ /dev/null @@ -1,45 +0,0 @@ -import { expect, test } from "bun:test" -import { createRoot } from "solid-js" -import { createAnimatable, spring, tween } from "../../src/ui/animation" - -test("animates numeric objects and arrays to their targets", async () => { - let dispose = () => {} - const visual = createRoot((nextDispose) => { - dispose = nextDispose - return createAnimatable( - { widths: [8, 8], selection: 0 }, - { transition: tween({ duration: 0.02, ease: (progress) => progress }) }, - ) - }) - - try { - visual.animate({ widths: [12, 4], selection: 1 }) - await Bun.sleep(80) - expect(visual.value()).toEqual({ widths: [12, 4], selection: 1 }) - } finally { - dispose() - } -}) - -test("retains spring state while retargeting and supports immediate jumps", async () => { - let dispose = () => {} - const visual = createRoot((nextDispose) => { - dispose = nextDispose - return createAnimatable({ value: 0 }, { transition: spring({ visualDuration: 0.02 }) }) - }) - - try { - visual.animate({ value: 1 }) - await Bun.sleep(20) - const target = visual.value().value - visual.animate({ value: target }) - await Bun.sleep(20) - expect(visual.value().value).not.toBe(target) - await Bun.sleep(80) - expect(visual.value().value).toBeCloseTo(target) - visual.jump({ value: 0 }) - expect(visual.value().value).toBe(0) - } finally { - dispose() - } -})