Windows 1/5: foundation, durable runtime, and stack workflow - #1006
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Warning Review limit reached
Next review available in: 56 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughChangesThis PR adds Windows foundation support across service supervision, named-pipe IPC, runtime startup, process identity, desktop behavior, development tooling, CI, and review workflows. It also updates platform-neutral wording and cross-platform test infrastructure. Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ade-cli/src/headlessLinearServices.ts (1)
1121-1136: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
requestRawWithCredentialFallbackbypasses the injectedfetchImpl.Every other GitHub request path in this file (
validateToken,probeRepoAccess, bothapiRequestcall sites, and the GitHub App auth service) now routes throughrequestGitHub, which honorsoptions.fetchImpl.requestRawWithCredentialFallbackstill passesfetchImpl: fetchGitHubdirectly.fetchGitHub's third parameter defaults to the globalfetchwhen omitted, so this call always uses the real network fetch, notoptions.fetchImpl.This breaks the stated goal that this diff makes GitHub requests use the injectable transport. Any caller of
requestRawWithCredentialFallbackthat relies on a customfetchImpl(tests, or an alternate transport on a platform without direct network access) silently falls back to real network calls.🐛 Proposed fix to route raw requests through the injected fetch
const requestRawWithCredentialFallback = async ( args: GithubRawRequestArgs, ): Promise<Response> => await requestGithubRawWithCredentialFallback({ ...args, candidates: (await readCredentialInventoryAsync()).candidates, - fetchImpl: fetchGitHub, + fetchImpl: requestGitHub, userAgent: "ade-cli",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/headlessLinearServices.ts` around lines 1121 - 1136, Update requestRawWithCredentialFallback to pass the injected options.fetchImpl through the requestGithubRawWithCredentialFallback call instead of always using fetchGitHub, while preserving the existing credential candidates, user agent, auth-missing message, and fallback logging.
🧹 Nitpick comments (8)
scripts/dev-shared.test.mjs (1)
71-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for the unresolvable npm entry point.
resolveNpmInvocationthrows when no candidate exists. That error message is the only guidance a Windows developer gets for a broken Node installation. No test covers it.Add one case with
pathExists: () => falseand assert the thrown error.💚 Proposed test
+test("reports a clear failure when npm's entry point is missing on Windows", () => { + assert.throws( + () => resolveNpmInvocation(["run", "dev"], { + platform: "win32", + execPath: "C:\\Program Files\\nodejs\\node.exe", + env: {}, + pathExists: () => false, + }), + /Unable to resolve npm's JavaScript entry point/, + ); +});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/dev-shared.test.mjs` around lines 71 - 94, Add a test for resolveNpmInvocation using Windows options and pathExists: () => false, asserting that it throws when no npm entry point can be resolved and validating the resulting error message. Keep the existing successful invocation test unchanged.scripts/dev-shared.mjs (1)
260-270: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReject
npm_execpathvalues that are not JavaScript entry points.The resolved candidate is executed as
node <candidate>.npm_execpathnormally points atnpm-cli.js, but some shells and wrappers set it tonpm.cmdor a shell script. Node then fails with a syntax error instead of the clear message this function provides.Accept only
.js,.cjs, and.mjscandidates.♻️ Proposed guard
const candidates = [ env.npm_execpath?.trim(), path.join(path.dirname(execPath), "node_modules", "npm", "bin", "npm-cli.js"), path.join(path.dirname(execPath), "node_modules", "corepack", "dist", "npm.js"), - ].filter(Boolean); + ] + .filter(Boolean) + .filter((candidate) => /\.(c|m)?js$/i.test(candidate)); const npmCliPath = candidates.find((candidate) => pathExists(candidate));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/dev-shared.mjs` around lines 260 - 270, Update the candidate filtering in the npm entry-point resolution flow around candidates and npmCliPath to accept only existing JavaScript entry points with .js, .cjs, or .mjs extensions, rejecting npm_execpath values such as npm.cmd or shell scripts before selection. Preserve the existing fallback candidates and clear error behavior when no valid path remains.apps/desktop/scripts/dev.cjs (1)
334-341: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winRestart detection can miss builds that share a modification-time tick.
The watcher compares only
curr.mtimeMs. Some filesystems report a coarse modification time. Two successful builds inside the same tick then produce equalmtimeMs, and Electron does not restart. The marker content already contains a unique value, so a content comparison is exact.♻️ Proposed content-based restart trigger
- let lastReadyMarkerMtimeMs = initialReadyMarkerStat.mtimeMs; + let lastReadyMarkerToken = fs.readFileSync(mainReadyMarker, "utf8"); fs.watchFile(mainReadyMarker, { interval: 250 }, (curr) => { if (shuttingDown) return; if (!curr || curr.nlink === 0) return; - if (curr.mtimeMs <= lastReadyMarkerMtimeMs) return; - lastReadyMarkerMtimeMs = curr.mtimeMs; + let token; + try { + token = fs.readFileSync(mainReadyMarker, "utf8"); + } catch { + return; + } + if (token === lastReadyMarkerToken) return; + lastReadyMarkerToken = token; requestElectronRestart("main build completed"); });
initialReadyMarkerStatthen becomes unused at line 282.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/scripts/dev.cjs` around lines 334 - 341, Update the ready-marker watcher around lastReadyMarkerMtimeMs to detect changes by reading and comparing the marker’s content rather than relying on mtimeMs; initialize the previous content from the existing marker and update it before calling requestElectronRestart. Remove the now-unused initialReadyMarkerStat value and retain the shuttingDown and missing-link guards.apps/ade-cli/src/serviceManager/windowsSupervisor.ts (1)
108-108: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueWrite the PID record as UTF-8.
lastLaunchErrorcarries an exception message that can contain non-ASCII characters, for example a localized Windows error or a path with accented characters.[Text.Encoding]::ASCIIreplaces those characters with?. The reader usesfs.readFileSync(pidPath, "utf8"), so UTF-8 without BOM keeps the diagnostic readable and stays JSON-compatible.♻️ Proposed change
- " [IO.File]::WriteAllText($pidPath, ($record | ConvertTo-Json -Compress), [Text.Encoding]::ASCII)", + " [IO.File]::WriteAllText($pidPath, ($record | ConvertTo-Json -Compress), (New-Object System.Text.UTF8Encoding($false)))",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/serviceManager/windowsSupervisor.ts` at line 108, Update the PowerShell PID-record write in the Windows supervisor to use UTF-8 encoding without a BOM instead of [Text.Encoding]::ASCII, preserving non-ASCII characters in lastLaunchError and keeping compatibility with the reader’s utf8 decoding.apps/ade-cli/src/services/projects/machineLayout.test.ts (1)
33-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case that pins the effect of
ADE_RUNTIME_SERVICE_NAME.
windowsChannelIdentityprefersADE_RUNTIME_SERVICE_NAMEoverADE_PACKAGE_CHANNELfor the hashed identity. No test states the expected result when only one process sets that variable. Add a case that documents the intended behavior, for example that{ ADE_HOME: ".ade-beta", ADE_PACKAGE_CHANNEL: "beta" }and the same environment plusADE_RUNTIME_SERVICE_NAME: "com.ade.runtime.beta"produce the same pipe name.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/services/projects/machineLayout.test.ts` around lines 33 - 54, Add a test alongside the Windows channel layout case that compares two resolveMachineAdeLayout calls: one using ADE_HOME and ADE_PACKAGE_CHANNEL, and the other using the same values plus ADE_RUNTIME_SERVICE_NAME set to the beta service name. Assert both results have the same socketPath, documenting the expected service-name identity behavior.apps/ade-cli/src/serviceManager/installWindows.ts (1)
169-174: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider sharing one PowerShell quoting helper.
powerShellSingleQuotedLiteralexists here and inwindowsSupervisor.ts(line 63) with the same escaping rule and different error text. Export one implementation and import it, so a future escaping fix applies to both command builders.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/serviceManager/installWindows.ts` around lines 169 - 174, Consolidate the duplicate PowerShell quoting logic by exporting the existing powerShellSingleQuotedLiteral implementation from one module and importing it in the other, including the Windows scheduled-task and supervisor command builders. Remove the local duplicate and retain identical single-quote escaping and NUL-byte validation through the shared helper.apps/ade-cli/src/serviceManager/installWindows.test.ts (1)
495-526: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case that lets uninstall resolve identity from the command environment.
Every uninstall test passes
serviceNameandlauncherPathexplicitly. That hides the identity-resolution path used in production, whereADE_PACKAGE_CHANNELandADE_HOMEarrive throughcommand.env. Add a case that omitsserviceNameandlauncherPathand asserts that the Run-key value name equals the task name produced by the install path. This case covers the install/uninstall asymmetry flagged ininstallWindows.tsline 602.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/serviceManager/installWindows.test.ts` around lines 495 - 526, Add an uninstall test that omits serviceName and launcherPath, supplies ADE_PACKAGE_CHANNEL and ADE_HOME through command.env, and verifies identity resolution uses the install-path task name as the Run-key value name. Anchor the assertions in uninstallWindowsService and the existing buildWindowsRunKeyQueryArgs helpers, while preserving the current cleanup and call-sequence expectations.apps/ade-cli/src/services/runtime/socketSpawnLock.ts (1)
93-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShare one
isWindowsNamedPipePathimplementation.
localIpcListenOptions.tsuses a function with the same name and the same normalization rule. Export one helper and import it in both modules, so the pipe-detection rule cannot drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/services/runtime/socketSpawnLock.ts` around lines 93 - 99, Consolidate the duplicate isWindowsNamedPipePath implementations by exporting a single helper from the existing shared location and importing it in both socketSpawnLock.ts and localIpcListenOptions.ts. Remove the local duplicate while preserving the current trim, slash normalization, lowercasing, and Windows named-pipe prefix check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agents/skills/test/SKILL.md:
- Around line 69-73: Update the test command instructions so TEST_REVIEW_BASE is
resolved before the empty-feature-hint inference that runs git diff and git
status. Move that inference after the review-base setup, preserving both tracked
and untracked file detection and the existing --base behavior.
- Around line 303-305: Update each parity-pass context step to include untracked
files by appending git ls-files --others --exclude-standard to the existing
changed-file commands. Apply this in .agents/skills/test/SKILL.md at lines
303-305, 376-379, 447-449, and 527-529, and require the docs, mobile, CLI, and
TUI agents respectively to read or inspect those untracked files.
In @.github/workflows/ci.yml:
- Line 417: Update the actions/checkout@v4 step in the CI workflow to set
persist-credentials to false, ensuring the checkout token is not retained in
.git/config before the npm ci commands run.
In `@apps/ade-cli/src/cli.ts`:
- Line 14891: Update the CLI runtime-connect boundary that invokes
repairMachineRuntimeServiceConnection() to catch
RuntimeServiceRecoveryOwnedError and classify it as an expected
connection/conflict failure with a clear CLI message. Preserve the existing null
handling for blocked installs and allow unrelated errors to propagate normally.
In `@apps/ade-cli/src/lib/trustedWindowsTools.test.ts`:
- Around line 69-96: Update the child-process setup around the test’s module URL
and `tsxCli` resolution to invoke `tsx` through the package manager, using the
npm-script-compatible `npm exec tsx --eval ...` command rather than cwd-relative
`src/lib` and `node_modules/tsx/dist/cli.mjs` paths. Preserve the existing
`childScript` contents and result handling while ensuring the same
package-manager-provided binary is used locally and in CI.
In `@apps/ade-cli/src/serviceManager/installWindows.test.ts`:
- Line 175: Update the uniqueness assertion around the Set containing
stableArul, betaArul, and betaOtherUser to assert its size equals 3 rather than
using toHaveLength, ensuring duplicate task names are detected.
In `@apps/ade-cli/src/serviceManager/installWindows.ts`:
- Around line 598-617: Update uninstallWindowsService to resolve serviceName
with serviceCommand and construct the merged runtime environment from env plus
serviceCommand.env, matching installWindowsService and getWindowsServiceStatus.
Use that merged environment when resolving the launcher path and related service
identity so uninstall targets the same task, Run-key entry, and launcher created
by installation.
In `@apps/ade-cli/src/services/projects/machineLayout.ts`:
- Around line 49-78: The Windows pipe identity in windowsChannelIdentity must be
derived from the resolved channel label, matching localRuntimeConnectionPool.ts.
Keep the existing label resolution logic, but return label as identity instead
of using ADE_RUNTIME_SERVICE_NAME or ADE_PACKAGE_CHANNEL directly, so equivalent
channel configurations always use the same endpoint.
In `@apps/ade-cli/src/services/sync/syncHostSingleton.ts`:
- Around line 103-105: Update windowsPidStopCommand and the
buildQuitCommand/detectSyncHostSingletonConflict stop flow to preserve the lock
owner’s process identity, not only its PID. Include identity metadata in the
persisted Windows stop command, and validate that metadata against the current
process identity before invoking Stop-Process; reject mismatched owners without
killing the reused PID.
In `@apps/ade-cli/src/services/sync/syncService.ts`:
- Around line 1468-1473: Update the crdtSyncAvailable-false branch in the
blockingStateText expression to avoid asserting that crsqlite.dll failed to
load. Use a capability-level message covering both unavailable and
runtime-unusable CR-SQLite states, while preserving the existing
live-chat/terminal message and platform branching.
In `@apps/ade-cli/src/test/crrModelPickerWorker.ts`:
- Around line 27-39: Update the worker success path around the JSON output and
process.exit(0) so the process exits only after stdout has finished writing; use
the write callback or drain handling for process.stdout.write. Preserve the
existing serialized payload and Windows teardown behavior, and keep the error
path unchanged.
In `@apps/desktop/src/main/services/attention/attentionNotchHelper.ts`:
- Line 206: Update getHealth() so the unsupported-platform state is returned
before the disabled ADE Notch state, preventing non-darwin users from receiving
enablement instructions for an unavailable feature. Preserve the existing
disabled-state behavior and user-facing copy for supported platforms.
In `@apps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.ts`:
- Around line 2320-2358: Update connectSpawnedRuntime to immediately rethrow
LocalRuntimeCompatibilityError from connectClient instead of storing it in
lastError and retrying. Preserve the existing retry behavior for other
connection errors and retain the compatibility error instance, including its
pid, runtimeVersion, and runtimeBuildHash fields.
In `@scripts/dev-runtime-stop.mjs`:
- Around line 22-26: Update the usage/help text for the socket option near
parseArgs to describe the default as platform-specific instead of stating only
/tmp/ade-runtime-dev.sock, while preserving the existing argument parsing and
resolveDevSocketPath behavior.
- Around line 4-8: Update the cleanup logic in dev-runtime-stop.mjs to skip
fs.unlinkSync(socketPath) when isWindowsNamedPipe is true, while preserving the
existing tcp:// skip guard. Ensure unlinkSync runs only for filesystem socket
paths.
In `@scripts/validate-docs.mjs`:
- Around line 89-105: Update targetExists to reject normalized targets equal to
"/.." or prefixed by "/../" before constructing repoPath or performing
filesystem access. Add a validation fixture covering a relative link such as
"../../outside.mdx" and assert that validation fails.
---
Outside diff comments:
In `@apps/ade-cli/src/headlessLinearServices.ts`:
- Around line 1121-1136: Update requestRawWithCredentialFallback to pass the
injected options.fetchImpl through the requestGithubRawWithCredentialFallback
call instead of always using fetchGitHub, while preserving the existing
credential candidates, user agent, auth-missing message, and fallback logging.
---
Nitpick comments:
In `@apps/ade-cli/src/serviceManager/installWindows.test.ts`:
- Around line 495-526: Add an uninstall test that omits serviceName and
launcherPath, supplies ADE_PACKAGE_CHANNEL and ADE_HOME through command.env, and
verifies identity resolution uses the install-path task name as the Run-key
value name. Anchor the assertions in uninstallWindowsService and the existing
buildWindowsRunKeyQueryArgs helpers, while preserving the current cleanup and
call-sequence expectations.
In `@apps/ade-cli/src/serviceManager/installWindows.ts`:
- Around line 169-174: Consolidate the duplicate PowerShell quoting logic by
exporting the existing powerShellSingleQuotedLiteral implementation from one
module and importing it in the other, including the Windows scheduled-task and
supervisor command builders. Remove the local duplicate and retain identical
single-quote escaping and NUL-byte validation through the shared helper.
In `@apps/ade-cli/src/serviceManager/windowsSupervisor.ts`:
- Line 108: Update the PowerShell PID-record write in the Windows supervisor to
use UTF-8 encoding without a BOM instead of [Text.Encoding]::ASCII, preserving
non-ASCII characters in lastLaunchError and keeping compatibility with the
reader’s utf8 decoding.
In `@apps/ade-cli/src/services/projects/machineLayout.test.ts`:
- Around line 33-54: Add a test alongside the Windows channel layout case that
compares two resolveMachineAdeLayout calls: one using ADE_HOME and
ADE_PACKAGE_CHANNEL, and the other using the same values plus
ADE_RUNTIME_SERVICE_NAME set to the beta service name. Assert both results have
the same socketPath, documenting the expected service-name identity behavior.
In `@apps/ade-cli/src/services/runtime/socketSpawnLock.ts`:
- Around line 93-99: Consolidate the duplicate isWindowsNamedPipePath
implementations by exporting a single helper from the existing shared location
and importing it in both socketSpawnLock.ts and localIpcListenOptions.ts. Remove
the local duplicate while preserving the current trim, slash normalization,
lowercasing, and Windows named-pipe prefix check.
In `@apps/desktop/scripts/dev.cjs`:
- Around line 334-341: Update the ready-marker watcher around
lastReadyMarkerMtimeMs to detect changes by reading and comparing the marker’s
content rather than relying on mtimeMs; initialize the previous content from the
existing marker and update it before calling requestElectronRestart. Remove the
now-unused initialReadyMarkerStat value and retain the shuttingDown and
missing-link guards.
In `@scripts/dev-shared.mjs`:
- Around line 260-270: Update the candidate filtering in the npm entry-point
resolution flow around candidates and npmCliPath to accept only existing
JavaScript entry points with .js, .cjs, or .mjs extensions, rejecting
npm_execpath values such as npm.cmd or shell scripts before selection. Preserve
the existing fallback candidates and clear error behavior when no valid path
remains.
In `@scripts/dev-shared.test.mjs`:
- Around line 71-94: Add a test for resolveNpmInvocation using Windows options
and pathExists: () => false, asserting that it throws when no npm entry point
can be resolved and validating the resulting error message. Keep the existing
successful invocation test unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a9c9e635-d5a3-4e79-8de3-e25ec3655c5b
⛔ Files ignored due to path filters (3)
AGENTS.mdis excluded by!*.mddocs/ARCHITECTURE.mdis excluded by!docs/**docs/playbooks/ship-lane.mdis excluded by!docs/**
📒 Files selected for processing (92)
.agents/skills/quality/SKILL.md.agents/skills/quality/references/ade-review-rules.md.agents/skills/quality/references/correctness-security-review.md.agents/skills/quality/references/thermo-nuclear-review.md.agents/skills/ship/SKILL.md.agents/skills/test/SKILL.md.github/workflows/ci.ymlapps/ade-cli/src/bootstrap.test.tsapps/ade-cli/src/cli.tsapps/ade-cli/src/headlessLinearServices.test.tsapps/ade-cli/src/headlessLinearServices.tsapps/ade-cli/src/lib/trustedWindowsTools.test.tsapps/ade-cli/src/lib/trustedWindowsTools.tsapps/ade-cli/src/multiProjectRpcServer.tsapps/ade-cli/src/serviceManager/common.test.tsapps/ade-cli/src/serviceManager/common.tsapps/ade-cli/src/serviceManager/index.tsapps/ade-cli/src/serviceManager/installWindows.test.tsapps/ade-cli/src/serviceManager/installWindows.tsapps/ade-cli/src/serviceManager/windowsSupervisor.test.tsapps/ade-cli/src/serviceManager/windowsSupervisor.tsapps/ade-cli/src/services/agentRegistry.test.tsapps/ade-cli/src/services/builtInBrowser/desktopBridgeClient.test.tsapps/ade-cli/src/services/modelPickerStore.test.tsapps/ade-cli/src/services/projects/machineLayout.test.tsapps/ade-cli/src/services/projects/machineLayout.tsapps/ade-cli/src/services/projects/projectIconResolver.test.tsapps/ade-cli/src/services/projects/projectRegistry.test.tsapps/ade-cli/src/services/projects/projectRegistry.tsapps/ade-cli/src/services/runtime/brainLoopWatchdog.test.tsapps/ade-cli/src/services/runtime/brainLoopWatchdog.tsapps/ade-cli/src/services/runtime/localIpcListenOptions.test.tsapps/ade-cli/src/services/runtime/localIpcListenOptions.tsapps/ade-cli/src/services/runtime/socketSpawnLock.tsapps/ade-cli/src/services/sync/machineIdentitySigningStore.test.tsapps/ade-cli/src/services/sync/syncHostService.test.tsapps/ade-cli/src/services/sync/syncHostService.tsapps/ade-cli/src/services/sync/syncHostSingleton.test.tsapps/ade-cli/src/services/sync/syncHostSingleton.tsapps/ade-cli/src/services/sync/syncLoopbackCollision.test.tsapps/ade-cli/src/services/sync/syncPairingStore.test.tsapps/ade-cli/src/services/sync/syncService.test.tsapps/ade-cli/src/services/sync/syncService.tsapps/ade-cli/src/test/crrModelPickerWorker.tsapps/ade-cli/src/test/filesystem.tsapps/ade-cli/src/tuiClient/app.tsxapps/ade-cli/src/tuiClient/imageTargets.tsapps/desktop/scripts/dev.cjsapps/desktop/src/main/main.tsapps/desktop/src/main/services/attention/attentionAccountCoordinator.test.tsapps/desktop/src/main/services/attention/attentionAccountCoordinator.tsapps/desktop/src/main/services/attention/attentionNotchHelper.test.tsapps/desktop/src/main/services/attention/attentionNotchHelper.tsapps/desktop/src/main/services/computerUse/localComputerUse.test.tsapps/desktop/src/main/services/computerUse/localComputerUse.tsapps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.test.tsapps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.tsapps/desktop/src/main/services/remoteRuntime/runtimeRpcClient.tsapps/desktop/src/main/services/runtime/projectRecoveryService.tsapps/desktop/src/main/services/shared/utils.tsapps/desktop/src/main/services/storage/diskPressure.tsapps/desktop/src/main/windowAppearance.test.tsapps/desktop/src/main/windowAppearance.tsapps/desktop/src/renderer/components/activity/ActivitySettingsPopover.test.tsxapps/desktop/src/renderer/components/activity/ActivitySettingsPopover.tsxapps/desktop/src/renderer/components/activity/ActivitySettingsPopover.windows.test.tsxapps/desktop/src/renderer/components/activity/HeaderActivityControl.test.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.test.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/lanes/CreateLaneDialogHostBinding.test.tsxapps/desktop/src/renderer/components/lanes/laneMachines.test.tsapps/desktop/src/renderer/components/lanes/laneMachines.tsapps/desktop/src/renderer/components/settings/ActivitySection.test.tsxapps/desktop/src/renderer/components/settings/ActivitySettingsControls.tsxapps/desktop/src/renderer/lib/platform.test.tsapps/desktop/src/renderer/lib/platform.tsapps/desktop/src/renderer/main.tsxapps/desktop/src/shared/machineIdentity.tsapps/desktop/src/shared/types/attention.tsapps/desktop/src/shared/types/core.tsapps/desktop/src/shared/types/sessions.tsapps/desktop/src/shared/types/sync.tsapps/desktop/tsup.config.tsapps/ios/ADE/Shared/AttentionActionIntents.swiftapps/ios/ADE/Views/Activity/ActivityDrawerModel.swiftscripts/dev-desktop.mjsscripts/dev-runtime-stop.mjsscripts/dev-shared.mjsscripts/dev-shared.test.mjsscripts/run-desktop-test-shards.mjsscripts/validate-docs.mjsscripts/validate-docs.test.mjs
Based-on: nsxdavid/ADE#999
Move the shared runtime initialize timeout contract into the foundation layer while deferring release-repository wiring to packaging. Co-authored-by: David Whatley <nsxdavid@gmail.com> Based-on: nsxdavid/ADE#999
Based-on: nsxdavid/ADE#999 Co-authored-by: David Whatley <nsxdavid@gmail.com>
Based-on: nsxdavid/ADE#999 Co-authored-by: David Whatley <nsxdavid@gmail.com>
Based-on: nsxdavid/ADE#999 Co-authored-by: David Whatley <nsxdavid@gmail.com>
Prevent the CLI from launching a competing manual brain while a registered supervisor owns readiness recovery, and make stack readiness block on exact-SHA proof required at the current position. Co-authored-by: David Whatley <nsxdavid@gmail.com> Based-on: nsxdavid/ADE#999
Route the CLI brain listener through intended-user named-pipe options and replace an empty CI filter with a real native Windows contract test. Co-authored-by: David Whatley <nsxdavid@gmail.com> Based-on: nsxdavid/ADE#999
Resolve ADE-owned PowerShell, registry, task scheduler, and process termination commands through the kernel SystemRoot alias, validate their canonical System32 paths, and persist the absolute PowerShell path for startup supervision. Add native Windows cwd, PATH, SystemRoot, and windir poisoning regressions covering service and clipboard launch paths. Based-on: nsxdavid/ADE#999
Move platform-neutral Activity naming and native Notch gating into the foundation layer so every stacked diff is internally testable. Co-authored-by: David Whatley <nsxdavid@gmail.com> Based-on: nsxdavid/ADE#999
Based-on: nsxdavid/ADE#999
Based-on: nsxdavid/ADE#999
Stack-ready mode was attestation-only: it required a coordinator-created PR, refused to commit, push, open a PR, or fix red CI/review, and converted actionable feedback into a stack-coordinator-fix-required bail-out. A lane could not reach ready-stacked without a human doing its work first. Stack mode now runs the same loop as ordinary /ship — Phase 0 through Phase 5, the same poll/decide/fix machinery, the same 5-iteration budget — and stops one step short of landing. The lane commits and pushes its own layer branch, opens its PR against the resolved direct parent, polls CI and review bots, fixes red CI and verified findings on its own layer, and rebinds commit-bound quality revalidation to the exact resulting head. Reserved to the coordinator: rebases and restacks, gh stack sync/rebase/push/submit, base retargeting, and the merge. Force-finalize and every bypass-review path stay off in stack mode; the spent iteration budget escalates instead of forcing. The stack-coordinator-* states remain, re-scoped to what the lane genuinely cannot do, with a Stack escalation states table naming the exact case and the coordinator action for each. ready-stacked can no longer be recorded over a known gap. Every required proof scenario needs a link bound to the validated head, or a deferredProofScenarios entry naming a higher position and branch that exist in this stack; the top layer defers nothing, and the state-file jq check enforces it. Based-on: nsxdavid/ADE#999 (cherry picked from commit d0f922e10d292c1fd887ac19c82a8736b676def1)
The pre-channel Windows installer registered a single global "ADE Runtime" scheduled task. Install and uninstall both removed that name unconditionally before touching the channel-scoped task, so a Beta install, repair, or uninstall would end and delete a Stable channel's legacy runtime task -- a side-by-side isolation violation that silently kills a running Stable brain. Legacy migration is still needed, so gate it on ownership instead of dropping it: query the legacy task's actions and migrate only when its Execute path and CLI entry match the current channel's serve command. A legacy action never carried the runtime environment, so ADE_HOME is not recoverable from it; the packaged executable plus entry script is the ownership evidence that survives. Requiring both matters because development builds of every channel share process.execPath. When the action cannot be read the operation fails rather than guessing, and a foreign legacy task is left running untouched. The existing migration test asserted the broken behavior; it now pins the owned-task path, and new regressions assert the recorded schtasks argv for a Beta install and uninstall that find a Stable-owned legacy task. Based-on: nsxdavid/ADE#999 (cherry picked from commit d094524ef1662382ac09a3160584c3e7a2ea4beb)
getWindowsServiceStatus claimed "A legacy ADE Scheduled Task is installed... Run `ade brain start` to migrate it", but it only ever queried the channel-scoped task name, never the global pre-channel "ADE Runtime" task. The message could not fire for the situation it described. This matters more now that legacy migration is ownership-gated: a legacy task belonging to another install is deliberately left running, and until now no command anywhere told the user it was there. Status now probes the global task on the otherwise-"not installed" path, where the answer is actionable and no extra spawn is added to a healthy channel, and reuses isWindowsLegacyTaskOwnedByCommand to separate three real states: a task this channel owns (migratable via `ade brain start`), one belonging to a different ADE install (left running on purpose), and one whose owner could not be determined. The pre-existing channel-scoped message now says so explicitly so the two cannot be confused. The probe is supplementary, so its failure degrades to "no legacy task detected" instead of turning an answerable status into an error. Based-on: nsxdavid/ADE#999 (cherry picked from commit f2349cc170b12789796f217f5fd73a1f7ba4a91e)
validate-docs.mjs fed native `path.relative()` output straight into its route/link namespace. On Windows every doc id came out backslash-separated (`configuration\ai-providers`), so no `.mdx` route ever matched a forward-slash link target and the `docs/` prefix test never fired -- 248 phantom errors, exit 1, and all 77 `docs/**/*.md` files silently skipped. Linux CI never saw it. Route ids are a URL-ish namespace and are now forward-slash on every host. `toRouteId()` is the single boundary where a native path becomes an id; `targetExists()` and the doc reader convert back to native segments before touching the filesystem. `toRouteId()` is the identity function when `path.sep === "/"`, so POSIX behaviour is bit-for-bit unchanged. Also sorts directory entries so error output is ordered identically on both platforms, and extracts the pure helpers behind an `isDirectRun` guard (matching scripts/posthog/provision.mjs) so the separator handling is unit-testable from a POSIX runner. Windows now reports 0 errors / exit 0, matching ubuntu CI. No validation rule was loosened. Based-on: nsxdavid/ADE#999 (cherry picked from commit 3689bebce3a489ef3e5fbaa18fc18574be72cddd)
PR1 added Windows runtime startup timing and a `connectSpawnedRuntime` retry path, but the native Windows CI job ran only three unrelated desktop files, so none of it was gated. Adding the owning suite to that job first required making the suite runnable on Windows: it was 50 passed / 12 failed natively under Node 22. All 12 were test-layer portability defects, not production bugs: - Eight daemon-backed tests hardcoded `<ADE_HOME>/sock/ade.sock`. Node maps a path-style endpoint onto a named pipe on Windows, so a filesystem path is not a connectable address and each died with `connect ENOENT`. They now derive the endpoint through `resolveMachineAdeLayout`, exactly as production does, which is byte-identical on macOS/Linux and yields the real per-user pipe on Windows. - Those same eight also hit a second, stacked failure: the tsx loader was injected as `--import <absolute path>`. Node's ESM loader rejects a Windows absolute path (ERR_UNSUPPORTED_ESM_URL_SCHEME, "Received protocol 'c:'"), killing the spawned daemon before it could listen and surfacing only as a downstream connect error. It now passes a file:// URL, which is equally valid on POSIX. - The NODE_PATH test compared against forward-slash literals although the helper joins with the host path module; it now joins the expectation the same way. - Two release-build-output tests compared raw POSIX literals against values the production helpers return `path.resolve`d. No assertion was dropped or skipped; the POSIX expectations are unchanged. The suite is now 62 passed / 0 failed on Windows, and it is gated by a new step in the windows-foundation job. The readiness wait is also given the same Windows budget production already uses for runtime startup (30s vs 10s), since a cold Windows daemon start is genuinely slower. It is a ceiling rather than a sleep, so the suite still finishes in ~38s locally. Based-on: nsxdavid/ADE#999 (cherry picked from commit fa2f30e2a9237b2350a9e9c7a5c0ae820e0e29c9)
A "This Mac" -> "This computer" copy sweep also matched the substring inside the machine-name fixture "This MacBook", leaving the nonsense name "This computerBook" in the Attention coordinator suite. That silently weakened the copy/parity regression: the assertion that the degraded-availability message names the host machine was checking for a string no machine could ever be called, so it could no longer catch the host name being dropped or replaced. These are user machine *names* (test data), not product copy, and were never in the sweep's scope: the same file's "Studio Mac" and "First Mac" fixtures are untouched, and the identical "This MacBook" fixture on the inline override at line 85 survived. Restoring the name makes the file self-consistent again. Swept the repo for other collateral damage from the same pass; these were the only three occurrences. Every remaining `computer`-adjacent token is a legitimate pre-existing identifier (`computerUse`, and the `desktopcomputer` / `laptopcomputer` SF Symbol names on iOS). Based-on: nsxdavid/ADE#999 (cherry picked from commit 25df8905cd784637e38db812a5f67d0c269e660c)
…nt guard
`isPrimaryMachineRuntimeSocketPath` refuses to spawn an app-owned brain
onto the machine's primary runtime endpoint. On Windows it was
defeatable, because it compared named pipes as raw strings.
Win32 accepts `/` and `\` interchangeably in a pipe path and matches
pipe names case-insensitively, so `\.\pipe\ade-runtime-stable-abc`,
`//./pipe/ade-runtime-stable-abc`, and a case variant all address one
pipe. Verified on Windows 11 / Node 22.13.1: a server listening on the
first accepts connections addressed to the other two.
`isAdeRuntimeNamedPipePath` already encodes this by accepting both
separator forms after lowercasing, but `normalizeComparableSocketPath`
returned pipe paths verbatim, so equivalent spellings compared unequal.
Because `createConnection` honours an operator-supplied
ADE_RUNTIME_SOCKET_PATH, any equivalent spelling of the layout pipe slid
past the guard and got a second brain on the machine's primary sync
endpoint -- the split-brain the guard exists to prevent.
Pipe paths are now folded to a lowercase, backslash-separated
comparison key. That key is used only to compare: it is built inside a
module-private helper whose sole consumer returns a boolean, and every
address that is connected to, listened on, probed, logged, or reported
still comes from the caller's original `socketPath`, so the lowercasing
cannot reach a real endpoint. POSIX socket paths keep `path.resolve`
and stay case-sensitive, since `/tmp/ADE.sock` and `/tmp/ade.sock` are
genuinely different files.
`defaultChannelRuntimeSocketPaths` -- the cross-channel half of the same
guard -- was separately dead on Windows. It hardcoded
`~/.ade{,-alpha,-beta}/sock/ade.sock`, addresses that never exist there,
so a Windows user running Stable and Beta together got none of the
protection macOS users get. It now derives each channel endpoint through
`resolveMachineAdeLayout`, the same production derivation used
everywhere else, which is byte-identical on POSIX. ADE_PACKAGE_CHANNEL
and ADE_RUNTIME_SERVICE_NAME are dropped while enumerating: both
outrank the home-name-inferred channel, so leaving them in would pin all
three homes onto the running channel's own pipe.
Regressions assert the guard's decision through `createConnection`, not
the normalizer's return value, with `spawnRuntime` stubbed to throw so a
guard that lets the spawn through fails loudly. All three Windows cases
fail without this change; the cross-channel one reported
`blocked: false, spawnAttempted: true` with requested
`\.\pipe\ade-runtime-beta-*` against layout
`\.\pipe\ade-runtime-stable-*`. A POSIX-only case pins that the pipe
canonicalization does not bleed onto case-sensitive filesystem sockets.
Based-on: nsxdavid/ADE#999
(cherry picked from commit c95efe27257125840f7bc4204156be86b028bed2)
`scripts/validate-docs.test.mjs` (7 tests, `node --test`) covers the docs validator that the `validate-docs` job runs, but nothing invoked it. Added its path to the existing `node --test` step in `typecheck-ade-cli`, alongside the archive and packaging guards. The file arrives with a sibling lane and does not exist on this branch, so this step cannot pass until the stack is composed; it is wired blind by design and needs verifying after composition. Based-on: nsxdavid/ADE#999 (cherry picked from commit 021e865baeaa4a50885b66c474bba035ff088a55)
…fires
`readParentPid` is the default ancestry backend for
`isCurrentProcessDescendantOfPid`, which `runtimeSelfShutdownBlock` and
`runtimeServiceSelfShutdownBlock` use to refuse `ade runtime repair` /
`ade runtime stop` when the command was issued from a shell running inside
the runtime it is about to kill. It only ever ran `ps -o ppid= -p <pid>`.
On Windows that query cannot succeed. Git Bash ships a `ps` that is found
and then rejects the POSIX flags (status 1, no error code); a box without
Git Bash reports ENOENT. Either way `status !== 0` returned null, the
ancestry walk terminated on its first iteration, and the guard never fired
— a user tore down their own live runtime and every active session on it
with no warning.
Dispatch per platform, matching how `serviceManager/index.ts` dispatches
`getRuntimeServiceMainPid`. The win32 branch reads ParentProcessId from
`Get-CimInstance Win32_Process` (not `wmic`, which is deprecated and being
removed from Windows) through `resolveTrustedWindowsTool("powershell")`,
so a poisoned PATH/SystemRoot cannot redirect a query that gates a
destructive operation.
Ancestry lookups now distinguish "no further parent" from "the query could
not be answered". The guard fails CLOSED on the latter: a false block costs
one refusal that names the ADE_ALLOW_RUNTIME_SERVICE_SELF_MUTATION=1
override, while a false allow destroys live state. Only the first is
recoverable. POSIX keeps its previous end-of-chain behaviour because `ps`
exits 1 for both cases and cannot tell them apart.
The existing tests injected a fake `parentPid` on all three cases, so
`readParentPid` was never reached and the Windows CI job stayed green over
a symbol that could not work. The added coverage drives the real default
backend, plus two zero-mock cases that check the resolved parent against
`process.ppid` on a real Windows host.
Based-on: nsxdavid/ADE#999
(cherry picked from commit c8d6e4d0f951c6583ed806a13790cbfbc3a5a285)
`inspectSyncListenerPort` shelled out to `lsof` and `ps` unconditionally. Neither exists on Windows — and under Git Bash `ps` is found but rejects the POSIX flags — while `execFileText` swallows the failure and returns null, so every Windows diagnosis came back with an empty `holders`. Two consequences follow from that empty list. `createSharedSyncListener` diagnoses a port only after EADDRINUSE, and with no holders `findStaleHolder` can never recognise a wedged same-channel sibling, so the reclaim path is dead and mobile sync drifts onto a fallback port permanently — the port paired phones have saved is never recovered. The same empty list also stops the port being marked occupied, so the bind burns all eight preferred-port retries before drifting. And `ade doctor` fell through to its "no holders visible to this user" branch, which advises `tailscale serve status` and a root-owned tailscaled holder: advice that cannot apply on Windows. Dispatch per platform. The win32 branch pairs `Get-NetTCPConnection` with `Get-CimInstance Win32_Process` in a single PowerShell invocation, resolved through `resolveTrustedWindowsTool`, and returns the pid, command line, and an ISO creation time for the PID-reuse guard. One spawn replaces the 1 + 2N the POSIX path needs. `Get-NetTCPConnection` is the supported replacement for scraping `netstat`; when it is unavailable the query simply yields no holders, which is the current behaviour, so a `netstat -ano` fallback would add a PATH-reachable executable for no coverage gain. The 200ms budget that suits lsof/ps would kill every PowerShell query before it answered — the real query takes ~2s on a warm host — so the timeout is now per-call. Added coverage drives the real dispatch rather than the injected `inspectPort` seam the existing reclaim test uses, including a zero-mock case that binds a port and asserts this process is named as its holder on a real Windows host. Based-on: nsxdavid/ADE#999 (cherry picked from commit 8144099e63817d3fd24439584b849f83f687f0ef)
A platform-gated test assertion could exist with no CI runner that ever executed it, and nothing detected that. The gate reported as "skipped" on every runner the repo has -- or, in the `if (process.platform === "win32") return;` form, as a green pass -- so the file's presence in a job read as coverage that did not exist. scripts/validate-platform-gates.mjs parses every apps/**/*.test.ts(x) for platform gates in each form this repo uses: it.skipIf / it.runIf, the ternary-to-it form, alias constants such as `itUnix` and `crdtHostIt` whose call sites read as a plain `it(...)`, and the vacuous early `return`. It then parses .github/workflows/ci.yml for which test files each job runs and on which runs-on, and fails when a gated assertion has no runner -- including when no job for that platform exists at all, which is the case today for macOS. Adoption is incremental. scripts/platform-gate-baseline.json records the 16 known violations, keyed on (file, kind, form, requires) rather than line number so it survives edits. It is a ratchet: a new or grown entry fails immediately, and an entry that shrank fails too, with an instruction to re-record, so the backlog cannot go stale. `// WINDOWS-GATE: <reason>` and `// DARWIN-GATE: <reason>` are the documented escape hatch. The check is dependency-free and rides in the existing validate-docs job; its node --test suite joins the existing typecheck-ade-cli step. No new job. Based-on: nsxdavid/ADE#999 (cherry picked from commit 9105ca507743aac98cf3d5bc15170261d6930378)
windows-foundation ran 17 files. Two groups were missing.
The suites docs/development/windows-port-lane.md names as the Windows
validation -- pathUtils and processExecution, plus the window, app-control,
and auto-update services -- had an empty intersection with this job, so the
documented gate had never actually run on Windows. Measured here: 5 files,
87 tests, ~4.3s.
trustedWindowsTools is a security control whose only substantive case is
win32-gated, so before this it ran on no runner at all; the credential store
and the `ade://` deeplink command-injection guard are both filesystem- and
quoting-sensitive in ways a Linux-hosted job cannot exercise. Measured here:
3 files, 54 tests, ~3.5s.
All eight files verified green on a native Windows 11 host under Node
22.13.1. Total measured wall-clock added: 8-13s warm against a 25 minute
budget.
Deliberately not added:
- ptyService.test.ts, 81 of 354 cases fail natively -- the reap path
asserts POSIX process-group signalling, `kill(-pid, "SIGKILL")`;
- cli.test.ts, 5 of 340 fail natively on POSIX-absolute path assumptions
such as expecting "/explicit/project-root" where Windows resolves
"C:\explicit\project-root";
- kvDb.rebuildRecovery.test.ts and the CR-SQLite/kvDb group, where 12
cases fail with EBUSY on `fs.rmSync` because a SQLite handle is not
closed before teardown -- fatal on Windows, a no-op on POSIX.
Each needs a source fix before it can join a required job.
Adding trustedWindowsTools to the job retires its two baseline entries, so
the platform-gate baseline shrinks from 16 violations to 14.
Based-on: nsxdavid/ADE#999
(cherry picked from commit a4a30a980099ce9b07cc72c3b4b56ba1562858d8)
…elper PR1 renamed the local machine's absolute display name from the macOS-only "This Mac" to the platform-neutral "This computer" and moved it behind `shared/machineIdentity.ts`, because ADE now runs on Windows and calling a Windows box "This Mac" is wrong. Four renderer tests still spelled the old literal and failed against the new implementation. Rather than re-typing the new string, the assertions now import THIS_MACHINE_NAME and compose from it: TopBar matches the machine menu on a regex built from the constant (the menu appends a lane count, so the accessible name is a substring), PersonalChatsPage and ProjectlessSidebar interpolate it into the picker's aria-label sentence, and SessionCard's hover-card row checks it directly. A future rename of the label now moves these tests with it instead of breaking them a fourth time. ProjectlessSidebar was not failing — it feeds the label in as a prop — but it is the same fixture and is switched over for the same reason. The SessionCard case is renamed to "on this computer": it describes local-machine behaviour, not Apple hardware. Docs that quoted the constant's value, the machine picker's option label, or the push-divergence warning are corrected; the "This Mac" card in Connections is a component name whose UI copy PR1 did not change, so those references stand. Based-on: nsxdavid/ADE#999 (cherry picked from commit d6fc636843a904512cadfa32ebc9d6525784f333)
The docs-validator guard landed with the foundation layer, so the path it runs resolves on this branch. The note described a transient state during stack composition. Based-on: nsxdavid/ADE#999
The holder lookup is win32-gated: on Windows it goes through PowerShell rather than lsof/ps, and a broken lookup silently disables stale-port reclaim instead of failing. Only a Windows runner executes that gate. Caught by validate-platform-gates at stack composition -- the gate and the checker landed in separate lanes, so neither could see the gap alone. Based-on: nsxdavid/ADE#999
`windows-foundation` failed five projectIconResolver cases on every stacked
PR. The GitHub runner's account is `runneradmin`, so `os.tmpdir()` reports the
8.3 short form `C:\Users\RUNNER~1\AppData\Local\Temp`. The fixtures
canonicalized that with the JS `fs.realpathSync`, which resolves symlinks but
leaves 8.3 aliases alone, while the resolver canonicalizes with
`fs.realpathSync.native`, which expands them. Every `path.join(root, …)`
expectation then compared two spellings of one directory.
Point both icon fixtures at `fs.realpathSync.native` so they build roots in the
spelling the resolver answers in.
The investigation also turned up a real defect behind the fixture mismatch.
`resolveProjectIcon`, `resolveProjectIconPath`, `setProjectIconOverride`,
`removeProjectIconOverride` and `setProjectIconOverrideFromSelection` normalized
the project root with `path.resolve` alone, while every path they return is
realpath-canonical. A root that arrives as an 8.3 short name — or through a
junction or symlink — therefore makes `toProjectRelative` emit a `..`
traversal, and `setProjectIconOverride` persists that traversal into the
shared, committed `.ade/ade.yaml` as the project's `iconPath`:
iconPath: ../../../../shortpath-repro/runneradmin/Temp/…/brand/custom-icon.png
Today's shipped callers all pre-canonicalize (the desktop IPC through
`resolveAllowedProjectRoot`, the CLI through `normalizeProjectRootPath`), so
this is latent rather than reachable — but the resolver never stated that
precondition and cannot rely on it. Canonicalize the root inside the module
with the realpath it already uses for candidates, and cover it with a desktop
test that drives a junction-spelled root; that test fails on the old code on
every platform. A matching CLI test pins the "icons come back in the
filesystem's spelling" contract the fixtures depend on.
Verified by reproducing the runner exactly: with `TEMP` pointed at a real 8.3
short path, the step's twelve suites failed 5/183 before and pass 189/189 after.
Based-on: nsxdavid/ADE#999
(cherry picked from commit 106cb1d69f2e748c14cd93832647055a77e67ce6)
arul28
force-pushed
the
codex/windows-foundation-runtime
branch
from
August 2, 2026 17:51
74f7ca4 to
c70076e
Compare
`isCrsqliteAvailable()` resolves `vendor/crsqlite/<platform>-<arch>/`, and only darwin-arm64, darwin-x64, and win32-x64 are vendored. `test-desktop` runs on ubuntu-latest, where that gate is false, so 57 tests across kvDb, kvDb.migrations, kvDb.sync, deviceRegistryService, syncHostService, and syncService skip silently — including two files that skip in full. The ubuntu shard logs confirm it: syncService 19/19 skipped, kvDb.sync 10/10 skipped, syncHostService 17 of 24, kvDb 9 of 20, and one `it.skipIf` each in kvDb.migrations and deviceRegistryService. No other job picked them up, so the CRDT replication, sync host, and device registry contracts had no coverage on any runner. windows-foundation already runs on windows-latest with the vendored crsqlite.dll present, so it is the one place these can execute. Run the six suites there. Locally all 88 tests execute with zero skips. kvDb.rebuildRecovery.test.ts is deliberately excluded: its temp-dir teardown unlinks a still-open SQLite handle and fails EBUSY on Windows. Based-on: nsxdavid/ADE#999 (cherry picked from commit e487c79457e66dc86657e5a8bd6c471ef7b23a7d)
`windows-foundation` failed four localRuntimeConnectionPool cases on every
stacked PR, all with the same shape:
expected 'C:\Users\runneradmin\AppData\Local\Te…'
to be 'C:\Users\RUNNER~1\AppData\Local\Temp\…'
Same bug class as 106cb1d6. The GitHub runner's account is `runneradmin`, so
`os.tmpdir()` reports the 8.3 short form `C:\Users\RUNNER~1\AppData\Local\Temp`.
Five assertions compared `fs.realpathSync(registered.rootPath)` against
`fs.realpathSync(projectRoot)`, and the JS `fs.realpathSync` resolves symlinks
but leaves 8.3 aliases alone. The runtime registers roots through
`normalizeProjectRootPath` -> `realpathIfExists` -> `fs.realpathSync.native`,
which does expand them, so the two sides named one directory with two
spellings. The earlier 8.3 sweep predated the `resolveMachineAdeLayout`-derived
socket path that let these daemon-backed tests run on Windows at all, so it
never saw them.
This is a fixture defect, not a production one: the value coming back over the
wire is the canonical long form, which is what the registry is supposed to
return. Point the fixtures at the runtime's own `realpathIfExists` through a
single named helper, and route the one assertion that already reached for
`fs.realpathSync.native` through it too, so the file has one canonicalizer
rather than three spellings of the same intent.
Reproduced the runner exactly by pointing `TEMP`/`TMP` at a real 8.3 path: the
suite went 4 failed / 61 passed / 1 skipped before and 65 passed / 1 skipped
after, and is unchanged at 65 passed / 1 skipped under a normal `TEMP`.
Swept the rest of the file and every other suite in the `windows-foundation`
job for the same JS-vs-native mismatch; these five were the only ones. The
remaining `fs.realpathSync` fixture in `apps/ade-cli/src/bootstrap.test.ts`
matches its production counterpart, which also uses the JS realpath, so both
sides agree in either spelling.
Based-on: nsxdavid/ADE#999
(cherry picked from commit f065f68950aa878ea9007ab3ed14bba8c19abf02)
The windows-foundation "service, layout, and IPC contracts" step went red on
a runner that was ~6x slower than its peers: the same 12 files finished in
17.98s on the sibling job for the branch stacked directly on top of this one,
and in 115.50s here. Nothing in the composed tree explains it — the failing
files' import graph contains no behavioural change from this layer — but every
win32-gated probe that waits on a real detached subprocess has a hard-coded
deadline with no headroom, so a slow host turns each of them into a null read:
- readWindowsParentPid gave powershell.exe + the first CIM call of the
session 5s. On a loaded host that expires, and because a timeout is
indistinguishable from a broken mechanism the lookup degrades to
PARENT_PID_UNKNOWN. That is a product bug, not just a test one: the guard
fails closed, so a busy machine makes ADE refuse a runtime teardown the
user is entitled to. The ancestry walk stops at the first unknown, so the
wider budget is spent at most once per chain.
- The supervisor, bootstrap, and CRR-worker specs waited 5s/5s/15s on a
detached PowerShell or a cold tsx child, then asserted on whatever had been
written by then. The assertions are unchanged; only the patience is.
- The bootstrap spec's teardown read the pid record once to find the
supervisor to kill. Losing that race against the supervisor's first
Write-PidRecord orphaned a process that kept restarting its child for the
rest of the run, holding the temp tree open (EBUSY on unlink) and starving
every later suite. Teardown now waits for the record before giving up.
Reproduced by pinning the run to two logical CPUs under contention, which
yields the same "expected null to be +0" and "expected 'unknown' to be <pid>"
failures seen across this branch's runs; the same starved run is green after,
and leaves no orphaned supervisor.
Based-on: nsxdavid/ADE#999
(cherry picked from commit d20d7363826a0108905ab719684ce590322b911f)
…robe The readiness probe matched the runtime's command line against '(?:^|\s)serve(?:\s|$)', requiring the verb to sit between whitespace or string boundaries. Windows quotes spawned arguments, so a live brain's command line actually ends: "...\node.exe" "...\cli.cjs" "serve" The verb is wrapped in quotes, which are neither whitespace nor a boundary, so the predicate was always false and the probe exited 4 -- reporting a healthy runtime as "stale or does not match this channel executable". Found by running `ade brain start` on Windows, not by a test. It failed after 17.8s claiming the brain never became ready, while the brain was in fact running and serving on its named pipe. `ade brain status` then contradicted itself in one response: the runtime section reported the pid healthy while the service section called that same pid stale. A user would reasonably have repaired a working runtime. Verified against the live brain: with no restart and no other change, service.running went false -> true and the diagnostic became "ADE per-user channel brain is ready". Optional quotes still reject a different verb (serveless) and a different subcommand (rpc --stdio). Based-on: nsxdavid/ADE#999
`ade brain start` launched the PowerShell supervisor with `Process.Start` / `ShellExecuteEx`, which makes it an ordinary descendant of whoever ran the command. On Windows, job-object membership is inherited and cannot be escaped: `CREATE_BREAKAWAY_FROM_JOB` fails with ERROR_ACCESS_DENIED unless the job sets `JOB_OBJECT_LIMIT_BREAKAWAY_OK`, which `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` jobs do not. Terminals, editors, CI agents and Electron all place their children in exactly such jobs, so when the session that installed the brain went away Windows called TerminateProcess on the supervisor *and* the brain it guards -- no exit code, no log, no restart, and no chance for the supervisor's `finally` to run. A later `brain status` then cleared the now-stale PID record and reported "The startup entry has no valid PID record yet", erasing the last evidence that anything had ever been running. macOS never had this failure because `launchctl load` hands ownership of the process to launchd rather than to the invoking shell. Windows now gets the same handover: the supervisor is started through a transient one-shot scheduled task, so the Task Scheduler service spawns it and it is parented to `svchost.exe`, belonging to no job of ours. This also matches the login path, where `explorer.exe` -- likewise job-free -- runs the HKCU `Run` entry. Login persistence deliberately stays on the HKCU `Run` key: registering an ONLOGON task requires elevation (verified: `schtasks /Create /SC ONLOGON` and `Register-ScheduledTask -AtLogOn` both fail with "Access is denied" for a standard user), while a one-shot task does not. `Register-ScheduledTask` is used instead of `schtasks /Create` because the latter caps `/TR` at 261 characters, which a deep `ADE_HOME` exceeds, and `ExecutionTimeLimit` is zeroed so the scheduler cannot terminate an always-on brain after its three-day default. The supervisor also gained a log file next to its launcher, the equivalent of launchd's `StandardOutPath`/`StandardErrorPath`. It is spawned detached with a hidden window and no redirection, so until now every supervisor death was completely invisible; it now records its own start, each brain spawn and exit with code and lifetime, each backoff, and any terminating error that unwinds the supervise loop. If the Task Scheduler route is unavailable or denied by policy the in-session launch is still used as a fallback, so the brain always comes up -- it is just bound to the session, which status reports. Based-on: nsxdavid/ADE#999 (cherry picked from commit ba0f8f5cd9d9dcd6a38331e8b1fd5b111a6a8161)
`ade brain start` escaped the caller's job object through a transient one-shot scheduled task. On a machine where Group Policy denies task registration that route fails and the launch fell back to an in-session `Process.Start`, which puts the supervisor right back inside the caller's kill-on-close job. The brain came up, `install` reported "channel brain is ready", `status` reported `running: true`, and then Windows terminated supervisor and brain together the moment the session exited -- after which `status` said only "Cleared stale supervisor PID record", erasing the evidence. Silent degradation of the headline guarantee, in the shape that looks exactly like success. Reproduced end to end by shadowing the ScheduledTasks module so `Register-ScheduledTask` fails with the same "Access is denied." that policy blocking produces, running `ade brain start` inside a JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE job, and closing the job. Four launch routes were measured on a real kill-on-close job, not reasoned about: `Process.Start`/`ShellExecuteEx`, `WScript.Shell.Run` and `Shell.Application.ShellExecute` all stayed in the caller's job and died with it -- the shell delegates nothing for a plain executable, so the "explorer.exe is job-free" intuition does not transfer. The one-shot task produced a supervisor parented to `svchost.exe`, and `Win32_Process.Create` produced one parented to `WmiPrvSE.exe` and in no job at all; both outlived the job close. WMI is now the second escape. This is documented behaviour, not a trick: *Job Objects* states that child processes created using `Win32_Process.Create` are not associated with the job. It is the fallback rather than the primary because WMI process creation is the more commonly blocked of the two -- it is a known lateral-movement technique that endpoint-protection rules disable -- and the two failure modes are largely independent, which is what makes chaining them worth doing. `Win32_ProcessStartup.ShowWindow` is set to SW_HIDE because a process created this way gets default startup information rather than the caller's. If a machine refuses both, the in-session launch still runs, but it is no longer silent. The supervisor probes its own job at startup with `QueryInformationJobObject(NULL, JobObjectExtendedLimitInformation)` -- documented to use the job associated with the calling process, so no handle is needed -- and publishes `sessionBound` in its PID record; `brain start` and `brain status` both report in full that the always-on guarantee does not hold and why. That is measured about the running supervisor rather than inferred from which route the installer took, because the two disagree: the same launcher is started job-free by `explorer.exe` at the next sign-in, and a record of the installer's intent would keep warning about a brain that is no longer session-bound. This is the Windows cost of what launchd gives macOS for free, where process ownership makes "installed" and "survives this session" the same fact. Here they are two facts, and reporting only the first is how a session-bound brain came to look like a healthy one. Based-on: nsxdavid/ADE#999 (cherry picked from commit be0c0cd40a5e7d73c69d51d443276cb3fa7caaca)
Eight test bodies opened with `if (process.platform === "win32") return;`. On Windows that reports as a passing test while asserting nothing, so the Windows suite overstated its own coverage by eight assertions. Convert each to `it.skipIf(process.platform === "win32")(...)`, the idiom already used in builtInBrowserSecurity.test.ts and modelPickerStore.test.ts, so the runner reports them as skipped. All eight are POSIX-only harnesses: `/bin/sh` wrapper execution, AF_UNIX `listen(path)`, POSIX mode bits, and `fs.symlinkSync`. No assertion changes. Shrink scripts/platform-gate-baseline.json by exactly those entries: the ratchet goes from 8 entries / 14 violations to 4 entries / 6 violations, and no vacuous-return entry remains. Based-on: nsxdavid/ADE#999 (cherry picked from commit cd4e0c45537753df8373e809d6feb484ef999fa7)
`npm --prefix apps/<app> install` does not mean "install apps/<app>". `--prefix` only redirects where npm writes node_modules; the package npm treats as the one being installed is still the package in the *current working directory*. Run from the repo root -- which is what `install:apps` did, seven times -- that package is the root `ade`, so npm dutifully installed the repo into each sub-app: `"ade": "file:../.."` in the app's package.json and package-lock.json, plus an `apps/<app>/node_modules/ade` symlink back to the root. Confirmed by bisecting the invocation form on this branch: `cd apps/ade-cli && npm install` leaves the manifest untouched, `npm --prefix apps/ade-cli install` from the repo root reproduces the churn every time. Running with `--prefix` from a cwd that has no package.json fails outright with `ENOENT ... /package.json`, which is the same fact stated the other way. Replace `install:apps` with scripts/install-apps.mjs, which spawns `npm install` with `cwd` set per app -- the same shape .github/workflows/ci.yml already uses (`(cd apps/<app> && npm ci)`), so CI was never affected and needs no change. Also fix the two user-facing hints in adeCliService's shell fallbacks and apps/webhook-relay/README.md, which told people to run the polluting command, and document the trap in AGENTS.md's validation section. `npm --prefix <app> run <script>` and `npm --prefix <app> exec` install nothing and stay valid. Verified: `npm run install:apps` across all seven apps leaves zero package.json changes and creates no node_modules/ade symlink. Based-on: nsxdavid/ADE#999 (cherry picked from commit 4fe9f9165557cdfe5afa99ff0c11cf648f561ca3)
…ays valid
`build.mac.artifactName` interpolated `${productName}`. app-builder-lib's
`updateInfoBuilder` rewrites the updater feed's `url` to `safeArtifactName`
whenever `publish.provider === "github"` (out/publish/updateInfoBuilder.js:101),
and `computeSafeArtifactNameIfNeeded` produces that name by replacing spaces
with dashes -- returning null only when the name is already GitHub-safe
(out/platformPackager.js:709). electron-builder can do that rewrite because its
own GitHub publisher uploads under the safe name. This repo packages with
`--publish never` and uploads via `gh release upload`, which uses the on-disk
basename. A `productName` with a space would therefore put a name in
latest-mac.yml that nothing ever published, and mac auto-update would 404 on
the first channel release.
`productName` is `ADE` today, so this is latent, not live -- but it becomes
live the moment a channel build (`ADE Beta`) reaches the mac path. Pinning the
literal keeps `safeArtifactName` null forever.
The pinned value renders byte-identically to the current output
(ADE-<version>-<arch>.{dmg,zip}), and every consumer already assumes that
literal prefix:
- apps/desktop/scripts/validate-mac-artifacts.mjs:756 -- /^ADE-.+-<arch>\.dmg$/
- apps/desktop/scripts/create-mac-dmg.mjs:68 -- `ADE-${version}-${arch}.dmg`
- .github/workflows/release-core.yml -- name-agnostic globs (*.dmg, *.zip)
- .agents/skills/release/SKILL.md:381-385 -- required-assets list
- release-assets/runtime/SHA256SUMS covers the standalone runtime only
So this is a config-to-consumer alignment, not a rename.
Based-on: nsxdavid/ADE#999
(cherry picked from commit 3b02c3e6cdd6ffddf112a5a58210040800419340)
`openKvDb(...).close()` called `DatabaseSync.close()` directly. cr-sqlite requires `select crsql_finalize()` first so the extension can tear down its virtual tables and per-connection state; without it the extension keeps resources attached to the connection. On POSIX that is an invisible no-op, which is why it went unnoticed. On Windows the OS keeps the `ade.db` file handle open, so after a caller has closed the database the file still cannot be unlinked, renamed or moved -- a Windows user could not delete or relocate their own `.ade` directory, and any in-process consumer that closes and then replaces the file gets EBUSY. Route every teardown through a `closeDatabase()` helper that finalizes best-effort and then closes. This covers the public `close()`, the failed-init cleanup path, and the three reopen points in `openKvDb` (primary-key retrofit, foreign-key retrofit, site-id correction), each of which previously abandoned a live handle to the same file. Verified on native Windows: before, `openKvDb().close()` followed by `fs.rmSync` throws `EBUSY: resource busy or locked, unlink ...\ade.db` while a plain `DatabaseSync` open/close does not; after, both succeed. `kvDb.rebuildRecovery.test.ts` goes from 4/16 to 16/16 with no test changes. Based-on: nsxdavid/ADE#999 (cherry picked from commit 6e60d643692d1b6db6073569391fa99f69059948)
`disposeOwnedRuntimeChild` tore down an owned `ade serve` daemon with `child.kill()`, which signals exactly one pid. A runtime daemon is not a leaf: observed live on a Windows box, 6 of 8 daemons this code spawns had their own `node.exe` children at the moment of teardown. Windows has no process groups for these children and no reaping parent, so every grandchild survived the daemon and kept holding the runtime named pipe. That is how `node cli.cjs serve` trees were found still alive ~9.5 hours after the process that owned them exited, still holding `\.\pipe\ade-runtime`. For a desktop user this means quitting ADE leaves runtime processes behind that block the next launch from binding its own pipe. Route disposal through `signalChildProcessTree` from shared/utils -- the helper `ptyService` and `agentChatService` already use, which signals the process group on POSIX and shells out to `taskkill /T` on Windows, falling back to a direct `child.kill()` if the tree signal fails. Also spawn the daemon with `detached` on POSIX so the group signal has a group to hit, matching the convention `spawnAsync` in shared/utils already follows. Previously the POSIX branch had the same one-pid blind spot, just with orphans reparented to init rather than stranded. Based-on: nsxdavid/ADE#999 (cherry picked from commit 1670b6e1c67d283f9d99aaa432e427f9af35f0c9)
The suite spawns real `ade serve` daemons and relied entirely on per-test `finally` blocks calling `child.kill()`. That left two gaps on Windows: a test that threw before its `finally` was reached leaked its daemon outright, and even the blocks that did run killed a single pid while the daemon's `node` children survived holding the runtime named pipe. Since this suite is a gate in the `windows-foundation` CI job, a leak also means the job can exit with live children. Track every daemon the suite starts in a set, and add an `afterEach` that tree-kills whatever is still registered, so cleanup no longer depends on any individual test reaching its own teardown. Route the existing per-test cleanups through the same `reapDaemonTree` helper so the pipe is released before the next test starts rather than at end of file, and spawn detached on POSIX so the group signal has a group. Verified on native Windows: three consecutive runs, each followed by a `Win32_Process` scan for `cli.cjs serve`/`brain-service` and a `\.\pipe\` scan for `ade` pipes, reported zero survivors and zero pipes every time. Suite result is unchanged at 65 passed / 1 skipped. Based-on: nsxdavid/ADE#999 (cherry picked from commit 85fda9f31ade873389b7c32872f953728fb8c021)
The suite was excluded because 12 of its 16 tests failed with EBUSY on Windows. The comment blamed the test's temp-dir teardown; that diagnosis was wrong. openKvDb's close() never called crsql_finalize(), so the OS kept the ade.db handle open after every caller believed the database was closed -- fixed in 6e60d643, with the test file itself untouched. It now passes 16/16, and this also makes its skipIf(linux) CRR tombstone compaction case reachable for the first time: the file otherwise only ran in test-desktop on ubuntu, where that case is skipped. Based-on: nsxdavid/ADE#999
kvDb.rebuildRecovery.test.ts carries an it.skipIf(linux) gate that ran on no runner while the file was excluded from the Windows job. Gating it in that job covers the gate, so its baseline entry is stale and the ratchet correctly refuses to pass until the baseline shrinks. Based-on: nsxdavid/ADE#999
A Windows brain skipped every socket-ownership check on its way to `listen()`. All three guards were gated on `!isAdeRuntimeNamedPipePath`, and `probeLocalSocketForLiveness` returned `"unknown"` for any pipe without dialing it, so the brain went straight to a bare bind. When the endpoint was already owned the raw `EADDRINUSE` escaped, the recovery classifier could not match Node's wording, and the failure was filed as `code: "unknown"` with "ADE's background service could not start." and no next action. The gating inherited a POSIX assumption that is not just unnecessary on Windows but backwards. A unix socket leaves a file behind when its owner dies, so the path existing proves nothing and only a probe can tell. A named pipe has no filesystem corpse: the name lives exactly as long as some process holds a handle and the kernel releases it the moment the last handle closes -- SIGKILL the owner and the next `listen()` succeeds. So dialing a pipe is decisive where dialing a socket is a hint, and `ENOENT` on connect is itself the existence check that `existsSync` was being asked for. Probe pipes like any other local endpoint, run the ownership check before binding one (without the unlink step, which has nothing to unlink), and translate a racing `EADDRINUSE` at the bind into the same `socket_owned_by_other` error the pre-bind path already raises. Windows now reports what macOS has always reported, with a cause that describes Windows' actual semantics rather than hedging about a stale endpoint the platform cannot produce. Verified against a scratch ADE_HOME and pipe name: the recorded failure goes from `code: "unknown"` with Node's raw text to `code: "socket_owned_by_other"` with a cause and a next action, and a brain binding a free pipe still starts normally. Based-on: nsxdavid/ADE#999
…ion-runtime # Conflicts: # apps/desktop/src/renderer/components/app/TopBar.test.tsx
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
apps/ade-cli/src/serviceManager/installWindows.ts (1)
771-790: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUninstall still resolves the service identity without the command environment.
Line 775 calls
resolvedServiceName(deps)and line 789 builds the launcher path from the bareenv.installWindowsService(line 621) andgetWindowsServiceStatus(line 894) both mergeserviceCommand.envfirst. WhenADE_PACKAGE_CHANNELorADE_HOMEexist only incommand.env, uninstall computes a differentserviceName,taskName, Run-key value name, andlauncherPath, then reports success while the real startup entry and launcher remain.This was flagged in a previous review and marked as addressed, but the current code still shows the unmerged form.
Proposed fix
const serviceCommand = deps.command ?? resolveAdeServeCommand(); - const serviceName = resolvedServiceName(deps); + const serviceName = resolvedServiceName(deps, serviceCommand); @@ const taskName = resolveWindowsTaskName({ serviceName, userName }); - const launcherPath = deps.launcherPath ?? resolveWindowsServiceLauncherPath({ env, serviceName }); + const runtimeEnv = { ...env, ...(serviceCommand.env ?? {}) }; + const launcherPath = deps.launcherPath + ?? resolveWindowsServiceLauncherPath({ env: runtimeEnv, serviceName });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/serviceManager/installWindows.ts` around lines 771 - 790, Update uninstallWindowsService to merge serviceCommand.env with env before resolving the service identity and launcher path, matching installWindowsService and getWindowsServiceStatus. Use the merged environment when calling resolvedServiceName, resolveWindowsTaskUser, and resolveWindowsServiceLauncherPath so command-specific ADE_PACKAGE_CHANNEL and ADE_HOME values produce consistent cleanup targets.
🧹 Nitpick comments (7)
apps/ade-cli/src/cli.test.ts (1)
2812-2815: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the rollup position, not only its presence.
The test name states that the rollup leads the output. The assertions only check substring presence, so a rollup printed after the rows still passes. Compare indexes to lock the ordering the test claims.
♻️ Proposed assertion
expect(notRun).toContain("ADE PR checks - not run"); expect(notRun).toContain("3 checks reported"); + expect(notRun.indexOf("ADE PR checks - not run")).toBeLessThan(notRun.indexOf("CodeRabbit")); // The raw enum must never reach the reader — the phrase is the point. expect(notRun).not.toContain("not_run");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/cli.test.ts` around lines 2812 - 2815, Update the rollup-output assertions in the affected test to compare the index of “ADE PR checks - not run” with the index of the detailed check rows, asserting that the rollup appears first. Retain the existing presence and raw-enum exclusion checks.apps/desktop/src/main/services/cli/adeCliService.ts (1)
215-215: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider dropping
--prefixfrom user-facing guidance.The repository guideline directs contributors away from
npm --prefix apps/<app>invocations for dependency work. The build command shown here is not an install, so the guideline is not violated. Still, the message teaches the--prefixshape and users may reuse it for installs. Suggestcd apps/ade-cli && npm run buildfor both messages.♻️ Proposed wording
- " echo ade: Local source CLI fallback requires repo-local tsx. Run npm run install:apps or npm --prefix apps/ade-cli run build. 1>&2", + " echo ade: Local source CLI fallback requires repo-local tsx. Run npm run install:apps, or build with cd apps/ade-cli ^&^& npm run build. 1>&2",Note:
&requires^escaping inside acmdecho. Verify the rendered shim text if you adopt this wording.As per coding guidelines: "Install dependencies with
npm run install:appsfrom the repository root orcd apps/<app> && npm install; never usenpm --prefix apps/<app> install."Also applies to: 377-377
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/services/cli/adeCliService.ts` at line 215, Update the user-facing fallback guidance strings in the CLI shim to replace “npm --prefix apps/ade-cli run build” with “cd apps/ade-cli && npm run build” in both occurrences. Preserve cmd-compatible escaping for the ampersand and verify the rendered shim text remains correct.Source: Coding guidelines
apps/desktop/src/renderer/components/app/TopBar.test.tsx (1)
27-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueEscape the constant before building the pattern.
THIS_MACHINE_NAMEis data, not a pattern. The current value is safe, but a future label with.,(, or+would silently change the match. Escape the string, or pass a predicate to the query instead.♻️ Proposed change
-const THIS_MACHINE_NAME_PATTERN = new RegExp(THIS_MACHINE_NAME); +const THIS_MACHINE_NAME_PATTERN = new RegExp( + THIS_MACHINE_NAME.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"), +);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/app/TopBar.test.tsx` around lines 27 - 33, Update THIS_MACHINE_NAME_PATTERN to escape regex metacharacters in THIS_MACHINE_NAME before constructing the RegExp, preserving substring matching for the machine menu’s accessible name.apps/ade-cli/src/serviceManager/common.test.ts (1)
404-414: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThe ancestry assertion can pass without exercising the query.
isCurrentProcessDescendantOfPidfails closed: it returnstruewhen the lookup returnsPARENT_PID_UNKNOWN. On a host where the trusted PowerShell query fails, this test still passes and proves nothing about the win32 branch. Add a negative case that must returnfalse, so a broken query is detected.♻️ Proposed addition
it("reports the real parent as an ancestor of this process", () => { expect(isCurrentProcessDescendantOfPid({ targetPid: process.ppid })).toBe(true); }, 30_000); + + it("does not report an unrelated live pid as an ancestor", () => { + // A working query must answer `false` here; a failing query fails closed + // with `true` and this assertion catches that. + expect(readParentPid(spawnChildSync, process.pid)).toBe(process.ppid); + expect(isCurrentProcessDescendantOfPid({ + targetPid: process.pid, + currentPid: process.ppid, + })).toBe(false); + }, 30_000);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/serviceManager/common.test.ts` around lines 404 - 414, Add a negative Windows-host test alongside the existing isCurrentProcessDescendantOfPid assertion that supplies a PID which cannot be an ancestor and expects false, ensuring the ancestry query is actually exercised and query failures are detected. Keep the existing real-parent positive case unchanged.apps/ade-cli/src/serviceManager/common.ts (1)
107-137: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffEach ancestor costs one PowerShell start.
readWindowsParentPidspawnspowershell.exeper pid.isCurrentProcessDescendantOfPidwalks the chain pid by pid, so a successful deep walk pays the cold-start plus CIM cost repeatedly, with a 15s ceiling each. A single query that returnsProcessId/ParentProcessIdfor all processes would resolve the whole chain in one spawn.This is a guard on a teardown path, so the current cost may be acceptable. Consider the single-query form if teardown latency on Windows becomes visible.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/serviceManager/common.ts` around lines 107 - 137, The current implementation performs one PowerShell spawn per ancestor in readWindowsParentPid, making deep isCurrentProcessDescendantOfPid walks expensive; if teardown latency requires optimization, replace the per-PID lookup with one PowerShell/CIM query that retrieves ProcessId and ParentProcessId for all processes, then resolve the ancestry locally while preserving the existing unknown and root termination behavior.apps/ade-cli/src/serviceManager/windowsSupervisor.ts (1)
168-181: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider UTF-8 for the PID record.
Write-PidRecordwrites the JSON with[Text.Encoding]::ASCII.lastLaunchErrorcomes from a Windows exception message, which can contain non-ASCII characters on a localized system. ASCII encoding replaces those characters with?, so the diagnostic thatbrain statusprints is degraded.readWindowsServicePidRecordreads the file asutf8, so UTF-8 output stays compatible.Proposed change
- " [IO.File]::WriteAllText($pidPath, ($record | ConvertTo-Json -Compress), [Text.Encoding]::ASCII)", + " [IO.File]::WriteAllText($pidPath, ($record | ConvertTo-Json -Compress), (New-Object Text.UTF8Encoding($false)))",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/serviceManager/windowsSupervisor.ts` around lines 168 - 181, Update the PowerShell Write-PidRecord function to write the JSON using UTF-8 encoding instead of ASCII, preserving non-ASCII characters in lastLaunchError and remaining compatible with readWindowsServicePidRecord’s UTF-8 decoding.apps/ade-cli/src/serviceManager/installWindows.ts (1)
181-186: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider sharing one PowerShell quoting helper.
powerShellSingleQuotedLiteralexists here and inwindowsSupervisor.ts(line 78) with the same behavior and different error text. Both files are security-relevant quoting paths. Export one implementation fromcommon.tsorwindowsSupervisor.tsand reuse it, so a future quoting fix cannot land in only one copy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/serviceManager/installWindows.ts` around lines 181 - 186, Consolidate the duplicate PowerShell quoting logic by exporting a single powerShellSingleQuotedLiteral implementation from common.ts or windowsSupervisor.ts, then update installWindows.ts and windowsSupervisor.ts to reuse it. Preserve NUL-byte rejection and single-quote escaping, using one consistent error message.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 425-434: Replace the npm --prefix usage in the ADE CLI and desktop
dependency installation steps with per-app commands that change into each app
directory before running npm ci. Apply the same pattern to both typecheck
commands in the “Typecheck Windows runtime foundations” step, using cd
apps/<app> && npm run typecheck while preserving the existing app-specific
commands.
In `@apps/ade-cli/src/services/sync/syncHostSingleton.ts`:
- Around line 220-223: Update the fallback buildQuitCommand call within
safeReadLock to pass its platform parameter instead of relying on
process.platform, keeping the same platform value threaded through
withPidKillFallback so generated commands use consistent syntax.
In `@apps/desktop/src/main/services/chat/cursorSdkHooks.test.ts`:
- Line 114: Add Windows coverage for the fail-closed ADE hook wrapper behavior
in the test describing missing Node runners: select and invoke ade-tool-gate.cmd
on Windows, while retaining ade-tool-gate.sh for POSIX, and assert both wrappers
deny the invocation when no runner is available instead of skipping Windows
entirely.
In `@apps/desktop/src/main/services/state/kvDb.ts`:
- Around line 214-234: Extend closeDatabase() to support readonly connections,
using a readonly-compatible finalization path when needed while preserving
writable cleanup. Update openReadonlyDatabase() lifecycle handling and every
explicit ade.db open to close through closeDatabase(), ensuring no direct
db.close() remains for these connections.
---
Duplicate comments:
In `@apps/ade-cli/src/serviceManager/installWindows.ts`:
- Around line 771-790: Update uninstallWindowsService to merge
serviceCommand.env with env before resolving the service identity and launcher
path, matching installWindowsService and getWindowsServiceStatus. Use the merged
environment when calling resolvedServiceName, resolveWindowsTaskUser, and
resolveWindowsServiceLauncherPath so command-specific ADE_PACKAGE_CHANNEL and
ADE_HOME values produce consistent cleanup targets.
---
Nitpick comments:
In `@apps/ade-cli/src/cli.test.ts`:
- Around line 2812-2815: Update the rollup-output assertions in the affected
test to compare the index of “ADE PR checks - not run” with the index of the
detailed check rows, asserting that the rollup appears first. Retain the
existing presence and raw-enum exclusion checks.
In `@apps/ade-cli/src/serviceManager/common.test.ts`:
- Around line 404-414: Add a negative Windows-host test alongside the existing
isCurrentProcessDescendantOfPid assertion that supplies a PID which cannot be an
ancestor and expects false, ensuring the ancestry query is actually exercised
and query failures are detected. Keep the existing real-parent positive case
unchanged.
In `@apps/ade-cli/src/serviceManager/common.ts`:
- Around line 107-137: The current implementation performs one PowerShell spawn
per ancestor in readWindowsParentPid, making deep
isCurrentProcessDescendantOfPid walks expensive; if teardown latency requires
optimization, replace the per-PID lookup with one PowerShell/CIM query that
retrieves ProcessId and ParentProcessId for all processes, then resolve the
ancestry locally while preserving the existing unknown and root termination
behavior.
In `@apps/ade-cli/src/serviceManager/installWindows.ts`:
- Around line 181-186: Consolidate the duplicate PowerShell quoting logic by
exporting a single powerShellSingleQuotedLiteral implementation from common.ts
or windowsSupervisor.ts, then update installWindows.ts and windowsSupervisor.ts
to reuse it. Preserve NUL-byte rejection and single-quote escaping, using one
consistent error message.
In `@apps/ade-cli/src/serviceManager/windowsSupervisor.ts`:
- Around line 168-181: Update the PowerShell Write-PidRecord function to write
the JSON using UTF-8 encoding instead of ASCII, preserving non-ASCII characters
in lastLaunchError and remaining compatible with readWindowsServicePidRecord’s
UTF-8 decoding.
In `@apps/desktop/src/main/services/cli/adeCliService.ts`:
- Line 215: Update the user-facing fallback guidance strings in the CLI shim to
replace “npm --prefix apps/ade-cli run build” with “cd apps/ade-cli && npm run
build” in both occurrences. Preserve cmd-compatible escaping for the ampersand
and verify the rendered shim text remains correct.
In `@apps/desktop/src/renderer/components/app/TopBar.test.tsx`:
- Around line 27-33: Update THIS_MACHINE_NAME_PATTERN to escape regex
metacharacters in THIS_MACHINE_NAME before constructing the RegExp, preserving
substring matching for the machine menu’s accessible name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: af962974-f26c-435c-861c-a2e661bfc1c9
⛔ Files ignored due to path filters (1)
AGENTS.mdis excluded by!*.md
📒 Files selected for processing (94)
.agents/skills/quality/SKILL.md.agents/skills/quality/references/ade-review-rules.md.agents/skills/quality/references/correctness-security-review.md.agents/skills/quality/references/thermo-nuclear-review.md.agents/skills/ship/SKILL.md.agents/skills/test/SKILL.md.github/workflows/ci.ymlapps/ade-cli/README.mdapps/ade-cli/src/bootstrap.test.tsapps/ade-cli/src/cli.test.tsapps/ade-cli/src/cli.tsapps/ade-cli/src/headlessLinearServices.test.tsapps/ade-cli/src/headlessLinearServices.tsapps/ade-cli/src/lib/trustedWindowsTools.test.tsapps/ade-cli/src/lib/trustedWindowsTools.tsapps/ade-cli/src/multiProjectRpcServer.tsapps/ade-cli/src/serviceManager/common.test.tsapps/ade-cli/src/serviceManager/common.tsapps/ade-cli/src/serviceManager/index.tsapps/ade-cli/src/serviceManager/installWindows.test.tsapps/ade-cli/src/serviceManager/installWindows.tsapps/ade-cli/src/serviceManager/windowsSupervisor.test.tsapps/ade-cli/src/serviceManager/windowsSupervisor.tsapps/ade-cli/src/services/agentRegistry.test.tsapps/ade-cli/src/services/builtInBrowser/desktopBridgeClient.test.tsapps/ade-cli/src/services/credentials/credentialStore.test.tsapps/ade-cli/src/services/modelPickerStore.test.tsapps/ade-cli/src/services/projects/machineLayout.test.tsapps/ade-cli/src/services/projects/machineLayout.tsapps/ade-cli/src/services/projects/projectIconResolver.test.tsapps/ade-cli/src/services/projects/projectRegistry.test.tsapps/ade-cli/src/services/projects/projectRegistry.tsapps/ade-cli/src/services/runtime/brainLoopWatchdog.test.tsapps/ade-cli/src/services/runtime/brainLoopWatchdog.tsapps/ade-cli/src/services/runtime/localIpcListenOptions.test.tsapps/ade-cli/src/services/runtime/localIpcListenOptions.tsapps/ade-cli/src/services/runtime/socketSpawnLock.tsapps/ade-cli/src/services/sync/machineIdentitySigningStore.test.tsapps/ade-cli/src/services/sync/sharedSyncListener.test.tsapps/ade-cli/src/services/sync/sharedSyncListener.tsapps/ade-cli/src/services/sync/syncHostService.test.tsapps/ade-cli/src/services/sync/syncHostService.tsapps/ade-cli/src/services/sync/syncHostSingleton.test.tsapps/ade-cli/src/services/sync/syncHostSingleton.tsapps/ade-cli/src/services/sync/syncLoopbackCollision.test.tsapps/ade-cli/src/services/sync/syncPairingStore.test.tsapps/ade-cli/src/services/sync/syncService.test.tsapps/ade-cli/src/services/sync/syncService.tsapps/ade-cli/src/test/crrModelPickerWorker.tsapps/ade-cli/src/test/filesystem.tsapps/ade-cli/src/tuiClient/app.tsxapps/ade-cli/src/tuiClient/imageTargets.tsapps/desktop/package.jsonapps/desktop/scripts/dev.cjsapps/desktop/src/main/main.tsapps/desktop/src/main/services/attention/attentionAccountCoordinator.test.tsapps/desktop/src/main/services/attention/attentionAccountCoordinator.tsapps/desktop/src/main/services/attention/attentionNotchHelper.test.tsapps/desktop/src/main/services/attention/attentionNotchHelper.tsapps/desktop/src/main/services/chat/cursorSdkHooks.test.tsapps/desktop/src/main/services/chat/cursorSdkPolicy.test.tsapps/desktop/src/main/services/cli/adeCliService.tsapps/desktop/src/main/services/computerUse/localComputerUse.test.tsapps/desktop/src/main/services/computerUse/localComputerUse.tsapps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.test.tsapps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.tsapps/desktop/src/main/services/projects/projectIconResolver.test.tsapps/desktop/src/main/services/projects/projectIconResolver.tsapps/desktop/src/main/services/remoteRuntime/runtimeRpcClient.tsapps/desktop/src/main/services/runtime/projectRecoveryService.tsapps/desktop/src/main/services/shared/utils.tsapps/desktop/src/main/services/state/kvDb.tsapps/desktop/src/main/services/storage/diskPressure.tsapps/desktop/src/main/windowAppearance.test.tsapps/desktop/src/main/windowAppearance.tsapps/desktop/src/renderer/components/activity/ActivitySettingsPopover.test.tsxapps/desktop/src/renderer/components/activity/ActivitySettingsPopover.tsxapps/desktop/src/renderer/components/activity/ActivitySettingsPopover.windows.test.tsxapps/desktop/src/renderer/components/activity/HeaderActivityControl.test.tsxapps/desktop/src/renderer/components/app/TopBar.test.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.test.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/lanes/CreateLaneDialogHostBinding.test.tsxapps/desktop/src/renderer/components/lanes/laneMachines.test.tsapps/desktop/src/renderer/components/lanes/laneMachines.tsapps/desktop/src/renderer/components/personalChats/PersonalChatsPage.test.tsxapps/desktop/src/renderer/components/personalChats/ProjectlessSidebar.test.tsxapps/desktop/src/renderer/components/settings/ActivitySection.test.tsxapps/desktop/src/renderer/components/settings/ActivitySettingsControls.tsxapps/desktop/src/renderer/components/terminals/SessionCard.test.tsxapps/desktop/src/renderer/components/terminals/SessionCard.tsxapps/desktop/src/renderer/lib/platform.test.tsapps/desktop/src/renderer/lib/platform.tsapps/desktop/src/renderer/main.tsx
🚧 Files skipped from review as they are similar to previous changes (62)
- apps/desktop/src/renderer/components/lanes/CreateLaneDialogHostBinding.test.tsx
- apps/desktop/src/main/services/attention/attentionAccountCoordinator.test.ts
- apps/ade-cli/src/multiProjectRpcServer.ts
- apps/desktop/src/renderer/components/activity/HeaderActivityControl.test.tsx
- apps/desktop/src/renderer/components/activity/ActivitySettingsPopover.test.tsx
- apps/desktop/src/main/services/shared/utils.ts
- apps/ade-cli/src/services/projects/projectIconResolver.test.ts
- apps/desktop/src/renderer/components/chat/AgentChatPane.tsx
- apps/ade-cli/src/test/filesystem.ts
- .agents/skills/quality/references/ade-review-rules.md
- apps/desktop/src/renderer/lib/platform.test.ts
- apps/ade-cli/src/services/projects/projectRegistry.test.ts
- apps/desktop/src/renderer/main.tsx
- apps/desktop/src/renderer/components/lanes/laneMachines.test.ts
- apps/ade-cli/src/services/sync/syncHostSingleton.test.ts
- apps/ade-cli/src/services/runtime/localIpcListenOptions.ts
- apps/ade-cli/src/services/agentRegistry.test.ts
- apps/ade-cli/src/services/sync/syncHostService.test.ts
- apps/ade-cli/src/services/sync/syncLoopbackCollision.test.ts
- apps/desktop/src/renderer/components/settings/ActivitySection.test.tsx
- apps/ade-cli/src/bootstrap.test.ts
- apps/ade-cli/src/headlessLinearServices.ts
- apps/ade-cli/src/services/sync/syncService.test.ts
- .agents/skills/quality/references/thermo-nuclear-review.md
- apps/desktop/src/renderer/components/activity/ActivitySettingsPopover.tsx
- apps/ade-cli/src/services/projects/machineLayout.ts
- apps/ade-cli/src/services/runtime/localIpcListenOptions.test.ts
- apps/ade-cli/src/services/sync/syncService.ts
- apps/desktop/src/renderer/components/activity/ActivitySettingsPopover.windows.test.tsx
- apps/desktop/scripts/dev.cjs
- apps/desktop/src/main/services/computerUse/localComputerUse.ts
- apps/ade-cli/src/services/sync/machineIdentitySigningStore.test.ts
- apps/ade-cli/src/test/crrModelPickerWorker.ts
- apps/ade-cli/src/services/builtInBrowser/desktopBridgeClient.test.ts
- apps/ade-cli/src/tuiClient/imageTargets.ts
- apps/ade-cli/src/headlessLinearServices.test.ts
- .agents/skills/quality/references/correctness-security-review.md
- apps/ade-cli/src/lib/trustedWindowsTools.ts
- apps/desktop/src/renderer/lib/platform.ts
- apps/desktop/src/main/windowAppearance.test.ts
- apps/ade-cli/src/services/modelPickerStore.test.ts
- apps/desktop/src/main/windowAppearance.ts
- apps/desktop/src/main/services/computerUse/localComputerUse.test.ts
- apps/ade-cli/src/services/runtime/brainLoopWatchdog.test.ts
- apps/desktop/src/main/services/storage/diskPressure.ts
- apps/desktop/src/main/services/attention/attentionNotchHelper.test.ts
- apps/ade-cli/src/services/projects/projectRegistry.ts
- apps/desktop/src/renderer/components/settings/ActivitySettingsControls.tsx
- apps/desktop/src/renderer/components/lanes/laneMachines.ts
- apps/ade-cli/src/tuiClient/app.tsx
- apps/desktop/src/renderer/components/chat/AgentChatPane.test.tsx
- .agents/skills/ship/SKILL.md
- apps/desktop/src/main/services/remoteRuntime/runtimeRpcClient.ts
- apps/ade-cli/src/services/sync/syncHostService.ts
- apps/ade-cli/src/services/runtime/brainLoopWatchdog.ts
- apps/desktop/src/main/services/attention/attentionAccountCoordinator.ts
- .agents/skills/test/SKILL.md
- apps/desktop/src/main/main.ts
- apps/ade-cli/src/cli.ts
- apps/desktop/src/main/services/runtime/projectRecoveryService.ts
- .agents/skills/quality/SKILL.md
- apps/ade-cli/src/services/runtime/socketSpawnLock.ts
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 1/5 · parent: main · downstream: #1007
This is the foundation 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
CLI/provider UX belongs to #1007; desktop auth/sync to #1008; packaging/updating to #1009; release proof to #1010. WSL, ARM64, and native Windows computer use are out of scope.
Validation
Draft checkpoint: direct-parent desktop and ADE CLI typechecks pass on native Windows with Node 22.13.1. Worker integration, focused tests, /quality, /test, and ready-stacked binding are in progress.
Evidence and provenance
Summary by CodeRabbit