Recurring desktop topology polls currently cross synchronous Electron IPC, so a slow main-process reply can stall typing and rendering. This keeps the synchronous startup bridge for compatibility, caches its initial snapshot and relaunch-only enabled setting, and refreshes topology through asynchronous IPC during polling.
The sync and async handlers share main's current bootstrap builder. Successful async replies replace the cached snapshot, including an empty topology; failed refreshes retain the last successful snapshot. Concurrent refreshes share one request. Older desktop bridges retain the existing synchronous fallback, and unchanged topology keeps its array identity.
Maintainer takeover of #19300, credit to @daniyal-sh for the original fix. Contributor commits are retained, main was merged, and this updates the original PR. Push-based topology is outside this change.
The actual web app ran against isolated worktree state. A controlled bridge exercised the actual desktop-local reader and preload snapshot helper while its async topology request was held pending. All 23 trusted keystrokes appeared in the real composer before request completion; overlapping refreshes shared one invoke, cached reads used one startup sync read, and resolving an empty result cleared the topology. No message was submitted.
This is a controlled integration proof, not a native Electron or natural-workload latency benchmark. The bridge was private to the probe; it did not replace the web app's global desktop bridge or simulate a live WSL backend. The historical benchmark claims from the original body have been removed.
Typing in the actual composer while the controlled topology refresh remains pending
Recording of typing with the pending refresh · Probe observations
Packaged Electron cannot start on this Linux VM because libcups.so.2 is missing. Native Windows typing/FPS under natural workload, Electron contextBridge behavior, live WSL startup/removal and endpoint/token changes still need a desktop pass. macOS, Windows and native Linux Electron were not verified; iOS/Android were not run and have no changed client path. Web-only rendering was exercised as described above. Current-head CI passed lint, build/typecheck, all eight server shards, general tests, web tests, release smoke and the aggregate Check. One unchanged ACP Registry browser timing test failed on the first run; the failed-job retry passed without source changes.
Implementation: GPT-6.1 Sol; tests: GPT-6 Astra; adversarial reviews: Claude Opus 5.5 and GPT-6 Sol. Codex and Claude Code harnesses in T3 Code.
Review rate limited
Added 1 commit
Verdict: Approved at 1def7bb
Macroscope's review found this PR approvable — This is a focused desktop performance fix that moves recurring topology polling off synchronous renderer-blocking IPC while preserving startup behavior, failure handling, snapshot identity, and older-bridge compatibility. The implementation is localized, additive at the contract boundary, and covered by targeted tests.
You can add or adjust custom eligibility rules. Learn more.
Warning
Only developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing.
Next included review available in 9 minutes.
Limit details: You’ve used all 10 included reviews currently available.
Review configuration:
e9175fec-e2a2-4ac3-b860-9a8ba03706a7Reviewing files that changed from the base of the PR and between 1def7bb217bd5ded4724e2f2315654c87905f349 and 362656905f7733df5bb71504607aa6b21ce4d668.
apps/desktop/src/ipc/DesktopIpcHandlers.tsapps/desktop/src/ipc/methods/window.tsapps/desktop/src/preload.tsapps/desktop/src/preloadSnapshot.test.tsapps/desktop/src/preloadSnapshot.tsapps/web/src/connection/desktopLocal.test.tsapps/web/src/connection/desktopLocal.tsapps/web/src/connection/platform.tsdocs/internals/performance-regressions.mdpackages/contracts/src/ipc.tsNo actionable comments were generated in the recent review. 🎉
421cc5db-f925-4a4f-b477-e3ba02b8d146Reviewing files that changed from the base of the PR and between 8855a6dd052a8cbc9aac125dc2e8d14a0c2decd6 and 1def7bb217bd5ded4724e2f2315654c87905f349.
apps/desktop/src/ipc/DesktopIpcHandlers.tsapps/desktop/src/ipc/methods/window.tsapps/desktop/src/preload.tsapps/desktop/src/preloadSnapshot.test.tsapps/desktop/src/preloadSnapshot.tsapps/web/src/connection/desktopLocal.test.tsapps/web/src/connection/desktopLocal.tsapps/web/src/connection/platform.tsdocs/internals/performance-regressions.mdpackages/contracts/src/ipc.tsIncluded review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
The desktop now provides asynchronous local-environment bootstrap reads alongside the existing synchronous method. The preload and web readers use asynchronous refreshes while retaining cached topology for synchronous reads and older desktop bridges.
Changes: Local-environment topology refresh
| Layer / File(s) | Summary |
|---|---|
Add asynchronous bootstrap IPC packages/contracts/src/ipc.ts, apps/desktop/src/ipc/methods/window.ts, apps/desktop/src/ipc/DesktopIpcHandlers.ts | DesktopBridge adds an optional asynchronous bootstrap method. The desktop registers it on the existing channel and shares bootstrap snapshot construction with the synchronous method. |
Cache preload bootstrap snapshots apps/desktop/src/preloadSnapshot.ts, apps/desktop/src/preload.ts, apps/desktop/src/preloadSnapshot.test.ts | The preload snapshot helper caches the initial value, shares in-flight refreshes, and replaces the value after successful refreshes. The preload bridge uses the helper for bootstrap reads and caches the enabled setting. Tests cover pending reads, failures, recovery, and asynchronous startup results. |
Refresh web topology through the bridge apps/web/src/connection/desktopLocal.ts, apps/web/src/connection/desktopLocal.test.ts, apps/web/src/connection/platform.ts, docs/internals/performance-regressions.md | The web reader refreshes asynchronously when supported and falls back to the synchronous method otherwise. Failed reads leave the prior snapshot intact. Platform registration awaits the topology read, and the documentation describes asynchronous polling. |
Priority: ➖ Normal
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
| 1 | sequenceDiagram |
| 2 | participant Renderer |
| 3 | participant WebReader as DesktopSecondaryBootstrapsReader |
| 4 | participant Preload as DesktopBridge |
| 5 | participant MainIPC as readLocalEnvironmentBootstraps |
| 6 | participant Snapshot as readLocalEnvironmentBootstrapSnapshot |
| 7 | Renderer->>WebReader: refresh topology |
| 8 | WebReader->>Preload: readLocalEnvironmentBootstraps |
| 9 | Preload->>MainIPC: asynchronous IPC request |
| 10 | MainIPC->>Snapshot: read bootstrap snapshot |
| 11 | Snapshot-->>MainIPC: bootstrap list |
| 12 | MainIPC-->>Preload: bootstrap list |
| 13 | Preload-->>WebReader: refreshed bootstrap list |
| 14 | WebReader-->>Renderer: topology read result |
Suggested reviewers: juliusmarminge
1def7This change moves recurring desktop topology reads off synchronous IPC so the renderer is no longer blocked. It keeps startup reads synchronous and keeps the fallback for older bridges. No concrete merge-blocking risk was found.
1def7The change preserves the existing permissions and credential flow without demonstrating a new access path. Remaining risk concerns stalled refreshes leaving connection information stale.
Retained concerns No architecture-level concerns identified.
Security Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
Failed Failed checks: 1 warning
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check | Warning | The description clearly explains the problem, implementation, compatibility behavior, verification, limitations, and contributors. However, it does not provide the required explicit maintainer approva… | Add a Scope and approval section that links the triaged issue or discussion and quotes or links the maintainer approval comment. If no prior approval was required, explain why this focused fix qualifies for the exemption. |
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check | Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | Passed | Check skipped because no linked issues were found for this pull request. |
| Title check | Passed | The title clearly and concisely identifies the main change: preventing renderer blocking during desktop topology polling. |
Explanation
The description clearly explains the problem, implementation, compatibility behavior, verification, limitations, and contributors. However, it does not provide the required explicit maintainer approval of the direction and scope, including an approval comment or a clear small-fix exemption.
Resolution
Add a Scope and approval section that links the triaged issue or discussion and quotes or links the maintainer approval comment. If no prior approval was required, explain why this focused fix qualifies for the exemption.
Comment @coderabbitai help to get the list of available commands.
Thanks @daniyal-sh — I built on this in #19511. Your commit is cherry-picked unchanged as the first commit, and a second commit makes the desktop push topology changes into your preload cache, so the renderer no longer polls the main process at all (it also fixes the trace flooding in #19505). That made the async readLocalEnvironmentBootstraps refresh unnecessary, so #19511 removes it. Before/after desktop traces are in the PR body.
Taking over this one as maintainer for desktop-app-10. I will preserve @daniyal-sh’s commits, integrate main, minimize the behavior tests and review the recurring async topology path across preload, IPC and web. The separate push-topology proposal is outside this batch. Current-head natural-workload native verification will remain explicit; this does not include merging.
Added 3 commits
Current-head CI passed lint, build/typecheck, all eight server shards, the general test job and release smoke. Web tests passed 8,048/8,049; the single failure is in the unchanged ACP Registry browser test's “submits a typed query at once…” case at line 169.
The test uses real time between filling the input and pressing Enter, while the unchanged component submits after a 300 ms debounce. If that debounce wins, Enter legitimately refreshes the already-submitted query and violates the test's no-refresh assumption. Neither the component nor this test differs from pinned main. I am rerunning the failed jobs once to distinguish this timing race from a persistent failure; no topology source change or unrelated test edit is justified by this result.
The takeover's five focused files passed 45/45, and independent review also passed 87/87 with adjacent auth/bootstrap suites. Native Windows/WSL natural-workload verification remains a human follow-up as documented in the PR body.
Added 1 commit
Reverting this in #22222 together with the other desktop fixes the takeover campaign merged yesterday. Nothing was wrong with the review of this change; it was merged on CI and code audits only, and after #20907 from the same batch broke existing installs the maintainer decided that desktop changes do not ship without being run on a real desktop build. To bring it back: open a fresh PR from the original commit, run it on a real build that already has user state on each platform it touches, and put the before/after proof in the body.
