From ad00517a0ea5c9ab1a2b992e0cad622b0f2d4503 Mon Sep 17 00:00:00 2001 From: Anso Date: Mon, 20 Jul 2026 21:40:35 -0400 Subject: [PATCH] fix(settings): toast and block save when node settings fail to load (#1654) * fix(settings): toast and block save when node settings fail to load Failed GET /settings no longer seeds defaults as the dirty baseline. Shared loader pins node identity, shows an error callout until an authoritative load succeeds, and keeps Save disabled across all eight node-scoped sections. * test(settings): cover load-failure toast and App Store refresh pin Remove the incorrect settings-load-failure docs image. Assert toast ownership for current vs stale/aborted loads, and pin App Store cache refresh to the PATCH node across a mid-save switch. * fix(settings): reject malformed settings bodies and skip pre-node bootstrap load Treat non-object 200 /settings payloads as load failures so null or array bodies cannot seed editable defaults. Do not fetch or toast while the active node id is still undefined, so hard refresh settles to a single failure toast. --- docs/reference/settings.mdx | 2 + .../components/settings/AppStoreSection.tsx | 65 +-- .../settings/ContainerAlertsSection.tsx | 54 +-- .../settings/DataRetentionSection.tsx | 62 +-- .../components/settings/DeveloperSection.tsx | 54 +-- .../settings/DockerStorageSection.tsx | 58 +-- .../components/settings/FleetMeshSection.tsx | 56 +-- .../components/settings/HostAlertsSection.tsx | 62 +-- .../components/settings/SettingsLoadError.tsx | 36 ++ .../src/components/settings/StacksSection.tsx | 62 +-- .../__tests__/SettingsLoadFailures.test.tsx | 388 ++++++++++++++++++ .../__tests__/fetchNodeSettings.test.ts | 94 +++++ .../__tests__/useNodeSettingsLoad.test.ts | 175 ++++++++ .../components/settings/fetchNodeSettings.ts | 46 +++ .../settings/useNodeSettingsLoad.ts | 89 ++++ 15 files changed, 1086 insertions(+), 217 deletions(-) create mode 100644 frontend/src/components/settings/SettingsLoadError.tsx create mode 100644 frontend/src/components/settings/__tests__/SettingsLoadFailures.test.tsx create mode 100644 frontend/src/components/settings/__tests__/fetchNodeSettings.test.ts create mode 100644 frontend/src/components/settings/__tests__/useNodeSettingsLoad.test.ts create mode 100644 frontend/src/components/settings/fetchNodeSettings.ts create mode 100644 frontend/src/components/settings/useNodeSettingsLoad.ts diff --git a/docs/reference/settings.mdx b/docs/reference/settings.mdx index 5e7e460e..a7aae0bb 100644 --- a/docs/reference/settings.mdx +++ b/docs/reference/settings.mdx @@ -51,6 +51,8 @@ Click **Filter** at the top of the sidebar, or press `Ctrl+K` / `⌘K` while the For node-scoped sections the masthead replaces the **SCOPE** pill with a **NODE** pill showing the active node name. Switch the active node from the top bar's **Switch node** button to edit a different node's per-node settings. +If settings for the active node fail to load, Sencho shows an error and keeps Save unavailable until an authoritative load for that node succeeds. + Host Alerts section showing the NODE pill in the masthead diff --git a/frontend/src/components/settings/AppStoreSection.tsx b/frontend/src/components/settings/AppStoreSection.tsx index 8780e6af..4f69e03a 100644 --- a/frontend/src/components/settings/AppStoreSection.tsx +++ b/frontend/src/components/settings/AppStoreSection.tsx @@ -5,10 +5,13 @@ import { Skeleton } from '@/components/ui/skeleton'; import { toast } from '@/components/ui/toast-store'; import { apiFetch } from '@/lib/api'; import { useAuth } from '@/context/AuthContext'; +import { useNodes } from '@/context/NodeContext'; import { RefreshCw } from 'lucide-react'; import { SettingsSection } from './SettingsSection'; import { SettingsField } from './SettingsField'; import { SettingsActions, SettingsPrimaryButton } from './SettingsActions'; +import { useNodeSettingsLoad } from './useNodeSettingsLoad'; +import { SettingsLoadGate } from './SettingsLoadError'; function SectionSkeleton() { return ( @@ -21,33 +24,30 @@ function SectionSkeleton() { export function AppStoreSection() { const { isAdmin } = useAuth(); + const { activeNode } = useNodes(); const readOnly = !isAdmin; const [templateRegistryUrl, setTemplateRegistryUrl] = useState(''); const serverUrl = useRef(''); - const [isLoading, setIsLoading] = useState(false); + const { phase, isCurrentNodeLoaded, load, isSaveOwner, captureSaveGuard } = useNodeSettingsLoad(activeNode?.id); const [isSavingRegistry, setIsSavingRegistry] = useState(false); useEffect(() => { - const fetchSettings = async () => { - setIsLoading(true); - try { - const res = await apiFetch('/settings'); - if (res.ok) { - const data: Record = await res.json(); - const url = data.template_registry_url ?? ''; - setTemplateRegistryUrl(url); - serverUrl.current = url; - } - } catch (e) { - console.error('Failed to fetch app store settings', e); - } finally { - setIsLoading(false); - } + let cancelled = false; + setIsSavingRegistry(false); + void (async () => { + const nodeData = await load(); + if (cancelled || !nodeData) return; + const url = nodeData.template_registry_url ?? ''; + setTemplateRegistryUrl(url); + serverUrl.current = url; + })(); + return () => { + cancelled = true; }; - fetchSettings(); - }, []); + }, [activeNode?.id, load]); const saveRegistrySettings = async () => { + const saveGuard = captureSaveGuard(); const trimmedUrl = templateRegistryUrl.trim(); if (trimmedUrl && !/^https?:\/\/./.test(trimmedUrl)) { toast.error('Registry URL must start with http:// or https://'); @@ -57,26 +57,38 @@ export function AppStoreSection() { try { const res = await apiFetch('/settings', { method: 'PATCH', + nodeId: saveGuard.nodeId, body: JSON.stringify({ template_registry_url: trimmedUrl }), }); if (!res.ok) { - const err = await res.json().catch(() => ({})); - toast.error(err?.error || err?.message || 'Failed to save registry settings.'); + if (isSaveOwner(saveGuard)) { + const err = await res.json().catch(() => ({})); + toast.error(err?.error || err?.message || 'Failed to save registry settings.'); + } + return; + } + // Always refresh the node that received the PATCH, even after a switch. + const refresh = await apiFetch('/templates/refresh-cache', { + method: 'POST', + nodeId: saveGuard.nodeId, + }); + if (!isSaveOwner(saveGuard)) return; + if (!refresh.ok) { + toast.error('Registry saved, but refreshing the App Store cache failed.'); return; } serverUrl.current = templateRegistryUrl; - await apiFetch('/templates/refresh-cache', { method: 'POST' }); toast.success('Registry saved. App Store will reload from the new source.'); } catch (e: unknown) { + if (!isSaveOwner(saveGuard)) return; toast.error((e as Error)?.message || 'Failed to save registry settings.'); } finally { - setIsSavingRegistry(false); + if (isSaveOwner(saveGuard)) setIsSavingRegistry(false); } }; - if (isLoading) return ; - return ( + }>
setTemplateRegistryUrl('')} - disabled={isSavingRegistry || !templateRegistryUrl} + disabled={isSavingRegistry || !templateRegistryUrl || !isCurrentNodeLoaded} > Reset to default - + {isSavingRegistry ? ( <> @@ -129,5 +141,6 @@ export function AppStoreSection() {
+ ); } diff --git a/frontend/src/components/settings/ContainerAlertsSection.tsx b/frontend/src/components/settings/ContainerAlertsSection.tsx index ef58e3e6..53bf90a0 100644 --- a/frontend/src/components/settings/ContainerAlertsSection.tsx +++ b/frontend/src/components/settings/ContainerAlertsSection.tsx @@ -13,6 +13,8 @@ import { SettingsField } from './SettingsField'; import { SettingsActions, SettingsPrimaryButton } from './SettingsActions'; import { useMastheadStats } from './MastheadStatsContext'; import { useSettingsDirty } from './useSettingsDirty'; +import { useNodeSettingsLoad } from './useNodeSettingsLoad'; +import { SettingsLoadGate } from './SettingsLoadError'; interface ContainerAlertsSectionProps { onDirtyChange?: (dirty: boolean) => void; @@ -37,15 +39,17 @@ export function ContainerAlertsSection({ onDirtyChange }: ContainerAlertsSection const { activeNode } = useNodes(); const readOnly = !isAdmin; const { settings, setSettings, dirtyCount, hasChanges, reset, markSaved } = useSettingsDirty({ ...DEFAULT_CONTAINER_ALERTS }); - const [isLoading, setIsLoading] = useState(false); + const { phase, isCurrentNodeLoaded, load, isSaveOwner, captureSaveGuard } = useNodeSettingsLoad(activeNode?.id); const [isSaving, setIsSaving] = useState(false); + const reportDirty = isCurrentNodeLoaded && hasChanges; + useEffect(() => { - onDirtyChange?.(hasChanges); - }, [hasChanges, onDirtyChange]); + onDirtyChange?.(reportDirty); + }, [reportDirty, onDirtyChange]); useMastheadStats( - isLoading + !isCurrentNodeLoaded ? null : [ { @@ -57,37 +61,36 @@ export function ContainerAlertsSection({ onDirtyChange }: ContainerAlertsSection ); useEffect(() => { - const fetchSettings = async () => { - setIsLoading(true); - try { - const nodeRes = await apiFetch('/settings'); - const nodeData: Record = nodeRes.ok ? await nodeRes.json() : {}; - const safe: ContainerAlertFields = { - global_crash: (nodeData.global_crash as '0' | '1') ?? DEFAULT_SETTINGS.global_crash, - }; - reset(safe); - } catch (e) { - console.error('Failed to fetch container alert settings', e); - } finally { - setIsLoading(false); - } + let cancelled = false; + setIsSaving(false); + void (async () => { + const nodeData = await load(); + if (cancelled || !nodeData) return; + const safe: ContainerAlertFields = { + global_crash: (nodeData.global_crash as '0' | '1') ?? DEFAULT_SETTINGS.global_crash, + }; + reset(safe); + })(); + return () => { + cancelled = true; }; - fetchSettings(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [activeNode?.id]); + }, [activeNode?.id, load, reset]); const onSettingChange = (key: K, value: ContainerAlertFields[K]) => { setSettings(prev => ({ ...prev, [key]: value })); }; const saveSettings = async () => { + const saveGuard = captureSaveGuard(); const submitted = { ...settings }; setIsSaving(true); try { const res = await apiFetch('/settings', { method: 'PATCH', + nodeId: saveGuard.nodeId, body: JSON.stringify({ global_crash: submitted.global_crash }), }); + if (!isSaveOwner(saveGuard)) return; if (!res.ok) { const err = await res.json().catch(() => ({})); toast.error(err?.error || err?.message || 'Failed to save settings.'); @@ -96,15 +99,15 @@ export function ContainerAlertsSection({ onDirtyChange }: ContainerAlertsSection markSaved(submitted); toast.success('Container alert settings saved.'); } catch (e: unknown) { + if (!isSaveOwner(saveGuard)) return; toast.error((e as Error)?.message || 'Something went wrong.'); } finally { - setIsSaving(false); + if (isSaveOwner(saveGuard)) setIsSaving(false); } }; - if (isLoading) return ; - return ( + }>
{!readOnly && ( - + {isSaving ? ( <> @@ -133,5 +136,6 @@ export function ContainerAlertsSection({ onDirtyChange }: ContainerAlertsSection )}
+ ); } diff --git a/frontend/src/components/settings/DataRetentionSection.tsx b/frontend/src/components/settings/DataRetentionSection.tsx index 9e964187..9d850f76 100644 --- a/frontend/src/components/settings/DataRetentionSection.tsx +++ b/frontend/src/components/settings/DataRetentionSection.tsx @@ -16,6 +16,8 @@ import { SettingsField } from './SettingsField'; import { SettingsActions, SettingsPrimaryButton } from './SettingsActions'; import { useMastheadStats } from './MastheadStatsContext'; import { useSettingsDirty } from './useSettingsDirty'; +import { useNodeSettingsLoad } from './useNodeSettingsLoad'; +import { SettingsLoadGate } from './SettingsLoadError'; import { TogglePill } from '@/components/ui/toggle-pill'; interface DataRetentionSectionProps { @@ -48,15 +50,17 @@ export function DataRetentionSection({ onDirtyChange }: DataRetentionSectionProp const { activeNode } = useNodes(); const readOnly = !isAdmin; const { settings, setSettings, dirtyCount, hasChanges, reset, markSaved } = useSettingsDirty({ ...DEFAULT_DATA_RETENTION }); - const [isLoading, setIsLoading] = useState(false); + const { phase, isCurrentNodeLoaded, load, isSaveOwner, captureSaveGuard } = useNodeSettingsLoad(activeNode?.id); const [isSaving, setIsSaving] = useState(false); + const reportDirty = isCurrentNodeLoaded && hasChanges; + useEffect(() => { - onDirtyChange?.(hasChanges); - }, [hasChanges, onDirtyChange]); + onDirtyChange?.(reportDirty); + }, [reportDirty, onDirtyChange]); useMastheadStats( - isLoading + !isCurrentNodeLoaded ? null : [ { @@ -68,34 +72,31 @@ export function DataRetentionSection({ onDirtyChange }: DataRetentionSectionProp ); useEffect(() => { - const fetchSettings = async () => { - setIsLoading(true); - try { - const nodeRes = await apiFetch('/settings'); - const nodeData: Record = nodeRes.ok ? await nodeRes.json() : {}; - const safe: DataRetentionFields = { - metrics_retention_hours: nodeData.metrics_retention_hours ?? DEFAULT_SETTINGS.metrics_retention_hours, - log_retention_days: nodeData.log_retention_days ?? DEFAULT_SETTINGS.log_retention_days, - audit_retention_days: nodeData.audit_retention_days ?? DEFAULT_SETTINGS.audit_retention_days, - scan_history_per_image_limit: nodeData.scan_history_per_image_limit ?? DEFAULT_SETTINGS.scan_history_per_image_limit, - prune_orphaned_scans: (nodeData.prune_orphaned_scans as '0' | '1') ?? DEFAULT_SETTINGS.prune_orphaned_scans, - }; - reset(safe); - } catch (e) { - console.error('Failed to fetch data retention settings', e); - } finally { - setIsLoading(false); - } + let cancelled = false; + setIsSaving(false); + void (async () => { + const nodeData = await load(); + if (cancelled || !nodeData) return; + const safe: DataRetentionFields = { + metrics_retention_hours: nodeData.metrics_retention_hours ?? DEFAULT_SETTINGS.metrics_retention_hours, + log_retention_days: nodeData.log_retention_days ?? DEFAULT_SETTINGS.log_retention_days, + audit_retention_days: nodeData.audit_retention_days ?? DEFAULT_SETTINGS.audit_retention_days, + scan_history_per_image_limit: nodeData.scan_history_per_image_limit ?? DEFAULT_SETTINGS.scan_history_per_image_limit, + prune_orphaned_scans: (nodeData.prune_orphaned_scans as '0' | '1') ?? DEFAULT_SETTINGS.prune_orphaned_scans, + }; + reset(safe); + })(); + return () => { + cancelled = true; }; - fetchSettings(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [activeNode?.id]); + }, [activeNode?.id, load, reset]); const onSettingChange = (key: K, value: DataRetentionFields[K]) => { setSettings(prev => ({ ...prev, [key]: value })); }; const saveSettings = async () => { + const saveGuard = captureSaveGuard(); const submitted = { ...settings }; const payload: DataRetentionFields = { metrics_retention_hours: submitted.metrics_retention_hours, @@ -114,8 +115,10 @@ export function DataRetentionSection({ onDirtyChange }: DataRetentionSectionProp try { const res = await apiFetch('/settings', { method: 'PATCH', + nodeId: saveGuard.nodeId, body: JSON.stringify(payload), }); + if (!isSaveOwner(saveGuard)) return; if (!res.ok) { const err = await res.json().catch(() => ({})); toast.error(err?.error || err?.message || 'Failed to save settings.'); @@ -127,15 +130,15 @@ export function DataRetentionSection({ onDirtyChange }: DataRetentionSectionProp detail: { changedKeys: Object.keys(payload) }, })); } catch (e: unknown) { + if (!isSaveOwner(saveGuard)) return; toast.error((e as Error)?.message || 'Something went wrong.'); } finally { - setIsSaving(false); + if (isSaveOwner(saveGuard)) setIsSaving(false); } }; - if (isLoading) return ; - return ( + }>
{!readOnly && ( - + {isSaving ? ( <> @@ -234,5 +237,6 @@ export function DataRetentionSection({ onDirtyChange }: DataRetentionSectionProp )}
+ ); } diff --git a/frontend/src/components/settings/DeveloperSection.tsx b/frontend/src/components/settings/DeveloperSection.tsx index 235a9f26..411e0055 100644 --- a/frontend/src/components/settings/DeveloperSection.tsx +++ b/frontend/src/components/settings/DeveloperSection.tsx @@ -15,6 +15,8 @@ import { SettingsField } from './SettingsField'; import { SettingsActions, SettingsPrimaryButton } from './SettingsActions'; import { useMastheadStats } from './MastheadStatsContext'; import { useSettingsDirty } from './useSettingsDirty'; +import { useNodeSettingsLoad } from './useNodeSettingsLoad'; +import { SettingsLoadGate } from './SettingsLoadError'; interface DeveloperSectionProps { onDirtyChange?: (dirty: boolean) => void; @@ -39,15 +41,17 @@ export function DeveloperSection({ onDirtyChange }: DeveloperSectionProps) { const { activeNode } = useNodes(); const readOnly = !isAdmin; const { settings, setSettings, hasChanges, reset, markSaved } = useSettingsDirty({ ...DEFAULT_DEVELOPER }); - const [isLoading, setIsLoading] = useState(false); + const { phase, isCurrentNodeLoaded, load, isSaveOwner, captureSaveGuard } = useNodeSettingsLoad(activeNode?.id); const [isSaving, setIsSaving] = useState(false); + const reportDirty = isCurrentNodeLoaded && hasChanges; + useEffect(() => { - onDirtyChange?.(hasChanges); - }, [hasChanges, onDirtyChange]); + onDirtyChange?.(reportDirty); + }, [reportDirty, onDirtyChange]); useMastheadStats( - isLoading + !isCurrentNodeLoaded ? null : [ { @@ -59,30 +63,27 @@ export function DeveloperSection({ onDirtyChange }: DeveloperSectionProps) { ); useEffect(() => { - const fetchSettings = async () => { - setIsLoading(true); - try { - const nodeRes = await apiFetch('/settings'); - const nodeData: Record = nodeRes.ok ? await nodeRes.json() : {}; - const safe: DeveloperFields = { - developer_mode: (nodeData.developer_mode as '0' | '1') ?? DEFAULT_SETTINGS.developer_mode, - }; - reset(safe); - } catch (e) { - console.error('Failed to fetch developer settings', e); - } finally { - setIsLoading(false); - } + let cancelled = false; + setIsSaving(false); + void (async () => { + const nodeData = await load(); + if (cancelled || !nodeData) return; + const safe: DeveloperFields = { + developer_mode: (nodeData.developer_mode as '0' | '1') ?? DEFAULT_SETTINGS.developer_mode, + }; + reset(safe); + })(); + return () => { + cancelled = true; }; - fetchSettings(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [activeNode?.id]); + }, [activeNode?.id, load, reset]); const onSettingChange = (key: K, value: DeveloperFields[K]) => { setSettings(prev => ({ ...prev, [key]: value })); }; const saveSettings = async () => { + const saveGuard = captureSaveGuard(); const submitted = { ...settings }; const payload = { developer_mode: submitted.developer_mode, @@ -91,8 +92,10 @@ export function DeveloperSection({ onDirtyChange }: DeveloperSectionProps) { try { const res = await apiFetch('/settings', { method: 'PATCH', + nodeId: saveGuard.nodeId, body: JSON.stringify(payload), }); + if (!isSaveOwner(saveGuard)) return; if (!res.ok) { const err = await res.json().catch(() => ({})); toast.error(err?.error || err?.message || 'Failed to save settings.'); @@ -104,15 +107,15 @@ export function DeveloperSection({ onDirtyChange }: DeveloperSectionProps) { detail: { changedKeys: Object.keys(payload) }, })); } catch (e: unknown) { + if (!isSaveOwner(saveGuard)) return; toast.error((e as Error)?.message || 'Something went wrong.'); } finally { - setIsSaving(false); + if (isSaveOwner(saveGuard)) setIsSaving(false); } }; - if (isLoading) return ; - return ( + }>
{!readOnly && ( - + {isSaving ? ( <> @@ -142,5 +145,6 @@ export function DeveloperSection({ onDirtyChange }: DeveloperSectionProps) { )}
+ ); } diff --git a/frontend/src/components/settings/DockerStorageSection.tsx b/frontend/src/components/settings/DockerStorageSection.tsx index 0aa008fb..2e397344 100644 --- a/frontend/src/components/settings/DockerStorageSection.tsx +++ b/frontend/src/components/settings/DockerStorageSection.tsx @@ -12,6 +12,8 @@ import { SettingsField } from './SettingsField'; import { SettingsActions, SettingsPrimaryButton } from './SettingsActions'; import { useMastheadStats } from './MastheadStatsContext'; import { useSettingsDirty } from './useSettingsDirty'; +import { useNodeSettingsLoad } from './useNodeSettingsLoad'; +import { SettingsLoadGate } from './SettingsLoadError'; import { TogglePill } from '@/components/ui/toggle-pill'; import { NumberChip } from './SystemControls'; @@ -42,15 +44,17 @@ export function DockerStorageSection({ onDirtyChange }: DockerStorageSectionProp const { isAdmin } = useAuth(); const readOnly = !isAdmin; const { settings, setSettings, dirtyCount, hasChanges, reset, markSaved } = useSettingsDirty({ ...DEFAULT_DOCKER_STORAGE }); - const [isLoading, setIsLoading] = useState(false); + const { phase, isCurrentNodeLoaded, load, isSaveOwner, captureSaveGuard } = useNodeSettingsLoad(activeNode?.id); const [isSaving, setIsSaving] = useState(false); + const reportDirty = isCurrentNodeLoaded && hasChanges; + useEffect(() => { - onDirtyChange?.(hasChanges); - }, [hasChanges, onDirtyChange]); + onDirtyChange?.(reportDirty); + }, [reportDirty, onDirtyChange]); useMastheadStats( - isLoading + !isCurrentNodeLoaded ? null : [ { @@ -62,39 +66,38 @@ export function DockerStorageSection({ onDirtyChange }: DockerStorageSectionProp ); useEffect(() => { - const fetchSettings = async () => { - setIsLoading(true); - try { - const nodeRes = await apiFetch('/settings'); - const nodeData: Record = nodeRes.ok ? await nodeRes.json() : {}; - const safe: DockerStorageFields = { - docker_janitor_gb: nodeData.docker_janitor_gb ?? DEFAULT_SETTINGS.docker_janitor_gb, - prune_on_update: (nodeData.prune_on_update as '0' | '1') ?? DEFAULT_SETTINGS.prune_on_update, - reclaim_hero: (nodeData.reclaim_hero as '0' | '1') ?? DEFAULT_SETTINGS.reclaim_hero, - }; - reset(safe); - } catch (e) { - console.error('Failed to fetch Docker & storage settings', e); - } finally { - setIsLoading(false); - } + let cancelled = false; + setIsSaving(false); + void (async () => { + const nodeData = await load(); + if (cancelled || !nodeData) return; + const safe: DockerStorageFields = { + docker_janitor_gb: nodeData.docker_janitor_gb ?? DEFAULT_SETTINGS.docker_janitor_gb, + prune_on_update: (nodeData.prune_on_update as '0' | '1') ?? DEFAULT_SETTINGS.prune_on_update, + reclaim_hero: (nodeData.reclaim_hero as '0' | '1') ?? DEFAULT_SETTINGS.reclaim_hero, + }; + reset(safe); + })(); + return () => { + cancelled = true; }; - fetchSettings(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [activeNode?.id]); + }, [activeNode?.id, load, reset]); const onSettingChange = (key: K, value: DockerStorageFields[K]) => { setSettings(prev => ({ ...prev, [key]: value })); }; const saveSettings = async () => { + const saveGuard = captureSaveGuard(); const submitted = { ...settings }; setIsSaving(true); try { const res = await apiFetch('/settings', { method: 'PATCH', + nodeId: saveGuard.nodeId, body: JSON.stringify(submitted), }); + if (!isSaveOwner(saveGuard)) return; if (!res.ok) { const err = await res.json().catch(() => ({})); toast.error(err?.error || err?.message || 'Failed to save settings.'); @@ -103,15 +106,15 @@ export function DockerStorageSection({ onDirtyChange }: DockerStorageSectionProp markSaved(submitted); toast.success('Docker & storage settings saved.'); } catch (e: unknown) { + if (!isSaveOwner(saveGuard)) return; toast.error((e as Error)?.message || 'Something went wrong.'); } finally { - setIsSaving(false); + if (isSaveOwner(saveGuard)) setIsSaving(false); } }; - if (isLoading) return ; - return ( + }>
{!readOnly && ( - + {isSaving ? ( <> @@ -165,5 +168,6 @@ export function DockerStorageSection({ onDirtyChange }: DockerStorageSectionProp )}
+ ); } diff --git a/frontend/src/components/settings/FleetMeshSection.tsx b/frontend/src/components/settings/FleetMeshSection.tsx index 3120f634..0afa63db 100644 --- a/frontend/src/components/settings/FleetMeshSection.tsx +++ b/frontend/src/components/settings/FleetMeshSection.tsx @@ -13,6 +13,8 @@ import { SettingsField } from './SettingsField'; import { SettingsActions, SettingsPrimaryButton } from './SettingsActions'; import { useMastheadStats } from './MastheadStatsContext'; import { useSettingsDirty } from './useSettingsDirty'; +import { useNodeSettingsLoad } from './useNodeSettingsLoad'; +import { SettingsLoadGate } from './SettingsLoadError'; import { TogglePill } from '@/components/ui/toggle-pill'; interface FleetMeshSectionProps { @@ -42,15 +44,17 @@ export function FleetMeshSection({ onDirtyChange }: FleetMeshSectionProps) { const showMesh = experimentalReady && experimental; const readOnly = !isAdmin; const { settings, setSettings, dirtyCount, hasChanges, reset, markSaved } = useSettingsDirty({ ...DEFAULT_FLEET_MESH }); - const [isLoading, setIsLoading] = useState(false); + const { phase, isCurrentNodeLoaded, load, isSaveOwner, captureSaveGuard } = useNodeSettingsLoad(activeNode?.id); const [isSaving, setIsSaving] = useState(false); + const reportDirty = isCurrentNodeLoaded && hasChanges; + useEffect(() => { - onDirtyChange?.(hasChanges); - }, [hasChanges, onDirtyChange]); + onDirtyChange?.(reportDirty); + }, [reportDirty, onDirtyChange]); useMastheadStats( - isLoading + !isCurrentNodeLoaded ? null : [ { @@ -62,25 +66,21 @@ export function FleetMeshSection({ onDirtyChange }: FleetMeshSectionProps) { ); useEffect(() => { - const fetchSettings = async () => { - setIsLoading(true); - try { - const nodeRes = await apiFetch('/settings'); - const nodeData: Record = nodeRes.ok ? await nodeRes.json() : {}; - const safe: FleetMeshFields = { - mesh_auto_recreate: (nodeData.mesh_auto_recreate as '0' | '1') ?? DEFAULT_SETTINGS.mesh_auto_recreate, - snapshot_documentation: (nodeData.snapshot_documentation as '0' | '1') ?? DEFAULT_SETTINGS.snapshot_documentation, - }; - reset(safe); - } catch (e) { - console.error('Failed to fetch fleet settings', e); - } finally { - setIsLoading(false); - } + let cancelled = false; + setIsSaving(false); + void (async () => { + const nodeData = await load(); + if (cancelled || !nodeData) return; + const safe: FleetMeshFields = { + mesh_auto_recreate: (nodeData.mesh_auto_recreate as '0' | '1') ?? DEFAULT_SETTINGS.mesh_auto_recreate, + snapshot_documentation: (nodeData.snapshot_documentation as '0' | '1') ?? DEFAULT_SETTINGS.snapshot_documentation, + }; + reset(safe); + })(); + return () => { + cancelled = true; }; - fetchSettings(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [activeNode?.id]); + }, [activeNode?.id, load, reset]); const onSettingChange = (key: K, value: FleetMeshFields[K]) => { setSettings(prev => ({ ...prev, [key]: value })); @@ -90,6 +90,7 @@ export function FleetMeshSection({ onDirtyChange }: FleetMeshSectionProps) { // When Mesh discovery is off, never write mesh_auto_recreate: a failed // settings read would otherwise push the default and overwrite a real // Mesh config the operator cannot see. + const saveGuard = captureSaveGuard(); const submitted: FleetMeshFields | SnapshotOnlyFields = showMesh ? { ...settings } : { snapshot_documentation: settings.snapshot_documentation }; @@ -97,8 +98,10 @@ export function FleetMeshSection({ onDirtyChange }: FleetMeshSectionProps) { try { const res = await apiFetch('/settings', { method: 'PATCH', + nodeId: saveGuard.nodeId, body: JSON.stringify(submitted), }); + if (!isSaveOwner(saveGuard)) return; if (!res.ok) { const err = await res.json().catch(() => ({})); toast.error(err?.error || err?.message || 'Failed to save settings.'); @@ -114,15 +117,15 @@ export function FleetMeshSection({ onDirtyChange }: FleetMeshSectionProps) { } toast.success('Fleet settings saved.'); } catch (e: unknown) { + if (!isSaveOwner(saveGuard)) return; toast.error((e as Error)?.message || 'Something went wrong.'); } finally { - setIsSaving(false); + if (isSaveOwner(saveGuard)) setIsSaving(false); } }; - if (isLoading) return ; - return ( + }>
{showMesh && ( @@ -152,7 +155,7 @@ export function FleetMeshSection({ onDirtyChange }: FleetMeshSectionProps) { {!readOnly && ( - + {isSaving ? ( <> @@ -165,5 +168,6 @@ export function FleetMeshSection({ onDirtyChange }: FleetMeshSectionProps) { )}
+ ); } diff --git a/frontend/src/components/settings/HostAlertsSection.tsx b/frontend/src/components/settings/HostAlertsSection.tsx index 4a82cefa..eca51693 100644 --- a/frontend/src/components/settings/HostAlertsSection.tsx +++ b/frontend/src/components/settings/HostAlertsSection.tsx @@ -12,6 +12,8 @@ import { SettingsField } from './SettingsField'; import { SettingsActions, SettingsPrimaryButton } from './SettingsActions'; import { useMastheadStats } from './MastheadStatsContext'; import { useSettingsDirty } from './useSettingsDirty'; +import { useNodeSettingsLoad } from './useNodeSettingsLoad'; +import { SettingsLoadGate } from './SettingsLoadError'; import { TogglePill } from '@/components/ui/toggle-pill'; import { NumberChip } from './SystemControls'; @@ -45,15 +47,17 @@ export function HostAlertsSection({ onDirtyChange }: HostAlertsSectionProps) { const { isAdmin } = useAuth(); const readOnly = !isAdmin; const { settings, setSettings, dirtyCount, hasChanges, reset, markSaved } = useSettingsDirty({ ...DEFAULT_HOST_ALERTS }); - const [isLoading, setIsLoading] = useState(false); + const { phase, isCurrentNodeLoaded, load, isSaveOwner, captureSaveGuard } = useNodeSettingsLoad(activeNode?.id); const [isSaving, setIsSaving] = useState(false); + const reportDirty = isCurrentNodeLoaded && hasChanges; + useEffect(() => { - onDirtyChange?.(hasChanges); - }, [hasChanges, onDirtyChange]); + onDirtyChange?.(reportDirty); + }, [reportDirty, onDirtyChange]); useMastheadStats( - isLoading + !isCurrentNodeLoaded ? null : [ { @@ -65,41 +69,40 @@ export function HostAlertsSection({ onDirtyChange }: HostAlertsSectionProps) { ); useEffect(() => { - const fetchSettings = async () => { - setIsLoading(true); - try { - const nodeRes = await apiFetch('/settings'); - const nodeData: Record = nodeRes.ok ? await nodeRes.json() : {}; - const safe: HostAlertFields = { - host_alerts_enabled: (nodeData.host_alerts_enabled as '0' | '1') ?? DEFAULT_SETTINGS.host_alerts_enabled, - host_cpu_limit: nodeData.host_cpu_limit ?? DEFAULT_SETTINGS.host_cpu_limit, - host_ram_limit: nodeData.host_ram_limit ?? DEFAULT_SETTINGS.host_ram_limit, - host_disk_limit: nodeData.host_disk_limit ?? DEFAULT_SETTINGS.host_disk_limit, - host_alert_suppression_mins: nodeData.host_alert_suppression_mins ?? DEFAULT_SETTINGS.host_alert_suppression_mins, - }; - reset(safe); - } catch (e) { - console.error('Failed to fetch host alert settings', e); - } finally { - setIsLoading(false); - } + let cancelled = false; + setIsSaving(false); + void (async () => { + const nodeData = await load(); + if (cancelled || !nodeData) return; + const safe: HostAlertFields = { + host_alerts_enabled: (nodeData.host_alerts_enabled as '0' | '1') ?? DEFAULT_SETTINGS.host_alerts_enabled, + host_cpu_limit: nodeData.host_cpu_limit ?? DEFAULT_SETTINGS.host_cpu_limit, + host_ram_limit: nodeData.host_ram_limit ?? DEFAULT_SETTINGS.host_ram_limit, + host_disk_limit: nodeData.host_disk_limit ?? DEFAULT_SETTINGS.host_disk_limit, + host_alert_suppression_mins: nodeData.host_alert_suppression_mins ?? DEFAULT_SETTINGS.host_alert_suppression_mins, + }; + reset(safe); + })(); + return () => { + cancelled = true; }; - fetchSettings(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [activeNode?.id]); + }, [activeNode?.id, load, reset]); const onSettingChange = (key: K, value: HostAlertFields[K]) => { setSettings(prev => ({ ...prev, [key]: value })); }; const saveSettings = async () => { + const saveGuard = captureSaveGuard(); const submitted = { ...settings }; setIsSaving(true); try { const res = await apiFetch('/settings', { method: 'PATCH', + nodeId: saveGuard.nodeId, body: JSON.stringify(submitted), }); + if (!isSaveOwner(saveGuard)) return; if (!res.ok) { const err = await res.json().catch(() => ({})); toast.error(err?.error || err?.message || 'Failed to save settings.'); @@ -108,15 +111,15 @@ export function HostAlertsSection({ onDirtyChange }: HostAlertsSectionProps) { markSaved(submitted); toast.success('Host alert settings saved.'); } catch (e: unknown) { + if (!isSaveOwner(saveGuard)) return; toast.error((e as Error)?.message || 'Something went wrong.'); } finally { - setIsSaving(false); + if (isSaveOwner(saveGuard)) setIsSaving(false); } }; - if (isLoading) return ; - return ( + }>
{!readOnly && ( - + {isSaving ? ( <> @@ -200,5 +203,6 @@ export function HostAlertsSection({ onDirtyChange }: HostAlertsSectionProps) { )}
+ ); } diff --git a/frontend/src/components/settings/SettingsLoadError.tsx b/frontend/src/components/settings/SettingsLoadError.tsx new file mode 100644 index 00000000..05a92090 --- /dev/null +++ b/frontend/src/components/settings/SettingsLoadError.tsx @@ -0,0 +1,36 @@ +import type { ReactNode } from 'react'; +import { AlertTriangle } from 'lucide-react'; +import { SettingsCallout } from './SettingsCallout'; +import type { NodeSettingsLoadPhase } from './useNodeSettingsLoad'; + +/** Persistent error state when node settings failed to load for the active node. */ +export function SettingsLoadError() { + return ( + } + title="Could not load settings" + subtitle="Settings for this node could not be loaded. Save stays unavailable until loading succeeds." + /> + ); +} + +/** + * Renders content only after an authoritative load for the active node. + * Mid-switch / in-flight loads show `skeleton`; a failed load shows SettingsLoadError. + */ +export function SettingsLoadGate({ + phase, + isCurrentNodeLoaded, + skeleton, + children, +}: { + phase: NodeSettingsLoadPhase; + isCurrentNodeLoaded: boolean; + skeleton: ReactNode; + children: ReactNode; +}) { + if (isCurrentNodeLoaded) return children; + if (phase === 'error') return ; + return skeleton; +} diff --git a/frontend/src/components/settings/StacksSection.tsx b/frontend/src/components/settings/StacksSection.tsx index de5cd603..7cf9b979 100644 --- a/frontend/src/components/settings/StacksSection.tsx +++ b/frontend/src/components/settings/StacksSection.tsx @@ -17,6 +17,8 @@ import { SettingsField } from './SettingsField'; import { SettingsActions, SettingsPrimaryButton } from './SettingsActions'; import { useMastheadStats } from './MastheadStatsContext'; import { useSettingsDirty } from './useSettingsDirty'; +import { useNodeSettingsLoad } from './useNodeSettingsLoad'; +import { SettingsLoadGate } from './SettingsLoadError'; import { TogglePill } from '@/components/ui/toggle-pill'; import { NumberChip } from './SystemControls'; @@ -59,15 +61,17 @@ export function StacksSection({ onDirtyChange }: StacksSectionProps) { const { isAdmin } = useAuth(); const readOnly = !isAdmin; const { settings, setSettings, dirtyCount, hasChanges, reset, markSaved } = useSettingsDirty({ ...DEFAULT_GUARDRAILS }); - const [isLoading, setIsLoading] = useState(false); + const { phase, isCurrentNodeLoaded, load, isSaveOwner, captureSaveGuard } = useNodeSettingsLoad(activeNode?.id); const [isSaving, setIsSaving] = useState(false); + const reportDirty = isCurrentNodeLoaded && hasChanges; + useEffect(() => { - onDirtyChange?.(hasChanges); - }, [hasChanges, onDirtyChange]); + onDirtyChange?.(reportDirty); + }, [reportDirty, onDirtyChange]); useMastheadStats( - isLoading + !isCurrentNodeLoaded ? null : [ { @@ -79,40 +83,39 @@ export function StacksSection({ onDirtyChange }: StacksSectionProps) { ); useEffect(() => { - const fetchSettings = async () => { - setIsLoading(true); - try { - const nodeRes = await apiFetch('/settings'); - const nodeData: Record = nodeRes.ok ? await nodeRes.json() : {}; - const safe: GuardrailFields = { - health_gate_enabled: (nodeData.health_gate_enabled as '0' | '1') ?? DEFAULT_SETTINGS.health_gate_enabled, - health_gate_window_seconds: nodeData.health_gate_window_seconds ?? DEFAULT_SETTINGS.health_gate_window_seconds, - env_block_deploy_on_missing_required: (nodeData.env_block_deploy_on_missing_required as '0' | '1') ?? DEFAULT_SETTINGS.env_block_deploy_on_missing_required, - auto_create_missing_external_networks: (nodeData.auto_create_missing_external_networks as '0' | '1') ?? DEFAULT_SETTINGS.auto_create_missing_external_networks, - }; - reset(safe); - } catch (e) { - console.error('Failed to fetch deploy guardrail settings', e); - } finally { - setIsLoading(false); - } + let cancelled = false; + setIsSaving(false); + void (async () => { + const nodeData = await load(); + if (cancelled || !nodeData) return; + const safe: GuardrailFields = { + health_gate_enabled: (nodeData.health_gate_enabled as '0' | '1') ?? DEFAULT_SETTINGS.health_gate_enabled, + health_gate_window_seconds: nodeData.health_gate_window_seconds ?? DEFAULT_SETTINGS.health_gate_window_seconds, + env_block_deploy_on_missing_required: (nodeData.env_block_deploy_on_missing_required as '0' | '1') ?? DEFAULT_SETTINGS.env_block_deploy_on_missing_required, + auto_create_missing_external_networks: (nodeData.auto_create_missing_external_networks as '0' | '1') ?? DEFAULT_SETTINGS.auto_create_missing_external_networks, + }; + reset(safe); + })(); + return () => { + cancelled = true; }; - fetchSettings(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [activeNode?.id]); + }, [activeNode?.id, load, reset]); const onGuardrailChange = (key: K, value: GuardrailFields[K]) => { setSettings(prev => ({ ...prev, [key]: value })); }; const saveGuardrails = async () => { + const saveGuard = captureSaveGuard(); const submitted = { ...settings }; setIsSaving(true); try { const res = await apiFetch('/settings', { method: 'PATCH', + nodeId: saveGuard.nodeId, body: JSON.stringify(submitted), }); + if (!isSaveOwner(saveGuard)) return; if (!res.ok) { const err = await res.json().catch(() => ({})); toast.error(err?.error || err?.message || 'Failed to save settings.'); @@ -121,9 +124,10 @@ export function StacksSection({ onDirtyChange }: StacksSectionProps) { markSaved(submitted); toast.success('Deploy guardrail settings saved.'); } catch (e: unknown) { + if (!isSaveOwner(saveGuard)) return; toast.error((e as Error)?.message || 'Something went wrong.'); } finally { - setIsSaving(false); + if (isSaveOwner(saveGuard)) setIsSaving(false); } }; @@ -187,9 +191,7 @@ export function StacksSection({ onDirtyChange }: StacksSectionProps) { ⓘ saved to this browser only · every device remembers its own choice

- {isLoading ? ( - - ) : ( + }>

@@ -238,7 +240,7 @@ export function StacksSection({ onDirtyChange }: StacksSectionProps) { {!readOnly && ( - + {isSaving ? ( <> @@ -251,7 +253,7 @@ export function StacksSection({ onDirtyChange }: StacksSectionProps) { )}

- )} + ); } diff --git a/frontend/src/components/settings/__tests__/SettingsLoadFailures.test.tsx b/frontend/src/components/settings/__tests__/SettingsLoadFailures.test.tsx new file mode 100644 index 00000000..7918bce7 --- /dev/null +++ b/frontend/src/components/settings/__tests__/SettingsLoadFailures.test.tsx @@ -0,0 +1,388 @@ +/** + * Load-failure and node-ownership guards for node-scoped settings sections. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, fireEvent, waitFor, act } from '@testing-library/react'; +import type { ComponentType } from 'react'; + +vi.mock('@/lib/api', () => ({ apiFetch: vi.fn() })); +vi.mock('@/components/ui/toast-store', () => ({ + toast: { error: vi.fn(), success: vi.fn(), warning: vi.fn(), info: vi.fn(), loading: vi.fn(), dismiss: vi.fn() }, +})); +vi.mock('@/context/AuthContext', () => ({ useAuth: () => ({ isAdmin: true }) })); +vi.mock('@/context/LicenseContext', () => ({ useLicense: vi.fn(() => ({ isPaid: true })) })); +vi.mock('../MastheadStatsContext', () => ({ useMastheadStats: () => {} })); +const useExperimentalMock = vi.fn(() => ({ experimental: true, experimentalReady: true })); +vi.mock('@/hooks/useExperimental', () => ({ + useExperimental: () => useExperimentalMock(), +})); + +const activeNodeState = { id: 1 as number }; +vi.mock('@/context/NodeContext', () => ({ + useNodes: () => ({ activeNode: { id: activeNodeState.id } }), +})); + +import { apiFetch } from '@/lib/api'; +import { toast } from '@/components/ui/toast-store'; +import { HostAlertsSection } from '../HostAlertsSection'; +import { ContainerAlertsSection } from '../ContainerAlertsSection'; +import { DockerStorageSection } from '../DockerStorageSection'; +import { FleetMeshSection } from '../FleetMeshSection'; +import { DataRetentionSection } from '../DataRetentionSection'; +import { DeveloperSection } from '../DeveloperSection'; +import { StacksSection } from '../StacksSection'; +import { AppStoreSection } from '../AppStoreSection'; + +const mockedFetch = apiFetch as unknown as ReturnType; +const mockedToast = toast as unknown as { error: ReturnType; success: ReturnType }; + +const FULL_SETTINGS: Record = { + host_cpu_limit: '90', + host_ram_limit: '90', + host_disk_limit: '90', + host_alert_suppression_mins: '60', + host_alerts_enabled: '1', + global_crash: '1', + docker_janitor_gb: '5', + prune_on_update: '1', + reclaim_hero: '1', + mesh_auto_recreate: '0', + snapshot_documentation: '0', + metrics_retention_hours: '24', + log_retention_days: '30', + audit_retention_days: '90', + scan_history_per_image_limit: '50', + prune_orphaned_scans: '1', + developer_mode: '0', + health_gate_enabled: '1', + health_gate_window_seconds: '90', + env_block_deploy_on_missing_required: '0', + auto_create_missing_external_networks: '0', + template_registry_url: 'https://example.com/templates.json', +}; + +function okSettings(extra: Record = {}) { + return { ok: true, json: async () => ({ ...FULL_SETTINGS, ...extra }) }; +} + +function failSettings(status = 502) { + return { ok: false, status, json: async () => ({ error: 'unavailable' }) }; +} + +function patchCalls() { + return mockedFetch.mock.calls.filter((c) => c[1]?.method === 'PATCH'); +} + +function refreshCacheCalls() { + return mockedFetch.mock.calls.filter( + (c) => typeof c[0] === 'string' && c[0].includes('refresh-cache'), + ); +} + +function expectFailedLoadToast() { + expect(mockedToast.error).toHaveBeenCalledTimes(1); + expect(mockedToast.error).toHaveBeenCalledWith('Failed to load settings.'); +} + +async function waitForActiveNodeFetch(nodeId: number) { + await waitFor(() => { + expect(mockedFetch.mock.calls.some((c) => c[1]?.nodeId === nodeId && c[1]?.method !== 'PATCH')).toBe(true); + }); +} + +beforeEach(() => { + activeNodeState.id = 1; + mockedFetch.mockReset(); + mockedToast.error.mockReset(); + mockedToast.success.mockReset(); + useExperimentalMock.mockReturnValue({ experimental: true, experimentalReady: true }); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +interface DirtyCase { + name: string; + Section: ComponentType<{ onDirtyChange?: (d: boolean) => void }>; + saveName: RegExp; + edit: () => void; +} + +const dirtyCases: DirtyCase[] = [ + { + name: 'HostAlertsSection', + Section: HostAlertsSection, + saveName: /save alerts/i, + edit: () => { + fireEvent.click(screen.getAllByRole('button', { name: /90\s*%/i })[0]); + fireEvent.change(screen.getByRole('spinbutton'), { target: { value: '95' } }); + fireEvent.blur(screen.getByRole('spinbutton')); + }, + }, + { + name: 'ContainerAlertsSection', + Section: ContainerAlertsSection, + saveName: /save settings/i, + edit: () => fireEvent.click(screen.getByRole('switch')), + }, + { + name: 'DockerStorageSection', + Section: DockerStorageSection, + saveName: /save settings/i, + edit: () => fireEvent.click(screen.getAllByRole('switch')[0]), + }, + { + name: 'FleetMeshSection', + Section: FleetMeshSection, + saveName: /save settings/i, + edit: () => fireEvent.click(screen.getAllByRole('switch')[0]), + }, + { + name: 'DataRetentionSection', + Section: DataRetentionSection, + saveName: /save settings/i, + edit: () => fireEvent.change(screen.getAllByRole('spinbutton')[0], { target: { value: '48' } }), + }, + { + name: 'DeveloperSection', + Section: DeveloperSection, + saveName: /save settings/i, + edit: () => fireEvent.click(screen.getByRole('switch')), + }, + { + name: 'StacksSection', + Section: StacksSection, + saveName: /save settings/i, + edit: () => fireEvent.click(screen.getAllByRole('switch')[0]), + }, +]; + +describe('settings load failures (dirty sections)', () => { + for (const c of dirtyCases) { + it(`${c.name}: failed load shows error, toasts once, blocks save, and never PATCHes`, async () => { + mockedFetch.mockResolvedValue(failSettings()); + const onDirty = vi.fn(); + render(); + + await waitFor(() => { + expect(screen.getByText(/could not load settings/i)).toBeTruthy(); + }); + expectFailedLoadToast(); + expect(screen.queryByRole('button', { name: c.saveName })).toBeNull(); + expect(patchCalls()).toHaveLength(0); + expect(onDirty).not.toHaveBeenCalledWith(true); + }); + + it(`${c.name}: successful load recovers and allows save after edit`, async () => { + mockedFetch.mockResolvedValue(okSettings()); + render(); + const save = await screen.findByRole('button', { name: c.saveName }); + expect(save).toBeDisabled(); + c.edit(); + await waitFor(() => expect(save).not.toBeDisabled()); + fireEvent.click(save); + await waitFor(() => expect(patchCalls().length).toBeGreaterThan(0)); + const opts = patchCalls()[0][1] as { nodeId?: number | null }; + expect(opts.nodeId).toBe(1); + }); + } + + it('HostAlertsSection: malformed 200 body fails closed instead of seeding defaults', async () => { + mockedFetch.mockResolvedValue({ ok: true, json: async () => null }); + const onDirty = vi.fn(); + render(); + + await waitFor(() => { + expect(screen.getByText(/could not load settings/i)).toBeTruthy(); + }); + expectFailedLoadToast(); + expect(screen.queryByRole('button', { name: /save alerts/i })).toBeNull(); + expect(screen.queryByRole('spinbutton')).toBeNull(); + expect(patchCalls()).toHaveLength(0); + expect(onDirty).not.toHaveBeenCalledWith(true); + }); +}); + +describe('AppStoreSection load failures', () => { + it('failed load shows error, toasts once, and keeps Save disabled with no PATCH', async () => { + mockedFetch.mockResolvedValue(failSettings()); + render(); + await waitFor(() => { + expect(screen.getByText(/could not load settings/i)).toBeTruthy(); + }); + expectFailedLoadToast(); + expect(screen.queryByRole('button', { name: /save & refresh/i })).toBeNull(); + expect(patchCalls()).toHaveLength(0); + }); + + it('successful load shows registry URL and pins save nodeId', async () => { + mockedFetch.mockImplementation((url: string, opts?: { method?: string; nodeId?: number | null }) => { + if (opts?.method === 'PATCH') { + return Promise.resolve({ ok: true, json: async () => ({}) }); + } + if (typeof url === 'string' && url.includes('refresh-cache')) { + return Promise.resolve({ ok: true, json: async () => ({}) }); + } + return Promise.resolve(okSettings()); + }); + render(); + const input = await screen.findByLabelText(/registry url/i); + expect((input as HTMLInputElement).value).toBe('https://example.com/templates.json'); + fireEvent.change(input, { target: { value: 'https://example.com/other.json' } }); + const save = screen.getByRole('button', { name: /save & refresh/i }); + fireEvent.click(save); + await waitFor(() => expect(patchCalls()).toHaveLength(1)); + expect((patchCalls()[0][1] as { nodeId?: number | null }).nodeId).toBe(1); + }); + + it('refreshes App Store cache on the PATCH node after a mid-save switch', async () => { + let resolvePatch: ((v: unknown) => void) | undefined; + mockedFetch.mockImplementation((url: string, opts?: { method?: string; nodeId?: number | null }) => { + if (opts?.method === 'PATCH') { + return new Promise((resolve) => { + resolvePatch = resolve; + }); + } + if (typeof url === 'string' && url.includes('refresh-cache')) { + return Promise.resolve({ ok: true, json: async () => ({}) }); + } + if (opts?.nodeId === 2) { + return Promise.resolve(okSettings({ template_registry_url: 'https://b.example.com/templates.json' })); + } + return Promise.resolve(okSettings()); + }); + + const { rerender } = render(); + const input = await screen.findByLabelText(/registry url/i); + fireEvent.change(input, { target: { value: 'https://example.com/other.json' } }); + fireEvent.click(screen.getByRole('button', { name: /save & refresh/i })); + + await waitFor(() => expect(patchCalls()).toHaveLength(1)); + expect((patchCalls()[0][1] as { nodeId?: number | null }).nodeId).toBe(1); + + activeNodeState.id = 2; + rerender(); + await waitForActiveNodeFetch(2); + + mockedToast.success.mockClear(); + mockedToast.error.mockClear(); + await act(async () => { + resolvePatch?.({ ok: true, json: async () => ({}) }); + }); + + await waitFor(() => expect(refreshCacheCalls()).toHaveLength(1)); + expect((refreshCacheCalls()[0][1] as { nodeId?: number | null }).nodeId).toBe(1); + expect(mockedToast.success).not.toHaveBeenCalled(); + // Node B's form must not adopt A's edited URL from the stale save completion. + const bInput = await screen.findByLabelText(/registry url/i); + expect((bInput as HTMLInputElement).value).toBe('https://b.example.com/templates.json'); + }); +}); + +describe('node ownership races', () => { + it('does not attribute node A dirty state to node B after B load fails', async () => { + let resolveA: ((v: unknown) => void) | undefined; + mockedFetch.mockImplementation((_url: string, opts?: { nodeId?: number | null }) => { + if (opts?.nodeId === 1) { + return new Promise((resolve) => { + resolveA = resolve; + }); + } + return Promise.resolve(failSettings()); + }); + + const onDirty = vi.fn(); + const { rerender } = render(); + + // Finish A successfully, then dirty it. + await act(async () => { + resolveA?.(okSettings({ host_cpu_limit: '70' })); + }); + const saveA = await screen.findByRole('button', { name: /save alerts/i }); + fireEvent.click(screen.getAllByRole('button', { name: /70\s*%/i })[0]); + fireEvent.change(screen.getByRole('spinbutton'), { target: { value: '75' } }); + fireEvent.blur(screen.getByRole('spinbutton')); + await waitFor(() => expect(onDirty).toHaveBeenCalledWith(true)); + expect(saveA).not.toBeDisabled(); + + activeNodeState.id = 2; + onDirty.mockClear(); + rerender(); + + await waitFor(() => { + expect(screen.getByText(/could not load settings/i)).toBeTruthy(); + }); + expect(screen.queryByRole('button', { name: /save alerts/i })).toBeNull(); + expect(screen.queryByDisplayValue('75')).toBeNull(); + expect(onDirty).not.toHaveBeenCalledWith(true); + expect(patchCalls()).toHaveLength(0); + }); + + it('keeps node B loading while a stale node A response settles', async () => { + let resolveA: ((v: unknown) => void) | undefined; + let resolveB: ((v: unknown) => void) | undefined; + mockedFetch.mockImplementation((_url: string, opts?: { nodeId?: number | null }) => { + if (opts?.nodeId === 1) { + return new Promise((resolve) => { + resolveA = resolve; + }); + } + return new Promise((resolve) => { + resolveB = resolve; + }); + }); + + const { rerender } = render(); + // Switch before A resolves. + activeNodeState.id = 2; + rerender(); + + // Stale A finishes with success; B must still be loading (skeleton, no form/error yet). + await act(async () => { + resolveA?.(okSettings({ host_cpu_limit: '11' })); + }); + expect(screen.queryByRole('button', { name: /save alerts/i })).toBeNull(); + expect(screen.queryByText(/could not load settings/i)).toBeNull(); + + await act(async () => { + resolveB?.(okSettings({ host_cpu_limit: '22' })); + }); + await screen.findByRole('button', { name: /save alerts/i }); + // Form shows B's value, not A's. + expect(screen.getAllByRole('button', { name: /22\s*%/i }).length).toBeGreaterThan(0); + expect(screen.queryByRole('button', { name: /11\s*%/i })).toBeNull(); + }); + + it('suppresses stale save toasts after switching nodes', async () => { + let resolvePatch: ((v: unknown) => void) | undefined; + mockedFetch.mockImplementation((_url: string, opts?: { method?: string; nodeId?: number | null }) => { + if (opts?.method === 'PATCH') { + return new Promise((resolve) => { + resolvePatch = resolve; + }); + } + if (opts?.nodeId === 2) { + return Promise.resolve(okSettings({ host_cpu_limit: '50' })); + } + return Promise.resolve(okSettings({ host_cpu_limit: '90' })); + }); + + const { rerender } = render(); + const save = await screen.findByRole('button', { name: /save settings/i }); + fireEvent.click(screen.getByRole('switch')); + fireEvent.click(save); + + activeNodeState.id = 2; + rerender(); + // Allow node B's load effect to run; do not require a dirty Save button. + await waitForActiveNodeFetch(2); + + mockedToast.success.mockClear(); + await act(async () => { + resolvePatch?.({ ok: true, json: async () => ({}) }); + }); + expect(mockedToast.success).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/components/settings/__tests__/fetchNodeSettings.test.ts b/frontend/src/components/settings/__tests__/fetchNodeSettings.test.ts new file mode 100644 index 00000000..8f12a8cb --- /dev/null +++ b/frontend/src/components/settings/__tests__/fetchNodeSettings.test.ts @@ -0,0 +1,94 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +vi.mock('@/lib/api', () => ({ apiFetch: vi.fn() })); +vi.mock('@/components/ui/toast-store', () => ({ + toast: { error: vi.fn(), success: vi.fn(), warning: vi.fn(), info: vi.fn(), loading: vi.fn(), dismiss: vi.fn() }, +})); + +import { apiFetch } from '@/lib/api'; +import { toast } from '@/components/ui/toast-store'; +import { fetchNodeSettings } from '../fetchNodeSettings'; + +const mockedFetch = apiFetch as unknown as ReturnType; +const mockedToast = toast as unknown as { error: ReturnType }; + +beforeEach(() => { + mockedFetch.mockReset(); + mockedToast.error.mockReset(); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('fetchNodeSettings', () => { + it('returns settings on success and passes nodeId', async () => { + const settings = { host_cpu_limit: '80' }; + mockedFetch.mockResolvedValue({ ok: true, json: async () => settings }); + + const result = await fetchNodeSettings(7); + + expect(result).toEqual({ ok: true, settings }); + expect(mockedFetch).toHaveBeenCalledWith('/settings', expect.objectContaining({ nodeId: 7 })); + expect(mockedToast.error).not.toHaveBeenCalled(); + }); + + it('treats a successful empty object as success', async () => { + mockedFetch.mockResolvedValue({ ok: true, json: async () => ({}) }); + const result = await fetchNodeSettings(1); + expect(result).toEqual({ ok: true, settings: {} }); + expect(mockedToast.error).not.toHaveBeenCalled(); + }); + + it.each([null, [], 'nope', 42, true])( + 'returns ok false without toasting for malformed 200 body %j', + async (body) => { + mockedFetch.mockResolvedValue({ ok: true, json: async () => body }); + const result = await fetchNodeSettings(1); + expect(result).toEqual({ ok: false }); + expect(mockedToast.error).not.toHaveBeenCalled(); + }, + ); + + it('returns ok false without toasting on non-ok response', async () => { + mockedFetch.mockResolvedValue({ ok: false, status: 502, json: async () => ({}) }); + const result = await fetchNodeSettings(2); + expect(result).toEqual({ ok: false }); + expect(mockedToast.error).not.toHaveBeenCalled(); + }); + + it('returns ok false without toasting when fetch rejects', async () => { + mockedFetch.mockRejectedValue(new Error('network down')); + const result = await fetchNodeSettings(null); + expect(result).toEqual({ ok: false }); + expect(mockedToast.error).not.toHaveBeenCalled(); + }); + + it('returns ok false without toasting when JSON parsing fails', async () => { + mockedFetch.mockResolvedValue({ + ok: true, + json: async () => { + throw new SyntaxError('bad json'); + }, + }); + const result = await fetchNodeSettings(3); + expect(result).toEqual({ ok: false }); + expect(mockedToast.error).not.toHaveBeenCalled(); + }); + + it('returns ok false without toast when aborted', async () => { + const controller = new AbortController(); + mockedFetch.mockImplementation((_url: string, opts?: { signal?: AbortSignal }) => { + return new Promise((_resolve, reject) => { + opts?.signal?.addEventListener('abort', () => { + reject(new DOMException('Aborted', 'AbortError')); + }); + }); + }); + const pending = fetchNodeSettings(4, controller.signal); + controller.abort(); + await expect(pending).resolves.toEqual({ ok: false }); + expect(mockedToast.error).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/components/settings/__tests__/useNodeSettingsLoad.test.ts b/frontend/src/components/settings/__tests__/useNodeSettingsLoad.test.ts new file mode 100644 index 00000000..ec7b4252 --- /dev/null +++ b/frontend/src/components/settings/__tests__/useNodeSettingsLoad.test.ts @@ -0,0 +1,175 @@ +/** + * Toast ownership and generation guards for useNodeSettingsLoad. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { renderHook, act, waitFor } from '@testing-library/react'; + +vi.mock('@/components/ui/toast-store', () => ({ + toast: { error: vi.fn(), success: vi.fn(), warning: vi.fn(), info: vi.fn(), loading: vi.fn(), dismiss: vi.fn() }, +})); + +const fetchNodeSettingsMock = vi.fn(); +vi.mock('../fetchNodeSettings', () => ({ + fetchNodeSettings: (...args: unknown[]) => fetchNodeSettingsMock(...args), +})); + +import { toast } from '@/components/ui/toast-store'; +import { useNodeSettingsLoad } from '../useNodeSettingsLoad'; + +const mockedToast = toast as unknown as { error: ReturnType }; + +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((r) => { + resolve = r; + }); + return { promise, resolve }; +} + +beforeEach(() => { + fetchNodeSettingsMock.mockReset(); + mockedToast.error.mockReset(); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('useNodeSettingsLoad toast ownership', () => { + it('toasts once on a current non-OK load failure', async () => { + fetchNodeSettingsMock.mockResolvedValue({ ok: false }); + const { result } = renderHook(() => useNodeSettingsLoad(1)); + + const settings = await act(() => result.current.load()); + + expect(settings).toBeNull(); + expect(result.current.phase).toBe('error'); + expect(mockedToast.error).toHaveBeenCalledTimes(1); + expect(mockedToast.error).toHaveBeenCalledWith('Failed to load settings.'); + }); + + it('does not toast when a newer load supersedes a failing one', async () => { + const first = deferred(); + let call = 0; + fetchNodeSettingsMock.mockImplementation(() => { + call += 1; + if (call === 1) return first.promise; + return Promise.resolve({ ok: true, settings: { developer_mode: '0' } }); + }); + + const { result } = renderHook(() => useNodeSettingsLoad(1)); + + let firstPromise: Promise | null>; + act(() => { + firstPromise = result.current.load(); + }); + + await act(async () => { + await result.current.load(); + }); + + mockedToast.error.mockClear(); + await act(async () => { + first.resolve({ ok: false }); + await firstPromise!; + }); + + expect(mockedToast.error).not.toHaveBeenCalled(); + expect(result.current.phase).toBe('ready'); + }); + + it('does not toast when the current load was aborted', async () => { + fetchNodeSettingsMock.mockImplementation((_nodeId: number | null, signal: AbortSignal) => { + return new Promise((resolve) => { + const finish = () => resolve({ ok: false }); + if (signal.aborted) { + finish(); + return; + } + signal.addEventListener('abort', finish, { once: true }); + }); + }); + + const { result, unmount } = renderHook(() => useNodeSettingsLoad(1)); + + let loadPromise: Promise | null>; + act(() => { + loadPromise = result.current.load(); + }); + + unmount(); + await act(async () => { + await loadPromise!; + }); + + expect(mockedToast.error).not.toHaveBeenCalled(); + }); + + it('does not toast when a stale failure settles after a node switch', async () => { + const loadA = deferred(); + fetchNodeSettingsMock.mockImplementation((nodeId: number | null) => { + if (nodeId === 1) return loadA.promise; + return Promise.resolve({ ok: true, settings: {} }); + }); + + const { result, rerender } = renderHook( + ({ id }: { id: number | undefined }) => useNodeSettingsLoad(id), + { initialProps: { id: 1 as number | undefined } }, + ); + + let promiseA: Promise | null>; + act(() => { + promiseA = result.current.load(); + }); + + rerender({ id: 2 }); + await act(async () => { + await result.current.load(); + }); + + mockedToast.error.mockClear(); + await act(async () => { + loadA.resolve({ ok: false }); + await promiseA!; + }); + + expect(mockedToast.error).not.toHaveBeenCalled(); + await waitFor(() => expect(result.current.phase).toBe('ready')); + }); + + it('does not fetch or toast while the active node id is still undefined', async () => { + const { result } = renderHook(() => useNodeSettingsLoad(undefined)); + + const settings = await act(() => result.current.load()); + + expect(settings).toBeNull(); + expect(result.current.phase).toBe('loading'); + expect(fetchNodeSettingsMock).not.toHaveBeenCalled(); + expect(mockedToast.error).not.toHaveBeenCalled(); + }); + + it('toasts only once when bootstrap settles from undefined to a failing node', async () => { + fetchNodeSettingsMock.mockResolvedValue({ ok: false }); + const { result, rerender } = renderHook( + ({ id }: { id: number | undefined }) => useNodeSettingsLoad(id), + { initialProps: { id: undefined as number | undefined } }, + ); + + await act(async () => { + await result.current.load(); + }); + expect(fetchNodeSettingsMock).not.toHaveBeenCalled(); + expect(mockedToast.error).not.toHaveBeenCalled(); + + rerender({ id: 1 }); + await act(async () => { + await result.current.load(); + }); + + expect(fetchNodeSettingsMock).toHaveBeenCalledTimes(1); + expect(mockedToast.error).toHaveBeenCalledTimes(1); + expect(mockedToast.error).toHaveBeenCalledWith('Failed to load settings.'); + expect(result.current.phase).toBe('error'); + }); +}); diff --git a/frontend/src/components/settings/fetchNodeSettings.ts b/frontend/src/components/settings/fetchNodeSettings.ts new file mode 100644 index 00000000..71c42823 --- /dev/null +++ b/frontend/src/components/settings/fetchNodeSettings.ts @@ -0,0 +1,46 @@ +import { apiFetch } from '@/lib/api'; + +export type FetchNodeSettingsResult = + | { ok: true; settings: Record } + | { ok: false }; + +function isAbort(err: unknown, signal?: AbortSignal): boolean { + return Boolean(signal?.aborted) + || (err instanceof DOMException && err.name === 'AbortError'); +} + +/** Authoritative /settings bodies are plain objects; null and arrays must not seed defaults. */ +export function isSettingsRecord(value: unknown): value is Record { + return value !== null && typeof value === 'object' && !Array.isArray(value); +} + +/** + * Load node-scoped settings for an explicitly captured node. + * Does not toast: callers that own UI generation (useNodeSettingsLoad) decide + * whether a failure is still current before notifying the operator. + * Abort returns `{ ok: false }` without logging as a hard failure. + */ +export async function fetchNodeSettings( + nodeId: number | null, + signal?: AbortSignal, +): Promise { + try { + const res = await apiFetch('/settings', { nodeId, signal }); + if (signal?.aborted) return { ok: false }; + if (!res.ok) { + console.error('Failed to load settings:', res.status); + return { ok: false }; + } + const body: unknown = await res.json(); + if (signal?.aborted) return { ok: false }; + if (!isSettingsRecord(body)) { + console.error('Failed to load settings: malformed body'); + return { ok: false }; + } + return { ok: true, settings: body }; + } catch (err) { + if (isAbort(err, signal)) return { ok: false }; + console.error('Failed to load settings:', err); + return { ok: false }; + } +} diff --git a/frontend/src/components/settings/useNodeSettingsLoad.ts b/frontend/src/components/settings/useNodeSettingsLoad.ts new file mode 100644 index 00000000..cd2bf3ab --- /dev/null +++ b/frontend/src/components/settings/useNodeSettingsLoad.ts @@ -0,0 +1,89 @@ +import { useCallback, useEffect, useRef, useState } from 'react'; +import { toast } from '@/components/ui/toast-store'; +import { fetchNodeSettings } from './fetchNodeSettings'; + +export type NodeSettingsLoadPhase = 'loading' | 'ready' | 'error'; + +export interface SettingsSaveGuard { + nodeId: number | null; + gen: number; +} + +/** + * Race-safe load ownership for node-scoped settings sections. + * Clears ownership on node switch, ignores stale responses for value adoption + * and loading finalization, and exposes whether the active node has an + * authoritative load. + */ +export function useNodeSettingsLoad(activeNodeId: number | undefined) { + const [phase, setPhase] = useState('loading'); + const [loadedNodeId, setLoadedNodeId] = useState(undefined); + const genRef = useRef(0); + const abortRef = useRef(null); + const activeNodeIdRef = useRef(activeNodeId); + activeNodeIdRef.current = activeNodeId; + + const captureSaveGuard = useCallback((): SettingsSaveGuard => ({ + nodeId: activeNodeIdRef.current ?? null, + gen: genRef.current, + }), []); + + const isCurrentNodeLoaded = + loadedNodeId !== undefined && loadedNodeId === (activeNodeId ?? null); + + const isSaveOwner = useCallback((guard: SettingsSaveGuard): boolean => { + return (activeNodeIdRef.current ?? null) === guard.nodeId + && genRef.current === guard.gen; + }, []); + + const load = useCallback(async (): Promise | null> => { + // Hard refresh boots with activeNode=null; a fetch for that frame is not + // authoritative and would toast again once the real node id settles. + if (activeNodeIdRef.current === undefined) { + abortRef.current?.abort(); + setLoadedNodeId(undefined); + setPhase('loading'); + return null; + } + + abortRef.current?.abort(); + const ac = new AbortController(); + abortRef.current = ac; + const gen = ++genRef.current; + const captured = activeNodeIdRef.current ?? null; + + setLoadedNodeId(undefined); + setPhase('loading'); + + const result = await fetchNodeSettings(captured, ac.signal); + + // Stale generation: leave the newer load's phase alone; do not toast. + if (genRef.current !== gen) return null; + + if (!result.ok) { + if (ac.signal.aborted) return null; + toast.error('Failed to load settings.'); + setPhase('error'); + return null; + } + + setLoadedNodeId(captured); + setPhase('ready'); + return result.settings; + }, []); + + useEffect(() => { + return () => { + abortRef.current?.abort(); + }; + }, []); + + return { + phase, + isCurrentNodeLoaded, + loadedNodeId, + load, + isSaveOwner, + captureSaveGuard, + }; +}