From f294111b47f8cd55ece42d6ec1958068ee43c7a6 Mon Sep 17 00:00:00 2001 From: brady gaster Date: Sat, 22 Aug 2026 17:27:22 -0700 Subject: [PATCH] fix: repair CRLF working trees that .gitattributes cannot reach (#1793) `.gitattributes` governs checkout, not files already on disk. #1790 added `*.mjs text eol=lf`, but git only rewrites a working file when a pull also changes that path's index content. Those files are already stored LF, so the merge is a no-op and every pre-#1790 Windows checkout stays CRLF forever. The failure is silent: a CRLF shebang survives Vite's shebang stripping as a bare `#`, the module fails to parse, and the suite reports `no tests` -- a zero that reads as green (#1788). - `npm run fix:crlf` repairs a tree from the index with `git checkout-index -f`. Gated on "no content difference", so a file with real uncommitted edits is skipped and reported, never overwritten. Re-measures after writing rather than trusting the write. - `squad doctor` gains a `working tree line endings` check that flags any eol=lf-pinned file still CRLF on disk, names them, and cites the fix. - Detection lives in `scripts/check-shebang-eol.mjs`, which already owns this invariant family, rather than a third parallel implementation. Deliberately not a `git add --renormalize .`: that rewrites the index -- the opposite side of the defect -- and would sweep nearly every CRLF-storing `.ts` blob into one churn commit, an exclusion `.gitattributes` documents on purpose. And deliberately not a CI gate: CI always has a fresh checkout, so a working-tree assertion there could never observe the failure it exists to catch. Verified capable of failing: with the three files from #1793 forced to CRLF, doctor reports `3 of 174 ... still have CRLF on disk` and the suites collapse from 29 tests to 6. After `npm run fix:crlf`, doctor passes and all 29 run. Review follow-ups (FIDO on #1831): - The check covers every eol=lf-pinned path, but both remediation hints printed `git ls-files --eol "*.mjs"`. That verification cannot observe a pinned non-.mjs file left CRLF -- the same defect class this PR fixes. Widened both hints to the real scope; a test now asserts the message does not re-narrow. - Added spaced-path coverage for `checkWorktreeEol`, asserting it *names* the file rather than merely failing. Its record parsing is a separate implementation from `listWorktreeCrlf` and could otherwise silently diverge. - The batching comment claimed an argv-length guarantee it does not provide; it batches by file count. Reworded to state the actual bound and the math. - Stopped pinning the CRLF-storing `.ts` blob count, which drifts with the tree. Closes #1793 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .changeset/doctor-worktree-eol-check.md | 5 + CONTRIBUTING.md | 30 +++ package.json | 1 + packages/squad-cli/src/cli/commands/doctor.ts | 99 ++++++++ scripts/check-shebang-eol.mjs | 77 ++++++- scripts/fix-crlf-worktree.mjs | 126 +++++++++++ test/scripts/crlf-worktree-repair.test.ts | 213 ++++++++++++++++++ 7 files changed, 548 insertions(+), 3 deletions(-) create mode 100644 .changeset/doctor-worktree-eol-check.md create mode 100644 scripts/fix-crlf-worktree.mjs create mode 100644 test/scripts/crlf-worktree-repair.test.ts diff --git a/.changeset/doctor-worktree-eol-check.md b/.changeset/doctor-worktree-eol-check.md new file mode 100644 index 000000000..f3c6e3aad --- /dev/null +++ b/.changeset/doctor-worktree-eol-check.md @@ -0,0 +1,5 @@ +--- +'@bradygaster/squad-cli': patch +--- + +`squad doctor` now checks that every file pinned to `eol=lf` by `.gitattributes` is actually LF **on disk**, not just in the index. `.gitattributes` governs checkout, so adding an `eol=lf` rule never repairs a working tree that already exists — the affected files stay CRLF indefinitely, and a CRLF shebang makes a vitest suite load zero tests while still looking green. The check names the stale files and points at `npm run fix:crlf`, a new repair script that rewrites them from the index and refuses to overwrite any file with uncommitted changes. Closes #1793. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ffdac417f..70d964960 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -241,6 +241,36 @@ Keep every change as small and focused as the task requires. Incidental formatti - **Separate genuine reformats:** if a file genuinely needs reformatting, do it in a dedicated PR so it can be reviewed independently of functional changes. - **Sanity-check the diff before pushing:** run `git diff --stat` / `git diff --numstat`. If a small change shows a whole-file delta, investigate (usually whitespace or line endings) before committing. +### Repairing a working tree that predates a `.gitattributes` rule + +`.gitattributes` governs **checkout**, not files already on disk. Git only rewrites a working file when a pull also changes that file's *index* content — so adding a rule like `*.mjs text eol=lf` leaves every already-LF-in-index path **still CRLF on disk** in checkouts that already exist. Pulling the fix does not repair your tree (#1793). + +This does not present as an error. A CRLF shebang survives Vite's shebang stripping as a bare `#`, the module fails to parse, and every vitest suite importing it reports **`no tests`** — a zero that reads as green (#1788). + +If `squad doctor` reports `working tree line endings — N ... still have CRLF on disk`, run: + +```bash +npm run fix:crlf +``` + +Then confirm — `squad doctor` re-runs the same check the repair is scoped to: + +```bash +squad doctor +``` + +To inspect the raw state, list every tracked path and its line endings: + +```bash +git ls-files --eol +``` + +Every entry whose `attr` includes `eol=lf` should read `w/lf`. Don't scope that to `"*.mjs"` — `.mjs` is simply where the symptom is loudest, but the pin (and the doctor check) covers every path `.gitattributes` pins, so an `*.mjs` filter can report all-clear while a pinned file elsewhere is still CRLF. + +The repair rewrites the affected files from the index with `git checkout-index -f`. It is safe: it only touches paths git reports as having **no content difference** from the index, and it skips (never overwrites) any file with real uncommitted edits. `git checkout -- ` is *not* a substitute — in this state the file is content-clean, so git has nothing to restore and the command can silently no-op. + +Do **not** fix this with `git add --renormalize .`. That rewrites the index — the opposite side of the defect — and in this repo it would sweep nearly every CRLF-storing `.ts` blob into a single line-ending churn commit. `.gitattributes` documents that exclusion deliberately. This is a local repair that should produce no commit at all. + ## Documentation - **README.md** — User-facing guide, quick start, architecture overview diff --git a/package.json b/package.json index b78c1cb0a..35eafccfb 100644 --- a/package.json +++ b/package.json @@ -26,6 +26,7 @@ "experiment:real-cli-ab": "node scripts/real-cli-ab.js", "bench:cold-start": "node scripts/measure-cold-start.mjs", "lint": "tsc --noEmit -p packages/squad-sdk/tsconfig.json && tsc --noEmit -p packages/squad-cli/tsconfig.json", + "fix:crlf": "node scripts/fix-crlf-worktree.mjs", "lint:eslint": "eslint packages/ test/", "lint:docs": "markdownlint-cli2 && cspell --no-progress --dot \"docs/src/content/**/*.md\" \"README.md\"", "dev:link": "npm run build && cd packages/squad-sdk && npm link && cd ../squad-cli && npm link", diff --git a/packages/squad-cli/src/cli/commands/doctor.ts b/packages/squad-cli/src/cli/commands/doctor.ts index 44916d142..3e0b0bbde 100644 --- a/packages/squad-cli/src/cli/commands/doctor.ts +++ b/packages/squad-cli/src/cli/commands/doctor.ts @@ -592,6 +592,100 @@ export function checkGitSyncHooks(cwd: string, squadDir: string): DoctorCheck | }; } +// ── working-tree EOL check ─────────────────────────────────────────── + +/** Repair command surfaced to the developer when the check fails. */ +const CRLF_FIX_COMMAND = 'npm run fix:crlf'; + +/** + * Parse one `git ls-files --eol -z` record. + * + * Shape is `i/` `w/` `attr/` TAB ``. The first three + * fields are space-padded to fixed columns and the attr value itself contains + * a space ("text eol=lf"), so the path is everything after the first TAB and + * the field block is split on whitespace runs rather than by column. + */ +function parseEolRecord(record: string): { worktree: string; attr: string; file: string } | undefined { + const tab = record.indexOf('\t'); + if (tab === -1) return undefined; + const match = /^i\/(\S*)\s+w\/(\S*)\s+attr\/(.*)$/.exec(record.slice(0, tab)); + if (!match) return undefined; + return { worktree: match[2] ?? '', attr: (match[3] ?? '').trim(), file: record.slice(tab + 1) }; +} + +/** + * Check that every LF-pinned file is actually LF *on disk*. + * + * `.gitattributes` governs checkout, not files already on disk: git only + * re-smudges a working file when the pull also changes that file's index + * content. So adding an `eol=lf` rule leaves every already-LF-in-index path + * still CRLF on disk in existing Windows checkouts, indefinitely (#1793). + * The symptom is not an error — a CRLF shebang survives Vite's shebang + * stripping as a bare `#`, the module fails to parse, and the vitest suite + * importing it reports "no tests". A green-looking zero (#1788). + * + * DELIBERATELY A SEPARATE IMPLEMENTATION FROM scripts/check-shebang-eol.mjs, + * which owns the same invariant family for repo tooling. That script is a repo + * script and is not published inside this package, so `dist/` cannot import it + * without breaking every installed copy of the CLI. Do not "deduplicate" these + * by adding an import across that boundary. What they must keep in sync is the + * record-parsing shape above, which is pinned by tests on both sides. + * + * Returns undefined when the check does not apply (not a git repo, git absent, + * or no LF-pinned files at all), matching the other conditional checks. + */ +export function checkWorktreeEol(cwd: string): DoctorCheck | undefined { + let records: string[]; + try { + records = execFileSync('git', ['ls-files', '--eol', '-z'], { + cwd, + encoding: 'utf-8', + maxBuffer: 1 << 28, + stdio: ['ignore', 'pipe', 'pipe'], + }) + .split('\0') + .filter(Boolean); + } catch { + return undefined; // not a git repo, or git unavailable — not applicable + } + + const stale: string[] = []; + let pinned = 0; + for (const record of records) { + const parsed = parseEolRecord(record); + if (!parsed) continue; + if (!/(^|\s)eol=lf(\s|$)/.test(parsed.attr)) continue; + pinned += 1; + // `mixed` is the same defect partially applied: under an eol=lf pin any CR + // in the working file is wrong. + if (parsed.worktree === 'crlf' || parsed.worktree === 'mixed') stale.push(parsed.file); + } + + if (pinned === 0) return undefined; // no eol=lf rules — nothing to assert + + if (stale.length > 0) { + const sample = stale.slice(0, 5).join(', '); + const more = stale.length > 5 ? `, +${stale.length - 5} more` : ''; + return { + name: 'working tree line endings', + status: 'fail', + message: + `${stale.length} of ${pinned} LF-pinned file(s) still have CRLF on disk (${sample}${more}). ` + + `A .gitattributes eol=lf rule does not rewrite files that were already checked out, so pulling the ` + + `fix does not repair an existing checkout. A CRLF shebang makes a vitest suite silently load zero ` + + `tests. Run '${CRLF_FIX_COMMAND}' to repair, then re-run 'squad doctor' to confirm. To inspect ` + + `the raw state: 'git ls-files --eol' — every entry whose attr includes eol=lf should read w/lf ` + + `(the pin is not limited to *.mjs, and neither is this check).`, + }; + } + + return { + name: 'working tree line endings', + status: 'pass', + message: `${pinned} LF-pinned file(s) all LF on disk`, + }; +} + // ── public API ────────────────────────────────────────────────────── /** @@ -649,6 +743,11 @@ export async function runDoctor(cwd?: string): Promise { // 13. Copilot CLI availability (needed by watch capabilities) checks.push(await checkCopilotCli()); + // 14. Working-tree line endings (#1793) — an eol=lf rule does not repair a + // checkout that predates it, and the failure mode is a silent zero-test run. + const worktreeEol = checkWorktreeEol(resolvedCwd); + if (worktreeEol) checks.push(worktreeEol); + return checks; } diff --git a/scripts/check-shebang-eol.mjs b/scripts/check-shebang-eol.mjs index 43f6b179f..83d7bda97 100644 --- a/scripts/check-shebang-eol.mjs +++ b/scripts/check-shebang-eol.mjs @@ -1,8 +1,14 @@ #!/usr/bin/env node -// check-shebang-eol.mjs -- Two EOL invariants over the git index. +// check-shebang-eol.mjs -- EOL invariants over a git repository. // -// UNPINNED Every tracked file starting with `#!` must be pinned to LF. -// CRLF-BLOB Every file pinned to LF must actually store an LF blob. +// UNPINNED Every tracked file starting with `#!` must be pinned to LF. +// CRLF-BLOB Every file pinned to LF must actually store an LF blob. +// WORKTREE-CRLF Every file pinned to LF must also BE LF on disk. (#1793) +// +// The first two read the index and are enforced by main() as a CI gate. The +// third reads the working tree, is local-only by nature, and is exported for +// `squad doctor` / scripts/fix-crlf-worktree.mjs rather than gated here -- see +// listWorktreeCrlf below for why putting it in CI would be a no-op gate. // // A CRLF shebang is never correct. The trailing \r becomes part of the // interpreter argument on POSIX ("env: node\r: No such file or directory"), and @@ -159,6 +165,71 @@ export function eolAttributes(cwd, files) { return map; } +/** + * Parse one `git ls-files --eol -z` record into `{ index, worktree, attr, file }`. + * + * Record shape is `i/` `w/` `attr/` then a TAB then the path; + * the first three fields are space-padded to fixed columns and the attr value + * itself contains a space ("text eol=lf"), so the path is everything after the + * first TAB and the field block is split by whitespace runs, not by column. + */ +function parseEolRecord(record) { + const tab = record.indexOf('\t'); + if (tab === -1) return undefined; + const match = /^i\/(\S*)\s+w\/(\S*)\s+attr\/(.*)$/.exec(record.slice(0, tab)); + if (!match) return undefined; + return { index: match[1], worktree: match[2], attr: match[3].trim(), file: record.slice(tab + 1) }; +} + +/** + * WORKTREE-CRLF -- paths pinned to LF whose file ON DISK still has CRLF. + * + * THIS IS A THIRD INVARIANT, AND IT IS NOT REDUNDANT WITH THE TWO ABOVE. + * UNPINNED and CRLF-BLOB both read the *index*. This one reads the *working + * tree*, and the gap between them is exactly #1793: `.gitattributes` governs + * checkout, not files already on disk. Adding `*.mjs text eol=lf` (#1790) only + * rewrites a working file when the merge also changes that file's index + * content. For a path already stored LF, the index does not move, so git never + * re-smudges it and it stays CRLF on disk forever. Vite's shebang stripping + * then leaves a bare `#`, the module fails to parse, and the importing vitest + * suite reports "no tests" -- a green-looking zero (#1788). + * + * DELIBERATELY NOT WIRED INTO main() BELOW. This lint runs in CI, where the + * checkout is always fresh, so a working-tree assertion there could never + * observe the failure it is meant to catch -- a permanently green gate is + * equivalent to no gate. The condition is local-only by nature, so it is + * surfaced by `squad doctor` and repaired by scripts/fix-crlf-worktree.mjs, + * both of which run on the developer's actual disk. + * + * `w/mixed` counts too: under an eol=lf pin, any CR in the working file is the + * same defect, just partially applied. + */ +export function listWorktreeCrlf(cwd) { + const out = git(['ls-files', '--eol', '-z'], cwd).split(NUL).filter(Boolean); + const found = []; + for (const record of out) { + const parsed = parseEolRecord(record); + if (!parsed) continue; + if (!/(^|\s)eol=lf(\s|$)/.test(parsed.attr)) continue; + if (parsed.worktree !== 'crlf' && parsed.worktree !== 'mixed') continue; + found.push(parsed); + } + return found; +} + +/** + * Paths whose working-tree content differs from the index in more than line + * endings. Git's checkin filter normalizes CRLF away before comparing, so a + * pure EOL mismatch produces NO entry here while a genuine edit does. + * + * This is the safety gate for any repair that overwrites from the index: + * `git checkout-index -f` is destructive to real uncommitted work, and this is + * what makes it safe to point at a path. + */ +export function listContentModified(cwd) { + return new Set(git(['diff', '--name-only', '-z'], cwd).split(NUL).filter(Boolean)); +} + /** * Pure decision step, separated so tests can drive it with synthetic input. * Returns one violation per problem, not per file, so a file can report both. diff --git a/scripts/fix-crlf-worktree.mjs b/scripts/fix-crlf-worktree.mjs new file mode 100644 index 000000000..333c44260 --- /dev/null +++ b/scripts/fix-crlf-worktree.mjs @@ -0,0 +1,126 @@ +#!/usr/bin/env node +// fix-crlf-worktree.mjs -- Repair a working tree that predates an eol=lf rule. +// +// THE PROBLEM THIS SOLVES (#1793) +// `.gitattributes` governs CHECKOUT, not files already on disk. When #1790 +// added `*.mjs text eol=lf`, git only rewrote the working file for paths whose +// INDEX content also changed. The paths already stored LF in the index did not +// move, so git never re-smudged them and they stayed CRLF on disk. Vite's +// shebang stripping leaves a bare `#` on such a file, the module fails to +// parse, and every vitest suite importing it reports "no tests" -- a zero that +// reads as green. Every Windows checkout created before the rule is in this +// state and nothing about pulling the fix repairs it. +// +// WHY `git checkout -- ` IS NOT THE ANSWER +// In this state the file is content-clean: git's checkin filter normalizes the +// CRLF away, so the cleaned blob equals the index blob exactly. `git checkout` +// has nothing to restore and can no-op, which is precisely what makes the +// condition so durable. `git checkout-index -f` writes from the index +// unconditionally and is the primitive that actually repairs it. +// +// WHY NOT `git add --renormalize .` +// That rewrites the INDEX, which is the opposite side of the defect, and in +// this repo it would sweep every CRLF-storing .ts blob -- most of them, and +// none of them ours -- into one line-ending churn commit. `.gitattributes` +// documents that exclusion deliberately. (It cites a specific count; treat +// that as illustrative, since it drifts with the tree.) This is a local +// working-tree repair; it must produce no commit at all. +// +// SAFETY +// `git checkout-index -f` overwrites the file on disk. It is only ever pointed +// at paths that git reports as having NO content difference from the index, so +// there is nothing to lose. Paths with real uncommitted edits are skipped and +// listed, never overwritten. +// +// Uses only Node.js built-ins (child_process, url). + +import { execFileSync } from 'node:child_process'; +import { pathToFileURL } from 'node:url'; + +import { listContentModified, listWorktreeCrlf } from './check-shebang-eol.mjs'; + +/** + * Rewrite the given paths from the index, forcing git past its "nothing to do" + * shortcut. + * + * Batched by FILE COUNT, which bounds argv length only indirectly. 200 paths + * sit well under the 32767-character Windows limit at this repo's path lengths + * (longest tracked path is ~80 chars, so a full batch is ~16K), but that is a + * property of the tree, not a guarantee: it would take a ~164-character mean + * path to overflow a batch. Deliberately not doing explicit length accounting + * -- the repair set is normally a handful of files, and a tree with 200 paths + * averaging 164 characters has larger problems. + */ +function checkoutIndex(cwd, files) { + const BATCH = 200; + for (let i = 0; i < files.length; i += BATCH) { + execFileSync('git', ['checkout-index', '-f', '--', ...files.slice(i, i + BATCH)], { + cwd, + stdio: ['ignore', 'pipe', 'pipe'], + }); + } +} + +/** + * Repair every LF-pinned path whose working file is CRLF. + * Returns what was repaired, what was skipped, and what survived the repair. + */ +export function repair(cwd) { + const stale = listWorktreeCrlf(cwd); + if (stale.length === 0) return { repaired: [], skipped: [], remaining: [] }; + + const modified = listContentModified(cwd); + const skipped = stale.map((e) => e.file).filter((f) => modified.has(f)); + const repairable = stale.map((e) => e.file).filter((f) => !modified.has(f)); + + checkoutIndex(cwd, repairable); + + // Re-measure rather than trusting the write: the whole class of bug this + // script exists for is a repair that silently did not take effect. + const stillStale = new Set(listWorktreeCrlf(cwd).map((e) => e.file)); + return { + repaired: repairable.filter((f) => !stillStale.has(f)), + skipped, + remaining: repairable.filter((f) => stillStale.has(f)), + }; +} + +function main() { + const cwd = process.cwd(); + const { repaired, skipped, remaining } = repair(cwd); + + if (repaired.length === 0 && skipped.length === 0 && remaining.length === 0) { + console.log('Working tree EOL check passed: no LF-pinned file has CRLF on disk.'); + process.exit(0); + } + + if (repaired.length > 0) { + console.log(`Repaired ${repaired.length} file(s) from CRLF to LF on disk:`); + for (const file of repaired) console.log(` ${file}`); + } + + if (skipped.length > 0) { + console.error(`\nSKIPPED ${skipped.length} file(s) with uncommitted changes -- not overwritten:`); + for (const file of skipped) console.error(` ${file}`); + console.error('\nCommit or stash these, then re-run. Their content would have been lost.'); + } + + if (remaining.length > 0) { + console.error(`\nSTILL CRLF after repair -- ${remaining.length} file(s):`); + for (const file of remaining) console.error(` ${file}`); + console.error('\nThis should not happen. Check that .gitattributes pins these paths to eol=lf.'); + } + + if (skipped.length > 0 || remaining.length > 0) process.exit(1); + + console.log( + '\nVerified: re-measured after the repair and no LF-pinned path reports CRLF on disk.\n' + + "To inspect the raw state: git ls-files --eol (every entry whose attr includes eol=lf\n" + + 'should read w/lf -- the pin is not limited to *.mjs).' + ); + process.exit(0); +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + main(); +} diff --git a/test/scripts/crlf-worktree-repair.test.ts b/test/scripts/crlf-worktree-repair.test.ts new file mode 100644 index 000000000..13cf9fce4 --- /dev/null +++ b/test/scripts/crlf-worktree-repair.test.ts @@ -0,0 +1,213 @@ +/** + * #1793 — a `.gitattributes` eol=lf rule does not repair a working tree that + * already exists. These tests drive real git repositories rather than synthetic + * fixtures, because the entire defect lives in git's own behaviour: the file is + * content-clean (the checkin filter normalizes the CRLF away, so the cleaned + * blob equals the index blob) while still being CRLF on disk. A hand-built + * fixture would prove the parser and nothing else. + * + * Each test that asserts a repair FIRST asserts the broken state, so a repair + * that silently no-ops cannot pass. + */ + +import { execFileSync } from 'node:child_process'; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; + +import { checkWorktreeEol } from '@bradygaster/squad-cli/commands/doctor'; +import { listContentModified, listWorktreeCrlf } from '../../scripts/check-shebang-eol.mjs'; +import { repair } from '../../scripts/fix-crlf-worktree.mjs'; + +const SHEBANG_SCRIPT = '#!/usr/bin/env node\nconsole.log("hi");\n'; + +const created: string[] = []; + +afterEach(() => { + while (created.length > 0) { + const dir = created.pop(); + if (dir) rmSync(dir, { recursive: true, force: true }); + } +}); + +/** + * A repo in the exact post-#1790 state: `*.mjs text eol=lf` is committed, the + * blob is stored LF, and the working file is CRLF. That combination is what + * every pre-#1790 Windows checkout looks like after pulling the fix. + */ +function makeRepo(files: Record = { 'tool.mjs': SHEBANG_SCRIPT }): string { + const dir = mkdtempSync(join(tmpdir(), 'crlf-worktree-')); + created.push(dir); + execFileSync('git', ['init', '-q'], { cwd: dir }); + execFileSync('git', ['config', 'user.email', 'test@example.com'], { cwd: dir }); + execFileSync('git', ['config', 'user.name', 'Test'], { cwd: dir }); + execFileSync('git', ['config', 'core.autocrlf', 'false'], { cwd: dir }); + + writeFileSync(join(dir, '.gitattributes'), '*.mjs text eol=lf\n'); + for (const [name, content] of Object.entries(files)) { + writeFileSync(join(dir, name), content); // LF — this is what the blob stores + } + execFileSync('git', ['add', '--', '.gitattributes', ...Object.keys(files)], { cwd: dir }); + execFileSync('git', ['commit', '-qm', 'init'], { cwd: dir }); + return dir; +} + +/** Rewrite a working file to CRLF without touching the index — the #1793 state. */ +function crlfOnDisk(dir: string, name: string): void { + const text = readFileSync(join(dir, name), 'utf8'); + writeFileSync(join(dir, name), text.replace(/\r?\n/g, '\r\n')); +} + +function eolOf(dir: string, name: string): string { + return execFileSync('git', ['ls-files', '--eol', '--', name], { cwd: dir, encoding: 'utf8' }); +} + +describe('#1793 detection — listWorktreeCrlf', () => { + it('finds nothing on a freshly checked-out tree', () => { + expect(listWorktreeCrlf(makeRepo())).toEqual([]); + }); + + it('flags an LF-pinned file that is CRLF on disk', () => { + const dir = makeRepo(); + crlfOnDisk(dir, 'tool.mjs'); + + // Guard the premise: git must consider this file content-clean. If it did + // not, the bug would be visible as an ordinary diff and need no detector. + expect(listContentModified(dir).has('tool.mjs')).toBe(false); + expect(eolOf(dir, 'tool.mjs')).toContain('w/crlf'); + + const found = listWorktreeCrlf(dir); + expect(found.map((e) => e.file)).toEqual(['tool.mjs']); + expect(found[0]?.index).toBe('lf'); // index already LF — this is why git never re-smudges + }); + + it('ignores a CRLF file that carries no eol=lf pin', () => { + const dir = makeRepo({ 'tool.mjs': SHEBANG_SCRIPT, 'notes.txt': 'a\nb\n' }); + crlfOnDisk(dir, 'notes.txt'); + expect(listWorktreeCrlf(dir)).toEqual([]); + }); + + it('parses paths containing spaces', () => { + const dir = makeRepo({ 'my tool.mjs': SHEBANG_SCRIPT }); + crlfOnDisk(dir, 'my tool.mjs'); + expect(listWorktreeCrlf(dir).map((e) => e.file)).toEqual(['my tool.mjs']); + }); +}); + +describe('#1793 doctor check — checkWorktreeEol', () => { + it('FAILS on a working tree that predates the eol=lf rule', () => { + const dir = makeRepo(); + crlfOnDisk(dir, 'tool.mjs'); + + const result = checkWorktreeEol(dir); + expect(result?.status).toBe('fail'); + expect(result?.message).toContain('tool.mjs'); + expect(result?.message).toContain('npm run fix:crlf'); + // The suggested verification must cover the same scope the check does. + // An `*.mjs`-scoped hint can report all-clear while a pinned file + // elsewhere is still CRLF — a verification that cannot observe the + // failure it exists to catch, which is this PR's own bug class. + expect(result?.message).not.toContain('--eol "*.mjs"'); + }); + + it('names a CRLF path containing spaces', () => { + // Same delimiter-sensitive record parsing as listWorktreeCrlf, but a + // separate implementation (see the note on checkWorktreeEol), so the two + // parsers can silently diverge unless both are pinned to this shape. + const dir = makeRepo({ 'my tool.mjs': SHEBANG_SCRIPT }); + crlfOnDisk(dir, 'my tool.mjs'); + + const result = checkWorktreeEol(dir); + expect(result?.status).toBe('fail'); + // Naming it, not merely failing: a parser that truncates at the space + // would still fail, just uselessly. + expect(result?.message).toContain('my tool.mjs'); + }); + + it('PASSES on the same repo once repaired — the check is not stuck on fail', () => { + const dir = makeRepo(); + crlfOnDisk(dir, 'tool.mjs'); + expect(checkWorktreeEol(dir)?.status).toBe('fail'); + + execFileSync('git', ['checkout-index', '-f', '--', 'tool.mjs'], { cwd: dir }); + + const result = checkWorktreeEol(dir); + expect(result?.status).toBe('pass'); + expect(result?.message).toContain('LF on disk'); + }); + + it('is not applicable outside a git repo', () => { + const dir = mkdtempSync(join(tmpdir(), 'crlf-nogit-')); + created.push(dir); + expect(checkWorktreeEol(dir)).toBeUndefined(); + }); + + it('is not applicable when the repo pins nothing to eol=lf', () => { + const dir = mkdtempSync(join(tmpdir(), 'crlf-nopin-')); + created.push(dir); + execFileSync('git', ['init', '-q'], { cwd: dir }); + execFileSync('git', ['config', 'user.email', 'test@example.com'], { cwd: dir }); + execFileSync('git', ['config', 'user.name', 'Test'], { cwd: dir }); + writeFileSync(join(dir, 'notes.txt'), 'a\n'); + execFileSync('git', ['add', '--', 'notes.txt'], { cwd: dir }); + execFileSync('git', ['commit', '-qm', 'init'], { cwd: dir }); + expect(checkWorktreeEol(dir)).toBeUndefined(); + }); +}); + +describe('#1793 repair — fix-crlf-worktree', () => { + it('rewrites the working file to LF and reports it', () => { + const dir = makeRepo(); + crlfOnDisk(dir, 'tool.mjs'); + expect(eolOf(dir, 'tool.mjs')).toContain('w/crlf'); + + const result = repair(dir); + + expect(result.repaired).toEqual(['tool.mjs']); + expect(result.skipped).toEqual([]); + expect(result.remaining).toEqual([]); + expect(eolOf(dir, 'tool.mjs')).toContain('w/lf'); + expect(readFileSync(join(dir, 'tool.mjs'), 'utf8')).not.toContain('\r'); + }); + + it('is a no-op on an already-healthy tree', () => { + expect(repair(makeRepo())).toEqual({ repaired: [], skipped: [], remaining: [] }); + }); + + it('NEVER overwrites a file with real uncommitted edits', () => { + const dir = makeRepo(); + // A genuine edit that also happens to use CRLF: the repair would clobber it. + writeFileSync(join(dir, 'tool.mjs'), '#!/usr/bin/env node\r\nconsole.log("PRECIOUS");\r\n'); + expect(listContentModified(dir).has('tool.mjs')).toBe(true); + + const result = repair(dir); + + expect(result.skipped).toEqual(['tool.mjs']); + expect(result.repaired).toEqual([]); + expect(readFileSync(join(dir, 'tool.mjs'), 'utf8')).toContain('PRECIOUS'); + }); + + it('repairs every stale file in one pass', () => { + const names = ['a.mjs', 'b.mjs', 'c.mjs']; + const dir = makeRepo(Object.fromEntries(names.map((n) => [n, SHEBANG_SCRIPT]))); + for (const name of names) crlfOnDisk(dir, name); + expect(listWorktreeCrlf(dir)).toHaveLength(3); + + expect(repair(dir).repaired.sort()).toEqual(names); + expect(listWorktreeCrlf(dir)).toEqual([]); + }); + + it('leaves a CRLF shebang parseable after repair — the actual #1788 symptom', () => { + const dir = makeRepo(); + crlfOnDisk(dir, 'tool.mjs'); + // Vite strips through the first LF; with CRLF that leaves a bare `#`. + const broken = readFileSync(join(dir, 'tool.mjs'), 'utf8'); + expect(broken.slice(0, broken.indexOf('\n') + 1)).toContain('\r'); + + repair(dir); + + const fixed = readFileSync(join(dir, 'tool.mjs'), 'utf8'); + expect(fixed.slice(0, fixed.indexOf('\n'))).toBe('#!/usr/bin/env node'); + }); +});