From e9a426aeebd52785097ed79edd18908433ce4db2 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Tue, 22 Sep 2026 09:43:00 +0100 Subject: [PATCH] fix(web): correlate PBS host history across guest and agent rows (#1723) A standalone source=pbs host row and the PVE guest carrying the same agent are one machine. Collapse correlation candidates that share an agent identity and prefer the guest metrics target, whose persisted host series the Backups drawer renders; the PBS service target names the service key and has no host history. Distinct agent identities, or rows without a provable identity, still decline to choose so the drawer cannot substitute an unrelated host. Re-compose the reviewed runtime change with its three subsystem contracts, offline browser fixture, check script and a content-addressed browser receipt in one governed commit. Change-source: pulse-maintainer --- .../subsystems/frontend-primitives.md | 5 +- .../internal/subsystems/storage-recovery.md | 5 +- .../internal/subsystems/unified-resources.md | 5 +- .../pbs-host-history-correlation.html | 10 ++ .../pbs-host-history-correlation.tsx | 80 ++++++++++++++++ frontend-modern/browser-verification.json | 40 ++++---- .../proxmox/ProxmoxBackupServersTable.tsx | 51 +++++++++- .../ProxmoxBackupServersTable.drawer.test.tsx | 57 +++++++++++ .../__tests__/ProxmoxBackupsTable.test.tsx | 8 +- .../check-pbs-host-history-correlation.cjs | 96 +++++++++++++++++++ 10 files changed, 328 insertions(+), 29 deletions(-) create mode 100644 frontend-modern/browser-tests/pbs-host-history-correlation.html create mode 100644 frontend-modern/browser-tests/pbs-host-history-correlation.tsx create mode 100644 scripts/check-pbs-host-history-correlation.cjs diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 4109f6d402..4743a5e169 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -627,8 +627,11 @@ that manual-channel consequence must stay visible at desktop and narrow widths. The Backups surface passes its complete deduplicated route inventory to the PBS table. Only PBS servers render as rows; other resources supply correlation inputs. Preserve PBS drawer identity and use the correlated canonical history target. Missing disk utilisation does not gate CPU/memory history. +One agent can be surfaced twice for a single PBS host: folded into its PVE guest and as a standalone `source=pbs` host row. Those two rows are one machine, not an ambiguous pair. Correlation must collapse candidates that share an agent identity and prefer the guest representation, whose canonical metrics target carries the persisted host history; the PBS service target has no host series and renders the collecting-history state. Two candidates with distinct agent identities remain ambiguous, and a candidate with no agent identity must not be treated as proof of sameness. + Verification: ProxmoxBackupServersTable.drawer.test.tsx covers standalone and -merged guest targets, missing disks and ambiguous identities; +merged guest targets, missing disks, duplicate guest/host representations of one +agent, and genuinely ambiguous identities; ProxmoxPageSurface.contract.test.tsx covers hydration and deduplication. diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index ee92aea499..e54a9dadc6 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -311,8 +311,11 @@ command-capable profile. Backups hydrates type=pbs,agent with source=pbs and reuses Overview guest inventory. Deduplicate combined snapshots by canonical ID before correlation, avoiding false ambiguity for agents in both queries. Standalone non-PVE PBS telemetry remains available without downloading the guest estate twice. +The same agent can still arrive as two distinct resources: a PVE guest carrying its telemetry and the standalone `source=pbs` host row. Collapse those candidates by agent identity and resolve the Backups PBS row to the guest target, whose persisted host series the drawer charts; keeping the PBS service target leaves History on a key with no host data. Distinct agent identities stay ambiguous, and a missing agent identity is not sameness proof. Do not add a second guest-estate request to compensate. + Verification: ProxmoxBackupServersTable.drawer.test.tsx covers standalone and -merged guest targets, missing disks and ambiguous identities; +merged guest targets, missing disks, duplicate guest/host representations of one +agent, and genuinely ambiguous identities; ProxmoxPageSurface.contract.test.tsx covers hydration and deduplication. diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index a14354e1bc..e6cf52d39a 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -456,8 +456,11 @@ about the same disk cannot diverge. PBS presentation correlation accepts standalone agents and VM/system-container resources with an agent facet. Preserve unique identity matching and canonical metricsTarget type and ID. Guests without telemetry and mixed ambiguous agent/guest matches must not replace the PBS target. +A single agent surfaced as both a VM/system-container guest and a standalone agent row is one identity, not an ambiguous pair. Collapse candidates that share an agent identity before deciding, and prefer the guest's canonical metricsTarget because it names the persisted host series; the PBS service target names the service key and has no host history. Only distinct agent identities, or candidates whose identity cannot be proven equal, decline to choose. Never use input order or a fallback that discards the PBS row id. + Verification: ProxmoxBackupServersTable.drawer.test.tsx covers standalone and -merged guest targets, missing disks and ambiguous identities; +merged guest targets, missing disks, duplicate guest/host representations of one +agent, and genuinely ambiguous identities; ProxmoxPageSurface.contract.test.tsx covers hydration and deduplication. diff --git a/frontend-modern/browser-tests/pbs-host-history-correlation.html b/frontend-modern/browser-tests/pbs-host-history-correlation.html new file mode 100644 index 0000000000..72720ed0d1 --- /dev/null +++ b/frontend-modern/browser-tests/pbs-host-history-correlation.html @@ -0,0 +1,10 @@ + + + + + + +
+ + + diff --git a/frontend-modern/browser-tests/pbs-host-history-correlation.tsx b/frontend-modern/browser-tests/pbs-host-history-correlation.tsx new file mode 100644 index 0000000000..dc7f87f460 --- /dev/null +++ b/frontend-modern/browser-tests/pbs-host-history-correlation.tsx @@ -0,0 +1,80 @@ +// Browser fixture: a Proxmox Backups PBS host whose Pulse agent is surfaced +// both as a PVE guest (with agent telemetry) and as a standalone source=pbs +// host row. Reproduces #1723: before the fix the two identity matches were +// treated as ambiguous, so the PBS row kept its service metrics target and the +// History tab showed "Collecting history" for a host that has history. +// Synthetic props only; the check script intercepts the metrics-history request. +import { render } from 'solid-js/web'; + +import { ProxmoxBackupServersTable } from '../src/features/proxmox/ProxmoxBackupServersTable'; +import type { Resource } from '../src/types/resource'; +import '../src/index.css'; + +const sharedAgent = { agentId: 'agent-proxback', hostname: 'proxback-vm' }; + +const pbs = { + id: 'pbs-1', + type: 'pbs', + name: 'proxback', + displayName: 'proxback', + platformId: 'pbs-1', + platformType: 'proxmox-pbs', + sourceType: 'hybrid', + sources: ['pbs'], + status: 'online', + lastSeen: Date.now(), + cpu: { current: 4.7 }, + memory: { current: 20.3, total: 8000, used: 1624, free: 6376 }, + pbs: { + instanceId: 'proxback', + hostname: 'proxback-vm', + version: '3.2.1', + connectionHealth: 'healthy', + datastores: [{ name: 'tank', total: 1000, used: 400, available: 600, usagePercent: 40 }], + }, + // The PBS service target: correct for the service, but it has no host series. + metricsTarget: { resourceType: 'agent', resourceId: 'pbs-1' }, + platformData: { + sources: ['pbs'], + pbs: { instanceId: 'proxback', hostname: 'proxback-vm', datastoreCount: 1 }, + }, +} as unknown as Resource; + +const guest = { + id: 'vm-100', + type: 'vm', + name: 'proxback-vm', + displayName: 'proxback-vm', + platformId: 'proxmox:100', + platformType: 'proxmox-pve', + sourceType: 'hybrid', + sources: ['proxmox', 'agent'], + status: 'online', + lastSeen: Date.now(), + cpu: { current: 15.4 }, + memory: { current: 16.9, total: 8000, used: 1352, free: 6648 }, + agent: sharedAgent, + metricsTarget: { resourceType: 'vm', resourceId: 'proxmox:100' }, + platformData: { sources: ['proxmox', 'agent'], agent: sharedAgent }, +} as unknown as Resource; + +const standalone = { + id: 'agent-proxback', + type: 'agent', + name: 'proxback-vm', + displayName: 'proxback-vm', + platformId: 'agent-proxback', + platformType: 'proxmox-pbs', + sourceType: 'hybrid', + sources: ['agent', 'pbs'], + status: 'online', + lastSeen: Date.now(), + agent: sharedAgent, + metricsTarget: { resourceType: 'agent', resourceId: 'agent-proxback' }, + platformData: { sources: ['agent', 'pbs'], agent: sharedAgent }, +} as unknown as Resource; + +render( + () => , + document.getElementById('root') as HTMLElement, +); diff --git a/frontend-modern/browser-verification.json b/frontend-modern/browser-verification.json index 4d84052c9e..19d90ae206 100644 --- a/frontend-modern/browser-verification.json +++ b/frontend-modern/browser-verification.json @@ -1,38 +1,42 @@ { "version": 1, - "base_sha": "eb47a4e653581661d4b267710d9c1bb60a904733", - "verified_at": "2026-09-21T14:31:00Z", + "base_sha": "c3213510e6719d486b46fd226b7a208cff375ebd", + "verified_at": "2026-09-22T03:24:00Z", "result": "passed", "changed_paths": [ - "frontend-modern/src/utils/alertOverviewPresentation.ts" + "frontend-modern/src/features/proxmox/ProxmoxBackupServersTable.tsx" ], "content_sha256": { - "frontend-modern/src/utils/alertOverviewPresentation.ts": "7f68a70800deb0cb8214d4fd25c7454c5b0fde1084e08365eb93bc361b80846a" + "frontend-modern/src/features/proxmox/ProxmoxBackupServersTable.tsx": "18535683c37f5b5a481ac012a49b7dcdd6264ac992076381c74fdfbb037e31eb" }, "routes": [ - "/browser-tests/alert-card-footer.html" + "/proxmox/backups (production surface represented by the component fixture)", + "/browser-tests/pbs-host-history-correlation.html" ], "viewports": [ { - "width": 2010, - "height": 1072 + "width": 1280, + "height": 900 }, { - "width": 480, - "height": 800 + "width": 390, + "height": 844 } ], "states": [ - "Alerts overview alert-card footer with the Started timestamp and the adjacent delivery-status run", - "The Started run classed by getAlertOverviewStartedAtClass() inside the items-center footer flex row", - "Desktop 2010x1072 and narrow 480x800 layouts" + "The Backups PBS server row renders for a host whose agent is surfaced as both a PVE guest and a standalone source=pbs host row", + "Expanding the row opens the canonical drawer; History is selectable and renders the host metric groups", + "The drawer's merged header shows the guest's current CPU/memory (15.4% / 16.9%) rather than the PBS service values, confirming the guest presentation is merged", + "The History request resolves to the guest series vm/proxmox:100; the PBS service target agent/pbs-1 is never requested", + "The same correlation holds at the narrow 390px layout" ], "interactions": [ - "Mount the production footer markup from AlertOverviewAlertCard.tsx with the class returned by getAlertOverviewStartedAtClass() and the project CSS", - "Measure the text bounding rects of the Started run and the status run and assert their top and bottom edges match within 1px", - "Before the fix the same run reported the Started run 2px below the status run (started top 70 / bottom 86 vs status top 68 / bottom 84); after the fix both report top 66 / bottom 82 at both viewports", - "Inspect the desktop and narrow screenshots" + "Mount the production ProxmoxBackupServersTable with synthetic PBS server, PVE guest and standalone agent resources sharing one agent identity", + "Focus the row disclosure button and activate it with Enter, then select the History subtab", + "Intercept /api/metrics-store/history requests and assert the requested resourceType/resourceId pair", + "Repeat at 1280x900 and 390x844 and capture a screenshot of each rendered History tab", + "Record Chromium from the offline Playwright runtime with no uncaught page errors" ], - "command": "pulse-worker-browser scripts/check-alert-card-footer.cjs", - "notes": "Offline Playwright Chromium 141.0.7390.37 run of the production Alerts overview alert-card footer markup with the class returned by getAlertOverviewStartedAtClass() and the project CSS; no backend, API or WebSocket path. The footer is an items-center flex row that already carries the top margin, so the child 'mt-1' pushed the Started run 2px below the delivery-status span; this matches the two runs the reporter underlined at different heights in #2119. Removing the child margin aligns both runs (top 66 / bottom 82 at desktop and narrow, delta 0). No page errors and no failed responses. This receipt is content-addressed to alertOverviewPresentation.ts sha256 7f68a708…." + "command": "pulse-worker-browser scripts/check-pbs-host-history-correlation.cjs", + "notes": "Offline Playwright Chromium run of the production ProxmoxBackupServersTable component, not a live Pulse. The fixture feeds synthetic resources only and the check script answers /api/metrics-store/history with empty points, so the panels correctly show the collecting-history state; the assertion is the requested history target, which in production carries the persisted host series. No backend, WebSocket or credential path is exercised. This receipt is content-addressed to the changed frontend source at this candidate base." } diff --git a/frontend-modern/src/features/proxmox/ProxmoxBackupServersTable.tsx b/frontend-modern/src/features/proxmox/ProxmoxBackupServersTable.tsx index 5850ec5b7b..1cd03c2d1f 100644 --- a/frontend-modern/src/features/proxmox/ProxmoxBackupServersTable.tsx +++ b/frontend-modern/src/features/proxmox/ProxmoxBackupServersTable.tsx @@ -126,6 +126,24 @@ const stringValues = (...candidates: unknown[]): string[] => : [], ); +const isGuestWithAgent = (resource: Resource): boolean => + (resource.type === 'vm' || resource.type === 'system-container') && + Boolean(resource.agent ?? resource.platformData?.agent); + +// The agent identity is the same on a PVE guest and on the standalone host row +// that represents the same machine. It is the only reliable way to tell one +// host surfaced twice from two genuinely different hosts sharing a name. +const correlatedAgentKey = (resource: Resource): string | undefined => { + const direct = resource.agent?.agentId?.trim(); + if (direct) return direct; + const platformAgent = resource.platformData?.agent; + if (platformAgent && typeof platformAgent === 'object') { + const agentId = (platformAgent as { agentId?: unknown }).agentId; + if (typeof agentId === 'string' && agentId.trim()) return agentId.trim(); + } + return undefined; +}; + const uniquelyCorrelatedAgent = ( server: Resource, candidates: readonly Resource[], @@ -135,16 +153,39 @@ const uniquelyCorrelatedAgent = ( const matches = candidates.filter((candidate) => { // Host telemetry can be merged into a PVE guest rather than a standalone // agent. Keep the unique-identity check and require an actual agent facet. - const guestWithAgent = - (candidate.type === 'vm' || candidate.type === 'system-container') && - Boolean(candidate.agent ?? candidate.platformData?.agent); - if (candidate.type !== 'agent' && !guestWithAgent) return false; + if (candidate.type !== 'agent' && !isGuestWithAgent(candidate)) return false; for (const token of identityTokens(candidate)) { if (serverTokens.has(token)) return true; } return false; }); - return matches.length === 1 ? matches[0] : undefined; + if (matches.length === 0) return undefined; + if (matches.length === 1) return matches[0]; + + // A single agent can surface twice: folded into its PVE guest and as the + // standalone host row. Those are one host, not an ambiguous pair. Collapse by + // agent identity and prefer the guest, whose metrics target carries the + // persisted history the Backups drawer renders. + const byAgentKey = new Map(); + for (const match of matches) { + const key = correlatedAgentKey(match); + // Without an agent identity we cannot prove the rows are the same host, so + // stay conservative and decline to guess. + if (!key) return undefined; + const bucket = byAgentKey.get(key); + if (bucket) { + bucket.push(match); + } else { + byAgentKey.set(key, [match]); + } + } + if (byAgentKey.size !== 1) return undefined; + const group = Array.from(byAgentKey.values())[0]; + return ( + group.find((match) => isGuestWithAgent(match) && match.metricsTarget) ?? + group.find((match) => match.metricsTarget) ?? + group[0] + ); }; const mergePBSAgentPresentation = (server: Resource, agent: Resource): Resource => { diff --git a/frontend-modern/src/features/proxmox/__tests__/ProxmoxBackupServersTable.drawer.test.tsx b/frontend-modern/src/features/proxmox/__tests__/ProxmoxBackupServersTable.drawer.test.tsx index 6efedf95e3..7d3282f1bf 100644 --- a/frontend-modern/src/features/proxmox/__tests__/ProxmoxBackupServersTable.drawer.test.tsx +++ b/frontend-modern/src/features/proxmox/__tests__/ProxmoxBackupServersTable.drawer.test.tsx @@ -194,4 +194,61 @@ describe('ProxmoxBackupServersTable details', () => { ); }, ); + + it('reuses the PVE guest when the same agent is listed as both guest and standalone host', () => { + const pbs = makePbsResource(); + pbs.sources = ['pbs']; + pbs.agent = undefined; + pbs.name = 'proxback'; + pbs.displayName = 'proxback'; + pbs.platformId = 'pbs-1'; + pbs.pbs = { ...pbs.pbs!, instanceId: 'proxback', hostname: 'proxback-vm' }; + pbs.metricsTarget = { resourceType: 'agent', resourceId: 'pbs-1' }; + pbs.platformData = { + sources: ['pbs'], + pbs: { instanceId: 'proxback', hostname: 'proxback-vm', datastoreCount: 1 }, + }; + const sharedAgent = { agentId: 'agent-proxback', hostname: 'proxback-vm' }; + const guest = { + id: 'vm-100', + type: 'vm', + name: 'proxback-vm', + displayName: 'proxback-vm', + platformId: 'proxmox:100', + platformType: 'proxmox-pve', + sourceType: 'hybrid', + sources: ['proxmox', 'agent'], + status: 'online', + lastSeen: pbs.lastSeen, + agent: sharedAgent, + metricsTarget: { resourceType: 'vm', resourceId: 'proxmox:100' }, + platformData: { sources: ['proxmox', 'agent'], agent: sharedAgent }, + } as Resource; + const standalone = { + id: 'agent-proxback', + type: 'agent', + name: 'proxback-vm', + displayName: 'proxback-vm', + platformId: 'agent-proxback', + platformType: 'proxmox-pbs', + sourceType: 'hybrid', + sources: ['agent', 'pbs'], + status: 'online', + lastSeen: pbs.lastSeen, + agent: sharedAgent, + metricsTarget: { resourceType: 'agent', resourceId: 'agent-proxback' }, + } as Resource; + + render(() => ); + fireEvent.click(screen.getByRole('button', { name: 'Expand details for proxback' })); + + expect(screen.getByTestId('pbs-resource-detail')).toHaveAttribute( + 'data-metrics-resource-type', + 'vm', + ); + expect(screen.getByTestId('pbs-resource-detail')).toHaveAttribute( + 'data-metrics-resource-id', + 'proxmox:100', + ); + }); }); diff --git a/frontend-modern/src/features/proxmox/__tests__/ProxmoxBackupsTable.test.tsx b/frontend-modern/src/features/proxmox/__tests__/ProxmoxBackupsTable.test.tsx index e31940899a..e612530ad7 100644 --- a/frontend-modern/src/features/proxmox/__tests__/ProxmoxBackupsTable.test.tsx +++ b/frontend-modern/src/features/proxmox/__tests__/ProxmoxBackupsTable.test.tsx @@ -585,9 +585,11 @@ describe('ProxmoxBackupsTable', () => { expect(proxmoxBackupServersTableSource).toContain('resource={row.resource}'); expect(proxmoxBackupServersTableSource).toContain('initialShowHostDetails'); expect(proxmoxBackupServersTableSource).toContain('uniquelyCorrelatedAgent'); - expect(proxmoxBackupServersTableSource).toContain( - 'return matches.length === 1 ? matches[0] : undefined;', - ); + // A single agent can surface as both a PVE guest and a standalone host row; + // collapse those by agent identity, and keep declining genuinely ambiguous + // matches. + expect(proxmoxBackupServersTableSource).toContain('correlatedAgentKey'); + expect(proxmoxBackupServersTableSource).toContain('if (byAgentKey.size !== 1) return undefined;'); expect(proxmoxBackupServersTableSource).toContain( 'metricsTarget: agent.metricsTarget ?? server.metricsTarget', ); diff --git a/scripts/check-pbs-host-history-correlation.cjs b/scripts/check-pbs-host-history-correlation.cjs new file mode 100644 index 0000000000..215a75a4c1 --- /dev/null +++ b/scripts/check-pbs-host-history-correlation.cjs @@ -0,0 +1,96 @@ +// Offline real-browser regression for #1723. +// +// Mounts the production ProxmoxBackupServersTable with a PBS host whose agent +// is present twice (PVE guest with agent telemetry, and a standalone source=pbs +// host row). Expands the PBS row, opens History, and records the metrics +// history target the drawer requests. The correct target is the guest's vm +// series; the PBS service target has no host history and leaves the tab on +// "Collecting history". Run with: +// pulse-worker-browser scripts/check-pbs-host-history-correlation.cjs +const path = require('node:path'); +const { chromium } = require('playwright'); + +const ROOT = path.resolve(process.cwd(), 'frontend-modern'); +const launchOptions = { headless: true, channel: 'chromium', args: ['--no-sandbox'] }; + +(async () => { + process.chdir(ROOT); + const { createServer } = await import( + path.join(ROOT, 'node_modules', 'vite', 'dist', 'node', 'index.js') + ); + const server = await createServer({ + root: ROOT, + configFile: path.join(ROOT, 'vite.config.ts'), + server: { host: '127.0.0.1', port: 5198, strictPort: true }, + }); + let browser; + const results = []; + const pageErrors = []; + const expected = 'vm/proxmox:100'; + const wrong = 'agent/pbs-1'; + try { + await server.listen(); + browser = await chromium.launch(launchOptions); + for (const viewport of [ + { width: 1280, height: 900 }, + { width: 390, height: 844 }, + ]) { + const targets = []; + const page = await browser.newPage({ viewport }); + page.on('pageerror', (error) => pageErrors.push(`${viewport.width}px: ${error.message}`)); + + await page.route('**/api/metrics-store/history?**', async (route) => { + const query = new URL(route.request().url()).searchParams; + targets.push(`${query.get('resourceType')}/${query.get('resourceId')}`); + await route.fulfill({ + json: { + resourceType: query.get('resourceType'), + resourceId: query.get('resourceId'), + points: [], + }, + }); + }); + + await page.goto('http://127.0.0.1:5198/browser-tests/pbs-host-history-correlation.html'); + const expand = page.getByRole('button', { + name: 'Expand details for proxback', + exact: true, + }); + await expand.waitFor({ timeout: 20000 }); + await expand.focus(); + await page.keyboard.press('Enter'); + await page.getByRole('tab', { name: 'History', exact: true }).click(); + // Give the history fetch a beat to fire. + await page.waitForTimeout(750); + + if (!targets.includes(expected)) { + throw new Error( + `${viewport.width}px: expected the guest history target ${expected}; recorded ${JSON.stringify(targets)}`, + ); + } + if (targets.includes(wrong)) { + throw new Error( + `${viewport.width}px: PBS service target ${wrong} was requested instead of the guest series`, + ); + } + + await page.screenshot({ + path: path.join(ROOT, 'browser-tests', `pbs-host-history-${viewport.width}.png`), + }); + results.push({ viewport, targets }); + await page.close(); + } + + if (pageErrors.length > 0) { + throw new Error(`page errors: ${pageErrors.join(' | ')}`); + } + + console.log(JSON.stringify({ result: 'passed', expected, results }, null, 2)); + } finally { + if (browser) await browser.close(); + await server.close(); + } +})().catch((error) => { + console.error('FAILED:', error && error.stack ? error.stack : error); + process.exitCode = 1; +});