From 0ebfc989cb812c35ce29d1e5f5ab506bbf2b5fbf Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 4 Jul 2026 00:25:21 -0700 Subject: [PATCH] Fix flaky terminal-rendering-golden repo-load race on macOS CI (#7330) The release-blocking `terminal rendering golden mac` job was failing ~40% of Cut Release runs with `Expected e2e repo to be loaded`, leaving the RC stuck as a draft (publish-release depends on this job). Root cause: the sharedPage fixture did a single-shot fetchRepos() + find() + throw. window.api.repos.add() fires a repos:changed echo that triggers a concurrent fetchRepos() in the renderer; the store's reposFetchGeneration guard then drops the fixture's own awaited fetch result, leaving `repos` briefly stale, so find() returns undefined and throws. The repo lands a few ms later (the failure screenshot's sidebar actually shows it). Wrap the repo load in expect.poll (matching the seeded-worktree poll right below it) so it retries fetchRepos until the repo lands, then runs the idempotent updateRepo. Also harden the single-shot hasWebgl/cursorHidden diagnostics reads in the golden spec: WebGL reattaches asynchronously after a worktree switch, so poll those eventually-consistent fields until they settle before the golden asserts. Regression detection is preserved: a real WebGL/cursor regression times out the poll and still fails the test; the geometry/wrap/overpaint golden checks stay single-shot. --- tests/e2e/helpers/orca-app.ts | 49 ++++++++++++------- ...nal-raw-emoji-table-scroll-restore.spec.ts | 46 +++++++++++++++-- 2 files changed, 74 insertions(+), 21 deletions(-) diff --git a/tests/e2e/helpers/orca-app.ts b/tests/e2e/helpers/orca-app.ts index ecfa238d9..64dfe9d21 100644 --- a/tests/e2e/helpers/orca-app.ts +++ b/tests/e2e/helpers/orca-app.ts @@ -314,23 +314,38 @@ export const test = base.extend({ await window.api.repos.add({ path: repoPath }) }, repoPath) - // Fetch repos in the renderer store so it picks up the new repo - await page.evaluate(async (repoPath) => { - const store = window.__store - if (!store) { - return - } - - await store.getState().fetchRepos() - const repo = store.getState().repos.find((candidate) => candidate.path === repoPath) - if (!repo) { - throw new Error(`Expected e2e repo to be loaded: ${repoPath}`) - } - // Why: the fixture deliberately creates external Git worktrees. New - // repos hide those by default after the visibility rollout, so opt this - // disposable repo into showing them before specs assert on worktree state. - await store.getState().updateRepo(repo.id, { externalWorktreeVisibility: 'show' }) - }, repoPath) + // Fetch repos in the renderer store so it picks up the new repo, then opt + // this disposable repo into showing external worktrees. + // Why: repos.add() fires a repos:changed echo that triggers a *concurrent* + // fetchRepos() in the renderer; the store's generation guard can then drop + // this awaited fetch's result, leaving `repos` briefly stale. Poll the + // public fetch path until the repo lands instead of asserting on the first + // tick (mirrors the seeded-worktree poll below). updateRepo is idempotent, + // so running it once the repo appears is safe across poll ticks. + await playwrightExpect + .poll( + () => + page.evaluate(async (repoPath) => { + const store = window.__store + if (!store) { + return false + } + await store.getState().fetchRepos() + const repo = store.getState().repos.find((candidate) => candidate.path === repoPath) + if (!repo) { + return false + } + // Why: the fixture deliberately creates external Git worktrees. New + // repos hide those by default after the visibility rollout. + await store.getState().updateRepo(repo.id, { externalWorktreeVisibility: 'show' }) + return true + }, repoPath), + { + timeout: 30_000, + message: `Expected e2e repo to be loaded: ${repoPath}` + } + ) + .toBe(true) // Wait for the repo to appear and fetch its worktrees await page.evaluate(async () => { diff --git a/tests/e2e/terminal-raw-emoji-table-scroll-restore.spec.ts b/tests/e2e/terminal-raw-emoji-table-scroll-restore.spec.ts index 043fbe610..238bd4f9d 100644 --- a/tests/e2e/terminal-raw-emoji-table-scroll-restore.spec.ts +++ b/tests/e2e/terminal-raw-emoji-table-scroll-restore.spec.ts @@ -486,9 +486,28 @@ test.describe('Terminal raw emoji table scroll restore repro', () => { await sendToTerminal(orcaPage, ptyId, `printf ${JSON.stringify(`${marker}\\n`)}\r`) await waitForTerminalOutput(orcaPage, marker, 10_000) - const diagnostics = await readTerminalRenderDiagnostics(orcaPage) + const expectedWebgl = await expectAutoWebgl(orcaPage) + // Why: WebGL (re)attaches asynchronously via React visibility effects and a + // transient ESC[?25l during a redraw can momentarily set cursorHidden. Let + // those eventually-consistent fields settle before the single-shot golden + // asserts so runner timing can't flake-block the release. hasComplexScriptOutput + // stays single-shot: its not-ready default is also false, so timing can't + // turn it into a false failure. + let diagnostics = await readTerminalRenderDiagnostics(orcaPage) + await expect + .poll( + async () => { + diagnostics = await readTerminalRenderDiagnostics(orcaPage) + return diagnostics.hasWebgl === expectedWebgl && diagnostics.cursorHidden === false + }, + { + timeout: 15_000, + message: `terminal render diagnostics did not settle (expected hasWebgl=${expectedWebgl}, cursorHidden=false)` + } + ) + .toBe(true) expect(diagnostics.hasComplexScriptOutput).toBe(false) - expect(diagnostics.hasWebgl).toBe(await expectAutoWebgl(orcaPage)) + expect(diagnostics.hasWebgl).toBe(expectedWebgl) expect(diagnostics.cursorHidden).toBe(false) }) @@ -553,7 +572,26 @@ test.describe('Terminal raw emoji table scroll restore repro', () => { await scrollActiveTerminalToText(orcaPage, 'Singer') await closeFeatureTips(orcaPage) - const diagnostics = await readTerminalRenderDiagnostics(orcaPage) + const expectedWebgl = await expectAutoWebgl(orcaPage) + // Why: after the worktree switch, WebGL reattaches asynchronously (React + // visibility effect + attach backoff) and a transient ESC[?25l during the + // restore redraw can momentarily set cursorHidden. Let those settle before + // the single-shot golden asserts so runner timing can't flake-block the + // release; the geometry/wrap/overpaint checks below stay single-shot as the + // real regression signal. + let diagnostics = await readTerminalRenderDiagnostics(orcaPage) + await expect + .poll( + async () => { + diagnostics = await readTerminalRenderDiagnostics(orcaPage) + return diagnostics.hasWebgl === expectedWebgl && diagnostics.cursorHidden === false + }, + { + timeout: 15_000, + message: `terminal render diagnostics did not settle (expected hasWebgl=${expectedWebgl}, cursorHidden=false)` + } + ) + .toBe(true) const overpaint = await readTerminalRightEdgeOverpaint(orcaPage) const wrapDiagnostics = await readTerminalBoxTableWrapDiagnostics(orcaPage) const singerGeometry = await readVisibleSingerRowGeometry(orcaPage) @@ -579,7 +617,7 @@ test.describe('Terminal raw emoji table scroll restore repro', () => { expect(wrapDiagnostics.cols).toBeGreaterThanOrEqual(RAW_EMOJI_BOX_TABLE_WIDTH) expect(diagnostics.hasComplexScriptOutput).toBe(false) - expect(diagnostics.hasWebgl).toBe(await expectAutoWebgl(orcaPage)) + expect(diagnostics.hasWebgl).toBe(expectedWebgl) expect(diagnostics.cursorHidden).toBe(false) expect(overpaint.offenders).toEqual([]) expect(wrapDiagnostics.wrappedBoxLines).toEqual([])