diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index e7b4f59ab3..b8ed3f012b 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -237,6 +237,17 @@ state; the monitoring readback requires an actual Proxmox source observation. This leaves Agent source identity, report admission and command authority unchanged. The repeated-cycle regression is `TestPhysicalDiskSkippedPollDoesNotPromoteAgentOnlySMARTToPVEInventory`. +A linked Agent's SMART row that matches a PVE disk only by device path is +refused when its WWN contradicts the disk's, or its serial does where both +producers read the drive's own serial (NVMe, and disks presented with SCSI +vendor `ATA`), +and the registry's SAS path join refuses a contradicting WWN, so a silent +Agent's retained row for a swapped-out disk lends the replacement neither its +serial, and with it the old disk's canonical resource, nor its readings. +Without comparable identity (a SAS, SCSI or USB replacement with no WWN on one +side) the row still matches. Report admission and the SMART wire contract are +unchanged; the regression is +`TestHostAgentSMARTRowForSwappedOutDiskDoesNotLendItsIdentity`. Assistant historical metric wiring uses the current monitor's retained store and registry metrics coordinates. Historical reads do not alter enrollment, diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 5df9ac3bd2..9d7946179e 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -49,6 +49,19 @@ recorded message. Every disk alert on a guest carries the guest's name, so the presentation names the disk from `metadata.label`, as the per-disk message does. +### History leads its open row with the live reading — issue #2068 + +Alerts > History is a log of occurrences, but its open row is the one +occurrence still in progress: it already shows an Active badge and a running +duration, so a breach message beside them reads as the reading now. An active +history row for a threshold alert with a usable live status leads with the +presentation's summary; the desktop row carries the clear rule and last +breach in its hover, and the phone card shows them as muted lines because it +has no hover. `buildAlertHistoryItems` keeps `description` as the recorded +message, so search and the Assistant handoff still read the record, and +attaches the source alert as `liveAlert` only to active, non-system rows. +Closed rows, and open ones without a live status, keep their recorded message. + ### Retained guest observations are not alert evidence Guest memory marked `last-known`, `unavailable` or an unknown observation state @@ -3337,6 +3350,27 @@ delivery pause because those policies must not disable observation of Pulse. Its Refresh and Show/Hide actions are the shared outline `Button` (xs and `settingsAction` sizes), not a copied border shell, so they match the other destination controls and keep a 44px touch target on phones. +Last success and Monitor progress are ages, so they read the frontend-primitives +shared relative-time clock and keep moving while the panel stays open, and the +panel re-reads `GET /api/alerts/deadman/status` in the background every 30 +seconds. A moving age over a status read once at mount would claim the +heartbeat stopped while Pulse kept pinging every minute, and a status that is +never re-read kept its read-time age through a real stall, which is the one +case this panel exists to show. A background re-read leaves the Refresh control +alone, and a failed one keeps the last status and marks it unavailable. The +status read has no timeout, so reads carry a sequence number and a read's +outcome applies only when it is newer than the last read that settled: a +success shows its status and clears the unavailable mark, a failure sets it. A +slow or hung read can therefore neither overwrite a newer outcome nor block +later reads, and reads slower than the poll still land in order. While the +panel's poll runs, the next background read goes out at the first poll once the +newest read has been pending at least 29 seconds (one second of slack absorbs +timer jitter), and a Refresh is released by its own read, by any newer read that +settles, or by the first poll at which it has been pending that long, so a hung +Refresh frees within about two intervals of the click. +The delivery log's relative hover time on each row reads the same clock: a +logged attempt's time never changes, so its age is correct over a log that is +only re-read on demand. Watchdog transport and monitoring progress remain notifications- and monitoring-owned respectively. Alerts owns the system-alert projection: diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index ff1007b5d0..4b75f52918 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -4859,6 +4859,12 @@ clients must not treat their absence as evidence of stability. `frontend-modern/src/api/patrolAttention.ts` (`AttentionFlapping`) and `frontend-modern/src/api/patrol.ts` (`FindingFlapping`, `mirrors_alert_*`) are the typed client boundary. +The Patrol page's paused banner, selected-run banner and attention detail +measure their ages (`blocked_at`, a run's `started_at`, acknowledgement, +transition and evidence times) from the frontend-primitives shared +relative-time clock. That is presentation over the same `GET +/api/ai/patrol/status`, run history and `GET /api/ai/patrol/attention/{id}` +payloads: no field, request or polling cadence changes. ### Resource-list facets preserve scoped navigation evidence diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index df8ce38dec..ae9e8b2d27 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -43,6 +43,15 @@ card's left column (status icon and text) keeps a 16rem floor and the text breaks long words, so the action buttons wrap instead of squeezing the reading to its longest word beside them. +### History rows read held alerts through the same helper — issue #2068 + +`AlertHistoryTableAlertRow` and `AlertHistoryMobileList` take their message +copy from `getAlertHistoryRowCopy` in `features/alerts/alertHistoryModel.ts`, +which wraps the same `metricAlertPresentation.ts` helper for an active row's +`liveAlert`. It reads the `useRelativeTimeNow` clock only for rows with a live +alert, so a reading that stops updating turns stale on screen while closed +rows never re-render on the tick. + ### Canonical drawer History preserves guest read provenance The shared resource drawer passes selected memory observation state/source/time @@ -5240,6 +5249,17 @@ too. Every read of the clock returns the wall clock; the 30-second tick only tells readers to re-read, so a cell that mounts between ticks never measures from a stale time and a timestamp from the last few seconds never reads as a future time. +The rule has one deliberate exception. The age of a latest reading (last used, +last seen, last success, last checked) on data the surface reads once and does +not re-read stays the age at read time, because a moving age over a snapshot +that never refreshes claims the reading stopped when it may not have. Such a +surface either re-reads the snapshot in the background, as Proxmox replication +and the external watchdog panel do, or keeps the read-time age, as the Patrol +attention detail does for Last seen. The external watchdog panel orders its +re-reads by request, so a slow or hung read can neither pin it to an older +snapshot nor overwrite a newer outcome while its ages keep moving. An +immutable event time (when a delivery was attempted, a transition happened, a +policy was set) ages correctly over any snapshot and reads the clock. Read-only metadata badges follow the same primitive-owned shell rule. `frontend-modern/src/components/shared/MetadataBadge.tsx` owns filled and outlined appearances, compact sizing, shape, typed tone vocabulary, fit @@ -7827,6 +7847,23 @@ When the cell's value is a retained host-agent SMART temperature, it renders muted with a dotted underline and screen-reader "last known", as does a retained value in the guest card. Tooltip and Thermals rows say "(last known)", and the Thermals row carries the collection reason as its title. +The shared drawer History reads the same decision. For a physical disk, +`resourceDetailDrawerMetricsHistoryModel.ts` takes the reading from +`physicalDisk.temperature`, beside the collection state that qualifies it, +because websocket rows carry no top-level `temperature` for a disk. When that +state is set and is not `available` (an empty or absent state predates the +contract and stays current), `getResourceMetricsHistoryCurrentMetrics` offers +no current `smart_temp`. A positive retained reading then reaches +`getResourceMetricsHistoryDeferredMetrics` as a deferred metric: with no +history points the legend says "last known", never "current", and the reason +renders above the chart. +TrueNAS disk rows follow it too: the drawer's Temperature row reads +"(last known)" with the reason as title and no heat tone, and the one-line +summary leaves a retained reading out because it has no room for the reason. +The Proxmox outdated sensor setup notice (`features/platformPage/sensorSetup.ts`) +counts a SATA or SAS disk as having a temperature only when the reading is +current, so a retained one cannot hide the notice for a disk Pulse cannot read +now. The focused browser proofs are `frontend-modern/src/features/patrol/__tests__/patrolRunAcceptance.test.ts`, diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 60ff94e27d..3c011447f5 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -1508,6 +1508,51 @@ later coarse healthy value and lets explicit SMART endurance replace contradictory Proxmox wearout. Missing permission, ambiguous identity, standby, and absent SMART fields remain neutral rather than borrowing telemetry from another disk. +The device-path match is the last resort, and a path names a slot, not a +disk: the row there may describe the slot's previous occupant, because the +agent has not reported since a swap or went silent and keeps its last rows. +`mergeHostAgentSMARTIntoDisks` refuses such a row when its identity +contradicts the Proxmox disk's (`diskinventory.HardwareIdentityConflict`): +nothing agrees across serial and WWN, and a field both producers report like +for like disagrees. WWNs are compared for every disk. Serials are compared +only where Proxmox's serial is the drive's own, as smartctl's is: a Proxmox +disk typed `nvme` against an NVMe row, and one presented with SCSI vendor +`ATA` (libata, or a SAS HBA's SCSI-ATA translation, where udev's `ata_id` +reads IDENTIFY) and not typed `usb` or `sas` against an ATA row. Any other +Proxmox serial may be a SAS address or SCSI designator (#1595) or a USB +bridge's own serial, so it counts toward agreement only; a USB bridge that +does expose the drive's serial is excluded too unless it reports vendor `ATA`. +Reporter spellings are not disagreement: serials compare on letters and digits +only (udev writes whitespace as underscores), a T10 vendor designator for an +ATA drive (`ATA`, the model, then the serial) is no evidence against the serial +it ends with, and a WWN that begins with the other is no evidence (udev's `ID_WWN` keeps only +the first 64 bits of a 128-bit NAA 6 identifier). The refusal comes after the +unique-path check, so a second row on the path still makes it ambiguous. The +WWN match itself compares through `HardwareIdentityMatch`, so the agent's +`5-c50-a1b2c3d4` spelling finds the Proxmox `0x5000c500a1b2c3d4` disk. The +registry's SAS path join applies the same WWN refusal (unified-resources). +Proxmox's disk inventory (`PVE::Diskmanage`) writes the literal `unknown` for +a serial or WWN udev cannot read; the poller records it as empty, so the +serial's collection state reads `missing` and the agent's serial or WWN can +fill it, where the placeholder used to read as a collected serial. Retained +evidence carries a previous serial only when it is a usable identity, so a +record from before this change cannot restore the placeholder. Identity keys +already ignored the placeholder, so the empty value keys the same; a disk whose +serial the linked agent now fills keys its Proxmox SMART samples on that serial +instead of its WWN or path, as the agent's own samples already were. Before the +guard, the retained row of a swapped-out disk filled or promoted the old serial +onto the replacement, which keyed it to the old disk's canonical resource and +history, and lent it the old disk's temperature and failed health +(`TestHostAgentSMARTRowForSwappedOutDiskDoesNotLendItsIdentity`, +`TestHostAgentSMARTMergeRefusesPathMatchAcrossContradictingIdentity`, +`TestRegistrySASPathJoinRefusesContradictingWWN`, +`TestProxmoxUnknownDiskIdentityIsRecordedAsUnreported`, +`TestHardwareIdentityConflict`). Without comparable evidence there is nothing +to act on: a row without a usable serial or WWN still matches its path, and a +replacement whose serial is not comparable (SAS, SCSI, USB) still takes a +stale row's identity when either record lacks a WWN. The registry's seeded +source mapping, which a restart reuses by the slot-shaped Proxmox source ID, +does not apply this guard yet. Retaining evidence a disk poll could not collect follows the same identity rule. `previousPhysicalDiskEvidence` looks up the earlier record by serial, then by WWN, each compared through `diskinventory.HardwareIdentityMatch` against @@ -1529,10 +1574,9 @@ serial, which keyed it to the old disk's canonical resource and SMART metric series, so every later poll carried the borrowed serial forward (`TestPhysicalDiskReplacementInSameSlotDoesNotInheritPreviousSerial`, `TestPreviousPhysicalDiskEvidenceRejectsSlotMatchAcrossConflictingIdentity`). -This lookup is the only place the rule applies: a replacement that reports no -usable serial or WWN cannot be told apart from the same disk and still -inherits, and the linked agent's SMART merge, which runs first, can still fill -or promote a serial from a stale SMART row it matched by device path. +A replacement that reports no usable serial or WWN cannot be told apart from +the same disk and still inherits. The linked agent's SMART merge, which runs +first, refuses a contradicting device-path row on its own terms (above). Negative percentage-used counters remain unknown; values above 100 clamp to exhausted before deriving remaining life, so invalid or over-limit controller data cannot wrap into a fabricated healthy value. diff --git a/docs/release-control/v6/internal/subsystems/patrol-intelligence.md b/docs/release-control/v6/internal/subsystems/patrol-intelligence.md index a35d6021e9..4756dc70fd 100644 --- a/docs/release-control/v6/internal/subsystems/patrol-intelligence.md +++ b/docs/release-control/v6/internal/subsystems/patrol-intelligence.md @@ -2675,6 +2675,17 @@ finding and carried onto the unified finding as `failureCause`, and a provider preflight success no longer clears a budget-exhausted runtime finding, so the setup card still routes to the budget after a restart clears the in-memory block state. +The paused banner's `Blocked ...` age reads the frontend-primitives shared +relative-time clock: each status re-read returns the same `blocked_at` while +Patrol stays paused, so the banner kept the age it had when Patrol first +paused. The selected run's `Patrol run ...` banner reads the clock for the same +reason, since the selected run stays as picked when run history is re-read. The +attention detail is read once per selection, so its fixed event times (the +acknowledgement, each timeline transition, each evidence observation) read the +clock while it stays open. Its `Last seen` and flapping window are latest +readings of a record that may have moved since, so they stay the ages at +read time, and the queue rows, summary, recent work, run history and digest +re-derive on their own 30- or 60-second re-reads. ### This week card answers what Patrol did for the customer diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index 9e557430f7..fe049424b9 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -1121,6 +1121,13 @@ for the Physical Disks verdict. The heat reason follows any native TrueNAS alert text, and a hot disk's status dot turns warning though its source state stays `online`, because phones show only the dot. `truenasPageModel.test.ts` and `TrueNASStorageTopologyTable.test.tsx` pin it. +The Temp column reads the same collection state through +`getPhysicalDiskTemperaturePresentation`. A disk temperature that is not +current, such as a host agent's last reading kept on a merged disk row after +the agent stopped reporting, renders as last known: muted, with its reason as +the title and screen-reader "last known" text. The Temp sort ranks it as no +reading, and the tree sort orders each sibling group, so a retained value +never leads its group hottest first. The vSphere Datastores and Networks tables follow the same exception-first Health column. `getVmwareDatastoreIssue` and `getVmwareNetworkIssue` return nothing for a green row; otherwise the reasons are, in order, an impaired @@ -5288,6 +5295,48 @@ serial or WWN only inside a compatible physical-disk parent scope; when those identifiers are absent or placeholders, host/device/topology fallback identity is authoritative. An exact seeded source mapping may be reused after restart only when its resource type and parent scope still agree. +A usable serial or WWN identifies a disk on one machine only, and the canonical +ID a new disk takes must honour that, not just matching. Dual-ported SAS +shelves, cloned VMs with an explicit serial, fixed-serial USB bridges and +TrueNAS systems report one identifier on several machines, and merging them on +a hardware-keyed ID collision dropped every machine but the last from +inventory. `physicalDiskIDForMachineLocked` resolves each disk's parent to its +machine (the nearest host ancestor, so a TrueNAS pool or Unraid array counts as +its system or host) and treats two disks carrying one identity as different +machines only on evidence: one reporter placing the identity on two machines +(for a single-source disk rehydrated without per-source parents, its own +parent), or differing machines whose hostnames share nothing. Reporters on one +machine still join across grouping parents and unmerged host resources (the +TrueNAS API and the agent on that box), and an unknown machine never splits a +disk. A split (operator exclusion) recorded against any ID the disk can hold, +unscoped, current or machine-scoped, keeps the observation on its +source-specific ID, keyed to its machine when another machine's disk already +holds that ID (agent disk source IDs are the bare serial). Once the identity +spans machines the unscoped ID no longer names one disk, so a split recorded +against it applies to every copy, erring towards an extra row rather than a +merge the operator forbade, and a manual link recorded against it stops +applying. More than one joinable disk joins none, except that the holder of the +unscoped ID keeps it. Once an identity spans machines, every copy not split off +onto its source-specific ID takes the identity keyed to the machine of its +canonical parent at that point, and the holder of the unscoped ID is re-keyed +with its source mappings, retired-ID claims and relationship endpoints. +Machines may be ingested in any order, and the unscoped ID never passes between +disks that coexist on different machines. A disk only one machine reports keeps +its unscoped ID, apart from observations an operator split off. Residuals: like +any serial-keyed ID, the unscoped ID still passes to a same-serial disk that +appears once the previous holder is gone; a disk's ID changes when a +same-serial disk first appears on another machine and back when it goes away, +and alert-history rows owned under the unscoped ID do not follow a re-key; a +disk two reporters share is scoped to its canonical parent when the identity +first spans machines, which the fixed ingest order (snapshot sources, then +supplemental records) and source priority decide; agent SMART and Unraid disk +source IDs are the bare serial, so same-serial agent disks on different hosts +share one `SourceAgent` mapping and only the last ingested keeps the agent +source target; same-serial disks share one metrics history key; and across +reporters, hostnames that normalize alike (default TrueNAS names, short names +across domains) still join. `IdentityMatcher` keeps one resource per machine +ID, so the registry indexes disks by hardware key (`physicalDisksByHardware`) +to see every copy. That same canonical physical-disk view must also expose source-independent host context. When a disk is API-backed rather than node-backed, typed views should fall back to canonical host identity such as `identity.hostnames` instead of @@ -6376,6 +6425,22 @@ ambiguity test rejects multiple opposite-source candidates in both directions. These are synthetic source proofs, not appliance acceptance or a claim about which collector produced a reporter's row. +### Linked SAS path join refuses a contradicting WWN + +`resolveLinkedPhysicalDisk` falls back to joining a Proxmox disk to its linked +agent's disk on the same path when the agent reports SAS, because Proxmox may +report a SAS address as the serial (#1595). That path fallback is refused when +both sides carry WWNs that name different disks +(`diskinventory.HardwareIdentityConflict`, under which the agent's NAA field +spelling equals the Proxmox WWN and udev's 64-bit NAA 6 prefix is no +conflict): the agent's row may be the slot's previous occupant, retained by a +silent agent, and the join would hand the replacement that disk's serial, +readings and canonical resource. A hardware identity match still joins on its +own, and without a WWN on either side the path fallback stands. The seeded +source mapping reused after a restart is a separate path this does not cover. +`TestRegistrySASPathJoinRefusesContradictingWWN` covers the stale row and both +same-disk shapes; monitoring's SMART merge applies the matching guard first. + ### Drawer tab selection survives a transient snapshot change (#1723) The shared resource detail drawer keeps the operator's selected tab while a diff --git a/frontend-modern/browser-verification/0b7a94ad4faa1eb7.json b/frontend-modern/browser-verification/0b7a94ad4faa1eb7.json new file mode 100644 index 0000000000..0886986878 --- /dev/null +++ b/frontend-modern/browser-verification/0b7a94ad4faa1eb7.json @@ -0,0 +1,38 @@ +{ + "version": 1, + "base_sha": "337e908b3577351ece5c4ffdf51ff17380e09b4e", + "verified_at": "2026-10-06T19:43:25Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/features/alerts/AlertDeadManDestinationSection.tsx" + ], + "content_sha256": { + "frontend-modern/src/features/alerts/AlertDeadManDestinationSection.tsx": "022a2f4b9412890a66420f18bda6105400c29c6deedf185dc102eccc063323d0" + }, + "routes": [ + "/alerts/destinations" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Private mock stack (backend :7731, Vite :5251 serving this branch, merged with main 9aaac4d3e5) with Playwright page.clock installed before load and every /api/alerts/deadman/status read held until the script answers it", + "Reads slower than the poll: each poll starts a read while the previous is pending; answers applied in order (10 mins ago -> 3 mins ago -> 1 min ago), and an older read answered after a newer one did not overwrite it (stayed 1 min ago); every poll tick issued a read (no tick skipped to timer jitter)", + "A Refresh that never settles, followed by a background read that also never settles: Refresh showed Refreshing\u2026 disabled, then was enabled again at the first poll after it had been pending a full interval, and a second Refresh applied the fresh status", + "Earlier hang case still holds: with a hung background read, Refresh applies the fresh status and the hung read's late 2-hour-old answer is discarded", + "Control with the previous loader under the running Vite: slow reads never showed a status (every answer discarded) and the hung Refresh stayed disabled", + "No page errors at either viewport" + ], + "interactions": [ + "Loaded /alerts/destinations at 1440x900 and 390x844, ran the page clock in 30-second steps answering held reads in and out of order", + "Clicked Refresh with the read held, ran the clock 61 seconds, clicked Refresh again and answered it", + "Repeated with the previous loader as the control" + ] +} diff --git a/frontend-modern/browser-verification/294409f678589ef2.json b/frontend-modern/browser-verification/294409f678589ef2.json new file mode 100644 index 0000000000..726e7fa021 --- /dev/null +++ b/frontend-modern/browser-verification/294409f678589ef2.json @@ -0,0 +1,52 @@ +{ + "version": 1, + "base_sha": "41cea2ffab5ca2d39240d14138975bc4e902bb61", + "verified_at": "2026-10-07T11:28:58Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/components/Infrastructure/resourceDetailDrawerMetricsHistoryModel.ts", + "frontend-modern/src/components/Infrastructure/resourceDetailDrawerTrueNASModel.ts", + "frontend-modern/src/features/platformPage/sensorSetup.ts", + "frontend-modern/src/features/truenas/TrueNASStorageTopologyTable.tsx" + ], + "content_sha256": { + "frontend-modern/src/components/Infrastructure/resourceDetailDrawerMetricsHistoryModel.ts": "e6e8b46a156df4a1d1b391de19f041c0a1f74eb7e2ecdb78fdfa773a4c336262", + "frontend-modern/src/components/Infrastructure/resourceDetailDrawerTrueNASModel.ts": "fa1c743eaec09f424c80aa5d3aee9082817e54353a853b542620eb45792f5887", + "frontend-modern/src/features/platformPage/sensorSetup.ts": "5b92f1fc07b513a11f0e8f2654b0717baa4b99d742eae76368844731a5f89eb9", + "frontend-modern/src/features/truenas/TrueNASStorageTopologyTable.tsx": "dd7c5189a950adba606a7ea4d0d40233d158e965883135b6fe82b772a6dcf5cd" + }, + "routes": [ + "/truenas/storage?kind=disks", + "/proxmox/storage", + "/proxmox" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Vite dev server on this tree (origin/main 41cea2ffab plus the change) against a private PULSE_MOCK_MODE backend (8-node estate) built with a local-only go build -overlay (never committed). The overlay stamps TrueNAS disk sdb with physicalDisk.temperature 61 and collection.temperature unavailable 'host agent stopped reporting' (the shape a TrueNAS disk merged with a silent host agent row reaches), pins sdd to a current 58, gives pve3 SATA disk /dev/nvme2 the same retained state (51) and marks pve3's sensor payload legacySensorsFormat; /api/resources and the websocket rows carried those states", + "Before control in the same session (the four runtime files swapped to origin/main under the running Vite, then restored and compared byte for byte): sdb Temp cell plain 61.0°C sorting first in hottest-first, drawer Temperature row 61°C in the amber warning tone, TrueNAS summary 'SATA, Passed, 16.0 TB, 61°C', History with no smart_temp points showing '-', and no outdated sensor setup notice for pve3", + "After, TrueNAS storage disks view: sdb Temp cell '61.0°C' muted (rgb 71,85,105) with a dotted underline, title 'Last known reading, not current: host agent stopped reporting', data-temperature-reading=last-known and screen-reader ', last known'; current disks (sdd 58.0°C, sda 46.0°C) unchanged; no Attention reason for sdb", + "After, Temp sort: descending sdd 58, sdf 47, nvme0n1 46, sda 46, sde 42, nvme1n1 41, sdc 39, then sdb last; ascending sdc 39 ... sdd 58, then sdb last (a retained reading sorts as no reading)", + "After, sdb row drawer (Platform details > Show TrueNAS): Health Temperature row '61°C (last known)' in text-muted with the reason as its title; the collapsed TrueNAS summary reads 'SATA, Passed, 16.0 TB' with no temperature", + "After, sdb drawer History tab: with seeded points the legend shows the stored 53°C and the line 'Disk reading: Last known reading, not current: host agent stopped reporting.'; with the smart_temp series emptied by a Playwright route the legend reads 'Disk 61°C last known' (data-history-last-known=smart_temp, no 'current') with the same line; current disk sdd with no points reads 'Disk 58°C current'", + "After, /proxmox/storage: 'pve3 is using an older temperature monitoring setup that cannot read SATA/SAS disk temperatures. Re-run the node setup script to upgrade it.' notice shows at both widths (hidden on main); /proxmox Overview loads no physical disks so the notice cannot show there on main or after (pre-existing, chipped separately)", + "No page errors at 1440x900 or 390x844", + "Re-run after review edits (History model reads a disk's value only from physicalDisk, never a top-level fallback): every reading above was unchanged at both widths" + ], + "interactions": [ + "Opened /truenas/storage?kind=disks at 1440x900 and 390x844 and read every disk row's Temp cell (text, title, data attribute, computed colour and decoration)", + "Clicked the Temp column header twice at 1440x900 and read the disk row order for descending and ascending", + "Opened the sdb row drawer with 'Expand details for sdb', opened Platform details and clicked 'Show TrueNAS', read the summary and the Health Temperature row at both widths", + "Opened the drawer History tab with stored points, returned to Overview, then reopened History with the smart_temp series emptied, and read the Temperature legend and reason line at both widths; repeated for current disk sdd", + "Opened /proxmox and /proxmox/storage at both widths and read the outdated sensor setup notice", + "Ran the same Playwright pass against the origin/main runtime files as the before control, restored the change and confirmed the full git diff was byte-identical" + ] +} diff --git a/frontend-modern/browser-verification/314b4db153c663c6.json b/frontend-modern/browser-verification/314b4db153c663c6.json new file mode 100644 index 0000000000..dac88b007e --- /dev/null +++ b/frontend-modern/browser-verification/314b4db153c663c6.json @@ -0,0 +1,34 @@ +{ + "version": 1, + "base_sha": "26bee50a8a43015b751ce936aec3327c8c862005", + "verified_at": "2026-10-07T12:11:32Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/features/patrol/PatrolAttentionWorkbench.tsx" + ], + "content_sha256": { + "frontend-modern/src/features/patrol/PatrolAttentionWorkbench.tsx": "9ed4f1bf64a63c54336636786e54c1b7edcceea5f663056aec827c3cd9904b29" + }, + "routes": [ + "/patrol" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Combined content of PatrolAttentionWorkbench.tsx after merging origin/main (with 844e1dbb97, suppression-end labels that follow the review state) into this branch (relative-time clock for the open attention detail)", + "Private mock stack (backend :7741 built from the merged tree, Vite :5261 serving it) with Playwright page.clock installed before load; /ws data frames held with a data-free keep-alive per fake minute (one live socket), GETs replayed, collection refetches stalled", + "Patrol inbox: opened the first attention item with an injected acknowledgement and a forced suppressed state; the lifecycle card read 'Acknowledged by admin 3m ago' with an 'End suppression' button (no 'Return to active attention'), and evidence/timeline ages 3m/5m/7m moved to 13m/15m/17m after ten held minutes, at 1440 and 390", + "No page errors at either viewport; Patrol and Alerts suites (100 files, 1588 tests) and tsc pass on the merged tree" + ], + "interactions": [ + "Loaded /patrol, opened the first attention detail, held the stream and advanced the page clock ten one-minute steps, at both viewports" + ] +} diff --git a/frontend-modern/browser-verification/530561c68f6c71e9.json b/frontend-modern/browser-verification/530561c68f6c71e9.json new file mode 100644 index 0000000000..0a97bb0a6a --- /dev/null +++ b/frontend-modern/browser-verification/530561c68f6c71e9.json @@ -0,0 +1,51 @@ +{ + "version": 1, + "base_sha": "bffd21f47026ca2408ee1d0b5bb071c02e60c623", + "verified_at": "2026-10-06T15:46:26Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/features/alerts/AlertDeadManDestinationSection.tsx", + "frontend-modern/src/features/alerts/AlertDeliveryLogCard.tsx", + "frontend-modern/src/features/patrol/PatrolAttentionWorkbench.tsx", + "frontend-modern/src/features/patrol/PatrolIntelligenceBanners.tsx", + "frontend-modern/src/features/patrol/PatrolIntelligenceWorkspace.tsx" + ], + "content_sha256": { + "frontend-modern/src/features/alerts/AlertDeadManDestinationSection.tsx": "df5d337cbac9e091aa0e9dfaa2623a7169eedc7609829a76338f0e4208685e0b", + "frontend-modern/src/features/alerts/AlertDeliveryLogCard.tsx": "90ab80e79b7516c5693768657739ab1b6426615312e0dc03ff9c8636f39f35af", + "frontend-modern/src/features/patrol/PatrolAttentionWorkbench.tsx": "7cbb73c2b4e903ba9bb4c783fada2493189f325f312c23a13f600541520e8a79", + "frontend-modern/src/features/patrol/PatrolIntelligenceBanners.tsx": "2c5818b026c1976786bf858fddcbbce552217eed4410302db573cbebef3ac944", + "frontend-modern/src/features/patrol/PatrolIntelligenceWorkspace.tsx": "5d68a4b1e2904bdfb76245bfda24f77bd330424f830952b4d134b1931fba0518" + }, + "routes": [ + "/alerts/destinations", + "/patrol", + "/patrol/activity" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Private mock stack (backend :7731 built from this clone, Vite :5251 serving this branch, base bffd21f470, after #2578 landed) with Playwright page.clock installed before load; data held by dropping /ws data frames and replaying cached API GETs while a data-free keep-alive frame per fake minute keeps one live socket (no heartbeat reconnect)", + "Alerts > Destinations external watchdog with an injected healthy status (lastSuccessAt 30s, lastMonitoringProgress 20s before load; the mock has no watchdog): Last success 33s ago and Monitor progress 23s ago read 10 mins ago after 10 held minutes at 1440 and 390; 20 background re-reads of /api/alerts/deadman/status in those 10 minutes, Refresh button untouched", + "Same page, delivery log with one injected sent email attempt 5 minutes old: the row time tooltip moved 5 mins ago to 15 mins ago", + "Patrol activity with an injected blocked status (blocked_at 2 minutes before load): 'Blocked 2m ago' read 'Blocked 12m ago' after 10 held minutes", + "Patrol activity > Finding options and history > History, newest demo run start rewritten to 3 minutes before load: row read 3m ago then 13m ago; selecting it showed 'Patrol run 3m ago' and then 'Patrol run 13m ago' while held", + "Patrol inbox attention detail with injected acknowledgement, timeline and evidence times (3, 5 and 7 minutes before load): read 13m, 15m and 17m ago after 10 held minutes; 'Last seen' and the flapping window stay read-time by design", + "Same-session control: with these five runtime files swapped back to origin/main under the running Vite, the watchdog ages, delivery tooltip, Blocked banner, selected-run banner and attention detail ages stayed frozen (one status read, no background re-reads); only the run history row moved, because history re-reads on Patrol's 60-second refresh, so that row is not changed here", + "No page errors at either viewport" + ], + "interactions": [ + "Loaded /alerts/destinations at 1440x900 and 390x844, held the data stream and advanced the page clock ten one-minute steps", + "Loaded /patrol/activity, opened Finding options and history, opened History, selected the newest run, held and advanced ten minutes", + "Loaded /patrol, opened the first attention item's detail, held and advanced ten minutes", + "Repeated every scenario with the pre-change files as the control" + ] +} diff --git a/frontend-modern/browser-verification/9c0e53268f0d014f.json b/frontend-modern/browser-verification/9c0e53268f0d014f.json new file mode 100644 index 0000000000..fb11cc38ea --- /dev/null +++ b/frontend-modern/browser-verification/9c0e53268f0d014f.json @@ -0,0 +1,45 @@ +{ + "version": 1, + "base_sha": "a570d9f335cf8cd13a9df00a123a52234ec04a23", + "verified_at": "2026-10-07T07:58:00Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/features/alerts/AlertHistoryMobileList.tsx", + "frontend-modern/src/features/alerts/AlertHistoryTableAlertRow.tsx", + "frontend-modern/src/features/alerts/alertHistoryModel.ts" + ], + "content_sha256": { + "frontend-modern/src/features/alerts/AlertHistoryMobileList.tsx": "71cf89c4a4bba27a8efdf3332260d033273185359c45eb01a6dcf7317cf1b53f", + "frontend-modern/src/features/alerts/AlertHistoryTableAlertRow.tsx": "b2b4199c76b0ccee79fad658bf91cb44174d5dbe5491942e846dc196af06e608", + "frontend-modern/src/features/alerts/alertHistoryModel.ts": "5e81adf1ad8c42224f250fbf9dc5e9b13fbfa0ed92b96802bb1af56cd139cbd6" + }, + "routes": [ + "/alerts/history?q=pve2", + "/alerts/history?q=95.0" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Private mock stack built from this tree (main bea0cec8b9 + alert-card follow-up #2637 + this change), with an uncommitted generator overlay pinning pve2 CPU temperature; node temperature alert level 80\u00b0C, clear level 75\u00b0C, 5 minute recovery delay", + "History desktop row, held at 78\u00b0C: \"Temperature 78\u00b0C now, back under the 80\u00b0C alert level\", hover adds \"Stays open until it reaches 75\u00b0C or lower and stays there for 5 minutes.\" and \"Last reading at or above 80\u00b0C: 95\u00b0C\"; the breach message \"Node temperature at 95.0\u00b0C\" is not shown; light and dark", + "History phone card at 390px, held: live summary, clear rule and last breach as muted lines under it, no horizontal scroll, light and dark", + "History desktop row and phone card, recovering at 72\u00b0C: \"Temperature 72\u00b0C now, recovering\" with \"Clears after 5 minutes at 75\u00b0C or lower, 1 minute so far.\"", + "Search q=95.0 finds the held pve2 row through its recorded message while the row reads 72\u00b0C", + "Active non-threshold row (powered off) and resolved/acknowledged rows keep their recorded messages; no console errors on either route", + "Earlier pass on the pre-merge content (identical row code; the phone card now also carries main's moved-close line for closed rows): breaching at 95\u00b0C, Fahrenheit preference (172\u00b0F now, 176\u00b0F alert level, 203\u00b0F breach), resolved alert leaving the list" + ], + "interactions": [ + "Walked pve2 temperature 95 -> 78 -> 72 via the control file", + "Read description cell text and hover titles (desktop) and card paragraphs (phone) with Playwright in each phase", + "Loaded the q=pve2 and q=95.0 search URLs and collected console errors", + "Captured light and dark screenshots at 1440x900 and 390x844" + ] +} diff --git a/frontend-modern/browser-verification/d7b9435b593e965e.json b/frontend-modern/browser-verification/d7b9435b593e965e.json new file mode 100644 index 0000000000..23522425f0 --- /dev/null +++ b/frontend-modern/browser-verification/d7b9435b593e965e.json @@ -0,0 +1,38 @@ +{ + "version": 1, + "base_sha": "b81f0478d3d779f8c6e2be856f1e2b99acfc6437", + "verified_at": "2026-10-06T19:52:24Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/features/alerts/AlertDeadManDestinationSection.tsx" + ], + "content_sha256": { + "frontend-modern/src/features/alerts/AlertDeadManDestinationSection.tsx": "ba80adc764c5b0e8055345654bf4fb0bf0aaeb17338ad0c0f5be820ff680dea5" + }, + "routes": [ + "/alerts/destinations" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Private mock stack (backend :7731, Vite :5251 serving this branch) with Playwright page.clock installed before load and every /api/alerts/deadman/status read held until the script answers it, with a status or an HTTP 500", + "Failures slower than the poll: with two reads in flight the older one fails, and the badge changes from Heartbeat healthy to Status unavailable; the newest read then fails and an older read succeeds late, and the badge stays Status unavailable (no false recovery)", + "Successes slower than the poll still apply in order (10 mins ago -> 3 mins ago -> 1 min ago) and an older late success does not overwrite a newer one", + "A hung Refresh followed by a hung background read: the button is enabled again at the first poll after a full interval, and a second Refresh applies", + "Control with the previous loader under the running Vite: the slow failure was hidden and the badge stayed Heartbeat healthy", + "No page errors at either viewport" + ], + "interactions": [ + "Loaded /alerts/destinations at 1440x900 and 390x844, ran the page clock in 30-second steps answering held reads in and out of order with successes and HTTP 500s", + "Clicked Refresh with the read held, ran the clock 61 seconds, clicked Refresh again", + "Repeated with the previous loader as the control" + ] +} diff --git a/frontend-modern/browser-verification/e24ba99586e7ea96.json b/frontend-modern/browser-verification/e24ba99586e7ea96.json new file mode 100644 index 0000000000..9b91d5a3cd --- /dev/null +++ b/frontend-modern/browser-verification/e24ba99586e7ea96.json @@ -0,0 +1,36 @@ +{ + "version": 1, + "base_sha": "278c403d9674cb2eb2066707637e88770636658b", + "verified_at": "2026-10-06T16:44:58Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/features/alerts/AlertDeadManDestinationSection.tsx" + ], + "content_sha256": { + "frontend-modern/src/features/alerts/AlertDeadManDestinationSection.tsx": "8df8585fbe2838b19227a80af8304cdfdd022b31f73f9deada8cb235bc73d8a2" + }, + "routes": [ + "/alerts/destinations" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Private mock stack (backend :7731 built from this clone, Vite :5251 serving this branch) with Playwright page.clock installed before load and /api/alerts/deadman/status intercepted: the mount read answers a healthy status 10 minutes old, the first background read hangs until released and then answers a status 2 hours old, every later read answers a fresh status", + "After one 31-second poll the hung background read is pending; Refresh still issues a third read and applies it: Last success 10 mins ago -> 34s ago, Refresh enabled", + "Releasing the hung read's 2-hour-old answer leaves Last success at 34s ago (stale answer discarded), and the next poll issues a fourth read", + "Control with this file swapped back to the previous commit under the running Vite: Refresh did nothing (no third read, still 10 mins ago) and the hung read's stale answer won (2 hours ago)", + "No page errors at either viewport" + ], + "interactions": [ + "Loaded /alerts/destinations at 1440x900 and 390x844, ran the page clock 31 seconds, clicked Refresh, released the hung read, ran another 30 seconds", + "Repeated with the previous file as the control" + ] +} diff --git a/frontend-modern/browser-verification/f996b4691de6f0ce.json b/frontend-modern/browser-verification/f996b4691de6f0ce.json new file mode 100644 index 0000000000..e1b524165d --- /dev/null +++ b/frontend-modern/browser-verification/f996b4691de6f0ce.json @@ -0,0 +1,34 @@ +{ + "version": 1, + "base_sha": "104af6eb6e6894055bfff2b6dc8cc9e046920735", + "verified_at": "2026-10-06T19:21:15Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/features/patrol/PatrolAttentionWorkbench.tsx" + ], + "content_sha256": { + "frontend-modern/src/features/patrol/PatrolAttentionWorkbench.tsx": "a5666d3229bf78cc04eb16dd85c1b13ec3617e74d938ea08ac8d40a97bcf7ccb" + }, + "routes": [ + "/patrol" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Combined content of PatrolAttentionWorkbench.tsx after merging origin/main (with #2591, moved-alert presentation) into this branch (relative-time clock for the open attention detail)", + "Private mock stack (backend :7731, Vite :5251 serving the merged tree) with Playwright page.clock installed before load; /ws data frames held with a data-free keep-alive per fake minute (one live socket), GETs replayed, collection refetches stalled", + "Patrol inbox: opened the first attention item; injected acknowledgement, timeline and evidence times 3, 5 and 7 minutes before load read 'Acknowledged by admin 3m ago' and 3m/5m/7m, then 13m/15m/17m after ten held minutes, at 1440 and 390", + "No page errors at either viewport; Patrol, Alerts and run history suites (106 files) pass on the merged tree" + ], + "interactions": [ + "Loaded /patrol, opened the first attention detail, held the stream and advanced the page clock ten one-minute steps, at both viewports" + ] +} diff --git a/frontend-modern/src/components/Infrastructure/__tests__/resourceDetailDrawerMetricsHistoryModel.branchcov0712.test.ts b/frontend-modern/src/components/Infrastructure/__tests__/resourceDetailDrawerMetricsHistoryModel.branchcov0712.test.ts index bae3d26c0e..9fe9a0ac3b 100644 --- a/frontend-modern/src/components/Infrastructure/__tests__/resourceDetailDrawerMetricsHistoryModel.branchcov0712.test.ts +++ b/frontend-modern/src/components/Infrastructure/__tests__/resourceDetailDrawerMetricsHistoryModel.branchcov0712.test.ts @@ -513,6 +513,66 @@ describe('getResourceMetricsHistoryCurrentMetrics branch coverage', () => { }); }); + it('offers a retained physical disk temperature only as last known, with its reason', () => { + // useUnifiedResources copies physicalDisk.temperature onto the resource. + // A disk in standby keeps its last reading under an unavailable state, so + // with no history points the chart must not label it current. + const disk = (state?: 'available' | 'unavailable', temperature = 47) => + baseResource({ + type: 'physical_disk', + metricsTarget: { resourceType: 'disk', resourceId: 'SERIAL-1' }, + temperature, + physicalDisk: { + devPath: '/dev/sdb', + temperature, + collection: state + ? { temperature: { state, source: 'smartctl', reason: 'disk is in standby' } } + : undefined, + }, + }); + + expect(getResourceMetricsHistoryCurrentMetrics(disk('unavailable'))).toMatchObject({ + smart_temp: undefined, + temperature: undefined, + }); + expect(getResourceMetricsHistoryDeferredMetrics(disk('unavailable')).smart_temp).toEqual({ + lastKnownValue: 47, + message: 'Last known reading, not current: disk is in standby.', + }); + // An unavailable state with no retained reading has nothing to offer. + expect(getResourceMetricsHistoryCurrentMetrics(disk('unavailable', 0)).smart_temp).toBe( + undefined, + ); + expect(getResourceMetricsHistoryDeferredMetrics(disk('unavailable', 0)).smart_temp).toBe( + undefined, + ); + + for (const current of [disk('available'), disk()]) { + expect(getResourceMetricsHistoryCurrentMetrics(current).smart_temp).toBe(47); + expect(getResourceMetricsHistoryDeferredMetrics(current).smart_temp).toBeUndefined(); + } + + // Websocket rows carry the reading only on physicalDisk, beside its state. + const websocketRow = (state: 'available' | 'unavailable') => { + const row = disk(state); + delete row.temperature; + return row; + }; + expect(getResourceMetricsHistoryCurrentMetrics(websocketRow('available')).smart_temp).toBe(47); + expect( + getResourceMetricsHistoryCurrentMetrics(websocketRow('unavailable')).smart_temp, + ).toBeUndefined(); + expect( + getResourceMetricsHistoryDeferredMetrics(websocketRow('unavailable')).smart_temp, + ).toMatchObject({ lastKnownValue: 47 }); + + // A top-level value never stands in for the disk's own missing reading, + // which the disk's collection state could not qualify. + const unpaired = disk('available'); + delete unpaired.physicalDisk!.temperature; + expect(getResourceMetricsHistoryCurrentMetrics(unpaired).smart_temp).toBeUndefined(); + }); + it('uses the source-aware host subset for API-only Proxmox node targets', () => { const resource = baseResource({ type: 'agent', diff --git a/frontend-modern/src/components/Infrastructure/__tests__/resourceDetailDrawerTrueNASModel.test.ts b/frontend-modern/src/components/Infrastructure/__tests__/resourceDetailDrawerTrueNASModel.test.ts index 539aee7b89..1afd532170 100644 --- a/frontend-modern/src/components/Infrastructure/__tests__/resourceDetailDrawerTrueNASModel.test.ts +++ b/frontend-modern/src/components/Infrastructure/__tests__/resourceDetailDrawerTrueNASModel.test.ts @@ -361,6 +361,43 @@ describe('resourceDetailDrawerTrueNASModel', () => { ]); }); + it('shows a retained disk temperature as last known, with its reason and no heat tone', () => { + // A TrueNAS disk merged with a host agent's row keeps the agent's last + // reading after the agent stops reporting. It is not current, so it must + // not read as a disk running hot now, nor sit in the summary as a reading. + const disk = (state: 'available' | 'unavailable') => + baseResource({ + type: 'physical_disk', + displayName: 'sdc', + platformScopes: ['truenas'], + physicalDisk: { + devPath: '/dev/sdc', + diskType: 'sata', + sizeBytes: 24 * 1024 ** 4, + health: 'PASSED', + temperature: 61, + collection: { + temperature: { state, source: 'agent', reason: 'host agent stopped reporting' }, + }, + }, + }); + const temperatureRow = (resource: Resource) => + buildTrueNASDetailSections(resource) + .find((section) => section.label === 'Health') + ?.rows.find((row) => row.label === 'Temperature'); + + expect(temperatureRow(disk('unavailable'))).toMatchObject({ + value: '61°C (last known)', + tone: 'muted', + title: 'Last known reading, not current: host agent stopped reporting', + }); + expect(buildTrueNASDetailsSummary(disk('unavailable'))).toBe('SATA, Passed, 24.0 TB'); + + expect(temperatureRow(disk('available'))).toMatchObject({ value: '61°C', tone: 'warning' }); + expect(temperatureRow(disk('available'))?.title).toBeUndefined(); + expect(buildTrueNASDetailsSummary(disk('available'))).toBe('SATA, Passed, 24.0 TB, 61°C'); + }); + it('shows the vdev layout as the storage kind while topology stays the pool discriminator', () => { // Regression guard for 599c8e634. `topology` is the cross-provider // discriminator ('pool' | 'dataset' | ...); the ZFS layout belongs in diff --git a/frontend-modern/src/components/Infrastructure/resourceDetailDrawerMetricsHistoryModel.ts b/frontend-modern/src/components/Infrastructure/resourceDetailDrawerMetricsHistoryModel.ts index 2fc02a3e4c..8d4ee55a4b 100644 --- a/frontend-modern/src/components/Infrastructure/resourceDetailDrawerMetricsHistoryModel.ts +++ b/frontend-modern/src/components/Infrastructure/resourceDetailDrawerMetricsHistoryModel.ts @@ -14,6 +14,10 @@ import { import type { HostGPUSensor } from '@/types/api'; import type { Resource } from '@/types/resource'; import { getDiskPercent, getMemoryPercent } from '@/types/resource'; +import { + getPhysicalDiskTemperaturePresentation, + isPhysicalDiskTemperatureCurrent, +} from '@/features/storageBackups/diskTemperaturePresentation'; import { asTrimmedString } from '@/utils/stringUtils'; import { getMemoryObservationPresentation } from '@/utils/memoryObservation'; import { getWorkloadGuestDiskStatusMessage } from '@/utils/workloadGuestPresentation'; @@ -174,6 +178,18 @@ const diskReadReason = (resource: Resource): string | undefined => ? resource.proxmox?.diskStatusReason || undefined : undefined; +// A physical disk's reading and its collection state both live on +// physicalDisk, so read them only together: websocket rows carry no top-level +// temperature for a disk, which only the REST mapper copies across. The +// reading may be a last known one kept while the current observation could +// not collect it, such as a disk in standby or a host agent past its +// reporting lease, and is then never current. +const resourceTemperature = (resource: Resource): number | undefined => + resource.physicalDisk ? resource.physicalDisk.temperature : resource.temperature; + +const isResourceTemperatureCurrent = (resource: Resource): boolean => + isPhysicalDiskTemperatureCurrent(resource.physicalDisk?.collection); + export const getResourceMetricsHistoryDeferredMetrics = ( resource: Resource, ): Record => { @@ -197,6 +213,19 @@ export const getResourceMetricsHistoryDeferredMetrics = ( message: getWorkloadGuestDiskStatusMessage(reason), }; } + if (!isResourceTemperatureCurrent(resource)) { + const temperature = resourceTemperature(resource); + const retained = getPhysicalDiskTemperaturePresentation({ + temperature: temperature ?? 0, + collection: resource.physicalDisk?.collection, + }); + if (retained?.title) { + metrics.smart_temp = { + lastKnownValue: temperature, + message: `${retained.title}.`, + }; + } + } return metrics; }; @@ -204,7 +233,9 @@ export const getResourceMetricsHistoryCurrentMetrics = ( resource: Resource, ): Record => { const diskPercent = resource.disk ? finiteMetric(getDiskPercent(resource)) : undefined; - const temperature = finiteMetric(resource.temperature); + const temperature = isResourceTemperatureCurrent(resource) + ? finiteMetric(resourceTemperature(resource)) + : undefined; const memory = memoryReading(resource); return { cpu: finiteMetric(resource.cpu?.current), diff --git a/frontend-modern/src/components/Infrastructure/resourceDetailDrawerTrueNASModel.ts b/frontend-modern/src/components/Infrastructure/resourceDetailDrawerTrueNASModel.ts index f8a666aa5d..57d3fca8c9 100644 --- a/frontend-modern/src/components/Infrastructure/resourceDetailDrawerTrueNASModel.ts +++ b/frontend-modern/src/components/Infrastructure/resourceDetailDrawerTrueNASModel.ts @@ -20,6 +20,7 @@ import { type DetailSection, type DetailValueTone, } from '@/components/shared/detailSectionModel'; +import { getPhysicalDiskTemperaturePresentation } from '@/features/storageBackups/diskTemperaturePresentation'; import { hasImpairedResourceSource } from '@/utils/resourceSourceHealth'; export type ResourceDetailDrawerTrueNASRowTone = DetailValueTone; @@ -51,11 +52,6 @@ const formatPercent = (percent?: number): string | null => { return `${percent.toFixed(percent >= 10 ? 1 : 2)}%`; }; -const formatTemperature = (celsius?: number): string | null => { - if (typeof celsius !== 'number' || !Number.isFinite(celsius) || celsius <= 0) return null; - return `${celsius.toFixed(0)}°C`; -}; - const formatDurationSeconds = (seconds?: number): string | null => { const value = asPositiveNumber(seconds); if (!value) return null; @@ -396,6 +392,35 @@ const formatDiskHours = (hours?: number): string | null => { return `${formatDetailIntegerValue(value) ?? value.toFixed(0)}h`; }; +const diskTemperature = (disk: ResourcePhysicalDiskMeta) => + getPhysicalDiskTemperaturePresentation({ + temperature: disk.temperature ?? 0, + collection: disk.collection, + }); + +// A retained reading, such as a silent host agent's on a merged disk row, is +// not current: it takes no heat tone and its title says why. +const diskTemperatureRow = ( + disk: ResourcePhysicalDiskMeta, +): ResourceDetailDrawerTrueNASRow | null => { + const reading = diskTemperature(disk); + if (!reading) return null; + if (!reading.current) { + return row('Temperature', `${reading.label} (last known)`, { + title: reading.title, + tone: 'muted', + }); + } + return row('Temperature', reading.label, { + tone: (disk.temperature ?? 0) >= 55 ? 'warning' : 'default', + }); +}; + +const currentDiskTemperatureLabel = (disk: ResourcePhysicalDiskMeta): string | null => { + const reading = diskTemperature(disk); + return reading?.current ? reading.label : null; +}; + const buildTrueNASDiskSections = ( resource: Resource, disk: ResourcePhysicalDiskMeta, @@ -414,9 +439,7 @@ const buildTrueNASDiskSections = ( const healthRows = compactRows([ row('Health', normalizeDelimitedLabel(disk.health), { tone: diskStateTone(disk) }), - row('Temperature', formatTemperature(disk.temperature), { - tone: disk.temperature && disk.temperature >= 55 ? 'warning' : 'default', - }), + diskTemperatureRow(disk), row( 'Wearout', disk.wearout === undefined || disk.wearout < 0 ? null : formatPercent(disk.wearout), @@ -795,7 +818,8 @@ export const buildTrueNASDetailsSummary = (resource: Resource): string | null => diskTypeLabel(disk.diskType), normalizeDelimitedLabel(disk.health), formatDetailBytesValue(disk.sizeBytes), - formatTemperature(disk.temperature), + // The summary has no room to say a retained reading is not current. + currentDiskTemperatureLabel(disk), ].filter((value): value is string => Boolean(value)); return summary.length > 0 ? summary.join(', ') : null; } diff --git a/frontend-modern/src/features/alerts/AlertDeadManDestinationSection.tsx b/frontend-modern/src/features/alerts/AlertDeadManDestinationSection.tsx index 83174ca881..7347d76fb2 100644 --- a/frontend-modern/src/features/alerts/AlertDeadManDestinationSection.tsx +++ b/frontend-modern/src/features/alerts/AlertDeadManDestinationSection.tsx @@ -1,4 +1,4 @@ -import { createSignal, createUniqueId, onMount, Show, type Accessor } from 'solid-js'; +import { createSignal, createUniqueId, onCleanup, onMount, Show, type Accessor } from 'solid-js'; import { AlertsAPI } from '@/api/alerts'; import { Button } from '@/components/shared/Button'; @@ -6,6 +6,7 @@ import { SettingsPanel } from '@/components/shared/SettingsPanel'; import type { DeadManStatus } from '@/types/alerts'; import { formatRelativeTime } from '@/utils/format'; import { logger } from '@/utils/logger'; +import { useRelativeTimeNow } from '@/utils/relativeTimeClock'; interface AlertDeadManDestinationSectionProps { pingUrl: Accessor; @@ -14,6 +15,13 @@ interface AlertDeadManDestinationSectionProps { } const REDACTED_PING_URL = '***REDACTED***'; +// Pulse pings the watchdog every minute, so a status read once at mount goes +// stale while the panel stays open. +export const DEAD_MAN_STATUS_POLL_MS = 30_000; +// A read pending this long at a poll tick counts as a full interval old. The +// slack absorbs timer jitter: the next tick can land a few milliseconds short +// of a full interval after the read the previous tick started. +const DEAD_MAN_STATUS_STALE_MS = DEAD_MAN_STATUS_POLL_MS - 1_000; const statusPresentation: Record = { disabled: { @@ -54,21 +62,72 @@ export function AlertDeadManDestinationSection(props: AlertDeadManDestinationSec const [unavailable, setUnavailable] = createSignal(false); const [showUrl, setShowUrl] = createSignal(false); - const loadStatus = async () => { - if (loading()) return; - setLoading(true); + // The status read has no timeout. Every read gets a sequence number, and a + // read's outcome applies only when it is newer than the last read that + // settled: a success shows its status and clears the unavailable mark, a + // failure sets it. A slow or hung read therefore cannot overwrite a newer + // outcome or block later reads, and reads slower than the poll interval + // still land in order. A background read waits while the newest read is + // younger than one poll interval (less a second of timer slack); a Refresh + // still pending at such a poll counts as stalled and frees the button. + // Nothing applies after the panel unmounts. + let latestStarted = 0; + let latestSettled = 0; + let pendingSince: number | undefined; + let foregroundRequest = 0; + let foregroundSince = 0; + let disposed = false; + onCleanup(() => { + disposed = true; + }); + const loadStatus = async (options: { background?: boolean } = {}) => { + const startedAt = Date.now(); + if (options.background) { + if (loading() && startedAt - foregroundSince >= DEAD_MAN_STATUS_STALE_MS) { + setLoading(false); + } + if (pendingSince !== undefined && startedAt - pendingSince < DEAD_MAN_STATUS_STALE_MS) { + return; + } + } else if (loading()) { + return; + } + const request = ++latestStarted; + pendingSince = startedAt; + if (!options.background) { + foregroundRequest = request; + foregroundSince = startedAt; + setLoading(true); + } try { - setStatus(await AlertsAPI.getDeadManStatus()); + const next = await AlertsAPI.getDeadManStatus(); + if (disposed || request < latestSettled) return; + latestSettled = request; + setStatus(next); setUnavailable(false); } catch (error) { + if (disposed || request < latestSettled) return; + latestSettled = request; logger.error('Failed to load external watchdog status', error); setUnavailable(true); } finally { - setLoading(false); + if (!disposed) { + if (request === latestStarted) pendingSince = undefined; + // A Refresh is answered by its own read or by any newer one. + if (request >= foregroundRequest) setLoading(false); + } } }; - onMount(() => void loadStatus()); + // Last success and Monitor progress keep aging on the shared clock, and the + // status is re-read in the background so a moving age never describes a + // heartbeat that has since been sent. + const now = useRelativeTimeNow(); + onMount(() => { + void loadStatus(); + const timer = setInterval(() => void loadStatus({ background: true }), DEAD_MAN_STATUS_POLL_MS); + onCleanup(() => clearInterval(timer)); + }); const presentation = () => { const current = status(); @@ -162,7 +221,10 @@ export function AlertDeadManDestinationSection(props: AlertDeadManDestinationSec
Last success
{current().lastSuccessAt - ? formatRelativeTime(current().lastSuccessAt, { emptyText: 'Never' }) + ? formatRelativeTime(current().lastSuccessAt, { + emptyText: 'Never', + now: now(), + }) : 'Never'}
@@ -170,7 +232,10 @@ export function AlertDeadManDestinationSection(props: AlertDeadManDestinationSec
Monitor progress
{current().lastMonitoringProgress - ? formatRelativeTime(current().lastMonitoringProgress, { emptyText: 'Waiting' }) + ? formatRelativeTime(current().lastMonitoringProgress, { + emptyText: 'Waiting', + now: now(), + }) : 'Waiting'}
diff --git a/frontend-modern/src/features/alerts/AlertDeliveryLogCard.test.tsx b/frontend-modern/src/features/alerts/AlertDeliveryLogCard.test.tsx index 4fb6b60f9a..47d20243a8 100644 --- a/frontend-modern/src/features/alerts/AlertDeliveryLogCard.test.tsx +++ b/frontend-modern/src/features/alerts/AlertDeliveryLogCard.test.tsx @@ -4,6 +4,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import type { NotificationDeliveryLog, Webhook } from '@/api/notifications'; import type { AlertEvent } from '@/types/api'; +import { RELATIVE_TIME_TICK_MS } from '@/utils/relativeTimeClock'; import { AlertDeliveryLogCard, mergeDeliveryLogRows } from './AlertDeliveryLogCard'; @@ -437,4 +438,36 @@ describe('AlertDeliveryLogCard', () => { expect(screen.getByRole('status')).toHaveTextContent('Loading delivery attempts...'); expect(screen.queryByText(/No alert deliveries were attempted/)).not.toBeInTheDocument(); }); + + it("keeps each row's age tooltip moving while the log is not refreshed", () => { + vi.useFakeTimers({ + toFake: ['Date', 'setInterval', 'clearInterval'], + now: Date.parse('2026-08-27T12:05:00Z'), + }); + + try { + render(() => ( + + )); + const time = () => document.querySelector('time[datetime="2026-08-27T12:00:00Z"]'); + + expect(time()).toHaveAttribute('title', '5 mins ago'); + + // The log is not re-read: the entry never changes and only the clock moves. + vi.advanceTimersByTime(2 * 60 * 2 * RELATIVE_TIME_TICK_MS); + + expect(time()).toHaveAttribute('title', '2 hours ago'); + } finally { + vi.useRealTimers(); + } + }); }); diff --git a/frontend-modern/src/features/alerts/AlertDeliveryLogCard.tsx b/frontend-modern/src/features/alerts/AlertDeliveryLogCard.tsx index cb1be99e27..74953bfc06 100644 --- a/frontend-modern/src/features/alerts/AlertDeliveryLogCard.tsx +++ b/frontend-modern/src/features/alerts/AlertDeliveryLogCard.tsx @@ -21,6 +21,7 @@ import { getAlertDestinationsDeliveryRefreshLabel, } from '@/utils/alertDestinationsPresentation'; import { formatRelativeTime } from '@/utils/format'; +import { useRelativeTimeNow } from '@/utils/relativeTimeClock'; import { describeAlertEventReason } from './deliveryDiagnosisPresentation'; @@ -76,6 +77,9 @@ const outcomeBadgeClasses: Record { @@ -215,7 +219,7 @@ export function AlertDeliveryLogCard(props: AlertDeliveryLogCardProps) { @@ -259,7 +263,7 @@ export function AlertDeliveryLogCard(props: AlertDeliveryLogCardProps) { diff --git a/frontend-modern/src/features/alerts/AlertHistoryMobileList.tsx b/frontend-modern/src/features/alerts/AlertHistoryMobileList.tsx index 0cffed81e0..beee22a902 100644 --- a/frontend-modern/src/features/alerts/AlertHistoryMobileList.tsx +++ b/frontend-modern/src/features/alerts/AlertHistoryMobileList.tsx @@ -1,5 +1,6 @@ import { Show, createMemo, createSignal, onCleanup, onMount } from 'solid-js'; import { aiChatStore } from '@/stores/aiChat'; +import { useRelativeTimeNow } from '@/utils/relativeTimeClock'; import { getAlertHistoryResourceTypeBadgeClass } from '@/utils/alertHistoryPresentation'; import { @@ -8,6 +9,7 @@ import { } from '@/utils/alertIncidentPresentation'; import { AlertHistoryItemActions } from './AlertHistoryItemActions'; +import { getAlertHistoryRowCopy } from './alertHistoryModel'; import { PlatformWindowedList } from '@/features/platformPage/PlatformWindowedList'; import { getGroupSummaryLabel } from './AlertHistoryTableGroupRow'; import { @@ -30,6 +32,9 @@ export function AlertHistoryMobileList(props: AlertHistoryMobileListProps) { const [investigation, setInvestigation] = createSignal( null, ); + // Open threshold alerts lead with their live reading, which turns stale when + // evaluations stop, so it rechecks on the shared clock. + const now = useRelativeTimeNow(); const mobileItems = createMemo(() => { const items: AlertHistoryMobileItem[] = []; @@ -199,6 +204,7 @@ export function AlertHistoryMobileList(props: AlertHistoryMobileListProps) { const alert = item.alert; const rowKey = () => props.state.getIncidentRowKey(alert); const status = () => getAlertHistoryStatusPresentation(alert.status); + const descriptionCopy = () => getAlertHistoryRowCopy(alert, now); return (
-

{alert.description}

+

+ {descriptionCopy().text} +

+ {/* A phone has no hover, so what clears the alert and its last + breach stay visible under the live reading. */} + + {(detail) =>

{detail()}

} + + + {(lastBreach) => ( +

{lastBreach()}

+ )} + {(detail) =>

{detail()}

} diff --git a/frontend-modern/src/features/alerts/AlertHistoryTableAlertRow.tsx b/frontend-modern/src/features/alerts/AlertHistoryTableAlertRow.tsx index 13d1b3b797..b760d3ae24 100644 --- a/frontend-modern/src/features/alerts/AlertHistoryTableAlertRow.tsx +++ b/frontend-modern/src/features/alerts/AlertHistoryTableAlertRow.tsx @@ -8,8 +8,10 @@ import { getAlertHistoryStatusPresentation, getAlertIncidentLevelBadgeClass, } from '@/utils/alertIncidentPresentation'; +import { useRelativeTimeNow } from '@/utils/relativeTimeClock'; import { AlertHistoryItemActions } from './AlertHistoryItemActions'; +import { getAlertHistoryRowCopy } from './alertHistoryModel'; import { AlertResourceIncidentsPanel } from './AlertResourceIncidentsPanel'; import type { AlertHistoryState } from './useAlertHistoryState'; @@ -23,6 +25,10 @@ interface AlertHistoryTableAlertRowProps { export function AlertHistoryTableAlertRow(props: AlertHistoryTableAlertRowProps) { const rowKey = () => props.state.getIncidentRowKey(props.alert); const historyStatusPresentation = () => getAlertHistoryStatusPresentation(props.alert.status); + // An open threshold alert's live reading turns stale when evaluations stop, + // so it rechecks on the shared clock; the last breach moves to the hover. + const now = useRelativeTimeNow(); + const descriptionCopy = () => getAlertHistoryRowCopy(props.alert, now); return ( <> @@ -62,9 +68,9 @@ export function AlertHistoryTableAlertRow(props: AlertHistoryTableAlertRowProps) - {props.alert.description} + {descriptionCopy().text} ({ AlertsAPI: { @@ -113,4 +116,245 @@ describe('AlertDeadManDestinationSection', () => { screen.getByText('Saved external watchdog configuration could not be read'), ).toBeInTheDocument(); }); + + it('keeps Last success aging and re-reads the status while the panel stays open', async () => { + const start = Date.parse('2026-08-27T12:00:30Z'); + vi.useFakeTimers({ toFake: ['Date', 'setInterval', 'clearInterval'], now: start }); + const [pingUrl, setPingUrl] = createSignal('***REDACTED***'); + + try { + render(() => ( + + )); + const lastSuccess = () => screen.getByText('Last success').nextElementSibling; + await waitFor(() => expect(lastSuccess()).toHaveTextContent('30s ago')); + expect(AlertsAPI.getDeadManStatus).toHaveBeenCalledTimes(1); + + // The watchdog stops being reached: every background re-read returns the + // same last success, and its age keeps moving on the clock. + for (let poll = 2; poll <= 5; poll += 1) { + vi.advanceTimersByTime(DEAD_MAN_STATUS_POLL_MS); + await waitFor(() => expect(AlertsAPI.getDeadManStatus).toHaveBeenCalledTimes(poll)); + } + expect(lastSuccess()).toHaveTextContent('2 mins ago'); + expect(screen.getByRole('button', { name: 'Refresh' })).not.toBeDisabled(); + + // A later heartbeat lands: the next background re-read picks it up. + vi.mocked(AlertsAPI.getDeadManStatus).mockResolvedValue({ + ...(await AlertsAPI.getDeadManStatus()), + lastSuccessAt: '2026-08-27T12:02:30Z', + }); + vi.advanceTimersByTime(DEAD_MAN_STATUS_POLL_MS); + await waitFor(() => expect(lastSuccess()).toHaveTextContent('30s ago')); + } finally { + vi.useRealTimers(); + } + }); + + it('keeps Refresh working and ignores a stale answer when a background read hangs', async () => { + const start = Date.parse('2026-08-27T12:00:30Z'); + vi.useFakeTimers({ toFake: ['Date', 'setInterval', 'clearInterval'], now: start }); + const base = await AlertsAPI.getDeadManStatus(); + vi.mocked(AlertsAPI.getDeadManStatus).mockClear(); + let releaseHungRead: (value: typeof base) => void = () => undefined; + vi.mocked(AlertsAPI.getDeadManStatus) + .mockResolvedValueOnce(base) + .mockImplementationOnce( + () => + new Promise((resolve) => { + releaseHungRead = resolve; + }), + ) + .mockResolvedValue({ ...base, lastSuccessAt: '2026-08-27T12:00:55Z' }); + const [pingUrl, setPingUrl] = createSignal('***REDACTED***'); + + try { + render(() => ( + + )); + const lastSuccess = () => screen.getByText('Last success').nextElementSibling; + await waitFor(() => expect(lastSuccess()).toHaveTextContent('30s ago')); + + // The first background read never settles. + vi.advanceTimersByTime(DEAD_MAN_STATUS_POLL_MS); + await waitFor(() => expect(AlertsAPI.getDeadManStatus).toHaveBeenCalledTimes(2)); + + // Refresh still reads and applies the newer status. + fireEvent.click(screen.getByRole('button', { name: 'Refresh' })); + await waitFor(() => expect(AlertsAPI.getDeadManStatus).toHaveBeenCalledTimes(3)); + await waitFor(() => expect(lastSuccess()).toHaveTextContent('5s ago')); + expect(screen.getByRole('button', { name: 'Refresh' })).not.toBeDisabled(); + + // The hung read finally answers with the older status; it must not win. + releaseHungRead(base); + await Promise.resolve(); + await Promise.resolve(); + expect(lastSuccess()).toHaveTextContent('5s ago'); + + // Background reads keep going after the hang. + vi.advanceTimersByTime(DEAD_MAN_STATUS_POLL_MS); + await waitFor(() => expect(AlertsAPI.getDeadManStatus).toHaveBeenCalledTimes(4)); + } finally { + vi.useRealTimers(); + } + }); + + it('applies reads slower than the poll in order and never lets an older answer win', async () => { + const start = Date.parse('2026-08-27T12:10:00Z'); + vi.useFakeTimers({ toFake: ['Date', 'setInterval', 'clearInterval'], now: start }); + const base = await AlertsAPI.getDeadManStatus(); + vi.mocked(AlertsAPI.getDeadManStatus).mockReset(); + const pending: Array<(value: typeof base) => void> = []; + vi.mocked(AlertsAPI.getDeadManStatus).mockImplementation( + () => + new Promise((resolve) => { + pending.push(resolve); + }), + ); + const at = (time: string) => ({ ...base, lastSuccessAt: time }); + const [pingUrl, setPingUrl] = createSignal('***REDACTED***'); + + try { + render(() => ( + + )); + await waitFor(() => expect(pending).toHaveLength(1)); + + // Every read takes longer than one poll interval. + vi.advanceTimersByTime(DEAD_MAN_STATUS_POLL_MS); + await waitFor(() => expect(pending).toHaveLength(2)); + pending[0](at('2026-08-27T12:00:00Z')); + const lastSuccess = () => screen.getByText('Last success').nextElementSibling; + await waitFor(() => expect(lastSuccess()).toHaveTextContent('10 mins ago')); + + vi.advanceTimersByTime(DEAD_MAN_STATUS_POLL_MS); + await waitFor(() => expect(pending).toHaveLength(3)); + pending[1](at('2026-08-27T12:10:00Z')); + await waitFor(() => expect(lastSuccess()).toHaveTextContent('1 min ago')); + + // A newer read answers before an older one: the older answer is dropped. + vi.advanceTimersByTime(DEAD_MAN_STATUS_POLL_MS); + await waitFor(() => expect(pending).toHaveLength(4)); + pending[3](at('2026-08-27T12:11:30Z')); + await waitFor(() => expect(lastSuccess()).toHaveTextContent('0s ago')); + pending[2](at('2026-08-27T11:00:00Z')); + await Promise.resolve(); + await Promise.resolve(); + expect(lastSuccess()).toHaveTextContent('0s ago'); + } finally { + vi.useRealTimers(); + } + }); + + it('frees a Refresh that hangs for a whole poll interval', async () => { + const start = Date.parse('2026-08-27T12:00:30Z'); + vi.useFakeTimers({ toFake: ['Date', 'setInterval', 'clearInterval'], now: start }); + const base = await AlertsAPI.getDeadManStatus(); + vi.mocked(AlertsAPI.getDeadManStatus).mockReset(); + vi.mocked(AlertsAPI.getDeadManStatus) + .mockResolvedValueOnce(base) + .mockImplementationOnce(() => new Promise(() => undefined)) + .mockImplementationOnce(() => new Promise(() => undefined)) + .mockResolvedValue({ ...base, lastSuccessAt: '2026-08-27T12:00:55Z' }); + const [pingUrl, setPingUrl] = createSignal('***REDACTED***'); + + try { + render(() => ( + + )); + const lastSuccess = () => screen.getByText('Last success').nextElementSibling; + await waitFor(() => expect(lastSuccess()).toHaveTextContent('30s ago')); + + fireEvent.click(screen.getByRole('button', { name: 'Refresh' })); + await waitFor(() => + expect(screen.getByRole('button', { name: 'Refreshing…' })).toBeDisabled(), + ); + + // The Refresh read never settles, and neither does the background read + // that the next poll starts. The button is usable again after one poll + // interval, and a second Refresh brings the newer status. + vi.advanceTimersByTime(DEAD_MAN_STATUS_POLL_MS); + await waitFor(() => expect(AlertsAPI.getDeadManStatus).toHaveBeenCalledTimes(3)); + await waitFor(() => + expect(screen.getByRole('button', { name: 'Refresh' })).not.toBeDisabled(), + ); + fireEvent.click(screen.getByRole('button', { name: 'Refresh' })); + await waitFor(() => expect(lastSuccess()).toHaveTextContent('5s ago')); + } finally { + vi.useRealTimers(); + } + }); + + it('lets the newest settled read decide availability, even when reads outlast the poll', async () => { + const start = Date.parse('2026-08-27T12:10:00Z'); + vi.useFakeTimers({ toFake: ['Date', 'setInterval', 'clearInterval'], now: start }); + const base = await AlertsAPI.getDeadManStatus(); + vi.mocked(AlertsAPI.getDeadManStatus).mockReset(); + const pending: Array<{ + resolve: (value: typeof base) => void; + reject: (error: Error) => void; + }> = []; + vi.mocked(AlertsAPI.getDeadManStatus).mockImplementation( + () => + new Promise((resolve, reject) => { + pending.push({ resolve, reject }); + }), + ); + const [pingUrl, setPingUrl] = createSignal('***REDACTED***'); + const badge = () => screen.getByText(/^(Status unavailable|Heartbeat healthy|Not configured)$/); + const settle = async () => { + await Promise.resolve(); + await Promise.resolve(); + }; + + try { + render(() => ( + + )); + await waitFor(() => expect(pending).toHaveLength(1)); + pending[0].resolve(base); + await waitFor(() => expect(badge()).toHaveTextContent('Heartbeat healthy')); + + // Every read now fails, and each failure arrives after the next poll has + // already started a newer read. The failures still mark the status. + vi.advanceTimersByTime(DEAD_MAN_STATUS_POLL_MS); + await waitFor(() => expect(pending).toHaveLength(2)); + vi.advanceTimersByTime(DEAD_MAN_STATUS_POLL_MS); + await waitFor(() => expect(pending).toHaveLength(3)); + pending[1].reject(new Error('timeout')); + await waitFor(() => expect(badge()).toHaveTextContent('Status unavailable')); + + // The newest read fails; an older read then succeeds late. The older + // success must not report a recovery. + vi.advanceTimersByTime(DEAD_MAN_STATUS_POLL_MS); + await waitFor(() => expect(pending).toHaveLength(4)); + pending[3].reject(new Error('timeout')); + await settle(); + pending[2].resolve(base); + await settle(); + expect(badge()).toHaveTextContent('Status unavailable'); + } finally { + vi.useRealTimers(); + } + }); }); diff --git a/frontend-modern/src/features/alerts/__tests__/AlertHistoryMobileList.test.tsx b/frontend-modern/src/features/alerts/__tests__/AlertHistoryMobileList.test.tsx index 38dbf0caea..555050cd16 100644 --- a/frontend-modern/src/features/alerts/__tests__/AlertHistoryMobileList.test.tsx +++ b/frontend-modern/src/features/alerts/__tests__/AlertHistoryMobileList.test.tsx @@ -6,6 +6,11 @@ import { aiChatStore } from '@/stores/aiChat'; import { AlertHistoryMobileList } from '../AlertHistoryMobileList'; import type { AlertHistoryState } from '../useAlertHistoryState'; +// Open rows offer the Assistant investigation, which needs the router. +vi.mock('@/components/Alerts/InvestigateAlertButton', () => ({ + InvestigateAlertButton: () => null, +})); + function createState() { const alert = { id: 'alert-1', @@ -138,6 +143,59 @@ describe('AlertHistoryMobileList', () => { expect(screen.getAllByRole('heading', { name: 'Resource incidents' })).toHaveLength(1); }); + it('leads an open threshold alert with its live reading and keeps the breach visible', () => { + const { state, setGroupedAlerts } = createState(); + const observedAt = new Date(Date.now() - 10_000).toISOString(); + setGroupedAlerts([ + { + label: 'Today (August 4th)', + fullLabel: 'Today, August 4th 2026', + alerts: [ + { + id: 'minipc::metric-threshold:temperature', + source: 'alert', + resourceId: 'node-1', + resourceName: 'minipc', + resourceType: 'node', + title: 'Temperature', + description: 'Node temperature at 95.0°C', + severity: 'warning', + status: 'active', + duration: '30m', + startTime: '2026-08-04T14:30:00.000Z', + node: 'minipc', + nodeDisplayName: 'minipc', + rawAlertType: 'temperature', + liveAlert: { + type: 'temperature', + value: 95, + lastSeen: new Date(Date.now() - 20 * 60 * 1000).toISOString(), + metricStatus: { + phase: 'latched', + value: 78, + unit: '°C', + observedAt, + trigger: 80, + recovery: 75, + recoveryDelaySeconds: 300, + }, + }, + } as never, + ], + }, + ]); + + render(() => ); + + expect(screen.getByText('Temperature 78°C now, back under the 80°C alert level')).toBeVisible(); + expect( + screen.getByText('Stays open until it reaches 75°C or lower and stays there for 5 minutes.'), + ).toBeVisible(); + // A phone has no hover, so the last breach stays on the card. + expect(screen.getByText(/^Last reading at or above 80°C: 95°C, /)).toBeVisible(); + expect(screen.queryByText('Node temperature at 95.0°C')).toBeNull(); + }); + it('keeps resource investigation scrolling outside the virtualized history row', async () => { const { state } = createState(); diff --git a/frontend-modern/src/features/alerts/__tests__/AlertHistoryTableAlertRow.test.tsx b/frontend-modern/src/features/alerts/__tests__/AlertHistoryTableAlertRow.test.tsx new file mode 100644 index 0000000000..03f55e75db --- /dev/null +++ b/frontend-modern/src/features/alerts/__tests__/AlertHistoryTableAlertRow.test.tsx @@ -0,0 +1,122 @@ +import { render, screen } from '@solidjs/testing-library'; +import { createSignal } from 'solid-js'; +import { createStore } from 'solid-js/store'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { Alert, MetricAlertStatus } from '@/types/api'; + +import { AlertHistoryTableAlertRow } from '../AlertHistoryTableAlertRow'; +import type { HistoryItem } from '../alertHistoryModel'; +import type { AlertHistoryState } from '../useAlertHistoryState'; + +// Open rows offer the Assistant investigation, which needs the router. +vi.mock('@/components/Alerts/InvestigateAlertButton', () => ({ + InvestigateAlertButton: () => null, +})); + +const NOW = Date.parse('2026-10-06T19:00:00Z'); + +function createState() { + const [expandedIncidents] = createSignal(new Set()); + const [resourceIncidentPanel] = createSignal(null); + return { + getIncidentRowKey: (alert: HistoryItem) => `${alert.id}-row`, + expandedIncidents, + resourceIncidentPanel, + formatAlertRowTime: () => '18:30', + formatAlertRowTimestamp: () => 'Tuesday, 6 October 2026 at 18:30:00', + } as unknown as AlertHistoryState; +} + +const historyItem = (overrides: Partial): HistoryItem => ({ + id: 'minipc::metric-threshold:temperature', + source: 'alert', + status: 'active', + startTime: '2026-10-06T18:30:00Z', + duration: '30m', + resourceName: 'minipc', + resourceType: 'Node', + resourceId: 'minipc', + node: 'minipc', + severity: 'warning', + title: 'Temperature', + rawAlertType: 'temperature', + description: 'Node temperature at 95.0°C', + ...overrides, +}); + +const renderRow = (alert: HistoryItem) => + render(() => ( + + + + +
+ )); + +describe('AlertHistoryTableAlertRow', () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(NOW); + }); + afterEach(() => { + vi.useRealTimers(); + }); + + // minipc on 2026-10-06: the node read 78°C under its 80°C trigger and the + // open row still said "Node temperature at 95.0°C". + it('leads an open threshold alert with the reading now and follows it live', () => { + const [liveAlert, setLiveAlert] = createStore< + Pick + >({ + type: 'temperature', + value: 95, + lastSeen: '2026-10-06T18:40:00Z', + metricStatus: { + phase: 'latched', + value: 78, + unit: '°C', + observedAt: new Date(NOW - 10_000).toISOString(), + trigger: 80, + recovery: 75, + recoveryDelaySeconds: 300, + }, + }); + renderRow(historyItem({ liveAlert })); + + const cell = screen.getByText('Temperature 78°C now, back under the 80°C alert level'); + expect(cell.getAttribute('title')?.split('\n')).toEqual([ + 'Temperature 78°C now, back under the 80°C alert level', + 'Stays open until it reaches 75°C or lower and stays there for 5 minutes.', + 'Last reading at or above 80°C: 95°C, 20 mins ago', + ]); + expect(screen.queryByText('Node temperature at 95.0°C')).toBeNull(); + + // The next evaluation reaches the clear level; the row follows it. + setLiveAlert('metricStatus', (status) => ({ + ...(status as MetricAlertStatus), + phase: 'recovering', + value: 72, + observedAt: new Date(NOW).toISOString(), + })); + expect(cell.textContent).toBe('Temperature 72°C now, recovering'); + + // Evaluations stop: on the shared clock the reading stops being "now". + vi.advanceTimersByTime(15 * 60 * 1000); + expect(cell.textContent).toMatch(/^Last reading: Temperature 72°C, /); + }); + + it('keeps the recorded message on a resolved row', () => { + renderRow( + historyItem({ + status: 'resolved', + endTime: '2026-10-06T18:50:00Z', + description: 'Resolved: Node temperature at 74.0°C', + }), + ); + expect(screen.getByText('Resolved: Node temperature at 74.0°C')).toHaveAttribute( + 'title', + 'Resolved: Node temperature at 74.0°C', + ); + }); +}); diff --git a/frontend-modern/src/features/alerts/__tests__/alertHistoryModel.test.ts b/frontend-modern/src/features/alerts/__tests__/alertHistoryModel.test.ts index 0a9bc19072..58aa21e381 100644 --- a/frontend-modern/src/features/alerts/__tests__/alertHistoryModel.test.ts +++ b/frontend-modern/src/features/alerts/__tests__/alertHistoryModel.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import type { Alert } from '@/types/api'; import type { Resource } from '@/types/resource'; @@ -11,6 +11,7 @@ import { buildSelectedBucketDetails, formatAlertHistoryDuration, getAlertBucketDurationLabel, + getAlertHistoryRowCopy, type HistoryItem, } from '../alertHistoryModel'; @@ -120,3 +121,114 @@ describe('alertHistoryModel', () => { expect(sorted.map((item) => item.id)).toEqual(['newest', 'alpha', 'mid', 'zeta']); }); }); + +// minipc on 2026-10-06: the node temperature alert opened at 95°C, the node +// then read 78°C under its 80°C trigger, and the History row for the open +// alert still said "Node temperature at 95.0°C". +describe('alert history rows for held threshold alerts', () => { + const NOW = Date.parse('2026-10-06T19:00:00Z'); + const heldAlert = (status?: Partial>): Alert => + ({ + id: 'homelab-minipc::metric-threshold:temperature', + type: 'temperature', + level: 'warning', + resourceId: 'homelab-minipc', + resourceName: 'minipc', + node: 'minipc', + instance: 'homelab', + message: 'Node temperature at 95.0°C', + value: 95, + threshold: 80, + startTime: '2026-10-06T18:30:00Z', + lastSeen: '2026-10-06T18:40:00Z', + acknowledged: false, + metricStatus: status + ? { + phase: 'latched', + value: 78, + unit: '°C', + observedAt: new Date(NOW - 10_000).toISOString(), + trigger: 80, + recovery: 75, + recoveryDelaySeconds: 300, + ...status, + } + : undefined, + }) as Alert; + const build = (activeAlerts: Record, alertHistory: Alert[] = []) => + buildAlertHistoryItems({ + activeAlerts, + alertHistory, + getResource: () => undefined, + allResources: [], + now: NOW, + }); + + it('leads the open row with the live reading and keeps the breach for hover', () => { + const alert = heldAlert({ phase: 'latched', value: 78 }); + const [item] = build({ [alert.id]: alert }); + + // The record itself is unchanged: search and the Assistant handoff read it. + expect(item.description).toBe('Node temperature at 95.0°C'); + const copy = getAlertHistoryRowCopy(item, () => NOW); + expect(copy.text).toBe('Temperature 78°C now, back under the 80°C alert level'); + expect(copy.detail).toBe( + 'Stays open until it reaches 75°C or lower and stays there for 5 minutes.', + ); + expect(copy.lastBreach).toMatch(/^Last reading at or above 80°C: 95°C, /); + expect(copy.title.split('\n')).toEqual([copy.text, copy.detail, copy.lastBreach]); + }); + + it('says a recovering open row is recovering, with its progress', () => { + const alert = heldAlert({ phase: 'recovering', value: 72, recoveryElapsedSeconds: 130 }); + const copy = getAlertHistoryRowCopy(build({ [alert.id]: alert })[0], () => NOW); + expect(copy.text).toBe('Temperature 72°C now, recovering'); + expect(copy.detail).toBe('Clears after 5 minutes at 75°C or lower, 2 minutes so far.'); + }); + + it('stops calling a reading "now" once evaluations stop', () => { + const alert = heldAlert({ phase: 'latched', value: 78 }); + const [item] = build({ [alert.id]: alert }); + const copy = getAlertHistoryRowCopy(item, () => NOW + 15 * 60 * 1000); + expect(copy.text).toMatch(/^Last reading: Temperature 78°C, /); + expect(copy.text).not.toContain('now'); + }); + + it('keeps the recorded message on closed rows and open rows without a live status', () => { + const open = heldAlert(); + const closed = { + ...heldAlert({ phase: 'latched', value: 78 }), + id: 'homelab-minipc::metric-threshold:cpu', + message: 'Resolved: Node temperature at 74.0°C', + }; + const clock = vi.fn(() => NOW); + const items = build({ [open.id]: open }, [closed]); + const openItem = items.find((item) => item.id === open.id)!; + const closedItem = items.find((item) => item.id === closed.id)!; + + expect(closedItem.status).toBe('resolved'); + expect(closedItem.liveAlert).toBeUndefined(); + expect(getAlertHistoryRowCopy(closedItem, clock)).toEqual({ + text: 'Resolved: Node temperature at 74.0°C', + title: 'Resolved: Node temperature at 74.0°C', + }); + // Closed rows never read the clock, so they do not re-render on its tick. + expect(clock).not.toHaveBeenCalled(); + expect(getAlertHistoryRowCopy(openItem, clock)).toEqual({ + text: 'Node temperature at 95.0°C', + title: 'Node temperature at 95.0°C', + }); + }); + + it('gives Pulse system alerts no live reading', () => { + const alert = { + ...heldAlert({ phase: 'latched', value: 78 }), + id: 'pulse-system::storage', + metadata: { systemAlert: true }, + } as Alert; + const [item] = build({ [alert.id]: alert }); + expect(item.systemAlert).toBe(true); + expect(item.liveAlert).toBeUndefined(); + expect(getAlertHistoryRowCopy(item, () => NOW).text).toBe('Node temperature at 95.0°C'); + }); +}); diff --git a/frontend-modern/src/features/alerts/alertHistoryModel.ts b/frontend-modern/src/features/alerts/alertHistoryModel.ts index 097a560da5..0be5a89f2b 100644 --- a/frontend-modern/src/features/alerts/alertHistoryModel.ts +++ b/frontend-modern/src/features/alerts/alertHistoryModel.ts @@ -7,6 +7,7 @@ import { import { isPulseSystemAlert } from '@/utils/alertScope'; import { alertTypeDisplayLabel, unifiedTypeToAlertDisplayType } from './helpers'; +import { getMetricAlertPresentation } from './metricAlertPresentation'; export const MS_PER_HOUR = 60 * 60 * 1000; @@ -33,6 +34,22 @@ export interface HistoryItem { closeDetail?: string; acknowledged?: boolean; systemAlert?: boolean; + /** + * The open alert behind an active row, so the row can describe a threshold + * alert held below its trigger from its live reading. Closed rows have none. + */ + liveAlert?: Pick; +} + +export interface AlertHistoryRowCopy { + /** What the row leads with. */ + text: string; + /** What clears an open threshold alert from here. */ + detail?: string; + /** The last reading that met the trigger, once the live one differs. */ + lastBreach?: string; + /** The full copy, for hover where the row truncates. */ + title: string; } export interface AlertTrendSeries { @@ -205,6 +222,7 @@ export function buildAlertHistoryItems({ rawAlertType: alert.type, description: alert.message, acknowledged: false, + ...(systemScoped ? {} : { liveAlert: alert }), }); }); @@ -251,6 +269,29 @@ export function buildAlertHistoryItems({ return items; } +// The open row is the one occurrence still in progress. A threshold alert can +// stay open below its trigger while its message keeps the last breach, so the +// row leads with the reading Pulse is evaluating now, as the Alerts card does. +// Closed rows, and open ones without a live status, keep the recorded message, +// which search and the Assistant handoff also read. The clock is read only for +// open rows, so closed rows do not re-render on every tick. +export function getAlertHistoryRowCopy( + item: Pick, + now: () => number = Date.now, +): AlertHistoryRowCopy { + const recorded = item.description ?? ''; + const presentation = item.liveAlert ? getMetricAlertPresentation(item.liveAlert, now()) : null; + if (!presentation) return { text: recorded, title: recorded }; + return { + text: presentation.summary, + detail: presentation.detail, + lastBreach: presentation.lastBreach, + title: [presentation.summary, presentation.detail, presentation.lastBreach] + .filter(Boolean) + .join('\n'), + }; +} + export function filterAlertHistoryItems( items: HistoryItem[], severityFilter: AlertSeverityFilter, diff --git a/frontend-modern/src/features/patrol/PatrolAttentionWorkbench.tsx b/frontend-modern/src/features/patrol/PatrolAttentionWorkbench.tsx index 3b34ff89bc..933012a2a6 100644 --- a/frontend-modern/src/features/patrol/PatrolAttentionWorkbench.tsx +++ b/frontend-modern/src/features/patrol/PatrolAttentionWorkbench.tsx @@ -64,6 +64,7 @@ import type { EvidenceEnvelope } from '@/types/operationalTrust'; import type { ActionDetailResponse } from '@/types/actionAudit'; import { getAlertResourceIncidentAcknowledgedByLabel } from '@/utils/alertIncidentPresentation'; import { formatRelativeTime } from '@/utils/format'; +import { useRelativeTimeNow } from '@/utils/relativeTimeClock'; import { copyToClipboard } from '@/utils/clipboard'; import type { PatrolAutonomyLevel } from '@/api/patrol'; import { @@ -1283,6 +1284,9 @@ function AttentionLifecycleControls(props: { onSuppress: (itemId: string, reason: string, expiresAt: string) => Promise; onUnsuppress: (itemId: string, reviewed: boolean) => Promise; }) { + // The acknowledgement time never changes while the detail stays open, so its + // age reads the shared clock. + const now = useRelativeTimeNow(); const [showSuppression, setShowSuppression] = createSignal(false); const [reason, setReason] = createSignal(''); const [durationMs, setDurationMs] = createSignal(SUPPRESSION_DURATIONS[1].value); @@ -1312,7 +1316,7 @@ function AttentionLifecycleControls(props: { {(acknowledgement) => (

{getAlertResourceIncidentAcknowledgedByLabel(acknowledgement().by)}{' '} - {formatRelativeTime(acknowledgement().at, { compact: true })}. + {formatRelativeTime(acknowledgement().at, { compact: true, now: now() })}.

)} @@ -1451,6 +1455,9 @@ function AttentionLifecycleControls(props: { } function AttentionTimelineList(props: { timeline: AttentionItemDetail['timeline'] }) { + // The open detail is read once per selection and its transitions never + // change, so their ages read the shared clock. + const now = useRelativeTimeNow(); return (
    @@ -1460,7 +1467,7 @@ function AttentionTimelineList(props: { timeline: AttentionItemDetail['timeline' {formatLabel(transition.from)} to {formatLabel(transition.to)}

    - {formatRelativeTime(transition.at, { compact: true })} ·{' '} + {formatRelativeTime(transition.at, { compact: true, now: now() })} ·{' '} {formatLabel(transition.cause)}

    @@ -1784,6 +1791,9 @@ function DetailSection(props: { title: string; children: import('solid-js').JSX. } function EvidenceObservation(props: { evidence: EvidenceEnvelope }) { + // An evidence record's observation time is fixed, so its age reads the + // shared clock while the detail stays open. + const now = useRelativeTimeNow(); return (
  1. @@ -1807,7 +1817,7 @@ function EvidenceObservation(props: { evidence: EvidenceEnvelope }) {

    {props.evidence.source.collector} · observed{' '} - {formatRelativeTime(props.evidence.observedAt, { compact: true })} + {formatRelativeTime(props.evidence.observedAt, { compact: true, now: now() })}

    {(reason) =>

    {reason()}

    } diff --git a/frontend-modern/src/features/patrol/PatrolIntelligenceBanners.tsx b/frontend-modern/src/features/patrol/PatrolIntelligenceBanners.tsx index 059d311f05..d491135fb1 100644 --- a/frontend-modern/src/features/patrol/PatrolIntelligenceBanners.tsx +++ b/frontend-modern/src/features/patrol/PatrolIntelligenceBanners.tsx @@ -11,6 +11,7 @@ import { presentationPolicyHidesUpgradePrompts, } from '@/stores/sessionPresentationPolicy'; import { formatRelativeTime } from '@/utils/format'; +import { useRelativeTimeNow } from '@/utils/relativeTimeClock'; import { getAIProviderDisplayName } from '@/utils/aiProviderPresentation'; import { getPatrolSetupAction } from '@/utils/patrolRuntimeActions'; import { getPatrolAutonomyAvailabilityPresentation } from './patrolAutonomyAvailability'; @@ -30,6 +31,9 @@ function modelIdWithoutProviderPrefix(modelId: string | undefined): string { export function PatrolIntelligenceBanners(props: { state: PatrolIntelligenceState }) { const state = props.state; + // Patrol status re-reads keep the same blocked time while Patrol stays + // paused, so the Blocked age reads the shared clock. + const now = useRelativeTimeNow(); const setupAction = createMemo(() => getPatrolSetupAction( resolvePatrolBlockedActionCause(state.blockedCause(), state.patrolReadiness()?.cause), @@ -267,7 +271,7 @@ export function PatrolIntelligenceBanners(props: { state: PatrolIntelligenceStat

    {state.blockedReason()}

    - Blocked {formatRelativeTime(state.blockedAt(), { compact: true })} + Blocked {formatRelativeTime(state.blockedAt(), { compact: true, now: now() })}

    diff --git a/frontend-modern/src/features/patrol/PatrolIntelligenceWorkspace.tsx b/frontend-modern/src/features/patrol/PatrolIntelligenceWorkspace.tsx index 1bd38e07c6..b3c70a6068 100644 --- a/frontend-modern/src/features/patrol/PatrolIntelligenceWorkspace.tsx +++ b/frontend-modern/src/features/patrol/PatrolIntelligenceWorkspace.tsx @@ -12,6 +12,7 @@ import { isPatrolRuntimeFinding, } from '@/utils/aiFindingPresentation'; import { formatRelativeTime } from '@/utils/format'; +import { useRelativeTimeNow } from '@/utils/relativeTimeClock'; import { formatTriggerReason } from '@/utils/patrolFormat'; import { getPatrolRunRecordSummaryPresentation, @@ -51,6 +52,9 @@ export function PatrolIntelligenceWorkspace(props: { findingResourceId?: string; }) { const state = props.state; + // The selected run stays as picked when run history is re-read, so its start + // age reads the shared clock instead of freezing at selection. + const now = useRelativeTimeNow(); const queueDisplayGroups = createMemo(() => buildPatrolFindingDisplayGroups(state.findingsTabBadgeFindings()), ); @@ -207,7 +211,7 @@ export function PatrolIntelligenceWorkspace(props: {
    - Patrol run {formatRelativeTime(run().started_at, { compact: true })} + Patrol run {formatRelativeTime(run().started_at, { compact: true, now: now() })} {formatTriggerReason(run().trigger_reason)} diff --git a/frontend-modern/src/features/patrol/__tests__/PatrolAttentionWorkbench.test.tsx b/frontend-modern/src/features/patrol/__tests__/PatrolAttentionWorkbench.test.tsx index 50c5a2fb7f..5feb869b96 100644 --- a/frontend-modern/src/features/patrol/__tests__/PatrolAttentionWorkbench.test.tsx +++ b/frontend-modern/src/features/patrol/__tests__/PatrolAttentionWorkbench.test.tsx @@ -7,6 +7,7 @@ import type { AttentionListResponse, AttentionSummary, } from '@/api/patrolAttention'; +import { RELATIVE_TIME_TICK_MS } from '@/utils/relativeTimeClock'; const apiMocks = vi.hoisted(() => ({ getList: vi.fn(), @@ -479,6 +480,45 @@ describe('PatrolAttentionWorkbench', () => { expect(window.location.search).toBe('?attention=record-1'); }); + it('keeps the open detail transition, evidence and acknowledgement ages moving', async () => { + vi.useFakeTimers({ + toFake: ['Date', 'setInterval', 'clearInterval'], + now: Date.parse('2026-07-19T08:10:00Z'), + }); + const active = item({ state: 'acknowledged' }); + const opened = detail(active); + opened.timeline[0].at = '2026-07-19T08:03:00Z'; + opened.evidence[0].observedAt = '2026-07-19T08:05:00Z'; + opened.operationalRecord.acknowledgement = { by: 'admin', at: '2026-07-19T08:07:00Z' }; + apiMocks.getList.mockResolvedValue( + listResponse([active], summary({ activeCount: 1, acknowledgedCount: 1, calm: false })), + ); + apiMocks.getDetail.mockResolvedValue(opened); + + try { + renderWorkbench(); + fireEvent.click( + await screen.findByRole('button', { name: 'Open Database VM · Disk pressure' }), + ); + const detailRegion = await screen.findByRole('complementary', { + name: 'Database VM · Disk pressure', + }); + await waitFor(() => expect(detailRegion).toHaveTextContent('7m ago')); + expect(detailRegion).toHaveTextContent('5m ago'); + expect(detailRegion).toHaveTextContent('3m ago'); + + // The detail is not re-read: its records never change and only the + // clock moves. + vi.advanceTimersByTime(2 * 60 * 2 * RELATIVE_TIME_TICK_MS); + + expect(detailRegion).not.toHaveTextContent(/[357]m ago/); + expect(detailRegion.textContent?.match(/2h ago/g)?.length).toBeGreaterThanOrEqual(3); + expect(apiMocks.getDetail).toHaveBeenCalledTimes(1); + } finally { + vi.useRealTimers(); + } + }); + it('keeps the detail header and queue controls pinned while a long decision scrolls', async () => { const active = item(); apiMocks.getList.mockResolvedValue( diff --git a/frontend-modern/src/features/patrol/__tests__/PatrolRunBanners.relativeAge.test.tsx b/frontend-modern/src/features/patrol/__tests__/PatrolRunBanners.relativeAge.test.tsx new file mode 100644 index 0000000000..bb50875020 --- /dev/null +++ b/frontend-modern/src/features/patrol/__tests__/PatrolRunBanners.relativeAge.test.tsx @@ -0,0 +1,96 @@ +import { cleanup, render, screen } from '@solidjs/testing-library'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import type { PatrolRunRecord } from '@/api/patrol'; +import { RELATIVE_TIME_TICK_MS } from '@/utils/relativeTimeClock'; +import { PatrolIntelligenceBanners } from '../PatrolIntelligenceBanners'; +import { PatrolIntelligenceWorkspace } from '../PatrolIntelligenceWorkspace'; +import type { PatrolIntelligenceState } from '../usePatrolIntelligenceState'; + +vi.mock('@/components/AI/FindingsPanel', () => ({ + FindingsPanel: () =>
    , +})); + +vi.mock('@/components/patrol/RunHistoryPanel', () => ({ + RunHistoryPanel: () =>
    , +})); + +// Both surfaces read many accessors; the ones a test does not set answer +// with nothing, and list-valued ones with an empty list. +const LIST_ACCESSORS = new Set([ + 'findingsTabBadgeFindings', + 'displayRunHistory', + 'selectedRunFindingIds', + 'selectedRunScopeResourceIds', +]); +const stubState = (overrides: Record): PatrolIntelligenceState => + new Proxy(overrides, { + get(target, key: string) { + if (key in target) return target[key]; + if (key === 'patrolStream') { + return new Proxy({}, { get: () => () => undefined }); + } + if (key === 'patrolRunHistory') return { value: () => [], refetch: vi.fn() }; + return LIST_ACCESSORS.has(key) ? () => [] : () => undefined; + }, + }) as unknown as PatrolIntelligenceState; + +afterEach(() => { + cleanup(); + vi.useRealTimers(); +}); + +describe('Patrol run and pause banners relative ages', () => { + it('keeps the Patrol paused Blocked age moving while Patrol stays paused', () => { + vi.useFakeTimers({ + toFake: ['Date', 'setInterval', 'clearInterval'], + now: Date.parse('2026-07-19T08:05:00Z'), + }); + + render(() => ( + true, + blockedReason: () => 'The monthly AI budget is used up.', + blockedAt: () => '2026-07-19T08:00:00Z', + })} + /> + )); + + expect(screen.getByText('Blocked 5m ago')).toBeInTheDocument(); + + // Each status re-read returns the same blocked time: only the clock moves. + vi.advanceTimersByTime(3 * 60 * 2 * RELATIVE_TIME_TICK_MS); + + expect(screen.getByText('Blocked 3h ago')).toBeInTheDocument(); + }); + + it('keeps the selected Patrol run start age moving while the run stays selected', () => { + vi.useFakeTimers({ + toFake: ['Date', 'setInterval', 'clearInterval'], + now: Date.parse('2026-07-19T08:05:00Z'), + }); + const run = { + id: 'run-1', + started_at: '2026-07-19T08:00:00Z', + trigger_reason: 'scheduled', + status: 'healthy', + finding_ids: [], + } as unknown as PatrolRunRecord; + + render(() => ( + 'findings', + selectedRun: () => run, + })} + /> + )); + + expect(screen.getByText('Patrol run 5m ago')).toBeInTheDocument(); + + vi.advanceTimersByTime(3 * 60 * 2 * RELATIVE_TIME_TICK_MS); + + expect(screen.getByText('Patrol run 3h ago')).toBeInTheDocument(); + }); +}); diff --git a/frontend-modern/src/features/platformPage/sensorSetup.test.ts b/frontend-modern/src/features/platformPage/sensorSetup.test.ts index 9d42cbecc9..3b2ada61d9 100644 --- a/frontend-modern/src/features/platformPage/sensorSetup.test.ts +++ b/frontend-modern/src/features/platformPage/sensorSetup.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import type { Resource } from '@/types/resource'; +import type { PhysicalDiskCollectionStatus, Resource } from '@/types/resource'; import { collectOutdatedSensorSetupNodes } from './sensorSetup'; const node = (over: Partial & { proxmox?: Resource['proxmox'] }): Resource => @@ -11,6 +11,7 @@ const disk = (over: { nodeName?: string; diskType?: string; temperature?: number; + collection?: PhysicalDiskCollectionStatus; }): Resource => ({ id: over.id ?? 'disk-1', @@ -20,6 +21,7 @@ const disk = (over: { physicalDisk: { diskType: over.diskType ?? 'sata', temperature: over.temperature ?? 0, + collection: over.collection, }, platformData: over.nodeName ? { proxmox: { nodeName: over.nodeName } } : {}, }) as unknown as Resource; @@ -80,6 +82,28 @@ describe('collectOutdatedSensorSetupNodes', () => { ).toEqual([]); }); + it('treats a retained temperature that is not current as missing', () => { + // Normalization keeps a disk's last known temperature when the current + // observation did not collect one, such as after a linked host agent + // stopped reporting. The disk cannot be read now, so the notice stays. + const retained = (state: 'available' | 'unavailable' | 'missing') => + disk({ + nodeName: 'pve1', + diskType: 'sata', + temperature: 34, + collection: { + temperature: { state, source: 'agent', reason: 'host agent stopped reporting' }, + }, + }); + expect(collectOutdatedSensorSetupNodes([legacyNode()], [retained('unavailable')])).toEqual([ + { id: 'node-1', name: 'pve1' }, + ]); + expect(collectOutdatedSensorSetupNodes([legacyNode()], [retained('missing')])).toEqual([ + { id: 'node-1', name: 'pve1' }, + ]); + expect(collectOutdatedSensorSetupNodes([legacyNode()], [retained('available')])).toEqual([]); + }); + it('ignores legacy nodes with only NVMe disks missing temperatures', () => { expect( collectOutdatedSensorSetupNodes( diff --git a/frontend-modern/src/features/platformPage/sensorSetup.ts b/frontend-modern/src/features/platformPage/sensorSetup.ts index 04c1378370..af8ff285b2 100644 --- a/frontend-modern/src/features/platformPage/sensorSetup.ts +++ b/frontend-modern/src/features/platformPage/sensorSetup.ts @@ -1,5 +1,6 @@ import type { Resource } from '@/types/resource'; import { matchesPhysicalDiskNode } from '@/components/Storage/diskResourceUtils'; +import { isPhysicalDiskTemperatureCurrent } from '@/features/storageBackups/diskTemperaturePresentation'; // Disk transports whose temperatures only arrive via SMART (the pulse-sensors // wrapper). NVMe temps come from kernel hwmon and work even on legacy setups. @@ -18,7 +19,9 @@ const isSMARTOnlyDiskWithoutTemperature = (disk: Resource): boolean => { if (!meta) return false; const diskType = (meta.diskType || '').trim().toLowerCase(); if (!SMART_ONLY_DISK_TYPES.has(diskType)) return false; - return (meta.temperature ?? 0) <= 0; + // A last known temperature kept for a disk that could not be read now, such + // as after a linked host agent stopped reporting, is not one arriving. + return (meta.temperature ?? 0) <= 0 || !isPhysicalDiskTemperatureCurrent(meta.collection); }; // Flags PVE nodes whose SSH temperature monitoring still runs the pre-rc.6 diff --git a/frontend-modern/src/features/truenas/TrueNASStorageTopologyTable.tsx b/frontend-modern/src/features/truenas/TrueNASStorageTopologyTable.tsx index 27c9caf59a..baa629330f 100644 --- a/frontend-modern/src/features/truenas/TrueNASStorageTopologyTable.tsx +++ b/frontend-modern/src/features/truenas/TrueNASStorageTopologyTable.tsx @@ -37,6 +37,10 @@ import { createPlatformResourceLabelResolver, getPlatformResourceDetailRowClass, } from '@/features/platformPage/PlatformResourceDetailTableRow'; +import { + PHYSICAL_DISK_TEMPERATURE_LAST_KNOWN_CLASS, + getPhysicalDiskTemperaturePresentation, +} from '@/features/storageBackups/diskTemperaturePresentation'; import { useAlertsActivation } from '@/stores/alertsActivation'; import type { Resource } from '@/types/resource'; import { @@ -200,6 +204,40 @@ const DiskEnduranceCell: Component<{ row: TrueNASStorageTopologyRow }> = (props) ); }; +// A disk row may carry a last known temperature it did not collect now, such +// as a host agent's reading after the agent stopped reporting on a merged disk. +const diskTemperatureReading = (row: TrueNASStorageTopologyRow) => { + const disk = row.kind === 'disk' ? row.resource.physicalDisk : undefined; + return disk + ? getPhysicalDiskTemperaturePresentation({ + temperature: disk.temperature ?? 0, + collection: disk.collection, + }) + : null; +}; + +// A retained reading reads as last known, never as the disk's temperature now. +const DiskTemperatureCell: Component<{ row: TrueNASStorageTopologyRow }> = (props) => { + const reading = () => diskTemperatureReading(props.row); + const value = () => + props.row.kind === 'disk' ? props.row.resource.physicalDisk?.temperature : undefined; + return ( + } + > + + + , last known + + + ); +}; + const ISSUE_TONES: Record = { attention: 'warning', offline: 'danger', @@ -253,9 +291,10 @@ const getTrueNASStorageSortValue = ( case 'endurance': return row.kind === 'disk' ? (diskEndurance(row)?.value ?? null) : null; case 'temp': { - if (row.kind !== 'disk') return null; - const temperature = row.resource.physicalDisk?.temperature; - return typeof temperature === 'number' && Number.isFinite(temperature) ? temperature : null; + // Hottest first ranks current readings only; a last known one sorts as + // no reading. + const reading = diskTemperatureReading(row); + return reading?.current ? (row.resource.physicalDisk?.temperature ?? null) : null; } case 'health': return HEALTH_SORT_RANK[ @@ -575,14 +614,7 @@ export const TrueNASStorageTopologyTable: Component<{ kindFilter() === 'disks' ? 'table-cell' : 'hidden lg:table-cell' } text-base-content`} > - + { ).toEqual(['sda', 'nvme0n1']); }); + it('shows a retained disk temperature as last known and sorts it as no reading', async () => { + // A TrueNAS disk merged with a host agent's row keeps the agent's last + // reading after the agent stops reporting, under an unavailable state. + const disk = (id: string, temperature: number, state: 'available' | 'unavailable') => + makeStorageResource({ + id, + type: 'physical_disk', + name: id, + storage: undefined, + physicalDisk: { + devPath: `/dev/${id}`, + serial: `serial-${id}`, + diskType: 'sata', + temperature, + collection: { + temperature: { state, source: 'agent', reason: 'host agent stopped reporting' }, + }, + }, + }); + const resources = [ + disk('sda', 64, 'unavailable'), + disk('sdb', 41, 'available'), + disk('sdc', 38, 'available'), + ]; + const { container } = render(() => ( + } + emptyTitle="No storage" + emptyDescription="No storage" + showToolbar={false} + /> + )); + const row = (id: string) => container.querySelector(`[data-truenas-storage-resource="${id}"]`); + const order = () => + [...container.querySelectorAll('[data-truenas-storage-resource]')].map((element) => + element.getAttribute('data-truenas-storage-resource'), + ); + + try { + const retained = row('sda')?.querySelector('[data-temperature-reading="last-known"]'); + expect(retained).toHaveTextContent('64.0°C, last known'); + expect(retained).toHaveAttribute( + 'title', + 'Last known reading, not current: host agent stopped reporting', + ); + expect(retained).toHaveClass('text-muted'); + // 64C is over the factory SATA trigger, but a retained reading is not heat. + expect(row('sda')?.querySelector('[data-truenas-storage-health]')).toBeNull(); + expect(row('sdb')?.querySelector('[data-temperature-reading="last-known"]')).toBeNull(); + expect(row('sdb')).toHaveTextContent('41.0°C'); + + // Hottest first ranks current readings only; the retained one stays last. + await fireEvent.click(screen.getByRole('columnheader', { name: /Temp/ })); + expect(order()).toEqual(['sdb', 'sdc', 'sda']); + await fireEvent.click(screen.getByRole('columnheader', { name: /Temp/ })); + expect(order()).toEqual(['sdc', 'sdb', 'sda']); + } finally { + window.localStorage.clear(); + } + }); + it('follows a disk temperature trigger the user raised in Alerts', async () => { const getConfig = vi.spyOn(AlertsAPI, 'getConfig').mockResolvedValue({ enabled: true, diff --git a/frontend-modern/src/pages/__tests__/Alerts.helpers.test.ts b/frontend-modern/src/pages/__tests__/Alerts.helpers.test.ts index 31016b5670..ab56ce918e 100644 --- a/frontend-modern/src/pages/__tests__/Alerts.helpers.test.ts +++ b/frontend-modern/src/pages/__tests__/Alerts.helpers.test.ts @@ -549,6 +549,10 @@ describe('tab path helpers', () => { expect(alertHistoryTableAlertRowSource).toContain('export function AlertHistoryTableAlertRow'); expect(alertHistoryTableAlertRowSource).toContain('IncidentTimelinePanel'); expect(alertHistoryTableAlertRowSource).toContain('AlertHistoryItemActions'); + // An open threshold alert's row leads with its live reading, not the + // breach message it opened with (#2068). + expect(alertHistoryTableAlertRowSource).toContain('getAlertHistoryRowCopy(props.alert, now)'); + expect(alertHistoryTableAlertRowSource).not.toContain('{props.alert.description}'); expect(alertHistoryItemActionsSource).toContain('InvestigateAlertButton'); expect(alertHistoryAdministrationCardSource).toContain( 'export function AlertHistoryAdministrationCard', diff --git a/internal/monitoring/issue1595_collection_trust_test.go b/internal/monitoring/issue1595_collection_trust_test.go index daa3cf2605..7a52f71d8b 100644 --- a/internal/monitoring/issue1595_collection_trust_test.go +++ b/internal/monitoring/issue1595_collection_trust_test.go @@ -1,16 +1,21 @@ package monitoring import ( + "context" "encoding/json" "os" "path/filepath" + "strings" "testing" "time" + "github.com/rcourtman/pulse-go-rewrite/internal/alerts" + "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" "github.com/rcourtman/pulse-go-rewrite/pkg/metrics" + "github.com/rcourtman/pulse-go-rewrite/pkg/proxmox" ) type issue1595TopologyFixture struct { @@ -367,3 +372,243 @@ func TestWriteHostSMARTMetricsRecordsOnlyCollectedTemperatures(t *testing.T) { } } } + +// A SMART row found only by device path may describe the slot's previous +// occupant. It is refused when its identity contradicts the Proxmox disk's, +// judged only on what both producers report like for like: WWNs for every +// disk, serials only where Proxmox's serial is the drive's own, for NVMe and +// for disks the kernel reaches through libata (vendor "ATA"). A SAS disk's +// Proxmox serial may be a transport address and a USB bridge's is its own. +// Reporter spellings of one identity (udev's underscores, a T10 designator +// ending with the drive serial, the agent's NAA fields, udev's 64-bit NAA 6 +// prefix) are not disagreement. +func TestHostAgentSMARTMergeRefusesPathMatchAcrossContradictingIdentity(t *testing.T) { + type mergeCase struct { + name string + disk models.PhysicalDisk + smart models.HostDiskSMART + // others are further rows on the same path, which make it ambiguous. + others []models.HostDiskSMART + merged bool + serial string + } + smartctlSerial := &diskinventory.CollectionStatus{Serial: diskinventory.Available("smartctl")} + pveDisk := func(device, vendor, serial, wwn, diskType string) models.PhysicalDisk { + return models.PhysicalDisk{ + ID: "pve1-node1-" + device, Node: "node1", Instance: "pve1", DevPath: "/dev/" + device, + Vendor: vendor, Serial: serial, WWN: wwn, Type: diskType, Health: "PASSED", + } + } + smartRow := func(device, serial, wwn, diskType string) models.HostDiskSMART { + return models.HostDiskSMART{ + Device: "/dev/" + device, Serial: serial, WWN: wwn, Type: diskType, + Health: "FAILED", Temperature: 47, Collection: diskinventory.CloneStatus(smartctlSerial), + } + } + for _, tc := range []mergeCase{ + { + name: "SAS replacement beside a stale row with another WWN", + disk: pveDisk("sda", "SEAGATE", "5000c500b0000001", "0x5000c500b0000001", "unknown"), + smart: smartRow("sda", "ZR5OLD0001", "naa.5000c500a0000001", "sas"), + }, + { + name: "SATA replacement beside a stale row with another serial", + disk: pveDisk("sdb", "ATA", "WD-NEW0001", "", "hdd"), + smart: smartRow("sdb", "WD-OLD0001", "", "sata"), + }, + { + name: "same SAS disk whose agent row has no WWN", + disk: pveDisk("sdc", "SEAGATE", "5000c500a0000003", "0x5000c500a0000003", "unknown"), + smart: smartRow("sdc", "ZR5TESTA0003", "", "sas"), + merged: true, serial: "ZR5TESTA0003", + }, + { + name: "same SAS disk renamed, agent WWN in smartctl NAA fields", + disk: pveDisk("sdd", "SEAGATE", "5000c500a0000004", "0x5000c500a0000004", "unknown"), + smart: smartRow("sdx", "ZR5TESTA0004", "5-c50-a0000004", "sas"), + merged: true, serial: "ZR5TESTA0004", + }, + { + name: "same disk with udev's spelling of the serial", + disk: pveDisk("sde", "ATA", "WD-WX_1234", "", "hdd"), + smart: smartRow("sde", "WD-WX 1234", "", "sata"), + merged: true, serial: "WD-WX_1234", + }, + { + name: "NVMe replacement whose serial looks like a SAS address", + disk: pveDisk("nvme0n1", "", "50026B7282A0FB69", "", "nvme"), + smart: smartRow("nvme0n1", "50026B7282A0FB11", "", "nvme"), + }, + { + name: "ambiguous path where only one row contradicts", + disk: pveDisk("sdg", "ATA", "WD-NEW0007", "", "hdd"), + smart: smartRow("sdg", "WD-OLD0007", "", "sata"), + others: []models.HostDiskSMART{ + smartRow("sdg", "", "", "sata"), + }, + }, + { + name: "same disk in a USB enclosure whose serial udev reports", + disk: pveDisk("sdh", "JMicron", "000000000000000F1E2D", "", "usb"), + smart: smartRow("sdh", "WD-WX0008", "", "sata"), + merged: true, serial: "000000000000000F1E2D", + }, + { + name: "same SSD in a USB enclosure Proxmox types ssd", + disk: pveDisk("sdj", "ASMT", "00000000000000000009", "", "ssd"), + smart: smartRow("sdj", "S3Z9NB0K123456X", "", "sata"), + merged: true, serial: "00000000000000000009", + }, + { + name: "same disk behind a T10 designator ending with its serial", + disk: pveDisk("sdi", "ATA", "ATA_ST4000NM000A-2HZ_ZC1ABCDE", "", "unknown"), + smart: smartRow("sdi", "ZC1ABCDE", "", "sata"), + merged: true, serial: "ATA_ST4000NM000A-2HZ_ZC1ABCDE", + }, + { + name: "same NAA 6 volume with udev's 64-bit WWN", + disk: pveDisk("sdf", "HP", "PDNLH0BRH8V0AB", "0x600508b1001c4d5e", "hdd"), + smart: smartRow("sdf", "", "naa.600508b1001c4d5e7f8a9b0c1d2e3f40", "scsi"), + merged: true, serial: "PDNLH0BRH8V0AB", + }, + } { + t.Run(tc.name, func(t *testing.T) { + got := mergeHostAgentSMARTIntoDisks( + []models.PhysicalDisk{tc.disk}, + []models.Node{{Name: "node1", LinkedAgentID: "agent-1"}}, + []models.Host{{ID: "agent-1", Sensors: models.HostSensorSummary{ + SMART: append([]models.HostDiskSMART{tc.smart}, tc.others...), + }}}, + )[0] + if !tc.merged { + if got.Serial != tc.disk.Serial || got.WWN != tc.disk.WWN || got.Type != tc.disk.Type || + got.Temperature != 0 || got.Health != "PASSED" { + t.Fatalf("contradicting row was merged into the disk: %+v", got) + } + return + } + if got.Serial != tc.serial || got.Temperature != 47 || got.Health != "FAILED" { + t.Fatalf("same disk's row was not merged: serial %q (want %q), temperature %d, health %q", + got.Serial, tc.serial, got.Temperature, got.Health) + } + }) + } +} + +// Proxmox's disk inventory reports a serial or WWN udev cannot read as the +// literal "unknown". The poller records it as unreported rather than as a +// collected serial; the disk's identity keys were already blind to it. +func TestProxmoxUnknownDiskIdentityIsRecordedAsUnreported(t *testing.T) { + t.Setenv("PULSE_DATA_DIR", t.TempDir()) + state := models.NewState() + state.UpdateNodesForInstance("pve1", []models.Node{{ + ID: "pve1-node1", Name: "node1", Instance: "pve1", Status: "online", LastSeen: time.Now(), + }}) + alertManager := alerts.NewManager() + t.Cleanup(alertManager.Stop) + m := &Monitor{ + state: state, alertManager: alertManager, + startTime: time.Now().Add(-time.Hour), lastPhysicalDiskPoll: make(map[string]time.Time), + } + // A record from before the placeholder was recorded as unreported must + // not carry it back in as the disk's last known serial. + state.UpdatePhysicalDisks("pve1", []models.PhysicalDisk{{ + ID: unifiedresources.ProxmoxPhysicalDiskSourceID("pve1", "node1", "/dev/sdb", "", ""), + Node: "node1", Instance: "pve1", DevPath: "/dev/sdb", Serial: "unknown", WWN: "unknown", + Collection: &diskinventory.CollectionStatus{Serial: diskinventory.Available("proxmox_disks")}, + }}) + client := &slotDiskPVEClient{} + client.setDisk(proxmox.Disk{ + DevPath: "/dev/sdb", Model: "USB3.0 Bridge", Serial: "unknown", WWN: "unknown", + Type: "hdd", Health: "PASSED", Wearout: 100, Size: 1000204886016, + }) + started := time.Now() + m.maybePollPhysicalDisksAsync(context.Background(), "pve1", &config.PVEInstance{}, client, + []proxmox.Node{{Node: "node1", Status: "online"}}, map[string]string{"node1": "online"}, nil) + deadline := started.Add(3 * time.Second) + for disks := state.GetSnapshot().PhysicalDisks; len(disks) != 1 || disks[0].LastChecked.Before(started); disks = state.GetSnapshot().PhysicalDisks { + if time.Now().After(deadline) { + t.Fatal("physical disk poll did not land in state") + } + time.Sleep(10 * time.Millisecond) + } + got := state.GetSnapshot().PhysicalDisks[0] + if got.Serial != "" || got.WWN != "" { + t.Fatalf("placeholder identity recorded as reported: serial %q, wwn %q", got.Serial, got.WWN) + } + if got.Collection == nil || got.Collection.Serial.State != diskinventory.FieldMissing { + t.Fatalf("serial collection = %+v, want missing", got.Collection) + } + placeholder := got + placeholder.Serial, placeholder.WWN = "unknown", "unknown" + if unifiedresources.PhysicalDiskMetricID(got) != unifiedresources.PhysicalDiskMetricID(placeholder) { + t.Fatalf("metric key moved: %q, was %q", + unifiedresources.PhysicalDiskMetricID(got), unifiedresources.PhysicalDiskMetricID(placeholder)) + } +} + +// The registry joins a Proxmox disk to its linked agent's disk on the same +// path when the agent reports SAS, because Proxmox puts the SAS address in +// the serial (#1595). A WWN naming another disk refuses that join: the agent's +// row may be the slot's previous occupant, retained by a silent agent, and the +// join used to hand the replacement that disk's serial, readings and canonical +// resource. +func TestRegistrySASPathJoinRefusesContradictingWWN(t *testing.T) { + const replacementWWN = "0x5000c500bbbb0002" + for _, tc := range []struct { + name string + agentWWN string + joined bool + }{ + {name: "retained row of the previous occupant", agentWWN: "5-c50-aaaa0001"}, + {name: "same disk in the agent's WWN spelling", agentWWN: "5-c50-bbbb0002", joined: true}, + {name: "same disk without an agent WWN", agentWWN: "", joined: true}, + } { + t.Run(tc.name, func(t *testing.T) { + now := time.Now() + node := models.Node{ + ID: "pve1-node1", Name: "node1", Instance: "pve1", Status: "online", + LastSeen: now, LinkedAgentID: "agent-1", + } + host := models.Host{ + ID: "agent-1", Hostname: "node1", LinkedNodeID: "pve1-node1", Status: "online", LastSeen: now, + Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{{ + Device: "/dev/sdb", Serial: "ZR5AGENT0001", WWN: tc.agentWWN, Type: "sas", + Health: "FAILED", Temperature: 52, + Collection: &diskinventory.CollectionStatus{ + Serial: diskinventory.Available("smartctl"), + Temperature: diskinventory.Available("smartctl"), + }, + }}}, + } + replacement := models.PhysicalDisk{ + ID: "pve1-node1--dev-sdb", Node: "node1", Instance: "pve1", DevPath: "/dev/sdb", + Serial: strings.TrimPrefix(replacementWWN, "0x"), WWN: replacementWWN, Type: "unknown", + Health: "PASSED", Wearout: -1, LastChecked: now, + } + registry := unifiedresources.NewRegistry(nil) + registry.IngestSnapshot(models.StateSnapshot{ + Nodes: []models.Node{node}, Hosts: []models.Host{host}, PhysicalDisks: []models.PhysicalDisk{replacement}, + }) + + var proxmoxDisk *unifiedresources.Resource + disks := registry.ListByType(unifiedresources.ResourceTypePhysicalDisk) + for index := range disks { + if hasIssue1595Source(disks[index].Sources, unifiedresources.SourceProxmox) { + proxmoxDisk = &disks[index] + } + } + if proxmoxDisk == nil || proxmoxDisk.PhysicalDisk == nil { + t.Fatalf("no canonical Proxmox disk among %d disks", len(disks)) + } + joined := hasIssue1595Source(proxmoxDisk.Sources, unifiedresources.SourceAgent) + if joined != tc.joined || (tc.joined && len(disks) != 1) || (!tc.joined && len(disks) != 2) { + t.Fatalf("joined = %v with %d disks, want joined = %v", joined, len(disks), tc.joined) + } + if !tc.joined && (proxmoxDisk.PhysicalDisk.Serial == "ZR5AGENT0001" || + proxmoxDisk.PhysicalDisk.Temperature != 0 || proxmoxDisk.PhysicalDisk.Health == "FAILED") { + t.Fatalf("replacement took the retained row's identity or readings: %+v", proxmoxDisk.PhysicalDisk) + } + }) + } +} diff --git a/internal/monitoring/merge_temps_test.go b/internal/monitoring/merge_temps_test.go index 18f884afa7..a95ada4b46 100644 --- a/internal/monitoring/merge_temps_test.go +++ b/internal/monitoring/merge_temps_test.go @@ -435,7 +435,7 @@ func TestMergeHostAgentSMARTIntoDisks_PreservesExistingProxmoxIdentity(t *testin DevPath: "/dev/sdb", Model: "Proxmox Model", Serial: "PVE-SERIAL", - WWN: "pve-wwn", + WWN: "0x5000c500a1b2c3d4", Type: "sas", Size: 100, Health: "PASSED", @@ -453,7 +453,7 @@ func TestMergeHostAgentSMARTIntoDisks_PreservesExistingProxmoxIdentity(t *testin Device: "/dev/sdb", Model: "Agent Model", Serial: "AGENT-SERIAL", - WWN: "agent-wwn", + WWN: "5-c50-a1b2c3d4", // the same disk, in smartctl's spelling Type: "sata", SizeBytes: 200, Temperature: 35, @@ -466,7 +466,7 @@ func TestMergeHostAgentSMARTIntoDisks_PreservesExistingProxmoxIdentity(t *testin } got := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)[0] - if got.Model != "Proxmox Model" || got.Serial != "PVE-SERIAL" || got.WWN != "pve-wwn" || got.Type != "sas" { + if got.Model != "Proxmox Model" || got.Serial != "PVE-SERIAL" || got.WWN != "0x5000c500a1b2c3d4" || got.Type != "sas" { t.Fatalf("existing Proxmox identity was overwritten: %+v", got) } if got.Size != 100 || got.Temperature != 41 || got.StorageGroup != "api-pool" { diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 69c5c33f37..25be288f63 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -560,11 +560,12 @@ func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.No return &smartData[matchIndex] } - // Try to match by WWN (most reliable) + // Try to match by WWN (most reliable). PVE and the agent spell one WWN + // differently (0x5000c500a1b2c3d4 against smartctl's 5-c50-a1b2c3d4), + // so compare the normalized identity, not the reported string. if diskinventory.IsUsableHardwareID(updated[i].WWN) { matched = uniqueMatch(func(candidate models.HostDiskSMART) bool { - return diskinventory.IsUsableHardwareID(candidate.WWN) && - strings.EqualFold(candidate.WWN, updated[i].WWN) && + return diskinventory.HardwareIdentityMatch("", updated[i].WWN, "", candidate.WWN) && diskTopologyCompatible(updated[i].Controller, updated[i].Target, candidate.Controller, candidate.Target) }) } @@ -578,7 +579,12 @@ func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.No }) } - // Last resort: match by device path + // Last resort: match by device path. A path names a slot, not a disk, + // so the row there may describe the slot's previous occupant: the agent + // has not reported since the swap, or it went silent and its last rows + // are retained. A row whose identity contradicts the disk's is refused, + // or it would fill or promote the old serial onto the replacement, + // which would then take the old disk's canonical resource and history. if matched == nil { normalizedDevPath := normalizeSMARTDeviceIdentifier(updated[i].DevPath) matched = uniqueMatch(func(candidate models.HostDiskSMART) bool { @@ -587,6 +593,15 @@ func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.No normalizedDiskDev == normalizedDevPath && diskTopologyCompatible(updated[i].Controller, updated[i].Target, candidate.Controller, candidate.Target) }) + if matched != nil && hostAgentSMARTIdentityConflicts(updated[i], *matched) { + log.Debug(). + Str("node", updated[i].Node). + Str("device", updated[i].DevPath). + Str("serial", updated[i].Serial). + Str("smartSerial", matched.Serial). + Msg("Skipping host agent SMART row whose disk identity contradicts the Proxmox disk at the same path") + matched = nil + } } if matched == nil { @@ -703,6 +718,40 @@ func diskTopologyCompatible(leftController, leftTarget, rightController, rightTa return true } +// hostAgentSMARTIdentityConflicts reports whether a SMART row found only by +// device path names a different disk than the Proxmox disk at that path. +// WWNs are compared for every disk; serials only where both producers read +// the drive's own serial (hostAgentSMARTSerialComparable). Any other Proxmox +// serial still counts toward agreement. +func hostAgentSMARTIdentityConflicts(disk models.PhysicalDisk, smart models.HostDiskSMART) bool { + if diskinventory.HardwareIdentityMatch(disk.Serial, disk.WWN, smart.Serial, smart.WWN) { + return false + } + serial := "" + if hostAgentSMARTSerialComparable(disk, smart) { + serial = disk.Serial + } + return diskinventory.HardwareIdentityConflict(serial, disk.WWN, smart.Serial, smart.WWN) +} + +// hostAgentSMARTSerialComparable reports whether the Proxmox serial was read +// from the drive itself, as smartctl's is. Proxmox takes the serial from udev: +// for an NVMe namespace that is the controller's serial, and for a disk +// presented with SCSI vendor "ATA" (libata, or a SAS HBA's SCSI-ATA +// translation) udev's ata_id reads it from IDENTIFY. Any other disk's Proxmox +// serial may be a SAS address or SCSI designator from VPD page 0x83 (#1595), +// or a USB bridge's own serial. +func hostAgentSMARTSerialComparable(disk models.PhysicalDisk, smart models.HostDiskSMART) bool { + agentType := strings.ToLower(strings.TrimSpace(smart.Type)) + switch strings.ToLower(strings.TrimSpace(disk.Type)) { + case "nvme": + return agentType == "nvme" + case "usb", "sas": + return false + } + return agentType == "sata" && strings.EqualFold(strings.TrimSpace(disk.Vendor), "ATA") +} + func shouldPromoteHostAgentSerial(disk models.PhysicalDisk, smart models.HostDiskSMART) bool { if smart.Collection != nil && strings.EqualFold(strings.TrimSpace(smart.Type), "sas") && diff --git a/internal/monitoring/monitor_host_agents_test.go b/internal/monitoring/monitor_host_agents_test.go index 8fc230e0b6..a55f0881a3 100644 --- a/internal/monitoring/monitor_host_agents_test.go +++ b/internal/monitoring/monitor_host_agents_test.go @@ -1,6 +1,7 @@ package monitoring import ( + "context" "encoding/json" "errors" "fmt" @@ -23,6 +24,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" agentsdocker "github.com/rcourtman/pulse-go-rewrite/pkg/agents/docker" agentshost "github.com/rcourtman/pulse-go-rewrite/pkg/agents/host" + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" "github.com/rcourtman/pulse-go-rewrite/pkg/metrics" "github.com/rcourtman/pulse-go-rewrite/pkg/proxmox" ) @@ -6945,3 +6947,137 @@ func TestSilentLinkedAgentStopsFeedingNodeDisk(t *testing.T) { t.Fatalf("agent reporting again: node disk = %+v from %q, agent %q; want the agent's summary and online", disk, source, host.Status) } } + +// A disk swapped into a slot must not take its identity from the linked host +// agent's row for the slot's previous occupant. A silent agent keeps its last +// rows, and that row used to match the replacement by device path alone: it +// filled a serial Proxmox reported as "unknown" (recorded as unreported, so +// the agent can fill it) or, for SAS, promoted its serial over the SAS address +// Proxmox reports, and the registry joined a SAS replacement to the agent's +// disk on the same path. Either way the replacement took the old disk's +// serial, canonical resource and history, temperature and failed health. +func TestHostAgentSMARTRowForSwappedOutDiskDoesNotLendItsIdentity(t *testing.T) { + const ( + oldSerial = "ZA1OLD0001" + oldAgentWWN = "5-c50-aaaa0001" // smartctl's NAA field spelling + oldPVEWWN = "0x5000c500aaaa0001" + newPVEWWN = "0x5000c500bbbb0002" + ) + for _, transport := range []struct { + agentType string + // pveSerial is what Proxmox reports for a disk with the given WWN. + pveSerial func(wwn string) string + // replacementSerial is the serial the replacement's record keeps. + replacementSerial string + serialState diskinventory.FieldState + }{ + { + agentType: "sata", + pveSerial: func(string) string { return "unknown" }, + replacementSerial: "", + serialState: diskinventory.FieldMissing, + }, + { + agentType: "sas", + pveSerial: func(wwn string) string { return strings.TrimPrefix(wwn, "0x") }, + replacementSerial: strings.TrimPrefix(newPVEWWN, "0x"), + serialState: diskinventory.FieldAvailable, + }, + } { + t.Run(transport.agentType, func(t *testing.T) { + t.Setenv("PULSE_DATA_DIR", t.TempDir()) + now := time.Now() + state := models.NewState() + state.UpdateNodesForInstance("pve1", []models.Node{{ + ID: "pve1-node1", Name: "node1", Instance: "pve1", Status: "online", + LastSeen: now, LinkedAgentID: "agent-1", + }}) + state.UpsertHost(models.Host{ + ID: "agent-1", Hostname: "node1", LinkedNodeID: "pve1-node1", + Status: "online", IntervalSeconds: 30, LastSeen: now, + Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{{ + Device: "/dev/sdb", Serial: oldSerial, WWN: oldAgentWWN, Type: transport.agentType, + Health: "FAILED", Temperature: 52, + Collection: &diskinventory.CollectionStatus{ + Serial: diskinventory.Available("smartctl"), + Temperature: diskinventory.Available("smartctl"), + }, + }}}, + }) + alertManager := alerts.NewManager() + t.Cleanup(alertManager.Stop) + adapter := unifiedresources.NewMonitorAdapter(unifiedresources.NewRegistry(nil)) + m := &Monitor{ + state: state, resourceStore: adapter, alertManager: alertManager, + metricsHistory: NewMetricsHistory(100, time.Hour), + startTime: now.Add(-time.Hour), lastPhysicalDiskPoll: make(map[string]time.Time), + } + client := &slotDiskPVEClient{} + pveDisk := func(wwn string) proxmox.Disk { + return proxmox.Disk{ + DevPath: "/dev/sdb", Model: "ST4000NM000A", Serial: transport.pveSerial(wwn), WWN: wwn, + Type: "hdd", Health: "PASSED", Wearout: 100, Size: 4000787030016, + } + } + poll := func() (models.PhysicalDisk, *unifiedresources.PhysicalDiskView) { + t.Helper() + adapter.PopulateFromSnapshot(state.GetSnapshot()) + started := time.Now() + delete(m.lastPhysicalDiskPoll, "pve1") + m.maybePollPhysicalDisksAsync(context.Background(), "pve1", &config.PVEInstance{}, client, + []proxmox.Node{{Node: "node1", Status: "online"}}, map[string]string{"node1": "online"}, nil) + deadline := time.Now().Add(3 * time.Second) + for { + if disks := state.GetSnapshot().PhysicalDisks; len(disks) == 1 && !disks[0].LastChecked.Before(started) { + adapter.PopulateFromSnapshot(state.GetSnapshot()) + var proxmoxViews []*unifiedresources.PhysicalDiskView + for _, view := range adapter.PhysicalDisks() { + if _, ok := view.SourceStatus(unifiedresources.SourceProxmox); ok { + proxmoxViews = append(proxmoxViews, view) + } + } + if len(proxmoxViews) != 1 { + t.Fatalf("canonical Proxmox physical disks = %d, want one", len(proxmoxViews)) + } + return disks[0], proxmoxViews[0] + } + if time.Now().After(deadline) { + t.Fatal("physical disk poll did not land in state") + } + time.Sleep(10 * time.Millisecond) + } + } + + client.setDisk(pveDisk(oldPVEWWN)) + original, originalView := poll() + if original.Serial != oldSerial || originalView.Serial() != oldSerial || + original.Collection == nil || original.Collection.Serial.State != diskinventory.FieldAvailable { + t.Fatalf("agent serial not merged into the disk it describes: state %q, canonical %q, collection %+v", + original.Serial, originalView.Serial(), original.Collection) + } + + // The agent stops reporting, keeping its rows, and the disk is replaced. + state.TouchHost("agent-1", now.Add(-hostAgentHealthWindow(30)-time.Minute)) + m.evaluateHostAgents(now) + client.setDisk(pveDisk(newPVEWWN)) + for round := 1; round <= 2; round++ { + got, view := poll() + if got.Serial != transport.replacementSerial || view.Serial() == oldSerial { + t.Fatalf("poll %d: replacement took the retained row's serial: state %q, canonical %q", + round, got.Serial, view.Serial()) + } + if view.ID() == originalView.ID() { + t.Fatalf("poll %d: replacement took over the old disk's canonical resource %q", round, view.ID()) + } + if got.Temperature != 0 || got.Health != "PASSED" || view.Temperature() != 0 || view.Health() != "PASSED" { + t.Fatalf("poll %d: replacement took the old disk's readings: state %d/%q, canonical %d/%q", + round, got.Temperature, got.Health, view.Temperature(), view.Health()) + } + if got.WWN != newPVEWWN || + got.Collection == nil || got.Collection.Serial.State != transport.serialState { + t.Fatalf("poll %d: replacement wwn %q, serial collection %+v", round, got.WWN, got.Collection) + } + } + }) + } +} diff --git a/internal/monitoring/monitor_pve.go b/internal/monitoring/monitor_pve.go index 4456d56f65..1e11c0b3d5 100644 --- a/internal/monitoring/monitor_pve.go +++ b/internal/monitoring/monitor_pve.go @@ -1105,8 +1105,8 @@ func (m *Monitor) maybePollPhysicalDisksAsync( DevPath: disk.DevPath, Model: disk.Model, Vendor: disk.Vendor, - Serial: disk.Serial, - WWN: disk.WWN, + Serial: proxmoxReportedDiskIdentity(disk.Serial), + WWN: proxmoxReportedDiskIdentity(disk.WWN), Type: disk.Type, Size: disk.Size, Health: normalizeProxmoxDiskHealth(disk.Health), @@ -1121,7 +1121,7 @@ func (m *Monitor) maybePollPhysicalDisksAsync( }, LastChecked: time.Now(), } - if strings.TrimSpace(disk.Serial) != "" { + if strings.TrimSpace(physicalDisk.Serial) != "" { physicalDisk.Collection.Serial = diskinventory.Available("proxmox_disks") } else { physicalDisk.Collection.Serial = diskinventory.Missing("proxmox_disks", "disk serial was not reported") @@ -1220,6 +1220,22 @@ func normalizeProxmoxDiskHealth(health string) string { } } +// proxmoxUnreportedDiskIdentity is what PVE's disk inventory +// (PVE::Diskmanage) writes into a disk's serial and wwn when udev has none. +const proxmoxUnreportedDiskIdentity = "unknown" + +// proxmoxReportedDiskIdentity records PVE's placeholder for an unreported +// serial or WWN as empty. Kept verbatim, the placeholder read as a collected +// serial, so the disk's collection state hid the gap and the linked host +// agent's serial or WWN never filled it. Identity keys are unaffected: +// diskinventory.IsUsableHardwareID already rejected the placeholder. +func proxmoxReportedDiskIdentity(value string) string { + if strings.EqualFold(strings.TrimSpace(value), proxmoxUnreportedDiskIdentity) { + return "" + } + return value +} + // physicalDisksFromHostAgentSMART builds PhysicalDisk entries for a node from // its linked host agent's SMART inventory. This is the fallback when the // Proxmox disks/list query fails: PVE probes SMART per disk inside that call, @@ -1371,8 +1387,11 @@ func preserveUnavailablePhysicalDiskEvidence(current, previous models.PhysicalDi if current.Collection == nil { return current } + // Only an identity is worth keeping: a record from before Proxmox's + // "unknown" was recorded as unreported would otherwise restore it. if current.Collection.Serial.State != diskinventory.FieldAvailable && - strings.TrimSpace(current.Serial) == "" { + strings.TrimSpace(current.Serial) == "" && + diskinventory.IsUsableHardwareID(previous.Serial) { current.Serial = previous.Serial } if current.Collection.Temperature.State != diskinventory.FieldAvailable && diff --git a/internal/monitoring/physical_disk_roundtrip_test.go b/internal/monitoring/physical_disk_roundtrip_test.go index 7c6840beaf..c5b4bb4f93 100644 --- a/internal/monitoring/physical_disk_roundtrip_test.go +++ b/internal/monitoring/physical_disk_roundtrip_test.go @@ -811,9 +811,9 @@ func (client *slotDiskPVEClient) GetDisks(context.Context, string) ([]proxmox.Di // it: the registry keys the disk on that serial, so the replacement would take // over the old disk's canonical resource, and every later poll would copy the // borrowed serial forward again. Current Proxmox spells a missing serial as -// "unknown", which the poller records as reported; an empty serial comes from -// producers that omit the field, such as the host-agent fallback rows. The -// fake client below stands in for any of them. +// "unknown", which the poller records as empty, as it does for producers that +// omit the field, such as the host-agent fallback rows. The fake client below +// stands in for any of them. func TestPhysicalDiskReplacementInSameSlotDoesNotInheritPreviousSerial(t *testing.T) { t.Setenv("PULSE_DATA_DIR", t.TempDir()) const ( diff --git a/internal/unifiedresources/registry.go b/internal/unifiedresources/registry.go index 6c9a786cad..7b452be5b7 100644 --- a/internal/unifiedresources/registry.go +++ b/internal/unifiedresources/registry.go @@ -104,6 +104,11 @@ type ResourceRegistry struct { // nil during a batch means reference reads use the live scan until the // final identity refresh rebuilds the index. Empty values are ambiguous. canonicalIdentityIndex map[string]string + // physicalDisksByHardware lists every disk resource ever indexed under a + // hardware key (physicalDiskHardwareKey). The IdentityMatcher keeps one + // resource per machine ID, so it cannot see same-serial disks on several + // machines. Entries are validated on read, never removed eagerly. + physicalDisksByHardware map[string]map[string]struct{} // Cached typed view indexes. Invalidated on ingest, rebuilt lazily on // first access. Protected by mu — callers hold RLock to read, and the @@ -835,6 +840,7 @@ func (rr *ResourceRegistry) ingestResources(resources []Resource, thresholds map rr.seedSourceMappingsFromResourceLocked(rr.resources[resourceID]) if resource := rr.resources[resourceID]; resource != nil { rr.indexSupersededCanonicalIDsLocked(resourceID, resource.SupersededCanonicalIDs) + rr.indexPhysicalDiskHardwareLocked(resource) } } rr.applyManualLinks(thresholds) @@ -2929,6 +2935,7 @@ func (rr *ResourceRegistry) ingestRecord(source DataSource, sourceID string, res } stampPhysicalDiskTemperatureReading(&resource, source, sourceID) rr.resources[resource.ID] = &resource + rr.indexPhysicalDiskHardwareLocked(&resource) rr.bySource[source][sourceID] = resource.ID rr.matcher.Add(resource.ID, identity) return resource.ID @@ -2936,6 +2943,9 @@ func (rr *ResourceRegistry) ingestRecord(source DataSource, sourceID string, res } resource.ID = rr.chooseNewID(resource.Type, identity, source, sourceID) + if resource.Type == ResourceTypePhysicalDisk { + resource.ID = rr.physicalDiskIDForMachineLocked(resource.ID, candidateID, &resource, source, onlyMissing) + } normalizeResourceRelationships(&resource) if existing := rr.resources[resource.ID]; existing != nil { if onlyMissing { @@ -2948,6 +2958,7 @@ func (rr *ResourceRegistry) ingestRecord(source DataSource, sourceID string, res } stampPhysicalDiskTemperatureReading(&resource, source, sourceID) rr.resources[resource.ID] = &resource + rr.indexPhysicalDiskHardwareLocked(&resource) rr.bySource[source][sourceID] = resource.ID rr.matcher.Add(resource.ID, identity) @@ -3272,8 +3283,14 @@ func (rr *ResourceRegistry) resolveLinkedPhysicalDisk(source DataSource, incomin existing.PhysicalDisk.Serial, existing.PhysicalDisk.WWN, ) existingDevice := strings.ToLower(normalizePhysicalDiskDeviceToken(existing.PhysicalDisk.DevPath)) - agentReportsSAS := (source == SourceProxmox && strings.EqualFold(existing.PhysicalDisk.DiskType, "sas")) || - (source == SourceAgent && strings.EqualFold(incoming.PhysicalDisk.DiskType, "sas")) + // Proxmox may put a SAS disk's transport address in its serial, so + // the serials cannot refuse this path join, but a WWN naming another + // disk can: the agent's row may be the slot's previous occupant, + // retained by a silent agent, and joining it hands the replacement + // that disk's serial, readings and canonical resource. + agentReportsSAS := ((source == SourceProxmox && strings.EqualFold(existing.PhysicalDisk.DiskType, "sas")) || + (source == SourceAgent && strings.EqualFold(incoming.PhysicalDisk.DiskType, "sas"))) && + !diskinventory.HardwareIdentityConflict("", incomingWWN, "", existing.PhysicalDisk.WWN) deviceMatch := (agentReportsSAS || physicalDiskMissingIdentityPathCompatible(incoming.PhysicalDisk, existing.PhysicalDisk)) && incomingDevice != "" && incomingDevice == existingDevice && @@ -3836,6 +3853,7 @@ func (rr *ResourceRegistry) mergeInto(existing *Resource, incoming Resource, sou existing.Name = incoming.Name } } + rr.indexPhysicalDiskHardwareLocked(existing) } func clearUnavailableSourceMemoryMetric(metrics *ResourceMetrics, incoming *Resource, source DataSource) *ResourceMetrics { @@ -5260,6 +5278,267 @@ func (rr *ResourceRegistry) chooseNewID(resourceType ResourceType, identity Reso return rr.sourceSpecificID(resourceType, source, sourceID) } +// physicalDiskIDForMachineLocked decides which resource a new disk +// observation joins once findMatch has refused every candidate. A usable +// serial or WWN identifies a disk only on one machine: dual-ported SAS shelves, +// cloned VMs with an explicit serial, fixed-serial USB bridges and TrueNAS +// systems report one identifier on several machines. id is chooseNewID's +// hardware-keyed ID and candidateID the observation's source-specific ID. +// +// With no other disk carrying the identity, the disk keeps id, so a disk seen +// on one machine never changes ID. Otherwise it joins the one disk it does not +// conflict with (a second reporter on the same machine), unless an operator +// exclusion separates them, which leaves it on candidateID as findMatch's +// excluded branch does. An exclusion recorded against any ID the disk can hold +// (id, its current ID or its machine-scoped ID) applies, so a split survives a +// re-key; once the identity spans machines, id no longer names one disk, so a +// split recorded against it applies to every copy (erring towards an extra +// row, never a merge the operator forbade). Manual links recorded against id +// stop applying for the same reason. When every disk carrying the identity +// sits on another machine, each +// copy takes the identity keyed to the machine of its canonical parent at that +// point, and the copy holding id is re-keyed too. So machines can be ingested +// in any order, and id never passes between disks that coexist on different +// machines. Like any serial-keyed ID it still passes to a same-serial disk +// that appears once the previous holder is gone, and a disk's ID changes when +// a same-serial disk first appears on another machine and back when it goes +// away. More than one joinable disk joins none, except that the holder of id +// keeps it. +func (rr *ResourceRegistry) physicalDiskIDForMachineLocked(id, candidateID string, incoming *Resource, source DataSource, onlyMissing bool) string { + key := physicalDiskHardwareKey(incoming.Identity) + if key == "" { + return id + } + siblings := rr.physicalDisksWithHardwareKeyLocked(key) + if len(siblings) == 0 { + return id + } + var joinable []*Resource + excluded := false + for _, sibling := range siblings { + if rr.physicalDiskMachinesConflictLocked(sibling, incoming, source) { + continue + } + if rr.isExcluded(sibling.ID, candidateID) || rr.isExcluded(id, candidateID) || + rr.isExcluded(rr.physicalDiskScopedIDLocked(sibling), candidateID) { + excluded = true + continue + } + joinable = append(joinable, sibling) + } + if len(joinable) == 1 { + return joinable[0].ID + } + for _, sibling := range joinable { + if sibling.ID == id { + return id + } + } + if excluded && len(joinable) == 0 { + return rr.physicalDiskFallbackIDLocked(candidateID, incoming, source) + } + scopedID := rr.physicalDiskScopedIDLocked(incoming) + if scopedID == "" { + return id + } + if holder := rr.resources[scopedID]; holder != nil && + (holder.Type != ResourceTypePhysicalDisk || rr.physicalDiskMachinesConflictLocked(holder, incoming, source)) { + return rr.physicalDiskFallbackIDLocked(candidateID, incoming, source) + } + if len(joinable) == 0 && !onlyMissing { + for _, sibling := range siblings { + if sibling.ID == id { + rr.rekeyPhysicalDiskLocked(sibling, rr.physicalDiskScopedIDLocked(sibling)) + } + } + } + return scopedID +} + +// physicalDiskMachinesConflictLocked reports whether two disks carrying one +// hardware identity provably sit on different machines. One reporter placing +// the identity on two machines is proof even when their hostnames match, as +// cloned VMs keep both. Different reporters may sit on machine resources the +// registry never merged, such as a TrueNAS system and the agent running on it, +// so there differing machines conflict only when both disks name hosts with no +// hostname in common. An unknown machine is never a conflict. +func (rr *ResourceRegistry) physicalDiskMachinesConflictLocked(existing, incoming *Resource, source DataSource) bool { + incomingMachine := rr.physicalDiskMachineLocked(incoming.ParentID) + reported, ok := existing.parentBySource[source] + if !ok && existing.ParentID != nil && len(existing.Sources) == 1 && existing.Sources[0] == source { + // Per-source parents do not survive serialization; a single-source + // disk's own parent is that source's. + reported, ok = *existing.ParentID, true + } + if ok && incomingMachine != "" { + if reportedMachine := rr.physicalDiskMachineLocked(&reported); reportedMachine != "" && reportedMachine != incomingMachine { + return true + } + } + if existingMachine := rr.physicalDiskMachineLocked(existing.ParentID); existingMachine != "" && existingMachine == incomingMachine { + return false + } + if physicalDiskHostname(existing) == "" || physicalDiskHostname(incoming) == "" { + return false + } + return !identitiesShareHostname(existing.Identity, incoming.Identity) +} + +// physicalDiskMachineLocked resolves a disk's parent to the machine it sits +// on: the nearest host-shaped ancestor (a Proxmox node, agent host or TrueNAS +// system above a pool or Unraid array), else the topmost known ancestor. +func (rr *ResourceRegistry) physicalDiskMachineLocked(parentID *string) string { + if parentID == nil { + return "" + } + id := CanonicalResourceID(strings.TrimSpace(*parentID)) + for depth := 0; id != "" && depth < 8; depth++ { + parent := rr.resources[id] + if parent == nil || parent.Type == ResourceTypeAgent || parent.ParentID == nil { + return id + } + next := CanonicalResourceID(strings.TrimSpace(*parent.ParentID)) + if next == "" || next == id { + return id + } + id = next + } + return id +} + +// physicalDiskScopedIDLocked keys a disk's hardware identity to its machine, +// or to its hostname when the machine is unknown. +func (rr *ResourceRegistry) physicalDiskScopedIDLocked(disk *Resource) string { + key := physicalDiskHardwareKey(disk.Identity) + scope := rr.physicalDiskScopeLocked(disk) + if key == "" || scope == "" { + return "" + } + return buildHashID(ResourceTypePhysicalDisk, key+"@"+scope) +} + +// physicalDiskFallbackIDLocked returns the observation's source-specific ID, +// keyed to its machine when another machine's disk already holds it: agent +// disk source IDs are the bare serial, so one candidate ID serves every host. +func (rr *ResourceRegistry) physicalDiskFallbackIDLocked(candidateID string, incoming *Resource, source DataSource) string { + holder := rr.resources[candidateID] + if holder == nil || (holder.Type == ResourceTypePhysicalDisk && !rr.physicalDiskMachinesConflictLocked(holder, incoming, source)) { + return candidateID + } + if scope := rr.physicalDiskScopeLocked(incoming); scope != "" { + return buildHashID(ResourceTypePhysicalDisk, candidateID+"@"+scope) + } + return candidateID +} + +func (rr *ResourceRegistry) physicalDiskScopeLocked(disk *Resource) string { + if machine := rr.physicalDiskMachineLocked(disk.ParentID); machine != "" { + return "machine:" + machine + } + if hostname := physicalDiskHostname(disk); hostname != "" { + return "host:" + hostname + } + return "" +} + +// physicalDiskHardwareKey is the stable key chooseNewID hashes into a disk's +// hardware-keyed canonical ID, or "" when the disk has no hardware identity. +func physicalDiskHardwareKey(identity ResourceIdentity) string { + if machineID := strings.TrimSpace(identity.MachineID); machineID != "" { + return "machine:" + machineID + } + if dmiUUID := strings.TrimSpace(identity.DMIUUID); dmiUUID != "" { + return "dmi:" + dmiUUID + } + return "" +} + +// physicalDiskHostname is the disk's least full hostname, so a scoped ID does +// not depend on the order merges appended hostnames in. +func physicalDiskHostname(disk *Resource) string { + least := "" + for _, hostname := range disk.Identity.Hostnames { + if NormalizeHostname(hostname) == "" { + continue + } + if full := NormalizeFullHostname(hostname); least == "" || full < least { + least = full + } + } + return least +} + +func (rr *ResourceRegistry) indexPhysicalDiskHardwareLocked(disk *Resource) { + if disk == nil || disk.Type != ResourceTypePhysicalDisk { + return + } + key := physicalDiskHardwareKey(disk.Identity) + if key == "" { + return + } + if rr.physicalDisksByHardware == nil { + rr.physicalDisksByHardware = make(map[string]map[string]struct{}) + } + ids := rr.physicalDisksByHardware[key] + if ids == nil { + ids = make(map[string]struct{}) + rr.physicalDisksByHardware[key] = ids + } + ids[disk.ID] = struct{}{} +} + +// physicalDisksWithHardwareKeyLocked returns the live disks carrying key, +// sorted by ID, and drops index entries that no longer match. +func (rr *ResourceRegistry) physicalDisksWithHardwareKeyLocked(key string) []*Resource { + ids := rr.physicalDisksByHardware[key] + disks := make([]*Resource, 0, len(ids)) + for id := range ids { + disk := rr.resources[id] + if disk == nil || disk.ID != id || disk.Type != ResourceTypePhysicalDisk || physicalDiskHardwareKey(disk.Identity) != key { + delete(ids, id) + continue + } + disks = append(disks, disk) + } + sort.Slice(disks, func(i, j int) bool { return disks[i].ID < disks[j].ID }) + return disks +} + +// rekeyPhysicalDiskLocked moves a disk to newID, carrying its source mappings, +// identity index entries, retired-ID claims, child parent references and +// relationship endpoints. +func (rr *ResourceRegistry) rekeyPhysicalDiskLocked(disk *Resource, newID string) { + oldID := disk.ID + if newID == "" || newID == oldID || rr.resources[newID] != nil { + return + } + delete(rr.resources, oldID) + disk.ID = newID + rr.resources[newID] = disk + rr.updateSourceMappings(oldID, newID) + rr.matcher.Add(newID, disk.Identity) + rr.indexPhysicalDiskHardwareLocked(disk) + for retiredID, claimant := range rr.supersededIndex { + if claimant == oldID { + rr.supersededIndex[retiredID] = newID + } + } + for _, resource := range rr.resources { + if resource == nil { + continue + } + for index := range resource.Relationships { + relationship := &resource.Relationships[index] + if relationship.SourceID == oldID { + relationship.SourceID = newID + } + if relationship.TargetID == oldID { + relationship.TargetID = newID + } + } + } +} + func (rr *ResourceRegistry) sourceResourceID(source DataSource, sourceID string) string { rr.mu.RLock() defer rr.mu.RUnlock() diff --git a/internal/unifiedresources/registry_merge_policy_test.go b/internal/unifiedresources/registry_merge_policy_test.go index 5b7a92adb6..89fafc70b0 100644 --- a/internal/unifiedresources/registry_merge_policy_test.go +++ b/internal/unifiedresources/registry_merge_policy_test.go @@ -2,6 +2,7 @@ package unifiedresources import ( "encoding/json" + "slices" "strconv" "strings" "testing" @@ -1926,3 +1927,464 @@ func TestPhysicalDiskMergeTemperatureReadingsAcrossIngestBoundaries(t *testing.T } }) } + +const sharedDiskSerial = "SHARED-SERIAL-1" + +// sharedSerialDiskSnapshot puts one /dev/sda carrying sharedDiskSerial under +// each named PVE node of one instance, optionally with a linked host agent +// reporting that disk over SMART on every node. pve1 also carries a disk whose +// serial no other machine reports. +func sharedSerialDiskSnapshot(now time.Time, withAgents bool, nodeNames ...string) models.StateSnapshot { + var snapshot models.StateSnapshot + for _, name := range nodeNames { + node := models.Node{ + ID: proxmoxNodeSourceID("pve", name), + Name: name, + Instance: "pve", + Host: "https://" + name + ":8006", + Status: "online", + LastSeen: now, + } + if withAgents { + node.LinkedAgentID = "host-" + name + snapshot.Hosts = append(snapshot.Hosts, models.Host{ + ID: "host-" + name, + Hostname: name, + MachineID: "machine-" + name, + LinkedNodeID: node.ID, + Status: "online", + LastSeen: now, + Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{{ + Device: "/dev/sda", + Model: "SEAGATE ST4000NM0023", + Serial: sharedDiskSerial, + Type: "sas", + Temperature: 31, + Health: "PASSED", + }}}, + }) + } + snapshot.Nodes = append(snapshot.Nodes, node) + snapshot.PhysicalDisks = append(snapshot.PhysicalDisks, models.PhysicalDisk{ + ID: ProxmoxPhysicalDiskSourceID("pve", name, "/dev/sda", "", ""), + Instance: "pve", + Node: name, + DevPath: "/dev/sda", + Model: "SEAGATE ST4000NM0023", + Serial: sharedDiskSerial, + Type: "sas", + Health: "PASSED", + LastChecked: now, + }) + if name == "pve1" { + snapshot.PhysicalDisks = append(snapshot.PhysicalDisks, models.PhysicalDisk{ + ID: ProxmoxPhysicalDiskSourceID("pve", name, "/dev/sdb", "", ""), + Instance: "pve", + Node: name, + DevPath: "/dev/sdb", + Model: "Samsung SSD 870", + Serial: "UNIQUE-SERIAL-1", + Type: "ssd", + Health: "PASSED", + LastChecked: now, + }) + } + } + return snapshot +} + +// reversedSnapshot reverses every slice sharedSerialDiskSnapshot fills, so a +// test can prove IDs do not depend on ingest order. +func reversedSnapshot(snapshot models.StateSnapshot) models.StateSnapshot { + slices.Reverse(snapshot.Nodes) + slices.Reverse(snapshot.Hosts) + slices.Reverse(snapshot.PhysicalDisks) + return snapshot +} + +// sharedSerialDisksByNode indexes the registry's sharedDiskSerial disks by +// Proxmox node, failing when two claim one node or one lost its node. +func sharedSerialDisksByNode(t *testing.T, rr *ResourceRegistry) map[string]Resource { + t.Helper() + byNode := make(map[string]Resource) + for _, disk := range rr.ListByType(ResourceTypePhysicalDisk) { + if disk.PhysicalDisk == nil || disk.PhysicalDisk.Serial != sharedDiskSerial { + continue + } + if disk.Proxmox == nil || disk.Proxmox.NodeName == "" { + t.Fatalf("disk %s has no Proxmox node: %+v", disk.ID, disk.Proxmox) + } + if _, dup := byNode[disk.Proxmox.NodeName]; dup { + t.Fatalf("two disks claim node %s", disk.Proxmox.NodeName) + } + byNode[disk.Proxmox.NodeName] = disk + } + return byNode +} + +func TestPhysicalDisksSharingASerialStayOnEachMachine(t *testing.T) { + now := time.Date(2026, 10, 7, 12, 0, 0, 0, time.UTC) + for _, tc := range []struct { + name string + withAgents bool + sources []DataSource + }{ + {name: "proxmox only", sources: []DataSource{SourceProxmox}}, + {name: "linked host agents", withAgents: true, sources: []DataSource{SourceProxmox, SourceAgent}}, + } { + t.Run(tc.name, func(t *testing.T) { + rr := NewRegistry(nil) + rr.IngestSnapshot(sharedSerialDiskSnapshot(now, tc.withAgents, "pve1", "pve2", "pve3")) + + byNode := sharedSerialDisksByNode(t, rr) + if len(byNode) != 3 { + t.Fatalf("shared-serial disks by node = %d, want one per node", len(byNode)) + } + bareID := rr.canonicalIDFromIdentity(ResourceTypePhysicalDisk, ResourceIdentity{MachineID: sharedDiskSerial}) + for _, name := range []string{"pve1", "pve2", "pve3"} { + disk := byNode[name] + nodeID := rr.bySource[SourceProxmox][proxmoxNodeSourceID("pve", name)] + if disk.ParentID == nil || *disk.ParentID != nodeID { + t.Fatalf("disk on %s parent = %v, want its node %s", name, disk.ParentID, nodeID) + } + for _, source := range tc.sources { + if !containsDataSource(disk.Sources, source) { + t.Fatalf("disk on %s sources = %v, want %s merged in", name, disk.Sources, source) + } + } + if !slices.ContainsFunc(rr.GetChildren(nodeID), func(child Resource) bool { return child.ID == disk.ID }) { + t.Fatalf("node %s does not list its disk %s as a child", name, disk.ID) + } + // No machine keeps the unscoped ID, so it can never pass + // from one machine's disk to another's. + if disk.ID == bareID { + t.Fatalf("disk on %s holds the unscoped ID %s shared by every machine", name, bareID) + } + } + + uniqueID := rr.canonicalIDFromIdentity(ResourceTypePhysicalDisk, ResourceIdentity{MachineID: "UNIQUE-SERIAL-1"}) + if unique, ok := rr.Get(uniqueID); !ok || unique.PhysicalDisk == nil || unique.PhysicalDisk.Serial != "UNIQUE-SERIAL-1" { + t.Fatalf("a disk only one machine reports must keep its unscoped ID %s", uniqueID) + } + + reordered := NewRegistry(nil) + reordered.IngestSnapshot(reversedSnapshot(sharedSerialDiskSnapshot(now, tc.withAgents, "pve1", "pve2", "pve3"))) + for name, disk := range sharedSerialDisksByNode(t, reordered) { + if disk.ID != byNode[name].ID { + t.Fatalf("reversed ingest moved the disk on %s from %s to %s", name, byNode[name].ID, disk.ID) + } + } + }) + } +} + +func TestPhysicalDiskIDsHoldAcrossRehydrationWhenAMachineJoins(t *testing.T) { + now := time.Date(2026, 10, 7, 12, 0, 0, 0, time.UTC) + first := NewRegistry(nil) + first.IngestSnapshot(sharedSerialDiskSnapshot(now, false, "pve2", "pve3")) + before := sharedSerialDisksByNode(t, first) + + rehydrated := NewRegistry(nil) + rehydrated.IngestResources(first.List()) + rehydrated.IngestSnapshot(sharedSerialDiskSnapshot(now.Add(time.Minute), false, "pve1", "pve2", "pve3")) + after := sharedSerialDisksByNode(t, rehydrated) + if len(after) != 3 { + t.Fatalf("shared-serial disks by node = %d, want one per node", len(after)) + } + for name, disk := range before { + if after[name].ID != disk.ID { + t.Fatalf("disk on %s moved from %s to %s when pve1 joined", name, disk.ID, after[name].ID) + } + } + + fresh := NewRegistry(nil) + fresh.IngestSnapshot(sharedSerialDiskSnapshot(now.Add(time.Minute), false, "pve1", "pve2", "pve3")) + for name, disk := range sharedSerialDisksByNode(t, fresh) { + if after[name].ID != disk.ID { + t.Fatalf("disk on %s is %s after rehydration but %s after a fresh rebuild", name, after[name].ID, disk.ID) + } + } +} + +// trueNASDiskRecords describes one TrueNAS system holding one disk, under a +// pool when pool is set. +func trueNASDiskRecords(now time.Time, system, hostname, pool, serial string) []IngestRecord { + records := []IngestRecord{{ + SourceID: "system:" + system, + Resource: Resource{Type: ResourceTypeAgent, Name: hostname, Status: StatusOnline, LastSeen: now, TrueNAS: &TrueNASData{Hostname: hostname}}, + Identity: ResourceIdentity{Hostnames: []string{hostname}}, + }} + diskParent := "system:" + system + if pool != "" { + diskParent = "pool:" + system + ":" + pool + records = append(records, IngestRecord{ + SourceID: diskParent, + ParentSourceID: "system:" + system, + Resource: Resource{Type: ResourceTypeStorage, Name: pool, Status: StatusOnline, LastSeen: now}, + Identity: ResourceIdentity{Hostnames: []string{hostname}}, + }) + } + return append(records, IngestRecord{ + SourceID: "disk:" + system + ":sda", + ParentSourceID: diskParent, + Resource: Resource{ + Type: ResourceTypePhysicalDisk, Name: "sda", Status: StatusOnline, LastSeen: now, + PhysicalDisk: &PhysicalDiskMeta{DevPath: "/dev/sda", Serial: serial, Wearout: WearoutUnreported}, + }, + Identity: ResourceIdentity{MachineID: serial, Hostnames: []string{hostname}}, + }) +} + +func TestPhysicalDiskReportersOnOneMachineJoinAcrossGroupingParents(t *testing.T) { + now := time.Date(2026, 10, 7, 12, 0, 0, 0, time.UTC) + agentOnArchive := models.StateSnapshot{Hosts: []models.Host{{ + ID: "host-archive", Hostname: "archive", MachineID: "machine-archive", Status: "online", LastSeen: now, + Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{{ + Device: "/dev/sda", Model: "WDC WD80EFAX", Serial: sharedDiskSerial, Type: "sata", Temperature: 30, Health: "PASSED", + }}}, + }}} + + // The TrueNAS API and the agent on the same box report one disk under + // different parents (pool and agent host) that the registry never merged. + rr := NewRegistry(nil) + rr.IngestSnapshot(agentOnArchive) + rr.IngestRecords(SourceTrueNAS, trueNASDiskRecords(now, "a", "archive", "tank", sharedDiskSerial)) + disks := rr.ListByType(ResourceTypePhysicalDisk) + if len(disks) != 1 || !containsDataSource(disks[0].Sources, SourceAgent) || !containsDataSource(disks[0].Sources, SourceTrueNAS) { + t.Fatalf("disks = %+v, want the agent and TrueNAS rows of one box merged", disks) + } + + // A cloned TrueNAS keeps both the serial and the hostname; one reporter + // placing the serial on two systems still proves two disks. + rr.IngestRecords(SourceTrueNAS, trueNASDiskRecords(now, "b", "archive", "tank", sharedDiskSerial)) + disks = rr.ListByType(ResourceTypePhysicalDisk) + if len(disks) != 2 { + t.Fatalf("disks = %d, want the cloned system's disk kept apart", len(disks)) + } + bySystem := make(map[string]Resource) + for _, disk := range disks { + for _, source := range []string{"disk:a:sda", "disk:b:sda"} { + if rr.bySource[SourceTrueNAS][source] == disk.ID { + bySystem[source] = disk + } + } + } + if len(bySystem) != 2 { + t.Fatalf("TrueNAS disk mappings = %+v, want one disk per system", rr.bySource[SourceTrueNAS]) + } + if !containsDataSource(bySystem["disk:a:sda"].Sources, SourceAgent) { + t.Fatalf("system a disk sources = %v, want the agent row still merged", bySystem["disk:a:sda"].Sources) + } + + // A disk moving into a pool on the same machine is the same disk. + moved := NewRegistry(nil) + moved.IngestRecords(SourceTrueNAS, trueNASDiskRecords(now, "a", "archive", "", sharedDiskSerial)) + moved.IngestRecords(SourceTrueNAS, trueNASDiskRecords(now.Add(time.Minute), "a", "archive", "tank", sharedDiskSerial)) + if disks := moved.ListByType(ResourceTypePhysicalDisk); len(disks) != 1 { + t.Fatalf("disks after moving into a pool = %d, want 1", len(disks)) + } +} + +func TestPhysicalDiskWithoutItsNodeRowJoinsItsOwnMachinesDisk(t *testing.T) { + now := time.Date(2026, 10, 7, 12, 0, 0, 0, time.UTC) + snapshot := sharedSerialDiskSnapshot(now, false, "pve1", "pve2") + // pve3 has an agent but no node row this poll, so its PVE disk arrives + // without a parent while the serial already spans other machines. + snapshot.Hosts = append(snapshot.Hosts, models.Host{ + ID: "host-pve3", Hostname: "pve3", MachineID: "machine-pve3", Status: "online", LastSeen: now, + Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{{ + Device: "/dev/sda", Model: "SEAGATE ST4000NM0023", Serial: sharedDiskSerial, Type: "sas", Temperature: 31, Health: "PASSED", + }}}, + }) + snapshot.PhysicalDisks = append(snapshot.PhysicalDisks, models.PhysicalDisk{ + ID: ProxmoxPhysicalDiskSourceID("pve", "pve3", "/dev/sda", "", ""), Instance: "pve", Node: "pve3", + DevPath: "/dev/sda", Model: "SEAGATE ST4000NM0023", Serial: sharedDiskSerial, Type: "sas", Health: "PASSED", LastChecked: now, + }) + + rr := NewRegistry(nil) + rr.IngestSnapshot(snapshot) + byNode := sharedSerialDisksByNode(t, rr) + if len(byNode) != 3 { + t.Fatalf("shared-serial disks by node = %d, want one per node", len(byNode)) + } + if sources := byNode["pve3"].Sources; !containsDataSource(sources, SourceAgent) || !containsDataSource(sources, SourceProxmox) { + t.Fatalf("pve3 disk sources = %v, want its agent and parentless PVE rows merged", sources) + } +} + +func TestPhysicalDiskSiblingJoinHonoursManualExclusions(t *testing.T) { + now := time.Date(2026, 10, 7, 12, 0, 0, 0, time.UTC) + // An agent on pve1 that the registry never linked to the node reports the + // disk after both nodes' rows; hostname evidence alone joins it to pve1's. + agentOnPVE1 := models.StateSnapshot{Hosts: []models.Host{{ + ID: "host-pve1", Hostname: "pve1", MachineID: "machine-pve1", Status: "online", LastSeen: now, + Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{{ + Device: "/dev/sda", Model: "SEAGATE ST4000NM0023", Serial: sharedDiskSerial, Type: "sas", Temperature: 31, Health: "PASSED", + }}}, + }}} + ingest := func(store ResourceStore) *ResourceRegistry { + rr := NewRegistry(store) + rr.IngestSnapshot(sharedSerialDiskSnapshot(now, false, "pve1", "pve2")) + rr.IngestSnapshot(agentOnPVE1) + return rr + } + + joined := ingest(nil) + pve1Disk := sharedSerialDisksByNode(t, joined)["pve1"] + if !containsDataSource(pve1Disk.Sources, SourceAgent) { + t.Fatalf("pve1 disk sources = %v, want the agent row joined", pve1Disk.Sources) + } + + store := NewMemoryStore() + agentCandidate := buildHashID(ResourceTypePhysicalDisk, string(SourceAgent)+":"+normalizeSourceID(sharedDiskSerial)) + if err := store.AddExclusion(ResourceExclusion{ResourceA: pve1Disk.ID, ResourceB: agentCandidate}); err != nil { + t.Fatal(err) + } + split := ingest(store) + if disk, ok := split.Get(pve1Disk.ID); !ok || containsDataSource(disk.Sources, SourceAgent) { + t.Fatalf("excluded pve1 disk = %+v, want it kept apart from the agent row", disk) + } + if agentDisk, ok := split.Get(agentCandidate); !ok || !containsDataSource(agentDisk.Sources, SourceAgent) { + t.Fatalf("agent row should keep its source-specific ID %s once excluded", agentCandidate) + } +} + +func TestPhysicalDiskClonesStayApartAfterSerializedRehydration(t *testing.T) { + now := time.Date(2026, 10, 7, 12, 0, 0, 0, time.UTC) + first := NewRegistry(nil) + first.IngestRecords(SourceTrueNAS, trueNASDiskRecords(now, "a", "archive", "tank", sharedDiskSerial)) + payload, err := json.Marshal(first.List()) + if err != nil { + t.Fatal(err) + } + var persisted []Resource + if err := json.Unmarshal(payload, &persisted); err != nil { + t.Fatal(err) + } + + rehydrated := NewRegistry(nil) + rehydrated.IngestResources(persisted) + // A clone keeps the hostname; the persisted disk lost its per-source + // parents, so only its own parent says which system TrueNAS saw it on. + rehydrated.IngestRecords(SourceTrueNAS, trueNASDiskRecords(now, "b", "archive", "tank", sharedDiskSerial)) + if disks := rehydrated.ListByType(ResourceTypePhysicalDisk); len(disks) != 2 { + t.Fatalf("disks = %d, want the clone's disk kept apart after rehydration", len(disks)) + } +} + +func TestPhysicalDiskRekeyCarriesRetiredIDClaims(t *testing.T) { + now := time.Date(2026, 10, 7, 12, 0, 0, 0, time.UTC) + records := trueNASDiskRecords(now, "a", "archive-a", "tank", sharedDiskSerial) + records[len(records)-1].SupersededCanonicalIDs = []string{"physical_disk-retired"} + rr := NewRegistry(nil) + rr.IngestRecords(SourceTrueNAS, records) + bareID := rr.canonicalIDFromIdentity(ResourceTypePhysicalDisk, ResourceIdentity{MachineID: sharedDiskSerial}) + if got := rr.supersededResourceIDLocked("physical_disk-retired"); got != bareID { + t.Fatalf("retired ID resolves to %q, want the disk %s", got, bareID) + } + + rr.IngestRecords(SourceTrueNAS, trueNASDiskRecords(now, "b", "archive-b", "tank", sharedDiskSerial)) + systemADisk := rr.bySource[SourceTrueNAS]["disk:a:sda"] + if systemADisk == bareID || systemADisk == "" { + t.Fatalf("system a disk = %q, want it re-keyed off the unscoped ID", systemADisk) + } + if got := rr.supersededResourceIDLocked("physical_disk-retired"); got != systemADisk { + t.Fatalf("retired ID resolves to %q after the re-key, want %s", got, systemADisk) + } +} + +func TestPhysicalDiskSplitSurvivesARekey(t *testing.T) { + now := time.Date(2026, 10, 7, 12, 0, 0, 0, time.UTC) + agentDisk := func(id, hostname string) models.Host { + return models.Host{ + ID: id, Hostname: hostname, MachineID: "machine-" + hostname, Status: "online", LastSeen: now, + Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{{ + Device: "/dev/sda", Model: "WDC WD80EFAX", Serial: sharedDiskSerial, Type: "sata", Temperature: 30, Health: "PASSED", + }}}, + } + } + trueNASCandidate := buildHashID(ResourceTypePhysicalDisk, string(SourceTrueNAS)+":"+normalizeSourceID("disk:a:sda")) + bareID := MachineIdentityCanonicalID(ResourceTypePhysicalDisk, sharedDiskSerial) + ingest := func(t *testing.T, excludedID string, hosts ...models.Host) *ResourceRegistry { + t.Helper() + store := NewMemoryStore() + if err := store.AddExclusion(ResourceExclusion{ResourceA: excludedID, ResourceB: trueNASCandidate}); err != nil { + t.Fatal(err) + } + rr := NewRegistry(store) + rr.IngestSnapshot(models.StateSnapshot{Hosts: hosts}) + rr.IngestRecords(SourceTrueNAS, trueNASDiskRecords(now, "a", "archive", "tank", sharedDiskSerial)) + return rr + } + split := func(t *testing.T, rr *ResourceRegistry) { + t.Helper() + trueNASDisk, ok := rr.Get(trueNASCandidate) + if !ok || containsDataSource(trueNASDisk.Sources, SourceAgent) { + t.Fatalf("TrueNAS row = %+v, want it kept on %s apart from the agent row", trueNASDisk, trueNASCandidate) + } + } + + t.Run("split recorded against the unscoped ID after another machine re-keyed the disk", func(t *testing.T) { + rr := ingest(t, bareID, agentDisk("host-archive", "archive"), agentDisk("host-other", "other")) + if _, ok := rr.Get(bareID); ok { + t.Fatalf("the unscoped ID %s should be re-keyed once two machines report the serial", bareID) + } + split(t, rr) + }) + t.Run("split recorded against the scoped ID before the serial spans machines", func(t *testing.T) { + scoped := ingest(t, bareID, agentDisk("host-archive", "archive"), agentDisk("host-other", "other")) + archiveAgentDisk := "" + for _, disk := range scoped.ListByType(ResourceTypePhysicalDisk) { + if disk.ParentID != nil && *disk.ParentID == scoped.bySource[SourceAgent]["host-archive"] { + archiveAgentDisk = disk.ID + } + } + if archiveAgentDisk == "" || archiveAgentDisk == bareID { + t.Fatalf("archive agent disk = %q, want a machine-scoped ID", archiveAgentDisk) + } + split(t, ingest(t, archiveAgentDisk, agentDisk("host-archive", "archive"))) + }) +} + +func TestPhysicalDiskExclusionFallbackStaysPerMachine(t *testing.T) { + now := time.Date(2026, 10, 7, 12, 0, 0, 0, time.UTC) + agents := models.StateSnapshot{} + for _, name := range []string{"pve1", "pve2"} { + agents.Hosts = append(agents.Hosts, models.Host{ + ID: "host-" + name, Hostname: name, MachineID: "machine-" + name, Status: "online", LastSeen: now, + Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{{ + Device: "/dev/sda", Model: "SEAGATE ST4000NM0023", Serial: sharedDiskSerial, Type: "sas", Temperature: 31, Health: "PASSED", + }}}, + }) + } + probe := NewRegistry(nil) + probe.IngestSnapshot(sharedSerialDiskSnapshot(now, false, "pve1", "pve2")) + byNode := sharedSerialDisksByNode(t, probe) + + // Both nodes' disks are split from the agent observation, whose + // source-specific ID is the same on every host. + store := NewMemoryStore() + agentCandidate := buildHashID(ResourceTypePhysicalDisk, string(SourceAgent)+":"+normalizeSourceID(sharedDiskSerial)) + for _, name := range []string{"pve1", "pve2"} { + if err := store.AddExclusion(ResourceExclusion{ResourceA: byNode[name].ID, ResourceB: agentCandidate}); err != nil { + t.Fatal(err) + } + } + rr := NewRegistry(store) + rr.IngestSnapshot(sharedSerialDiskSnapshot(now, false, "pve1", "pve2")) + rr.IngestSnapshot(agents) + + agentDisks := 0 + for _, disk := range rr.ListByType(ResourceTypePhysicalDisk) { + if !containsDataSource(disk.Sources, SourceAgent) { + continue + } + agentDisks++ + if containsDataSource(disk.Sources, SourceProxmox) { + t.Fatalf("disk %s merged an excluded agent row into a node's disk", disk.ID) + } + } + if agentDisks != 2 { + t.Fatalf("agent disks = %d, want one per host rather than both hosts on the shared fallback ID", agentDisks) + } +} diff --git a/pkg/diskinventory/identity_conflict.go b/pkg/diskinventory/identity_conflict.go new file mode 100644 index 0000000000..32517cdc1b --- /dev/null +++ b/pkg/diskinventory/identity_conflict.go @@ -0,0 +1,61 @@ +package diskinventory + +import "strings" + +// HardwareIdentityConflict reports whether two observations of one disk slot +// carry hardware identities that name different disks: nothing agrees across +// serial and WWN (HardwareIdentityMatch), and at least one field reported on +// both sides disagrees. A field missing or unusable on either side is no +// evidence, so an observation without identity never conflicts. +// +// Reporters spell one identity differently, and spelling is not disagreement: +// - serials are compared on letters and digits only, because udev rewrites +// whitespace and unusual characters in a serial as underscores while +// smartctl keeps them, and a T10 vendor designator for an ATA drive +// ("ATA", the model, then the drive serial) is no evidence against the +// serial it ends with, because udev reports one when a SCSI-ATA +// translation layer hides IDENTIFY from it; +// - a WWN that begins with the other is no evidence, because udev's ID_WWN +// carries only the first 64 bits of a 128-bit NAA 6 identifier that sysfs +// and smartctl report in full. +// +// A caller blanks a field its producer may not report as disk identity, such +// as a SAS disk's serial, where Proxmox may report a SAS address, and checks +// HardwareIdentityMatch with the field kept if it may still show agreement. +func HardwareIdentityConflict(leftSerial, leftWWN, rightSerial, rightWWN string) bool { + if HardwareIdentityMatch(leftSerial, leftWWN, rightSerial, rightWWN) { + return false + } + return serialsDisagree(leftSerial, rightSerial) || wwnsDisagree(leftWWN, rightWWN) +} + +func serialsDisagree(left, right string) bool { + left, right = compactSerial(normalizeHardwareID(left)), compactSerial(normalizeHardwareID(right)) + return left != "" && right != "" && left != right && + !isATADesignatorOf(left, right) && !isATADesignatorOf(right, left) +} + +// isATADesignatorOf reports whether designator, compacted, is the T10 vendor +// designator of the ATA drive whose serial is given: the "ata" vendor field, +// a model, then that serial. +func isATADesignatorOf(designator, serial string) bool { + const vendor = "ata" + return len(designator) > len(vendor)+len(serial) && + strings.HasPrefix(designator, vendor) && strings.HasSuffix(designator, serial) +} + +func compactSerial(value string) string { + var builder strings.Builder + for _, char := range value { + if (char >= 'a' && char <= 'z') || (char >= '0' && char <= '9') { + builder.WriteRune(char) + } + } + return builder.String() +} + +func wwnsDisagree(left, right string) bool { + left, right = normalizeHardwareWWN(left), normalizeHardwareWWN(right) + return left != "" && right != "" && + !strings.HasPrefix(left, right) && !strings.HasPrefix(right, left) +} diff --git a/pkg/diskinventory/identity_conflict_test.go b/pkg/diskinventory/identity_conflict_test.go new file mode 100644 index 0000000000..ae84722881 --- /dev/null +++ b/pkg/diskinventory/identity_conflict_test.go @@ -0,0 +1,36 @@ +package diskinventory + +import "testing" + +func TestHardwareIdentityConflict(t *testing.T) { + for _, tc := range []struct { + name string + leftSerial, leftWWN string + rightSerial, rightWWN string + want bool + }{ + {name: "different serials", leftSerial: "WD-NEW0001", rightSerial: "WD-OLD0001", want: true}, + {name: "different WWNs", leftWWN: "0x5000c500b0000001", rightWWN: "naa.5000c500a0000001", want: true}, + {name: "WWN disagrees while serial is absent on one side", leftSerial: "PVE-ONLY", leftWWN: "0x5000c500b0000001", rightWWN: "5-c50-a0000001", want: true}, + {name: "same serial", leftSerial: "WD-WX1234", rightSerial: "wd-wx1234"}, + {name: "udev underscores for whitespace", leftSerial: "WD-WX_1234", rightSerial: "WD-WX 1234"}, + {name: "T10 designator ending with the drive serial", leftSerial: "ATA_ST4000NM000A-2HZ_ZC1ABCDE", rightSerial: "ZC1ABCDE"}, + {name: "serial that only begins with the other", leftSerial: "SN1234", rightSerial: "SN123", want: true}, + {name: "bare serial that ends with the other", leftSerial: "001234567890", rightSerial: "1234567890", want: true}, + {name: "agent NAA fields against PVE hex", leftSerial: "5000c500a0000001", leftWWN: "0x5000c500a0000001", rightSerial: "ZR5TESTA0001", rightWWN: "5-c50-a0000001"}, + {name: "udev 64-bit prefix of an NAA 6 WWN", leftWWN: "0x600508b1001c4d5e", rightWWN: "naa.600508b1001c4d5e7f8a9b0c1d2e3f40"}, + {name: "RAID volume NAA as serial against WWN", leftSerial: "600508b1001c4d5e7f8a9b0c1d2e3f40", leftWWN: "0x600508b1001c4d5e", rightSerial: "OTHER", rightWWN: "naa.600508b1001c4d5e7f8a9b0c1d2e3f40"}, + {name: "serial against WWN only is no evidence", leftSerial: "WD-NEW0001", rightWWN: "naa.5000c500a0000001"}, + {name: "placeholders are no evidence", leftSerial: "unknown", leftWWN: "0x0000000000000000", rightSerial: "WD-OLD0001", rightWWN: "naa.5000c500a0000001"}, + {name: "one side without identity", rightSerial: "WD-OLD0001", rightWWN: "naa.5000c500a0000001"}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := HardwareIdentityConflict(tc.leftSerial, tc.leftWWN, tc.rightSerial, tc.rightWWN); got != tc.want { + t.Fatalf("HardwareIdentityConflict = %v, want %v", got, tc.want) + } + if got := HardwareIdentityConflict(tc.rightSerial, tc.rightWWN, tc.leftSerial, tc.leftWWN); got != tc.want { + t.Fatalf("HardwareIdentityConflict (swapped) = %v, want %v", got, tc.want) + } + }) + } +}