Fix live toast stale callback race

[jjoanna2-debug]

Summary

• bind live-mode toast rAF/timeout callbacks to the toast element that scheduled them
• prevent dismissToast() or a newer toast from leaving stale callbacks that touch toastEl
• add a regression guard for the [Bug] Live mode: unguarded requestAnimationFrame in showToast() throws "Cannot read properties of null (reading 'style')" #268 timing race and stale timer ownership

Why

Fixes #268.

showToast() schedules an enter requestAnimationFrame, but dismissToast() can remove the current toast and set toastEl = null before that frame runs. The old callback then read toastEl.style and could throw. A plain null guard would close that crash, but it still leaves older timeout callbacks able to animate or remove a newer toast.

This patch captures the created element as currentToast and makes each delayed callback no-op unless toastEl === currentToast.

Verification

• node --test tests/live-browser-regression.test.mjs
• node --test tests/live-browser-regression.test.mjs tests/live-browser-source.test.mjs tests/live-browser-dom.test.mjs tests/live-browser-script-parts.test.mjs tests/live-browser-session.test.mjs
• git diff --check
• npx --yes bun run test:live still fails on tests/live-copy-edit-agent.test.mjs (flags invalid JSX syntax in post-apply checks); confirmed the same failure on clean origin/main.
• npx --yes bun run test:live-e2e is blocked in this checkout because @anthropic-ai/sdk is not installed.

---

Note

Low Risk
Scoped UI toast lifecycle fix with regression tests; no auth, data, or API surface changes.

Overview
Fixes a timing race in live-mode showToast() where enter requestAnimationFrame and auto-dismiss setTimeout callbacks could run after dismissToast() or a newer toast replaced the global toastEl, causing crashes or animating/removing the wrong toast.

Each toast now binds delayed work to a local currentToast; enter, fade-out, and removal callbacks no-op unless toastEl === currentToast. Regression tests in live-browser-regression.test.mjs lock in that ownership behavior.

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

[greptile-apps]

Greptile Summary

Fixes a timing race in showToast() where a requestAnimationFrame or setTimeout callback scheduled by one toast could fire after dismissToast() (or a subsequent showToast()) had already nulled out toastEl, causing a crash or animating/removing the wrong toast.

• skill/scripts/live-browser.js: showToast now captures the newly-created element in a const currentToast before assigning to toastEl. Every deferred callback (enter rAF, auto-dismiss timer, and the removal sub-timer) guards with if (toastEl !== currentToast) return;, so stale callbacks from a superseded toast are always no-ops.
• tests/live-browser-regression.test.mjs: Adds a source-level regex test that locks in the currentToast capture pattern and each of the three callback guards, and asserts the old unsafe toastEl.style read is gone.

Confidence Score: 5/5

The change is scoped entirely to the toast UI lifecycle; the logic is correct and the new regression test locks in the fix.

The captured-closure pattern is the standard solution for this class of race, the three callback guards are consistent and correct, and dismissToast (which sets toastEl = null) correctly makes all pending callbacks from prior toasts no-ops. The regression test covers all four guard sites. No data, auth, or API surface is touched.

No files require special attention.

Important Files Changed

