Harden Workloads refresh stability

Prevent Workloads connection-ledger refreshes from tripping the app-level loading fallback, and suppress stale or duplicate auto-registration success notifications.

Add regression coverage for both refresh stability and replayed lifecycle events.
This commit is contained in:
rcourtman
2026-05-14 09:35:02 +01:00
parent 8554754f3e
commit bb77f42868
6 changed files with 211 additions and 25 deletions
@@ -276,6 +276,11 @@ profile and assignment columns, but embedded table framing must route through
nodes may emit `node_auto_registered`, while matched existing nodes that
rotate or refresh credentials must emit a non-toast configuration refresh
such as `nodes_changed`.
Browser consumers must treat `node_auto_registered` as a real-time,
timestamped lifecycle event rather than durable infrastructure state:
fresh first-time events may show one success toast, but replayed, stale, or
duplicate events must refresh configuration silently so an old registration
cannot present as a newly connected node.
Shared `internal/api/` session and auth changes consumed by lifecycle
routes must preserve durable principal IDs as the authorization key. Agent
lifecycle surfaces may display contact email when supplied by the shared
@@ -214,6 +214,11 @@ regression protection.
Workloads source-health messaging must derive from the canonical
`/api/connections` ledger through
`frontend-modern/src/components/Workloads/workloadInventorySourceIssues.ts`.
Connection-ledger refreshes on the Workloads page are steady-state health
probes, not route-loading authority: after the Workloads shell has mounted,
those refreshes must retain the last rendered table state and must not trip
the app-level Suspense fallback or blank the table while the next
`/api/connections` request is in flight.
The Workloads page may show a bounded partial-inventory banner when a
configured workload-capable source is unauthorized, unreachable, stale,
pending, or paused, but it must not fabricate VM/container rows from host
@@ -85,6 +85,9 @@ let wsConnected = true;
let wsInitialDataReceived = true;
let wsReconnecting = false;
let setWsConnectedSignal: ((next: boolean) => void) | null = null;
const connectionsApiMocks = vi.hoisted(() => ({
list: vi.fn(),
}));
const pushMockWorkloads = (next: Array<Record<string, unknown>>) => {
mockWorkloads = next;
@@ -176,7 +179,7 @@ vi.mock('@/hooks/useUnifiedResources', () => ({
vi.mock('@/api/connections', () => ({
ConnectionsAPI: {
list: vi.fn().mockResolvedValue({ connections: [], systems: [] }),
list: connectionsApiMocks.list,
},
}));
@@ -351,6 +354,8 @@ describe('Workloads performance contract', () => {
setMockWorkloadsSignal = null;
setWsConnectedSignal = null;
workloadsRefetchMock.mockReset();
connectionsApiMocks.list.mockReset();
connectionsApiMocks.list.mockResolvedValue({ connections: [], systems: [] });
guestRowMountCount = 0;
guestRowUnmountCount = 0;
wsConnected = true;
@@ -413,6 +418,23 @@ describe('Workloads performance contract', () => {
expect(document.body).not.toHaveTextContent('Attempting to reconnect…');
});
it('keeps workload rows visible while connection inventory refresh is still pending', async () => {
mockLocationSearch = '?type=all';
mockWorkloads = [makeGuest(1, { name: 'refresh-stable-workload' })];
connectionsApiMocks.list.mockImplementationOnce(() => new Promise(() => undefined));
render(() => <WorkloadsSurface vms={[]} containers={[]} nodes={[]} useWorkloads />);
await waitFor(() => {
expect(
document.querySelector('[data-testid="guest-row-refresh-stable-workload"]'),
).toBeInTheDocument();
});
expect(document.body).not.toHaveTextContent('Loading view...');
expect(document.body).not.toHaveTextContent('Loading...');
});
it('does not label an empty workload resource list as missing infrastructure when sources exist', async () => {
mockLocationSearch = '?type=all';
mockWorkloads = [];
@@ -664,6 +686,9 @@ describe('Workloads performance contract', () => {
expect(workloadsStateSource).toContain('useWorkloadsDerivedState');
expect(workloadsStateSource).toContain('useWorkloadRouteState');
expect(workloadsStateSource).toContain('buildWorkloadInventorySourceIssues');
expect(workloadsStateSource).toContain('createNonSuspendingQuery<ConnectionsListResponse');
expect(workloadsStateSource).toContain('connectionsSnapshot.refetch({ background: true })');
expect(workloadsStateSource).not.toContain('createResource<ConnectionsListResponse');
expect(workloadsStateCardsSource).toContain('workloadInventoryIssues');
expect(workloadInventorySourceIssuesSource).toContain('WORKLOAD_CAPABLE_TYPES');
expect(workloadInventorySourceIssuesSource).toContain('formatConnectionErrorMessage');
@@ -1,4 +1,4 @@
import { createEffect, createMemo, createResource, onCleanup } from 'solid-js';
import { createEffect, createMemo, onCleanup } from 'solid-js';
import { useNavigate } from '@solidjs/router';
import { ConnectionsAPI, type ConnectionsListResponse } from '@/api/connections';
import type { VM, Container, Node } from '@/types/api';
@@ -6,6 +6,7 @@ import type { WorkloadGuest } from '@/types/workloads';
import { useWebSocket } from '@/contexts/appRuntime';
import { useAlertsActivation } from '@/stores/alertsActivation';
import { usePersistentSignal } from '@/hooks/usePersistentSignal';
import { createNonSuspendingQuery } from '@/hooks/createNonSuspendingQuery';
import { useUnifiedResources } from '@/hooks/useUnifiedResources';
import { useWorkloads } from '@/hooks/useWorkloads';
import { useKioskMode } from '@/hooks/useKioskMode';
@@ -67,14 +68,12 @@ export function useWorkloadsState(props: WorkloadsSurfaceProps) {
enabled: workloadsEnabled,
});
const connectionsResourceKey = createMemo(() => (workloadsEnabled() ? 'enabled' : null));
const [connectionsSnapshot, { refetch: refetchConnectionsSnapshot }] =
createResource<ConnectionsListResponse, string | null>(
connectionsResourceKey,
async () => ConnectionsAPI.list(),
{
initialValue: EMPTY_CONNECTIONS_RESPONSE,
},
);
const connectionsSnapshot = createNonSuspendingQuery<ConnectionsListResponse, string>({
source: connectionsResourceKey,
fetcher: () => ConnectionsAPI.list(),
initialValue: EMPTY_CONNECTIONS_RESPONSE,
cacheKey: (key) => `workloads-connections:${key}`,
});
const dedupeGuests = (guests: WorkloadGuest[]): WorkloadGuest[] => {
const seen = new Set<string>();
@@ -176,7 +175,7 @@ export function useWorkloadsState(props: WorkloadsSurfaceProps) {
getWorkloadsDisconnectedState(reconnecting()),
);
const workloadInventoryIssues = createMemo(() =>
buildWorkloadInventorySourceIssues(connectionsSnapshot()?.connections ?? []),
buildWorkloadInventorySourceIssues(connectionsSnapshot.value().connections ?? []),
);
const hasWorkloadsData = createMemo(() => allGuests().length > 0);
const hasInfrastructureSources = createMemo(() =>
@@ -202,7 +201,7 @@ export function useWorkloadsState(props: WorkloadsSurfaceProps) {
const reconnectSurface = () => {
if (workloadsEnabled()) {
void workloads.refetch();
void refetchConnectionsSnapshot();
void connectionsSnapshot.refetch({ background: true });
}
reconnect();
};
@@ -210,7 +209,7 @@ export function useWorkloadsState(props: WorkloadsSurfaceProps) {
createEffect(() => {
if (!workloadsEnabled()) return;
const handle = window.setInterval(() => {
void refetchConnectionsSnapshot();
void connectionsSnapshot.refetch({ background: true });
}, WORKLOADS_CONNECTIONS_POLL_INTERVAL_MS);
onCleanup(() => window.clearInterval(handle));
});
@@ -1,6 +1,17 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { createRoot } from 'solid-js';
const notificationMocks = vi.hoisted(() => ({
success: vi.fn(),
error: vi.fn(),
info: vi.fn(),
warning: vi.fn(),
}));
vi.mock('@/stores/notifications', () => ({
notificationStore: notificationMocks,
}));
interface MockWebSocketInstance {
url: string;
readyState: number;
@@ -68,6 +79,8 @@ describe('websocket store resilience', () => {
autoOpenSockets = true;
currentInstance = null;
instances.length = 0;
vi.setSystemTime(new Date('2026-05-14T08:00:00.000Z'));
notificationMocks.success.mockClear();
installWebSocketMock();
});
@@ -148,4 +161,63 @@ describe('websocket store resilience', () => {
dispose();
}
});
it('shows auto-registration success once for a fresh event and suppresses replayed copies', async () => {
const { dispose } = await createStoreHarness();
try {
vi.advanceTimersByTime(1);
expect(currentInstance).not.toBeNull();
const event = {
type: 'node_auto_registered',
timestamp: '2026-05-14T08:00:00.000Z',
data: {
type: 'pve',
host: 'https://minipc:8006',
name: 'minipc',
nodeId: 'minipc',
tokenId: 'pulse-monitor@pve!pulse-minipc',
hasToken: true,
},
};
currentInstance!.onmessage?.({ data: JSON.stringify(event) } as MessageEvent);
currentInstance!.onmessage?.({ data: JSON.stringify(event) } as MessageEvent);
expect(notificationMocks.success).toHaveBeenCalledTimes(1);
expect(notificationMocks.success).toHaveBeenCalledWith(
'Proxmox VE node "minipc" was successfully auto-registered and is now being monitored!',
8000,
);
} finally {
dispose();
}
});
it('suppresses stale auto-registration success notifications', async () => {
const { dispose } = await createStoreHarness();
try {
vi.advanceTimersByTime(1);
expect(currentInstance).not.toBeNull();
currentInstance!.onmessage?.({
data: JSON.stringify({
type: 'node_auto_registered',
timestamp: '2026-05-14T07:50:00.000Z',
data: {
type: 'pve',
host: 'https://minipc:8006',
name: 'minipc',
nodeId: 'minipc',
tokenId: 'pulse-monitor@pve!pulse-minipc',
hasToken: true,
},
}),
} as MessageEvent);
expect(notificationMocks.success).not.toHaveBeenCalled();
} finally {
dispose();
}
});
});
+92 -12
View File
@@ -21,12 +21,89 @@ import {
import { mergeCanonicalResourceSnapshot } from '@/utils/resourceStateAdapters';
const MAX_INBOUND_WEBSOCKET_MESSAGE_BYTES = 8 * 1024 * 1024; // 8 MiB
const AUTO_REGISTER_NOTIFICATION_FRESH_MS = 2 * 60 * 1000;
const AUTO_REGISTER_NOTIFICATION_FUTURE_SKEW_MS = 30 * 1000;
const AUTO_REGISTER_NOTIFICATION_DEDUPE_MS = 10 * 60 * 1000;
type TimestampedWSMessage = WSMessage & { timestamp?: number | string };
type AutoRegisterNotificationPayload = {
type?: string;
host?: string;
name?: string;
nodeId?: string;
nodeName?: string;
timestamp?: number | string;
};
const shownAutoRegisterNotifications = new Map<string, number>();
const asRecord = (value: unknown): Record<string, unknown> | undefined =>
value && typeof value === 'object' ? (value as Record<string, unknown>) : undefined;
const asString = (value: unknown): string | undefined =>
typeof value === 'string' && value.trim().length > 0 ? value.trim() : undefined;
const parseTimestampMs = (value: unknown): number | null => {
if (typeof value === 'number' && Number.isFinite(value)) {
return value < 1_000_000_000_000 ? value * 1000 : value;
}
if (typeof value !== 'string' || value.trim().length === 0) {
return null;
}
const numeric = Number(value);
if (Number.isFinite(numeric)) {
return numeric < 1_000_000_000_000 ? numeric * 1000 : numeric;
}
const parsed = Date.parse(value);
return Number.isNaN(parsed) ? null : parsed;
};
const resolveMessageTimestampMs = (message: TimestampedWSMessage): number | null => {
const envelopeTimestamp = parseTimestampMs(message.timestamp);
if (envelopeTimestamp !== null) {
return envelopeTimestamp;
}
return parseTimestampMs(asRecord((message as { data?: unknown }).data)?.timestamp);
};
const buildAutoRegisterNotificationKey = (node: AutoRegisterNotificationPayload): string => {
const nodeIdentity = node.nodeId || node.nodeName || node.name || node.host || 'unknown';
return [node.type || 'unknown', nodeIdentity, node.host || ''].join('|');
};
const shouldShowAutoRegisterNotification = (
message: TimestampedWSMessage,
node: AutoRegisterNotificationPayload,
now = Date.now(),
): boolean => {
const eventTimestampMs = resolveMessageTimestampMs(message);
if (eventTimestampMs === null) {
return false;
}
if (
eventTimestampMs < now - AUTO_REGISTER_NOTIFICATION_FRESH_MS ||
eventTimestampMs > now + AUTO_REGISTER_NOTIFICATION_FUTURE_SKEW_MS
) {
return false;
}
for (const [key, shownAt] of shownAutoRegisterNotifications) {
if (now - shownAt > AUTO_REGISTER_NOTIFICATION_DEDUPE_MS) {
shownAutoRegisterNotifications.delete(key);
}
}
const key = buildAutoRegisterNotificationKey(node);
const previousShownAt = shownAutoRegisterNotifications.get(key);
if (
typeof previousShownAt === 'number' &&
now - previousShownAt <= AUTO_REGISTER_NOTIFICATION_DEDUPE_MS
) {
return false;
}
shownAutoRegisterNotifications.set(key, now);
return true;
};
// Type-safe WebSocket store
export function createWebSocketStore(url: string) {
let wsUrl = url;
@@ -336,7 +413,7 @@ export function createWebSocketStore(url: string) {
}
try {
const message: WSMessage = data;
const message = data as TimestampedWSMessage;
if (
message.type === WEBSOCKET.MESSAGE_TYPES.INITIAL_STATE ||
@@ -481,22 +558,25 @@ export function createWebSocketStore(url: string) {
setUpdateProgress(message.data);
logger.info('Update progress:', message.data);
} else if (message.type === 'node_auto_registered') {
// Node was successfully auto-registered
// Received node_auto_registered message
const node = message.data;
const nodeName = node.name || node.host;
const nodeType = node.type === 'pve' ? 'Proxmox VE' : 'Proxmox Backup Server';
notificationStore.success(
`${nodeType} node "${nodeName}" was successfully auto-registered and is now being monitored!`,
8000,
);
logger.info('Node auto-registered:', node);
if (shouldShowAutoRegisterNotification(message, node)) {
notificationStore.success(
`${nodeType} node "${nodeName}" was successfully auto-registered and is now being monitored!`,
8000,
);
eventBus.emit('node_auto_registered', node);
logger.info('Node auto-registered:', node);
} else {
logger.debug('Suppressed stale or duplicate node auto-registration notification', {
nodeName,
nodeType: node.type,
timestamp: message.timestamp,
});
}
// Emit event to trigger UI updates
eventBus.emit('node_auto_registered', node);
// Trigger a refresh of nodes
eventBus.emit('refresh_nodes');
} else if (message.type === 'node_deleted' || message.type === 'nodes_changed') {
// Nodes configuration has changed, refresh the list