diff --git a/src/tui/components/prompt-shell.tsx b/src/tui/components/prompt-shell.tsx index 230a5977..75e7c44d 100644 --- a/src/tui/components/prompt-shell.tsx +++ b/src/tui/components/prompt-shell.tsx @@ -102,12 +102,19 @@ export function PromptShell(props: PromptShellProps): ReactElement { mouseLayer, ...editorProps } = props; + // Rotate only while the phrase is on screen. `effectivePlaceholder` + // below already blanks it for a non-empty buffer; without the same + // condition on the timer, typing left a four-second full-frame repaint + // running behind the composer for the rest of the session. + const placeholderVisible = value.length === 0; const rotated = useRotatingPlaceholder( rotatingPlaceholders ?? [], placeholderRotationMs, + placeholderVisible, ); - const effectivePlaceholder = - value.length === 0 ? (rotated ?? placeholder ?? "") : ""; + const effectivePlaceholder = placeholderVisible + ? (rotated ?? placeholder ?? "") + : ""; const accent = focus && !disabled ? theme.colors.accent : theme.colors.border; // Send is live on exactly the condition Enter is: a non-blank buffer // in an editor that is accepting input. `handleEditorSubmit` drops a diff --git a/src/tui/hooks/use-rotating-placeholder.test.tsx b/src/tui/hooks/use-rotating-placeholder.test.tsx index 4bd60e6d..fa1c293f 100644 --- a/src/tui/hooks/use-rotating-placeholder.test.tsx +++ b/src/tui/hooks/use-rotating-placeholder.test.tsx @@ -7,10 +7,11 @@ import { useRotatingPlaceholder } from "./use-rotating-placeholder.js"; interface ProbeProps { phrases: readonly string[]; intervalMs?: number; + active?: boolean; } -function Probe({ phrases, intervalMs }: ProbeProps): ReactElement { - const value = useRotatingPlaceholder(phrases, intervalMs); +function Probe({ phrases, intervalMs, active }: ProbeProps): ReactElement { + const value = useRotatingPlaceholder(phrases, intervalMs, active); return {value ?? ""}; } @@ -60,4 +61,54 @@ describe("useRotatingPlaceholder", () => { expect(strip(lastFrame() ?? "")).toContain("a"); unmount(); }); + + /** + * The repaint that was not going anywhere. + * + * The phrase is drawn only while the composer is empty, but the timer + * ran regardless — so from the first character typed, the app took a + * full Ink frame every four seconds to advance a string nobody could + * see. On a terminal that renders as bytes arrive, a full frame is a + * visible flicker. + */ + it("schedules nothing while inactive", () => { + const spy = vi.spyOn(global, "setInterval"); + const { unmount } = render( + , + ); + expect(spy).not.toHaveBeenCalled(); + spy.mockRestore(); + unmount(); + }); + + it("stops the timer when it goes inactive, and restarts when it returns", () => { + vi.spyOn(Math, "random").mockReturnValue(0); + const { lastFrame, rerender, unmount } = render( + , + ); + vi.advanceTimersByTime(1000); + rerender(); + expect(strip(lastFrame() ?? "")).toContain("b"); + + rerender(); + vi.advanceTimersByTime(5000); + rerender(); + // Five intervals passed and the phrase did not move, which means no + // render was scheduled for it either. + expect(strip(lastFrame() ?? "")).toContain("b"); + + rerender(); + vi.advanceTimersByTime(1000); + rerender(); + expect(strip(lastFrame() ?? "")).toContain("c"); + unmount(); + }); + + it("defaults to active, so existing callers are unchanged", () => { + const spy = vi.spyOn(global, "setInterval"); + const { unmount } = render(); + expect(spy).toHaveBeenCalled(); + spy.mockRestore(); + unmount(); + }); }); diff --git a/src/tui/hooks/use-rotating-placeholder.ts b/src/tui/hooks/use-rotating-placeholder.ts index ff25fea7..525b1fdb 100644 --- a/src/tui/hooks/use-rotating-placeholder.ts +++ b/src/tui/hooks/use-rotating-placeholder.ts @@ -10,20 +10,32 @@ import { useEffect, useState } from "react"; * Used by the prompt shell to surface a rotating set of "what could I * ask?" hints in the empty-input state — mirrors the opencode prompt * `placeholders.normal` behaviour without depending on Solid signals. + * + * `active` is what stops it repainting the app for nothing. The + * placeholder is drawn only while the composer is empty, but the timer + * used to run regardless: from the first character typed the phrase was + * invisible and the interval went on firing, and each fire is a + * `setState` at the root of the tree — a full Ink frame, every four + * seconds, for a string nobody could see. On a terminal that renders as + * bytes arrive, a full frame is a visible flicker (see + * `synchronized-output.ts`), so the cheapest repaint is the one that + * never happens. Same shape as `useSpinner` and `useAtomField`, which + * were already gated this way. */ export function useRotatingPlaceholder( phrases: readonly string[], intervalMs: number = 4000, + active: boolean = true, ): string | undefined { const initial = phrases.length > 0 ? Math.floor(Math.random() * phrases.length) : 0; const [idx, setIdx] = useState(initial); useEffect(() => { - if (phrases.length <= 1) return; + if (!active || phrases.length <= 1) return; const handle = setInterval(() => { setIdx((prev) => (prev + 1) % phrases.length); }, intervalMs); return () => clearInterval(handle); - }, [phrases, intervalMs]); + }, [active, phrases, intervalMs]); if (phrases.length === 0) return undefined; const safeIdx = idx % phrases.length; return phrases[safeIdx]; diff --git a/src/tui/synchronized-output.test.ts b/src/tui/synchronized-output.test.ts new file mode 100644 index 00000000..1ad05fd3 --- /dev/null +++ b/src/tui/synchronized-output.test.ts @@ -0,0 +1,115 @@ +import { describe, expect, it } from "vitest"; +import { + enableSynchronizedOutput, + looksLikeFrame, +} from "./synchronized-output.js"; + +const BSU = "\u001B[?2026h"; +const ESU = "\u001B[?2026l"; + +/** A stdout stand-in that records exactly what reached the terminal. */ +function fakeStdout(isTTY: boolean): NodeJS.WriteStream & { writes: string[] } { + const writes: string[] = []; + const stream = { + isTTY, + writes, + write(chunk: unknown): boolean { + writes.push(String(chunk)); + return true; + }, + }; + return stream as unknown as NodeJS.WriteStream & { writes: string[] }; +} + +const FRAME = "\u001B[H\u001B[2Kline one\nline two\nline three\n"; + +describe("enableSynchronizedOutput", () => { + it("wraps a frame in one write, not three", () => { + // Three writes would put the stream's own chunking between a marker + // and the frame it brackets — which is the thing this prevents. + const stdout = fakeStdout(true); + const controller = enableSynchronizedOutput({ stdout, env: {} }); + stdout.write(FRAME); + controller.restore(); + + expect(stdout.writes[0]).toBe(`${BSU}${FRAME}${ESU}`); + }); + + it("leaves short control sequences alone", () => { + const stdout = fakeStdout(true); + const controller = enableSynchronizedOutput({ stdout, env: {} }); + stdout.write("[?25l"); + controller.restore(); + + expect(stdout.writes[0]).toBe("[?25l"); + }); + + it("does nothing at all off a TTY", () => { + // A pipe cannot tear, and the markers would be noise in a captured + // log or a snapshot test. + const stdout = fakeStdout(false); + const controller = enableSynchronizedOutput({ stdout, env: {} }); + stdout.write(FRAME); + controller.restore(); + + expect(stdout.writes).toEqual([FRAME]); + }); + + it("honours the opt-out", () => { + const stdout = fakeStdout(true); + const controller = enableSynchronizedOutput({ + stdout, + env: { ATOMIC_AGENT_NO_SYNC_OUTPUT: "1" }, + }); + stdout.write(FRAME); + controller.restore(); + + expect(stdout.writes).toEqual([FRAME]); + }); + + it("restores the original write, and closes any open update", () => { + const stdout = fakeStdout(true); + const controller = enableSynchronizedOutput({ stdout, env: {} }); + controller.restore(); + stdout.write(FRAME); + + // The trailing ESU is insurance: a crash between the markers would + // otherwise leave the terminal holding its display. + expect(stdout.writes[0]).toBe(ESU); + expect(stdout.writes[1]).toBe(FRAME); + }); + + it("is safe to restore twice", () => { + const stdout = fakeStdout(true); + const controller = enableSynchronizedOutput({ stdout, env: {} }); + controller.restore(); + controller.restore(); + + expect(stdout.writes.filter((w) => w === ESU)).toHaveLength(1); + }); + + it("does not clobber a patch installed after ours", () => { + const stdout = fakeStdout(true); + const controller = enableSynchronizedOutput({ stdout, env: {} }); + const later = ((chunk: unknown) => { + stdout.writes.push(`later:${String(chunk)}`); + return true; + }) as NodeJS.WriteStream["write"]; + stdout.write = later; + controller.restore(); + + expect(stdout.write).toBe(later); + }); +}); + +describe("looksLikeFrame", () => { + it("counts anything with a newline, or anything long", () => { + expect(looksLikeFrame("a\nb")).toBe(true); + expect(looksLikeFrame("x".repeat(65))).toBe(true); + }); + + it("does not count a bare mode toggle", () => { + expect(looksLikeFrame("[?25h")).toBe(false); + expect(looksLikeFrame("[?1049l")).toBe(false); + }); +}); diff --git a/src/tui/synchronized-output.ts b/src/tui/synchronized-output.ts new file mode 100644 index 00000000..d75723a8 --- /dev/null +++ b/src/tui/synchronized-output.ts @@ -0,0 +1,123 @@ +/** + * Frame tearing, and the one escape sequence that ends it. + * + * Ink paints by writing a whole frame to stdout: cursor home, then every + * line, then the trailing clear. On a terminal that renders as bytes + * arrive, the moment between the first line and the last is a moment + * where the screen holds half of the previous frame and half of the + * next. Redraw often enough and that reads as the UI *shaking* — which + * is the word the Windows 10 report used, and the right one. + * + * It is not equally visible everywhere. macOS terminals coalesce + * aggressively enough to hide most of it; Windows Terminal and conhost + * do not, which is how a cross-platform behaviour arrived as a Windows + * bug report. + * + * DEC private mode 2026 — "synchronized output" — is the fix the + * terminal side already implemented. `CSI ? 2026 h` tells the emulator + * to hold what it renders until `CSI ? 2026 l`, so a frame appears whole + * or not at all. + * + * **Why it is safe to send unconditionally.** An unrecognised DEC private + * mode is ignored — that is what the "private" in the name buys, and it + * is why terminals without 2026 have always received these bytes from + * other TUIs without complaint. There is no probe here for the same + * reason there is none in `alt-screen.ts`: asking costs a round trip on + * a stdin something else is reading, and the fallback is exactly the + * behaviour we have today. + * + * `ATOMIC_AGENT_NO_SYNC_OUTPUT=1` turns it off for anyone who finds a + * terminal that mishandles it. + */ +import { registerTerminalRestore } from "./terminal-restore.js"; + +/** Begin Synchronized Update. */ +const BSU = "\u001B[?2026h"; +/** End Synchronized Update. */ +const ESU = "\u001B[?2026l"; + +export interface SynchronizedOutputController { + /** Puts the original `write` back. Safe to call twice. */ + restore(): void; +} + +export interface SynchronizedOutputOptions { + readonly stdout?: NodeJS.WriteStream; + /** Env source for the opt-out; injectable for tests. */ + readonly env?: NodeJS.ProcessEnv; +} + +/** + * True when `chunk` is worth bracketing. + * + * Wrapping a lone escape sequence in a synchronized update is not wrong, + * but it is two extra sequences spent rendering nothing — and this + * `write` sees every cursor nudge and mode toggle the app makes, not + * only Ink's frames. A frame is many bytes and contains a newline; a + * mode toggle is neither. + */ +export function looksLikeFrame(chunk: string): boolean { + return chunk.length > 64 || chunk.includes("\n"); +} + +/** + * Bracket each frame Ink writes in a synchronized update. + * + * Implemented by replacing `stdout.write` rather than by handing Ink a + * wrapper stream, because Ink reads `columns`, `rows` and the `resize` + * event off that same object — a proxy would have to forward all of it + * correctly, and getting that subtly wrong breaks resize handling in a + * way much harder to spot than tearing. Patching one method on the real + * stream leaves every other property where it was. `alt-screen.ts` and + * `mouse-tracking.ts` already own the terminal this way. + * + * Non-TTY streams (pipes, CI, the test renderer) are left alone: there + * is nothing to tear, and the markers would be noise in a captured log. + */ +export function enableSynchronizedOutput( + options: SynchronizedOutputOptions = {}, +): SynchronizedOutputController { + const stdout = options.stdout ?? process.stdout; + const env = options.env ?? process.env; + if (!stdout.isTTY || env.ATOMIC_AGENT_NO_SYNC_OUTPUT === "1") { + return { restore: () => {} }; + } + const original = stdout.write.bind(stdout) as ( + chunk: unknown, + ...rest: unknown[] + ) => boolean; + let restored = false; + + const patched = (( + chunk: unknown, + encoding?: unknown, + callback?: unknown, + ): boolean => { + const rest = [encoding, callback].filter((arg) => arg !== undefined); + if (typeof chunk === "string" && looksLikeFrame(chunk)) { + // One `write`, not three. Three would put the stream's own + // chunking between a marker and the frame it is meant to bracket, + // which is the very thing this exists to prevent. + return original(`${BSU}${chunk}${ESU}`, ...rest); + } + return original(chunk, ...rest); + }) as NodeJS.WriteStream["write"]; + + stdout.write = patched; + + const restore = (): void => { + if (restored) return; + restored = true; + // Only put it back if nothing else has patched over us since; + // clobbering a later patch would be worse than leaving ours in. + if (stdout.write === patched) { + stdout.write = original as NodeJS.WriteStream["write"]; + } + // A crash between BSU and ESU would leave the terminal holding its + // display indefinitely. Cheap insurance, and inert where 2026 is not + // implemented. + original(ESU); + }; + registerTerminalRestore(restore); + return { restore }; +} diff --git a/src/tui/tui-command.ts b/src/tui/tui-command.ts index 1296e318..c7d203ab 100644 --- a/src/tui/tui-command.ts +++ b/src/tui/tui-command.ts @@ -17,6 +17,7 @@ import type { MetricSample, MetricSink } from "../tracing/metrics-collector.js"; import { isKnownLocalModelId } from "../local-llm/index.js"; import { registerSession } from "../local-llm/session-registry.js"; import { enterAltScreen } from "./alt-screen.js"; +import { enableSynchronizedOutput } from "./synchronized-output.js"; import { ChatOrchestrator } from "./chat-orchestrator.js"; import { parseTuiArgs, nonInteractiveStdinError, @@ -233,6 +234,11 @@ export async function tuiCommand(args: string[]): Promise { const releaseSession = registerSession(config.paths.localModelsDataDir); const altScreen = enterAltScreen({ stdout: process.stdout, hideCursor: false }); + // Immediately after the alt screen and before the first render: every + // frame from here on is bracketed as one synchronized update, so a + // terminal that renders as bytes arrive shows whole frames instead of + // half of the old one and half of the new. + const synchronizedOutput = enableSynchronizedOutput({ stdout: process.stdout }); // Mouse support. Enabling SGR tracking (1000 + 1006) is what makes // clicking panels, rows, tabs and the prompt work at all — the app @@ -633,6 +639,9 @@ export async function tuiCommand(args: string[]): Promise { mouseTracking?.disable(); mouseStdin.dispose(); altScreen.restore(); + // After the alt screen, so the restore's own writes are still + // bracketed, and before `ink.clear()` for the same reason. + synchronizedOutput.restore(); ink.clear(); } catch { // After SIGHUP the tty is gone and these writes raise EIO; the