From 732c9e6d954151b38f2a2c74b7c3de3a9c18ecd1 Mon Sep 17 00:00:00 2001 From: Turtle Date: Fri, 31 Jul 2026 15:11:24 +0800 Subject: [PATCH] fix(tui): release the adapter-registration listener on channel detach Review follow-up: the llm/adapters-updated listener's disposer was discarded, leaving it firing (harmlessly, behind isDisposed()) between TUI shutdown and fiber disposal, asymmetric with the sibling channel listeners. The controller now exposes detach(), and the channel's detachListeners() calls it on both the dispose() and startup-failure paths. --- ...30-tui-adapter-registration-race.i18n.yaml | 4 +-- ...026-07-30-tui-adapter-registration-race.md | 2 +- ...-07-30-tui-adapter-registration-race.zh.md | 2 +- packages/ui/tui/src/chat/model-command.ts | 10 ++++++-- packages/ui/tui/src/index.ts | 1 + packages/ui/tui/tests/tui.spec.ts | 25 +++++++++++++++++++ 6 files changed, 38 insertions(+), 6 deletions(-) diff --git a/.agents/notes/implemented/bug-fix/2026-07-30-tui-adapter-registration-race.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-07-30-tui-adapter-registration-race.i18n.yaml index 93fd2279d1..b3d987b985 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-30-tui-adapter-registration-race.i18n.yaml +++ b/.agents/notes/implemented/bug-fix/2026-07-30-tui-adapter-registration-race.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-07-30-tui-adapter-registration-race.md -2026-07-30-tui-adapter-registration-race.md: e77a338d6b23a48dd140bd9160f22746c43f9fae -2026-07-30-tui-adapter-registration-race.zh.md: ce60b88ca7c62e09d7ad1a166932d86e4b240e03 +2026-07-30-tui-adapter-registration-race.md: fd08e7b6130bc8f7e3cd5287a9970f5fb47244a8 +2026-07-30-tui-adapter-registration-race.zh.md: 0c6bba4bbc8c3303d9c471c3164faa816438b333 diff --git a/.agents/notes/implemented/bug-fix/2026-07-30-tui-adapter-registration-race.md b/.agents/notes/implemented/bug-fix/2026-07-30-tui-adapter-registration-race.md index e77a338d6b..fd08e7b613 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-30-tui-adapter-registration-race.md +++ b/.agents/notes/implemented/bug-fix/2026-07-30-tui-adapter-registration-race.md @@ -26,4 +26,4 @@ The TUI model controller treats a `NO_ADAPTER` rejection of its context-window r ## Consequences -A genuinely misconfigured provider no longer prints the context-resolution error at startup — it surfaces at first dispatch instead, which is where the failure is actionable. The controller subscribes to every `llm/adapters-updated` commit but acts only while a wait is parked. Covered by two TUI tests: the deferred resolution stays silent through an unrelated commit and completes when the route's commit arrives, and a target change drops the stale wait. +A genuinely misconfigured provider no longer prints the context-resolution error at startup — it surfaces at first dispatch instead, which is where the failure is actionable. The controller subscribes to every `llm/adapters-updated` commit but acts only while a wait is parked; the listener's disposer is released by the channel's `detachListeners()` through the controller's `detach()`, symmetric with the sibling channel listeners. Covered by three TUI tests: the deferred resolution stays silent through an unrelated commit and completes when the route's commit arrives, a target change drops the stale wait, and after channel detach a registry commit no longer re-enters resolution. diff --git a/.agents/notes/implemented/bug-fix/2026-07-30-tui-adapter-registration-race.zh.md b/.agents/notes/implemented/bug-fix/2026-07-30-tui-adapter-registration-race.zh.md index ce60b88ca7..0c6bba4bbc 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-30-tui-adapter-registration-race.zh.md +++ b/.agents/notes/implemented/bug-fix/2026-07-30-tui-adapter-registration-race.zh.md @@ -26,4 +26,4 @@ TUI 模型控制器把上下文窗口解析中的 `NO_ADAPTER` 拒绝视为瞬 ## Consequences -真正配置错误的提供方不再在启动时打印上下文解析错误——它改在首次分派时暴露,那才是该失败可以被处理的地方。控制器订阅每次 `llm/adapters-updated` 提交,但只在有等待被搁置时才动作。由两个 TUI 测试覆盖:延后的解析在无关提交中保持沉默、在该路由的提交到来时完成;目标变更丢弃陈旧等待。 +真正配置错误的提供方不再在启动时打印上下文解析错误——它改在首次分派时暴露,那才是该失败可以被处理的地方。控制器订阅每次 `llm/adapters-updated` 提交,但只在有等待被搁置时才动作;监听器的 disposer 经由控制器的 `detach()` 在频道的 `detachListeners()` 中释放,与同级频道监听器保持对称。由三个 TUI 测试覆盖:延后的解析在无关提交中保持沉默、在该路由的提交到来时完成;目标变更丢弃陈旧等待;频道 detach 之后注册表提交不再重新进入解析。 diff --git a/packages/ui/tui/src/chat/model-command.ts b/packages/ui/tui/src/chat/model-command.ts index 794f293496..c86b3b7e3d 100644 --- a/packages/ui/tui/src/chat/model-command.ts +++ b/packages/ui/tui/src/chat/model-command.ts @@ -37,6 +37,8 @@ export interface ModelController { resetContextResolution(): void /** Forget the tracked selector overlay (shutdown). */ clearOverlay(): void + /** Remove the adapter-registration listener (channel detach). */ + detach(): void } type ContextResolution = @@ -88,8 +90,9 @@ export function createModelController(deps: ModelControllerDeps): ModelControlle // The wait cannot go stale against `target.current`: every target change // re-enters resolveContextWindow, which clears it. A commit that still // lacks the route parks the resolution again rather than erroring, so - // unrelated topology changes stay silent. - ctx.on('llm/adapters-updated', () => { + // unrelated topology changes stay silent. The disposer rides the channel's + // detachListeners() through detach(), matching the sibling listeners. + const disposeAdapterListener = ctx.on('llm/adapters-updated', () => { if (deps.isDisposed() || !awaitingAdapter) return resolveContextWindow(target.current) }) @@ -206,5 +209,8 @@ export function createModelController(deps: ModelControllerDeps): ModelControlle clearOverlay(): void { modelOverlay = undefined }, + detach(): void { + disposeAdapterListener() + }, } } diff --git a/packages/ui/tui/src/index.ts b/packages/ui/tui/src/index.ts index a1250bb5b3..e2eaf89986 100644 --- a/packages/ui/tui/src/index.ts +++ b/packages/ui/tui/src/index.ts @@ -1563,6 +1563,7 @@ export function createTuiChat( disposeAgent() disposeSchemeListener() disposeTargetListeners() + modelController.detach() } // Sweep reveal of the whole banner: the header wipes in left-to-right over diff --git a/packages/ui/tui/tests/tui.spec.ts b/packages/ui/tui/tests/tui.spec.ts index 9e93cd49b9..962df7bd2d 100644 --- a/packages/ui/tui/tests/tui.spec.ts +++ b/packages/ui/tui/tests/tui.spec.ts @@ -3671,6 +3671,31 @@ describe('pi-tui chat lifecycle and transcript', () => { await dispose(result) }) + it('stops listening for adapter registrations after channel detach', async () => { + // The listener disposer rides detachListeners() through the controller's + // detach(): after dispose, a registry commit must not re-enter resolution + // at all (the isDisposed() guard is a fallback, not the removal). + const calls: string[] = [] + const result = await setup({ + agentOptions: { provider: 'openai-codex', model: 'gpt-x' }, + catalog: { + providers: [], + models: [], + resolveModelInfo: (provider) => { + calls.push(provider) + return Promise.reject(new LlmError('no adapter registered for provider "openai-codex"', 'NO_ADAPTER')) + }, + }, + }) + await tick() + const callsAtDetach = calls.length + await result.controller.dispose() + result.ctx.emit('llm/adapters-updated') + await tick() + expect(calls.length).toBe(callsAtDetach) + await result.ctx.fiber.dispose() + }) + it('drops a deferred NO_ADAPTER resolution when the target moved before the adapter registered', async () => { const result = await setup({ agentOptions: { provider: 'openai-codex', model: 'gpt-x' },