Skip to content

fix(desktop): stop the subframe navigation e2e racing a fixed timer - #97145

Merged
trunk-io[bot] merged 2 commits into
masterfrom
posthog/fix-desktop-subframe-e2e-race
Sep 9, 2026
Merged

fix(desktop): stop the subframe navigation e2e racing a fixed timer#97145
trunk-io[bot] merged 2 commits into
masterfrom
posthog/fix-desktop-subframe-e2e-race

Conversation

@puemos

@puemos puemos commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Problem

  • Every desktop release has failed since #96677 landed. The x64 macOS leg fails, so no x64 build reaches the update feed and finalize-release skips.
  • One test fails, three attempts in a row, on three different runners: main-process.spec.ts › blocks external protocol navigation from a renderer subframe (run 34298858904).
  • The test arms a will-frame-navigate listener behind a fixed 5 s timer, then builds an iframe in the renderer, then clicks. The timer measures the setup, not the behavior.
  • The x64 build runs under Rosetta on an Apple silicon runner. The trace from attempt 2 shows the iframe creation alone took 16 s, so the timer expired about 10 s before the click could happen.
  • fix(desktop): gate the release tag on master Desktop Tests #96677 was the first time this test ran under Rosetta. Before it, the x64 leg ran only the smoke spec.

Changes

  • The x64 release leg passes again. No user-facing change.
  • The listener now records the matching event on globalThis in the main process instead of rejecting after 5 s. The test polls for the record after the click with expect.poll, bounded to 30 s. That bound starts after the setup, so it measures event delivery alone and cannot be beaten by slow setup.
  • The listener keeps an explicit on/off pair rather than once. The app can emit unrelated subframe navigations during boot, so a match filter is required.
  • The app code under test, setupExternalLinkHandlers in external-links.ts, is unchanged.

Note

The release run cannot be re-run to green. The failure is deterministic, so this fix must ship in a new tag.

How did you test this code?

  • Packaged the Linux app and ran the full Electron suite under xvfb the way desktop-test.yml does. 15 of 15 pass, including the rewritten test.
  • Not run: the x64 macOS leg under Rosetta. This sandbox is Linux. The next release run is the real check for that leg.
  • The regression this guards: a slow environment failing the test before the navigation is ever attempted.

Automatic notifications

  • Publish to changelog?

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

  • Claude Code in PostHog Desktop, directed to find why the release failed and fix the root cause.
  • Skills invoked: /writing-tests, /writing-pr-descriptions.
  • The investigation covered all three attempts of the failing run and the Playwright trace artifact. A timeout bump was rejected because it only moves the cliff. Limiting the x64 leg to the smoke spec was discussed and deferred to a separate decision.
  • Duplicate search found no open PR for this fix.

Created with PostHog Desktop

The main-process listener started a 5s clock before the renderer setup ran. Under Rosetta on the x64 release leg that setup takes tens of seconds, so the clock expired before the iframe existed and every x64 release failed.

The listener now records the event and the test polls for it after the click, bounded by the test timeout instead of a fixed constant.

Generated-By: PostHog Desktop
Task-Id: 97a9058d-24d1-43d4-8df5-b976709b5233
@puemos puemos self-assigned this Sep 9, 2026
@trunk-io

trunk-io Bot commented Sep 9, 2026

Copy link
Copy Markdown

😎 Merged successfully - details.

@github-actions github-actions Bot added the feature/desktop Feature Tag: Desktop label Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

React Doctor found no issues in the changed files. 🎉

Reviewed by React Doctor for commit 1c8ecf1.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

Trunk lane — non-backend lane (fe:product:desktop)

This PR is assigned to the non-backend lane (fe:product:desktop). It does not run backend Python tests and may merge in parallel with PRs in other lanes.

@hosthog

hosthog Bot commented Sep 9, 2026

Copy link
Copy Markdown

HostHog preview — posthog-desktop-web

The previews for this PR have been torn down and no longer serve.

@puemos
puemos marked this pull request as ready for review September 9, 2026 06:59
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team September 9, 2026 07:00
The event fires as soon as the click lands, so a wait tied to the test budget only delays the report. A fixed bound placed after the setup measures delivery alone and fails fast with the last polled value.

Generated-By: PostHog Desktop
Task-Id: 97a9058d-24d1-43d4-8df5-b976709b5233
@posthog

posthog Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

🦔 PostHog Review reviewed this pull request

Nothing worth raising this time, so here's a calming picture instead:

A happy dog on a sunny path

@puemos puemos added the stamphog Request AI approval (no full review) label Sep 9, 2026
@puemos

puemos commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

/trunk merge

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

Test-only fix replacing a flaky fixed-timer wait with an event-record-and-poll pattern in an e2e spec; no production code touched, diff matches description, single owning-team author.

  • 1 of the 1 changed files are governed by products/desktop/AGENT_APPROVALS.md.
Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list no deny categories matched
size 0L, 0F substantive, 70L/1F incl. docs/generated/snapshots — within ceiling
tier T0 auto-approve: T0-deterministic (70L, 1F, single-area, fix)
stamphog 2.0.0b4 .stamphog/policy.yml @ 1c8ecf1 · reviewed head 1c8ecf1

@trunk-io
trunk-io Bot merged commit 8188c48 into master Sep 9, 2026
244 checks passed
@trunk-io
trunk-io Bot deleted the posthog/fix-desktop-subframe-e2e-race branch September 9, 2026 07:40
@deployment-status-posthog

deployment-status-posthog Bot commented Sep 9, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-09-09 07:57 UTC Run
prod-us ✅ Deployed 2026-09-09 08:16 UTC Run
prod-eu ✅ Deployed 2026-09-09 08:18 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature/desktop Feature Tag: Desktop stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants