From 0c6907adc3c897155eb7a88471de93636dbbd3ef Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 4 Apr 2026 14:33:52 +0900 Subject: [PATCH] fix(config): use canonical path after legacy migration and make writes atomic --- src/plugin-config.test.ts | 73 +++++++++++++++++------- src/plugin-config.ts | 14 ++++- src/shared/index.ts | 1 + src/shared/migrate-legacy-config-file.ts | 15 +---- src/shared/migration/config-migration.ts | 3 +- src/shared/write-file-atomically.ts | 13 +++++ 6 files changed, 80 insertions(+), 39 deletions(-) create mode 100644 src/shared/write-file-atomically.ts diff --git a/src/plugin-config.test.ts b/src/plugin-config.test.ts index 19da20ece..9ebe7637b 100644 --- a/src/plugin-config.test.ts +++ b/src/plugin-config.test.ts @@ -8,6 +8,10 @@ import { OhMyOpenCodeConfigSchema, type OhMyOpenCodeConfig } from "./config"; const tempDirs: string[] = [] +function createConfig(config: Partial): OhMyOpenCodeConfig { + return OhMyOpenCodeConfigSchema.parse(config) +} + afterEach(() => { mock.restore() @@ -23,7 +27,7 @@ describe("mergeConfigs", () => { // then should deep merge categories, not override completely it("should deep merge categories from base and override", () => { - const base = { + const base = createConfig({ categories: { general: { model: "openai/gpt-5.4", @@ -33,9 +37,9 @@ describe("mergeConfigs", () => { model: "anthropic/claude-haiku-4-5", }, }, - } as OhMyOpenCodeConfig; + }); - const override = { + const override = createConfig({ categories: { general: { temperature: 0.3, @@ -44,7 +48,7 @@ describe("mergeConfigs", () => { model: "google/gemini-3.1-pro", }, }, - } as unknown as OhMyOpenCodeConfig; + }); const result = mergeConfigs(base, override); @@ -59,15 +63,15 @@ describe("mergeConfigs", () => { }); it("should preserve base categories when override has no categories", () => { - const base: OhMyOpenCodeConfig = { + const base = createConfig({ categories: { general: { model: "openai/gpt-5.4", }, }, - }; + }); - const override: OhMyOpenCodeConfig = {}; + const override = createConfig({}); const result = mergeConfigs(base, override); @@ -75,15 +79,15 @@ describe("mergeConfigs", () => { }); it("should use override categories when base has no categories", () => { - const base: OhMyOpenCodeConfig = {}; + const base = createConfig({}); - const override: OhMyOpenCodeConfig = { + const override = createConfig({ categories: { general: { model: "openai/gpt-5.4", }, }, - }; + }); const result = mergeConfigs(base, override); @@ -93,18 +97,18 @@ describe("mergeConfigs", () => { describe("existing behavior preservation", () => { it("should deep merge agents", () => { - const base: OhMyOpenCodeConfig = { + const base = createConfig({ agents: { oracle: { model: "openai/gpt-5.4" }, }, - }; + }); - const override: OhMyOpenCodeConfig = { + const override = createConfig({ agents: { oracle: { temperature: 0.5 }, explore: { model: "anthropic/claude-haiku-4-5" }, }, - }; + }); const result = mergeConfigs(base, override); @@ -114,13 +118,13 @@ describe("mergeConfigs", () => { }); it("should merge disabled arrays without duplicates", () => { - const base: OhMyOpenCodeConfig = { + const base = createConfig({ disabled_hooks: ["comment-checker", "think-mode"], - }; + }); - const override: OhMyOpenCodeConfig = { + const override = createConfig({ disabled_hooks: ["think-mode", "session-recovery"], - }; + }); const result = mergeConfigs(base, override); @@ -131,13 +135,13 @@ describe("mergeConfigs", () => { }); it("should union disabled_tools from base and override without duplicates", () => { - const base: OhMyOpenCodeConfig = { + const base = createConfig({ disabled_tools: ["todowrite", "interactive_bash"], - }; + }); - const override: OhMyOpenCodeConfig = { + const override = createConfig({ disabled_tools: ["interactive_bash", "look_at"], - }; + }); const result = mergeConfigs(base, override); @@ -350,4 +354,29 @@ describe("loadPluginConfig", () => { expect(readFileSync(canonicalConfigPath, "utf-8")).toContain('"openai/gpt-5.4"') expect(reloadedConfig.agents?.oracle?.model).toBe("openai/gpt-5.4") }) + + it("should load migrated legacy project config on the first load", () => { + // given + const rootDir = mkdtempSync(join(tmpdir(), "omo-plugin-config-first-load-")) + 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 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 + const config = loadPluginConfig(projectDir, {}) + + // then + expect(existsSync(legacyConfigPath)).toBe(false) + expect(existsSync(canonicalConfigPath)).toBe(true) + expect(config.agents?.oracle?.model).toBe("openai/gpt-5.4") + }) }) diff --git a/src/plugin-config.ts b/src/plugin-config.ts index fad949375..d6547f814 100644 --- a/src/plugin-config.ts +++ b/src/plugin-config.ts @@ -11,7 +11,7 @@ import { migrateConfigFile, } from "./shared"; import { migrateLegacyConfigFile } from "./shared/migrate-legacy-config-file"; -import { LEGACY_CONFIG_BASENAME } from "./shared/plugin-identity"; +import { CONFIG_BASENAME, LEGACY_CONFIG_BASENAME } from "./shared/plugin-identity"; const PARTIAL_STRING_ARRAY_KEYS = new Set([ "disabled_mcps", @@ -172,7 +172,7 @@ export function loadPluginConfig( // User-level config path - prefer .jsonc over .json const configDir = getOpenCodeConfigDir({ binary: "opencode" }); const userDetected = detectPluginConfigFile(configDir); - const userConfigPath = + let userConfigPath = userDetected.format !== "none" ? userDetected.path : path.join(configDir, "oh-my-opencode.json"); @@ -187,12 +187,16 @@ export function loadPluginConfig( // Auto-copy legacy config file to canonical name if needed if (userDetected.format !== "none" && path.basename(userDetected.path).startsWith(LEGACY_CONFIG_BASENAME)) { migrateLegacyConfigFile(userDetected.path); + userConfigPath = path.join( + path.dirname(userDetected.path), + `${CONFIG_BASENAME}${path.extname(userDetected.path)}` + ); } // Project-level config path - prefer .jsonc over .json const projectBasePath = path.join(directory, ".opencode"); const projectDetected = detectPluginConfigFile(projectBasePath); - const projectConfigPath = + let projectConfigPath = projectDetected.format !== "none" ? projectDetected.path : path.join(projectBasePath, "oh-my-opencode.json"); @@ -207,6 +211,10 @@ export function loadPluginConfig( // Auto-copy legacy project config file to canonical name if needed if (projectDetected.format !== "none" && path.basename(projectDetected.path).startsWith(LEGACY_CONFIG_BASENAME)) { migrateLegacyConfigFile(projectDetected.path); + projectConfigPath = path.join( + path.dirname(projectDetected.path), + `${CONFIG_BASENAME}${path.extname(projectDetected.path)}` + ); } // Load user config first (base). Parse empty config through Zod to apply field defaults. diff --git a/src/shared/index.ts b/src/shared/index.ts index 32f428cc8..fff73f3fb 100644 --- a/src/shared/index.ts +++ b/src/shared/index.ts @@ -28,6 +28,7 @@ export * from "./permission-compat" export * from "./external-plugin-detector" export * from "./zip-extractor" export * from "./binary-downloader" +export * from "./write-file-atomically" export * from "./agent-variant" export * from "./session-cursor" export * from "./shell-env" diff --git a/src/shared/migrate-legacy-config-file.ts b/src/shared/migrate-legacy-config-file.ts index ea7ad30a9..03c0c6241 100644 --- a/src/shared/migrate-legacy-config-file.ts +++ b/src/shared/migrate-legacy-config-file.ts @@ -1,8 +1,9 @@ -import { closeSync, existsSync, fsyncSync, openSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs" +import { existsSync, readFileSync, renameSync, rmSync } from "node:fs" import { join, dirname, basename } from "node:path" import { log } from "./logger" import { CONFIG_BASENAME, LEGACY_CONFIG_BASENAME } from "./plugin-identity" +import { writeFileAtomically } from "./write-file-atomically" function buildCanonicalPath(legacyPath: string): string { const dir = dirname(legacyPath) @@ -10,18 +11,6 @@ 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` diff --git a/src/shared/migration/config-migration.ts b/src/shared/migration/config-migration.ts index abb90bccd..58a4b4b33 100644 --- a/src/shared/migration/config-migration.ts +++ b/src/shared/migration/config-migration.ts @@ -1,5 +1,6 @@ import * as fs from "fs" import { log } from "../logger" +import { writeFileAtomically } from "../write-file-atomically" import { AGENT_NAME_MAP, migrateAgentNames } from "./agent-names" import { migrateHookNames } from "./hook-names" import { migrateModelVersions } from "./model-versions" @@ -123,7 +124,7 @@ export function migrateConfigFile( let writeSucceeded = false try { - fs.writeFileSync(configPath, JSON.stringify(copy, null, 2) + "\n", "utf-8") + writeFileAtomically(configPath, JSON.stringify(copy, null, 2) + "\n") writeSucceeded = true } catch (err) { log(`Failed to write migrated config to ${configPath}:`, err) diff --git a/src/shared/write-file-atomically.ts b/src/shared/write-file-atomically.ts new file mode 100644 index 000000000..81bcc5249 --- /dev/null +++ b/src/shared/write-file-atomically.ts @@ -0,0 +1,13 @@ +import { closeSync, fsyncSync, openSync, renameSync, writeFileSync } from "node:fs" + +export 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) +}