From dab8daa9a71fe1f5afceb60a22188165f2bb3ad9 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Tue, 22 Sep 2026 12:11:59 +0100 Subject: [PATCH] fix(web): preserve PBS datastore identity across snapshot reordering Detach each rendered datastore from the nested PBS snapshot so independent row reconciliation cannot overwrite another datastore's identity. Cover switching and refreshed History in the component regression and real-browser fixture for #1723. Contract-Neutral: Restore existing PBS datastore row identity during snapshot reconciliation; no API or canonical ownership change. Change-source: pulse-maintainer --- .../pbs-host-history-correlation.tsx | 38 +++++- frontend-modern/browser-verification.json | 47 +++---- .../proxmox/ProxmoxBackupServersTable.tsx | 5 +- .../ProxmoxBackupServersTable.drawer.test.tsx | 29 ++++ .../check-pbs-host-history-correlation.cjs | 127 ++++++++++++++++-- 5 files changed, 193 insertions(+), 53 deletions(-) diff --git a/frontend-modern/browser-tests/pbs-host-history-correlation.tsx b/frontend-modern/browser-tests/pbs-host-history-correlation.tsx index dc7f87f46..8b787ae09 100644 --- a/frontend-modern/browser-tests/pbs-host-history-correlation.tsx +++ b/frontend-modern/browser-tests/pbs-host-history-correlation.tsx @@ -4,6 +4,7 @@ // 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 { createSignal } from 'solid-js'; import { render } from 'solid-js/web'; import { ProxmoxBackupServersTable } from '../src/features/proxmox/ProxmoxBackupServersTable'; @@ -30,13 +31,16 @@ const pbs = { hostname: 'proxback-vm', version: '3.2.1', connectionHealth: 'healthy', - datastores: [{ name: 'tank', total: 1000, used: 400, available: 600, usagePercent: 40 }], + datastores: [ + { name: 'tank', total: 1000, used: 400, available: 600, usagePercent: 40 }, + { name: 'archive', total: 2000, used: 600, available: 1400, usagePercent: 30 }, + ], }, // 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 }, + pbs: { instanceId: 'proxback', hostname: 'proxback-vm', datastoreCount: 2 }, }, } as unknown as Resource; @@ -74,7 +78,29 @@ const standalone = { platformData: { sources: ['agent', 'pbs'], agent: sharedAgent }, } as unknown as Resource; -render( - () => , - document.getElementById('root') as HTMLElement, -); +const Fixture = () => { + const [servers, setServers] = createSignal(structuredClone([pbs, guest, standalone])); + let snapshot = 0; + return ( + <> + + + + ); +}; + +render(() => , document.getElementById('root') as HTMLElement); diff --git a/frontend-modern/browser-verification.json b/frontend-modern/browser-verification.json index e0ed0fc12..d2bc7ad83 100644 --- a/frontend-modern/browser-verification.json +++ b/frontend-modern/browser-verification.json @@ -1,57 +1,40 @@ { "version": 1, - "base_sha": "76f96f990638652ed3d4af890bbcdaa8d47374aa", - "verified_at": "2026-09-22T09:47:00Z", + "base_sha": "014190ea4c630397e13088e3888bffb93012b8f3", + "verified_at": "2026-09-22T11:10:32.326040Z", "result": "passed", "changed_paths": [ - "frontend-modern/src/features/proxmox/ProxmoxBackupServersTable.tsx", - "frontend-modern/src/utils/alertOverviewPresentation.ts" + "frontend-modern/src/features/proxmox/ProxmoxBackupServersTable.tsx" ], "content_sha256": { - "frontend-modern/src/features/proxmox/ProxmoxBackupServersTable.tsx": "18535683c37f5b5a481ac012a49b7dcdd6264ac992076381c74fdfbb037e31eb", - "frontend-modern/src/utils/alertOverviewPresentation.ts": "7f68a70800deb0cb8214d4fd25c7454c5b0fde1084e08365eb93bc361b80846a" + "frontend-modern/src/features/proxmox/ProxmoxBackupServersTable.tsx": "b463452e00e796a0d9ff3639f21ff7ac0d7219539cfacf50f6101282d829f78a" }, "routes": [ - "/browser-tests/alert-card-footer.html", - "/proxmox/backups (production surface represented by the component fixture)", + "/proxmox/backups (production 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", - "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 PBS row opens the canonical drawer; History is selectable and renders the host metric groups", - "The PBS drawer's merged header shows the guest's current CPU/memory (15.4% / 16.9%) rather than the PBS service values", - "The History request resolves to the guest series vm/proxmox:100; the PBS service target agent/pbs-1 is never requested", - "Desktop and narrow layouts for both surfaces" + "Two datastores on one PBS, with a single agent represented by guest and standalone rows", + "History on tank, archive, then tank again, with populated synthetic CPU/memory/disk/network history", + "Fresh resource snapshots with changing usage and alternating datastore array order" ], "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", - "Mount the production ProxmoxBackupServersTable with synthetic PBS server, PVE guest and standalone agent resources sharing one agent identity", - "Focus the PBS 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 both surfaces at desktop and narrow widths and capture a screenshot of each rendered surface", - "Record Chromium from the offline Playwright runtime with no uncaught page errors" + "Keyboard-expand each datastore row and select History", + "Check both datastore identities before each switch", + "Refresh three snapshots at each viewport; assert updated usage, retained drawer DOM, selected History tab and seven chart paths", + "Assert all six history requests use vm/proxmox:100, never the PBS service target", + "Inspect desktop and mobile screenshots; no uncaught page errors" ], - "command": "pulse-worker-browser scripts/check-alert-card-footer.cjs; pulse-worker-browser scripts/check-pbs-host-history-correlation.cjs", - "notes": "Two offline Playwright Chromium runs, each against the production component with the project CSS and synthetic props; no live Pulse, backend, WebSocket or credential path. #2119: the alert-card 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; removing it aligns both runs (top 66 / bottom 82 at 2010x1072 and 480x800, delta 0). #1723: the fixture feeds synthetic PBS/guest/agent resources and answers /api/metrics-store/history with empty points, so the panels correctly show the collecting-history state; the assertion is the requested history target, vm/proxmox:100 (the guest series), never agent/pbs-1. No page errors and no failed responses. This receipt is content-addressed to the two changed frontend sources at this candidate base." + "command": "pulse-worker-browser .pbs-proof/check.cjs (assigned workspace root; invokes scripts/check-pbs-host-history-correlation.cjs in the disposable source copy)", + "notes": "Chromium 141.0.7390.37 / installed Playwright 1.56.1. Production source SHA-256 matched the assigned tree; package-lock SHA-256 87e579d28080ad1fb42992dd5b398c373aa08aff63bbf3f0ed63ac8b4a8f89a0, matching offline dependency snapshot dac6a9b347a1c76ad3bf0f5eadc17b84d468ce0d3878354e5d39e745101ccb53. Before the fix, a reversed datastore array overwrote the archive row to tank, reproduced in Chromium and the new component test. Isolating the row datastore object corrects it. Synthetic metrics/capability/AI responses, not live backend or reporter data. History reset to Summary was not reproduced, and this is not whole-issue resolution. Final screenshots visually inspected. Logs retained in web-product-h8pgc3la/pbs-browser-final.log; disposable copy retained in that output directory." } diff --git a/frontend-modern/src/features/proxmox/ProxmoxBackupServersTable.tsx b/frontend-modern/src/features/proxmox/ProxmoxBackupServersTable.tsx index 1cd03c2d1..8d6272e57 100644 --- a/frontend-modern/src/features/proxmox/ProxmoxBackupServersTable.tsx +++ b/frontend-modern/src/features/proxmox/ProxmoxBackupServersTable.tsx @@ -279,7 +279,10 @@ export function buildBackupServerRows( rows.push({ key: `${server.id}:${datastore.name}`, ...host, - datastore, + // Row stores reconcile independently from the nested PBS snapshot. Do + // not alias its datastore object: a reordered snapshot reconciles that + // array by position and would overwrite this row's datastore identity. + datastore: { ...datastore }, backupCount: countFor(server, datastore.name), }); } 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 7d3282f1b..c8782b289 100644 --- a/frontend-modern/src/features/proxmox/__tests__/ProxmoxBackupServersTable.drawer.test.tsx +++ b/frontend-modern/src/features/proxmox/__tests__/ProxmoxBackupServersTable.drawer.test.tsx @@ -83,6 +83,35 @@ describe('ProxmoxBackupServersTable details', () => { expect(screen.getByTestId('pbs-resource-detail')).toBeInTheDocument(); }); + it('keeps each datastore identity and open drawer through reordered snapshots', async () => { + const makeServers = (reverse = false) => { + const pbs = makePbsResource(); + pbs.pbs!.datastores!.push({ + name: 'archive', + total: 2000, + used: 600, + available: 1400, + usagePercent: 30, + }); + if (reverse) pbs.pbs!.datastores!.reverse(); + return [pbs]; + }; + const [servers, setServers] = createSignal(makeServers()); + const { container } = render(() => ); + const datastoreNames = () => + Array.from(container.querySelectorAll('td[title]')).map((cell) => cell.getAttribute('title')); + fireEvent.click(screen.getAllByRole('button', { name: 'Expand details for pbs-main' })[1]); + const detail = screen.getByTestId('pbs-resource-detail'); + expect(datastoreNames()).toEqual(['pbs-main · archive', 'pbs-main · tank']); + + setServers(makeServers(true)); + await waitFor(() => + expect(datastoreNames()).toEqual(['pbs-main · archive', 'pbs-main · tank']), + ); + expect(screen.getByTestId('pbs-resource-detail')).toBe(detail); + expect(resourceDetailDrawerMount).toHaveBeenCalledTimes(1); + }); + it('opens the canonical resource drawer with merged host details expanded', () => { render(() => ); diff --git a/scripts/check-pbs-host-history-correlation.cjs b/scripts/check-pbs-host-history-correlation.cjs index 215a75a4c..8e12ba2e1 100644 --- a/scripts/check-pbs-host-history-correlation.cjs +++ b/scripts/check-pbs-host-history-correlation.cjs @@ -2,16 +2,23 @@ // // 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 +// host row), plus two datastores. Switches between them, replaces/reorders +// snapshots with fresh CPU telemetry, and checks chart paths and tab retention. +// 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 assert = require('node:assert/strict'); const { chromium } = require('playwright'); const ROOT = path.resolve(process.cwd(), 'frontend-modern'); -const launchOptions = { headless: true, channel: 'chromium', args: ['--no-sandbox'] }; +const launchOptions = { + headless: true, + channel: 'chromium', + args: ['--no-sandbox'], +}; (async () => { process.chdir(ROOT); @@ -39,6 +46,22 @@ const launchOptions = { headless: true, channel: 'chromium', args: ['--no-sandbo const page = await browser.newPage({ viewport }); page.on('pageerror', (error) => pageErrors.push(`${viewport.width}px: ${error.message}`)); + await page.route('**/api/license/runtime-capabilities', (route) => + route.fulfill({ + json: { + capabilities: [], + limits: [], + hosted_mode: false, + max_history_days: 7, + runtime: { build: 'community', label: 'Pulse Community runtime' }, + blocked_capabilities: [], + }, + }), + ); + await page.route('**/api/settings/ai', (route) => + route.fulfill({ json: { enabled: false } }), + ); + 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')}`); @@ -46,22 +69,96 @@ const launchOptions = { headless: true, channel: 'chromium', args: ['--no-sandbo json: { resourceType: query.get('resourceType'), resourceId: query.get('resourceId'), - points: [], + range: query.get('range'), + start: Date.now() - 3600000, + end: Date.now(), + source: 'store', + metrics: Object.fromEntries( + ['cpu', 'memory', 'disk', 'netin', 'netout', 'diskread', 'diskwrite'].map( + (metric) => [ + metric, + [30, 20, 10].map((minutes, i) => ({ + timestamp: Date.now() - minutes * 60000, + value: 10 + i * 5, + min: 10 + i * 5, + max: 10 + i * 5, + })), + ], + ), + ), }, }); }); 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, + const rows = page.locator('tr').filter({ + has: 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); + await rows.first().waitFor({ timeout: 20000 }); + const openDatastore = async (name) => { + const row = page + .locator('tr') + .filter({ has: page.locator(`td[title="proxback · ${name}"]`) }); + await row.getByRole('button').focus(); + await page.keyboard.press('Enter'); + await page.getByRole('tab', { name: 'History', exact: true }).click(); + await page.locator('[data-testid="guest-history-plot"] path').first().waitFor(); + assert.equal( + await page + .getByRole('tab', { name: 'History', exact: true }) + .getAttribute('aria-selected'), + 'true', + ); + }; + const observations = []; + // Both datastore rows must use the same host series, including revisiting + // the first row after its history component has been disposed. + for (const [index, datastore] of ['tank', 'archive', 'tank'].entries()) { + assert.deepEqual( + await page + .locator('td[title^="proxback · "]') + .evaluateAll((nodes) => nodes.map((node) => node.getAttribute('title'))), + ['proxback · archive', 'proxback · tank'], + ); + await openDatastore(datastore); + const detail = page.locator('[data-inline-platform-resource-detail-for="pbs-1"]'); + await detail.evaluate((node) => { + node.dataset.proofIdentity = 'retained'; + }); + const pathsBefore = await detail.locator('[data-testid="guest-history-plot"] path').count(); + assert.ok(pathsBefore > 0); + await page.getByRole('button', { name: 'Refresh resource snapshot' }).click(); + // Datastore usage is visible at both widths and proves that the fresh + // snapshot reached the UI (the CPU column is hidden on narrow screens). + await page + .getByText(`${(41 + index).toFixed(1)}%`, { exact: true }) + .first() + .waitFor(); + assert.equal( + await detail.getAttribute('data-proof-identity'), + 'retained', + 'drawer remounted', + ); + assert.equal( + await page + .getByRole('tab', { name: 'History', exact: true }) + .getAttribute('aria-selected'), + 'true', + 'history reset after snapshot', + ); + assert.equal( + await detail.locator('[data-testid="guest-history-plot"] path').count(), + pathsBefore, + ); + observations.push({ + datastore, + paths: pathsBefore, + refreshPreservedHistory: true, + }); + } if (!targets.includes(expected)) { throw new Error( @@ -77,7 +174,7 @@ const launchOptions = { headless: true, channel: 'chromium', args: ['--no-sandbo await page.screenshot({ path: path.join(ROOT, 'browser-tests', `pbs-host-history-${viewport.width}.png`), }); - results.push({ viewport, targets }); + results.push({ viewport, targets, observations }); await page.close(); } @@ -85,7 +182,9 @@ const launchOptions = { headless: true, channel: 'chromium', args: ['--no-sandbo throw new Error(`page errors: ${pageErrors.join(' | ')}`); } - console.log(JSON.stringify({ result: 'passed', expected, results }, null, 2)); + console.log( + JSON.stringify({ result: 'passed', browser: browser.version(), expected, results }, null, 2), + ); } finally { if (browser) await browser.close(); await server.close();