fix(main): route all window fan-outs through guarded broadcastToAllWindows + lint guard

[h4yfans]

Fixes #1000

Complete fix. Every BrowserWindow.getAllWindows() fan-out in the main process now goes through the guarded broadcastToAllWindows helper, and an ESLint rule stops the hand-rolled pattern coming back a fourth time.

Verification (the issue was real on current main)

getAllWindows() can still list a destroyed short-lived window (splash, quick capture, print/export) — that is why broadcastToAllWindows (apps/desktop/src/main/lib/window-broadcast.ts) exists. Any access to such a window throws Object has been destroyed.

The worst case is sync/item-handlers/settings-handler.ts, which runs inside the sync item's DB transaction: a throw there escapes ctx.emit and rolls back an item that was already applied. sync/token-manager.ts (SESSION_EXPIRED) is next — it fires during teardown, exactly when windows are dying.

Sites

I re-derived the list with a fresh grep of main rather than replaying a patch, and classified every hit.

38 fan-out loops migrated (all of them; the issue's "39" counted a slightly older tree):

Area	Sites
ipc/settings-handlers.ts	8
ipc/sync-attachment-handlers.ts	4
vault/index.ts	4
ipc/auth-device-handlers.ts, ipc/auth-oauth-handlers.ts	2 each
import/import-context.ts, ipc/ai-inline-handlers.ts, ipc/bookmarks-handlers.ts, ipc/calendar-handlers.ts, ipc/canvas-handlers.ts, ipc/journal-handlers.ts, ipc/locale-handler.ts, ipc/saved-filters-handlers.ts, ipc/sync-core-handlers.ts, ipc/tags-handlers.ts, sync/crdt-writeback.ts, sync/item-handlers/settings-handler.ts, sync/linking-service.ts, sync/token-manager.ts, test-hooks.ts, vault/notes-io.ts, vault/rename-tracker.ts, vault/watcher.ts	1 each

Skipped as already fixed or not applicable (no dead code resurrected):

• vault/templates.ts — listed in the issue, but already migrated on main by an earlier commit. Untouched.
• ipc/agent-handlers.ts:462, ipc/agent-lazy-handlers.ts:194 — resolveSenderWindowId, window selection, not a fan-out.
• agent/mcp/tools/canvas-write.ts:40, menu.ts:31, index.ts (×7) — selection sites (find / filter / [0] / length), most already isDestroyed-guarded.
• sync/crdt-provider.ts:716 — iterates a doc's own windowIds and must skip sourceWindowId. Rewriting it as a fan-out would change the target set and reintroduce the IPC echo loop. Deliberately left alone, and the lint rule is written so it does not flag it.

Also in this PR (from the first commit, the two sites the issue lists that are selection rather than fan-out): inbox/review-notification.ts and lib/reminders.ts took getAllWindows()[0] in a Notification click handler and threw if that window had died — focusing nothing even with a live window next in the list. Both now take the first live window.

Relationship to #937

#937 covers overlapping ground with the same approach and is @h4yfans' to close, keep, or land as he prefers — nothing here touches that branch. Where the two differ:

• fix(main): migrate remaining window fan-outs to guarded broadcastToAllWindows #937 wraps all three sends in settings-handler.ts in a bare catch {}. This PR keeps the tolerance but logs, so a real fault is not swallowed.
• fix(main): migrate remaining window fan-outs to guarded broadcastToAllWindows #937 makes the helper's data optional, which would send (channel, undefined) for the zero-payload quick-capture:open broadcast. This PR uses a rest parameter so the arity is byte-identical — settings-handlers.test.ts asserts toHaveBeenCalledWith('quick-capture:open') with exactly one argument.
• The ESLint rule and the per-window fault isolation are not in fix(main): migrate remaining window fan-outs to guarded broadcastToAllWindows #937.

Semantics

Every migrated site keeps its channel, payload, and target set. Deliberate preservations:

• Zero-payload arity. broadcastToAllWindows(channel, ...args) forwards exactly what it was given.
• Teardown tolerance in settings-handler.ts. The old code caught getAllWindows() throwing and returned silently. The helper does not catch that, so the try stays — but now logs via the existing log rather than swallowing. This function must never throw; it is inside a DB transaction.
• test-hooks.ts ordering. The original loop sent up to three events per window, with only the middle one conditional. Per-window ordering is preserved.
• settings-handlers.ts:1051 already had a window-level isDestroyed() check; it now also gets the webContents-level check. Same targets.

New helper behaviour: a window that dies between the guard and the send is logged and skipped instead of aborting the fan-out and throwing into the caller. A fan-out that silently stops reaching windows is worse than the throw it replaced, so the failure is logged, never dropped.

Tests

Per your steer, I covered the helper's guarantees once rather than writing 38 near-duplicate tests, and extended the existing window-broadcast.test.ts instead of adding a new file. It went 3 → 6 cases:

• (existing) delivers to every live window; skips a destroyed window; skips a window whose webContents is destroyed
• new — a throw from one window does not abort delivery to the rest and does not propagate to the caller
• new — a failed delivery is logged rather than dropped silently
• new — zero-payload send arity is preserved

Plus the two notification-click regression tests from the first commit (reminders.test.ts, review-notification.test.ts), which went red→green with Object has been destroyed and survived a mutation check.

30 existing test files had electron mocks returning windows with no isDestroyed, which the guard requires. Those mocks were updated (isDestroyed: () => false) — mock-shape only, no assertion was weakened or deleted.

Checks

main suite   5426 tests: 5422 passed, 0 failed, 4 skipped
typecheck    pass (contracts + architecture boundaries, RPC bindings, IPC invoke map up to date)
lint         0 errors, 14 warnings — all 14 pre-existing in unrelated renderer files
prettier     clean

Lint rule proof — reintroducing a raw loop in main/lib/synthetic-violation.ts:

4:3  error  Do not hand-roll a window fan-out loop. Use broadcastToAllWindows() ...  no-restricted-syntax
10:3  error  Do not hand-roll a window fan-out loop. Use broadcastToAllWindows() ...  no-restricted-syntax

✖ 2 problems (2 errors, 0 warnings)

Both loop shapes (for...of and .forEach) are caught. The two-step form (const windows = getAllWindows() then iterating the variable) is not caught — it is not statically distinguishable from legitimate window selection, and forcing it would false-positive on agent-handlers.ts. That limitation is written into the rule's comment rather than left implied.

Docs updated in apps/docs/src/architecture/ipc.md: per-window fault containment, arity forwarding, the lint rule and why subset loops stay allowed, and the first-live-window idiom for single-window targeting.

[github-actions]

React Doctor found no new issues. 🎉

_{Reviewed by React Doctor for commit 08a0e48.}

[codecov]

Codecov Report

❌ Patch coverage is 93.54839% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...s/desktop/src/main/ipc/sync-attachment-handlers.ts	50.00%	2 Missing ⚠️
apps/desktop/src/main/ipc/settings-handlers.ts	87.50%	1 Missing ⚠️
...op/src/main/sync/item-handlers/settings-handler.ts	85.71%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!