Uh oh!
There was an error while loading. Please reload this page.
perf(windows): reduce first-start editor discovery time - #6124
perf(windows): reduce first-start editor discovery time#6124simon-curtis wants to merge 2 commits into
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Uh oh!
There was an error while loading. Please reload this page.
| @@ -177,17 +177,20 @@ export const ClaudeDriver: ProviderDriver<ClaudeSettings, ClaudeDriverEnv> = { | |||
| const snapshotSettings = makeProviderSnapshotSettingsSource(effectiveConfig, serverSettings); | |||
| const snapshot = yield* makeManagedServerProvider<ProviderSnapshotSettings<ClaudeSettings>>({ | |||
| maintenanceCapabilities, | |||
| maintenanceCapabilities: maintenanceCapabilities.get, | |||
There was a problem hiding this comment.
🟠 HighDrivers/ClaudeDriver.ts:180
maintenanceCapabilities.get() returns the npm updater immediately after ClaudeDriver.create(), even when claude on PATH actually resolves to a native or Homebrew install. Before the deferred maintenanceCapabilities.refresh completes, ProviderRegistry.getProviderMaintenanceCapabilitiesForInstance can therefore return the wrong update action, so an update requested during startup runs npm instead of claude update/Homebrew. makeProviderMaintenanceCapabilitiesSource initializes its current value via resolver.resolve(options) before command-path/realpath discovery, and that synchronous resolve falls back to the npm updater for a bare claude command. Consider initializing current with a non-actionable value (or gating maintenance access) until the deferred resolution completes.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Drivers/ClaudeDriver.ts around line 180:
`maintenanceCapabilities.get()` returns the npm updater immediately after `ClaudeDriver.create()`, even when `claude` on PATH actually resolves to a native or Homebrew install. Before the deferred `maintenanceCapabilities.refresh` completes, `ProviderRegistry.getProviderMaintenanceCapabilitiesForInstance` can therefore return the wrong update action, so an update requested during startup runs `npm` instead of `claude update`/Homebrew. `makeProviderMaintenanceCapabilitiesSource` initializes its `current` value via `resolver.resolve(options)` before command-path/realpath discovery, and that synchronous resolve falls back to the npm updater for a bare `claude` command. Consider initializing `current` with a non-actionable value (or gating maintenance access) until the deferred resolution completes.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
6ed8120 to
bbfad77CompareUh oh!
There was an error while loading. Please reload this page.
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. You can customize Macroscope's approvability policy. Learn more. |
309637f to
fef06d7CompareThere was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fef06d741f8c710f0957139af036297f3f3aad90. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
CDVolvik
left a comment
There was a problem hiding this comment.
Read the Windows shell half against today's main. CONFLICTING.
Desktop: drops the -NoProfile PATH probe entirely and keeps only the profile probe, merging process PATH + profile PATH + knownWindowsCliDirs. That is the ~2s startup win (each PowerShell spawn was ~2s). It also deletes the no-profile source #6356 is trying to make read Machine+User, so a CLI installed after launch still will not show up until the next process start unless the profile sees it.
Shared: directory listing + case-insensitive name map instead of probing every PATHEXT candidate, with a fallback when readDirectory fails (ACL / unreadable share). That is the right Windows lookup. Returning [command] when the input already has a PATHEXT suffix is also right — the old code generated three casings of the same extension.
Do not land this and #6356/#4896/#6301 independently; they all rewrite installWindowsEnvironment.
fef06d7 to
2ead4e7Compare324294b to
96a86d6Comparesimon-curtis
commented
Aug 17, 2026
Scope update: I have reframed this PR around reducing first-start/editor-discovery time on Windows. After comparing the original branch with current What remains is the independently useful shared lookup optimization: list each Windows PATH directory once, match PATHEXT candidates case-insensitively, preserve PATH order, and fall back to the existing direct probes when listing fails. I also restored A targeted five-run benchmark on this machine's 71-entry PATH and 12 PATHEXT values measured provider discovery at 199 ms → 49 ms and first editor discovery at 1,964 ms → 425 ms. This is a resolver benchmark, not an end-to-end startup claim. |
Uh oh!
There was an error while loading. Please reload this page.
99f0376 to
d5a0e7aComparecfb0d3e to
9ecfbc4Compare
Co-authored-by: macroscopeapp[bot] <170038800+macroscopeapp[bot]@users.noreply.github.com>
9ecfbc4 to
ae911ebComparet3dotgg
commented
Sep 4, 2026
Note 🤖 GPT-6 Astra (preview) responding on behalf of Theo This note is part of an automated cleanup pass. Preserving these details from items reviewed in the cleanup pass. Carryover from #6221 at 59849f94e2: retain the Carry over the slow-discovery evidence from #5050 at 61278bee47. On the reported Windows host with 46 PATH entries and 12 PATHEXT entries, getConfig timed out near 5,000 ms on every call. Concurrent discovery finished in 4,787 ms in that report. Its focused test interrupts one caller during discovery, then requires a later caller to obtain editors. Check cold discovery and caller interruption while retaining bounded expiry and current Linux/WSL file-manager checks. These are reported measurements and a test case to retain, not results rerun here. Keep the trace evidence from #4778 at dc7f1727a6 in the Windows resolver review. Its author reported 28,577 shell.isExecutableFile spans from 22 command lookups in 13.7 seconds, plus trace files rotating at 10 MB about every 40 seconds. The distinct code change makes isExecutableFile untraced. Main still traces that helper, although its command cache and VCS limits are now bounded. Check first-scan and miss-heavy trace cost with this PR. Do not copy the older unbounded cache or treat the reported timings as rerun results. |
What Changed
On Windows, command discovery now lists each PATH directory once, matches PATHEXT candidates case-insensitively, and performs the existing executable-file check only for matching entries.
PATH order and
resolveCommandCandidatesremain unchanged. If a directory cannot be listed, lookup falls back to upstream's direct candidate probes.Why
The first client configuration probes up to 22 editor commands. Upstream performs a filesystem probe for every PATH × PATHEXT candidate, so editor discovery can consume seconds on a long Windows PATH and approaches its five-second timeout.
This narrows the PR to reducing first-start/editor-discovery time without changing desktop PATH hydration or packaging behavior.
Measured Impact
Targeted Windows command-discovery benchmark on 71 PATH entries and 12 PATHEXT extensions; medians from five alternating runs:
This is a resolver benchmark rather than an end-to-end launch measurement. The exact gain depends on PATH and PATHEXT size.
Scope
No PowerShell environment probing, provider lifecycle, installer, packaging, or WSL behavior changes are included.
Verification
vp fmt --check packages/shared/src/shell.ts packages/shared/src/shell.test.tsvp lint packages/shared/src/shell.ts packages/shared/src/shell.test.ts --report-unused-disable-directivesvp run --filter @t3tools/shared typecheckvp test run packages/shared/src/shell.test.ts— 30 tests passedmodel: gpt-5.6-sol
harness: Codex in T3 Code
Note
Reduce first-start editor discovery time on Windows with case-insensitive PATH resolution
resolveCommandPathForPlatformin shell.ts now lists each PATH directory and builds a case-insensitive filename map to match command candidates, avoiding failed probes caused by case mismatches.Macroscope summarized ae911eb.
Note
Medium Risk
Changes Windows command resolution used by editor and CLI discovery; behavior is narrowed with a listing fallback, but incorrect matching could miss or mis-resolve executables on win32.
Overview
On Windows,
resolveCommandPathForPlatformno longer probes every PATHEXT candidate in each PATH directory. For each PATH entry it lists the directory (when listing succeeds), maps filenames case-insensitively, and only runs the existing executable check for names that actually match the command candidates.If a PATH directory cannot be listed, resolution falls back to the prior behavior of probing candidates directly. PATH order and candidate generation are unchanged.
A new test creates a mixed-case
.eXeunder a temp PATH entry and assertsresolveCommandPathresolvesprovider-toolonwin32.Reviewed by Cursor Bugbot for commit ae911eb. Bugbot is set up for automated code reviews on this repo. Configure here.