mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
`execCommand()` (the function behind `pi.exec()`) decoded each stdout
and stderr chunk on its own with `data.toString()`. When a multi-byte
UTF-8 character was split across two pipe chunks, each half was turned
into U+FFFD, so the result was corrupted. For example, `"你好"` came back
as `"���好"` while the command still exited with code 0.
This change gives stdout and stderr each their own
`StringDecoder("utf8")`. A decoder holds back incomplete bytes until the
rest of the character arrives. Both decoders are flushed with `end()`
before the promise resolves, on both the success path and the error
path. `waitForChildProcess()` can resolve without a stream `end` event,
so the decoders are flushed there and not in an `end` handler. The
return shape, exit codes, and kill behavior are unchanged.
**Testing**
- Added a regression test to `packages/coding-agent/test/exec.test.ts`.
The child process splits `"你好"` across two writes on both stdout and
stderr. The test fails without the fix and passes with it: `node
../../node_modules/vitest/dist/cli.js --run test/exec.test.ts` reports 3
passed.
- `npm run check` passes.
- Reproduction from the issue: a child that writes the first byte of
`"你好"` and then the rest 100 ms later. Before the fix the result was
`{"stdout":"���好",...}`. After the fix it is
`{"stdout":"你好","stderr":"","code":0,"killed":false}`.
Fixes #11312
Requested by @davidbrai
---------
Co-authored-by: triagent <triagent@localhost>
Co-authored-by: David Brailovsky <davidbrai@gmail.com>
116 lines
2.9 KiB
TypeScript
116 lines
2.9 KiB
TypeScript
/**
|
|
* Shared command execution utilities for extensions and custom tools.
|
|
*/
|
|
|
|
import { spawn } from "node:child_process";
|
|
import { StringDecoder } from "node:string_decoder";
|
|
import { waitForChildProcess } from "../utils/child-process.ts";
|
|
|
|
/**
|
|
* Options for executing shell commands.
|
|
*/
|
|
export interface ExecOptions {
|
|
/** AbortSignal to cancel the command */
|
|
signal?: AbortSignal;
|
|
/** Timeout in milliseconds */
|
|
timeout?: number;
|
|
/** Working directory */
|
|
cwd?: string;
|
|
}
|
|
|
|
/**
|
|
* Result of executing a shell command.
|
|
*/
|
|
export interface ExecResult {
|
|
stdout: string;
|
|
stderr: string;
|
|
code: number;
|
|
killed: boolean;
|
|
}
|
|
|
|
/**
|
|
* Execute a shell command and return stdout/stderr/code.
|
|
* Supports timeout and abort signal.
|
|
*/
|
|
export async function execCommand(
|
|
command: string,
|
|
args: string[],
|
|
cwd: string,
|
|
options?: ExecOptions,
|
|
): Promise<ExecResult> {
|
|
return new Promise((resolve) => {
|
|
const proc = spawn(command, args, {
|
|
cwd,
|
|
shell: false,
|
|
stdio: ["ignore", "pipe", "pipe"],
|
|
});
|
|
|
|
let stdout = "";
|
|
let stderr = "";
|
|
// Decode across chunks: a multi-byte UTF-8 character can be split between two chunks,
|
|
// and decoding each chunk separately would corrupt it (earendil-works/pi#11312).
|
|
const stdoutDecoder = new StringDecoder("utf8");
|
|
const stderrDecoder = new StringDecoder("utf8");
|
|
let killed = false;
|
|
let timeoutId: NodeJS.Timeout | undefined;
|
|
|
|
const killProcess = () => {
|
|
if (!killed) {
|
|
killed = true;
|
|
proc.kill("SIGTERM");
|
|
// Force kill after 5 seconds if SIGTERM doesn't work
|
|
setTimeout(() => {
|
|
if (!proc.killed) {
|
|
proc.kill("SIGKILL");
|
|
}
|
|
}, 5000);
|
|
}
|
|
};
|
|
|
|
// Handle abort signal
|
|
if (options?.signal) {
|
|
if (options.signal.aborted) {
|
|
killProcess();
|
|
} else {
|
|
options.signal.addEventListener("abort", killProcess, { once: true });
|
|
}
|
|
}
|
|
|
|
// Handle timeout
|
|
if (options?.timeout && options.timeout > 0) {
|
|
timeoutId = setTimeout(() => {
|
|
killProcess();
|
|
}, options.timeout);
|
|
}
|
|
|
|
proc.stdout?.on("data", (data) => {
|
|
stdout += stdoutDecoder.write(data);
|
|
});
|
|
|
|
proc.stderr?.on("data", (data) => {
|
|
stderr += stderrDecoder.write(data);
|
|
});
|
|
|
|
// Wait for process termination without hanging on inherited stdio handles
|
|
// held open by detached descendants.
|
|
waitForChildProcess(proc)
|
|
.then((code) => {
|
|
if (timeoutId) clearTimeout(timeoutId);
|
|
stdout += stdoutDecoder.end();
|
|
stderr += stderrDecoder.end();
|
|
if (options?.signal) {
|
|
options.signal.removeEventListener("abort", killProcess);
|
|
}
|
|
resolve({ stdout, stderr, code: code ?? 0, killed });
|
|
})
|
|
.catch((error: Error) => {
|
|
if (timeoutId) clearTimeout(timeoutId);
|
|
stdout += stdoutDecoder.end();
|
|
stderr += stderrDecoder.end();
|
|
if (options?.signal) {
|
|
options.signal.removeEventListener("abort", killProcess);
|
|
}
|
|
resolve({ stdout, stderr: stderr || error.message, code: 1, killed });
|
|
});
|
|
});
|
|
}
|