Merge pull request #3092 from code-yeongyu/fix/prepublish-legacy-config

fix(shared): close legacy config migration gaps

src/hooks/legacy-plugin-toast/auto-migrate-runner.ts

@@ -0,0 +1,2 @@
+export { autoMigrateLegacyPluginEntry } from "./auto-migrate"
+export type { MigrationResult } from "./auto-migrate"

src/hooks/legacy-plugin-toast/auto-migrate.test.ts

@@ -1,7 +1,14 @@
-import { afterEach, beforeEach, describe, expect, it } from "bun:test"
-import { mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"
+import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test"
+import { mkdirSync, rmSync, writeFileSync } from "node:fs"
 import { tmpdir } from "node:os"
 import { join } from "node:path"
+
+const mockMigrateLegacyPluginEntry = mock(() => true)
+
+mock.module("./plugin-entry-migrator", () => ({
+  migrateLegacyPluginEntry: mockMigrateLegacyPluginEntry,
+}))
+
 async function importFreshAutoMigrateModule(): Promise<typeof import("./auto-migrate")> {
   return import(`./auto-migrate?test=${Date.now()}-${Math.random()}`)
 }
@@ -12,6 +19,8 @@ describe("autoMigrateLegacyPluginEntry", () => {
   beforeEach(() => {
     testConfigDir = join(tmpdir(), `omo-legacy-migrate-${Date.now()}-${Math.random().toString(36).slice(2)}`)
     mkdirSync(testConfigDir, { recursive: true })
+    mockMigrateLegacyPluginEntry.mockReset()
+    mockMigrateLegacyPluginEntry.mockReturnValue(true)
   })
 
   afterEach(() => {
@@ -35,8 +44,7 @@ describe("autoMigrateLegacyPluginEntry", () => {
       expect(result.migrated).toBe(true)
       expect(result.from).toBe("oh-my-opencode")
       expect(result.to).toBe("oh-my-openagent")
-      const saved = JSON.parse(readFileSync(join(testConfigDir, "opencode.json"), "utf-8"))
-      expect(saved.plugin).toEqual(["oh-my-openagent"])
+      expect(mockMigrateLegacyPluginEntry).toHaveBeenCalledWith(join(testConfigDir, "opencode.json"))
     })
   })
 
@@ -57,8 +65,7 @@ describe("autoMigrateLegacyPluginEntry", () => {
       expect(result.migrated).toBe(true)
       expect(result.from).toBe("oh-my-opencode@3.10.0")
       expect(result.to).toBe("oh-my-openagent@3.10.0")
-      const saved = JSON.parse(readFileSync(join(testConfigDir, "opencode.json"), "utf-8"))
-      expect(saved.plugin).toEqual(["oh-my-openagent@3.10.0"])
+      expect(mockMigrateLegacyPluginEntry).toHaveBeenCalledWith(join(testConfigDir, "opencode.json"))
     })
   })
 
@@ -77,8 +84,8 @@ describe("autoMigrateLegacyPluginEntry", () => {
 
       // then
       expect(result.migrated).toBe(true)
-      const saved = JSON.parse(readFileSync(join(testConfigDir, "opencode.json"), "utf-8"))
-      expect(saved.plugin).toEqual(["oh-my-openagent"])
+      expect(result.to).toBe("oh-my-openagent")
+      expect(mockMigrateLegacyPluginEntry).toHaveBeenCalledWith(join(testConfigDir, "opencode.json"))
     })
   })
 
@@ -93,6 +100,7 @@ describe("autoMigrateLegacyPluginEntry", () => {
       // then
       expect(result.migrated).toBe(false)
       expect(result.from).toBeNull()
+      expect(mockMigrateLegacyPluginEntry).not.toHaveBeenCalled()
     })
   })
 
@@ -111,10 +119,35 @@ describe("autoMigrateLegacyPluginEntry", () => {
 
       // then
       expect(result.migrated).toBe(true)
-      const content = readFileSync(join(testConfigDir, "opencode.jsonc"), "utf-8")
-      expect(content).toContain("// my config")
-      expect(content).toContain("oh-my-openagent")
-      expect(content).not.toContain("oh-my-opencode")
+      expect(result.to).toBe("oh-my-openagent")
+      expect(mockMigrateLegacyPluginEntry).toHaveBeenCalledWith(join(testConfigDir, "opencode.jsonc"))
+    })
+  })
+
+  describe("#given opencode.jsonc has a nested plugin key before the root plugin array", () => {
+    it("#then migrates only the root plugin entry", 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)
+      expect(result.from).toBe("oh-my-opencode@latest")
+      expect(result.to).toBe("oh-my-openagent@latest")
+      expect(mockMigrateLegacyPluginEntry).toHaveBeenCalledWith(join(testConfigDir, "opencode.jsonc"))
     })
   })
 
