Flaky `Can ignore suggestion` on Firefox: stale lint `context_hash` makes Dismiss a permanent no-op

[rodbegbie]

Disclaimer: I'm a Claude Code agent. This report was written by me at
the direction of @rodbegbie, who asked me to investigate why a docs-only PR
went red and then to file the findings — flagging this per the "Be Honest"
section of AGENT_POLICY.md. The code reading and CI archaeology below are
mine; I have flagged the parts I could not verify.

Summary

Can ignore suggestion (defined in testCanIgnoreSuggestion,
packages/chrome-plugin/tests/testUtils.ts) fails intermittently on the
firefox project. It is currently the most frequent cause of red
test-firefox-plugin runs on master, and it fails unrelated PRs — the run
that prompted this investigation was a documentation-only change.

I believe the test is catching a genuine race in LintFramework, not merely
being slow, and that a timeout increase will not fix it.

The failure

expect(locator('#harper-highlight')).toHaveCount(0) failed

Expected: 0
Received: 1
Timeout:  10000ms

Call log:
  - waiting for locator('#harper-highlight')
    24 × locator resolved to 1 element
      - unexpected value "1"

testUtils.ts:307. The highlight never disappears after Dismiss is clicked —
24 polls across the full 10s with the count pinned at 1. That is a stuck
state, not a slow one, which is why raising the timeout would not help.

Root cause (proposed)

Two behaviours combine.

1. requestLintUpdate drops lint requests instead of queueing them
(packages/lint-framework/src/lint/LintFramework.ts:148):

if ((!this.lintRequested && this.targets.size !== 0) || immediate) {
    if (!immediate) this.lintRequested = true;
    // ... await this.lintProvider(text) ...
    if (!immediate) this.lintRequested = false;

While a lint is in flight, every subsequent input event hits that guard and
is discarded. Nothing re-schedules it. The default delay is 0
(background/index.ts:747), so there is no debounce to coalesce them either.
The only recovery is the 1000 ms safety-net setTimeout in the constructor.

So when replaceEditorContent types This is a mistaek. on a loaded runner,
the first keystroke's lint request can swallow the remaining ~17, and the
rendered lint then belongs to a prefix of the final text.
remapLintToCurrentSource still positions the highlight correctly, so this is
invisible on screen.

2. The ignore hash is context-sensitive
(harper-core/src/ignored_lints/lint_context.rs:38):

let sequel_tokens = document.token_indices_intersecting(lint.span.with_len(2).pushed_by(2));

LintContext hashes the tokens following the lint span. The trailing . is
one of them. Ignoring a lint derived from the pre-period prefix therefore
records a context_hash that never matches the lint produced from the final
text. The forced re-lint at LintFramework.ts:372 brings the highlight
straight back, permanently — matching the observed signature exactly.

This also explains why Can ignore suggestion is the flaky one while its
siblings are not: it is the only helper whose assertion depends on
context_hash agreeing across the action, and the only one with no settle
wait between typing and acting (testCanBlockRuleSuggestion waits 1s,
testMultipleSuggestionsAndUndo waits 4–12s).

Worth stating plainly: if this reading is right, a real user typing quickly on
a loaded machine and immediately clicking Dismiss would hit the same thing.

This is not a recent regression

I initially suspected #3678 (removal of the unconditional 100 ms lint poll,
merged 2026-07-03T19:51Z). CI disproves that — the same assertion was already
failing before it landed.

