Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/doctor-worktree-eol-check.md
Original file line number Diff line number Diff line change
@@ -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.
30 changes: 30 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 -- <path>` 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
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
99 changes: 99 additions & 0 deletions packages/squad-cli/src/cli/commands/doctor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/<eol>` `w/<eol>` `attr/<value>` TAB `<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 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 ──────────────────────────────────────────────────────

/**
Expand Down Expand Up @@ -649,6 +743,11 @@ export async function runDoctor(cwd?: string): Promise<DoctorCheck[]> {
// 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;
}

Expand Down
77 changes: 74 additions & 3 deletions scripts/check-shebang-eol.mjs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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/<eol>` `w/<eol>` `attr/<value>` 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.
Expand Down
126 changes: 126 additions & 0 deletions scripts/fix-crlf-worktree.mjs
Original file line number Diff line number Diff line change
@@ -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 -- <path>` 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();
}
Loading
Loading