Ver código fonte

fix(review): tolerate mid-teardown personal-watcher registration

CI caught a startup race the PTY smoke stages naturally: a TUI /exit typed
while watchPersonalPatches is still opening its watcher disposes the tree,
and the HMR effect registration rejects with INACTIVE_EFFECT, crashing an
app that exited exactly as asked. Return a no-op disposer for that exact
code; every other registration failure still propagates (covered both ways).
Tianyi Cui 2 meses atrás
pai
commit
c96f48e176

+ 11 - 1
packages/ui/app-boot/src/index.ts

@@ -323,7 +323,7 @@ export async function watchPersonalPatches(
   const entry = bootstrapIncludes.get(ctx)
   if (entry === undefined) throw new Error(`${binName}: personal config watching requires the root Include entry`)
   const filename = join(dir, PERSONAL_CONFIG_FILENAME)
-  return hmr.registerConfig(filename, async () => {
+  const register = hmr.registerConfig(filename, async () => {
     // Re-read the include's non-patch options per refresh: a writer that
     // updates the root Include's other options between refreshes (none exists
     // today) must not have them silently reverted by a personal reload.
@@ -337,6 +337,16 @@ export async function watchPersonalPatches(
       },
     })
   })
+  try {
+    return await register
+  } catch (error) {
+    // A surface can dispose the whole tree while the watcher is still opening
+    // (a TUI `/exit` typed during startup): the HMR effect registration then
+    // fails with INACTIVE_EFFECT. That is the app exiting exactly as asked,
+    // not a watch failure — return a no-op disposer instead of crashing.
+    if ((error as { code?: string } | null)?.code === 'INACTIVE_EFFECT') return async () => {}
+    throw error
+  }
 }
 
 /**

+ 34 - 0
packages/ui/app-boot/tests/personal-config.spec.ts

@@ -233,4 +233,38 @@ describe('boot with personal patches', () => {
     await expect(watchPersonalPatches(withoutInclude, { binName: NAME, dir: tmp() })).rejects.toThrow('requires the root Include entry')
     await withoutInclude.fiber.dispose()
   })
+
+  it('returns a no-op disposer when the tree is disposed while the watcher opens', async () => {
+    // A TUI `/exit` typed during startup disposes the whole tree while
+    // registerConfig's effect registration is still in flight (the HMR effect
+    // then fails with INACTIVE_EFFECT); the app is exiting exactly as asked,
+    // so the watcher must not crash the process. The stub makes the race
+    // deterministic — the live-teardown ordering itself is not stageable.
+    const dir = tmp()
+    const ctx = await boot(NAME, writeTree(dir))
+    try {
+      const teardown = Object.assign(new Error('cannot create effect on inactive context'), { code: 'INACTIVE_EFFECT' })
+      ctx.provide('hmr', { registerConfig: () => Promise.reject(teardown) })
+      const dispose = await watchPersonalPatches(ctx, { binName: NAME, dir: tmp() })
+      await expect(dispose()).resolves.toBeUndefined()
+    } finally {
+      await ctx.fiber.dispose()
+    }
+  })
+
+  it('propagates registration failures other than mid-teardown', async () => {
+    const dir = tmp()
+    const personal = tmp()
+    const ctx = await boot(NAME, writeTree(dir))
+    try {
+      await ctx.plugin(Timer)
+      await ctx.plugin(Hmr, { root: [], ignored: [], debounce: 0 })
+      const dispose = await watchPersonalPatches(ctx, { binName: NAME, dir: personal })
+      // Same personal path registered twice: HMR refuses; not a teardown race.
+      await expect(watchPersonalPatches(ctx, { binName: NAME, dir: personal })).rejects.toThrow('already registered')
+      await dispose()
+    } finally {
+      await ctx.fiber.dispose()
+    }
+  })
 })