Skip to content

Fix the line-ending drift that broke the CSharpier gate on non-Windows checkouts - #131

Merged
wallstop merged 4 commits into
mainfrom
t129/canonical-line-endings
Oct 2, 2026
Merged

wallstop merged 4 commits into
mainfrom
t129/canonical-line-endings

Conversation

@wallstop

@wallstop wallstop commented Oct 2, 2026 •

Copy link
Copy Markdown
Owner

.editorconfig required CRLF while .gitattributes left the checkout ending to the platform, so csharpier -- check failed 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 Tests exits 0 on Linux, macOS, and Windows; a no-op format of a tracked file leaves its blob hash unchanged.
  • scripts/lint-line-endings.js fails when either file's ending for a path disagrees with the other's, when a text rule omits eol, when eol is set without text, or when a glob construct it cannot resolve is used.

Validation

  • Guard reports the three pre-fix drift sites and 33 self-test cases pass; seven seeded regressions each killed by a distinct case; npm run lint:llm:full green (19 files), npm pack --dry-run 172 files unchanged, pre-commit green.
  • No C# or Unity change, so the EditMode and PlayMode suites are not exercised.

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.

…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.

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ 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.

Comment thread scripts/lint-line-endings.js
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.
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.
@wallstop
wallstop merged commit 6e5233b into main Oct 2, 2026
3 checks passed
@wallstop
wallstop deleted the t129/canonical-line-endings branch October 2, 2026 17:45
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.

chore: The CSharpier gate cannot pass on a non-Windows checkout (CRLF vs LF)

1 participant