From 2a0ccdbfd4589ba2181624f25c55ed4ac0546b2d Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Wed, 12 Aug 2026 12:45:43 -0400 Subject: [PATCH] refactor(cli): simplify service replacement policy --- packages/cli/src/commands/handlers/default.ts | 7 --- .../cli/src/services/server-connection.ts | 49 +++++----------- packages/cli/src/services/service-config.ts | 4 -- .../cli/src/services/update-preflight.tsx | 52 ----------------- packages/cli/test/server-connection.test.ts | 58 +++++++++---------- packages/cli/test/service.test.ts | 16 ++--- packages/client/src/effect/service.ts | 18 ++---- packages/client/src/promise/service.ts | 15 +++-- packages/client/src/service-version.ts | 11 ++++ packages/client/src/service.ts | 12 +--- packages/client/test/fixture/service.ts | 4 +- packages/client/test/service.test.ts | 1 + 12 files changed, 74 insertions(+), 173 deletions(-) diff --git a/packages/cli/src/commands/handlers/default.ts b/packages/cli/src/commands/handlers/default.ts index 1db4e67e62..b323457589 100644 --- a/packages/cli/src/commands/handlers/default.ts +++ b/packages/cli/src/commands/handlers/default.ts @@ -23,14 +23,7 @@ export default Runtime.handler(Commands, (input) => server: Option.getOrUndefined(input.server), standalone: input.standalone, mismatch: "replace", - confirmDowngrade: (serverVersion) => - Effect.promise(async () => { - const confirmed = await preflight.confirmDowngrade(serverVersion) - if (confirmed) preflight.begin(serverVersion) - return confirmed - }), onStart: (reason, previousVersion) => { - if (preflight.active()) return if (reason === "version-mismatch" && preflight.begin(previousVersion)) return process.stderr.write( reason === "version-mismatch" diff --git a/packages/cli/src/services/server-connection.ts b/packages/cli/src/services/server-connection.ts index 6e639e2a01..0ee79b6a60 100644 --- a/packages/cli/src/services/server-connection.ts +++ b/packages/cli/src/services/server-connection.ts @@ -1,7 +1,7 @@ import { Service, VersionMismatchError, type Endpoint, type EnsureOptions } from "@opencode-ai/client/effect/service" import { ClientError, isUnauthorizedError, OpenCode } from "@opencode-ai/client/promise" import { OPENCODE_VERSION } from "../version" -import { Effect, Redacted, Result } from "effect" +import { Effect, Redacted } from "effect" import { Env } from "../env" import { ServiceConfig } from "./service-config" import { Standalone } from "./standalone" @@ -11,7 +11,6 @@ export type Args = { readonly standalone?: boolean readonly mismatch?: "replace" | "ignore" | "error" readonly onStart?: EnsureOptions["onStart"] - readonly confirmDowngrade?: (serverVersion: string, clientVersion: string) => Effect.Effect } export type Resolved = { @@ -46,7 +45,7 @@ export const resolve = Effect.fn("cli.server-connection.resolve")(function* (arg const mismatch = args.mismatch ?? "ignore" const options = yield* ServiceConfig.options({ checkVersion: mismatch !== "ignore" }) return { - endpoint: yield* resolveManaged({ ...options, onStart: args.onStart }, mismatch, args.confirmDowngrade), + endpoint: yield* resolveManaged({ ...options, onStart: args.onStart }, mismatch), service: managedService(options), } satisfies Resolved }) @@ -63,16 +62,17 @@ function managedService(options: EnsureOptions) { } } -const resolveManaged = Effect.fnUntraced(function* ( - options: EnsureOptions, - mismatch: NonNullable, - confirmDowngrade?: Args["confirmDowngrade"], -) { - if (mismatch === "replace") { - const result = yield* Effect.result(Service.ensure(options)) - if (Result.isSuccess(result)) return result.success - return yield* confirmManagedDowngrade(options, result.failure, confirmDowngrade) - } +const resolveManaged = Effect.fnUntraced(function* (options: EnsureOptions, mismatch: NonNullable) { + if (mismatch === "replace") + return yield* Service.ensure(options).pipe( + Effect.mapError((error) => + error instanceof VersionMismatchError + ? new Error(`${error.message}. Run \`opencode2 service restart\` to activate this installed version.`, { + cause: error, + }) + : error, + ), + ) if (mismatch === "ignore") return yield* Service.ensure({ ...options, version: undefined }) const compatible = yield* Service.discover(options) @@ -83,29 +83,6 @@ const resolveManaged = Effect.fnUntraced(function* ( return yield* Service.ensure(options) }) -export const confirmManagedDowngrade = Effect.fnUntraced(function* ( - options: EnsureOptions, - error: unknown, - confirm?: Args["confirmDowngrade"], -) { - if ( - !(error instanceof VersionMismatchError) || - error.serverVersion === undefined || - error.clientVersion === undefined || - !Service.canReplaceVersion(error.clientVersion, error.serverVersion) || - confirm === undefined - ) - return yield* Effect.fail(error) - if (!(yield* confirm(error.serverVersion, error.clientVersion))) - return yield* Effect.fail( - new Error(`${error.message}. Run \`opencode2 service restart\` to activate this installed version.`, { - cause: error, - }), - ) - yield* Service.stop(options) - return yield* Service.ensure(options) -}) - function connectError(endpoint: Endpoint, cause: unknown) { if (isUnauthorizedError(cause)) { return new Error( diff --git a/packages/cli/src/services/service-config.ts b/packages/cli/src/services/service-config.ts index fdbd40b135..94e072dc69 100644 --- a/packages/cli/src/services/service-config.ts +++ b/packages/cli/src/services/service-config.ts @@ -109,10 +109,6 @@ export const options = Effect.fnUntraced(function* (input: { readonly checkVersi } }) -export function canReplaceVersion(serverVersion: string | undefined, clientVersion = OPENCODE_VERSION) { - return Service.canReplaceVersion(serverVersion, clientVersion) -} - export const read = Effect.fn("cli.service-config.read")(function* () { const { fs, configFile, legacyConfigFile } = yield* paths if (legacyConfigFile) yield* migrateConfig(legacyConfigFile, configFile) diff --git a/packages/cli/src/services/update-preflight.tsx b/packages/cli/src/services/update-preflight.tsx index 3fb5d73047..f18f4a8ff3 100644 --- a/packages/cli/src/services/update-preflight.tsx +++ b/packages/cli/src/services/update-preflight.tsx @@ -28,9 +28,7 @@ const transitionDuration = 420 const completionHold = 650 export type Handle = { - readonly active: () => boolean readonly begin: (from?: string) => boolean - readonly confirmDowngrade: (from: string) => Promise readonly loading: () => void readonly finish: () => Promise readonly fail: (message: string) => Promise @@ -46,7 +44,6 @@ export type Handoff = { export const make = (): Handle => { let session: Promise | undefined return { - active: () => session !== undefined, begin: (from) => { if (!process.stdout.isTTY || !process.stdin.isTTY) return false session ??= open(from).catch(() => { @@ -55,7 +52,6 @@ export const make = (): Handle => { }) return true }, - confirmDowngrade: (from) => confirmDowngrade(from), loading: () => { void session?.then((active) => active?.loading()) }, @@ -201,40 +197,6 @@ async function open(from?: string): Promise { } } -async function confirmDowngrade(from: string) { - if (!process.stdout.isTTY || !process.stdin.isTTY) return false - const renderer = await createCliRenderer({ - stdin: process.stdin, - useMouse: false, - autoFocus: true, - openConsoleOnError: false, - exitOnCtrlC: false, - screenMode: "split-footer", - footerHeight: 4, - targetFps: 30, - useKittyKeyboard: {}, - externalOutputMode: "capture-stdout", - consoleMode: "disabled", - }) - const result = Promise.withResolvers() - const onKeypress = (event: { readonly name: string; readonly ctrl: boolean }) => { - if (event.name === "return") return finish(true) - if (event.name === "escape" || (event.ctrl && event.name === "c")) finish(false) - } - const finish = (confirmed: boolean) => { - renderer.keyInput.off("keypress", onKeypress) - if (!renderer.isDestroyed) renderer.destroy() - result.resolve(confirmed) - } - renderer.keyInput.on("keypress", onKeypress) - await render(() => , renderer).catch((error) => { - renderer.keyInput.off("keypress", onKeypress) - if (!renderer.isDestroyed) renderer.destroy() - throw error - }) - return result.promise -} - const colors = { accent: RGBA.fromHex("#a6b8ff"), accentBright: RGBA.fromHex("#eef1ff"), @@ -509,20 +471,6 @@ function UpdateFooter(props: { ) } -function DowngradeFooter(props: { from: string }) { - return ( - - - Background service - {props.from} - is newer than installed - {OPENCODE_VERSION} - - Press Enter to downgrade and restart ยท Esc to cancel - - ) -} - function CellLine(props: { cells: ReadonlyArray }) { return ( diff --git a/packages/cli/test/server-connection.test.ts b/packages/cli/test/server-connection.test.ts index fd2578796f..9fe73e62b8 100644 --- a/packages/cli/test/server-connection.test.ts +++ b/packages/cli/test/server-connection.test.ts @@ -8,7 +8,6 @@ import os from "node:os" import path from "node:path" import { ServerConnection } from "../src/services/server-connection" import { ServiceConfig } from "../src/services/service-config" -import { VersionMismatchError } from "@opencode-ai/client/effect/service" test("resolution groups Effect-native lifecycle operations only for the managed service", async () => { const root = await fs.mkdtemp(path.join(os.tmpdir(), "opencode-server-resolution-")) @@ -71,34 +70,33 @@ test("service options only require a matching version when requested", async () } }) -test("downgrade confirmation is offered only when the running service is newer", async () => { - const options = { version: "0.0.0-next-17271" } - const offered: string[] = [] - const confirm = (serverVersion: string) => - Effect.sync(() => { - offered.push(serverVersion) - return false - }) +test("normal launch refuses to replace a newer managed service", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "opencode-newer-service-")) + const layer = Global.layerWith({ config: path.join(root, "config"), state: path.join(root, "state") }) + const registration = path.join(root, "state", ServiceConfig.filename()) + using server = Bun.serve({ + port: 0, + fetch() { + return Response.json({ healthy: true, version: "999.0.0", pid: process.pid }) + }, + }) - await expect( - Effect.runPromise( - ServerConnection.confirmManagedDowngrade( - options, - new VersionMismatchError(options.version, "0.0.0-next-17272"), - confirm, - ).pipe(Effect.provide(NodeFileSystem.layer)), - ), - ).rejects.toThrow("Run `opencode2 service restart`") - expect(offered).toEqual(["0.0.0-next-17272"]) - - await expect( - Effect.runPromise( - ServerConnection.confirmManagedDowngrade( - { version: "0.0.0-next-17272" }, - new VersionMismatchError("0.0.0-next-17272", "0.0.0-next-17271"), - confirm, - ).pipe(Effect.provide(NodeFileSystem.layer)), - ), - ).rejects.toThrow("does not match") - expect(offered).toEqual(["0.0.0-next-17272"]) + try { + await fs.mkdir(path.dirname(registration), { recursive: true }) + await fs.writeFile( + registration, + JSON.stringify({ id: "newer-service", version: "999.0.0", url: server.url.toString(), pid: process.pid }), + ) + await expect( + Effect.runPromise( + ServerConnection.resolve({ mismatch: "replace" }).pipe( + Effect.provide(layer), + Effect.provide(NodeFileSystem.layer), + Effect.scoped, + ), + ), + ).rejects.toThrow("Run `opencode2 service restart` to activate this installed version") + } finally { + await fs.rm(root, { recursive: true, force: true }) + } }) diff --git a/packages/cli/test/service.test.ts b/packages/cli/test/service.test.ts index a7c3a2cdf5..02839693e6 100644 --- a/packages/cli/test/service.test.ts +++ b/packages/cli/test/service.test.ts @@ -1,5 +1,5 @@ import { NodeFileSystem } from "@effect/platform-node" -import { Service, type Info } from "@opencode-ai/client/effect/service" +import { Service, canReplaceVersion, type Info } from "@opencode-ai/client/effect/service" import { Global } from "@opencode-ai/util/global" import { OPENCODE_VERSION } from "../src/version" import { expect, test } from "bun:test" @@ -48,12 +48,12 @@ test("service filenames share release channels and identify preview channels", ( }) test("only newer clients replace managed service versions", () => { - expect(ServiceConfig.canReplaceVersion("0.0.0-next-17271", "0.0.0-next-17272")).toBe(true) - expect(ServiceConfig.canReplaceVersion("0.0.0-next-17272", "0.0.0-next-17271")).toBe(false) - expect(ServiceConfig.canReplaceVersion("0.0.0-next-17272", "0.0.0-next-17272")).toBe(false) - expect(ServiceConfig.canReplaceVersion(undefined, "0.0.0-next-17272")).toBe(false) - expect(ServiceConfig.canReplaceVersion("development-a", "development-b")).toBe(false) - expect(ServiceConfig.canReplaceVersion("development-b", "development-a")).toBe(false) + expect(canReplaceVersion("0.0.0-next-17271", "0.0.0-next-17272")).toBe(true) + expect(canReplaceVersion("0.0.0-next-17272", "0.0.0-next-17271")).toBe(false) + expect(canReplaceVersion("0.0.0-next-17272", "0.0.0-next-17272")).toBe(false) + expect(canReplaceVersion(undefined, "0.0.0-next-17272")).toBe(false) + expect(canReplaceVersion("development-a", "development-b")).toBe(false) + expect(canReplaceVersion("development-b", "development-a")).toBe(false) }) test("managed version replacement can never be mutual", () => { @@ -71,7 +71,7 @@ test("managed version replacement can never be mutual", () => { for (const left of versions) { for (const right of versions) { if (left === undefined || right === undefined) continue - expect(ServiceConfig.canReplaceVersion(left, right) && ServiceConfig.canReplaceVersion(right, left)).toBe(false) + expect(canReplaceVersion(left, right) && canReplaceVersion(right, left)).toBe(false) } } }) diff --git a/packages/client/src/effect/service.ts b/packages/client/src/effect/service.ts index 72af418e5d..4c0c0cbd6d 100644 --- a/packages/client/src/effect/service.ts +++ b/packages/client/src/effect/service.ts @@ -43,7 +43,7 @@ export const incumbent = Effect.fn("service.incumbent")(function* ( options: DiscoverOptions & { readonly url: string }, ) { const info = yield* read(options.file) - const found = info === undefined ? undefined : yield* probe({ ...info, url: options.url }) + const found = info === undefined ? undefined : yield* probeResult({ ...info, url: options.url }) if (found === undefined || found.legacy) return undefined if (!matchesVersion(found.version, options)) return undefined return { endpoint: found.endpoint, state: found.state } @@ -131,9 +131,9 @@ export const ensure = Effect.fn("service.ensure")(function* (options: EnsureOpti /** Stop the registered local service. */ export const stop = Effect.fn("service.stop")(function* (options: StopOptions = {}) { - const existing = yield* find(options) - if (existing !== undefined) yield* kill(existing, defaultEnsureTiming) - if (existing === undefined && (yield* read(options.file)) !== undefined) + const registration = yield* registered(options.file, true) + if (registration.service !== undefined) yield* kill(registration.service, defaultEnsureTiming) + if (registration.service === undefined && registration.info !== undefined) return yield* Effect.fail( new Error("Background service is not responding; stop its process manually and try again"), ) @@ -180,10 +180,6 @@ type LocalService = { readonly legacy: boolean } -const probe = Effect.fnUntraced(function* (info: Info, allowLegacy = false) { - return yield* probeResult(info, allowLegacy) -}) - const probeResult = Effect.fnUntraced(function* ( info: Info, allowLegacy = false, @@ -238,12 +234,6 @@ const registered = Effect.fnUntraced(function* (file?: string, allowLegacy = fal return { info, service: yield* probeResult(info, allowLegacy, timeout) } }) -// Health-checked lookup without the version gate: lifecycle operations must be -// able to see (and replace or stop) a server from a different version. -const find = Effect.fnUntraced(function* (options: { readonly file?: string }) { - return (yield* registered(options.file, true)).service -}) - // Poll until an authenticated stop exits, bounded by the configured stop window. const poll = (timing: EnsureTiming) => Schedule.max([Schedule.spaced(timing.stopPollInterval), Schedule.recurs(timing.stopPollAttempts)]) diff --git a/packages/client/src/promise/service.ts b/packages/client/src/promise/service.ts index bca19684a5..cadcee2d1c 100644 --- a/packages/client/src/promise/service.ts +++ b/packages/client/src/promise/service.ts @@ -76,7 +76,10 @@ export async function ensure(options: EnsureOptions = {}): Promise { if (compatible && service.state === "failed") throw new Error("Background service failed to start") if (!compatible) { if (options.canReplace?.(service.version) !== true) - throw new VersionMismatchError(typeof options.version === "string" ? options.version : undefined, service.version) + throw new VersionMismatchError( + typeof options.version === "string" ? options.version : undefined, + service.version, + ) await kill(service, timing) announce("version-mismatch", service.version) lastSpawn = 0 @@ -106,9 +109,9 @@ export async function ensure(options: EnsureOptions = {}): Promise { /** Stop the registered local service. */ export async function stop(options: StopOptions = {}) { - const existing = await find(options) - if (existing !== undefined) await kill(existing, defaultEnsureTiming) - if (existing === undefined && (await read(options.file)) !== undefined) + const registration = await registered(options.file, true) + if (registration.service !== undefined) await kill(registration.service, defaultEnsureTiming) + if (registration.service === undefined && registration.info !== undefined) throw new Error("Background service is not responding; stop its process manually and try again") } @@ -215,10 +218,6 @@ async function registered(file?: string, allowLegacy = false, timeout?: number) return { info, service: await probeResult(info, allowLegacy, timeout) } } -async function find(options: { readonly file?: string }) { - return (await registered(options.file, true)).service -} - function stopped(pid: number) { try { process.kill(pid, 0) diff --git a/packages/client/src/service-version.ts b/packages/client/src/service-version.ts index 5586f111a9..9e88cd31f1 100644 --- a/packages/client/src/service-version.ts +++ b/packages/client/src/service-version.ts @@ -1,4 +1,5 @@ import type { DiscoverOptions } from "./service.js" +import semver from "semver" export function matchesVersion(version: string | undefined, options: DiscoverOptions) { if (options.version === undefined) return true @@ -6,3 +7,13 @@ export function matchesVersion(version: string | undefined, options: DiscoverOpt if (typeof options.version === "function") return options.version(version) return version === options.version } + +/** Whether a client version is strictly newer than a service version. */ +export function canReplaceVersion(serverVersion: string | undefined, clientVersion: string) { + if (serverVersion === undefined) return false + // Compare preview build numbers numerically rather than as semver prerelease strings. + const server = serverVersion.replace(/^(0\.0\.0-.+)-(\d+(?:\.\d+)?)$/, "$1.$2") + const client = clientVersion.replace(/^(0\.0\.0-.+)-(\d+(?:\.\d+)?)$/, "$1.$2") + if (!semver.valid(server) || !semver.valid(client)) return false + return semver.lt(server, client) +} diff --git a/packages/client/src/service.ts b/packages/client/src/service.ts index c52c9e42af..0cfde81446 100644 --- a/packages/client/src/service.ts +++ b/packages/client/src/service.ts @@ -1,4 +1,4 @@ -import semver from "semver" +export { canReplaceVersion } from "./service-version.js" /** Connection details for a local OpenCode service. */ export type Endpoint = { @@ -48,16 +48,6 @@ export class VersionMismatchError extends Error { } } -/** Whether a client version is strictly newer than a service version. */ -export function canReplaceVersion(serverVersion: string | undefined, clientVersion: string) { - if (serverVersion === undefined) return false - // Compare preview build numbers numerically rather than as semver prerelease strings. - const server = serverVersion.replace(/-(\d+)(?=(?:\.\d+)?$)/, ".$1") - const client = clientVersion.replace(/-(\d+)(?=(?:\.\d+)?$)/, ".$1") - if (!semver.valid(server) || !semver.valid(client)) return false - return semver.lt(server, client) -} - /** Options used to stop the local OpenCode service. */ export type StopOptions = { /** Absolute registration file path. Defaults to the XDG state directory. */ diff --git a/packages/client/test/fixture/service.ts b/packages/client/test/fixture/service.ts index 09dee01465..dcdf66969a 100644 --- a/packages/client/test/fixture/service.ts +++ b/packages/client/test/fixture/service.ts @@ -44,7 +44,7 @@ const server = Bun.serve({ await appendFile(registration + ".stop-attempts", process.pid + "\n") return new Promise(() => {}) } - if (pathname === "/api/service/stop" && (mode === "graceful" || mode === "old")) { + if (pathname === "/api/service/stop" && (mode === "graceful" || mode === "old" || mode === "incompatible")) { const body = await request.json() if (typeof body !== "object" || body === null || body.instanceID !== id) return Response.json({ accepted: false }) await writeFile(registration + ".stop", JSON.stringify(body)) @@ -67,8 +67,6 @@ const server = Bun.serve({ if (mode === "starting" && !(await Bun.file(registration + ".release").exists())) return Response.json({ healthy: true, version, pid: process.pid }, { status: 503 }) if (mode === "failed-owner") return Response.json({ healthy: true, version, pid: process.pid }, { status: 500 }) - if (mode === "starting" || mode === "graceful" || mode === "reject-stop" || mode === "stop-hanging") - return Response.json({ healthy: true, version, pid: process.pid }) return Response.json({ healthy: true, version, pid: process.pid }) }, }) diff --git a/packages/client/test/service.test.ts b/packages/client/test/service.test.ts index adb9fa4d37..1fbd9f7df1 100644 --- a/packages/client/test/service.test.ts +++ b/packages/client/test/service.test.ts @@ -79,6 +79,7 @@ test("replaces an incompatible registered service", async () => { ensure({ file: registration, version: (version) => version.startsWith("2."), + canReplace: () => true, command: [process.execPath, fixture, registration, "delayed-compatible", "10"], onStart: (reason) => starts.push(reason), }),