Skip to content

feat(desktop): expand active profile identity - #94450

Closed
SmelterLabs wants to merge 4 commits into
NousResearch:mainfrom
SmelterLabs:feat/active-profile-owner-rail
Closed

SmelterLabs wants to merge 4 commits into
NousResearch:mainfrom
SmelterLabs:feat/active-profile-owner-rail

Conversation

@SmelterLabs

Copy link
Copy Markdown

Summary

  • expand the currently active profile into a pinned avatar-and-name pill while keeping inactive profiles compact
  • render the inactive default profile as a circular, ringed initial so the machine owner remains distinct from ordinary rounded-square profiles
  • keep hover text to the profile name and preserve the All Profiles layers control
  • preserve the hidden active profile's position while reordering the visible compact profiles
  • keep legacy profile-only follow-up requests on the active remote registry connection instead of spawning a same-named local backend

Verification

  • npm test -- --run src/app/chat/sidebar/profile-rail-connect.test.tsx src/store/gateway-profile-request.test.ts src/store/profile-agent-activation.test.ts src/plugin-socket-scope.test.ts src/store/connections.test.ts — 50 passed
  • npm run typecheck — passed
  • targeted ESLint for all changed TypeScript files — passed with no warnings
  • npm run build — passed
  • full Desktop Vitest run — 7,439 passed, 4 skipped, 31 failed. The failures were outside the changed files and concentrated in POSIX permission/symlink assertions, SSH control-socket path assumptions, and WSL fixtures under Windows Git Bash; every focused changed-path test passed.

Notes

Profile portraits remain profile-owned assets served by profiles.get_asset; this change does not bundle any user-specific avatar artwork.

@SmelterLabs

Copy link
Copy Markdown
Author

Follow-up from independent review: fixed two owner-identity edge cases before maintainer handoff. The rail now recognizes machine owners from backend is_default metadata instead of assuming the root key is literally default, and it suppresses stale owner identity while a newly selected gateway's profile roster loads. Added regressions for root-keyed owners and cross-gateway stale-name flashes. Revalidated: 68 focused tests, Desktop TypeScript checks, targeted ESLint, and production build all pass.

@alt-glitch alt-glitch added type/feature New feature or request comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have labels Aug 25, 2026
@SmelterLabs

Copy link
Copy Markdown
Author

Second independent-review follow-up: fixed the remaining three rail defects. Sortable IDs now use stable profile keys even when display names differ; remote-profile hover and accessible labels stay name-only; and active/owner colors resolve from profile keys so named overrides remain stable while the default owner stays neutral. Added regressions for all three. Revalidated: 71 focused tests, Desktop TypeScript checks, targeted ESLint, production build, diff check, and secret/injection scan pass.

@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference, author can ignore or act on any point.

Strong PR overall — the $profilesConnectionId source-stamping (apps/desktop/src/store/profile.ts:52-56, published only on epoch match) plus the identity-scoped avatar commit in ActiveProfilePill is a careful answer to the flash-of-wrong-owner problem, and requestGatewayForProfile's active-agent shortcut fixes a real same-named-profile misrouting. Points:

  1. Inconsistent identity input to ProfileGlyph — in apps/desktop/src/app/chat/sidebar/profile-switcher.tsx, ActiveProfilePill's fallback renders <ProfileGlyph ... name={label} /> where label is the display name. Everywhere else this PR deliberately moves off display names onto stable keys (sortable ids at line ~833, colors resolved by profile.name). If ProfileGlyph derives anything from name, two renames of the same profile will flip its appearance. Pass profileName instead.

  2. The layers pill became a no-op in all-view — the isAll branch (~line 298) renders an active pill whose onSelect calls setShowAllProfiles(true) — i.e., it's already-all and clicking does nothing. The old control doubled as the escape hatch back to default ("leaving a profile never lands on all"); that return path now exists only via the tiny owner circle. If intentional, fine — but consider making the active-state pill visually inert (drop hover affordance) or wire it to selectProfile(defaultProfile.name) so the most prominent control isn't dead.

  3. Shortcut normalization mismatch risk — apps/desktop/src/store/gateway.ts:630-641: normKey(profile) is compared against activeGatewayProfileKey(). If normKey and normalizeProfileKey differ in any edge (casing, whitespace), a profile that is the active agent would miss the shortcut and spawn the duplicate local backend this PR fixes. Worth asserting their equivalence once, and adding a negative test: same-named profile requested while not active must still go through gatewayForProfile/spawn local.

  4. Avatar has no retry after onError — clearing avatar on <img> error (line ~737) permanently downgrades to the glyph until gateway/profileName changes; a transient IPC hiccup never recovers. A cheap retry counter in the effect deps (or re-request on window focus) would do.

Minor: mergeCompactProfileOrder returning allIds unchanged when indices are invalid is correct, but callers compare next !== ids for the haptic — document that identity-equality is the "no-op" signal so nobody replaces it with a deep compare.

@SmelterLabs

Copy link
Copy Markdown
Author

@Enough1122 Thanks for the careful review. I checked each point against the current behavior and made the changes that improve the rail without weakening its identity rules.

Changes made

  • Fixed the active All profiles layers pill: selecting it now returns to the current gateway's is_default machine owner instead of calling the already-active all-profiles state.
  • Refined the active-profile layout so the selected profile expands in its existing saved rail slot instead of moving to the far left. Selection now changes emphasis without changing identity order.
  • Updated drag handling for the resulting variable-width rail. It now clamps continuously on the x-axis and lets closestCenter resolve positions rather than assuming every slot has one fixed pitch.
  • Documented the referential-equality no-op contract in mergeCompactProfileOrder, as suggested.
  • Added regressions for in-place ordering, All Profiles → owner return, and variable-width drag clamping.

Points intentionally left unchanged

  1. ProfileGlyph fallback input: label is intentional there. That prop controls only the visible fallback initial, so it should match the display name the user sees. Stable profile identity is still used for routing, colors, avatar lookup, sortable IDs, and React keys. Passing profileName would make a renamed profile display an initial that disagrees with its visible name; for a root key such as default, it could also produce D instead of the owner's visible initial.

  2. Normalization: normKey and normalizeProfileKey currently have the same contract: trim whitespace and map an empty value to default. The active-registry regression covers the shortcut path, and the pooled-profile regression covers the non-active path that must still resolve/spawn normally, so no implementation change was needed.

  3. Avatar retry: I did not add an automatic <img onError> retry counter. That handler also receives malformed or undecodable image data, so retrying on every image error can create a loop without evidence that the failure was transient. IPC request failures and image decode failures are different cases; if logs show a real transient avatar-fetch problem, it should get a bounded retry at the fetch boundary rather than an unconditional render-error retry.

Revalidated after the follow-up: 74 focused tests, Desktop TypeScript, targeted ESLint, production build, git diff --check, and an independent pre-push review all pass. The separate session-row owner-badge work remains outside this PR.

@Enough1122

Copy link
Copy Markdown

Verified - the changes improve the rail without weakening identity rules: returning to is_default instead of re-calling active all-profiles state, keeping the selected profile in its existing rail slot (selection changes emphasis not order), and the drag handle refinements. Nothing further from me.

@SmelterLabs

Copy link
Copy Markdown
Author

Closing. This was decomposed: the connection-scoped profile cache and stale-backend recovery moved to #102673 and the sidebar launch-target hint to #102728. What remains here is local UI preference that does not belong upstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants