docs(e2e): authoritative Windows test baseline at 369bba8f - #1797
Merged
Conversation
Triages all 9 failing test files on Windows against a fully-green Linux CI run, so tomorrow's E2E series can tell a real red from expected noise. Key findings: - Separates LOAD FAILURES (suite ran zero tests, coverage silently zero) from assertion failures. Found 2 previously-unknown instances of the #1788 CRLF-shebang dead gate: promote-insider-tag (0 of 15 tests) and patch-esm-imports (3 of 6 tests), the latter previously mislabeled as 'ESM patching noise'. - Proves #1790 does not repair an existing working tree; a forced re-checkout is required or three suites stay red after the merge. - Reclassifies 'scheduler timing' as a real cross-platform argv defect. - Documents that the baseline count moves with terminal (WT_SESSION) and checkout topology (worktree vs clone), independent of any code change. Every real-defect verdict is backed by a controlled single-variable experiment, not by reading code. One item is labeled unknown rather than guessed. Filed: #1793, #1794, #1795, #1796. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70370e36-33b0-4786-bd72-4cf15518daa6
Contributor
🟢 Impact Analysis — PR #1797Risk tier: 🟢 LOW 📊 Summary
🎯 Risk Factors
📦 Modules Affectedsquad-state (1 file)
This report is generated automatically for every PR. See #733 for details. |
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 | 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 | No Copilot review threads |
| ❌ | CI passing | 1 check(s) still running |
Files Changed (1 file, +256 −0)
| File | +/− |
|---|---|
.squad/e2e/windows-test-baseline.md |
+256 −0 |
Total: +256 −0
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
Contributor
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a Windows E2E test baseline for commit 369bba8f, documenting failure counts, environment-specific behavior, experiments, and follow-up issues.
Changes:
- Classifies load and assertion failures.
- Documents CRLF, checkout, topology, and scheduler findings.
- Provides remediation guidance and links issues #1793–#1796.
Show a summary per file
| File | Summary | Review findings |
|---|---|---|
.squad/e2e/windows-test-baseline.md |
Adds the authoritative Windows E2E triage baseline. | Four nit-level comments, each with 4 votes: qualify the baseline run, correct the personal squad path conclusion, add clean-tree warnings, and replace the machine-local artifact reference. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Suppressed comments (6)
.squad/e2e/windows-test-baseline.md:39
- The headline count in §1 is
Test Files, butacceptance.test.tshas both 9a and 9b. Removing only 9a leaves that file failing because 9b remains, so the failed-file count stays at 9; only the test-failure count drops by one. Please label which metric this modifier changes, otherwise the final “expect 9 failed” guidance is misleading.
| Running in a **plain clone** instead of a worktree | `acceptance.test.ts` init failure disappears → one fewer | ⚠️ mechanism proven, not re-run in a plain clone |
.squad/e2e/windows-test-baseline.md:178
- The proposed controlled experiment pins
SQUAD_HOME/HOME, butsetupConsultModegets its root fromgetPersonalSquadRoot()→resolveGlobalSquadPath(), which uses%APPDATA%on Windows and does not read either variable. The two runs can therefore still observe different personal-squad directories; pinAPPDATA(and its fallback) instead.
and (b) this worktree, with `SQUAD_HOME`/`HOME` pinned identically, and diff which
.squad/e2e/windows-test-baseline.md:81
- Section 6 says 9a was not rerun in a plain Windows clone and its proof is partial, but this row says it "would not reproduce". That is stronger than the evidence and conflicts with the document's own uncertainty standard; qualify it as likely topology-specific until the plain-clone experiment is run.
| 9a | `test/acceptance/acceptance.test.ts` › *Init in existing project* | Expected `Squad initialized`, got `⚠ Git worktree detected / Main checkout: C:\src\squad` | Yes | **NO — worktree-only** | `init.ts:208` worktree guard fires because `.git` is a **file** (worktree) and the main checkout already has `.squad/`. Git topology, not OS | **expected-noise in a worktree** — would not reproduce in a plain clone |
.squad/e2e/windows-test-baseline.md:239
- The caveat says row 6's security-guard test throws during setup and is never exercised on Windows, while this section calls rows 6 and 9a "correctly classified as noise". That wording makes the final baseline treat an unverified security check and a partially verified topology assumption as harmless; mark them as unverified/likely noise and track the row 6 follow-up.
**Not filed — correctly classified as noise:** rows 6, 7, 8, 9a.
.squad/e2e/windows-test-baseline.md:149
- The preceding sentence says every real defect was proven by a one-variable experiment, but the table only shows experiments for the EOL, terminal, and checkout claims. There is no variable flip/result for the scheduler (row 4) or packaging-smoke (row 5) findings, so this overstates the evidence; add those experiments or narrow the sentence.
Every mechanism above marked *real-defect* was proven by flipping one variable and
re-running — not by reading code.
.squad/e2e/windows-test-baseline.md:6
- For an authoritative, reproducible baseline, the header needs the actual Node and Git versions. The documented findings depend on Vitest/Vite, Node shebang handling, and Git EOL behavior;
vitest 4.1.10alone is not enough for another Windows run to reproduce these results. Please recordnode --version,npm --version, andgit --versionalongside the existing environment details.
**Host:** Windows, Node via vitest 4.1.10, non-Windows-Terminal console
- Files reviewed: 1/1 changed files
- Comments generated: 4
- Review effort level: Lite
Comment on lines
+28
to
+29
| Linux CI on the same tree is **fully green — 274/274 files passed** (run `32461944794`). | ||
| Every failure below is therefore Windows-and/or-topology specific *by construction*. |
| | 7 | `test/repl-ux.test.ts` | 1 assertion: expected `──────────`, got ASCII `------` | Yes | **Terminal-dependent, not OS-dependent** | `terminal.ts:84` — `supportsUnicode: plat !== 'win32' \|\| Boolean(process.env['WT_SESSION'])`. Production is behaving **correctly**; the test hardcodes the Unicode glyph | **expected-noise** (test-side platform assumption) | | ||
| | 8 | `test/cli/watch-capabilities.test.ts` | 1 assertion: expected `StringContaining ".squad/ralph-instructions.md"`, got `"\some\repo\.squad\ralph-instructions.md"` | Yes | Yes | Production builds the path with `path.join` (correct on Windows); the test asserts a POSIX-separator substring | **expected-noise** (test-side) | | ||
| | 9a | `test/acceptance/acceptance.test.ts` › *Init in existing project* | Expected `Squad initialized`, got `⚠ Git worktree detected / Main checkout: C:\src\squad` | Yes | **NO — worktree-only** | `init.ts:208` worktree guard fires because `.git` is a **file** (worktree) and the main checkout already has `.squad/`. Git topology, not OS | **expected-noise in a worktree** — would not reproduce in a plain clone | | ||
| | 9b | `test/acceptance/acceptance.test.ts` › *Consult blocked in squadified project* | Expected `No personal squad found`, got `This project already has a .squad/ directory…` | Yes | **Unknown** | `setupConsultMode` throws a different error than the feature file expects. Ruled out: `~/.squad` does **not** exist here, so the obvious "developer has a personal squad" explanation is **wrong**. Ordering difference between `SquadifiedProjectError` and `PersonalSquadNotFoundError` not isolated | **UNKNOWN** — see §6 | |
Comment on lines
+121
to
+125
| Remove-Item scripts\promote-insider-tag.mjs, scripts\check-changeset-drift.mjs, packages\squad-cli\scripts\patch-esm-imports.mjs -Force | ||
| git checkout -- scripts/promote-insider-tag.mjs scripts/check-changeset-drift.mjs packages/squad-cli/scripts/patch-esm-imports.mjs | ||
|
|
||
| # or repo-wide | ||
| git rm --cached -r . ; git reset --hard |
Comment on lines
+159
to
+160
| Raw full-run output archived at | ||
| `~/.copilot/session-state/a70ee969-.../files/full-test-run.txt`. |
bradygaster
added a commit
that referenced
this pull request
Aug 21, 2026
… add the #1793 red herring (#1808) 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: 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.
What
Adds
.squad/e2e/windows-test-baseline.md— an authoritative triage of every failing test on Windows at commit369bba8f, so tomorrow's E2E series can distinguish a real regression from expected noise.Docs only. No source, no tests, no changeset.
Why
Three times this workstream has been bitten by reports nothing -> mistaken for passing.
check-changeset-drift.test.tsran 0 of 8 tests for a month while everyone read it as environment noise (#1788). The gap was never the failures themselves — it was not knowing which ones were expected.The number
At
369bba8f, in a git worktree, non-Windows-Terminal console:Linux CI on the same tree is 274/274 green (run
32461944794), so every failure is Windows- and/or topology-specific by construction.What the triage found
Two previously-unknown dead gates, both #1788's mechanism. Separating load failures (suite ran zero tests) from assertion failures surfaced them immediately:
check-changeset-drift.test.tspromote-insider-tag.test.tspatch-esm-imports.test.tspatch-esm-importshad been categorized as "ESM patching" — a plausible-sounding grouping that concealed a third instance of the same CRLF-shebang bug.#1790 will not fix your working tree.
.gitattributesgoverns checkout, not files already on disk. Proven by applying #1790's rule locally: attribute applied (attr/text eol=lf), disk stillw/crlf, suite stillno tests. A forced re-checkout is required — filed as #1793 with the exact command."Scheduler timing" is a real cross-platform defect.
LocalPollingProvidersplitstask.refon whitespace with no quote handling, soC:\Program Files\nodejs\node.exefails to spawn. Filed as #1794.The count moves without any code changing — three modifiers, all measured: Windows Terminal (
WT_SESSION) makesrepl-uxpass; a plain clone makes anacceptancefailure disappear; #1790 + renormalize drops three more. A single-number baseline is a lie unless the environment is stated alongside it.Method
Every real-defect verdict is backed by a controlled single-variable experiment, not by reading code:
.mjsto LF, re-run0 tests-> 23 passedpatch-esm-importsis the same bug.mjsto LF, re-runrepl-uxis terminal-, not OS-dependentWT_SESSION, re-runno testsgit checkout --Reconciliation
Procedures independently measured 9 failing files; my count on clean
devis also 9 — counts agree, no file-level flakiness. But two of his three category labels did not survive verification, and both wrong ones concealed real defects.Honesty
One item (
acceptance> Consult blocked in squadified project) is labeled unknown, not noise. The obvious explanation — a personal squad at~/.squadshort-circuiting the check — is disproven; that path does not exist here. Section 6 states what was ruled out and the experiment that would settle it. An unknown reported as expected-noise is exactly how #1788 survived a month.Issues filed
Separate issues per defect, each with mechanism, controlled proof, suggested fix, and success criteria: #1793, #1794, #1795, #1796.
Rows 1-3 are not filed separately — #1790's global rule and its byte-level guard test (which scans every tracked shebanged
.mjs) already cover them. #1793 is the residual #1790 does not cover.Scope discipline
Nothing was fixed. Triage, document, file — as instructed. #1788 untouched.
Checks
git diff --cached --stat-> 1 file, +256git diff --cached --diff-filter=D --name-only-> emptygit ls-files --error-unmatch-> tracked (neededgit add -f;.squad/is in.git/info/exclude)npm run buildmutates version files