From 64bc03a837adbf1bce677a4a5bd481bf8342f3bd Mon Sep 17 00:00:00 2001 From: rcourtman Date: Mon, 30 Mar 2026 02:49:42 +0100 Subject: [PATCH] fix(storage): use canonical disk history identity --- .../subsystems/frontend-primitives.md | 33 ++- .../internal/subsystems/storage-recovery.md | 7 + .../src/components/Storage/DiskDetail.tsx | 6 +- .../Storage/__tests__/DiskList.test.tsx | 40 ++++ .../__tests__/diskResourceUtils.test.ts | 33 +++ .../__tests__/useDiskDetailModel.test.ts | 31 ++- .../components/Storage/diskResourceUtils.ts | 20 ++ .../components/Storage/useDiskDetailModel.ts | 9 +- .../__tests__/diskDetailPresentation.test.ts | 2 +- .../storageBackups/diskDetailPresentation.ts | 2 +- .../27-truenas-storage-disk-history.spec.ts | 211 ++++++++++++++++++ 11 files changed, 373 insertions(+), 21 deletions(-) create mode 100644 tests/integration/tests/27-truenas-storage-disk-history.spec.ts diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index b003be1e9..82effa140 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -159,65 +159,70 @@ work extends shared components instead of creating new local variants. plus `tests/integration/tests/15-settings-shell-consistency.spec.ts` 3. Update this contract when a new canonical UI pattern is adopted 4. Remove local forks after the shared primitive is introduced -5. When a settings route header and a top-level settings shell describe the same +5. Keep shared feature-level presenters on capability truth. When reusable + presenters under `frontend-modern/src/features/` explain why a control, + chart, or detail surface is unavailable, they must describe the owned + identity or capability gap instead of prescribing a provider-local install + path that conflicts with API-backed platforms like TrueNAS. +6. When a settings route header and a top-level settings shell describe the same commercial surface, keep them on the same shared presentation owner instead of allowing route metadata in `settingsHeaderMeta.ts` or labels in `settingsNavCatalog.ts` to drift into independent title or description copy, and keep adjacent settings-shell referrals such as `InfrastructureWorkspace.tsx` on that same shared owner instead of reintroducing local “go to Pulse Pro” variants. -6. Keep hosted settings-shell framing imports safe for bundle initialization. +7. Keep hosted settings-shell framing imports safe for bundle initialization. Self-hosted billing titles, descriptions, and referral copy used by `settingsHeaderMeta.ts`, `settingsNavCatalog.ts`, and adjacent settings shells must flow through `frontend-modern/src/components/Settings/selfHostedBillingPresentation.ts` instead of importing generic commercial presentation helpers directly into hosted settings route shells. -7. Keep first-session dashboard empty-state copy on +8. Keep first-session dashboard empty-state copy on `frontend-modern/src/utils/dashboardEmptyStatePresentation.ts`, and make infrastructure setup guidance name the canonical destination explicitly instead of falling back to generic settings CTA labels. -8. Keep the live first-session wizard on the canonical three-step runtime +9. Keep the live first-session wizard on the canonical three-step runtime shape in `frontend-modern/src/components/SetupWizard/SetupWizard.tsx` (`Welcome`, `Security`, then `Install`), and keep the step indicator plus completion CTA language aligned with the governed infrastructure install workspace instead of regressing to a route jump that leaves the next action implicit. -8. Keep the first welcome screen in +10. Keep the first welcome screen in `frontend-modern/src/components/SetupWizard/steps/WelcomeStep.tsx` explicit about operator context. The shell must explain that the bootstrap token only unlocks first-run setup, state where the command should run, and adapt command/help text to detected Docker or containerized deployments instead of assuming the operator already knows which host or container owns the Pulse install. -9. Keep the settings-shell infrastructure landing path aligned with that same +11. Keep the settings-shell infrastructure landing path aligned with that same first-session story. `frontend-modern/src/components/Settings/settingsNavigationModel.ts` must treat `/settings` and the infrastructure settings tab as the canonical path to `/settings/infrastructure/install`, not to reporting/control, so the shell does not send first-time operators to the wrong infrastructure subview by default. -10. Keep dashboard onboarding copy on the shared presentation owner in +12. Keep dashboard onboarding copy on the shared presentation owner in `frontend-modern/src/utils/dashboardEmptyStatePresentation.ts`. Both the infrastructure empty state and the dashboard route's no-resources state must name the canonical install workspace explicitly, keep `Platform connections` visible as the API-backed alternative for Proxmox and TrueNAS, and expose the same first-host next step instead of falling back to passive “nothing here yet” wording. -11. Keep the authenticated app root aligned with that same first-session path. +13. Keep the authenticated app root aligned with that same first-session path. `frontend-modern/src/App.tsx` must land `/` on the dashboard shell and let the governed dashboard empty state route first-time operators into Infrastructure Install, instead of preserving a separate root-only jump to `/infrastructure` that drifts from the rest of the onboarding contract. -12. Keep relay settings shell copy on the shared presentation owner in +14. Keep relay settings shell copy on the shared presentation owner in `frontend-modern/src/utils/relayPresentation.ts`. The route metadata in `settingsHeaderMeta.ts` and the leading `SettingsPanel` in `RelaySettingsPanel.tsx` must reuse the same description and availability copy instead of drifting into separate rollout or pairing wording. -13. Keep shared settings-shell legal and docs referrals on +15. Keep shared settings-shell legal and docs referrals on `frontend-modern/src/utils/docsLinks.ts`. Shared settings surfaces such as `AIRuntimeControlsSection.tsx` must not hardcode GitHub `main` doc URLs for privacy, security, proxy-auth, scope-reference, or Terms-of-Service links. -14. Keep shared settings-shell telemetry transparency controls on the governed +16. Keep shared settings-shell telemetry transparency controls on the governed general settings panel. Preview/reset affordances for anonymous telemetry must stay rendered inside `frontend-modern/src/components/Settings/GeneralSettingsPanel.tsx` @@ -1538,3 +1543,9 @@ summary state. `useInfrastructureSettingsState.ts`, Proxmox/PBS/PMG/TrueNAS counts and availability from one shared infrastructure settings state source instead of letting the reporting summary and the provider-specific panel fetch the same TrueNAS connection state separately. +That same shared feature-presentation boundary also owns storage disk-detail +fallback messaging in `frontend-modern/src/features/storageBackups/`. Shared +detail presenters must describe the actual capability or identity gap that +prevents history from rendering, rather than reviving agent-install guidance +on API-backed platforms like TrueNAS when the canonical disk metrics target is +already the owning history path. diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 98be39d41..578f37c96 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -138,6 +138,13 @@ contract in `frontend-modern/src/utils/storageSources.ts`: storage pages and cross-surface storage links must reuse one canonical ordering, label, tone, and default-option model for sources like PVE, PBS, Ceph, and TrueNAS instead of re-sorting or re-presenting those source options locally. +That same storage ownership also includes the physical-disk detail identity +contract in `frontend-modern/src/components/Storage/` and +`frontend-modern/src/features/storageBackups/`: historical disk charts must +resolve through the canonical disk metrics target when one exists, then fall +back to stable hardware identity, and operator-facing fallback copy must +describe that identity gap instead of prescribing agent installation on +API-backed platforms like TrueNAS. The recovery backend is a real product boundary, not just a helper package: `internal/recovery/` owns per-tenant SQLite persistence, rollup derivation, diff --git a/frontend-modern/src/components/Storage/DiskDetail.tsx b/frontend-modern/src/components/Storage/DiskDetail.tsx index 39c236d18..418bae8d6 100644 --- a/frontend-modern/src/components/Storage/DiskDetail.tsx +++ b/frontend-modern/src/components/Storage/DiskDetail.tsx @@ -45,7 +45,7 @@ export const DiskDetail: Component = (props) => { chartRange, setChartRange, diskData, - resId, + historyResourceId, attributeCards, historyCharts, metricResourceId, @@ -148,7 +148,7 @@ export const DiskDetail: Component = (props) => { {/* Historical charts */} {getDiskDetailHistoryFallbackMessage()} @@ -163,7 +163,7 @@ export const DiskDetail: Component = (props) => {
{ fireEvent.click(screen.getByText('Cache SSD')); expect(screen.getByTestId('disk-detail')).toHaveTextContent('sdb'); }); + + it('keeps api-backed TrueNAS disks on the canonical physical-disk surface even without hardware ids', () => { + render(() => ( + + )); + + expect(screen.getByText('TrueNAS')).toBeInTheDocument(); + fireEvent.click(screen.getByText('Seagate IronWolf')); + expect(screen.getByTestId('disk-detail')).toHaveTextContent('sda'); + }); }); diff --git a/frontend-modern/src/components/Storage/__tests__/diskResourceUtils.test.ts b/frontend-modern/src/components/Storage/__tests__/diskResourceUtils.test.ts index cdda681ab..b867b2dc0 100644 --- a/frontend-modern/src/components/Storage/__tests__/diskResourceUtils.test.ts +++ b/frontend-modern/src/components/Storage/__tests__/diskResourceUtils.test.ts @@ -3,6 +3,7 @@ import type { Resource } from '@/types/resource'; import { getPhysicalDiskNodeIdentity, matchesPhysicalDiskNode, + resolvePhysicalDiskHistoryResourceId, resolvePhysicalDiskMetricResourceId, } from '@/components/Storage/diskResourceUtils'; @@ -74,4 +75,36 @@ describe('diskResourceUtils', () => { ), ).toBe('existing-target'); }); + + it('resolves canonical physical disk history ids through metrics targets before raw hardware ids', () => { + expect( + resolvePhysicalDiskHistoryResourceId( + buildDisk({ + metricsTarget: { resourceType: 'disk', resourceId: 'disk:truenas-main:sda' } as any, + physicalDisk: { serial: '', wwn: '' } as any, + platformData: { + physicalDisk: { serial: '', wwn: '' }, + } as any, + }), + ), + ).toBe('disk:truenas-main:sda'); + + expect( + resolvePhysicalDiskHistoryResourceId( + buildDisk({ + metricsTarget: undefined, + physicalDisk: { serial: 'SERIAL-1', wwn: '' } as any, + }), + ), + ).toBe('SERIAL-1'); + + expect( + resolvePhysicalDiskHistoryResourceId( + buildDisk({ + metricsTarget: undefined, + physicalDisk: { serial: '', wwn: 'WWN-1' } as any, + }), + ), + ).toBe('WWN-1'); + }); }); diff --git a/frontend-modern/src/components/Storage/__tests__/useDiskDetailModel.test.ts b/frontend-modern/src/components/Storage/__tests__/useDiskDetailModel.test.ts index 0a20d428e..8e136b006 100644 --- a/frontend-modern/src/components/Storage/__tests__/useDiskDetailModel.test.ts +++ b/frontend-modern/src/components/Storage/__tests__/useDiskDetailModel.test.ts @@ -12,7 +12,7 @@ vi.mock('@/stores/diskMetricsHistory', () => ({ ], })); -const buildDisk = (): Resource => +const buildDisk = (overrides: Partial = {}): Resource => ({ id: 'disk-1', type: 'physical_disk', @@ -40,6 +40,7 @@ const buildDisk = (): Resource => reallocatedSectors: 0, }, }, + ...overrides, }) as unknown as Resource; describe('useDiskDetailModel', () => { @@ -55,7 +56,7 @@ describe('useDiskDetailModel', () => { ); expect(result.chartRange()).toBe('24h'); - expect(result.resId()).toBe('SERIAL-1'); + expect(result.historyResourceId()).toBe('agent-tower:sda'); expect(result.metricResourceId()).toBe('agent-tower:sda'); expect(result.attributeCards().length).toBeGreaterThan(0); expect(result.historyCharts().map((chart) => chart.metric)).toEqual([ @@ -75,4 +76,30 @@ describe('useDiskDetailModel', () => { { timestamp: 2000, value: 35, min: 35, max: 35 }, ]); }); + + it('keeps historical charts available for api-backed disks without serial or wwn when a canonical disk target exists', () => { + const [disk] = createSignal( + buildDisk({ + metricsTarget: { resourceType: 'disk', resourceId: 'disk:truenas-main:sda' }, + physicalDisk: { + devPath: '/dev/sda', + model: 'Seagate IronWolf', + serial: '', + wwn: '', + diskType: 'hdd', + temperature: 39, + }, + } as Partial), + ); + const [nodes] = createSignal([]); + + const { result } = renderHook(() => + useDiskDetailModel({ + disk, + nodes, + }), + ); + + expect(result.historyResourceId()).toBe('disk:truenas-main:sda'); + }); }); diff --git a/frontend-modern/src/components/Storage/diskResourceUtils.ts b/frontend-modern/src/components/Storage/diskResourceUtils.ts index d65790c72..b39c199f1 100644 --- a/frontend-modern/src/components/Storage/diskResourceUtils.ts +++ b/frontend-modern/src/components/Storage/diskResourceUtils.ts @@ -6,6 +6,10 @@ type DiskPlatformData = { nodeName?: string; instance?: string; }; + physicalDisk?: { + serial?: string; + wwn?: string; + }; }; export interface PhysicalDiskNodeIdentity { @@ -14,6 +18,7 @@ export interface PhysicalDiskNodeIdentity { } const normalize = (value: string | null | undefined): string => value?.trim().toLowerCase() || ''; +const trim = (value: string | null | undefined): string => value?.trim() || ''; export const getPhysicalDiskNodeIdentity = (resource: Resource): PhysicalDiskNodeIdentity => { const platformData = ((resource.platformData as DiskPlatformData | undefined) || @@ -72,3 +77,18 @@ export const resolvePhysicalDiskMetricResourceId = ( const deviceName = devPath.replace('/dev/', ''); return `${agentId}:${deviceName}`; }; + +export const resolvePhysicalDiskHistoryResourceId = (disk: Resource): string | null => { + const metricsTargetType = normalize(disk.metricsTarget?.resourceType); + const metricsTargetId = trim(disk.metricsTarget?.resourceId); + if (metricsTargetId && (!metricsTargetType || metricsTargetType === 'disk')) { + return metricsTargetId; + } + + const platformData = ((disk.platformData as DiskPlatformData | undefined) || {}) as DiskPlatformData; + const serial = trim(disk.physicalDisk?.serial || platformData.physicalDisk?.serial); + if (serial) return serial; + + const wwn = trim(disk.physicalDisk?.wwn || platformData.physicalDisk?.wwn); + return wwn || null; +}; diff --git a/frontend-modern/src/components/Storage/useDiskDetailModel.ts b/frontend-modern/src/components/Storage/useDiskDetailModel.ts index f03027bf7..8f5284291 100644 --- a/frontend-modern/src/components/Storage/useDiskDetailModel.ts +++ b/frontend-modern/src/components/Storage/useDiskDetailModel.ts @@ -10,7 +10,10 @@ import { } from '@/features/storageBackups/diskDetailPresentation'; import { getDiskMetricHistory, getDiskMetricsVersion } from '@/stores/diskMetricsHistory'; import type { Resource } from '@/types/resource'; -import { resolvePhysicalDiskMetricResourceId } from './diskResourceUtils'; +import { + resolvePhysicalDiskHistoryResourceId, + resolvePhysicalDiskMetricResourceId, +} from './diskResourceUtils'; type UseDiskDetailModelOptions = { disk: Accessor; @@ -35,7 +38,7 @@ export const useDiskDetailModel = (options: UseDiskDetailModelOptions) => { const diskData = createMemo(() => extractPhysicalDiskPresentationData(options.disk()), ); - const resId = createMemo(() => diskData().serial || diskData().wwn || null); + const historyResourceId = createMemo(() => resolvePhysicalDiskHistoryResourceId(options.disk())); const attributeCards = createMemo(() => getDiskDetailAttributeCards(diskData())); const historyCharts = createMemo(() => getDiskDetailHistoryCharts(diskData())); const metricResourceId = createMemo(() => @@ -68,7 +71,7 @@ export const useDiskDetailModel = (options: UseDiskDetailModelOptions) => { chartRange, setChartRange, diskData, - resId, + historyResourceId, attributeCards, historyCharts, metricResourceId, diff --git a/frontend-modern/src/features/storageBackups/__tests__/diskDetailPresentation.test.ts b/frontend-modern/src/features/storageBackups/__tests__/diskDetailPresentation.test.ts index ab0b64464..20c4f61dd 100644 --- a/frontend-modern/src/features/storageBackups/__tests__/diskDetailPresentation.test.ts +++ b/frontend-modern/src/features/storageBackups/__tests__/diskDetailPresentation.test.ts @@ -104,7 +104,7 @@ describe('diskDetailPresentation', () => { ]); expect(DISK_DETAIL_LIVE_CHARTS.map((chart) => chart.label)).toEqual(['Read', 'Write', 'Busy']); expect(getDiskDetailLiveBadgeLabel()).toBe('Real-time'); - expect(getDiskDetailHistoryFallbackMessage()).toContain('Pulse agent'); + expect(getDiskDetailHistoryFallbackMessage()).toContain('stable identity'); expect( getDiskDetailHistoryCharts({ diff --git a/frontend-modern/src/features/storageBackups/diskDetailPresentation.ts b/frontend-modern/src/features/storageBackups/diskDetailPresentation.ts index ab2d2ee4a..207ba7804 100644 --- a/frontend-modern/src/features/storageBackups/diskDetailPresentation.ts +++ b/frontend-modern/src/features/storageBackups/diskDetailPresentation.ts @@ -68,7 +68,7 @@ export const DISK_DETAIL_LIVE_CHARTS: readonly DiskDetailLiveChartConfig[] = [ export const getDiskDetailLiveBadgeLabel = (): string => 'Real-time'; export const getDiskDetailHistoryFallbackMessage = (): string => - 'Install the Pulse agent for detailed SMART monitoring and historical charts.'; + 'Historical disk charts are unavailable until Pulse can resolve a stable identity for this disk.'; export function getDiskDetailAttributeCards( disk: PhysicalDiskPresentationData, diff --git a/tests/integration/tests/27-truenas-storage-disk-history.spec.ts b/tests/integration/tests/27-truenas-storage-disk-history.spec.ts new file mode 100644 index 000000000..9a9eea2c1 --- /dev/null +++ b/tests/integration/tests/27-truenas-storage-disk-history.spec.ts @@ -0,0 +1,211 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { test as base, expect } from '@playwright/test'; +import { createAuthenticatedStorageState } from './helpers'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); + +type WorkerFixtures = { + authStorageStatePath: string; +}; + +const SCREENSHOT_PATH = '/tmp/truenas-storage-disk-history.png'; + +const test = base.extend<{}, WorkerFixtures>({ + storageState: async ({ authStorageStatePath }, use) => { + await use(authStorageStatePath); + }, + authStorageStatePath: [async ({ browser }, use, workerInfo) => { + const storageStatePath = path.resolve( + __dirname, + '..', + '..', + 'tmp', + 'playwright-auth', + `truenas-storage-disk-history-${workerInfo.project.name}.json`, + ); + fs.mkdirSync(path.dirname(storageStatePath), { recursive: true }); + await createAuthenticatedStorageState(browser, storageStatePath); + try { + await use(storageStatePath); + } finally { + fs.rmSync(storageStatePath, { force: true }); + } + }, { scope: 'worker' }], +}); + +test.describe('TrueNAS storage disk history', () => { + test.setTimeout(180_000); + + test('uses the canonical disk metrics target for TrueNAS disk history even without serial or WWN', async ({ + page, + }) => { + const historyRequests: string[] = []; + + await page.route('**/api/resources**', async (route) => { + const requestUrl = new URL(route.request().url()); + if (requestUrl.pathname !== '/api/resources') { + await route.continue(); + return; + } + + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + data: [ + { + id: 'truenas-main', + type: 'agent', + name: 'truenas-main', + displayName: 'TrueNAS Main', + platformId: 'truenas-main', + platformType: 'truenas', + sourceType: 'hybrid', + sources: ['agent', 'truenas'], + status: 'online', + lastSeen: '2026-03-29T22:00:00Z', + canonicalIdentity: { + displayName: 'TrueNAS Main', + hostname: 'truenas-main', + platformId: 'truenas-main', + }, + agent: { + hostname: 'truenas-main', + platform: 'TrueNAS SCALE', + uptimeSeconds: 86400, + }, + platformData: { + sources: ['agent', 'truenas'], + }, + }, + { + id: 'disk-truenas-sda', + type: 'physical_disk', + name: 'disk-truenas-sda', + displayName: 'disk-truenas-sda', + parentId: 'truenas-main', + parentName: 'truenas-main', + platformId: 'truenas-main', + platformType: 'truenas', + sourceType: 'api', + sources: ['truenas'], + status: 'online', + lastSeen: '2026-03-29T22:00:00Z', + metricsTarget: { + resourceType: 'disk', + resourceId: 'disk:truenas-main:sda', + }, + canonicalIdentity: { + displayName: 'disk-truenas-sda', + hostname: 'truenas-main', + platformId: 'truenas-main', + }, + physicalDisk: { + devPath: '/dev/sda', + model: 'Seagate IronWolf', + serial: '', + wwn: '', + diskType: 'hdd', + sizeBytes: 4_000 * 1024 * 1024 * 1024, + health: 'PASSED', + temperature: 41, + smart: {}, + }, + platformData: { + sources: ['truenas'], + physicalDisk: { + serial: '', + wwn: '', + }, + }, + }, + ], + meta: { + page: 1, + limit: 200, + total: 2, + totalPages: 1, + }, + }), + }); + }); + + await page.route('**/api/storage-charts**', async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + pools: {}, + disks: {}, + stats: { + oldestDataTimestamp: Date.parse('2026-03-29T20:00:00Z'), + }, + }), + }); + }); + + await page.route('**/api/metrics-store/history**', async (route) => { + const requestUrl = new URL(route.request().url()); + historyRequests.push(requestUrl.search); + + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + resourceType: requestUrl.searchParams.get('resourceType') || 'disk', + resourceId: requestUrl.searchParams.get('resourceId') || '', + metric: requestUrl.searchParams.get('metric') || 'smart_temp', + range: requestUrl.searchParams.get('range') || '24h', + start: Date.parse('2026-03-29T20:00:00Z'), + end: Date.parse('2026-03-29T22:00:00Z'), + points: [ + { + timestamp: Date.parse('2026-03-29T20:30:00Z'), + value: 39, + min: 39, + max: 39, + }, + { + timestamp: Date.parse('2026-03-29T21:30:00Z'), + value: 41, + min: 41, + max: 41, + }, + ], + source: 'store', + }), + }); + }); + + await page.goto('/storage?tab=disks&source=truenas&node=truenas-main', { + waitUntil: 'domcontentloaded', + }); + + await expect(page).toHaveURL(/\/storage\?tab=disks&source=truenas&node=truenas-main/); + await expect(page.getByRole('tab', { name: 'Physical Disks' })).toHaveAttribute( + 'aria-selected', + 'true', + ); + + await page.getByRole('button', { name: 'Toggle details for Seagate IronWolf' }).click(); + + await expect(page.getByText('Historical disk charts are unavailable', { exact: false })).toHaveCount(0); + await expect(page.getByText('Temperature').first()).toBeVisible(); + await expect + .poll(() => + historyRequests.some((query) => { + const params = new URLSearchParams(query); + return ( + params.get('resourceType') === 'disk' && + params.get('resourceId') === 'disk:truenas-main:sda' && + params.get('metric') === 'smart_temp' + ); + }), + ) + .toBe(true); + + await page.screenshot({ path: SCREENSHOT_PATH, fullPage: true }); + }); +});