Repository navigation
fix(ci): the invisible-character gate never matched anything - #25
Conversation
MEASURED 2026-08-27: this gate's pattern caught 0 OF 6 invisible-character test
cases. It has never detected an NBSP, zero-width space, BOM, soft hyphen, bidi
override or word joiner.
ROOT CAUSE: the pattern used UTF-8 BYTE sequences (\xc2\xa0) while grep -P
matches CHARACTERS. Bytes c2 a0 are ONE character U+00A0; \xc2\xa0 asks for TWO
characters, U+00C2 then U+00A0, which is never present.
grep -P '\xc2\xa0' -> miss
grep -P '\x{a0}' -> MATCH
Only \x00 worked, being single-byte in both readings.
FIXED: codepoint escapes; C0 control characters \x01-\x08,\x0B,\x0C,\x0E-\x1F
added (TAB/LF/CR excluded); and grep -a, without which grep skips any NUL-bearing
file as binary.
The C0 range matters: a stray BACKSPACE byte made a workflow unparseable in
developer-ecosystem, so it never ran, and this linter called it clean.
Canonical fix: hyperpolymath/empty-linter#70. 1 file(s) here.
VERIFIED: YAML re-parsed, and the corrected pattern was confirmed to catch a real
NBSP before the change was kept.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Free Run ID: 📒 Files selected for processing (1)
📝 SummarySummary by CodeRabbit
WalkthroughThe dogfood gate workflow now detects targeted control and invisible Unicode characters with a PCRE pattern. It also scans binary-marked files as text. ChangesInvisible character scan
Estimated code review effort: 2 (Simple) | ~5 minutes Poem
Note 🎁 Summarized by CodeRabbit FreeThe PR author is not assigned a seat. If you are a newly provisioned user and your organization has automatic seat assignment enabled, open a new pull request to trigger seat assignment. Otherwise, ask an organization administrator to assign a seat through the team management page at https://app.coderabbit.ai/settings/team-management. A rabbit reads each line, Comment |
This comment has been minimized.
This comment has been minimized.
Up to standards ✅🟢 Issues
|
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
This PR improves the invisible character detection gate by utilizing PCRE codepoint escapes and ensuring files with NUL bytes are scanned. While the regex improvements align with the intent, the implementation contains a logic error in how the process exit status is captured, which may result in the CI passing even when violations are found. Additionally, the file traversal logic is inefficient and uses redundant flags. There is currently no evidence of automated tests verifying that the new patterns correctly catch non-breaking spaces, control characters, or byte-order marks.
About this PR
- The gate's reliability is currently compromised by the way exit codes are handled. If the intention is to fail the build when invisible characters are detected, the logic needs to evaluate the grep results directly or use a find execution mode that propagates grep failures.
Test suggestions
- Verify detection of a Non-Breaking Space (U+00A0) in a supported source file
- Verify detection of C0 control characters (e.g., Backspace \x08) in a source file
- Verify that files containing NUL bytes (\x00) are scanned and reported rather than skipped as binary
- Verify detection of Byte Order Marks (U+FEFF) and Soft Hyphens (U+00AD)
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify detection of a Non-Breaking Space (U+00A0) in a supported source file
2. Verify detection of C0 control characters (e.g., Backspace \x08) in a source file
3. Verify that files containing NUL bytes (\x00) are scanned and reported rather than skipped as binary
4. Verify detection of Byte Order Marks (U+FEFF) and Soft Hyphens (U+00AD)
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| -o -name '*.gleam' -o -name '*.hs' -o -name '*.ml' -o -name '*.sh' \) \ | ||
| -exec grep -Prl "$PATTERNS" {} \; > /tmp/empty-lint-results.txt 2>/dev/null | ||
| -exec grep -aPrl "$PATTERNS" {} \; > /tmp/empty-lint-results.txt 2>/dev/null | ||
| EL_EXIT=$? |
There was a problem hiding this comment.
🟡 MEDIUM RISK
The exit code captured here reflects the status of the find command, not the success or failure of the grep scans. Since find ignores the exit status of -exec ... \; unless it refers to a filesystem error, EL_EXIT will likely always be 0. To accurately signal a failure, you should derive the status from the count of findings or use find ... -exec ... + which returns non-zero if any invocation fails.
| -o -name '*.idr' -o -name '*.zig' -o -name '*.v' -o -name '*.jl' \ | ||
| -o -name '*.gleam' -o -name '*.hs' -o -name '*.ml' -o -name '*.sh' \) \ | ||
| -exec grep -Prl "$PATTERNS" {} \; > /tmp/empty-lint-results.txt 2>/dev/null | ||
| -exec grep -aPrl "$PATTERNS" {} \; > /tmp/empty-lint-results.txt 2>/dev/null |
There was a problem hiding this comment.
⚪ LOW RISK
Suggestion: Using + instead of \; allows find to pass multiple file paths to a single grep process, which is significantly faster. Additionally, the -r (recursive) flag is redundant because find is already traversing the directory structure.
Head branch was modified
|
Measured 2026-08-27: this gate caught 0 of 6 invisible-character test cases. It has never detected an NBSP, zero-width space, BOM, soft hyphen, bidi override or word joiner.
Root cause
The pattern used UTF-8 byte sequences (
\xc2\xa0) whilegrep -Pmatches characters. Bytesc2 a0are one character U+00A0;\xc2\xa0asks for two, U+00C2 then U+00A0 — never present.Only
\x00worked, being single-byte in both readings. The gate ran, passed, and could not see what it exists to see.Fixed
\x01-\x08,\x0B,\x0C,\x0E-\x1Fadded (TAB/LF/CR excluded)grep -a— without it grep skips any NUL-bearing file as binaryThe C0 range matters: a stray backspace byte made a workflow unparseable in
developer-ecosystem, so it never ran — and this linter called it clean.Canonical fix: hyperpolymath/empty-linter#70. 1 file(s) here.
Verified: YAML re-parsed, and the corrected pattern was confirmed to catch a real NBSP before the change was kept.