Merge remote-tracking branch 'origin/farm/b15f12c7/ssh-repaint-topology'
# Conflicts: # packages/coding-agent/src/tools/renderers.ts
This commit is contained in:
@@ -53,6 +53,9 @@
|
||||
### Fixed
|
||||
|
||||
- Fixed macOS SSHFS mount detection to skip remounting an already-mounted remote when `mountpoint` is unavailable. ([#4319](https://github.com/can1357/oh-my-pi/issues/4319))
|
||||
### Fixed
|
||||
|
||||
- Fixed SSH streamed placeholders and provisional partial frames leaving stale pending rows in the TUI viewport or native scrollback. ([#4314](https://github.com/can1357/oh-my-pi/issues/4314))
|
||||
|
||||
## [16.3.1] - 2026-07-02
|
||||
|
||||
|
||||
@@ -281,6 +281,14 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
|
||||
// history, so progress renders static gray and further partial snapshots are
|
||||
// dropped (see #maybeFreezeBackgroundTask).
|
||||
#backgroundTaskFrozen = false;
|
||||
// Set on each `render()` when the last painted shape carried the streamed
|
||||
// SSH-style placeholder / partial-result chrome. Reset gates key off these
|
||||
// so a topology-changing update that lands before the shape reaches the
|
||||
// terminal never triggers a full-viewport replay (which on direct terminals
|
||||
// wipes native scrollback and flashes the user's history — reviewer note on
|
||||
// PR #4315).
|
||||
#placeholderShapePainted = false;
|
||||
#partialResultShapePainted = false;
|
||||
#renderState: {
|
||||
spinnerFrame?: number;
|
||||
expanded: boolean;
|
||||
@@ -485,6 +493,12 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
|
||||
if (isPartial && this.#toolName === "task" && this.#maybeFreezeBackgroundTask()) {
|
||||
return;
|
||||
}
|
||||
const hadNoResult = this.#result === undefined;
|
||||
const wasPartialResult = this.#result !== undefined && this.#isPartial;
|
||||
const placeholderPainted = this.#placeholderShapePainted;
|
||||
const partialResultPainted = this.#partialResultShapePainted;
|
||||
this.#placeholderShapePainted = false;
|
||||
this.#partialResultShapePainted = false;
|
||||
this.#result = result;
|
||||
this.#resultVersion++;
|
||||
this.#isPartial = isPartial;
|
||||
@@ -496,6 +510,11 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
|
||||
this.#updateSpinnerAnimation();
|
||||
this.#updateTodoStrikeAnimation();
|
||||
this.#updateDisplay();
|
||||
this.#resetDisplayForResultTopologyChange(
|
||||
hadNoResult && placeholderPainted,
|
||||
wasPartialResult && partialResultPainted,
|
||||
isPartial,
|
||||
);
|
||||
// Convert non-PNG images to PNG for Kitty protocol (async)
|
||||
this.#maybeConvertImagesForKitty();
|
||||
}
|
||||
@@ -830,6 +849,49 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
|
||||
this.#displayBuilt = true;
|
||||
}
|
||||
|
||||
#rendererFlag(name: "forceFirstResultViewportRepaint" | "forceResultViewportRepaintOnSettle"): boolean {
|
||||
const toolValue = (this.#tool as Record<string, unknown> | undefined)?.[name];
|
||||
const rendererValue = toolRenderers[this.#toolName]?.[name];
|
||||
return toolValue === true || (toolValue === undefined && rendererValue === true);
|
||||
}
|
||||
|
||||
/**
|
||||
* True while the last painted shape uses the streamed placeholder path
|
||||
* (`⏳ SSH: […]` / `$ …`) — the render call ran with `__partialJson` args
|
||||
* and no result. Kept as a per-paint fact so a topology-changing update
|
||||
* that lands before the placeholder reaches the terminal skips the reset.
|
||||
*/
|
||||
#isPlaceholderShapeAtRender(): boolean {
|
||||
if (this.#result !== undefined) return false;
|
||||
if (!this.#rendererFlag("forceFirstResultViewportRepaint")) return false;
|
||||
return partialJsonOf(this.#args) !== undefined;
|
||||
}
|
||||
|
||||
#resetDisplayForResultTopologyChange(
|
||||
firstResultAfterPlaceholderPaint: boolean,
|
||||
partialResultPaintedBeforeSettle: boolean,
|
||||
isPartial: boolean,
|
||||
): void {
|
||||
const firstResultReplacesStreamedPlaceholder =
|
||||
firstResultAfterPlaceholderPaint && this.#rendererFlag("forceFirstResultViewportRepaint");
|
||||
const provisionalResultSettled =
|
||||
partialResultPaintedBeforeSettle && !isPartial && this.#rendererFlag("forceResultViewportRepaintOnSettle");
|
||||
if (firstResultReplacesStreamedPlaceholder || provisionalResultSettled) {
|
||||
this.#ui.resetDisplay();
|
||||
}
|
||||
}
|
||||
|
||||
override render(width: number): readonly string[] {
|
||||
const lines = super.render(width);
|
||||
// Update the paint-tracking flags after `super.render(width)` — the
|
||||
// override runs on every compose the parent Container performs, so a
|
||||
// frame that never gets composed leaves the flags false and prevents a
|
||||
// spurious `resetDisplay()`.
|
||||
this.#placeholderShapePainted = this.#isPlaceholderShapeAtRender();
|
||||
this.#partialResultShapePainted = this.#result !== undefined && this.#isPartial;
|
||||
return lines;
|
||||
}
|
||||
|
||||
// Viewport-/settings-dependent image sizing folded into the memo key only when
|
||||
// the last rebuild actually emitted images, so a terminal resize re-shapes an
|
||||
// image-bearing result (to rescale it) without re-shaping every image-free
|
||||
@@ -873,7 +935,8 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
|
||||
if (shouldRenderCall) {
|
||||
if (tool.renderCall) {
|
||||
try {
|
||||
const callComponent = tool.renderCall(this.#getCallArgsForRender(), this.#renderState, theme);
|
||||
const callArgs = this.#getCallArgsForRender();
|
||||
const callComponent = tool.renderCall(callArgs, this.#renderState, theme);
|
||||
if (callComponent) this.#contentBox.addChild(callComponent as Component);
|
||||
} catch (err) {
|
||||
logger.warn("Tool renderer failed", { tool: this.#toolName, error: String(err) });
|
||||
@@ -1007,7 +1070,8 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
|
||||
if (shouldRenderCall) {
|
||||
// Render call component
|
||||
try {
|
||||
const callComponent = renderer.renderCall(this.#getCallArgsForRender(), this.#renderState, theme);
|
||||
const callArgs = this.#getCallArgsForRender();
|
||||
const callComponent = renderer.renderCall(callArgs, this.#renderState, theme);
|
||||
if (callComponent) this.#contentBox.addChild(callComponent);
|
||||
} catch (err) {
|
||||
logger.warn("Tool renderer failed", { tool: this.#toolName, error: String(err) });
|
||||
|
||||
@@ -74,6 +74,19 @@ export type ToolRenderer = {
|
||||
* `options.spinnerFrame`.
|
||||
*/
|
||||
animatedPartialResult?: boolean | ((args: unknown) => boolean);
|
||||
/**
|
||||
* Whether replacing a streamed pending placeholder with the first result
|
||||
* requires a full viewport repaint. Use for merged renderers whose pending
|
||||
* streamed args may have committed placeholder rows that the result render
|
||||
* re-anchors instead of preserving.
|
||||
*/
|
||||
forceFirstResultViewportRepaint?: boolean;
|
||||
/**
|
||||
* Whether settling a provisional partial result into the final render requires
|
||||
* a full viewport repaint. Use when the result renderer changes chrome or
|
||||
* frame topology at `options.isPartial: true -> false`.
|
||||
*/
|
||||
forceResultViewportRepaintOnSettle?: boolean;
|
||||
};
|
||||
|
||||
export const toolRenderers: Record<string, ToolRenderer> = {
|
||||
|
||||
@@ -402,4 +402,12 @@ export const sshToolRenderer = {
|
||||
// land below and strand a duplicate pending header above the final frame
|
||||
// ([#3177](https://github.com/can1357/oh-my-pi/issues/3177)).
|
||||
provisionalPartialResult: true,
|
||||
// Streamed args can initially render the SSH placeholder (`⏳ SSH: […]` /
|
||||
// `$ …`), then the first partial result inserts the `Output` section and
|
||||
// re-anchors the frame. Force a full repaint at that seam so placeholder rows
|
||||
// do not survive in viewport/native scrollback.
|
||||
forceFirstResultViewportRepaint: true,
|
||||
// The provisional pending-result frame settles into the final `⇄ SSH: [host]`
|
||||
// frame, so clear/replay the viewport at that topology flip too.
|
||||
forceResultViewportRepaintOnSettle: true,
|
||||
};
|
||||
|
||||
@@ -0,0 +1,176 @@
|
||||
import { afterEach, beforeAll, describe, expect, it, vi } from "bun:test";
|
||||
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 Component, TUI } from "@oh-my-pi/pi-tui";
|
||||
import { StressRenderScheduler } from "../../tui/test/render-stress-scheduler";
|
||||
import { VirtualTerminal } from "../../tui/test/virtual-terminal";
|
||||
|
||||
function sshResult(text: string) {
|
||||
return { content: [{ type: "text", text }] };
|
||||
}
|
||||
|
||||
class Footer implements Component {
|
||||
constructor(readonly rows: number) {}
|
||||
invalidate(): void {}
|
||||
render(_width: number): string[] {
|
||||
return Array.from({ length: this.rows }, (_, i) => `editor-${i}`);
|
||||
}
|
||||
}
|
||||
|
||||
function plainBuffer(term: VirtualTerminal): string[] {
|
||||
return term
|
||||
.getScrollBuffer()
|
||||
.map(row => Bun.stripANSI(row).trimEnd())
|
||||
.filter(Boolean);
|
||||
}
|
||||
|
||||
async function drain(scheduler: StressRenderScheduler, term: VirtualTerminal): Promise<void> {
|
||||
await scheduler.drain(term);
|
||||
}
|
||||
|
||||
describe("ToolExecutionComponent SSH repaint seams", () => {
|
||||
const components: ToolExecutionComponent[] = [];
|
||||
|
||||
beforeAll(async () => {
|
||||
await initTheme();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
for (const component of components) component.stopAnimation();
|
||||
components.length = 0;
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
function makeComponent(args: unknown) {
|
||||
const resetDisplay = vi.fn();
|
||||
const ui = { requestRender() {}, resetDisplay } as unknown as TUI;
|
||||
const component = new ToolExecutionComponent("ssh", args, {}, undefined, ui);
|
||||
components.push(component);
|
||||
resetDisplay.mockClear();
|
||||
return { component, resetDisplay };
|
||||
}
|
||||
|
||||
it("forces a viewport repaint when a painted streamed SSH placeholder receives its first result", () => {
|
||||
const { component, resetDisplay } = makeComponent({ __partialJson: '{"host"' });
|
||||
// A paint has to land for the placeholder to actually reach the terminal.
|
||||
component.render(80);
|
||||
|
||||
component.updateResult(sshResult("partial output"), true);
|
||||
|
||||
expect(resetDisplay).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does not repaint when the streamed placeholder never reaches the terminal", () => {
|
||||
const { component, resetDisplay } = makeComponent({ __partialJson: '{"host"' });
|
||||
// The placeholder shape was built in memory but never painted — a
|
||||
// resetDisplay here would wipe scrollback for a shape the user never saw.
|
||||
|
||||
component.updateResult(sshResult("partial output"), true);
|
||||
|
||||
expect(resetDisplay).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not repaint complete SSH args on the first result", () => {
|
||||
const { component, resetDisplay } = makeComponent({ host: "router", command: "uptime" });
|
||||
component.render(80);
|
||||
|
||||
component.updateResult(sshResult("partial output"), true);
|
||||
|
||||
expect(resetDisplay).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("forces a viewport repaint when a painted provisional SSH partial result settles", () => {
|
||||
const { component, resetDisplay } = makeComponent({ host: "router", command: "uptime" });
|
||||
component.updateResult(sshResult("partial output"), true);
|
||||
component.render(80);
|
||||
resetDisplay.mockClear();
|
||||
|
||||
component.updateResult(sshResult("final output"), false);
|
||||
|
||||
expect(resetDisplay).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does not repaint when the provisional partial result never reaches the terminal", () => {
|
||||
const { component, resetDisplay } = makeComponent({ host: "router", command: "uptime" });
|
||||
component.updateResult(sshResult("partial output"), true);
|
||||
// No render() between the partial and the final update — the provisional
|
||||
// frame never reached the terminal, so no reset should fire.
|
||||
|
||||
component.updateResult(sshResult("final output"), false);
|
||||
|
||||
expect(resetDisplay).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("removes streamed SSH placeholder rows from the terminal buffer when the first result arrives", async () => {
|
||||
const term = new VirtualTerminal(90, 8, 1_000);
|
||||
const scheduler = new StressRenderScheduler();
|
||||
const tui = new TUI(term, undefined, { renderScheduler: scheduler });
|
||||
const component = new ToolExecutionComponent("ssh", { __partialJson: '{"host"' }, {}, undefined, tui);
|
||||
components.push(component);
|
||||
tui.addChild(component);
|
||||
tui.addChild(new Footer(5));
|
||||
|
||||
try {
|
||||
tui.start();
|
||||
await drain(scheduler, term);
|
||||
expect(plainBuffer(term).some(row => row.includes("SSH: […]"))).toBe(true);
|
||||
expect(plainBuffer(term).some(row => row.includes("$ …"))).toBe(true);
|
||||
|
||||
component.updateArgs({
|
||||
host: "router",
|
||||
command: "uptime",
|
||||
__partialJson: '{"host":"router","command":"uptime"}',
|
||||
});
|
||||
component.setArgsComplete();
|
||||
tui.requestRender();
|
||||
await drain(scheduler, term);
|
||||
|
||||
component.updateResult(sshResult("partial output"), true);
|
||||
tui.requestRender();
|
||||
await drain(scheduler, term);
|
||||
|
||||
const rows = plainBuffer(term);
|
||||
expect(rows.some(row => row.includes("SSH: […]"))).toBe(false);
|
||||
expect(rows.some(row => row.includes("$ …"))).toBe(false);
|
||||
expect(rows.some(row => row.includes("⏳ SSH: [router]"))).toBe(true);
|
||||
expect(rows.some(row => row.includes("Output"))).toBe(true);
|
||||
expect(rows.some(row => row.includes("partial output"))).toBe(true);
|
||||
} finally {
|
||||
tui.stop();
|
||||
await term.flush();
|
||||
}
|
||||
});
|
||||
|
||||
it("removes provisional SSH partial chrome from the terminal buffer when the result settles", async () => {
|
||||
const term = new VirtualTerminal(90, 8, 1_000);
|
||||
const scheduler = new StressRenderScheduler();
|
||||
const tui = new TUI(term, undefined, { renderScheduler: scheduler });
|
||||
const component = new ToolExecutionComponent("ssh", { host: "router", command: "uptime" }, {}, undefined, tui);
|
||||
components.push(component);
|
||||
tui.addChild(component);
|
||||
tui.addChild(new Footer(5));
|
||||
|
||||
try {
|
||||
tui.start();
|
||||
await drain(scheduler, term);
|
||||
component.updateResult(sshResult("partial output"), true);
|
||||
tui.requestRender();
|
||||
await drain(scheduler, term);
|
||||
expect(plainBuffer(term).some(row => row.includes("⏳ SSH: [router]"))).toBe(true);
|
||||
|
||||
component.updateResult(sshResult("final output"), false);
|
||||
tui.requestRender();
|
||||
await drain(scheduler, term);
|
||||
|
||||
const rows = plainBuffer(term);
|
||||
expect(rows.some(row => row.includes("⏳ SSH: [router]"))).toBe(false);
|
||||
expect(rows.some(row => row.includes("partial output"))).toBe(false);
|
||||
expect(rows.some(row => row.includes("⇄ SSH: [router]"))).toBe(true);
|
||||
expect(rows.some(row => row.includes("Output"))).toBe(true);
|
||||
expect(rows.some(row => row.includes("final output"))).toBe(true);
|
||||
} finally {
|
||||
tui.stop();
|
||||
await term.flush();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -17,7 +17,7 @@ import { ToolExecutionComponent } from "@oh-my-pi/pi-coding-agent/modes/componen
|
||||
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;
|
||||
const uiStub = { requestRender() {}, resetDisplay() {} } as unknown as TUI;
|
||||
|
||||
function makeSshComponent() {
|
||||
return new ToolExecutionComponent("ssh", { host: "sccpu", command: "uptime" }, {}, undefined, uiStub);
|
||||
|
||||
Reference in New Issue
Block a user