From c1a2f4dde73f8010d15cdbc8b2f7600a0917da14 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Wed, 5 Aug 2026 00:19:53 +0800 Subject: [PATCH] fix(tui): keep whitespace-prefixed bang input in chat (#119245) * chore: start T06 local shell proof * fix(tui): preserve whitespace before bang submits * fix(tui): defer paste expansion until submit * fix(tui): preserve bang spacing across blocked retry * fix(tui): keep whitespace bang chat out of history * fix(tui): preserve bang routing across trim --- qa/scenarios/ui/tui-local-shell-pty.yaml | 44 ++++++++++ src/tui/components/custom-editor.test.ts | 53 ++++++++++++ src/tui/components/custom-editor.ts | 16 +++- src/tui/tui-pty-local.e2e.test.ts | 71 +++++++++++++++ src/tui/tui-submit.ts | 19 +++- src/tui/tui.submit-handler.test.ts | 106 ++++++++++++++++++++--- 6 files changed, 293 insertions(+), 16 deletions(-) create mode 100644 qa/scenarios/ui/tui-local-shell-pty.yaml diff --git a/qa/scenarios/ui/tui-local-shell-pty.yaml b/qa/scenarios/ui/tui-local-shell-pty.yaml new file mode 100644 index 000000000000..37b5c8aa476d --- /dev/null +++ b/qa/scenarios/ui/tui-local-shell-pty.yaml @@ -0,0 +1,44 @@ +title: TUI local shell routing PTY contracts +scenario: + id: tui-local-shell-pty + surface: tui + category: tui.local-shell-execution + coverage: + primary: + - tui.bang-command-routing + - tui.approval-prompt + - tui.command-output-display + - tui.execution-environment-marker + risk: high + objective: Prove local shell routing, confirmation, output, and environment behavior through the built local TUI and a real PTY. + successCriteria: + - Whitespace-prefixed bang input remains chat input after local shell execution has already been approved. + - Column-one bang input requires explicit session approval before execution. + - Approved commands visibly render stdout, stderr, and their terminal exit status. + - Approved commands receive the TUI local shell environment marker. + codeRefs: + - src/tui/components/custom-editor.ts + - src/tui/tui-submit.ts + - src/tui/tui-local-shell.ts + - src/tui/tui-pty-local.e2e.test.ts + execution: + kind: script + path: test/e2e/qa-lab/tui/tui-pty-evidence-producer.ts + summary: Runs exact built local-TUI PTY assertions and authenticates their fresh Vitest results as QA evidence. + timeoutMs: 360000 + args: [--artifact-base, "${outputDir}", --scenario-id, "${scenarioId}"] + config: + requireBuiltCli: true + tuiPtyCases: + - coverageId: tui.bang-command-routing + testFile: src/tui/tui-pty-local.e2e.test.ts + testNamePattern: ^TUI PTY real backends keeps whitespace-prefixed bang input in chat after local shell approval$ + - coverageId: tui.approval-prompt + testFile: src/tui/tui-pty-local.e2e.test.ts + testNamePattern: ^TUI PTY real backends confirms and renders local shell output and environment through a real local PTY$ + - coverageId: tui.command-output-display + testFile: src/tui/tui-pty-local.e2e.test.ts + testNamePattern: ^TUI PTY real backends confirms and renders local shell output and environment through a real local PTY$ + - coverageId: tui.execution-environment-marker + testFile: src/tui/tui-pty-local.e2e.test.ts + testNamePattern: ^TUI PTY real backends confirms and renders local shell output and environment through a real local PTY$ diff --git a/src/tui/components/custom-editor.test.ts b/src/tui/components/custom-editor.test.ts index 0c8827e499a4..976095dee9fa 100644 --- a/src/tui/components/custom-editor.test.ts +++ b/src/tui/components/custom-editor.test.ts @@ -212,4 +212,57 @@ describe("CustomEditor", () => { expect(onSubmit).toHaveBeenCalledWith("/help"); }); + + it.each([" !cmd", " !cmd\n", "!cmd\n", "\n!cmd\n"])( + "preserves %j when trimming would create executable bang input", + (input) => { + const tui = { requestRender: vi.fn() } as unknown as TUI; + const editor = new CustomEditor(tui, editorTheme); + const onSubmit = vi.fn(); + editor.onSubmit = onSubmit; + editor.setText(input); + + editor.handleInput("\r"); + + expect(onSubmit).toHaveBeenCalledExactlyOnceWith(input); + expect(editor.getText()).toBe(""); + }, + ); + + it("leaves harmless bang-prefixed multiline chat on pi-tui's normal submit path", () => { + const tui = { requestRender: vi.fn() } as unknown as TUI; + const editor = new CustomEditor(tui, editorTheme); + const onSubmit = vi.fn(); + editor.onSubmit = onSubmit; + editor.setText(" \n!cmd\nnotes"); + + editor.handleInput("\r"); + + expect(onSubmit).toHaveBeenCalledExactlyOnceWith("!cmd\nnotes"); + expect(editor.getText()).toBe(""); + }); + + it("does not expand stored paste text for ordinary input", () => { + const tui = { requestRender: vi.fn() } as unknown as TUI; + const editor = new CustomEditor(tui, editorTheme); + editor.setText("draft"); + const getExpandedText = vi.spyOn(editor, "getExpandedText"); + + editor.handleInput("x"); + + expect(getExpandedText).not.toHaveBeenCalled(); + expect(editor.getText()).toBe("draftx"); + }); + + it("keeps pi-tui trimming for ordinary submissions", () => { + const tui = { requestRender: vi.fn() } as unknown as TUI; + const editor = new CustomEditor(tui, editorTheme); + const onSubmit = vi.fn(); + editor.onSubmit = onSubmit; + editor.setText(" ordinary message "); + + editor.handleInput("\r"); + + expect(onSubmit).toHaveBeenCalledExactlyOnceWith("ordinary message"); + }); }); diff --git a/src/tui/components/custom-editor.ts b/src/tui/components/custom-editor.ts index 0cfc9adc5135..37365545a578 100644 --- a/src/tui/components/custom-editor.ts +++ b/src/tui/components/custom-editor.ts @@ -1,5 +1,6 @@ // Custom editor component handles multiline TUI input and key bindings. import { Editor, getKeybindings, isKeyRelease, Key, matchesKey } from "@earendil-works/pi-tui"; +import { trimWouldCreateExecutableBangLine } from "../tui-submit.js"; // Kitty keyboard protocol uses CSI-u sequences for AltGr on international layouts. const KITTY_CSI_U_SUFFIX_REGEX = /^(\d+)(?::(\d*))?(?::(\d+))?(?:;(\d+))?(?::(\d+))?u$/u; @@ -57,7 +58,7 @@ export class CustomEditor extends Editor { onAltUp?: () => void; shouldSubmitAutocomplete?: (text: string) => boolean; - /** Dispatches TUI shortcuts before falling back to normal editor input handling. */ + /** Preserves text when pi-tui trimming would create an executable bang line. */ override handleInput(data: string): void { if (isKeyRelease(data)) { return; @@ -130,6 +131,19 @@ export class CustomEditor extends Editor { this.setText(this.getText()); } + if (keybindings.matches(data, "tui.input.submit") && this.onSubmit) { + const expandedText = this.getExpandedText(); + if (trimWouldCreateExecutableBangLine(expandedText)) { + const onSubmit = this.onSubmit; + this.onSubmit = () => onSubmit(expandedText); + try { + super.handleInput(data); + } finally { + this.onSubmit = onSubmit; + } + return; + } + } super.handleInput(data); } } diff --git a/src/tui/tui-pty-local.e2e.test.ts b/src/tui/tui-pty-local.e2e.test.ts index 4882ad0e1f34..8ccd9f9cd573 100644 --- a/src/tui/tui-pty-local.e2e.test.ts +++ b/src/tui/tui-pty-local.e2e.test.ts @@ -1126,6 +1126,77 @@ describe("TUI PTY real backends", () => { LOCAL_TEST_TIMEOUT_MS, ); + it( + "keeps whitespace-prefixed bang input in chat after local shell approval", + async ({ onTestFinished }) => { + const fixture = await startLocalModeTui(onTestFinished, { + replyText: "T06_CHAT_RESPONSE", + }); + try { + await fixture.run.waitForOutput("local ready", LOCAL_STARTUP_TIMEOUT_MS); + await fixture.run.write("!node -e \"console.log('T06_APPROVAL_PRIMER')\"\r"); + await fixture.run.waitForOutput("Allow local shell commands for this session?"); + await fixture.run.write("\u001b[B\r", { delay: false }); + await fixture.run.waitForOutput("local shell: enabled for this session"); + await fixture.run.waitForOutput("[local] T06_APPROVAL_PRIMER"); + await fixture.run.waitForOutput("[local] exit 0"); + + const chatOffset = fixture.run.visibleOutput().length; + const command = " !node -e \"console.log('T06_UNEXPECTED_EXECUTION')\""; + await fixture.run.write(`${command}\r`); + await waitFor({ + timeoutMs: LOCAL_OUTPUT_TIMEOUT_MS, + read: () => (fixture.mockModel.requests().length === 1 ? true : null), + onTimeout: () => + new Error(`whitespace-prefixed bang did not reach chat\n${fixture.run.output()}`), + }); + expect(JSON.stringify(fixture.mockModel.requests()[0]?.body)).toContain( + "T06_UNEXPECTED_EXECUTION", + ); + await waitForOutputAfter(fixture.run, "T06_CHAT_RESPONSE", chatOffset); + expect(fixture.run.visibleOutput().slice(chatOffset)).not.toContain( + "[local] T06_UNEXPECTED_EXECUTION", + ); + + await fixture.run.write("/exit\r", { delay: false }); + expect((await fixture.run.waitForExit()).exitCode).toBe(0); + } finally { + await fixture.cleanup(); + } + }, + LOCAL_TEST_TIMEOUT_MS, + ); + + it( + "confirms and renders local shell output and environment through a real local PTY", + async ({ onTestFinished }) => { + const fixture = await startLocalModeTui(onTestFinished); + try { + await fixture.run.waitForOutput("local ready", LOCAL_STARTUP_TIMEOUT_MS); + await fixture.run.write( + "!node -e \"console.log('T06_STDOUT'); console.error('T06_STDERR'); console.log('T06_ENV='+process.env.OPENCLAW_SHELL); process.exitCode=7\"\r", + ); + await fixture.run.waitForOutput("Allow local shell commands for this session?"); + await fixture.run.waitForOutput("Select Yes/No (arrows + Enter), Esc to cancel."); + expect(fixture.run.visibleOutput()).toContain("No"); + expect(fixture.run.visibleOutput()).toContain("Yes"); + + await fixture.run.write("\u001b[B\r", { delay: false }); + await fixture.run.waitForOutput("local shell: enabled for this session"); + await fixture.run.waitForOutput("[local] T06_STDOUT"); + await fixture.run.waitForOutput("[local] T06_STDERR"); + await fixture.run.waitForOutput("[local] T06_ENV=tui-local"); + await fixture.run.waitForOutput("[local] exit 7"); + + await fixture.run.write("/exit\r", { delay: false }); + expect((await fixture.run.waitForExit()).exitCode).toBe(0); + } finally { + await fixture.cleanup(); + } + }, + LOCAL_TEST_TIMEOUT_MS, + ); + function registerValidationLoopTest(mode: "gateway" | "local") { it( `renders safe validation-loop abort diagnostics through the real ${mode} backend`, diff --git a/src/tui/tui-submit.ts b/src/tui/tui-submit.ts index 547d70eb6d01..5ba040403fc6 100644 --- a/src/tui/tui-submit.ts +++ b/src/tui/tui-submit.ts @@ -8,6 +8,14 @@ import type { export type TuiSubmitAction = "local shell" | "command" | "message"; +function isExecutableBangLine(text: string): boolean { + return !text.includes("\n") && text.startsWith("!") && text !== "!"; +} + +export function trimWouldCreateExecutableBangLine(text: string): boolean { + return !isExecutableBangLine(text) && isExecutableBangLine(text.trim()); +} + function runSubmitAction( action: TuiSubmitAction, run: () => Promise | void, @@ -53,6 +61,7 @@ export function createEditorSubmitHandler(params: { const raw = text; const value = raw.trim(); const multiline = raw.includes("\n"); + const trimCreatesExecutableBangLine = trimWouldCreateExecutableBangLine(raw); // Keep previous behavior: ignore empty/whitespace-only submissions. if (!value) { @@ -63,7 +72,7 @@ export function createEditorSubmitHandler(params: { // Bash mode: only if the very first character is '!' and it's not just '!'. // IMPORTANT: use the raw (untrimmed) text so leading spaces do NOT trigger. // Per requirement: a lone '!' should be treated as a normal message. - if (!multiline && raw.startsWith("!") && raw !== "!") { + if (isExecutableBangLine(raw)) { clearSubmittedEditor(); params.editor.addToHistory(raw); runSubmitAction("local shell", () => params.handleBangLine(raw), params.onSubmitError); @@ -82,14 +91,16 @@ export function createEditorSubmitHandler(params: { ? params.admitMessage?.(value, snapshot) : params.admitMessage?.(value)) ?? { status: "allowed" }; if (admission.status === "blocked") { - restoreBlockedEditor(value); + restoreBlockedEditor(trimCreatesExecutableBangLine ? raw : value); params.onBlockedMessageSubmit?.(value, admission); return; } clearSubmittedEditor(); - // Enable built-in editor prompt history navigation (up/down). - params.editor.addToHistory(value); + // Omit chat text whose trimmed history recall would become executable shell input. + if (!trimCreatesExecutableBangLine) { + params.editor.addToHistory(value); + } runSubmitAction("message", () => params.sendMessage(value), params.onSubmitError); }; } diff --git a/src/tui/tui.submit-handler.test.ts b/src/tui/tui.submit-handler.test.ts index 1f003bdb8249..49d79e8e3c5b 100644 --- a/src/tui/tui.submit-handler.test.ts +++ b/src/tui/tui.submit-handler.test.ts @@ -10,16 +10,39 @@ import { shouldEnableWindowsGitBashPasteFallback, } from "./tui-submit.js"; -describe("createEditorSubmitHandler", () => { - it("routes lines starting with ! to handleBangLine", () => { - const { handleCommand, sendMessage, handleBangLine, onSubmit } = createSubmitHarness(); +function createRealEditorSubmitHarness( + admitMessage?: NonNullable[0]["admitMessage"]>, +) { + const tui = { requestRender: vi.fn() } as unknown as TUI; + const editor = new CustomEditor(tui, editorTheme); + const sendMessage = vi.fn(); + const handleBangLine = vi.fn(); + editor.onSubmit = createEditorSubmitHandler({ + editor, + handleCommand: vi.fn(), + sendMessage, + handleBangLine, + onSubmitError: vi.fn(), + ...(admitMessage ? { admitMessage } : {}), + }); + return { editor, sendMessage, handleBangLine }; +} - onSubmit("!ls"); +describe("createEditorSubmitHandler", () => { + it("routes genuine bang input to local shell and history", () => { + const { editor, sendMessage, handleBangLine } = createRealEditorSubmitHarness(); + editor.setText("!cmd"); + + editor.handleInput("\r"); expect(handleBangLine).toHaveBeenCalledTimes(1); - expect(handleBangLine).toHaveBeenCalledWith("!ls"); + expect(handleBangLine).toHaveBeenCalledWith("!cmd"); expect(sendMessage).not.toHaveBeenCalled(); - expect(handleCommand).not.toHaveBeenCalled(); + expect(editor.getText()).toBe(""); + + editor.handleInput("\u001b[A"); + + expect(editor.getText()).toBe("!cmd"); }); it("treats a lone ! as a normal message", () => { @@ -32,14 +55,75 @@ describe("createEditorSubmitHandler", () => { expect(sendMessage).toHaveBeenCalledWith("!"); }); - it("does not treat leading whitespace before ! as a bang command", () => { - const { editor, sendMessage, handleBangLine, onSubmit } = createSubmitHarness(); + it.each([ + { name: "a whitespace-prefixed lone bang", input: " !", expected: "!" }, + { + name: "bang-prefixed true multiline chat", + input: " \n!cmd\nnotes", + expected: "!cmd\nnotes", + }, + ])("stores, recalls, and safely resubmits $name", ({ input, expected }) => { + const { editor, sendMessage, handleBangLine } = createRealEditorSubmitHarness(); + editor.setText(input); - onSubmit(" !ls"); + editor.handleInput("\r"); expect(handleBangLine).not.toHaveBeenCalled(); - expect(sendMessage).toHaveBeenCalledWith("!ls"); - expect(editor.addToHistory).toHaveBeenCalledWith("!ls"); + expect(sendMessage).toHaveBeenCalledExactlyOnceWith(expected); + expect(editor.getText()).toBe(""); + + editor.handleInput("\u001b[A"); + expect(editor.getText()).toBe(expected); + + editor.handleInput("\r"); + + expect(sendMessage).toHaveBeenCalledTimes(2); + expect(sendMessage).toHaveBeenNthCalledWith(2, expected); + expect(handleBangLine).not.toHaveBeenCalled(); + }); + + it.each([" !cmd", " !cmd\n", "!cmd\n", "\n!cmd\n"])( + "keeps %j in chat and omits it from history", + (input) => { + const { editor, sendMessage, handleBangLine } = createRealEditorSubmitHarness(); + editor.setText(input); + + editor.handleInput("\r"); + + expect(sendMessage).toHaveBeenCalledExactlyOnceWith("!cmd"); + expect(handleBangLine).not.toHaveBeenCalled(); + expect(editor.getText()).toBe(""); + + editor.handleInput("\u001b[A"); + expect(editor.getText()).toBe(""); + + editor.handleInput("\r"); + + expect(sendMessage).toHaveBeenCalledExactlyOnceWith("!cmd"); + expect(handleBangLine).not.toHaveBeenCalled(); + }, + ); + + it("preserves whitespace bang routing across a blocked retry", () => { + const admitMessage = vi + .fn() + .mockReturnValueOnce({ status: "blocked", reason: "pending" }) + .mockReturnValueOnce({ status: "allowed" }); + const { editor, sendMessage, handleBangLine } = createRealEditorSubmitHarness(admitMessage); + editor.setText(" !cmd"); + + editor.handleInput("\r"); + + expect(editor.getText()).toBe(" !cmd"); + expect(sendMessage).not.toHaveBeenCalled(); + expect(handleBangLine).not.toHaveBeenCalled(); + + editor.handleInput("\r"); + + expect(admitMessage).toHaveBeenCalledTimes(2); + expect(sendMessage).toHaveBeenCalledExactlyOnceWith("!cmd"); + expect(handleBangLine).not.toHaveBeenCalled(); + expect(editor.getText()).toBe(""); }); it("trims normal messages before sending and adding to history", () => {