Skip to content

The native Windows suite passes nightly - #479

Merged
kixelated merged 6 commits into
mainfrom
quest/m1/windows-suite
Nov 4, 2026
Merged

kixelated merged 6 commits into
mainfrom
quest/m1/windows-suite

Conversation

@kixelated

@kixelated kixelated commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Finishes quest/m1/windows-suite.md: the native Windows suite passes on windows-2022 and windows-11-arm, and runs nightly on main and on workflow_dispatch instead of on every push.

Changes

  • Nightly workflow. The native Windows jobs move from Check to a new .github/workflows/windows.yml with schedule (08:23 UTC) and workflow_dispatch, and no push or pull-request trigger. A separate workflow needs no per-job event conditions, and Check's cancel-in-progress on main pushes can't cancel the scheduled run. GitHub's failed-run notification is the only signal, the same as sandbox.yml.
  • Submodule listing without git submodule (root cause of arm timeouts). Since moq-dev/quest#445, every worktree create, release, and removal ran git submodule foreach/update, even with no submodules. Each call is git-submodule.sh: one sh and 5 more Git processes, which costs seconds on Windows. On windows-2022, worktree cases went from ~1.5 s to ~11 s, and on arm 3 to 4 of them passed 20 s. submodules() now walks the index's gitlinks (one ls-files --stage per repository, then recursion into populated ones), and create runs submodule update --init only when the checkout has gitlinks. On Linux, "refuses scratch that is tracked…" went from 123 Git processes to 53.
  • Submodule hook on Windows (a real bug). git rev-parse --show-superproject-working-tree prints forward slashes on Windows, so a SubagentStop/SubagentStart from inside a submodule never matched its checkout's canonical registry path, and the checkout stayed active. superproject() now canonicalizes the path, for both the subagent lookup and WorktreeCreate's walk.
  • The quest's other test failures, fixed where each lies:
    • runner.test.ts expects PATH joined with path.delimiter.
    • move.test.ts and worktree.test.ts compare text Git checked out under core.autocrlf with line endings normalized.
    • "fails explicitly without a supported backend" awaits main for the async worktree create.
  • Timeouts for the six cases that clone submodules. They run 100-300 Git processes each and now get 60 s instead of the describe's 20 s, which its comment sizes for "up to 20". On arm, "initializes nested submodules…" takes 24.6 s even after the fixes.
  • Deletes the quest and its link from quest/m1/README.md.

Public API and wire impact

None. One behavior change: a populated gitlink with no .gitmodules entry used to make git submodule foreach die, failing release or removal. It is now inspected like any other submodule (checked for changes, unreached commits, and module stores).

Validation

  • Local (Linux): just check and just test pass on the final tree.
  • Windows, dispatched Check run 41795155607 on 947f0525: all jobs pass, both native Windows jobs included. That commit's tree is the final head's except for check.yml, which still held the Windows jobs (see below).
  • The first dispatched run 41792400119, on the first fix commit, showed the hook bug and the arm timeouts fixed above. Its Linux check job also failed gate.test.ts › "lets a phase finish its cleanup once the enclosing scope signals it". This PR doesn't touch the gate; the test passed in the 9 latest main Checks and 8 of 8 local reruns, so I count it as a flake.
  • After merge, the first nightly run (or gh workflow run windows.yml) is the first run of windows.yml itself. Its steps are the old Check job's, unchanged.

Choices made without asking

  • Nightly only, with no push runs, as the quest preferred. I found no concrete reason to keep push runs.
  • A separate windows.yml rather than a schedule in Check (the quest left this to the work). Check would need event conditions on all three jobs, and its main concurrency group would cancel scheduled runs.
  • To dispatch the Windows jobs before windows.yml is on main (GitHub only dispatches workflows on the default branch), commit 947f0525 temporarily put them back in Check, and 63579334 removed them again. The squash merge drops this churn.
  • Fixed the two root causes in product code (src/cli/worktree.ts and src/cli/worktree-hook.ts) rather than only the tests, since the arm timeouts and the hook failure came from them.

