sync: latch the crypto worker bridge off after repeated failures

[h4yfans]

Closes #964

Builds directly on #960.

Problem

#960 made a rejected worker crypto request degrade to main-thread crypto instead of failing the batch. It deliberately did not stop asking the worker, and the two failure shapes are very far apart in cost.

A worker that crashes is already fine — the exit handler nulls this.worker, isRunning goes false, and later batches skip it. A worker that answers with an error is fine too: the reply is immediate and the fallback costs sub-millisecond IPC.

The gap is the worker that is alive but silent. isRunning was this.worker !== null, which only knows the thread has not exited, so a hung-but-alive worker kept reporting healthy and kept being chosen. Nothing anywhere short-circuits: sendRequest arms a fresh 60s timer per request, and isRunning has exactly two production consumers, both of them the per-batch gate in sync-crypto-batch.ts. Every push batch and every pull batch therefore paid a full minute before degrading. Sync still completed — that is #960 working — but at a rate that reads as broken.

Fix

SyncWorkerBridge counts consecutive failed requests. After three it latches: isRunning reports false, and sync-crypto-batch.ts takes the main-thread path with no round trip. A successful batch resets the count to zero.

Both encryptBatch and decryptBatch are wrapped, so the count also covers the post-response throws ({ type: 'error' } protocol drift, unexpected response type), not just sendRequest rejections.

Design decisions

N = 3. One failure is noise — a single transient timeout under load should not cost the session its worker. Three consecutive failures is not noise. Because the penalty is paid in whole minutes, a larger N is expensive: three silent batches is a bounded ~3 minutes of degraded-but-correct sync, after which the worker costs nothing for the rest of the session. The reasoning lives in a comment on the constant.

Latch scope: session-lifetime, not a timed retry. The acceptance criterion asks for a bounded, one-time penalty. A cooldown-and-reprobe scheme cannot deliver that — every expiry buys another 60s stall — whereas a session latch gives a hard ceiling of three timeouts total. It is also simpler and deterministic: no timers, no clock. The cost of being wrong is small and bounded in the right direction: main-thread crypto is correct (the whole point of #960), just slower, so losing the worker for one session is a performance regression and never a correctness one. Re-probing a wedged worker, by contrast, costs the user another frozen minute each time.

The reset-on-success still does the work that matters, because it applies before the latch trips — that is exactly the transient-hiccup case. In-session recovery after a latch is stop() then start(), which spawns a fresh thread and clears the count; a fresh thread is not the thread that failed.

The worker thread is not terminated. Raised as an open question in the issue. Terminating buys nothing once the bridge has stopped routing to it, and it would add a failure mode (terminating a wedged thread, exit racing rejectAll) while removing the only in-session way back. stop() deliberately still keys off this.worker rather than isRunning, so a latched-but-alive thread is still shut down cleanly and does not leak.

Why this cannot mask a crypto or auth failure

Re-verified against current worker.ts, not assumed from #960. Per-item crypto outcomes still come back in-band: encrypt-batch-result carries errors[], decrypt-batch-result carries failures[] including signature mismatches. Neither rejects the request. The only producer of { type: 'error' } is handleUnknownMessage, i.e. protocol drift. So an encryptBatch/decryptBatch rejection is purely infrastructure, and the latch counts nothing else. Latching cannot suppress a bad item — the main-thread path re-runs the identical encryption and signature verification.

No key material or plaintext is logged: the latch warning carries only the failure count and the transport error text, and crypto error messages never travel that path.

Backward compatibility

No schema, contract, wire-protocol, or persisted-format change. Purely in-process bridge state. The main↔worker protocol is untouched, so mixed-build installs behave exactly as they do today. Behaviour when the worker answers normally is unchanged — the new path is reachable only after three consecutive infrastructure failures, which previously meant three 60s stalls.

Tests

worker-bridge.test.ts — 4 new: still routes to the worker below the threshold; latches at the threshold and stops issuing round trips; a success clears the count and the threshold is then re-counted from that success; a restarted bridge is no longer latched.

sync-crypto-batch.test.ts — 2 new, pinning the other half of the contract: an isRunning: false bridge encrypts and decrypts on the main thread without ever calling the worker.

Whole src/main/sync suite green:

Test Files  136 passed (136)
     Tests  1653 passed | 1 expected fail | 3 skipped (1657)

Mutation check — worker-bridge.ts stashed, tests re-run:

 × worker-bridge.test.ts > #then still routes to the worker below the latch threshold
 × worker-bridge.test.ts > #then latches off at the threshold so no further round trip is paid
 × worker-bridge.test.ts > #then a successful batch clears the failure count
 × worker-bridge.test.ts > #then a restarted bridge is no longer latched
Test Files  1 failed | 1 passed (2)
     Tests  4 failed | 31 passed (35)

Fix restored, green again. pnpm typecheck 16/16 successful, pnpm lint 0 errors (10 pre-existing renderer warnings, none in changed files), pnpm docs:impact --strict covered, pnpm docs:build complete.

The first draft of the reset test asserted only isRunning === true, which passes with no latch at all; it was tightened to also assert the latch still fires on the next failure, so all four are mutation-sensitive.

Deliberately out of scope

• Bounding main-thread fallback latency for large pull batches. Raised in sync: worker crypto rejection fails the whole batch instead of degrading to main-thread crypto #957 and still open. No measurements exist for how long a realistic large batch takes on the main thread, and inventing a bound without them is guesswork.
• encryptItemForPush throwing in the fallback (e.g. "Item too large for sync") still propagates out of encryptPushBatch instead of becoming a per-item queue.markFailed. That is how the main-thread path has always behaved when no worker is configured, so it is a pre-existing rough edge rather than a regression.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

React Doctor found no new issues. 🎉

_{Reviewed by React Doctor for commit 20948f7.}

[codecov]

Codecov Report

❌ Patch coverage is 84.37500% with 5 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
apps/desktop/src/main/sync/worker-bridge.ts	84.37%	5 Missing ⚠️

📢 Thoughts on this report? Let us know!