diff --git a/src/cli/config-manager/add-plugin-to-opencode-config.test.ts b/src/cli/config-manager/add-plugin-to-opencode-config.test.ts new file mode 100644 index 000000000..0b37fde16 --- /dev/null +++ b/src/cli/config-manager/add-plugin-to-opencode-config.test.ts @@ -0,0 +1,109 @@ +import { afterEach, beforeEach, describe, expect, it } from "bun:test" +import { mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs" +import { tmpdir } from "node:os" +import { join } from "node:path" + +import { initConfigContext, resetConfigContext } from "./config-context" +import { addPluginToOpenCodeConfig } from "./add-plugin-to-opencode-config" + +describe("addPluginToOpenCodeConfig", () => { + let testConfigDir = "" + let testConfigPath = "" + + beforeEach(() => { + testConfigDir = join(tmpdir(), `omo-add-plugin-${Date.now()}-${Math.random().toString(36).slice(2)}`) + testConfigPath = join(testConfigDir, "opencode.json") + + mkdirSync(testConfigDir, { recursive: true }) + process.env.OPENCODE_CONFIG_DIR = testConfigDir + resetConfigContext() + initConfigContext("opencode", null) + }) + + afterEach(() => { + rmSync(testConfigDir, { recursive: true, force: true }) + resetConfigContext() + delete process.env.OPENCODE_CONFIG_DIR + }) + + describe("#given opencode.json with oh-my-opencode plugin", () => { + it("replaces oh-my-opencode with new plugin entry", async () => { + // given + const config = { plugin: ["oh-my-opencode"] } + writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + + // when + const result = await addPluginToOpenCodeConfig("3.11.0") + + // then + expect(result.success).toBe(true) + const savedConfig = JSON.parse(readFileSync(testConfigPath, "utf-8")) + // getPluginNameWithVersion returns just the name (without version) for non-dist-tag versions + expect(savedConfig.plugin).toContain("oh-my-openagent") + expect(savedConfig.plugin).not.toContain("oh-my-opencode") + }) + + it("replaces oh-my-opencode@old-version with new plugin entry", async () => { + // given + const config = { plugin: ["oh-my-opencode@1.0.0"] } + writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + + // when + const result = await addPluginToOpenCodeConfig("3.11.0") + + // then + expect(result.success).toBe(true) + const savedConfig = JSON.parse(readFileSync(testConfigPath, "utf-8")) + expect(savedConfig.plugin).toContain("oh-my-openagent") + expect(savedConfig.plugin).not.toContain("oh-my-opencode@1.0.0") + }) + }) + + describe("#given opencode.json with oh-my-openagent plugin", () => { + it("keeps existing oh-my-openagent when no version change needed", async () => { + // given + const config = { plugin: ["oh-my-openagent"] } + writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + + // when + const result = await addPluginToOpenCodeConfig("3.11.0") + + // then + expect(result.success).toBe(true) + const savedConfig = JSON.parse(readFileSync(testConfigPath, "utf-8")) + // Should keep the existing entry since getPluginNameWithVersion returns same name + expect(savedConfig.plugin).toContain("oh-my-openagent") + }) + + it("replaces oh-my-openagent@old-version with new plugin entry", async () => { + // given + const config = { plugin: ["oh-my-openagent@1.0.0"] } + writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + + // when + const result = await addPluginToOpenCodeConfig("3.11.0") + + // then + expect(result.success).toBe(true) + const savedConfig = JSON.parse(readFileSync(testConfigPath, "utf-8")) + expect(savedConfig.plugin).toContain("oh-my-openagent") + }) + }) + + describe("#given opencode.json with other plugins", () => { + it("adds new plugin alongside existing plugins", async () => { + // given + const config = { plugin: ["some-other-plugin"] } + writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + + // when + const result = await addPluginToOpenCodeConfig("3.11.0") + + // then + expect(result.success).toBe(true) + const savedConfig = JSON.parse(readFileSync(testConfigPath, "utf-8")) + expect(savedConfig.plugin).toContain("oh-my-openagent") + expect(savedConfig.plugin).toContain("some-other-plugin") + }) + }) +}) diff --git a/src/cli/config-manager/add-plugin-to-opencode-config.ts b/src/cli/config-manager/add-plugin-to-opencode-config.ts index cbe6fcc42..6d0d40ba3 100644 --- a/src/cli/config-manager/add-plugin-to-opencode-config.ts +++ b/src/cli/config-manager/add-plugin-to-opencode-config.ts @@ -1,6 +1,6 @@ import { readFileSync, writeFileSync } from "node:fs" +import { LEGACY_PLUGIN_NAME, PLUGIN_NAME } from "../../shared" import type { ConfigMergeResult } from "../types" -import { PLUGIN_NAME, LEGACY_PLUGIN_NAME } from "../../shared" import { getConfigDir } from "./config-context" import { ensureConfigDirectoryExists } from "./ensure-config-directory-exists" import { formatErrorWithSuggestion } from "./format-error-with-suggestion" @@ -13,11 +13,11 @@ const PACKAGE_NAME = PLUGIN_NAME export async function addPluginToOpenCodeConfig(currentVersion: string): Promise { try { ensureConfigDirectoryExists() - } catch (err) { + } catch (error) { return { success: false, configPath: getConfigDir(), - error: formatErrorWithSuggestion(err, "create config directory"), + error: formatErrorWithSuggestion(error, "create config directory"), } } @@ -42,7 +42,6 @@ export async function addPluginToOpenCodeConfig(currentVersion: string): Promise const config = parseResult.config const plugins = config.plugin ?? [] - const currentNameIndex = plugins.findIndex( (plugin) => plugin === PLUGIN_NAME || plugin.startsWith(`${PLUGIN_NAME}@`) ) @@ -69,7 +68,7 @@ export async function addPluginToOpenCodeConfig(currentVersion: string): Promise const match = content.match(pluginArrayRegex) if (match) { - const formattedPlugins = plugins.map((p) => `"${p}"`).join(",\n ") + const formattedPlugins = plugins.map((plugin) => `"${plugin}"`).join(",\n ") const newContent = content.replace(pluginArrayRegex, `"plugin": [\n ${formattedPlugins}\n ]`) writeFileSync(path, newContent) } else { @@ -81,11 +80,11 @@ export async function addPluginToOpenCodeConfig(currentVersion: string): Promise } return { success: true, configPath: path } - } catch (err) { + } catch (error) { return { success: false, configPath: path, - error: formatErrorWithSuggestion(err, "update opencode config"), + error: formatErrorWithSuggestion(error, "update opencode config"), } } } diff --git a/src/cli/config-manager/detect-current-config.test.ts b/src/cli/config-manager/detect-current-config.test.ts new file mode 100644 index 000000000..304cf70c5 --- /dev/null +++ b/src/cli/config-manager/detect-current-config.test.ts @@ -0,0 +1,106 @@ +import { afterEach, beforeEach, describe, expect, it } from "bun:test" +import { mkdirSync, rmSync, writeFileSync } from "node:fs" +import { tmpdir } from "node:os" +import { join } from "node:path" + +import { initConfigContext, resetConfigContext } from "./config-context" +import { detectCurrentConfig } from "./detect-current-config" + +describe("detectCurrentConfig", () => { + let testConfigDir = "" + let testConfigPath = "" + + beforeEach(() => { + testConfigDir = join(tmpdir(), `omo-detect-config-${Date.now()}-${Math.random().toString(36).slice(2)}`) + testConfigPath = join(testConfigDir, "opencode.json") + + mkdirSync(testConfigDir, { recursive: true }) + process.env.OPENCODE_CONFIG_DIR = testConfigDir + resetConfigContext() + initConfigContext("opencode", null) + }) + + afterEach(() => { + rmSync(testConfigDir, { recursive: true, force: true }) + resetConfigContext() + delete process.env.OPENCODE_CONFIG_DIR + }) + + describe("#given opencode.json with oh-my-opencode plugin", () => { + it("returns isInstalled: true when plugin array contains oh-my-opencode", () => { + // given + const config = { plugin: ["oh-my-opencode"] } + writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + + // when + const result = detectCurrentConfig() + + // then + expect(result.isInstalled).toBe(true) + }) + + it("returns isInstalled: true when plugin array contains oh-my-opencode with version", () => { + // given + const config = { plugin: ["oh-my-opencode@3.11.0"] } + writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + + // when + const result = detectCurrentConfig() + + // then + expect(result.isInstalled).toBe(true) + }) + }) + + describe("#given opencode.json with oh-my-openagent plugin", () => { + it("returns isInstalled: true when plugin array contains oh-my-openagent", () => { + // given + const config = { plugin: ["oh-my-openagent"] } + writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + + // when + const result = detectCurrentConfig() + + // then + expect(result.isInstalled).toBe(true) + }) + + it("returns isInstalled: true when plugin array contains oh-my-openagent with version", () => { + // given + const config = { plugin: ["oh-my-openagent@3.11.0"] } + writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + + // when + const result = detectCurrentConfig() + + // then + expect(result.isInstalled).toBe(true) + }) + }) + + describe("#given opencode.json with no plugin", () => { + it("returns isInstalled: false when plugin array is empty", () => { + // given + const config = { plugin: [] } + writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + + // when + const result = detectCurrentConfig() + + // then + expect(result.isInstalled).toBe(false) + }) + + it("returns isInstalled: false when plugin array does not contain our plugin", () => { + // given + const config = { plugin: ["some-other-plugin"] } + writeFileSync(testConfigPath, JSON.stringify(config, null, 2) + "\n", "utf-8") + + // when + const result = detectCurrentConfig() + + // then + expect(result.isInstalled).toBe(false) + }) + }) +}) diff --git a/src/cli/config-manager/detect-current-config.ts b/src/cli/config-manager/detect-current-config.ts index 3679d5bd6..aada6247f 100644 --- a/src/cli/config-manager/detect-current-config.ts +++ b/src/cli/config-manager/detect-current-config.ts @@ -36,12 +36,12 @@ function detectProvidersFromOmoConfig(): { } } - const configStr = JSON.stringify(omoConfig) - const hasOpenAI = configStr.includes('"openai/') - const hasOpencodeZen = configStr.includes('"opencode/') - const hasZaiCodingPlan = configStr.includes('"zai-coding-plan/') - const hasKimiForCoding = configStr.includes('"kimi-for-coding/') - const hasOpencodeGo = configStr.includes('"opencode-go/') + const configString = JSON.stringify(omoConfig) + const hasOpenAI = configString.includes('"openai/') + const hasOpencodeZen = configString.includes('"opencode/') + const hasZaiCodingPlan = configString.includes('"zai-coding-plan/') + const hasKimiForCoding = configString.includes('"kimi-for-coding/') + const hasOpencodeGo = configString.includes('"opencode-go/') return { hasOpenAI, hasOpencodeZen, hasZaiCodingPlan, hasKimiForCoding, hasOpencodeGo } } catch { @@ -56,8 +56,12 @@ function detectProvidersFromOmoConfig(): { } function isOurPlugin(plugin: string): boolean { - return plugin === PLUGIN_NAME || plugin.startsWith(`${PLUGIN_NAME}@`) || - plugin === LEGACY_PLUGIN_NAME || plugin.startsWith(`${LEGACY_PLUGIN_NAME}@`) + return ( + plugin === PLUGIN_NAME || + plugin.startsWith(`${PLUGIN_NAME}@`) || + plugin === LEGACY_PLUGIN_NAME || + plugin.startsWith(`${LEGACY_PLUGIN_NAME}@`) + ) } export function detectCurrentConfig(): DetectedConfig { diff --git a/src/plugin-config-path.ts b/src/plugin-config-path.ts new file mode 100644 index 000000000..af03f4f44 --- /dev/null +++ b/src/plugin-config-path.ts @@ -0,0 +1,65 @@ +import * as fs from "fs"; +import { detectConfigFile, log } from "./shared"; + +interface ResolveConfigPathOptions { + preferredBasePath: string; + legacyBasePath: string; +} + +function getDefaultConfigPath(basePath: string): string { + return `${basePath}.json`; +} + +function getTargetPath(preferredBasePath: string, format: "json" | "jsonc"): string { + return `${preferredBasePath}.${format}`; +} + +function migrateLegacyConfigPath( + legacyPath: string, + preferredBasePath: string, + format: "json" | "jsonc" +): string { + const targetPath = getTargetPath(preferredBasePath, format); + const timestamp = new Date().toISOString().replace(/[:.]/g, "-"); + const backupPath = `${legacyPath}.bak.${timestamp}`; + + try { + fs.copyFileSync(legacyPath, backupPath); + } catch (error) { + log(`Failed to create backup before config path migration: ${legacyPath}`, error); + return legacyPath; + } + + try { + fs.renameSync(legacyPath, targetPath); + log(`Migrated legacy config path to new name: ${legacyPath} -> ${targetPath} (backup: ${backupPath})`); + return targetPath; + } catch (error) { + log(`Failed to migrate legacy config path: ${legacyPath} -> ${targetPath}`, error); + return legacyPath; + } +} + +export function resolveConfigPathWithLegacyMigration({ + preferredBasePath, + legacyBasePath, +}: ResolveConfigPathOptions): string { + const preferredDetected = detectConfigFile(preferredBasePath); + const legacyDetected = detectConfigFile(legacyBasePath); + + if (preferredDetected.format !== "none") { + if (legacyDetected.format !== "none") { + log( + `Detected legacy config also exists at ${legacyDetected.path}. Using new config path: ${preferredDetected.path}` + ); + } + return preferredDetected.path; + } + + if (legacyDetected.format === "none") { + return getDefaultConfigPath(preferredBasePath); + } + + log(`Legacy config basename detected at ${legacyDetected.path}. Attempting auto-migration to new basename.`); + return migrateLegacyConfigPath(legacyDetected.path, preferredBasePath, legacyDetected.format); +} diff --git a/src/plugin-config.test.ts b/src/plugin-config.test.ts index 3d6c82030..0cf77da08 100644 --- a/src/plugin-config.test.ts +++ b/src/plugin-config.test.ts @@ -1,6 +1,18 @@ -import { describe, expect, it } from "bun:test"; -import { mergeConfigs, parseConfigPartially } from "./plugin-config"; +import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { + loadPluginConfig, + mergeConfigs, + parseConfigPartially, +} from "./plugin-config"; import type { OhMyOpenAgentConfig } from "./config"; +import { + CONFIG_BASENAME, + LEGACY_CONFIG_BASENAME, +} from "./shared/plugin-identity"; +import { getLogFilePath } from "./shared"; describe("mergeConfigs", () => { describe("categories merging", () => { @@ -237,3 +249,145 @@ describe("parseConfigPartially", () => { }); }); }); + +describe("loadPluginConfig", () => { + let tempDirectory: string; + let opencodeConfigDirectory: string; + let projectOpencodeDirectory: string; + let originalOpencodeConfigDirectory: string | undefined; + + beforeEach(() => { + tempDirectory = fs.mkdtempSync(path.join(os.tmpdir(), "omo-config-test-")); + opencodeConfigDirectory = path.join(tempDirectory, "user-config"); + projectOpencodeDirectory = path.join(tempDirectory, ".opencode"); + fs.mkdirSync(opencodeConfigDirectory, { recursive: true }); + fs.mkdirSync(projectOpencodeDirectory, { recursive: true }); + + originalOpencodeConfigDirectory = process.env.OPENCODE_CONFIG_DIR; + process.env.OPENCODE_CONFIG_DIR = opencodeConfigDirectory; + + fs.writeFileSync(getLogFilePath(), "", "utf-8"); + }); + + afterEach(() => { + if (originalOpencodeConfigDirectory === undefined) { + delete process.env.OPENCODE_CONFIG_DIR; + } else { + process.env.OPENCODE_CONFIG_DIR = originalOpencodeConfigDirectory; + } + + fs.rmSync(tempDirectory, { recursive: true, force: true }); + }); + + describe("#given new config file exists", () => { + describe("#when loading config", () => { + it("#then finds oh-my-openagent.jsonc", () => { + const newConfigPath = path.join( + opencodeConfigDirectory, + `${CONFIG_BASENAME}.jsonc` + ); + fs.writeFileSync(newConfigPath, '{"disabled_hooks":["comment-checker"]}\n', "utf-8"); + + const config = loadPluginConfig(tempDirectory, {}); + + expect(config.disabled_hooks).toEqual(["comment-checker"]); + }); + }); + }); + + describe("#given only legacy config file exists", () => { + describe("#when loading config", () => { + it("#then falls back to oh-my-opencode.jsonc", () => { + const legacyConfigPath = path.join( + opencodeConfigDirectory, + `${LEGACY_CONFIG_BASENAME}.jsonc` + ); + fs.writeFileSync(legacyConfigPath, '{"disabled_hooks":["think-mode"]}\n', "utf-8"); + + const config = loadPluginConfig(tempDirectory, {}); + + expect(config.disabled_hooks).toEqual(["think-mode"]); + }); + + it("#then auto-renames oh-my-opencode.jsonc to oh-my-openagent.jsonc with backup", () => { + const legacyConfigPath = path.join( + opencodeConfigDirectory, + `${LEGACY_CONFIG_BASENAME}.jsonc` + ); + const legacyContent = '{"disabled_hooks":["session-recovery"]}\n'; + fs.writeFileSync(legacyConfigPath, legacyContent, "utf-8"); + + const config = loadPluginConfig(tempDirectory, {}); + + const newConfigPath = path.join( + opencodeConfigDirectory, + `${CONFIG_BASENAME}.jsonc` + ); + expect(config.disabled_hooks).toEqual(["session-recovery"]); + expect(fs.existsSync(newConfigPath)).toBe(true); + expect(fs.existsSync(legacyConfigPath)).toBe(false); + expect(fs.readFileSync(newConfigPath, "utf-8")).toBe(legacyContent); + + const backupFiles = fs + .readdirSync(opencodeConfigDirectory) + .filter((fileName) => fileName.startsWith(`${LEGACY_CONFIG_BASENAME}.jsonc.bak.`)); + expect(backupFiles.length).toBe(1); + expect( + fs.readFileSync(path.join(opencodeConfigDirectory, backupFiles[0]!), "utf-8") + ).toBe(legacyContent); + }); + }); + }); + + describe("#given both new and legacy config files exist", () => { + describe("#when loading config", () => { + it("#then new name wins and logs warning about old file", () => { + const newConfigPath = path.join( + opencodeConfigDirectory, + `${CONFIG_BASENAME}.jsonc` + ); + const legacyConfigPath = path.join( + opencodeConfigDirectory, + `${LEGACY_CONFIG_BASENAME}.jsonc` + ); + fs.writeFileSync(newConfigPath, '{"disabled_hooks":["comment-checker"]}\n', "utf-8"); + fs.writeFileSync(legacyConfigPath, '{"disabled_hooks":["think-mode"]}\n', "utf-8"); + + const config = loadPluginConfig(tempDirectory, {}); + + expect(config.disabled_hooks).toEqual(["comment-checker"]); + const logs = fs.readFileSync(getLogFilePath(), "utf-8"); + expect(logs).toContain("legacy config also exists"); + }); + }); + }); + + describe("#given legacy rename fails", () => { + describe("#when loading config", () => { + it("#then logs warning and continues loading legacy config", () => { + const legacyConfigPath = path.join( + opencodeConfigDirectory, + `${LEGACY_CONFIG_BASENAME}.jsonc` + ); + fs.writeFileSync(legacyConfigPath, '{"disabled_hooks":["hashline-read-enhancer"]}\n', "utf-8"); + fs.chmodSync(legacyConfigPath, 0o444); + + const renameError = new Error("EACCES: permission denied"); + const renameSpy = spyOn(fs, "renameSync").mockImplementation(() => { + throw renameError; + }); + + try { + const config = loadPluginConfig(tempDirectory, {}); + + expect(config.disabled_hooks).toEqual(["hashline-read-enhancer"]); + const logs = fs.readFileSync(getLogFilePath(), "utf-8"); + expect(logs).toContain("Failed to migrate legacy config path"); + } finally { + renameSpy.mockRestore(); + fs.chmodSync(legacyConfigPath, 0o644); + } + }); + }); + }); +}); diff --git a/src/plugin-config.ts b/src/plugin-config.ts index a1154b853..a9fb06708 100644 --- a/src/plugin-config.ts +++ b/src/plugin-config.ts @@ -1,14 +1,14 @@ -import { CONFIG_BASENAME } from "./shared/plugin-identity" +import { CONFIG_BASENAME, LEGACY_CONFIG_BASENAME } from "./shared/plugin-identity" import * as fs from "fs"; import * as path from "path"; import { OhMyOpenAgentConfigSchema, type OhMyOpenAgentConfig } from "./config"; +import { resolveConfigPathWithLegacyMigration } from "./plugin-config-path"; import { log, deepMerge, getOpenCodeConfigDir, addConfigLoadError, parseJsonc, - detectConfigFile, migrateConfigFile, } from "./shared"; @@ -161,22 +161,16 @@ export function loadPluginConfig( directory: string, ctx: unknown ): OhMyOpenAgentConfig { - // User-level config path - prefer .jsonc over .json const configDir = getOpenCodeConfigDir({ binary: "opencode" }); - const userBasePath = path.join(configDir, CONFIG_BASENAME); - const userDetected = detectConfigFile(userBasePath); - const userConfigPath = - userDetected.format !== "none" - ? userDetected.path - : userBasePath + ".json"; + const userConfigPath = resolveConfigPathWithLegacyMigration({ + preferredBasePath: path.join(configDir, CONFIG_BASENAME), + legacyBasePath: path.join(configDir, LEGACY_CONFIG_BASENAME), + }); - // Project-level config path - prefer .jsonc over .json - const projectBasePath = path.join(directory, ".opencode", CONFIG_BASENAME); - const projectDetected = detectConfigFile(projectBasePath); - const projectConfigPath = - projectDetected.format !== "none" - ? projectDetected.path - : projectBasePath + ".json"; + const projectConfigPath = resolveConfigPathWithLegacyMigration({ + preferredBasePath: path.join(directory, ".opencode", CONFIG_BASENAME), + legacyBasePath: path.join(directory, ".opencode", LEGACY_CONFIG_BASENAME), + }); // Load user config first (base) let config: OhMyOpenAgentConfig =