From 11ee88f28ffb1c7f937902460258ba2122f4d265 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Tue, 31 Mar 2026 17:06:46 -0700 Subject: [PATCH] fix: use jsonc-parser for safe JSONC migration and add project-local config detection --- bun-test.d.ts | 25 ++++- .../legacy-plugin-toast/auto-migrate.test.ts | 33 +++++++ src/hooks/legacy-plugin-toast/auto-migrate.ts | 27 +----- src/hooks/legacy-plugin-toast/hook.test.ts | 34 ++++++- src/hooks/legacy-plugin-toast/hook.ts | 6 +- src/shared/legacy-plugin-warning.test.ts | 24 +++++ src/shared/legacy-plugin-warning.ts | 92 +++++++++++++------ 7 files changed, 187 insertions(+), 54 deletions(-) diff --git a/bun-test.d.ts b/bun-test.d.ts index 41d164f6a..43bdc481b 100644 --- a/bun-test.d.ts +++ b/bun-test.d.ts @@ -1,18 +1,41 @@ declare module "bun:test" { + interface MockMetadata { + calls: TArgs[] + } + + interface MockFunction { + (...args: TArgs): TReturn + mock: MockMetadata + mockReset(): void + mockReturnValue(value: TReturn): void + mockResolvedValue(value: Awaited): void + } + export function describe(name: string, fn: () => void): void export function it(name: string, fn: () => void | Promise): void export function beforeEach(fn: () => void | Promise): void export function afterEach(fn: () => void | Promise): void export function beforeAll(fn: () => void | Promise): void export function afterAll(fn: () => void | Promise): void - export function mock unknown>(fn: T): T + export function mock( + fn: (...args: TArgs) => TReturn, + ): MockFunction + + export namespace mock { + function module(modulePath: string, factory: () => Record): void + function restore(): void + } interface Matchers { toBe(expected: unknown): void + toBeNull(): void toEqual(expected: unknown): void toContain(expected: unknown): void toMatch(expected: RegExp | string): void toHaveLength(expected: number): void + toHaveBeenCalled(): void + toHaveBeenCalledTimes(expected: number): void + toHaveBeenCalledWith(...expected: unknown[]): void toBeGreaterThan(expected: number): void toThrow(expected?: RegExp | string): void toStartWith(expected: string): void diff --git a/src/hooks/legacy-plugin-toast/auto-migrate.test.ts b/src/hooks/legacy-plugin-toast/auto-migrate.test.ts index 0ee33cb8c..cc8be7497 100644 --- a/src/hooks/legacy-plugin-toast/auto-migrate.test.ts +++ b/src/hooks/legacy-plugin-toast/auto-migrate.test.ts @@ -1,3 +1,5 @@ +/// + import { afterEach, beforeEach, describe, expect, it } from "bun:test" import { mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs" import { tmpdir } from "node:os" @@ -118,6 +120,37 @@ describe("autoMigrateLegacyPluginEntry", () => { }) }) + describe("#given opencode.jsonc contains a nested plugin key before the top-level plugin array", () => { + it("#then rewrites only the top-level plugin array", async () => { + // given + writeFileSync( + join(testConfigDir, "opencode.jsonc"), + `{ + "nested": { + "plugin": ["oh-my-opencode"] + }, + "plugin": ["oh-my-opencode@latest"] +} +`, + ) + + const { autoMigrateLegacyPluginEntry } = await importFreshAutoMigrateModule() + + // when + const result = autoMigrateLegacyPluginEntry(testConfigDir) + + // then + expect(result.migrated).toBe(true) + const content = readFileSync(join(testConfigDir, "opencode.jsonc"), "utf-8") + expect(content).toContain(`"nested": { + "plugin": ["oh-my-opencode"] + }`) + expect(content).toContain(`"plugin": [ + "oh-my-openagent@latest" + ]`) + }) + }) + describe("#given only canonical entry exists", () => { it("#then returns migrated false and leaves file untouched", async () => { // given diff --git a/src/hooks/legacy-plugin-toast/auto-migrate.ts b/src/hooks/legacy-plugin-toast/auto-migrate.ts index 34bc4bbc0..f1ce1090a 100644 --- a/src/hooks/legacy-plugin-toast/auto-migrate.ts +++ b/src/hooks/legacy-plugin-toast/auto-migrate.ts @@ -1,7 +1,8 @@ -import { existsSync, readFileSync, writeFileSync } from "node:fs" +import { existsSync, readFileSync } from "node:fs" import { join } from "node:path" import { parseJsoncSafe } from "../../shared/jsonc-parser" +import { migrateLegacyPluginEntry } from "../../shared/migrate-legacy-plugin-entry" import { getOpenCodeConfigPaths } from "../../shared/opencode-config-dir" import { LEGACY_PLUGIN_NAME, PLUGIN_NAME } from "../../shared/plugin-identity" @@ -20,10 +21,6 @@ function isLegacyEntry(entry: string): boolean { return entry === LEGACY_PLUGIN_NAME || entry.startsWith(`${LEGACY_PLUGIN_NAME}@`) } -function isCanonicalEntry(entry: string): boolean { - return entry === PLUGIN_NAME || entry.startsWith(`${PLUGIN_NAME}@`) -} - function toLegacyCanonical(entry: string): string { if (entry === LEGACY_PLUGIN_NAME) return PLUGIN_NAME if (entry.startsWith(`${LEGACY_PLUGIN_NAME}@`)) { @@ -60,29 +57,13 @@ export function autoMigrateLegacyPluginEntry(overrideConfigDir?: string): Migrat const legacyEntries = plugins.filter(isLegacyEntry) if (legacyEntries.length === 0) return { migrated: false, from: null, to: null, configPath } - const hasCanonical = plugins.some(isCanonicalEntry) const from = legacyEntries[0] const to = toLegacyCanonical(from) - const normalized = hasCanonical - ? plugins.filter((p) => !isLegacyEntry(p)) - : plugins.map((p) => (isLegacyEntry(p) ? toLegacyCanonical(p) : p)) - - const isJsonc = configPath.endsWith(".jsonc") - if (isJsonc) { - const pluginArrayRegex = /((?:"plugin"|plugin)\s*:\s*)\[([\s\S]*?)\]/ - const match = content.match(pluginArrayRegex) - if (match) { - const formattedPlugins = normalized.map((p) => `"${p}"`).join(",\n ") - const newContent = content.replace(pluginArrayRegex, `$1[\n ${formattedPlugins}\n ]`) - writeFileSync(configPath, newContent) - return { migrated: true, from, to, configPath } - } + if (!migrateLegacyPluginEntry(configPath)) { + return { migrated: false, from: null, to: null, configPath } } - const parsed = JSON.parse(content) as Record - parsed.plugin = normalized - writeFileSync(configPath, JSON.stringify(parsed, null, 2) + "\n") return { migrated: true, from, to, configPath } } catch { return { migrated: false, from: null, to: null, configPath } diff --git a/src/hooks/legacy-plugin-toast/hook.test.ts b/src/hooks/legacy-plugin-toast/hook.test.ts index 490908429..d71d0d9f1 100644 --- a/src/hooks/legacy-plugin-toast/hook.test.ts +++ b/src/hooks/legacy-plugin-toast/hook.test.ts @@ -1,10 +1,15 @@ +/// + import { afterAll, beforeEach, describe, expect, it, mock } from "bun:test" + +import type { LegacyPluginCheckResult } from "../../shared/legacy-plugin-warning" import type { MigrationResult } from "./auto-migrate" -const mockCheckForLegacyPluginEntry = mock(() => ({ +const mockCheckForLegacyPluginEntry = mock((): LegacyPluginCheckResult => ({ hasLegacyEntry: false, hasCanonicalEntry: false, legacyEntries: [] as string[], + configPath: null, })) const mockAutoMigrate = mock((): MigrationResult => ({ @@ -67,6 +72,7 @@ describe("createLegacyPluginToastHook", () => { hasLegacyEntry: false, hasCanonicalEntry: true, legacyEntries: [], + configPath: null, }) mockAutoMigrate.mockReturnValue({ migrated: false, from: null, to: null, configPath: null }) mockShowToast.mockResolvedValue(undefined) @@ -93,6 +99,7 @@ describe("createLegacyPluginToastHook", () => { hasLegacyEntry: true, hasCanonicalEntry: false, legacyEntries: ["oh-my-opencode"], + configPath: "/tmp/opencode.json", }) mockAutoMigrate.mockReturnValue({ migrated: true, @@ -120,6 +127,7 @@ describe("createLegacyPluginToastHook", () => { hasLegacyEntry: true, hasCanonicalEntry: false, legacyEntries: ["oh-my-opencode"], + configPath: "/tmp/opencode.json", }) mockAutoMigrate.mockReturnValue({ migrated: false, @@ -147,6 +155,7 @@ describe("createLegacyPluginToastHook", () => { hasLegacyEntry: true, hasCanonicalEntry: false, legacyEntries: ["oh-my-opencode"], + configPath: "/tmp/opencode.json", }) mockAutoMigrate.mockReturnValue({ migrated: true, @@ -173,6 +182,7 @@ describe("createLegacyPluginToastHook", () => { hasLegacyEntry: true, hasCanonicalEntry: false, legacyEntries: ["oh-my-opencode"], + configPath: "/tmp/opencode.json", }) const { createLegacyPluginToastHook } = await importFreshModule() const hook = createLegacyPluginToastHook(createMockCtx()) @@ -192,6 +202,7 @@ describe("createLegacyPluginToastHook", () => { hasLegacyEntry: true, hasCanonicalEntry: false, legacyEntries: ["oh-my-opencode"], + configPath: "/tmp/opencode.json", }) const { createLegacyPluginToastHook } = await importFreshModule() const hook = createLegacyPluginToastHook(createMockCtx()) @@ -203,4 +214,25 @@ describe("createLegacyPluginToastHook", () => { expect(mockCheckForLegacyPluginEntry).not.toHaveBeenCalled() }) }) + + describe("#given a project directory is available", () => { + it("#then passes the project directory into legacy config detection", async () => { + // given + mockCheckForLegacyPluginEntry.mockReturnValue({ + hasLegacyEntry: true, + hasCanonicalEntry: false, + legacyEntries: ["oh-my-opencode"], + configPath: "/tmp/test/.opencode/opencode.json", + }) + const { createLegacyPluginToastHook } = await importFreshModule() + const hook = createLegacyPluginToastHook(createMockCtx()) + + // when + await hook.event(createEvent("session.created")) + + // then + expect(mockCheckForLegacyPluginEntry).toHaveBeenCalledWith(undefined, "/tmp/test") + expect(mockAutoMigrate).toHaveBeenCalledWith("/tmp/test/.opencode") + }) + }) }) diff --git a/src/hooks/legacy-plugin-toast/hook.ts b/src/hooks/legacy-plugin-toast/hook.ts index 4d6f55918..89b086a8a 100644 --- a/src/hooks/legacy-plugin-toast/hook.ts +++ b/src/hooks/legacy-plugin-toast/hook.ts @@ -1,3 +1,5 @@ +import { dirname } from "node:path" + import type { PluginInput } from "@opencode-ai/plugin" import { checkForLegacyPluginEntry } from "../../shared/legacy-plugin-warning" @@ -17,10 +19,10 @@ export function createLegacyPluginToastHook(ctx: PluginInput) { fired = true - const result = checkForLegacyPluginEntry() + const result = checkForLegacyPluginEntry(undefined, ctx.directory) if (!result.hasLegacyEntry) return - const migration = autoMigrateLegacyPluginEntry() + const migration = autoMigrateLegacyPluginEntry(result.configPath ? dirname(result.configPath) : undefined) if (migration.migrated) { log("[legacy-plugin-toast] Auto-migrated opencode.json plugin entry", { diff --git a/src/shared/legacy-plugin-warning.test.ts b/src/shared/legacy-plugin-warning.test.ts index 9d114f9db..11cef173d 100644 --- a/src/shared/legacy-plugin-warning.test.ts +++ b/src/shared/legacy-plugin-warning.test.ts @@ -1,3 +1,5 @@ +/// + import { afterEach, beforeEach, describe, expect, it } from "bun:test" import { mkdirSync, rmSync, writeFileSync } from "node:fs" import { tmpdir } from "node:os" @@ -80,4 +82,26 @@ describe("checkForLegacyPluginEntry", () => { expect(result.legacyEntries).toEqual([]) expect(result.configPath).toBeNull() }) + + describe("#given a project-local .opencode config contains a legacy plugin entry", () => { + it("#then detects the project-local config path", () => { + // given + const projectDir = join(testConfigDir, "project") + const projectConfigDir = join(projectDir, ".opencode") + mkdirSync(projectConfigDir, { recursive: true }) + writeFileSync( + join(projectConfigDir, "opencode.json"), + JSON.stringify({ plugin: ["oh-my-opencode"] }, null, 2), + ) + + // when + const result = checkForLegacyPluginEntry(undefined, projectDir) + + // then + expect(result.hasLegacyEntry).toBe(true) + expect(result.hasCanonicalEntry).toBe(false) + expect(result.legacyEntries).toEqual(["oh-my-opencode"]) + expect(result.configPath).toBe(join(projectConfigDir, "opencode.json")) + }) + }) }) diff --git a/src/shared/legacy-plugin-warning.ts b/src/shared/legacy-plugin-warning.ts index 6ab2a77ef..28fdf624e 100644 --- a/src/shared/legacy-plugin-warning.ts +++ b/src/shared/legacy-plugin-warning.ts @@ -16,20 +16,36 @@ export interface LegacyPluginCheckResult { configPath: string | null } -function getOpenCodeConfigPath(overrideConfigDir?: string): string | null { +function getConfigPathFromDirectory(configDir: string): string | null { + const jsonPath = join(configDir, "opencode.json") + const jsoncPath = join(configDir, "opencode.jsonc") + + if (existsSync(jsoncPath)) return jsoncPath + if (existsSync(jsonPath)) return jsonPath + return null +} + +function getOpenCodeConfigPathsToCheck(overrideConfigDir?: string, projectDir?: string): string[] { if (overrideConfigDir) { - const jsonPath = join(overrideConfigDir, "opencode.json") - const jsoncPath = join(overrideConfigDir, "opencode.jsonc") - if (existsSync(jsoncPath)) return jsoncPath - if (existsSync(jsonPath)) return jsonPath - return null + const overridePath = getConfigPathFromDirectory(overrideConfigDir) + return overridePath ? [overridePath] : [] + } + + const configPaths: string[] = [] + + if (projectDir) { + const projectConfigPath = getConfigPathFromDirectory(join(projectDir, ".opencode")) + if (projectConfigPath) { + configPaths.push(projectConfigPath) + } } const { configJsonc, configJson } = getOpenCodeConfigPaths({ binary: "opencode", version: null }) - if (existsSync(configJsonc)) return configJsonc - if (existsSync(configJson)) return configJson - return null + if (existsSync(configJsonc)) configPaths.push(configJsonc) + else if (existsSync(configJson)) configPaths.push(configJson) + + return configPaths } function isLegacyPluginEntry(entry: string): boolean { @@ -40,29 +56,51 @@ function isCanonicalPluginEntry(entry: string): boolean { return entry === PLUGIN_NAME || entry.startsWith(`${PLUGIN_NAME}@`) } -export function checkForLegacyPluginEntry(overrideConfigDir?: string): LegacyPluginCheckResult { - const configPath = getOpenCodeConfigPath(overrideConfigDir) - if (!configPath) { +export function checkForLegacyPluginEntry( + overrideConfigDir?: string, + projectDir?: string, +): LegacyPluginCheckResult { + const configPaths = getOpenCodeConfigPathsToCheck(overrideConfigDir, projectDir) + if (configPaths.length === 0) { return { hasLegacyEntry: false, hasCanonicalEntry: false, legacyEntries: [], configPath: null } } - try { - const content = readFileSync(configPath, "utf-8") - const parseResult = parseJsoncSafe(content) - if (!parseResult.data) { - return { hasLegacyEntry: false, hasCanonicalEntry: false, legacyEntries: [], configPath } - } + let hasCanonicalEntry = false + let detectedConfigPath: string | null = null - const legacyEntries = (parseResult.data.plugin ?? []).filter(isLegacyPluginEntry) - const hasCanonicalEntry = (parseResult.data.plugin ?? []).some(isCanonicalPluginEntry) + for (const configPath of configPaths) { + detectedConfigPath ??= configPath - return { - hasLegacyEntry: legacyEntries.length > 0, - hasCanonicalEntry, - legacyEntries, - configPath, + try { + const content = readFileSync(configPath, "utf-8") + const parseResult = parseJsoncSafe(content) + if (!parseResult.data) { + continue + } + + const pluginEntries = parseResult.data.plugin ?? [] + const legacyEntries = pluginEntries.filter(isLegacyPluginEntry) + const fileHasCanonicalEntry = pluginEntries.some(isCanonicalPluginEntry) + + if (legacyEntries.length > 0) { + return { + hasLegacyEntry: true, + hasCanonicalEntry: fileHasCanonicalEntry, + legacyEntries, + configPath, + } + } + + hasCanonicalEntry ||= fileHasCanonicalEntry + } catch { + continue } - } catch { - return { hasLegacyEntry: false, hasCanonicalEntry: false, legacyEntries: [], configPath: null } + } + + return { + hasLegacyEntry: false, + hasCanonicalEntry, + legacyEntries: [], + configPath: detectedConfigPath, } }