Browse Source

fix(core): keep config root watches alive and ignore vendored trees (#39239)

Kit Langton 2 tuần trước cách đây
mục cha
commit
5bd3da40a5
2 tập tin đã thay đổi với 77 bổ sung14 xóa
  1. 14 13
      packages/core/src/config.ts
  2. 63 1
      packages/core/test/config/config.test.ts

+ 14 - 13
packages/core/src/config.ts

@@ -4,7 +4,7 @@ import { makeLocationNode } from "@opencode-ai/util/effect/app-node"
 import path from "path"
 import { isDeepStrictEqual } from "node:util"
 import { type ParseError, parse } from "jsonc-parser"
-import { Context, Effect, Fiber, Layer, Option, PubSub, Ref, Schema, Semaphore, Stream } from "effect"
+import { Context, Effect, Layer, Option, PubSub, Ref, Schema, Semaphore, Stream } from "effect"
 import { Permission } from "@opencode-ai/schema/permission"
 import { Config as ConfigSchema } from "@opencode-ai/schema/config"
 import { Integration } from "@opencode-ai/schema/integration"
@@ -373,29 +373,30 @@ export const layer = (options?: Options) => Layer.effect(
     const initial = yield* discover()
     let configs = initial
     const updates = yield* PubSub.unbounded<Watcher.Update>()
-    const subscriptions = new Map<string, Effect.Effect<unknown>>()
+    // Vendored trees inside config roots (a plugin's node_modules, a nested
+    // .git) produce event blizzards that can never change discovery output.
+    const ignore = ["node_modules", ".git", "**/{node_modules,.git}/**"]
+    // Watch-once: roots leave discovery only by deletion, so a stale watch is
+    // inert, bounded, and dies with this layer — and keeping a deleted root's
+    // watch alive is exactly what makes its recreation observable.
+    const watched = new Set<string>()
     const reconcile = Effect.fn("Config.reconcileWatches")(function* (entries: readonly Entry[]) {
       const directories = entries.flatMap((entry) => (entry.type === "directory" ? [entry.path] : []))
       const files = entries.flatMap((entry) => (entry.type === "file" ? [entry.path] : []))
       const targets = [
-        ...directories.map((path) => ({ path, type: "directory" as const })),
+        ...directories.map((path) => ({ path, type: "directory" as const, ignore })),
         ...files
           .filter((file) => !directories.some((directory) => FSUtil.contains(directory, file)))
           .map((path) => ({ path, type: "file" as const })),
       ]
-      const next = new Map(targets.map((target) => [JSON.stringify(target), target]))
-      for (const [key, stop] of subscriptions) {
-        if (next.has(key)) continue
-        yield* stop
-        subscriptions.delete(key)
-      }
-      for (const [key, target] of next) {
-        if (subscriptions.has(key)) continue
-        const fiber = yield* watcher.subscribe(target).pipe(
+      for (const target of targets) {
+        const key = JSON.stringify(target)
+        if (watched.has(key)) continue
+        watched.add(key)
+        yield* watcher.subscribe(target).pipe(
           Stream.runForEach((update) => PubSub.publish(updates, update)),
           Effect.forkScoped({ startImmediately: true }),
         )
-        subscriptions.set(key, Fiber.interrupt(fiber))
       }
     })
 

+ 63 - 1
packages/core/test/config/config.test.ts

@@ -209,6 +209,64 @@ describe("Config", () => {
     ),
   )
 
+  // Real watcher on purpose: the regression this pins (a deleted config file's
+  // watch being torn down, making recreation invisible) only reproduces with
+  // path-faithful event delivery.
+  it.live("keeps watching a deleted config file so recreating it reloads", () =>
+    Effect.acquireRelease(
+      Effect.promise(() => tmpdir()),
+      (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
+    ).pipe(
+      Effect.flatMap((tmp) =>
+        Effect.gen(function* () {
+          const global = path.join(tmp.path, "global")
+          const project = path.join(tmp.path, "project")
+          const file = path.join(project, "opencode.json")
+          yield* Effect.promise(async () => {
+            await fs.mkdir(global, { recursive: true })
+            await fs.mkdir(project, { recursive: true })
+            await fs.writeFile(file, JSON.stringify({ shell: "one" }))
+          })
+          return yield* Effect.gen(function* () {
+            const config = yield* Config.Service
+            const bus = yield* Bus.Service
+            expect(Config.latest(yield* config.entries(), "shell")).toBe("one")
+            yield* Effect.sleep("10 millis")
+
+            const removed = yield* bus
+              .subscribe(ConfigSchema.Event.Updated)
+              .pipe(Stream.take(1), Stream.runDrain, Effect.forkScoped({ startImmediately: true }))
+            yield* Effect.promise(() => fs.rm(file))
+            yield* Fiber.join(removed).pipe(Effect.timeout("5 seconds"))
+            expect(Config.latest(yield* config.entries(), "shell")).toBeUndefined()
+
+            const recreated = yield* bus
+              .subscribe(ConfigSchema.Event.Updated)
+              .pipe(Stream.take(1), Stream.runDrain, Effect.forkScoped({ startImmediately: true }))
+            yield* Effect.promise(() => fs.writeFile(file, JSON.stringify({ shell: "two" })))
+            yield* Fiber.join(recreated).pipe(Effect.timeout("5 seconds"))
+            expect(Config.latest(yield* config.entries(), "shell")).toBe("two")
+          }).pipe(
+            Effect.provide(
+              AppNodeBuilder.build(LayerNode.group([Config.node, Bus.node]), [
+                [
+                  Location.node,
+                  Layer.succeed(
+                    Location.Service,
+                    Location.Service.of(location({ directory: AbsolutePath.make(project) })),
+                  ),
+                ],
+                [Global.node, Global.layerWith({ config: global, home: path.join(global, "home") })],
+                [Credential.node, emptyCredentialNode],
+                [WellKnown.node, emptyWellknownNode],
+              ]),
+            ),
+          )
+        }),
+      ),
+    ),
+  )
+
   it.effect("backs Config.Service and Config.Test with one shared test implementation", () =>
     Effect.gen(function* () {
       const config = yield* Config.Service
@@ -524,7 +582,11 @@ describe("Config", () => {
             yield* config.entries()
 
             expect(yield* watcher.subscriptions()).toEqual([
-              { type: "directory", path: AbsolutePath.make(path.join(tmp.path, "global")) },
+              {
+                type: "directory",
+                path: AbsolutePath.make(path.join(tmp.path, "global")),
+                ignore: ["**/{node_modules,.git}/**", ".git", "node_modules"],
+              },
             ])
           }).pipe(Effect.provide(testLayer(tmp.path, undefined, undefined, undefined, Watcher.testLayer)))
         }),