fix(desktop): stop pin/unpin sidebar races against lagging session pages - #75705
fix(desktop): stop pin/unpin sidebar races against lagging session pages#75705drewTuzson wants to merge 1 commit into
Conversation
Keep local pin intent sticky until the backend confirms it. Push both pin and unpin writes before pull-adoption so a stale sessions page cannot strip a brand-new pin or resurrect a just-unpinned chat in the same tick.
alt-glitch
added
type/bug
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing the real stale-page race: current main pulls remote rows before it pushes local intent in apps/desktop/src/store/session-pin-sync.ts:102.
Problems
apps/desktop/src/store/session-pin-sync.ts:314callsunpinSession(pinId, row.id), butunpinSessionaccepts onestringatapps/desktop/src/store/layout.ts:379; this does not type-check.apps/desktop/src/store/session-pin-sync.ts:190drops an unpin if the preceding pin PATCH is still pending. That path neither records unpin intent nor schedules it after the pending write settles, so a rapid pin → unpin can leave the originalpinned: truewrite as the only backend mutation.
Suggested changes
- Make lineage alias removal use the actual
unpinSessionAPI. - Queue or serialize the latest per-id intent, and add a deferred-Promise pin → unpin test that resolves the earlier pin request last.
Automated hermes-sweeper review.
| } | ||
|
|
||
| for (const id of toUnpin) { | ||
| if (unconfirmed.has(id)) { |
There was a problem hiding this comment.
This skips a just-requested unpin while the preceding pin PATCH is pending, but does not record or queue the unpin for after that request settles. A rapid pin → unpin can therefore send only pinned: true; retain the latest desired value and issue it once the in-flight request completes.
| unpinSession(local.has(pinId) ? pinId : row.id) | ||
| // Drop every alias that names this conversation (legacy live-id pins and | ||
| // durable lineage-root pins both count). | ||
| unpinSession(pinId, row.id) |
There was a problem hiding this comment.
unpinSession currently accepts one string argument (apps/desktop/src/store/layout.ts:379), so this second string argument fails type-checking. Remove both aliases through an API-compatible operation.
teknium1
added
the
sweeper:blast-moderate
|
Superseded by #80711. Pin/unpin no longer flips back when a stale list page lands after the PATCH ack — the guard holds until a page confirms the written value. |
4 participants
Summary
Fixes the Desktop sidebar pin/unpin race where a lagging sessions list page undoes the user click in the same tick.
Root cause
pullRemotePins()treated the current page as authoritative before local pin/unpin intent was pushed. A page still carryingpinned: falsestripped brand-new pins; a page still carryingpinned: trueresurrected just-unpinned chats. The PATCH never stuck.Fix
Test plan
cd apps/desktop && npm test -- --run src/store/session-pin-sync.test.ts→ 14/14 passRelated open reports/PRs: #75342, #74638, #75295