feat(cli): native Windows support + cross-platform CI matrix#581
feat(cli): native Windows support + cross-platform CI matrix#581JOhnsonKC201 wants to merge 2 commits into
Conversation
The bin entry was a POSIX shell script (bin/colony.sh) which npm could not execute on Windows without WSL, breaking every Windows install of the package. Replace it with a Node ES module (bin/colony.mjs) using only node:* builtins so npm's generated .cmd / .ps1 wrappers run it natively under cmd, PowerShell, and Git Bash. The daemon fast-path for `colony bridge lifecycle --json` is preserved: the HTTP POST to 127.0.0.1:$COLONY_WORKER_PORT/api/bridge/lifecycle now goes through node:http, with a node:net connect probe (1s) before the request (2s) so the fallback latency stays close to the curl-based version when the daemon isn't running. Stdin is buffered and replayed on fallback, preserving rule recodeeai#10 (a dead daemon must never lose or block a write). bin-shim.test.ts is rewritten to spawn the shim via `node` directly rather than `sh`, so the regression contract runs on all three OSes. CI now runs the build matrix on ubuntu-latest, macos-latest, and windows-latest across Node 20 and 22 with fail-fast disabled, so this regression cannot recur. The Linux-only jobs (e2e-publish, scenarios) stay on ubuntu-latest.
![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: f190cf4fa4
ℹ️ 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".
|
|
||
| function probeDaemon() { | ||
| return new Promise((resolveOuter) => { | ||
| const socket = connect({ port: PORT, host: HOST }); |
There was a problem hiding this comment.
Parsing COLONY_WORKER_PORT with Number(...) allows NaN/out-of-range values to reach net.connect, which throws synchronously and aborts the shim before CLI fallback. In this commit that means colony bridge lifecycle --json can exit with an error instead of replaying stdin to the in-process CLI when the port env var is malformed (for example COLONY_WORKER_PORT=abc), breaking the documented "daemon failures must fall back" contract for write-path safety.
Useful? React with 👍 / 👎.
| if (a === '--ide') { | ||
| out.ide = rest[i + 1] ?? ''; | ||
| i += 2; |
There was a problem hiding this comment.
The new flag parser treats --ide/--cwd without a following value as empty strings and then rebuildSafeArgv removes them, so malformed invocations are silently rewritten (e.g. ... --json --ide becomes ... --json). Before this change, the shell shim failed fast on this invalid input; now the command can proceed with different semantics, hiding caller bugs and potentially running lifecycle writes with missing context.
Useful? React with 👍 / 👎.
1 participant
Summary
apps/cli/bin/colony.sh) with a cross-platform Node ES module (apps/cli/bin/colony.mjs). npm's generated.cmd/.ps1wrappers now runcolonynatively on Windows (cmd, PowerShell, Git Bash) without WSL.colony bridge lifecycle --jsonusingnode:http+ anode:netconnect probe — same 1s connect / 2s request timeouts as the curl-based version. Stdin is buffered and replayed on fallback so rule feat(hooks): auto-claim edited files + cross-session overlap warnings #10 (a dead daemon must never lose or block a write) still holds.macos-latestandwindows-latestto the build CI matrix across Node 20 and 22 withfail-fast: false, so this regression cannot recur. The Linux-only jobs (e2e-publish,scenarios) stay onubuntu-latestbecause they run bash scripts.Why
apps/cli/package.json#binpointed atbin/colony.sh. On Windows, npm cannot execute a POSIX shell script as a CLI bin —colonywas effectively broken for every Windows user without WSL. CI ran only onubuntu-latest, so the regression had no chance of being caught.The shim is now pure
node:*builtins — no curl, no/bin/sh, no external deps. The fast-path's optimization is intact because we dispatch the non-fast-path through a dynamicimport()ofdist/index.jsin the same Node process (no second Node fork).process.argv[1]is rewritten to point at the resolved CLI entry soisMainEntry()inapps/cli/src/index.tsstill matches via realpath comparison.Notable changes
apps/cli/bin/colony.mjs(new) — Node-based bin shim.colony.shstays in the repo for reference but is no longer inpackage.json#files.apps/cli/package.json—binandfilesupdated to point at the.mjs.apps/cli/test/bin-shim.test.ts— rewritten to spawn the shim vianodedirectly rather thansh. Same six contract assertions (fallback preserves stdin + argv with spaces,COLONY_BRIDGE_FAST=0, passthrough variants). Uses a newCOLONY_CLI_ENTRYenv-var test seam to point at a stub instead of needingdist/.apps/worker/src/server.ts,scripts/bench-bridge-fastpath.mjs,CLAUDE.md— updated live references fromcolony.shtocolony.mjs..github/workflows/ci.yml—buildjob now runs on[ubuntu-latest, macos-latest, windows-latest]× Node[20, 22]..changeset/native-windows-support.md— patch-level changeset oncolonyq.Test plan
--versionpassthrough → CLI receives correct argv.bridge lifecycle --json --ide claude-code --cwd "/tmp/has spaces"with daemon down → falls back to CLI with stdin replayed and the cwd-with-spaces preserved.COLONY_BRIDGE_FAST=0→ skips fast-path entirely.bridge replay foo.json→ falls through immediately (no fast-path eligibility).bridge lifecyclewithout--json→ falls through (humans want pretty output).pnpm lint/typecheck/test/buildacross the new OS × Node matrix. The shim tests are the only ones I rewrote for cross-platform; if other tests in the suite have latent POSIX assumptions, they'll surface here. I'd suggest landing those follow-ups as separate PRs rather than letting this one balloon.bash scripts/e2e-publish.sh) stays on Linux. The bin shim it installs is nowcolony.mjs; thetest -x "$BIN"check at step 5 will still pass because the file is committed with mode100755.Risk / scope notes
.skiponprocess.platform === 'win32'in follow-up PRs and either accept the partial coverage or switch tocontinue-on-error: ${{ matrix.os == 'windows-latest' }}until the suite is fully ported.