ci: add a Windows native check job - #1213
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a6f01bc26
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let (response, client) = @http.get_stream( | ||
| "http://127.0.0.1:\{server.addr.port()}/package.zip", | ||
| headers={ "Accept-Encoding": "identity" }, | ||
| ) |
There was a problem hiding this comment.
On Windows, the test immediately above is excluded specifically because its client never observes the localhost response, but this replacement uses the same @http.get_stream call against the same in-process server and drains that client in the same way; only the file/progress wrapper is removed. Because the new workflow runs the full native workspace suite, this test can reproduce the documented hang and hold check-windows until its 60-minute timeout. Fix or avoid the underlying Windows HTTP roundtrip before enabling this twin.
Useful? React with 👍 / 👎.
| // colon in the name (a `path:line:col` suffix) is invalid, so `exists` | ||
| // raises instead of returning false. Treat that as missing — never | ||
| // reinterpret the literal as an editor position. | ||
| let literal_exists = @fs.exists(self.0) catch { _ => false } |
There was a problem hiding this comment.
When exists raises for an access-denied, I/O, or other non-invalid-name error, this catch converts it to false, after which the reference is reported missing or even reparsed as an editor-position path. That regresses the function's documented error propagation and prevents the host from distinguishing an unreadable path from a nonexistent one; catch only the Windows invalid-name condition needed for colon suffixes and re-raise unrelated failures.
AGENTS.md reference: AGENTS.md:L27-L35
Useful? React with 👍 / 👎.
0ce6554 to
eecd928
Compare
6bdd580 to
b6d584e
Compare
Adds a check-windows job (windows-latest): MoonBit via the official PowerShell installer, moon fmt --check, full-workspace moon check --target native --deny-warn (MSVC), the bgjobs Windows suite, and the editor module's native tests. The root module's and desktop member's native suites are not Windows-port-ready yet and stay out of the test scope (noted in the workflow comments). To keep --deny-warn green on Windows: add Windows twins of the bgjobs process suite (cmd builtins instead of sh) and JSON-escape the editor-server freshness fixture path. Co-Authored-By: SeekMoon <noreply@moonbitlang.cn>
The async/fs import is otherwise only read by the Windows-gated spill twin, so on Linux the package is unused and `moon check --deny-warn` fails with unused_package. Co-Authored-By: SeekMoon <noreply@moonbitlang.cn>
…ests Windows CI now runs `moon test --target native` across the whole workspace. Tests that assume POSIX paths, symlink privileges, or unix shell semantics are gated with #cfg(not(platform="windows")) at their declarations; helpers and test-scope imports that only those tests used are gated or removed alongside. Each gate documents the Windows-port work that would re-enable it. Real fixes included instead of gates where small: - openpath: a literal path that cannot be stat'ed (invalid on Windows because ':' is illegal in filenames) is now treated as missing instead of raising. - desktop internal/update: the localhost download test hangs under the Windows sandbox, so it is gated and a Windows twin covers the local server roundtrip; the socket import stays live on both platforms. - bgjobs keeps its Windows twins from the first commit (they keep the snapshot fields read and the --deny-warn gate green). The editor freshness fixture JSON-escape fix and bgjobs @fs.tmpdir fix from the first commits remain. Co-Authored-By: SeekMoon <noreply@moonbitlang.cn>
Co-Authored-By: SeekMoon <noreply@moonbitlang.cn>
The Windows check gate no longer uses --deny-warn (Linux keeps it), so the additions that existed only to keep that gate green are reverted to origin/main: the bgjobs Windows twins and fs import, the update localhost twin, and the cfg/unused-package cleanups around helpers and test-scope imports. The failing-test gates stay (the full native suite is green on Windows, 3144/3144 locally), as do the two real fixes (openpath unstat-able literal paths; editor freshness fixture JSON escaping). Co-Authored-By: SeekMoon <noreply@moonbitlang.cn>
The full native test build compiles Proton's C sources, which include CEF headers from Proton's user-level store. A fresh runner has no store, so `moon test --target native` failed with missing include/capi headers; run cef setup (with the same cache keys as the Linux job) before tests. Co-Authored-By: SeekMoon <noreply@moonbitlang.cn>
Temporary probe: the full-suite Windows run stalls ~10 min in with no output (two reproductions); locally the same tree passes 3113/3113. Split the test step into sequential area steps so the hanging area identifies itself, then gate the culprit per policy. Co-Authored-By: SeekMoon <noreply@moonbitlang.cn>
CI bisection found the runner-only Windows failures/hang in agent_workflow/worker_test.mbt (git-worktree slice lifecycle; same family as the gated cmd/openseek subtask tests). Five tests fail on the Windows runner while passing locally, so they are gated per the Windows-port policy. The probe steps are removed and the full native suite restored. Co-Authored-By: SeekMoon <noreply@moonbitlang.cn>
The stage_wbtest download test deferred server.close() in the test body while the spawned background task was still accepting, racing the accept loop against the close (use-after-free on the live listener handle). On Windows this hangs the test forever; unix never noticed. Defer the close inside the spawn_bg task so it runs only after run_forever returns on cancellation. Verified: update package 8/8 on Windows (was a permanent hang), standalone repro passes 3/3. Co-Authored-By: SeekMoon <seekmoon@moonbitlang.com>
b6d584e to
5828420
Compare
1 participant
Adds a
check-windowsjob (windows-latest) to the CI workflow:core.autocrlf falsebefore checkout (the repo has no.gitattributes).moon fmt --check, a full-workspacemoon check --target native(MSVC; warnings tolerated on Windows, Linux keeps--deny-warn), and the full native test suitemoon test --target nativeacross every workspace member, desktop included.Windows test porting in this PR:
#cfg(not(platform="windows"))at their declarations (helpers/imports that only they used are handled alongside); each gate documents the port work that would re-enable it.desktop/internal/openpathtreats a literal path that cannot be stat'ed as missing (on Windows:is illegal in filenames, soexistsraised); the editor-server freshness fixture JSON-escapes its path so the canned moon-check output parses on Windows.desktop/internal/update's localhost download test is gated (it hangs under the Windows test sandbox).Verified on a Windows machine with the MSVC toolchain:
moon check --target native(0 errors),moon fmt --check, and the full native suite (3144/3144 on the rebased tree). The workflow YAML parses cleanly.Note: after ~08:00 UTC on 2026-09-02 GitHub Actions stopped creating runs for this PR's heads (external checks and other branches still run); CI confirmation for the final head is pending that scheduling recovery.
Generated with SeekMoon