Browse Source

refactor(tui): move config cache to InstanceState (#22378)

Kit Langton 4 days ago
parent
commit
0a8b6298cd
2 changed files with 104 additions and 33 deletions
  1. 36 0
      packages/opencode/specs/effect/loose-ends.md
  2. 68 33
      packages/opencode/src/config/tui.ts

+ 36 - 0
packages/opencode/specs/effect/loose-ends.md

@@ -0,0 +1,36 @@
+# Effect loose ends
+
+Small follow-ups that do not fit neatly into the main facade, route, tool, or schema migration checklists.
+
+## Config / TUI
+
+- [ ] `config/tui.ts` - finish the internal Effect migration after the `Instance.state(...)` removal.
+      Keep the current precedence and migration semantics intact while converting the remaining internal async helpers (`loadState`, `mergeFile`, `loadFile`, `load`) to `Effect.gen(...)` / `Effect.fn(...)`.
+- [ ] `config/tui.ts` callers - once the internal service is stable, migrate plain async callers to use `TuiConfig.Service` directly where that actually simplifies the code.
+      Likely first callers: `cli/cmd/tui/attach.ts`, `cli/cmd/tui/thread.ts`, `cli/cmd/tui/plugin/runtime.ts`.
+- [ ] `env/index.ts` - move the last production `Instance.state(...)` usage onto `InstanceState` (or its replacement) so `Instance.state` can be deleted.
+
+## ConfigPaths
+
+- [ ] `config/paths.ts` - split pure helpers from effectful helpers.
+      Keep `fileInDirectory(...)` as a plain function.
+- [ ] `config/paths.ts` - add a `ConfigPaths.Service` for the effectful operations so callers do not inherit `AppFileSystem.Service` directly.
+      Initial service surface should cover:
+  - `projectFiles(...)`
+  - `directories(...)`
+  - `readFile(...)`
+  - `parseText(...)`
+- [ ] `config/config.ts` - switch internal config loading from `Effect.promise(() => ConfigPaths.*(...))` to `yield* paths.*(...)` once the service exists.
+- [ ] `config/tui.ts` - switch TUI config loading from async `ConfigPaths.*` wrappers to the `ConfigPaths.Service` once that service exists.
+- [ ] `config/tui-migrate.ts` - decide whether to leave this as a plain async module using wrapper functions or effectify it fully after `ConfigPaths.Service` lands.
+
+## Instance cleanup
+
+- [ ] `project/instance.ts` - remove `Instance.state(...)` once `env/index.ts` is migrated.
+- [ ] `project/state.ts` - delete the bespoke per-instance state helper after the last production caller is gone.
+- [ ] `test/project/state.test.ts` - replace or delete the old `Instance.state(...)` tests after the removal.
+
+## Notes
+
+- Prefer small, semantics-preserving config migrations. Config precedence, legacy key migration, and plugin origin tracking are easy to break accidentally.
+- When changing config loading internals, rerun the config and TUI suites first before broad package sweeps.

+ 68 - 33
packages/opencode/src/config/tui.ts

@@ -1,16 +1,19 @@
 import { existsSync } from "fs"
 import z from "zod"
 import { mergeDeep, unique } from "remeda"
+import { Context, Effect, Fiber, Layer } from "effect"
 import { Config } from "./config"
 import { ConfigPaths } from "./paths"
 import { migrateTuiConfig } from "./tui-migrate"
 import { TuiInfo } from "./tui-schema"
-import { Instance } from "@/project/instance"
 import { Flag } from "@/flag/flag"
 import { Log } from "@/util/log"
 import { isRecord } from "@/util/record"
 import { Global } from "@/global"
-import { AppRuntime } from "@/effect/app-runtime"
+import { Filesystem } from "@/util/filesystem"
+import { InstanceState } from "@/effect/instance-state"
+import { makeRuntime } from "@/effect/run-service"
+import { AppFileSystem } from "@/filesystem"
 
 export namespace TuiConfig {
   const log = Log.create({ service: "tui.config" })
@@ -21,13 +24,26 @@ export namespace TuiConfig {
     result: Info
   }
 
+  type State = {
+    config: Info
+    deps: Array<Fiber.Fiber<void, AppFileSystem.Error>>
+  }
+
   export type Info = z.output<typeof Info> & {
     // Internal resolved plugin list used by runtime loading.
     plugin_origins?: Config.PluginOrigin[]
   }
 
-  function pluginScope(file: string): Config.PluginScope {
-    if (Instance.containsPath(file)) return "local"
+  export interface Interface {
+    readonly get: () => Effect.Effect<Info>
+    readonly waitForDependencies: () => Effect.Effect<void, AppFileSystem.Error>
+  }
+
+  export class Service extends Context.Service<Service, Interface>()("@opencode/TuiConfig") {}
+
+  function pluginScope(file: string, ctx: { directory: string; worktree: string }): Config.PluginScope {
+    if (Filesystem.contains(ctx.directory, file)) return "local"
+    if (ctx.worktree !== "/" && Filesystem.contains(ctx.worktree, file)) return "local"
     return "global"
   }
 
@@ -51,16 +67,12 @@ export namespace TuiConfig {
     }
   }
 
-  function installDeps(dir: string): Promise<void> {
-    return AppRuntime.runPromise(Config.Service.use((cfg) => cfg.installDependencies(dir)))
-  }
-
-  async function mergeFile(acc: Acc, file: string) {
+  async function mergeFile(acc: Acc, file: string, ctx: { directory: string; worktree: string }) {
     const data = await loadFile(file)
     acc.result = mergeDeep(acc.result, data)
     if (!data.plugin?.length) return
 
-    const scope = pluginScope(file)
+    const scope = pluginScope(file, ctx)
     const plugins = Config.deduplicatePluginOrigins([
       ...(acc.result.plugin_origins ?? []),
       ...data.plugin.map((spec) => ({ spec, scope, source: file })),
@@ -69,46 +81,48 @@ export namespace TuiConfig {
     acc.result.plugin_origins = plugins
   }
 
-  const state = Instance.state(async () => {
+  async function loadState(ctx: { directory: string; worktree: string }) {
     let projectFiles = Flag.OPENCODE_DISABLE_PROJECT_CONFIG
       ? []
-      : await ConfigPaths.projectFiles("tui", Instance.directory, Instance.worktree)
-    const directories = await ConfigPaths.directories(Instance.directory, Instance.worktree)
+      : await ConfigPaths.projectFiles("tui", ctx.directory, ctx.worktree)
+    const directories = await ConfigPaths.directories(ctx.directory, ctx.worktree)
     const custom = customPath()
     const managed = Config.managedConfigDir()
     await migrateTuiConfig({ directories, custom, managed })
     // Re-compute after migration since migrateTuiConfig may have created new tui.json files
     projectFiles = Flag.OPENCODE_DISABLE_PROJECT_CONFIG
       ? []
-      : await ConfigPaths.projectFiles("tui", Instance.directory, Instance.worktree)
+      : await ConfigPaths.projectFiles("tui", ctx.directory, ctx.worktree)
 
     const acc: Acc = {
       result: {},
     }
 
     for (const file of ConfigPaths.fileInDirectory(Global.Path.config, "tui")) {
-      await mergeFile(acc, file)
+      await mergeFile(acc, file, ctx)
     }
 
     if (custom) {
-      await mergeFile(acc, custom)
+      await mergeFile(acc, custom, ctx)
       log.debug("loaded custom tui config", { path: custom })
     }
 
     for (const file of projectFiles) {
-      await mergeFile(acc, file)
+      await mergeFile(acc, file, ctx)
     }
 
-    for (const dir of unique(directories)) {
+    const dirs = unique(directories).filter((dir) => dir.endsWith(".opencode") || dir === Flag.OPENCODE_CONFIG_DIR)
+
+    for (const dir of dirs) {
       if (!dir.endsWith(".opencode") && dir !== Flag.OPENCODE_CONFIG_DIR) continue
       for (const file of ConfigPaths.fileInDirectory(dir, "tui")) {
-        await mergeFile(acc, file)
+        await mergeFile(acc, file, ctx)
       }
     }
 
     if (existsSync(managed)) {
       for (const file of ConfigPaths.fileInDirectory(managed, "tui")) {
-        await mergeFile(acc, file)
+        await mergeFile(acc, file, ctx)
       }
     }
 
@@ -122,27 +136,48 @@ export namespace TuiConfig {
     }
     acc.result.keybinds = Config.Keybinds.parse(keybinds)
 
-    const deps: Promise<void>[] = []
-    if (acc.result.plugin?.length) {
-      for (const dir of unique(directories)) {
-        if (!dir.endsWith(".opencode") && dir !== Flag.OPENCODE_CONFIG_DIR) continue
-        deps.push(installDeps(dir))
-      }
-    }
-
     return {
       config: acc.result,
-      deps,
+      dirs: acc.result.plugin?.length ? dirs : [],
     }
-  })
+  }
+
+  export const layer = Layer.effect(
+    Service,
+    Effect.gen(function* () {
+      const cfg = yield* Config.Service
+      const state = yield* InstanceState.make<State>(
+        Effect.fn("TuiConfig.state")(function* (ctx) {
+          const data = yield* Effect.promise(() => loadState(ctx))
+          const deps = yield* Effect.forEach(data.dirs, (dir) => cfg.installDependencies(dir).pipe(Effect.forkScoped), {
+            concurrency: "unbounded",
+          })
+          return { config: data.config, deps }
+        }),
+      )
+
+      const get = Effect.fn("TuiConfig.get")(() => InstanceState.use(state, (s) => s.config))
+
+      const waitForDependencies = Effect.fn("TuiConfig.waitForDependencies")(() =>
+        InstanceState.useEffect(state, (s) =>
+          Effect.forEach(s.deps, Fiber.join, { concurrency: "unbounded" }).pipe(Effect.asVoid),
+        ),
+      )
+
+      return Service.of({ get, waitForDependencies })
+    }),
+  )
+
+  export const defaultLayer = layer.pipe(Layer.provide(Config.defaultLayer))
+
+  const { runPromise } = makeRuntime(Service, defaultLayer)
 
   export async function get() {
-    return state().then((x) => x.config)
+    return runPromise((svc) => svc.get())
   }
 
   export async function waitForDependencies() {
-    const deps = await state().then((x) => x.deps)
-    await Promise.all(deps)
+    await runPromise((svc) => svc.waitForDependencies())
   }
 
   async function loadFile(filepath: string): Promise<Info> {