Add native Windows support and portable releases - #313
Conversation
Embed the Zeron icon as resource 1, which GPUI loads from the executable, for Windows debug and release builds.
Separate iOS, edge, and shared Rust test triggers, restrict deployment runs to relevant paths, and build the Windows GUI fixture only for explicit native GUI runs.
Contain children at creation, preserve launch and stdio behavior, and share startup-attribute handling. Keep idle terminal batching demand-driven and cover cleanup, discovery, and launch regressions.
Remove unrelated workflow reorganization, retain shared Rust coverage, and shorten the Windows setup guide.
wingleeio
left a comment
There was a problem hiding this comment.
Reviewed a48e1fb. Requesting changes for the three issues below: a failed Windows update can remove the launchable installation, Claude availability no longer honors its environment override, and Windows npm Codex discovery is not used by account login.
Validation on Linux: 174 engine unit tests, 130 harness unit tests, 8 auth integration tests, and 5 updater unit tests passed (317 total). A separate isolated regression probe reproduced the Claude override failure. I inspected both pinned UI dependency changes and the new updater dependency. I could not run the Windows build, ConPTY/Job Object tests, GUI probes, or a real Windows update here; this head currently has no successful Windows CI result.
WorkOS on Windows: I found no Windows-specific blocker in the implemented authentication path. The desktop uses GPUI's Windows ShellExecuteW browser opener, then receives the OAuth callback on http://127.0.0.1:<port>/callback; exchange, refresh, and organization requests go through the existing edge API. It does not depend on registering a custom URL scheme. Session persistence uses the new application data directory, shared by GUI and CLI. WorkOS documents loopback redirects for native clients (redirect requirements); the configured client must allow the callback URI/port pattern.
That is a source-level compatibility assessment plus passing stub-server auth tests on Linux, not confirmation of an authenticated Windows run. Please include cargo test --release --locked -p zeron-engine --test auth in Windows CI (the current workflow skips that integration target), and verify browser sign-in, app restart/session restore, refresh, org selection, and logout on Windows before claiming authenticated support. Windows session-file privacy currently relies on inherited directory ACLs; the Unix-only 0600 enforcement does not apply there.
The PR description also needs updating: the current diff adds portable packaging, publication, and in-app updates, although the description still lists them as follow-ups.
A failed Windows update could remove the launchable installation: self-replace renames the running executable away and schedules its deletion before copying the replacement back. Replace it with a hand-rolled swap that copies and re-verifies first, moves the running image aside only then, and restores the backup on every post-move failure; the relaunched instance deletes the renamed backup once the old image exits, and the next update attempt heals a crash between the two renames. The new failure-injection test holds a real sharing violation on the incoming copy after the old image has moved. Claude availability ignored CLAUDE_CODE_EXECUTABLE: installed() used bare discovery while the launch resolver honored the override, so an override-only install showed as unavailable (also on Linux/macOS). installed() now uses the validated launch resolver, with a cross-platform override-only availability test. Codex account login spawned a bare `codex` command that neither followed the npm payload layout nor honored CODEX_EXECUTABLE. The engine now launches `codex login` through the harness's own resolution and child-PATH composition via codex::login_command. Windows CI additionally runs the zeron-engine auth integration tests.
Both the resolver and the spawner were hard-coded to .exe, so
npm-installed agents were undiscoverable and unlaunchable: npm leaves
only a `name.cmd` shim on PATH with the real binary buried under
node_modules. Discovery is now PATHEXT-aware — per directory,
extensions in PATHEXT order, limited to what we can launch (.com,
.exe, .bat, .cmd) — and overrides accept batch shims that exist on
disk, so availability and launches agree.
Resolved .cmd/.bat scripts spawn through `cmd.exe /d /s /c` with
cross-spawn-style per-argument escaping: the shim is the only
interpreted layer, agent arguments stay literal (spaces, backslashes,
quotes, unicode, cmd metacharacters round-trip verbatim — proven by
re-invoking a real program through a shim fixture), and Job Object
ownership covers the whole tree through cmd.exe's membership. The
codex node_modules vendor special-case is deleted: it searched the
wrong vendor subdirectory (`bin/` instead of `codex/`), and its
fixture encoded that wrong layout, so CI passed while real npm
installs failed. GUI launches additionally backfill %APPDATA%\npm,
%LOCALAPPDATA%\{pnpm,Programs\nodejs}, scoop shims, and
%USERPROFILE%\{.local\bin,.bun\bin} from PATH.
opencode and cursor resolution migrate off raw $HOME to the shared
USERPROFILE-aware home helper; cursor also probes
%LOCALAPPDATA%\cursor-agent. The engine catalog and login tests use
the real npm shim layout, batch-override harness tests assert safe
launch instead of rejection, and the shell-script fixture suites are
gated to Unix where they actually run.
wingleeio
left a comment
There was a problem hiding this comment.
Reviewed 73b245c. The three earlier requests are addressed on this head (the swap now copies and verifies before moving the running image aside, Claude installed() uses the launch resolver, and Codex login goes through codex::login_command). Requesting changes again for one behavior regression and three new tests that cannot pass on Linux.
Validation on Linux (this head): cargo check --locked --workspace --all-targets clean; engine 174 unit tests, auth 8, updater 10, app 5, claude 9, codex 20, cursor 20, acp_quiet 1, acp_stall 6 all passing. Failing: harness unit tests 129/131, tests/acp.rs 26/27, codex_login_resolution 3/4, claude_availability 2/3 — details inline. The terminals integration test (m5_repos_diffs_terminals) passed 21/22 with the one failure being a pre-existing SHA-prefix fuzzy-match flake that also reproduces independently of this branch. I rebuilt against the pinned Zui/gpui-base revisions; both match their upstream main heads.
CI: I approved the fork workflow runs on this head. Preview networking and UI tests (all five jobs) are green. Windows tests: green on this head (build, shader layout, app/updater, engine+harness unit, native harness integration, catalog, auth, UI, and the no-HOME CLI startup probe; the native GUI job is dispatch-only and was not run). CI, a fresh release build, and the GUI probes are no longer "pending" for this revision..
Nits (no change required to land):
release.yml:publishnow requires the Windows job, so a Windows-only build failure (fxc discovery, runner image drift) blocks Linux and macOS releases. That is a reasonable "every platform" policy, just calling it out as a deliberate change from the previous comment's intent.crates/update/src/lib.rsrelease_base:ZERON_RELEASES_URLis honored on every platform and is not HTTPS-checked, unlike the Windowszeron-update.jsonfeed. Consider the samehttps://guard.process/windows/command.rsquote_batch_argument: agent arguments passed to a.cmdshim are quoted but not%-escaped, so a%NAME%inside an argv element is still expanded by cmd.exe. Today every harness argv is program-controlled (flags, model ids, settings JSON), so this is not reachable from prompts; worth a comment or^%escaping so it stays that way.terminals/windows.rsProcess::wait_for: terminating the job when the shell exits also kills anything the user detached from that shell (start notepad, background servers), which differs from the Unix terminals where orphans survive. If intentional, a doc note inwindows-development.mdwould help.
|
Pushed three commits to this branch (maintainer edit) so it can land without another round trip:
Linux on the merged head: engine 177, harness 131, acp 28, claude_availability 3, codex_login_resolution 3, terminals 22, auth 8, updater 5, app 10 — all passing; |
Resolve Cargo.toml (keep main's newer Zui/gpui-base pins), crates/ui/Cargo.toml (keep both the windows-render-fixture and appshots-fixture examples), and Cargo.lock (main's lockfile plus this branch's windows-sys/embed-resource dependency entries).
1064191 to
d106661
Compare
…breaks - AcpHarness::installed() now validates `executable`/env overrides the same way resolve_launch does, so a missing override is not advertised as an installed agent whose model discovery and launch both fail. The static-catalog fallback test now uses an executable that exists but cannot speak ACP (the probe-failure case it was written for); a new test covers the missing-override case as not-installed + NotInstalled. - executable.rs: the two PATHEXT-casing tests wrote one case to disk and looked up another, which only resolves on case-insensitive filesystems. Match the on-disk name so they pass on Linux too. - codex_login_resolution: the npm .cmd shim test depends on PATHEXT discovery and is now cfg(windows). - claude_availability: the not-installed probe skips on machines with a system-wide /usr/local/bin or /opt/homebrew/bin claude, like its sibling.
d106661 to
61306e4
Compare
appshot_destination is serde(skip) outside macOS and Linux, so it reloads as the default there. The round-trip test hard-coded NewSession and could only pass where the field is persisted; the first Windows CI run surfaced it.
Use the GUI subsystem and attach to an existing parent console before argument parsing, preserving redirected standard handles. Add a binary subsystem and CLI output regression check to Windows CI.
Stop native caption hit-testing at the tab scroller so Windows does not treat the tab controls as draggable titlebar space. Preserve horizontal scrolling and exercise the production titlebar wrapper in tab tests. Verified native tab clicks and add-menu actions.
Sync remaining Windows fixes from Zeron zeronsh#313
wingleeio
left a comment
There was a problem hiding this comment.
Security review of 01bcb8ee4cbe5323d3dc1329f49830754a2aa6a9: requesting changes; not cleared for approval/merge yet.
I found no evidence of malicious behavior in the contributor's changes. I reviewed the complete diff against current main, Windows process/job/handle ownership, argument encoding, executable discovery, ConPTY, updater/packaging, build workflows, and related ZUI/dependency changes. This is a code review, not proof of anyone's intent or a guarantee of zero vulnerabilities.
Remaining dependency blocker: a clean resolution of the unchanged @cursor/sdk@1.0.28 pin installs @connectrpc/connect-node@1.7.0 -> undici@5.29.0. Current npm audit reports twelve Undici advisories, with three affected package nodes after propagation and no compatible automatic fix. Connect uses Undici for a Headers polyfill, but importing it also initializes Symbol.for("undici.globalDispatcher.1") when absent. The SDK uses global fetch, including its local fetch executor. I therefore cannot dismiss all HTTP-client exposure as dead code.
The published SDK additionally bundles Undici 6.23.0, outside the npm audit graph. Its tree-shaken auth module imports ProxyAgent, and module evaluation likewise initializes the shared dispatcher when absent. An npm override alone does not replace this bundled copy. For example, GHSA-35p6-xmwp-9g52 affects HTTP/1.1 keep-alive response handling below 6.27.0 and requires an attacker-controlled upstream. I have not reproduced an exploit against Zeron, and did not establish reachability for the reported high-severity WebSocket issues. A supported dependency update or tested transport mitigation is required for the requested whole-dependency clearance. The registry's latest SDK 1.0.31 still declares Connect Node ^1.6.1, so an SDK version bump alone is not an established fix. This dependency debt already exists on main and is not evidence against this contributor.
Fixes pushed during this review:
- Replaced unsafe Windows batch argument encoding with Rust's hardened strategy; added native injection-sentinel, round-trip, Unicode-path and rejection tests.
- Removed unresolved Windows shell fallback to current-directory executable search.
- Required HTTPS update feeds and rejected redirect downgrades; patched Rustls 0.23.43 to 0.23.45 for GHSA-2mjx-qc3c-rqvc.
- Resolved main conflicts while preserving shared Codex login resolution; kept unsupported Windows in-app blur opaque.
- Added Windows login, startup and portable packaging checks; fixed the packaging version probe they exposed.
- Fixed the Linux browser test's menu-dismissal timing race while retaining both click-isolation assertions.
ZUI remains pinned to the same revision as main. Reviewed ZUI #7 and #9, including DirectX shader layout parity. Inspected draft #10; it is not included or approved here. Grok/pi-acp metadata, archive integrity and relevant install code were checked; provider prebuilt binaries were not reverse-engineered.
Validation: workspace all-target check passed; 1,032 UI unit tests passed; broad integration invocation passed 589 tests with 16 skips (overlapping other unit runs). Final native X11 and Wayland browser checks passed locally. Cargo audit now has zero vulnerability findings, with the same pre-existing maintenance/soundness/yanked warnings as main. The final Windows native harness security stage passed; Windows CI and UI/platform CI are still running. Their completion will not by itself resolve the npm dependency blocker.
wingleeio
left a comment
There was a problem hiding this comment.
Reviewed the final PR changes, including the Windows generated-image import hardening added after native CI exposed a replacement race. Windows imports now hold the source and traversed directories open without write/delete sharing, reject reparse points, and release the handles after publication. The Windows regression attempts source overwrite/replacement and parent rename; Unix retains its mutation-detection regression.
Native font geometry now exercises DirectWrite on Windows, and deterministic UI schedulers use their own timers. A separate real waitable-timer test preserves native clock coverage. The full Windows UI stage passed after these fixes.
The previous audit also corrected Windows batch command injection, unresolved shell fallback, insecure update-feed overrides, the Rustls advisory, unsupported in-app blur, Codex login merge conflicts, portable packaging and browser-test timing. The ZUI pin is unchanged from main; related ZUI #7 and #9 were reviewed, and draft #10 is not included.
No evidence of malicious behavior or remaining PR-introduced security vulnerabilities was identified in the reviewed changes. This is a scoped review, not a guarantee that the application or third-party binaries contain no vulnerabilities.
Per the maintainer's explicit scope decision, the pre-existing Cursor SDK/Undici findings documented in the earlier review are deferred separately and do not block this Windows-support PR. Those findings have not been fixed or withdrawn. Existing Cargo maintenance/soundness warnings and the limitations of reviewing prebuilt provider binaries likewise remain documented.
Final reviewed commit: 0cc2f0778d136d8477c6ef265baba928eb4cd896. All applicable CI checks passed: native Windows tests/build/portable packaging/startup probes, UI/platform and session regressions, and preview networking. Local workspace checks, Linux UI/browser tests, and targeted security regressions also passed. Opt-in native GUI/pixel probes and live authenticated provider runs were not executed.
This approval supersedes my earlier changes-requested review: the PR-specific issues are fixed, and the maintainer has accepted tracking the existing Cursor dependency findings separately.
Adds native Windows x64 source builds and portable release ZIPs, covering application storage and locking, agent discovery and launching, terminal process cleanup, desktop behavior, and in-app updates. Installers and background services remain unsupported.
Windows agents and terminals use creation-time Job Object membership. npm
.cmd/.batlaunchers use hardened argument escaping; unsafe line breaks and expansion syntax in executable paths are rejected. Terminal discovery fails closed when a bare shell cannot be resolved. Generated-image imports pin their Windows source and directory handles, reject reparse points during traversal, and reject concurrent writers or replacement attempts. Portable updates require HTTPS and SHA-256 verification, verify a replacement copy before swapping, and restore the previous installation when the final rename fails.The pinned ZUI revision includes ZUI #7 and ZUI #9. ZUI #10 is not included: Windows uses native Acrylic window chrome while in-app panels stay opaque until that renderer work is ready. The lockfile also updates Rustls to 0.23.45 for RUSTSEC-2026-0285.
See Windows development for setup, supported behavior, and remaining acceptance work. CI covers Windows release compilation, shader layouts, synthetic native agent/process tests, ConPTY, locking, auth fixtures, UI, and startup. Native font geometry is tested with DirectWrite; deterministic UI tests use their own animation scheduler, and a separate test exercises the Windows high-resolution timer. Live authenticated provider runs and broader GPU/DPI coverage remain manual acceptance work. See the review for validation of the final commit.