FE-1589: Skip the flaky browser-plugin inference Playwright tests

[claude]

Requested by Ciaran Morinan · Slack thread

🌟 What is the purpose of this PR?

Before: the extension test user can configure a one-off inference, and the settings are persisted failed often on CI. It accounted for most of the Test workflow's failures over 3 and 4 September. The suite runs with no retries by design, so one flake fails the whole run and ejects unrelated PRs from the merge queue. The failures point at the tests' own waits rather than at the extension, so there is no sign the plugin is broken for users.

After: that test and its sibling user can enable automatic inference, and the settings are persisted are skipped, so they can no longer block unrelated work. The other four tests in the file still run, including the logged-out and logged-in popup checks and the options page checks, so the extension keeps its coverage everywhere else. The one-off and automatic inference paths are uncovered until the tests are fixed.

This is a temporary quarantine, not a fix. FE-1589 tracks restoring both.

🔗 Related links

• FE-1589: Restore the quarantined browser-plugin inference Playwright tests (internal)
• Run 33853704936, failing job — the failure that prompted this PR
• Run 33858095517, failing job — the sibling test failing on this branch, which is why the quarantine was widened

🚫 Blocked by

• Nothing

🔍 What does this change?

Two tokens: test( becomes test.skip( on the two tests with observed failures.

No comment is added at either skip. House style keeps comments timeless and free of ticket references, and the file's existing test.skip carries none either. The tracking lives in FE-1589.

Which tests are skipped

Two:

• browser-plugin.spec.ts:256 — user can configure a one-off inference, and the settings are persisted
• browser-plugin.spec.ts:291 — user can enable automatic inference, and the settings are persisted

The first was the only one with observed failures when this PR was opened. I checked five runs individually and all five reported it:

Run	Reported line
33853704936	:281
33826462736	:160
33818050502	:160
33811421092	:32
33809636978	:32
33790576601	:281

The varying line numbers do not implicate other tests. Lines 32, 70 and 160 sit in helpers (waitForPopupStateLoaded, openPopupTab, signOutAndReloadPopup) that this test calls, so they appear in its own stack.

The sibling automatic-inference test was left running on that evidence. It then failed on this branch, in the run that quarantining the first test made possible:

1) [extension-chromium] › tests/extension/browser-plugin.spec.ts:291:1 › user can enable automatic inference, and the settings are persisted
   Error: page.evaluate: Target page, context or browser has been closed
     at waitForPopupStateLoaded (tests/extension/browser-plugin.spec.ts:32:8)
     at openPopupTab (tests/extension/browser-plugin.spec.ts:71:9)
     at tests/extension/browser-plugin.spec.ts:320:3
   Test timeout of 30000ms exceeded.

That is the same helper and the same end state as the one-off test's failures. It shares openPopupTab and waitForPopupStateLoaded, so the quarantine covers it too.

Diagnosis

The cause is not established. Two findings about the spec's waits are the place to start, and both are hypotheses:

1. waitForPopupStateLoaded treats "chrome.storage.local idle for 500 ms" as the signal that the popup has loaded. The popup polls flow runs every 2 seconds and writes each result to storage, so an idle window says nothing about readiness. When writes bunch up the wait never resolves and spends the whole 30 s budget, which matches the Target page, context or browser has been closed end state in both tests. When they do not, it resolves before the popup's user fetch has landed.

2. getUser overwrites the locally stored inference settings with the database copy every time it runs, and the popup renders as soon as a user is present in storage, which can precede its own getUser returning. A getUser landing after the test has picked its entity types would revert the settings, and the debounced save would then persist the reverted value. That would keep the type picked first and drop the one picked second — the shape of the Actor present / Document missing failure.

Both tests reaching the timeout inside waitForPopupStateLoaded points at the first hypothesis over the second.

Also worth changing when the tests are fixed: the chips are matched by a MUI class plus a text regex (.MuiChip-label filtered on /^Document$/), which is fragile and gives a poor failure message. There is no data-testid on them today; adding one would be better than the class-plus-regex match.

Pre-Merge Checklist 🚀

🚢 Has this modified a publishable library?

This PR:

• does not modify any publishable blocks or libraries, or modifications do not need publishing

📜 Does this require a change to the docs?

The changes in this PR:

• are internal and do not require a docs change

🕸️ Does this require a change to the Turbo Graph?

The changes in this PR:

• do not affect the execution graph

⚠️ Known issues

The one-off and automatic inference flows have no automated coverage while the skips are in place. That is the cost of the quarantine and the reason FE-1589 exists.

🐾 Next steps

FE-1589: establish the cause, replace the storage-idle waits with waits on the specific requests that populate the popup, and remove both skips. Fixed sleeps, Playwright retries and a longer test timeout are all ruled out.

🛡 What tests cover this?

None — this removes two tests from the run. The remaining four tests in browser-plugin.spec.ts are unchanged.

❓ How to test this?

1. Check out the branch.
2. Run yarn workspace @tests/hash-playwright test:integration.
3. Confirm both inference tests report as skipped and the rest of browser-plugin.spec.ts passes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LreriWwCC66TPmMjQVNATJ

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

4 Skipped Deployments

Project	Deployment	Actions	Updated
hash	Ignored	Preview	Sep 4, 2026 10:01am UTC
hashdotdesign-tokens	Ignored	Preview	Sep 4, 2026 10:01am UTC
petrinaut	Skipped		Sep 4, 2026 10:01am UTC
petrinaut-docs	Skipped		Sep 4, 2026 10:01am UTC

[cursor]

PR Summary

Low Risk
Test-only change that reduces extension E2E coverage without modifying product code.

Overview
Quarantines two flaky Playwright specs in browser-plugin.spec.ts by changing test( to test.skip( for the one-off and automatic inference persistence flows.

Those scenarios still exercise popup tabs, entity-type selection, debounced updateEntity saves, and sign-out/reload — the same helpers and storage-sync timing that were failing CI. Skipping them stops the no-retry integration workflow from failing on unstable waits; the other popup/options specs in the file keep running. One-off inference and automatic inference settings persistence are no longer covered until the tests are fixed (tracked as FE-1589 for the one-off case).

^{Reviewed by Cursor Bugbot for commit bbefb78. Bugbot is set up for automated code reviews on this repo. Configure here.}

[Copilot]

🟢 Approval recommended

The focused change correctly implements the documented temporary quarantine.

Pull request overview

Temporarily quarantines a flaky browser-plugin Playwright test to prevent unrelated merge-queue failures.

Changes:

• Marks the one-off inference persistence test as skipped.
• Leaves the remaining extension tests enabled.

File summaries

File	Description
tests/hash-playwright/tests/extension/browser-plugin.spec.ts	Skips the flaky one-off inference test.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 0
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit bbefb78. Configure here.}