chore: enforce LF line endings repository-wide#900
Conversation
- Add global `* text=auto eol=lf` rule to .gitattributes - Document cross-platform Git configuration in CONTRIBUTING.md - Add developer setup instructions for core.autocrlf - Create Normalize-LineEndings.ps1 script for one-time normalization - Audit shows 393 files currently have CRLF (need normalization) This prevents YAML frontmatter parsing errors on Windows documented in: - GitHub Copilot CLI issue #694 - GitHub Copilot CLI issue #673 Users should run the normalization script after pulling: pwsh scripts/Normalize-LineEndings.ps1 Then commit the normalized files in a follow-up commit. Fixes #896 Co-authored-by: Richard Murillo <rjmurillo@users.noreply.github.com>
PR Validation ReportNote ✅ Status: PASS Description Validation
QA Validation
⚡ Warnings
Powered by PR Validation workflow |
There was a problem hiding this comment.
Code Review
This pull request introduces a repository-wide policy to enforce LF line endings, which is a great step for improving cross-platform development consistency and preventing common issues with tools that are sensitive to line endings. The changes are well-documented in both .gitattributes and CONTRIBUTING.md, providing clear guidance for all contributors. The inclusion of the Normalize-LineEndings.ps1 script is a thoughtful addition to handle the one-time migration of existing files.
My review focuses on the new PowerShell script, where I've identified a few areas for improvement regarding correctness and adherence to PowerShell best practices, particularly in its main conditional logic, normalization steps, and error handling. These suggestions aim to make the script more robust and align it with standard procedures.
Overall, this is a solid and much-needed change for the repository.
AI Quality Gate ReviewWarning WalkthroughThis PR was reviewed by six AI agents in parallel, analyzing different aspects of the changes:
Review Summary
💡 Quick Access: Click on individual agent jobs (e.g., "🔒 security Review", "🧪 qa Review") in the workflow run to see detailed findings and step summaries. Security Review DetailsLet me also check the analysis file to complete the review. Security Review: PR #900PR Type Classification
Findings
Analysis SummaryPowerShell Script (
.gitattributes:
CONTRIBUTING.md:
RecommendationsNone required. The PR implements standard Git line ending normalization with no security implications. VerdictQA Review DetailsLet me check the line-endings-before.txt file structure and verify there are tests for the new script: Based on my analysis, I have all the information needed to provide the QA verdict. QA Review: PR #900 - chore: enforce LF line endings repository-widePR TYPE: MIXEDFiles by category:
Test Coverage Assessment
Quality Concerns
Fail-Safe Pattern Verification
Regression Risk Assessment
Code Quality
Evidence SummaryRationaleThis PR adds a well-structured PowerShell utility script with:
However, the script lacks unit tests. Per repository patterns, other scripts in
Recommendation: Create
Analyst Review DetailsI have gathered sufficient information to complete the analysis. Analyst Review: PR #900Code Quality Score
Overall: 4.75/5 Impact Assessment
Findings
Recommendations
VerdictArchitect Review DetailsDesign Quality Assessment
Overall Design Score: 4.8/5 Architectural Concerns
Breaking Change Assessment
Technical Debt Analysis
ADR Assessment
Recommendations
DevOps Review DetailsLet me also check the analysis file that was added: The analysis file is a large audit log of line endings. This is an expected artifact from the normalization script. Now I have all the information needed to complete the DevOps review. Pipeline Impact Assessment
CI/CD Quality Checks
Findings
Shell Script Quality AssessmentThe
.gitattributes ReviewThe
CONTRIBUTING.md ReviewThe Git Configuration section provides clear, platform-specific instructions for developers on Windows, Linux, and macOS. Recommendations
VerdictRoadmap Review DetailsStrategic Alignment Assessment
Feature Completeness
Impact Analysis
Concerns
Recommendations
VerdictRun Details
Powered by AI Quality Gate workflow |
There was a problem hiding this comment.
Pull request overview
This PR implements comprehensive line ending enforcement to prevent YAML frontmatter parsing errors on Windows. The changes establish LF as the standard line ending across all files through .gitattributes configuration, provide developer setup guidance, and offer a normalization script for existing files.
Changes:
- Added global
* text=auto eol=lfrule in.gitattributesto enforce LF line endings repository-wide - Added "Git Configuration" section in
CONTRIBUTING.mdwith platform-specificcore.autocrlfinstructions - Created
scripts/Normalize-LineEndings.ps1PowerShell script for one-command normalization of existing files with audit trails
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| .gitattributes | Added comprehensive line ending normalization section with * text=auto eol=lf rule and extensive documentation explaining the rationale, Git behavior, and developer configuration requirements |
| CONTRIBUTING.md | Added "Git Configuration" section with platform-specific instructions for Windows (core.autocrlf=true) and Linux/macOS (core.autocrlf=input), plus explanation of why line ending consistency matters |
| scripts/Normalize-LineEndings.ps1 | New PowerShell script to normalize existing files to LF, with before/after statistics, audit file generation, and clear reporting of changes |
| # In addition to this .gitattributes file, developers should configure their | ||
| # global Git settings based on their operating system: | ||
| # | ||
| # Windows: | ||
| # git config --global core.autocrlf true | ||
| # (Converts LF → CRLF on checkout, CRLF → LF on commit) | ||
| # | ||
| # Linux/macOS: | ||
| # git config --global core.autocrlf input | ||
| # (Accepts LF as-is, converts CRLF → LF on commit) | ||
| # | ||
| # This combination ensures: | ||
| # - Repository always has LF (.gitattributes enforces it) | ||
| # - Windows users get native CRLF for text editors (core.autocrlf=true) | ||
| # - Unix users get native LF (core.autocrlf=input) | ||
| # - No line ending conflicts or parsing errors |
There was a problem hiding this comment.
The documentation in .gitattributes states that with eol=lf, Git checks out files with LF line endings in the working directory, which overrides platform defaults. However, the guidance recommends Windows users set core.autocrlf=true, which would convert LF to CRLF on checkout. This creates a conflict:
.gitattributeswitheol=lfforces LF in working directorycore.autocrlf=truetries to convert LF to CRLF on checkout
When eol=lf is set in .gitattributes, it takes precedence over core.autocrlf, meaning Windows users will get LF in their working directory regardless of their core.autocrlf setting. This contradicts the documentation claim at lines 44-45 that "Windows users get native CRLF for text editors."
Consider either:
- Removing the
core.autocrlfrecommendations entirely (sinceeol=lfoverrides it) - Changing to
* text=autowithouteol=lfand relying oncore.autocrlfsettings - Clarifying that Windows users will have LF in working directory despite
core.autocrlf=true
| # In addition to this .gitattributes file, developers should configure their | |
| # global Git settings based on their operating system: | |
| # | |
| # Windows: | |
| # git config --global core.autocrlf true | |
| # (Converts LF → CRLF on checkout, CRLF → LF on commit) | |
| # | |
| # Linux/macOS: | |
| # git config --global core.autocrlf input | |
| # (Accepts LF as-is, converts CRLF → LF on commit) | |
| # | |
| # This combination ensures: | |
| # - Repository always has LF (.gitattributes enforces it) | |
| # - Windows users get native CRLF for text editors (core.autocrlf=true) | |
| # - Unix users get native LF (core.autocrlf=input) | |
| # - No line ending conflicts or parsing errors | |
| # .gitattributes with "eol=lf" takes precedence over core.autocrlf: | |
| # - Git stores and checks out all files with LF line endings | |
| # - This happens on ALL platforms, regardless of core.autocrlf | |
| # | |
| # Recommended global Git settings: | |
| # | |
| # Windows: | |
| # git config --global core.autocrlf false | |
| # # or, if you sometimes commit from tools that may introduce CRLF: | |
| # # git config --global core.autocrlf input | |
| # | |
| # Linux/macOS: | |
| # git config --global core.autocrlf input | |
| # | |
| # This combination ensures: | |
| # - Repository and working directory always use LF (.gitattributes enforces it) | |
| # - All platforms see consistent LF line endings in editors | |
| # - Avoids conflicts between core.autocrlf and .gitattributes | |
| # - Prevents line ending–related parsing errors (e.g., YAML frontmatter issues) |
| - **On checkout:** Git converts LF → CRLF for your text editors (Windows native) | ||
| - **On commit:** Git converts CRLF → LF for the repository | ||
| - **Result:** Repository always has LF, your editor always shows CRLF |
There was a problem hiding this comment.
The same issue exists in CONTRIBUTING.md - the documentation states that Windows users with core.autocrlf=true will get CRLF in their working directory ("your editor always shows CRLF"), but this is incorrect when .gitattributes contains * text=auto eol=lf.
The eol=lf directive in .gitattributes takes precedence over core.autocrlf, meaning all users (including Windows) will have LF line endings in their working directory. This makes the recommendation to set core.autocrlf=true ineffective and the documentation misleading.
The documentation should accurately reflect that with the current .gitattributes configuration, all developers will work with LF line endings in their working directory, regardless of platform or core.autocrlf setting.
| - **On checkout:** Git converts LF → CRLF for your text editors (Windows native) | |
| - **On commit:** Git converts CRLF → LF for the repository | |
| - **Result:** Repository always has LF, your editor always shows CRLF | |
| - **On checkout (in general):** Git converts LF → CRLF for your text editors (Windows native) | |
| - **On checkout (in this repo):** `.gitattributes` with `* text=auto eol=lf` forces LF in your working directory, overriding the usual CRLF conversion | |
| - **On commit:** Git converts any CRLF → LF for the repository | |
| - **Result (this repo):** Both the repository and your working directory use LF line endings, regardless of platform or `core.autocrlf` setting |
| git rm --cached -r . 2>&1 | Out-Null | ||
|
|
||
| # Step 2: Re-add all files, applying new .gitattributes rules | ||
| Write-Host " [2/3] Re-adding files with normalized line endings..." -ForegroundColor Gray | ||
| git add --renormalize . 2>&1 | Out-Null |
There was a problem hiding this comment.
The error handling suppresses git command output but doesn't check for command failures. If git rm --cached -r . or git add --renormalize . fail, the script continues silently and may report misleading success.
Consider capturing command exit codes and handling failures appropriately. For example:
$result = git rm --cached -r . 2>&1
if ($LASTEXITCODE -ne 0) {
throw "Failed to remove files from index: $result"
}This ensures the script fails fast if git operations encounter errors rather than proceeding with potentially incomplete normalization.
| git rm --cached -r . 2>&1 | Out-Null | |
| # Step 2: Re-add all files, applying new .gitattributes rules | |
| Write-Host " [2/3] Re-adding files with normalized line endings..." -ForegroundColor Gray | |
| git add --renormalize . 2>&1 | Out-Null | |
| $gitRmOutput = git rm --cached -r . 2>&1 | |
| if ($LASTEXITCODE -ne 0) { | |
| throw "Failed to remove files from index: $gitRmOutput" | |
| } | |
| # Step 2: Re-add all files, applying new .gitattributes rules | |
| Write-Host " [2/3] Re-adding files with normalized line endings..." -ForegroundColor Gray | |
| $gitAddOutput = git add --renormalize . 2>&1 | |
| if ($LASTEXITCODE -ne 0) { | |
| throw "Failed to add files with normalized line endings: $gitAddOutput" | |
| } |
| - `.gitattributes` enforces LF in the repository (`* text=auto eol=lf`) | ||
| - `core.autocrlf` gives you native line endings in your working directory | ||
| - Together, they ensure consistency without sacrificing developer experience |
There was a problem hiding this comment.
The documentation states that .gitattributes enforces LF in the repository and core.autocrlf gives native line endings in the working directory, working together to ensure consistency. However, this is inaccurate because eol=lf in .gitattributes forces LF in both the repository AND the working directory, overriding core.autocrlf.
The actual behavior is that .gitattributes with eol=lf enforces LF everywhere, making core.autocrlf settings irrelevant for this repository. The documentation should clarify this to avoid confusion.
| # add specific exceptions below. Currently, no exceptions are known in this repository. | ||
| # | ||
| # Evidence: https://github.com/github/copilot-cli/issues/694 | ||
| # https://github.com/github/copilot-cli/issues/673 |
There was a problem hiding this comment.
The reference to "GitHub Copilot CLI Issue #673" in the Evidence section appears to be incorrect. According to the PR metadata, issue #673 is about standardizing skill output formats, not line ending issues.
Only issue #694 should be referenced as evidence since it's the one that specifically documents CRLF line ending YAML parsing errors.
| # https://github.com/github/copilot-cli/issues/673 |
| try { | ||
| git rev-parse --is-inside-work-tree 2>$null | Out-Null | ||
| return $true | ||
| } | ||
| catch { | ||
| return $false | ||
| } |
There was a problem hiding this comment.
The Test-GitRepository function catches all exceptions and returns false, which will mask the actual error from git rev-parse. If git is not installed or there's another error, the generic message at line 108 won't help users diagnose the problem.
Consider letting the exception propagate or providing more specific error messages. The calling code at line 107-110 already has error handling that could provide context about the actual failure.
| try { | |
| git rev-parse --is-inside-work-tree 2>$null | Out-Null | |
| return $true | |
| } | |
| catch { | |
| return $false | |
| } | |
| git rev-parse --is-inside-work-tree 2>$null | Out-Null | |
| return $LASTEXITCODE -eq 0 |
| - **On checkout:** Git leaves LF as-is (Unix native) | ||
| - **On commit:** Git converts any CRLF → LF for the repository | ||
| - **Result:** Repository always has LF, your editor always shows LF |
There was a problem hiding this comment.
The same documentation issue extends to the explanation of Linux/macOS behavior. The statement "your editor always shows LF" is technically correct, but it's misleading to imply this is due to core.autocrlf=input when the .gitattributes file's eol=lf setting is what actually enforces LF in the working directory for all platforms.
The core.autocrlf=input setting is effectively redundant with * text=auto eol=lf in .gitattributes.
| - **On checkout:** Git leaves LF as-is (Unix native) | |
| - **On commit:** Git converts any CRLF → LF for the repository | |
| - **Result:** Repository always has LF, your editor always shows LF | |
| - **On checkout:** `.gitattributes` with `* text=auto eol=lf` ensures files are written with LF line endings in your working directory | |
| - **On commit:** With `core.autocrlf=input`, Git converts any CRLF you might have introduced back to LF before storing changes in the repository | |
| - **Result:** Both the repository and your working directory use LF; your editor shows LF because files are checked out with LF, not because of `core.autocrlf` | |
| - **Note:** Given the `.gitattributes` setting, `core.autocrlf=input` mainly acts as a safety net and is largely redundant in typical workflows |
| # | ||
| # Problem: Cross-platform development (Windows/Linux/macOS) with inconsistent line endings | ||
| # - Windows Git defaults to CRLF, causing YAML frontmatter parsing failures | ||
| # - GitHub Copilot CLI issues #694 and #673: "Unexpected scalar at node end" |
There was a problem hiding this comment.
The reference to "GitHub Copilot CLI Issue #673" appears to be incorrect. According to the PR metadata, the external GitHub Copilot CLI issue #673 is about standardizing skill output formats, not line ending issues.
The comment should only reference issue #694, which is specifically about CRLF line ending YAML parsing errors. The internal issue reference (#896) is correct and should be kept.
| **References:** | ||
|
|
||
| - [GitHub Copilot CLI Issue #694](https://github.com/github/copilot-cli/issues/694) | ||
| - [GitHub Copilot CLI Issue #673](https://github.com/github/copilot-cli/issues/673) |
There was a problem hiding this comment.
The reference to "GitHub Copilot CLI Issue #673" appears to be incorrect. According to the PR metadata, issue #673 is about standardizing skill output formats, not line ending issues.
Only issue #694 should be referenced here as it specifically documents CRLF line ending YAML parsing errors. Issue #896 is correctly referenced as it's the internal issue for this PR.
| - [GitHub Copilot CLI Issue #673](https://github.com/github/copilot-cli/issues/673) |
| # Perform normalization | ||
| Write-Host "`nNormalizing line endings..." -ForegroundColor Yellow | ||
|
|
||
| if ($PSCmdlet.ShouldProcess("Repository files", "Normalize line endings to LF")) { |
There was a problem hiding this comment.
The -WhatIf parameter is declared via [CmdletBinding(SupportsShouldProcess)] but there's no confirmation prompt implemented. The ShouldProcess check at line 133 will always return true in normal execution and false with -WhatIf, but users might expect to see what would change before normalization occurs.
Consider adding output when -WhatIf is specified to show users what files would be modified, or remove the SupportsShouldProcess attribute if no confirmation/preview is intended.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7a4aaded5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| git rm --cached -r . 2>&1 | Out-Null | ||
|
|
||
| # Step 2: Re-add all files, applying new .gitattributes rules | ||
| Write-Host " [2/3] Re-adding files with normalized line endings..." -ForegroundColor Gray | ||
| git add --renormalize . 2>&1 | Out-Null |
There was a problem hiding this comment.
Add clean-tree guard before resetting index
The script unconditionally runs git rm --cached -r . followed by git add --renormalize ., which clears the index and then restages every tracked file. If a contributor runs this with any staged hunks or in-progress changes, their partial staging is destroyed and unrelated changes become staged, making accidental commits likely. Consider aborting when git status --porcelain or git diff --cached is non-empty (or require an explicit -Force) before resetting the index.
Useful? React with 👍 / 👎.
Summary
Comprehensive fix for line ending issues across all platforms (Windows/Linux/macOS).
Changes
Global LF enforcement in
.gitattributes:* text=auto eol=lfruleDeveloper setup documentation in
CONTRIBUTING.md:core.autocrlfinstructionsgit config --global core.autocrlf truegit config --global core.autocrlf inputNormalization script (
scripts/Normalize-LineEndings.ps1):Current State
Next Steps
After merging this PR, run:
Then commit the normalized files in a follow-up PR.
Rationale
Prevents YAML frontmatter parsing errors on Windows documented in:
Addresses feedback from @rjmurillo that PR #895 needs comprehensive line ending enforcement, not just pattern-specific rules.
Fixes #896
Generated with Claude Code) • View job run • [Branch: claude/issue-896-20260113-2126](https://github.com/rjmurillo/ai-agents/tree/claude/issue-896-20260113-2126