Skip to content

fix(server): retain editors when Windows discovery times out - #12404

Open
Salloxy wants to merge 1 commit into
pingdotgg:mainfrom
Salloxy:fix/windows-editor-discovery
Open

Salloxy wants to merge 1 commit into
pingdotgg:mainfrom
Salloxy:fix/windows-editor-discovery

Conversation

@Salloxy

@Salloxy Salloxy commented Sep 18, 2026 •

Copy link
Copy Markdown

What Changed

Windows editor discovery can find VS Code or Cursor, then discard them when a later PATH probe exceeds the five-second deadline. Preserve the editors found before timeout and give Windows scans ten seconds. Fast scans still return immediately; macOS and Linux retain their five-second budget.

Only completed scans enter the existing discovery cache. Timeouts can be retried on the next request, and client cancellation still propagates without poisoning the cache.

Run editor, file-reveal, and remote-target discovery concurrently so their deadlines do not add up against the client's 15-second connection-establishment limit. File reveal is still advertised only when a usable file manager was discovered. The service API and client protocol are unchanged.

Why

Fixes #4697.

Related approaches: #10694 and #11071. This change is limited to retaining partial results, the Windows timeout, and keeping config discovery within its connection budget; it does not redesign PATH lookup or add background refresh machinery.

Validation

  • Windows launcher suite: 15 passed; 15 platform-specific tests skipped.
  • Targeted config tests: 4 passed, including concurrent discovery with a ten-second editor scan and a stalled remote-target probe.
  • Regression coverage uses the test clock: partial results, immediate retry after timeout, completion after five but before ten seconds, stalled first probes on Windows/macOS/Linux, cache reuse, and interruption.
  • Server tsc --noEmit passed.
  • Targeted lint passed with existing warnings in unrelated portions of server.test.ts; git diff --check passed.
  • Read-only discovery against the actual Windows installation returned Cursor, VS Code, Zed, and File Explorer in 2.2 seconds. No installed-app changes, build, or browser verification.

Checklist

  • Small, focused backend fix
  • Explained the failure and the timeout tradeoff
  • Focused regression tests and server typecheck

Made using GPT-6 Astra on high reasoning, through Codex.

Summary by CodeRabbit

  • Bug Fixes

    • Editor discovery now stops waiting after a defined timeout, preventing stalled scans from blocking server startup.
    • Timed-out editor scans are not cached, allowing subsequent connection attempts to retry and find newly available editors.
    • File-manager and remote-target discovery now complete within the expected startup time, even when a provider is unresponsive.
  • Performance

    • Editor, file-manager, and remote-target discovery now run concurrently for faster configuration loading.

@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 Sep 18, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at d27eff9

Macroscope's review found this PR approvable — This is a focused server bug fix that preserves partial Windows editor discovery, avoids caching incomplete scans, and keeps optional config discovery within a bounded connection budget. Existing APIs and normal discovery paths remain unchanged, with targeted regression coverage for timeout, retry, caching, and concurrency behavior.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 18, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 0a6cdf79-1ab1-4e6a-bc99-dfc3ee8cbe72

📥 Commits

Reviewing files that changed from the base of the PR and between 7a2cce8 and d27eff9.

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

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Editor discovery now has platform-specific timeouts and reports whether scanning completed. Incomplete scans are not cached. Server configuration resolves remote open targets independently and concurrently, with timeout fallback and expanded tests.

Changes

Discovery timeout handling

Layer / File(s) Summary
Editor scan timeout and cache behavior
apps/server/src/process/externalLauncher.ts, apps/server/src/process/externalLauncher.test.ts
Editor scans return { editors, complete }, use 10-second Windows or 5-second non-Windows timeouts, and cache results only when complete. Tests cover interruption, partial results, retries, and platform bounds.
Concurrent remote target resolution
apps/server/src/ws.ts
resolveRemoteOpenTargetsForConfig replaces the previous resolver. loadServerConfig resolves editors, file-manager support, and remote open targets concurrently.
Timeout and concurrency test coverage
apps/server/src/server.test.ts
Tests cover concurrent discovery and interrupted remote-target discovery with empty-list fallback.

Priority: ⚪ Not assessed

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

Change: Bug fix · Severity of issue fixed: Low

Sequence Diagram(s)

sequenceDiagram
  participant loadServerConfig
  participant cachedAvailableEditors
  participant resolveRemoteOpenTargetsForConfig
  participant RemoteOpenTargets
  loadServerConfig->>cachedAvailableEditors: discover editors
  loadServerConfig->>resolveRemoteOpenTargetsForConfig: discover remote open targets
  resolveRemoteOpenTargetsForConfig->>RemoteOpenTargets: apply five-second timeout
  cachedAvailableEditors-->>loadServerConfig: editor results
  resolveRemoteOpenTargetsForConfig-->>loadServerConfig: targets or empty list
Loading

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to d27ef

The bounded discovery, retry-safe cache behavior, and concurrent remote-target resolution have no identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the Windows timeout fix, caching behavior, concurrent discovery, rationale, validation, and checklist status. The UI section is omitted appropriately because this is a…
Title check ✅ Passed The title is concise and accurately describes the primary change: preserving editors when Windows discovery times out.
Linked Issues check ✅ Passed Issue #4697 requires the Windows editor dropdown to show installed applications instead of an empty result. The change bounds Windows editor discovery at ten seconds, returns editors found before a la…
Out of Scope Changes check ✅ Passed The changes remain connected to issue #4697. Concurrent editor, file-manager, and remote-target discovery prevents the longer Windows editor budget from blocking server configuration. The resolver ren…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 4…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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

@MajesteitBart

Copy link
Copy Markdown

Thanks for addressing this. I reviewed d27eff9 while investigating #4697 on an affected Windows desktop. Preserving partial results and running config discovery concurrently are useful improvements. Two cases still appear to leave installed Open targets missing, including File Explorer:

  1. An earlier stalled probe can prevent Explorer from being checked. The scan is still sequential, and file-manager is last in EDITORS. Extending the Windows budget to ten seconds helps slow scans, but a probe that stalls until that deadline can still leave later targets undiscovered. If nothing has been found yet, the result is still []. The new stalled-first-probe tests explicitly cover and accept that empty result.

  2. An incomplete refresh can replace a previously complete list. After the 60-second cache expires, the scan starts with an empty accumulator. On timeout, cachedAvailableEditors leaves the old cache entry untouched but returns the new partial or empty editors list. Keeping the old entry in memory therefore does not preserve those targets in the config response.

Could this PR cover those cases too? The behavior I would look for is:

  • A stalled editor probe does not prevent other healthy targets, including Explorer, from being discovered.
  • A previously detected target stays available when its new probe times out, but is removed when a completed probe confirms it is missing.
  • Incomplete discovery remains eligible for retry, as this PR already allows.

Two useful regression scenarios would be an early stalled editor probe with a resolvable file manager, and a successful scan followed by cache expiry and a stalled refresh. The latter should also check that a later completed scan can remove a genuinely missing target.

This is based on code and test review of this PR; I have not run this branch in the desktop app. I reproduced the equivalent gaps with a focused module test against #10694. The affected user's released Windows app lost the entire Open menu, including Explorer, so retaining only targets reached before the deadline does not fully cover the reported behavior.

Reviewed and written with Codex (GPT-6), at the affected user's request.

This branch has not been deployed

No deployments
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]: No installed editors found

2 participants