Skip to content

fix(server): speed up Windows diagnostics and editor discovery#3911

Open
SivanTechDev wants to merge 3 commits into
pingdotgg:mainfrom
SivanTechDev:fix/windows-process-diagnostics
Open

fix(server): speed up Windows diagnostics and editor discovery#3911
SivanTechDev wants to merge 3 commits into
pingdotgg:mainfrom
SivanTechDev:fix/windows-process-diagnostics

Conversation

@SivanTechDev

@SivanTechDev SivanTechDev commented Jul 12, 2026

Copy link
Copy Markdown

What Changed

  • Replace the Windows process-diagnostics N+1 CIM query with two bulk queries joined by process ID.
  • Use a five-second diagnostics timeout on Windows while retaining the existing one-second POSIX timeout.
  • Bound non-Windows editor discovery to four concurrent probes and a five-second config fallback.
  • Use the existing Windows executable resolver for editor discovery instead of repeatedly scanning the full PATH for every editor.
  • Add focused coverage for the bulk Windows query and exact editor results.

Why

Fixes #3610.

On the affected Windows machine, process diagnostics and optional editor discovery delayed server.getConfig long enough for the client to treat the local environment as disconnected. Project history and provider configuration then failed to load.

Windows diagnostics previously queried performance data once per process. Reading both CIM classes once and joining them by process ID reduced the measured query to about 866 ms, so this change gives Windows a five-second budget without changing the POSIX timeout.

The first editor-discovery fallback used 500 ms, but that hid installed editors. A trace on the affected machine showed that its 37-entry PATH still took more than five seconds when scanned separately for every editor, even with four concurrent probes. Windows now reuses the existing executable resolver, which restores VS Code and File Manager discovery without introducing another PATH/PATHEXT implementation.

The Windows resolver uses synchronous filesystem checks, so the surrounding Effect timeout cannot preempt a filesystem call while the event loop is blocked. This PR keeps that existing tradeoff to remain a focused fix; a fully interruptible batch resolver would be broader shell infrastructure work.

Validation

  • Manually verified with pnpm dev:desktop that VS Code and File Manager are discovered on the affected machine.
  • Typecheck passed.
  • Formatting passed.
  • apps/server/src/process/externalLauncher.test.ts passed.
  • The changed test-file run passed 111 tests and reported two Windows-environment failures: an existing POSIX-specific diagnostics test that does not pin the platform, and a server test that could not create a symlink (EPERM).
  • Confirmed that config loading completes and the local environment connects.

Checklist

  • This PR is small and focused.
  • I explained what changed and why.
  • No UI changes; screenshots are not applicable.
  • No animation or interaction changes; video is not applicable.

Note

Speed up Windows process diagnostics and editor discovery

  • Windows process diagnostics now fetches perf data once via a single Get-CimInstance Win32_PerfFormattedData_PerfProc_Process call and uses a dictionary lookup per process, replacing per-process -Filter CIM queries.
  • Windows editor discovery uses SpawnExecutableResolution instead of async PATH scans; non-Windows discovery runs probes with bounded concurrency (4).
  • Editor discovery in the WebSocket server config snapshot is capped at a 5-second timeout, returning an empty availableEditors list on timeout instead of blocking.
  • Separate timeout constants are introduced: POSIX_PROCESS_QUERY_TIMEOUT_MS (1s) and WINDOWS_PROCESS_QUERY_TIMEOUT_MS (5s), replacing a single shared constant.

Macroscope summarized f302154.

Summary by CodeRabbit

  • New Features

    • Server configuration now remains responsive even when editor discovery is delayed or unavailable.
    • Editor availability checks run more efficiently, including faster discovery on Windows.
  • Bug Fixes

    • Added a timeout and safe fallback for editor discovery, preventing configuration requests from hanging.
    • Improved Windows process diagnostics by gathering performance data more efficiently.
    • Platform-specific diagnostic timeouts improve reliability across Windows and POSIX systems.

Note

Medium Risk
Touches server config and Windows diagnostics/editor paths with timeout fallbacks that can hide editors if discovery is slow; changes are localized with tests but affect local environment connectivity behavior.

Overview
Fixes slow serverGetConfig on Windows by cutting process-diagnostics cost and keeping editor discovery from blocking config responses.

Windows process diagnostics no longer runs a per-process Get-CimInstance perf query. One PowerShell script loads all perf rows into $perfById, joins them when enumerating Win32_Process, and drops -Filter. Query timeouts are 5s on Windows and 1s on POSIX via a per-call timeoutMillis on runProcess.

Editor discovery on Windows uses SpawnExecutableResolution instead of serial async PATH scans per editor. On other platforms, probes run with concurrency 4. withEditorDiscoveryTimeout wraps discovery in config/WebSocket handling so availableEditors becomes [] after 5s instead of hanging the response.

Tests cover the bulk Windows CIM script shape, the editor-discovery timeout helper, and updated Windows editor discovery expectations.

Reviewed by Cursor Bugbot for commit f302154. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Process diagnostics now use platform-specific timeouts and batched Windows performance queries. Editor discovery uses concurrent command resolution, while server configuration applies a timeout and empty fallback for stalled discovery. Tests cover Windows querying, resolver behavior, and non-blocking configuration.

Changes

Diagnostics and editor discovery

