Merge remote-tracking branch 'origin/farm/e2beffcf/ssh-render-multiline-command-block'
This commit is contained in:
@@ -18,6 +18,10 @@
|
||||
|
||||
- Fixed scoped mnemopi recall in `MnemopiSessionState.collectScopedRecallResults`/`recallResultsScoped` to await the async `Mnemopi.recallEnhanced` so the new auto-derived `queryEmbedding` flows through. Without this, the embedding-enabled mnemopi backend silently kept running FTS-only on every recall. ([#1832](https://github.com/can1357/oh-my-pi/issues/1832))
|
||||
|
||||
### 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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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))}`;
|
||||
}
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user