diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d027da5be..8ce20eb81 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,10 +1,15 @@ # Changelog ## [Unreleased] +### Added + +- Added a `ChatBlock` transcript primitive (`modes/components/chat-block.ts`) and a single `ctx.present(...)` sink (with `ctx.resetTranscript()`) so chat output is mounted in one place instead of the repeated `chatContainer.addChild(...)` + `ui.requestRender()` pattern scattered across controllers. `ChatBlock` carries a React/Svelte-style lifecycle — `onMount` starts effects, `onCleanup` registers teardown, `finish()` self-completes (stops timers and freezes the block at its final content), and `dispose()`/`resetTranscript()` tears everything down — so animated blocks own their own resources instead of leaking `setInterval`/`requestRender` bookkeeping into callers. The MCP "Connecting…" spinner is now such a block. + ### Changed - Changed the directory grouping for `find`, `search`, `ast_grep`, `ast_edit`, and `lsp` diagnostics from a single flat `# dir/` heading per immediate directory to a multi-level tree that folds the common path prefix into one heading. Previously every group repeated the full directory path — so results rooted outside cwd printed the absolute prefix (e.g. `/Users/me/proj/`) on every heading and nested directories were never collapsed. Now a single-child directory chain folds into one heading (`# packages/pkg/src/`, including an absolute root for out-of-cwd results), subdirectories nest one `#` deeper (`## nested/` → `### child.ts`), and each directory's own files are listed before its subdirectories. TUI hyperlink reconstruction tracks the nested directory stack across the whole output so file and code-frame links keep resolving to the correct absolute paths. - Changed the plan-mode approval surface from an inline transcript block plus a separate bottom selector into a single fullscreen overlay (like `/copy`). The overlay owns its entire content via `ScrollView`: the plan renders once as Markdown and scrolls inside the outlined box (PageUp/PageDown, g/G), with the approval options and the model-tier slider beneath it. ↑/↓ move the option cursor, ←/→ drive the slider, Enter confirms, the external-editor key opens the plan, and Esc cancels — so a tall plan no longer competes with the selector for vertical space or clips its head on ED3-risk terminals. +- Changed the interactive controllers (command, MCP, selector, extension-UI, event), debug panels, and the status/error/warning helpers to render chat output through `ctx.present(...)` instead of appending to `chatContainer` and calling `ui.requestRender()` directly; transcript rebuilds dispose live blocks via `ctx.resetTranscript()` so animated blocks' timers stop on reset. ### Fixed @@ -13,6 +18,7 @@ - Fixed `omp dry-balance --bench` flooding the terminal with staircased, duplicated spinner/status lines (and an indented summary) when the tty has ONLCR/OPOST disabled (raw mode). The interactive progress region separated rows with a bare LF and repositioned with a column-preserving `\x1b[A` cursor-up, both of which only land at column 0 when the terminal translates LF→CRLF; with that translation off, every 80 ms redraw cascaded down and to the right into scrollback. The live region now carriage-returns before every cleared row, terminates each row with CRLF, and caps each row to the terminal width so a wrapped line cannot desync the cursor-up from the logical line count. - Fixed inconsistent vertical spacing between transcript blocks: some blocks (tool results from `search`/`find` and other renderer-backed tools) rendered with a doubled gap (a leading `Spacer` plus the content box's own `paddingY`), while others (the grouped `read` card, file-mention lists, IRC cards) rendered with no gap at all. Vertical spacing is now owned entirely by the chat renderer: `TranscriptContainer` strips each block's plain-blank top/bottom edges and inserts exactly one blank line between consecutive blocks, so every block is separated by a single consistent gap regardless of which component produced it. Individual components (assistant/user/tool/read-group/bash/eval/skill/custom/hook/compaction/branch/todo-reminder/plan-review messages) no longer emit their own leading `Spacer`/`paddingY` for separation, and multi-row groups (IRC cards, file-mention lists, completed-job batches, and the bordered command/`/changelog`/`/context`/version/OAuth/debug panels) are wrapped as single `TranscriptBlock` children so the renderer spaces them as one unit. Background-colored box padding is preserved as block-internal design. - Fixed `resolve` with `action: "discard"` surfacing a hard `isError` "No pending action to resolve" failure to the model when the agent asked to cancel a staged action (e.g. an `ast_edit` preview) but nothing was pending. A discard is a request to reach the "no staged change" end-state, which already holds in that case, so it is now honored as a successful cancellation (`"Nothing to discard; no pending action remains."` with `details.action: "discard"`) instead of an error. `action: "apply"` with no pending action still errors. +- Fixed the collapsed tool-output expand hint rendering double brackets (e.g. `((Ctrl+O for more))`) — the `EXPAND_HINT` text already carried its own parentheses and then `formatExpandHint` wrapped it again with the theme's bracket glyphs. The hint now resolves the key actually bound to `app.tools.expand` at render time and reads `⟨: Expand⟩` (e.g. `⟨Ctrl+O: Expand⟩`), so a single bracket pair surrounds it and a user remap of the expand keybinding is reflected instead of a hard-coded `Ctrl+O`. ## [15.10.0] - 2026-06-06 diff --git a/packages/coding-agent/src/debug/index.ts b/packages/coding-agent/src/debug/index.ts index 813afa65b..17840cc4b 100644 --- a/packages/coding-agent/src/debug/index.ts +++ b/packages/coding-agent/src/debug/index.ts @@ -157,8 +157,7 @@ export class DebugSelectorComponent extends Container { block.addChild( new Text(theme.fg("muted", "Reproduce the performance issue, then press Enter to stop profiling."), 1, 0), ); - this.ctx.chatContainer.addChild(block); - this.ctx.ui.requestRender(); + this.ctx.present(block); // Wait for Enter keypress const { promise, resolve } = Promise.withResolvers(); @@ -207,14 +206,12 @@ export class DebugSelectorComponent extends Container { block.addChild(new Text(theme.fg("success", `${theme.status.success} Performance report saved`), 1, 0)); block.addChild(new Text(theme.fg("dim", formatFileHyperlink(result.path)), 1, 0)); block.addChild(new Text(theme.fg("dim", `Files: ${result.files.length}`), 1, 0)); - this.ctx.chatContainer.addChild(block); + this.ctx.present(block); } catch (err) { loader.stop(); this.ctx.statusContainer.clear(); this.ctx.showError(`Failed to create report: ${err instanceof Error ? err.message : String(err)}`); } - - this.ctx.ui.requestRender(); } async #handleWorkReport(): Promise { @@ -232,15 +229,13 @@ export class DebugSelectorComponent extends Container { openPath(tmpPath); - this.ctx.chatContainer.addChild(new Spacer(1)); - this.ctx.chatContainer.addChild( + this.ctx.present([ + new Spacer(1), new Text(theme.fg("dim", `Opened flamegraph (${workProfile.sampleCount} samples)`), 1, 0), - ); + ]); } catch (err) { this.ctx.showError(`Failed to open profile: ${err instanceof Error ? err.message : String(err)}`); } - - this.ctx.ui.requestRender(); } async #handleDumpReport(): Promise { @@ -267,14 +262,12 @@ export class DebugSelectorComponent extends Container { block.addChild(new Text(theme.fg("success", `${theme.status.success} Report bundle saved`), 1, 0)); block.addChild(new Text(theme.fg("dim", formatFileHyperlink(result.path)), 1, 0)); block.addChild(new Text(theme.fg("dim", `Files: ${result.files.length}`), 1, 0)); - this.ctx.chatContainer.addChild(block); + this.ctx.present(block); } catch (err) { loader.stop(); this.ctx.statusContainer.clear(); this.ctx.showError(`Failed to create report: ${err instanceof Error ? err.message : String(err)}`); } - - this.ctx.ui.requestRender(); } async #handleMemoryReport(): Promise { @@ -305,14 +298,12 @@ export class DebugSelectorComponent extends Container { block.addChild(new Text(theme.fg("success", `${theme.status.success} Memory report saved`), 1, 0)); block.addChild(new Text(theme.fg("dim", formatFileHyperlink(result.path)), 1, 0)); block.addChild(new Text(theme.fg("dim", `Files: ${result.files.length}`), 1, 0)); - this.ctx.chatContainer.addChild(block); + this.ctx.present(block); } catch (err) { loader.stop(); this.ctx.statusContainer.clear(); this.ctx.showError(`Failed to create report: ${err instanceof Error ? err.message : String(err)}`); } - - this.ctx.ui.requestRender(); } async #handleViewLogs(): Promise { @@ -368,12 +359,10 @@ export class DebugSelectorComponent extends Container { block.addChild(new DynamicBorder()); block.addChild(new Text(formatted, 1, 0)); block.addChild(new DynamicBorder()); - this.ctx.chatContainer.addChild(block); + this.ctx.present(block); } catch (err) { this.ctx.showError(`Failed to collect system info: ${err instanceof Error ? err.message : String(err)}`); } - - this.ctx.ui.requestRender(); } async #handleViewTerminalState(): Promise { @@ -388,8 +377,7 @@ export class DebugSelectorComponent extends Container { block.addChild(new DynamicBorder()); block.addChild(new Text(formatted, 1, 0)); block.addChild(new DynamicBorder()); - this.ctx.chatContainer.addChild(block); - this.ctx.ui.requestRender(); + this.ctx.present(block); } async #handleViewProtocols(): Promise { @@ -408,15 +396,14 @@ export class DebugSelectorComponent extends Container { TERMINAL.sendNotification(notification); } - this.ctx.chatContainer.addChild(new Spacer(1)); - this.ctx.chatContainer.addChild( + this.ctx.present([ + new Spacer(1), new ProtocolProbeComponent({ image: buildSampleImage(), imageBudget: this.ctx.ui.imageBudget, notificationSuppressed: suppressed, }), - ); - this.ctx.ui.requestRender(); + ]); } async #handleTranscriptExport(): Promise { @@ -488,21 +475,19 @@ export class DebugSelectorComponent extends Container { loader.stop(); this.ctx.statusContainer.clear(); - this.ctx.chatContainer.addChild(new Spacer(1)); - this.ctx.chatContainer.addChild( + this.ctx.present([ + new Spacer(1), new Text( theme.fg("success", `${theme.status.success} Cleared ${result.removed} artifact directories`), 1, 0, ), - ); + ]); } catch (err) { loader.stop(); this.ctx.statusContainer.clear(); this.ctx.showError(`Failed to clear cache: ${err instanceof Error ? err.message : String(err)}`); } - - this.ctx.ui.requestRender(); } #getResolvedSettings(): Record { diff --git a/packages/coding-agent/src/modes/components/chat-block.ts b/packages/coding-agent/src/modes/components/chat-block.ts new file mode 100644 index 000000000..da79c2041 --- /dev/null +++ b/packages/coding-agent/src/modes/components/chat-block.ts @@ -0,0 +1,111 @@ +import { Container } from "@oh-my-pi/pi-tui"; + +/** + * Capabilities a mounted {@link ChatBlock} may use against its host transcript. + * Kept minimal so blocks never reach into the full TUI/InteractiveMode surface. + */ +export interface ChatBlockHost { + /** Schedule a repaint of the transcript. */ + requestRender(): void; +} + +/** + * Lifecycle-aware transcript block — the "return a block, let the host mount it" + * primitive, modelled on React/Svelte component lifecycles. + * + * Producers build and return a `ChatBlock` instead of poking `chatContainer` and + * `ui.requestRender()` directly. The host (`ctx.present`) appends it and calls + * {@link mount}, which runs {@link onMount}; effects started there register + * teardown via {@link onCleanup}. The block repaints through {@link requestRender} + * — never touching the TUI — and tears down exactly once on {@link finish} + * (self-complete: stop the animation, keep the final frame in the transcript) or + * {@link dispose} (host discards it, e.g. a transcript reset). + * + * While mounted and unfinished a block reports `isTranscriptBlockFinalized() === + * false` so {@link "../components/transcript-container".TranscriptContainer} + * keeps it in the live, repaintable region on ED3-risk terminals; after + * `finish()`/`dispose()` it reports `true` and freezes at its final content. + */ +export abstract class ChatBlock extends Container { + #host: ChatBlockHost | undefined; + #cleanups: Array<() => void> = []; + #active = false; + #disposed = false; + + /** + * Run setup after the block is in the transcript: start timers/subscriptions + * and register their teardown with {@link onCleanup}. Default: no-op (a block + * whose content is fixed at construction needs no mount work). + */ + protected onMount(): void {} + + /** + * Register a teardown to run on {@link finish}/{@link dispose}, à la a + * `useEffect` cleanup. If the block is already disposed the cleanup runs + * immediately so callers never leak. + */ + protected onCleanup(cleanup: () => void): void { + if (this.#disposed) { + cleanup(); + return; + } + this.#cleanups.push(cleanup); + } + + /** Ask the host to repaint. No-op before mount or after dispose. */ + protected requestRender(): void { + this.#host?.requestRender(); + } + + /** True between {@link mount} and {@link finish}/{@link dispose}. */ + protected get active(): boolean { + return this.#active; + } + + /** + * Host-only: attach the host and run {@link onMount}. Idempotent — a second + * call (e.g. a transcript rebuild that re-presents the same instance) is a + * no-op. + */ + mount(host: ChatBlockHost): void { + if (this.#host || this.#disposed) return; + this.#host = host; + this.#active = true; + this.onMount(); + } + + /** + * Self-complete: stop ongoing effects and freeze the block at its current + * content, leaving it rendered in the transcript. Use when the operation the + * block represents finishes (connection resolved, download done). + */ + finish(): void { + if (!this.#active) return; + this.#active = false; + this.#runCleanups(); + this.requestRender(); + } + + /** + * Host-only teardown: release everything and propagate to children. Called + * when the host permanently discards the block (transcript reset). Idempotent. + */ + override dispose(): void { + if (this.#disposed) return; + this.#disposed = true; + this.#active = false; + this.#runCleanups(); + super.dispose(); + this.#host = undefined; + } + + /** Live blocks stay repaintable; finished/disposed ones may freeze. */ + isTranscriptBlockFinalized(): boolean { + return !this.#active; + } + + #runCleanups(): void { + const cleanups = this.#cleanups.splice(0); + for (const cleanup of cleanups) cleanup(); + } +} diff --git a/packages/coding-agent/src/modes/controllers/command-controller-shared.ts b/packages/coding-agent/src/modes/controllers/command-controller-shared.ts index 7ee9c9499..2e3eb6471 100644 --- a/packages/coding-agent/src/modes/controllers/command-controller-shared.ts +++ b/packages/coding-agent/src/modes/controllers/command-controller-shared.ts @@ -105,6 +105,5 @@ export function showCommandMessage(ctx: InteractiveModeContext, text: string): v block.addChild(new DynamicBorder()); block.addChild(new Text(text, 1, 1)); block.addChild(new DynamicBorder()); - ctx.chatContainer.addChild(block); - ctx.ui.requestRender(); + ctx.present(block); } diff --git a/packages/coding-agent/src/modes/controllers/command-controller.ts b/packages/coding-agent/src/modes/controllers/command-controller.ts index 942392430..c19f93269 100644 --- a/packages/coding-agent/src/modes/controllers/command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/command-controller.ts @@ -53,8 +53,7 @@ function showMarkdownPanel(ctx: InteractiveModeContext, title: string, markdown: block.addChild(new Spacer(1)); block.addChild(new Markdown(markdown.trim(), 1, 1, getMarkdownTheme())); block.addChild(new DynamicBorder()); - ctx.chatContainer.addChild(block); - ctx.ui.requestRender(); + ctx.present(block); } export class CommandController { @@ -336,9 +335,7 @@ export class CommandController { } } - this.ctx.chatContainer.addChild(new Spacer(1)); - this.ctx.chatContainer.addChild(new Text(info, 1, 0)); - this.ctx.ui.requestRender(); + this.ctx.present([new Spacer(1), new Text(info, 1, 0)]); } async handleJobsCommand(): Promise { @@ -355,9 +352,7 @@ export class CommandController { if (snapshot.running.length === 0 && snapshot.recent.length === 0) { info += `\n${theme.fg("dim", "No async jobs yet.")}\n`; - this.ctx.chatContainer.addChild(new Spacer(1)); - this.ctx.chatContainer.addChild(new Text(info, 1, 0)); - this.ctx.ui.requestRender(); + this.ctx.present([new Spacer(1), new Text(info, 1, 0)]); return; } @@ -377,9 +372,7 @@ export class CommandController { } } - this.ctx.chatContainer.addChild(new Spacer(1)); - this.ctx.chatContainer.addChild(new Text(info.trimEnd(), 1, 0)); - this.ctx.ui.requestRender(); + this.ctx.present([new Spacer(1), new Text(info.trimEnd(), 1, 0)]); } async handleUsageCommand(reports?: UsageReport[] | null): Promise { @@ -405,9 +398,7 @@ export class CommandController { const availableWidth = Math.max(40, (this.ctx.ui.terminal.columns ?? 100) - 2); const output = renderUsageReports(usageReports, theme, Date.now(), availableWidth); - this.ctx.chatContainer.addChild(new Spacer(1)); - this.ctx.chatContainer.addChild(new Text(output, 1, 0)); - this.ctx.ui.requestRender(); + this.ctx.present([new Spacer(1), new Text(output, 1, 0)]); } async handleChangelogCommand(showFull = false): Promise { @@ -434,8 +425,7 @@ export class CommandController { block.addChild(new Spacer(1)); block.addChild(new Markdown(changelogMarkdown + hint, 1, 1, getMarkdownTheme())); block.addChild(new DynamicBorder()); - this.ctx.chatContainer.addChild(block); - this.ctx.ui.requestRender(); + this.ctx.present(block); } handleHotkeysCommand(): void { @@ -461,8 +451,7 @@ export class CommandController { block.addChild(new Spacer(1)); block.addChild(new Text(output, 1, 0)); block.addChild(new DynamicBorder()); - this.ctx.chatContainer.addChild(block); - this.ctx.ui.requestRender(); + this.ctx.present(block); } async handleMemoryCommand(text: string): Promise { @@ -483,8 +472,7 @@ export class CommandController { block.addChild(new Spacer(1)); block.addChild(new Markdown(payload, 1, 1, getMarkdownTheme())); block.addChild(new DynamicBorder()); - this.ctx.chatContainer.addChild(block); - this.ctx.ui.requestRender(); + this.ctx.present(block); return; } @@ -805,8 +793,7 @@ export class CommandController { this.ctx.streamingMessage = undefined; this.ctx.pendingTools.clear(); - this.ctx.chatContainer.addChild(new Spacer(1)); - this.ctx.chatContainer.addChild(new Text(`${theme.fg("accent", `${theme.status.success} ${label}`)}`, 1, 1)); + this.ctx.present([new Spacer(1), new Text(`${theme.fg("accent", `${theme.status.success} ${label}`)}`, 1, 1)]); await this.ctx.reloadTodos(); this.ctx.ui.requestRender(true, { clearScrollback: true }); } @@ -845,11 +832,10 @@ export class CommandController { const sessionFile = this.ctx.session.sessionFile; const shortPath = sessionFile ? sessionFile.split("/").pop() : "new session"; - this.ctx.chatContainer.addChild(new Spacer(1)); - this.ctx.chatContainer.addChild( + this.ctx.present([ + new Spacer(1), new Text(`${theme.fg("accent", `${theme.status.success} Session forked to ${shortPath}`)}`, 1, 1), - ); - this.ctx.ui.requestRender(); + ]); } async handleMoveCommand(targetPath: string): Promise { @@ -883,11 +869,10 @@ export class CommandController { await this.ctx.sessionManager.moveTo(resolvedPath); await this.ctx.applyCwdChange(resolvedPath); - this.ctx.chatContainer.addChild(new Spacer(1)); - this.ctx.chatContainer.addChild( + this.ctx.present([ + new Spacer(1), new Text(`${theme.fg("accent", `${theme.status.success} Session moved to ${resolvedPath}`)}`, 1, 1), - ); - this.ctx.ui.requestRender(); + ]); } catch (err) { this.ctx.showError(`Move failed: ${err instanceof Error ? err.message : String(err)}`); } @@ -918,7 +903,7 @@ export class CommandController { this.ctx.pendingMessagesContainer.addChild(this.ctx.bashComponent); this.ctx.pendingBashComponents.push(this.ctx.bashComponent); } else { - this.ctx.chatContainer.addChild(this.ctx.bashComponent); + this.ctx.present(this.ctx.bashComponent); } this.ctx.ui.requestRender(); @@ -959,7 +944,7 @@ export class CommandController { this.ctx.pendingMessagesContainer.addChild(this.ctx.pythonComponent); this.ctx.pendingPythonComponents.push(this.ctx.pythonComponent); } else { - this.ctx.chatContainer.addChild(this.ctx.pythonComponent); + this.ctx.present(this.ctx.pythonComponent); } this.ctx.ui.requestRender(); @@ -1148,10 +1133,10 @@ export class CommandController { this.ctx.updateEditorBorderColor(); await this.ctx.reloadTodos(); - this.ctx.chatContainer.addChild(new Spacer(1)); - this.ctx.chatContainer.addChild( + this.ctx.present([ + new Spacer(1), new Text(`${theme.fg("accent", `${theme.status.success} New session started with handoff context`)}`, 1, 1), - ); + ]); if (result.savedPath) { this.ctx.showStatus(`Handoff document saved to: ${result.savedPath}`); } diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index 66497e783..bdaeab4ec 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -830,14 +830,12 @@ export class EventController { async #handleTtsrTriggered(event: Extract): Promise { const component = new TtsrNotificationComponent(event.rules); component.setExpanded(this.ctx.toolOutputExpanded); - this.ctx.chatContainer.addChild(component); - this.ctx.ui.requestRender(); + this.ctx.present(component); } async #handleTodoReminder(event: Extract): Promise { const component = new TodoReminderComponent(event.todos, event.attempt, event.maxAttempts); - this.ctx.chatContainer.addChild(component); - this.ctx.ui.requestRender(); + this.ctx.present(component); } async #handleTodoAutoClear(_event: Extract): Promise { diff --git a/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts b/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts index e2b994fce..8f09b645b 100644 --- a/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts +++ b/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts @@ -176,10 +176,10 @@ export class ExtensionUiController { this.ctx.streamingMessage = undefined; this.ctx.pendingTools.clear(); - this.ctx.chatContainer.addChild(new Spacer(1)); - this.ctx.chatContainer.addChild( + this.ctx.present([ + new Spacer(1), new Text(`${theme.fg("accent", `${theme.status.success} New session started`)}`, 1, 1), - ); + ]); await this.ctx.reloadTodos(); this.ctx.ui.requestRender(true, { clearScrollback: true }); @@ -415,10 +415,10 @@ export class ExtensionUiController { this.ctx.streamingMessage = undefined; this.ctx.pendingTools.clear(); - this.ctx.chatContainer.addChild(new Spacer(1)); - this.ctx.chatContainer.addChild( + this.ctx.present([ + new Spacer(1), new Text(`${theme.fg("accent", `${theme.status.success} New session started`)}`, 1, 1), - ); + ]); await this.ctx.reloadTodos(); this.ctx.ui.requestRender(true, { clearScrollback: true }); @@ -562,8 +562,7 @@ export class ExtensionUiController { return; } const errorText = new Text(theme.fg("error", `Tool "${toolName}" error: ${error}`), 1, 0); - this.ctx.chatContainer.addChild(errorText); - this.ctx.ui.requestRender(); + this.ctx.present(errorText); } /** @@ -860,8 +859,7 @@ export class ExtensionUiController { showExtensionError(extensionPath: string, error: string): void { const errorText = new Text(theme.fg("error", `Extension "${extensionPath}" error: ${error}`), 1, 0); - this.ctx.chatContainer.addChild(errorText); - this.ctx.ui.requestRender(); + this.ctx.present(errorText); } async #handleInteractiveCompact(instructionsOrOptions: string | CompactOptions | undefined): Promise { if (this.ctx.isBackgrounded) { diff --git a/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts b/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts index 86fb385af..c63e57162 100644 --- a/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/mcp-command-controller.ts @@ -37,6 +37,7 @@ import type { MCPAuthConfig, MCPServerConfig, MCPServerConnection } from "../../ import type { OAuthCredential } from "../../session/auth-storage"; import { shortenPath } from "../../tools/render-utils"; import { openPath } from "../../utils/open"; +import { ChatBlock } from "../components/chat-block"; import { MCPAddWizard } from "../components/mcp-add-wizard"; import { TranscriptBlock } from "../components/transcript-container"; import { parseCommandArgs } from "../shared"; @@ -50,6 +51,42 @@ function withTimeout(promise: Promise, timeoutMs: number, message: string) return Promise.race([promise, timeoutPromise]).finally(() => clearTimeout(timer)); } +/** + * Animated "Connecting to …" transcript block. Owns its spinner interval: it + * starts on mount and is cleared on {@link ChatBlock.finish}/dispose, so callers + * never juggle `setInterval`/`clearInterval` or `requestRender` by hand. + */ +class McpConnectingBlock extends ChatBlock { + readonly #text: Text; + + constructor(private readonly serverName: string) { + super(); + this.addChild(new Spacer(1)); + const frame = theme.spinnerFrames[0] ?? "|"; + this.#text = new Text(theme.fg("muted", `${frame} Connecting to "${serverName}"...`), 1, 0); + this.addChild(this.#text); + } + + protected override onMount(): void { + const frames = theme.spinnerFrames; + let frame = 0; + const interval = setInterval(() => { + frame++; + this.#text.setText( + theme.fg("muted", `${frames[frame % frames.length] ?? "|"} Connecting to "${this.serverName}"...`), + ); + this.requestRender(); + }, 80); + this.onCleanup(() => clearInterval(interval)); + } + + /** Replace the spinner line with a terminal status; pair with {@link finish}. */ + setStatus(text: string): void { + this.#text.setText(text); + this.requestRender(); + } +} + /** * Outcome of {@link MCPCommandController}'s OAuth handler. * @@ -550,7 +587,7 @@ export class MCPCommandController { onAuth: (info: { url: string; instructions?: string }) => { // Show auth URL prominently in chat as one block const block = new TranscriptBlock(); - this.ctx.chatContainer.addChild(block); + this.ctx.present(block); block.addChild(new Text(theme.fg("accent", "━━━ OAuth Authorization Required ━━━"), 1, 0)); block.addChild(new Spacer(1)); block.addChild(new Text(theme.fg("muted", "Preparing browser authorization..."), 1, 0)); @@ -564,7 +601,6 @@ export class MCPCommandController { ); block.addChild(new Spacer(1)); block.addChild(new Text(theme.fg("accent", "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"), 1, 0)); - this.ctx.ui.requestRender(); // Try to open browser automatically try { openPath(info.url); @@ -587,9 +623,7 @@ export class MCPCommandController { } }, onProgress: (message: string) => { - this.ctx.chatContainer.addChild(new Spacer(1)); - this.ctx.chatContainer.addChild(new Text(theme.fg("muted", message), 1, 0)); - this.ctx.ui.requestRender(); + this.ctx.present([new Spacer(1), new Text(theme.fg("muted", message), 1, 0)]); }, }, ); @@ -597,9 +631,10 @@ export class MCPCommandController { // Execute OAuth flow with 5 minute timeout const credentials = await withTimeout(flow.login(), 5 * 60 * 1000, "OAuth flow timed out after 5 minutes"); - this.ctx.chatContainer.addChild(new Spacer(1)); - this.ctx.chatContainer.addChild(new Text(theme.fg("success", "✓ Authorization completed in browser."), 1, 0)); - this.ctx.ui.requestRender(); + this.ctx.present([ + new Spacer(1), + new Text(theme.fg("success", "✓ Authorization completed in browser."), 1, 0), + ]); // Generate a unique credential ID const credentialId = `mcp_oauth_${Date.now()}_${Math.random().toString(36).slice(2, 11)}`; @@ -752,19 +787,8 @@ export class MCPCommandController { ): Promise<"connected" | "connecting" | "disconnected"> { if (!this.ctx.mcpManager) return "disconnected"; - this.ctx.chatContainer.addChild(new Spacer(1)); - const frames = theme.spinnerFrames; - const initialFrame = frames[0] ?? "|"; - const statusText = new Text(theme.fg("muted", `${initialFrame} Connecting to "${name}"...`), 1, 0); - this.ctx.chatContainer.addChild(statusText); - this.ctx.ui.requestRender(); - - let frame = 0; - const interval = setInterval(() => { - statusText.setText(theme.fg("muted", `${frames[frame % frames.length]} Connecting to "${name}"...`)); - frame++; - this.ctx.ui.requestRender(); - }, 80); + const block = new McpConnectingBlock(name); + this.ctx.present(block); try { try { @@ -778,20 +802,19 @@ export class MCPCommandController { await this.ctx.session.refreshMCPTools(this.ctx.mcpManager.getTools()); } if (state === "connected") { - statusText.setText(theme.fg("success", `✓ Connected to "${name}"`)); + block.setStatus(theme.fg("success", `✓ Connected to "${name}"`)); } else if (state === "connecting") { - statusText.setText(theme.fg("muted", `◌ "${name}" is still connecting...`)); + block.setStatus(theme.fg("muted", `◌ "${name}" is still connecting...`)); } else { - statusText.setText( + block.setStatus( options?.suppressDisconnectedWarning ? theme.fg("muted", `◌ Connection check complete for "${name}"`) : theme.fg("warning", `⚠ Could not connect to "${name}" yet`), ); } - this.ctx.ui.requestRender(); return state; } finally { - clearInterval(interval); + block.finish(); } } diff --git a/packages/coding-agent/src/modes/controllers/selector-controller.ts b/packages/coding-agent/src/modes/controllers/selector-controller.ts index 7aa831822..2d2728db2 100644 --- a/packages/coding-agent/src/modes/controllers/selector-controller.ts +++ b/packages/coding-agent/src/modes/controllers/selector-controller.ts @@ -933,7 +933,6 @@ export class SelectorController { await this.ctx.session.modelRegistry.authStorage.login(providerId as OAuthProvider, { onAuth: (info: { url: string; instructions?: string }) => { const block = new TranscriptBlock(); - this.ctx.chatContainer.addChild(block); block.addChild(new Text(theme.fg("dim", info.url), 1, 0)); const hyperlink = `\x1b]8;;${info.url}\x07Click here to login\x1b]8;;\x07`; block.addChild(new Text(theme.fg("accent", hyperlink), 1, 0)); @@ -945,17 +944,16 @@ export class SelectorController { block.addChild(new Spacer(1)); block.addChild(new Text(theme.fg("dim", MANUAL_LOGIN_TIP), 1, 0)); } - this.ctx.ui.requestRender(); + this.ctx.present(block); this.ctx.openInBrowser(info.url); }, onPrompt: async (prompt: { message: string; placeholder?: string }) => { const promptBlock = new TranscriptBlock(); - this.ctx.chatContainer.addChild(promptBlock); promptBlock.addChild(new Text(theme.fg("warning", prompt.message), 1, 0)); if (prompt.placeholder) { promptBlock.addChild(new Text(theme.fg("dim", prompt.placeholder), 1, 0)); } - this.ctx.ui.requestRender(); + this.ctx.present(promptBlock); const { promise, resolve } = Promise.withResolvers(); const codeInput = new Input(); codeInput.onSubmit = () => { @@ -972,8 +970,7 @@ export class SelectorController { return promise; }, onProgress: (message: string) => { - this.ctx.chatContainer.addChild(new Text(theme.fg("dim", message), 1, 0)); - this.ctx.ui.requestRender(); + this.ctx.present(new Text(theme.fg("dim", message), 1, 0)); }, onManualCodeInput: useManualInput ? () => manualInput.waitForInput(providerId) : undefined, }); @@ -983,8 +980,7 @@ export class SelectorController { new Text(theme.fg("success", `${theme.status.success} Successfully logged in to ${providerId}`), 1, 0), ); block.addChild(new Text(theme.fg("dim", `Credentials saved to ${getAgentDbPath()}`), 1, 0)); - this.ctx.chatContainer.addChild(block); - this.ctx.ui.requestRender(); + this.ctx.present(block); } catch (error: unknown) { this.ctx.showError(`Login failed: ${error instanceof Error ? error.message : String(error)}`); } finally { @@ -1017,8 +1013,7 @@ export class SelectorController { new Text(theme.fg("warning", `${providerId} is still authenticated via ${remainingSource}`), 1, 0), ); } - this.ctx.chatContainer.addChild(block); - this.ctx.ui.requestRender(); + this.ctx.present(block); } catch (error: unknown) { this.ctx.showError(`Logout failed: ${error instanceof Error ? error.message : String(error)}`); } diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 6abe5cb10..eb5b8b630 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -94,6 +94,7 @@ import { getSessionAccentAnsi, getSessionAccentHex } from "../utils/session-colo import { popTerminalTitle, pushTerminalTitle, setSessionTerminalTitle } from "../utils/title-generator"; import type { AssistantMessageComponent } from "./components/assistant-message"; import type { BashExecutionComponent } from "./components/bash-execution"; +import { ChatBlock, type ChatBlockHost } from "./components/chat-block"; import { CustomEditor } from "./components/custom-editor"; import { DynamicBorder } from "./components/dynamic-border"; import { ErrorBannerComponent } from "./components/error-banner"; @@ -367,6 +368,7 @@ export class InteractiveMode implements InteractiveModeContext { #eventBus?: EventBus; #eventBusUnsubscribers: Array<() => void> = []; #welcomeComponent?: WelcomeComponent; + readonly #chatHost: ChatBlockHost = { requestRender: () => this.ui.requestRender() }; constructor( session: AgentSession, @@ -2529,6 +2531,25 @@ export class InteractiveMode implements InteractiveModeContext { } // UI helpers + present(content: Component | readonly Component[]): void { + if (Array.isArray(content)) { + for (const item of content) this.#mountChatChild(item); + } else { + this.#mountChatChild(content as Component); + } + this.ui.requestRender(); + } + + #mountChatChild(item: Component): void { + this.chatContainer.addChild(item); + if (item instanceof ChatBlock) item.mount(this.#chatHost); + } + + resetTranscript(): void { + this.chatContainer.dispose(); + this.chatContainer.clear(); + } + showStatus(message: string, options?: { dim?: boolean }): void { this.#uiHelpers.showStatus(message, options); } diff --git a/packages/coding-agent/src/modes/types.ts b/packages/coding-agent/src/modes/types.ts index de2cfa880..c49595d4e 100644 --- a/packages/coding-agent/src/modes/types.ts +++ b/packages/coding-agent/src/modes/types.ts @@ -158,6 +158,20 @@ export interface InteractiveModeContext { handleBackgroundEvent(event: AgentSessionEvent): Promise; // UI helpers + /** + * Mount transcript content and repaint once. The single sink for "show this in + * chat": producers build and return a `Component` (or a `ChatBlock` carrying + * its own lifecycle) and hand it here instead of touching `chatContainer` / + * `ui.requestRender()` directly. `ChatBlock`s are mounted (their `onMount` + * runs) so their timers/subscriptions start. + */ + present(content: Component | readonly Component[]): void; + /** + * Dispose every live block in the transcript (stopping timers/subscriptions) + * and clear it. Used before a full rebuild so animated/streaming blocks do not + * leak. + */ + resetTranscript(): void; showStatus(message: string, options?: { dim?: boolean }): void; showError(message: string): void; showPinnedError(message: string): void; diff --git a/packages/coding-agent/src/modes/utils/ui-helpers.ts b/packages/coding-agent/src/modes/utils/ui-helpers.ts index fca5bea57..140a43d4b 100644 --- a/packages/coding-agent/src/modes/utils/ui-helpers.ts +++ b/packages/coding-agent/src/modes/utils/ui-helpers.ts @@ -91,11 +91,9 @@ export class UiHelpers { const spacer = new Spacer(1); const text = new Text(rendered, 1, 0); - this.ctx.chatContainer.addChild(spacer); - this.ctx.chatContainer.addChild(text); + this.ctx.present([spacer, text]); this.ctx.lastStatusSpacer = spacer; this.ctx.lastStatusText = text; - this.ctx.ui.requestRender(); } addMessageToChat( @@ -492,8 +490,15 @@ export class UiHelpers { renderInitialMessages(prebuiltContext?: SessionContext, options: RenderInitialMessagesOptions = {}): void { // This path is used to rebuild the visible chat transcript (e.g. after custom/debug UI). // Clear existing rendered chat first to avoid duplicating the full session in the container. + // On a non-preserving rebuild the existing blocks are discarded for good, so + // dispose them (stopping any live timers/subscriptions) before clearing. When + // preserving, the same instances are re-added below, so detach without dispose. const preservedChatChildren = options.preserveExistingChat ? this.ctx.chatContainer.children : undefined; - this.ctx.chatContainer.clear(); + if (preservedChatChildren) { + this.ctx.chatContainer.clear(); + } else { + this.ctx.resetTranscript(); + } this.ctx.pendingMessagesContainer.clear(); this.ctx.pendingBashComponents = []; this.ctx.pendingPythonComponents = []; @@ -544,9 +549,7 @@ export class UiHelpers { process.stderr.write(`Error: ${errorMessage}\n`); return; } - this.ctx.chatContainer.addChild(new Spacer(1)); - this.ctx.chatContainer.addChild(new Text(theme.fg("error", `Error: ${errorMessage}`), 1, 0)); - this.ctx.ui.requestRender(); + this.ctx.present([new Spacer(1), new Text(theme.fg("error", `Error: ${errorMessage}`), 1, 0)]); } showWarning(warningMessage: string): void { @@ -554,9 +557,7 @@ export class UiHelpers { process.stderr.write(`Warning: ${warningMessage}\n`); return; } - this.ctx.chatContainer.addChild(new Spacer(1)); - this.ctx.chatContainer.addChild(new Text(theme.fg("warning", `Warning: ${warningMessage}`), 1, 0)); - this.ctx.ui.requestRender(); + this.ctx.present([new Spacer(1), new Text(theme.fg("warning", `Warning: ${warningMessage}`), 1, 0)]); } showNewVersionNotification(newVersion: string): void { @@ -573,8 +574,7 @@ export class UiHelpers { ), ); block.addChild(new DynamicBorder(text => theme.fg("warning", text))); - this.ctx.chatContainer.addChild(block); - this.ctx.ui.requestRender(); + this.ctx.present(block); } updatePendingMessagesDisplay(): void { diff --git a/packages/coding-agent/test/interactive-mode-status.test.ts b/packages/coding-agent/test/interactive-mode-status.test.ts index 92bfc8987..aae1a575a 100644 --- a/packages/coding-agent/test/interactive-mode-status.test.ts +++ b/packages/coding-agent/test/interactive-mode-status.test.ts @@ -4,7 +4,7 @@ import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types"; import { UiHelpers } from "@oh-my-pi/pi-coding-agent/modes/utils/ui-helpers"; import { buildSessionContext, type SessionContext } from "@oh-my-pi/pi-coding-agent/session/session-manager"; -import { Container } from "@oh-my-pi/pi-tui"; +import { type Component, Container } from "@oh-my-pi/pi-tui"; function renderLastLine(container: Container, width = 120): string { const last = container.children[container.children.length - 1]; @@ -25,6 +25,11 @@ function createInitialRenderHarness(): { ctx: InteractiveModeContext; helpers: U pendingPythonComponents: [], pendingTools: new Map(), ui: { requestRender: vi.fn() }, + present: (content: Component | readonly Component[]) => { + const items = Array.isArray(content) ? content : [content]; + for (const item of items) ctx.chatContainer.addChild(item); + ctx.ui.requestRender(); + }, isBackgrounded: false, sessionManager: { buildSessionContext: () => buildSessionContext([]), @@ -59,6 +64,11 @@ describe("InteractiveMode.showStatus", () => { const ctx = { chatContainer: new Container(), ui: { requestRender: vi.fn() }, + present: (content: Component | readonly Component[]) => { + const items = Array.isArray(content) ? content : [content]; + for (const item of items) ctx.chatContainer.addChild(item); + ctx.ui.requestRender(); + }, isBackgrounded: false, lastStatusSpacer: undefined, lastStatusText: undefined, @@ -80,6 +90,11 @@ describe("InteractiveMode.showStatus", () => { const ctx = { chatContainer: new Container(), ui: { requestRender: vi.fn() }, + present: (content: Component | readonly Component[]) => { + const items = Array.isArray(content) ? content : [content]; + for (const item of items) ctx.chatContainer.addChild(item); + ctx.ui.requestRender(); + }, isBackgrounded: false, lastStatusSpacer: undefined, lastStatusText: undefined, diff --git a/packages/coding-agent/test/issue-956-repro.test.ts b/packages/coding-agent/test/issue-956-repro.test.ts index f7a1b38c7..1d1f4872b 100644 --- a/packages/coding-agent/test/issue-956-repro.test.ts +++ b/packages/coding-agent/test/issue-956-repro.test.ts @@ -80,6 +80,10 @@ describe("issue #956: interactive /mcp test", () => { const disconnectServer = vi.spyOn(mcpClient, "disconnectServer").mockResolvedValue(); const controller = new MCPCommandController({ chatContainer: { addChild }, + present: (content: unknown) => { + for (const item of Array.isArray(content) ? content : [content]) addChild(item); + requestRender(); + }, ui: { requestRender }, editor: {}, showError, diff --git a/packages/coding-agent/test/modes/components/chat-block.test.ts b/packages/coding-agent/test/modes/components/chat-block.test.ts new file mode 100644 index 000000000..703504b9d --- /dev/null +++ b/packages/coding-agent/test/modes/components/chat-block.test.ts @@ -0,0 +1,143 @@ +import { beforeEach, describe, expect, it } from "bun:test"; +import { ChatBlock, type ChatBlockHost } from "@oh-my-pi/pi-coding-agent/modes/components/chat-block"; +import type { Component } from "@oh-my-pi/pi-tui"; + +/** Concrete subclass exposing the protected lifecycle seams for assertions. */ +class TestBlock extends ChatBlock { + mountCount = 0; + cleanupCount = 0; + + protected override onMount(): void { + this.mountCount++; + this.onCleanup(() => { + this.cleanupCount++; + }); + } + + /** Public proxy for the protected requestRender. */ + ping(): void { + this.requestRender(); + } + + /** Public proxy for the protected onCleanup. */ + register(cleanup: () => void): void { + this.onCleanup(cleanup); + } +} + +describe("ChatBlock lifecycle", () => { + let renders: number; + let host: ChatBlockHost; + + beforeEach(() => { + renders = 0; + host = { + requestRender: () => { + renders++; + }, + }; + }); + + it("runs onMount exactly once; a second mount is a no-op", () => { + const block = new TestBlock(); + expect(block.mountCount).toBe(0); + block.mount(host); + block.mount(host); + expect(block.mountCount).toBe(1); + }); + + it("is finalized until mounted, live while active, finalized after finish", () => { + const block = new TestBlock(); + expect(block.isTranscriptBlockFinalized()).toBe(true); + block.mount(host); + expect(block.isTranscriptBlockFinalized()).toBe(false); + block.finish(); + expect(block.isTranscriptBlockFinalized()).toBe(true); + }); + + it("finish runs cleanups once and requests one render", () => { + const block = new TestBlock(); + block.mount(host); + const before = renders; + block.finish(); + expect(block.cleanupCount).toBe(1); + expect(renders).toBe(before + 1); + block.finish(); + expect(block.cleanupCount).toBe(1); + }); + + it("dispose runs cleanups once, is idempotent, and finalizes", () => { + const block = new TestBlock(); + block.mount(host); + block.dispose(); + expect(block.cleanupCount).toBe(1); + expect(block.isTranscriptBlockFinalized()).toBe(true); + block.dispose(); + expect(block.cleanupCount).toBe(1); + }); + + it("finish then dispose does not double-run cleanups", () => { + const block = new TestBlock(); + block.mount(host); + block.finish(); + block.dispose(); + expect(block.cleanupCount).toBe(1); + }); + + it("requestRender routes to the host only between mount and dispose", () => { + const block = new TestBlock(); + block.ping(); + expect(renders).toBe(0); + block.mount(host); + block.ping(); + expect(renders).toBe(1); + block.dispose(); + const after = renders; + block.ping(); + expect(renders).toBe(after); + }); + + it("onCleanup registered after dispose runs immediately so callers never leak", () => { + const block = new TestBlock(); + block.mount(host); + block.dispose(); + let ran = false; + block.register(() => { + ran = true; + }); + expect(ran).toBe(true); + }); + + it("dispose propagates to child components", () => { + const block = new TestBlock(); + let childDisposed = 0; + const child: Component = { + render: () => [], + invalidate: () => {}, + dispose: () => { + childDisposed++; + }, + }; + block.addChild(child); + block.mount(host); + block.dispose(); + expect(childDisposed).toBe(1); + }); + + it("tears down a timer effect started in onMount when finished", async () => { + class TimerBlock extends ChatBlock { + protected override onMount(): void { + const id = setInterval(() => this.requestRender(), 5); + this.onCleanup(() => clearInterval(id)); + } + } + const block = new TimerBlock(); + block.mount(host); + await Bun.sleep(25); + expect(renders).toBeGreaterThan(0); // timer fired while active + block.finish(); + const settled = renders; // includes finish()'s own render + await Bun.sleep(25); + expect(renders).toBe(settled); // interval torn down — no further ticks + }); +}); diff --git a/packages/coding-agent/test/modes/utils/render-initial-messages-dedupe.test.ts b/packages/coding-agent/test/modes/utils/render-initial-messages-dedupe.test.ts index be1d43af3..52de0a13e 100644 --- a/packages/coding-agent/test/modes/utils/render-initial-messages-dedupe.test.ts +++ b/packages/coding-agent/test/modes/utils/render-initial-messages-dedupe.test.ts @@ -65,6 +65,7 @@ function makeCtx(sessionManager?: Pick ctx.chatContainer.clear(), } as unknown as InteractiveModeContext; return { ctx, buildSessionContextSpy, renderSessionContextSpy }; diff --git a/packages/coding-agent/test/tools/render-utils.test.ts b/packages/coding-agent/test/tools/render-utils.test.ts index db3eb865f..f1c32d99c 100644 --- a/packages/coding-agent/test/tools/render-utils.test.ts +++ b/packages/coding-agent/test/tools/render-utils.test.ts @@ -2,7 +2,7 @@ import { afterEach, beforeAll, beforeEach, describe, expect, it } from "bun:test import * as os from "node:os"; import * as path from "node:path"; import { KeybindingsManager } from "@oh-my-pi/pi-coding-agent/config/keybindings"; -import { getThemeByName, initTheme, theme, type Theme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import { getThemeByName, initTheme, type Theme, theme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; import { dedupeParseErrors, expandKeyHint, @@ -14,7 +14,7 @@ import { formatScreenshot, truncateDiffByHunk, } from "@oh-my-pi/pi-coding-agent/tools/render-utils"; -import { getKeybindings, setKeybindings } from "@oh-my-pi/pi-tui"; +import { getKeybindings, setKeybindings, type KeybindingsManager as TuiKeybindingsManager } from "@oh-my-pi/pi-tui"; describe("parse error formatting", () => { it("deduplicates parse errors while preserving order", () => { @@ -295,7 +295,7 @@ describe("formatExpandHint / expandKeyHint", () => { format: { bracketLeft: "[", bracketRight: "]" }, } as unknown as Theme; - let previous: ReturnType; + let previous: TuiKeybindingsManager; beforeEach(() => { previous = getKeybindings(); }); diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index d864ad9ba..412067f39 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -1,6 +1,11 @@ # Changelog ## [Unreleased] +### Added + +- Added an optional `dispose()` lifecycle method to `Component` so components can release timers and subscriptions during permanent teardown +- Added `Container.dispose()` to propagate teardown to child components when a component tree is permanently discarded +- Added `Loader.dispose()` to stop the loader animation timer when the component is disposed ## [15.10.0] - 2026-06-06 diff --git a/packages/tui/src/components/loader.ts b/packages/tui/src/components/loader.ts index b9b2ffca9..5f0393a77 100644 --- a/packages/tui/src/components/loader.ts +++ b/packages/tui/src/components/loader.ts @@ -71,6 +71,11 @@ export class Loader extends Text { } } + /** Lifecycle teardown: stop the animation timer. Idempotent. */ + dispose() { + this.stop(); + } + setMessage(message: string) { this.message = message; this.#updateDisplay(); diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index 62db855a2..4b35fc3d5 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -128,6 +128,14 @@ export interface Component { * Called when theme changes or when component needs to re-render from scratch. */ invalidate(): void; + + /** + * Optional teardown. Called when the component is permanently removed from + * the live tree (e.g. a transcript reset). Release timers, intervals, and + * subscriptions here. Must be idempotent. Containers propagate dispose to + * their children; leaf components without resources may omit it. + */ + dispose?(): void; } /** @@ -339,6 +347,17 @@ export class Container implements Component { } } + /** + * Propagate teardown to children. Call when the container's children are + * being permanently discarded (not when they are detached for reuse — use + * {@link clear} for that). Idempotent per child via each child's own dispose. + */ + dispose(): void { + for (const child of this.children) { + child.dispose?.(); + } + } + render(width: number): string[] { width = Math.max(1, width); const lines: string[] = []; diff --git a/packages/tui/test/container-dispose.test.ts b/packages/tui/test/container-dispose.test.ts new file mode 100644 index 000000000..7c497a2f6 --- /dev/null +++ b/packages/tui/test/container-dispose.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from "bun:test"; +import { type Component, Container } from "@oh-my-pi/pi-tui"; + +function inert(dispose?: () => void): Component { + return { render: () => [], invalidate: () => {}, dispose }; +} + +describe("Container.dispose", () => { + it("calls dispose on each child and tolerates children without dispose", () => { + const order: string[] = []; + const container = new Container(); + container.addChild(inert(() => order.push("a"))); + container.addChild(inert()); // no dispose — must not throw + container.addChild(inert(() => order.push("c"))); + + expect(() => container.dispose()).not.toThrow(); + expect(order).toEqual(["a", "c"]); + }); + + it("recurses through nested containers", () => { + let leafDisposed = 0; + const outer = new Container(); + const inner = new Container(); + inner.addChild(inert(() => leafDisposed++)); + outer.addChild(inner); + + outer.dispose(); + expect(leafDisposed).toBe(1); + }); +}); diff --git a/packages/tui/test/loader.test.ts b/packages/tui/test/loader.test.ts index 06b35dcfa..7081d68ca 100644 --- a/packages/tui/test/loader.test.ts +++ b/packages/tui/test/loader.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from "bun:test"; +import { describe, expect, it, spyOn } from "bun:test"; import { TUI } from "@oh-my-pi/pi-tui"; import { Loader } from "@oh-my-pi/pi-tui/components/loader"; import { visibleWidth } from "@oh-my-pi/pi-tui/utils"; @@ -28,4 +28,23 @@ describe("Loader component", () => { loader.stop(); tui.stop(); }); + + it("dispose() stops the animation so no further renders are scheduled", async () => { + const term = new VirtualTerminal(20, 4); + const tui = new TUI(term); + const loader = new Loader( + tui, + text => text, + text => text, + "Checking", + ["a", "b", "c"], + ); + const spy = spyOn(tui, "requestRender"); + loader.dispose(); + const after = spy.mock.calls.length; + await Bun.sleep(40); // longer than the spinner interval + expect(spy.mock.calls.length).toBe(after); + expect(() => loader.dispose()).not.toThrow(); // idempotent + tui.stop(); + }); });