refactor(cli): simplify service replacement policy

2026-08-12 12:45:43 -04:00
parent 88a9ee5d11
commit 2a0ccdbfd4
12 changed files with 74 additions and 173 deletions
@@ -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"
@@ -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<boolean>
 }

 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<Args["mismatch"]>,
-  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<Args["mismatch"]>) {
+  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(
@@ -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)
@@ -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<boolean>
  readonly loading: () => void
  readonly finish: () => Promise<Handoff | undefined>
  readonly fail: (message: string) => Promise<void>
@@ -46,7 +44,6 @@ export type Handoff = {
 export const make = (): Handle => {
  let session: Promise<Session | undefined> | 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<Session> {
  }
 }

-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<boolean>()
-  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(() => <DowngradeFooter from={from} />, 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 (
-    <box width="100%" height={4} flexDirection="column" paddingLeft={1}>
-      <text fg={colors.text}>
-        <span style={{ fg: colors.muted }}>Background service </span>
-        <span style={{ fg: colors.accent }}>{props.from}</span>
-        <span style={{ fg: colors.muted }}> is newer than installed </span>
-        <span style={{ fg: colors.accent }}>{OPENCODE_VERSION}</span>
-      </text>
-      <text fg={colors.muted}>Press Enter to downgrade and restart · Esc to cancel</text>
-    </box>
-  )
-}
-
 function CellLine(props: { cells: ReadonlyArray<Cell> }) {
  return (
    <text truncate>
@@ -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 })
+  }
 })
@@ -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)
    }
  }
 })
@@ -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)])
@@ -76,7 +76,10 @@ export async function ensure(options: EnsureOptions = {}): Promise<Endpoint> {
        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<Endpoint> {

 /** 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)
@@ -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)
+}
@@ -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. */
@@ -44,7 +44,7 @@ const server = Bun.serve({
      await appendFile(registration + ".stop-attempts", process.pid + "\n")
      return new Promise<Response>(() => {})
    }
-    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 })
  },
 })
@@ -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),
    }),