fix(config): fall back to legacy path when migration fails and use canonical basename everywhere (#3133)
Root cause: loadPluginConfig() unconditionally switched userConfigPath to the
canonical name after calling migrateLegacyConfigFile(), even when migration
failed (e.g. file lock on Windows, permission denied). This left the config
path pointing to a non-existent file, so the plugin config silently loaded
as empty defaults.
Additionally, several fallback/default paths were hardcoded to the legacy
'oh-my-opencode' basename instead of using CONFIG_BASENAME ('oh-my-openagent'),
causing CLI config commands (writeOmoConfig, detectCurrentConfig) to write to
the wrong filename.
Changes:
- plugin-config.ts: check migrateLegacyConfigFile() return value; only switch
to canonical path if migration succeeded OR the canonical file already exists
- opencode-config-dir.ts: use CONFIG_BASENAME for omoConfig path in
getOpenCodeConfigPaths()
- config-context.ts: getOmoConfigPath() now uses detectPluginConfigFile() to
find whichever name variant actually exists on disk
- plugin-config.ts: default fallback paths use CONFIG_BASENAME instead of
hardcoded legacy name
- Added test: loadPluginConfig still loads config when migration fails
(read-only directory simulation)
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
import { getOpenCodeConfigPaths } from "../../shared"
|
||||
import { getOpenCodeConfigPaths, detectPluginConfigFile } from "../../shared"
|
||||
import type {
|
||||
OpenCodeBinaryType,
|
||||
OpenCodeConfigPaths,
|
||||
@@ -42,5 +42,8 @@ export function getConfigJsonc(): string {
|
||||
}
|
||||
|
||||
export function getOmoConfigPath(): string {
|
||||
const configDir = getConfigContext().paths.configDir
|
||||
const detected = detectPluginConfigFile(configDir)
|
||||
if (detected.format !== "none") return detected.path
|
||||
return getConfigContext().paths.omoConfig
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { afterEach, describe, expect, it, mock, spyOn } from "bun:test";
|
||||
import { existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"
|
||||
import { chmodSync, existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"
|
||||
import { tmpdir } from "node:os"
|
||||
import { join } from "node:path"
|
||||
import * as shared from "./shared"
|
||||
@@ -355,6 +355,42 @@ describe("loadPluginConfig", () => {
|
||||
expect(reloadedConfig.agents?.oracle?.model).toBe("openai/gpt-5.4")
|
||||
})
|
||||
|
||||
it("should still load config from legacy path when migration fails", () => {
|
||||
// given - legacy config exists but canonical path is not writable
|
||||
const rootDir = mkdtempSync(join(tmpdir(), "omo-plugin-config-fail-"))
|
||||
const userConfigDir = join(rootDir, "user-config")
|
||||
const projectDir = join(rootDir, "project")
|
||||
const projectConfigDir = join(projectDir, ".opencode")
|
||||
const legacyConfigPath = join(projectConfigDir, "oh-my-opencode.json")
|
||||
|
||||
tempDirs.push(rootDir)
|
||||
mkdirSync(userConfigDir, { recursive: true })
|
||||
mkdirSync(projectConfigDir, { recursive: true })
|
||||
writeFileSync(legacyConfigPath, JSON.stringify({ agents: { oracle: { model: "openai/gpt-5.4" } } }))
|
||||
|
||||
// Make the directory read-only so migration write fails
|
||||
// (simulates Windows file lock / permission issues)
|
||||
if (process.platform !== "win32") {
|
||||
chmodSync(projectConfigDir, 0o555)
|
||||
}
|
||||
|
||||
spyOn(shared, "getOpenCodeConfigDir").mockReturnValue(userConfigDir)
|
||||
|
||||
// when
|
||||
let config: OhMyOpenCodeConfig
|
||||
try {
|
||||
config = loadPluginConfig(projectDir, {})
|
||||
} finally {
|
||||
// Restore permissions for cleanup
|
||||
if (process.platform !== "win32") {
|
||||
chmodSync(projectConfigDir, 0o755)
|
||||
}
|
||||
}
|
||||
|
||||
// then - should still load the config from legacy path
|
||||
expect(config.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-"))
|
||||
|
||||
+16
-6
@@ -175,7 +175,7 @@ export function loadPluginConfig(
|
||||
let userConfigPath =
|
||||
userDetected.format !== "none"
|
||||
? userDetected.path
|
||||
: path.join(configDir, "oh-my-opencode.json");
|
||||
: path.join(configDir, `${CONFIG_BASENAME}.json`);
|
||||
|
||||
if (userDetected.legacyPath) {
|
||||
log("Canonical plugin config detected alongside legacy config. Remove the legacy file to avoid confusion.", {
|
||||
@@ -186,11 +186,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(
|
||||
const migrated = migrateLegacyConfigFile(userDetected.path);
|
||||
const canonicalPath = path.join(
|
||||
path.dirname(userDetected.path),
|
||||
`${CONFIG_BASENAME}${path.extname(userDetected.path)}`
|
||||
);
|
||||
// Only switch to canonical path if migration succeeded OR canonical file already exists
|
||||
if (migrated || fs.existsSync(canonicalPath)) {
|
||||
userConfigPath = canonicalPath;
|
||||
}
|
||||
// Otherwise keep loading from the legacy path that was detected
|
||||
}
|
||||
|
||||
// Project-level config path - prefer .jsonc over .json
|
||||
@@ -199,7 +204,7 @@ export function loadPluginConfig(
|
||||
let projectConfigPath =
|
||||
projectDetected.format !== "none"
|
||||
? projectDetected.path
|
||||
: path.join(projectBasePath, "oh-my-opencode.json");
|
||||
: path.join(projectBasePath, `${CONFIG_BASENAME}.json`);
|
||||
|
||||
if (projectDetected.legacyPath) {
|
||||
log("Canonical plugin config detected alongside legacy config. Remove the legacy file to avoid confusion.", {
|
||||
@@ -210,11 +215,16 @@ 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(
|
||||
const projectMigrated = migrateLegacyConfigFile(projectDetected.path);
|
||||
const canonicalProjectPath = path.join(
|
||||
path.dirname(projectDetected.path),
|
||||
`${CONFIG_BASENAME}${path.extname(projectDetected.path)}`
|
||||
);
|
||||
// Only switch to canonical path if migration succeeded OR canonical file already exists
|
||||
if (projectMigrated || fs.existsSync(canonicalProjectPath)) {
|
||||
projectConfigPath = canonicalProjectPath;
|
||||
}
|
||||
// Otherwise keep loading from the legacy path that was detected
|
||||
}
|
||||
|
||||
// Load user config first (base). Parse empty config through Zod to apply field defaults.
|
||||
|
||||
@@ -289,7 +289,7 @@ describe("opencode-config-dir", () => {
|
||||
expect(paths.configJson).toBe(join(expectedDir, "opencode.json"))
|
||||
expect(paths.configJsonc).toBe(join(expectedDir, "opencode.jsonc"))
|
||||
expect(paths.packageJson).toBe(join(expectedDir, "package.json"))
|
||||
expect(paths.omoConfig).toBe(join(expectedDir, "oh-my-opencode.json"))
|
||||
expect(paths.omoConfig).toBe(join(expectedDir, "oh-my-openagent.json"))
|
||||
})
|
||||
|
||||
test("returns all config paths for desktop binary", () => {
|
||||
@@ -305,7 +305,7 @@ describe("opencode-config-dir", () => {
|
||||
expect(paths.configJson).toBe(join(expectedDir, "opencode.json"))
|
||||
expect(paths.configJsonc).toBe(join(expectedDir, "opencode.jsonc"))
|
||||
expect(paths.packageJson).toBe(join(expectedDir, "package.json"))
|
||||
expect(paths.omoConfig).toBe(join(expectedDir, "oh-my-opencode.json"))
|
||||
expect(paths.omoConfig).toBe(join(expectedDir, "oh-my-openagent.json"))
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -2,6 +2,8 @@ import { existsSync, realpathSync } from "node:fs"
|
||||
import { homedir } from "node:os"
|
||||
import { join, resolve, win32 } from "node:path"
|
||||
|
||||
import { CONFIG_BASENAME } from "./plugin-identity"
|
||||
|
||||
import type {
|
||||
OpenCodeBinaryType,
|
||||
OpenCodeConfigDirOptions,
|
||||
@@ -97,7 +99,7 @@ export function getOpenCodeConfigPaths(options: OpenCodeConfigDirOptions): OpenC
|
||||
configJson: join(configDir, "opencode.json"),
|
||||
configJsonc: join(configDir, "opencode.jsonc"),
|
||||
packageJson: join(configDir, "package.json"),
|
||||
omoConfig: join(configDir, "oh-my-opencode.json"),
|
||||
omoConfig: join(configDir, `${CONFIG_BASENAME}.json`),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user