From fb8defdc30cdcca5c6ea18807be61bbe1fc808f6 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Wed, 12 Aug 2026 19:58:21 -0400 Subject: [PATCH] fix(client): prevent stale service replacement --- bun.lock | 4 ++++ packages/cli/src/services/service-config.ts | 4 +++- packages/cli/test/server-connection.test.ts | 6 ++++- packages/client/package.json | 4 +++- packages/client/src/effect/service.ts | 4 ++-- packages/client/src/promise/service.ts | 4 ++-- packages/client/src/service-version.ts | 10 ++++++++ packages/client/src/service.ts | 2 ++ packages/client/test/fixture/service.ts | 1 + packages/client/test/service-version.test.ts | 13 +++++++++++ packages/client/test/service.test.ts | 24 ++++++++++++++++++++ packages/desktop/src/main/background-cli.ts | 2 +- 12 files changed, 70 insertions(+), 8 deletions(-) create mode 100644 packages/client/test/service-version.test.ts diff --git a/bun.lock b/bun.lock index 2380111d1d1..5015f1614ae 100644 --- a/bun.lock +++ b/bun.lock @@ -189,12 +189,14 @@ "dependencies": { "@opencode-ai/protocol": "workspace:*", "@opencode-ai/schema": "workspace:*", + "semver": "catalog:", }, "devDependencies": { "@effect/platform-node": "catalog:", "@opencode-ai/httpapi-codegen": "workspace:*", "@tsconfig/bun": "catalog:", "@types/bun": "catalog:", + "@types/semver": "catalog:", "@typescript/native-preview": "catalog:", "effect": "catalog:", }, @@ -6467,6 +6469,8 @@ "@opencode-ai/cli/semver": ["semver@7.7.4", "", { "bin": { "semver": "bin/semver.js" } }, "sha512-vFKC2IEtQnVhpT78h1Yp8wzwrf8CM+MzKMHGJZfBtzhZNycRFnXsHk6E5TxIkkMsgNS7mdX3AGB7x2QM2di4lA=="], + "@opencode-ai/client/semver": ["semver@7.7.4", "", { "bin": { "semver": "bin/semver.js" } }, "sha512-vFKC2IEtQnVhpT78h1Yp8wzwrf8CM+MzKMHGJZfBtzhZNycRFnXsHk6E5TxIkkMsgNS7mdX3AGB7x2QM2di4lA=="], + "@opencode-ai/console-app/@smithy/eventstream-codec": ["@smithy/eventstream-codec@4.2.7", "", { "dependencies": { "@aws-crypto/crc32": "5.2.0", "@smithy/types": "^4.11.0", "@smithy/util-hex-encoding": "^4.2.0", "tslib": "^2.6.2" } }, "sha512-DrpkEoM3j9cBBWhufqBwnbbn+3nf1N9FP6xuVJ+e220jbactKuQgaZwjwP5CP1t+O94brm2JgVMD2atMGX3xIQ=="], "@opencode-ai/console-app/@smithy/util-utf8": ["@smithy/util-utf8@4.2.0", "", { "dependencies": { "@smithy/util-buffer-from": "^4.2.0", "tslib": "^2.6.2" } }, "sha512-zBPfuzoI8xyBtR2P6WQj63Rz8i3AmfAaJLuNG8dWsfvPe8lO4aCPYLn879mEgHndZH1zQ2oXmG8O1GGzzaoZiw=="], diff --git a/packages/cli/src/services/service-config.ts b/packages/cli/src/services/service-config.ts index d348f390c2d..3ff3c4de606 100644 --- a/packages/cli/src/services/service-config.ts +++ b/packages/cli/src/services/service-config.ts @@ -103,7 +103,9 @@ export const options = Effect.fnUntraced(function* (input: { readonly checkVersi yield* Effect.forEach(legacyRegistrationFiles, (legacy) => migrateRegistration(legacy, file)) return { file, - version: input.checkVersion ? OPENCODE_VERSION : undefined, + version: input.checkVersion + ? (version: string) => Service.isServiceVersionCompatible(version, OPENCODE_VERSION) + : undefined, command: [...selfCommand(), "serve", "--service"], } }) diff --git a/packages/cli/test/server-connection.test.ts b/packages/cli/test/server-connection.test.ts index f5ec0dc6e8f..ee3c809ff08 100644 --- a/packages/cli/test/server-connection.test.ts +++ b/packages/cli/test/server-connection.test.ts @@ -64,7 +64,11 @@ test("service options only require a matching version when requested", async () try { expect((await runPromise(ServiceConfig.options())).version).toBeUndefined() - expect((await runPromise(ServiceConfig.options({ checkVersion: true }))).version).toBe(OPENCODE_VERSION) + const version = (await runPromise(ServiceConfig.options({ checkVersion: true }))).version + expect(version).toBeFunction() + if (typeof version !== "function") throw new Error("Expected a service version predicate") + expect(version(OPENCODE_VERSION)).toBe(true) + expect(version("999.0.0")).toBe(true) } finally { await fs.rm(root, { recursive: true, force: true }) } diff --git a/packages/client/package.json b/packages/client/package.json index fe379b54577..fba183ed6dd 100644 --- a/packages/client/package.json +++ b/packages/client/package.json @@ -33,7 +33,8 @@ }, "dependencies": { "@opencode-ai/schema": "workspace:*", - "@opencode-ai/protocol": "workspace:*" + "@opencode-ai/protocol": "workspace:*", + "semver": "catalog:" }, "peerDependencies": { "effect": "4.0.0-beta.101" @@ -48,6 +49,7 @@ "@opencode-ai/httpapi-codegen": "workspace:*", "@tsconfig/bun": "catalog:", "@types/bun": "catalog:", + "@types/semver": "catalog:", "@typescript/native-preview": "catalog:", "effect": "catalog:" } diff --git a/packages/client/src/effect/service.ts b/packages/client/src/effect/service.ts index 733bbf8a7a6..0b83c206b82 100644 --- a/packages/client/src/effect/service.ts +++ b/packages/client/src/effect/service.ts @@ -10,7 +10,7 @@ import { spawnServiceContender, } from "../service-contender.js" import { defaultEnsureTiming, ensureTiming, type EnsureTiming } from "../service-timing.js" -import { matchesVersion } from "../service-version.js" +import { isServiceVersionCompatible, matchesVersion } from "../service-version.js" export * from "../service.js" /** Contents of the local service registration file. */ @@ -325,4 +325,4 @@ const requestStop = Effect.fnUntraced(function* (service: LocalService, timeout }) /** Effect-based local service lifecycle operations. */ -export const Service = { discover, incumbent, ensure, stop, headers, Info } +export const Service = { discover, incumbent, ensure, stop, headers, isServiceVersionCompatible, Info } diff --git a/packages/client/src/promise/service.ts b/packages/client/src/promise/service.ts index d54da6c4e90..b51aadd014e 100644 --- a/packages/client/src/promise/service.ts +++ b/packages/client/src/promise/service.ts @@ -9,7 +9,7 @@ import { spawnServiceContender, } from "../service-contender.js" import { defaultEnsureTiming, ensureTiming, type EnsureTiming } from "../service-timing.js" -import { matchesVersion } from "../service-version.js" +import { isServiceVersionCompatible, matchesVersion } from "../service-version.js" import type { ServiceHealth, ServiceStopResponse } from "./generated/types.js" export * from "../service.js" @@ -278,4 +278,4 @@ function delay(milliseconds: number) { } /** Promise-based local service lifecycle operations. */ -export const Service = { discover, ensure, stop, headers } +export const Service = { discover, ensure, stop, headers, isServiceVersionCompatible } diff --git a/packages/client/src/service-version.ts b/packages/client/src/service-version.ts index 5586f111a9c..9fc53c82967 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,12 @@ export function matchesVersion(version: string | undefined, options: DiscoverOpt if (typeof options.version === "function") return options.version(version) return version === options.version } + +/** Whether a service is at least as new as its client. */ +export function isServiceVersionCompatible(serverVersion: string, clientVersion: string) { + if (serverVersion === clientVersion) return true + 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 true + return semver.gte(server, client) +} diff --git a/packages/client/src/service.ts b/packages/client/src/service.ts index 65c32266430..e222924b4f0 100644 --- a/packages/client/src/service.ts +++ b/packages/client/src/service.ts @@ -1,3 +1,5 @@ +export { isServiceVersionCompatible } from "./service-version.js" + /** Connection details for a local OpenCode service. */ export type Endpoint = { /** Base URL of the service. */ diff --git a/packages/client/test/fixture/service.ts b/packages/client/test/fixture/service.ts index 4faf13e6931..a094090bcc2 100644 --- a/packages/client/test/fixture/service.ts +++ b/packages/client/test/fixture/service.ts @@ -31,6 +31,7 @@ let version = "test" if (mode === "old" || mode === "reject-stop") version = "old" if (mode === "incompatible") version = "1.9.0" if (mode === "compatible" || mode === "delayed-compatible") version = "2.1.0-next.1" +if (mode === "newer") version = "0.0.0-next-17272" const id = crypto.randomUUID() const server = Bun.serve({ port: 0, diff --git a/packages/client/test/service-version.test.ts b/packages/client/test/service-version.test.ts new file mode 100644 index 00000000000..24043cadc04 --- /dev/null +++ b/packages/client/test/service-version.test.ts @@ -0,0 +1,13 @@ +import { expect, test } from "bun:test" +import { isServiceVersionCompatible } from "../src/service" + +test("accepts the same or a newer service version", () => { + expect(isServiceVersionCompatible("0.0.0-next-17272", "0.0.0-next-17272")).toBe(true) + expect(isServiceVersionCompatible("0.0.0-next-17272", "0.0.0-next-17271")).toBe(true) + expect(isServiceVersionCompatible("0.0.0-next-17271", "0.0.0-next-17272")).toBe(false) + expect(isServiceVersionCompatible("0.0.0-next-15000", "0.0.0-next-9999")).toBe(true) +}) + +test("accepts incomparable development versions", () => { + expect(isServiceVersionCompatible("development-a", "development-b")).toBe(true) +}) diff --git a/packages/client/test/service.test.ts b/packages/client/test/service.test.ts index 152f6cd15e1..66daa0413d0 100644 --- a/packages/client/test/service.test.ts +++ b/packages/client/test/service.test.ts @@ -68,6 +68,30 @@ test("reuses a compatible registered service", async () => { expect(existing.exitCode).toBe(null) }) +test("a stale client reuses a newer registered service", async () => { + const directory = await temp() + const registration = join(directory, "service.json") + const existing = spawn(registration, "newer") + await waitForFile(registration) + const original = await Bun.file(registration).json() + + const starts: EnsureReason[] = [] + const endpoint = await run( + ensure({ + file: registration, + version: (version) => Service.isServiceVersionCompatible(version, "0.0.0-next-17271"), + command: [process.execPath, fixture, registration, "record-start"], + onStart: (reason) => starts.push(reason), + }), + ) + + expect(endpoint.url).toBe(original.url) + expect(starts).toEqual([]) + expect(existing.exitCode).toBe(null) + expect(await Bun.file(registration).json()).toEqual(original) + expect(await Bun.file(registration + ".started").exists()).toBe(false) +}) + test("replaces an incompatible registered service", async () => { const directory = await temp() const registration = join(directory, "service.json") diff --git a/packages/desktop/src/main/background-cli.ts b/packages/desktop/src/main/background-cli.ts index 9e739bf15cf..cd4ff2b5bc4 100644 --- a/packages/desktop/src/main/background-cli.ts +++ b/packages/desktop/src/main/background-cli.ts @@ -29,7 +29,7 @@ export async function startBackgroundCli(logger: Logger) { isolated && process.env.OPENCODE_DESKTOP_SERVER_CHANNEL === "local" ? join(app.getPath("userData"), "opencode", "service-local.json") : undefined, - version, + version: (serverVersion) => Service.isServiceVersionCompatible(serverVersion, version), command: [binary, "serve", "--service"], onStart: (reason, previousVersion) => logger.log("v2 CLI background service starting", { reason, previousVersion }), })