diff --git a/.changeset/decode-sg-stderr.md b/.changeset/decode-sg-stderr.md new file mode 100644 index 00000000..e881a5d4 --- /dev/null +++ b/.changeset/decode-sg-stderr.md @@ -0,0 +1,17 @@ +--- +"@taskless/cli": patch +--- + +Stop corrupting non-ASCII characters in ast-grep's error output. + +`runAstGrepScan` and the runtime narrow both decoded ast-grep's stderr one +chunk at a time with `chunk.toString()`. A multi-byte UTF-8 sequence split +across a chunk boundary was decoded as two invalid sequences, and both halves +became replacement characters before the pieces were joined — the original +bytes unrecoverable by then. Each stream now uses a single `StringDecoder`, +flushed on close, matching what the Vale runner and `verify` already do. + +The corrupted text only ever reached an error message, so no scan result was +ever wrong. But that message is the one a user reads when ast-grep rejects a +rule file, naming a rule id or a path — which is exactly where a non-ASCII +character turns up. diff --git a/packages/cli/src/rules/runtime/narrow.ts b/packages/cli/src/rules/runtime/narrow.ts index 7f340777..0171cc3b 100644 --- a/packages/cli/src/rules/runtime/narrow.ts +++ b/packages/cli/src/rules/runtime/narrow.ts @@ -3,6 +3,7 @@ import { copyFile, mkdtemp, mkdir, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { createInterface } from "node:readline"; import { join } from "node:path"; +import { StringDecoder } from "node:string_decoder"; import { stringify } from "yaml"; @@ -45,25 +46,31 @@ function runSg( stdio: ["ignore", "pipe", "pipe"], env: { ...process.env, PATH: buildPath() }, }); + // One decoder for the stream — see the note in `runAstGrepScan`. stdout is + // read through `node:readline`, which already decodes across boundaries. + const stderrDecoder = new StringDecoder("utf8"); const stderrChunks: string[] = []; const rl = createInterface({ input: child.stdout }); rl.on("line", onLine); child.stderr.on("data", (chunk: Buffer) => - stderrChunks.push(chunk.toString()) + stderrChunks.push(stderrDecoder.write(chunk)) ); child.on("error", reject); child.on("close", (code, signal) => { + stderrChunks.push(stderrDecoder.end()); // ast-grep exits 1 when matches are found — expected. A `null` code means // the process was killed by a signal (e.g. OOM); treat that and any exit // >1 as a real failure rather than silently dropping matches. if (code === null || code > 1) { const cause = code === null ? `signal ${String(signal)}` : `exit ${String(code)}`; + // Test the joined text, not the chunk count: the decoder's final flush + // pushes an empty string on a stream that ended cleanly, so counting + // chunks would append a bare `: ` to every message. + const stderr = stderrChunks.join("").trim(); reject( new Error( - `ast-grep narrow failed (${cause})${ - stderrChunks.length > 0 ? `: ${stderrChunks.join("").trim()}` : "" - }` + `ast-grep narrow failed (${cause})${stderr === "" ? "" : `: ${stderr}`}` ) ); return; diff --git a/packages/cli/src/rules/scan.ts b/packages/cli/src/rules/scan.ts index 8f0847d3..c3baaddd 100644 --- a/packages/cli/src/rules/scan.ts +++ b/packages/cli/src/rules/scan.ts @@ -1,6 +1,7 @@ import { spawn } from "node:child_process"; import { dirname, resolve } from "node:path"; import { createInterface } from "node:readline"; +import { StringDecoder } from "node:string_decoder"; import { fileURLToPath } from "node:url"; import type { AstGrepMatch } from "../types/check"; @@ -149,6 +150,18 @@ export async function runAstGrepScan( }); const results: CheckResult[] = []; + + // One decoder for the stream, not `chunk.toString()` per chunk. A + // multi-byte UTF-8 sequence split across a chunk boundary would otherwise + // have each half independently replaced with U+FFFD, and the bytes are + // unrecoverable by the time the pieces are joined. What ast-grep writes + // here is the message a user reads when it rejects a rule file — naming a + // rule id or a path, which is exactly where a non-ASCII character shows up. + // Same treatment `vale/run.ts` and `verify.ts` already give their streams. + // + // stdout needs no decoder: it is consumed through `node:readline`, which + // handles character boundaries itself. + const stderrDecoder = new StringDecoder("utf8"); const stderrChunks: string[] = []; const rl = createInterface({ input: child.stdout }); @@ -164,7 +177,7 @@ export async function runAstGrepScan( }); child.stderr.on("data", (chunk: Buffer) => { - stderrChunks.push(chunk.toString()); + stderrChunks.push(stderrDecoder.write(chunk)); }); child.on("error", (error) => { @@ -183,6 +196,11 @@ export async function runAstGrepScan( }); child.on("close", (code) => { + // Flush whatever partial multi-byte sequence the decoder is holding, so a + // stream that ends mid-character contributes its replacement char once + // rather than leaving bytes unaccounted for. + stderrChunks.push(stderrDecoder.end()); + // ast-grep exits 1 when error-severity matches found — that's expected // Only treat spawn/binary failures (exit > 1) as errors if (code !== null && code > 1) {