Fix the line-ending drift that broke the CSharpier gate on non-Windows checkouts - #131
Merged
Merged
Conversation
…s checkouts Issue #129. `.editorconfig` required CRLF for every file while `.gitattributes` set `* text=auto` with no `eol`, so git stored LF and checked out the platform default. CSharpier follows `.editorconfig`, so `csharpier -- check` reported all 110 checked files on Linux and macOS and passed only on Windows, where the checkout is CRLF. The pre-commit hook rewrote staged files to an ending git normalized straight back, so the change never landed. LF is already the stored form of all 112 tracked `*.cs` blobs, so pinning it is a configuration change and rewrites no blob. `eol=lf` overrides `core.autocrlf`, so the checkout ending is LF on every platform and the gate is platform independent. `scripts/lint-line-endings.js` keeps the two configurations from drifting apart again. Every path either file names is resolved on both sides and the endings must match, so a narrowed `.editorconfig` section cannot disagree with a broad `.gitattributes` rule. A `text` rule without `eol`, an `eol` without `text`, a binary rule that pins an ending, a missing base rule, and a missing `[*]` section all fail closed. Wired into `lint:llm:full`, the llm-lint workflow (whose path filters now include both files), and pre-commit. Validation: the guard reports three drift sites against the pre-fix configuration and passes after it. `dotnet tool run csharpier -- check Editor Runtime Tests` reports `Checked 110 files` and exits 0, where it previously exited 1 on every file. A no-op format of a tracked file leaves its blob hash unchanged. `npm run lint:llm:full` green with 19 self-test files and 20 new line-ending cases.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e531975. Configure here.
A seeded-regression pass over the new guard found two problems. - `-text` was parsed as an attribute named `-text` with the value `true`, so a binary rule looked like text and the `binary` macro was ignored. Both now resolve to a binary path, which is what git does. - A recursive `**` glob was compiled as two single stars, which silently stops matching deeper paths and would hide the drift behind the rule. The pattern is now refused with the file and line that uses it, because neither file needs it. A byte-order-mark strip and the self-test that covered it were removed as redundant: parsing trims every line, and trimming already drops the mark. The drift report gained two fixtures that a matcher cannot satisfy by accident. A narrowed `.editorconfig` section now fails against a broad `.gitattributes` rule, which the first version read as in sync because the broad rule's representative path carries no extension. Validation: 25 self-test cases pass. Five seeded regressions (ignore `binary`, guess `**`, accept any ending name, skip the `[*]` requirement, accept a binary rule that pins an ending) are each killed by a distinct case.
This was referenced Oct 2, 2026
Bugbot review finding on PR #131. `samplePath` kept only the first `{a,b}` alternative, so later extensions in a brace group were never compared. With git pinning the first extension back to the default and leaving the rest on the broad rule, the guard reported the contract as in sync while the second extension still checked out LF against an editor that asked for CRLF. The same miss existed on the editor side when a narrowed section carried the brace group. Both directions were silent false passes, reproduced before the fix: - git side: `.editorconfig` `[ * ]` CRLF, `.gitattributes` `* eol=crlf` plus `{*.md,*.ps1} eol=lf` plus `*.md eol=crlf`. `sample.ps1` drifted and the guard exited 0. - editor side: `.editorconfig` `[ * ]` LF plus `[{*.md,*.ps1}]` CRLF, `.gitattributes` `* eol=lf` plus `*.md eol=crlf`. `sample.ps1` drifted and the guard exited 0. `samplePaths` now contributes one representative path per alternative, so the later ones reach the comparison. Brace expansion also became depth aware: it stopped at the first `}`, which left an inner group as literal text that matched nothing. Swept the same class in this matcher. A construct it cannot resolve exactly was read as literal text and silently stopped matching, which is how the finding arose. An anchored or directory-only `/`, a character class, and a backslash escape join the recursive `**` in a refusal that names the file and line, so a future pattern cannot be half-read. `.llm/references/forbidden-patterns.md` records the rule for every guard: resolve every alternative and nesting level, or refuse the construct. Validation: 33 self-test cases pass. Seven seeded regressions are each killed by a distinct case, including the two this finding describes and the two nested brace expansions. One earlier seeded regression turned out to be a no-op that the suite could not kill, so the nested-brace fixtures were rewritten to compare `sample.ps1` from the inner group.
Found while sweeping the same class of defect as the #131 review finding: the SKILL.md frontmatter reader tolerates unrecognized keys by design, which `manage-skills` documents. That tolerance has no teeth for a key that is recognizable but misspelled. A `catgeory:` under `metadata`, or `category:` written at the top level instead of under `metadata`, is ignored and the category falls back to `Feature`. The skill is filed under the wrong heading in the generated index and `generate-skills-index.ps1` plus `lint-llm-instructions.ps1` both report success. The reader cannot refuse unknown keys without contradicting its documented policy, so the preventive step goes in the skill: check the category heading in `.llm/skills/index.md` after regenerating.
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
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.

.editorconfigrequired CRLF while.gitattributesleft the checkout ending to the platform, socsharpier -- checkfailed on every file outside Windows. LF is now pinned in both, a guard keeps them in sync, and the guard resolves every brace alternative instead of the first.Behavior
dotnet tool run csharpier -- check Editor Runtime Testsexits 0 on Linux, macOS, and Windows; a no-op format of a tracked file leaves its blob hash unchanged.scripts/lint-line-endings.jsfails when either file's ending for a path disagrees with the other's, when atextrule omitseol, wheneolis set withouttext, or when a glob construct it cannot resolve is used.Validation
npm run lint:llm:fullgreen (19 files),npm pack --dry-run172 files unchanged, pre-commit green.Risk / Rollback
Risk is one working-tree change on Windows checkouts, which now see LF like every other platform.
Revert both config lines; the guard then reports the drift again.
Fixes #129. #132 records the same class of defect found in a different guard while sweeping.