@@ -131,8 +164,7 @@ describe("autoMigrateLegacyPluginEntry", () => {
 
       // then
       expect(result.migrated).toBe(false)
-      const content = readFileSync(join(testConfigDir, "opencode.json"), "utf-8")
-      expect(content).toBe(original)
+      expect(mockMigrateLegacyPluginEntry).not.toHaveBeenCalled()
     })
   })
 })

src/hooks/legacy-plugin-toast/auto-migrate.ts

@@ -1,9 +1,10 @@
-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 { getOpenCodeConfigPaths } from "../../shared/opencode-config-dir"
 import { LEGACY_PLUGIN_NAME, PLUGIN_NAME } from "../../shared/plugin-identity"
+import { migrateLegacyPluginEntry } from "./plugin-entry-migrator"
 
 export interface MigrationResult {
   migrated: boolean
@@ -63,27 +64,15 @@ export function autoMigrateLegacyPluginEntry(overrideConfigDir?: string): Migrat
     const hasCanonical = plugins.some(isCanonicalEntry)
     const from = legacyEntries[0]
     const to = toLegacyCanonical(from)
+    const migrated = migrateLegacyPluginEntry(configPath)
+    if (!migrated) return { migrated: false, from: null, to: null, configPath }
 
-    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 }
-      }
+    return {
+      migrated: true,
+      from,
+      to: hasCanonical ? PLUGIN_NAME : to,
+      configPath,
     }
-
-    const parsed = JSON.parse(content) as Record<string, unknown>
-    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 }
   }

src/hooks/legacy-plugin-toast/hook.test.ts

@@ -1,5 +1,5 @@
 import { afterAll, beforeEach, describe, expect, it, mock } from "bun:test"
