From 23c66791bce704e28d73311fbe3d4caf984aff3a Mon Sep 17 00:00:00 2001 From: can1357 Date: Sat, 18 Apr 2026 23:11:47 +0200 Subject: [PATCH] fix: stabilize merged review follow-ups --- packages/coding-agent/CHANGELOG.md | 2 + packages/coding-agent/src/edit/diff.ts | 2 +- .../coding-agent/src/edit/modes/hashline.ts | 6 +- .../src/modes/components/read-tool-group.ts | 2 +- .../src/modes/interactive-mode.ts | 2 +- packages/coding-agent/src/modes/shared.ts | 6 +- packages/coding-agent/src/tui/code-cell.ts | 19 +++--- packages/coding-agent/test/edit-diff.test.ts | 33 +++++++++++ .../coding-agent/test/read-tool-group.test.ts | 58 +++++++++++++++++++ .../test/status-text-sanitization.test.ts | 18 ++++++ 10 files changed, 133 insertions(+), 15 deletions(-) create mode 100644 packages/coding-agent/test/read-tool-group.test.ts create mode 100644 packages/coding-agent/test/status-text-sanitization.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 4f113bc26..c868f2e42 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -6,6 +6,8 @@ - Fixed `local://` URL path leak on Linux where `//` collapsing to `/` produced `local:/path` forms that bypassed the internal protocol handler and leaked as filesystem paths, breaking plan mode file resolution - Fixed Tavily web search silently returning off-topic news articles when `--recency` was set. The provider was unconditionally coupling `topic: "news"` to recency, which scoped Tavily's index to news publications and excluded documentation, release notes, GitHub, and all non-news technical content. Technical queries with `--recency` now return the correct corpus. +- Fixed status-line sanitization to strip OSC, DCS, PM, APC, and 8-bit CSI escape sequences instead of leaving payload fragments in the UI +- Fixed read tool previews to render content by default without paying full-file highlight cost in collapsed mode, while preserving warning styling for corrected paths ### Changed diff --git a/packages/coding-agent/src/edit/diff.ts b/packages/coding-agent/src/edit/diff.ts index e82316f4e..51fd661f2 100644 --- a/packages/coding-agent/src/edit/diff.ts +++ b/packages/coding-agent/src/edit/diff.ts @@ -761,9 +761,9 @@ export async function computeEditDiff( if (oldText.length === 0) { return { error: "oldText must not be empty." }; } - const absolutePath = resolveToCwd(path, cwd); try { + const absolutePath = resolveToCwd(path, cwd); let rawContent: string; try { rawContent = await readFileTextForDiff(path, absolutePath); diff --git a/packages/coding-agent/src/edit/modes/hashline.ts b/packages/coding-agent/src/edit/modes/hashline.ts index f0e435918..54fd36c99 100644 --- a/packages/coding-agent/src/edit/modes/hashline.ts +++ b/packages/coding-agent/src/edit/modes/hashline.ts @@ -1157,11 +1157,11 @@ export async function computeHashlineDiff( } > { const { path, edits, move } = input; - const absolutePath = resolveToCwd(path, cwd); - const movePath = move ? resolveToCwd(move, cwd) : undefined; - const isMoveOnly = Boolean(movePath) && movePath !== absolutePath && edits.length === 0; try { + const absolutePath = resolveToCwd(path, cwd); + const movePath = move ? resolveToCwd(move, cwd) : undefined; + const isMoveOnly = Boolean(movePath) && movePath !== absolutePath && edits.length === 0; const resolvedEdits = resolveHashlineEditsForDiff(edits); const file = Bun.file(absolutePath); diff --git a/packages/coding-agent/src/modes/components/read-tool-group.ts b/packages/coding-agent/src/modes/components/read-tool-group.ts index abb0e1e84..06b09ee37 100644 --- a/packages/coding-agent/src/modes/components/read-tool-group.ts +++ b/packages/coding-agent/src/modes/components/read-tool-group.ts @@ -176,7 +176,7 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa code: entry.contentText ?? "", language: lang, title, - status: entry.status === "error" ? "error" : entry.status === "pending" ? "pending" : "complete", + status: entry.status === "success" ? "complete" : entry.status, expanded, codeMaxLines: expanded ? undefined : COLLAPSED_PREVIEW_LINES, width, diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 7c1056268..f48189040 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -28,7 +28,6 @@ import type { import type { CompactOptions } from "../extensibility/extensions/types"; import { BUILTIN_SLASH_COMMANDS, loadSlashCommands } from "../extensibility/slash-commands"; import { resolveLocalUrlToPath } from "../internal-urls"; -import { normalizeLocalScheme } from "../tools/path-utils"; import { LSP_STARTUP_EVENT_CHANNEL, type LspStartupEvent } from "../lsp/startup-events"; import { renameApprovedPlanFile } from "../plan-mode/approved-plan"; import planModeApprovedPrompt from "../prompts/system/plan-mode-approved.md" with { type: "text" }; @@ -38,6 +37,7 @@ import type { SessionContext, SessionManager } from "../session/session-manager" import { getRecentSessions } from "../session/session-manager"; import { STTController, type SttState } from "../stt"; import type { ExitPlanModeDetails, LspStartupServerInfo } from "../tools"; +import { normalizeLocalScheme } from "../tools/path-utils"; import type { EventBus } from "../utils/event-bus"; import { getEditorCommand, openInEditor } from "../utils/external-editor"; import { getSessionAccentAnsi, getSessionAccentHexForTitle } from "../utils/session-color"; diff --git a/packages/coding-agent/src/modes/shared.ts b/packages/coding-agent/src/modes/shared.ts index b7c5aa847..74df220b0 100644 --- a/packages/coding-agent/src/modes/shared.ts +++ b/packages/coding-agent/src/modes/shared.ts @@ -5,14 +5,16 @@ import { theme } from "./theme/theme"; // Text Sanitization // ═══════════════════════════════════════════════════════════════════════════ -const ANSI_OSC_RE = /\x1b\][^\x07\x1b]*(?:\x07|\x1b\\)/g; -const ANSI_CSI_RE = /\x1b\[[0-?]*[ -/]*[@-~]/g; +const ANSI_OSC_RE = /\x1b\][^\x07\x1b]*(?:\x07|\x1b\\)|\x9d[^\x07\x9c]*(?:\x07|\x9c)/g; +const ANSI_STRING_RE = /\x1b(?:P|_|\^)[\s\S]*?\x1b\\|[\x90\x9e\x9f][\s\S]*?\x9c/g; +const ANSI_CSI_RE = /\x1b\[[0-?]*[ -/]*[@-~]|\x9b[0-?]*[ -/]*[@-~]/g; const ANSI_SINGLE_RE = /\x1b[@-Z\\-_]/g; /** Sanitize text for display in a single-line status. Strips ANSI escape sequences, C0/C1 control characters, collapses whitespace, trims. */ export function sanitizeStatusText(text: string): string { return text .replace(ANSI_OSC_RE, "") + .replace(ANSI_STRING_RE, "") .replace(ANSI_CSI_RE, "") .replace(ANSI_SINGLE_RE, "") .replace(/[\u0000-\u001f\u007f-\u009f]/g, " ") diff --git a/packages/coding-agent/src/tui/code-cell.ts b/packages/coding-agent/src/tui/code-cell.ts index 26288b877..999566603 100644 --- a/packages/coding-agent/src/tui/code-cell.ts +++ b/packages/coding-agent/src/tui/code-cell.ts @@ -18,7 +18,7 @@ export interface CodeCellOptions { index?: number; total?: number; title?: string; - status?: "pending" | "running" | "complete" | "error"; + status?: "pending" | "running" | "warning" | "complete" | "error"; spinnerFrame?: number; duration?: number; output?: string; @@ -32,6 +32,7 @@ function getState(status?: CodeCellOptions["status"]): State | undefined { if (!status) return undefined; if (status === "complete") return "success"; if (status === "error") return "error"; + if (status === "warning") return "warning"; if (status === "running") return "running"; return "pending"; } @@ -45,9 +46,11 @@ function formatHeader(options: CodeCellOptions, theme: Theme): { title: string; ? "success" : status === "error" ? "error" - : status === "running" - ? "running" - : "pending", + : status === "warning" + ? "warning" + : status === "running" + ? "running" + : "pending", theme, spinnerFrame, ); @@ -78,10 +81,12 @@ export function renderCodeCell(options: CodeCellOptions, theme: Theme): string[] const { title, meta } = formatHeader(options, theme); const state = getState(options.status); - const rawCodeLines = highlightCode(replaceTabs(code ?? ""), language); + const normalizedCode = replaceTabs(code ?? ""); + const rawCodeLines = normalizedCode.split("\n"); const maxCodeLines = expanded ? rawCodeLines.length : Math.min(rawCodeLines.length, codeMaxLines); - const codeLines = rawCodeLines.slice(0, maxCodeLines); - const hiddenCodeLines = rawCodeLines.length - codeLines.length; + const visibleCode = rawCodeLines.slice(0, maxCodeLines).join("\n"); + const codeLines = highlightCode(visibleCode, language); + const hiddenCodeLines = rawCodeLines.length - maxCodeLines; if (hiddenCodeLines > 0) { const hint = formatExpandHint(theme, expanded, hiddenCodeLines > 0); const moreLine = `${formatMoreItems(hiddenCodeLines, "line")}${hint ? ` ${hint}` : ""}`; diff --git a/packages/coding-agent/test/edit-diff.test.ts b/packages/coding-agent/test/edit-diff.test.ts index 77654d333..060a06fc8 100644 --- a/packages/coding-agent/test/edit-diff.test.ts +++ b/packages/coding-agent/test/edit-diff.test.ts @@ -4,6 +4,7 @@ import * as os from "node:os"; import * as path from "node:path"; import { adjustIndentation, + computeEditDiff, computeHashlineDiff, DEFAULT_FUZZY_THRESHOLD, findMatch, @@ -269,4 +270,36 @@ describe("computeHashlineDiff", () => { expect(result.firstChangedLine).toBeUndefined(); } }); + + test("returns a handled error when the source path is a local URL", async () => { + const result = await computeHashlineDiff({ path: "local://PLAN.md", edits: [] }, tempDir); + + expect("error" in result).toBe(true); + if ("error" in result) { + expect(result.error).toContain('internal scheme "local://"'); + } + }); +}); + +describe("computeEditDiff", () => { + let tempDir = ""; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "edit-diff-edit-")); + }); + + afterEach(async () => { + if (tempDir) { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }); + + test("returns a handled error when the source path is a local URL", async () => { + const result = await computeEditDiff("local:/PLAN.md", "old", "new", tempDir); + + expect("error" in result).toBe(true); + if ("error" in result) { + expect(result.error).toContain('internal scheme "local://"'); + } + }); }); diff --git a/packages/coding-agent/test/read-tool-group.test.ts b/packages/coding-agent/test/read-tool-group.test.ts new file mode 100644 index 000000000..bebf52fd2 --- /dev/null +++ b/packages/coding-agent/test/read-tool-group.test.ts @@ -0,0 +1,58 @@ +import { afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; +import { ReadToolGroupComponent } from "../src/modes/components/read-tool-group"; +import * as themeModule from "../src/modes/theme/theme"; + +describe("ReadToolGroupComponent", () => { + beforeAll(async () => { + await themeModule.initTheme(false, undefined, undefined, "dark", "light"); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("renders warning previews with warning styling instead of success styling", () => { + const component = new ReadToolGroupComponent(); + component.updateArgs({ path: "/tmp/example.ts" }, "read-1"); + component.updateResult( + { + content: [{ type: "text", text: "const a = 1;\nconst b = 2;\nconst c = 3;" }], + details: { suffixResolution: { from: "/tmp/exampl.ts", to: "/tmp/example.ts" } }, + }, + false, + "read-1", + ); + + const rendered = Bun.stripANSI(component.render(120).join("\n")); + + expect(rendered).toContain(themeModule.theme.status.warning); + expect(rendered).not.toContain(themeModule.theme.status.success); + expect(rendered).toContain("corrected from"); + }); + + it("highlights only the collapsed preview lines", () => { + const highlightSpy = vi.spyOn(themeModule, "highlightCode"); + const component = new ReadToolGroupComponent(); + component.updateArgs({ path: "/tmp/example.ts" }, "read-2"); + component.updateResult( + { + content: [ + { + type: "text", + text: "line 1\nline 2\nline 3\nline 4\nline 5", + }, + ], + }, + false, + "read-2", + ); + + const rendered = Bun.stripANSI(component.render(120).join("\n")); + const highlightedInput = highlightSpy.mock.calls[0]?.[0]; + + expect(highlightedInput).toBe("line 1\nline 2\nline 3"); + expect(rendered).toContain("line 1"); + expect(rendered).not.toContain("line 4"); + expect(rendered.toLowerCase()).toContain("ctrl+o"); + }); +}); diff --git a/packages/coding-agent/test/status-text-sanitization.test.ts b/packages/coding-agent/test/status-text-sanitization.test.ts new file mode 100644 index 000000000..aa579705a --- /dev/null +++ b/packages/coding-agent/test/status-text-sanitization.test.ts @@ -0,0 +1,18 @@ +import { describe, expect, it } from "bun:test"; +import { sanitizeStatusText } from "../src/modes/shared"; + +describe("sanitizeStatusText", () => { + it("strips OSC, DCS, PM, APC, and 8-bit CSI escape sequences", () => { + const input = + "prefix " + + "\x1b]8;;https://example.com\x07link\x1b]8;;\x07" + + " " + + "\x1bPhidden-dcs\x1b\\" + + "\x1b^hidden-pm\x1b\\" + + "\x1b_hidden-apc\x1b\\" + + "\x9b31mred\x9b0m" + + " suffix"; + + expect(sanitizeStatusText(input)).toBe("prefix link red suffix"); + }); +});