close
Skip to content

fix: keep windows gateway cold starts on the splash - #5964

Merged
bolichen97 merged 1 commit into
release/0.4.0from
codex/backport-0.4.0-slow-gateway-startup
Aug 25, 2026
Merged

fix: keep windows gateway cold starts on the splash#5964
bolichen97 merged 1 commit into
release/0.4.0from
codex/backport-0.4.0-slow-gateway-startup

Conversation

@bolichen97

Copy link
Copy Markdown
Collaborator

Problem / Motivation

The Windows first-launch fix in #5956 also belongs on release/0.4.0 so the
next Insider candidate does not show a false Gateway startup failure while a
freshly installed bundled Python runtime is still starting. The same release
also reports harmless Proactor WinError 10054 connection teardown as a crash.

Why it matters

Without the backport, Insider users can still see the broken first-launch
experience even after the fix is validated on main: the desktop gives up at
30 seconds, Retry succeeds after the roughly 55-second cold start, and teardown
noise makes the healthy Gateway look as though it crashed.

What changed (motivation → approach → change)

Cherry-pick the single validated commit from #5956 onto release/0.4.0:

  • Give only the primary locally spawned Windows gateway a 120-second readiness
    deadline. Remote connection tabs and non-Windows launches keep 30 seconds,
    and a child exit or spawn error still fails immediately.
  • Downgrade only a ConnectionResetError reported from
    _ProactorBasePipeTransport._call_connection_lost to warning-only teardown
    noise. Task-level and unrelated resets remain ERRORs with crash evidence.
  • Carry the matching Windows, Electron, and error-handling documentation.

Tests

  • On the release/0.4.0 backport: test/test_crash_guard.py — 10 passed.
  • On the release/0.4.0 backport: Electron gateway-wait.test.js — 24 passed.
  • Source PR fix: keep windows gateway cold starts on the splash #5956 was merged to main as 9dbdcd2de; this backport's release
    CI is the final validation for the release/0.4.0 baseline.

Manual verification

Inspected the installed Nightly launch log: the gateway child remained alive
and became healthy after roughly 55 seconds, beyond the old 30-second Electron
deadline. The same running gateway returned HTTP 200 after Retry.

Screenshots / video

Why no screenshot: The splash rendering is unchanged; only how long it keeps waiting for a live Windows gateway changes.

Related Issues

no linked issue: this is the release/0.4.0 backport of #5956.

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable)
  • No secrets, credentials, or internal references in the diff

Allow a live local Windows gateway to use the longer bundled Python
cold-start window while preserving fail-fast child exits and ordinary
connection deadlines.

Downgrade only the Proactor connection-lost reset callback to warning
noise; task-level resets still keep crash breadcrumbs.
@bolichen97
bolichen97 requested a review from a team as a code owner August 25, 2026 23:33
@bolichen97
bolichen97 requested review from cixuuz and removed request for a team August 25, 2026 23:33
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Aug 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 6444fc2e4943c5219c80ac8fd8f50f77023512e3 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 6444fc2

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 6444fc2e4943c5219c80ac8fd8f50f77023512e3: <one-sentence reason>

@github-actions

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed 6444fc2e4943c5219c80ac8fd8f50f77023512e3 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 6444fc2

Verdict parsed from the review's SHA-scoped output markers for commit 6444fc2e4943c5219c80ac8fd8f50f77023512e3.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 6444fc2e4943c5219c80ac8fd8f50f77023512e3: <one-sentence reason>

@github-actions

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

Design-level review of 6444fc2e4943c5219c80ac8fd8f50f77023512e3 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Faithful backport of a validated main fix; timeout widening is ownership-scoped, fail-fast child-exit paths survive, and the log downgrade fails loud if CPython internals shift.

The two risky degrees of freedom are both constrained correctly: the 120s deadline applies only when watchSpawn && gatewayOwnership === "spawned" on win32 (remote tabs, adopted gateways, and non-Windows keep 30s, and getFailure still wins on child exit), and the crash-guard downgrade requires both ConnectionResetError and the _ProactorBasePipeTransport._call_connection_lost callback name — so a CPython rename regresses toward noisy ERRORs, not silent suppression. Diverging from the cherry-picked commit here would defeat the backport's purpose, so no design changes belong in this PR.

