From c1d4c801114545f47c440921d8b3e04aeb1e565d Mon Sep 17 00:00:00 2001 From: Vegard Stikbakke Date: Mon, 7 Sep 2026 15:31:00 +0200 Subject: [PATCH] fix(coding-agent): embed all session status spinners in the editor border --- packages/coding-agent/CHANGELOG.md | 1 + packages/coding-agent/docs/tui.md | 2 +- .../interactive/components/custom-editor.ts | 8 +- .../components/status-indicator.ts | 18 ++-- .../src/modes/interactive/interactive-mode.ts | 14 ++- .../coding-agent/test/interactive-tui.test.ts | 86 +++++++++++++++---- .../test/status-indicator.test.ts | 34 ++++++++ 7 files changed, 124 insertions(+), 39 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index f1d1b9aab..762cfa9e2 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -8,6 +8,7 @@ ### Changed +- Moved compaction, branch summarization, and retry spinners into the editor border alongside the working indicator. Custom editors use the same embedding opt-in for all status spinners. - Enabled strict-prefer JSON-schema sampling by default for built-in `read`, `bash`, `powershell`, `edit`, and `write` tools, without requiring `PI_EXPERIMENTAL`. Extensions can re-register tool definitions with `constrainedSampling: false`. ### Fixed diff --git a/packages/coding-agent/docs/tui.md b/packages/coding-agent/docs/tui.md index e28f1fd2d..b1e737325 100644 --- a/packages/coding-agent/docs/tui.md +++ b/packages/coding-agent/docs/tui.md @@ -930,7 +930,7 @@ export default function (pi: ExtensionAPI) { - **Extend `CustomEditor`** (not base `Editor`) to get app keybindings (escape to abort, ctrl+d to exit, model switching, etc.) - **Call `super.handleInput(data)`** for keys you don't handle -- **Working status**: custom editors keep the standalone working row by default. Pass `{ embedWorkingStatus: true }` as the fourth `CustomEditor` constructor argument to use the built-in editor-border spinner instead. +- **Status spinners**: custom editors keep standalone status rows by default. Pass `{ embedWorkingStatus: true }` as the fourth `CustomEditor` constructor argument to embed working, compaction, branch summarization, and retry spinners in the editor border instead. - **Factory pattern**: `setEditorComponent` receives a factory function that gets `tui`, `theme`, and `keybindings` - **Pass `undefined`** to restore the default editor: `ctx.ui.setEditorComponent(undefined)` diff --git a/packages/coding-agent/src/modes/interactive/components/custom-editor.ts b/packages/coding-agent/src/modes/interactive/components/custom-editor.ts index 057dc8349..9d345b4e9 100644 --- a/packages/coding-agent/src/modes/interactive/components/custom-editor.ts +++ b/packages/coding-agent/src/modes/interactive/components/custom-editor.ts @@ -1,9 +1,9 @@ import { Editor, type EditorOptions, type EditorTheme, type TUI, visibleWidth } from "@earendil-works/pi-tui"; import type { AppKeybinding, KeybindingsManager } from "../../../core/keybindings.ts"; -import type { WorkingStatusIndicator } from "./status-indicator.ts"; +import type { StatusIndicator } from "./status-indicator.ts"; export type CustomEditorOptions = EditorOptions & { - /** Render the streaming working status in the editor's top border. */ + /** Render working, compaction, summarization, and retry status in the editor's top border. */ embedWorkingStatus?: boolean; }; @@ -12,7 +12,7 @@ export type CustomEditorOptions = EditorOptions & { */ export class CustomEditor extends Editor { private keybindings: KeybindingsManager; - private workingStatusIndicator: WorkingStatusIndicator | undefined; + private workingStatusIndicator: StatusIndicator | undefined; public readonly embedWorkingStatus: boolean; public actionHandlers: Map void> = new Map(); @@ -29,7 +29,7 @@ export class CustomEditor extends Editor { this.embedWorkingStatus = options?.embedWorkingStatus ?? false; } - setWorkingStatusIndicator(indicator: WorkingStatusIndicator | undefined): void { + setWorkingStatusIndicator(indicator: StatusIndicator | undefined): void { this.workingStatusIndicator = indicator; } diff --git a/packages/coding-agent/src/modes/interactive/components/status-indicator.ts b/packages/coding-agent/src/modes/interactive/components/status-indicator.ts index 4efeb41ee..be2732103 100644 --- a/packages/coding-agent/src/modes/interactive/components/status-indicator.ts +++ b/packages/coding-agent/src/modes/interactive/components/status-indicator.ts @@ -21,6 +21,15 @@ export class StatusIndicator extends Loader { this.kind = kind; } + renderInBorder(width: number): string { + const line = super.render(width + 2)[1] ?? ""; + return truncateToWidth(line.startsWith(" ") ? line.slice(1).trimEnd() : line.trimEnd(), width, ""); + } + + renderSpinnerInBorder(width: number): string { + return truncateToWidth(this.getRenderedIndicator(), width, ""); + } + dispose(): void { this.stop(); } @@ -37,15 +46,6 @@ export class WorkingStatusIndicator extends StatusIndicator { indicator, ); } - - renderInBorder(width: number): string { - const line = super.render(width + 2)[1] ?? ""; - return truncateToWidth(line.startsWith(" ") ? line.slice(1).trimEnd() : line.trimEnd(), width, ""); - } - - renderSpinnerInBorder(width: number): string { - return truncateToWidth(this.getRenderedIndicator(), width, ""); - } } export class RetryStatusIndicator extends StatusIndicator { diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index b84f96f14..e4f0fe064 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -182,7 +182,7 @@ interface Expandable { interface WorkingStatusEditor extends EditorComponent { readonly embedWorkingStatus: boolean; - setWorkingStatusIndicator(indicator: WorkingStatusIndicator | undefined): void; + setWorkingStatusIndicator(indicator: StatusIndicator | undefined): void; } function isWorkingStatusEditor(editor: EditorComponent): editor is WorkingStatusEditor { @@ -2092,7 +2092,7 @@ export class InteractiveMode { this.ui.requestRender(); } - private setEditorWorkingStatusIndicator(indicator: WorkingStatusIndicator | undefined): boolean { + private setEditorWorkingStatusIndicator(indicator: StatusIndicator | undefined): boolean { this.defaultEditor.setWorkingStatusIndicator(undefined); if (!isWorkingStatusEditor(this.editor)) return false; this.editor.setWorkingStatusIndicator(indicator); @@ -2105,7 +2105,7 @@ export class InteractiveMode { this.activeWorkingIndicatorEmbedded = false; this.statusContainer.clear(); this.setEditorWorkingStatusIndicator(undefined); - if (indicator instanceof WorkingStatusIndicator && this.setEditorWorkingStatusIndicator(indicator)) { + if (this.setEditorWorkingStatusIndicator(indicator)) { this.activeWorkingIndicatorEmbedded = true; return; } @@ -2117,7 +2117,7 @@ export class InteractiveMode { return; } const clearedIndicator = this.activeStatusIndicator; - const clearedIndicatorWasEmbedded = clearedIndicator?.kind === "working" && this.activeWorkingIndicatorEmbedded; + const clearedIndicatorWasEmbedded = this.activeWorkingIndicatorEmbedded; clearedIndicator?.dispose(); this.activeStatusIndicator = undefined; this.activeWorkingIndicatorEmbedded = false; @@ -2717,7 +2717,7 @@ export class InteractiveMode { } this.editorContainer.addChild(this.editor as Component); - if (this.activeStatusIndicator instanceof WorkingStatusIndicator) { + if (this.activeStatusIndicator) { this.statusContainer.clear(); this.activeWorkingIndicatorEmbedded = this.setEditorWorkingStatusIndicator(this.activeStatusIndicator); if (!this.activeWorkingIndicatorEmbedded) { @@ -4170,9 +4170,7 @@ export class InteractiveMode { const level = this.session.thinkingLevel || "off"; this.editor.borderColor = theme.getThinkingBorderColor(level); } - if (this.activeStatusIndicator?.kind === "working") { - this.activeStatusIndicator.invalidate(); - } + this.activeStatusIndicator?.invalidate(); this.ui.requestRender(); } diff --git a/packages/coding-agent/test/interactive-tui.test.ts b/packages/coding-agent/test/interactive-tui.test.ts index b646de82e..63ce7d35a 100644 --- a/packages/coding-agent/test/interactive-tui.test.ts +++ b/packages/coding-agent/test/interactive-tui.test.ts @@ -4,6 +4,14 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { VirtualTerminal } from "../../tui/test/virtual-terminal.ts"; import { KeybindingsManager } from "../src/core/keybindings.ts"; import type { FullscreenExitOutput, TuiMode } from "../src/core/settings-manager.ts"; +import { + BranchSummaryStatusIndicator, + CompactionStatusIndicator, + RetryStatusIndicator, + type StatusIndicator, + type StatusIndicatorKind, + WorkingStatusIndicator, +} from "../src/modes/interactive/components/status-indicator.ts"; import { createInteractiveTui, createInteractiveTuiReference, @@ -345,11 +353,11 @@ describe("InteractiveMode copy confirmation", () => { type StatusEditor = { embedWorkingStatus: boolean; - setWorkingStatusIndicator: (indicator: undefined) => void; + setWorkingStatusIndicator: (indicator: StatusIndicator | undefined) => void; }; type ClearStatusContext = { - activeStatusIndicator: { kind: "working" | "retry"; dispose: () => void } | undefined; + activeStatusIndicator: { kind: StatusIndicatorKind; dispose: () => void } | undefined; activeWorkingIndicatorEmbedded: boolean; statusContainer: Container; defaultEditor: StatusEditor; @@ -357,39 +365,83 @@ type ClearStatusContext = { options: { tuiMode?: TuiMode }; ui: { getClearOnShrink: () => boolean }; idleStatus: Component; - setEditorWorkingStatusIndicator(indicator: undefined): boolean; + setEditorWorkingStatusIndicator(indicator: StatusIndicator | undefined): boolean; }; type InteractiveModePrototype = { - clearStatusIndicator(this: ClearStatusContext, kind?: "working" | "retry"): void; - setEditorWorkingStatusIndicator(this: ClearStatusContext, indicator: undefined): boolean; + showStatusIndicator(this: ClearStatusContext, indicator: StatusIndicator): void; + clearStatusIndicator(this: ClearStatusContext, kind?: StatusIndicatorKind): void; + setEditorWorkingStatusIndicator(this: ClearStatusContext, indicator: StatusIndicator | undefined): boolean; }; const interactiveModePrototype = InteractiveMode.prototype as unknown as InteractiveModePrototype; describe("clear-on-shrink status spacing", () => { - it("does not reserve separate status height for the editor-border working indicator", () => { - const dispose = vi.fn(); - const editor: StatusEditor = { embedWorkingStatus: true, setWorkingStatusIndicator: vi.fn() }; + it.each([true, false])("routes every status through the editor opt-in (%s)", (embedWorkingStatus) => { + initTheme("dark"); + const tui = { requestRender: vi.fn() } as unknown as TUI; + const editor: StatusEditor = { embedWorkingStatus, setWorkingStatusIndicator: vi.fn() }; const context: ClearStatusContext = { - activeStatusIndicator: { kind: "working", dispose }, - activeWorkingIndicatorEmbedded: true, + activeStatusIndicator: undefined, + activeWorkingIndicatorEmbedded: false, statusContainer: new Container(), - defaultEditor: editor, + defaultEditor: { embedWorkingStatus: true, setWorkingStatusIndicator: vi.fn() }, editor, options: { tuiMode: "regular" }, ui: { getClearOnShrink: () => true }, idleStatus: new Text("", 0, 0), setEditorWorkingStatusIndicator: interactiveModePrototype.setEditorWorkingStatusIndicator, }; - - interactiveModePrototype.clearStatusIndicator.call(context); - - expect(dispose).toHaveBeenCalledOnce(); - expect(editor.setWorkingStatusIndicator).toHaveBeenCalledWith(undefined); - expect(context.statusContainer.children).toHaveLength(0); + const indicators = [ + new WorkingStatusIndicator(tui, "Working"), + new CompactionStatusIndicator(tui, "manual"), + new CompactionStatusIndicator(tui, "threshold"), + new CompactionStatusIndicator(tui, "overflow"), + new BranchSummaryStatusIndicator(tui), + new RetryStatusIndicator(tui, 1, 3, 1000), + ]; + try { + for (const indicator of indicators) { + interactiveModePrototype.showStatusIndicator.call(context, indicator); + expect(context.activeStatusIndicator).toBe(indicator); + expect(context.activeWorkingIndicatorEmbedded).toBe(embedWorkingStatus); + if (embedWorkingStatus) { + expect(editor.setWorkingStatusIndicator).toHaveBeenLastCalledWith(indicator); + expect(context.statusContainer.children).toHaveLength(0); + } else { + expect(context.statusContainer.children).toEqual([indicator]); + } + } + } finally { + for (const indicator of indicators) indicator.dispose(); + } }); + it.each(["working", "compaction", "branchSummary", "retry"])( + "does not reserve separate status height for an embedded %s indicator", + (kind) => { + const dispose = vi.fn(); + const editor: StatusEditor = { embedWorkingStatus: true, setWorkingStatusIndicator: vi.fn() }; + const context: ClearStatusContext = { + activeStatusIndicator: { kind, dispose }, + activeWorkingIndicatorEmbedded: true, + statusContainer: new Container(), + defaultEditor: editor, + editor, + options: { tuiMode: "regular" }, + ui: { getClearOnShrink: () => true }, + idleStatus: new Text("", 0, 0), + setEditorWorkingStatusIndicator: interactiveModePrototype.setEditorWorkingStatusIndicator, + }; + + interactiveModePrototype.clearStatusIndicator.call(context); + + expect(dispose).toHaveBeenCalledOnce(); + expect(editor.setWorkingStatusIndicator).toHaveBeenCalledWith(undefined); + expect(context.statusContainer.children).toHaveLength(0); + }, + ); + it("uses the standalone row for a custom editor that has not opted in", () => { for (const [tuiMode, expectedChildren] of [ ["regular", 1], diff --git a/packages/coding-agent/test/status-indicator.test.ts b/packages/coding-agent/test/status-indicator.test.ts index 412dff08e..5efc810f9 100644 --- a/packages/coding-agent/test/status-indicator.test.ts +++ b/packages/coding-agent/test/status-indicator.test.ts @@ -3,6 +3,8 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { KeybindingsManager } from "../src/core/keybindings.ts"; import { CustomEditor } from "../src/modes/interactive/components/custom-editor.ts"; import { + BranchSummaryStatusIndicator, + CompactionStatusIndicator, IdleStatus, RetryStatusIndicator, WorkingStatusIndicator, @@ -61,6 +63,38 @@ describe("status indicators", () => { indicator.dispose(); }); + it("embeds compaction, summary, and retry labels within the border width", () => { + initTheme("dark"); + vi.useFakeTimers(); + const tui = { requestRender: vi.fn(), terminal: { rows: 10 } } as unknown as TUI; + const editor = new CustomEditor(tui, getEditorTheme(), KeybindingsManager.create(), { + embedWorkingStatus: true, + }); + const indicators = [ + new CompactionStatusIndicator(tui, "manual"), + new CompactionStatusIndicator(tui, "threshold"), + new CompactionStatusIndicator(tui, "overflow"), + new BranchSummaryStatusIndicator(tui), + new RetryStatusIndicator(tui, 1, 3, 3000), + ]; + try { + for (const indicator of indicators) { + editor.setWorkingStatusIndicator(indicator); + const label = stripAnsi(indicator.render(120)[1]!).trim(); + expect(stripAnsi(editor.render(120)[0]!)).toContain(`── ${label} `); + for (const width of [1, 4, 10, 20, 80, 120]) { + expect(visibleWidth(editor.render(width)[0]!)).toBe(width); + } + } + vi.advanceTimersByTime(1000); + expect(stripAnsi(editor.render(120)[0]!)).toContain("Retrying (1/3) in 2s"); + editor.setWorkingStatusIndicator(undefined); + expect(stripAnsi(editor.render(120)[0]!)).toBe("─".repeat(120)); + } finally { + for (const indicator of indicators) indicator.dispose(); + } + }); + it("disposes retry countdown updates", () => { initTheme("dark"); vi.useFakeTimers();