Skip to content

fix(shell-gate): || is not a pipe, and |& is (backend#2264) - #777

Merged
LukasWodka merged 4 commits into
developfrom
fix/2264-scanner-double-bar
Aug 21, 2026
Merged

fix(shell-gate): || is not a pipe, and |& is (backend#2264)#777
LukasWodka merged 4 commits into
developfrom
fix/2264-scanner-double-bar

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Part of tracebloc/backend#2264.

The gate rejected the output of its own remediation

Converting the fleet for #2264 produces exactly this line:

cmd || grep -q needle <<<"$out"

There is no pipe in it. But the hazard regex matched the second bar of the ||, so the scanner reported the very form this repo documents as the house idiom. It would have blocked every cleanup PR in the #2264 wave — and a gate that rejects its own fix is how a gate teaches people to switch it off.

Found by running the gate against my own converted cli scripts, not by reading it.

Fix

Neutralise || into \001\001 on a probe copy before the hazard test. \001 cannot occur in a shell source line, and the original line is still what gets printed, so offender output is byte-identical.

Second, smaller gap found while writing that test: |& — bash's pipe-both-streams — is a pipe and carries the identical hazard, but the bar had to be followed by space-or-name, so noisy |& head -1 was missed entirely. The bar now takes an optional &. A gate that misses a real pipe form is the "advice, not a gate" failure from the house rules, so I fixed it here rather than filing it.

Mutation-proof

Baseline 33 green. Each mutation asserted its anchor matched exactly one line before running — an inert mutation and real coverage are indistinguishable in a log otherwise.

mutationreddens
remove the || neutralisation3 — tests 27, 28, 29
hazard test reads line, not probe1 — test 28
drop |& from the head arm1 — test 32
drop |& from the grep -q arm1 — test 33

Every mutation is caught by the test that names the property, not by one shared failure that any mutation would trip.

The load-bearing test is "a real pipe is still flagged on a line that also contains ||". Without it, the three new spare-cases would all pass if the hazard test were deleted outright — three tests asserting the right property and proving nothing. That's the shape client#776 is about, so it would have been a poor look to ship it here.

Verification

  • the gate reports zero over the whole tree
  • check-style.sh clean, gen-manifest.sh --check up to date
  • 23 other bats suites run, 0 failures (the remainder are unaffected — this touches only the scanner and its own test file)

Sequencing note

This wants to land before the remaining #2264 conversion PRs, or the gate will be wrong about them once it's armed. It does not block them from merging today, since the gate is not yet a required check anywhere.

🤖 Generated with Claude Code


Note

Low Risk
Linter-only regex and test changes; no runtime, auth, or data-path code. Residual risk is false positives/negatives in the style gate, which the new tests pin.

Overview
Stops the pipefail early-close scanner from flagging the house remediation idiom (cmd || grep -q …) by treating || as not a pipeline, while still catching real | / |& hazards on the same line.

The matcher now runs on a probe copy where || is replaced with a \001 stand-in that also acts as a regex boundary, so a later || grep -q cannot be borrowed by a plain | grep, and glued forms like head||die stay flagged. |& (pipe both streams) is now recognized as a pipe.

Also documents and pins a known false positive: set inside a multi-line quoted fixture is read as the file’s own options; # pipefail-guard: allow remains the workaround.

Reviewed by Cursor Bugbot for commit a0a9d3f. Bugbot is set up for automated code reviews on this repo. Configure here.

The early-close gate flagged the output of its own remediation. Converting
the fleet for #2264 produces exactly this line:
cmd || grep -q needle <<<"$out"
There is no pipe there -- but the hazard regex matched the SECOND bar of the
`||`, so the scanner reported the form this repo recommends. A gate that
rejects its own fix is how a gate teaches people to switch it off, and it
would have blocked every cleanup PR in the #2264 wave.
Fix: neutralise `||` into \001\001 on a probe copy before the hazard test.
\001 cannot occur in a shell source line, and the ORIGINAL line is still
what gets printed, so offender output is unchanged.
Second, smaller gap found while writing that: `|&` (bash's pipe-both-streams)
is a pipe and carries the identical hazard, but the bar had to be followed by
space-or-name, so `noisy |& head -1` was missed. The bar now takes an
optional `&`.
Mutation-proved, 33 -> 33 green baseline, each mutation asserted to match
exactly one line first:
remove the || neutralisation -> 3 red (tests 27, 28, 29)
hazard test reads line not probe -> 1 red (test 28)
drop |& from the head arm -> 1 red (test 32)
drop |& from the grep -q arm -> 1 red (test 33)
Each mutation is caught by the test that NAMES the property, not by a shared
failure -- and the "a real pipe on a line that also contains ||" case is what
stops the three new spare-tests from passing if the hazard test were deleted
outright.
Verified: the gate reports zero over the whole tree; check-style clean;
gen-manifest --check up to date; 23 other bats suites green (0 failures).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodkaLukasWodka self-assigned this Aug 20, 2026
Comment threadscripts/tests/pipefail-early-close.awk Outdated
…end#2264)
Measuring tracebloc/.github for the #2264 wave turned up its ONE reported
offender, and it is a false positive of a second, different kind:
house-rules-selftest.sh passes whole scripts as multi-line quoted arguments,
expect "a bare curl fires both rules" \
'#!/bin/bash
set -euo pipefail
curl -fsSL "$url" -o out' curl-timeout,curl-tls
and the scanner reads those `set -euo pipefail` lines as the FILE's options.
The file's real options are `set -uo pipefail` -- no errexit -- so the line
it flags carries no hazard at all.
I tried to fix it by tracking single-quoted regions and REJECTED the fix.
Shell has no escape inside '...', so counting quotes is exact in principle,
but apostrophes in prose ("the file's options") desynchronise the count --
measured, it is already out of phase by line 37 of that very file, before
any fixture appears. A desynchronised count HIDES real offenders, which is
strictly worse for a gate than reporting a false one.
So: documented in the header, and PINNED by a test asserting the current
(wrong) answer, so a future fix reddens it deliberately instead of changing
verdicts silently. A second test covers `# pipefail-guard: allow` as the
documented workaround for exactly this shape.
35 green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lient#777)
Bugbot caught the false positive one level deeper than my fix went, and it
is right. `grep[^|]*` used the `|` characters of a `||` as its STOP. Turning
them into something the class permitted let a plain `| grep` span the rest of
the line and reach the `-q` of a later `|| grep -q`:
producer | grep needle && cmd || grep -q x <<<"$y"
flagged again -- the same false positive my previous commit set out to remove.
Reproduced before fixing.
Fix: add \001 to the class, `grep[^|\001]*`, on both grep arms. Neutralising
the pipe was only half the job; the boundary has to survive it.
I also tried shrinking the stand-in from \001\001 to a single \001, on the
theory that the width mattered. Its mutation SURVIVED -- the class does all
the work -- so the change proved nothing and is reverted. Only the class
change ships.
Three tests added, 38 green. The third is the discrimination that makes the
other two mean something: on a line of that exact shape, a REAL `| grep -q`
is still flagged. Without it, widening the stand-in until everything is
spared would pass.
Mutation-proved:
drop \001 from the grep -q class -> test 36 red
drop \001 from the grep -m class -> test 37 red
shrink stand-in to one char -> nothing red (hence reverted)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit aa80689. Configure here.

Comment threadscripts/tests/pipefail-early-close.awk Outdated
…s (Arturo)
Arturo caught a FALSE NEGATIVE this PR introduced and develop did not have:
producer | head||die
`gsub` turns `||` into \001\001, so `head` is followed by \001 -- which was not
in the head terminator class, so the pipe was missed. Reproduced against both
scanners before fixing: develop flags the line, this PR silently dropped it.
It is the exact mirror of the boundary fix I made for the grep arms two commits
ago, and I only did half of it. `\001` went into `grep[^|\001]*` but not into
head's `[)"'`;|&]`, so the stand-in read as ordinary text on one arm and as a
boundary on the other. The neutralisation and the boundary are one change;
doing half moves the bug rather than fixing it -- which has now happened twice
on this same three-line edit.
Fix is his suggestion verbatim: add \001 to the head terminator class.
Four tests, 42 green. Two cover the glued form (`head||die`, `|& head||die`);
two are the discrimination that keeps them honest -- `head||true` and
`head|| :` must STILL be spared, or the fix would just be "flag anything with
head in it".
Mutation-proved: dropping \001 from the class reddens exactly tests 39 and 40,
and nothing else. Anchor asserted to match one line first.
Also checked the whole-tree parity that would have caught this earlier: running
develop's scanner and this one over every tracked .sh gives byte-identical
offender lists.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit a0a9d3f. Configure here.

@aptraceblocaptracebloc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review — approving. Fixes two operator misclassifications in pipefail-early-close.awk (the shell-lint early-close gate): || was wrongly read as a pipe, and |& was wrongly not.

The false-negative regression I flagged last pass — producer | head||die silently dropped because the ||\001 rewrite left head before a \001 not in its terminator class — is fixed in a0a9d3fab: \001 was added to the head terminator class ([)"'\;|&\001]), mirroring the grep arms' [^|\001]*. Verified in code; confirmed the || (true|:)guard still runs first sohead||true/head|| : aren't newly flagged; and confirmed the author added the pinning tests my finding said were missing (head||dieand|& head||dieflagged,head||true/head|| :` spared) — derived, mutation-proof, specific. My thread is resolved, CI green, 0 open threads, MERGEABLE.

— drafted with Claude Code

@LukasWodka
LukasWodka merged commit 3dd5c2e into developAug 21, 2026
36 checks passed
@LukasWodka
LukasWodka deleted the fix/2264-scanner-double-bar branch August 21, 2026 08:19
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@LukasWodka@aptracebloc@saadqbal