feat: All Servers mode with aggregated monitors (all-profiles phase 2) - #339
Open
pliablepixels wants to merge 89 commits into
Open
feat: All Servers mode with aggregated monitors (all-profiles phase 2)#339pliablepixels wants to merge 89 commits into
pliablepixels wants to merge 89 commits into
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
reLoginFor(id) ignored id and reused the current profile's reLogin, so a 401 on a non-current profile's session (aggregate readers) would re-auth the wrong profile against the wrong server. Looks up the profile fresh at call time and logs it in against its own client.
useProfileScope resolves single-profile or ALL_PROFILES_ID aggregate scope for later data-fetching consumers; useCurrentProfile gains isAllMode so route guards can stop redirecting to setup in All mode.
Deleting profiles one-by-one while in All mode leaves currentProfileId at the sentinel with zero real profiles (deleteProfile only resets it on a matching id, never for the sentinel). useProfileScope now returns null in that case so it means "route to setup" uniformly in both modes, since Task 6's route guards key off it.
useMonitorStream, useServerUrls, and useFreshAccessToken each gain an optional trailing profileId, so an All-mode monitor tile owned by a non-current profile can build its stream/snapshot URL from that profile's cgiUrl/minStreamingPort and attach its token, instead of always reading the globally-selected profile. Defaulted calls (no profileId) are unchanged. Adds useProfileById as the shared profile+settings-by-id resolver (useCurrentProfile.ts, additive only). server-resolver.ts and multiport.ts's getEffectiveMinStreamingPort already took explicit params/ids and needed no changes.
The explicit-profileId and defaulted useMonitorStream tests never observed minStreamingPort, so a regression falling back to the current profile's port would pass undetected. Forward options.minStreamingPort through the test's getStreamUrl mock and assert profile B's 40000 (explicit) vs profile A's 30000 (defaulted).
useProfileScope's `?? []` fallback lived outside the useShallow selector, allocating a fresh array every render and defeating the scope useMemo. useServerUrls depended on a version counter it never read inside the memo body; snapshotting the server map itself makes it a real dependency instead of an unused one.
- Profiles page: "All Servers" card above the list (>=2 profiles), switches to the ALL_PROFILES_ID sentinel via the existing handleSwitchProfile flow and navigates to /monitors. - ProfileSwitcher: matching "All Servers" entry and active-state label. - Route guards (App.tsx root redirect, AppLayout) now gate on useProfileScope() resolving instead of currentProfile alone, so All mode with >=1 profile no longer bounces to /profiles (Task 2 finding). - Monitors page: single code path via useScopedMonitors for both modes. All mode renders every profile's monitors with a profile chip, an ErrorBanner + retry strip per failed profile (regardless of isLoading, which never clears on a total outage per Task 4's finding), an all-failed empty state, and an optional group-by-server toggle (new ALL-bucket setting `monitorsGroupByServer`). - MonitorCard/LiveMonitorPlayer take an optional profileId so an All-mode tile streams from its own server (via useProfileById through the existing useMonitorStream/useServerUrls chain) and switches into the owning profile before any detail/events navigation; deep /all/... routing is Phase 3. - New strings added to all five locales. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Monitors.tsx's per-profile error strip referenced common.retry, which didn't exist (the app's only "retry" string lived under qr_scanner). npm run gates catches this via translation-keys.test.ts.
…#337) MonitorCard's owning-profile switch had no error handling: a failed switchProfile in All mode did nothing visible and left an unhandled rejection. It now reports the same switch-failed toast profile-switcher.tsx uses and skips the navigation that follows. Monitors.tsx's per-profile error strip showed for any errored profile, including one with cached monitors already rendering and only a transient background-refetch failure, a case the pre-Task-6 code deliberately suppressed (OfflineBanner covers it). Restored that suppression per profile, uniform across both modes: a strip only shows for a profile that produced zero monitors.
Adds the Task 7 e2e coverage and docs for All Servers mode: a feature covering aggregation across profiles and the partial-failure strip when one server is unreachable, a user-guide section on All Servers mode, and a new call-flow trace for how the scope hook and aggregation hook work.
useScopedMonitors and useMonitors gated their query on isAuthenticated, so an All-mode profile that had never authenticated this session stayed disabled forever - no data, no error strip, silently missing. Enable once a profile is in scope and let the API client's own proactiveLogin self-heal the first request. handleProfileRehydration also logged ERROR and skipped bootstrap for the ALL_PROFILES_ID sentinel; it now recognizes it and marks the app initialized with an INFO log.
server-resolver held one module-global serverMap populated by whichever profile bootstrapped last, so useServerUrls could resolve profile B's ServerId monitor against profile A's map, attaching B's token to A's host. It is now Map<ProfileId, ServerUrlMap>, keyed by the bootstrapping profile; getPortalUrlForMonitor/getPortalUrlForEvent take an optional profileId (defaulting to current via a gate, mirroring the sessions gate pattern) so their existing call sites are unaffected. dropSession now clears just that profile's map entry; dropAllSessions clears all. setReLoginCallback was only ever registered for the rehydrated current profile, so getFreshAccessToken(B) had nothing to fall through to and couldn't self-heal an expired refresh token in All mode. reLoginFor(id) now registers itself the first time a session is built for that profile (getSession calls it exactly once per profile), collapsing profile-initialization's old single-callback registration onto it.
Add profile-switcher coverage: profile-switcher-all is absent with one profile, present with two, and switches to ALL_PROFILES_ID when clicked. Radix DropdownMenu is portal/open-state driven and untestable in jsdom without a real trigger interaction (no test in this repo does that; Settings.test.tsx stubs Select the same way), so the primitives are stubbed as plain passthrough elements exposing the real menu-item render logic. Monitors: the all-profiles-failed empty state now gets its own title (monitors.all_failed_title, all 5 locales) instead of reusing monitors.no_cameras. The per-profile error strip drops the "ProfileName: " prefix in single mode, where it only ever names the one profile already shown as the page context.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SecureImage, thumbnail-chain, and MonitorHoverPreview gain optional profileId params (default current, zero single-mode change) so All-mode media resolves against its owning profile's session/server instead of the globally-selected one. connKeys and the Go2RTC failure cache switch from monitorId-only keys to profileId:monitorId (monitorCacheKey), since two profiles on independent ZM servers can otherwise collide on one monitor id.
useScopedEvents fans getEvents out over the active profile scope, reusing queryKeys.eventsList's exact shape so single mode shares its cache slot with Events.tsx. eventInstant (lib/event/) derives each event's true epoch instant from the owning profile's timezone via date-fns-tz, so merged events from different-timezone profiles sort correctly instead of by their raw server-local string.
useScopedEvents baked in bandwidth.eventsWidgetInterval unconditionally, but the Events page's current query has no polling at all - wiring the hook in Task 4 would have silently started 30s polling for single-profile users (times N in All mode), with no way to opt out. refetchInterval is now caller-supplied (undefined = no polling); the caller sources it from useBandwidthSettings itself when it wants live refresh, keeping the Polling contract at the call site instead of inside the hook.
New /all/monitors/:profileId/:monitorId and /all/events/:profileId/:eventId routes render the existing MonitorDetail/EventDetail pages against the route's owning profile instead of whichever profile (if any) is globally current. Their handler hooks (usePTZControl, useAlarmControl, useModeControl, useBulkDeleteEvents, useEventNavigation, useEventTags, useMonitorRecentEvents, useMonitorNavigation) take an optional profileId, defaulting to the current profile. MonitorCard now navigates straight into the deep route in All mode without switching profiles first; the events list button still switches (pending the events aggregation work). sessions.ts gains tryGetCurrentSession(), a non-throwing counterpart to getCurrentSession() for UI code that can render while All mode has no single current profile.
MonitorAppPreferences (per-monitor go2rtc/force-ZMS toggles) and MonitorSettingsDialog's credential-masking read both went through useCurrentProfile() directly, so a monitor reached via /all/monitors/:profileId/:id, or a differently-owned card in the All-mode monitor grid, would read and write the globally-current profile's settings instead of its own: a cross-profile settings leak, and for disableLogRedaction, a camera-credential exposure. Both now take an optional profileId (defaulting to current, same pattern as the other Task 3 handler hooks), threaded from MonitorDetail and Monitors' existing settingsProfileId.
Events.tsx aggregates via useScopedEvents in both modes (single mode shares the existing cache slot, no polling); All mode adds a profile chip per row, per-profile error strips + all-failed state (Monitors semantics), a server filter chip row (new eventsServerFilter ALL setting), and a monitor filter grouped by owning server. Row clicks in All mode deep-link to /all/events/:profileId/:eventId, and each row resolves its own owning-profile portal URL/token/monitor lookup so thumbnails and favorite/archive actions hit the right server instead of the (absent) current profile. useScopedEvents now converts a shared date-range filter per profile via new formatForServerInTz (lib/time.ts), instead of the caller pre-converting once against the current profile's timezone, and exposes totalCount/isFetching/refetchAll so Events.tsx's pagination and pull-to-refresh behave identically to the old single-query page. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Timeline aggregates in All mode via new useScopedTimelineEvents, selected alongside the existing single-profile useTimelineData (both hooks always render per React's rules; each disables its own queries via `enabled` so only the active mode fetches). Events plot by true absolute instant (eventInstant), not a naive browser-local parse, so two profiles in different real timezones interleave correctly on the shared canvas axis. Monitor rows and the event-preview popover carry a profile chip; clicking an event or tapping the scrubber in All mode deep-links to /all/events/:profileId/:eventId. Per-profile error strips + all-failed state follow the Monitors/Events semantics. v1 scope note: the All-mode aggregate skips live-mode notification injection and the per-monitor cause-filter fan-out (single-profile-only features) - see useScopedTimelineEvents' doc comment. formatAppDate/useDateTimeFormat gain an optional timeZone parameter (Date-time contract) for converting a true-instant Date into an owning profile's wall clock; ended up unused by this task's actual surfaces, since every event-timestamp display already round-trips the server's own wall-clock string unchanged. Kept for the one genuine need found (Timeline's popover) via formatForServerInTz instead - flagging for review in case the unused parameter should be trimmed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
MonitorCard's events button, MonitorRecentEvents' CompactEventRow event links, and EventDetail's "all events for this monitor" button now carry the owning profileId (query param or /all/ deep route) instead of switch-then-navigate or a bare monitor id. MonitorCard's events button in particular drops its profile switch entirely: the aggregated Events page (previous commit) reads `?profileId=` to focus its server filter on the right server, so no switch is needed before navigating, matching the deep-route detail view's existing behavior. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
) 1. useScopedEvents/useScopedTimelineEvents were falling back to 'UTC' for a timezone-less profile's date-range filter conversion; formatForServer (the single-mode path they replaced) falls back to the BROWSER zone, so a timezone-less profile's query window silently shifted once a page switched to the scoped hook. New resolveProfileTimezone() (lib/time.ts) shares the browser-zone fallback between formatForServer and both scoped hooks' date-filter conversion; the eventInstant/sort fallback deliberately keeps its own 'UTC' convention (matches getSession, a stable sort key rather than a user-facing query window) and is untouched. 2. Events page montage view in All mode showed broken tiles for every event (portalUrl/token from the null current profile - EventMontageView has no per-tile owning-profile wiring, unlike EventListView's EventItem). Gated off for v1: the toggle is disabled with a localized tooltip (events.montage_unavailable_all_mode, all 5 locales, data-testid events-montage-gate), and the render branch itself now checks a derived effectiveViewMode (isAllMode forces 'list') so a stale eventsViewMode='montage' setting or a `?view=montage` deep link can't bypass the disabled toggle either. 3. Removed the timeZone parameter from formatAppDate/formatAppTime/ formatAppTimeShort/formatAppDateTime/formatAppDateTimeShort/ useDateTimeFormat - no caller ever passed it (confirmed by grep and tsc); dead speculative surface per C2. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds e2e coverage for the Phase 4 All-mode work: Montage tile aggregation with per-tile chips, the un-gated Events montage view, and the Logs page picker (verified via its per-profile session token, since both e2e profiles share one real test server and can't be told apart by content). Brings docs/user-guide/profiles.md and the Flow 21 close in call-flows.rst up to date with what actually aggregates now versus what still resolves a single picked profile or stays single-profile only. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
useNotificationStore's useShallow selector minted a fresh array/objects every call (flatMap+spread+sort inside the selector), which defeats useShallow's element-wise Object.is compare and drives useSyncExternalStore into an infinite render loop the moment >=1 notification exists, in both modes. Subscribe to the raw per-profile event arrays only and tag/merge/sort outside the subscription in a plain useMemo. Regression test renders against the real zustand store (a selector(state) test double bypasses useSyncExternalStore and cannot catch this class of bug).
TimelineWidget's hour/day buckets and HeatmapWidget/EventHeatmap's
density buckets used a naive local Date parse of the server wall-clock
StartDateTime string. Once All mode can merge events from more than
one profile/timezone, two events with the same wall-clock time but
different owning-profile timezones collapsed into the same bucket.
Both now carry { item, timezone } through combine and bucket via
eventInstant (already used by EventsWidget). Events.tsx's own
EventHeatmap call site is updated for the same shared-component
contract change; single mode is unaffected (one profile, one
timezone).
AskPanel's system-prompt version probe called getCurrentSession(),
which reads the store's globally-selected profile. Under the All-mode
ALL_PROFILES_ID sentinel there is no session for the sentinel itself,
so it threw every turn; the throw was swallowed by the surrounding
try/catch, silently dropping the ZM version from the system prompt.
Resolves via getSession(profileId) instead, matching every other read
in that block (the resolved pinned/current profile).
Montage: isEditMode was not scoped to isAllMode. Switching into All mode with it left on from single mode stranded the grid in edit mode with no way to turn it off (the toggle disables in All mode), so drag/resize handlers stayed live but no-op. Reset during render (React's "adjusting state when a prop changes" pattern, same idiom as LiveActivitySettingsDialog's useClampedNumberField) rather than in an Effect, which would paint one extra frame with the stale edit-mode UI. Montage: the kebab menu's hidden-monitors list regressed from the full monitor list to the group-filtered one. A monitor hidden while outside the active group filter became permanently un-hideable. Restored a separate, never-group-filtered `enabledMonitors` for the kebab. MonitorWidget: SingleMonitor built LiveMonitorPlayer without its profileId prop, so the go2rtc failure cache / MJPEG token resolution fell back to the globally-selected profile instead of the tile's owning one in All mode. One line per LiveMonitorPlayer call site (both branches - the test caught the first fix only covering one). EventsWidget: isAllMode was inferred from profiles.length > 1, which silently flips to single-mode behavior (no chips, no /all deep links) once a delete brings the All-mode scope down to one profile while mode is still 'all'. Uses scope?.mode === 'all' instead.
heatmapDateRange (the min/max start/end shown on the Events page
heatmap) still derived its window from a naive local Date parse of the
server wall-clock StartDateTime string, while the buckets it feeds
(EventHeatmap, fixed earlier) now use real chronological instants via
eventInstant. A naively-derived window can fall short of an instant
the buckets would otherwise place inside it, silently dropping that
event from the heatmap - the other half of the timezone-bucket fix,
not an independent bug.
Derives eventDates from heatmapEvents' { item, timezone } pairs via
eventInstant instead. Regression test goes through the real
heatmapDateRange computation (two profiles, different timezones, no
explicit date filter) rather than passing explicit start/end props
straight to EventHeatmap, which is what let the per-widget timezone
tests miss this.
Maintainer acceptance testing found NotificationSettings gave no signal that notifications are per-profile in All mode: the picker showed one profile's config with no view of the others. Adds a read-only overview card above the picker (name, enabled state, mode, host per profile; clicking a row drives the existing picker) and a single-mode caption clarifying the per-profile scope. Caught during e2e: a useShallow selector over an array of freshly-merged getProfileSettings() objects never stabilizes (each element is a new object reference), causing an infinite render loop. Fixed by subscribing to the raw settings record and reading through the stable getProfileSettings action instead.
) Round-1 fix wired NotificationOverview to a stable useNotificationStore subscription instead of the useShallow-over-array-of-getProfileSettings shape that caused a "Maximum update depth exceeded" loop. Unit tests passed anyway because the file's mocked store calls selector(state) directly, bypassing useSyncExternalStore entirely - the exact gap NotificationHistory.realstore.test.tsx documents for the sibling bug. Adds the same guard here: renders NotificationSettings in All mode against the real store, seeded via getState().updateProfileSettings. Confirmed it discriminates - temporarily reverting the selector to the buggy shape throws "Maximum update depth exceeded" during render (not just a swallowed console.error); restored, it's green.
A disabled profile stays listed on the Profiles page but can't be selected: switchProfile rejects it, and useProfileScope filters it out of every All-mode aggregate (monitors, events, montage, notifications, badges) since they all fan out over scope.profiles. The active profile can't be disabled (ProfileGuardError, surfaced as a toast); enabling is unconditional. ProfileSwitcher hides disabled profiles outright.
Moves the All Servers card to the end of the Profiles list (and the matching dropdown entry to the end of ProfileSwitcher) so it reads as the aggregate view rather than competing with real profiles at the top. Gives it a persistent primary-tinted accent (border/background, not just the active-state ring) and a localized note that aggregating every server can be resource-heavy.
Notification services (ZMNotificationService, EventPollerService) were module-level singletons and the store tracked one flat connectionState / isConnected / currentProfileId for "the" connected profile. All mode needs every enabled profile to hold its own live connection, so: - services/notifications.ts and services/eventPoller.ts become per-profile registries (Map<profileId, instance>), mirroring the sessions.ts pattern. - stores/notifications.ts tracks connections: Record<profileId, ConnectionState> instead of one flag; connect/disconnect/reconnect take a profileId. _initialize's onEvent/onStateChange callbacks bind their own profileId via closure instead of reading a shared "current" field, so an event from one profile's socket can never land in another's history once more than one profile is connected. - currentProfileId stays as the mobile push/badge anchor only (unchanged semantics) - never consulted to decide whether a *specific* profile is connected. - Every consumer (NotificationHandler, NotificationSettings, SidebarContent, the push/delivered hooks) updated to the new per-profile signatures; single mode behavior is unchanged (one profile, one connection, same call shapes). - deleteProfile/deleteAllProfiles now tear down that profile's (or every profile's) live connection and poller too - previously neither was ever stopped on profile deletion. This is groundwork only: nothing yet opens more than one connection at once.
In All mode, every profile whose own notification settings have it enabled now gets its own live connection (ES websocket, or direct-mode poller on desktop/web) instead of only the app's single "current" profile - so notifications and events keep arriving for every server while aggregating. - ProfileNotificationConnector: mounted once per All-mode scope profile by NotificationHandler. Reuses useNotificationAutoConnect unchanged, bound to its own profile's credentials and connections[] state; each instance is fixed to one profile for its whole lifetime, so React's own mount/unmount (keyed on profile.id) is the fan-out and teardown mechanism - no scope-fanning loop inside the hook itself. Unmounting (profile disabled, deleted, switched away, or All mode exited) disconnects that profile's ES connection and stops its poller without touching any other profile's. - Single mode is untouched: connectors only render when scope.mode is 'all', so the existing single-profile code path (and its test coverage) never changes. - NotificationOverview: each All-mode profile row now shows its live ES connection status (connected/connecting/error/disconnected), reusing the same status copy as the NotificationSettings detail page. - e2e: live websockets aren't reliably e2e-able against the shared test server, so this relies on unit/component coverage; the existing all-profiles.feature suite (10 scenarios) stays green.
Addendum to the multi-connection notification rework:
- Each event's toast/sound now honors its OWNING profile's showToasts/
playSound - never the app's "current" profile, which is null while
aggregating anyway. New useNotificationAllModeToasts hook, delegated from
NotificationHandler exactly like the other useNotification*.ts hooks;
single mode is untouched (the hook no-ops unless scope.mode is 'all').
- Burst coalescing: events arriving within NOTIFICATIONS_SERVICE.
allModeBurstWindowMs (3s) of each other collapse into one localized
summary toast ("N new events across M servers", tap-through to the
aggregated /events page) instead of one toast per event; a lone event in
the window still gets the normal per-event toast. At most one
notification sound plays per window.
- New All-mode mute toggle (ProfileSettings.allModeMuteToasts, ALL-bucket
setting via the existing mergeProfileSettings machinery, default false):
suppresses toasts+sound entirely while aggregating; badge counts and
history are untouched (addEvent always runs regardless). Control lives on
the NotificationSettings page, above the picker, in All mode only.
- playNotificationSound extracted to lib/event/notification-sound.ts so the
new hook and NotificationHandler's existing single-mode toast share the
same tone instead of duplicating the Web Audio beep.
- Localized (en/de/es/fr/zh): mute toggle label+description, burst summary
message.
ponytail: All-mode toasts show no thumbnail (Bell icon only) - building one
would need a fresh access token for the OWNING profile rather than the
app's current one; add a per-profile token lookup if this is requested.
Review round 1 (needs-fixes) - critical and important findings: - C1 (critical): single-mode profile switch stopped disconnecting the outgoing websocket. isConnected became scoped to the NEW currentProfile after connections went per-profile, so it could never see whether the OLD (anchor) profile was still connected - the switch-path disconnect was dead code. Added isPreviousProfileConnected, derived from the store's own anchor (state.currentProfileId), as the value that gates it. Replaced the masking unit test with one driven through the real selector shape, and added a real-store integration test (useNotificationAutoConnect. realstore.test.tsx): connect A, switch to B, assert A's disconnect fires and connections[A] clears. - I2: ProfileNotificationConnector never subscribed to the raw profileSettings slice, only the stable getProfileSettings action - enabling notifications for a profile already mounted in All mode never re-rendered it, so it never connected. Added the subscription. - I3: the ES auto-connect 500ms setTimeout was never cancelled; unmounting mid-delay (or mid-getDecryptedPassword) could still connect a socket nobody would ever disconnect. Added clearTimeout + a cancelled flag checked after every await. - I4: connectors are now gated to desktop/web only (Platform. isDesktopOrWeb) - mobile keeps today's deterministic single-connection + FCM-anchor semantics; FCM already delivers every profile's events server-side, so N extra websockets would be pure battery cost. Also: connect() only anchors currentProfileId (the mobile push/badge bookkeeping field) when profileId is the app's real current profile (single mode); in All mode the anchor keeps its pre-All value instead of racing to "whichever connector connected last". - I5: deleteProfile/deleteAllProfiles now route notification teardown through the store's disconnect()/disconnectAll() (dynamic import to avoid the static cycle stores/notifications.ts already has back to this store), not the service registry directly, so connections[]/the anchor stay consistent. Added stopEventPoller(id), which evicts the registry entry and is a no-op for a profile that never had a poller - unlike getEventPoller(id).stop(), it never creates a phantom entry just to immediately stop it. Routed every teardown-only getEventPoller() call site (connector unmount, auto-connect cleanup, ES/direct mode switch) through it - same bug class, same shared fix. - #6: the All-mode burst timer is now cleared on unmount. - #7: a profile's pre-existing/persisted "latest" event is seeded on first observation instead of toasted - no stale-event summary toast at launch. - #9: the burst summary's count param renamed count -> eventCount (count is an i18next-reserved pluralization key). - #10: _initialize(profileId) now cleans up any previous registration for that profile before re-subscribing, so a retried connect() can't leak duplicate onEvent/onStateChange listeners on the service instance. - #11: the mute test now asserts badgeCount, not just event count. - #12: a profile with enabled=false never toasts either, even if showToasts is on - symmetric with the connection resource guard. - #8 (ledger, not fixed): same-batch multiple events for one profile still count as one entry toward the burst's server count. Known, not addressed this round.
Round 2 review findings: - Critical: the ES auto-connect effect depended on the whole currentProfile object. Bootstrap routinely writes a new profile object with the same id (e.g. after a token refresh) - routine on every switch/cold-start. That identity churn re-ran the effect; combined with the round-1 I3 fix's clearTimeout cleanup, the re-run cancelled the pending timer, then hit the hasAttemptedAutoConnect guard (still true) and returned without rescheduling - ES never connected for the rest of the session. Fixed both ways: the effect now reads currentProfile through an always-fresh ref and depends on its primitive fields (id/username/password/portalUrl) instead of the object itself, so identity-only churn doesn't re-run it at all; its cleanup also resets hasAttemptedAutoConnect as a backstop so any future re-run can still reschedule instead of permanently stalling. - Minor: connect() resolving after the user switched to a different real profile (single mode) left the old profile's websocket fully connected and anchor-less - the switch-teardown effect had already run and found it not-yet-connected. connect() now disconnects itself immediately when the app's real current profile no longer matches (excluding All mode's sentinel and the no-current-profile-yet case). - Minor: disconnectAll() only iterated the store's connections map, missing any service instance that existed in the registry without ever appearing there (e.g. a lazily created instance from a checkAlive() call). It now also sweeps the registry directly via resetAllNotificationServices, which also un-deadens that export.
The ES auto-connect effect depended on isConnected/connectionState. Any connection-state churn - including a routine drop - re-ran it, and combined with round 2's cleanup reset (hasAttemptedAutoConnect.current = false), that re-armed a second auto-connect attempt on top of the service's own exponential backoff (refs #274): duplicating it at best, flattening the backoff ladder at worst. This effect's only job is the ONE initial auto-connect attempt for a newly loaded/enabled profile - recovery after that belongs to the service alone (and to the network/visibility/app-resume listeners already in this hook, which call reconnect() deliberately). Removed isConnected/connectionState from its dependency array; the guard now reads live connection state straight from the store instead of the (no-longer-reactive) props, costing nothing since attemptConnect already re-checks the same store immediately before connecting. Belt and braces: ZMNotificationService.connect() now clears any armed backoff timer before proceeding. A manual/hook-triggered connect() can land while state sits at 'disconnected' between backoff attempts - the already-connected/connecting guard doesn't catch that - so without this, a manual connect() and a stale backoff timer could each produce their own socket. connectionState was left with no remaining callers inside the hook after this change; removed it from AutoConnectParams and both call sites (NotificationHandler, ProfileNotificationConnector) rather than leave it as dead-code plumbing.
Upgrades the All-mode mute toggle to a three-state setting. `allModeMuteToasts` (boolean) is replaced by `allModeNotifications: 'live' | 'muted' | 'off'`, migrated inside mergeProfileSettings (legacy true -> muted, false/absent/ invalid -> live). 'muted' keeps today's semantics unchanged (connections run, toasts/sound suppressed, badge/history still accumulate). 'off' gates ProfileNotificationConnector's render site in NotificationHandler alongside the desktop/web platform check, so no connector mounts and zero All-mode websockets/pollers exist; mobile FCM is untouched. UI: the Switch on NotificationSettings becomes a Select (data-testid="all-mode-notifications- select"), localized across all 5 locales.
useScopedAlarmStates fans getAlarmStatus out per (profile, monitor) pair via each pair's OWNING session, keyed by monitorCacheKey so two servers sharing a raw monitor id never collide - the All-mode counterpart to useAlarmStates. capWatchedRoundRobin caps a watched set evenly across profiles instead of truncating in profile order. New LIVE_ACTIVITY constants (allModeMaxWatched, allModePollFloorSeconds) bound the fan-out and its poll rate.
Live Activity aggregates every scope profile instead of gating All mode out. useLiveActivityAllMode assembles the watched set (each profile's OWN ignore list, never the shared ALL bucket), the capped/floored alarm fanout, and live-hint causes, all keyed by monitorCacheKey so two servers sharing a raw monitor id stay distinct tiles. The dwell/damping engine (reduceActiveMonitors etc.) is unchanged and generic over its state keys, so single mode keeps its byte-identical bare-id path while All mode feeds it composite keys. Tiles carry a profile chip and stream from their owning profile via MontageMonitor's existing profileId support. Body JSX extracted into LiveActivityGridBody (C2). Removed the now-unused all_mode_title/description locale keys; added watch_cap_overflow across all 5 locales for the new watched-set cap notice.
capWatchedRoundRobin gains a `resident` option: keys currently on screen and mid-alarm are pulled out and always kept before the round-robin spends its remaining budget on everyone else. Without this, a monitor- list or ignore-list change that re-slices the watched set could evict a tile still alarming right now with no dwell window at all - the #313 failure mode (CMD_QUIT + remount thrash), reached through the cap instead of the poll. useLiveActivityAllMode threads the page's damping-engine `active` list in as the exemption set (a reactive value, not a ref - react-hooks/refs forbids reading ref.current during a memo). Also: sanitize the composite monitorId in LiveActivityTile's viewTransitionName (a colon terminates a CSS custom-ident, silently dropping the transition); a defensive pairs[i] guard in useScopedAlarmStates' combine; and two comments that mis-attributed a pattern to useNotificationAllModeToasts instead of this page's own single-mode recentCauses selector, now carrying its ponytail caveat too.
AppLayout.tsx's route-memory effect wrote via updateProfileSettings(currentProfile.id, ...), gated on currentProfile - always null in All mode, so no page was ever remembered while aggregating (and the "Entering X View" banner log never fired there either, same accidental scope). New pure resolveLastRouteSaveTarget (lib/navigation.ts) keeps the same exclusion rules (setup/profile routes, notification-opened pages) and adds: saves to ALL_PROFILES_ID in All mode instead of returning null. AppLayout's effect now gates on `scope` (resolves in both modes) rather than currentProfile?.id. App.tsx needed no change: its root Navigate already reads lastRoute from useCurrentProfile().settings, which already resolves against the ALL bucket in All mode. Lint ratchet baseline lowered 208 -> 207 (react-hooks/exhaustive-deps 35 -> 34): adding location.state to the effect's deps, needed for the fix, also happened to close a pre-existing gap.
Three Live Activity menu buttons were silently inert in All mode, all
via the same root cause: writes gated on currentProfile?.id, which is
null there.
- useFullscreenMode (shared with Montage): takes profileId (the raw
currentProfileId - real profile id in single mode, ALL_PROFILES_ID in
All mode) instead of a Profile object, so All mode has a bucket to
write to. Fixes Montage's own fullscreen toggle in All mode as a
side effect of the shared hook.
- LiveActivity.tsx's grid-column handler now keys off currentProfileId
the same way, matching Montage's established pattern.
- The settings dialog now renders in All mode
({currentProfileId && ...} instead of {currentProfile && ...}), with
a two-tier split inside (AGENTS.project.md's Aggregation contract):
poll/dwell/tiles stay view-level and read/write the ALL bucket; the
ignore list is a per-server data preference and now sits behind the
shared ProfilePicker, reading/writing whichever profile is picked.
Single mode passes no scopeProfiles, so the picker never renders and
the ignore list falls back to profileId directly, unchanged.
useLiveActivityAllMode gained monitorsByProfile (full, uncapped
per-profile lists) to feed the picker.
AnalysisFramesToggle is also disabled in All mode, but that is a
pre-existing limitation shared with Montage, not introduced or fixed
here - noted in the report rather than expanded into a third component.
e2e: two new all-profiles.feature scenarios (reload-restores-route,
settings+fullscreen in All mode), reusing existing steps that were
never actually gated to single mode.
…files run Adds the Aggregation and Notifications contracts, extends Stores with the render-loop selector rule, records domain facts and multi-agent workflow lessons per the self-improvement protocol, and lands the execution retrospective. Refs #337. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ions Aggregation contract's Gate line no longer claims mechanization the gate does not perform (M1); Notifications Path names the store that owns the closure binding; retrospective's ratchet figures and review-tier attribution corrected per review. Refs #337. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
LiveActivitySettingsDialog is page-mounted and Radix only toggles it open/closed, so it never unmounts. pickedProfileId was a mount-time snapshot with nothing to invalidate it: any in-app profile switch while the dialog had been opened before - single mode included, switching to a different single profile, not just an All-mode re-pick - left the ignore list reading and writing the stale profile's bucket forever after. ignoreTarget is now re-derived every render: pickedProfileId wins only while it's still a live member of scopeProfiles; otherwise it falls back to the current first scope profile, or `profileId` in single mode (which tracks the live prop automatically, since scopeProfiles is undefined there). Regression test rerenders with a changed profileId and confirms the write follows it - no existing test exercised that before. Also: corrected a navigation.test.ts comment that wrongly implied useProfileStore never reports the ALL_PROFILES_ID sentinel in All mode (it does - LiveActivity.tsx and Montage.tsx both read it directly); and folded useLiveActivityAllMode's monitorsById/monitorsByProfile into one memo, since both were already a separate pass over the same scopedMonitors list.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an All Servers mode: once a user has two or more profiles, a card on the Profiles screen lets them switch into a combined view that aggregates monitors from every profile at once instead of a single server. Phase 2 (below) landed the sentinel/scope/monitors aggregation; Phase 3 (below) adds aggregated Events/Timeline,
/all/deep routes that carry the owning profile, working new-event badges, and direct notification taps. This builds on the sentinel/session groundwork phase 0+1 landed in #338 and merges after it (base branch isfeat/all-profiles-sessions, notmain).Phase 2: All Servers mode with aggregated monitors
The 7 tasks
reLoginForfix — token refresh now targets the profile that actually needs it, not always the current one, fixing a cross-profile refresh bug the All-mode work depended on.useProfileScoperesolves "the active scope" as either one profile or every profile behind theALL_PROFILES_IDsentinel, with a single branch point so every consumer fans out overscope.profilesidentically in both modes.useScopedMonitorsfans profile queries out viauseQueries(same cache keyuseMonitorsalready uses), tags each monitor with its owning profile, staggers polling, and isolates a failed profile into its ownProfileErrorentry instead of failing the whole hook.monitor-profile-chipper card, the group-by-server toggle, and theprofile-error-stripfor a down server on the Monitors page.Phase 2 verification
npx bddgen+npm run test:e2e -- all-profiles.feature: 2/2 passed.npm run gates(vitest + build + three lint configs): 3333 passed | 2 skipped, build succeeded, lint ratchet baseline held (and improved:react-hooks/exhaustive-deps38 → 37).npm run test:e2e(full suite, run twice): first run 126 passed / 4 failed / 9 flaky-then-passed; second run 135 passed / 2 failed / 2 flaky-then-passed. Every failure across both runs was a pre-existing test unrelated to this branch (login timeouts under parallel-worker contention against the single real ZM test server, and two live-server-content-dependent tests — event-thumbnail hover preview and live-activity alarm state). Re-running each failing test in isolation (--workers=1) confirmed they pass on their own; none touch profile or monitor-card selectors, and this PR made no application-code changes at that point, only new tests and docs.all-profiles.featureandprofiles.featurepassed cleanly in both full runs.Phase 3: aggregated Events/Timeline,
/all/deep routes, notificationsThe 8 tasks
useScopedEvents— mirrorsuseScopedMonitorsfor events: one query per profile viauseQueries, merged and sorted by true absolute instant (eventInstant, deriving each event's epoch from its OWNING profile's timezone so cross-timezone servers interleave correctly), per-profile errors isolated the same way./all/deep routes —/all/monitors/:profileId/:monitorIdand/all/events/:profileId/:eventIdresolve their session from the route param instead of the (possibly absent) current profile.MonitorCardnavigates straight to these routes in All mode instead of switch-then-navigate;tryGetCurrentSession()added for UI-layer handlers that may render with no current profile.event-profile-chip, with per-profile error strips matching the Monitors pattern. The Events montage (grid) view is deliberately gated off in All mode for v1 (events-view-toggledisabled) rather than wired with broken per-tile thumbnails.useScopedMonitorNewEventsfans the per-monitor "new since last seen" query out per (profile, monitor) pair, and the watermark a card click marks belongs to that card's OWNING profile, not the globally-selected one./all/events/:profileId/:eventIdwith no profile-switch confirmation, since there's no "wrong" profile to switch away from while in All mode.reLoginstore action was deleted (zero production callers once Phase 2's fix wave landed).@webscenarios (merged events with chips, deep-link into a monitor without a profile switch), a user-guide rewrite for the now-aggregated screens, and a new call-flow trace (Flow 22) for the merged-events/direct-tap-through journey.Phase 3 verification
npm run gates: 283 test files passed | 1 skipped, 3435 tests passed | 2 skipped, build succeeded,lint:a11y/lint:correctnessclean,lint:ratchetbacklog within baseline (210 problems, unchanged).npx bddgen+npm run test:e2e -- all-profiles.feature: passed 4/4 (see the final-review fix wave below — the first live run against a reachable network caught and fixed two e2e timing assertions in this feature).npm run test:e2e: see the final-review fix wave below for the complete run.Phase 3 final-review fix wave (post-review hardening)
A dedicated review pass after Phase 3 landed found 11 findings (1 critical, 9 important, 1 informational) plus, once live e2e access came back, 2 further e2e-only issues surfaced by the first real run. All fixed test-first; nothing here changes Phase 3's design, only closes gaps in it:
${profileId}:${monitorId}composite tokens) instead of the bare id.?profileId=/?view=montagedeep links wrote their one-tap context into persisted profile settings, permanently narrowing the user's saved filter/view. Made transient (this render only).A round-2 pass on the fix wave itself caught two further issues before they shipped: a composite monitor token leaking into event-detail prev/next navigation filters (stripped to the owning profile's bare ids before it reaches the API), and the
?view=montagefix racing a settings-sync effect on the same mount and silently reverting single-mode deep links back to the persisted view (sync effect now skips while the deep-link param is present).Final verification (fix wave + live e2e)
npx vitest run src/).lint:a11y/lint:correctness: clean.lint:ratchet: within baseline, no growth.npm run test:e2e -- all-profiles.feature: 4/4 passed. The first live run against a reachable network caught two genuine e2e timing bugs in the merged-events scenario (counting event cards during a partial render instead of the settled list, and a one-shot chip-diversity snapshot that could fire before the slower of two profiles' event queries had rendered) — both fixed withexpect.poll-based settle/diversity checks, nowaitForTimeout, no app-code changes.npm run test:e2e: 122 passed / 14 flaky-then-passed (retried once, passed) / 5 failed. All 5 failures are single-profileevents.featurefilter scenarios (date range, by monitor, favorites-only, archived-only, tag filter). Bisected against5a8cf654(the commit immediately before this branch's Phase 3 fix waves): all 5 fail identically on that pre-fix-wave baseline — same scenario names, same failure signature (a timeout on a filter-popover interaction). Confirmed pre-existing, not a regression from this branch; left as-is per the bisect evidence rather than papered over.Ledger: e2e infrastructure debt (not this branch's app code). The 5 pre-existing failures above, plus a same-day observation that re-running
events.featurein isolation produced more failures (10) than inside the full suite (5) — including plain navigation/login scenarios unrelated to filtering — point at flakiness fromplaywright.config.ts'sfullyParallel: true/workers: undefineddriving many concurrent browser contexts against one shared, live external ZM test server, worsening over repeated same-day runs (contention, session/rate-limit effects, or real server latency) rather than a deterministic app bug. Candidate fixes — capping worker concurrency for this project, standing up a dedicated e2e test server, or mocking the ZM API for filter-heavy scenarios — are a maintainer decision, not made here.Phase 4: Montage/Dashboard aggregation, event-montage un-gate, server pickers, assistant pinning
The 6 tasks
Montage.tsxrenders every profile's monitors in All mode viauseScopedMonitors(same hook the Monitors page uses), sections by server when the existing group-layout toggle is on,montage-profile-chipon every tile, per-profile error strips, and per-tile streams keyed by owning profile. A new All-mode-only stream cap (MONTAGE_GRID.allModeMaxStreams) bounds the total number of simultaneous live streams across every server, so a large combined camera count can't try to open a stream per camera per server at once. Single mode stays byte-identical.EventMontageViewresolves each tile's owning profile per-row exactly likeEventListViewalready does (portal URL, token, thumbnail chain), so colliding monitor/event ids across two servers render correctly. The event-preview popover, the timeline scrubber's hover thumbnail, and the scrubber's tap-to-open all carry the owning profile id through as well, so a scrubber tap on a colliding event id opens the right server's event.ProfilePicker(page-profile-picker, localized ×5) in All mode, defaulting to the first profile in scope; every query/action on that page targets the picked profile's session. Single mode: picker hidden, current profile used, byte-identical. Settings' view-level (ALL-bucket) sections stay editable regardless of the picker.assistant-pinned-banner, localized, with its ownProfilePickerto change it), so an answer about "the front door" stays attributable to a server.viewNameForPathgained the/all/monitors/...and/all/events/...deep-route patterns so the entry banner works on them.eventsServerFilter(and any otherProfileId[]setting) now reconciles against the live profiles list at its read site inEvents.tsx(not inmergeProfileSettings, which has no access to the live profile list without a store-to-store import cycle — commented in place). Events' "Showing X of Y" now reflects the active server filter, with a localized hint when every server is deselected.@webscenarios (Montage tile aggregation, un-gated Events montage view, Logs page picker verified via its per-profile session token), a user-guide rewrite for the now-aggregated screens, and a correction to Flow 21's closing note incall-flows.rst(it previously said Montage/Dashboard stayed single-profile, which Tasks 1 and 3 above made stale). Skipped a notification-badge e2e scenario — aggregation is already unit-tested, and a live-push-driven e2e was ruled out of scope per the brief.Phase 4 verification
npx bddgen && npm run test:e2e -- all-profiles.feature: 7/7 passed (4 from earlier phases + 3 new).npm run gates(vitest + build + three lint configs): 304 test files passed | 1 skipped (pre-existing, unrelated), build succeeded,lint:a11y/lint:correctnessclean,lint:ratchetbacklog within baseline (208 problems, unchanged).npm run test:e2e: 138 passed / 5 failed / 1 flaky-then-passed (144 total). All 5 failures are the same bisect-confirmed pre-existingevents.featurefilter scenarios from the Phase 3 ledger (date range, by monitor, favorites-only, archived-only, tag filter). The 1 flaky (montage.featurecolumn-count assertion) passed on retry. Zero new outright failures from this branch.Bug caught while writing the new e2e, fixed in the test only: the Montage tile-count locator (
[data-testid^="montage-monitor-"]) also matched the nestedmontage-monitor-mediaelement inside every tile, silently doubling every count. No app code was wrong — confirmed via a temporary debug dump that every tile in both profiles carried its chip correctly — the locator needed:not([data-testid="montage-monitor-media"]).Test plan
npx bddgen && npm run test:e2e -- all-profiles.feature(phase 2)npm run gates(phase 2 and phase 3)npm run test:e2e(full suite, phase 2)npx bddgen && npm run test:e2e -- all-profiles.feature(phase 3 fix wave — 4/4)npm run test:e2e(full suite, phase 3 fix wave — 122 passed / 14 flaky-then-passed / 5 pre-existing, bisect-confirmed against 5a8cf65)npx bddgen && npm run test:e2e -- all-profiles.feature(phase 4 — 7/7)npm run gates(phase 4 — 304 passed | 1 skipped, build clean, lints clean, ratchet baseline held)npm run test:e2e(full suite, phase 4 — 138 passed / 5 pre-existing / 1 flaky-then-passed)Claude assisting @pliablepixels
🤖 Generated with Claude Code
Feature-closing review (final)
Whole-branch Opus review of Phase 4 + spec-completeness audit: every spec UX item delivered except Live Activity aggregation (deliberately gated, tracked in #341). Closing fix wave (85ffc08..fc7cf65): notification-history render-loop Critical fixed with a real-store regression test, owning-timezone bucketing for dashboard charts and the heatmap range, pinned assistant version probe, montage edit-mode reset, and remaining All-mode nits. Final verification: 3509 unit tests green, build + 3 lints clean (ratchet net-improved across the phase), blast-radius e2e 46 passed / 0 new failures (the 5 documented pre-existing events-filter scenarios tracked in #342).
Post-close increments (maintainer acceptance testing)
Per-profile notification overview in All mode + real-store subscription guard; montage fullscreen button styling; per-profile DISABLE toggle (listed, unselectable, excluded from All mode); All Servers card moved last with accent + resource note.
Live notifications for every profile in All mode (web/Electron): per-profile connection registry (websockets + direct-mode pollers), connector-per-profile fan-out, per-owning-profile toast settings, burst coalescing (one summary toast + one sound per window), All-mode mute toggle, per-profile connection status in the overview. Desktop/web only by design: mobile keeps single-connection + FCM (which already delivers all registered profiles server-side). Four review rounds (Opus) closed 1 Critical socket-leak class, an auto-connect deadlock, a backoff-defeat regression, and 15 smaller findings; final verdict clean. 3571 unit tests, gates green, all-profiles e2e 10/10.
All-mode notifications setting upgraded to three states: Live / Muted (silent collection: badge+history, no popups) / Off (no connections at all); legacy mute boolean migrated in mergeProfileSettings. Reviewed clean; 3578 unit tests green.
All-mode Live Activity (closes the All mode: Live Activity aggregation (blocked on notification-store profile tagging) #341 gap): scoped alarm polling with a 24-monitor round-robin cap (resident tiles exempt from re-slices), 10s All-mode poll floor, per-profile live-hint promotion, per-profile ignore lists behind a picker in the settings dialog, All-mode screen memory (lastRoute in the ALL bucket), fullscreen/settings chrome unlocked. Three review rounds; final verdict clean.
Closeout: user/developer docs brought current; Aggregation/Notifications contracts + Stores render-loop rule added to AGENTS.project.md (word-budget gate enforced); execution retrospective at docs/superpowers/analysis/2026-08-04-all-profiles-retrospective.md.
Final: 77 commits across both branches, 3614 unit tests green, lint ratchet 38 at branch point to 34 at close, all-profiles e2e 12/12.