From c47b8eb8e9eb087aef20184c9d4a786cf189b1ec Mon Sep 17 00:00:00 2001 From: Anso Date: Wed, 12 Aug 2026 11:54:29 -0400 Subject: [PATCH] perf(frontend): overlap stack list and status hydration requests (#1820) * perf(frontend): overlap stack list and status hydration requests Fire /stacks and /stacks/statuses concurrently while committing the list first, so list_visible stays progressive and statuses overlap the list fetch instead of serializing behind it. The visible list now renders as soon as it commits (isLoading clears at list commit, not at status completion), and an atomic HydrationEvidence record derived at render time fails closed on node switches, list changes, and partial status payloads: lifecycle actions across the sidebar menu, editor toolbar, mobile editor, bulk bar, and deferred dialogs recheck a ref-backed readiness predicate immediately before any mutation. Status payloads are validated (bulk object format with typed fields, legacy string maps with recognized values); the legacy per-stack fallback targets the captured node and tracks coverage, so a total fallback failure escalates to a hard error instead of masquerading as ok. Pending, error, stale, and incomplete hydration states render distinctly in rows, filter chips, and the mobile masthead. Measured on the SEN-531 workload contract (local node, 6 stacks, Dev Mode ON): hydration gap 52.5ms median to 0ms, full hydration ~146ms to ~84-100ms (~42% improvement). 2590 frontend tests, visual regression 12/12, backend tsc clean. * fix(frontend): recheck hydration readiness in deferred lifecycle executors Deferred continuations (pre-deploy advisory proceed, external-network continue, update-readiness proceed, service-update proceed) dispatched against the node captured when the dialog opened, without rechecking readiness at the actual mutation boundary. A dialog opened on node A could therefore mutate node A after the operator switched to node B. Each executor now rechecks the ref-backed readiness predicate before starting any operation, releasing the pending deploy guard and surfacing a toast when blocked. Plain Save stays independent of status readiness (mobile editor and diff-preview plain-save mode are no longer gated); only Save & Deploy / Save & Reapply requires authoritative runtime evidence. Adds two-node delayed-response tests proving late prior-node list and status results cannot replace the current node's files, statuses, evidence, or loading ownership, plus deferred readiness-loss regression tests for the update and service-update dialogs. * fix(frontend): bind deferred executor readiness to the captured node The previous readiness rechecks validated the CURRENT node, but deferred continuations (pre-deploy advisory proceed, external-network continue, update-readiness proceed, service-update proceed) still dispatched with the node captured when the dialog opened. A dialog opened on node A could mutate node A after the operator switched to node B and B finished hydrating. The executor-boundary predicate now also requires the captured operation node to equal the current active node (effect-updated ref), so a fully-hydrated node switch still blocks the stale continuation. Adds ready-after-switch regression tests for the deploy continuation, stack update, and service update (switch to node B, hydrate, invoke the stored proceed, assert no operation session starts), plus focused plain Save vs Save & Deploy tests for the mobile editor and the diff preview. Restores the unrelated frontend lockfile metadata drift. * fix(frontend): guard external-network creation and reactive retry by node ownership The missing-external-networks dialog's createAndContinue posted network creation requests to the captured node before any ownership check, and the reactive 409 retry callback started a deploy directly without the executor guard. A dialog opened on node A could create Docker networks or deploy on node A after the operator switched to node B. Both paths now recheck hydrationReadyForNode before any mutation: the proactive dialog closes with the pending guard released and a toast when blocked, and the reactive retry refuses to start an operation session. Adds ready-after-switch regression tests for both flows asserting no /system/networks POST and no operation session after a switch to a fully hydrated node B. * fix(frontend): bind policy bypass, delete, and take-down confirmations to their opening node Three deferred confirmation paths remained unsafe across node switches: the policy-bypass retry validated current readiness but dispatched to the policy block's captured node; Delete and Take Down dialogs stored only a stack name and derived the operation node live at confirmation time, so a dialog opened on node A would mutate node B's same-named stack after a switch. Delete and Take Down targets now carry the opening node id in the overlay state (deleteTarget / takeDownTarget), and their confirm handlers verify hydrationReadyForNode against that id before mutating. The policy-bypass retry uses the same predicate against the block's captured node. The reactive 409 regression test now reaches the reactive path: the proactive preflight reports no missing networks, the deploy POST returns the 409, and the reactive refetch opens the dialog, after which an ownership change during network creation blocks the retried deploy. Also restores the unrelated frontend lockfile drift again. * fix(frontend): validate legacy fallback payloads and target stale refreshes at the current node The legacy per-stack fallback counted any successfully decoded JSON body as authoritative coverage, including a 200 non-array body or an array with malformed entries, which could authorize lifecycle actions on non-authoritative evidence. The fallback now requires a container array whose entries satisfy the minimal container shape; malformed responses fail closed and contribute zero coverage. A refreshStacks callback captured on node A and invoked after the operator switched to node B mixed a live list request (localStorage target) with node A statuses and evidence. refreshStacks now reads the current node from its render-synchronous ref, and the list request is targeted explicitly so both requests always share one authoritative node. Adds malformed-fallback regressions (non-array, malformed-array, partial-invalid) and a stale-callback-after-switch test asserting both requests target the current node and its state stays authoritative. --- frontend/src/components/EditorLayout.tsx | 40 +- .../components/EditorLayout/EditorView.tsx | 6 +- .../EditorLayout/MobileComposeEditor.tsx | 14 +- .../EditorLayout/MobileStackDetail.test.tsx | 8 + .../EditorLayout/MobileStackDetail.tsx | 3 + .../components/EditorLayout/ShellOverlays.tsx | 31 +- .../__tests__/ShellOverlays.test.tsx | 140 +++++ .../__tests__/StackIdentityHeader.test.tsx | 1 + .../EditorLayout/editor-view-blocks.tsx | 21 +- .../hooks/useOverlayState.test.ts | 10 +- .../EditorLayout/hooks/useOverlayState.ts | 20 +- .../hooks/useSidebarContextMenu.test.ts | 1 + .../hooks/useSidebarContextMenu.ts | 18 +- .../hooks/useStackActions.test.ts | 424 +++++++++++++- .../EditorLayout/hooks/useStackActions.ts | 128 ++++- .../hooks/useStackListState.test.ts | 528 ++++++++++++++++++ .../EditorLayout/hooks/useStackListState.ts | 480 +++++++++++++--- .../src/components/sidebar/SidebarBulkBar.tsx | 13 +- .../components/sidebar/SidebarFilterChips.tsx | 17 +- frontend/src/components/sidebar/StackList.tsx | 15 +- frontend/src/components/sidebar/StackRow.tsx | 20 +- .../src/components/sidebar/StackSidebar.tsx | 10 +- .../sidebar/__tests__/StackList.test.tsx | 96 ++++ .../src/components/sidebar/sidebar-types.ts | 3 + .../components/sidebar/stack-status-utils.ts | 99 +++- .../src/components/sidebar/stacksLoadUi.ts | 11 +- .../useStackKeyboardShortcuts.test.ts | 1 + .../__tests__/useStackMenuItems.test.tsx | 1 + frontend/src/hooks/useStackMenuItems.tsx | 7 +- 29 files changed, 1991 insertions(+), 175 deletions(-) create mode 100644 frontend/src/components/EditorLayout/__tests__/ShellOverlays.test.tsx diff --git a/frontend/src/components/EditorLayout.tsx b/frontend/src/components/EditorLayout.tsx index 5a432151..e16397aa 100644 --- a/frontend/src/components/EditorLayout.tsx +++ b/frontend/src/components/EditorLayout.tsx @@ -14,7 +14,7 @@ import { ShellOverlays } from './EditorLayout/ShellOverlays'; import type { VolumePreservationOnDelete } from './EditorLayout/DeleteStackDialog'; import { classifyFailedGate } from './EditorLayout/failed-gate-recovery'; import { useEditorViewState } from './EditorLayout/hooks/useEditorViewState'; -import { useStackListState } from './EditorLayout/hooks/useStackListState'; +import { useStackListState, type HydrationDisplayState } from './EditorLayout/hooks/useStackListState'; import { useViewNavigationState } from './EditorLayout/hooks/useViewNavigationState'; import { useUrlSync } from './EditorLayout/hooks/useUrlSync'; import { shouldClearPendingDetailStack } from './EditorLayout/mobile-pending-detail'; @@ -92,6 +92,15 @@ const GlobalObservabilityView = lazy(() => import('./GlobalObservabilityView').t * Reporting that as 'unsupported' would force the destructive delete default onto a node * that may well preserve volumes, so it maps to 'unknown' instead. */ +/** Masthead labels for non-current hydration states. Keyed by the display + * projection so the copy can never drift from the state machine. */ +const HYDRATION_MASTHEAD_LABEL: Record, string> = { + pending: 'Status loading', + error: 'Status unavailable', + stale: 'Status stale', + incomplete: 'Partial status', +}; + function resolveDeleteVolumePreservation(capabilities: string[] | undefined): VolumePreservationOnDelete { if (capabilities == null || capabilities.length === 0) return 'unknown'; return capabilities.includes(STACK_DELETE_PRUNE_VOLUMES_CAPABILITY) ? 'supported' : 'unsupported'; @@ -175,6 +184,11 @@ export default function EditorLayout() { stacksLoadStatus, stacksLoadError, stacksLoadNodeId, + hydrationStatus, + hydrationDisplay, + actionsReady, + hydrationReady, + filterStaleQualifier, } = stackListState; const { nodes, activeNode, setActiveNode, hasCapability, activeNodeMeta, isLoading: nodesLoading } = useNodes(); @@ -207,8 +221,11 @@ export default function EditorLayout() { const { canReapply: canReapplyCompose } = useActiveNodeReapplyEligibility(activeNode?.id); const composeReapply = useComposeReapplyAction(); const isSelfStackSelected = selectedFile ? stackSelfFlags[selectedFile] === true : false; - // Ordinary stacks keep Save & Deploy even when the node supports compose reapply. - const canSaveAndReapply = resolveCanSaveAndReapply(isAdmin, canReapplyCompose, isSelfStackSelected); + // Save & Reapply needs both the self identity AND current status evidence: + // without authoritative evidence the reapply would run against unknown state. + // Save Only stays available regardless. + const canSaveAndReapply = + actionsReady && resolveCanSaveAndReapply(isAdmin, canReapplyCompose, isSelfStackSelected); // Which mode the create dialog opens on (always empty after import tab removal). const [createDialogInitialMode, setCreateDialogInitialMode] = useState('empty'); @@ -721,6 +738,7 @@ export default function EditorLayout() { requestTakeDownStack={stackActions.requestTakeDownStack} showTakeDown={selectedFile ? stackActions.getStackMenuVisibility(selectedFile).showTakeDown : false} isSelfStack={isSelfStackSelected} + actionsReady={actionsReady} canSaveAndReapply={canSaveAndReapply} recoveryResult={selectedFile ? lastActionResult[selectedFile] : undefined} onRefreshState={async () => { @@ -976,7 +994,11 @@ export default function EditorLayout() { stacksLoadStatus, stacksLoadError, onRetryStacksLoad: () => { void retryFrozenRoute(); }, + hydrationDisplay, + hydrationStatus, }} + filterStale={filterStaleQualifier} + actionsReady={actionsReady} activitySummary={activitySummary} onActivityAction={handleActivityAction} bulkMode={bulkMode} @@ -1110,6 +1132,7 @@ export default function EditorLayout() { handleNavigate('fleet'); } }} + hydrationReady={hydrationReady} /> ); @@ -1241,7 +1264,14 @@ export default function EditorLayout() { const { all: stacksAll, up: stacksUp, down: stacksDown, updates: stacksUpdates } = filterCounts; let stacksState = 'All running'; let stacksTone: Tone = 'success'; - if (stacksAll === 0) { + if (hydrationDisplay !== 'current' && stacksAll > 0) { + // Without authoritative status evidence the masthead must never claim + // current health: pending shows a neutral summary, a terminal error is + // not "loading" (it needs a refresh), and stale/incomplete evidence is + // qualified rather than presented as live. + stacksState = HYDRATION_MASTHEAD_LABEL[hydrationDisplay]; + stacksTone = 'brand'; + } else if (stacksAll === 0) { stacksState = 'No stacks'; } else if (stacksDown > 0) { stacksState = `${stacksDown} down`; @@ -1258,7 +1288,7 @@ export default function EditorLayout() { kickerSlot={ openSettings('nodes')} />} state={stacksState} stateTone={stacksTone} - live={stacksDown > 0} + live={hydrationDisplay === 'current' && stacksDown > 0} meta={`${stacksAll} ${stacksAll === 1 ? 'stack' : 'stacks'} · ${stacksUp} up · ${stacksDown} down`} right={mobileMastheadActions} /> diff --git a/frontend/src/components/EditorLayout/EditorView.tsx b/frontend/src/components/EditorLayout/EditorView.tsx index 1c738b09..0801fae3 100644 --- a/frontend/src/components/EditorLayout/EditorView.tsx +++ b/frontend/src/components/EditorLayout/EditorView.tsx @@ -208,6 +208,8 @@ export interface EditorViewProps { showTakeDown: boolean; /** True when this stack is the running Sencho instance on the active node. */ isSelfStack?: boolean; + /** False while status evidence is not authoritative for the active node. */ + actionsReady?: boolean; /** Admin + node reapply eligibility + self-stack: show Save & Reapply instead of Save & Deploy. */ canSaveAndReapply?: boolean; @@ -304,6 +306,7 @@ export function EditorView(props: EditorViewProps) { requestTakeDownStack, showTakeDown, isSelfStack, + actionsReady, canSaveAndReapply = false, recoveryResult, onRefreshState, @@ -459,6 +462,7 @@ export function EditorView(props: EditorViewProps) { requestTakeDownStack={requestTakeDownStack} showTakeDown={showTakeDown} isSelfStack={isSelfStack} + actionsReady={actionsReady} stackMuteActions={stackMuteActions} onOpenMonitor={onOpenMonitor} /> @@ -613,7 +617,7 @@ export function EditorView(props: EditorViewProps) { )}
- diff --git a/frontend/src/components/EditorLayout/MobileComposeEditor.tsx b/frontend/src/components/EditorLayout/MobileComposeEditor.tsx index 812d1162..9ab6008a 100644 --- a/frontend/src/components/EditorLayout/MobileComposeEditor.tsx +++ b/frontend/src/components/EditorLayout/MobileComposeEditor.tsx @@ -28,6 +28,9 @@ interface MobileComposeEditorProps { requestSave: () => void; requestSaveAndDeploy: (e: React.MouseEvent) => void; canSaveAndReapply?: boolean; + /** False while status evidence is not authoritative; disables Save & Deploy + * (Save Only stays available). */ + actionsReady?: boolean; onClose: () => void; hasUnsavedChanges: () => boolean; } @@ -56,6 +59,7 @@ export function MobileComposeEditor(props: MobileComposeEditorProps) { requestSave, requestSaveAndDeploy, canSaveAndReapply = false, + actionsReady = false, onClose, hasUnsavedChanges, } = props; @@ -79,7 +83,11 @@ export function MobileComposeEditor(props: MobileComposeEditorProps) { // while there are unsaved edits (matches the desktop selector being disabled // mid-edit). The compose <-> .env toggle stays free: both buffers persist. const envSwitchDisabled = hasUnsavedChanges() || isFileLoading; - const actionsDisabled = isFileLoading || loadingAction === 'deploy'; + // Plain Save is never gated by status readiness (it is not a lifecycle + // action); only Save & Deploy / Save & Reapply requires authoritative + // runtime evidence. + const saveDisabled = isFileLoading || loadingAction === 'deploy'; + const saveAndDeployDisabled = saveDisabled || !actionsReady; // Read-only while an env-file fetch is in flight: changeEnvFile overwrites the // buffer when it resolves, so edits typed during the load would be silently lost. const editorReadOnly = !canEdit || isFileLoading; @@ -181,7 +189,7 @@ export function MobileComposeEditor(props: MobileComposeEditorProps) { type="button" variant="outline" onClick={requestSave} - disabled={actionsDisabled} + disabled={saveDisabled} data-testid="mobile-editor-save" className="h-11 flex-1 rounded-lg" > @@ -192,7 +200,7 @@ export function MobileComposeEditor(props: MobileComposeEditorProps) { type="button" variant="default" onClick={requestSaveAndDeploy} - disabled={actionsDisabled} + disabled={saveAndDeployDisabled} data-testid="mobile-editor-save-deploy" className="h-11 flex-1 rounded-lg" > diff --git a/frontend/src/components/EditorLayout/MobileStackDetail.test.tsx b/frontend/src/components/EditorLayout/MobileStackDetail.test.tsx index 073fae25..72684c69 100644 --- a/frontend/src/components/EditorLayout/MobileStackDetail.test.tsx +++ b/frontend/src/components/EditorLayout/MobileStackDetail.test.tsx @@ -72,6 +72,7 @@ function makeProps(over: Partial = {}): EditorViewProps { closeComposeEditor: vi.fn(), requestSave: vi.fn(), requestSaveAndDeploy: vi.fn(), + actionsReady: true, discardChanges: vi.fn(), setContent: vi.fn(), setEnvContent: vi.fn(), @@ -266,6 +267,13 @@ describe('MobileStackDetail mobile editing', () => { expect(requestSaveAndDeploy).toHaveBeenCalledTimes(1); }); + + it('keeps plain Save available while Save & Deploy is blocked when readiness is absent', () => { + render(); + expect(screen.getByTestId('mobile-editor-save')).toBeEnabled(); + expect(screen.getByTestId('mobile-editor-save-deploy')).toBeDisabled(); + }); + it('normalizes a files tab to compose so the visible edit saves to the visible file', () => { // Desktop can hand off activeTab='files' when it crosses into the mobile // breakpoint; the editor shows compose, so the shared tab must follow or diff --git a/frontend/src/components/EditorLayout/MobileStackDetail.tsx b/frontend/src/components/EditorLayout/MobileStackDetail.tsx index 4666689e..4abbecba 100644 --- a/frontend/src/components/EditorLayout/MobileStackDetail.tsx +++ b/frontend/src/components/EditorLayout/MobileStackDetail.tsx @@ -76,6 +76,7 @@ export function MobileStackDetail(props: EditorViewProps) { requestTakeDownStack, showTakeDown, isSelfStack = false, + actionsReady = false, canSaveAndReapply = false, onMobileBack, onCloseEditor, @@ -117,6 +118,7 @@ export function MobileStackDetail(props: EditorViewProps) { canEdit={canEditStack} requestSave={requestSave} requestSaveAndDeploy={requestSaveAndDeploy} + actionsReady={actionsReady} canSaveAndReapply={canSaveAndReapply} onClose={onCloseEditor} hasUnsavedChanges={hasUnsavedChanges} @@ -162,6 +164,7 @@ export function MobileStackDetail(props: EditorViewProps) { requestTakeDownStack={requestTakeDownStack} showTakeDown={showTakeDown} isSelfStack={isSelfStack} + actionsReady={actionsReady} stackMuteActions={stackMuteActions} onOpenMonitor={onOpenMonitor} /> diff --git a/frontend/src/components/EditorLayout/ShellOverlays.tsx b/frontend/src/components/EditorLayout/ShellOverlays.tsx index fc67022c..296dcbe4 100644 --- a/frontend/src/components/EditorLayout/ShellOverlays.tsx +++ b/frontend/src/components/EditorLayout/ShellOverlays.tsx @@ -1,4 +1,5 @@ import BashExecModal from '../BashExecModal'; +import { toast } from '@/components/ui/toast-store'; import { PolicyBlockDialog } from '../stack/PolicyBlockDialog'; import { PreDeployScanDialog } from '../stack/PreDeployScanDialog'; import { MissingExternalNetworksDialog } from '../stack/MissingExternalNetworksDialog'; @@ -44,6 +45,9 @@ interface ShellOverlaysProps { canOfferVolumeRemoval: boolean; deleteVolumePreservation: VolumePreservationOnDelete; onOpenFleetNodeUpdates: () => void; + /** Ref-backed readiness check, evaluated at confirmation time so a dialog + * opened while ready cannot dispatch after a node switch or failed refresh. */ + hydrationReady: () => boolean; } export function ShellOverlays({ @@ -65,10 +69,11 @@ export function ShellOverlays({ canOfferVolumeRemoval, deleteVolumePreservation, onOpenFleetNodeUpdates, + hydrationReady, }: ShellOverlaysProps) { const { - deleteDialogOpen, closeDeleteDialog, stackToDelete, - takeDownDialogOpen, closeTakeDownDialog, stackToTakeDown, + deleteDialogOpen, closeDeleteDialog, deleteTarget, + takeDownDialogOpen, closeTakeDownDialog, takeDownTarget, pendingUnsavedLoad, pendingLeaveAction, bashModalOpen, selectedContainer, logViewerOpen, logContainer, @@ -85,11 +90,11 @@ export function ShellOverlays({ } = overlayState; const isDeleteConfirming = - stackToDelete != null && - stackActionMap[resolveStackFileKey(stackFiles, stackToDelete)] === 'delete'; + deleteTarget != null && + stackActionMap[resolveStackFileKey(stackFiles, deleteTarget.name)] === 'delete'; const isTakeDownConfirming = - stackToTakeDown != null && - stackActionMap[resolveStackFileKey(stackFiles, stackToTakeDown)] === 'down'; + takeDownTarget != null && + stackActionMap[resolveStackFileKey(stackFiles, takeDownTarget.name)] === 'down'; const sheetImage = inspectImage && inspectImage.nodeId === activeNodeId ? inspectImage : null; @@ -102,7 +107,7 @@ export function ShellOverlays({ { if (!open) closeDeleteDialog(); }} - stackName={stackToDelete} + stackName={deleteTarget?.name ?? null} volumePreservation={deleteVolumePreservation} onConfirm={stackActions.deleteStack} confirming={isDeleteConfirming} @@ -111,7 +116,7 @@ export function ShellOverlays({ { if (!open) closeTakeDownDialog(); }} - stackName={stackToTakeDown} + stackName={takeDownTarget?.name ?? null} showVolumeOption={canOfferVolumeRemoval} onConfirm={stackActions.takeDownStack} confirming={isTakeDownConfirming} @@ -132,6 +137,9 @@ export function ShellOverlays({ if (!open) setComposeReapplyCapture(null); }} onConfirm={() => { + // Readiness loss keeps the dialog open with the confirmation blocked; + // the capture is only cleared on an actual dispatch. + if (!hydrationReady()) return; const capture = composeReapplyCapture; setComposeReapplyCapture(null); if (!capture || composeReapply.dispatching) return; @@ -276,9 +284,16 @@ export function ShellOverlays({ setDiffPreviewConfirming(true); try { if (snapshot?.mode === 'save-and-deploy') { + // Recheck before the PUT: Save & Deploy from the diff preview must + // not write the file while status evidence is not authoritative. + if (!hydrationReady()) { + toast.error('Status data unavailable. Refresh and try again.'); + return; + } const saved = await stackActions.saveFile(); if (saved) await stackActions.deployStack(); } else { + // Plain Save is never gated by status readiness. await stackActions.saveFile(); } } finally { diff --git a/frontend/src/components/EditorLayout/__tests__/ShellOverlays.test.tsx b/frontend/src/components/EditorLayout/__tests__/ShellOverlays.test.tsx new file mode 100644 index 00000000..6228cbc9 --- /dev/null +++ b/frontend/src/components/EditorLayout/__tests__/ShellOverlays.test.tsx @@ -0,0 +1,140 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; + +beforeEach(() => { + onConfirmMock.mockReset(); +}); + +// The dialog under test: capture its onConfirm so the tests exercise +// ShellOverlays' gating wiring (the corrected surface) without dragging in +// the real modal or its context providers. +const { onConfirmMock } = vi.hoisted(() => ({ onConfirmMock: vi.fn() })); +vi.mock('@/components/ComposeDiffPreviewDialog', () => ({ + ComposeDiffPreviewDialog: (props: { onConfirm: () => void | Promise }) => { + onConfirmMock.mockImplementation(() => props.onConfirm()); + return ; + }, +})); + +// ShellOverlays mounts several context-hungry sheets unconditionally; the +// diff-preview tests exercise only the confirm path, so stub them. +vi.mock('@/components/resources/ImageDetailsSheet', () => ({ ImageDetailsSheet: () => null })); +vi.mock('@/components/VulnerabilityScanSheet', () => ({ VulnerabilityScanSheet: () => null })); +vi.mock('@/components/StackAlertSheet', () => ({ StackAlertSheet: () => null })); +vi.mock('@/components/LogViewer', () => ({ default: () => null })); +vi.mock('@/components/stack/GitSourcePanel', () => ({ GitSourcePanel: () => null })); +vi.mock('@/components/BashExecModal', () => ({ default: () => null })); +vi.mock('@/components/stack/UpdateReadinessDialog', () => ({ UpdateReadinessDialog: () => null })); +vi.mock('@/components/stack/PreDeployScanDialog', () => ({ PreDeployScanDialog: () => null })); +vi.mock('@/components/stack/MissingExternalNetworksDialog', () => ({ MissingExternalNetworksDialog: () => null })); +vi.mock('@/components/stack/PolicyBlockDialog', () => ({ PolicyBlockDialog: () => null })); +vi.mock('@/components/stack/SelfStackProtectedDialog', () => ({ SelfStackProtectedDialog: () => null })); +vi.mock('../FleetView/LocalUpdateConfirmDialog', () => ({ LocalUpdateConfirmDialog: () => null })); +vi.mock('../FleetView/ReconnectingOverlay', () => ({ ReconnectingOverlay: () => null })); +vi.mock('./DeleteStackDialog', () => ({ DeleteStackDialog: () => null })); +vi.mock('./TakeDownStackDialog', () => ({ TakeDownStackDialog: () => null })); +vi.mock('./UnsavedChangesDialog', () => ({ UnsavedChangesDialog: () => null })); + +import { ShellOverlays } from '../ShellOverlays'; +import type { OverlayState } from '../hooks/useOverlayState'; +import type { StackActionsHook } from '../hooks/useStackActions'; + +function makeOverlay(over: Partial = {}): OverlayState { + return { + setPendingUnsavedLoad: vi.fn(), + setPendingLoadOptions: vi.fn(), + setPendingUnsavedNode: vi.fn(), + setPendingLeaveAction: vi.fn(), + pendingUnsavedLoad: null, + pendingLoadOptions: null, + pendingUnsavedNode: null, + pendingLeaveAction: null, + policyBlock: null, + setPolicyBlock: vi.fn(), + setPolicyBypassing: vi.fn(), + updateReadiness: null, + setUpdateReadiness: vi.fn(), + preDeployAdvisory: null, + setPreDeployAdvisory: vi.fn(), + openSelfStackProtected: vi.fn(), + setComposeReapplyCapture: vi.fn(), + composeReapplyCapture: null, + diffPreview: null, + setDiffPreview: vi.fn(), + diffPreviewConfirming: false, + setDiffPreviewConfirming: vi.fn(), + stackToDelete: null, + closeDeleteDialog: vi.fn(), + stackToTakeDown: null, + closeTakeDownDialog: vi.fn(), + ...over, + } as unknown as OverlayState; +} + +function renderShell(overlay: OverlayState, stackActions: Partial) { + return render( + false} + selectedFile="web.yml" + stackName="web" + activeNodeId={1} + gitSourceOpen={false} + setGitSourceOpen={() => {}} + canSelfUpdate={false} + composeReapply={{} as never} + canSaveAndReapply={false} + canOfferVolumeRemoval={false} + deleteVolumePreservation="unknown" + onOpenFleetNodeUpdates={() => {}} + hydrationReady={() => false} + />, + ); +} + +describe('ShellOverlays diff-preview confirmation', () => { + it('lets plain Save through the diff preview while readiness is absent', async () => { + const saveFile = vi.fn().mockResolvedValue(true); + const deployStack = vi.fn(); + renderShell( + makeOverlay({ + diffPreview: { + fileName: 'compose.yml', + language: 'yaml', + original: 'a', + modified: 'b', + mode: 'save', + }, + }), + { saveFile, deployStack }, + ); + fireEvent.click(screen.getByRole('button', { name: 'diff-confirm' })); + expect(saveFile).toHaveBeenCalledTimes(1); + expect(deployStack).not.toHaveBeenCalled(); + }); + + it('blocks Save & Deploy through the diff preview while readiness is absent', async () => { + const saveFile = vi.fn().mockResolvedValue(true); + const deployStack = vi.fn(); + renderShell( + makeOverlay({ + diffPreview: { + fileName: 'compose.yml', + language: 'yaml', + original: 'a', + modified: 'b', + mode: 'save-and-deploy', + }, + }), + { saveFile, deployStack }, + ); + fireEvent.click(screen.getByRole('button', { name: 'diff-confirm' })); + expect(saveFile).not.toHaveBeenCalled(); + expect(deployStack).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/components/EditorLayout/__tests__/StackIdentityHeader.test.tsx b/frontend/src/components/EditorLayout/__tests__/StackIdentityHeader.test.tsx index d26bc735..752862d5 100644 --- a/frontend/src/components/EditorLayout/__tests__/StackIdentityHeader.test.tsx +++ b/frontend/src/components/EditorLayout/__tests__/StackIdentityHeader.test.tsx @@ -52,6 +52,7 @@ function renderHeader(over: Partial> requestDeleteStack={vi.fn()} requestTakeDownStack={vi.fn()} showTakeDown={false} + actionsReady={true} {...over} />, ); diff --git a/frontend/src/components/EditorLayout/editor-view-blocks.tsx b/frontend/src/components/EditorLayout/editor-view-blocks.tsx index d94368ed..8d0d6be1 100644 --- a/frontend/src/components/EditorLayout/editor-view-blocks.tsx +++ b/frontend/src/components/EditorLayout/editor-view-blocks.tsx @@ -143,6 +143,9 @@ export interface StackIdentityHeaderProps { showTakeDown: boolean; /** True when this stack is the running Sencho instance on the active node. */ isSelfStack?: boolean; + /** False while status evidence is not authoritative; disables lifecycle + * buttons so the editor never mutates against unknown runtime state. */ + actionsReady?: boolean; stackMuteActions?: ReturnType; /** Opens the stack Monitor sheet on the Alerts tab. */ onOpenMonitor?: () => void; @@ -170,10 +173,14 @@ export function StackIdentityHeader({ requestTakeDownStack, showTakeDown, isSelfStack = false, + actionsReady = false, stackMuteActions, onOpenMonitor, }: StackIdentityHeaderProps) { + // Distinct from the self-stack protection: readiness loss (pending/error/ + // stale hydration) disables the same buttons without claiming self identity. const selfProtected = isSelfStack; + const actionsDisabled = loadingAction !== null || selfProtected || !actionsReady; return (
{/* Identity block */} @@ -215,18 +222,18 @@ export function StackIdentityHeader({ {canDeploy && ( <> {isRunning ? ( - ) : ( - )} {isRunning && ( - @@ -239,13 +246,13 @@ export function StackIdentityHeader({ data-testid="stack-take-down-button" className="rounded-lg max-md:h-11 border-warning/40 text-warning hover:bg-warning/10" onClick={() => requestTakeDownStack(stackName)} - disabled={loadingAction !== null || selfProtected} + disabled={actionsDisabled} > {loadingAction === 'down' ? 'Taking down...' : 'Take down'} )} - @@ -260,7 +267,7 @@ export function StackIdentityHeader({ {canRollback && ( - +
{loadingAction === 'rollback' ? 'Rolling back...' : 'Rollback'} @@ -291,7 +298,7 @@ export function StackIdentityHeader({ {canDelete && ( diff --git a/frontend/src/components/EditorLayout/hooks/useOverlayState.test.ts b/frontend/src/components/EditorLayout/hooks/useOverlayState.test.ts index 0f234e13..23e51442 100644 --- a/frontend/src/components/EditorLayout/hooks/useOverlayState.test.ts +++ b/frontend/src/components/EditorLayout/hooks/useOverlayState.test.ts @@ -14,7 +14,7 @@ describe('useOverlayState', () => { const { result } = renderHook(() => useOverlayState()); expect(result.current.createDialogOpen).toBe(false); expect(result.current.deleteDialogOpen).toBe(false); - expect(result.current.stackToDelete).toBeNull(); + expect(result.current.deleteTarget).toBeNull(); expect(result.current.pendingUnsavedLoad).toBeNull(); expect(result.current.pendingLoadOptions).toBeNull(); expect(result.current.pendingUnsavedNode).toBeNull(); @@ -48,17 +48,17 @@ describe('useOverlayState', () => { it('openDeleteDialog sets open flag and stack name', () => { const { result } = renderHook(() => useOverlayState()); - act(() => result.current.openDeleteDialog('my-stack')); + act(() => result.current.openDeleteDialog({ name: 'my-stack', nodeId: 1 })); expect(result.current.deleteDialogOpen).toBe(true); - expect(result.current.stackToDelete).toBe('my-stack'); + expect(result.current.deleteTarget).toEqual({ name: 'my-stack', nodeId: 1 }); }); it('closeDeleteDialog resets delete state', () => { const { result } = renderHook(() => useOverlayState()); - act(() => result.current.openDeleteDialog('my-stack')); + act(() => result.current.openDeleteDialog({ name: 'my-stack', nodeId: 1 })); act(() => result.current.closeDeleteDialog()); expect(result.current.deleteDialogOpen).toBe(false); - expect(result.current.stackToDelete).toBeNull(); + expect(result.current.deleteTarget).toBeNull(); }); it('openLogViewer sets open flag and container object', () => { diff --git a/frontend/src/components/EditorLayout/hooks/useOverlayState.ts b/frontend/src/components/EditorLayout/hooks/useOverlayState.ts index 51b24be3..9bf7dab7 100644 --- a/frontend/src/components/EditorLayout/hooks/useOverlayState.ts +++ b/frontend/src/components/EditorLayout/hooks/useOverlayState.ts @@ -52,25 +52,25 @@ export function useOverlayState() { const [createDialogOpen, setCreateDialogOpen] = useState(false); const [deleteDialogOpen, setDeleteDialogOpen] = useState(false); - const [stackToDelete, setStackToDelete] = useState(null); - const openDeleteDialog = useCallback((stackName: string) => { - setStackToDelete(stackName); + const [deleteTarget, setDeleteTarget] = useState<{ name: string; nodeId: number | null } | null>(null); + const openDeleteDialog = useCallback((target: { name: string; nodeId: number | null }) => { + setDeleteTarget(target); setDeleteDialogOpen(true); }, []); const closeDeleteDialog = useCallback(() => { setDeleteDialogOpen(false); - setStackToDelete(null); + setDeleteTarget(null); }, []); const [takeDownDialogOpen, setTakeDownDialogOpen] = useState(false); - const [stackToTakeDown, setStackToTakeDown] = useState(null); - const openTakeDownDialog = useCallback((stackName: string) => { - setStackToTakeDown(stackName); + const [takeDownTarget, setTakeDownTarget] = useState<{ name: string; nodeId: number | null } | null>(null); + const openTakeDownDialog = useCallback((target: { name: string; nodeId: number | null }) => { + setTakeDownTarget(target); setTakeDownDialogOpen(true); }, []); const closeTakeDownDialog = useCallback(() => { setTakeDownDialogOpen(false); - setStackToTakeDown(null); + setTakeDownTarget(null); }, []); const [pendingUnsavedLoad, setPendingUnsavedLoad] = useState(null); @@ -192,8 +192,8 @@ export function useOverlayState() { return { createDialogOpen, setCreateDialogOpen, - deleteDialogOpen, stackToDelete, openDeleteDialog, closeDeleteDialog, - takeDownDialogOpen, stackToTakeDown, openTakeDownDialog, closeTakeDownDialog, + deleteDialogOpen, deleteTarget, openDeleteDialog, closeDeleteDialog, + takeDownDialogOpen, takeDownTarget, openTakeDownDialog, closeTakeDownDialog, pendingUnsavedLoad, setPendingUnsavedLoad, pendingLoadOptions, setPendingLoadOptions, pendingUnsavedNode, setPendingUnsavedNode, diff --git a/frontend/src/components/EditorLayout/hooks/useSidebarContextMenu.test.ts b/frontend/src/components/EditorLayout/hooks/useSidebarContextMenu.test.ts index 1ee613f8..57a70db4 100644 --- a/frontend/src/components/EditorLayout/hooks/useSidebarContextMenu.test.ts +++ b/frontend/src/components/EditorLayout/hooks/useSidebarContextMenu.test.ts @@ -29,6 +29,7 @@ function makeOptions( pin: vi.fn(), unpin: vi.fn(), refreshLabels: vi.fn(), + hydrationReady: () => true, }; const stackActions = { getStackMenuVisibility: () => ({ showDeploy: false, showStop: true, showRestart: true, showUpdate: false }), diff --git a/frontend/src/components/EditorLayout/hooks/useSidebarContextMenu.ts b/frontend/src/components/EditorLayout/hooks/useSidebarContextMenu.ts index 2628237c..d714af35 100644 --- a/frontend/src/components/EditorLayout/hooks/useSidebarContextMenu.ts +++ b/frontend/src/components/EditorLayout/hooks/useSidebarContextMenu.ts @@ -53,12 +53,20 @@ export function useSidebarContextMenu({ const stackStatus = rawStatus === 'partial' ? 'running' : rawStatus; const nodeId = activeNode?.id ?? null; const canMuteNotifications = isAdmin && hasCapability('notification-suppression'); + // Fail closed without authoritative status evidence: missing self identity + // must not read as "ordinary stack", and an unverified port must not offer + // Open App. + const ready = stackListState.hydrationReady(); return { stackStatus, + ready, + // Identity stays factual: missing evidence must not mislabel an ordinary + // stack as the Sencho instance. The `ready` gate plus the handler-boundary + // rechecks are what block actions, not a falsified identity field. isSelfStack: stackListState.stackSelfFlags[file] === true, // Only offer "Open App" when a browser-reachable URL can actually be built // (a remote node with no API host, e.g. a pilot agent, yields none). - canOpenApp: mainPort !== undefined && buildServiceUrl({ node: activeNode, publicPort: mainPort }) !== null, + canOpenApp: ready && mainPort !== undefined && buildServiceUrl({ node: activeNode, publicPort: mainPort }) !== null, isBusy: stackListState.isStackBusy(file), isAdmin, canDelete: can('stack:delete', 'stack', sName, nodeId), @@ -84,11 +92,14 @@ export function useSidebarContextMenu({ update: () => stackActions.executeStackActionByFile(file, 'update', 'update'), takeDown: () => stackActions.requestTakeDownStack(sName), remove: () => { + // Readiness recheck at the handler boundary: a menu built while ready + // must not delete after a node switch or failed refresh. + if (!stackListState.hydrationReady()) return; if (stackListState.stackSelfFlags[file]) { overlayState.openSelfStackProtected(); return; } - overlayState.openDeleteDialog(sName); + overlayState.openDeleteDialog({ name: sName, nodeId: activeNode?.id ?? null }); }, pin: () => stackListState.pin(file), unpin: () => stackListState.unpin(file), @@ -146,6 +157,9 @@ export function useSidebarContextMenu({ }, openLabelManager: () => navState.handleOpenSettings('labels'), openScheduleTask: () => { + // Scheduling a lifecycle task needs the same authoritative runtime + // evidence as running it directly. + if (!stackListState.hydrationReady()) return; navState.setSchedulePrefill({ stackName: sName, nodeId: activeNode?.id ?? null }); navState.setActiveView('scheduled-ops'); }, diff --git a/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts b/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts index 65665ba1..86e70be7 100644 --- a/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts +++ b/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts @@ -27,6 +27,13 @@ type NavState = ReturnType; type ActiveNode = Parameters[0]['activeNode']; const DEFAULT_ACTIVE_NODE = { id: 1, name: 'Local', type: 'local' } as ActiveNode; +function okJson(payload: unknown): Response { + return new Response(JSON.stringify(payload), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); +} + function makeEditorState(over: Partial = {}): EditorState { const base = { content: 'services: {}', @@ -86,6 +93,7 @@ function makeStackListState(over: Partial = {}): StackListState recordActionSuccess: vi.fn(), clearActionRecords: vi.fn(), dismissActionResult: vi.fn(), + hydrationReady: vi.fn().mockReturnValue(true), }; return { ...base, ...over } as unknown as StackListState; } @@ -111,7 +119,8 @@ function makeOverlay(over: Partial = {}): OverlayState { setComposeReapplyCapture: vi.fn(), composeReapplyCapture: null, setDiffPreview: vi.fn(), - stackToDelete: null, + setMissingExternalNetworks: vi.fn(), + deleteTarget: null, closeDeleteDialog: vi.fn(), ...over, } as unknown as OverlayState; @@ -130,6 +139,7 @@ function setup(over: { navState?: Partial; getLastDeployOutputLine?: (stackName: string) => string | undefined; hasUpdateGuard?: boolean; + hasGuidedExternalNetworkPreflight?: boolean; canEditStack?: (stackNameOrFilename: string) => boolean; activeNode?: Parameters[0]['activeNode']; setActiveNode?: Parameters[0]['setActiveNode']; @@ -150,19 +160,26 @@ function setup(over: { const onDeletedOpenStack = over.onDeletedOpenStack ?? vi.fn(); const removeNotificationsForStack = over.removeNotificationsForStack ?? vi.fn(); - const { result } = renderHook(() => + // Live node holder so a test can re-render the hook with a different active + // node (e.g. to prove a deferred continuation captured for node A is blocked + // after the operator switches to node B). + const activeNodeHolder: { current: ActiveNode | null } = { + current: over.activeNode === undefined ? DEFAULT_ACTIVE_NODE : over.activeNode, + }; + const { result, rerender } = renderHook(() => useStackActions({ editorState, stackListState, navState, overlayState, - activeNode: over.activeNode === undefined ? DEFAULT_ACTIVE_NODE : over.activeNode, + activeNode: activeNodeHolder.current, setActiveNode, nodes: [], runWithLog, getLastDeployOutputLine: over.getLastDeployOutputLine ?? (() => undefined), diffPreviewEnabled: false, hasUpdateGuard: over.hasUpdateGuard ?? false, + hasGuidedExternalNetworkPreflight: over.hasGuidedExternalNetworkPreflight ?? false, canEditStack: over.canEditStack ?? (() => true), onDeletedOpenStack, removeNotificationsForStack, @@ -170,7 +187,7 @@ function setup(over: { canReapplyCompose: over.canReapplyCompose ?? false, }), ); - return { result, editorState, stackListState, overlayState, navState, setActiveNode, onDeletedOpenStack, removeNotificationsForStack }; + return { result, rerender, activeNodeHolder, editorState, stackListState, overlayState, navState, setActiveNode, onDeletedOpenStack, removeNotificationsForStack }; } describe('useStackActions.saveFile', () => { @@ -621,7 +638,7 @@ describe('useStackActions.bypassPolicyAndRetry', () => { vi.mocked(apiFetch).mockResolvedValueOnce(new Response(null, { status: 200 })); // update OK vi.mocked(apiFetch).mockResolvedValueOnce(new Response('[]', { status: 200 })); // containers refresh const { result } = setup({ - overlay: { policyBlock: { stackName: 'web', stackFile: 'web.yml', action: 'update', payload } as never }, + overlay: { policyBlock: { stackName: 'web', stackFile: 'web.yml', action: 'update', payload, nodeId: 1 } as never }, }); await result.current.bypassPolicyAndRetry(); const urls = vi.mocked(apiFetch).mock.calls.map(c => String(c[0])); @@ -633,7 +650,7 @@ describe('useStackActions.bypassPolicyAndRetry', () => { vi.mocked(apiFetch).mockResolvedValueOnce(new Response(null, { status: 200 })); // deploy OK vi.mocked(apiFetch).mockResolvedValueOnce(new Response('[]', { status: 200 })); // containers refresh const { result } = setup({ - overlay: { policyBlock: { stackName: 'web', stackFile: 'web.yml', action: 'deploy', payload } as never }, + overlay: { policyBlock: { stackName: 'web', stackFile: 'web.yml', action: 'deploy', payload, nodeId: 1 } as never }, }); await result.current.bypassPolicyAndRetry(); const urls = vi.mocked(apiFetch).mock.calls.map(c => String(c[0])); @@ -646,24 +663,37 @@ describe('useStackActions.bypassPolicyAndRetry', () => { vi.mocked(apiFetch).mockResolvedValueOnce(new Response('content', { status: 200 })); // content reload vi.mocked(apiFetch).mockResolvedValueOnce(new Response(JSON.stringify({ exists: true }), { status: 200 })); // backup info const { result } = setup({ - overlay: { policyBlock: { stackName: 'web', stackFile: 'web.yml', action: 'rollback', payload } as never }, + overlay: { policyBlock: { stackName: 'web', stackFile: 'web.yml', action: 'rollback', payload, nodeId: 1 } as never }, }); await result.current.bypassPolicyAndRetry(); const urls = vi.mocked(apiFetch).mock.calls.map(c => String(c[0])); expect(urls).toContain('/stacks/web.yml/rollback?ignorePolicy=true'); }); - it('retries on the node captured in the policy block, not the live active node', async () => { + it('blocks the bypass when the policy block was captured for another node, even once it is fully hydrated', async () => { + vi.mocked(apiFetch).mockReset(); + const { result, stackListState, rerender, activeNodeHolder } = setup({ + overlay: { policyBlock: { stackName: 'web', stackFile: 'web.yml', action: 'update', payload, nodeId: 1 } as never }, + }); + // The block was raised on node 1; the operator switches to node 2, which + // finishes hydrating (readiness true for node 2). + activeNodeHolder.current = { id: 2, type: 'remote' } as ActiveNode; + rerender(); + vi.mocked(stackListState.hydrationReady).mockReturnValue(true); + await result.current.bypassPolicyAndRetry(); + expect(apiFetch).not.toHaveBeenCalled(); + }); + + it('retries on the policy block node when it is still the active node', async () => { vi.mocked(apiFetch).mockResolvedValueOnce(new Response(null, { status: 200 })); // update OK vi.mocked(apiFetch).mockResolvedValueOnce(new Response('[]', { status: 200 })); // containers refresh const { result } = setup({ - activeNode: { id: 1, type: 'local' } as never, // active node has since moved to 1 - overlay: { policyBlock: { stackName: 'web', stackFile: 'web.yml', action: 'update', payload, nodeId: 9 } as never }, + overlay: { policyBlock: { stackName: 'web', stackFile: 'web.yml', action: 'update', payload, nodeId: 1 } as never }, }); await result.current.bypassPolicyAndRetry(); const updateCall = vi.mocked(apiFetch).mock.calls.find(c => String(c[0]).includes('/update?ignorePolicy=true')); expect(updateCall).toBeDefined(); - expect((updateCall![1] as { nodeId?: number | null }).nodeId).toBe(9); + expect((updateCall![1] as { nodeId?: number | null }).nodeId).toBe(1); }); it('does nothing when no policy block is stored', async () => { @@ -1731,7 +1761,7 @@ describe('useStackActions.deleteStack', () => { it('leaves the editor for dashboard when deleting the open stack by filename', async () => { vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 })); const { result, stackListState, overlayState, navState, onDeletedOpenStack } = setup({ - overlay: { stackToDelete: 'web.yml' }, + overlay: { deleteTarget: { name: 'web.yml', nodeId: 1 } }, stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, navState: { activeView: 'editor' }, }); @@ -1752,7 +1782,7 @@ describe('useStackActions.deleteStack', () => { it('passes pruneVolumes=true through unconditionally, including on nodes without the capability', async () => { vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 })); const { result } = setup({ - overlay: { stackToDelete: 'web.yml' }, + overlay: { deleteTarget: { name: 'web.yml', nodeId: 1 } }, stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, navState: { activeView: 'editor' }, }); @@ -1767,7 +1797,7 @@ describe('useStackActions.deleteStack', () => { it('clears isFileLoading on delete-leave so the URL writer is not blocked', async () => { vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 })); const { result, editorState } = setup({ - overlay: { stackToDelete: 'web.yml' }, + overlay: { deleteTarget: { name: 'web.yml', nodeId: 1 } }, stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, editorState: { isFileLoading: true }, navState: { activeView: 'editor' }, @@ -1783,7 +1813,7 @@ describe('useStackActions.deleteStack', () => { it('leaves the editor when sidebar delete passes a basename', async () => { vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 })); const { result, stackListState, navState, onDeletedOpenStack } = setup({ - overlay: { stackToDelete: 'web' }, + overlay: { deleteTarget: { name: 'web', nodeId: 1 } }, stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, navState: { activeView: 'editor' }, }); @@ -1801,7 +1831,7 @@ describe('useStackActions.deleteStack', () => { it('does not navigate when deleting a different stack', async () => { vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 })); const { result, stackListState, navState, onDeletedOpenStack } = setup({ - overlay: { stackToDelete: 'other.yml' }, + overlay: { deleteTarget: { name: 'other.yml', nodeId: 1 } }, stackList: { selectedFile: 'web.yml', files: ['web.yml', 'other.yml'], @@ -1822,7 +1852,7 @@ describe('useStackActions.deleteStack', () => { it('clears selection without navigating when the matching stack is hidden behind another view', async () => { vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 })); const { result, stackListState, navState, onDeletedOpenStack } = setup({ - overlay: { stackToDelete: 'web.yml' }, + overlay: { deleteTarget: { name: 'web.yml', nodeId: 1 } }, stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, navState: { activeView: 'resources' }, }); @@ -1841,7 +1871,7 @@ describe('useStackActions.deleteStack', () => { vi.mocked(apiFetch).mockResolvedValue(new Response('boom', { status: 500 })); const { toast } = await import('@/components/ui/toast-store'); const { result, stackListState, overlayState, navState, onDeletedOpenStack } = setup({ - overlay: { stackToDelete: 'web.yml' }, + overlay: { deleteTarget: { name: 'web.yml', nodeId: 1 } }, stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, navState: { activeView: 'editor' }, }); @@ -1870,7 +1900,7 @@ describe('useStackActions.deleteStack', () => { ); const { toast } = await import('@/components/ui/toast-store'); const { result } = setup({ - overlay: { stackToDelete: 'web.yml' }, + overlay: { deleteTarget: { name: 'web.yml', nodeId: 1 } }, stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, navState: { activeView: 'editor' }, }); @@ -1887,7 +1917,7 @@ describe('useStackActions.deleteStack', () => { new Response(JSON.stringify({ code: 'self_stack_protected' }), { status: 409 }), ); const { result, stackListState, overlayState, navState, onDeletedOpenStack } = setup({ - overlay: { stackToDelete: 'web.yml' }, + overlay: { deleteTarget: { name: 'web.yml', nodeId: 1 } }, stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, navState: { activeView: 'editor' }, }); @@ -1908,7 +1938,7 @@ describe('useStackActions.deleteStack', () => { vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 })); const removeNotificationsForStack = vi.fn(); const { result } = setup({ - overlay: { stackToDelete: 'web.yml' }, + overlay: { deleteTarget: { name: 'web.yml', nodeId: 7 } }, stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, navState: { activeView: 'editor' }, activeNode: { id: 7, type: 'local' } as Parameters[0]['activeNode'], @@ -1927,7 +1957,7 @@ describe('useStackActions.deleteStack', () => { vi.mocked(apiFetch).mockResolvedValue(new Response('boom', { status: 500 })); const removeNotificationsForStack = vi.fn(); const { result } = setup({ - overlay: { stackToDelete: 'web.yml' }, + overlay: { deleteTarget: { name: 'web.yml', nodeId: 1 } }, stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, navState: { activeView: 'editor' }, removeNotificationsForStack, @@ -2009,3 +2039,353 @@ describe('useStackActions.openInspectImage', () => { }); }); +describe('useStackActions hydration readiness gates', () => { + it('fails all lifecycle actions closed while readiness is absent', () => { + const { result } = setup({ stackList: { hydrationReady: () => false } as never }); + expect(result.current.getStackMenuVisibility('web.yml')).toEqual({ + showDeploy: false, showStop: false, showRestart: false, showUpdate: false, showTakeDown: false, + }); + }); + + it('blocks openStackApp without opening the self-stack modal while readiness is absent', () => { + const { result, overlayState } = setup({ stackList: { hydrationReady: () => false } as never }); + result.current.openStackApp('web.yml'); + expect(overlayState.openSelfStackProtected).not.toHaveBeenCalled(); + }); + + it('blocks restartStack while readiness is absent without starting an operation session', async () => { + lastRunWithLogParams = null; + const { result } = setup({ stackList: { hydrationReady: () => false } as never }); + await result.current.restartStack(); + expect(result.current.getStackMenuVisibility('web.yml')).toEqual({ + showDeploy: false, showStop: false, showRestart: false, showUpdate: false, showTakeDown: false, + }); + // No deploy-feedback session may start while blocked. + expect(lastRunWithLogParams).toBeNull(); + }); + + it('keeps restart available for a confirmed self stack when ready', () => { + const { result } = setup({ + stackList: { + stackStatuses: { 'sencho.yml': 'running' } as never, + stackSelfFlags: { 'sencho.yml': true }, + }, + }); + const v = result.current.getStackMenuVisibility('sencho.yml'); + expect(v.showRestart).toBe(true); + expect(v.showDeploy).toBe(false); + }); +}); + +describe('useStackActions deferred readiness-loss guards', () => { + it('blocks the update-readiness proceed when readiness was lost after the dialog opened', async () => { + lastRunWithLogParams = null; + const { result, overlayState, stackListState } = setup({ hasUpdateGuard: true }); + await act(async () => { + await result.current.updateStack(); + }); + // The mock was invoked with the object form (never the setter form). + const proceed = ( + vi.mocked(overlayState.setUpdateReadiness).mock.calls[0][0] as { proceed: () => void } + ).proceed; + // Readiness lost while the dialog is open (node switch, failed refresh). + vi.mocked(stackListState.hydrationReady).mockReturnValue(false); + await act(async () => { + proceed(); + }); + // No operation session may start against either the current or captured node. + expect(lastRunWithLogParams).toBeNull(); + }); + + it('blocks a deferred service update when readiness was lost after the dialog opened', async () => { + lastRunWithLogParams = null; + const { result, overlayState, stackListState } = setup({ hasUpdateGuard: true }); + await act(async () => { + await result.current.requestServiceUpdate('web.yml', 'web'); + }); + const proceed = ( + vi.mocked(overlayState.setUpdateReadiness).mock.calls[0][0] as { proceed: () => void } + ).proceed; + vi.mocked(stackListState.hydrationReady).mockReturnValue(false); + await act(async () => { + proceed(); + }); + expect(lastRunWithLogParams).toBeNull(); + }); + + it('blocks a deferred stack update captured for node A after the switch to node B is fully hydrated', async () => { + lastRunWithLogParams = null; + const { result, overlayState, stackListState, rerender, activeNodeHolder } = setup({ hasUpdateGuard: true }); + await act(async () => { + await result.current.updateStack(); + }); + const proceed = ( + vi.mocked(overlayState.setUpdateReadiness).mock.calls[0][0] as { proceed: () => void } + ).proceed; + // Switch to node B and let it finish hydrating: readiness is true for B, + // but the captured operation node is still A. + activeNodeHolder.current = { id: 2, name: 'B', type: 'remote' } as ActiveNode; + rerender(); + vi.mocked(stackListState.hydrationReady).mockReturnValue(true); + await act(async () => { + proceed(); + }); + expect(lastRunWithLogParams).toBeNull(); + }); + + it('blocks a deferred service update captured for node A after the switch to node B is fully hydrated', async () => { + lastRunWithLogParams = null; + const { result, overlayState, stackListState, rerender, activeNodeHolder } = setup({ hasUpdateGuard: true }); + await act(async () => { + await result.current.requestServiceUpdate('web.yml', 'web'); + }); + const proceed = ( + vi.mocked(overlayState.setUpdateReadiness).mock.calls[0][0] as { proceed: () => void } + ).proceed; + activeNodeHolder.current = { id: 2, name: 'B', type: 'remote' } as ActiveNode; + rerender(); + vi.mocked(stackListState.hydrationReady).mockReturnValue(true); + await act(async () => { + proceed(); + }); + expect(lastRunWithLogParams).toBeNull(); + }); + + it('blocks a deferred deploy captured for node A after the switch to node B is fully hydrated', async () => { + lastRunWithLogParams = null; + vi.mocked(apiFetch).mockReset(); + vi.mocked(apiFetch).mockResolvedValue( + new Response(JSON.stringify({ enabled: true, images: [{}] }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ); + const { result, overlayState, stackListState, rerender, activeNodeHolder } = setup(); + await act(async () => { + await result.current.deployStack(); + }); + // The advisory dialog opened; its proceed is the deferred continuation. + const proceed = ( + vi.mocked(overlayState.setPreDeployAdvisory).mock.calls[0][0] as { proceed: () => void } + ).proceed; + // Switch to node B and let it finish hydrating: readiness is true for B, + // but the captured operation node is still A. + activeNodeHolder.current = { id: 2, name: 'B', type: 'remote' } as ActiveNode; + rerender(); + vi.mocked(stackListState.hydrationReady).mockReturnValue(true); + await act(async () => { + proceed(); + }); + expect(lastRunWithLogParams).toBeNull(); + }); +}); + +describe('useStackActions external-network ownership guards', () => { + function okJson(payload: unknown): Response { + return new Response(JSON.stringify(payload), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + } + + it('does not create networks on the captured node from the proactive dialog after a switch', async () => { + lastRunWithLogParams = null; + vi.mocked(apiFetch).mockReset(); + const networkPosts: string[] = []; + vi.mocked(apiFetch).mockImplementation(async (url: string, init?: RequestInit) => { + if (url === '/security/stacks/web/pre-deploy-summary') { + return okJson({ enabled: false }); + } + if (url === '/stacks/web/missing-external-networks') { + return okJson({ + status: 'ok', + stackName: 'web', + networks: [{ name: 'ext-net', safe: false }], + autoCreateEnabled: false, + declaredExternalCount: 1, + }); + } + if (url === '/system/networks' && init?.method === 'POST') { + networkPosts.push(url); + return new Response(null, { status: 201 }); + } + return new Response(null, { status: 200 }); + }); + const { result, overlayState, stackListState, rerender, activeNodeHolder } = setup({ + hasGuidedExternalNetworkPreflight: true, + }); + await act(async () => { + await result.current.deployStack(); + }); + const dialog = ( + vi.mocked(overlayState.setMissingExternalNetworks).mock.calls[0][0] as unknown as { + createAndContinue: () => Promise; + } + ); + // Switch to node B and let it finish hydrating: readiness is true for B, + // but the dialog was captured for node A. + activeNodeHolder.current = { id: 2, name: 'B', type: 'remote' } as ActiveNode; + rerender(); + vi.mocked(stackListState.hydrationReady).mockReturnValue(true); + await act(async () => { + await dialog.createAndContinue(); + }); + expect(networkPosts).toEqual([]); + expect(lastRunWithLogParams).toBeNull(); + }); + + it('does not create networks on the captured node from the reactive 409 dialog after a switch', async () => { + lastRunWithLogParams = null; + vi.mocked(apiFetch).mockReset(); + const networkPosts: string[] = []; + vi.mocked(apiFetch).mockImplementation(async (url: string, init?: RequestInit) => { + if (url === '/security/stacks/web/pre-deploy-summary') { + return okJson({ enabled: false }); + } + if (url === '/stacks/web/deploy') { + // Reactive 409: the backend reports missing external networks. + return new Response(JSON.stringify({ code: 'missing_external_networks' }), { status: 409 }); + } + if (url === '/stacks/web/missing-external-networks') { + return okJson({ + status: 'ok', + stackName: 'web', + networks: [{ name: 'ext-net', safe: false }], + autoCreateEnabled: false, + declaredExternalCount: 1, + }); + } + if (url === '/system/networks' && init?.method === 'POST') { + networkPosts.push(url); + return new Response(null, { status: 201 }); + } + return new Response(null, { status: 200 }); + }); + const { result, overlayState, stackListState, rerender, activeNodeHolder } = setup({ + hasGuidedExternalNetworkPreflight: true, + }); + await act(async () => { + await result.current.deployStack(); + }); + const dialog = ( + vi.mocked(overlayState.setMissingExternalNetworks).mock.calls[0][0] as unknown as { + createAndContinue: () => Promise; + } + ); + activeNodeHolder.current = { id: 2, name: 'B', type: 'remote' } as ActiveNode; + rerender(); + vi.mocked(stackListState.hydrationReady).mockReturnValue(true); + await act(async () => { + await dialog.createAndContinue(); + }); + expect(networkPosts).toEqual([]); + expect(lastRunWithLogParams).toBeNull(); + }); +}); + +describe('useStackActions delete/take-down node-bound confirmations', () => { + it('blocks delete confirmed for node A after the switch to node B is fully hydrated', async () => { + vi.mocked(apiFetch).mockReset(); + const { result, stackListState, rerender, activeNodeHolder } = setup({ + overlay: { deleteTarget: { name: 'web.yml', nodeId: 1 } }, + stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, + }); + // The dialog opened on node 1; the operator switched to node 2, which + // finished hydrating (readiness true for node 2). + activeNodeHolder.current = { id: 2, type: 'remote' } as ActiveNode; + rerender(); + vi.mocked(stackListState.hydrationReady).mockReturnValue(true); + await act(async () => { + await result.current.deleteStack(false); + }); + expect(apiFetch).not.toHaveBeenCalled(); + }); + + it('blocks take down confirmed for node A after the switch to node B is fully hydrated', async () => { + vi.mocked(apiFetch).mockReset(); + const { result, stackListState, rerender, activeNodeHolder } = setup({ + overlay: { takeDownTarget: { name: 'web.yml', nodeId: 1 } }, + stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, + }); + activeNodeHolder.current = { id: 2, type: 'remote' } as ActiveNode; + rerender(); + vi.mocked(stackListState.hydrationReady).mockReturnValue(true); + await act(async () => { + await result.current.takeDownStack(false); + }); + expect(apiFetch).not.toHaveBeenCalled(); + }); +}); + +describe('useStackActions reactive external-network retry ownership', () => { + it('prevents the reactive retry from starting after ownership changes during creation', async () => { + lastRunWithLogParams = null; + vi.mocked(apiFetch).mockReset(); + let resolveNetworkCreate: (r: Response) => void; + const networkCreateGate = new Promise((r) => { resolveNetworkCreate = r; }); + let missingNetworksCalls = 0; + const deployCalls: string[] = []; + vi.mocked(apiFetch).mockImplementation(async (url: string, init?: RequestInit) => { + if (url === '/security/stacks/web/pre-deploy-summary') { + return okJson({ enabled: false }); + } + if (url === '/stacks/web/missing-external-networks') { + missingNetworksCalls += 1; + if (missingNetworksCalls === 1) { + // Proactive preflight: nothing missing, deploy proceeds. + return okJson({ status: 'ok', stackName: 'web', networks: [], autoCreateEnabled: true, declaredExternalCount: 0 }); + } + if (missingNetworksCalls === 2) { + // Reactive refetch after the 409: networks are missing. + return okJson({ status: 'ok', stackName: 'web', networks: [{ name: 'ext-net', safe: false }], autoCreateEnabled: false, declaredExternalCount: 1 }); + } + // Post-create verification: everything present. + return okJson({ status: 'ok', stackName: 'web', networks: [{ name: 'ext-net', safe: true }], autoCreateEnabled: true, declaredExternalCount: 1 }); + } + if (url === '/stacks/web/deploy') { + deployCalls.push(url); + if (deployCalls.length === 1) { + // The original deploy hits the reactive 409. + return new Response(JSON.stringify({ code: 'missing_external_networks' }), { status: 409 }); + } + return new Response(null, { status: 200 }); + } + if (url === '/system/networks' && init?.method === 'POST') { + return networkCreateGate; + } + return new Response(null, { status: 200 }); + }); + const { result, overlayState, stackListState, rerender, activeNodeHolder } = setup({ + hasGuidedExternalNetworkPreflight: true, + }); + await act(async () => { + await result.current.deployStack(); + }); + // The reactive 409 opened the dialog (second missing-networks call). + expect(missingNetworksCalls).toBe(2); + const dialog = ( + vi.mocked(overlayState.setMissingExternalNetworks).mock.calls[0][0] as unknown as { + createAndContinue: () => Promise; + } + ); + // Start create-and-continue; the network creation is pending when the + // operator switches to node B and node B finishes hydrating. + let createPromise: Promise | undefined; + await act(async () => { + createPromise = dialog.createAndContinue(); + await new Promise((r) => setTimeout(r, 0)); + }); + activeNodeHolder.current = { id: 2, type: 'remote' } as ActiveNode; + rerender(); + vi.mocked(stackListState.hydrationReady).mockReturnValue(true); + await act(async () => { + resolveNetworkCreate!(new Response(null, { status: 201 })); + await createPromise; + }); + // The create succeeded and reached the guarded retry callback: the retry + // (a second deploy POST) must NOT start. The single deploy call is the + // original attempt that hit the 409. + expect(deployCalls).toHaveLength(1); + }); +}); + diff --git a/frontend/src/components/EditorLayout/hooks/useStackActions.ts b/frontend/src/components/EditorLayout/hooks/useStackActions.ts index 7f0ab9df..19feaf6c 100644 --- a/frontend/src/components/EditorLayout/hooks/useStackActions.ts +++ b/frontend/src/components/EditorLayout/hooks/useStackActions.ts @@ -537,6 +537,17 @@ export function useStackActions(options: UseStackActionsOptions) { const isEnvDirty = () => editorState.envContent !== editorState.originalEnvContent; const getStackMenuVisibility = (file: string) => { + // Without authoritative status evidence every lifecycle action fails closed: + // undefined status must not read as "exited but deployable". + if (!hydrationReady()) { + return { + showDeploy: false, + showStop: false, + showRestart: false, + showUpdate: false, + showTakeDown: false, + }; + } // A partial stack has running containers, so it shows the running-stack // lifecycle actions (stop/restart/update) rather than deploy. const raw = stackListState.stackStatuses[file]; @@ -554,13 +565,34 @@ export function useStackActions(options: UseStackActionsOptions) { const isSelfStackFile = (file: string | null | undefined): boolean => !!file && stackListState.stackSelfFlags[file] === true; + // Ref-backed readiness: evaluates the CURRENT node/list/evidence at call + // time, so a dialog opened while ready cannot bypass a later readiness loss. + const hydrationReady = stackListState.hydrationReady; + + // Executor-boundary readiness: the evidence must be authoritative for the + // active node AND that node must still be the node the deferred operation + // was captured for. A dialog opened on node A cannot dispatch after the + // operator switched to node B, even once B finishes hydrating. Compares + // against the existing effect-updated activeNodeIdRef (see above): by the + // time a dialog is confirmed, the switch effect has long run. + const hydrationReadyForNode = (opNodeId: number | null): boolean => { + if (!hydrationReady()) return false; + if (opNodeId !== (activeNodeIdRef.current ?? null)) return false; + return true; + }; + const openSelfStackProtectedIfNeeded = (file: string | null | undefined): boolean => { + // Without authoritative self identity the frontend must not guess: block + // without opening the self-stack modal (which would falsely identify an + // ordinary stack as Sencho). The backend 409 remains the last line. + if (!hydrationReady()) return true; if (!isSelfStackFile(file)) return false; overlayState.openSelfStackProtected(); return true; }; const openStackApp = (file: string) => { + if (!hydrationReady()) return; const port = stackListState.stackPorts[file]; if (!port) return; const url = buildServiceUrl({ node: activeNode, publicPort: port }); @@ -1258,6 +1290,15 @@ export function useStackActions(options: UseStackActionsOptions) { async function createAndContinue(): Promise { if (settled) return; + // Final boundary recheck before ANY mutation: creating Docker networks + // on the captured node must not proceed after ownership was lost. + if (!hydrationReadyForNode(opNodeId)) { + settled = true; + overlayState.setMissingExternalNetworks(null); + deployPendingRef.current = false; + toast.error('Status data unavailable. Refresh and try again.'); + return; + } setCreating(true); const created = await createSafeExternalNetworks(payload.networks, opNodeId); @@ -1379,6 +1420,13 @@ export function useStackActions(options: UseStackActionsOptions) { throw parseStackActionError(await retry.text(), 'Deploy failed', retry.status); } openMissingExternalNetworksDialog(envelope, opNodeId ?? null, () => { + // Final boundary recheck: the reactive retry must not start a + // deploy on the captured node after ownership was lost. + if (!hydrationReadyForNode(opNodeId ?? null)) { + deployPendingRef.current = false; + toast.error('Status data unavailable. Refresh and try again.'); + return; + } stackListState.setStackAction(stackFile, 'deploy'); void runWithLog({ stackName, action: 'deploy', nodeId: opNodeId ?? null }, (startedRetry, ds) => runDeploy(stackName, stackFile, ignorePolicy, startedRetry, ds, opNodeId), @@ -1414,6 +1462,7 @@ export function useStackActions(options: UseStackActionsOptions) { const deployStack = async (e?: React.MouseEvent) => { e?.preventDefault(); e?.stopPropagation(); + if (!hydrationReady()) return; if ( !stackListState.selectedFile || stackListState.isStackBusy(stackListState.selectedFile) || @@ -1446,6 +1495,15 @@ export function useStackActions(options: UseStackActionsOptions) { // it without duplicating the action lifecycle. The stack action is set here // (not before the advisory) so cancelling the advisory leaves no stuck state. const runDeployFlow = async () => { + // Final boundary recheck: an advisory or external-network dialog opened + // while ready must not dispatch after readiness was lost or the active + // node changed (the operation would target the captured node). Release + // the pending guard so the UI is not stuck when blocked. + if (!hydrationReadyForNode(opNodeId)) { + deployPendingRef.current = false; + toast.error('Status data unavailable. Refresh and try again.'); + return; + } stackListState.setStackAction(stackFile, 'deploy'); let deferredNetworks = false; try { @@ -1518,6 +1576,9 @@ export function useStackActions(options: UseStackActionsOptions) { }; const handleSaveAndDeploy = async (e: React.MouseEvent) => { + // Readiness is rechecked before the PUT so a readiness loss while the file + // was being edited cannot slip a mutation through. + if (!hydrationReady()) return; const saved = await saveFile(); if (!saved) return; await deployStack(e); @@ -1530,9 +1591,14 @@ export function useStackActions(options: UseStackActionsOptions) { const bypassPolicyAndRetry = async () => { const policyBlock = overlayState.policyBlock; if (!policyBlock) return; - // Retry on the node the block was raised against, not the live active node, - // which may have changed while the dialog was open. + // The block is bound to the node it was raised against; the bypass may only + // dispatch if that node is still the active node with authoritative + // evidence. A switch to another fully-hydrated node must block the retry. const { stackName, stackFile, action, nodeId: opNodeId } = policyBlock; + if (!hydrationReadyForNode(opNodeId)) { + toast.error('Status data unavailable. Refresh and try again.'); + return; + } const existingFile = stackListState.files.includes(stackFile) ? stackFile : (stackListState.files.find(f => f.replace(/\.(yml|yaml)$/, '') === stackName) ?? stackFile); @@ -1559,6 +1625,7 @@ export function useStackActions(options: UseStackActionsOptions) { }; const rollbackStack = async (ignorePolicy = false, opNodeId: number | null = activeNode?.id ?? null) => { + if (!hydrationReady()) return; if (!stackListState.selectedFile || stackListState.isStackBusy(stackListState.selectedFile)) return; const stackFile = stackListState.selectedFile; @@ -1783,6 +1850,7 @@ export function useStackActions(options: UseStackActionsOptions) { const stopStack = async (e?: React.MouseEvent) => { e?.preventDefault(); e?.stopPropagation(); + if (!hydrationReady()) return; if (!stackListState.selectedFile) return; if (openSelfStackProtectedIfNeeded(stackListState.selectedFile)) return; await runStackAction(stackListState.selectedFile, 'stop', 'stop', 'exited', 'Stack stopped successfully!'); @@ -1791,6 +1859,7 @@ export function useStackActions(options: UseStackActionsOptions) { const restartStack = async (e?: React.MouseEvent) => { e?.preventDefault(); e?.stopPropagation(); + if (!hydrationReady()) return; if (!stackListState.selectedFile) return; await runStackAction(stackListState.selectedFile, 'restart', 'restart', 'running', 'Stack restarted successfully!'); }; @@ -1799,6 +1868,7 @@ export function useStackActions(options: UseStackActionsOptions) { action: 'start' | 'stop' | 'restart', serviceName: string, ) => { + if (!hydrationReady()) return; if (!stackListState.selectedFile) return; if (action === 'stop' && openSelfStackProtectedIfNeeded(stackListState.selectedFile)) return; const stackName = stackListState.selectedFile.replace(/\.(yml|yaml)$/, ''); @@ -1838,7 +1908,15 @@ export function useStackActions(options: UseStackActionsOptions) { // Capture the node now so the readiness fetch and the update both target it // even if the active node changes while the readiness dialog is open. const opNodeId = activeNode?.id ?? null; - const run = () => runStackAction(stackFile, 'update', 'update', 'running', 'Stack updated successfully!', false, opNodeId); + const run = () => { + // Final boundary recheck: the readiness dialog's proceed must not dispatch + // to the captured node after readiness was lost or the active node changed. + if (!hydrationReadyForNode(opNodeId)) { + toast.error('Status data unavailable. Refresh and try again.'); + return Promise.resolve(); + } + return runStackAction(stackFile, 'update', 'update', 'running', 'Stack updated successfully!', false, opNodeId); + }; if (hasUpdateGuard) { overlayState.setUpdateReadiness({ stackName, @@ -1863,10 +1941,17 @@ export function useStackActions(options: UseStackActionsOptions) { serviceName: string, mode: 'update' | 'rebuild' = 'update', ): Promise => { + if (!hydrationReady()) return; if (stackListState.isStackBusy(stackFile) || editorState.serviceUpdateInProgress) return; const stackName = stackFile.replace(/\.(yml|yaml)$/, ''); const opNodeId = activeNode?.id ?? null; const run = async () => { + // Final boundary recheck: the readiness dialog's proceed must not dispatch + // to the captured node after readiness was lost or the active node changed. + if (!hydrationReadyForNode(opNodeId)) { + toast.error('Status data unavailable. Refresh and try again.'); + return; + } editorState.setServiceUpdateInProgress({ service: serviceName, mode }); try { await runWithLog({ stackName, action: 'update', nodeId: opNodeId, serviceName }, async (started, ds) => { @@ -1925,6 +2010,7 @@ export function useStackActions(options: UseStackActionsOptions) { serviceName: string, recoveryId: string, ): Promise => { + if (!hydrationReady()) return; if (stackListState.isStackBusy(stackFile) || editorState.serviceUpdateInProgress) return; const stackName = stackFile.replace(/\.(yml|yaml)$/, ''); const opNodeId = activeNode?.id ?? null; @@ -1966,18 +2052,28 @@ export function useStackActions(options: UseStackActionsOptions) { const updateStack = async (e?: React.MouseEvent) => { e?.preventDefault(); e?.stopPropagation(); + if (!hydrationReady()) return; if (!stackListState.selectedFile) return; if (openSelfStackProtectedIfNeeded(stackListState.selectedFile)) return; await requestStackUpdate(stackListState.selectedFile); }; const deleteStack = async (pruneVolumes: boolean) => { - const stackToDelete = overlayState.stackToDelete; - if (!stackToDelete) return; + // Final confirmation boundary: the dialog is bound to the node it opened + // on. A switch to another node (even once it fully hydrates) must block the + // delete rather than mutating the new node with the old dialog's stack name. + // The dialog stays open and the toast explains why nothing happened. + const deleteTarget = overlayState.deleteTarget; + if (!deleteTarget) return; + if (!hydrationReadyForNode(deleteTarget.nodeId)) { + toast.error('Status data unavailable. Refresh and try again.'); + return; + } + const stackToDelete = deleteTarget.name; const deleteKey = resolveStackFileKey(stackListState.files, stackToDelete); const canonicalName = deleteKey.replace(/\.(yml|yaml)$/, ''); if (stackListState.isStackBusy(deleteKey)) return; - const opNodeId = activeNode?.id ?? null; + const opNodeId = deleteTarget.nodeId; stackListState.setStackAction(deleteKey, 'delete'); try { const url = pruneVolumes @@ -2020,15 +2116,23 @@ export function useStackActions(options: UseStackActionsOptions) { }; const requestTakeDownStack = (stackName: string) => { + if (!hydrationReady()) return; if (openSelfStackProtectedIfNeeded( stackListState.files.find(f => f.replace(/\.(yml|yaml)$/, '') === stackName) ?? stackName, )) return; - overlayState.openTakeDownDialog(stackName); + overlayState.openTakeDownDialog({ name: stackName, nodeId: activeNode?.id ?? null }); }; const takeDownStack = async (removeVolumes: boolean) => { - const stackToTakeDown = overlayState.stackToTakeDown; - if (!stackToTakeDown) return; + // Final confirmation boundary: the dialog is bound to the node it opened + // on; a switch must block the take down rather than mutating the new node. + const takeDownTarget = overlayState.takeDownTarget; + if (!takeDownTarget) return; + if (!hydrationReadyForNode(takeDownTarget.nodeId)) { + toast.error('Status data unavailable. Refresh and try again.'); + return; + } + const stackToTakeDown = takeDownTarget.name; if (removeVolumes && !canOfferVolumeRemoval) { toast.error('Volume removal is not supported on this node'); overlayState.closeTakeDownDialog(); @@ -2184,7 +2288,10 @@ export function useStackActions(options: UseStackActionsOptions) { const requestDeleteStack = () => { if (openSelfStackProtectedIfNeeded(stackListState.selectedFile)) return; - overlayState.openDeleteDialog(stackListState.selectedFile ?? ''); + overlayState.openDeleteDialog({ + name: stackListState.selectedFile ?? '', + nodeId: activeNode?.id ?? null, + }); }; const executeStackActionByFile = async ( @@ -2192,6 +2299,7 @@ export function useStackActions(options: UseStackActionsOptions) { action: StackAction, endpoint: string, ) => { + if (!hydrationReady()) return; if (stackListState.isStackBusy(stackFile)) return; if ( (action === 'deploy' || action === 'update' || action === 'stop' || action === 'delete' || action === 'rollback') && diff --git a/frontend/src/components/EditorLayout/hooks/useStackListState.test.ts b/frontend/src/components/EditorLayout/hooks/useStackListState.test.ts index aebc4da1..b838ae0b 100644 --- a/frontend/src/components/EditorLayout/hooks/useStackListState.test.ts +++ b/frontend/src/components/EditorLayout/hooks/useStackListState.test.ts @@ -198,3 +198,531 @@ describe('useStackListState Updates chip confirmed-only', () => { expect(result.current.chipFilteredFiles).toEqual(['ok.yml']); }); }); + +describe('useStackListState.hydration concurrency and evidence', () => { + it('starts the statuses request before the list resolves (concurrent dispatch)', async () => { + let resolveList: (r: Response) => void; + const listGate = new Promise((r) => { resolveList = r; }); + const statusCalls: string[] = []; + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return listGate; + if (endpoint === '/stacks/statuses') { statusCalls.push(endpoint); return Promise.resolve(okJson({})); } + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + let p: Promise | undefined; + await act(async () => { + p = result.current.refreshStacks(); + await new Promise((r) => setTimeout(r, 0)); + }); + expect(statusCalls).toContain('/stacks/statuses'); + await act(async () => { + resolveList!(okJson(['web.yml'])); + await p; + }); + expect(result.current.files).toEqual(['web.yml']); + }); + + it('clears isLoading at list commit while the status request is still pending', async () => { + let resolveStatus: (r: Response) => void; + const statusGate = new Promise((r) => { resolveStatus = r; }); + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['web.yml'])); + if (endpoint === '/stacks/statuses') return statusGate; + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + await act(async () => { + const p = result.current.refreshStacks(); + await new Promise((r) => setTimeout(r, 0)); + expect(result.current.isLoading).toBe(false); + expect(result.current.hydrationStatus).toBe('pending'); + resolveStatus!(okJson({ 'web.yml': { status: 'running' } })); + await p; + }); + expect(result.current.hydrationStatus).toBe('ok'); + expect(result.current.actionsReady).toBe(true); + }); + + it('holds a status-first result provisional until the list validates', async () => { + let resolveList: (r: Response) => void; + const listGate = new Promise((r) => { resolveList = r; }); + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return listGate; + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'web.yml': { status: 'running' } })); + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + let p: Promise | undefined; + await act(async () => { + p = result.current.refreshStacks(); + await new Promise((r) => setTimeout(r, 0)); + expect(result.current.hydrationStatus).toBe('pending'); + }); + await act(async () => { + resolveList!(okJson(['web.yml'])); + await p; + }); + expect(result.current.hydrationStatus).toBe('ok'); + }); + + it('transfers loading ownership to a superseding background refresh', async () => { + let resolveList1: (r: Response) => void; + const gate1 = new Promise((r) => { resolveList1 = r; }); + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return gate1; + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'web.yml': { status: 'running' } })); + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + let fg: Promise | undefined; + await act(async () => { + fg = result.current.refreshStacks(); + await new Promise((r) => setTimeout(r, 0)); + }); + expect(result.current.isLoading).toBe(true); + + let resolveList2: (r: Response) => void; + const gate2 = new Promise((r) => { resolveList2 = r; }); + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return gate2; + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'web.yml': { status: 'running' } })); + return Promise.resolve(notFound()); + }); + let bg: Promise | undefined; + await act(async () => { + bg = result.current.refreshStacks(true); + await new Promise((r) => setTimeout(r, 0)); + }); + await act(async () => { + resolveList1!(okJson(['web.yml'])); + await fg; + }); + expect(result.current.isLoading).toBe(true); + await act(async () => { + resolveList2!(okJson(['web.yml'])); + await bg; + }); + expect(result.current.isLoading).toBe(false); + }); + + it('never commits a completed status result when the list fails', async () => { + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(new Response('fail', { status: 500 })); + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'web.yml': { status: 'running' } })); + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.stacksLoadStatus).toBe('error'); + expect(result.current.hydrationStatus).toBe('pending'); + expect(result.current.actionsReady).toBe(false); + }); + + it('keeps the list visible with error evidence when the status promise rejects', async () => { + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['web.yml'])); + if (endpoint === '/stacks/statuses') return Promise.reject(new Error('network down')); + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.stacksLoadStatus).toBe('success'); + expect(result.current.files).toEqual(['web.yml']); + expect(result.current.hydrationStatus).toBe('error'); + expect(result.current.actionsReady).toBe(false); + }); + + it('blocks readiness when the status payload misses a list file even with extra keys', async () => { + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['web.yml', 'db.yml'])); + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ + 'web.yml': { status: 'running' }, + 'unrelated.yml': { status: 'exited' }, + })); + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.hydrationStatus).toBe('ok'); + expect(result.current.actionsReady).toBe(false); + expect(result.current.hydrationDisplay).toBe('incomplete'); + }); + + it('treats malformed status payloads as errors without per-stack fallback', async () => { + const calls: string[] = []; + apiFetchMock.mockImplementation((endpoint: string) => { + calls.push(endpoint); + if (endpoint === '/stacks') return Promise.resolve(okJson(['web.yml'])); + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'web.yml': { status: 'bogus' } })); + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.hydrationStatus).toBe('error'); + expect(result.current.actionsReady).toBe(false); + expect(calls.filter(c => c.includes('/containers'))).toEqual([]); + }); + + it('does not treat arbitrary string maps as legacy status payloads', async () => { + const calls: string[] = []; + apiFetchMock.mockImplementation((endpoint: string) => { + calls.push(endpoint); + if (endpoint === '/stacks') return Promise.resolve(okJson(['web.yml'])); + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ error: 'failed' })); + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.hydrationStatus).toBe('error'); + expect(calls.filter(c => c.includes('/containers'))).toEqual([]); + }); + + it('re-derives statuses from per-stack containers for a valid legacy payload', async () => { + const calls: string[] = []; + apiFetchMock.mockImplementation((endpoint: string) => { + calls.push(endpoint); + if (endpoint === '/stacks') return Promise.resolve(okJson(['web.yml'])); + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'web.yml': 'running' })); + if (endpoint === '/stacks/web.yml/containers') return Promise.resolve(okJson([{ State: 'running', Status: 'Up 2 hours' }])); + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(calls).toContain('/stacks/web.yml/containers'); + expect(result.current.hydrationStatus).toBe('ok'); + expect(result.current.actionsReady).toBe(true); + expect(result.current.stackStatuses['web.yml']).toBe('running'); + }); + + it('does not count failed per-stack derivations as coverage', async () => { + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['web.yml', 'db.yml'])); + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'web.yml': 'running' })); + if (endpoint === '/stacks/web.yml/containers') return Promise.resolve(okJson([{ State: 'running', Status: 'Up 2 hours' }])); + if (endpoint === '/stacks/db.yml/containers') return Promise.resolve(new Response('boom', { status: 500 })); + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.hydrationStatus).toBe('ok'); + expect(result.current.actionsReady).toBe(false); + expect(result.current.hydrationDisplay).toBe('incomplete'); + }); + + it('restores prior evidence as stale when a same-list foreground status fetch fails', async () => { + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['web.yml'])); + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'web.yml': { status: 'running' } })); + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.actionsReady).toBe(true); + + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['web.yml'])); + if (endpoint === '/stacks/statuses') return Promise.resolve(new Response('boom', { status: 500 })); + return Promise.resolve(notFound()); + }); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.hydrationStatus).toBe('ok'); + expect(result.current.actionsReady).toBe(false); + expect(result.current.hydrationDisplay).toBe('stale'); + }); + + it('marks prior evidence stale on a same-list background failure', async () => { + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['web.yml'])); + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'web.yml': { status: 'running' } })); + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.actionsReady).toBe(true); + + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['web.yml'])); + if (endpoint === '/stacks/statuses') return Promise.resolve(new Response('boom', { status: 500 })); + return Promise.resolve(notFound()); + }); + await act(async () => { + await result.current.refreshStacks(true); + }); + expect(result.current.hydrationDisplay).toBe('stale'); + expect(result.current.actionsReady).toBe(false); + }); + + it('establishes ok evidence with zero coverage on an empty list', async () => { + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson([])); + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({})); + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.stacksLoadStatus).toBe('success'); + expect(result.current.hydrationStatus).toBe('ok'); + expect(result.current.actionsReady).toBe(true); + }); + + it('fails closed on the first render after a node switch', async () => { + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['web.yml'])); + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'web.yml': { status: 'running' } })); + return Promise.resolve(notFound()); + }); + const { result, rerender } = renderHook(() => useStackListState()); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.actionsReady).toBe(true); + + useNodesMock.mockReturnValue({ + activeNode: { id: 2, name: 'Other', type: 'remote' }, + nodes: [{ id: 2, name: 'Other', type: 'remote' }], + }); + rerender(); + expect(result.current.actionsReady).toBe(false); + expect(result.current.hydrationDisplay).toBe('pending'); + }); + + it('zeroes Up/Down filter counts while hydration is pending and restores them after', async () => { + let resolveStatus: (r: Response) => void; + const statusGate = new Promise((r) => { resolveStatus = r; }); + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['web.yml'])); + if (endpoint === '/stacks/statuses') return statusGate; + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + await act(async () => { + const p = result.current.refreshStacks(); + await new Promise((r) => setTimeout(r, 0)); + expect(result.current.filterCounts.up).toBe(0); + expect(result.current.filterCounts.down).toBe(0); + resolveStatus!(okJson({ 'web.yml': { status: 'running' } })); + await p; + }); + expect(result.current.filterCounts.up).toBe(1); + act(() => result.current.setFilterChip('up')); + expect(result.current.chipFilteredFiles).toEqual(['web.yml']); + }); +}); + +describe('useStackListState delayed prior-node arbitration', () => { + it('never lets a delayed prior-node list response replace the current node state', async () => { + let resolveListA: (r: Response) => void; + const gateA = new Promise((r) => { resolveListA = r; }); + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return gateA; + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'a.yml': { status: 'running' } })); + return Promise.resolve(notFound()); + }); + const { result, rerender } = renderHook(() => useStackListState()); + let fgA: Promise | undefined; + await act(async () => { + fgA = result.current.refreshStacks(); + await new Promise((r) => setTimeout(r, 0)); + }); + + // Switch to node B and complete a full foreground refresh. + useNodesMock.mockReturnValue({ + activeNode: { id: 2, name: 'B', type: 'remote' }, + nodes: [{ id: 2, name: 'B', type: 'remote' }], + }); + rerender(); + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['b.yml'])); + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'b.yml': { status: 'running' } })); + return Promise.resolve(notFound()); + }); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.files).toEqual(['b.yml']); + expect(result.current.filesNodeId).toBe(2); + expect(result.current.hydrationStatus).toBe('ok'); + expect(result.current.actionsReady).toBe(true); + + // Node A's delayed list finally resolves: it must not overwrite B. + await act(async () => { + resolveListA!(okJson(['a.yml'])); + await fgA; + }); + expect(result.current.files).toEqual(['b.yml']); + expect(result.current.filesNodeId).toBe(2); + expect(result.current.hydrationStatus).toBe('ok'); + expect(result.current.actionsReady).toBe(true); + expect(result.current.isLoading).toBe(false); + }); + + it('never lets a delayed prior-node status response replace the current node state', async () => { + let resolveStatusA: (r: Response) => void; + const gateStatusA = new Promise((r) => { resolveStatusA = r; }); + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['a.yml'])); + if (endpoint === '/stacks/statuses') return gateStatusA; + return Promise.resolve(notFound()); + }); + const { result, rerender } = renderHook(() => useStackListState()); + let fgA: Promise | undefined; + await act(async () => { + fgA = result.current.refreshStacks(); + await new Promise((r) => setTimeout(r, 0)); + }); + + // Node B completes fully while A's statuses are still pending. + useNodesMock.mockReturnValue({ + activeNode: { id: 2, name: 'B', type: 'remote' }, + nodes: [{ id: 2, name: 'B', type: 'remote' }], + }); + rerender(); + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['b.yml'])); + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'b.yml': { status: 'running' } })); + return Promise.resolve(notFound()); + }); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.files).toEqual(['b.yml']); + expect(result.current.hydrationStatus).toBe('ok'); + + // A's delayed statuses resolve: the stale check must discard them. + await act(async () => { + resolveStatusA!(okJson({ 'a.yml': { status: 'exited' } })); + await fgA; + }); + expect(result.current.files).toEqual(['b.yml']); + expect(result.current.filesNodeId).toBe(2); + expect(result.current.hydrationStatus).toBe('ok'); + expect(result.current.stackStatuses).toEqual({ 'b.yml': 'running' }); + expect(result.current.actionsReady).toBe(true); + }); +}); + +describe('useStackListState legacy fallback payload validation', () => { + it('fails closed when a legacy per-stack response is a 200 non-array body', async () => { + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['web.yml'])); + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'web.yml': 'running' })); + if (endpoint === '/stacks/web.yml/containers') return Promise.resolve(okJson({ error: 'boom' })); + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.hydrationStatus).toBe('error'); + expect(result.current.actionsReady).toBe(false); + }); + + it('fails closed when a legacy per-stack response is a 200 malformed array', async () => { + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['web.yml'])); + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'web.yml': 'running' })); + if (endpoint === '/stacks/web.yml/containers') return Promise.resolve(okJson([{ id: 'no-state-field' }])); + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.hydrationStatus).toBe('error'); + expect(result.current.actionsReady).toBe(false); + }); + + it('keeps partial coverage incomplete when one legacy per-stack response is malformed', async () => { + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['web.yml', 'db.yml'])); + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'web.yml': 'running' })); + if (endpoint === '/stacks/web.yml/containers') return Promise.resolve(okJson([{ State: 'running', Status: 'Up 2 hours' }])); + if (endpoint === '/stacks/db.yml/containers') return Promise.resolve(okJson({ error: 'boom' })); + return Promise.resolve(notFound()); + }); + const { result } = renderHook(() => useStackListState()); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.hydrationStatus).toBe('ok'); + expect(result.current.actionsReady).toBe(false); + expect(result.current.hydrationDisplay).toBe('incomplete'); + }); +}); + +describe('useStackListState stale callback node targeting', () => { + it('refreshes the current node when a callback captured on another node is invoked late', async () => { + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['a.yml'])); + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'a.yml': { status: 'running' } })); + return Promise.resolve(notFound()); + }); + const { result, rerender } = renderHook(() => useStackListState()); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.files).toEqual(['a.yml']); + // The function from the node-A render, captured like an action's finally + // block would capture it. + const capturedRefresh = result.current.refreshStacks; + + // Switch to node B and fully hydrate it. + useNodesMock.mockReturnValue({ + activeNode: { id: 2, name: 'B', type: 'remote' }, + nodes: [{ id: 2, name: 'B', type: 'remote' }], + }); + rerender(); + apiFetchMock.mockImplementation((endpoint: string) => { + if (endpoint === '/stacks') return Promise.resolve(okJson(['b.yml'])); + if (endpoint === '/stacks/statuses') return Promise.resolve(okJson({ 'b.yml': { status: 'running' } })); + return Promise.resolve(notFound()); + }); + await act(async () => { + await result.current.refreshStacks(); + }); + expect(result.current.files).toEqual(['b.yml']); + expect(result.current.hydrationStatus).toBe('ok'); + expect(result.current.actionsReady).toBe(true); + + // Invoke the OLD callback: both requests must target node B (the current + // node), and B's state must remain authoritative. + await act(async () => { + await capturedRefresh(); + }); + const stacksTargets = apiFetchMock.mock.calls + .filter(c => c[0] === '/stacks') + .map(c => (c[1] as { nodeId?: number | null } | undefined)?.nodeId); + const statusTargets = apiFetchMock.mock.calls + .filter(c => c[0] === '/stacks/statuses') + .map(c => (c[1] as { nodeId?: number | null } | undefined)?.nodeId); + expect(stacksTargets.at(-1)).toBe(2); + expect(statusTargets.at(-1)).toBe(2); + expect(result.current.files).toEqual(['b.yml']); + expect(result.current.filesNodeId).toBe(2); + expect(result.current.hydrationStatus).toBe('ok'); + expect(result.current.actionsReady).toBe(true); + }); +}); diff --git a/frontend/src/components/EditorLayout/hooks/useStackListState.ts b/frontend/src/components/EditorLayout/hooks/useStackListState.ts index 2cc8a2d1..707459de 100644 --- a/frontend/src/components/EditorLayout/hooks/useStackListState.ts +++ b/frontend/src/components/EditorLayout/hooks/useStackListState.ts @@ -1,6 +1,6 @@ import { useState, useEffect, useRef, useMemo, useCallback } from 'react'; import { apiFetch } from '@/lib/api'; -import { fetchStackStatusesShared, type StackStatusesFetchResult } from '@/lib/stackStatusesFetch'; +import { fetchStackStatusesShared } from '@/lib/stackStatusesFetch'; import { newAttemptId, abortAttempt, @@ -25,31 +25,72 @@ import { isInputFocused, isPaletteOpen } from '@/lib/keyboard-guards'; import type { StackAction, StackActionResult } from '../EditorView'; import type { Label as StackLabel } from '../../label-types'; import type { FilterChip } from '../../sidebar/sidebar-types'; -import { isDownStatus, classifyContainersStatus, isBulkStatusObjectFormat } from '../../sidebar/stack-status-utils'; +import { isDownStatus, classifyContainersStatus, isContainerStateInfo, isValidBulkPayload, isValidLegacyPayload, parseBulkStatusPayload } from '../../sidebar/stack-status-utils'; import type { StackRowStatus } from '../../sidebar/stack-status-utils'; +/** Result of the legacy per-stack container derivation, with the number of + * stacks whose status comes from a successful, valid container response. + * Network or HTTP failures do not count as coverage, so a partially failing + * fallback cannot authorize actions on the files it could not inspect. */ +interface DerivedStatuses { + statuses: Record; + coveredFileCount: number; +} + /** Compatibility path for remote nodes whose `/stacks/statuses` is absent or * returns the legacy plain-string format: query each stack's containers and - * classify them so a degraded (partial) stack is not reported as healthy. */ + * classify them so a degraded (partial) stack is not reported as healthy. + * Requests target the captured node explicitly so a mid-switch fallback never + * drifts to whatever node is active when the per-stack calls resolve. + * Per-file failures are collected and logged so a total fallback failure is + * diagnosable instead of being indistinguishable from "no statuses yet". */ async function deriveStatusesFromContainers( fileList: string[], -): Promise> { + nodeId: number | null, +): Promise { + const failed: string[] = []; const results = await Promise.allSettled( - fileList.map(async (file) => { - const containersRes = await apiFetch(`/stacks/${file}/containers`); - if (!containersRes.ok) return { file, status: 'unknown' as StackRowStatus }; - const containers = await containersRes.json(); - return { - file, - status: Array.isArray(containers) ? classifyContainersStatus(containers) : 'unknown', - }; + fileList.map(async (file): Promise<{ file: string; status: StackRowStatus; valid: boolean }> => { + let containersRes: Response; + try { + containersRes = await apiFetch(`/stacks/${file}/containers`, { nodeId }); + } catch (err) { + failed.push(`${file} (${err instanceof Error ? err.message : 'network error'})`); + return { file, status: 'unknown', valid: false }; + } + if (!containersRes.ok) { + failed.push(`${file} (HTTP ${containersRes.status})`); + return { file, status: 'unknown', valid: false }; + } + try { + const containers: unknown = await containersRes.json(); + // Only a container ARRAY with well-shaped entries is authoritative + // evidence. A successful 200 carrying an error object or malformed + // entries must fail closed, not count as coverage. + const valid = Array.isArray(containers) && containers.every(isContainerStateInfo); + if (!valid) { + failed.push(`${file} (malformed container payload)`); + return { file, status: 'unknown', valid: false }; + } + return { file, status: classifyContainersStatus(containers), valid: true }; + } catch (err) { + failed.push(`${file} (decode: ${err instanceof Error ? err.message : 'invalid body'})`); + return { file, status: 'unknown', valid: false }; + } }), ); const out: Record = {}; + let coveredFileCount = 0; for (const result of results) { - if (result.status === 'fulfilled') out[result.value.file] = result.value.status; + if (result.status === 'fulfilled') { + out[result.value.file] = result.value.status; + if (result.value.valid) coveredFileCount += 1; + } } - return out; + if (failed.length > 0) { + console.error(`Legacy status derivation failed for ${failed.length}/${fileList.length} stacks:`, failed.join('; ')); + } + return { statuses: out, coveredFileCount }; } interface StackStatus { @@ -60,14 +101,6 @@ interface StackCounts { [key: string]: { running: number; total: number } | undefined; } -interface StackStatusInfo { - status: StackRowStatus; - mainPort?: number; - running?: number; - total?: number; - isSelf?: boolean; -} - export interface RemoteResult { nodeId: number; nodeName: string; @@ -78,6 +111,47 @@ const EMPTY_UPDATES: Record = {}; export type StacksLoadStatus = 'idle' | 'loading' | 'success' | 'error'; +/** The authoritative record of which status evidence the current list holds. + * `null` means pending: no current-node status result has landed for the + * committed list yet. Readiness and display are derived from this record at + * render time (see `hydrationReadyRef`), so a node switch or list change + * fails closed on the very first frame, before any effect runs. + * Discriminated on `outcome`: an error record carries no source or stale + * fields, so "error with stale identity" is unrepresentable. */ +export type HydrationSource = 'bulk' | 'legacy'; + +export type HydrationEvidence = + | { + nodeId: number; + /** Content identity of the committed list: JSON of sorted filenames. */ + listFingerprint: string; + outcome: 'ok'; + /** `bulk` = current object format; `legacy` = per-stack container derivation. */ + source: HydrationSource; + /** True when this is prior evidence preserved through a failed refresh + * and is no longer authoritative. Always false on fresh success. */ + stale: boolean; + /** Number of current-list files covered by a successful, valid status entry. */ + coveredFileCount: number; + } + | { + nodeId: number; + /** Content identity of the committed list: JSON of sorted filenames. */ + listFingerprint: string; + outcome: 'error'; + /** Error evidence never covers any file. */ + coveredFileCount: 0; + }; + +/** Display projection of hydration state, derived once per render. + * - pending: no status evidence for the current list yet + * - error: the status fetch failed terminally + * - stale: prior same-node evidence retained through a failed refresh, or the + * evidence no longer matches the active node / committed list + * - incomplete: evidence covers only part of the current list + * - current: evidence is authoritative for the visible list */ +export type HydrationDisplayState = 'pending' | 'error' | 'current' | 'stale' | 'incomplete'; + export function useStackListState() { const { nodes, activeNode } = useNodes(); @@ -126,6 +200,61 @@ export function useStackListState() { const [stacksLoadNodeId, setStacksLoadNodeId] = useState(null); const hadSuccessfulListRef = useRef(false); + // Hydration evidence: the single source of truth for whether the visible list + // has authoritative status data. The ref mirrors the state synchronously so + // readiness checks in async handlers read current values, and so the first + // render after a transition is already fail-closed (no effect needed). + const [hydrationEvidence, setHydrationEvidence] = useState(null); + const evidenceRef = useRef(null); + // Render-synchronous refs (updated inline during render, matching the + // NodeContext pattern) plus commit-synchronous fingerprint/count refs, so + // `hydrationReadyRef` always evaluates the CURRENT node/list/evidence. + const activeNodeIdRef = useRef(activeNode?.id ?? null); + activeNodeIdRef.current = activeNode?.id ?? null; + const currentFingerprintRef = useRef(''); + const currentFileCountRef = useRef(0); + // The attempt id that owns the sidebar loading state. Background refreshes + // take over an in-flight foreground owner so the skeleton dissolves when the + // current attempt's list lands, never when a stale attempt finishes. + const loadingOwnerRef = useRef(null); + + const setHydrationEvidenceSync = (next: HydrationEvidence | null): void => { + evidenceRef.current = next; + setHydrationEvidence(next); + }; + + // Readiness is a function of the evidence record against the current node and + // list, evaluated at call time (ref-backed), never captured in a closure. + const hydrationReadyRef = useRef<() => boolean>(() => false); + // Render-synchronous mirror so the predicate also fails when the list itself + // errored: stale bulk selection must not dispatch against a failed-to-load + // list even though the evidence record still matches the last list. + const stacksLoadStatusRef = useRef('idle'); + stacksLoadStatusRef.current = stacksLoadStatus; + hydrationReadyRef.current = () => { + if (stacksLoadStatusRef.current === 'error') return false; + const e = evidenceRef.current; + if (!e || e.outcome !== 'ok' || e.stale) return false; + if (e.nodeId !== activeNodeIdRef.current) return false; + if (e.listFingerprint !== currentFingerprintRef.current) return false; + if (e.coveredFileCount !== currentFileCountRef.current) return false; + return true; + }; + + const hydrationStatus: 'pending' | 'ok' | 'error' = hydrationEvidence?.outcome ?? 'pending'; + const actionsReady = hydrationReadyRef.current(); + const hydrationDisplay: HydrationDisplayState = (() => { + const e = hydrationEvidence; + if (!e) return 'pending'; + if (e.outcome === 'error') return 'error'; + // A node mismatch means the evidence (and the maps) belong to a different + // node: show pending, never the prior node's data as current or stale. + if (e.nodeId !== activeNodeIdRef.current) return 'pending'; + if (e.stale || e.listFingerprint !== currentFingerprintRef.current) return 'stale'; + if (e.coveredFileCount !== currentFileCountRef.current) return 'incomplete'; + return 'current'; + })(); + const { stackUpdates, refresh: fetchImageUpdates, sidebarIndicators } = useImageUpdates(activeNode?.id); const sidebarStackUpdates = sidebarIndicators ? stackUpdates : EMPTY_UPDATES; const { pinned, pin, unpin, isPinned, evictedOldest } = usePinnedStacks(activeNode?.id); @@ -146,6 +275,15 @@ export function useStackListState() { hadSuccessfulListRef.current = false; setStacksLoadStatus('idle'); setStacksLoadError(null); + // Cross-node data must never render under a repeated filename: drop the + // prior node's status-derived maps and evidence. Render-time derivation + // (nodeId checks in hydrationDisplay/actionsReady) already fails closed on + // the first frame; this effect is the cleanup, not the guard. + setStackStatuses({}); + setStackPorts({}); + setStackSelfFlags({}); + setStackCounts({}); + setHydrationEvidenceSync(null); }, [activeNode?.id]); // Ref is updated synchronously alongside the state setter so any code that @@ -211,7 +349,11 @@ export function useStackListState() { }, [refreshLabels]); const refreshStacks = async (background = false): Promise => { - const fetchNodeId = activeNode?.id ?? null; + // Read the CURRENT active node from the render-synchronous ref, not the + // closure: a callback captured on node A and invoked after the operator + // switched to node B must refresh B, never mix A's statuses with a live + // list request. + const fetchNodeId = activeNodeIdRef.current; const mySeq = ++fetchSeqRef.current; const stale = () => fetchSeqRef.current !== mySeq; @@ -223,13 +365,30 @@ export function useStackListState() { const attemptId = newAttemptId(); listAttemptRef.current = attemptId; - // True once the list itself is committed, so the shared catch below can tell - // a list-fetch failure (nothing visible) from a status-path failure (list is - // visible, hydration errored). + // True once the list itself is committed, so the status-failure paths can + // tell a list-fetch failure (nothing visible) from a status-path failure + // (list is visible, hydration errored). let listSucceeded = false; let proxied = false; - if (!background) setIsLoading(true); + // List-loading ownership: the foreground attempt that set the skeleton owns + // clearing it, at the moment its list commits (progressive visibility). A + // background refresh that supersedes an in-flight foreground load takes over + // the owner so the displaced foreground can never clear loading for a newer + // attempt, and the skeleton dissolves when the current attempt's list lands. + if (!background) { + loadingOwnerRef.current = attemptId; + setIsLoading(true); + } else if (loadingOwnerRef.current !== null) { + loadingOwnerRef.current = attemptId; + } + const settleLoading = () => { + if (loadingOwnerRef.current === attemptId) { + loadingOwnerRef.current = null; + setIsLoading(false); + } + }; + setStacksLoadNodeId(fetchNodeId); if (!background || !hadSuccessfulListRef.current) { setStacksLoadStatus('loading'); @@ -239,8 +398,8 @@ export function useStackListState() { // Tracks the most recently committed list for this attempt: `files` (the // render-time closure) is stale once the list itself has just succeeded // within this same call, e.g. the list decodes fine but the follow-up - // /stacks/statuses decode then throws. Seeded from `files` so a failure - // that happens before the list ever loads still consults the prior state. + // status path then throws. Seeded from `files` so a failure that happens + // before the list ever loads still consults the prior state. let latestFileList = files; // Soft (background) failure keeps a non-empty list visible, matching the @@ -260,10 +419,77 @@ export function useStackListState() { return []; }; + // --- Concurrent dispatch ------------------------------------------------- + // The list and status endpoints return independent data (each has its own + // keyset), so both requests start together. The list is still consumed and + // committed first, keeping list_visible progressive; the status outcome is + // observed at creation (never an unhandled rejection) and consumed after + // the list commits. + const stacksPromise = apiFetch('/stacks', { nodeId: fetchNodeId }); + const statusPromise = fetchNodeId === null ? null : (() => { + const statusSpan = beginSpan('fetch_headers', { attemptId, background }); + return fetchStackStatusesShared(fetchNodeId).then( + (result) => { + // Joined waiters mark network spans superseded so truncated + // join timings are not mistaken for fast fetches. + endSpan(statusSpan, { + outcome: result.coalesced ? 'superseded' : undefined, + proxied: result.proxied, + detail: { status: result.status, coalesced: result.coalesced }, + }); + return { + ok: result.ok, + status: result.status, + body: result.body, + proxied: result.proxied, + coalesced: result.coalesced, + error: null, + }; + }, + (statusErr) => { + endSpan(statusSpan, { outcome: 'error', detail: { coalesced: false } }); + return { + ok: false, + status: 0, + body: null, + proxied: false, + coalesced: false, + error: statusErr instanceof Error ? statusErr : new Error(String(statusErr)), + }; + }, + ); + })(); + const headersSpan = beginSpan('fetch_headers', { attemptId, background }); let bodySpan: SpanHandle | null = null; + // Evidence captured before this refresh, used to restore prior status data + // (marked stale) when a same-list foreground refresh's status fetch fails. + const priorEvidence = evidenceRef.current; + // 0 matches no real node, so an error record can never accidentally + // authorize one; the null-node path returns before any evidence is written. + const evidenceNodeId: number = fetchNodeId ?? 0; + + // Shared error path: record error evidence for the current list and drop + // the status maps so nothing renders as current. Used by the hard-error + // branch of failHydration and by the catch path. + const clearStatusMaps = (): void => { + setStackStatuses({}); + setStackPorts({}); + setStackSelfFlags({}); + setStackCounts({}); + }; + const recordHydrationError = (forFingerprint: string): void => { + setHydrationEvidenceSync({ + nodeId: evidenceNodeId, + listFingerprint: forFingerprint, + outcome: 'error', + coveredFileCount: 0, + }); + clearStatusMaps(); + }; + try { - const res = await apiFetch('/stacks'); + const res = await stacksPromise; proxied = res.headers.get('x-sencho-proxy') === '1'; endSpan(headersSpan, { proxied, detail: { status: res.status } }); if (stale()) { abortAttempt(attemptId); return []; } @@ -274,6 +500,7 @@ export function useStackListState() { const data = await res.json(); endSpan(bodySpan); bodySpan = null; + if (stale()) { abortAttempt(attemptId); return []; } if (!Array.isArray(data)) { return applyStacksFailure('Stack list response was invalid.'); } @@ -287,6 +514,12 @@ export function useStackListState() { setStacksLoadError(null); endSpan(listDispatch); listSucceeded = true; + // Commit-synchronous fingerprint refs: readiness derives from these, so a + // stale list can never carry an old list's evidence. + const fingerprint = JSON.stringify([...fileList].sort()); + currentFingerprintRef.current = fingerprint; + currentFileCountRef.current = fileList.length; + settleLoading(); // Token folds node + count so an empty->empty commit still fires once per // attempt even when the committed `files` is referentially equal. const listToken = `${fetchNodeId}:${fileList.length}`; @@ -294,57 +527,105 @@ export function useStackListState() { listVisiblePendingRef.current = { attemptId, token: listToken, proxied }; } - // Fetch all stack statuses in a single bulk call. Only the current object - // format can express `partial`; a node lacking the endpoint or returning - // the legacy plain-string format is re-derived from per-stack containers - // so a crashed container is not hidden behind a healthy sibling. // Skip statuses until activeNode resolves (null here means unresolved, not - // "local"); never pass an unknown target into the shared fetch. - if (fetchNodeId === null) { + // "local"); never pass an unknown target into the shared fetch. Guarding + // on statusPromise directly also narrows it for the await below. + if (statusPromise === null) { return fileList; } - const statusHeaders = beginSpan('fetch_headers', { attemptId, background, proxied }); - let statusResult: StackStatusesFetchResult; - try { - statusResult = await fetchStackStatusesShared(fetchNodeId); - } catch (statusErr) { - endSpan(statusHeaders, { outcome: 'error', detail: { coalesced: false } }); - throw statusErr; + // A foreground refresh blanks the evidence until its own status lands, so + // the new list cannot be authorized by a prior list's statuses. Background + // refreshes keep prior evidence (no flash) until the new result replaces it. + if (!background) { + setHydrationEvidenceSync(null); } - const statusProxied = statusResult.proxied || proxied; - // Joined waiters mark network spans superseded so truncated join timings - // are not mistaken for fast fetches. - endSpan(statusHeaders, { - outcome: statusResult.coalesced ? 'superseded' : undefined, - proxied: statusProxied, - detail: { status: statusResult.status, coalesced: statusResult.coalesced }, - }); + + // Classify the status outcome. Coverage counts only successful, valid + // entries for current-list files; unrelated keys never contribute, so a + // missing file plus an extra key still blocks readiness. + const statusOutcome = await statusPromise; if (stale()) { abortAttempt(attemptId); return fileList; } let bulkStatuses: Record = {}; - const bulkPorts: Record = {}; - const bulkSelf: Record = {}; - const bulkCounts: StackCounts = {}; + let bulkPorts: Record = {}; + let bulkSelf: Record = {}; + let bulkCounts: StackCounts = {}; + let source: HydrationSource = 'bulk'; + let coveredFileCount = 0; + const statusProxied = statusOutcome.proxied || proxied; - // Decode already happened inside the shared helper; do not emit a fake - // body_decode span that would look like near-zero network work. - const raw: unknown = statusResult.ok ? statusResult.body : null; - if (isBulkStatusObjectFormat(raw)) { - for (const [key, val] of Object.entries(raw as Record)) { - bulkStatuses[key] = val.status; - if (val.mainPort) bulkPorts[key] = val.mainPort; - if (val.isSelf) bulkSelf[key] = true; - if (val.running !== undefined && val.total !== undefined) { - bulkCounts[key] = { running: val.running, total: val.total }; - } + const failHydration = (reason: string): string[] => { + console.error(`Failed to refresh stack statuses: ${reason}`); + if (listSucceeded && !background) { + markMilestone('list_hydrated', { attemptId, outcome: 'error', proxied }); + // Foreground (user-initiated) failures get one explicit signal; the + // dashboard's background polls stay silent because they self-heal. + toast.error('Could not refresh stack statuses. Check the node connection and try again.'); } - } else { - bulkStatuses = await deriveStatusesFromContainers(fileList); + // Same-list failure restores the attempt-start snapshot (stale) so the + // operator keeps last-known statuses with an explicit stale marker. + // The snapshot, not the live ref, is used for both foreground and + // background: a concurrent foreground refresh blanks the live evidence, + // so a background failure must not fall through to the hard-error path + // (and flash an error) while the foreground attempt is still in flight. + // Changed list or missing prior replaces with a hard error and clears + // the maps so nothing renders as current. + if ( + priorEvidence && + priorEvidence.nodeId === evidenceNodeId && + priorEvidence.listFingerprint === fingerprint && + priorEvidence.outcome === 'ok' + ) { + setHydrationEvidenceSync({ ...priorEvidence, stale: true }); + } else { + recordHydrationError(fingerprint); + } + return latestFileList; + }; + + if (statusOutcome.error) { + return failHydration(statusOutcome.error.message); } + if (statusOutcome.ok && isValidBulkPayload(statusOutcome.body)) { + // Decode already happened inside the shared helper; do not emit a fake + // body_decode span that would look like near-zero network work. + const parsed = parseBulkStatusPayload(statusOutcome.body, fileList); + bulkStatuses = parsed.statuses; + bulkPorts = parsed.ports; + bulkSelf = parsed.self; + bulkCounts = parsed.counts; + coveredFileCount = parsed.coveredFileCount; + } else if ( + (statusOutcome.ok && isValidLegacyPayload(statusOutcome.body)) || + (!statusOutcome.ok && + (statusOutcome.status === 404 || statusOutcome.status === 405 || statusOutcome.status === 501)) + ) { + // A node returning the legacy plain-string format (partial already + // collapsed into running) or lacking the bulk endpoint entirely is + // re-derived from per-stack containers so a crashed container is not + // hidden behind a healthy sibling. + source = 'legacy'; + const derived = await deriveStatusesFromContainers(fileList, evidenceNodeId); + bulkStatuses = derived.statuses; + coveredFileCount = derived.coveredFileCount; + if (stale()) { abortAttempt(attemptId); return fileList; } + // A total fallback failure (proxy down, node unreachable) must not + // masquerade as "ok with zero coverage": it is a hard error. + if (coveredFileCount === 0 && fileList.length > 0) { + return failHydration(`legacy derivation covered 0 of ${fileList.length} stacks`); + } + } else if (!statusOutcome.ok) { + // 400/403/408/409/429/5xx and malformed payloads are errors: they never + // trigger per-stack fallback requests (which would amplify proxy work). + return failHydration(`status ${statusOutcome.status}`); + } else { + return failHydration('unrecognized status payload'); + } + const statusDispatch = beginSpan('state_dispatch', { attemptId, background, proxied: statusProxied, - detail: { coalesced: statusResult.coalesced }, + detail: { coalesced: statusOutcome.coalesced }, }); setStackStatuses(prev => { const next: StackStatus = {}; @@ -361,8 +642,16 @@ export function useStackListState() { }); setStackSelfFlags(bulkSelf); setStackCounts(bulkCounts); - endSpan(statusDispatch, { detail: { coalesced: statusResult.coalesced } }); + endSpan(statusDispatch, { detail: { coalesced: statusOutcome.coalesced } }); refreshLabels(); + setHydrationEvidenceSync({ + nodeId: evidenceNodeId, + listFingerprint: fingerprint, + outcome: 'ok', + source, + stale: false, + coveredFileCount, + }); if (!background) { listHydratedPendingRef.current = { attemptId, token: listToken, proxied: statusProxied }; } @@ -374,14 +663,19 @@ export function useStackListState() { if (stale()) { abortAttempt(attemptId); return []; } console.error('Failed to refresh stacks:', error); const message = error instanceof Error ? error.message : 'Failed to load stacks'; - // The list committed but hydrating its statuses threw: record the list - // path as hydrated-with-error rather than leaving it hanging. - if (listSucceeded && !background) { - markMilestone('list_hydrated', { attemptId, outcome: 'error', proxied }); + // The list committed but hydrating its statuses threw: keep the confirmed + // list visible with error evidence, rather than erasing it. A list that + // never loaded takes the existing failure path. + if (listSucceeded) { + if (!background) { + markMilestone('list_hydrated', { attemptId, outcome: 'error', proxied }); + } + recordHydrationError(currentFingerprintRef.current); + return latestFileList; } return applyStacksFailure(message); } finally { - setIsLoading(false); + settleLoading(); } }; @@ -452,20 +746,34 @@ export function useStackListState() { return info != null && isConfirmedImageUpdate(info); }; + // Runtime filters (Up/Down) are driven by the same hydration display state + // that the rows use, so a zero count can never coexist with rows shown under + // that chip. Pending, error, and incomplete coverage produce no Up/Down + // matches; stale evidence keeps counts with a stale qualifier. The All chip + // always matches `files` and Updates uses its own data source. Deriving from + // `hydrationDisplay` (not from weaker re-derivations of the evidence) keeps + // node-switch fail-closed on the very first frame. + const showRuntimeFilters = hydrationDisplay === 'current' || hydrationDisplay === 'stale'; + const filterStaleQualifier = hydrationDisplay === 'stale'; + + const runtimeVisibleFiles = showRuntimeFilters ? filteredFiles : []; + const upFiles = runtimeVisibleFiles.filter(f => stackStatuses[f] === 'running'); + const downFiles = runtimeVisibleFiles.filter(f => isDownStatus(stackStatuses[f])); + const filterCounts = useMemo(() => ({ all: filteredFiles.length, - up: filteredFiles.filter(f => stackStatuses[f] === 'running').length, - down: filteredFiles.filter(f => isDownStatus(stackStatuses[f])).length, + up: upFiles.length, + down: downFiles.length, updates: filteredFiles.filter(hasConfirmedSidebarUpdate).length, - }), [filteredFiles, stackStatuses, sidebarStackUpdates]); + }), [filteredFiles, upFiles, downFiles, sidebarStackUpdates]); const chipFilteredFiles = useMemo(() => { if (filterChip === 'all') return filteredFiles; - if (filterChip === 'up') return filteredFiles.filter(f => stackStatuses[f] === 'running'); - if (filterChip === 'down') return filteredFiles.filter(f => isDownStatus(stackStatuses[f])); + if (filterChip === 'up') return upFiles; + if (filterChip === 'down') return downFiles; if (filterChip === 'updates') return filteredFiles.filter(hasConfirmedSidebarUpdate); return filteredFiles; - }, [filteredFiles, filterChip, stackStatuses, sidebarStackUpdates]); + }, [filteredFiles, filterChip, upFiles, downFiles, sidebarStackUpdates]); const toggleBulkMode = useCallback(() => { setBulkMode(prev => { @@ -488,6 +796,13 @@ export function useStackListState() { }, []); const handleBulkAction = useCallback((action: BulkAction) => { + // Bulk lifecycle mutations need authoritative status evidence just like + // single-stack actions; without it the batch would run against unknown + // runtime state. + if (!hydrationReadyRef.current()) { + toast.error('Status data unavailable. Refresh and try again.'); + return; + } const filesToAction = Array.from(selectedFiles); runBulk(action, filesToAction, { onAfter: () => { @@ -589,5 +904,10 @@ export function useStackListState() { stacksLoadStatus, stacksLoadError, stacksLoadNodeId, + hydrationStatus, + hydrationDisplay, + actionsReady, + hydrationReady: () => hydrationReadyRef.current(), + filterStaleQualifier, } as const; } diff --git a/frontend/src/components/sidebar/SidebarBulkBar.tsx b/frontend/src/components/sidebar/SidebarBulkBar.tsx index f080b2eb..dacf288f 100644 --- a/frontend/src/components/sidebar/SidebarBulkBar.tsx +++ b/frontend/src/components/sidebar/SidebarBulkBar.tsx @@ -6,9 +6,12 @@ interface SidebarBulkBarProps { selectedCount: number; onAction: (action: BulkAction) => void; onClear: () => void; + /** False while status evidence is not authoritative; disables lifecycle + * buttons so bulk selection cannot mutate against unknown runtime state. */ + actionsReady?: boolean; } -export function SidebarBulkBar({ selectedCount, onAction, onClear }: SidebarBulkBarProps) { +export function SidebarBulkBar({ selectedCount, onAction, onClear, actionsReady = false }: SidebarBulkBarProps) { return (
@@ -23,10 +26,10 @@ export function SidebarBulkBar({ selectedCount, onAction, onClear }: SidebarBulk
- - - - + + + +
); diff --git a/frontend/src/components/sidebar/SidebarFilterChips.tsx b/frontend/src/components/sidebar/SidebarFilterChips.tsx index 22ea33cb..86246b40 100644 --- a/frontend/src/components/sidebar/SidebarFilterChips.tsx +++ b/frontend/src/components/sidebar/SidebarFilterChips.tsx @@ -16,6 +16,9 @@ interface SidebarFilterChipsProps { visible: boolean; onToggle: () => void; showUpdatesChip?: boolean; + /** True when the Up/Down counts come from retained (stale) evidence; the + * counts stay visible but are qualified so they are never read as current. */ + stale?: boolean; } const chips: { id: FilterChip; label: string }[] = [ @@ -25,7 +28,7 @@ const chips: { id: FilterChip; label: string }[] = [ { id: 'updates', label: 'Updates' }, ]; -export function SidebarFilterChips({ active, counts, onChange, visible, onToggle, showUpdatesChip = true }: SidebarFilterChipsProps) { +export function SidebarFilterChips({ active, counts, onChange, visible, onToggle, showUpdatesChip = true, stale = false }: SidebarFilterChipsProps) { const visibleChips = showUpdatesChip ? chips : chips.filter(c => c.id !== 'updates'); return (
@@ -58,11 +61,15 @@ export function SidebarFilterChips({ active, counts, onChange, visible, onToggle aria-pressed={isActive} > {label} - + {displayCount} + {stale && (id === 'up' || id === 'down') ? '*' : ''} ); diff --git a/frontend/src/components/sidebar/StackList.tsx b/frontend/src/components/sidebar/StackList.tsx index 956f0e3f..d95e4671 100644 --- a/frontend/src/components/sidebar/StackList.tsx +++ b/frontend/src/components/sidebar/StackList.tsx @@ -67,6 +67,10 @@ export interface StackListProps { stacksLoadStatus?: StacksLoadStatus; stacksLoadError?: string | null; onRetryStacksLoad?: () => void; + /** Hydration display projection, driving status/port/count truthfulness. */ + hydrationDisplay?: 'pending' | 'error' | 'current' | 'stale' | 'incomplete'; + /** Terminal hydration outcome; settles the data-stacks-loaded sentinel. */ + hydrationStatus?: 'pending' | 'ok' | 'error'; } interface BuiltGroup { @@ -159,10 +163,14 @@ export function StackList(props: StackListProps & StackListBulkProps) { stacksLoadStatus, stacksLoadError, onRetryStacksLoad, + hydrationDisplay, } = props; const [failedNodesExpanded, setFailedNodesExpanded] = useState(false); + const showCounts = hydrationDisplay === 'current' || hydrationDisplay === 'stale'; + const forcedUnknown = hydrationDisplay === 'pending' || hydrationDisplay === 'error'; + const groups = useMemo( () => buildGroups(files, pinnedFiles, stackLabelMap), [files, pinnedFiles, stackLabelMap], @@ -252,9 +260,10 @@ export function StackList(props: StackListProps & StackListBulkProps) { - {/* Status pill. Partial stacks add a hover tooltip with the running/total count. */} - + {/* Status pill. Partial stacks add a hover tooltip with the running/total + count. Hydration state shapes the pill: pending stays muted, error + turns the unknown indicator warning-colored (distinct from pending), + stale dims the retained value so it is never read as current. */} + {isBusy ? ( ) : status === 'partial' && running !== undefined && total !== undefined ? ( diff --git a/frontend/src/components/sidebar/StackSidebar.tsx b/frontend/src/components/sidebar/StackSidebar.tsx index e874df48..1f1ecb25 100644 --- a/frontend/src/components/sidebar/StackSidebar.tsx +++ b/frontend/src/components/sidebar/StackSidebar.tsx @@ -35,6 +35,10 @@ export interface StackSidebarProps { onClearSelection: () => void; onBulkAction: (action: BulkAction) => void; showUpdatesChip?: boolean; + /** True when Up/Down filter counts derive from retained stale evidence. */ + filterStale?: boolean; + /** False while status evidence is not authoritative; disables bulk buttons. */ + actionsReady?: boolean; } export function StackSidebar(props: StackSidebarProps) { @@ -44,6 +48,8 @@ export function StackSidebar(props: StackSidebarProps) { list, activitySummary, onActivityAction, bulkMode, selectedFiles, onToggleBulkMode, onToggleSelect, onClearSelection, onBulkAction, showUpdatesChip = true, + filterStale = false, + actionsReady = false, } = props; const [filtersVisible, setFiltersVisible] = useState(() => { @@ -91,18 +97,20 @@ export function StackSidebar(props: StackSidebarProps) { visible={filtersVisible} onToggle={handleToggleFilters} showUpdatesChip={showUpdatesChip} + stale={filterStale} /> {selectedFiles.size > 0 && ( )}
diff --git a/frontend/src/components/sidebar/__tests__/StackList.test.tsx b/frontend/src/components/sidebar/__tests__/StackList.test.tsx index 6946edfb..17804826 100644 --- a/frontend/src/components/sidebar/__tests__/StackList.test.tsx +++ b/frontend/src/components/sidebar/__tests__/StackList.test.tsx @@ -2,6 +2,7 @@ import type React from 'react'; import { describe, it, expect, vi } from 'vitest'; import { render, screen } from '@testing-library/react'; import { StackList } from '../StackList'; +import { Command } from '@/components/ui/command'; import { isStacksListSettled, isStacksListLoading } from '../stacksLoadUi'; type StackListRenderProps = React.ComponentProps; @@ -104,3 +105,98 @@ describe('StackList load gating', () => { expect(screen.queryByTestId('discovery-empty')).toBeNull(); }); }); + +describe('stacksLoadUi hydration sentinel', () => { + it('settles a completed list error regardless of hydration state', () => { + expect(isStacksListSettled(false, 'error', 'pending')).toBe(true); + }); + + it('does not settle a successful list while hydration is pending', () => { + expect(isStacksListSettled(false, 'success', 'pending')).toBe(false); + }); + + it('settles a successful list once hydration is terminal', () => { + expect(isStacksListSettled(false, 'success', 'ok')).toBe(true); + expect(isStacksListSettled(false, 'success', 'error')).toBe(true); + }); +}); + +describe('StackList hydration display', () => { + const minimalCtx = { + stackStatus: 'running', + ready: true, + isSelfStack: false, + canOpenApp: false, + isBusy: false, + isAdmin: false, + canDelete: false, + canDeploy: true, + canEditLabels: false, + canCreateLabels: false, + isPinned: false, + labels: [], + assignedLabelIds: [], + menuVisibility: { showDeploy: false, showStop: false, showRestart: false, showUpdate: false, showTakeDown: false }, + openAlertSheet: () => {}, + openAutoHeal: () => {}, + canViewMonitor: false, + canCheckUpdates: false, + checkUpdates: () => {}, + openStackApp: () => {}, + deploy: () => {}, + stop: () => {}, + restart: () => {}, + update: () => {}, + takeDown: () => {}, + remove: () => {}, + pin: () => {}, + unpin: () => {}, + toggleLabel: async () => {}, + openLabelManager: () => {}, + openScheduleTask: () => {}, + canMuteNotifications: false, + muteStackAll: () => {}, + muteStackDeploySuccess: () => {}, + muteStackMonitor: () => {}, + openStackMuteRules: () => {}, + }; + + it('renders rows with unknown indicators while hydration is pending', () => { + render( + + minimalCtx as never, + })} + /> + , + ); + const row = screen.getByTestId('stack-row'); + expect(row.textContent).toContain('--'); + expect(row.textContent).not.toContain('UP'); + }); + + it('renders rows normally when hydration is current', () => { + render( + + minimalCtx as never, + })} + /> + , + ); + const row = screen.getByTestId('stack-row'); + expect(row.textContent).toContain('UP'); + }); +}); diff --git a/frontend/src/components/sidebar/sidebar-types.ts b/frontend/src/components/sidebar/sidebar-types.ts index 839f3a94..db2169ce 100644 --- a/frontend/src/components/sidebar/sidebar-types.ts +++ b/frontend/src/components/sidebar/sidebar-types.ts @@ -23,6 +23,9 @@ export type StackLifecycleStatus = 'running' | 'exited' | 'unknown'; export interface StackMenuCtx { stackStatus: StackLifecycleStatus; + /** False while status evidence is not authoritative for the active node/list; + * gates schedule/delete items that permissions alone would expose. */ + ready: boolean; /** True when this stack is the running Sencho instance on the active node. */ isSelfStack: boolean; canOpenApp: boolean; diff --git a/frontend/src/components/sidebar/stack-status-utils.ts b/frontend/src/components/sidebar/stack-status-utils.ts index 025c049a..5e28ce1d 100644 --- a/frontend/src/components/sidebar/stack-status-utils.ts +++ b/frontend/src/components/sidebar/stack-status-utils.ts @@ -21,11 +21,22 @@ export function isDownStatus(status: StackRowStatus | undefined): boolean { } /** Minimal container shape needed to classify a stack's status. */ -interface ContainerStateInfo { +export interface ContainerStateInfo { State: string; Status?: string; } +/** Whether a value satisfies the minimal container shape. The legacy + * per-stack fallback must not count a successful-but-malformed response + * (e.g. `{ error: "..." }` or an array of junk) as authoritative coverage. */ +export function isContainerStateInfo(value: unknown): value is ContainerStateInfo { + return ( + value !== null && + typeof value === 'object' && + typeof (value as { State?: unknown }).State === 'string' + ); +} + /** Exit code parsed from a Docker status string like "Exited (1) 2 hours ago". * Returns null when no parenthesized code is present (e.g. "Up 3 hours"). */ function parseExitCode(status: string | undefined): number | null { @@ -64,6 +75,92 @@ export function isBulkStatusObjectFormat(raw: unknown): boolean { ); } +const STATUS_VALUES: ReadonlySet = new Set(['running', 'exited', 'unknown', 'partial']); + +/** Whether a value is a recognized runtime status string. Guards both payload + * validators so an arbitrary string (e.g. an error object) is never mistaken + * for a legacy status map, which would otherwise fan out N per-stack fallback + * requests for one malformed response. */ +function isStatusValue(value: unknown): value is StackRowStatus { + return typeof value === 'string' && STATUS_VALUES.has(value); +} + +/** Per-stack entry shape of the current bulk status payload. */ +export interface BulkStatusPayloadEntry { + status: StackRowStatus; + mainPort?: number; + running?: number; + total?: number; + isSelf?: boolean; +} + +/** Whether a bulk payload's entries all carry a recognized runtime status and + * well-typed optional fields. Distinguishes the current object format with + * valid values from a malformed payload that merely has a `status` property + * with an unrecognized value, or carries a mistyped port/count field that + * would flow into `buildServiceUrl` and the row counts. */ +export function isValidBulkPayload( + raw: unknown, +): raw is Record { + if (!isBulkStatusObjectFormat(raw)) return false; + return Object.values(raw as Record).every((val) => { + const entry = val as { + status?: unknown; + mainPort?: unknown; + running?: unknown; + total?: unknown; + isSelf?: unknown; + }; + return ( + isStatusValue(entry.status) && + (entry.mainPort === undefined || typeof entry.mainPort === 'number') && + (entry.running === undefined || typeof entry.running === 'number') && + (entry.total === undefined || typeof entry.total === 'number') && + (entry.isSelf === undefined || typeof entry.isSelf === 'boolean') + ); + }); +} + +/** Parse a validated bulk status payload into the row-facing maps plus the + * count of current-list files it covers. Coverage counts exact filename + * matches; unrelated keys never contribute. */ +export function parseBulkStatusPayload( + raw: Record, + fileList: string[], +): { + statuses: Record; + ports: Record; + self: Record; + counts: Record; + coveredFileCount: number; +} { + const statuses: Record = {}; + const ports: Record = {}; + const self: Record = {}; + const counts: Record = {}; + let coveredFileCount = 0; + for (const [key, val] of Object.entries(raw)) { + statuses[key] = val.status; + if (val.mainPort) ports[key] = val.mainPort; + if (val.isSelf) self[key] = true; + if (val.running !== undefined && val.total !== undefined) { + counts[key] = { running: val.running, total: val.total }; + } + } + for (const file of fileList) { + if (raw[file] !== undefined) coveredFileCount += 1; + } + return { statuses, ports, self, counts, coveredFileCount }; +} + +/** Whether a payload is a legacy string-value map whose values are recognized + * runtime statuses (the pre-object-format response). A string map with other + * values (e.g. `{ error: "failed" }`) is malformed, not legacy. */ +export function isValidLegacyPayload(raw: unknown): raw is Record { + if (raw === null || typeof raw !== 'object' || Array.isArray(raw)) return false; + return Object.values(raw as Record).every(isStatusValue); +} + /** Derive a stack's row status from its container list, distinguishing a fully-up * stack from one that is partially degraded (some running, some crashed). Used by * the compatibility path for remote nodes whose bulk status endpoint is absent or diff --git a/frontend/src/components/sidebar/stacksLoadUi.ts b/frontend/src/components/sidebar/stacksLoadUi.ts index c97add5f..b9ae11cb 100644 --- a/frontend/src/components/sidebar/stacksLoadUi.ts +++ b/frontend/src/components/sidebar/stacksLoadUi.ts @@ -1,11 +1,18 @@ import type { StacksLoadStatus } from '@/components/EditorLayout/hooks/useStackListState'; -/** True when the sidebar stack list request has finished for the active node. */ +/** True when the sidebar stack list request has finished for the active node. + * A completed list error settles regardless of status hydration; a successful + * list is fully settled only once status hydration is no longer pending, so + * readiness sentinels (E2E `data-stacks-loaded`) stay truthful. */ export function isStacksListSettled( isLoading: boolean, stacksLoadStatus: StacksLoadStatus | undefined, + hydrationStatus?: 'pending' | 'ok' | 'error', ): boolean { - return !isLoading && (stacksLoadStatus === 'success' || stacksLoadStatus === 'error'); + if (isLoading) return false; + if (stacksLoadStatus === 'error') return true; + if (stacksLoadStatus !== 'success') return false; + return hydrationStatus !== 'pending'; } /** True while the sidebar should show the stack-list skeleton. */ diff --git a/frontend/src/hooks/__tests__/useStackKeyboardShortcuts.test.ts b/frontend/src/hooks/__tests__/useStackKeyboardShortcuts.test.ts index 8ece08b7..611dbc3d 100644 --- a/frontend/src/hooks/__tests__/useStackKeyboardShortcuts.test.ts +++ b/frontend/src/hooks/__tests__/useStackKeyboardShortcuts.test.ts @@ -7,6 +7,7 @@ function makeCtx(overrides: Partial = {}): StackMenuCtx { return { stackStatus: 'running', isSelfStack: false, + ready: true, canOpenApp: true, isBusy: false, isAdmin: true, diff --git a/frontend/src/hooks/__tests__/useStackMenuItems.test.tsx b/frontend/src/hooks/__tests__/useStackMenuItems.test.tsx index d6f6ca7e..5d7d282d 100644 --- a/frontend/src/hooks/__tests__/useStackMenuItems.test.tsx +++ b/frontend/src/hooks/__tests__/useStackMenuItems.test.tsx @@ -8,6 +8,7 @@ function makeCtx(overrides: Partial = {}): StackMenuCtx { return { stackStatus: 'running', isSelfStack: false, + ready: true, canOpenApp: true, isBusy: false, isAdmin: true, diff --git a/frontend/src/hooks/useStackMenuItems.tsx b/frontend/src/hooks/useStackMenuItems.tsx index 07bf312b..e98d10c8 100644 --- a/frontend/src/hooks/useStackMenuItems.tsx +++ b/frontend/src/hooks/useStackMenuItems.tsx @@ -25,6 +25,7 @@ export function useStackMenuItems(_file: string, ctx: StackMenuCtx): MenuGroup[] deploy, stop, restart, update, takeDown, remove, pin, unpin, toggleLabel, menuVisibility, openScheduleTask, canMuteNotifications, muteStackAll, muteStackDeploySuccess, muteStackMonitor, openStackMuteRules, + ready, } = ctx; const { showDeploy, showStop, showRestart, showUpdate, showTakeDown } = menuVisibility; @@ -89,10 +90,10 @@ export function useStackMenuItems(_file: string, ctx: StackMenuCtx): MenuGroup[] if (showUpdate) lifecycle.push({ id: 'update', label: 'Update', icon: Download, shortcut: '⌘↑', onSelect: update, disabled: isBusy }); if (showTakeDown) lifecycle.push({ id: 'take-down', label: 'Take down', icon: ArrowDownToLine, shortcut: '⌘↓', onSelect: takeDown, disabled: isBusy || isSelfStack }); } - if (canDeploy) lifecycle.push({ id: 'schedule', label: 'Schedule task', icon: CalendarClock, onSelect: openScheduleTask }); + if (canDeploy && ready) lifecycle.push({ id: 'schedule', label: 'Schedule task', icon: CalendarClock, onSelect: openScheduleTask }); if (lifecycle.length > 0) groups.push({ id: 'lifecycle', items: lifecycle }); - if (canDelete) { + if (canDelete && ready) { groups.push({ id: 'destructive', items: [{ @@ -110,7 +111,7 @@ export function useStackMenuItems(_file: string, ctx: StackMenuCtx): MenuGroup[] return groups; }, [ stackStatus, isSelfStack, canOpenApp, isBusy, canDelete, canDeploy, canEditLabels, isPinned, labels, - showDeploy, showStop, showRestart, showUpdate, showTakeDown, + ready, showDeploy, showStop, showRestart, showUpdate, showTakeDown, openAlertSheet, openAutoHeal, canViewMonitor, canCheckUpdates, checkUpdates, openStackApp, deploy, stop, restart, update, takeDown, remove, pin, unpin, toggleLabel, openScheduleTask, canMuteNotifications, muteStackAll, muteStackDeploySuccess, muteStackMonitor, openStackMuteRules,