Retire the notification presentation wait when its service closes - #379
Conversation
A closed NotificationsService left its two-frame presentation wait and 100 ms cap timer running. When a jsdom test file ended first, the timer fired against a torn-down window (host.cancelAnimationFrame is not a function) and Vitest failed the run with an unhandled error. The wait now takes the service's lifetime signal and retires its frame and timer on close. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
No blocking findings. The lifetime signal cancels the current frame and timer; normal completion removes the abort listener. Existing disposal and permission-generation fences still prevent stale delivery.
Reviewed head d6a46e8e4048d01cfdd9a93cc885586bb45d178b against current base 112d2ca791c75b4b1cff47b810900eefa70f25d9 (diff merge-base 5ce7836b197fc4f19b9bd4c23d3f8cd56495df3b). Independent test review completed. A deterministic direct-import probe passed pre-abort, abort before/between frames, normal two-frame completion, timeout, late abort, and no-window fallback. Existing hosted CI is green at this head; broad suites were not duplicated locally. No native app launch or human acceptance is claimed.
Optional coverage: commit a between-frames abort case and assert non-delivery for an admitted candidate on service disposal. The current new tests abort before frame 1 and the service test uses an empty wait; source tracing and the probe support the implementation, so this is not a blocker.
Comment only; no approval submitted.
🤖 This PR was written by Larry, an AI agent.
Why
The Vitest JavaScript job can fail with every test passing:
Example: run 36490804594, attempt 2 (415/415 files and 4988/4988 tests passed; exit code 1).
NotificationsServicewaits for two animation frames before it shows an alert, with a 100 ms timer as a cap (afterPresentation). Closing the service did not stop that wait. When a jsdom test file finished before the timer fired, the window was already torn down, and the timer callback threw. The same leak exists in the app: a closed service keeps a frame and a timer alive until they fire.What changed
afterPresentationtakes an optionalAbortSignal. Aborting cancels its frame and timer at once. The wait does not resolve, so no delivery runs after close.NotificationsServiceowns one lifetimeAbortControllerand aborts it when it closes.Evidence
service-lifetime.test.ts(jsdom): selecting a viewer schedules a presentation wait; closing the service must leave no frames and no timers. It fails before the fix (1 frame left) and passes after.presentation.test.ts: an aborted wait cancels its frame and timer and never resolves. It fails before the fix and passes after.src/features/notifications/andsrc/app/NotificationSettings.test.tsx: 115/115 pass on Node 24 (Hermit). Biome andtscare clean.