From 38ddc6a7aee3600e26d097a2339ec162a829ae06 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 2 Oct 2026 08:29:56 -0700 Subject: [PATCH] Fix duplicated helper command echo in visual snapshots --- docs/specs/terminal-context.md | 4 +- .../lib/platform/fake-adapter-helper.test.ts | 40 +++++++++++++++++++ lib/src/lib/platform/fake-adapter.ts | 26 +++++++----- lib/src/stories/HelperPlacement.stories.tsx | 6 +++ 4 files changed, 65 insertions(+), 11 deletions(-) create mode 100644 lib/src/lib/platform/fake-adapter-helper.test.ts diff --git a/docs/specs/terminal-context.md b/docs/specs/terminal-context.md index f00fa3f5b..57bfc613c 100644 --- a/docs/specs/terminal-context.md +++ b/docs/specs/terminal-context.md @@ -53,11 +53,13 @@ Source of truth: `context` in `standalone/sidecar/pty-core.js`; `terminalContext **Must make diagnostic text drag-selectable**, including detail-dialog errors, without focusing the helper. Copy routing follows `docs/specs/mouse-and-clipboard.md` → "Terminal context input". +**Must drain the helper's queued xterm writes and verify a single autorun command echo before placement snapshots.** + **Must suppress xterm's auto-revealed scrollbar in visual snapshots**, while retaining terminal scrolling and layout. **Must fit every control inside the panel at its minimum width, label included.** Port action overflow follows `docs/specs/layout.md` → "Header context menu". The gallery's play check measures each button against the panel and against its own box. -Source of truth: `TerminalContextView` in `lib/src/components/wall/TerminalContextView.tsx`; `lib/src/stories/TerminalContext.stories.tsx` supplies sample output; `lib/src/stories/Wall.stories.tsx` exercises the live helper. `lib/src/stories/HelperPlacement.stories.tsx` checks rendered placement and real xterm input/focus retention; the gallery checks narrow controls and always-visible details. `visualSnapshot` in `lib/.storybook/preview.ts` suppresses scrollbar paint. +Source of truth: `TerminalContextView` in `lib/src/components/wall/TerminalContextView.tsx`; `lib/src/stories/TerminalContext.stories.tsx` supplies sample output; `lib/src/stories/Wall.stories.tsx` exercises the live helper. `lib/src/stories/HelperPlacement.stories.tsx` checks rendered placement and real xterm input/focus retention; the gallery checks narrow controls and always-visible details. `visualSnapshot` in `lib/.storybook/preview.ts` suppresses scrollbar paint. Tests: `lib/src/lib/platform/fake-adapter-helper.test.ts`. The Window-host workspace picker follows `docs/specs/layout.md` → Moving Surfaces between Workspaces. diff --git a/lib/src/lib/platform/fake-adapter-helper.test.ts b/lib/src/lib/platform/fake-adapter-helper.test.ts new file mode 100644 index 000000000..a6e71dc40 --- /dev/null +++ b/lib/src/lib/platform/fake-adapter-helper.test.ts @@ -0,0 +1,40 @@ +// @vitest-environment jsdom +import { afterEach, expect, it, vi } from 'vitest'; +import { Terminal } from '@xterm/xterm'; +import { UnicodeGraphemesAddon } from '@xterm/addon-unicode-graphemes'; +import { FakePtyAdapter } from './fake-adapter'; +import { removeTerminalPaneState } from '../terminal-state-store'; + +const ID = 'helper-reflow'; +afterEach(() => { vi.restoreAllMocks(); removeTerminalPaneState(ID); }); + +it('preserves one command echo when the helper resizes between PTY chunks', async () => { + const adapter = new FakePtyAdapter(); + const terminal = new Terminal({ cols: 33, rows: 7, allowProposedApi: true }); + terminal.loadAddon(new UnicodeGraphemesAddon()); + adapter.onPtyData(({ data }) => terminal.write(data)); + const drain = () => new Promise(resolve => terminal.write('', resolve)); + try { + adapter.spawnPty(ID, { helper: { parentId: 'source', command: 'git status' } }); + await Promise.resolve(); + await drain(); + // Make xterm yield after ten small writes: the echo, before its CRLF. + let clockReads = 0; + const clock = vi.spyOn(performance, 'now').mockImplementation(() => Math.floor(++clockReads / 11) * 20); + const resized = new Promise(resolve => { + const listener = terminal.onWriteParsed(() => { + listener.dispose(); + terminal.resize(26, 7); + resolve(); + }); + }); + adapter.writePty(ID, 'git status\r'); + await resized; + clock.mockRestore(); + await drain(); + const text = Array.from({ length: terminal.buffer.active.length }, (_, i) => + terminal.buffer.active.getLine(i)?.translateToString(true) ?? '').join(''); + expect(text.match(/git status/g)).toHaveLength(1); + expect(text.match(/On branch main/g)).toHaveLength(1); + } finally { terminal.dispose(); adapter.shutdown(); } +}); diff --git a/lib/src/lib/platform/fake-adapter.ts b/lib/src/lib/platform/fake-adapter.ts index b5fabecd4..90efede90 100644 --- a/lib/src/lib/platform/fake-adapter.ts +++ b/lib/src/lib/platform/fake-adapter.ts @@ -223,23 +223,29 @@ export class FakePtyAdapter implements PlatformAdapter { private startHelperShell(id: string): void { const helper = this.helpers.get(id)!; let input = ''; - const prompt = () => this.sendOutput(id, `\x1b]633;A\x07${helper.cwd} ❯ \x1b]633;B\x07`); + const prompt = () => `\x1b]633;A\x07${helper.cwd} ❯ \x1b]633;B\x07`; this.inputHandlers.set(id, data => { - if (data === '\x03') { helper.busy = false; input = ''; this.sendOutput(id, '^C\r\n\x1b]633;D;130\x07'); prompt(); return; } + if (data === '\x03') { helper.busy = false; input = ''; this.sendOutput(id, '^C\r\n\x1b]633;D;130\x07' + prompt()); return; } if (helper.busy) return; + // Submit the echo, command output, and returned prompt as one PTY chunk. + // xterm 6.1.0-beta.304 can yield after parsing the echo, then replay that + // parsed prefix when a resize flushes the remaining queue. Keep the CRLF + // in the same write as the echo (fake-adapter-helper.test.ts). + let output = ''; for (const char of data) { if (char === '\r' || char === '\n') { const command = input; input = ''; - this.sendOutput(id, `\r\n\x1b]633;E;${command}\x07\x1b]633;C\x07`); - if (/^(sleep|nano|vim)\b/.test(command)) { helper.busy = true; this.sendOutput(id, 'Demo process running. Ctrl+C stops it.\r\n'); continue; } - const output = command === 'git status' ? 'On branch main\r\nnothing to commit, working tree clean' : command.startsWith('echo ') ? command.slice(5) : command === 'pwd' ? helper.cwd : command ? `Demo shell: ${command}` : ''; - if (output) this.sendOutput(id, output + '\r\n'); - this.sendOutput(id, '\x1b]633;D;0\x07'); prompt(); - } else if (char === '\x7f') { if (input) { input = input.slice(0, -1); this.sendOutput(id, '\b \b'); } } - else { input += char; this.sendOutput(id, char); } + output += `\r\n\x1b]633;E;${command}\x07\x1b]633;C\x07`; + if (/^(sleep|nano|vim)\b/.test(command)) { helper.busy = true; output += 'Demo process running. Ctrl+C stops it.\r\n'; continue; } + const result = command === 'git status' ? 'On branch main\r\nnothing to commit, working tree clean' : command.startsWith('echo ') ? command.slice(5) : command === 'pwd' ? helper.cwd : command ? `Demo shell: ${command}` : ''; + if (result) output += result + '\r\n'; + output += '\x1b]633;D;0\x07' + prompt(); + } else if (char === '\x7f') { if (input) { input = input.slice(0, -1); output += '\b \b'; } } + else { input += char; output += char; } } + if (output) this.sendOutput(id, output); }); - queueMicrotask(prompt); + queueMicrotask(() => this.sendOutput(id, prompt())); } private resolveScenario(id: string): FakeScenario | null { diff --git a/lib/src/stories/HelperPlacement.stories.tsx b/lib/src/stories/HelperPlacement.stories.tsx index 0ff93fcec..afd5402ee 100644 --- a/lib/src/stories/HelperPlacement.stories.tsx +++ b/lib/src/stories/HelperPlacement.stories.tsx @@ -58,6 +58,12 @@ async function openContext() { await rightClickSourceHeader(); await settleTerminalContext(); expect(getHelper(SOURCE)?.status).toBe('completed'); + const helper = terminal(getHelper(SOURCE)!.id); + // Host completion precedes xterm's asynchronous parsing of the final chunk. + await new Promise(resolve => helper.write('', resolve)); + const text = Array.from({ length: helper.buffer.active.length }, (_, i) => + helper.buffer.active.getLine(i)?.translateToString(true) ?? '').join(''); + expect(text.match(/git status/g)).toHaveLength(1); } function expectedSide({ layout, zoomed, cursor, sourceAtEnd }: Props) { // Alone in the Wall, the helper avoids the cursor; beside a neighbor, it takes the neighbor's side.