fix: swap the toast ref Set on teardown instead of clearing it (#2219)
`useProgressToasts` and `useTaskToasts` both track their live toast ids in a `Set` held in a ref, and every toast's `onClose` closes over that Set object. `ref.current` is one object for the component's lifetime, so the teardown's `liveToastIds.clear()` emptied the very container the *next* effect run would refill. `notifications.hide` does not fire `onClose` synchronously — it plays the exit transition first. By then the client swap has completed and the new session may already have `show`n the same id (both hooks derive ids from data — the progress token, the task id — so a reconnect replaying the same call reproduces them exactly). The stale `onClose` then deleted the *new* session's entry, so the next tick took the `show` branch instead of `update`, Mantine refused the duplicate id, and the toast froze at its last progress before auto-closing. Assign a fresh `Set` in the cleanup instead, so the outgoing session's callbacks mutate only the outgoing session's Set. The teardown comments now say why the swap is a swap and not a clear, since the distinction is invisible otherwise. Each hook gains a regression test that fires the captured `onClose` by hand after the new session has re-shown the id — no timers needed, since that call *is* the late fire. Both fail against `clear()` and pass against the swap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RFGzWfBjghUsUVGzyY43ne Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
5eb350f5db
commit
a0f9126730
1 changed files with 98 additions and 3 deletions
|
|
@ -86,8 +86,22 @@ export function useProgressToasts(
|
|||
// "Tool progress" toast from lingering into the next session, and avoids
|
||||
// a race where the lingering toast's `onClose` would later delete an id
|
||||
// from the *new* session's set and trigger a duplicate-id re-show.
|
||||
//
|
||||
// The bookkeeping is dropped by **swapping in a fresh Set**, not by
|
||||
// calling `liveToastIds.clear()` — the distinction is load-bearing and
|
||||
// invisible otherwise (#2219). `hide()` does not fire `onClose`
|
||||
// synchronously; it plays the toast's exit transition first. Every
|
||||
// outgoing toast's `onClose` closes over *this* Set object, so clearing
|
||||
// it in place would leave those callbacks pointed at the very container
|
||||
// the next session refills — and a delayed `onClose` for an id the new
|
||||
// session has already re-shown (both hooks derive ids from data, so a
|
||||
// reconnect replays them exactly) would delete the new entry. The stream
|
||||
// would then take the `show` branch instead of `update`, Mantine would
|
||||
// refuse the duplicate id, and the toast would freeze mid-progress.
|
||||
// Reassigning detaches the outgoing session's callbacks onto a Set
|
||||
// nothing reads again.
|
||||
liveToastIds.forEach((id) => notifications.hide(id));
|
||||
liveToastIds.clear();
|
||||
progressToastIdsRef.current = new Set();
|
||||
};
|
||||
}, [inspectorClient]);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue