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, + }; +}