docs(e2e): make the renormalize selector attribute-driven, not glob-shaped - #1810
Merged
Conversation
…haped (#1809) The remediation shipped in #1808 selected `*.mjs`. That was correct against the known-affected set and wrong against the PINNED set, and Booster's #1809 measurement invalidated it about forty minutes after it merged. Measured on dev: 173 files carry `attr/text eol=lf`, and 26 of them were CRLF on disk. Only ~6 were .mjs. The old sweep would have repaired 6 of 26 and left 20 broken - including cli.js, index.cjs, lib/rework.cjs and every .ps1. That is worse than shipping no remediation at all. A partial repair produces a third distinct failure set matching neither the doc nor the card, which reads as "the remediation is unreliable" rather than "the selector was wrong" - it discredits the correct procedure. Same shape as converting a visibly-dirty file into an invisibly-broken one. The fix is to enumerate by attribute rather than by extension, so the command tracks .gitattributes automatically and cannot drift the next time a non-matching extension gets pinned. Verified end to end by executing the documented block verbatim: 26 -> 0 pinned files CRLF, tree clean, idempotent on a second run, and the two EOL suites go 13-failed / 0-tests to 26 passed. Structure follows the split we agreed: the doc keeps the WHY (the blind-oracle table, `--force` does not force, `git status` structurally cannot see it, the 26-test proof) because that argument is what makes someone actually run the step rather than skip it. The HOW is now stated exactly once here, and the card and runbook point at it instead of restating it - so selector drift between two copies becomes structurally impossible rather than merely fixed. Step 2 restores a scoped file list rather than `git checkout -- .`, which is prohibited by repo convention after it silently reverted in-progress work earlier tonight. Refs #1809, #1793. Supersedes the remediation block from #1808. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70370e36-33b0-4786-bd72-4cf15518daa6
Contributor
🛫 PR Readiness Check
PR Scope: 🔧 Infrastructure
|
| Status | Check | Details |
|---|---|---|
| ❌ | Single commit | 2 commits — consider squashing before review |
| ✅ | Not in draft | Ready for review |
| ✅ | Branch up to date | Up to date with dev |
| ❌ | Copilot review | No Copilot review yet — it may still be processing |
| ✅ | Changeset present | No source files changed — changeset not required |
| ✅ | Scope clean | |
| ✅ | No merge conflicts | No merge conflicts |
| ❌ | Copilot threads resolved | 1 unresolved Copilot thread(s) — fix and resolve before merging |
| ✅ | CI passing | All checks passing |
Files Changed (1 file, +63 −7)
| File | +/− |
|---|---|
.squad/e2e/windows-test-baseline.md |
+63 −7 |
Total: +63 −7
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
Contributor
🟢 Impact Analysis — PR #1810Risk tier: 🟢 LOW 📊 Summary
🎯 Risk Factors
📦 Modules Affectedsquad-state (1 file)
This report is generated automatically for every PR. See #733 for details. |
Contributor
|
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the Windows E2E baseline documentation to replace the previous glob-based renormalization remediation (*.mjs) with an attribute-driven, .gitattributes-aligned procedure, and consolidates the remediation into a single authoritative block to prevent future drift.
Changes:
- Replaces the remediation command sequence with an attribute-driven enumeration of
attr/text eol=lffiles and a scoped delete + re-checkout. - Adds rationale and measured proof for why glob-based selection is unreliable and why the sweep must be unconditional.
- Documents “oracle blindness” (why
git status,git checkout, and even green suites can mislead) and points togit ls-files --eolas the reliable verifier.
Show a summary per file
| File | Description |
|---|---|
.squad/e2e/windows-test-baseline.md |
Rewrites the remediation block to be attribute-driven and single-sourced, with supporting rationale and verification guidance. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
Comment on lines
+166
to
+173
| # 1. Enumerate PINNED files by attribute — never by glob. | ||
| $pinned = git ls-files --eol | | ||
| Where-Object { $_ -match 'attr/text eol=lf' } | | ||
| ForEach-Object { ($_ -split "`t")[-1].Trim() } | ||
|
|
||
| # 2. Delete, then restore scoped to that list. Sweep ALL of them, unconditionally. | ||
| $pinned | ForEach-Object { Remove-Item -LiteralPath $_ -Force -ErrorAction SilentlyContinue } | ||
| $pinned | ForEach-Object { git checkout -- $_ } |
The 26-vs-34 discrepancy against Booster's measurement resolves as benign, but finding out why exposed a real defect in the block I shipped an hour ago. Resolution of the gap: no pinned file on this tree is currently w/mixed, and docs/src/pages/rss.xml.js - his specific example - is already w/lf here. It was repaired by the sweep because step 1 enumerates by attribute and sweeps all 173 unconditionally rather than selecting what looks broken. So the repair was complete; only the COUNT was short. My "26" was measured with a w/crlf-only filter, which silently excludes w/mixed. His 34 used w/(crlf|mixed). Part of the 8-file gap is that counting artifact, part is genuine worktree difference. The defect: step 3, the post-condition, used the same w/crlf-only filter. A pinned file with a single stray CRLF line is reported w/mixed, not w/crlf, so the assertion could print 0 while a pinned file remained broken. That is a blind oracle sitting in the safety net of a procedure whose entire subject is blind oracles. Fixed by matching w/(crlf|mixed), with a comment saying the alternation is not a simplification target so nobody collapses it later. Verified with a discriminating control rather than by reading: i/lf w/mixed + pinned, crlf-only filter -> False (missed it, silently) i/lf w/mixed + pinned, crlf|mixed filter -> True (caught) i/lf w/lf + pinned, crlf|mixed filter -> False (no over-match) The doc's fenced block was then re-executed verbatim and still returns 0. The 26 figure is retained but explicitly labelled an undercount, with the reason, because the honest version of that number is the argument for why the sweep is unconditional: the count is an unreliable oracle, so it is never used to choose what to repair - only to confirm the result. Added the w/crlf-only filter to the blind-oracle table as its own row. Refs #1809, #1793. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70370e36-33b0-4786-bd72-4cf15518daa6
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
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.
Replaces the renormalize remediation shipped in #1808. Doc-only, one block.
The defect
#1808's remediation selected
*.mjs. That was correct against the known-affected set and wrong against the pinned set. Booster's #1809 measurement invalidated it roughly forty minutes after it merged.Measured on
dev:attr/text eol=lf.mjsThe old sweep would have repaired 6 of 26 and left 20 broken — including
cli.js,index.cjs,lib/rework.cjs, and every.ps1.Why that is worse than shipping nothing
A partial repair produces a third distinct failure set matching neither the doc nor the card. That reads as "the remediation is unreliable" rather than "the selector was wrong" — so it discredits the correct procedure. Same shape as converting a visibly-dirty file into an invisibly-broken one: a repair that degrades the signal is worse than no repair.
The fix
Enumerate by attribute, never by extension, so the command tracks
.gitattributesautomatically and cannot drift the next time a non-matching extension gets pinned.Verified end to end by executing the documented block verbatim — not a paraphrase of it:
13 failed/0 tests→ 26 passedcli.jsandindex.cjs(the two the old sweep missed) confirmedw/lfStructure — the WHY stays, the HOW is single-sourced
The doc keeps the argument: the blind-oracle table,
--forcedoes not force,git statusstructurally cannot see it, and the 26-test proof. That argument is what makes someone actually run the step instead of skipping it, and losing it loses compliance.The procedure is now stated exactly once, here, with the card and runbook pointing at it rather than restating it. Selector drift between two copies becomes structurally impossible rather than merely fixed — which matters given we now have a measurement of how fast it happens.
Step 2 restores a scoped file list rather than
git checkout -- ., which is prohibited by repo convention after it silently reverted in-progress work earlier tonight.Note
The claim in #1808 was true when it was verified. Nothing was wrong at merge time; a later measurement landed on the shipped surface and retroactively invalidated the selection, not the reasoning. Verify-before-claiming is necessary and not sufficient — the remaining defense is noticing when someone else's new finding touches something you already shipped.
Refs #1809, #1793. Supersedes the remediation block from #1808.