Skip to content

fix(desktop): surface pins hidden by profile scope - #75488

Open
davewilliamson wants to merge 2 commits into
NousResearch:mainfrom
davewilliamson:fix/cross-profile-pinned-sessions
Open

fix(desktop): surface pins hidden by profile scope#75488
davewilliamson wants to merge 2 commits into
NousResearch:mainfrom
davewilliamson:fix/cross-profile-pinned-sessions

Conversation

@davewilliamson

@davewilliamson davewilliamson commented Jul 31, 2026

Copy link
Copy Markdown

What does this PR do?

Keeps profile-scoped Desktop recents isolated while making durable pins in sibling profiles visible as a summary instead of silently making those conversations look deleted.

The sidebar returns a cheap hidden_pinned_count, shows +N beside Pinned, and offers Show all profiles. It deliberately does not merge sibling-profile rows into a concrete profile's recents: cloned profiles can legitimately contain identical session IDs.

Desktop pin persistence now uses profile-qualified identities, with conservative migration of legacy unqualified IDs. This lets cloned profiles sharing a session ID retain independent, opposing pin states without one profile's row overwriting or unpinning the other. Empty selected profiles also retain the hidden-pin notice and recovery action.

The Electron remote-profile merge preserves pinned backfills beyond the normal recency window and computes the same hidden-pin summary for remote configurations.

Related Issue

Related to #51685 and #74760.

This addresses the profile-scope/remote-pagination discovery path; no session-row deletion was demonstrated. It also resolves both issues from the automated review on the first revision: empty-profile notice visibility and cloned-ID pin ambiguity.

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

  • Add a filtered SessionDB.session_count(..., pinned_only=True) summary path.
  • Return sibling-profile durable pin counts separately from profile-scoped recents.
  • Store and clear the hidden-pin summary with the rest of gateway-bound session state.
  • Show the count in the Pinned section with a route to the all-profiles view, including when the selected profile has no local sessions or projects.
  • Persist Desktop pins as profile + lineage-root-id identities across sidebar, tab, command-centre, drag-order, archive/delete, and sync paths.
  • Migrate legacy ID-only pins when ownership is known; for ambiguous cloned IDs, adopt explicit durable pins rather than guessing a sibling profile.
  • Route default-profile pin writes explicitly, so all-profile operation cannot accidentally mutate the currently active named profile.
  • Preserve old pinned rows after Electron merges and windows local/remote profile results.
  • Keep concrete-profile payloads scoped: sibling session objects never enter that renderer index.

How to Test

  1. Pin a session in profile A, switch to an empty profile B, and confirm Pinned shows +1 plus Show all profiles without profile A's ordinary recents leaking into B.
  2. Clone profiles so both contain the same session ID, pin it in one profile only, select All profiles, and confirm the two rows retain opposing states.
  3. Run:
    • scripts/run_tests.sh tests/test_hermes_state.py tests/hermes_cli/test_web_server_sidebar_sessions.py -q — 142 passed
    • npx vitest run --project ui — 3,298 passed
    • npx vitest run --project electron — 935 passed, 2 skipped
    • npm run typecheck — passed (renderer, Electron, E2E)
    • npm run lint — 0 errors (82 existing warnings outside this change)
    • npm run build — passed
    • uv run ruff check hermes_state.py hermes_cli/web_routers/profiles.py tests/test_hermes_state.py tests/hermes_cli/test_web_server_sidebar_sessions.py — passed

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
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.5.2 (Apple Silicon)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A; no user configuration changes
  • 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 — platform-neutral Python/TypeScript plus full Electron platform suite
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

The empty-profile notice, cloned-ID indexing/migration, scoped hydration, UI interaction, and Electron pagination paths have automated regression coverage. The production Desktop build completes successfully.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) comp/cli CLI entry point, hermes_cli/, setup wizard area/profiles Multi-profile isolation, HERMES_HOME scoping area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 31, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracing the scoped-recents and remote-pagination paths. The current-main premise is valid: hermes_cli/web_routers/profiles.py:290-299 only returns recents for the concrete scope, so sibling durable pins are absent from the sidebar cache.

Problems

  • apps/desktop/src/app/chat/sidebar/index.tsx:1093 hides every session section when the concrete profile has no rows or projects. The new hidden-pin notice is inside that gated subtree, so the exact empty-profile case has no +N indicator or Show all profiles recovery action.
  • Cloned IDs remain ambiguous after the proposed route to All profiles. apps/desktop/src/store/session-pin-sync.ts:75-91 reconciles global, unqualified pin ids, and apps/desktop/src/app/chat/sidebar/session-index.ts:24-29 overwrites direct-id collisions. A sibling pinned: false row can therefore remove or mask the other profile's durable pin.

Suggested changes

  • Include the hidden-pin count in the session-section visibility condition and cover an empty scoped profile with a sibling pin.
  • Scope persisted pin identity by profile (with migration) and cover same-id cloned profiles with opposing pin states.

This is an automated hermes-sweeper review.

@@ -312,6 +326,7 @@ export function ChatSidebar({
// profile while scope is still ALL (persisted), the rail is hidden and they'd

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

crossProfilePinCount also needs to participate in showSessionSections. Otherwise an empty concrete profile has no rendered Pinned section, so this count and its Show all profiles action are unreachable.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping area/sessions Session lifecycle, resume, persistence, history comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants