Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .changeset/decode-sg-stderr.md
Original file line number Diff line number Diff line change
@@ -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.
15 changes: 11 additions & 4 deletions packages/cli/src/rules/runtime/narrow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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;
Expand Down
20 changes: 19 additions & 1 deletion packages/cli/src/rules/scan.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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 });
Expand All @@ -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) => {
Expand All @@ -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) {
Expand Down
Loading