diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 813af18663..fd3cba2828 100644 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -21,6 +21,7 @@ export class Service extends Context.Service()("@opencode/cl const decode = Schema.decodeUnknownOption(Info) const decodeRecord = Schema.decodeUnknownOption(Schema.Record(Schema.String, Schema.Any)) const empty: Info = {} +const lock = Semaphore.makeUnsafe(1) export const layer = Layer.effect( Service, @@ -28,7 +29,6 @@ export const layer = Layer.effect( const fs = yield* FileSystem.FileSystem const global = yield* Global.Service const file = path.join(global.config, "cli.json") - const lock = yield* Semaphore.make(1) const readJson = Effect.fnUntraced(function* () { const text = yield* fs.readFileString(file).pipe(Effect.catch(() => Effect.succeed(undefined))) diff --git a/packages/cli/src/config/migrate.ts b/packages/cli/src/config/migrate.ts index bc3639ab53..38ef032eb5 100644 --- a/packages/cli/src/config/migrate.ts +++ b/packages/cli/src/config/migrate.ts @@ -33,6 +33,12 @@ export const run = Effect.fn("cli.config.migrate")(function* (input: { if (config === undefined) return const keybinds = Option.getOrUndefined(decodeRecord(config.keybinds)) if (keybinds === undefined) return + const deduped = findKeybindObjects(text) + .slice(0, -1) + .reduce((text) => { + const property = findKeybindObjects(text)[0] + return property === undefined ? text : removeProperty(text, property) + }, text) const updated = Object.keys(keybinds).reduce((text, name) => { const target = TuiKeybind.CommandMap[name as keyof typeof TuiKeybind.CommandMap] ?? @@ -51,7 +57,7 @@ export const run = Effect.fn("cli.config.migrate")(function* (input: { const key = findKeybindProperties(updated, name)[0]?.children?.[0] if (key === undefined) return text return updated.slice(0, key.offset) + JSON.stringify(target) + updated.slice(key.offset + key.length) - }, text) + }, deduped) if (updated === text) return const temp = input.file + ".tmp" yield* fs.writeFileString(temp, updated, { mode: 0o600 }) @@ -79,10 +85,14 @@ export const run = Effect.fn("cli.config.migrate")(function* (input: { }) function findKeybindProperties(text: string, name: string) { + const keybinds = findKeybindObjects(text).at(-1)?.children?.[1] + return keybinds?.children?.filter((property) => property.children?.[0]?.value === name) ?? [] +} + +function findKeybindObjects(text: string) { const tree = parseTree(text) if (tree === undefined) return [] - const keybinds = tree.children?.findLast((property) => property.children?.[0]?.value === "keybinds")?.children?.[1] - return keybinds?.children?.filter((property) => property.children?.[0]?.value === name) ?? [] + return tree.children?.filter((property) => property.children?.[0]?.value === "keybinds") ?? [] } function removeProperty(text: string, property: Node) { diff --git a/packages/cli/test/config.test.ts b/packages/cli/test/config.test.ts index 2110f3249f..0653828531 100644 --- a/packages/cli/test/config.test.ts +++ b/packages/cli/test/config.test.ts @@ -312,6 +312,90 @@ test("does not overwrite a concurrent config update during migration", async () } }) +test("does not overwrite a concurrent update from another config layer", 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) + }, + }) + const make = () => + Effect.runPromise( + Effect.gen(function* () { + return yield* Config.Service + }).pipe( + Effect.provide(Config.layer), + Effect.provide(Global.layerWith({ config: directory, state: directory })), + Effect.provideService(FileSystem.FileSystem, fs), + ), + ) + + try { + const first = await make() + const second = await make() + const reading = Effect.runPromise(first.get()) + await started.promise + const updating = Effect.runPromise( + second.update((draft) => { + draft.mouse = false + }), + ) + await Promise.race([updating, Bun.sleep(100)]).finally(() => resume.resolve()) + await Promise.all([reading, updating]) + const config = await Effect.runPromise(second.get()) + + 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("updates 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 + yield* service.get() + return yield* service.update((draft) => { + draft.keybinds = { ...draft.keybinds, "session.delete": "changed" } + }) + }), + ) + + expect(config.keybinds).toEqual({ "session.delete": "changed" }) + expect(parse(await Bun.file(file).text()).keybinds).toEqual({ "session.delete": "changed" }) + } finally { + 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")