[MAJOR][ipc] 39 unguarded getAllWindows() send loops — destroyed-window throw can roll back sync (regression of #935)

[h4yfans]

Part of epic #987 — Memory & CPU full-app performance audit 2026-08.

Important

For the implementing agent: validate before implementing.
This issue was filed from a static audit on 2026-08-07 (main @ e4c9c90). Line numbers may have drifted.

1. First, CONFIRM the issue is still valid on current main: read the referenced files, verify the described behavior still exists in code, and check that no later commit already fixed it.
2. If it is no longer valid, comment your evidence and close the issue — do not implement.
3. If valid, implement following the conventions in CLAUDE.md/AGENTS.md: surgical change, tests included, run the Verify commands (pnpm lint, pnpm typecheck, relevant pnpm test:*, pnpm docs:impact) before opening a PR that references this issue.
4. Where a memory/CPU delta is measurable, attach before/after evidence (see Add memory snapshot harness for worktree vs main benchmarks #446 memory snapshot harness).

---

Problem. The guarded helper broadcastToAllWindows (apps/desktop/src/main/lib/window-broadcast.ts:12-18, checks isDestroyed() on both window and webContents) exists precisely because an unguarded throw inside a DB transaction rolled back applied sync items. Despite the migration (#935, commits c086ca2/430904df7/39b56e73c), 39 hand-rolled BrowserWindow.getAllWindows().forEach(win => win.webContents.send(...)) loops remain. Highest-risk sites: sync/item-handlers/settings-handler.ts:163-194 (inside sync apply → rollback), sync/token-manager.ts:107-109 (SESSION_EXPIRED, fires during teardown when windows are dying), ipc/sync-core-handlers.ts:177. Full list also covers vault/index.ts:200,210,220,247, vault/notes-io.ts:52, vault/watcher.ts:117, vault/rename-tracker.ts:80, sync/crdt-writeback.ts:151, sync/linking-service.ts:545, 7 sites in ipc/settings-handlers.ts, 4 in ipc/sync-attachment-handlers.ts, plus journal/bookmarks/tags/canvas/saved-filters/calendar/locale/ai-inline/auth-device/auth-oauth handlers, import/import-context.ts:41, inbox/review-notification.ts:24, lib/reminders.ts:281.

Trigger. Any window destroyed concurrently with an emit — near-certain over a full-day multi-window session.

Suggested fix. Mechanically replace all production loops with broadcastToAllWindows. Add an ESLint no-restricted-syntax rule banning webContents.send inside getAllWindows() loops so it cannot regress a fourth time.

Verification. Grep returns zero unguarded loops outside test hooks; lint rule fails on a synthetic violation.

[h4yfans]

Scope note so this does not read as fully closed by #1119.

Confirmed still valid on current main. Of the 39 sites listed here:

• 37 are fan-out loops already covered by the open draft fix(main): migrate remaining window fan-outs to guarded broadcastToAllWindows #937 (window-broadcast-fanout-remaining). Not re-done in fix(main): route all window fan-outs through guarded broadcastToAllWindows + lint guard #1119 — no duplicate helper, no duplicate migration. fix(main): migrate remaining window fan-outs to guarded broadcastToAllWindows #937 is currently CONFLICTING/DIRTY and 63 commits behind main, so it needs a rebase before the bulk of this issue can land.
• 2 are window-selection sites fix(main): migrate remaining window fan-outs to guarded broadcastToAllWindows #937 explicitly leaves alone (its commit message: "Window-selection sites (find/filter/[0]/length) are untouched") — inbox/review-notification.ts:24 and lib/reminders.ts:281. Both take getAllWindows()[0] in a Notification 'click' handler and throw Object has been destroyed if that window died, focusing nothing even when a live window is next in the list. Those two are fixed in fix(main): route all window fan-outs through guarded broadcastToAllWindows + lint guard #1119.
• vault/templates.ts was migrated on main independently and is already clean.

The suggested ESLint no-restricted-syntax rule is not in #1119 on purpose: on today's main it would fail pnpm lint on the 37 loops #937 owns. It should ride with #937 or land immediately after it.

So: #1119 closes the remainder; this issue should stay open until #937 lands (or is superseded) plus the lint rule.

[h4yfans]

Correction to my earlier comment — #1119 now covers this issue completely, not "2 of 39".

The earlier note said the 37 fan-out loops were left to #937. That would have left this issue effectively open, since #937 is CONFLICTING/DIRTY and 63 commits behind main. #1119 now carries the whole fix on top of current main.

What changed since that comment:

• All 38 fan-out loops migrated to broadcastToAllWindows (38, not 39 — the audit counted a slightly older tree). Site list re-derived from a fresh grep of main, not replayed from a patch.
• ESLint no-restricted-syntax rule added, rejecting for...of and .forEach fan-out loops over getAllWindows() in apps/desktop/src/main/**. Verified it fires on a synthetic violation (2 errors, one per loop shape). It is anchored on getAllWindows() so it does not false-positive on crdt-provider's per-doc windowIds loop, which must skip the source window and would break if turned into a fan-out.
• Helper hardened: a window dying between the isDestroyed() guard and the send is now logged and skipped instead of aborting the fan-out and throwing into a caller that is mid-transaction. Send arity is forwarded exactly, so the zero-payload quick-capture:open broadcast is unchanged.
• Skipped deliberately, not missed: vault/templates.ts was already migrated on main; agent-handlers/agent-lazy-handlers/menu.ts/index.ts/canvas-write.ts are window-selection sites, not fan-outs.

Verification: main suite 5422 passed / 0 failed, typecheck pass, pnpm lint 0 errors with the new rule enabled.

Note for @h4yfans: #1119 overlaps #937 (window-broadcast-fanout-remaining) by design. Nothing in #1119 touches that branch — it is yours to close, keep, or land as you prefer.