fix(cli): prevent stale service replacement
This commit is contained in:
@@ -5,6 +5,7 @@ import { Service } from "@opencode-ai/client/effect/service"
|
||||
import { Effect, FileSystem, Option, Schema } from "effect"
|
||||
import { randomBytes } from "crypto"
|
||||
import path from "path"
|
||||
import semver from "semver"
|
||||
import { selfCommand } from "../util/process"
|
||||
|
||||
// The CLI's service configuration file, plus the Service.EnsureOptions binding that
|
||||
@@ -104,10 +105,21 @@ export const options = Effect.fnUntraced(function* () {
|
||||
return {
|
||||
file,
|
||||
version: OPENCODE_VERSION,
|
||||
canReplace: (version: string | undefined) => canReplaceVersion(version),
|
||||
command: [...selfCommand(), "serve", "--service"],
|
||||
}
|
||||
})
|
||||
|
||||
export function canReplaceVersion(serverVersion: string | undefined, clientVersion = OPENCODE_VERSION) {
|
||||
if (serverVersion === undefined) return true
|
||||
// Preview versions end in `<channel>-<build>[.<attempt>]`. 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, legacyConfigFile } = yield* paths
|
||||
if (legacyConfigFile) yield* migrateConfig(legacyConfigFile, configFile)
|
||||
|
||||
@@ -47,6 +47,15 @@ test("service filenames share release channels and identify preview channels", (
|
||||
expect(ServiceConfig.versionBelongsToChannel("1.2.3", "preview-a")).toBe(false)
|
||||
})
|
||||
|
||||
test("only newer clients replace managed service versions", () => {
|
||||
expect(ServiceConfig.canReplaceVersion("1.2.3", "1.2.4")).toBe(true)
|
||||
expect(ServiceConfig.canReplaceVersion("1.2.4", "1.2.3")).toBe(false)
|
||||
expect(ServiceConfig.canReplaceVersion("1.2.3", "1.2.3")).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(undefined, "1.2.3")).toBe(true)
|
||||
})
|
||||
|
||||
test("service config migrates from the hashed channel filename", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "opencode-service-config-migration-"))
|
||||
const legacy = path.join(root, ServiceConfig.legacyFilename("preview-a")!)
|
||||
|
||||
@@ -3,7 +3,13 @@ import { Effect, FileSystem, Option, Schedule, Schema } from "effect"
|
||||
import { spawn, type ChildProcess } from "node:child_process"
|
||||
import { homedir } from "node:os"
|
||||
import { join } from "node:path"
|
||||
import type { DiscoverOptions, Endpoint, EnsureOptions, StopOptions } from "../service.js"
|
||||
import {
|
||||
VersionMismatchError,
|
||||
type DiscoverOptions,
|
||||
type Endpoint,
|
||||
type EnsureOptions,
|
||||
type StopOptions,
|
||||
} from "../service.js"
|
||||
|
||||
export * from "../service.js"
|
||||
/** Contents of the local service registration file. */
|
||||
@@ -89,6 +95,8 @@ export const ensure = Effect.fn("service.ensure")(function* (options: EnsureOpti
|
||||
if (compatible && service.state === "failed")
|
||||
return yield* Effect.fail(new Error("Background service failed to start"))
|
||||
if (compatible) return Option.none<LocalService>()
|
||||
if (options.canReplace?.(service.version) === false)
|
||||
return yield* Effect.fail(new VersionMismatchError(options.version, service.version))
|
||||
yield* announce("version-mismatch", service.version)
|
||||
yield* kill(service, options).pipe(Effect.ignore)
|
||||
lastSpawn = 0
|
||||
|
||||
@@ -2,7 +2,14 @@ import { readFile } from "node:fs/promises"
|
||||
import { spawn, type ChildProcess } from "node:child_process"
|
||||
import { homedir } from "node:os"
|
||||
import { join } from "node:path"
|
||||
import type { DiscoverOptions, Endpoint, Info, EnsureOptions, StopOptions } from "../service.js"
|
||||
import {
|
||||
VersionMismatchError,
|
||||
type DiscoverOptions,
|
||||
type Endpoint,
|
||||
type Info,
|
||||
type EnsureOptions,
|
||||
type StopOptions,
|
||||
} from "../service.js"
|
||||
import type { ServiceHealth, ServiceStopResponse } from "./generated/types.js"
|
||||
|
||||
export * from "../service.js"
|
||||
@@ -70,6 +77,8 @@ export async function ensure(options: EnsureOptions = {}): Promise<Endpoint> {
|
||||
if (compatible && service.state === "ready") return service.endpoint
|
||||
if (compatible && service.state === "failed") throw new Error("Background service failed to start")
|
||||
if (!compatible) {
|
||||
if (options.canReplace?.(service.version) === false)
|
||||
throw new VersionMismatchError(options.version, service.version)
|
||||
announce("version-mismatch", service.version)
|
||||
await kill(service, options).catch(() => undefined)
|
||||
lastSpawn = 0
|
||||
|
||||
@@ -28,10 +28,24 @@ export type EnsureReason = "missing" | "version-mismatch"
|
||||
export type EnsureOptions = DiscoverOptions & {
|
||||
/** Service command and arguments. Defaults to `opencode serve --service`. */
|
||||
readonly command?: ReadonlyArray<string>
|
||||
/** Decide whether a version-mismatched service may be replaced. Defaults to true. */
|
||||
readonly canReplace?: (version: string | undefined) => boolean
|
||||
/** Called once before spawning a new service process. */
|
||||
readonly onStart?: (reason: EnsureReason, previousVersion?: string) => void
|
||||
}
|
||||
|
||||
/** A healthy service exists, but the caller's replacement policy protects it. */
|
||||
export class VersionMismatchError extends Error {
|
||||
override readonly name = "VersionMismatchError"
|
||||
|
||||
constructor(
|
||||
readonly clientVersion: string | undefined,
|
||||
readonly serverVersion: string | undefined,
|
||||
) {
|
||||
super(`Client version ${clientVersion ?? "unknown"} cannot replace server version ${serverVersion ?? "unknown"}`)
|
||||
}
|
||||
}
|
||||
|
||||
/** Options used to stop the local OpenCode service. */
|
||||
export type StopOptions = {
|
||||
/** Absolute registration file path. Defaults to the XDG state directory. */
|
||||
|
||||
@@ -70,6 +70,25 @@ test("reports a failed registered service", async () => {
|
||||
)
|
||||
})
|
||||
|
||||
test("does not replace a version rejected by the caller", async () => {
|
||||
const registration = await setup("graceful")
|
||||
const directory = await temp()
|
||||
const contender = join(directory, "contender.json")
|
||||
const info = await Bun.file(registration).json()
|
||||
|
||||
await expect(
|
||||
Service.ensure({
|
||||
file: registration,
|
||||
version: "old",
|
||||
canReplace: () => false,
|
||||
command: [process.execPath, fixture, contender, "record-start"],
|
||||
}),
|
||||
).rejects.toThrow("Client version old cannot replace server version test")
|
||||
|
||||
expect(await Bun.file(contender + ".started").exists()).toBe(false)
|
||||
expect(process.kill(info.pid, 0)).toBe(true)
|
||||
})
|
||||
|
||||
test("requests graceful stop of the exact service instance", async () => {
|
||||
const registration = await setup("graceful")
|
||||
const info = await Bun.file(registration).json()
|
||||
|
||||
@@ -107,6 +107,28 @@ test("does not spawn contenders while an incompatible service rejects replacemen
|
||||
expect(existing.exitCode).toBe(null)
|
||||
})
|
||||
|
||||
test("does not replace a version rejected by the caller", async () => {
|
||||
const directory = await temp()
|
||||
const registration = join(directory, "service.json")
|
||||
const contender = join(directory, "contender.json")
|
||||
const existing = spawn(registration, "graceful")
|
||||
await waitForFile(registration)
|
||||
|
||||
await expect(
|
||||
run(
|
||||
Service.ensure({
|
||||
file: registration,
|
||||
version: "old",
|
||||
canReplace: () => false,
|
||||
command: [process.execPath, fixture, contender, "record-start"],
|
||||
}),
|
||||
),
|
||||
).rejects.toThrow("Client version old cannot replace server version test")
|
||||
|
||||
expect(await Bun.file(contender + ".started").exists()).toBe(false)
|
||||
expect(existing.exitCode).toBe(null)
|
||||
})
|
||||
|
||||
test("a legacy health response is still replaced", async () => {
|
||||
const directory = await temp()
|
||||
const registration = join(directory, "service.json")
|
||||
|
||||
Reference in New Issue
Block a user