From 33d9a26dd7db0c549ea30457b9a1c2c3d78c5ffc Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 13 Apr 2026 23:30:25 +0000 Subject: [PATCH] fix(tui): keep AUTO-mode widgets alive and drop duplicate health panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit InteractiveMode.renderWidgets() called Container.clear() on the widgetContainerAbove/Below render mounts, which disposed every mounted extension widget and then re-added the now-dead components. In AUTO mode updateProgressWidget re-registers gsd-progress on every unit dispatch, so gsd-notifications and gsd-health had their refresh timers and store subscriptions killed after the first dispatch. Renders kept returning the widgets' frozen cachedLines, making them look alive but never update (/gsd notifications clear appeared to do nothing, belowEditor last-commit went stale while the top-of-screen dashboard stayed correct). Split detach from dispose: add Container.detachChildren() and use it from the two widget-mount call sites. clear() still disposes for every other caller (chat, editor, status, pinned-message containers). The extensionWidgets* maps remain the single owner of widget disposal via removeExisting() and clearExtensionWidgets(). While in AUTO, gsd-progress duplicates gsd-health on last commit, cost/ budget, and the health signal. Make gsd-progress the single source of truth: hide gsd-health from auto-start and re-register it from every exit point in auto.ts (lock-lost stop, cleanupAfterLoopExit !paused guard, stopAuto, pauseAuto). gsd-notifications stays visible — it is independent state and, with the detach fix, its subscription + 5s refresh actually work again. Tests: Container.detachChildren()/clear() contract guards added to packages/pi-tui/src/__tests__/tui.test.ts. health-widget, notification-{store,widget,overlay}, notifications-handler, notifications, and auto-paused-ui-cleanup suites all pass. --- .../src/modes/interactive/interactive-mode.ts | 8 +++- packages/pi-tui/src/__tests__/tui.test.ts | 39 ++++++++++++++++++- packages/pi-tui/src/tui.ts | 11 ++++++ src/resources/extensions/gsd/auto-start.ts | 3 ++ src/resources/extensions/gsd/auto.ts | 5 +++ 5 files changed, 63 insertions(+), 3 deletions(-) diff --git a/packages/pi-coding-agent/src/modes/interactive/interactive-mode.ts b/packages/pi-coding-agent/src/modes/interactive/interactive-mode.ts index 33a185c04..fe0b69116 100644 --- a/packages/pi-coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/pi-coding-agent/src/modes/interactive/interactive-mode.ts @@ -1402,7 +1402,9 @@ export class InteractiveMode { // widgetContainerAbove: spacer collapses when pinned content is visible // so there's no extra blank line between pinned output and the editor border. - this.widgetContainerAbove.clear(); + // Use detachChildren() (not clear()) — the extensionWidgetsAbove map owns + // disposal; clear() would dispose every mounted widget on every re-render. + this.widgetContainerAbove.detachChildren(); const pinned = this.pinnedMessageContainer; this.widgetContainerAbove.addChild({ render: () => pinned.children.length > 0 ? [] : [""], @@ -1422,7 +1424,9 @@ export class InteractiveMode { spacerWhenEmpty: boolean, leadingSpacer: boolean, ): void { - container.clear(); + // Detach without disposing — the widgets map owns lifecycle; disposing + // here would kill refresh timers and subscriptions on every re-render. + container.detachChildren(); if (widgets.size === 0) { if (spacerWhenEmpty) { diff --git a/packages/pi-tui/src/__tests__/tui.test.ts b/packages/pi-tui/src/__tests__/tui.test.ts index 7c4903dc7..12be1a938 100644 --- a/packages/pi-tui/src/__tests__/tui.test.ts +++ b/packages/pi-tui/src/__tests__/tui.test.ts @@ -1,7 +1,8 @@ import assert from "node:assert/strict"; import { describe, it } from "node:test"; -import { TUI } from "../tui.js"; +import { Container, TUI } from "../tui.js"; +import type { Component } from "../tui.js"; import type { Terminal } from "../terminal.js"; function makeTerminal(): Terminal { @@ -48,3 +49,39 @@ describe("TUI", () => { assert.equal(anyTui.inputBuffer, ""); }); }); + +describe("Container", () => { + function makeDisposableChild(counter: { disposed: number }): Component & { dispose(): void } { + return { + render: () => [], + invalidate() {}, + dispose() { + counter.disposed++; + }, + }; + } + + it("detachChildren() removes children without disposing them", () => { + const c = new Container(); + const counter = { disposed: 0 }; + c.addChild(makeDisposableChild(counter)); + c.addChild(makeDisposableChild(counter)); + + c.detachChildren(); + + assert.equal(c.children.length, 0); + assert.equal(counter.disposed, 0); + }); + + it("clear() still disposes children (regression guard for detach/dispose split)", () => { + const c = new Container(); + const counter = { disposed: 0 }; + c.addChild(makeDisposableChild(counter)); + c.addChild(makeDisposableChild(counter)); + + c.clear(); + + assert.equal(c.children.length, 0); + assert.equal(counter.disposed, 2); + }); +}); diff --git a/packages/pi-tui/src/tui.ts b/packages/pi-tui/src/tui.ts index 7c58c0145..162c7756e 100644 --- a/packages/pi-tui/src/tui.ts +++ b/packages/pi-tui/src/tui.ts @@ -197,6 +197,17 @@ export class Container implements Component { this._prevRender = null; } + /** + * Remove all children without calling dispose on them. + * Use when child lifecycle is owned elsewhere and the container is only a + * render mount (e.g. extension widget containers in InteractiveMode, where + * the extensionWidgets* maps own disposal). + */ + detachChildren(): void { + this.children = []; + this._prevRender = null; + } + invalidate(): void { for (const child of this.children) { child.invalidate?.(); diff --git a/src/resources/extensions/gsd/auto-start.ts b/src/resources/extensions/gsd/auto-start.ts index 120a8e01b..8b9f7282f 100644 --- a/src/resources/extensions/gsd/auto-start.ts +++ b/src/resources/extensions/gsd/auto-start.ts @@ -806,6 +806,9 @@ export async function bootstrapAutoSession( ctx.ui.setStatus("gsd-auto", s.stepMode ? "next" : "auto"); ctx.ui.setFooter(hideFooter); + // Hide gsd-health during AUTO — gsd-progress is the single source of truth + // for last-commit / cost / health signal while auto is running. + ctx.ui.setWidget("gsd-health", undefined); const modeLabel = s.stepMode ? "Step-mode" : "Auto-mode"; const pendingCount = (state.registry ?? []).filter( (m) => m.status !== "complete" && m.status !== "parked", diff --git a/src/resources/extensions/gsd/auto.ts b/src/resources/extensions/gsd/auto.ts index 0ebf3a975..344055951 100644 --- a/src/resources/extensions/gsd/auto.ts +++ b/src/resources/extensions/gsd/auto.ts @@ -199,6 +199,7 @@ import { postUnitPostVerification, } from "./auto-post-unit.js"; import { bootstrapAutoSession, openProjectDbIfPresent, type BootstrapDeps } from "./auto-start.js"; +import { initHealthWidget } from "./health-widget.js"; import { autoLoop, resolveAgentEnd, resolveAgentEndCancelled, _resetPendingResolve, isSessionSwitchInFlight, type LoopDeps, type ErrorContext } from "./auto-loop.js"; // Slice-level parallelism (#2340) import { getEligibleSlices } from "./slice-parallel-eligibility.js"; @@ -650,6 +651,7 @@ function handleLostSessionLock( ctx?.ui.setStatus("gsd-auto", undefined); ctx?.ui.setWidget("gsd-progress", undefined); ctx?.ui.setFooter(undefined); + if (ctx) initHealthWidget(ctx); } /** @@ -684,6 +686,7 @@ function cleanupAfterLoopExit(ctx: ExtensionContext): void { ctx.ui.setStatus("gsd-auto", undefined); ctx.ui.setWidget("gsd-progress", undefined); ctx.ui.setFooter(undefined); + initHealthWidget(ctx); } // Restore CWD out of worktree back to original project root @@ -943,6 +946,7 @@ export async function stopAuto( ctx?.ui.setStatus("gsd-auto", undefined); ctx?.ui.setWidget("gsd-progress", undefined); ctx?.ui.setFooter(undefined); + if (ctx) initHealthWidget(ctx); restoreProjectRootEnv(); restoreMilestoneLockEnv(); @@ -1044,6 +1048,7 @@ export async function pauseAuto( ctx?.ui.setStatus("gsd-auto", "paused"); ctx?.ui.setWidget("gsd-progress", undefined); ctx?.ui.setFooter(undefined); + if (ctx) initHealthWidget(ctx); const resumeCmd = s.stepMode ? "/gsd next" : "/gsd auto"; ctx?.ui.notify( `${s.stepMode ? "Step" : "Auto"}-mode paused (Escape). Type to interact, or ${resumeCmd} to resume.`,