fix(ci): the invisible-character gate never matched anything - #67
Open
hyperpolymath wants to merge 1 commit into
Open
fix(ci): the invisible-character gate never matched anything#67hyperpolymath wants to merge 1 commit into
hyperpolymath wants to merge 1 commit into
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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
|
| Layer / File(s) | Summary |
|---|---|
Unicode scan pattern .github/workflows/dogfood-gate.yml |
The pattern uses PCRE Unicode code-point escapes. It adds C0 control characters, U+2060, and U+FEFF coverage. The scan uses grep -a for text-mode processing. |
Estimated code review effort: 1 (Trivial) | ~5 minutes
Merge Risk: 🟡 Moderate · up to bc5d4
The workflow change is intended to detect invisible characters, but the current pattern may be rejected before scanning files, leaving the gate unable to enforce the intended check. The expression should be validated and parity with the compiled linter confirmed before merging.
Poem
A rabbit checks the hidden marks,
In scripts and gates and workflow parts.
Unicode points now show their face,
Control bytes cannot hide in place.
The gate hops on, precise and bright.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check | The PR fixes the CI gate pattern, but issue #70 also requires a separate leading-BOM check, matching C0 detection in the compiled linter, and updates across the other inlined copies. These changes are… |
Implement all coding requirements from issue #70, or split the remaining work into linked PRs with explicit scope. Include the leading-BOM check, synchronised compiled-linter logic, and corrections for the affected estate-wide copies. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly identifies the main change: fixing the CI invisible-character gate. |
| Description check | ✅ Passed | The description provides a clear summary, root cause, implementation details, linked issue context, and verification results. It does not reproduce the repository checklist, but the required technical… |
| Out of Scope Changes check | ✅ Passed | The changes are limited to the invisible-character detection logic in dogfood-gate.yml and are related to the stated objective. No unrelated changes are shown. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… |
Full details: Description check
Explanation
The description provides a clear summary, root cause, implementation details, linked issue context, and verification results. It does not reproduce the repository checklist, but the required technical information is present.
Full details: Linked Issues check
Explanation
The PR fixes the CI gate pattern, but issue #70 also requires a separate leading-BOM check, matching C0 detection in the compiled linter, and updates across the other inlined copies. These changes are not present in the one-file PR.
Full details: Docstring Coverage
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
- Create stacked PR
- Commit on current branch
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Up to standards ✅🟢 Issues
|
There was a problem hiding this comment.
Pull Request Overview
The PR successfully addresses the logic gaps in the invisible-character gate by adopting PCRE codepoint escapes and expanding the character set to include C0 control characters. While Codacy analysis indicates the changes are up to standards, a critical technical risk was identified: the PCRE engine (grep -P) requires explicit UTF-8 mode to correctly interpret Unicode escapes. Without this, the linter may produce false positives on standard UTF-8 characters or fail silently due to existing error suppression on line 135. Furthermore, there is a lack of regression tests to ensure the new patterns correctly catch the characters that previously escaped detection.
About this PR
- The PR description indicates that the previous gate failed to catch 6 specific test cases. Automated regression tests (e.g., a test file containing these invisible characters) should be added to the repository to prevent future regressions of this linter logic.
Test suggestions
- Verify detection of multi-byte Unicode characters (e.g., NBSP, ZWSP, BOM) using codepoint escapes
- Verify detection of C0 control characters like Backspace (\x08)
- Verify that files containing NUL bytes are successfully scanned instead of being ignored as binary
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify detection of multi-byte Unicode characters (e.g., NBSP, ZWSP, BOM) using codepoint escapes
2. Verify detection of C0 control characters like Backspace (\x08)
3. Verify that files containing NUL bytes are successfully scanned instead of being ignored as binary
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| # non-breaking spaces, null bytes, and other invisible Unicode in source files. | ||
| set +e | ||
| PATTERNS='\xc2\xa0|\xe2\x80\x8b|\xe2\x80\x8c|\xe2\x80\x8d|\xef\xbb\xbf|\xc2\xad|\xe2\x80\x8e|\xe2\x80\x8f|\xe2\x80\xaa|\xe2\x80\xab|\xe2\x80\xac|\xe2\x80\xad|\xe2\x80\xae|\x00' | ||
| PATTERNS='\x00|[\x01-\x08\x0B\x0C\x0E-\x1F]|\x{a0}|\x{ad}|\x{200b}|\x{200c}|\x{200d}|\x{200e}|\x{200f}|\x{202a}|\x{202b}|\x{202c}|\x{202d}|\x{202e}|\x{2060}|\x{feff}' |
There was a problem hiding this comment.
🔴 HIGH RISK
The PCRE engine requires explicit UTF-8 mode to correctly interpret Unicode code point escapes and to avoid false positives on byte sequences in UTF-8 files. Since the error is currently silenced by 2>/dev/null on line 135, the linter will fail silently.
Suggested change
| PATTERNS='\x00|[\x01-\x08\x0B\x0C\x0E-\x1F]|\x{a0}|\x{ad}|\x{200b}|\x{200c}|\x{200d}|\x{200e}|\x{200f}|\x{202a}|\x{202b}|\x{202c}|\x{202d}|\x{202e}|\x{2060}|\x{feff}' | |
| PATTERNS='(*UTF8)\x00|[\x01-\x08\x0B\x0C\x0E-\x1F]|\x{a0}|\x{ad}|\x{200b}|\x{200c}|\x{200d}|\x{200e}|\x{200f}|\x{202a}|\x{202b}|\x{202c}|\x{202d}|\x{202e}|\x{2060}|\x{feff}' |
| -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: The recursive flag is unnecessary when targeting individual files, and the current command spawns a new process per file. Performance can be improved by batching files and removing the error suppression.
Try running the following prompt in your coding agent:
Update line 135 in
.github/workflows/dogfood-gate.ymlto use-exec grep -aPl "$PATTERNS" {} +and remove the2>/dev/nullredirection.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.