From 910af9a122cb828abc4a12555facf82c9f88e149 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sun, 5 Jul 2026 13:31:19 -0400 Subject: [PATCH] fix(cli): restart stale clients after updates --- packages/cli/bin/opencode2.cjs | 4 +- packages/cli/src/commands/handlers/default.ts | 37 +++++++++++--- packages/cli/src/index.ts | 2 +- packages/cli/src/services/service-config.ts | 12 +++++ packages/cli/src/tui.ts | 5 +- packages/cli/test/fixture/restart-child.ts | 6 +++ packages/cli/test/launcher.test.ts | 43 ++++++++++++++++ packages/cli/test/service.test.ts | 12 +++++ packages/client/src/effect/service.ts | 35 +++++++++++-- packages/client/test/service.test.ts | 49 +++++++++++++++++++ packages/tui/src/app.tsx | 19 +++++-- packages/tui/src/context/sdk.tsx | 12 ++++- packages/tui/test/cli/tui/use-event.test.tsx | 35 +++++++++++-- 13 files changed, 246 insertions(+), 25 deletions(-) create mode 100644 packages/cli/test/fixture/restart-child.ts create mode 100644 packages/cli/test/launcher.test.ts create mode 100644 packages/client/test/service.test.ts diff --git a/packages/cli/bin/opencode2.cjs b/packages/cli/bin/opencode2.cjs index 8795c71cd228..1ca52e5ae332 100644 --- a/packages/cli/bin/opencode2.cjs +++ b/packages/cli/bin/opencode2.cjs @@ -6,8 +6,9 @@ const path = require("path") const os = require("os") const forwardedSignals = ["SIGINT", "SIGTERM", "SIGHUP"] +const restartExitCode = 75 -function run(target) { +function run(target, restarted = false) { const child = childProcess.spawn(target, process.argv.slice(2), { stdio: "inherit" }) child.on("error", (error) => { console.error(error.message) @@ -25,6 +26,7 @@ function run(target) { child.on("exit", (code, signal) => { for (const forwardedSignal of forwardedSignals) process.removeListener(forwardedSignal, forwarders[forwardedSignal]) if (signal) return process.kill(process.pid, signal) + if (code === restartExitCode && !restarted) return run(target, true) process.exit(typeof code === "number" ? code : 0) }) } diff --git a/packages/cli/src/commands/handlers/default.ts b/packages/cli/src/commands/handlers/default.ts index d7ba88a8247c..e9bcb7360c6f 100644 --- a/packages/cli/src/commands/handlers/default.ts +++ b/packages/cli/src/commands/handlers/default.ts @@ -8,6 +8,8 @@ import { ServiceConfig } from "../../services/service-config" import { Standalone } from "../../services/standalone" import { Updater } from "../../services/updater" +const restartExitCode = 75 + export default Runtime.handler(Commands, (input) => Effect.gen(function* () { const directory = Option.getOrUndefined(input.directory) @@ -22,9 +24,7 @@ export default Runtime.handler(Commands, (input) => const password = yield* Env.password const explicit = { url: server, - headers: password - ? { authorization: "Basic " + btoa("opencode:" + Redacted.value(password)) } - : undefined, + headers: password ? { authorization: "Basic " + btoa("opencode:" + Redacted.value(password)) } : undefined, } satisfies Service.Transport // Fail loudly before entering the TUI: an explicit server that is // unreachable or rejects auth should not present as reconnect churn. @@ -60,7 +60,7 @@ export default Runtime.handler(Commands, (input) => Effect.gen(function* () { const found = yield* Service.discover(serviceOptions) return found ?? (yield* Service.start(serviceOptions)) - }).pipe(Effect.provide(NodeFileSystem.layer)), + }).pipe(Effect.catchIf(versionMismatch, restart), Effect.provide(NodeFileSystem.layer)), ) : () => Promise.resolve(transport) // Restart the managed service in place; start() resolves once the @@ -76,11 +76,36 @@ export default Runtime.handler(Commands, (input) => }).pipe(Effect.provide(NodeFileSystem.layer)), ) : undefined - yield* runTui( + return yield* runTui( transport, { continue: input.continue, sessionID: Option.getOrUndefined(input.session) }, discover, reload, + () => { + process.exitCode = restartExitCode + }, ) - }), + }).pipe( + Effect.catchIf(versionMismatch, (error) => + restart(error).pipe( + Effect.tap(() => + Effect.sync(() => { + process.exitCode = restartExitCode + }), + ), + Effect.asVoid, + ), + ), + ), ) + +function restart(error: Service.VersionMismatchError) { + return Effect.logInfo("restarting stale client", { + clientVersion: error.clientVersion, + serverVersion: error.serverVersion, + }).pipe(Effect.as({ restart: true as const })) +} + +function versionMismatch(error: unknown): error is Service.VersionMismatchError { + return error instanceof Service.VersionMismatchError +} diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 64116b9b7637..d043de5629ec 100755 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -56,6 +56,6 @@ Effect.logInfo("cli starting", { Effect.provide(LoggingLayer), Effect.provide(NodeServices.layer), Effect.scoped, - Effect.tap(() => Effect.sync(() => process.exit(0))), + Effect.tap(() => Effect.sync(() => process.exit(process.exitCode ?? 0))), NodeRuntime.runMain, ) diff --git a/packages/cli/src/services/service-config.ts b/packages/cli/src/services/service-config.ts index ecdd5bf19a0c..c22aea48c354 100644 --- a/packages/cli/src/services/service-config.ts +++ b/packages/cli/src/services/service-config.ts @@ -4,6 +4,7 @@ import { Service } from "@opencode-ai/client/effect" import { Effect, FileSystem, Schema } from "effect" import { randomBytes } from "crypto" import path from "path" +import semver from "semver" // The CLI's service configuration file, plus the Service.Options binding that // points the client package's service operations at this CLI: which @@ -45,10 +46,21 @@ export const options = Effect.fnUntraced(function* () { return { file, version: InstallationVersion, + canReplace: (version: string | undefined) => canReplaceVersion(version, InstallationVersion), command: [process.execPath, ...(entrypoint ? [entrypoint] : []), "serve", "--service"], } }) +export function canReplaceVersion(serverVersion: string | undefined, clientVersion: string) { + if (serverVersion === undefined) return true + // Preview versions end in `-[.]`. Convert the build + // to a numeric semver identifier so next-15000 sorts after next-9999. + const server = serverVersion.replace(/-(\d+)(?=(?:\.\d+)?$)/, ".$1") + const client = clientVersion.replace(/-(\d+)(?=(?:\.\d+)?$)/, ".$1") + if (!semver.valid(server) || !semver.valid(client)) return true + return semver.lt(server, client) +} + export const read = Effect.fn("cli.service-config.read")(function* () { const { fs, configFile } = yield* env return yield* fs.readFileString(configFile).pipe( diff --git a/packages/cli/src/tui.ts b/packages/cli/src/tui.ts index 96d92c50b266..a8644d5ccf2d 100644 --- a/packages/cli/src/tui.ts +++ b/packages/cli/src/tui.ts @@ -12,8 +12,9 @@ import type { Args } from "@opencode-ai/tui/context/args" export function runTui( transport: Service.Transport, args: Args, - discover?: () => Promise, + discover?: () => Promise, reload?: () => Promise, + restart?: () => void, ) { const config = TuiConfig.resolve({}, { terminalSuspend: false }) let disposeSlots: (() => void) | undefined @@ -32,12 +33,14 @@ export function runTui( discover: discover ? async () => { const next = await discover() + if ("restart" in next) return next return { client: createOpencodeClient({ baseUrl: next.url, headers: next.headers, directory }), api: OpenCode.make({ baseUrl: next.url, headers: next.headers }), } } : undefined, + restart, reload, args, config, diff --git a/packages/cli/test/fixture/restart-child.ts b/packages/cli/test/fixture/restart-child.ts new file mode 100644 index 000000000000..fca9dd11bf03 --- /dev/null +++ b/packages/cli/test/fixture/restart-child.ts @@ -0,0 +1,6 @@ +const stateFile = process.argv[2] +const exit = Number(process.argv[3]) +const count = (await Bun.file(stateFile).exists()) ? Number(await Bun.file(stateFile).text()) : 0 + +await Bun.write(stateFile, String(count + 1)) +process.exit(exit || (count === 0 ? 75 : 0)) diff --git a/packages/cli/test/launcher.test.ts b/packages/cli/test/launcher.test.ts new file mode 100644 index 000000000000..b2dddb8730f8 --- /dev/null +++ b/packages/cli/test/launcher.test.ts @@ -0,0 +1,43 @@ +import { expect, test } from "bun:test" +import fs from "node:fs/promises" +import os from "node:os" +import path from "node:path" + +const launcher = path.join(import.meta.dir, "../bin/opencode2.cjs") +const fixture = path.join(import.meta.dir, "fixture/restart-child.ts") + +test("restarts the installed binary once with the original arguments", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "opencode-launcher-")) + const state = path.join(root, "count") + + try { + const child = Bun.spawn([process.execPath, launcher, fixture, state, "0"], { + env: { ...process.env, OPENCODE_BIN_PATH: process.execPath }, + stdout: "pipe", + stderr: "pipe", + }) + + expect(await child.exited).toBe(0) + expect(await Bun.file(state).text()).toBe("2") + } finally { + await fs.rm(root, { recursive: true, force: true }) + } +}) + +test("bounds automatic restarts", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "opencode-launcher-")) + const state = path.join(root, "count") + + try { + const child = Bun.spawn([process.execPath, launcher, fixture, state, "75"], { + env: { ...process.env, OPENCODE_BIN_PATH: process.execPath }, + stdout: "pipe", + stderr: "pipe", + }) + + expect(await child.exited).toBe(75) + expect(await Bun.file(state).text()).toBe("2") + } 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 68073851c1b9..ff359f3d18e1 100644 --- a/packages/cli/test/service.test.ts +++ b/packages/cli/test/service.test.ts @@ -7,6 +7,18 @@ import os from "node:os" import path from "node:path" import { ServiceConfig } from "../src/services/service-config" +test("only replaces older semantic versions", () => { + expect(ServiceConfig.canReplaceVersion("1.0.0", "2.0.0")).toBe(true) + expect(ServiceConfig.canReplaceVersion("2.0.0", "1.0.0")).toBe(false) + expect(ServiceConfig.canReplaceVersion("1.0.0", "1.0.0")).toBe(false) + expect(ServiceConfig.canReplaceVersion("0.0.0-next-9999", "0.0.0-next-15000")).toBe(true) + expect(ServiceConfig.canReplaceVersion("0.0.0-next-15000", "0.0.0-next-9999")).toBe(false) + expect(ServiceConfig.canReplaceVersion("0.0.0-next-15000.1", "0.0.0-next-15000.2")).toBe(true) + expect(ServiceConfig.canReplaceVersion("0.0.0-next-15000.10", "0.0.0-next-15000.9")).toBe(false) + expect(ServiceConfig.canReplaceVersion("0.0.0-next-15000.10", "0.0.0-next-15000.10")).toBe(false) + expect(ServiceConfig.canReplaceVersion(undefined, "1.0.0")).toBe(true) +}) + test("local channel stores service config with the local service filename", async () => { const root = await fs.mkdtemp(path.join(os.tmpdir(), "opencode-service-")) try { diff --git a/packages/client/src/effect/service.ts b/packages/client/src/effect/service.ts index ca428f5aa62a..556953eb347c 100644 --- a/packages/client/src/effect/service.ts +++ b/packages/client/src/effect/service.ts @@ -1,4 +1,4 @@ -import { Effect, FileSystem, Option, Schedule, Schema } from "effect" +import { Data, Effect, FileSystem, Option, Schedule, Schema } from "effect" import { spawn } from "node:child_process" import { homedir } from "node:os" import { join } from "node:path" @@ -23,6 +23,9 @@ export type Options = { // When set, discovery only returns a server reporting this exact version, // and start() replaces a healthy server whose version differs. readonly version?: string + // Decides whether start() may terminate a healthy version-mismatched server. + // Defaults to true for callers that do not need directional version handling. + readonly canReplace?: (version: string | undefined) => boolean // Argv used to spawn the service. Defaults to ["opencode", "serve", // "--service"] resolved from PATH. readonly command?: ReadonlyArray @@ -44,7 +47,11 @@ export const start = Effect.fn("service.start")(function* (options: Options = {} const compatible = yield* discover(options) if (compatible !== undefined) return compatible const mismatched = yield* find(options) - if (mismatched !== undefined) yield* kill(mismatched.info, options).pipe(Effect.ignore) + if (mismatched !== undefined) { + const error = replacementError(mismatched.info, options) + if (error) return yield* Effect.fail(error) + yield* kill(mismatched.info, options).pipe(Effect.ignore) + } const [command, ...args] = options.command ?? ["opencode", "serve", "--service"] if (command === undefined) return yield* Effect.fail(new Error("Missing service command")) @@ -67,8 +74,12 @@ export const start = Effect.fn("service.start")(function* (options: Options = {} export const stop = Effect.fn("service.stop")(function* (options: Options = {}) { const fs = yield* FileSystem.FileSystem const existing = yield* find(options) - if (existing !== undefined) yield* kill(existing.info, options) - yield* fs.remove(options.file ?? fallback()).pipe(Effect.ignore) + if (existing !== undefined) { + const error = replacementError(existing.info, options) + if (error) return yield* Effect.fail(error) + yield* kill(existing.info, options) + } + return yield* fs.remove(options.file ?? fallback()).pipe(Effect.ignore) }) function fallback() { @@ -89,6 +100,22 @@ export const Info = Schema.Struct({ }) export type Info = typeof Info.Type +export class VersionMismatchError extends Data.TaggedError("ServiceVersionMismatchError")<{ + readonly clientVersion: string | undefined + readonly serverVersion: string | undefined + readonly message: string +}> {} + +function replacementError(info: Info, options: Options): VersionMismatchError | undefined { + if (options.version === undefined || info.version === options.version || options.canReplace?.(info.version) !== false) + return undefined + return new VersionMismatchError({ + clientVersion: options.version, + serverVersion: info.version, + message: `Client version ${options.version} cannot replace server version ${info.version ?? "unknown"}`, + }) +} + const decode = Schema.decodeUnknownEffect(Schema.fromJsonString(Info)) // A missing or corrupt file means no valid info; callers treat both diff --git a/packages/client/test/service.test.ts b/packages/client/test/service.test.ts new file mode 100644 index 000000000000..50314a2b33ba --- /dev/null +++ b/packages/client/test/service.test.ts @@ -0,0 +1,49 @@ +import { NodeFileSystem } from "@effect/platform-node" +import { afterEach, expect, test } from "bun:test" +import { Effect } from "effect" +import fs from "node:fs/promises" +import os from "node:os" +import path from "node:path" +import { Service } from "../src/effect/index" + +const cleanup: Array<() => void | Promise> = [] + +afterEach(async () => { + await Promise.all(cleanup.splice(0).map(async (run) => await run())) +}) + +test("does not start or stop a healthy service rejected by the version policy", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "opencode-client-service-")) + const child = Bun.spawn([process.execPath, "-e", "await new Promise(() => {})"]) + const server = Bun.serve({ port: 0, fetch: () => Response.json({ healthy: true }) }) + const file = path.join(root, "service.json") + cleanup.push( + () => child.kill(), + () => server.stop(true), + () => fs.rm(root, { recursive: true, force: true }), + ) + await Bun.write(file, JSON.stringify({ id: "newer", version: "2.0.0", url: server.url.toString(), pid: child.pid })) + + const options = { + file, + version: "1.0.0", + canReplace: () => false, + command: [process.execPath, "-e", "throw new Error('should not spawn')"], + } + const startError = await Service.start(options).pipe( + Effect.flip, + Effect.provide(NodeFileSystem.layer), + Effect.runPromise, + ) + const stopError = await Service.stop(options).pipe( + Effect.flip, + Effect.provide(NodeFileSystem.layer), + Effect.runPromise, + ) + + expect(startError).toBeInstanceOf(Service.VersionMismatchError) + expect(startError).toMatchObject({ clientVersion: "1.0.0", serverVersion: "2.0.0" }) + expect(stopError).toBeInstanceOf(Service.VersionMismatchError) + expect(process.kill(child.pid, 0)).toBe(true) + expect(await Bun.file(file).exists()).toBe(true) +}) diff --git a/packages/tui/src/app.tsx b/packages/tui/src/app.tsx index a3b2c3197375..345d2617ab80 100644 --- a/packages/tui/src/app.tsx +++ b/packages/tui/src/app.tsx @@ -30,7 +30,7 @@ import { PluginRouteMissing } from "./component/plugin-route-missing" import { ProjectProvider, useProject } from "./context/project" import { EditorContextProvider } from "./context/editor" import { useEvent } from "./context/event" -import { SDKProvider, useSDK } from "./context/sdk" +import { SDKProvider, useSDK, type SDKDiscovery } from "./context/sdk" import { StartupLoading } from "./component/startup-loading" import { Reconnecting } from "./component/reconnecting" import { SyncProvider, useSync } from "./context/sync" @@ -138,7 +138,8 @@ const appBindingCommands = [ export type TuiInput = { client: OpencodeClient api: OpenCodeClient - discover?: () => Promise<{ client: OpencodeClient; api: OpenCodeClient }> + discover?: () => Promise + restart?: () => void reload?: () => Promise args: Args config: TuiConfig.Resolved @@ -302,7 +303,16 @@ export const run = Effect.fn("Tui.run")(function* (input: TuiInput) { > - + { + input.restart?.() + destroyRenderer(renderer) + }} + reload={input.reload} + > @@ -812,8 +822,7 @@ function App(props: { onSnapshot?: () => Promise; pluginHost: TuiPlugi toast.show({ variant: "info", message: "Reloading server...", duration: 30000 }) // reload resolves once the replacement service is healthy; the // event stream reattaches through the reconnect loop. - await sdk - .reload!() + await sdk.reload!() .then(() => toast.show({ variant: "success", message: "Server reloaded" })) .catch(toast.error) }, diff --git a/packages/tui/src/context/sdk.tsx b/packages/tui/src/context/sdk.tsx index 0ad516ceaccf..925a5a37f39d 100644 --- a/packages/tui/src/context/sdk.tsx +++ b/packages/tui/src/context/sdk.tsx @@ -10,12 +10,15 @@ export type SDKConnectionStatus = "connected" | "connecting" type SDKEventMap = { [Type in V2Event["type"]]: Extract } const connectTimeout = 2_000 +export type SDKDiscovery = { client: OpencodeClient; api: OpenCodeClient } | { restart: true } + export const { use: useSDK, provider: SDKProvider } = createSimpleContext({ name: "SDK", init: (props: { client: OpencodeClient api: OpenCodeClient - discover?: () => Promise<{ client: OpencodeClient; api: OpenCodeClient }> + discover?: () => Promise + restart?: () => void // Stops and starts the managed service; present only in service mode. reload?: () => Promise }) => { @@ -66,7 +69,8 @@ export const { use: useSDK, provider: SDKProvider } = createSimpleContext({ return connection.signal.reason instanceof Error ? connection.signal.reason : new Error("Event stream disconnected") - if (first.value.type !== "server.connected") return new Error("Event stream did not start with server.connected") + if (first.value.type !== "server.connected") + return new Error("Event stream did not start with server.connected") clearTimeout(timeout) attempt = 0 events.emit(first.value.type, first.value) @@ -93,6 +97,10 @@ export const { use: useSDK, provider: SDKProvider } = createSimpleContext({ const next = await props.discover().catch(() => undefined) if (abort.signal.aborted || controller.signal.aborted) return if (next) { + if ("restart" in next) { + props.restart?.() + return + } client = next.client api = next.api } diff --git a/packages/tui/test/cli/tui/use-event.test.tsx b/packages/tui/test/cli/tui/use-event.test.tsx index 7a4465c92791..b2511662f116 100644 --- a/packages/tui/test/cli/tui/use-event.test.tsx +++ b/packages/tui/test/cli/tui/use-event.test.tsx @@ -1,11 +1,10 @@ /** @jsxImportSource @opentui/solid */ import { describe, expect, test } from "bun:test" -import type { OpenCodeClient } from "@opencode-ai/client/promise" import { testRender } from "@opentui/solid" -import type { OpencodeClient, V2Event } from "@opencode-ai/sdk/v2" +import type { V2Event } from "@opencode-ai/sdk/v2" import { onMount } from "solid-js" import { ProjectProvider, useProject } from "../../../src/context/project" -import { SDKProvider, useSDK } from "../../../src/context/sdk" +import { SDKProvider, useSDK, type SDKDiscovery } from "../../../src/context/sdk" import { useEvent } from "../../../src/context/event" import { createApi, createClient, createEventStream, createFetch } from "../../fixture/tui-sdk" import { TestTuiContexts } from "../../fixture/tui-environment" @@ -49,7 +48,7 @@ function update(version: string): V2Event { } } -async function mount(discover?: () => Promise<{ client: OpencodeClient; api: OpenCodeClient }>) { +async function mount(discover?: () => Promise, restart?: () => void) { const events = createEventStream() const calls = createFetch(undefined, events) const seen: V2Event[] = [] @@ -63,7 +62,12 @@ async function mount(discover?: () => Promise<{ client: OpencodeClient; api: Ope const app = await testRender(() => ( - + { @@ -198,4 +202,25 @@ describe("useEvent", () => { app.renderer.destroy() } }) + + test("requests a restart when discovery finds a newer service", async () => { + let restarts = 0 + const { app, events, sdk } = await mount( + async () => ({ restart: true }), + () => { + restarts += 1 + }, + ) + + try { + await wait(() => sdk.connection.status() === "connected") + events.disconnect() + await wait(() => restarts === 1) + await Bun.sleep(300) + + expect(restarts).toBe(1) + } finally { + app.renderer.destroy() + } + }) })