Просмотр исходного кода

refactor(config+core): drop ConfigPaths.readFile, add AppFileSystem.readFileStringSafe, flatten TuiConfig.loadState (#25602)

Kit Langton 3 месяцев назад
Родитель
Сommit
13ac849db5

+ 8 - 0
packages/core/src/filesystem.ts

@@ -24,6 +24,7 @@ export namespace AppFileSystem {
     readonly isDir: (path: string) => Effect.Effect<boolean>
     readonly isFile: (path: string) => Effect.Effect<boolean>
     readonly existsSafe: (path: string) => Effect.Effect<boolean>
+    readonly readFileStringSafe: (path: string) => Effect.Effect<string | undefined, Error>
     readonly readJson: (path: string) => Effect.Effect<unknown, Error>
     readonly writeJson: (path: string, data: unknown, mode?: number) => Effect.Effect<void, Error>
     readonly ensureDir: (path: string) => Effect.Effect<void, Error>
@@ -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,

+ 28 - 0
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",

+ 69 - 50
packages/opencode/src/cli/cmd/tui/config/tui.ts

@@ -68,29 +68,73 @@ function normalize(raw: Record<string, unknown>) {
   }
 }
 
-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
-}
+const loadState = Effect.fn("TuiConfig.loadState")(function* (ctx: { directory: string }) {
+  const afs = yield* AppFileSystem.Service
+
+  const resolvePlugins = (config: Info, configFilepath: string): Effect.Effect<Info> =>
+    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
+    })
 
-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 load = (text: string, configFilepath: string): Effect.Effect<Info> =>
+    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<Info> =>
+    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
+    })
 
-const loadState = Effect.fn("TuiConfig.loadState")(function* (ctx: { directory: string }) {
   // 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<Info> {
-  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<Info> {
-  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 {}
-    })
-}

+ 1 - 9
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,

+ 0 - 10
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 })
-  })
-}

+ 40 - 0
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()
+})