[DESIGN-REVIEWED] 6444fc2

@github-actions

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 6444fc2e4943c5219c80ac8fd8f50f77023512e3 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

First-Principles-Verdict: PASS

A validated main fix backported intact: two named defects (false 30s failure, phantom crash report), each removed at mechanism level, nothing riding along.

What this change ships

Intent: stop a healthy Windows first launch from being reported as a gateway failure/crash on the 0.4.0 Insider line — a FIX (backport of #5956, merged to main as 9dbdcd2de).

  1. Windows splash now waits 120s for the gateway the app itself spawned — justified (manual log: healthy at ~55s, old deadline 30s).
  2. Remote tabs, adopted gateways, and non-Windows keep the 30s deadline — justified scoping; fail-fast on child exit preserved.
  3. Proactor _call_connection_lost ConnectionResetError logs as warning, no crash.log — justified; matcher is exception-type + callback-name narrow, task-level resets stay ERROR (pinned by test).
  4. Docs updated in the same commit (windows-install, error-handling, electron README) — mandated by AGENTS.md's same-commit spec rule.
  5. gateway-wait.js exports two timeout constants and gatewayWaitTimeoutMs — one real consumer (main.js:1257); constants are test-only. Undeclared, minor.

Root-cause placement is sound: the 55s cold start is a platform cost (first import of a freshly installed bundled Python tree on Windows), and the double-reported reset is CPython Proactor behavior — both genuinely out of scope, so deadline-widening and narrow suppression are the correct level. The pre-existing Unclosed downgrade in _asyncio_exception_handler is the same handler extended, not a second spelling (grepped: one handler, one noise path each).

Subtractions

  • Drop the DEFAULT_GATEWAY_WAIT_MS / WINDOWS_LOCAL_GATEWAY_WAIT_MS exports from website/electron/gateway-wait.js:168-169 — zero non-test consumers (grepped _WAIT_MS across website/: only gateway-wait.js and its test); export gatewayWaitTimeoutMs alone and let the test assert the 120s/30s mapping by value.

[FIRST-PRINCIPLES-REVIEWED] 6444fc2

@github-actions

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — ✅ PASS

UX-level review of 6444fc2e4943c5219c80ac8fd8f50f77023512e3 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: PASS

Removes a false first-launch failure honestly — live splash counter persists, fail-fast on real exits is kept, and no copy lies about state.

Suggestions

  • In waitForGateway's onStatus ("Waiting for gateway… ${s}s"), add expectation-setting once elapsed passes the old 30s on the extended Windows path — e.g. "Still starting — first launch can take up to two minutes". A bare counter climbing to 90s at the exact once-per-install moment invites a force-quit; one clause sets the budget the code already knows (120s).

[UX-REVIEWED] 6444fc2

@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Aug 25, 2026
@bolichen97

Copy link
Copy Markdown
Collaborator Author
  • Remove the exported timeout constants — rebutted.

    gatewayWaitTimeoutMs consumes both constants in production, while exporting those same internal values lets tests assert the policy’s single source of truth without duplicating 30s/120s literals. gateway-wait.js is a private Electron helper, so these test-facing exports add no public compatibility surface. This backport intentionally remains patch-identical to the main fix.

@bolichen97

Copy link
Copy Markdown
Collaborator Author
  • Add expectation-setting copy after 30 seconds — rebutted (valid follow-up, not a backport divergence).

    The splash remains responsive and continuously reports elapsed seconds, and only a freshly spawned local Windows gateway can enter the extended window. New rendered copy/localization and screenshot surface should be handled on main, not introduced uniquely in this release cherry-pick; this PR's purpose is to carry the validated false-failure fix intact.

@bolichen97
bolichen97 merged commit f59dccc into release/0.4.0 Aug 25, 2026
18 checks passed
@bolichen97
bolichen97 deleted the codex/backport-0.4.0-slow-gateway-startup branch August 25, 2026 23:38
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant