From 0548ad718f4d15eb9d5cad429f7781ac0d1d075e Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Wed, 22 Jul 2026 21:36:34 +0000 Subject: [PATCH] test(cli): isolate service registration lease --- packages/cli/src/server-process.ts | 60 ++++--------------- .../cli/src/services/service-registration.ts | 53 ++++++++++++++++ packages/cli/test/service.test.ts | 43 +++++++------ 3 files changed, 87 insertions(+), 69 deletions(-) create mode 100644 packages/cli/src/services/service-registration.ts diff --git a/packages/cli/src/server-process.ts b/packages/cli/src/server-process.ts index 273b11a0da..e857221936 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, Logger, Option, Redacted, Schedule, Schema } from "effect" +import { Effect, Logger, 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" export type Mode = "default" | "service" | "stdio" @@ -124,7 +124,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, + instanceID, + serviceOptions.file, + shutdown, + ) }), }, ).pipe( @@ -161,52 +167,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), - Effect.orElseSucceed(() => undefined), - ) - const owns = (found: Info | undefined) => - 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.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 0000000000..c54ef21d99 --- /dev/null +++ b/packages/cli/src/services/service-registration.ts @@ -0,0 +1,53 @@ +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* ( + 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), + Effect.orElseSucceed(() => undefined), + ) + const owns = (found: Info | undefined) => + 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.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, + ) +}) diff --git a/packages/cli/test/service.test.ts b/packages/cli/test/service.test.ts index cd47de29b4..f0ff919ae8 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) @@ -413,32 +414,37 @@ 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", 60_000) - expect(new URL(info.url).port).toBe(String(port)) - expect(info.pid).toBe(owner.pid) + const cleanup = await Effect.runPromise( + ServiceRegistration.register( + { _tag: "TcpAddress", hostname: "127.0.0.1", port: 4321 }, + "secret", + "owner", + registration, + 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 }) } -}, 75_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-")) @@ -479,9 +485,8 @@ test("a failed service stays registered and owns the selected port until stopped } }, 30_000) -async function waitForInfo(file: string, accept: (info: Info) => boolean = () => true, timeout = 20_000) { - const deadline = Date.now() + timeout - while (Date.now() < deadline) { +async function waitForInfo(file: string, accept: (info: Info) => boolean = () => true) { + for (let attempt = 0; attempt < 400; attempt++) { const value = await Bun.file(file) .json() .catch(() => undefined)