Layer / File(s) Summary
Platform-specific process querying
apps/server/src/diagnostics/ProcessDiagnostics.ts, apps/server/src/diagnostics/ProcessDiagnostics.test.ts
Process execution accepts POSIX or Windows timeout values, and Windows performance data is indexed once by process ID instead of queried per process. Tests verify the generated PowerShell command.
Concurrent editor discovery
apps/server/src/process/externalLauncher.ts, apps/server/src/process/externalLauncher.test.ts
Windows discovery uses executable resolution, while other platforms probe editors concurrently with a limit of four. Tests mock executable resolution directly.
Timed server configuration loading
apps/server/src/ws.ts, apps/server/src/server.test.ts
Editor discovery in server configuration is bounded by a timeout and falls back to an empty list; websocket tests cover stalled discovery and path assertions.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ServerGetConfig
  participant EditorDiscovery
  participant ExecutableResolver
  Client->>ServerGetConfig: request configuration
  ServerGetConfig->>EditorDiscovery: resolve available editors
  EditorDiscovery->>ExecutableResolver: probe editor commands
  ExecutableResolver-->>EditorDiscovery: resolved commands
  EditorDiscovery-->>ServerGetConfig: editor IDs or timeout
  ServerGetConfig-->>Client: configuration with availableEditors
Loading

Poem

I’m a rabbit with a tidy queue,
PowerShell hops through processes two by two.
Editors race, but timeout gates the way,
Config still returns before the hay.
Tests nudge each path with paws of cheer.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address #3610 by fixing Windows diagnostics latency, editor discovery blocking, and config fallback behavior.
Out of Scope Changes check ✅ Passed The diff stays focused on Windows diagnostics, editor discovery, and related tests without clear unrelated additions.
Title check ✅ Passed The title clearly summarizes the main server-side performance fix for Windows diagnostics and editor discovery.
Description check ✅ Passed The description covers what changed, why, validation, and checklist items, matching the template well.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Jul 12, 2026
@SivanTechDev

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
apps/server/src/diagnostics/ProcessDiagnostics.test.ts (1)

223-257: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Test coverage is solid for the bulk query optimization.

The assertions verify each CIM class is queried exactly once, the $perfById join expression is present, and -Filter (which would indicate per-process queries) is absent. Consider also asserting the -ErrorAction SilentlyContinue flag is present on the perf query, since silent error handling is the degradation mechanism for environments where Win32_PerfFormattedData_PerfProc_Process is unavailable.

🤖 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/server/src/diagnostics/ProcessDiagnostics.test.ts` around lines 223 -
257, Extend the “queries each Windows CIM class once” test around
ProcessDiagnostics.readProcessRows to assert that the generated PowerShell
script applies “-ErrorAction SilentlyContinue” to the
Win32_PerfFormattedData_PerfProc_Process query. Keep the existing query-count,
join-expression, and -Filter assertions unchanged.
apps/server/src/server.test.ts (1)

3723-3741: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Test correctly validates non-blocking behavior, but runs for ~5 seconds.

Using Effect.never with TestClock.withLive means the test waits the full 5-second EDITOR_DISCOVERY_TIMEOUT in real time before the timeout fires and the fallback applies. This is functionally correct but adds ~5s to the test suite. If this becomes a concern, consider using a virtual TestClock with a shorter timeout override, or testing the timeout fallback at the loadServerConfig unit level rather than through the full WebSocket RPC.

🤖 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/server/src/server.test.ts` around lines 3723 - 3741, Shorten this test’s
runtime while preserving validation that server.getConfig is not blocked by
editor discovery. Replace the real-time five-second wait in the test around
buildAppUnderTest and the WebSocket RPC with a virtual TestClock and a shorter
editor-discovery timeout override, or move the timeout-fallback assertion to the
loadServerConfig unit path rather than the full RPC flow.
🤖 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.

Nitpick comments:
In `@apps/server/src/diagnostics/ProcessDiagnostics.test.ts`:
- Around line 223-257: Extend the “queries each Windows CIM class once” test
around ProcessDiagnostics.readProcessRows to assert that the generated
PowerShell script applies “-ErrorAction SilentlyContinue” to the
Win32_PerfFormattedData_PerfProc_Process query. Keep the existing query-count,
join-expression, and -Filter assertions unchanged.

In `@apps/server/src/server.test.ts`:
- Around line 3723-3741: Shorten this test’s runtime while preserving validation
that server.getConfig is not blocked by editor discovery. Replace the real-time
five-second wait in the test around buildAppUnderTest and the WebSocket RPC with
a virtual TestClock and a shorter editor-discovery timeout override, or move the
timeout-fallback assertion to the loadServerConfig unit path rather than the
full RPC flow.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d9466a82-f659-433c-9e74-3d47afb862ab

📥 Commits

Reviewing files that changed from the base of the PR and between c1ec191 and 9526c33.

📒 Files selected for processing (6)
  • apps/server/src/diagnostics/ProcessDiagnostics.test.ts
  • apps/server/src/diagnostics/ProcessDiagnostics.ts
  • apps/server/src/process/externalLauncher.test.ts
  • apps/server/src/process/externalLauncher.ts
  • apps/server/src/server.test.ts
  • apps/server/src/ws.ts

@SivanTechDev SivanTechDev force-pushed the fix/windows-process-diagnostics branch from 9526c33 to 23ac856 Compare July 12, 2026 12:18
@SivanTechDev SivanTechDev marked this pull request as ready for review July 12, 2026 13:46
@macroscopeapp

macroscopeapp Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

Performance optimization for Windows diagnostics and editor discovery. Changes batch CIM queries, add concurrency, and include timeout fallbacks for resilience. Well-tested and self-contained with no new functionality.

You can customize Macroscope's approvability policy. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Windows 0.0.28/nightly local backend appears disconnected; server.getConfig interrupted, 0.0.27 works

1 participant