Skip to content

check-shebang-eol's remediation message repairs the index only — and lands the developer at git status clean with a still-CRLF file #1809

Open

Description

@bradygaster

Follow-up from Flight's ⚠️ gate on #1805. The gate's detection is sound — this is the remediation text only. But it is worse than "incomplete": the advised sequence lands the developer in a state where git status reports clean while the file on disk is still CRLF, so the original defect (dead suite, broken shebang) survives with no remaining signal.

Current text (scripts/check-shebang-eol.mjs, merged dev)

console.error(`  ${unpinned.length > 0 ? '2' : '1'}. Run: git add --renormalize -- <path>   (a rule alone does not rewrite committed blobs)`);

That repairs the index. It does not repair the working tree. Result: i/lf w/crlf — the #1793 condition.

Measured, not reasoned

Scratch repos reproducing the exact production condition (CRLF blob committed with no rule, then *.js text eol=lf added in a second commit, pre-existing working tree). Blob state read as raw bytes in Node — never pipe git cat-file through PowerShell, it re-encodes line endings and gives a confidently wrong answer.

# Sequence HEAD index worktree git status
A git add --renormalize ← what we ship CRLF LF CRLF M partial
B A + git checkout -- <path> CRLF LF CRLF M silent no-op
C A + commit + git checkout -- <path> LF LF CRLF (clean) silently wrong
H A + commit + git checkout --force -- <path> LF LF CRLF (clean) --force does not force
E A + commit + delete file + git checkout LF LF LF (clean) ✅ repaired
F A + commit + git rm --cached -r . + git reset --hard LF LF LF (clean) ✅ repaired
G A + delete file + git checkout (no commit) CRLF LF LF M ✅ repaired, uncommitted

Three findings, in increasing order of nastiness

  1. git checkout -- <path> is a no-op here (row B). It looks like the obvious second step. Git skips the write because the stat cache says the file is current.
  2. --force does not force it either (row H). This is the one nobody would guess, and it is the natural escalation after B fails.
  3. Once the blob is LF, git status is structurally incapable of reporting a CRLF worktree (rows C/H). The clean filter normalizes CRLF→LF on the way in, so git compares LF-to-LF and sees no difference. The remediation we ship converts a visible, permanently-dirty file into an invisible broken one. That is a strict downgrade — the churn was annoying but at least it was a signal.

Only deleting the file first works. That is the single non-obvious step, and it is absent from the message.

Proposed replacement

console.error(`  ${unpinned.length > 0 ? '2' : '1'}. Repair the committed blob:`);
console.error('       git add --renormalize -- <path>');
console.error(`  ${unpinned.length > 0 ? '3' : '2'}. Force the working tree to be rewritten -- the step above fixes`);
console.error('     the index only, and the working tree keeps its CRLF copy:');
console.error('       rm <path> && git checkout -- <path>');
console.error('     Deleting first is REQUIRED. `git checkout` alone is a silent no-op (stat');
console.error('     cache), and `--force` does not override it. Once the blob is LF, `git');
console.error('     status` reports clean whether the working tree is CRLF or not -- the clean');
console.error('     filter normalizes on read -- so skipping this step leaves a broken file');
console.error('     with no remaining signal. Verify with a byte check, not `git status`.');

Verification that is actually valid (PowerShell-safe):

node -e "const b=require('fs').readFileSync(process.argv[1]);console.log([...b].filter((c,i)=>c===10&&b[i-1]===13).length+' CRLF')" <path>

Why this belongs on the record beyond the text fix

