Repository navigation
Flaky web-integration reconnect test: the injected clock collapses invoke's 120s timeout, not just the backoff #620
Description
electron/test/web-integration.test.js:298 — "reconnect: server drop → re-auth → threads:list resync + synthetic threads:changed" — fails intermittently with:
error: 'Timed out after 120000ms: threads:list'
stack: Timeout._onTimeout (src/wireClient.ts:163:16)
A 120-second timeout firing inside a test that takes 380 ms is the tell. Measured today on main (5b1c6da): 1 failure in 5 isolated runs, and it is the only red test in the whole suite (electron 1681/1682; renderer 1338/1338 green).
Root cause: one injected clock serves two unrelated timers
The test injects a fake setTimeout to collapse reconnect backoff to zero:
// web-integration.test.js:301
setTimeout: (fn, ms) => {
delays.push(ms);
if (!allowReconnect) return { unref() {} };
// Fire immediately; do not change production MIN_BACKOFF_MS (1000).
const t = setTimeout(fn, 0);
return t;
},But wireClient schedules both kinds of timer through that same seam (const schedule = opts.setTimeout ?? setTimeout, wireClient.ts:94):
scheduleReconnect()— backoff, ~1000 ms, should collapse (wireClient.ts:247)invoke()—INVOKE_TIMEOUT_MS= 120000 ms, must not collapse (wireClient.ts:159)
Once allowReconnect flips true, every subsequent invoke gets its 120 s abort timer fired at 0 ms. The final await api.threads.list() (line 333) then races: the socket round-trip and the immediate rejection are both macrotasks, and whichever lands first wins. When the timeout wins, the test reports a 120-second timeout after 380 ms.
The test's own comment at line 331 shows this was noticed but read as a nuisance rather than a bug: "invoke() also schedules INVOKE_TIMEOUT_MS (120s) on the same clock, so the backoff is no longer delays[0]."
Fix (verified)
Collapse only the backoff; let long timers keep their real delay.
setTimeout: (fn, ms) => {
delays.push(ms);
if (!allowReconnect) return { unref() {} };
- // Fire immediately; do not change production MIN_BACKOFF_MS (1000).
+ // Only collapse reconnect backoff. invoke() schedules its 120s
+ // timeout on this same clock; firing that at 0ms races the resync
+ // response and rejects a live call as "Timed out after 120000ms".
+ if (ms >= 60_000) return setTimeout(fn, ms);
const t = setTimeout(fn, 0);
return t;
},Before: 1 fail / 5 runs. After: 0 fail / 8 runs. No production code changes; delays.includes(1000) still asserts the real backoff.
makeClient's default injected clock (web-integration.test.js:177-183) has the identical shape and should get the same guard — it is only latent today because the other four tests do not flip allowReconnect.
Why this matters beyond one test
This is the failure the project has been treating as the known-red baseline — it is in shared memory as "the baseline on main, not your change." It is not a baseline; it is a fixable race, and carrying it has a cost: every agent and every human who runs the suite has to know which red is allowed, which is exactly how a second real failure slips through unnoticed.
With this fixed, both suites are green and the baseline becomes zero.
Related
#535 (claims 4 pre-existing red tests — stale, there is now only this one), #536 (closed; the other 7 are genuinely fixed), #481 (Windows suite hang — different cause, but the same "no timeout hygiene" family).
Activity
currentbits commentedon Aug 20, 2026
Failure rate is load-dependent, which sharpens the diagnosis and raises the priority.
That is exactly what the race predicts: under full-suite CPU contention the socket round-trip is likelier to lose to the immediately-fired abort timer. So CI, which always runs the full suite on a loaded box, sees this red most of the time — it is not the occasional blip the "known baseline" framing implies.
Also worth noting from the same 4 runs: no other flakes surfaced. Pass counts were 1684/1683/1683/1683 with the only delta being this test. Once it is fixed the suite is genuinely deterministic, which is what makes a zero-red baseline worth enforcing.
currentbits commentedon Aug 20, 2026
Supersedes #233, closed as a duplicate: same test (
web-integration.test.js, the reconnect case), same file, same120000 !== 1000symptom, filed 2026-08-17 before the mechanism was known.#233 offered two candidate causes ("either
wireClients backoff schedule changed and the test was never updated, or the production-vs-test branch is no longer taking the production path"). Neither was right - the root cause is the one in this issue:wireClientroutes bothscheduleReconnect()backoff andinvoke()sINVOKE_TIMEOUT_MSthrough the singleopts.setTimeout ?? setTimeoutseam (wireClient.ts:94), so collapsing the backoff to 0 ms also fires the 120 s invoke abort immediately.Note for whoever picks this up: the roadmap issue #335 lists "#233 (reconnect backoff test)" under Phase 0 Hygiene - that line should point here instead.
currentbits commentedon Aug 21, 2026
Fixed on
coder/fork-flaky-web-integration-reconnect-tes-470f77. Test-only: one injected clock (scheduleTestTimer) now collapses reconnect backoff and leaves invoke's 120s abort on the real delay.makeClient's default clock shares the same helper.Verified:
electron/test/web-integration.test.js5/5test/wireClient.test.ts14/14 (untouched; already queues timers instead of firing them)No production change.
delays.includes(1000)still asserts the real first backoff.currentbits commentedon Aug 22, 2026
Verified shipped on
main(audit 2026-08-22): implementation present in the working tree, not just on a worker branch. Closing — the planboard never auto-closed these (#632).currentbits commentedon Aug 22, 2026
Reopening: this is closed `plan:done` but the fix is not on main.
Verified on `2d33d30` (2026-08-21). `electron/test/web-integration.test.js:301` still collapses every timer:
```js
if (!allowReconnect) return { unref() {} };
// Fire immediately; do not change production MIN_BACKOFF_MS (1000).
const t = setTimeout(fn, 0);
```
So `invoke()`'s 120s abort timer fires at 0ms alongside the reconnect backoff, and the in-flight `threads:list` reports `Timed out after 120000ms` in a 140ms test.
Reproduced today:
Same load-dependent profile as the original report. The one-line discriminator from the issue still applies: only collapse the backoff, let the 120s timer keep its real delay.
currentbits commentedon Aug 22, 2026
Landed on
mainin the 2026-08-22 backlog merge (2d33d30..bf7c4dc, released as nightlynightly-202608220926-bf7c4dc).Evidence on main: electron/test/web-integration.test.js (injected clock no longer collapses invoke's 120s timeout)
The branch had been finished for a while but was never merged; verification after landing: typecheck clean,
vite buildok, core 38/38, renderer 1468/1468, memory 204/204, electron 1848/1848.