fix(glob,grep): keep exit-code gate at >1 — --no-messages alone is enough
Addresses cubic-dev-ai P1 + P2 findings on #4115. The original PR relaxed `exitCode > 1` to `> 2` based on the (wrong) claim that ripgrep exits 2 only on non-fatal I/O issues. ripgrep actually uses exit code 2 for BOTH fatal errors (pattern syntax, invalid args) AND non-fatal I/O issues; GNU grep (the fallback backend in grep/cli.ts) likewise uses 2 for fatal errors. So `> 2` would silently suppress fatal errors. The correct fix is just `--no-messages`, which suppresses ripgrep's stderr only for soft I/O issues (broken symlinks, permission denied) while leaving fatal-error messages intact. With the gate kept at `exitCode > 1 && stderr.trim()`: - Broken symlink: ripgrep exits 2, stderr is empty (suppressed) → `stderr.trim()` is falsy → gate fails → partial results survive. - Fatal error: ripgrep exits 2, stderr has the real error message (not suppressed by --no-messages) → gate triggers → error returned. Reverting both `exitCode > 1` → `> 2` changes; keeping the `--no-messages` flag additions and the regression test (test comment updated to describe the cleaner architecture). Verification: bun test src/tools/glob/ src/tools/grep/ → 30 pass / 0 fail. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -57,8 +57,10 @@ describe("buildRgArgs", () => {
|
|||||||
|
|
||||||
// Regression for #3726: broken/dangling symlinks should not surface as
|
// Regression for #3726: broken/dangling symlinks should not surface as
|
||||||
// tool errors. --no-messages silences ripgrep's non-fatal stderr warnings
|
// tool errors. --no-messages silences ripgrep's non-fatal stderr warnings
|
||||||
// so the downstream "exit code > 2 && stderr.trim()" gate sees a clean
|
// for I/O issues (broken symlinks, permission denied) without suppressing
|
||||||
// stream when only I/O warnings were emitted.
|
// fatal-error messages, so the existing "exit code > 1 && stderr.trim()"
|
||||||
|
// gate sees a clean stream for soft I/O issues but still triggers on real
|
||||||
|
// fatal ripgrep errors.
|
||||||
it("includes --no-messages so broken symlinks do not error the tool (#3726)", () => {
|
it("includes --no-messages so broken symlinks do not error the tool (#3726)", () => {
|
||||||
const args = buildRgArgs({ pattern: "*.ts" })
|
const args = buildRgArgs({ pattern: "*.ts" })
|
||||||
expect(args).toContain("--no-messages")
|
expect(args).toContain("--no-messages")
|
||||||
|
|||||||
@@ -152,10 +152,7 @@ async function runRgFilesInternal(
|
|||||||
const stderr = await new Response(proc.stderr).text()
|
const stderr = await new Response(proc.stderr).text()
|
||||||
const exitCode = await proc.exited
|
const exitCode = await proc.exited
|
||||||
|
|
||||||
// ripgrep exits 2 on non-fatal I/O errors (broken symlinks, permission
|
if (exitCode > 1 && stderr.trim()) {
|
||||||
// denied) even when stdout still contains valid file matches; treat only
|
|
||||||
// exit codes >2 as fatal so partial results survive. See #3726.
|
|
||||||
if (exitCode > 2 && stderr.trim()) {
|
|
||||||
return {
|
return {
|
||||||
files: [],
|
files: [],
|
||||||
totalFiles: 0,
|
totalFiles: 0,
|
||||||
|
|||||||
@@ -197,10 +197,7 @@ async function runRgInternal(options: GrepOptions, resolvedCli?: ResolvedCli): P
|
|||||||
const truncated = stdout.length >= DEFAULT_MAX_OUTPUT_BYTES
|
const truncated = stdout.length >= DEFAULT_MAX_OUTPUT_BYTES
|
||||||
const outputToProcess = truncated ? stdout.substring(0, DEFAULT_MAX_OUTPUT_BYTES) : stdout
|
const outputToProcess = truncated ? stdout.substring(0, DEFAULT_MAX_OUTPUT_BYTES) : stdout
|
||||||
|
|
||||||
// ripgrep exits 2 on non-fatal I/O errors (broken symlinks, permission
|
if (exitCode > 1 && stderr.trim()) {
|
||||||
// denied) even when stdout still contains valid matches; treat only
|
|
||||||
// exit codes >2 as fatal so partial results survive. See #3726.
|
|
||||||
if (exitCode > 2 && stderr.trim()) {
|
|
||||||
return {
|
return {
|
||||||
matches: [],
|
matches: [],
|
||||||
totalMatches: 0,
|
totalMatches: 0,
|
||||||
|
|||||||
Reference in New Issue
Block a user