From d60ca63ed4ddd35fc2a837657c08d1ef919f2428 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 4 Apr 2026 01:32:17 +0900 Subject: [PATCH] test(config): add regression coverage for legacy migration bugs Lock the current legacy config and plugin migration failures in place before the fixes land so the three regressions stay covered. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../legacy-plugin-toast/auto-migrate.test.ts | 60 ++++++++++++++----- src/plugin-config.test.ts | 31 +++++++++- src/shared/external-plugin-detector.test.ts | 35 ++++++++++- src/shared/migrate-legacy-config-file.test.ts | 6 +- 4 files changed, 115 insertions(+), 17 deletions(-) diff --git a/src/hooks/legacy-plugin-toast/auto-migrate.test.ts b/src/hooks/legacy-plugin-toast/auto-migrate.test.ts index 0ee33cb8c..b38686019 100644 --- a/src/hooks/legacy-plugin-toast/auto-migrate.test.ts +++ b/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 { 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() }) }) }) diff --git a/src/plugin-config.test.ts b/src/plugin-config.test.ts index 8ecaea7f0..19da20ece 100644 --- a/src/plugin-config.test.ts +++ b/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") + }) }) diff --git a/src/shared/external-plugin-detector.test.ts b/src/shared/external-plugin-detector.test.ts index 03ecfd5a8..a220dff90 100644 --- a/src/shared/external-plugin-detector.test.ts +++ b/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 { + 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") diff --git a/src/shared/migrate-legacy-config-file.test.ts b/src/shared/migrate-legacy-config-file.test.ts index 0e032c8b9..7fd8a1d7a 100644 --- a/src/shared/migrate-legacy-config-file.test.ts +++ b/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": {} }') }) }) })