-import type { MigrationResult } from "./auto-migrate"
+import type { MigrationResult } from "./auto-migrate-runner"
 
 const mockCheckForLegacyPluginEntry = mock(() => ({
   hasLegacyEntry: false,
@@ -26,7 +26,7 @@ mock.module("../../shared/logger", () => ({
   log: mockLog,
 }))
 
-mock.module("./auto-migrate", () => ({
+mock.module("./auto-migrate-runner", () => ({
   autoMigrateLegacyPluginEntry: mockAutoMigrate,
 }))
 

src/hooks/legacy-plugin-toast/hook.ts

@@ -3,7 +3,7 @@ import type { PluginInput } from "@opencode-ai/plugin"
 import { checkForLegacyPluginEntry } from "../../shared/legacy-plugin-warning"
 import { log } from "../../shared/logger"
 import { LEGACY_PLUGIN_NAME, PLUGIN_NAME } from "../../shared/plugin-identity"
-import { autoMigrateLegacyPluginEntry } from "./auto-migrate"
+import { autoMigrateLegacyPluginEntry } from "./auto-migrate-runner"
 
 export function createLegacyPluginToastHook(ctx: PluginInput) {
   let fired = false

src/hooks/legacy-plugin-toast/plugin-entry-migrator.ts

@@ -0,0 +1 @@
+export { migrateLegacyPluginEntry } from "../../shared/migrate-legacy-plugin-entry"

src/plugin-config.test.ts

@@ -1,5 +1,5 @@
 import { afterEach, describe, expect, it, mock, spyOn } from "bun:test";
-import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs"
+import { existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"
 import { tmpdir } from "node:os"
 import { join } from "node:path"
 import * as shared from "./shared"
@@ -321,4 +321,33 @@ describe("loadPluginConfig", () => {
     // then
     expect(config.mcp_env_allowlist).toEqual(["USER_ONLY_TOKEN"])
   })
+
+  it("should ignore edits to the renamed legacy backup after migration", () => {
+    // given
+    const rootDir = mkdtempSync(join(tmpdir(), "omo-plugin-config-legacy-"))
+    const userConfigDir = join(rootDir, "user-config")
+    const projectDir = join(rootDir, "project")
+    const projectConfigDir = join(projectDir, ".opencode")
+    const legacyConfigPath = join(projectConfigDir, "oh-my-opencode.jsonc")
+    const backupConfigPath = `${legacyConfigPath}.bak`
+    const canonicalConfigPath = join(projectConfigDir, "oh-my-openagent.jsonc")
+
+    tempDirs.push(rootDir)
+    mkdirSync(userConfigDir, { recursive: true })
+    mkdirSync(projectConfigDir, { recursive: true })
+    writeFileSync(legacyConfigPath, JSON.stringify({ agents: { oracle: { model: "openai/gpt-5.4" } } }))
+
+    spyOn(shared, "getOpenCodeConfigDir").mockReturnValue(userConfigDir)
+
+    // when
+    loadPluginConfig(projectDir, {})
+    writeFileSync(backupConfigPath, JSON.stringify({ agents: { oracle: { model: "openai/gpt-5-nano" } } }))
+    const reloadedConfig = loadPluginConfig(projectDir, {})
+
+    // then
+    expect(existsSync(legacyConfigPath)).toBe(false)
+    expect(existsSync(backupConfigPath)).toBe(true)
+    expect(readFileSync(canonicalConfigPath, "utf-8")).toContain('"openai/gpt-5.4"')
+    expect(reloadedConfig.agents?.oracle?.model).toBe("openai/gpt-5.4")
+  })
 })

src/shared/external-plugin-detector.test.ts

@@ -1,18 +1,26 @@
-import { describe, expect, test, beforeEach, afterEach } from "bun:test"
+import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"
 import { detectExternalNotificationPlugin, getNotificationConflictWarning, detectExternalSkillPlugin, getSkillPluginConflictWarning } from "./external-plugin-detector"
 import * as fs from "node:fs"
 import * as path from "node:path"
 import * as os from "node:os"
 
+async function importFreshExternalPluginDetectorModule(): Promise<typeof import("./external-plugin-detector")> {
+  return import(`./external-plugin-detector?test=${Date.now()}-${Math.random()}`)
+}
+
 describe("external-plugin-detector", () => {
   let tempDir: string
+  let tempHomeDir: string
 
   beforeEach(() => {
     tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "omo-test-"))
+    tempHomeDir = fs.mkdtempSync(path.join(os.tmpdir(), "omo-home-"))
   })
 
   afterEach(() => {
+    mock.restore()
     fs.rmSync(tempDir, { recursive: true, force: true })
+    fs.rmSync(tempHomeDir, { recursive: true, force: true })
   })
 
   describe("detectExternalNotificationPlugin", () => {
@@ -399,6 +407,31 @@ describe("external-plugin-detector", () => {
       expect(result.pluginName).toBe("opencode-skills")
     })
 
+    test("should detect user-level opencode-skills when project config exists without plugins", async () => {
+      // given
+      const projectConfigDir = path.join(tempDir, ".opencode")
+      const userConfigDir = path.join(tempHomeDir, ".config", "opencode")
+      fs.mkdirSync(projectConfigDir, { recursive: true })
+      fs.mkdirSync(userConfigDir, { recursive: true })
+      fs.writeFileSync(path.join(projectConfigDir, "opencode.json"), JSON.stringify({}))
+      fs.writeFileSync(path.join(userConfigDir, "opencode.json"), JSON.stringify({ plugin: ["opencode-skills"] }))
+
+      const nodeOs = await import("node:os")
+      mock.module("node:os", () => ({
+        ...nodeOs,
+        homedir: () => tempHomeDir,
+      }))
+      const { detectExternalSkillPlugin: detectExternalSkillPluginFresh } = await importFreshExternalPluginDetectorModule()
+
+      // when
+      const result = detectExternalSkillPluginFresh(tempDir)
+
+      // then
+      expect(result.detected).toBe(true)
+      expect(result.pluginName).toBe("opencode-skills")
+      expect(result.allPlugins).toEqual(["opencode-skills"])
+    })
+
     test("should NOT match opencode-skills-extra (suffix variation)", () => {
       // given - plugin with similar name but different suffix
       const opencodeDir = path.join(tempDir, ".opencode")

src/shared/external-plugin-detector.ts

@@ -3,15 +3,8 @@
  * Used to prevent crashes from concurrent notification plugins.
  */
 
-import * as fs from "node:fs"
-import * as path from "node:path"
-import * as os from "node:os"
+import { loadOpencodePlugins } from "./load-opencode-plugins"
 import { log } from "./logger"
-import { parseJsoncSafe } from "./jsonc-parser"
-
-interface OpencodeConfig {
-  plugin?: string[]
-}
 
 /**
  * Known notification plugins that conflict with oh-my-opencode's session-notification.
@@ -34,46 +27,6 @@ const KNOWN_SKILL_PLUGINS = [
   "@opencode/skills",
 ]
 
-function getWindowsAppdataDir(): string | null {
-  return process.env.APPDATA || null
-}
-
-function getConfigPaths(directory: string): string[] {
-  const crossPlatformDir = path.join(os.homedir(), ".config")
-  const paths = [
-    path.join(directory, ".opencode", "opencode.json"),
-    path.join(directory, ".opencode", "opencode.jsonc"),
-    path.join(crossPlatformDir, "opencode", "opencode.json"),
-    path.join(crossPlatformDir, "opencode", "opencode.jsonc"),
-  ]
-
-  if (process.platform === "win32") {
-    const appdataDir = getWindowsAppdataDir()
-    if (appdataDir) {
-      paths.push(path.join(appdataDir, "opencode", "opencode.json"))
-      paths.push(path.join(appdataDir, "opencode", "opencode.jsonc"))
-    }
-  }
-
-  return paths
-}
-
-function loadOpencodePlugins(directory: string): string[] {
-  for (const configPath of getConfigPaths(directory)) {
-    try {
-      if (!fs.existsSync(configPath)) continue
-      const content = fs.readFileSync(configPath, "utf-8")
-      const result = parseJsoncSafe<OpencodeConfig>(content)
-      if (result.data) {
-        return result.data.plugin ?? []
-      }
-    } catch {
-      continue
-    }
-  }
-  return []
-}
-
 /**
  * Check if a plugin entry matches a known notification plugin.
  * Handles various formats: "name", "name@version", "npm:name", "file://path/name"

src/shared/load-opencode-plugins.ts

@@ -0,0 +1,58 @@
+import * as fs from "node:fs"
+import * as os from "node:os"
+import * as path from "node:path"
+
+import { parseJsoncSafe } from "./jsonc-parser"
+
+interface OpencodeConfig {
+  plugin?: string[]
+}
+
+function getWindowsAppdataDir(): string | null {
+  return process.env.APPDATA || null
+}
+
+function getConfigPaths(directory: string): string[] {
+  const crossPlatformDir = path.join(os.homedir(), ".config")
+  const paths = [
+    path.join(directory, ".opencode", "opencode.json"),
+    path.join(directory, ".opencode", "opencode.jsonc"),
+    path.join(crossPlatformDir, "opencode", "opencode.json"),
+    path.join(crossPlatformDir, "opencode", "opencode.jsonc"),
+  ]
+
+  if (process.platform === "win32") {
+    const appdataDir = getWindowsAppdataDir()
+    if (appdataDir) {
+      paths.push(path.join(appdataDir, "opencode", "opencode.json"))
+      paths.push(path.join(appdataDir, "opencode", "opencode.jsonc"))
+    }
+  }
+
+  return paths
+}
+
+export function loadOpencodePlugins(directory: string): string[] {
+  const pluginEntries: string[] = []
+  const seenPluginEntries = new Set<string>()
+
+  for (const configPath of getConfigPaths(directory)) {
+    try {
+      if (!fs.existsSync(configPath)) continue
+
+      const content = fs.readFileSync(configPath, "utf-8")
+      const result = parseJsoncSafe<OpencodeConfig>(content)
+      const plugins = result.data?.plugin ?? []
+
+      for (const plugin of plugins) {
+        if (seenPluginEntries.has(plugin)) continue
+        seenPluginEntries.add(plugin)
+        pluginEntries.push(plugin)
+      }
+    } catch {
+      continue
+    }
+  }
+
+  return pluginEntries
+}

src/shared/log-legacy-plugin-startup-warning.test.ts

@@ -26,7 +26,7 @@ mock.module("./logger", () => ({
   log: mockLog,
 }))
 
-mock.module("./migrate-legacy-plugin-entry", () => ({
+mock.module("./plugin-entry-migrator", () => ({
   migrateLegacyPluginEntry: mockMigrateLegacyPluginEntry,
 }))
 

src/shared/log-legacy-plugin-startup-warning.ts

@@ -1,6 +1,6 @@
 import { checkForLegacyPluginEntry } from "./legacy-plugin-warning"
 import { log } from "./logger"
-import { migrateLegacyPluginEntry } from "./migrate-legacy-plugin-entry"
+import { migrateLegacyPluginEntry } from "./plugin-entry-migrator"
 import { LEGACY_PLUGIN_NAME, PLUGIN_NAME } from "./plugin-identity"
 
 function toCanonicalEntry(entry: string): string {

src/shared/migrate-legacy-config-file.test.ts

@@ -18,15 +18,19 @@ describe("migrateLegacyConfigFile", () => {
 
   describe("#given oh-my-opencode.jsonc exists but oh-my-openagent.jsonc does not", () => {
     describe("#when migrating the config file", () => {
-      it("#then copies to oh-my-openagent.jsonc", () => {
+      it("#then writes oh-my-openagent.jsonc and renames the legacy file to a backup", () => {
         const legacyPath = join(testDir, "oh-my-opencode.jsonc")
+        const backupPath = join(testDir, "oh-my-opencode.jsonc.bak")
         writeFileSync(legacyPath, '{ "agents": {} }')
 
         const result = migrateLegacyConfigFile(legacyPath)
 
         expect(result).toBe(true)
         expect(existsSync(join(testDir, "oh-my-openagent.jsonc"))).toBe(true)
+        expect(existsSync(legacyPath)).toBe(false)
+        expect(existsSync(backupPath)).toBe(true)
         expect(readFileSync(join(testDir, "oh-my-openagent.jsonc"), "utf-8")).toBe('{ "agents": {} }')
+        expect(readFileSync(backupPath, "utf-8")).toBe('{ "agents": {} }')
       })
     })
   })

src/shared/migrate-legacy-config-file.ts

@@ -1,4 +1,4 @@
-import { existsSync, copyFileSync, renameSync } from "node:fs"
+import { closeSync, existsSync, fsyncSync, openSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"
 import { join, dirname, basename } from "node:path"
 
 import { log } from "./logger"
@@ -10,6 +10,49 @@ function buildCanonicalPath(legacyPath: string): string {
   return join(dir, `${CONFIG_BASENAME}${ext}`)
 }
 
+function writeFileAtomically(filePath: string, content: string): void {
+  const tempPath = `${filePath}.tmp`
+  writeFileSync(tempPath, content, "utf-8")
+  const tempFileDescriptor = openSync(tempPath, "r")
+  try {
+    fsyncSync(tempFileDescriptor)
+  } finally {
+    closeSync(tempFileDescriptor)
+  }
+  renameSync(tempPath, filePath)
+}
+
+function archiveLegacyConfigFile(legacyPath: string): boolean {
+  const backupPath = `${legacyPath}.bak`
+
+  try {
+    renameSync(legacyPath, backupPath)
+    log("[migrateLegacyConfigFile] Legacy config was migrated and renamed to backup. Update the canonical file only.", {
+      legacyPath,
+      backupPath,
+    })
+    return true
+  } catch (renameError) {
+    try {
+      rmSync(legacyPath)
+      log("[migrateLegacyConfigFile] Legacy config was migrated and removed after backup rename failed. Update the canonical file only.", {
+        legacyPath,
+        backupPath,
+        renameError,
+      })
+      return true
+    } catch (removeError) {
+      log("[migrateLegacyConfigFile] WARNING: canonical config was written but the legacy file still exists and will be ignored. Remove or rename it manually.", {
+        legacyPath,
+        backupPath,
+        renameError,
+        removeError,
+      })
+      return false
+    }
+  }
+}
+
 export function migrateLegacyConfigFile(legacyPath: string): boolean {
   if (!existsSync(legacyPath)) return false
   if (!basename(legacyPath).startsWith(LEGACY_CONFIG_BASENAME)) return false
@@ -18,14 +61,17 @@ export function migrateLegacyConfigFile(legacyPath: string): boolean {
   if (existsSync(canonicalPath)) return false
 
   try {
-    copyFileSync(legacyPath, canonicalPath)
-    log("[migrateLegacyConfigFile] Copied legacy config to canonical path", {
+    const content = readFileSync(legacyPath, "utf-8")
+    writeFileAtomically(canonicalPath, content)
+    const archivedLegacyConfig = archiveLegacyConfigFile(legacyPath)
+    log("[migrateLegacyConfigFile] Migrated legacy config to canonical path", {
       from: legacyPath,
       to: canonicalPath,
+      archivedLegacyConfig,
     })
-    return true
+    return archivedLegacyConfig
   } catch (error) {
-    log("[migrateLegacyConfigFile] Failed to copy legacy config file", { legacyPath, error })
+    log("[migrateLegacyConfigFile] Failed to migrate legacy config file", { legacyPath, error })
     return false
   }
 }

src/shared/plugin-entry-migrator.ts

@@ -0,0 +1 @@
+export { migrateLegacyPluginEntry } from "./migrate-legacy-plugin-entry"