From 7a1280784ee615633af6838e168a13a5fab81a41 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 4 Jun 2026 07:44:52 +0000 Subject: [PATCH] fix(tools/ssh): rendered multiline remote commands in a framed body block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SSH tool renderer fed the raw command into renderStatusLine's description, so any newline in the remote command expanded the single-line tool header — the bordered output block then opened mid-command and the rest of the SSH cell rendered against a broken frame. The renderer now keeps only [host] in the header and renders the full command (with a dim $ prefix and tab sanitization) as its own framed section above Output, matching the bash renderer's shape. renderStatusLine also flattens CR/LF in title, description, meta, and badge labels so no future caller can accidentally produce a multiline tool header. Covered by: - test/tools/ssh-render.test.ts: multiline command stays out of the header and every command line is present in the body, for both renderCall and renderResult. - test/tui/status-line-newline-guard.test.ts: embedded LF, CRLF, and lone CR in description/meta are flattened to spaces. Fixes #1828 --- packages/coding-agent/CHANGELOG.md | 4 ++ packages/coding-agent/src/tools/ssh.ts | 36 ++++++++---- packages/coding-agent/src/tui/status-line.ts | 19 ++++-- .../test/tools/ssh-render.test.ts | 58 +++++++++++++++++++ .../tui/status-line-newline-guard.test.ts | 47 +++++++++++++++ 5 files changed, 150 insertions(+), 14 deletions(-) create mode 100644 packages/coding-agent/test/tools/ssh-render.test.ts create mode 100644 packages/coding-agent/test/tui/status-line-newline-guard.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index dcbd68a54..00c37c826 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed the SSH tool renderer inlining multiline remote commands into its single-line status header, which produced a malformed cell where the bordered output block opened mid-command. The renderer now drops the command from the header (which keeps only `[host]`) and renders the full command in a framed section above `Output`, mirroring the bash renderer. `renderStatusLine` also flattens any embedded CR/LF in `description`, `meta`, and `title` so no tool can accidentally expand the header into multiple rows ([#1828](https://github.com/can1357/oh-my-pi/issues/1828)). + ## [15.9.0] - 2026-06-04 ### Breaking Changes diff --git a/packages/coding-agent/src/tools/ssh.ts b/packages/coding-agent/src/tools/ssh.ts index 676834d41..9f821beb1 100644 --- a/packages/coding-agent/src/tools/ssh.ts +++ b/packages/coding-agent/src/tools/ssh.ts @@ -1,6 +1,5 @@ import type { AgentTool, AgentToolContext, AgentToolResult, AgentToolUpdateCallback } from "@oh-my-pi/pi-agent-core"; import type { Component } from "@oh-my-pi/pi-tui"; -import { Text } from "@oh-my-pi/pi-tui"; import { prompt } from "@oh-my-pi/pi-utils"; import * as z from "zod/v4"; import type { SSHHost } from "../capability/ssh"; @@ -18,6 +17,7 @@ import { CachedOutputBlock } from "../tui/output-block"; import type { ToolSession } from "."; import { truncateForPrompt } from "./approval"; import { formatStyledTruncationWarning, type OutputMeta, stripOutputNotice } from "./output-meta"; +import { replaceTabs } from "./render-utils"; import { ToolError } from "./tool-errors"; import { toolResult } from "./tool-result"; import { clampTimeout } from "./tool-timeouts"; @@ -230,12 +230,30 @@ interface SshRenderContext { totalVisualLines?: number; } +function formatSshCommandLines(command: string, uiTheme: Theme): string[] { + const sanitized = replaceTabs(command); + const rawLines = sanitized.length > 0 ? sanitized.split("\n") : ["…"]; + const prefix = uiTheme.fg("dim", "$ "); + return rawLines.map((line, i) => (i === 0 ? `${prefix}${line}` : line)); +} + export const sshToolRenderer = { renderCall(args: SshRenderArgs, _options: RenderResultOptions, uiTheme: Theme): Component { const host = args.host || "…"; - const command = args.command || "…"; - const text = renderStatusLine({ icon: "pending", title: "SSH", description: `[${host}] $ ${command}` }, uiTheme); - return new Text(text, 0, 0); + const command = args.command ?? ""; + const header = renderStatusLine({ icon: "pending", title: "SSH", description: `[${host}]` }, uiTheme); + const cmdLines = formatSshCommandLines(command, uiTheme); + const outputBlock = new CachedOutputBlock(); + return { + render: (width: number): string[] => + outputBlock.render( + { header, state: "pending", sections: [{ lines: cmdLines }], width, animate: true }, + uiTheme, + ), + invalidate: () => { + outputBlock.invalidate(); + }, + }; }, renderResult( @@ -249,11 +267,9 @@ export const sshToolRenderer = { ): Component { const details = result.details; const host = args?.host || "…"; - const command = args?.command || "…"; - const header = renderStatusLine( - { icon: "success", title: "SSH", description: `[${host}] $ ${command}` }, - uiTheme, - ); + const command = args?.command ?? ""; + const header = renderStatusLine({ icon: "success", title: "SSH", description: `[${host}]` }, uiTheme); + const cmdLines = formatSshCommandLines(command, uiTheme); const textContent = result.content?.find(c => c.type === "text")?.text ?? ""; const outputBlock = new CachedOutputBlock(); @@ -303,7 +319,7 @@ export const sshToolRenderer = { { header, state: "success", - sections: [{ label: uiTheme.fg("toolTitle", "Output"), lines: outputLines }], + sections: [{ lines: cmdLines }, { label: uiTheme.fg("toolTitle", "Output"), lines: outputLines }], width, }, uiTheme, diff --git a/packages/coding-agent/src/tui/status-line.ts b/packages/coding-agent/src/tui/status-line.ts index d40d370f6..043b78e41 100644 --- a/packages/coding-agent/src/tui/status-line.ts +++ b/packages/coding-agent/src/tui/status-line.ts @@ -15,22 +15,33 @@ export interface StatusLineOptions { meta?: string[]; } +/** + * Flatten CR/LF runs in caller-supplied header fragments so a single newline + * embedded in `description` or `meta` cannot expand the status line into + * multiple rows — which would otherwise break the bordered output block the + * header sits on. Tab characters are left alone; tool renderers that need + * tab-safe text run `replaceTabs()` themselves. + */ +function flattenForHeader(text: string): string { + return text.replace(/\r\n?|\n/g, " "); +} + export function renderStatusLine(options: StatusLineOptions, theme: Theme): string { const icon = options.icon ? formatStatusIcon(options.icon, theme, options.spinnerFrame) : ""; const titleColor = options.titleColor ?? "accent"; - const title = theme.fg(titleColor, options.title); + const title = theme.fg(titleColor, flattenForHeader(options.title)); let line = icon ? `${icon} ${title}` : title; if (options.description) { - line += `: ${theme.fg("muted", options.description)}`; + line += `: ${theme.fg("muted", flattenForHeader(options.description))}`; } if (options.badge) { const { label, color } = options.badge; - line += ` ${theme.fg(color, `${theme.format.bracketLeft}${label}${theme.format.bracketRight}`)}`; + line += ` ${theme.fg(color, `${theme.format.bracketLeft}${flattenForHeader(label)}${theme.format.bracketRight}`)}`; } - const meta = options.meta?.filter(value => value.trim().length > 0) ?? []; + const meta = options.meta?.map(flattenForHeader).filter(value => value.trim().length > 0) ?? []; if (meta.length > 0) { line += ` ${theme.fg("dim", meta.join(theme.sep.dot))}`; } diff --git a/packages/coding-agent/test/tools/ssh-render.test.ts b/packages/coding-agent/test/tools/ssh-render.test.ts new file mode 100644 index 000000000..f94e59217 --- /dev/null +++ b/packages/coding-agent/test/tools/ssh-render.test.ts @@ -0,0 +1,58 @@ +import { beforeAll, describe, expect, it } from "bun:test"; +import { getThemeByName, initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import { sshToolRenderer } from "@oh-my-pi/pi-coding-agent/tools/ssh"; +import { sanitizeText } from "@oh-my-pi/pi-utils"; + +beforeAll(async () => { + await initTheme(); +}); + +describe("sshToolRenderer", () => { + it("keeps the status header on one line when the command spans multiple lines", async () => { + const uiTheme = (await getThemeByName("dark"))!; + expect(uiTheme).toBeDefined(); + const command = ["set -e", "cat > /etc/apt/sources.list <<'EOF'", "# mirrors", "EOF"].join("\n"); + const component = sshToolRenderer.renderResult( + { content: [{ type: "text", text: "" }] }, + { expanded: false, isPartial: false }, + uiTheme, + { host: "router", command }, + ); + const rendered = component.render(120); + const sanitized = rendered.map(line => sanitizeText(line)); + // First visible row is the status header, and it MUST remain a single line. + // It carries the host but NOT the command — the command lives in the body. + const header = sanitized[0]!; + expect(header).toContain("SSH"); + expect(header).toContain("[router]"); + expect(header).not.toContain("set -e"); + expect(header).not.toContain("EOF"); + // Every command line still appears, inside the framed body. + const body = sanitized.slice(1).join("\n"); + expect(body).toContain("$ set -e"); + expect(body).toContain("cat > /etc/apt/sources.list <<'EOF'"); + expect(body).toContain("# mirrors"); + expect(body).toContain("EOF"); + }); + + it("keeps the pending-call header on one line for multiline commands", async () => { + const uiTheme = (await getThemeByName("dark"))!; + expect(uiTheme).toBeDefined(); + const command = "set -e\ndo-something"; + const component = sshToolRenderer.renderCall( + { host: "router", command }, + { expanded: false, isPartial: true }, + uiTheme, + ); + const rendered = component.render(120); + const sanitized = rendered.map(line => sanitizeText(line)); + const header = sanitized[0]!; + expect(header).toContain("SSH"); + expect(header).toContain("[router]"); + expect(header).not.toContain("set -e"); + expect(header).not.toContain("do-something"); + const body = sanitized.slice(1).join("\n"); + expect(body).toContain("$ set -e"); + expect(body).toContain("do-something"); + }); +}); diff --git a/packages/coding-agent/test/tui/status-line-newline-guard.test.ts b/packages/coding-agent/test/tui/status-line-newline-guard.test.ts new file mode 100644 index 000000000..aa4dfd24e --- /dev/null +++ b/packages/coding-agent/test/tui/status-line-newline-guard.test.ts @@ -0,0 +1,47 @@ +import { beforeAll, describe, expect, it } from "bun:test"; +import { getThemeByName, initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import { renderStatusLine } from "@oh-my-pi/pi-coding-agent/tui"; +import { sanitizeText } from "@oh-my-pi/pi-utils"; + +beforeAll(async () => { + await initTheme(); +}); + +describe("renderStatusLine", () => { + it("flattens newlines in description so a tool cannot break the header", async () => { + const uiTheme = (await getThemeByName("dark"))!; + expect(uiTheme).toBeDefined(); + const rendered = sanitizeText( + renderStatusLine( + { icon: "success", title: "SSH", description: "[router] $ set -e\ncat > /etc/apt/sources.list" }, + uiTheme, + ), + ); + expect(rendered).not.toContain("\n"); + expect(rendered).toContain("[router]"); + expect(rendered).toContain("set -e"); + expect(rendered).toContain("cat > /etc/apt/sources.list"); + }); + + it("flattens newlines in meta entries", async () => { + const uiTheme = (await getThemeByName("dark"))!; + expect(uiTheme).toBeDefined(); + const rendered = sanitizeText( + renderStatusLine({ icon: "success", title: "X", meta: ["first\nsecond", "third"] }, uiTheme), + ); + expect(rendered).not.toContain("\n"); + expect(rendered).toContain("first second"); + expect(rendered).toContain("third"); + }); + + it("flattens CRLF the same way as LF", async () => { + const uiTheme = (await getThemeByName("dark"))!; + expect(uiTheme).toBeDefined(); + const rendered = sanitizeText( + renderStatusLine({ icon: "success", title: "X", description: "a\r\nb\rc" }, uiTheme), + ); + expect(rendered).not.toContain("\r"); + expect(rendered).not.toContain("\n"); + expect(rendered).toContain("a b c"); + }); +});