diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index e655802b7b..813af18663 100644 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -50,10 +50,14 @@ export const layer = Layer.effect( Effect.provideService(FileSystem.FileSystem, fs), ) - const get = Effect.fn("cli.config.get")(function* () { - yield* migrate.pipe(Effect.catchCause((cause) => Effect.logWarning("failed to migrate cli config", { cause }))) - return Option.getOrElse(decode(yield* readJson()), () => empty) - }) + const get = Effect.fn("cli.config.get")(() => + lock.withPermits(1)( + Effect.gen(function* () { + yield* migrate.pipe(Effect.catchCause((cause) => Effect.logWarning("failed to migrate cli config", { cause }))) + return Option.getOrElse(decode(yield* readJson()), () => empty) + }), + ), + ) const update = Effect.fn("cli.config.update")((update: (draft: Draft) => void) => lock diff --git a/packages/cli/src/config/migrate.ts b/packages/cli/src/config/migrate.ts index 432a5f25a5..bc3639ab53 100644 --- a/packages/cli/src/config/migrate.ts +++ b/packages/cli/src/config/migrate.ts @@ -6,7 +6,6 @@ import { Definitions } from "@opencode-ai/tui/config/keybind" import { Effect, FileSystem, Option, Schema } from "effect" import { createScanner, - findNodeAtLocation, parse, parseTree, type Node, @@ -82,11 +81,8 @@ export const run = Effect.fn("cli.config.migrate")(function* (input: { function findKeybindProperties(text: string, name: string) { const tree = parseTree(text) if (tree === undefined) return [] - return ( - findNodeAtLocation(tree, ["keybinds"])?.children?.filter( - (property) => property.children?.[0]?.value === name, - ) ?? [] - ) + const keybinds = tree.children?.findLast((property) => property.children?.[0]?.value === "keybinds")?.children?.[1] + return keybinds?.children?.filter((property) => property.children?.[0]?.value === name) ?? [] } function removeProperty(text: string, property: Node) { diff --git a/packages/cli/test/config.test.ts b/packages/cli/test/config.test.ts index 28908869a7..2110f3249f 100644 --- a/packages/cli/test/config.test.ts +++ b/packages/cli/test/config.test.ts @@ -1,6 +1,6 @@ import { NodeFileSystem } from "@effect/platform-node" import { Global } from "@opencode-ai/util/global" -import { Effect } from "effect" +import { Effect, FileSystem } from "effect" import { expect, test } from "bun:test" import { parse } from "jsonc-parser" import path from "path" @@ -232,6 +232,86 @@ test("preserves the effective value when migrating duplicate legacy keybinds", a } }) +test("migrates the effective duplicate top-level keybinds", async () => { + const directory = await Bun.$`mktemp -d`.text().then((value) => value.trim()) + const file = path.join(directory, "cli.json") + await Bun.write(file, `{"keybinds":{"session_delete":"first"},"keybinds":{"session_delete":"last"}}`) + + try { + const config = await run( + directory, + Effect.gen(function* () { + const service = yield* Config.Service + return yield* service.get() + }), + ) + + expect(config.keybinds).toEqual({ "session.delete": "last" }) + expect(parse(await Bun.file(file).text()).keybinds).toEqual({ "session.delete": "last" }) + } finally { + await Bun.$`rm -rf ${directory}` + } +}) + +test("does not overwrite a concurrent config update during migration", async () => { + const directory = await Bun.$`mktemp -d`.text().then((value) => value.trim()) + const file = path.join(directory, "cli.json") + await Bun.write(file, `{"keybinds":{"session_delete":"ctrl+d"}}`) + const node = await Effect.runPromise( + Effect.gen(function* () { + return yield* FileSystem.FileSystem + }).pipe(Effect.provide(NodeFileSystem.layer)), + ) + const started = Promise.withResolvers() + const resume = Promise.withResolvers() + const state = { writes: 0 } + const writeFileString: FileSystem.FileSystem["writeFileString"] = (path, data, options) => { + state.writes++ + if (state.writes !== 1) return node.writeFileString(path, data, options) + started.resolve() + return Effect.gen(function* () { + yield* Effect.promise(() => resume.promise) + yield* node.writeFileString(path, data, options) + }) + } + const fs = new Proxy(node, { + get(target, property, receiver) { + if (property === "writeFileString") return writeFileString + return Reflect.get(target, property, receiver) + }, + }) + + try { + const config = await Effect.runPromise( + Effect.gen(function* () { + const service = yield* Config.Service + return yield* Effect.promise(async () => { + const reading = Effect.runPromise(service.get()) + await started.promise + const updating = Effect.runPromise( + service.update((draft) => { + draft.mouse = false + }), + ) + await Promise.race([updating, Bun.sleep(100)]).finally(() => resume.resolve()) + await Promise.all([reading, updating]) + return Effect.runPromise(service.get()) + }) + }).pipe( + Effect.provide(Config.layer), + Effect.provide(Global.layerWith({ config: directory, state: directory })), + Effect.provideService(FileSystem.FileSystem, fs), + ), + ) + + expect(config).toMatchObject({ keybinds: { "session.delete": "ctrl+d" }, mouse: false }) + expect(await Bun.file(file).json()).toMatchObject({ keybinds: { "session.delete": "ctrl+d" }, mouse: false }) + } finally { + resume.resolve() + await Bun.$`rm -rf ${directory}` + } +}) + test("removes a sole orphaned keybind with a trailing comma", async () => { const directory = await Bun.$`mktemp -d`.text().then((value) => value.trim()) const file = path.join(directory, "cli.json")