fix(windows): preserve a file's existing line endings on rewrite - #1
Merged
Merged
Conversation
The parsers normalize CRLF to LF on read, but nothing restored it on write. On a Windows checkout (core.autocrlf=true) that turned every rewrite into a whole-file change: applying a delta that added one requirement produced a diff of 21 insertions and 14 deletions, burying the real change. Archiving the same spec now writes 7 insertions and 0 deletions. - specs-apply: write an updated spec back with the convention the file already used; a spec that does not exist yet stays LF. - file-system: same fix for updateFileWithMarkers, so installing shell completions into a CRLF .bashrc/.zshrc no longer leaves mixed endings, which bash reports as "$'\r': command not found". - pack-version-check: spawn npm through cross-spawn, since execFile cannot resolve npm.cmd on Windows. Adds src/utils/line-endings.ts for the detect/restore pair, plus tests pinning the CRLF round trip through the real write paths. Also adds regression tests for path containment under Windows case variance: path.win32.relative already folds case, and those tests pin both halves of the contract so a future "case-insensitive" change cannot quietly loosen the traversal guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Brings the Windows line-ending fix into our fork's
main.Also submitted upstream as Fission-AI#1936 (tracking issue Fission-AI#1935). Merging here does not depend on upstream accepting it; if they do, this branch is the same commit and will reconcile cleanly.
What
The parsers normalize CRLF to LF on read, but nothing restored the file's convention on write. On a Windows checkout (
core.autocrlf=true) that turned every rewrite into a whole-file change.Archiving a change that adds one requirement to a CRLF spec, before and after:
The file also stays CRLF instead of being silently converted to LF.
Changes
specs-apply—writeUpdatedSpecwrites an updated spec back with the convention the file already used. A spec that does not exist yet stays LF.file-system— same fix forupdateFileWithMarkers, so installing or updating shell completions in a CRLF.bashrc/.zshrcno longer leaves mixed endings, whichbashreports as$'\r': command not found.pack-version-check— spawnsnpmthroughcross-spawn, sinceexecFilecannot resolvenpm.cmdon Windows.New
src/utils/line-endings.tsholds the detect/restore pair. The internal LF-only invariant is unchanged — only the write boundary differs.Tests
29 new tests covering the CRLF round trip through the real write paths, the utility itself (including the CRLF/LF tie and a lone
CR), marker insert/replace, and path containment under Windows case variance.pnpm build,pnpm test,pnpm exec tsc --noEmit,pnpm lintall pass locally.Two tests fail on
mainbefore this branch (artifact-workflow› Cursor skills,config-profile› in-process apply) and still fail after — pre-existing and unrelated.Verified, not assumed
openspec validatewas not broken: a CRLF copy of this repo's own 62-itemopenspec/tree produces byte-identical validate output.isPathWithinalready handles Windows case variance correctly. Both were left alone; the containment tests exist to keep it that way.Written with Claude Code (Claude Opus 5), verified by reproduction — the
21/14 → 7/0numbers are measured from an actualopenspec archiverun.