From 371435e1eb9bbace47c07ea1c2299e6eb5fb6622 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Thu, 8 Oct 2026 00:08:09 +0100 Subject: [PATCH] Name sensor setup notice nodes the way the nodes table does The outdated sensor setup notice named each node by its Proxmox node name ("pve3") while the nodes table below it, the outdated agent notice beside it and Infrastructure settings label the same node by its display name ("West Production C"). A user with display names set could not tell which row the notice meant. The notice now leads with the label the nodes table shows for the row (the resource name, else the id) and adds the Proxmox node name in parentheses: "West Production C (pve3)". Both names are needed. The nodes table switches to the Proxmox node name in its narrowest layout, and the setup script is re-run on that node, so the exact native name is what the user needs to find the host. The node name is left out only when the label already contains it as a whole word, ignoring ASCII case, or as a domain's host part ("PVE1", "pve3.lan"). This is deliberately stricter than hasAlternateDisplayName in the guest table's group header, which normalises punctuation and would show "PVE3" alone for node "pve-3", hiding the host to act on. The notice's own test file was never registered as proof for the frontend-primitives policy that covers the notice, unlike the sibling agent notice's test, so it is added to that policy's exact_files. --- .../subsystems/frontend-primitives.md | 10 ++ .../v6/internal/subsystems/registry.json | 1 + .../07000f92ba692351.json | 50 ++++++++ ...PlatformOutdatedSensorSetupNotice.test.tsx | 15 ++- .../PlatformOutdatedSensorSetupNotice.tsx | 10 +- .../features/platformPage/sensorSetup.test.ts | 112 +++++++++++++++--- .../src/features/platformPage/sensorSetup.ts | 45 ++++++- 7 files changed, 222 insertions(+), 21 deletions(-) create mode 100644 frontend-modern/browser-verification/07000f92ba692351.json diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 0d45e31828..7c19ed515a 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -6664,6 +6664,16 @@ notice renders on. `collectOutdatedSensorSetupNodes` lists the PVE nodes that carry the registry's `proxmox.sensorSetupOutdated` verdict and takes no disk rows, because only the Proxmox Storage tab loads them; a predicate over them left the notice silent on Overview. +A notice names each affected resource by the label its page's table shows for +the row. The sensor-setup notice leads with the node's trimmed resource name +(the operator-set display name when one exists, else the Proxmox node name), +falling back to the id as the nodes table does, and adds the Proxmox node name +in parentheses unless the label already contains it as a whole word, ignoring +ASCII case, or as the host part of a domain name, as in +`West Production C (pve3)`. Both names are needed: the nodes table switches to +the Proxmox node name in its narrowest layout, and the setup script is re-run +on that Proxmox node, so the rule is stricter than `hasAlternateDisplayName` in +the guest table's node group header and keeps `pve-3` beside a `PVE3` label. Alert incident-event filter containers, labels, and chips must now route through the shared presentation helpers in diff --git a/docs/release-control/v6/internal/subsystems/registry.json b/docs/release-control/v6/internal/subsystems/registry.json index 95466d4ebd..78a73168b9 100644 --- a/docs/release-control/v6/internal/subsystems/registry.json +++ b/docs/release-control/v6/internal/subsystems/registry.json @@ -5836,6 +5836,7 @@ "frontend-modern/src/features/platformPage/__tests__/platformEstateOverviewModel.test.ts", "frontend-modern/src/features/platformPage/__tests__/platformOverviewLayout.guardrails.test.ts", "frontend-modern/src/features/platformPage/PlatformOutdatedAgentNotice.test.tsx", + "frontend-modern/src/features/platformPage/PlatformOutdatedSensorSetupNotice.test.tsx", "frontend-modern/src/features/platformPage/platformSearchSuggestions.test.ts" ] }, diff --git a/frontend-modern/browser-verification/07000f92ba692351.json b/frontend-modern/browser-verification/07000f92ba692351.json new file mode 100644 index 0000000000..5a82e7bbd2 --- /dev/null +++ b/frontend-modern/browser-verification/07000f92ba692351.json @@ -0,0 +1,50 @@ +{ + "version": 1, + "base_sha": "c5416b6800add376e674ab64292df3e4d8e88f7f", + "verified_at": "2026-10-07T22:31:44Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/features/platformPage/PlatformOutdatedSensorSetupNotice.tsx", + "frontend-modern/src/features/platformPage/sensorSetup.ts" + ], + "content_sha256": { + "frontend-modern/src/features/platformPage/PlatformOutdatedSensorSetupNotice.tsx": "404d9a23881683e94ec74400120a33c432ffa4b8f24238be3d127a6a7e243abc", + "frontend-modern/src/features/platformPage/sensorSetup.ts": "153a28a603520156129738bd09d3d5dedab03ab8f5de2075edeb57b777aed778" + }, + "routes": [ + "/proxmox", + "/proxmox/storage" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 768, + "height": 1024 + }, + { + "width": 414, + "height": 896 + }, + { + "width": 375, + "height": 812 + } + ], + "states": [ + "Private mock stack (8-node estate; every node carries an operator-style display name such as West Production C for pve3) with a local-only generator overlay: pve1, pve3 and standalone2 report the legacy sensors -j payload with a SATA disk lacking a current temperature, so the registry sets sensorSetupOutdated on those three nodes. Frontend served by Vite from this tree; backend binary built from dace9295c6 with the overlay (main's later commits do not touch this data path).", + "Three affected nodes: 'Affected: Automation Lab Standalone (standalone2), West Production A (pve1), West Production C (pve3).', sorted by table label, on Overview and Storage at all four viewports, with no notice or page overflow and no console errors; the action link targets /settings/infrastructure.", + "The nodes table container measures 1320, 695 and 385px at 1440, 768 and 414 wide and shows display names; at 375 wide it measures 346px (its narrow layout) and labels rows standalone2, standalone1, pve1. The notice carries both names, so each layout can match it to a row.", + "Single affected node (pve3 only, earlier revision of this change with the same label output): 'West Production C (pve3) is using an older temperature monitoring setup ...'.", + "Before control on the same stack with origin/main's sensorSetup.ts and PlatformOutdatedSensorSetupNotice.tsx swapped in: 'Affected: pve1, pve3, standalone2.' above rows labelled Automation Lab Standalone, West Production A and West Production C.", + "Labels that already contain the node name (pve2.lan, 'Rack 4 (pve3)', quoted, case-only) and labels that only resemble it (PVE3 for pve-3, pve5-backup, pve10 for pve1, a Kelvin-sign lookalike) are covered by sensorSetup.test.ts only; every mock node carries a distinct display name. The last edit after the pass was a code comment.", + "Mock data only; not a real pre-rc.6 SSH node or a real operator-renamed cluster." + ], + "interactions": [ + "Loaded /proxmox and /proxmox/storage by URL in headless Chromium at each viewport and read the notice text, node row labels, table container width and overflow from the DOM; screenshots kept locally.", + "Restarted the backend with two more legacy nodes to move from the single-node sentence to the multi-node list.", + "Swapped origin/main's two source files in and out through Vite HMR for the before control, then restored this change byte-for-byte and re-read the notice." + ] +} diff --git a/frontend-modern/src/features/platformPage/PlatformOutdatedSensorSetupNotice.test.tsx b/frontend-modern/src/features/platformPage/PlatformOutdatedSensorSetupNotice.test.tsx index 1fe4c929df..e6447986c7 100644 --- a/frontend-modern/src/features/platformPage/PlatformOutdatedSensorSetupNotice.test.tsx +++ b/frontend-modern/src/features/platformPage/PlatformOutdatedSensorSetupNotice.test.tsx @@ -29,12 +29,23 @@ describe('PlatformOutdatedSensorSetupNotice', () => { ); }); + it('names a renamed node by its table label and its Proxmox node', () => { + render(() => ( + + )); + expect(screen.getByTestId('platform-outdated-sensor-setup-notice')).toHaveTextContent( + 'West Production C (pve3) is using an older temperature monitoring setup that cannot read SATA/SAS disk temperatures.', + ); + }); + it('summarises multiple affected nodes and lists them', () => { render(() => ( )); @@ -42,6 +53,6 @@ describe('PlatformOutdatedSensorSetupNotice', () => { expect(notice).toHaveTextContent( '2 nodes are using an older temperature monitoring setup that cannot read SATA/SAS disk temperatures.', ); - expect(notice).toHaveTextContent('Affected: pve1, pve2.'); + expect(notice).toHaveTextContent('Affected: pve1, West Production C (pve3).'); }); }); diff --git a/frontend-modern/src/features/platformPage/PlatformOutdatedSensorSetupNotice.tsx b/frontend-modern/src/features/platformPage/PlatformOutdatedSensorSetupNotice.tsx index aada988edd..0f50af8a0d 100644 --- a/frontend-modern/src/features/platformPage/PlatformOutdatedSensorSetupNotice.tsx +++ b/frontend-modern/src/features/platformPage/PlatformOutdatedSensorSetupNotice.tsx @@ -10,6 +10,12 @@ type PlatformOutdatedSensorSetupNoticeProps = { actionLabel?: string; }; +// Leads with the label the nodes table shows so the user can match the row, +// then names the Proxmox node the setup script is re-run on when that label +// does not already contain it. +const formatNodeLabel = (node: OutdatedSensorSetupNode): string => + node.nodeName ? `${node.name} (${node.nodeName})` : node.name; + // Inline notice shown on the Proxmox page when a node's SSH temperature // monitoring was set up by a pre-v6.0.0-rc.6 setup script. That setup locks // the SSH key to `sensors -j`, so SATA/SAS disk temperatures silently never @@ -18,12 +24,12 @@ type PlatformOutdatedSensorSetupNoticeProps = { // clean in the healthy case. Sibling of PlatformOutdatedAgentNotice. export function PlatformOutdatedSensorSetupNotice(props: PlatformOutdatedSensorSetupNoticeProps) { const count = createMemo(() => props.nodes.length); - const names = createMemo(() => props.nodes.map((node) => node.name).join(', ')); + const names = createMemo(() => props.nodes.map(formatNodeLabel).join(', ')); const actionLabel = createMemo(() => props.actionLabel || 'Open Infrastructure settings'); const message = createMemo(() => { if (count() === 1) { - return `${props.nodes[0].name} is using an older temperature monitoring setup that cannot read SATA/SAS disk temperatures. Re-run the node setup script to upgrade it.`; + return `${formatNodeLabel(props.nodes[0])} is using an older temperature monitoring setup that cannot read SATA/SAS disk temperatures. Re-run the node setup script to upgrade it.`; } return `${count()} nodes are using an older temperature monitoring setup that cannot read SATA/SAS disk temperatures. Re-run the node setup script on each to upgrade them. Affected: ${names()}.`; }); diff --git a/frontend-modern/src/features/platformPage/sensorSetup.test.ts b/frontend-modern/src/features/platformPage/sensorSetup.test.ts index 949c649186..2480648e4c 100644 --- a/frontend-modern/src/features/platformPage/sensorSetup.test.ts +++ b/frontend-modern/src/features/platformPage/sensorSetup.test.ts @@ -34,24 +34,108 @@ describe('collectOutdatedSensorSetupNodes', () => { expect(collectOutdatedSensorSetupNodes([reading])).toEqual([]); }); - it('names a node by its Proxmox node, then node name, then resource name, trimmed', () => { - const nodes = [ - node({ id: 'by-node', proxmox: { node: ' alpha ', sensorSetupOutdated: true } }), - node({ id: 'by-node-name', proxmox: { nodeName: 'bravo', sensorSetupOutdated: true } }), - node({ id: 'by-name', name: 'charlie', proxmox: { sensorSetupOutdated: true } }), - ]; - expect(collectOutdatedSensorSetupNodes(nodes)).toEqual([ - { id: 'by-node', name: 'alpha' }, - { id: 'by-node-name', name: 'bravo' }, - { id: 'by-name', name: 'charlie' }, + it('names a node by its table label and adds the Proxmox node name when they differ', () => { + // The nodes table labels the row by the operator-set display name, while + // the setup script is re-run on the Proxmox node itself. + const renamed = node({ + id: 'node-3', + name: 'West Production C', + proxmox: { node: 'pve3', nodeName: 'pve3', sensorSetupOutdated: true }, + }); + expect(collectOutdatedSensorSetupNodes([renamed])).toEqual([ + { id: 'node-3', name: 'West Production C', nodeName: 'pve3' }, ]); }); - it('sorts multiple affected nodes by name', () => { + it('omits the Proxmox node name when the table label contains it as a word', () => { + const flagged = (id: string, name: string, nodeName?: string) => + node({ id, name, proxmox: { nodeName, sensorSetupOutdated: true } }); + expect( + collectOutdatedSensorSetupNodes([ + flagged('same', ' PVE1 ', 'pve1'), + flagged('fqdn', 'pve2.lan', 'pve2'), + flagged('embedded', 'Rack 4 (pve3)', 'pve3'), + flagged('punctuated', '"pve5": primary', 'pve5'), + flagged('unknown', 'pve4'), + ]), + ).toEqual([ + { id: 'punctuated', name: '"pve5": primary' }, + { id: 'same', name: 'PVE1' }, + { id: 'fqdn', name: 'pve2.lan' }, + { id: 'unknown', name: 'pve4' }, + { id: 'embedded', name: 'Rack 4 (pve3)' }, + ]); + }); + + it('keeps the exact Proxmox node name when the label only resembles it', () => { + // The setup script is re-run on this host, so a label that differs inside + // the name (punctuation, spacing, a suffix or a non-ASCII lookalike + // letter) must not stand in for it. + const flagged = (id: string, name: string, nodeName: string) => + node({ id, name, proxmox: { nodeName, sensorSetupOutdated: true } }); + expect( + collectOutdatedSensorSetupNodes([ + flagged('punctuation', 'PVE3', 'pve-3'), + flagged('spacing', 'PVE 4', 'pve-4'), + flagged('suffix', 'pve5-backup', 'pve5'), + flagged('prefix', 'pve10', 'pve1'), + flagged('lookalike', 'PVE-\u212A', 'pve-k'), + ]), + ).toEqual([ + { id: 'spacing', name: 'PVE 4', nodeName: 'pve-4' }, + { id: 'lookalike', name: 'PVE-\u212A', nodeName: 'pve-k' }, + { id: 'prefix', name: 'pve10', nodeName: 'pve1' }, + { id: 'punctuation', name: 'PVE3', nodeName: 'pve-3' }, + { id: 'suffix', name: 'pve5-backup', nodeName: 'pve5' }, + ]); + }); + + it('labels a node without a resource name by its id, as the table does', () => { const nodes = [ - flaggedNode({ id: 'node-b', proxmox: { node: 'pve-b', sensorSetupOutdated: true } }), - flaggedNode({ id: 'node-a', proxmox: { node: 'pve-a', sensorSetupOutdated: true } }), + node({ id: 'by-id', name: ' ', proxmox: { node: ' alpha ', sensorSetupOutdated: true } }), + node({ id: 'bare', name: '', proxmox: { sensorSetupOutdated: true } }), ]; - expect(collectOutdatedSensorSetupNodes(nodes).map((n) => n.name)).toEqual(['pve-a', 'pve-b']); + expect(collectOutdatedSensorSetupNodes(nodes)).toEqual([ + { id: 'bare', name: 'bare' }, + { id: 'by-id', name: 'by-id', nodeName: 'alpha' }, + ]); + }); + + it('reads the Proxmox node name from nodeName before node', () => { + const nodes = [ + node({ + id: 'both', + name: 'West Production C', + proxmox: { node: 'stale', nodeName: ' pve3 ', sensorSetupOutdated: true }, + }), + node({ + id: 'blank', + name: 'West Production D', + proxmox: { node: 'pve4', nodeName: ' ', sensorSetupOutdated: true }, + }), + ]; + expect(collectOutdatedSensorSetupNodes(nodes)).toEqual([ + { id: 'both', name: 'West Production C', nodeName: 'pve3' }, + { id: 'blank', name: 'West Production D', nodeName: 'pve4' }, + ]); + }); + + it('sorts multiple affected nodes by their table label', () => { + const nodes = [ + flaggedNode({ + id: 'node-a', + name: 'West Production C', + proxmox: { nodeName: 'pve-a', sensorSetupOutdated: true }, + }), + flaggedNode({ + id: 'node-b', + name: 'East Backup', + proxmox: { nodeName: 'pve-b', sensorSetupOutdated: true }, + }), + ]; + expect(collectOutdatedSensorSetupNodes(nodes).map((n) => n.name)).toEqual([ + 'East Backup', + 'West Production C', + ]); }); }); diff --git a/frontend-modern/src/features/platformPage/sensorSetup.ts b/frontend-modern/src/features/platformPage/sensorSetup.ts index 2e27d70d5a..31f0dc9964 100644 --- a/frontend-modern/src/features/platformPage/sensorSetup.ts +++ b/frontend-modern/src/features/platformPage/sensorSetup.ts @@ -2,11 +2,50 @@ import type { Resource } from '@/types/resource'; export type OutdatedSensorSetupNode = { id: string; + // The label the Proxmox nodes table shows for the node's row outside its + // narrowest layout, which shows the Proxmox node name instead. name: string; + // The Proxmox node name, which the setup script is re-run on, when `name` + // does not already contain it as a whole word or a domain's host part. + nodeName?: string; }; -const getNodeName = (node: Resource): string => - (node.proxmox?.node || node.proxmox?.nodeName || node.name || '').trim(); +const trimmed = (value: string | undefined): string => (value ?? '').trim(); + +const getProxmoxNodeName = (node: Resource): string => + trimmed(node.proxmox?.nodeName) || trimmed(node.proxmox?.node); + +// Mirrors the nodes table's row label: the resource name, which carries the +// operator-set display name when one exists and the Proxmox node name +// otherwise, falling back to the id. +const getNodeLabel = (node: Resource): string => trimmed(node.name) || node.id; + +// Folds only ASCII letters, the alphabet of Proxmox node names, so a +// non-ASCII lookalike (the Kelvin sign for K) cannot pass for one. +const foldAscii = (value: string): string => value.replace(/[A-Z]/g, (c) => c.toLowerCase()); + +// True when the label already contains the node name as a whole word, ignoring +// ASCII case (`PVE1`, `Rack 4 (pve3)`), or as the host part of a domain name +// (`pve3.lan`). Words are runs of ASCII letters, digits, dots, hyphens and +// underscores, so a label that differs inside the name (`PVE3` or `PVE 3` for +// `pve-3`, `pve3-backup` for `pve3`) keeps the exact name. Stricter than the +// guest table's decorative node group header (`hasAlternateDisplayName`), +// because the notice names the host the user must act on. +const labelShowsNodeName = (label: string, nodeName: string): boolean => { + const target = foldAscii(nodeName); + return foldAscii(label) + .split(/[^a-z0-9._-]+/) + .some((word) => word === target || word.startsWith(`${target}.`)); +}; + +const toOutdatedSensorSetupNode = (node: Resource): OutdatedSensorSetupNode => { + const name = getNodeLabel(node); + const nodeName = getProxmoxNodeName(node); + if (!nodeName || labelShowsNodeName(name, nodeName)) { + return { id: node.id, name }; + } + return { id: node.id, name, nodeName }; +}; // Lists PVE nodes whose SSH temperature monitoring still runs the pre-rc.6 // setup (authorized_keys locked to `sensors -j`). Such a payload parses fine @@ -18,6 +57,6 @@ const getNodeName = (node: Resource): string => export function collectOutdatedSensorSetupNodes(nodes: Resource[]): OutdatedSensorSetupNode[] { return nodes .filter((node) => node.proxmox?.sensorSetupOutdated === true) - .map((node) => ({ id: node.id, name: getNodeName(node) })) + .map(toOutdatedSensorSetupNode) .sort((a, b) => a.name.localeCompare(b.name)); }