From adfa9141a36f7a3df2aad67fe39e25f2dfd05333 Mon Sep 17 00:00:00 2001 From: bcode Date: Sat, 16 May 2026 22:54:53 -0700 Subject: [PATCH] fix(plugin): drop scope-close deregistration of shutdown hooks Fix 3 from the cubic review on PR #75 was a regression. The plugin-layer scope (under InstanceState) closes BEFORE the top-level finally in src/index.ts runs, via store.dispose(ctx) in effect-cmd's own finally. A scope-close finalizer empties pluginShutdownHooks before the host can iterate it, defeating the entire feature. Verified live: v0.1.8-rc1 in staging cloud V4 still produced orphan turn parents (trace 0ee0b31c-..., top_span_landed=False, total_cost=0, all session.llm children referencing one unlanded parent span). Multi-instance TUI mode bloat from accumulated closures is acceptable -- rare in practice, closures are tiny, and the hooks Set is intentionally module-level so it outlives Effect scopes. --- packages/opencode/src/plugin/index.ts | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/packages/opencode/src/plugin/index.ts b/packages/opencode/src/plugin/index.ts index 004627806..c42c39c7b 100644 --- a/packages/opencode/src/plugin/index.ts +++ b/packages/opencode/src/plugin/index.ts @@ -279,19 +279,17 @@ export const layer = Layer.effect( // OTel spans (e.g. bcode-laminar's turn span) — the bus-based // session.idle / server.instance.disposed paths race with scope // teardown and don't reliably deliver, so plugins need a direct sync - // entry point. Deregister on instance disposal so multi-instance TUI - // mode doesn't accumulate stale closures across reopens. - const registered: Array<() => void> = [] + // entry point. + // + // Intentionally NOT deregistered on scope close: the plugin-layer + // scope (under InstanceState) closes BEFORE the top-level finally in + // src/index.ts runs (via store.dispose(ctx) in effect-cmd's own + // finally). A scope-close finalizer would empty the Set before the + // host could call it, defeating the entire feature. Multi-instance + // TUI bloat from accumulated closures is acceptable. for (const hook of hooks) { - if (!hook.shutdown) continue - pluginShutdownHooks.add(hook.shutdown) - registered.push(hook.shutdown) + if (hook.shutdown) pluginShutdownHooks.add(hook.shutdown) } - yield* Effect.addFinalizer(() => - Effect.sync(() => { - for (const fn of registered) pluginShutdownHooks.delete(fn) - }), - ) return { hooks } }),