fix(desktop): prevent stale session page from undoing pin/unpin clicks

[TheRealMisterFix]

Problem

Clicking pin or unpin on a session in the desktop app had no effect — the pin state would snap back to its previous value immediately.

Root Cause

session-pin-sync.ts reconciles the sidebar's local pin state (localStorage) with the backend sessions.pinned flag in state.db. The reconcile() function runs pullRemotePins() before the push pass writes the user's change to the backend.

When the user clicks unpin:

1. `` removes the id → triggers reconcile()
2. pullRemotePins() runs first — but the `` page still carries the old server value (pinned=true)
3. pullRemotePins sees: server says pinned=true, local set doesn't have it, no unconfirmed write in flight yet → re-pins it immediately and adds it to mirrored
4. The push pass then sees the id is already in mirrored → skips the unpin write

The same race happens in reverse when pinning a session the server says is pinned=false.

The existing unconfirmed guard only protected against stale pages arriving during an in-flight write — not the gap between the user's click and writePin() being called.

Fix

Compute a justChanged set — the symmetric difference between the local pinned set and what's been successfully mirrored — and pass it to pullRemotePins(), which now skips those ids entirely. This prevents the stale server page from undoing the user's click before the push pass writes the correct value.

The pinned set is re-read after pullRemotePins() returns (since it may have adopted/dropped pins from the server) so the push pass sees the updated state.

Test Plan

• All 13 existing tests in session-pin-sync.test.ts pass
• Added regression test: stale page with pinned=false does not undo a just-made pin
• Added regression test: stale page with pinned=true does not re-pin a just-unpinned session

✓  ui  src/store/session-pin-sync.test.ts (13 tests) 8ms
 Test Files  1 passed (1)
      Tests  13 passed (13)

[alt-glitch]

This was generated by AI during triage.

Related: #74418 and #74444 repair the same Desktop pin-sync race with different ordering guarantees. This PR skips just-changed IDs during the stale pull; maintainers should choose or consolidate the intended concurrency policy.

[teknium1]

Thanks for isolating the pre-PATCH stale-page race. The premise holds on current main: reconcile() pulls remote rows before the push pass at apps/desktop/src/store/session-pin-sync.ts:102, while the existing stale-response guard is only installed by writePin() at lines 42-45.

Problems

• The added justChanged guard covers the initial click-to-write gap, but rapid opposite clicks still lose the newest guard. writePin() overwrites unconfirmed by id at apps/desktop/src/store/session-pin-sync.ts:43; either older completion deletes that id unconditionally at lines 47 and 50. A stale page can then be accepted by pullRemotePins() and restore the old local state. The new tests cover one click in each direction, not overlapping writes.

Suggested changes

• Preserve only the latest write's guard, for example with per-id generations, and add deferred-promise tests for pin→unpin and unpin→pin. This aligns with the related concurrency-policy discussion on fix(desktop): preserve local pin intent during sync #74418 and fix(desktop): preserve concurrent session pin intent #74444.

Automated hermes-sweeper review.

[GottZ]

This was generated by AI during triage.

Summary

Three PRs address the same pull-before-push race in Desktop session pin synchronization: #74638 skips locally changed IDs during the stale pull, #75295 suppresses the pull when reconciliation originates from a local pin-set change, and #75342 tracks current-process intent separately from restored pins while preserving server authority at boot.

Related pull requests

• #74638 related — (+69/-2) — close as duplicate of #75295: its justChanged symmetric-difference guard fixes the immediate pin/unpin reversal, but it does not address overlapping writes whose older completion can clear the latest unconfirmed guard. This differs from the automated keep-open review on #74638 because the complete diff still lacks the requested pin→unpin and unpin→pin deferred-promise coverage, while #75295 provides the narrower source-based reconciliation split.
• #75295 duplicate — (+44/-5) — keep open with a salvage path: the source-tagged reconciliation directly separates local push-only events from boot/session-list pulls and tests both stale pinned:false and stale pinned:true rows. Consistent with its automated keep-open review, retain this focused mechanism, then add overlapping-write regression tests and preserve only the latest per-ID write guard.
• #75342 duplicate — (+154/-33) — close as duplicate of #75295 after salvaging its restart-after-remote-unpin test: the revised diff now distinguishes current-process localIntent from restored localStorage state and therefore addresses the contributor's cross-app authority objection. Despite the keep-open review on #75342, the updated mechanism remains a substantially larger competing implementation of the same race; its unique boot regression can be ported without retaining the duplicate synchronization design.

Duplicates

#74638, #75295, and #75342 repair the same stale pull-before-push pin-sync race with different guards. #75295 is also explicitly identified in discussion as a duplicate of #74788; the consolidation chain should therefore be checked against #74788 before final closure.

Suggested consolidation

Keep #75295 open with a salvage path: use its focused source-based pull gating as the consolidation point, add deferred pin→unpin and unpin→pin tests with a generation-safe per-ID unconfirmed guard, and port #75342's restart-after-remote-unpin regression. Then close #74638 as a duplicate of #75295 because its narrower justChanged guard leaves the documented overlapping-write gap, and close #75342 as a duplicate of #75295 once its boot-authority test is preserved; also resolve the existing #75295 → #74788 duplicate relationship before choosing the surviving PR.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    subgraph Dup74638 ["PRs duplicating each other"]
        P74638["PR #74638 (open)"]
        P75295["PR #75295 (open)"]
        P75342["PR #75342 (open)"]
    end
    class P74638 open
    class P75295 open
    class P75342 open
    class P74638 target
    click P74638 "https://github.com/NousResearch/hermes-agent/pull/74638"
    click P75295 "https://github.com/NousResearch/hermes-agent/pull/75295"
    click P75342 "https://github.com/NousResearch/hermes-agent/pull/75342"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 19 kB of PR diffs, 7 kB of issue/PR text, 3 kB of discussion (6 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[OutThisLife]

Superseded by #80711. Same stale-page-undoes-the-click race; the pin guard now holds until the written value comes back.

[OutThisLife]

Superseded by #80711. Same stale-page-undoes-the-click race; the pin guard now holds until the written value comes back.