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
This commit is contained in:
roboomp
@@ -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
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<string, ToolRenderer> = {
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user