From 517865520a34abf3060daad65ce8801ad6b4a112 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Thu, 20 Aug 2026 16:07:39 -0700 Subject: [PATCH] fix(cli): decode ast-grep stderr with a StringDecoder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `runAstGrepScan` decoded stderr one chunk at a time with `chunk.toString()`. A multi-byte UTF-8 sequence split across a chunk boundary is decoded as two invalid sequences, both halves become U+FFFD, and the original bytes are unrecoverable by the time the pieces are joined. Each stream now uses a single StringDecoder, flushed with `.end()` on close — the remedy `vale/run.ts` and `verify.ts` already carry. stdout needs nothing: readline decodes across boundaries itself. `runtime/narrow.ts` carried a fourth copy of the same pattern, with the same consequence, and is fixed alongside. Its stderr suffix was gated on `stderrChunks.length > 0`, which the decoder's final flush would have made unconditionally true — an empty string is still a chunk — so the condition now tests the joined text and no message gains a bare `: `. The corrupted text only ever reached an error message, so no scan result was ever wrong. It is the message a user reads when ast-grep rejects a rule file, naming a rule id or a path, which is where a non-ASCII character turns up. No regression test: reproducing this needs stderr split at a chosen byte offset, and both spawn sites resolve their binary internally, leaving no seam to inject a fake process. The two earlier fixes shipped the same way. Fixes #124 Refs #99 Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/decode-sg-stderr.md | 17 +++++++++++++++++ packages/cli/src/rules/runtime/narrow.ts | 15 +++++++++++---- packages/cli/src/rules/scan.ts | 20 +++++++++++++++++++- 3 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 .changeset/decode-sg-stderr.md 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) {