Windows: full native support (consolidated 2/5-5/5) - #1018
Merged
Conversation
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
Based-on: nsxdavid/ADE#999
Keep PR 3 independently buildable and defer release-repository IPC wiring to PR 4. Co-authored-by: David Whatley <nsxdavid@gmail.com> Based-on: nsxdavid/ADE#999
Protect the shared desktop and background-brain credential key with Windows CurrentUser DPAPI, preserve legacy ciphertext reads, and add the installed-build PR 3 proof procedure. Co-authored-by: David Whatley <nsxdavid@gmail.com> Based-on: nsxdavid/ADE#999
Resolve Windows PowerShell through the kernel SystemRoot namespace and atomically bind legacy ciphertext on first read. Co-authored-by: David Whatley <nsxdavid@gmail.com> Based-on: nsxdavid/ADE#999
Select App Control's compound package-script command after the PTY has actually chosen PowerShell, cmd, or Git Bash. Add native Git Bash path rendering and regressions across the service-to-PTY contract. Based-on: nsxdavid/ADE#999
The App Control Git Bash startup-command assertions hard coded an MSYS drive-letter root (`^cd -- '?/[a-z]/`) while the fixture package directory came from `os.tmpdir()`. On windows-latest the two coincide; on ubuntu-latest the fixture is `/tmp/...`, which has no drive letter, so the assertion could never match and `test-desktop` failed 2 of 1418. `gitBashPath` itself is correct: it folds a drive-letter root into MSYS form and passes a rootless path through untouched, because Git Bash accepts POSIX and UNC paths as-is. Only the tests encoded a host-dependent expectation while forcing `platform: "win32"`. Both assertions now derive the expected `cd` target from the fixture's own path, so each runner asserts the exact command it really produces, and the drive-letter rewrite is pinned separately by a `gitBashPath` unit test over literal Windows inputs. That test runs on every host, so the MSYS conversion is now genuinely covered on the Linux runner instead of merely failing there. Based-on: nsxdavid/ADE#999 (cherry picked from commit de007f02c9d0930badb4842dcf439a6a7741fd8d)
…ity helper Settings and Account still spelled the local machine's name out by hand. The literals happened to read "This computer" here, but nothing tied them to `shared/machineIdentity`, so the next time that name moves these two screens drift again — which is exactly how they missed the macOS-only "This Mac" rename. Compose the badge, the pairing-code note, the phone-tab cross-references, and the unnamed-machine fallback from THIS_MACHINE_NAME instead, and pin the tests to the helper rather than to a copy of its output. The account removal sheet also borrowed the local machine's name for a machine that is never the local one: removal is only reachable from a non-local row's options menu. It now names the machine being removed in the title, and the options button's accessible name falls back to the same "Unnamed computer" label the row itself uses. Based-on: nsxdavid/ADE#999 (cherry picked from commit aeed77d0059aff4e6d4e71e216f9531623b3a106)
appControlLaunchCommand.test.ts holds gitBashPath, the repo's only drive-letter to MSYS conversion, and ran on no Windows runner. Nothing in it is platform-gated today, so this is coverage rather than a validator fix -- but a future win32 gate there would have had no runner to satisfy it. Based-on: nsxdavid/ADE#999
`machineIdentity.THIS_MACHINE_NAME` made the local-machine label platform-neutral, but three settings surfaces kept the macOS literal and rendered it ungated on Windows. `ScopeChip`'s machine scope is the worst of them: it appears on every machine-scoped setting row, and the same object's `affects` line already read "Only this computer", so one object named two different machines. It now sources the label. The signed-out Activity banner promised "the notch settings below still apply to this Mac". Off macOS the notch cards are disabled by `!notchSupported` and everything else is disabled by `signedOut`, so nothing below applies at all; the sentence is dropped there rather than reworded. The popover's notch section header and badge follow `ScopeChip`. They only render on macOS, but they are scope labels for the same rows the settings page chips, and this component exists so the two surfaces cannot say different things about one setting. The group description "the menu bar on this Mac" stays macOS-specific: it is prose about a macOS surface, and "menu bar" already commits it. Its sibling fallback did need fixing. `notchSupported` is false both in the web client and on any non-macOS desktop, and the copy assumed only the first, so a Windows user was told they were running the web client. Based-on: nsxdavid/ADE#999 (cherry picked from commit 30eb87359361f4c920f03ddcb2ff7e36e9b3d04e)
The DPAPI helper gave a Windows PowerShell 5.1 cold start a 5s budget and reported anything slower as "Windows DPAPI credential protection is unavailable" - a hard credential failure. DPAPI is a local sub-millisecond call, so that budget bought process startup: CLR load, System.Security from disk, and Defender's on-access scan of powershell.exe and each assembly the first time they are touched. On a contended machine that runs past 5s. CI shows the shape exactly. In one batch the same job on PR1 ran the file in 2685ms and on PR5 in 3228ms, while PR4 - same 24 tests, same code as PR5 - took 9636ms and lost the first protect() spawn to the deadline. Nothing about PR4 differs; it drew a slower runner. Give the helper a bound that reflects what it is waiting for, and report a deadline separately from an unavailable helper so the sync path stops diagnosing a busy machine as a broken one, matching the async path. Based-on: nsxdavid/ADE#999 (cherry picked from commit d8a15f551a33a594988dbc69805cbde2498a8a95)
macOS reads its keychain material once per process and caches it. The win32 branch returned above that cache, so every credential read spawned a fresh `powershell.exe` -- and unlike `security`, which is a small native binary, PowerShell 5.1 pays CLR load, System.Security from disk, and Defender's on-access scan every time. Concurrent reads each spawned their own, which is what made a cold start slow enough to hit the timeout that was raised in the previous commit; raising it treated the symptom. The cache is keyed by resolved secrets directory, which is where this differs from macOS: keychain material is one global item, but DPAPI material is protected per directory (`<secretsDir>/.credential-key.dpapi`). A single shared slot would hand one store another store's key -- the account-binding test catches exactly that, and caught it here. In-flight dedup is included for the same reason it exists on macOS: without it, concurrent first reads race and each pays the spawn. The negative cache is deliberately not extended to Windows. A locked keychain is a durable state worth backing off from, but a DPAPI failure is usually a transient timeout, and suppressing retries would make one slow cold start look like permanently unavailable credentials. Based-on: nsxdavid/ADE#999
… them ADE reported every agent CLI as installed on Windows. `commandExists` decided "installed" from the exit code of `<cmd> --version`, but `spawnAsync` routes an extension-less command through `cmd.exe /d /s /c "…"`, and cmd.exe always starts: a missing binary comes back as exit 1 with "is not recognized as an internal or external command", never as the ENOENT spawn error (`status === null`) that means "missing" on macOS. Probed on this machine with no Claude Code, cursor-agent or droid installed, all three reported exit 1 and were recorded as installed with a bare `claude`/`droid` path — which then flowed into DetectedAuth and on into `pathToClaudeCodeExecutable`, so Settings advertised a runtime the chat lane could not spawn. Detection and execution now share one answer: `resolveCommandLocation` returns the executable file or null, `installed` is exactly "that file exists", and the reported path is exactly what gets launched. Executable resolution itself assumed a Unix layout. `npm i -g` writes three shims side by side — `codex` (a `#!/bin/sh` script for Git Bash), `codex.cmd` and `codex.ps1` — and `resolveFromDirs` probed the bare name first, so it handed callers the sh script. ADE's own spawns survived because the cmd.exe wrapper re-applies PATHEXT, but anything spawning the resolved path directly (Claude Agent SDK, node-pty, provider SDKs) gets ENOENT. Windows now resolves through PATHEXT only, with `.ps1` as a last resort, executed via PowerShell since cmd.exe cannot run it. Known install dirs gained the generic `%LOCALAPPDATA%\Programs\<cmd>` and `%ProgramFiles%\<cmd>` layouts alongside `%APPDATA%\npm`, `%USERPROFILE%\.local\bin` and the WinGet Links dir, which are the documented Windows homes for these CLIs. Session discovery resolved `$HOME` before `%USERPROFILE%`; under Git Bash that is `/c/Users/<name>`, a path no `fs` call can open, so every provider's session directory silently came back empty. Its POSIX-only command quoter now defers to the shared platform-aware one. Based-on: nsxdavid/ADE#999
The Cursor hard guard only recognised POSIX path shapes, so on Windows a shell tool call written the way Windows shells actually write paths -- `..\..\.ssh\id_rsa`, `.\..\secret.txt`, `%USERPROFILE%\.ssh\id_rsa`, `$env:USERPROFILE\.aws\credentials` -- never produced a path candidate and fell through to "ask" instead of "deny". `.ade\secrets\token` slipped the protected-path check the same way. The POSIX spellings of all five are denied on macOS, so this was a Windows-only hole in the lane containment and secrets guard. Teach `looksLikePathToken` the Windows separator and `%VAR%` / `$env:VAR` home prefixes (win32-gated, since `\` is a legal filename character on POSIX), and expand those prefixes in `resolveCandidatePath` the way `$HOME/` already is. Also fix the pre-existing Windows failure in the policy suite: the transcript-read test hard-coded a POSIX lane root, and `path.resolve` prefixes the current drive on Windows, so the expected slug never matched. Based-on: nsxdavid/ADE#999
`cursorProjectSlugForCwd` hyphenated every separator, so on Windows `C:\Users\me\repo` became `C:-Users-me-repo`. That string can never equal a real directory name -- `mkdir` on it fails with EINVAL -- and `resolveCursorCwdFromSlug` returns null for it, while the drive-less `Users-me-repo` round-trips back to the real path. The structural slug comparison in Cursor CLI session discovery was therefore dead on Windows. Visible effect: a Cursor session whose workspace directory no longer exists is dropped entirely on Windows, because the slug-to-cwd resolver cannot help once the directory is gone and the slug comparison is all that is left. macOS has no drive prefix and keeps the session. The two Cursor cases in the external-session suites built their fixture slugs with POSIX separators only and failed on Windows before this change; they now derive the slug the same way production does. Based-on: nsxdavid/ADE#999
The AI runtimes band hard-coded Cursor's POSIX install one-liner -- `mkdir -p "$HOME/.local/bin" && curl https://cursor.com/install -fsS | bash` -- on every platform. Cursor documents that command for macOS, Linux and WSL only; native Windows has its own installer, and the agent registry already emits the right one. A Windows user copying the onboarding command into PowerShell gets nothing but errors. Derive the command from the renderer platform, matching `cursorInstallCommand()` in the agent registry. Based-on: nsxdavid/ADE#999
Codex's own installer (https://chatgpt.com/codex/install.ps1) unpacks the release into `$CODEX_HOME\packages\standalone\current` and exposes it through `%CODEX_INSTALL_DIR%`, defaulting to `%LOCALAPPDATA%\Programs\OpenAI\Codex\bin`, which it prepends to the *persisted* user PATH. A persisted PATH edit is invisible to an already-running session, so ADE cannot rely on PATH alone. macOS gets this for free: the installer's Unix default is `$HOME/.local/bin`, which the shared known-bin-dir list already carries. The Windows list carries `%ProgramFiles%\Codex` and `%LOCALAPPDATA%\Programs\Codex` instead — neither of which any Codex installer ever writes to. The result was that a standalone Windows install resolved to the bare command `codex`, indistinguishable from "Codex is not installed" and guaranteed to ENOENT on spawn. Probe the installer's real layout from `resolveCodexExecutable` before falling back to the bare command, on every platform, and thread the caller-supplied platform through `pathExists` so the win32 no-execute-bit branch is honoured when the platform is injected rather than inferred. Based-on: nsxdavid/ADE#999
`runCommand` routes every provider CLI through `resolveCliSpawnInvocation`. On Windows a `.cmd`/extensionless launcher cannot be handed to CreateProcess, so the invocation becomes `%ComSpec% /d /s /c "<quoted command line>"` and all arguments collapse into a single string. The Codex and Claude task tests asserted the POSIX shape — a bare launcher path and a flat argv — so both failed on Windows, and the Codex case failed silently in a way that hid the assertion it existed to make: the fake process never found `--output-last-message`, never wrote the result file, and the run came back empty rather than `DONE`. Assert argument content through the same quoting primitive the launcher uses, so the tests describe Codex's actual Windows invocation instead of only its macOS one. Based-on: nsxdavid/ADE#999
Droid names each directory under ~/.factory/sessions with the CLI's own sanitizePathToDirectoryName(), which is not a plain separator swap: posix /Users/dev/ADE -> "-Users-dev-ADE" win32 C:\Users\dev\ADE -> "-C-Users-dev-ADE" (drive colon dropped) discoverDroid used the generic slashEscapedCwd() helper, which coincides with the posix form -- so macOS worked -- but on Windows produced "C:-Users-dev-ADE", a name NTFS can never contain. Every Droid project directory failed the scope filter and no CLI sessions were importable. Reproduced against a session tree laid out exactly as @factory/droid-sdk writes it: 0 sessions discovered before, 1 after. Based-on: nsxdavid/ADE#999
The Windows packaging path assumed a password-protected PFX delivered through WINDOWS_CSC_LINK and WINDOWS_CSC_KEY_PASSWORD. That model cannot exist: since June 2023 CA/Browser Forum rules require code-signing private keys to live on FIPS-validated hardware, exportable .pfx delivery ended for OV and EV alike, and Azure Artifact Signing never releases the certificate at all - it is held in the service and reachable only at the moment of signing. Neither secret is set in this repository and neither ever will be. Rewire run-electron-builder.mjs onto electron-builder 26's native win.azureSignOptions. electron-builder selects the Azure signing manager above the single chokepoint every Windows artifact passes through, so one configuration covers the packaged channel executable, its bundled DLLs, the NSIS installer, and the uninstaller. A post-build signing step could only reach the finished installer, leaving the executable already embedded inside it unsigned. The signed path now requires the AZURE_TENANT_ID / AZURE_CLIENT_ID / AZURE_CLIENT_SECRET triple that Azure.Identity's EnvironmentCredential reads, and hands it to electron-builder only on that path so an unsigned dist:win can never reach the signing service. EnvironmentCredential is first in the DefaultAzureCredential chain, so a complete triple is resolved before any managed-identity probe against an instance-metadata endpoint a GitHub-hosted runner does not have. Pin the publisher by certificate Subject and nothing else. The service renews its certificate daily and expires it after 72 hours, so a pinned thumbprint would fail every release within days. WINDOWS_SIGNING_EXPECTED_THUMBPRINT is therefore refused outright rather than ignored, so a pin cannot quietly stop pinning. The same Subject is passed as electron-builder's publisherName, which electron-updater parses as a Distinguished Name before running a downloaded installer, so the updater and the release validator agree on one publisher. validate-win-artifacts.mjs stays exactly as strict: valid Authenticode status, a trusted RFC3161 timestamp, the pinned Subject, and one certificate shared by the installer and the ADE.exe it installs. Based-on: nsxdavid/ADE#999 (cherry picked from commit d49aa1ec698604a77d4cc54b52afd7f9a416bd17)
…act Signing The standalone runtime signer still imported a PFX from WINDOWS_CSC_LINK and WINDOWS_CSC_KEY_PASSWORD, so it would have failed for the same reason the desktop packaging path did: there is no exportable certificate to import, and Azure Artifact Signing never releases one. Sign through Invoke-TrustedSigning, the same mechanism electron-builder 26 uses for the desktop installer, so both Windows artifacts ADE publishes come from one signing path with one set of parameter names. All local key-material handling is gone; the only credential the script carries is the Entra service principal. Verification is unchanged in strictness: valid Authenticode status, a trusted RFC3161 timestamp against the service's own timestamp authority, and the pinned publisher Subject. WINDOWS_SIGNING_EXPECTED_THUMBPRINT is refused rather than ignored, because a certificate that expires after 72 hours cannot be pinned by thumbprint. Based-on: nsxdavid/ADE#999 (cherry picked from commit d8c26adf40837716d0ecd8e62a5498afc58d2623)
Both release workflows gated Windows on WINDOWS_CSC_LINK and WINDOWS_CSC_KEY_PASSWORD, secrets that are not set and never will be. Replace that job-level requirement with the Azure Artifact Signing service principal: AZURE_TENANT_ID, AZURE_CLIENT_ID, and AZURE_CLIENT_SECRET, the three names Azure.Identity's EnvironmentCredential reads. Microsoft's guidance for runners outside Azure is exactly this triple, because the credential chain otherwise falls through to a managed-identity probe a GitHub-hosted runner cannot answer and raises CredentialUnavailableException. verify keeps failing about a minute in rather than after a full package build, and now also refuses to start while WINDOWS_SIGNING_EXPECTED_THUMBPRINT exists. The service renews its certificate daily and expires it after 72 hours, so leaving a thumbprint pin accepted-but-ignored would look like a pin while pinning nothing. The macOS and Linux release paths are untouched; CSC_LINK and CSC_KEY_PASSWORD remain the macOS Developer ID secrets. Based-on: nsxdavid/ADE#999 (cherry picked from commit d07d802b627b210bf9d7f47f6673c96b97ab3742)
…t Signing The playbook documented a PFX flow that cannot exist and told maintainers to adapt the workflow themselves if they picked a service without one. The pipeline now signs through Azure Artifact Signing, so document that service and the exact resources behind it: the arulsigning account in rg-signing, the East US endpoint, the adePublicTrust certificate profile, and the ade-signing-ci service principal holding the Artifact Signing Certificate Profile Signer role. State plainly why there is no certificate to hold and why the publisher is pinned by Subject: the service holds the key, never releases the certificate, renews it daily, and expires it after 72 hours. Record that WINDOWS_SIGNING_EXPECTED_THUMBPRINT is refused at every layer rather than ignored, and that RFC3161 timestamping is what keeps a shipped installer verifiable past the certificate's three-day life. Correct the audited settings table to the four secrets that now exist, explain why electron-builder's native win.azureSignOptions was chosen over a post-build signing action, and add the failure modes a maintainer will actually hit: a 403 from a lost role assignment, a credential chain that fell past EnvironmentCredential, a build that straddled the daily certificate rotation, and a runner that could not reach PSGallery. Based-on: nsxdavid/ADE#999 (cherry picked from commit ce999120c28ecf3833cbd145cb8a00dc7f3c377d)
GitHub refused to start prepare-release.yml outright: Invalid workflow file: .github/workflows/prepare-release.yml#L112 The nested job 'publish-release' is requesting 'contents: write', but is only allowed 'contents: read'. release-core.yml is called by two entry points with different intents. release.yml is the real release and grants contents: write. prepare-release.yml is the non-publishing validation run and grants contents: read deliberately. GitHub validates a called workflow's job permissions statically, at parse time, before any job-level `if:` is evaluated, so publish-release's `permissions: contents: write` broke the dry run even though its `if: always() && inputs.publish && ...` meant it could never run there. Granting contents: write to prepare-release.yml would have fixed the parse error and destroyed the only thing that actually guarantees a dry run cannot create a release. Its header comment promises "It never creates or updates a GitHub Release"; the read-only token is what enforces that promise, not the hardcoded publish input. So the capability moves instead of the permission. publish-release now lives in release-publish.yml, a reusable workflow carrying its own contents: write, called only by release.yml. Nothing in release-core.yml requests more than contents: read any more, and the publish input is gone with it, so a dry run cannot reach the publishing path at all rather than being trusted to pass publish: false. Do not merge publish-release back into release-core.yml. That would reintroduce this exact failure the next time anyone dispatches the dry run. The publish gate is reconstructed, not relaxed. release-core.yml gains a build-results job with `if: always()` that republishes the three build results the gate reads as workflow outputs, and release.yml's publish-release job applies the same condition it always had: with the Windows gate on, a failed or skipped Windows build blocks the draft exactly as a failed macOS build does, and always() still lets the gate evaluate when build-win-release is legitimately skipped with the gate off. Artifacts are scoped to the run, not the workflow file, so the publish job still downloads ade-mac-release-*, ade-win-release-* and ade-runtime-* by name from the same run. Based-on: nsxdavid/ADE#999
The main-ancestor check ran on every path, including windows_proof -- whose entire purpose is collecting clean-host packaging and signing evidence for a commit that has not merged yet. Requiring the commit be on main first made the mode unusable for what it exists to do. Skipped only for windows_proof. That mode is reachable only from prepare-release.yml, which holds contents: read and cannot create a release or tag, so nothing can ship from a commit this step did not vet. Every publishing path leaves windows_proof false and is still gated, as is the ordinary platform-neutral dry run. Based-on: nsxdavid/ADE#999
CI triggers on pull_request with branches: [main], so it only fires for the one PR in a stack whose base is main. The other four target their parent branch and therefore accumulate no check runs at all -- including ci-pass, which release-core.yml's verify job requires on the exact SHA it builds. That made a signed proof build of a stacked branch unreachable: the gate could never be satisfied. workflow_dispatch runs the same jobs against a given ref and produces the same ci-pass check run on the same SHA, so the release gate is satisfied by real CI rather than weakened to accommodate the stack. Based-on: nsxdavid/ADE#999
…i-shells-providers
…desktop-sync # Conflicts: # apps/desktop/src/main/services/externalSessions/discoverCursor.ts # apps/desktop/src/main/services/externalSessions/discoverDroid.ts # apps/desktop/src/main/services/externalSessions/discoverProviders.test.ts # apps/desktop/src/main/services/externalSessions/externalSessionsService.ts # apps/desktop/src/renderer/components/onboarding/AiRuntimesBand.tsx
gitleaks-action scans only the PR range on pull_request, but the whole repository history on workflow_dispatch. Fingerprints are commit-scoped, so every finding already waived in .gitleaksignore reappears under the SHA of each later commit that touched the same line -- 19 of them here, all previously accepted false positives (a variable named secretPath, a STORAGE_KEY constant, a code comment, Clerk public client IDs). Pinning the dispatched scan to origin/main..HEAD gives it the same scope a pull_request run has, so identical code is judged identically however CI was started. The alternative -- appending 19 more commit-pinned waivers -- would need repeating every time one of those lines moves. Based-on: nsxdavid/ADE#999
gitleaks-action exposes no input for narrowing its commit range, so the previous attempt to pass one through the environment had no effect and the dispatched scan still walked all history. Call the pinned binary directly with --log-opts instead. Verified before pushing: of the six files carrying already-waived findings, five are untouched by this branch's 155 commits, and the change to the sixth adds windowsHide at line ~800 -- far from the flagged line 48. The range therefore contains no secret-shaped content. Based-on: nsxdavid/ADE#999
Same reasoning as the main-ancestor skip directly above it. windows_proof packages and signs a commit that has not merged, to prove the installer before review; requiring a green ci-pass on that exact SHA forces a full CI cycle per iteration and makes the mode unusable for what it is for. The mode is reachable only from prepare-release.yml, which holds contents: read and cannot create a release or tag, so nothing ships from a commit this step did not vet. Real releases leave windows_proof false and remain gated on both checks. Based-on: nsxdavid/ADE#999
…i-shells-providers
@factory/droid-sdk's ProcessTransport.connect() spawns the CLI with only
{ stdio, cwd, env } -- no windowsHide -- and createSession() exposes no way
to pass spawn options through. On Windows that allocates a console for
droid.exe, and with Windows Terminal as the default console host it is not
a brief flash: a real window titled with the droid path opens and stays for
the life of the session.
Reproduced with droid v0.186.0 spawned from a CREATE_NO_WINDOW parent, the
way the Electron-forked worker runs:
without windowsHide -> conhost child of the droid PID, plus a visible
WindowsTerminal window titled
"C:/Users/arul2/bin/droid.exe"
with windowsHide -> console still allocated but hidden; no visible
window
Since the SDK takes no options, default windowsHide for droid spawns only,
matched on the executable basename so no other spawn site in the process
can be affected and an explicit windowsHide from a caller always wins.
Applied in two places, because two of them spawn droid:
- droidSdkWorker, before the SDK is loaded
- droidModelsDiscovery, which calls createSession() in-process and so
spawns droid straight from the Electron main process on a passive
model-warm path
Verified end to end against the built worker: forking the real
dist/droidSdkWorker.cjs from a windowless parent reached "ready" with a
live droid.exe and zero visible droid windows.
Based-on: nsxdavid/ADE#999
(cherry picked from commit 2d4ffab21857c397b6a9f12d419a69d7b0aa2b85)
Verified against a real droid.exe v0.186.0 at C:\Users\arul2\bin with no Factory
account. Forced detection returned authenticated:true, verified:true while
`droid exec "say hi"` returned "Authentication failed. Please log in using
/login or set a valid FACTORY_API_KEY environment variable."
Two compounding causes, both measured here. `droid exec --list-tools` exits 0
with no account at all — it prints the local tool policy and never contacts
Factory — and that was the branch asserting authentication. And the real refusal
string matched none of STRONG_UNAUTH_INDICATORS, so even a genuine failure could
not be recognised. Dropped the probe as an auth signal and added
/authentication failed/i to the shared indicator list.
The two fallback probes were dead code masked by that bug, and expensive.
`droid --help` lists exec, daemon, search, update, mcp, plugin, computer and
help — `whoami` and `account status` are not subcommands, so droid took each as
a *prompt* and booted the full interactive TUI until spawnAsync's timeout reaped
it. A forced refresh spawned a 150MB agent twice to learn nothing. Both are
gone, and CLI_AUTH_PROBES.droid now lists only documented flags: `version` had
the same defect one fallback deeper.
Droid exposes no cheap auth probe — the only authoritative signal is a real
`droid exec` round trip, which costs a model call on a signed-in machine — so
detection now stops at "installed, auth unknown". verified:false keeps this out
of the explicitly-signed-out state, and the connection renders as "installed but
no credentials were detected", which is exactly true. The forced and passive
paths now agree; before, forcing a refresh made the answer worse.
After, forced, with the real binary present:
{"cli":"droid","installed":true,"path":"C:\Users\arul2\bin\droid.exe",
"authenticated":false,"verified":false}
and detection of all four CLIs completes in 954ms.
hasDroidConfiguredCredentials is left as-is but documented: ~/.factory/settings.json
holds UI preferences only ({"logoAnimation":"off"} on this machine) and nothing
credential-shaped exists anywhere under ~/.factory, while a stack trace in
~/.factory/logs names a dedicated CredentialsStorage module. Confirming the real
location needs a signed-in account, so the comment records the evidence and the
invariant that `false` means "no credential ADE can see", never "signed out".
The replaced test asserted the bug in its name.
Based-on: nsxdavid/ADE#999
(cherry picked from commit f189e8cdbeb3ced3bb77331d6c9ff1d820455d3f)
Official Node.js releases are Authenticode-signed on Windows just as they are codesigned on macOS. postject rewrites the executable to embed NODE_SEA_BLOB, so a signature left in place covers bytes that no longer exist and signtool refuses to re-sign the result: SignTool Error: SignedCode::Sign returned error: 0x800700C1 0x800700C1 is ERROR_BAD_EXE_FORMAT. Node's SEA documentation requires removing the signature before injection on both platforms, but removeSignatureIfNeeded returned early for anything that was not darwin, so the step silently did nothing on Windows and every signed runtime build failed. Four platforms built; only win32-x64 died, at signing. signtool.exe ships with the Windows SDK and is not on PATH, so resolve it from the versioned SDK bin directories, newest first. A missing SDK warns rather than fails, keeping unsigned local builds working. Verified on this machine against a real signed node.exe: resolver located signtool 10.0.19041.0, 'remove /s' exited 0, and the binary went from SIGNED to UNSIGNED. Based-on: nsxdavid/ADE#999
…i-shells-providers
|
Too many files changed for review. ( Bypass the limit by tagging |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Important Review skippedToo many files! This PR contains 294 files, which is 194 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (23)
📒 Files selected for processing (294)
You can disable this status message by setting the 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 |
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.
Consolidates the remaining Windows stack into main.
#1006 merged to main directly. #1007-#1010 merged into their parent branches rather than main after the stack was unlinked, so this brings the complete work up in one PR.
Contains every commit from PRs #1007, #1008, #1009 and #1010, plus latest main.
Key fixes since the original stack:
🤖 Generated with Claude Code