Filename	Overview
skill/scripts/live-browser.js	Fixes stale-closure race in showToast by capturing each new element as a local currentToast and guarding every delayed callback with an identity check against the module-level toastEl; no other logic changes.
tests/live-browser-regression.test.mjs	Adds a regression test that asserts the source-level pattern for the fix — local currentToast capture, rAF guard, outer-timeout guard, inner-removal-timeout guard, and absence of the old raw toastEl.style read.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant showToast
    participant dismissToast
    participant rAF as requestAnimationFrame
    participant T1 as setTimeout duration
    participant T2 as setTimeout 250ms

    Caller->>showToast: showToast A 3000
    showToast->>dismissToast: dismissToast noop toastEl null
    showToast->>showToast: currentToast equals new el, toastEl equals currentToast
    showToast->>rAF: schedule enter animation
    showToast->>T1: schedule fade-out after 3000ms

    Note over Caller,T2: Race scenario dismissToast fires before rAF

    Caller->>dismissToast: dismissToast called
    dismissToast->>dismissToast: toastEl.remove then toastEl null

    rAF-->>showToast: fires toastEl null not equal currentToast A return early
    T1-->>showToast: fires toastEl null not equal currentToast A return early

    Note over Caller,T2: Normal path no race

    Caller->>showToast: showToast B 2000
    showToast->>showToast: currentToast equals new el, toastEl equals currentToast
    showToast->>rAF: schedule enter animation
    showToast->>T1: schedule fade-out after 2000ms
    rAF-->>showToast: fires toastEl equals currentToast animate in
    T1-->>showToast: fires toastEl equals currentToast fade out
    T1->>T2: schedule removal after 250ms
    T2-->>showToast: fires toastEl equals currentToast remove and toastEl null

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant showToast
    participant dismissToast
    participant rAF as requestAnimationFrame
    participant T1 as setTimeout duration
    participant T2 as setTimeout 250ms

    Caller->>showToast: showToast A 3000
    showToast->>dismissToast: dismissToast noop toastEl null
    showToast->>showToast: currentToast equals new el, toastEl equals currentToast
    showToast->>rAF: schedule enter animation
    showToast->>T1: schedule fade-out after 3000ms

    Note over Caller,T2: Race scenario dismissToast fires before rAF

    Caller->>dismissToast: dismissToast called
    dismissToast->>dismissToast: toastEl.remove then toastEl null

    rAF-->>showToast: fires toastEl null not equal currentToast A return early
    T1-->>showToast: fires toastEl null not equal currentToast A return early

    Note over Caller,T2: Normal path no race

    Caller->>showToast: showToast B 2000
    showToast->>showToast: currentToast equals new el, toastEl equals currentToast
    showToast->>rAF: schedule enter animation
    showToast->>T1: schedule fade-out after 2000ms
    rAF-->>showToast: fires toastEl equals currentToast animate in
    T1-->>showToast: fires toastEl equals currentToast fade out
    T1->>T2: schedule removal after 250ms
    T2-->>showToast: fires toastEl equals currentToast remove and toastEl null

Loading

_{Reviews (2): Last reviewed commit: "Fix live toast stale callback race" | Re-trigger Greptile}

[abdulwahabone]

I am not able to repro this bug, can you share a minimal repo for us to test and repro it.

[jjoanna2-debug]

Sure. I added a minimal dependency-free repro repo here:

https://github.com/jjoanna2-debug/impeccable-toast-race-repro

Steps:

git clone https://github.com/jjoanna2-debug/impeccable-toast-race-repro.git
cd impeccable-toast-race-repro
python3 -m http.server 4173

Then open http://localhost:4173 and click Crash old implementation.

What it does:

1. Calls showToastOld().
2. Immediately calls dismissToastOld() before the next animation frame.
3. The pending requestAnimationFrame callback then runs after oldToastEl has been set to null.
4. The old callback reads oldToastEl.style and throws Cannot read properties of null (reading 'style').

The same page has Run fixed implementation, which uses the currentToast identity guard from this PR. That path no-ops the stale callback instead of touching a nulled or superseded toast.

I kept it static and framework-free so the repro isolates the toast lifecycle race itself rather than depending on a particular React/Vite page shape.

[jjoanna2-debug]

Quick status refresh: this is still current and green.

I re-ran the focused regression guard locally on Node v26.4.0:

node --test tests/live-browser-regression.test.mjs

Result: 28 pass, 0 fail.

The minimal repro is still here:
https://github.com/jjoanna2-debug/impeccable-toast-race-repro

It isolates the failing sequence: showToastOld() schedules the enter requestAnimationFrame, then dismissToastOld() clears toastEl before that callback runs. The PR fix captures currentToast and makes stale callbacks no-op unless the global toast reference still points at the same element.

Current PR checks are green, including changes, test, live-e2e smoke jobs, and Cursor Bugbot. Scope is limited to the live toast lifecycle; no API, auth, data, or persistence surface is touched.

[pbakaus]

Seems sensible - thank you for your contribution!