docs(e2e): correct the Windows baseline — retract a wrong prediction, flag the #1793 red herring - #1808
Merged
Conversation
… add the #1793 red herring Corrections to .squad/e2e/windows-test-baseline.md, all from measurements taken after it merged in #1797. Three of the four are corrections to my own claims. 1. RETRACTED: "#1798 should stabilise template-sync." That prediction was wrong. Flight ran the full suite with #1798 applied and template-sync still failed, with an unrelated mechanism (EBUSY in its own beforeAll). I then confirmed it independently against merged dev - failed one run, passed the next, no code change between. The suite's own beforeAll shells out to sync-templates.mjs, which writes 65 files into the live tree while 273 other files run in parallel workers. It is its own counterparty. The #1796 byte-compare race is retained as a possible SECOND contributor, but demoted to a hypothesis with its evidence rather than a closed case. The original beforeAll comment in that file diagnosed a race, named the wrong counterparty, and closed the case - which is exactly why the bug survived. A correction that repeats the shape of the thing it corrects is worth nothing. 2. NEW, and the most likely false alarm tomorrow: Booster's #1788 guard (mjs-shebang-loadable) fails 13 tests on any un-renormalized checkout, and check-changeset-drift still loads 0 tests. This looks exactly like "the #1788 fix is broken." It is not - the guard is working as designed on a tree that is still CRLF. Verified: attr/text eol=lf with w/crlf, 113 CRLF pairs on disk. Controlled proof, one variable flipped: 13 failed -> 18 passed, and 0 tests -> 8 passed. 26 tests recovered by changing only bytes on disk. Remediation steps included; a plain `git checkout --` is a no-op, the files must be deleted first. 3. #1796 is NOT fully fixed, and my own #1798 guard is the detector. On merged dev a full run still stamps squad.agent.md 0.0.0-source -> 0.13.0. The guard PASSES in isolation (26 passed) and FAILS in the full suite, so the remaining writer is a different suite in a parallel worker. At least 16 suites build sandboxes inside the repo via join(process.cwd(), ...); #1798 fixed one instance of a recurring pattern. Filed as #1807 rather than expanded into. 4. The headline number is marked superseded in place, and the range is now conditioned on tree state rather than stated flat. Stable/moving/EOL-dependent sets are separated, since a suite that fails only on an un-renormalized tree is not noise and must not be filed as such. Also: working-tree side effects are now three rows with mechanism, risk and owner each - squad.agent.md (content, high risk), the .snap churn (pure EOL, folded into #1790 so it rides one renormalize instead of stranding a second file), and the docs/pagefind.yml + .sh + .ps1 churn. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70370e36-33b0-4786-bd72-4cf15518daa6
Contributor
🟢 Impact Analysis — PR #1808Risk tier: 🟢 LOW 📊 Summary
🎯 Risk Factors
📦 Modules Affectedsquad-state (1 file)
This report is generated automatically for every PR. See #733 for details. |
Contributor
|
Contributor
🔒 Security Review🔒 Security review: 1 error(s).
Automated security review — informational only. |
Contributor
🛫 PR Readiness Check
PR Scope: 🔧 Infrastructure
|
| Status | Check | Details |
|---|---|---|
| ✅ | Single commit | 1 commit — clean history |
| ✅ | Not in draft | Ready for review |
| ❌ | Branch up to date | dev is 3 commit(s) ahead — rebase recommended |
| ❌ | 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 | 2 unresolved Copilot thread(s) — fix and resolve before merging |
| ✅ | CI passing | All checks passing |
Files Changed (1 file, +193 −5)
| File | +/− |
|---|---|
.squad/e2e/windows-test-baseline.md |
+193 −5 |
Total: +193 −5
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the Windows E2E test baseline documentation to reflect new measurements on merged dev, retract a prior (incorrect) stabilization claim about template-sync, and add clearer guidance for distinguishing real regressions from flaky / EOL-dependent failures.
Changes:
- Marks the original single-number baseline as superseded; adds run-to-run variability (“moving set”) guidance and updated expected ranges.
- Documents the confirmed
template-syncEBUSY mechanism as self-induced (writes into the live working tree during parallel execution) and retracts the earlier #1798 prediction. - Adds a prominent warning/remediation section for the #1788 guard on un-renormalized (CRLF) Windows working trees, plus expanded “working-tree side effects” table and #1796 residual notes.
Show a summary per file
| File | Description |
|---|---|
.squad/e2e/windows-test-baseline.md |
Refines Windows test baseline with corrected causal analysis, updated measurements, and clearer operational remediation steps. |
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: 2
- Review effort level: Lite
Comment on lines
+161
to
+163
| git ls-files -- '*.mjs' | % { Remove-Item -LiteralPath $_ -Force } | ||
| git checkout -- . | ||
| git ls-files --eol scripts/check-changeset-drift.mjs # must show w/lf |
Comment on lines
+170
to
+172
| ### Post-renormalize numbers on merged `dev` | ||
|
|
||
| Same tree, immediately after the renormalize above: |
bradygaster
added a commit
that referenced
this pull request
Aug 21, 2026
…haped (#1810) * docs(e2e): make the renormalize selector attribute-driven, not glob-shaped (#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 * docs(e2e): post-condition must match w/mixed, not just w/crlf 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 --------- Co-authored-by: brady gaster <bradygaster@github.com> 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.
Corrects
.squad/e2e/windows-test-baseline.mdafter #1797 merged. Three of the four corrections are to my own claims, made after measurements taken tonight against mergeddev.1. Retracted: "#1798 should stabilise
template-sync"That prediction was wrong.
Flight ran the full suite with #1798 applied and
template-syncstill failed, with a mechanism unrelated to the writer #1798 removed (EBUSYin its ownbeforeAll). I then confirmed it independently against mergeddev— failed one run, passed the next, no code change between.Confirmed mechanism: the suite's own
beforeAllshells out toscripts/sync-templates.mjs, which writes 65 files into the live working tree (verified:Synced 65 file(s)) while 273 other files run in parallel workers. It is its own counterparty.The #1796 byte-compare race is retained as a possible second contributor but demoted to a hypothesis with its evidence, not a closed case.
Worth stating plainly: the original
beforeAllcomment in that file diagnosed a race, named the wrong counterparty, and closed the case — which is exactly why the bug survived. A correction that repeats the shape of the thing it corrects is worth nothing, so this one is deliberately hedged where the evidence is thin.2. New — the most likely false alarm tomorrow morning
Booster's #1788 guard (
mjs-shebang-loadable) fails 13 tests on any un-renormalized checkout, andcheck-changeset-driftstill loads 0 tests.This looks exactly like "the #1788 fix is broken." It is not. The guard is working as designed on a tree that is still CRLF — this is #1793 manifesting as a test failure.
Verified state:
attr/text eol=lfwithw/crlf, 113 CRLF pairs on disk.Controlled proof, one variable flipped:
mjs-shebang-loadablecheck-changeset-drift26 tests recovered by changing only the bytes on disk. Remediation steps are in the doc — note a plain
git checkout --on an unmodified file is a no-op; the files must be deleted first.3. #1796 is not fully fixed — and my own guard is the detector
On merged
dev, a full run still stampssquad.agent.md0.0.0-source→0.13.0.The guard I added in #1798 passes in isolation (26 passed) and fails in the full suite — so the remaining writer is a different suite in a parallel worker. At least 16 suites build sandboxes inside the repo via
join(process.cwd(), ...). #1798 fixed one instance of a recurring pattern.Filed as #1807 rather than expanded into, per the standing instruction to stop and report if #1796 turned out to be more than a contained fix.
4. The headline number is marked superseded in place
The original figure was a single measurement presented as a constant. The range is now conditioned on tree state, and three sets are separated — stable, moving, and EOL-dependent. That last distinction matters: a suite that fails only on an un-renormalized tree is not noise and must not be filed as such.
Also: working-tree side effects are now three rows with mechanism, risk and owner each. The
.snapchurn is folded into #1790 so it rides one renormalize instead of stranding a second file needing a second remediation.Why this PR exists at all
The document I shipped yesterday would have had Brady chasing a phantom on the first anomaly. Catching that in my own work — while running a negative control for an unrelated fix — is the point. Doc-only; no code paths touched.
Related: #1793, #1795, #1796, #1803, #1807, PRs #1790, #1798, #1802.