Windows 2/5: CLI, shells, and providers - #1007
Merged
arul28 merged 15 commits intoAug 3, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
arul28
changed the title
This was referenced Aug 1, 2026
arul28
marked this pull request as ready for review
August 2, 2026 02:55
arul28
force-pushed
the
codex/windows-cli-shells-providers
branch
from
August 2, 2026 02:55
14615bd to
8cdf335
Compare
Owner
Author
|
@CodeRabbit review. @greptile-apps review. |
|
✅ Action performedReview finished.
|
arul28
force-pushed
the
codex/windows-cli-shells-providers
branch
4 times, most recently
from
August 2, 2026 07:34
b1a3fa3 to
3a5a140
Compare
arul28
force-pushed
the
codex/windows-foundation-runtime
branch
from
August 2, 2026 17:51
74f7ca4 to
c70076e
Compare
arul28
force-pushed
the
codex/windows-cli-shells-providers
branch
8 times, most recently
from
August 3, 2026 03:09
2f73b65 to
abc3210
Compare
Based-on: nsxdavid/ADE#999
Complete Windows-native shell, provider, lifecycle, RPC, desktop discovery, and deeplink behavior derived from the rebased #999 work. Co-authored-by: David Whatley <nsxdavid@gmail.com> Based-on: nsxdavid/ADE#999
Keep the foundation-owned named-pipe listener import when the CLI and shell layer is stacked above it. Co-authored-by: David Whatley <nsxdavid@gmail.com> Based-on: nsxdavid/ADE#999
Register Factory Droid install and interactive authentication recovery across Windows and POSIX hosts, with classifier regressions for missing binaries and credentials. Based-on: nsxdavid/ADE#999
Based-on: nsxdavid/ADE#999
`spawn_agent` resolved a provider executable identically on every platform long before the Windows CLI/PTY hardening pass: `resolveExecutableOnPath` has had a `where.exe` arm since the Windows port foundations, and the `createFakePathExecutable` fixture writes `<name>.cmd` on Windows for exactly that reason. When a provider resolves, the spawn path deliberately launches it as `command`/`args` with worker identity in `env`, because the POSIX `VAR=value cmd` startup prefix does not work on Windows. `.cmd` targets are supported end to end: the PTY direct launch runs them through `resolveCliSpawnInvocation`, which wraps `.cmd`/`.bat`/extensionless commands in `cmd.exe /d /s /c`. The `process.platform === "win32"` branches asserting `command` is undefined were added as a test-only change that left `adeRpcServer.ts` untouched, so they never described this repository's behaviour. They passed in CI only because `adeRpcServer.test.ts` runs on ubuntu-latest, where the `else` branch executes; on native Windows all three failed. Drop the forks so both platforms assert the same resolved-command contract, and capture the codex fixture path so that assertion is exact instead of a `/codex$/` regex that a `.cmd` suffix would defeat. The remaining platform forks in this file are genuine: the shell launch fork matches `resolveCleanShellLaunchFields`, and the startup-command env-prefix fork matches `startupEnvPrefixParts`, which is empty on win32. apps/ade-cli: 3 failed/2690 passed -> 0 failed/2693 passed (26 skipped). Based-on: nsxdavid/ADE#999 (cherry picked from commit 601e860c987b20c39d3d53bd1fd599d495639fdd)
cli.test.ts carries the win32-gated headless-RPC named-pipe case, which until now executed on no runner at all. The suite is green only once this layer's CLI fixes compose -- it is red against the foundation layer alone -- so the step belongs here rather than at the stack root. Caught by validate-platform-gates after composition. Based-on: nsxdavid/ADE#999
The seven tests in `stdioRpcDaemon.test.ts` are the whole daemon supervision, restart, and version/role-compatibility surface, and all seven were `it.skip`ped on Windows behind an `itUnix` gate. This stack's headline deliverable is a durable Windows brain with crash restart, so that contract was asserted on Windows by zero tests. They were skipped because each hardcoded `<ADE_HOME>/sock/ade.sock` as `ADE_RUNTIME_SOCKET_PATH`. Windows has no Unix domain sockets, so `net` reads a path-style endpoint as a named pipe name; a filesystem path is not a connectable address there. They now derive the endpoint through `resolveMachineAdeLayout`, exactly as `resolveMachineRuntimeSocketPath` does in production. That is byte-identical off win32 -- the layout returns precisely `path.join(adeHome, "sock", "ade.sock")` -- and yields the real per-user pipe on Windows. Three of the seven pass on that alone. The other four uncovered a production bug. `isEphemeralRuntimeSocketPath` decides whether an auto-spawned brain is a throwaway: an ephemeral one is launched with `--no-sync`, given an idle-exit budget, and excluded from runtime-service repair. It answered by inspecting the socket path, and returned `false` for every named pipe -- so on Windows the ephemeral classification was structurally unreachable and every scratch brain was misread as the machine's real, service-managed brain. The consequence is not cosmetic. A sync-enabled brain runs the sync-host startup loop BEFORE it binds its RPC socket, and a first-attempt cross-channel singleton conflict is fatal there. So a Windows scratch brain that collided with the user's actual brain exited without ever listening, surfacing only as a downstream `connect ENOENT` on the pipe -- indistinguishable from a wrong socket path. Measured on Windows 11 / Node 22.13.1: a `--no-sync` brain listens in ~1.25s, while the same brain with sync enabled never listens at all against a live rival (60s cap). Such a brain also never idle-exits, and under a packaged Electron CLI it was eligible to trigger repair of the installed runtime service. What the POSIX branch actually asks is "does this brain belong to a scratch ADE_HOME under the temp dir", because the socket always lives inside that home. Windows now asks that question of the home directly and confirms the endpoint is the pipe that home derives, comparing through a lowercase backslash-folded key since Win32 treats `/` and `\` interchangeably in a pipe path and matches pipe names case-insensitively. The old body is extracted verbatim as `isEphemeralRuntimeScratchPath` and still handles every path-style socket, so POSIX behaviour is unchanged by construction: `isAdeRuntimeNamedPipePath` is a pure prefix test that no POSIX socket path satisfies, and the one string that could (`//./pipe/...`) returned `false` before and still does, because a POSIX layout socket never equals a pipe. The readiness wait is given the asymmetry production already uses for runtime startup (`LOCAL_RUNTIME_STARTUP_TIMEOUT_MS`, 30s on win32 against 10s elsewhere). It is a ceiling, not a sleep. The `itUnix` gate is gone; nothing is left gated in that file. A separate win32-gated case in `cli.test.ts` pins the pipe classification directly against a scratch `ADE_HOME`, including the equivalent-spelling and not-my-endpoint cases; it fails with `expected false to be true` without this change. That file already runs on the Windows runner. Windows, Node 22.13.1: the suite was 7 skipped and is now 7 passed, six consecutive runs with no flake (35-46s). The full CLI suite goes from 2709 passed / 26 skipped to 2717 passed / 19 skipped, measured against a stashed baseline on the same machine. Based-on: nsxdavid/ADE#999 (cherry picked from commit 61233d9186af39275f3e6cbe3decf2ad57dcea25)
The seven daemon supervision, restart, and version/role-compatibility tests were skipped on Windows via itUnix and now run natively, so the always-on-brain contract had zero Windows assertions behind it. This is the only gate that spawns real `ade serve` daemons over a named pipe. Runs ~45s locally; the file's existing 45s per-test timeouts are unchanged. Based-on: nsxdavid/ADE#999
Concurrent credential writes on Windows fail roughly a fifth of the time with `EPERM: operation not permitted, open credentials.json.enc.lock`. The acquisition loop treated EEXIST as the only "someone else holds the lock" signal. That is a POSIX assumption. On Windows a delete only detaches the name once every open handle to it closes, so between the holder's unlink and the last handle drop the lock name is still in the directory in a delete-pending state, and a concurrent `open(lockPath, "wx")` fails with a delete-pending or sharing violation that Node surfaces as EPERM, EACCES or EBUSY. The loop rethrew those, so a contending writer aborted instead of waiting its turn. Classify those Windows codes as contention alongside EEXIST, and attach the underlying error as the timeout's cause so a genuine permission failure is still legible after the deadline expires. Reproduced with 16 concurrent writers against the real store: 2 of 8 rounds failed before, 0 of 12 after. Based-on: nsxdavid/ADE#999 (cherry picked from commit b990889508b3fbbb6789aab896ac759988d71ac9)
AutoUpdatesSection renders its toggles immediately but disables them until the stored preferences load, and React drops clicks on a disabled button. The first test clicked as soon as `findByRole` returned the switch, so whenever the preference fetch's commit landed after that point the click was swallowed and `updateSetPreferences` was never called -- the shard-1 failure, `Number of calls: 0` against an already-enabled switch in the diagnostic DOM. Nothing in the component is wrong: the disabled window is a deliberate guard against persisting defaults over preferences still in flight. The test just never waited for it to close, leaving the outcome dependent on event-loop phase ordering, which a loaded runner flips. Wait for the toggle to become enabled before clicking, and await the saved-preference commit in the sibling test, which had the same latent shape at its final assertion. Reproduced by delaying the preference fetch: the old shape fails with `Number of calls: 0`, the new one passes. Based-on: nsxdavid/ADE#999 (cherry picked from commit 4b439629fecd50dc1209ff950e071f228c9abe50)
The socket spawn lock retried only on EEXIST, which is the POSIX shape. Windows keeps a deleted name in the directory until the last handle closes, so between the holder's unlink and that final drop a contending open(..., "wx") hits the delete-pending name and Node reports EPERM, EACCES or EBUSY. The loop rethrew those, so a contending spawn aborted instead of waiting its turn. Same defect and same fix as the credential store lock in b9908895; this one sits in the brain-spawn path, where burst contention is the normal case rather than the exception. The timeout now carries the underlying error as `cause` so a genuine permission failure stays legible. Based-on: nsxdavid/ADE#999
arul28
force-pushed
the
codex/windows-cli-shells-providers
branch
from
August 3, 2026 03:11
abc3210 to
aefcb0f
Compare
…i-shells-providers
…i-shells-providers
…i-shells-providers
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
3 participants
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.
Stack position
Windows 2/5 · parent: #1006 · downstream: #1008
This is layer 2 of stack #1011. No layer merges independently; PR #1010 is the cumulative full-system head.
Based on the Windows implementation by @nsxdavid in #999.
Responsibilities
Exclusions
Runtime foundations are in #1006; desktop/account/sync UX in #1008; packaging in #1009; cumulative proof in #1010.
Validation
Draft checkpoint: direct-parent desktop and ADE CLI typechecks pass on native Windows/Node 22. The worker branch has repaired the native PTY suite to 276/276; coordinator import and exact-head revalidation are pending.
Evidence and provenance