From 13ac849db5c378ed04d02d644006f01e70db31b6 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sun, 3 May 2026 11:21:34 -0400 Subject: [PATCH] refactor(config+core): drop ConfigPaths.readFile, add AppFileSystem.readFileStringSafe, flatten TuiConfig.loadState (#25602) --- packages/core/src/filesystem.ts | 8 ++ .../core/test/filesystem/filesystem.test.ts | 28 ++++ .../opencode/src/cli/cmd/tui/config/tui.ts | 121 ++++++++++-------- packages/opencode/src/config/config.ts | 10 +- packages/opencode/src/config/paths.ts | 10 -- packages/opencode/test/config/tui.test.ts | 40 ++++++ 6 files changed, 147 insertions(+), 70 deletions(-) diff --git a/packages/core/src/filesystem.ts b/packages/core/src/filesystem.ts index 44346be8f9..8a1cc3a08f 100644 --- a/packages/core/src/filesystem.ts +++ b/packages/core/src/filesystem.ts @@ -24,6 +24,7 @@ export namespace AppFileSystem { readonly isDir: (path: string) => Effect.Effect readonly isFile: (path: string) => Effect.Effect readonly existsSafe: (path: string) => Effect.Effect + readonly readFileStringSafe: (path: string) => Effect.Effect readonly readJson: (path: string) => Effect.Effect readonly writeJson: (path: string, data: unknown, mode?: number) => Effect.Effect readonly ensureDir: (path: string) => Effect.Effect @@ -47,6 +48,12 @@ export namespace AppFileSystem { return yield* fs.exists(path).pipe(Effect.orElseSucceed(() => false)) }) + const readFileStringSafe = Effect.fn("FileSystem.readFileStringSafe")(function* (path: string) { + return yield* fs + .readFileString(path) + .pipe(Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(undefined))) + }) + const isDir = Effect.fn("FileSystem.isDir")(function* (path: string) { const info = yield* fs.stat(path).pipe(Effect.catch(() => Effect.void)) return info?.type === "Directory" @@ -163,6 +170,7 @@ export namespace AppFileSystem { return Service.of({ ...fs, existsSafe, + readFileStringSafe, isDir, isFile, readDirectoryEntries, diff --git a/packages/core/test/filesystem/filesystem.test.ts b/packages/core/test/filesystem/filesystem.test.ts index b77f4e356f..1d9405333d 100644 --- a/packages/core/test/filesystem/filesystem.test.ts +++ b/packages/core/test/filesystem/filesystem.test.ts @@ -65,6 +65,34 @@ describe("AppFileSystem", () => { ) }) + describe("readFileStringSafe", () => { + it( + "returns file contents when file exists", + Effect.gen(function* () { + const fs = yield* AppFileSystem.Service + const filesys = yield* FileSystem.FileSystem + const tmp = yield* filesys.makeTempDirectoryScoped() + const file = path.join(tmp, "exists.txt") + yield* filesys.writeFileString(file, "hello") + + const result = yield* fs.readFileStringSafe(file) + expect(result).toBe("hello") + }), + ) + + it( + "returns undefined for missing file (NotFound)", + Effect.gen(function* () { + const fs = yield* AppFileSystem.Service + const filesys = yield* FileSystem.FileSystem + const tmp = yield* filesys.makeTempDirectoryScoped() + + const result = yield* fs.readFileStringSafe(path.join(tmp, "does-not-exist.txt")) + expect(result).toBeUndefined() + }), + ) + }) + describe("readJson / writeJson", () => { it( "round-trips JSON data", diff --git a/packages/opencode/src/cli/cmd/tui/config/tui.ts b/packages/opencode/src/cli/cmd/tui/config/tui.ts index fbedcccc1b..e9824a09d6 100644 --- a/packages/opencode/src/cli/cmd/tui/config/tui.ts +++ b/packages/opencode/src/cli/cmd/tui/config/tui.ts @@ -68,29 +68,73 @@ function normalize(raw: Record) { } } -async function resolvePlugins(config: Info, configFilepath: string) { - if (!config.plugin) return config - for (let i = 0; i < config.plugin.length; i++) { - config.plugin[i] = await ConfigPlugin.resolvePluginSpec(config.plugin[i], configFilepath) - } - return config -} - -async function mergeFile(acc: Acc, file: string, ctx: { directory: string }) { - const data = await loadFile(file) - acc.result = mergeDeep(acc.result, data) - if (!data.plugin?.length) return - - const scope = pluginScope(file, ctx) - const plugins = ConfigPlugin.deduplicatePluginOrigins([ - ...(acc.result.plugin_origins ?? []), - ...data.plugin.map((spec) => ({ spec, scope, source: file })), - ]) - acc.result.plugin = plugins.map((item) => item.spec) - acc.result.plugin_origins = plugins -} - const loadState = Effect.fn("TuiConfig.loadState")(function* (ctx: { directory: string }) { + const afs = yield* AppFileSystem.Service + + const resolvePlugins = (config: Info, configFilepath: string): Effect.Effect => + Effect.gen(function* () { + const plugins = config.plugin + if (!plugins) return config + for (let i = 0; i < plugins.length; i++) { + plugins[i] = yield* Effect.promise(() => ConfigPlugin.resolvePluginSpec(plugins[i], configFilepath)) + } + return config + }) + + const load = (text: string, configFilepath: string): Effect.Effect => + Effect.gen(function* () { + const expanded = yield* Effect.promise(() => + ConfigVariable.substitute({ text, type: "path", path: configFilepath, missing: "empty" }), + ) + const data = ConfigParse.jsonc(expanded, configFilepath) + if (!isRecord(data)) return {} as Info + // Flatten a nested "tui" key so users who wrote `{ "tui": { ... } }` inside tui.json + // (mirroring the old opencode.json shape) still get their settings applied. + const validated = ConfigParse.schema(Info, normalize(data), configFilepath) + return yield* resolvePlugins(validated, configFilepath) + }).pipe( + // catchCause (not tapErrorCause + orElseSucceed) because ConfigParse.jsonc/.schema + // can sync-throw — those become defects, which orElseSucceed wouldn't catch. + Effect.catchCause((cause) => + Effect.sync(() => { + log.warn("invalid tui config", { path: configFilepath, cause }) + return {} as Info + }), + ), + ) + + const loadFile = (filepath: string): Effect.Effect => + Effect.gen(function* () { + // Silent-swallow non-NotFound read errors (perms, EISDIR, IO) → log + skip. + // Matches how parse/schema/plugin failures in load() are handled — every + // broken-config path degrades gracefully rather than crashing TUI startup. + const text = yield* afs.readFileStringSafe(filepath).pipe( + Effect.catchCause((cause) => + Effect.sync(() => { + log.warn("failed to read tui config", { path: filepath, cause }) + return undefined + }), + ), + ) + if (!text) return {} as Info + return yield* load(text, filepath) + }) + + const mergeFile = (acc: Acc, file: string) => + Effect.gen(function* () { + const data = yield* loadFile(file) + acc.result = mergeDeep(acc.result, data) + if (!data.plugin?.length) return + + const scope = pluginScope(file, ctx) + const plugins = ConfigPlugin.deduplicatePluginOrigins([ + ...(acc.result.plugin_origins ?? []), + ...data.plugin.map((spec) => ({ spec, scope, source: file })), + ]) + acc.result.plugin = plugins.map((item) => item.spec) + acc.result.plugin_origins = plugins + }) + // Every config dir we may read from: global config dir, any `.opencode` // folders between cwd and home, and OPENCODE_CONFIG_DIR. const directories = yield* ConfigPaths.directories(ctx.directory) @@ -104,19 +148,19 @@ const loadState = Effect.fn("TuiConfig.loadState")(function* (ctx: { directory: // 1. Global tui config (lowest precedence). for (const file of ConfigPaths.fileInDirectory(Global.Path.config, "tui")) { - yield* Effect.promise(() => mergeFile(acc, file, ctx)).pipe(Effect.orDie) + yield* mergeFile(acc, file) } // 2. Explicit OPENCODE_TUI_CONFIG override, if set. if (Flag.OPENCODE_TUI_CONFIG) { const configFile = Flag.OPENCODE_TUI_CONFIG - yield* Effect.promise(() => mergeFile(acc, configFile, ctx)).pipe(Effect.orDie) + yield* mergeFile(acc, configFile) log.debug("loaded custom tui config", { path: configFile }) } // 3. Project tui files, applied root-first so the closest file wins. for (const file of projectFiles) { - yield* Effect.promise(() => mergeFile(acc, file, ctx)).pipe(Effect.orDie) + yield* mergeFile(acc, file) } // 4. `.opencode` directories (and OPENCODE_CONFIG_DIR) discovered while @@ -127,7 +171,7 @@ const loadState = Effect.fn("TuiConfig.loadState")(function* (ctx: { directory: for (const dir of dirs) { if (!dir.endsWith(".opencode") && dir !== Flag.OPENCODE_CONFIG_DIR) continue for (const file of ConfigPaths.fileInDirectory(dir, "tui")) { - yield* Effect.promise(() => mergeFile(acc, file, ctx)).pipe(Effect.orDie) + yield* mergeFile(acc, file) } } @@ -193,28 +237,3 @@ export async function get() { return runPromise((svc) => svc.get()) } -async function loadFile(filepath: string): Promise { - const text = await ConfigPaths.readFile(filepath) - if (!text) return {} - return load(text, filepath).catch((error) => { - log.warn("failed to load tui config", { path: filepath, error }) - return {} - }) -} - -async function load(text: string, configFilepath: string): Promise { - return ConfigVariable.substitute({ text, type: "path", path: configFilepath, missing: "empty" }) - .then((expanded) => ConfigParse.jsonc(expanded, configFilepath)) - .then((data) => { - if (!isRecord(data)) return {} - - // Flatten a nested "tui" key so users who wrote `{ "tui": { ... } }` inside tui.json - // (mirroring the old opencode.json shape) still get their settings applied. - return ConfigParse.schema(Info, normalize(data), configFilepath) - }) - .then((data) => resolvePlugins(data, configFilepath)) - .catch((error) => { - log.warn("invalid tui config", { path: configFilepath, error }) - return {} - }) -} diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index c6557360bb..3a933f81e9 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -355,15 +355,7 @@ export const layer = Layer.effect( const env = yield* Env.Service const npmSvc = yield* Npm.Service - const readConfigFile = Effect.fnUntraced(function* (filepath: string) { - return yield* fs.readFileString(filepath).pipe( - Effect.catchIf( - (e) => e.reason._tag === "NotFound", - () => Effect.succeed(undefined), - ), - Effect.orDie, - ) - }) + const readConfigFile = (filepath: string) => fs.readFileStringSafe(filepath).pipe(Effect.orDie) const loadConfig = Effect.fnUntraced(function* ( text: string, diff --git a/packages/opencode/src/config/paths.ts b/packages/opencode/src/config/paths.ts index 90f49ee799..82fca570f4 100644 --- a/packages/opencode/src/config/paths.ts +++ b/packages/opencode/src/config/paths.ts @@ -1,11 +1,9 @@ export * as ConfigPaths from "./paths" import path from "path" -import { Filesystem } from "@/util/filesystem" import { Flag } from "@opencode-ai/core/flag/flag" import { Global } from "@opencode-ai/core/global" import { unique } from "remeda" -import { JsonError } from "./error" import * as Effect from "effect/Effect" import { AppFileSystem } from "@opencode-ai/core/filesystem" @@ -45,11 +43,3 @@ export const directories = Effect.fn("ConfigPaths.directories")(function* (direc export function fileInDirectory(dir: string, name: string) { return [path.join(dir, `${name}.json`), path.join(dir, `${name}.jsonc`)] } - -/** Read a config file, returning undefined for missing files and throwing JsonError for other failures. */ -export async function readFile(filepath: string) { - return Filesystem.readText(filepath).catch((err: NodeJS.ErrnoException) => { - if (err.code === "ENOENT") return - throw new JsonError({ path: filepath }, { cause: err }) - }) -} diff --git a/packages/opencode/test/config/tui.test.ts b/packages/opencode/test/config/tui.test.ts index a3f2a1b5fb..5053a7e1f7 100644 --- a/packages/opencode/test/config/tui.test.ts +++ b/packages/opencode/test/config/tui.test.ts @@ -627,3 +627,43 @@ test("merges plugin_enabled flags across config layers", async () => { "local.plugin": true, }) }) + +test("silently skips malformed tui.json — load failures degrade to {}", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await Bun.write(path.join(dir, "tui.json"), '{ "theme": "broken",') + await Bun.write(path.join(dir, ".opencode", "tui.json"), JSON.stringify({ theme: "fallback" })) + }, + }) + + const config = await getTuiConfig(tmp.path) + // Project tui.json is malformed → silently skipped (logs a warning) + // .opencode/tui.json (lower precedence in this path) still loads + expect(config.theme).toBe("fallback") +}) + +test("silently skips non-ENOENT read failures (e.g. tui.json is a directory) — fallback layer still loads", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + // tui.json exists as a DIRECTORY rather than a file → readFileString fails + // with EISDIR (PlatformError reason ≠ NotFound). The fix in this PR routes + // that through catchCause → log + skip, so a fallback layer should still load. + await fs.mkdir(path.join(dir, "tui.json"), { recursive: true }) + await Bun.write(path.join(dir, ".opencode", "tui.json"), JSON.stringify({ theme: "fallback" })) + }, + }) + + const config = await getTuiConfig(tmp.path) + // Did NOT crash; .opencode/tui.json (lower precedence) still loads. + expect(config.theme).toBe("fallback") +}) + +test("missing tui.json — silently treated as empty (ENOENT path)", async () => { + await using tmp = await tmpdir({}) + + // No tui.json anywhere. Should not throw. + const config = await getTuiConfig(tmp.path) + expect(config).toBeDefined() + // No theme set anywhere. + expect(config.theme).toBeUndefined() +})