Skip to content

[FIX]: Force LF line endings for helpdesk mitigation.patch on Windows - #13

Merged
Nina Chikanov (nina-msft) merged 4 commits into
microsoft:mainfrom
sumit1kr:fix/crlf-patch-line-endings
Aug 5, 2026
Merged

[FIX]: Force LF line endings for helpdesk mitigation.patch on Windows#13
Nina Chikanov (nina-msft) merged 4 commits into
microsoft:mainfrom
sumit1kr:fix/crlf-patch-line-endings

Conversation

@sumit1kr

Copy link
Copy Markdown
Contributor

Closes #12

test_patch fails on Windows because git apply converts LF to CRLF,
causing a byte-level mismatch in the patch round-trip test.

Adding helpdesk-bot/mitigation.patch text eol=lf to .gitattributes
forces Git to preserve LF on all platforms, fixing the test on Windows
without affecting any other files.

@sumit1kr
Sumit Kumar (sumit1kr) requested a review from a team June 3, 2026 12:10

@nina-msft Nina Chikanov (nina-msft) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey Sumit Kumar (@sumit1kr) - sorry it took a while to get back to you! We are working on a better process to monitor both repos :-)

This PR title advertises a narrow fix, specifically for the tagged issue (#12) ...but the code changes ship an entirely new langgraph-rag-poisoning showcase plus the change.

If you would like to contribute the new example, please separate from this branch and open a new PR with a [FEAT] tag. Once that is done and this PR is refocused, feel free to tag me and I'll take another look.

Thanks!

@sumit1kr
Sumit Kumar (sumit1kr) force-pushed the fix/crlf-patch-line-endings branch from 4693f3b to bfb9ae2 Compare July 22, 2026 04:36
Copilot AI review requested due to automatic review settings July 22, 2026 04:36

Copilot AI 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

This PR addresses a Windows-only test failure where mitigation.patch is checked out with CRLF (due to Git autocrlf), causing a byte-level mismatch in the patch round-trip test. It updates Git attributes so the patch’s line endings remain LF across platforms.

Changes:

  • Updates .gitattributes to force LF line endings for helpdesk-bot/mitigation.patch.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .gitattributes Outdated
@sumit1kr

Copy link
Copy Markdown
Contributor Author

Hi Nina Chikanov (@nina-msft) — done! The langgraph-rag-poisoning showcase has been moved to a separate PR: #15. This PR now only contains the .gitattributes fix for #12. Also addressed Copilot's review — restored the general *.patch -text -eol rule so only helpdesk-bot/mitigation.patch gets the LF override. Please re-review when ready!

@nina-msft

Copy link
Copy Markdown
Contributor

Sumit Kumar (@sumit1kr) can you verify that this fix addresses the issue on Windows?

I asked GitHub Copilot to verify and it is asserting the error still stands:

assert post_reverse_bytes == original_bytes
E  At index 38 diff: b'\r' != b'\n'

Here is the provided rationale:

Here's why. The problem is in a different file (agent.py), not mitigation.patch. The test creates a brand-new temporary Git repo to do its work — and that temp repo does not have a .gitattributes file. So the .gitattributes line you added never gets used by the test.

Inside that temp repo, Git has a setting (core.autocrlf=true, which is normal on Windows) that quietly changes agent.py's line endings from LF to CRLF when it applies the patch. That change is what makes the test fail.

(A normal person cloning this repo won't hit this bug, because the real repo already forces LF. Only the empty temp repo in the test has the problem.)

How to fix it. The fix needs to go in the test file (tests/helpdesk/test_patch.py), not .gitattributes. Make the temp repo behave like the real one. I tested two ways that both work:

  • Add a .gitattributes file (with * text=auto eol=lf) into the temp repo before the test runs its Git commands, or

  • Turn off the line-ending setting in the temp repo: git("config", "core.autocrlf", "false")

⚠️ One warning: doing git -c core.autocrlf=false init does not work. That setting only lasts for that one command, and the test runs each Git command separately, so the patch step still uses the old setting. I tried it — it still fails.

Small note: the line you added is missing a newline at the end of the file. A local pre-commit run will complain about it (though CI won't, since CI doesn't run pre-commit).

@sumit1kr

Copy link
Copy Markdown
Contributor Author

Hi Nina Chikanov (@nina-msft),

Thank you for catching that! You were completely right — the previous fix modifying .gitattributes only applied to the outer repository. The pytest execution builds and runs the patch check in a sandbox temporary repository (tmp_path / "demo") outside the project root, meaning it never read the .gitattributes rules and still fell back to the system/global core.autocrlf setting (which defaults to true on Windows).

To fix this properly, I went with Option B:- Added git("config", "core.autocrlf", "false") immediately after git init in the temp repo inside test_patch.py. This ensures no CRLF conversion occurs during the test runtime on Windows.

  • Also verified .gitattributes terminates with a trailing newline.

Let me know if this looks good to go!

@nina-msft Nina Chikanov (nina-msft) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing this!

@nina-msft
Nina Chikanov (nina-msft) merged commit 4627516 into microsoft:main Aug 5, 2026
5 checks passed
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.

test_patch fails on Windows due to CRLF line ending mismatch

3 participants