fix(desktop): respect profile scope for cron jobs and messaging sessions - #52910
fix(desktop): respect profile scope for cron jobs and messaging sessions#52910lerryzou wants to merge 1 commit into
Conversation
The Desktop App was displaying cron jobs, messaging sessions, and messaging channels from all profiles regardless of which profile was selected. This broke the profile isolation boundary. Root cause: Three data-fetching calls hardcoded 'all' as the profile parameter instead of respecting the active profile scope: - getCronJobs() in hermes.ts didn't pass profile query param - refreshCronSessions() hardcoded profile='all' - refreshMessagingSessions() hardcoded profile='all' - loadMoreMessagingForPlatform() hardcoded profile='all' Fix: Pass the current profileScope (from store) to all these calls, matching the pattern already used by refreshSessions(). The backend already supports profile filtering, so this is purely a frontend fix. Fixes NousResearch#52401
Related: competing-cluster member for desktop profile-scoping. Fixes #52401 (superset issue) and overlaps the cron-sidebar cluster (#51520 spec; competing fix PRs #42654 call-site arg, #47429 helper-internal |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the cross-profile sidebar leak. The underlying bug remains on current main, but this patch needs a targeted salvage.
Problems
- The three session callbacks moved from
apps/desktop/src/app/desktop-controller.tsxtoapps/desktop/src/app/session/hooks/use-session-list-actions.ts:84-139in25c7900fb58239552c530fd6cf8be28e5efbcec4, so the frontend hunk no longer applies. apps/desktop/src/hermes.tsin this PR builds the cron query from_apiProfile, whereas the sidebar is governed byprofileScope.apps/desktop/src/store/profile.ts:294-295supports explicitALL_PROFILES; in that mode_apiProfilestill follows the active gateway profile, so this would filter the All profiles view to one profile.- Please add profile-scoping regression coverage. Current
apps/desktop/src/hermes.test.ts:96-112covers only the unscoped cron path.
Suggested changes
- Port the three session calls to
use-session-list-actions.tsand passprofileScope === ALL_PROFILES ? 'all' : profileScope. - Give
getCronJobsan explicit scope argument and pass the sidebar scope through all callers, including the Cron view.
Automated hermes-sweeper review.
| } | ||
|
|
||
| export function getCronJobs(): Promise<CronJob[]> { | ||
| const qs = _apiProfile ? `?profile=${encodeURIComponent(_apiProfile)}` : '' |
There was a problem hiding this comment.
_apiProfile follows the active gateway profile, but the sidebar may be in explicit All profiles mode ($profileScope === ALL_PROFILES). In that mode this sends one active profile rather than profile=all. Accept the sidebar scope as a getCronJobs argument and pass profileScope === ALL_PROFILES ? 'all' : profileScope from the callers.
|
Status update: the cron-jobs half of this PR is now superseded — profileScoped() routing landed via #67493, the contract test via #67602, and the endpoint-level ?profile= list scoping (sidebar + overlay) via #67615, merged today. The messaging/cron session fetch scoping half overlaps #60688's territory and is not yet covered on main — that half still stands. If you rebase onto current main and narrow to the listAllProfileSessions scoping (coordinating with #60688, which targets the same call sites), it can be reviewed on its own merits. Leaving open for that half. |
|
Resolved on main by #87566 — cron and messaging sidebar slices now follow the selected profile end to end. You submitted this early in the cluster (June 26); thanks for the contribution! |
Problem
When a non-default profile is selected in the Desktop App sidebar, cron jobs from ALL profiles are displayed instead of just the active profile's jobs. This violates profile isolation and confuses users who expect to see only their current profile's data.
Root Cause
Four functions in the Desktop App frontend were hardcoded to fetch data from
'all'profiles, ignoring the active profile context:refreshCronSessions()- always passedprofile='all'refreshMessagingSessions()- always passedprofile='all'loadMoreMessagingForPlatform()- always passedprofile='all'getCronJobs()- didn't pass any profile parameter (backend defaulted to'all')Solution
Updated these functions to respect the
profileScopecomputed store, which reflects the currently selected profile in the sidebar. Each function now:profileScopeinstead of hardcoded'all'profileScopein theuseCallbackdependency array to ensure refetch on profile changeTesting
npm run lintnpm testfor cron and messaging componentsImpact