Review

  • Reviewer: Codex, gpt-6.1-sol, xhigh effort (quest review --base origin/main).
  • Round 1 on f37df478: no findings.
  • Round 2 on 63579334 (current head): no findings.

Suggested follow-ups

  • Drop: the gate.test.ts flake above. It was seen once, in a dispatched run, and the land watcher's flake tracking catches a recurrence on PRs.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

kixelated and others added 6 commits November 3, 2026 22:58
Quest-Claim: d2fed8986a870fc9
…uite

`git submodule` is a shell script that spawns sh and five more Git
processes per call, which costs seconds on Windows runners. Since #445,
every create, release, and removal ran it even without submodules, pushing
worktree tests past their timeout on windows-11-arm. List initialized
submodules from the index's gitlinks instead, and run
`submodule update --init` only when the checkout has any.

Fix the suite's other Windows failures where each lies: compare text that
Git checks out with core.autocrlf with line endings normalized, expect
PATH joined with path.delimiter, and await main for `worktree create`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Windows is low priority: a regression may land, and the next nightly run
on main catches it, with GitHub's failed-run notification as the signal.
Its own workflow keeps Check's push concurrency from cancelling the
scheduled run, and needs no per-job event conditions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Git prints `--show-superproject-working-tree` with forward slashes on
Windows, so a SubagentStop or SubagentStart from inside a submodule never
matched its checkout's canonical registry path, and the checkout stayed
active. Canonicalize it as the toplevel already is.

Give the cases that clone submodules 60 s: they run 100-300 Git
processes, which the Windows ARM runner spawns slowly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…branch

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ly workflow

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review November 4, 2026 08:02
@coderabbitai

coderabbitai Bot commented Nov 4, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 9 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: faa578b0-79c2-443a-82cf-a524f3f5886c
Commits

Reviewing files that changed from the base of the PR and between 58696bc and 6357933.

Files selected for processing: 9
  • .github/workflows/check.yml
  • .github/workflows/windows.yml
  • quest/m1/README.md
  • quest/m1/windows-suite.md
  • src/cli/worktree-hook.ts
  • src/cli/worktree.ts
  • test/move.test.ts
  • test/runner.test.ts
  • test/worktree.test.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — one stale comment worth fixing in passing.

Reviewed changes

  • Nightly Windows workflow. The native Windows jobs move from check.yml into windows.yml on schedule and workflow_dispatch, mirroring sandbox.yml's shape, with the steps unchanged from the old Check job.
  • Index-based submodule enumeration. submodules() walks index gitlinks recursively instead of git submodule foreach, removing the shell-script cost that timed out Windows-arm worktree cases; the enumeration stays a fail-safe superset of the old set, with root git status --ignore-submodules=none remaining the backstop for removal safety.
  • Superproject canonicalization. superproject() canonicalizes rev-parse --show-superproject-working-tree's forward-slash Windows output before the hook's registry lookups, and both former raw call sites now route through it.
  • Windows test fixes. CRLF-normalized text comparisons, a path.delimiter-joined PATH expectation, and an awaited main() for the async worktree create.
  • Quest retired. quest/m1/windows-suite.md is deleted and its link dropped from the milestone README.

ℹ️ Stale claim that Check's suite covers Windows

release.yml's comment above the macOS-only native gate test still says "Check's suite runs on Linux and Windows only." Moving the Windows jobs out of Check makes that false, though the step's own conclusion — macOS needs its native gate.test.ts run — still holds.

Technical details
# release.yml comment claims Check's suite still covers Windows

## Affected sites
- .github/workflows/release.yml:133 — comment reads "Check's suite runs on Linux and Windows only"; this PR removes the Windows jobs from Check, so Check's suite is Linux-only and the Windows runs live in the nightly `windows.yml`.

## Required outcome
- The comment must state where the suite's Windows coverage now lives, without changing the macOS-only step itself.

Pullfrog  | Fix it ➔ | View workflow run | Using glm-5.3-flash | 𝕏

@kixelated
kixelated added this pull request to the merge queue Nov 4, 2026
Merged via the queue into main with commit 490a5ff Nov 4, 2026
20 checks passed
@kixelated
kixelated deleted the quest/m1/windows-suite branch November 4, 2026 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant