Skip to content

fix(ci): the invisible-character gate never matched anything - #54

Open
hyperpolymath wants to merge 1 commit into
mainfrom
fix/empty-linter-pattern-never-matched
Open

fix(ci): the invisible-character gate never matched anything#54
hyperpolymath wants to merge 1 commit into
mainfrom
fix/empty-linter-pattern-never-matched

Conversation

@hyperpolymath

Copy link
Copy Markdown
Owner

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) while grep -P matches characters. Bytes c2 a0 are one character U+00A0; \xc2\xa0 asks for two, U+00C2 then U+00A0 — never present.

grep -P '\xc2\xa0'  ->  miss
grep -P '\x{a0}'    ->  MATCH

Only \x00 worked, being single-byte in both readings. The gate ran, passed, and could not see what it exists to see.

Fixed

  • codepoint escapes in place of byte sequences
  • C0 controls \x01-\x08,\x0B,\x0C,\x0E-\x1F added (TAB/LF/CR excluded)
  • grep -a — without it 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.

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.
@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2ef41329-0e75-48ab-9359-05ca38aa4344

📥 Commits

Reviewing files that changed from the base of the PR and between b2853f9 and d15d71a.

📒 Files selected for processing (1)
  • .github/workflows/dogfood-gate.yml

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. (24)
  • GitHub Check: Gitar
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: rust-ci / Detect Cargo.toml
  • GitHub Check: scan / gitleaks
  • GitHub Check: scan / rust-secrets
  • GitHub Check: scan / shell-secrets
  • GitHub Check: governance / Trusted-base reduction policy
  • GitHub Check: governance / Code quality + docs
  • GitHub Check: governance / Workflow security linter
  • GitHub Check: governance / Language / package anti-pattern policy
  • GitHub Check: governance / Security policy checks
  • GitHub Check: governance / Check Workflow Staleness
  • GitHub Check: governance / Licence consistency
  • GitHub Check: governance / Guix primary / Nix fallback policy
  • GitHub Check: scan / Hypatia Neurosymbolic Analysis
  • GitHub Check: governance / Well-Known (RFC 9116 + RSR)
  • GitHub Check: panic-attack assail
  • GitHub Check: Hypatia neurosymbolic scan
  • GitHub Check: Validate K9 contracts
  • GitHub Check: Groove manifest check
  • GitHub Check: Validate A2ML manifests
  • GitHub Check: Empty-linter (invisible characters)
  • GitHub Check: Validate eclexiaiser manifest
  • GitHub Check: analyze (actions, none)
🔇 Additional comments (1)
.github/workflows/dogfood-gate.yml (1)

122-133: 🎯 Functional Correctness

A separate leading-BOM check is not required for this reason. GNU grep -aPrl detects both leading and embedded UTF-8 BOMs when the pattern is valid.


📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved detection of invisible and control characters during automated checks.
    • Updated scanning to reliably inspect files that may contain binary content.

Walkthrough

The workflow gate now detects invisible characters with Unicode code-point escapes, includes additional C0 controls, and scans binary files as text.

Changes

Invisible-character gate

Layer / File(s) Summary
Gate pattern and file scanning
.github/workflows/dogfood-gate.yml
The PATTERNS expression now uses Unicode code-point escapes and includes additional C0 control characters. The grep scan uses -a to process binary files as text.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to d15d7

The workflow gate now matches the intended invisible characters, including BOMs, without requiring a separate BOM check. No actionable merge-blocking risk remains beyond normal checks and review.

Poem

A rabbit checks the hidden marks,
With careful paws and watchful ears.
Unicode points now show their tracks,
Binary files lose their fears.
The gate can spot what once stayed dark.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR implements the codepoint escapes, C0 control detection, and grep -a changes required by issue #70. It does not show the required separate leading-BOM check, compiled linter alignment, or update… Add the separate leading-BOM check, update the compiled linter and configuration to match the CI gate, and apply the correction to all remaining inlined dogfood-gate.yml copies required by issue #70. Verify the targeted characters and clean…
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: fixing the CI gate that failed to detect invisible characters.
Description check ✅ Passed The description clearly explains the measured failure, root cause, implemented fixes, and verification. It does not reproduce every template heading or checklist item, but it provides the key required…
Out of Scope Changes check ✅ Passed The reported change is confined to the invisible-character detection logic in .github/workflows/dogfood-gate.yml and is directly related to issue #70. No unrelated changes are identified.
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 clearly explains the measured failure, root cause, implemented fixes, and verification. It does not reproduce every template heading or checklist item, but it provides the key required information.

Full details: Linked Issues check

Explanation

The PR implements the codepoint escapes, C0 control detection, and grep -a changes required by issue #70. It does not show the required separate leading-BOM check, compiled linter alignment, or updates to the remaining inlined dogfood-gate.yml copies across the estate.

Resolution

Add the separate leading-BOM check, update the compiled linter and configuration to match the CI gate, and apply the correction to all remaining inlined dogfood-gate.yml copies required by issue #70. Verify the targeted characters and clean-file cases after these changes are complete.

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

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gitar-bot

gitar-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

Gitar

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

@codacy-production codacy-production Bot 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.

Pull Request Overview

The PR successfully addresses the issue where the invisible-character linter failed to match targeted characters by migrating to Unicode codepoint escapes and adding the binary-as-text flag.

While the implementation logic is sound and Codacy results are up to standards, there is a lack of test fixtures containing the targeted characters. This means the fix is not automatically verified within the PR and remains vulnerable to regression. Additionally, there are opportunities to optimize the CI workflow's performance by batching file processing.

About this PR

  • The PR does not include any test files or fixtures containing the targeted invisible characters (e.g., NBSP, BOM, or C0 controls). Without these, there is no automated verification that the gate now correctly identifies these characters or will continue to do so in the future.

Test suggestions

  • Verify detection of Non-Breaking Space (U+00A0)
  • Verify detection of Byte Order Mark (U+FEFF)
  • Verify detection of C0 control characters (e.g., Backspace \x08)
  • Verify that files containing NUL bytes (\x00) are scanned rather than skipped
  • Verify detection of Zero-Width Space (U+200B) and Joiners
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify detection of Non-Breaking Space (U+00A0)
2. Verify detection of Byte Order Mark (U+FEFF)
3. Verify detection of C0 control characters (e.g., Backspace \x08)
4. Verify that files containing NUL bytes (\x00) are scanned rather than skipped
5. Verify detection of Zero-Width Space (U+200B) and Joiners

TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback

-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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚪ LOW RISK

Suggestion: The current find command is inefficient because it spawns a new grep process for every file, and the -r flag is redundant since find handles the traversal. Switching to the + terminator allows find to batch multiple filenames into fewer grep invocations:

Suggested change
-exec grep -aPrl "$PATTERNS" {} \; > /tmp/empty-lint-results.txt 2>/dev/null
-exec grep -aPl "$PATTERNS" {} + > /tmp/empty-lint-results.txt 2>/dev/null

Sign up for free to 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.

1 participant