Conversation
✅ Deploy Preview for hyprnote canceled.
|
✅ Deploy Preview for hyprnote-storybook canceled.
|
| addDeletion: (data, onConfirm) => { | ||
| const sessionId = data.session.id; | ||
|
|
||
| const timeoutId = setTimeout(() => { | ||
| get().confirmDeletion(sessionId); | ||
| }, UNDO_TIMEOUT_MS); | ||
|
|
||
| set((state) => ({ | ||
| pendingDeletions: { | ||
| ...state.pendingDeletions, | ||
| [sessionId]: { | ||
| data, | ||
| timeoutId, | ||
| isPaused: false, | ||
| remainingTime: UNDO_TIMEOUT_MS, | ||
| onDeleteConfirm: onConfirm ?? null, | ||
| addedAt: Date.now(), | ||
| }, | ||
| }, | ||
| })); |
There was a problem hiding this comment.
🔴 addDeletion does not clear existing timeout when re-adding the same sessionId
When addDeletion is called for a sessionId that already has a pending deletion, the old timeout is not cleared before creating a new one. This leaks the old setTimeout and causes premature confirmation of the replacement deletion.
Root Cause and Impact
In the old code, setTimeoutId explicitly cleared any existing timeout (apps/desktop/src/store/zustand/undo-delete.ts old lines 90-95):
setTimeoutId: (id) => {
const currentId = get().timeoutId;
if (currentId) { clearTimeout(currentId); }
set({ timeoutId: id });
}The new addDeletion at apps/desktop/src/store/zustand/undo-delete.ts:81-100 creates a new setTimeout and overwrites the entry in pendingDeletions without clearing the previous timeout:
addDeletion: (data, onConfirm) => {
const sessionId = data.session.id;
const timeoutId = setTimeout(() => {
get().confirmDeletion(sessionId);
}, UNDO_TIMEOUT_MS);
set((state) => ({ pendingDeletions: { ...state.pendingDeletions, [sessionId]: { ... } } }));
}If a session is deleted, undone via the restore button (which calls clearDeletion), and then deleted again, this is safe because clearDeletion clears the timeout. However, if addDeletion is called twice for the same session without an intervening clearDeletion (e.g., the user triggers delete from the context menu while the dissolving animation is still active), the old timeout fires first and calls confirmDeletion(sessionId). This executes the NEW onDeleteConfirm callback prematurely (before the full 5-second undo window elapses for the second deletion), and then clears the pending deletion entirely. The second timeout then fires on an already-cleared entry and is a no-op.
Impact: The undo window for the replacement deletion is shortened to whatever time remained on the original timeout, potentially confirming a destructive delete before the user has the full 5 seconds to undo.
| addDeletion: (data, onConfirm) => { | |
| const sessionId = data.session.id; | |
| const timeoutId = setTimeout(() => { | |
| get().confirmDeletion(sessionId); | |
| }, UNDO_TIMEOUT_MS); | |
| set((state) => ({ | |
| pendingDeletions: { | |
| ...state.pendingDeletions, | |
| [sessionId]: { | |
| data, | |
| timeoutId, | |
| isPaused: false, | |
| remainingTime: UNDO_TIMEOUT_MS, | |
| onDeleteConfirm: onConfirm ?? null, | |
| addedAt: Date.now(), | |
| }, | |
| }, | |
| })); | |
| addDeletion: (data, onConfirm) => { | |
| const sessionId = data.session.id; | |
| const existing = get().pendingDeletions[sessionId]; | |
| if (existing?.timeoutId) { | |
| clearTimeout(existing.timeoutId); | |
| } | |
| const timeoutId = setTimeout(() => { | |
| get().confirmDeletion(sessionId); | |
| }, UNDO_TIMEOUT_MS); | |
| set((state) => ({ | |
| pendingDeletions: { | |
| ...state.pendingDeletions, | |
| [sessionId]: { | |
| data, | |
| timeoutId, | |
| isPaused: false, | |
| remainingTime: UNDO_TIMEOUT_MS, | |
| onDeleteConfirm: onConfirm ?? null, | |
| addedAt: Date.now(), | |
| }, | |
| }, | |
| })); | |
| }, |
Was this helpful? React with 👍 or 👎 to provide feedback.
Changes