fix(desktop): stop pin-sync pull from reverting the user's own pin click

[hutao562]

Summary

Sidebar pinning in the Desktop app could never stick against a current backend: clicking Pin (or Unpin) looked like a no-op. The session never moved to the Pinned section, and the pin was silently reverted within milliseconds.

Root cause

watchSessionPins() in apps/desktop/src/store/session-pin-sync.ts runs reconcile() on every trigger — including changes to $pinnedSessionIds itself. reconcile() unconditionally starts with pullRemotePins(), which treats server rows as authoritative and drops local pins the server reports as unpinned.

But a reconcile triggered by the user's own click fires before the mirroring PATCH has even been issued, so every session row still carries the pre-click value:

• Pin: local set gains the id → reconcile → pull sees row.pinned === false → "server says it's gone" → unpinSession() reverts the click. The PATCH is never sent.
• Unpin: local set loses the id → reconcile → pull sees row.pinned === true → pinSession() re-adds it.

Backends that omit the pinned flag on session rows are unaffected (the pull skips undefined), which is why the existing tests — all using rows without the flag — passed while the feature was broken in production, where the list endpoint returns pinned on every row.

Verified live: after a pin click, localStorage's hermes.desktop.pinnedSessions is written and immediately deleted, no PATCH reaches /api/sessions/{id}, and sessions.pinned in state.db stays 0.

Fix

Gate the pull pass on the reconcile source:

• session-list refresh / boot → pull (that's where new server truth arrives)
• local pin-set change → push only (a local click carries no new server information, so there is nothing to pull)

.listen(() => reconcile('pins')) vs $sessions.listen(() => reconcile('sessions')); the push/mirror logic is unchanged, including the in-flight unconfirmed guard for stale list pages.

Tests

Two regression tests reproducing the production shape (rows that carry the flag):

• keeps a fresh local pin when the row already carries pinned=false — failed before the fix (expected [] to include 'a'), passes after
• keeps a fresh local unpin when the row still carries pinned=true

Full suite: npx vitest run src/store/session-pin-sync.test.ts → 13/13 green (11 pre-existing + 2 new), no behavior change for cross-app pin adoption, stale-page races, or legacy backends without the column.

[teknium1]

Thanks for the focused regression fix. The premise is confirmed on current main: apps/desktop/src/store/session-pin-sync.ts:144 routes local pin-set changes to reconcile(), whose unconditional pullRemotePins() call at :102 can execute unpinSession() at :88-89 before the push pass issues its PATCH. Current session APIs serialize pinned as a boolean (hermes_cli/web_routers/sessions.py:150), so this is a live path rather than a legacy-backend edge case.

The source-tagged reconciliation preserves server pulls for boot/session refreshes while making a local click push-only, and the added tests exercise both conflicting initial row values. GitHub comparison reports the PR base is identical to current main, so this should salvage as a clean cherry-pick.

Automated hermes-sweeper review.

[alt-glitch]

This was generated by AI during triage.

Duplicate of #74788: both separate local pin-click push handling from session-list remote reconciliation so stale pinned values cannot immediately reverse a user action.

[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 makes local pin-set reconciliation push-only, and #75342 tracks current-process intent separately from restored pins while preserving server authority at boot.

Related pull requests

• fix(desktop): prevent stale session page from undoing pin/unpin clicks #74638 duplicate — (+69/-2) — close as duplicate of fix(desktop): stop pin-sync pull from reverting the user's own pin click #75295: its justChanged symmetric-difference guard fixes the immediate pin/unpin reversal, but overlapping opposite writes can still let an older completion clear the newest unconfirmed guard. Despite the automated keep-open review on fix(desktop): prevent stale session page from undoing pin/unpin clicks #74638, the diff lacks the requested pin→unpin and unpin→pin deferred-promise coverage, while fix(desktop): stop pin-sync pull from reverting the user's own pin click #75295 provides the narrower source-based separation.
• fix(desktop): stop pin-sync pull from reverting the user's own pin click #75295 related — (+44/-5) — keep open with a salvage path: its source-tagged reconciliation directly prevents a local click from consulting stale server rows while retaining remote pulls for boot and session-list updates, with tests for both stale pinned:false and stale pinned:true values. This is consistent with the automated keep-open verdict; before consolidation, compare it directly with the contributor-identified duplicate fix(desktop): preserve local pin clicks during server sync #74788 and add overlapping-write coverage with a latest-write-safe per-ID guard if that race remains reachable.
• fix(desktop): pin/unpin mirroring — pull-pass reverts brand-new pins and resurrects just-unpinned ones #75342 duplicate — (+154/-33) — close as duplicate of fix(desktop): stop pin-sync pull from reverting the user's own pin click #75295 after salvaging its restart-after-remote-unpin regression test: the revised diff separates current-process localIntent from restored localStorage state and thereby addresses the contributor's cross-app server-authority objection. Despite the keep-open review on fix(desktop): pin/unpin mirroring — pull-pass reverts brand-new pins and resurrects just-unpinned ones #75342, its updated diff remains a substantially larger competing implementation of the same immediate race, while its distinct boot-authority coverage can be retained in fix(desktop): stop pin-sync pull from reverting the user's own pin click #75295.

Duplicates

#74638, #75295, and #75342 implement competing repairs for the same stale pull-before-push pin-sync race. Discussion also identifies #75295 as a duplicate of #74788, so the duplicate chain is #74638/#75342 → #75295 → potentially #74788, subject to direct diff comparison with #74788.

Suggested consolidation

Keep #75295 open with a salvage path: use its focused source-based pull gating as the consolidation point, verify the contributor-reported overlap with #74788, add generation-safe overlapping pin→unpin and unpin→pin tests if needed, and port #75342's restart-after-remote-unpin regression. Then close #74638 as a duplicate of #75295 because its guard leaves the documented overlapping-write gap, and close #75342 as a duplicate of #75295 once its boot-authority test is preserved; if direct comparison confirms that #74788 already contains the same viable mechanism, close #75295 through that explicit duplicate chain instead.

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 P75295 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 pin-sync pull-before-push revert; the guard now holds until a page confirms the written value.