From 3faeddd3a6a047f10cc92b4bb14e4a67af1fd2cc Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 21 Jun 2026 08:13:56 +0000 Subject: [PATCH] fix(tui): kept ssh partial result commit-unstable Reviewer flagged that ToolExecutionComponent.isTranscriptBlockCommitStable\ntreated any block with a defined #result as commit-stable, so a long\nstreaming SSH partial render could sit on its stable pending header until\nderiveLiveCommitState's stable-prefix ratchet promoted it to native\nscrollback, then the final SSH glyph render would land below and strand a\nduplicate pending header above the settled frame.\n\n- Added ToolRenderer.provisionalPartialResult opt-in for renderers whose\n partial-result chrome differs from the final render.\n- Honored it in ToolExecutionComponent.isTranscriptBlockCommitStable so the\n block stays commit-unstable while isPartial holds and flips stable as soon\n as the result settles.\n- Set provisionalPartialResult: true on the SSH renderer.\n- Added test/tools/ssh-commit-stability.test.ts covering the partial/final\n transition and the non-regression for bash.\n\nFixes #3177 --- packages/coding-agent/CHANGELOG.md | 2 +- .../src/modes/components/tool-execution.ts | 34 +++++++--- packages/coding-agent/src/tools/renderers.ts | 11 +++ packages/coding-agent/src/tools/ssh.ts | 8 +++ .../test/tools/ssh-commit-stability.test.ts | 68 +++++++++++++++++++ 5 files changed, 113 insertions(+), 10 deletions(-) create mode 100644 packages/coding-agent/test/tools/ssh-commit-stability.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index f4a47bb2f..13f3b6b9c 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed partial SSH result rendering switching to the final SSH glyph before the tool settled, which could leave a stale pending SSH header above the final header in terminal scrollback ([#3177](https://github.com/can1357/oh-my-pi/issues/3177)) +- Fixed long-running SSH command boxes leaving a stale `⏳ SSH: [host]` header above the final `⇄ SSH: [host]` header in terminal scrollback. The SSH renderer now keeps its partial-result chrome on the pending icon/state and opts the block out of stream-commit while `isPartial` holds (via the new `ToolRenderer.provisionalPartialResult` flag honored by `ToolExecutionComponent.isTranscriptBlockCommitStable`), so the stable-prefix ratchet can't promote the partial header to native scrollback only to have the final render strand it above the settled frame ([#3177](https://github.com/can1357/oh-my-pi/issues/3177)). ## [16.1.10] - 2026-06-21 diff --git a/packages/coding-agent/src/modes/components/tool-execution.ts b/packages/coding-agent/src/modes/components/tool-execution.ts index a5d1937f8..86b5f3207 100644 --- a/packages/coding-agent/src/modes/components/tool-execution.ts +++ b/packages/coding-agent/src/modes/components/tool-execution.ts @@ -635,15 +635,31 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac // `provisionalPendingPreview` describes only the PENDING call preview // (`renderCall`, before any result): the result render may re-anchor it // wholesale, so its rows must never commit. Once a (streaming partial) - // result exists the result renderer is the live shape — its body is - // top-anchored and grows append-only, and `deriveLiveCommitState` gates - // per-row durability — so the block is commit-stable like any settled - // stream. Gating the flag on the pending phase is what keeps a collapsed - // streaming eval/bash/ssh whose box outgrows the viewport from stranding - // its head: while commit-unstable its scrolled-off top committed nowhere - // and repainted nowhere, so it read as truncated until ctrl+o (expanded) - // flipped it stable. - if (this.#result !== undefined) return true; + // result exists the result renderer is usually the live shape — its body + // is top-anchored and grows append-only, and `deriveLiveCommitState` + // gates per-row durability — so the block is commit-stable like any + // settled stream. Gating the flag on the pending phase is what keeps a + // collapsed streaming eval/bash/ssh whose box outgrows the viewport from + // stranding its head: while commit-unstable its scrolled-off top + // committed nowhere and repainted nowhere, so it read as truncated until + // ctrl+o (expanded) flipped it stable. + // + // Renderers whose partial-result chrome (header glyph, frame state) + // differs from the final result render set `provisionalPartialResult` + // to opt out of stream-commit while `isPartial` holds: the ratchet + // would otherwise promote the stable partial chrome to native scrollback + // after `STABLE_PREFIX_COMMIT_FRAMES` and leave it stranded above the + // final frame once the chrome flips. Once the result settles + // (`isPartial === false`) the block is commit-stable again. + if (this.#result !== undefined) { + if (this.#isPartial) { + const tool = this.#tool as { provisionalPartialResult?: boolean } | undefined; + const provisionalPartialResult = + tool?.provisionalPartialResult ?? toolRenderers[this.#toolName]?.provisionalPartialResult; + if (provisionalPartialResult) return false; + } + return true; + } const tool = this.#tool as { provisionalPendingPreview?: boolean | "collapsed" } | undefined; const provisionalPendingPreview = tool?.provisionalPendingPreview ?? toolRenderers[this.#toolName]?.provisionalPendingPreview; diff --git a/packages/coding-agent/src/tools/renderers.ts b/packages/coding-agent/src/tools/renderers.ts index 7c3dddec1..39889af41 100644 --- a/packages/coding-agent/src/tools/renderers.ts +++ b/packages/coding-agent/src/tools/renderers.ts @@ -52,6 +52,17 @@ export type ToolRenderer = { * streams rows the result render preserves. */ provisionalPendingPreview?: boolean | "collapsed"; + /** + * Whether the partial-result render is provisional: chrome rows (header + * glyph, frame state) that change between `options.isPartial === true` and + * the final result render. When `true`, the block is treated as + * commit-unstable while a partial result is in flight, so the + * stable-prefix ratchet in `deriveLiveCommitState` cannot promote the + * partial chrome to native scrollback only to have the final render strand + * it above the settled frame. Absent = the partial render is byte-stable + * with the final render and may commit like any settled stream. + */ + provisionalPartialResult?: boolean; }; export const toolRenderers: Record = { diff --git a/packages/coding-agent/src/tools/ssh.ts b/packages/coding-agent/src/tools/ssh.ts index 238c071a8..269115567 100644 --- a/packages/coding-agent/src/tools/ssh.ts +++ b/packages/coding-agent/src/tools/ssh.ts @@ -373,4 +373,12 @@ export const sshToolRenderer = { // that shifts while args stream. Expanded output is top-anchored enough for // the transcript to commit its settled prefix. provisionalPendingPreview: "collapsed", + // Partial-result chrome (pending icon and frame state) differs from the + // final SSH glyph/state, so the block stays commit-unstable while + // `options.isPartial` holds. Without this, a long-running SSH command's + // stable pending header would be promoted by the stable-prefix ratchet and + // committed to native scrollback, then the final render's SSH glyph would + // land below and strand a duplicate pending header above the final frame + // ([#3177](https://github.com/can1357/oh-my-pi/issues/3177)). + provisionalPartialResult: true, }; diff --git a/packages/coding-agent/test/tools/ssh-commit-stability.test.ts b/packages/coding-agent/test/tools/ssh-commit-stability.test.ts new file mode 100644 index 000000000..a264bba63 --- /dev/null +++ b/packages/coding-agent/test/tools/ssh-commit-stability.test.ts @@ -0,0 +1,68 @@ +/** + * Issue #3177: `sshToolRenderer.renderResult` swaps the pending icon/frame + * state for the SSH glyph + success state when `options.isPartial` flips + * false. Without the renderer's `provisionalPartialResult: true` opt-out, a + * long-running SSH command keeps the same partial header bytes for the whole + * `STABLE_PREFIX_COMMIT_FRAMES` window, the transcript's stable-prefix + * ratchet promotes them to native scrollback, and the final render strands a + * pending `⏳ SSH: [host]` header above the final `⇄ SSH: [host]` header + * (the bug the user reported). Contract: while a partial SSH result is in + * flight, the block reports commit-unstable so `deriveLiveCommitState` keeps + * its rows in the live region; once the result settles it is commit-stable + * again. + */ +import { afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; +import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { ToolExecutionComponent } from "@oh-my-pi/pi-coding-agent/modes/components/tool-execution"; +import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import type { TUI } from "@oh-my-pi/pi-tui"; + +const uiStub = { requestRender() {} } as unknown as TUI; + +function makeSshComponent() { + return new ToolExecutionComponent("ssh", { host: "sccpu", command: "uptime" }, {}, undefined, uiStub); +} + +function partialResult(text: string) { + return { content: [{ type: "text" as const, text }] }; +} + +describe("ssh tool block commit stability", () => { + beforeAll(async () => { + resetSettingsForTest(); + await Settings.init({ inMemory: true }); + await initTheme(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("reports commit-unstable while an SSH result is partial", () => { + const component = makeSshComponent(); + component.updateResult(partialResult("connecting…"), true); + + expect(component.isTranscriptBlockFinalized()).toBe(false); + expect(component.isTranscriptBlockCommitStable()).toBe(false); + }); + + it("flips commit-stable as soon as the SSH result settles", () => { + const component = makeSshComponent(); + component.updateResult(partialResult("connecting…"), true); + expect(component.isTranscriptBlockCommitStable()).toBe(false); + + component.updateResult(partialResult("done\n"), false); + expect(component.isTranscriptBlockFinalized()).toBe(true); + expect(component.isTranscriptBlockCommitStable()).toBe(true); + }); + + it("does not opt other foreground tools out of partial-result stream commits", () => { + // Sanity: bash and friends still get the existing `isPartial` + // commit-stable behaviour — the SSH opt-in must be renderer-scoped. + const component = new ToolExecutionComponent("bash", { command: "ls" }, {}, undefined, uiStub); + component.updateResult(partialResult("a\nb\n"), true); + + expect(component.isTranscriptBlockFinalized()).toBe(false); + expect(component.isTranscriptBlockCommitStable()).toBe(true); + }); +});