fix(coding-agent): persist bash output on line truncation closes #2852
This commit is contained in:
@@ -4,6 +4,7 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed bash output truncation by line count to always persist full output to a temp file, preventing data loss when output exceeds 2000 lines but stays under the byte threshold ([#2852](https://github.com/badlogic/pi-mono/issues/2852))
|
||||
- RpcClient now forwards subprocess stderr to parent process in real-time ([#2805](https://github.com/badlogic/pi-mono/issues/2805))
|
||||
- Theme file watcher now handles async `fs.watch` error events instead of crashing the process ([#2791](https://github.com/badlogic/pi-mono/issues/2791))
|
||||
- Fixed CLI extension paths like `git:gist.github.com/...` being incorrectly resolved against cwd instead of being passed through to the package manager ([#2845](https://github.com/badlogic/pi-mono/pull/2845) by [@aliou](https://github.com/aliou))
|
||||
|
||||
@@ -78,6 +78,18 @@ export async function executeBashWithOperations(
|
||||
let tempFileStream: WriteStream | undefined;
|
||||
let totalBytes = 0;
|
||||
|
||||
const ensureTempFile = () => {
|
||||
if (tempFilePath) {
|
||||
return;
|
||||
}
|
||||
const id = randomBytes(8).toString("hex");
|
||||
tempFilePath = join(tmpdir(), `pi-bash-${id}.log`);
|
||||
tempFileStream = createWriteStream(tempFilePath);
|
||||
for (const chunk of outputChunks) {
|
||||
tempFileStream.write(chunk);
|
||||
}
|
||||
};
|
||||
|
||||
const decoder = new TextDecoder();
|
||||
|
||||
const onData = (data: Buffer) => {
|
||||
@@ -87,13 +99,8 @@ export async function executeBashWithOperations(
|
||||
const text = sanitizeBinaryOutput(stripAnsi(decoder.decode(data, { stream: true }))).replace(/\r/g, "");
|
||||
|
||||
// Start writing to temp file if exceeds threshold
|
||||
if (totalBytes > DEFAULT_MAX_BYTES && !tempFilePath) {
|
||||
const id = randomBytes(8).toString("hex");
|
||||
tempFilePath = join(tmpdir(), `pi-bash-${id}.log`);
|
||||
tempFileStream = createWriteStream(tempFilePath);
|
||||
for (const chunk of outputChunks) {
|
||||
tempFileStream.write(chunk);
|
||||
}
|
||||
if (totalBytes > DEFAULT_MAX_BYTES) {
|
||||
ensureTempFile();
|
||||
}
|
||||
|
||||
if (tempFileStream) {
|
||||
@@ -126,6 +133,9 @@ export async function executeBashWithOperations(
|
||||
|
||||
const fullOutput = outputChunks.join("");
|
||||
const truncationResult = truncateTail(fullOutput);
|
||||
if (truncationResult.truncated) {
|
||||
ensureTempFile();
|
||||
}
|
||||
const cancelled = options?.signal?.aborted ?? false;
|
||||
|
||||
return {
|
||||
@@ -144,6 +154,9 @@ export async function executeBashWithOperations(
|
||||
if (options?.signal?.aborted) {
|
||||
const fullOutput = outputChunks.join("");
|
||||
const truncationResult = truncateTail(fullOutput);
|
||||
if (truncationResult.truncated) {
|
||||
ensureTempFile();
|
||||
}
|
||||
return {
|
||||
output: truncationResult.truncated ? truncationResult.content : fullOutput,
|
||||
exitCode: undefined,
|
||||
|
||||
@@ -292,14 +292,18 @@ export function createBashToolDefinition(
|
||||
let chunksBytes = 0;
|
||||
const maxChunksBytes = DEFAULT_MAX_BYTES * 2;
|
||||
|
||||
const ensureTempFile = () => {
|
||||
if (tempFilePath) return;
|
||||
tempFilePath = getTempFilePath();
|
||||
tempFileStream = createWriteStream(tempFilePath);
|
||||
for (const chunk of chunks) tempFileStream.write(chunk);
|
||||
};
|
||||
|
||||
const handleData = (data: Buffer) => {
|
||||
totalBytes += data.length;
|
||||
// Start writing to a temp file once output exceeds the in-memory threshold.
|
||||
if (totalBytes > DEFAULT_MAX_BYTES && !tempFilePath) {
|
||||
tempFilePath = getTempFilePath();
|
||||
tempFileStream = createWriteStream(tempFilePath);
|
||||
// Write all buffered chunks to the file.
|
||||
for (const chunk of chunks) tempFileStream.write(chunk);
|
||||
if (totalBytes > DEFAULT_MAX_BYTES) {
|
||||
ensureTempFile();
|
||||
}
|
||||
// Write to temp file if we have one.
|
||||
if (tempFileStream) tempFileStream.write(data);
|
||||
@@ -316,6 +320,9 @@ export function createBashToolDefinition(
|
||||
const fullBuffer = Buffer.concat(chunks);
|
||||
const fullText = fullBuffer.toString("utf-8");
|
||||
const truncation = truncateTail(fullText);
|
||||
if (truncation.truncated) {
|
||||
ensureTempFile();
|
||||
}
|
||||
onUpdate({
|
||||
content: [{ type: "text", text: truncation.content || "" }],
|
||||
details: {
|
||||
@@ -333,13 +340,16 @@ export function createBashToolDefinition(
|
||||
env: spawnContext.env,
|
||||
})
|
||||
.then(({ exitCode }) => {
|
||||
// Close temp file stream before building the final result.
|
||||
if (tempFileStream) tempFileStream.end();
|
||||
// Combine the rolling buffer chunks.
|
||||
const fullBuffer = Buffer.concat(chunks);
|
||||
const fullOutput = fullBuffer.toString("utf-8");
|
||||
// Apply tail truncation for the final display payload.
|
||||
const truncation = truncateTail(fullOutput);
|
||||
if (truncation.truncated) {
|
||||
ensureTempFile();
|
||||
}
|
||||
// Close temp file stream before building the final result.
|
||||
if (tempFileStream) tempFileStream.end();
|
||||
let outputText = truncation.content || "(no output)";
|
||||
let details: BashToolDetails | undefined;
|
||||
if (truncation.truncated) {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { mkdirSync, readFileSync, rmSync, writeFileSync } from "fs";
|
||||
import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "fs";
|
||||
import { tmpdir } from "os";
|
||||
import { join } from "path";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
@@ -443,6 +443,47 @@ describe("Coding Agent Tools", () => {
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.output).toBe("red\n");
|
||||
});
|
||||
|
||||
it("should persist full output when truncation happens by line count only", async () => {
|
||||
const bash = createBashTool(testDir);
|
||||
const result = await bash.execute("test-call-line-truncation", { command: "seq 3000" });
|
||||
const output = getTextOutput(result);
|
||||
const fullOutputPath = result.details?.fullOutputPath;
|
||||
|
||||
expect(result.details?.truncation?.truncated).toBe(true);
|
||||
expect(result.details?.truncation?.truncatedBy).toBe("lines");
|
||||
expect(fullOutputPath).toBeDefined();
|
||||
expect(output).toMatch(/\[Showing lines \d+-\d+ of \d+\. Full output: /);
|
||||
expect(output).not.toContain("Full output: undefined");
|
||||
|
||||
for (let i = 0; i < 20 && (!fullOutputPath || !existsSync(fullOutputPath)); i++) {
|
||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
}
|
||||
|
||||
expect(fullOutputPath).toBeDefined();
|
||||
expect(existsSync(fullOutputPath!)).toBe(true);
|
||||
const fullOutput = readFileSync(fullOutputPath!, "utf-8");
|
||||
expect(fullOutput).toContain("1\n2\n3");
|
||||
expect(fullOutput).toContain("2998\n2999\n3000");
|
||||
});
|
||||
|
||||
it("executeBash should persist full output when truncation happens by line count only", async () => {
|
||||
const result = await executeBash("seq 3000");
|
||||
const fullOutputPath = result.fullOutputPath;
|
||||
|
||||
expect(result.truncated).toBe(true);
|
||||
expect(fullOutputPath).toBeDefined();
|
||||
|
||||
for (let i = 0; i < 20 && (!fullOutputPath || !existsSync(fullOutputPath)); i++) {
|
||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
}
|
||||
|
||||
expect(fullOutputPath).toBeDefined();
|
||||
expect(existsSync(fullOutputPath!)).toBe(true);
|
||||
const fullOutput = readFileSync(fullOutputPath!, "utf-8");
|
||||
expect(fullOutput).toContain("1\n2\n3");
|
||||
expect(fullOutput).toContain("2998\n2999\n3000");
|
||||
});
|
||||
});
|
||||
|
||||
describe("grep tool", () => {
|
||||
|
||||
Reference in New Issue
Block a user