mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): do not split surrogate pairs when truncating text (#11757)
Several places truncate text by UTF-16 code units with `slice()`. When the cut lands between the two halves of a surrogate pair (an emoji or other character outside the Basic Multilingual Plane), the result keeps a lone high or low surrogate. That is invalid UTF-16. Example: a grep match line with 499 ASCII characters followed by `😀` is cut at 500 units, so the output ends in `\uD83D... [truncated]`. The bundled providers strip lone surrogates before sending (`sanitizeSurrogates` in pi-ai), so the usual effect is a dropped character. But the broken text still reaches the session file, RPC and JSON clients (strict UTF-8 encoders such as Rust's serde_json reject lone surrogates), extensions, and custom providers. This adds `sliceHead(text, n)` and `sliceTail(text, n)` to `core/tools/truncate.ts`. They behave like `slice(0, n)` and `slice(-n)` but return one unit fewer when the cut would split a pair, and are used where truncated text reaches the model: - `truncateLine` (grep match lines), the case reported in #7121 - codemode `truncateOutput` head and tail, and its argument and error previews - compaction `truncateForSummary` (tool results sent to the summarizer) - MCP server summaries in the system prompt and stderr tails in connection errors The cost is two `charCodeAt` checks per truncation, and nothing on the untruncated path. The surrogate check follows the fix in #7122 by @IKEASven69, which @cristinaponcela reviewed in a follow-up branch. That PR was closed before merging. Not changed: TUI-only previews (renderers, visual truncation) and line-based slicing. pi-private's pi-durable codemode `output.ts` has the same head/tail slicing and needs a separate change there. **Testing** - New `test/truncate-surrogates.test.ts`: an emoji at the cut in `truncateLine` is dropped whole, an emoji ending exactly at the limit is kept, `sliceHead`/`sliceTail` never leave a lone surrogate for any length, and they match `slice` on text without surrogates. - `test/tools.test.ts`, compaction, MCP extension, and codemode tests pass. - `npm run check` passes. fixes #7121 This PR is AI-generated.
This commit is contained in:
parent
8889e623d5
commit
9e870f6f00
7 changed files with 86 additions and 9 deletions
|
|
@ -27,6 +27,7 @@
|
|||
- Fixed RPC mode cutting off shutdown cleanup, such as a pending MCP token refresh, when stdin closed and SIGTERM arrived together, which process managers like Paseo and systemd do. Pi lost the refreshed token and the MCP server revoked the sign-in. Once a shutdown has started, a stdin EOF or a single signal no longer interrupts it; a second signal still exits at once. Print mode likewise no longer dies from a signal that arrives during its final cleanup ([#11599](https://github.com/earendil-works/pi/issues/11599))
|
||||
- Fixed the grep tool reading every matched file into memory when `context` was set, which ran out of memory on large files; context now comes from ripgrep, which also fixes UTF-16 files and lone CRs in matched lines, and the match limit notice no longer appears when exactly `limit` matches exist ([#9276](https://github.com/earendil-works/pi/issues/9276), [#11423](https://github.com/earendil-works/pi/issues/11423))
|
||||
- Fixed the find tool reporting the result limit when exactly `limit` files match, and `limit: 0` in find and ls returning every result or an empty listing; limits below 1 now mean 1 ([#7121](https://github.com/earendil-works/pi/issues/7121))
|
||||
- Fixed grep match lines, codemode output, compaction summaries of tool results, and MCP error and server summaries cutting an emoji or other non-BMP character in half when truncating, which left an invalid lone surrogate in the text ([#7121](https://github.com/earendil-works/pi/issues/7121))
|
||||
|
||||
|
||||
## [1.5.0] - 2026-11-02
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@
|
|||
|
||||
import type { AgentMessage } from "@earendil-works/pi-agent-core";
|
||||
import { contentText, type Message } from "@earendil-works/pi-ai";
|
||||
import { sliceHead } from "../tools/truncate.ts";
|
||||
|
||||
// ============================================================================
|
||||
// File Operation Tracking
|
||||
|
|
@ -100,7 +101,7 @@ const TOOL_RESULT_MAX_CHARS = 2000;
|
|||
function truncateForSummary(text: string, maxChars: number): string {
|
||||
if (text.length <= maxChars) return text;
|
||||
const truncatedChars = text.length - maxChars;
|
||||
return `${text.slice(0, maxChars)}\n\n[... ${truncatedChars} more characters truncated]`;
|
||||
return `${sliceHead(text, maxChars)}\n\n[... ${truncatedChars} more characters truncated]`;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -272,5 +272,31 @@ export function truncateLine(
|
|||
if (line.length <= maxChars) {
|
||||
return { text: line, wasTruncated: false };
|
||||
}
|
||||
return { text: `${line.slice(0, maxChars)}... [truncated]`, wasTruncated: true };
|
||||
return { text: `${sliceHead(line, maxChars)}... [truncated]`, wasTruncated: true };
|
||||
}
|
||||
|
||||
/** Returns at most the first `maxUnits` UTF-16 code units of `text`, without splitting a surrogate pair. */
|
||||
export function sliceHead(text: string, maxUnits: number): string {
|
||||
const end = Math.max(0, maxUnits);
|
||||
// A lone high surrogate before the cut would be invalid UTF-16; keep the whole character out instead.
|
||||
return isHighSurrogate(text.charCodeAt(end - 1)) && isLowSurrogate(text.charCodeAt(end))
|
||||
? text.slice(0, end - 1)
|
||||
: text.slice(0, end);
|
||||
}
|
||||
|
||||
/** Returns at most the last `maxUnits` UTF-16 code units of `text`, without splitting a surrogate pair. */
|
||||
export function sliceTail(text: string, maxUnits: number): string {
|
||||
if (maxUnits <= 0) return "";
|
||||
const start = Math.max(0, text.length - maxUnits);
|
||||
return isLowSurrogate(text.charCodeAt(start)) && isHighSurrogate(text.charCodeAt(start - 1))
|
||||
? text.slice(start + 1)
|
||||
: text.slice(start);
|
||||
}
|
||||
|
||||
function isHighSurrogate(code: number): boolean {
|
||||
return code >= 0xd800 && code <= 0xdbff;
|
||||
}
|
||||
|
||||
function isLowSurrogate(code: number): boolean {
|
||||
return code >= 0xdc00 && code <= 0xdfff;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ import {
|
|||
import { getCodemodeWorkerSpecifier, getQuickJSWasmPath } from "../../config.ts";
|
||||
import type { ExtensionToolContext, ToolNamespace } from "../../core/extensions/types.ts";
|
||||
import type { SessionEntry } from "../../core/session-manager.ts";
|
||||
import { formatSize } from "../../core/tools/truncate.ts";
|
||||
import { formatSize, sliceHead, sliceTail } from "../../core/tools/truncate.ts";
|
||||
import { combineUsage } from "../../core/usage-totals.ts";
|
||||
import { writeOutputFile } from "../../utils/output-files.ts";
|
||||
import { Bm25Ranker, createToolSearchDocument, DEFAULT_TOOL_SEARCH_LIMIT } from "../tool-search/tool.ts";
|
||||
|
|
@ -112,7 +112,7 @@ function releaseCodemodeWorker(): void {
|
|||
const MODEL_TYPES: ReadonlySet<string> = new Set<ModelType>(["chat", "image", "classifier"]);
|
||||
|
||||
function truncateText(text: string, maxChars: number): string {
|
||||
return text.length > maxChars ? `${text.slice(0, maxChars - 3)}...` : text;
|
||||
return text.length > maxChars ? `${sliceHead(text, maxChars - 3)}...` : text;
|
||||
}
|
||||
|
||||
function previewArgs(args: unknown): string {
|
||||
|
|
@ -457,8 +457,8 @@ async function truncateOutput(
|
|||
const headChars = Math.floor(budget / 2);
|
||||
const tailChars = budget - headChars;
|
||||
const removed = combined.length - headChars - tailChars;
|
||||
const head = combined.slice(0, headChars);
|
||||
const tail = tailChars > 0 ? combined.slice(-tailChars) : "";
|
||||
const head = sliceHead(combined, headChars);
|
||||
const tail = sliceTail(combined, tailChars);
|
||||
let text = `Warning: truncated output (original token count: ${Math.ceil(combined.length / CHARS_PER_TOKEN)})\nTotal output lines: ${combined.split("\n").length}\n\n${head}…${Math.ceil(removed / CHARS_PER_TOKEN)} tokens truncated…${tail}`;
|
||||
const spilled = await spillOutput(combined);
|
||||
text +=
|
||||
|
|
|
|||
|
|
@ -39,6 +39,7 @@ import type {
|
|||
ToolDefinition,
|
||||
} from "../../core/extensions/types.ts";
|
||||
import type { ModelRegistry } from "../../core/model-registry.ts";
|
||||
import { sliceHead } from "../../core/tools/truncate.ts";
|
||||
import { openBrowser } from "../../utils/open-browser.ts";
|
||||
import { CODEMODE_TOOL_NAME } from "../codemode/tool.ts";
|
||||
import { TOOL_SEARCH_TOOL_NAME } from "../tool-search/tool.ts";
|
||||
|
|
@ -172,7 +173,7 @@ function serversSectionIntro(reaches: ReadonlySet<string>): string {
|
|||
|
||||
function truncate(text: string, max: number): string {
|
||||
if (text.length <= max) return text;
|
||||
return max <= 1 ? "" : `${text.slice(0, max - 1).trimEnd()}…`;
|
||||
return max <= 1 ? "" : `${sliceHead(text, max - 1).trimEnd()}…`;
|
||||
}
|
||||
|
||||
/** What the `mcp_servers` section needs of a server. */
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@ import {
|
|||
} from "@earendil-works/pi-mcp/oauth";
|
||||
import { VERSION } from "../../config.ts";
|
||||
import { resolveConfigValueOrThrow, resolveHeadersOrThrow } from "../../core/resolve-config-value.ts";
|
||||
import { sliceTail } from "../../core/tools/truncate.ts";
|
||||
import type { McpServerEntry } from "./config.ts";
|
||||
import type { McpServerLog } from "./log.ts";
|
||||
import { createMcpAuthProvider, type McpOAuthCredentialStore } from "./oauth.ts";
|
||||
|
|
@ -399,7 +400,7 @@ export class McpServerConnection implements McpToolCaller, McpResourceServer {
|
|||
} catch (error) {
|
||||
await closeClient();
|
||||
if (transport instanceof StdioTransport) {
|
||||
this.stderrTail = transport.stderr.trim().slice(-STDERR_TAIL_CHARS) || undefined;
|
||||
this.stderrTail = sliceTail(transport.stderr.trim(), STDERR_TAIL_CHARS) || undefined;
|
||||
}
|
||||
throw error;
|
||||
} finally {
|
||||
|
|
@ -423,7 +424,7 @@ export class McpServerConnection implements McpToolCaller, McpResourceServer {
|
|||
if (this.client !== client || this.closed) return;
|
||||
this.client = undefined;
|
||||
this.state = "disconnected";
|
||||
const stderr = stdio?.stderr.trim().slice(-STDERR_TAIL_CHARS);
|
||||
const stderr = stdio ? sliceTail(stdio.stderr.trim(), STDERR_TAIL_CHARS) : undefined;
|
||||
this.error = stderr ? `Connection closed\n${stderr}` : "Connection closed";
|
||||
this.changed();
|
||||
}
|
||||
|
|
|
|||
47
packages/coding-agent/test/truncate-surrogates.test.ts
Normal file
47
packages/coding-agent/test/truncate-surrogates.test.ts
Normal file
|
|
@ -0,0 +1,47 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
import { sliceHead, sliceTail, truncateLine } from "../src/core/tools/truncate.ts";
|
||||
|
||||
const EMOJI = "\u{1F600}"; // two UTF-16 code units: \uD83D\uDE00
|
||||
|
||||
function hasLoneSurrogate(text: string): boolean {
|
||||
return /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(?<![\uD800-\uDBFF])[\uDC00-\uDFFF]/.test(text);
|
||||
}
|
||||
|
||||
// https://github.com/earendil-works/pi/issues/7121
|
||||
describe("surrogate-safe slicing", () => {
|
||||
it("truncateLine does not cut an emoji in half at the limit", () => {
|
||||
const line = `${"a".repeat(499)}${EMOJI}tail`;
|
||||
const { text, wasTruncated } = truncateLine(line, 500);
|
||||
expect(wasTruncated).toBe(true);
|
||||
expect(text).toBe(`${"a".repeat(499)}... [truncated]`);
|
||||
expect(hasLoneSurrogate(text)).toBe(false);
|
||||
});
|
||||
|
||||
it("truncateLine keeps an emoji that ends exactly at the limit", () => {
|
||||
const line = `${"a".repeat(498)}${EMOJI}tail`;
|
||||
expect(truncateLine(line, 500).text).toBe(`${"a".repeat(498)}${EMOJI}... [truncated]`);
|
||||
});
|
||||
|
||||
it("sliceHead and sliceTail never split a pair", () => {
|
||||
const text = `${EMOJI}x${EMOJI}`;
|
||||
for (let units = 0; units <= text.length + 1; units++) {
|
||||
const head = sliceHead(text, units);
|
||||
const tail = sliceTail(text, units);
|
||||
expect(hasLoneSurrogate(head)).toBe(false);
|
||||
expect(hasLoneSurrogate(tail)).toBe(false);
|
||||
expect(text.startsWith(head)).toBe(true);
|
||||
expect(text.endsWith(tail)).toBe(true);
|
||||
expect(head.length).toBeGreaterThanOrEqual(Math.min(units, text.length) - 1);
|
||||
expect(tail.length).toBeGreaterThanOrEqual(Math.min(units, text.length) - 1);
|
||||
}
|
||||
});
|
||||
|
||||
it("sliceHead and sliceTail match slice on text without surrogates", () => {
|
||||
expect(sliceHead("abcdef", 3)).toBe("abc");
|
||||
expect(sliceTail("abcdef", 3)).toBe("def");
|
||||
expect(sliceHead("abc", 10)).toBe("abc");
|
||||
expect(sliceTail("abc", 10)).toBe("abc");
|
||||
expect(sliceHead("abc", 0)).toBe("");
|
||||
expect(sliceTail("abc", 0)).toBe("");
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue