fix(desktop): migrate active profile preference from legacy signals on first boot

[DavidMetcalfe]

Summary

Fixes #64160 (active-profile half): on first boot after a Desktop update (or fresh install), seed active-profile.json from the best available signal so the Desktop launches into the user's primary profile instead of always defaulting to "default".

What this PR does

Adds migrateActiveProfileIfMissing() in apps/desktop/electron/main.ts, called during startHermes() before readActiveDesktopProfile(). The migration is a no-op once the file exists.

Priority ladder:

1. Legacy ~/.hermes/active_profile — explicit CLI choice via hermes profile use. Highest confidence.
2. Running gateway — checks profiles/*/gateway.pid, validates PID liveness (kill(pid, 0)) AND process identity (command-line contains "hermes" via /proc or ps). Avoids false positives from stale PIDs recycled by the OS.
3. state.db heuristics — hybrid recency × log₁₀(size) score picks the primary workspace. A 409MB database beats a 28MB one even if touched at similar times during boot/update.

_migrated flag: The stored JSON includes _migrated: true so the renderer can optionally surface a one-time notification that the profile was auto-detected. Once the user interacts or switches profiles, writeActiveDesktopProfile() writes without the flag.

Design decisions

• PID validation includes command-line check (not just process.kill(pid, 0)) to avoid the case where macOS recycles a stale PID to an unrelated process (e.g. Chrome, Slack).
• Hybrid scoring formula per cross-vendor design review: recencyWeight × sizeWeight where recencyWeight = max(0.1, 30 − daysSinceModified) and sizeWeight = log₁₀(max(1024, size)). This correctly handles the case where a massive primary database (409MB) is slightly less recently touched than a tiny secondary one (3.6MB).
• Only writes non-default profiles — single-profile users and fallback cases preserve the legacy behavior.
• Scoped to active-profile.json only — the config preservation (model provider rewritten, MoA dropped) is tracked separately in the updater/Rust codebase.

Notes

• The config preservation half of [Bug]: Desktop updater drops active-profile preference and rewrites config model settings #64160 will need a separate PR in the updater codebase.
• The _migrated flag enables a future UI notification (e.g. "We restored your active profile to 'coder' after the update. [Keep] [Switch Profile]") — not implemented here, but the data is available.

[teknium1]

Thanks for targeting a real Desktop migration gap: current main maps a missing Electron preference to default in apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts:233-239.

Problems

• apps/desktop/electron/main.ts:6843 invokes the new migration only after startHermes() has called resolveRemoteBackend(primaryProfileKey()) (main.ts:6738). The remote branch returns at main.ts:6740-6761, so remote users with no active-profile.json still resolve as default and never run this migration. Move migration before the first profile-dependent remote resolution.
• The PR adds a multi-rung resolver and persistence behavior without Electron tests. apps/desktop/vitest.config.ts:13-20 already discovers electron/**/*.test.ts; please cover precedence, stale PID rejection, fallback behavior, and the remote boot path.

Suggested changes

• Initialize the preference before primaryProfileKey() is first used in startHermes().
• Put the migration decision in a testable Electron helper and add behavior tests for its validation and fallback rungs.

Automated hermes-sweeper review.

[teknium1]

startHermes() resolves resolveRemoteBackend(primaryProfileKey()) before this point and returns immediately for remote mode. With a missing preference that first lookup is default, and this migration never executes. Move initialization before the first primaryProfileKey()/remote-resolution call so remote profile overrides and the persisted preference agree.

[DavidMetcalfe]

Thanks for the close read — both findings were legitimate, and the rework addresses them.

Finding #1 (ordering) — fixed. The migration now runs at the top of startHermes(), before connectionPromise = (async () => {…}) and therefore before resolveRemoteBackend(primaryProfileKey()) inside it. Remote-mode users with a missing preference file no longer fall through to 'default' silently — the migration seeds the file first, then primaryProfileKey() reads it. Commit bb6006baa.

Finding #2 (tests) — fixed. Pulled the decision logic into a new pure module apps/desktop/electron/profile-migration.ts with a MigrationDeps bag (following the same dep-injection shape as profile-delete-routing.ts), then added apps/desktop/electron/profile-migration.test.ts with 29 vitest cases covering precedence (legacy > single-running-gateway > heuristic), stale-PID rejection (recycled PID to a non-hermes process is dropped), malformed pid files (non-integer / zero / negative PIDs), scoring edge cases (recency floored at 0.1, size floored at MIN_SIZE, larger DB beats smaller at similar recency), and single-profile fallback (best === 'default' suppresses the write). Commits 9ebfcada9 (helpers) and a6da58e93 (tests).

Two small things the cross-vendor review (Gemini Flash + GPT-OSS, parallel) flagged that aren't in your review but were worth catching:

• 'default' matched PROFILE_NAME_RE, so the original reader would have accepted a legacy active_profile file containing default and suppressed the heuristic rung. Added an explicit name === 'default' guard in readLegacyActiveProfile.
• readdirSync wasn't in the deps bag, so the orchestrator would have called require('fs').readdirSync directly and made the tests depend on the host filesystem. Added it to the bag so the test fixture is hermetic.

The remote-boot ordering is verified by code review of the placement — the test suite covers the pure decision logic the function relies on, which matches the repo's existing testable-helper pattern. There's no precedent in apps/desktop/electron/ for spawning an Electron process from a vitest case (only pure-helper tests); flagging that as a follow-up if you'd rather see an integration test added, but it'd be the first of its kind in the repo.

Happy to address any further concerns.

[DavidMetcalfe]

Rework pushed to fix/profile-migration-on-update (commits on top of d83213ad0). Both findings from your review addressed:

1. Remote-boot ordering (BLOCKER-class) → fixed. migrateActiveProfileIfMissing() now runs at the top of startHermes() — before connectionPromise = (async () => {…}) and therefore before resolveRemoteBackend(primaryProfileKey()) inside it. Remote-mode users with a missing active-profile.json no longer fall through to 'default' silently; the migration seeds the file first, then primaryProfileKey() reads it.

2. Test coverage (BLOCKER-class) → fixed. Pulled the decision logic into a new pure module apps/desktop/electron/profile-migration.ts with a MigrationDeps bag (same dep-injection shape as profile-delete-routing.ts), then added apps/desktop/electron/profile-migration.test.ts with 29 vitest cases covering precedence, stale-PID rejection, malformed pid files, scoring edge cases, and the single-profile fallback.

Two adjacent fixes the cross-vendor review (Gemini Flash + GPT-OSS in parallel) caught:

• 'default' matched PROFILE_NAME_RE so a legacy active_profile containing default would have suppressed the heuristic. Added an explicit guard.
• readdirSync was missing from the deps bag, making the orchestrator touch the host filesystem. Added it so the test fixture is hermetic.

The remote-boot ordering is verified by code review of the placement; the test suite covers the pure decision logic the orchestrator relies on. There's no precedent in apps/desktop/electron/ for spawning an Electron process from vitest — the existing testable-helper pattern (pure functions + deps injection, no process spawn) is the only established shape, so I matched it. If you'd rather have an integration test that exercises the boot ordering end-to-end, happy to add one as a follow-up — but it'd be the first of its kind in the directory.

Inline reply also posted under the original line-level comment.

[DavidMetcalfe]

Rebased onto current origin/main (was CONFLICTING). One conflict in apps/desktop/electron/main.ts: origin/main split the connectionPromise short-circuit into existingConnectionPromise (state-tracked) + connectionAttempt (state-tracked), so my insertion point landed inside that change. Resolved by placing the migration between the existingConnectionPromise short-circuit and the backendConnectionState.startAttempt() call — still before primaryBackendIsRemote() (line 6822) and resolveRemoteBackend(primaryProfileKey()) (line 6830), which are the first-reads that need the migration to have run.

Verification on rebased branch:

• tsc -p tsconfig.electron.json --noEmit → clean
• vitest run --project electron → 461 passed, 1 pre-existing skip (29 new + 432 existing)
• All 4 commit authors use the auto-skip +ID@users.noreply.github.com form

Branch force-pushed (--force-with-lease). Ready for review.

[DavidMetcalfe]

Antigravity review of the rebase resolution (Flash + GPT-OSS in parallel) confirmed the placement is correct. Consensus:

• Placement (between existingConnectionPromise short-circuit and backendConnectionState.startAttempt()) — ACCEPTABLE by both. startAttempt() is pure in-process state (verified: apps/desktop/electron/backend-connection-state.ts:17-19 just returns { generation, promise: null }), so it doesn't read primaryProfileKey() or primaryBackendIsRemote(). Placement before startAttempt() is the conservative choice.
• Re-entry semantics — ACCEPTABLE. Short-circuit guard means the migration runs exactly once per cold start; subsequent re-entries return the existing promise before reaching the migration line.
• Latched-failure interaction — ACCEPTABLE. Migration sits after the bootstrapFailure / backendStartFailure guards, which is correct: don't migrate when the app is stuck.
• Side-effect ordering — ACCEPTABLE. Migration is synchronous fs-only; no race with the startAttempt() token.

Both reviewers suggested one small comment polish (GPT-OSS: be explicit about which call reads which profile-state, since primaryBackendIsRemote() is on the next line while primaryProfileKey() is inside the IIFE). Applied as an amendment to the same fix(desktop) commit. Branch force-pushed with --force-with-lease.

Re-pushed SHA: 32f6a92bf. All 461 tests still pass. No blockers from the review.

The SHOULD-FIX finding (one reviewer asked for an Electron-process-spawn integration test covering the ordering) was raised and discussed in the previous review round — no precedent in apps/desktop/electron/ for that test pattern, and the existing pure-helper tests cover the decision logic. Leaving as-is matches the repo's established testable-helper convention. Happy to add an integration test as a follow-up if you'd rather see one.

[GottZ]

This was generated by AI during triage.

Summary

One PR addresses #64160. #64195 implements first-boot migration for the missing Desktop active-profile preference, while its three-file diff does not change the separate config.yaml update path implicated in the reported model/provider and Mixture of Agents settings rewrite.

Related pull requests

• fix(desktop): migrate active profile preference from legacy signals on first boot #64195 best fix — (+706/-0) — n/a; recorded best fix: the diff seeds active-profile.json before the first profile-dependent boot reads, selects from legacy preference, validated gateway, or state.db signals, and adds 29 migration tests. It addresses the active-profile half only; the visible COMMENTED keep_open review identified remote-boot ordering and test gaps, and the shown rework addresses those points by moving migration ahead of primaryBackendIsRemote()/primaryProfileKey() and extracting tested decision logic.

Suggested consolidation

Keep #64195 open with a salvage path as the recorded best existing fix: retain its tested active-profile migration, narrow its closing scope to that half of #64160, and track the config.yaml preservation failure separately. There are no duplicate PRs to close in this complex.

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
    I64160(["issue #64160 (open)"])
    P64195["PR #64195 (open)"]
    P64195 -->|best fix| I64160
    class I64160 open
    class P64195 open
    class P64195 best
    class P64195 target
    click I64160 "https://github.com/NousResearch/hermes-agent/issues/64160"
    click P64195 "https://github.com/NousResearch/hermes-agent/pull/64195"

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 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 29 kB of PR diffs, 6 kB of issue/PR text, 7 kB of discussion (5 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.