Merge pull request #3111 from code-yeongyu/fix/prepublish-config-regression
fix(config): use canonical path after legacy migration and make writes atomic
This commit is contained in:
+51
-22
@@ -8,6 +8,10 @@ import { OhMyOpenCodeConfigSchema, type OhMyOpenCodeConfig } from "./config";
|
|||||||
|
|
||||||
const tempDirs: string[] = []
|
const tempDirs: string[] = []
|
||||||
|
|
||||||
|
function createConfig(config: Partial<OhMyOpenCodeConfig>): OhMyOpenCodeConfig {
|
||||||
|
return OhMyOpenCodeConfigSchema.parse(config)
|
||||||
|
}
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
mock.restore()
|
mock.restore()
|
||||||
|
|
||||||
@@ -23,7 +27,7 @@ describe("mergeConfigs", () => {
|
|||||||
// then should deep merge categories, not override completely
|
// then should deep merge categories, not override completely
|
||||||
|
|
||||||
it("should deep merge categories from base and override", () => {
|
it("should deep merge categories from base and override", () => {
|
||||||
const base = {
|
const base = createConfig({
|
||||||
categories: {
|
categories: {
|
||||||
general: {
|
general: {
|
||||||
model: "openai/gpt-5.4",
|
model: "openai/gpt-5.4",
|
||||||
@@ -33,9 +37,9 @@ describe("mergeConfigs", () => {
|
|||||||
model: "anthropic/claude-haiku-4-5",
|
model: "anthropic/claude-haiku-4-5",
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
} as OhMyOpenCodeConfig;
|
});
|
||||||
|
|
||||||
const override = {
|
const override = createConfig({
|
||||||
categories: {
|
categories: {
|
||||||
general: {
|
general: {
|
||||||
temperature: 0.3,
|
temperature: 0.3,
|
||||||
@@ -44,7 +48,7 @@ describe("mergeConfigs", () => {
|
|||||||
model: "google/gemini-3.1-pro",
|
model: "google/gemini-3.1-pro",
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
} as unknown as OhMyOpenCodeConfig;
|
});
|
||||||
|
|
||||||
const result = mergeConfigs(base, override);
|
const result = mergeConfigs(base, override);
|
||||||
|
|
||||||
@@ -59,15 +63,15 @@ describe("mergeConfigs", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("should preserve base categories when override has no categories", () => {
|
it("should preserve base categories when override has no categories", () => {
|
||||||
const base: OhMyOpenCodeConfig = {
|
const base = createConfig({
|
||||||
categories: {
|
categories: {
|
||||||
general: {
|
general: {
|
||||||
model: "openai/gpt-5.4",
|
model: "openai/gpt-5.4",
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
};
|
});
|
||||||
|
|
||||||
const override: OhMyOpenCodeConfig = {};
|
const override = createConfig({});
|
||||||
|
|
||||||
const result = mergeConfigs(base, override);
|
const result = mergeConfigs(base, override);
|
||||||
|
|
||||||
@@ -75,15 +79,15 @@ describe("mergeConfigs", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("should use override categories when base has no categories", () => {
|
it("should use override categories when base has no categories", () => {
|
||||||
const base: OhMyOpenCodeConfig = {};
|
const base = createConfig({});
|
||||||
|
|
||||||
const override: OhMyOpenCodeConfig = {
|
const override = createConfig({
|
||||||
categories: {
|
categories: {
|
||||||
general: {
|
general: {
|
||||||
model: "openai/gpt-5.4",
|
model: "openai/gpt-5.4",
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
};
|
});
|
||||||
|
|
||||||
const result = mergeConfigs(base, override);
|
const result = mergeConfigs(base, override);
|
||||||
|
|
||||||
@@ -93,18 +97,18 @@ describe("mergeConfigs", () => {
|
|||||||
|
|
||||||
describe("existing behavior preservation", () => {
|
describe("existing behavior preservation", () => {
|
||||||
it("should deep merge agents", () => {
|
it("should deep merge agents", () => {
|
||||||
const base: OhMyOpenCodeConfig = {
|
const base = createConfig({
|
||||||
agents: {
|
agents: {
|
||||||
oracle: { model: "openai/gpt-5.4" },
|
oracle: { model: "openai/gpt-5.4" },
|
||||||
},
|
},
|
||||||
};
|
});
|
||||||
|
|
||||||
const override: OhMyOpenCodeConfig = {
|
const override = createConfig({
|
||||||
agents: {
|
agents: {
|
||||||
oracle: { temperature: 0.5 },
|
oracle: { temperature: 0.5 },
|
||||||
explore: { model: "anthropic/claude-haiku-4-5" },
|
explore: { model: "anthropic/claude-haiku-4-5" },
|
||||||
},
|
},
|
||||||
};
|
});
|
||||||
|
|
||||||
const result = mergeConfigs(base, override);
|
const result = mergeConfigs(base, override);
|
||||||
|
|
||||||
@@ -114,13 +118,13 @@ describe("mergeConfigs", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("should merge disabled arrays without duplicates", () => {
|
it("should merge disabled arrays without duplicates", () => {
|
||||||
const base: OhMyOpenCodeConfig = {
|
const base = createConfig({
|
||||||
disabled_hooks: ["comment-checker", "think-mode"],
|
disabled_hooks: ["comment-checker", "think-mode"],
|
||||||
};
|
});
|
||||||
|
|
||||||
const override: OhMyOpenCodeConfig = {
|
const override = createConfig({
|
||||||
disabled_hooks: ["think-mode", "session-recovery"],
|
disabled_hooks: ["think-mode", "session-recovery"],
|
||||||
};
|
});
|
||||||
|
|
||||||
const result = mergeConfigs(base, override);
|
const result = mergeConfigs(base, override);
|
||||||
|
|
||||||
@@ -131,13 +135,13 @@ describe("mergeConfigs", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("should union disabled_tools from base and override without duplicates", () => {
|
it("should union disabled_tools from base and override without duplicates", () => {
|
||||||
const base: OhMyOpenCodeConfig = {
|
const base = createConfig({
|
||||||
disabled_tools: ["todowrite", "interactive_bash"],
|
disabled_tools: ["todowrite", "interactive_bash"],
|
||||||
};
|
});
|
||||||
|
|
||||||
const override: OhMyOpenCodeConfig = {
|
const override = createConfig({
|
||||||
disabled_tools: ["interactive_bash", "look_at"],
|
disabled_tools: ["interactive_bash", "look_at"],
|
||||||
};
|
});
|
||||||
|
|
||||||
const result = mergeConfigs(base, override);
|
const result = mergeConfigs(base, override);
|
||||||
|
|
||||||
@@ -350,4 +354,29 @@ describe("loadPluginConfig", () => {
|
|||||||
expect(readFileSync(canonicalConfigPath, "utf-8")).toContain('"openai/gpt-5.4"')
|
expect(readFileSync(canonicalConfigPath, "utf-8")).toContain('"openai/gpt-5.4"')
|
||||||
expect(reloadedConfig.agents?.oracle?.model).toBe("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")
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
+11
-3
@@ -11,7 +11,7 @@ import {
|
|||||||
migrateConfigFile,
|
migrateConfigFile,
|
||||||
} from "./shared";
|
} from "./shared";
|
||||||
import { migrateLegacyConfigFile } from "./shared/migrate-legacy-config-file";
|
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([
|
const PARTIAL_STRING_ARRAY_KEYS = new Set([
|
||||||
"disabled_mcps",
|
"disabled_mcps",
|
||||||
@@ -172,7 +172,7 @@ export function loadPluginConfig(
|
|||||||
// User-level config path - prefer .jsonc over .json
|
// User-level config path - prefer .jsonc over .json
|
||||||
const configDir = getOpenCodeConfigDir({ binary: "opencode" });
|
const configDir = getOpenCodeConfigDir({ binary: "opencode" });
|
||||||
const userDetected = detectPluginConfigFile(configDir);
|
const userDetected = detectPluginConfigFile(configDir);
|
||||||
const userConfigPath =
|
let userConfigPath =
|
||||||
userDetected.format !== "none"
|
userDetected.format !== "none"
|
||||||
? userDetected.path
|
? userDetected.path
|
||||||
: path.join(configDir, "oh-my-opencode.json");
|
: path.join(configDir, "oh-my-opencode.json");
|
||||||
@@ -187,12 +187,16 @@ export function loadPluginConfig(
|
|||||||
// Auto-copy legacy config file to canonical name if needed
|
// Auto-copy legacy config file to canonical name if needed
|
||||||
if (userDetected.format !== "none" && path.basename(userDetected.path).startsWith(LEGACY_CONFIG_BASENAME)) {
|
if (userDetected.format !== "none" && path.basename(userDetected.path).startsWith(LEGACY_CONFIG_BASENAME)) {
|
||||||
migrateLegacyConfigFile(userDetected.path);
|
migrateLegacyConfigFile(userDetected.path);
|
||||||
|
userConfigPath = path.join(
|
||||||
|
path.dirname(userDetected.path),
|
||||||
|
`${CONFIG_BASENAME}${path.extname(userDetected.path)}`
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Project-level config path - prefer .jsonc over .json
|
// Project-level config path - prefer .jsonc over .json
|
||||||
const projectBasePath = path.join(directory, ".opencode");
|
const projectBasePath = path.join(directory, ".opencode");
|
||||||
const projectDetected = detectPluginConfigFile(projectBasePath);
|
const projectDetected = detectPluginConfigFile(projectBasePath);
|
||||||
const projectConfigPath =
|
let projectConfigPath =
|
||||||
projectDetected.format !== "none"
|
projectDetected.format !== "none"
|
||||||
? projectDetected.path
|
? projectDetected.path
|
||||||
: path.join(projectBasePath, "oh-my-opencode.json");
|
: 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
|
// Auto-copy legacy project config file to canonical name if needed
|
||||||
if (projectDetected.format !== "none" && path.basename(projectDetected.path).startsWith(LEGACY_CONFIG_BASENAME)) {
|
if (projectDetected.format !== "none" && path.basename(projectDetected.path).startsWith(LEGACY_CONFIG_BASENAME)) {
|
||||||
migrateLegacyConfigFile(projectDetected.path);
|
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.
|
// Load user config first (base). Parse empty config through Zod to apply field defaults.
|
||||||
|
|||||||
@@ -28,6 +28,7 @@ export * from "./permission-compat"
|
|||||||
export * from "./external-plugin-detector"
|
export * from "./external-plugin-detector"
|
||||||
export * from "./zip-extractor"
|
export * from "./zip-extractor"
|
||||||
export * from "./binary-downloader"
|
export * from "./binary-downloader"
|
||||||
|
export * from "./write-file-atomically"
|
||||||
export * from "./agent-variant"
|
export * from "./agent-variant"
|
||||||
export * from "./session-cursor"
|
export * from "./session-cursor"
|
||||||
export * from "./shell-env"
|
export * from "./shell-env"
|
||||||
|
|||||||
@@ -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 { join, dirname, basename } from "node:path"
|
||||||
|
|
||||||
import { log } from "./logger"
|
import { log } from "./logger"
|
||||||
import { CONFIG_BASENAME, LEGACY_CONFIG_BASENAME } from "./plugin-identity"
|
import { CONFIG_BASENAME, LEGACY_CONFIG_BASENAME } from "./plugin-identity"
|
||||||
|
import { writeFileAtomically } from "./write-file-atomically"
|
||||||
|
|
||||||
function buildCanonicalPath(legacyPath: string): string {
|
function buildCanonicalPath(legacyPath: string): string {
|
||||||
const dir = dirname(legacyPath)
|
const dir = dirname(legacyPath)
|
||||||
@@ -10,18 +11,6 @@ function buildCanonicalPath(legacyPath: string): string {
|
|||||||
return join(dir, `${CONFIG_BASENAME}${ext}`)
|
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 {
|
function archiveLegacyConfigFile(legacyPath: string): boolean {
|
||||||
const backupPath = `${legacyPath}.bak`
|
const backupPath = `${legacyPath}.bak`
|
||||||
|
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import * as fs from "fs"
|
import * as fs from "fs"
|
||||||
import { log } from "../logger"
|
import { log } from "../logger"
|
||||||
|
import { writeFileAtomically } from "../write-file-atomically"
|
||||||
import { AGENT_NAME_MAP, migrateAgentNames } from "./agent-names"
|
import { AGENT_NAME_MAP, migrateAgentNames } from "./agent-names"
|
||||||
import { migrateHookNames } from "./hook-names"
|
import { migrateHookNames } from "./hook-names"
|
||||||
import { migrateModelVersions } from "./model-versions"
|
import { migrateModelVersions } from "./model-versions"
|
||||||
@@ -123,7 +124,7 @@ export function migrateConfigFile(
|
|||||||
|
|
||||||
let writeSucceeded = false
|
let writeSucceeded = false
|
||||||
try {
|
try {
|
||||||
fs.writeFileSync(configPath, JSON.stringify(copy, null, 2) + "\n", "utf-8")
|
writeFileAtomically(configPath, JSON.stringify(copy, null, 2) + "\n")
|
||||||
writeSucceeded = true
|
writeSucceeded = true
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
log(`Failed to write migrated config to ${configPath}:`, err)
|
log(`Failed to write migrated config to ${configPath}:`, err)
|
||||||
|
|||||||
@@ -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)
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user