Date	Run	Evidence
2025-09-05	c10b4c63 (#1868)	Commit titled "fix(chrome-ext): flaky tests" touches this test
2026-06-02	26847241856	slate-Can-ignore-suggestion--chromium, retries 1–2
2026-06-17	27724569857	draft/lexical/prosemirror --firefox, retry1–retry4 all exhausted
2026-07-01	28538152002	Same three specs, same toHaveCount failure
2026-07-02	28616878476	Same
2026-07-27	30285285268	github-Can-ignore-suggestion--firefox, 3 of 3 repeats
2026-07-28	30396872424, 30396871222	Same, 1 of 3 repeats each

Why it became visible now

Two recent changes, neither of which created the race:

#3783 (718ae49a, 2026-07-15) replaced retries: process.env.CI ? 4 : 0
with retries: 0, repeatEach: 3. Previously a flake that passed on attempt 2
never went red. Now the first failure is fatal and repeatEach: 3 gives it
three chances per run to bite.

#3854 (18cbb7e9, 2026-07-22), whose final commit is "test(chrome-ext):
stabilize ignore suggestion test":

-		const cacheSalt = randomString(5);
-		await replaceEditorContent(editor, cacheSalt);
+		const testText = 'This is a mistaek.';
+		await replaceEditorContent(editor, testText);

A 5-character random string became an 18-character sentence ending in a
period. That widens both halves of the race: ~18 keystrokes for the in-flight
mutex to swallow instead of ~5, and a trailing . that exists only in the
final text and lands squarely in LintContext's sequel tokens. The github
variant only starts failing from 07-27, five days after this landed.

I'd call that correlation suggestive rather than proven — draft was still
failing on 07-21 under the old text.

Suggested fixes

1. Product (root cause). In requestLintUpdate, mark dirty when a request
   is dropped and re-run once the in-flight one resolves, rather than relying
   on the 1000 ms timer.
2. Test. Replace the bare toHaveCount(1) at testUtils.ts:291 with a
   condition-based wait that the rendered lint's source matches the editor's
   current text before clicking. UnpackedLint already carries source, and
   LintFramework.getLastIgnorableLintBoxes() is public. A fixed
   waitForTimeout would only paper over it.

Unrelated diagnostics gap

playwright.config.ts currently pairs trace: 'on-first-retry' with
retries: 0, so traces are never captured. Every failure since #3783 has
produced screenshots only. trace: 'retain-on-failure' would have made this a
five-minute diagnosis. This is orthogonal to the retries/repeatEach values
that the accompanying comment is protecting.

Caveats

I have not reproduced this locally — the analysis is from reading the code
and correlating CI logs. Happy to open a PR for either or both fixes if the
direction looks right, or to gather more evidence first.

[rodbegbie]

Hello. Actual human Rod here! I’m going to have Claude work on this, and will open a couple of PRs for this shortly.

1. A PR that just contains the suggested product fix (marking lints as dirty when they are dropped).
2. A second (stacked) PR that follows the suggestions to improve the test.

Y’all are then welcome to review and merge them separately or together.

[rodbegbie]

Same Claude Code agent as the report above, still working under
@rodbegbie's direction. Correcting one of my own claims.

Correction to the "diagnostics gap" section

I wrote that pairing trace: 'retain-on-failure' with the current retries: 0
"would have made this a five-minute diagnosis". That is wrong as stated, and I
should have checked before asserting it.

There is no upload-artifact step in just_checks.yml. The only uploads in
the repository are extension zips and release binaries in chrome_plugin.yml,
wp_plugin.yml and binaries.yml; nothing references test-results or
playwright-report. Enabling traces on CI would write them to the runner's disk
and then discard them with the runner. It would cost runtime and deliver nothing
retrievable.

The part that still stands, and is worse than I described

screenshot: 'only-on-failure' is already enabled, so those screenshots are
being discarded too. Every test-results/…/test-failed-1.png path in the
failure logs of the runs listed above points at a file nobody can download. That
is why the analysis in the original report is derived from raw log text rather
than from looking at what the page actually showed.

So the useful change is not the trace setting — it is an upload step:

- name: Upload Playwright results
  if: failure()
  uses: actions/upload-artifact@v7
  with:
    name: playwright-results-${{ matrix.task }}
    path: packages/chrome-plugin/test-results/
    retention-days: 7

Zero runtime cost, fires only on failure, and immediately makes the screenshots
that are already being captured retrievable.

Why I would now leave trace alone

retain-on-failure records a trace for every test and deletes it on pass — DOM
snapshots, screenshots and network capture across 174 tests × repeatEach: 3, on
two plugin suites that already run 27–37 minutes each, on every PR. That cost is
paid by every passing test to benefit the occasional failing one.

It is also unnecessary for local debugging, where the config setting is beside
the point: Playwright takes --trace on the command line, so anyone
investigating this locally can already get a full trace without touching the
committed config.

cd packages/chrome-plugin
pnpm exec playwright test --project firefox \
  --trace retain-on-failure -g "Can ignore suggestion"
pnpm exec playwright show-trace test-results/…/trace.zip

My recommendation is therefore narrower than what I originally filed: add the
artifact upload, and change trace only if someone later wants trace-level
detail on CI specifically.

Apologies for the noise — better to correct it than leave a wrong suggestion
sitting in the issue.

[rodbegbie]

Hello. Human Rod here again! The PRs are open. Both are drafts, since I’m not a repo contributor and already have a PR open. I split this across two PRs since I don’t know your collective bandwidth to review things. 3912 is the faster thing to grok, review and land, 3913 is a bit more involved but more valuable.

• fix(lint-framework): coalesce lint requests dropped while one is in flight #3912 is the smallest possible change to fix the lint-framework code. Looks bigger than it actually is since there’s a bunch of whitespace changes due to wrapping a try/finally block around the code. There are no changes to the tests on this branch, and Claude is rightly being persnickety that the fact that the tests being green on this PR don’t mean it’s fixed, but could still be a flake!
• test(lint-framework): prove lint request coalescing and settle the ignore-suggestion e2e test #3913 is stacked on top of the previous one. This PR also fixes the e2e test, and adds vitest to the lint-framework project, to prove that the fix actually does what it’s supposed to. This was implemented using the superpowers skills using Red -> Green TDD, so validated that the tests failed before the fix and pass now.