Toast teardown clears the shared ref Set, so a delayed onClose deletes the next session's entry

[cliffhall]

Found in the v2.5.0 milestone-merge review (#2215), in code that shipped on v2/main during the milestone. Filed here rather than fixed in the merge PR, whose tree is byte-identical to origin/v2/main.

The bug

useProgressToasts.ts and useTaskToasts.ts both track which toast ids are currently on screen in a Set held in a ref:

const liveToastIds = progressToastIdsRef.current;   // and taskToastIdsRef.current
…
notifications.show({ id, …, onClose: () => liveToastIds.delete(id) });
…
return () => {
  liveToastIds.forEach((id) => notifications.hide(id));
  liveToastIds.clear();
};

ref.current is one Set object for the lifetime of the component, and every onClose closes over that object. So clear() empties the container the next effect run will use, while the outgoing toasts still hold a reference to it.

notifications.hide does not fire onClose synchronously — it plays the toast's exit transition first. By the time those callbacks run, the client swap has completed and the new session may already have called show for the same id. The stale onClose then deletes the new session's entry.

The consequence is the one the existing comment in useProgressToasts was written to prevent: the set no longer knows the toast is live, so the next tick takes the show branch instead of update. Mantine will not re-show a duplicate id, so the toast simply stops advancing — it freezes at whatever progress it last displayed and then auto-closes, while the stream is still running.

Both ids are derived from data rather than from the session — progressToastId(token) from the progress token, the task id for tasks — so a reconnect to the same server replaying the same call reproduces the id exactly. This is the likely case, not a contrived one.

The intent was already correct; the mechanism is what falls short. useProgressToasts's teardown comment names this exact race and reaches for hide() + clear() to close it. hide() handles the visual half (no stale toast lingering into the next session); clear() does not handle the bookkeeping half, because it mutates the shared object rather than detaching from it.

Fix

Assign a fresh Set to the ref during cleanup instead of clearing the existing one, so callbacks from the outgoing session mutate only the outgoing session's Set:

liveToastIds.forEach((id) => notifications.hide(id));
progressToastIdsRef.current = new Set();   // not liveToastIds.clear()

Same change in useTaskToasts, which mirrors the pattern deliberately (its comment says so). Update the teardown comments to say why the swap is a swap and not a clear — the distinction is invisible otherwise, and a later reader will "simplify" it back.

A test can drive it without real timers: show a progress toast, run the effect cleanup, then invoke the captured onClose after re-running the effect and adding the same id, and assert the new session's set still holds it.

Reported by Copilot on #2215.