fix(desktop): preserve local pin intent during sync

[dyreckt]

What does this PR do?

Preserves a user's new local pin or unpin while the Desktop reconciles session rows from the backend.

#74234 made the server row authoritative, but reconcile() pulled remote state before the local push pass installed its in-flight guard. With realistic list data, clicking Pin updates the local store, then the still-stale pinned: false row immediately removes it before any PATCH is sent. The inverse race also prevents an unpin while the row still says pinned: true.

This moves pullRemotePins() after the push pass. writePin() installs the existing unconfirmed guard synchronously before issuing the PATCH, so a stale list row cannot contradict the user's newest local action. Remote adoption/drop behavior remains unchanged.

Related Issue

Regression introduced by #74234.

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• apps/desktop/src/store/session-pin-sync.ts: push local pin intent before pulling remote rows, allowing the existing in-flight guard to reject stale list state.
• apps/desktop/src/store/session-pin-sync.test.ts: exercise realistic pinned: false and pinned: true rows for both local pin and local unpin.

How to Test

1. Run npm exec -- vitest run src/store/session-pin-sync.test.ts from apps/desktop.
2. Confirm a new local pin remains selected and sends PATCH pinned=true while the loaded row still reports pinned: false.
3. Confirm a new local unpin remains removed and sends PATCH pinned=false while the loaded row still reports pinned: true.

Before the production change, those two realistic cases fail: the pin is removed locally and the unpin sends no PATCH. After the change, all 11 focused tests pass.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass — attempted with scripts/run_tests.sh; the local Python suite is not green due unrelated optional-dependency/platform baseline failures. This PR changes no Python.
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS 26.6, Node 24.18.0

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — N/A; behavior and code comments only
• I've updated cli-config.yaml.example if I added/changed config keys — N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — no platform-specific APIs changed
• I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

No visual surface changed.

Validation:

• npm test: 411 files passed, 1 skipped; 3,842 tests passed, 2 skipped
• npm run typecheck: passed
• npm run lint: passed with 0 errors (existing repository warnings remain)
• npm exec -- vitest run src/store/session-pin-sync.test.ts: 11/11 passed
• git diff --check: passed

[PRATHAMESH75]

Reviewed against #74570 (filed after this PR, same root cause) — this fixes it correctly.

The race. #74570 traces it precisely: pinSession() updates the local store, the synchronous $pinnedSessionIds listener runs reconcile(), and the old ordering called pullRemotePins() first — before the push pass had issued writePin(). So the just-pinned id had no unconfirmed entry yet, the still-stale pinned:false row satisfied !row.pinned && heldLocally, and unpinSession() reverted the pin instantly.

The fix. Moving pullRemotePins() to the end of reconcile() means the push pass runs first: the flush loop calls writePin(id, true, ...), which sets unconfirmed.set(id, true) before any pull reads the row. Traced through: when pullRemotePins() then hits the stale row, awaited = unconfirmed.get(pinId) = true, awaited !== row.pinned (false) → continue, so the pin is preserved. Correct.

No echo regression. The old docstring justified pull-first as avoiding a redundant PATCH of a genuinely-remote pin. That property survives the reorder: pullRemotePins() still mirrored.add(pinId) when it adopts a remote pin, and pinSession() re-triggers reconcile(), whose newly-pinned loop skips ids already in mirrored — so no echo PATCH. Verified against the full session-pin-sync.ts at this head.

Coverage. The test updates make the rows carry explicit pinned values (row('a', { pinned: false }) / row('b', { pinned: true })), which is what actually exercises the race the previous fixtures masked.

LGTM — resolves #74570.

[teknium1]

Thanks for the focused regression fix. The premise holds on current main: reconcile() calls pullRemotePins() before the local push pass at apps/desktop/src/store/session-pin-sync.ts:102, while writePin() only installs the stale-response guard at apps/desktop/src/store/session-pin-sync.ts:42-45. A stale server row can therefore reverse a local click before its intended PATCH is guarded.

The PR moves the pull after the push/flush pass and strengthens both fixtures with explicit opposite stale values. This preserves the existing unconfirmed mechanism rather than adding new state or transport surface.

Automated hermes-sweeper review.

[monerostar]

Win11 live-verify (monerostar)

Env: Windows 11 native · Hermes Desktop from apps/desktop/release/win-unpacked · multi-profile (tech/main/lifestyle/x) · install tree 4a798f4b

Control (broken on main before this class of fix)

• Sidebar pin/unpin clicked but state snapped back immediately.
• state.db still had correct sessions.pinned rows (tech 7 / main 5 / lifestyle 6 / x 1) while hermes.desktop.pinnedSessions localStorage was empty/[].
• Root cause match: list APIs always coerce pinned to a real boolean; reconcile() pulled before push, so a click saw stale pinned: false|true with no unconfirmed guard and undid itself.

This PR (#74418)

• Same minimal fix we landed locally: push before pull so writePin stamps unconfirmed first.
• Tests updated to the production boolean shape (pinned: false on pin path) — that is the gap the old suite missed.
• Focused +9/−8 on only session-pin-sync.ts + test — prefer this over larger concurrent-intent rewrites unless maintainers want the extra machinery.

Local after equivalent change

• Vitest session-pin-sync.test.ts: 13/13 pass (including prod-shaped pin/unpin regressions).
• Rebuilt Desktop renderer, restarted app: pin/unpin sticks again; DB pins hydrate into the Pinned section.

Siblings (do not land all)

Same race class also open as #74638, #74788, #74444 (and messaging-slice #74550). Adjacent missing-row work: #74760 (hydrate pins outside recents). One of the push-before-pull / intent-guard PRs is enough for the click race.

LGTM on #74418 as the smallest correct fix for the click race.

[teknium1]

Merged PR #75973 (#75973) for #74570, which implements the same push-first/pull-last reorder you identified — your PR was submitted first and pointed at exactly the right root cause; credit noted here. #75973 additionally fences pending pin ids out of the pull and fixes the re-entrant mirrored bookkeeping, with sabotage-verified regressions for all three shapes. Closing this one as superseded — thanks for the correct early diagnosis.