This is tooling reproducing the defect it was built to catch — third occurrence in this family (the CRLF-shebang lint authored with CRLF; check-shebang-eol's enumeration blind to its own churn class, #1804; now its remediation manufacturing the invisible variant). It is also the sixth instance of a silent no-op masquerading as success in this area, alongside --renormalize staging nothing for the LF-blob/CRLF-worktree mirror case (*.snap).

The generalization worth keeping: for text eol=lf files, git status is not a valid oracle for working-tree line endings in either direction. Any runbook step, gate message, or repair block in this repo that verifies with git status is verifying nothing. Byte-count or blob-SHA only.

Related: #1793 (same condition, reached a different way), #1804, #1805, #1790.

Activity

  1. added
    squadSquad triage inbox — Lead will assign to a member
    on Aug 21, 2026
  2. github-actions commented on Aug 21, 2026

    @github-actions
    Contributor

    📋 Assigned to Booster (CI/CD Engineer)

    Issue: #1809 — check-shebang-eol's remediation message repairs the index only — and lands the developer at git status clean with a still-CRLF file

    Booster will pick this up in the next Copilot session.

    For Copilot coding agent: If enabled, this issue will be worked automatically.
    Otherwise, start a Copilot session and say:
    Booster, work on issue #1809

  3. bradygaster commented on Aug 21, 2026

    @bradygaster
    CollaboratorAuthor

    Scale of the invisible class, measured on a real checkout

    Follow-up to the point above that git status cannot see a CRLF working tree once the blob is LF. I measured how many files are actually in that state right now, on this worktree, immediately after tonight''s merges.

    tracked files:                 1811
    pinned eol=lf:                  173   <- the repair surface
    WORKING-TREE files still CRLF:   34
      with a shebang (at risk):      16
      no shebang (cosmetic drift):   18
    

    At the same moment:

    Oracle Says
    git status --porcelain clean (empty)
    check-shebang-eol.mjs pass (index-only by design)
    mjs-shebang-loadable + check-shebang-eol suites 29/29 passed

    Every oracle we have reports success while 34 files are in the bad state, 16 of them shebanged — including cli.js and index.cjs, which are shipped entrypoints. A publish from a working tree in this state ships CRLF shebangs.

    Two of the 34 are packages/squad-cli/src/remote-ui/app.js and docs/src/pages/rss.xml.js — the exact two files #1805 renormalized tonight. The blob fix landed correctly and the working tree kept its CRLF copy, which is this issue''s failure mode reproducing on the change that introduced the rule. Consistent with the probe table: renormalize repairs the index, and nothing repairs the working tree.

    The actual gap

    check-shebang-eol.mjs reads the index — deliberately, and that is right for a CI gate, since a fresh CI checkout has index == HEAD == correct. But it means nothing in this repo checks working-tree EOL state, and git status structurally cannot. That is why 34 files accumulated silently.

    Working detector, ~40 lines, exits non-zero (this is what produced the numbers above):

    const tracked = git('ls-files', '-z').split('\0').filter(Boolean);
    // batch via stdin -- ~1800 paths as argv exceeds Windows' 32767 limit
    const out = execFileSync('git', ['check-attr', '--stdin', '-z', 'eol'],
      { input: Buffer.from(tracked.join('\0')), encoding: 'buffer' }).toString();
    const f = out.split('\0'), pinned = [];
    for (let i = 0; i + 2 < f.length; i += 3) if (f[i + 2] === 'lf') pinned.push(f[i]);
    const dirty = pinned.filter(p => {
      const b = readFileSync(p);                       // raw bytes, no shell
      for (let i = 1; i < b.length; i++) if (b[i] === 0x0a && b[i - 1] === 0x0d) return true;
      return false;
    });

    Suggested shape: a --worktree flag on the existing gate. Local/pre-commit only, never in CI — a fresh CI checkout is correct by construction, so a CI-side worktree check would only ever produce false confidence.

    Consequence for how the repair gets verified

    Any runbook confirming an EOL repair with git status, or with a test suite, is sampling. The suites cover 26 of 173 pinned files; the 34 currently-CRLF files include zero scripts/*.mjs, which is exactly why they pass. A suite-based check is valid for the narrow claim "the dead suites load again" and invalid for "the repair worked."

  4. bradygaster commented on Aug 21, 2026

    @bradygaster
    CollaboratorAuthor

    Correction to my previous comment — git ships the oracle already

    I wrote that "nothing in this repo checks working-tree EOL state, and git status structurally cannot" and supplied a 40-line byte detector. The first half of that is misleading and the detector is redundant. git status is genuinely blind — that part is verified and stands. But git ls-files --eol reads the working tree directly and reports it accurately:

    i/lf    w/crlf  attr/text eol=lf    cli.js
    i/lf    w/mixed attr/text eol=lf    docs/src/pages/rss.xml.js
    i/lf    w/crlf  attr/text eol=lf    index.cjs
    i/lf    w/crlf  attr/text eol=lf    packages/squad-cli/src/remote-ui/app.js
    i/lf    w/lf    attr/text eol=lf    scripts/check-changeset-drift.mjs
    

    Ground truth by raw byte count on the same five files: CRLF(13), CRLF(1), CRLF(1937), CRLF(334), LF. Every field matches, including w/mixed for the file with exactly one CRLF line.

    At full-repo scale the two methods agree exactly:

    Method Count
    git ls-files --eol filtered to attr/text eol=lf + w/(crlf|mixed) 34
    independent byte-level scan of all 173 pinned files 34

    So the complete detector is one command, not forty lines:

    git ls-files --eol | grep 'attr/text eol=lf' | grep -E 'w/(crlf|mixed)'

    w/mixed must be included — docs/src/pages/rss.xml.js has a single CRLF line and reports mixed, not crlf. Filtering on w/crlf alone silently misses it, which is the same class of near-miss this issue is about.

    What this changes

    • The --worktree flag becomes trivial — parse one git ls-files --eol call. No byte reading, no check-attr batching, and none of the Windows argv-limit handling that needed. Still local/pre-commit only; a fresh CI checkout is correct by construction.
    • Repair can be targeted and, more importantly, verified. Enumerate → rm → git checkout -- → re-run the same command → expect empty. That is a real post-condition, which neither git status nor a test suite can provide.
    • My framing was wrong in a way worth naming. The problem was never a missing instrument. Git has shipped this since 1.7.2. The problem is that everyone reached for git status, which cannot answer the question, and nobody checked whether a better instrument existed. That is a worse finding than "there is a gap," and a more useful one.

    The byte detector above is still fine as an independent cross-check — it is how I caught this — but it should not be what gets built.

  5. bradygaster commented on Aug 21, 2026

    @bradygaster
    CollaboratorAuthor

    Validated against the real specimen, not a scratch repo

    Three things confirmed on this branch''s 34-file working tree.

    1. The repair procedure works on a real file

    BEFORE   i/lf  w/crlf  attr/text eol=lf   .squad/hooks/tests/run-tests.ps1
             bytes=6731  CRLF=200
    
             rm <path> && git checkout -- <path>
    
    AFTER    i/lf  w/lf    attr/text eol=lf   .squad/hooks/tests/run-tests.ps1
             bytes=6531  CRLF=0
    

    Byte delta is exactly 200 — one byte per CR removed. git status reported clean both before and after, which is the blindness demonstrated end to end on a real file rather than a fixture.

    2. The blindness is structural, not a stale stat cache — verified

    I suspected the clean git status might just be git skipping the comparison on a cached stat. It isn't. Touching cli.js''s mtime with byte-identical content forces git to re-read and fully compare — and it still reports clean:

    untouched          -> (clean)
    mtime touched      -> (clean)     <- git re-read it and still cannot see the CRLF
    mtime restored     -> (clean)
    bytes=558  CRLF=13 <- unchanged throughout
    

    The clean filter normalizes CRLF→LF on read, so git compares LF to LF. The original mechanism claim in this issue stands, now with a controlled test behind it rather than an inference.

    3. New gotcha — git status can report M with an empty git diff

    Rewriting a file''s line endings in place produced:

    git status --porcelain   ->  M .squad/hooks/tests/run-tests.ps1
    git diff --stat          ->  (no diff -- only "CRLF will be replaced by LF" warning)
    git diff                 ->  (no content lines at all)
    

    Status and diff disagree, and diff is right. This is index staleness, not a change. git update-index --refresh clears it with zero content modification — verified byte-identical afterwards (6731 / CRLF=200 / w/crlf / clean).

    Worth knowing for anyone running the repair: if you see M with an empty diff, do not git add it. Refresh the index instead. Staging it would commit a line-ending flip that no diff was showing you. The repair sequence itself does not produce this state — only in-place rewriting does.

    The specimen is reproducible, so it does not need protecting

    I destroyed and rebuilt one of the 34 in place and restored it byte-identically. So this branch is not a precious artifact and no "do not run the sweep here" convention is needed — a social protection that would fail silently anyway.

    // worktree only; blob and index untouched
    const b = fs.readFileSync(p);
    const s = b.toString('binary').replace(/\r\n/g, '\n').replace(/\n/g, '\r\n');
    fs.writeFileSync(p, Buffer.from(s, 'binary'));
    // then: git update-index --refresh <path>

    Post-condition after rebuild: git ls-files --eol reports w/crlf, git status reports clean, repo-wide count returns to 34. Anyone implementing --worktree can recreate the state from scratch rather than depending on this checkout surviving.

  6. github-actions commented on Sep 10, 2026

    @github-actions
    Contributor

    🔄 Ralph — Auto-Triage

    Assigned to: INCO
    Reason: Matched routing keyword(s): copy
    Source: routing-rule

    Ralph auto-triaged this issue using routing rules.
    To reassign, swap the squad:* label.

  7. github-actions commented on Sep 10, 2026

    @github-actions
    Contributor

    🔄 Ralph — Auto-Triage

    Assigned to: INCO
    Reason: Matched routing keyword(s): copy
    Source: routing-rule

    Ralph auto-triaged this issue using routing rules.
    To reassign, swap the squad:* label.

  8. github-actions commented on Sep 11, 2026

    @github-actions
    Contributor

    🔄 Ralph — Auto-Triage

    Assigned to: INCO
    Reason: Matched routing keyword(s): copy
    Source: routing-rule

    Ralph auto-triaged this issue using routing rules.
    To reassign, swap the squad:* label.

  9. github-actions commented on Sep 11, 2026

    @github-actions
    Contributor

    🔄 Ralph — Auto-Triage

    Assigned to: INCO
    Reason: Matched routing keyword(s): copy
    Source: routing-rule

    Ralph auto-triaged this issue using routing rules.
    To reassign, swap the squad:* label.

  10. github-actions commented on Sep 13, 2026

    @github-actions
    Contributor

    🔄 Ralph — Auto-Triage

    Assigned to: architect
    Reason: No module, routing, or role keyword match — routed to Lead/Architect
    Source: lead-fallback

    Ralph auto-triaged this issue using routing rules.
    To reassign, swap the squad:* label.

  11. added
    squad:release-engineerAssigned to release-engineer (Release Engineer)
    and removed
    squad:architectAssigned to architect (Lead Architect)
    on Sep 24, 2026
  12. github-actions commented on Sep 24, 2026

    @github-actions
    Contributor

    📋 Assigned to release-engineer (Release Engineer)

    Issue: #1809 — check-shebang-eol's remediation message repairs the index only — and lands the developer at git status clean with a still-CRLF file

    release-engineer will pick this up in the next Copilot session.

    For Copilot coding agent: If enabled, this issue will be worked automatically.
    Otherwise, start a Copilot session and say:
    release-engineer, work on issue #1809

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    squadSquad triage inbox — Lead will assign to a membersquad:release-engineerAssigned to release-engineer (Release Engineer)wave1:P7Wave 1 PR group P7wave:1-nextWave 1 — immediate next sprint

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions