diff --git a/frontend/src/components/AppStoreView.tsx b/frontend/src/components/AppStoreView.tsx index d83670c2..e51cea15 100644 --- a/frontend/src/components/AppStoreView.tsx +++ b/frontend/src/components/AppStoreView.tsx @@ -197,11 +197,15 @@ export function AppStoreView({ onDeploySuccess }: AppStoreViewProps) { if (ce.key.trim()) finalEnvVars[ce.key.trim()] = ce.value; }); + // Snapshot the node once so the install stays bound to it even if the + // active node changes while images are pulling. + const opNodeId = activeNode?.id ?? null; try { - const result = await runWithLog({ stackName: stackName.trim(), action: 'install', nodeId: activeNode?.id ?? null }, async (started, ds) => { + const result = await runWithLog({ stackName: stackName.trim(), action: 'install', nodeId: opNodeId }, async (started, ds) => { await started; const res = await apiFetch('/templates/deploy', withDeploySession(ds, { method: 'POST', + nodeId: opNodeId, body: JSON.stringify({ stackName: stackName.trim(), template: modifiedTemplate, diff --git a/frontend/src/components/DeployFeedbackModal.tsx b/frontend/src/components/DeployFeedbackModal.tsx index 49754f00..225d2997 100644 --- a/frontend/src/components/DeployFeedbackModal.tsx +++ b/frontend/src/components/DeployFeedbackModal.tsx @@ -308,6 +308,7 @@ export function DeployFeedbackModal({ isMinimized, onMinimize }: DeployFeedbackM style={{ height: showRaw ? '200px' : 0, overflow: 'hidden' }} > (null); useEffect(() => { - if (!healthGate || healthGate.status !== 'failed' || handledGateRef.current === healthGate.gateId) return; - const stackFile = stackListState.files.find( - f => f.replace(/\.(yml|yaml)$/, '') === healthGate.stackName, + if (!healthGate || handledGateRef.current === healthGate.gateId) return; + // Record only on the node the gate ran on. A wrong node or a not-yet-loaded + // stack file leaves it unhandled (not marked), so the effect retries when the + // active node returns or the files refresh, recording exactly once. + const outcome = classifyFailedGate( + healthGate, + activeNodeIdRef.current, + stackListState.filesNodeId, + stackListState.files, ); - if (!stackFile) { - // Do not mark handled: the files list may be mid-refresh, and the - // effect's files dependency retries once it lands. + if (outcome.kind === 'no-file') { console.warn('[HealthGate] no stack file matches failed gate for', healthGate.stackName); return; } + if (outcome.kind === 'skip') return; handledGateRef.current = healthGate.gateId; - stackListState.recordActionFailure(stackFile, { + stackListState.recordActionFailure(outcome.stackFile, { action: healthGate.trigger, rolledBack: false, errorMessage: `Health gate failed: ${healthGate.reason ?? 'containers did not stay healthy after the update'}`, @@ -228,7 +234,7 @@ export default function EditorLayout() { suggestion: 'Check the container logs; roll back if the previous version was healthy.', }, }); - }, [healthGate, stackListState.files, stackListState.recordActionFailure]); + }, [healthGate, stackListState.files, stackListState.filesNodeId, stackListState.recordActionFailure]); const buildMenuCtx = useSidebarContextMenu({ stackListState, diff --git a/frontend/src/components/EditorLayout/__tests__/StackOperationBanner.test.tsx b/frontend/src/components/EditorLayout/__tests__/StackOperationBanner.test.tsx index 1c2cf8b4..01bbd78e 100644 --- a/frontend/src/components/EditorLayout/__tests__/StackOperationBanner.test.tsx +++ b/frontend/src/components/EditorLayout/__tests__/StackOperationBanner.test.tsx @@ -71,7 +71,7 @@ beforeEach(() => { }); const passedGate = (): HealthGateUiState => ({ - stackName: 'web', gateId: 'g', trigger: 'update', status: 'passed', reason: null, windowSeconds: 90, startedAt: Date.now() - 90_000, + stackName: 'web', nodeId: null, gateId: 'g', trigger: 'update', status: 'passed', reason: null, windowSeconds: 90, startedAt: Date.now() - 90_000, }); function renderBanner(activeNode: Node | null = null, panelStartedAt: number | null = Date.now() - 12_000) { @@ -117,7 +117,7 @@ describe('StackOperationBanner', () => { unmount(); mockPanelState = panel({ status: 'succeeded' }); - mockHealthGate = { stackName: 'web', gateId: 'g', trigger: 'update', status: 'failed', reason: 'exited', windowSeconds: 90, startedAt: Date.now() }; + mockHealthGate = { stackName: 'web', nodeId: null, gateId: 'g', trigger: 'update', status: 'failed', reason: 'exited', windowSeconds: 90, startedAt: Date.now() }; renderBanner(); expect(screen.queryByTestId('stack-operation-banner')).toBeNull(); }); @@ -137,7 +137,7 @@ describe('StackOperationBanner', () => { it('shows the observing health gate and keeps present tense', () => { mockPanelState = panel({ status: 'succeeded' }); - mockHealthGate = { stackName: 'web', gateId: 'g', trigger: 'update', status: 'observing', reason: null, windowSeconds: 90, startedAt: Date.now() - 12_000 }; + mockHealthGate = { stackName: 'web', nodeId: null, gateId: 'g', trigger: 'update', status: 'observing', reason: null, windowSeconds: 90, startedAt: Date.now() - 12_000 }; renderBanner(); expect(screen.getByText('Updating')).toBeInTheDocument(); expect(screen.getByText('Verifying health')).toBeInTheDocument(); @@ -146,7 +146,7 @@ describe('StackOperationBanner', () => { it('shows the passed health gate', () => { mockPanelState = panel({ status: 'succeeded' }); - mockHealthGate = { stackName: 'web', gateId: 'g', trigger: 'update', status: 'passed', reason: null, windowSeconds: 90, startedAt: Date.now() - 90_000 }; + mockHealthGate = { stackName: 'web', nodeId: null, gateId: 'g', trigger: 'update', status: 'passed', reason: null, windowSeconds: 90, startedAt: Date.now() - 90_000 }; renderBanner(); expect(screen.getByText('Health gate passed')).toBeInTheDocument(); expect(screen.getByText('Updated')).toBeInTheDocument(); @@ -154,7 +154,7 @@ describe('StackOperationBanner', () => { it('shows the unknown health gate state with its reason', () => { mockPanelState = panel({ status: 'succeeded' }); - mockHealthGate = { stackName: 'web', gateId: 'g', trigger: 'update', status: 'unknown', reason: 'no healthcheck defined', windowSeconds: 90, startedAt: Date.now() }; + mockHealthGate = { stackName: 'web', nodeId: null, gateId: 'g', trigger: 'update', status: 'unknown', reason: 'no healthcheck defined', windowSeconds: 90, startedAt: Date.now() }; renderBanner(); expect(screen.getByText('Health check unknown')).toBeInTheDocument(); expect(screen.getByText('no healthcheck defined')).toBeInTheDocument(); @@ -253,7 +253,7 @@ describe('StackOperationBanner', () => { vi.useFakeTimers(); try { mockPanelState = panel({ status: 'succeeded' }); - mockHealthGate = { stackName: 'web', gateId: 'g', trigger: 'update', status: 'observing', reason: null, windowSeconds: 90, startedAt: Date.now() }; + mockHealthGate = { stackName: 'web', nodeId: null, gateId: 'g', trigger: 'update', status: 'observing', reason: null, windowSeconds: 90, startedAt: Date.now() }; renderBanner(); act(() => { vi.advanceTimersByTime(8000); }); expect(onPanelClose).not.toHaveBeenCalled(); diff --git a/frontend/src/components/EditorLayout/failed-gate-recovery.test.ts b/frontend/src/components/EditorLayout/failed-gate-recovery.test.ts new file mode 100644 index 00000000..42519901 --- /dev/null +++ b/frontend/src/components/EditorLayout/failed-gate-recovery.test.ts @@ -0,0 +1,46 @@ +import { describe, it, expect } from 'vitest'; +import { classifyFailedGate } from './failed-gate-recovery'; +import type { HealthGateUiState } from '@/context/DeployFeedbackContext'; + +type Gate = Pick; +const gate = (over: Partial = {}): Gate => ({ status: 'failed', nodeId: null, stackName: 'web', ...over }); + +describe('classifyFailedGate', () => { + it('skips when there is no gate', () => { + expect(classifyFailedGate(null, null, null, ['web.yml'])).toEqual({ kind: 'skip' }); + }); + + it('skips a gate that has not failed', () => { + expect(classifyFailedGate(gate({ status: 'observing' }), null, null, ['web.yml'])).toEqual({ kind: 'skip' }); + expect(classifyFailedGate(gate({ status: 'passed' }), null, null, ['web.yml'])).toEqual({ kind: 'skip' }); + }); + + it('records on the local node when the gate ran locally and a file matches', () => { + expect(classifyFailedGate(gate({ nodeId: null }), null, null, ['web.yml'])).toEqual({ kind: 'record', stackFile: 'web.yml' }); + }); + + it('records on a remote node when the gate, the active node, and the file list all match', () => { + expect(classifyFailedGate(gate({ nodeId: 3 }), 3, 3, ['web.yaml'])).toEqual({ kind: 'record', stackFile: 'web.yaml' }); + }); + + it('skips when the gate ran on a different node than the active one', () => { + // Remote gate, active node is local: must not attach to a same-named local stack. + expect(classifyFailedGate(gate({ nodeId: 3 }), null, null, ['web.yml'])).toEqual({ kind: 'skip' }); + // Local gate, active node is remote. + expect(classifyFailedGate(gate({ nodeId: null }), 3, 3, ['web.yml'])).toEqual({ kind: 'skip' }); + // Two different remote nodes. + expect(classifyFailedGate(gate({ nodeId: 2 }), 5, 5, ['web.yml'])).toEqual({ kind: 'skip' }); + }); + + it('skips when the active node matches but the loaded file list is still another node\'s', () => { + // Switch-back gap: active node is the gate's node again, but files (and its + // filesNodeId) have not refreshed yet, so the list belongs to the node we + // just left. A same-named stack there must not capture the recovery entry. + expect(classifyFailedGate(gate({ nodeId: 3 }), 3, 2, ['web.yml'])).toEqual({ kind: 'skip' }); + expect(classifyFailedGate(gate({ nodeId: null }), null, 2, ['web.yml'])).toEqual({ kind: 'skip' }); + }); + + it('reports no-file when the node and file list match but no stack file matches the name yet', () => { + expect(classifyFailedGate(gate({ nodeId: 3, stackName: 'web' }), 3, 3, ['other.yml'])).toEqual({ kind: 'no-file' }); + }); +}); diff --git a/frontend/src/components/EditorLayout/failed-gate-recovery.ts b/frontend/src/components/EditorLayout/failed-gate-recovery.ts new file mode 100644 index 00000000..29b6ace6 --- /dev/null +++ b/frontend/src/components/EditorLayout/failed-gate-recovery.ts @@ -0,0 +1,37 @@ +import type { HealthGateUiState } from '@/context/DeployFeedbackContext'; + +/** + * Decide whether a failed health gate should record a recovery entry, and for + * which stack file. Extracted as a pure function so the cross-node guard (the + * load-bearing rule that a gate's failure records only on the node it ran on) + * is unit-testable without standing up the editor. + * + * - `skip`: not a failed gate, or it ran on a different node than the active one, + * or the loaded file list does not yet belong to the gate's node. Stack + * filenames repeat across nodes and recovery records are cleared on node + * switch, so recording against another node's list would attach the failure to + * the wrong stack. `filesNodeId` (the node `files` was fetched for) must match + * too: right after a switch the active node updates before the new list lands, + * and a name lookup against the stale list could resolve to a wrong filename. + * - `no-file`: the gate's node matches and its file list is loaded, but no stack + * file matches its name yet (the list may be mid-refresh). The caller leaves it + * unhandled so the effect retries once the files land. + * - `record`: record a recovery entry against `stackFile`. + */ +export type FailedGateOutcome = + | { kind: 'skip' } + | { kind: 'no-file' } + | { kind: 'record'; stackFile: string }; + +export function classifyFailedGate( + healthGate: Pick | null, + activeNodeId: number | null, + filesNodeId: number | null, + files: string[], +): FailedGateOutcome { + if (!healthGate || healthGate.status !== 'failed') return { kind: 'skip' }; + // Record only while on the gate's node AND with that node's file list loaded. + if (healthGate.nodeId !== activeNodeId || healthGate.nodeId !== filesNodeId) return { kind: 'skip' }; + const stackFile = files.find(f => f.replace(/\.(yml|yaml)$/, '') === healthGate.stackName); + return stackFile ? { kind: 'record', stackFile } : { kind: 'no-file' }; +} diff --git a/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts b/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts index 14e6c5b5..b99545d2 100644 --- a/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts +++ b/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts @@ -90,8 +90,11 @@ function makeOverlay(over: Partial = {}): OverlayState { } as unknown as OverlayState; } -const runWithLog: Parameters[0]['runWithLog'] = async (_p, run) => - run(Promise.resolve(), 'test-session'); +let lastRunWithLogParams: Parameters[0]['runWithLog']>[0] | null = null; +const runWithLog: Parameters[0]['runWithLog'] = async (params, run) => { + lastRunWithLogParams = params; + return run(Promise.resolve(), 'test-session'); +}; function setup(over: { editorState?: Partial; @@ -197,6 +200,44 @@ describe('useStackActions.handleSaveAndDeploy', () => { }); }); +describe('useStackActions node binding', () => { + const mouseEvent = { preventDefault: vi.fn(), stopPropagation: vi.fn() } as unknown as React.MouseEvent; + + beforeEach(() => { + vi.mocked(apiFetch).mockReset(); + vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 })); + lastRunWithLogParams = null; + }); + + function postCallFor(fragment: string): RequestInit | undefined { + const call = vi.mocked(apiFetch).mock.calls.find( + c => String(c[0]).includes(fragment) && (c[1] as RequestInit | undefined)?.method === 'POST', + ); + return call?.[1] as RequestInit | undefined; + } + + it('binds the deploy POST and the runWithLog session to the captured node', async () => { + const { result } = setup(); // activeNode.id = 1 + await result.current.deployStack(mouseEvent); + expect(postCallFor('/deploy')).toEqual(expect.objectContaining({ nodeId: 1 })); + expect(lastRunWithLogParams).toEqual(expect.objectContaining({ nodeId: 1 })); + }); + + it('binds the update POST and the runWithLog session to the captured node', async () => { + const { result } = setup(); + await result.current.updateStack(mouseEvent); + expect(postCallFor('/update')).toEqual(expect.objectContaining({ nodeId: 1 })); + expect(lastRunWithLogParams).toEqual(expect.objectContaining({ nodeId: 1 })); + }); + + it('binds a non-update action (restart) POST and session to the captured node', async () => { + const { result } = setup(); + await result.current.restartStack(mouseEvent); + expect(postCallFor('/restart')).toEqual(expect.objectContaining({ nodeId: 1 })); + expect(lastRunWithLogParams).toEqual(expect.objectContaining({ nodeId: 1 })); + }); +}); + describe('useStackActions policy-block dialog wiring', () => { const policyPayload = { error: 'Policy "block-high" blocked deploy: 1 image(s) exceed HIGH', diff --git a/frontend/src/components/EditorLayout/hooks/useStackActions.ts b/frontend/src/components/EditorLayout/hooks/useStackActions.ts index 4686c041..1848fe55 100644 --- a/frontend/src/components/EditorLayout/hooks/useStackActions.ts +++ b/frontend/src/components/EditorLayout/hooks/useStackActions.ts @@ -654,6 +654,7 @@ export function useStackActions(options: UseStackActionsOptions) { ignorePolicy: boolean, started?: Promise, deploySessionId?: string, + opNodeId?: number | null, ): Promise => { const previousStatus = stackListState.stackStatuses[stackFile]; const startedAt = Date.now(); @@ -663,7 +664,7 @@ export function useStackActions(options: UseStackActionsOptions) { ? `/stacks/${stackName}/deploy?ignorePolicy=true` : `/stacks/${stackName}/deploy`; if (started) await started; - const response = await apiFetch(path, withDeploySession(deploySessionId ?? '', { method: 'POST' })); + const response = await apiFetch(path, withDeploySession(deploySessionId ?? '', { method: 'POST', nodeId: opNodeId })); if (!response.ok) { const rawBody = await response.text(); if (response.status === 409) { @@ -729,9 +730,12 @@ export function useStackActions(options: UseStackActionsOptions) { const stackFile = stackListState.selectedFile; const stackName = stackFile.replace(/\.(yml|yaml)$/, ''); stackListState.setStackAction(stackFile, 'deploy'); + // Snapshot the node once so the request stays bound to it even if the active + // node changes while the operation is in flight. + const opNodeId = activeNode?.id ?? null; try { - await runWithLog({ stackName, action: 'deploy', nodeId: activeNode?.id ?? null }, (started, ds) => - runDeploy(stackName, stackFile, false, started, ds), + await runWithLog({ stackName, action: 'deploy', nodeId: opNodeId }, (started, ds) => + runDeploy(stackName, stackFile, false, started, ds, opNodeId), ); } finally { stackListState.clearStackAction(stackFile); @@ -764,9 +768,10 @@ export function useStackActions(options: UseStackActionsOptions) { await rollbackStack(true); } else { stackListState.setStackAction(existingFile, 'deploy'); + const opNodeId = activeNode?.id ?? null; try { - await runWithLog({ stackName, action: 'deploy', nodeId: activeNode?.id ?? null }, (started, ds) => - runDeploy(stackName, existingFile, true, started, ds), + await runWithLog({ stackName, action: 'deploy', nodeId: opNodeId }, (started, ds) => + runDeploy(stackName, existingFile, true, started, ds, opNodeId), ); } finally { stackListState.clearStackAction(existingFile); @@ -895,14 +900,17 @@ export function useStackActions(options: UseStackActionsOptions) { const startedAt = Date.now(); stackListState.setStackAction(stackFile, action); stackListState.setOptimisticStatus(stackFile, optimisticStatus); + // Snapshot the node once so stop/restart/update stays bound to it even if + // the active node changes while the operation is in flight. + const opNodeId = activeNode?.id ?? null; try { - await runWithLog({ stackName, action, nodeId: activeNode?.id ?? null }, async (started, ds) => { + await runWithLog({ stackName, action, nodeId: opNodeId }, async (started, ds) => { await started; try { const url = ignorePolicy ? `/stacks/${stackName}/${endpoint}?ignorePolicy=true` : `/stacks/${stackName}/${endpoint}`; - const response = await apiFetch(url, withDeploySession(ds, { method: 'POST' })); + const response = await apiFetch(url, withDeploySession(ds, { method: 'POST', nodeId: opNodeId })); if (!response.ok) { const errText = await response.text(); if (response.status === 409) { diff --git a/frontend/src/components/EditorLayout/hooks/useStackListState.ts b/frontend/src/components/EditorLayout/hooks/useStackListState.ts index d7c284f5..33bdb100 100644 --- a/frontend/src/components/EditorLayout/hooks/useStackListState.ts +++ b/frontend/src/components/EditorLayout/hooks/useStackListState.ts @@ -33,10 +33,20 @@ export function useStackListState() { const { nodes, activeNode } = useNodes(); const [files, setFiles] = useState([]); + // Node the current `files` list belongs to (null = local). Stamped together + // with `files` from the node active when the fetch started, so a consumer can + // tell whether the list it is reading is the one it expects, even during the + // async gap right after a node switch when `files` still holds the old node's + // entries. Filenames repeat across nodes, so a name lookup against the wrong + // list would resolve to the wrong file. + const [filesNodeId, setFilesNodeId] = useState(null); const [selectedFile, setSelectedFile] = useState(null); const [isLoading, setIsLoading] = useState(false); const [stackActions, setStackActions] = useState>({}); const stackActionsRef = useRef>({}); + // Monotonic token per refreshStacks call; lets a superseded fetch skip its + // state writes so a rapid node switch cannot leave a stale files/filesNodeId. + const fetchSeqRef = useRef(0); // Per-stack terminal failure records driving the in-detail recovery panel. // In-memory only. Node scoping is enforced by the caller, which clears these @@ -133,18 +143,28 @@ export function useStackListState() { const refreshStacks = async (background = false): Promise => { if (!background) setIsLoading(true); + // Snapshot the node this fetch targets and a sequence token so a superseded + // or out-of-order resolution (from a rapid node switch) cannot overwrite a + // newer node's list, keeping `files` and `filesNodeId` consistent. + const fetchNodeId = activeNode?.id ?? null; + const mySeq = ++fetchSeqRef.current; + const stale = () => fetchSeqRef.current !== mySeq; try { const res = await apiFetch('/stacks'); + if (stale()) return []; if (!res.ok) { setFiles([]); + setFilesNodeId(fetchNodeId); return []; } const data = await res.json(); const fileList: string[] = Array.isArray(data) ? data : []; setFiles(fileList); + setFilesNodeId(fetchNodeId); // Fetch all stack statuses in a single bulk call (falls back to per-stack queries for older remote nodes) const statusRes = await apiFetch('/stacks/statuses'); + if (stale()) return fileList; let bulkStatuses: Record | null = null; const bulkPorts: Record = {}; if (statusRes.ok) { @@ -194,8 +214,10 @@ export function useStackListState() { refreshLabels(); return fileList; } catch (error) { + if (stale()) return []; console.error('Failed to refresh stacks:', error); setFiles([]); + setFilesNodeId(fetchNodeId); return []; } finally { setIsLoading(false); @@ -341,7 +363,7 @@ export function useStackListState() { }, [remoteStackResults, nodes]); return { - files, setFiles, + files, setFiles, filesNodeId, selectedFile, setSelectedFile, isLoading, setIsLoading, stackActions, stackActionsRef, diff --git a/frontend/src/components/Terminal.tsx b/frontend/src/components/Terminal.tsx index 8221f015..765f81fc 100644 --- a/frontend/src/components/Terminal.tsx +++ b/frontend/src/components/Terminal.tsx @@ -7,6 +7,11 @@ import { buildXtermTheme } from '@/lib/terminalTheme'; interface TerminalComponentProps { stackName?: string; + /** Node the socket connects to (a number, or null for local), captured when + * the operation started. Binds the progress stream to that node so an + * active-node switch mid-stream cannot split it from the deploy request. + * Leave undefined to track the active node (the standalone stack-logs view). */ + nodeId?: number | null; /** Correlation id sent on the generic `connectTerminal` handshake so the * backend streams the matching deploy's output to this socket. */ deploySessionId?: string; @@ -17,7 +22,7 @@ interface TerminalComponentProps { onMessage?: (text: string) => void; } -export default function TerminalComponent({ stackName, deploySessionId, onReady, onError, onMessage }: TerminalComponentProps) { +export default function TerminalComponent({ stackName, nodeId, deploySessionId, onReady, onError, onMessage }: TerminalComponentProps) { const terminalRef = useRef(null); const terminalInstance = useRef(null); const fitAddonRef = useRef(null); @@ -116,13 +121,20 @@ export default function TerminalComponent({ stackName, deploySessionId, onReady, const wsProtocol = window.location.protocol === 'https:' ? 'wss:' : 'ws:'; const cleanStackName = stackName?.replace(/\.(yml|yaml)$/, ''); - const activeNodeId = localStorage.getItem('sencho-active-node') || ''; + // A captured nodeId (number, or null for local) wins over the active + // node; undefined falls back to the active node for standalone views. + let resolvedNodeId: string; + if (nodeId !== undefined) { + resolvedNodeId = nodeId === null ? '' : String(nodeId); + } else { + resolvedNodeId = localStorage.getItem('sencho-active-node') || ''; + } // If a stackName is provided, connect to the dedicated logs WebSocket // Otherwise, fall back to the generic terminal WebSocket const wsUrl = cleanStackName - ? `${wsProtocol}//${window.location.host}/api/stacks/${cleanStackName}/logs${activeNodeId ? `?nodeId=${activeNodeId}` : ''}` - : `${wsProtocol}//${window.location.host}/ws${activeNodeId ? `?nodeId=${activeNodeId}` : ''}`; + ? `${wsProtocol}//${window.location.host}/api/stacks/${cleanStackName}/logs${resolvedNodeId ? `?nodeId=${resolvedNodeId}` : ''}` + : `${wsProtocol}//${window.location.host}/ws${resolvedNodeId ? `?nodeId=${resolvedNodeId}` : ''}`; const ws = new WebSocket(wsUrl); wsRef.current = ws; @@ -213,7 +225,7 @@ export default function TerminalComponent({ stackName, deploySessionId, onReady, searchAddonRef.current = null; serializeAddonRef.current = null; }; - }, [stackName, deploySessionId, onReady, onError, onMessage]); + }, [stackName, nodeId, deploySessionId, onReady, onError, onMessage]); const handleDownload = () => { if (!serializeAddonRef.current) return; diff --git a/frontend/src/components/__tests__/AppStoreView.test.tsx b/frontend/src/components/__tests__/AppStoreView.test.tsx new file mode 100644 index 00000000..d8aa76c9 --- /dev/null +++ b/frontend/src/components/__tests__/AppStoreView.test.tsx @@ -0,0 +1,101 @@ +/** + * Verifies that an App Store install binds both the runWithLog session and the + * /templates/deploy POST to the node captured when the install starts, so the + * install does not retarget if the active node changes while images pull. The + * heavy child components are stubbed to a minimal select-then-deploy path. + */ +import type { ReactNode } from 'react'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; + +const dfCtl = vi.hoisted(() => ({ params: null as null | { stackName: string; action: string; nodeId: number | null } })); + +function jsonRes(body: unknown, ok = true, status = 200) { + return { ok, status, json: async () => body, text: async () => '' } as unknown as Response; +} + +vi.mock('@/lib/api', () => ({ + apiFetch: vi.fn(), + withDeploySession: (ds: string, options: RequestInit = {}) => ({ + ...options, + headers: { ...(options.headers as Record | undefined), 'x-deploy-session-id': ds }, + }), +})); +vi.mock('@/context/AuthContext', () => ({ useAuth: () => ({ can: () => true }) })); +vi.mock('@/context/NodeContext', () => ({ useNodes: () => ({ activeNode: { id: 5, type: 'local', name: 'local' } }) })); +vi.mock('@/context/DeployFeedbackContext', () => ({ + useDeployFeedback: () => ({ + runWithLog: vi.fn( + async ( + params: { stackName: string; action: string; nodeId: number | null }, + run: (started: Promise, ds: string) => Promise<{ ok: boolean }>, + ) => { + dfCtl.params = params; + return run(Promise.resolve(), 'ds-1'); + }, + ), + }), +})); +vi.mock('@/components/ui/toast-store', () => ({ + toast: { success: vi.fn(), error: vi.fn(), info: vi.fn(), warning: vi.fn() }, +})); +vi.mock('@/components/appstore/CategorySidebar', () => ({ CategorySidebar: () => null })); +// Both featured and grid surfaces select the template; whichever renders, one +// select button exists. +vi.mock('@/components/appstore/TemplateTile', () => ({ + TemplateTile: ({ template, onSelect }: { template: { title: string }; onSelect: (t: unknown) => void }) => ( + + ), +})); +vi.mock('@/components/appstore/FeaturedHero', () => ({ + FeaturedHero: ({ template, onOpen }: { template: { title: string }; onOpen: (t: unknown) => void }) => ( + + ), +})); +vi.mock('@/components/ui/system-sheet', () => ({ + SystemSheet: ({ open, primaryAction }: { open: boolean; primaryAction?: { onClick: () => void; disabled?: boolean } }) => + open && primaryAction + ? + : null, + SheetSection: ({ children }: { children?: ReactNode }) =>
{children}
, +})); + +import { apiFetch } from '@/lib/api'; +import { AppStoreView } from '../AppStoreView'; + +const TEMPLATE = { + title: 'Nginx', + description: 'web server', + categories: ['Web'], + env: [], + ports: [], + volumes: [], + architectures: [], +}; + +beforeEach(() => { + dfCtl.params = null; + vi.mocked(apiFetch).mockReset(); + vi.mocked(apiFetch).mockImplementation((url: string) => { + const u = String(url); + if (u.includes('/templates/deploy')) return Promise.resolve(jsonRes({})); + if (u.includes('/templates')) return Promise.resolve(jsonRes([TEMPLATE])); + if (u.includes('/stacks')) return Promise.resolve(jsonRes([])); + return Promise.resolve(jsonRes({})); + }); +}); + +describe('AppStoreView install node binding', () => { + it('binds both runWithLog and the /templates/deploy POST to the captured node', async () => { + render(); + + fireEvent.click((await screen.findAllByTestId('select-template'))[0]); + fireEvent.click(await screen.findByTestId('deploy')); + + await waitFor(() => { + const deployCall = vi.mocked(apiFetch).mock.calls.find(c => String(c[0]).includes('/templates/deploy')); + expect(deployCall?.[1]).toEqual(expect.objectContaining({ nodeId: 5 })); + }); + expect(dfCtl.params).toEqual(expect.objectContaining({ action: 'install', nodeId: 5 })); + }); +}); diff --git a/frontend/src/components/__tests__/DeployFeedbackModal.test.tsx b/frontend/src/components/__tests__/DeployFeedbackModal.test.tsx index 1d7be18e..f0b19a47 100644 --- a/frontend/src/components/__tests__/DeployFeedbackModal.test.tsx +++ b/frontend/src/components/__tests__/DeployFeedbackModal.test.tsx @@ -9,12 +9,14 @@ import { apiFetch } from '@/lib/api'; // Lets a test simulate a mid-stream drop (onReady then onError) so the panel // reaches 'streaming' with progressUnavailable set. -const ctl = vi.hoisted(() => ({ drop: false })); +const ctl = vi.hoisted(() => ({ drop: false, lastNodeId: undefined as number | null | undefined })); // The real Terminal mounts xterm + a WebSocket; mock it to a no-op that signals -// the stream connected on mount so the panel reaches the 'streaming' state. +// the stream connected on mount so the panel reaches the 'streaming' state, and +// records the captured nodeId it was mounted with. vi.mock('@/components/Terminal', () => { - const MockTerminal = ({ onReady, onError }: { onReady?: () => void; onError?: () => void }) => { + const MockTerminal = ({ onReady, onError, nodeId }: { onReady?: () => void; onError?: () => void; nodeId?: number | null }) => { + ctl.lastNodeId = nodeId; React.useEffect(() => { onReady?.(); if (ctl.drop) onError?.(); @@ -29,11 +31,13 @@ vi.mock('@/components/Terminal', () => { let resolveRun: ((r: { ok: boolean; errorMessage?: string; healthGateId?: string | null }) => void) | null = null; // The runWithLog promise itself, so a test can await full result propagation. let runOuter: Promise | null = null; +// Node the driver captures for the operation; default local, overridden per test. +let driverNodeId: number | null = null; function Driver() { const { runWithLog } = useDeployFeedback(); React.useEffect(() => { - runOuter = runWithLog({ stackName: 'web', action: 'update', nodeId: null }, async (started) => { + runOuter = runWithLog({ stackName: 'web', action: 'update', nodeId: driverNodeId }, async (started) => { await started; return new Promise<{ ok: boolean; errorMessage?: string; healthGateId?: string | null }>((res) => { resolveRun = res; }); }); @@ -77,6 +81,8 @@ describe('DeployFeedbackModal health gate', () => { localStorage.clear(); resolveRun = null; ctl.drop = false; + ctl.lastNodeId = undefined; + driverNodeId = null; vi.mocked(apiFetch).mockReset(); vi.mocked(apiFetch).mockResolvedValue(new Response('{}', { status: 200 })); }); @@ -97,6 +103,12 @@ describe('DeployFeedbackModal health gate', () => { }); } + it('binds the modal progress terminal to the captured panel node', async () => { + driverNodeId = 5; + await renderStreaming(); + expect(ctl.lastNodeId).toBe(5); + }); + it('shows the observing banner and suspends auto-close while the gate observes', async () => { routeGateApi([{ id: 'gate-1', status: 'observing' }]); await succeedWithGate('gate-1'); diff --git a/frontend/src/components/__tests__/DeployFeedbackPortal.test.tsx b/frontend/src/components/__tests__/DeployFeedbackPortal.test.tsx index 33feae02..d2591425 100644 --- a/frontend/src/components/__tests__/DeployFeedbackPortal.test.tsx +++ b/frontend/src/components/__tests__/DeployFeedbackPortal.test.tsx @@ -35,7 +35,13 @@ vi.mock('../DeployFeedbackModal', () => ({ DeployFeedbackModal: () =>
({ DeployFeedbackPill: ({ isVisible }: { isVisible: boolean }) => (isVisible ?
: null), })); -vi.mock('../Terminal', () => ({ default: () =>
})); +const termSpy = vi.hoisted(() => ({ nodeId: undefined as number | null | undefined })); +vi.mock('../Terminal', () => ({ + default: (props: { nodeId?: number | null }) => { + termSpy.nodeId = props.nodeId; + return
; + }, +})); function panel(over: Partial = {}): DeployPanelState { return { @@ -61,6 +67,13 @@ describe('DeployFeedbackPortal', () => { expect(screen.getByTestId('portal-terminal')).toBeInTheDocument(); }); + it('binds the inline progress terminal to the captured panel node', () => { + mockStyle = 'inline'; + mockPanelState = panel({ isOpen: true, nodeId: 7 }); + render(); + expect(termSpy.nodeId).toBe(7); + }); + it('does not mount the portal terminal in modal style (the modal owns it)', () => { mockStyle = 'modal'; mockPanelState = panel({ isOpen: true }); diff --git a/frontend/src/components/__tests__/Terminal.test.tsx b/frontend/src/components/__tests__/Terminal.test.tsx new file mode 100644 index 00000000..fc249beb --- /dev/null +++ b/frontend/src/components/__tests__/Terminal.test.tsx @@ -0,0 +1,102 @@ +/** + * Unit tests for TerminalComponent's WebSocket URL construction, specifically + * that a captured `nodeId` prop binds the socket to that node (number, or null + * for local) and that an absent prop falls back to the active node. This is the + * progress-stream half of keeping a deploy bound to the node it started on. + */ +import { render, waitFor, cleanup } from '@testing-library/react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +// The real loader pulls the ~660 KB xterm chunk; stub it so init resolves +// synchronously with no-op terminal/addons and the socket is built immediately. +vi.mock('@/lib/xtermLoader', () => { + class FakeTerminal { + loadAddon() {} + open() {} + attachCustomKeyEventHandler() {} + write() {} + dispose() {} + } + class FakeFit { fit() {} } + class FakeSearch { findNext() {} findPrevious() {} } + class FakeSerialize { serialize() { return ''; } } + return { + loadXtermModules: vi.fn().mockResolvedValue({ + Terminal: FakeTerminal, + FitAddon: FakeFit, + SearchAddon: FakeSearch, + SerializeAddon: FakeSerialize, + }), + }; +}); +vi.mock('@/lib/terminalTheme', () => ({ buildXtermTheme: () => ({}) })); + +import TerminalComponent from '../Terminal'; + +class MockWS { + static instances: MockWS[] = []; + url: string; + onopen: (() => void) | null = null; + onmessage: ((e: { data: string }) => void) | null = null; + onclose: (() => void) | null = null; + onerror: ((e?: unknown) => void) | null = null; + send = vi.fn(); + close = vi.fn(); + constructor(url: string) { this.url = url; MockWS.instances.push(this); } + static reset() { MockWS.instances = []; } +} + +beforeEach(() => { + MockWS.reset(); + vi.stubGlobal('WebSocket', MockWS); + localStorage.setItem('sencho-active-node', '9'); +}); + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + localStorage.removeItem('sencho-active-node'); + vi.clearAllMocks(); +}); + +// Mount and resolve the socket URL. Init runs after a 50ms timer + the async +// loader, so poll until the socket is constructed rather than racing it. +async function urlFor(props: { nodeId?: number | null; stackName?: string }): Promise { + render(); + await waitFor(() => expect(MockWS.instances.length).toBeGreaterThan(0)); + return MockWS.instances[0].url; +} + +describe('TerminalComponent WebSocket URL', () => { + it('binds the generic stream to an explicit numeric nodeId, overriding the active node', async () => { + const url = await urlFor({ nodeId: 7 }); + expect(url).toContain('/ws?nodeId=7'); + }); + + it('omits the nodeId query when nodeId is null (local), even with an active node set', async () => { + const url = await urlFor({ nodeId: null }); + expect(url).toMatch(/\/ws$/); + expect(url).not.toContain('nodeId='); + }); + + it('falls back to the active node when no nodeId prop is given', async () => { + const url = await urlFor({}); + expect(url).toContain('/ws?nodeId=9'); + }); + + it('binds the stack-logs stream to the captured nodeId', async () => { + const url = await urlFor({ nodeId: 7, stackName: 'web' }); + expect(url).toContain('/api/stacks/web/logs?nodeId=7'); + }); + + it('omits the nodeId query on the stack-logs stream when nodeId is null', async () => { + const url = await urlFor({ nodeId: null, stackName: 'web' }); + expect(url).toMatch(/\/api\/stacks\/web\/logs$/); + expect(url).not.toContain('nodeId='); + }); + + it('falls back to the active node on the stack-logs stream when no nodeId prop is given', async () => { + const url = await urlFor({ stackName: 'web' }); + expect(url).toContain('/api/stacks/web/logs?nodeId=9'); + }); +}); diff --git a/frontend/src/components/stack/GitSourcePanel.test.tsx b/frontend/src/components/stack/GitSourcePanel.test.tsx index 29e759df..2e298b64 100644 --- a/frontend/src/components/stack/GitSourcePanel.test.tsx +++ b/frontend/src/components/stack/GitSourcePanel.test.tsx @@ -5,14 +5,36 @@ * treating the sentinel as a configured source. */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { render, screen, waitFor } from '@testing-library/react'; +import { render, screen, waitFor, fireEvent } from '@testing-library/react'; + +// Mutable controls so a deploy-mode test can set the active node and capture the +// runWithLog params, while the load tests keep the default (no active node). +const nodeCtl = vi.hoisted(() => ({ activeNode: null as { id: number; type?: string } | null })); +const dfCtl = vi.hoisted(() => ({ params: null as null | { stackName: string; action: string; nodeId: number | null } })); vi.mock('@/lib/api', () => ({ apiFetch: vi.fn() })); vi.mock('@/context/DeployFeedbackContext', () => ({ - useDeployFeedback: () => ({ runWithLog: vi.fn() }), + useDeployFeedback: () => ({ + runWithLog: vi.fn( + async ( + params: { stackName: string; action: string; nodeId: number | null }, + run: (started: Promise) => Promise<{ ok: boolean }>, + ) => { + dfCtl.params = params; + return run(Promise.resolve()); + }, + ), + }), })); vi.mock('@/context/NodeContext', () => ({ - useNodes: () => ({ activeNode: null }), + useNodes: () => ({ activeNode: nodeCtl.activeNode }), +})); +// Drive applyPull(commitSha, deploy=true) directly without standing up the real +// diff UI; the panel passes applyPull as onApply. +vi.mock('./GitSourceDiffDialog', () => ({ + GitSourceDiffDialog: ({ onApply }: { onApply: (sha: string, deploy: boolean) => void }) => ( + + ), })); vi.mock('@/components/ui/toast-store', () => ({ toast: { @@ -64,6 +86,8 @@ function panel() { beforeEach(() => { vi.mocked(apiFetch).mockReset(); + nodeCtl.activeNode = null; + dfCtl.params = null; }); describe('GitSourcePanel load', () => { @@ -95,3 +119,21 @@ describe('GitSourcePanel load', () => { ); }); }); + +describe('GitSourcePanel deploy-mode apply node binding', () => { + beforeEach(() => { + nodeCtl.activeNode = { id: 4, type: 'local' }; + vi.mocked(apiFetch).mockResolvedValue(jsonRes({ applied: true, deployed: true })); + }); + + it('binds both runWithLog and the apply POST to the captured node when deploying', async () => { + render(panel()); + fireEvent.click(await screen.findByTestId('apply-deploy')); + + await waitFor(() => { + const applyCall = vi.mocked(apiFetch).mock.calls.find(c => String(c[0]).includes('/git-source/apply')); + expect(applyCall?.[1]).toEqual(expect.objectContaining({ nodeId: 4 })); + }); + expect(dfCtl.params).toEqual(expect.objectContaining({ action: 'deploy', nodeId: 4 })); + }); +}); diff --git a/frontend/src/components/stack/GitSourcePanel.tsx b/frontend/src/components/stack/GitSourcePanel.tsx index 54519480..814205dd 100644 --- a/frontend/src/components/stack/GitSourcePanel.tsx +++ b/frontend/src/components/stack/GitSourcePanel.tsx @@ -230,11 +230,15 @@ export function GitSourcePanel({ const applyPull = async (commitSha: string, deploy: boolean) => { setApplying(true); const loadingId = toast.loading(deploy ? 'Applying and deploying...' : 'Applying changes...'); + // Snapshot the node once so the apply (and any deploy it triggers) stays + // bound to it even if the active node changes while the operation runs. + const opNodeId = activeNode?.id ?? null; try { const runApply = async (started: Promise) => { if (deploy) await started; const res = await apiFetch(`/stacks/${encodeURIComponent(stackName)}/git-source/apply`, { method: 'POST', + nodeId: opNodeId, body: JSON.stringify({ commitSha, deploy }), }); if (res.ok) { @@ -260,7 +264,7 @@ export function GitSourcePanel({ }; if (deploy) { - await runWithLog({ stackName, action: 'deploy', nodeId: activeNode?.id ?? null }, runApply); + await runWithLog({ stackName, action: 'deploy', nodeId: opNodeId }, runApply); } else { await runApply(Promise.resolve()); } diff --git a/frontend/src/context/DeployFeedbackContext.tsx b/frontend/src/context/DeployFeedbackContext.tsx index 034a5654..2dc220d1 100644 --- a/frontend/src/context/DeployFeedbackContext.tsx +++ b/frontend/src/context/DeployFeedbackContext.tsx @@ -63,6 +63,10 @@ interface RunResult { /** Post-update health gate state for the current deploy session. */ export interface HealthGateUiState { stackName: string; + /** Node the gate runs on (null = local), captured from the operation. Polled + * on this node and matched against the active node before its failure routes + * into recovery, so a same-named stack on another node never inherits it. */ + nodeId: number | null; gateId: string; trigger: 'update' | 'deploy'; status: 'observing' | 'passed' | 'failed' | 'unknown'; @@ -257,9 +261,9 @@ export function DeployFeedbackProvider({ children }: { children: React.ReactNode // instead of this session showing a newer run's result. Mirrors the backend // gate's own degradation: repeated failures (or an absurdly long poll) // resolve to an honest client-side unknown rather than observing forever. - const startGatePolling = useCallback((stackName: string, gateId: string, trigger: 'update' | 'deploy', mySession: number) => { + const startGatePolling = useCallback((stackName: string, nodeId: number | null, gateId: string, trigger: 'update' | 'deploy', mySession: number) => { stopGatePolling(); - setHealthGate({ stackName, gateId, trigger, status: 'observing', reason: null, windowSeconds: null, startedAt: null }); + setHealthGate({ stackName, nodeId, gateId, trigger, status: 'observing', reason: null, windowSeconds: null, startedAt: null }); let strikes = 0; // Single-flight: skip a tick while one request is outstanding so two // overlapping responses cannot land out of order. @@ -286,7 +290,7 @@ export function DeployFeedbackProvider({ children }: { children: React.ReactNode } inFlight = true; try { - const res = await apiFetch(`/stacks/${stackName}/health-gate?gateId=${encodeURIComponent(gateId)}`); + const res = await apiFetch(`/stacks/${stackName}/health-gate?gateId=${encodeURIComponent(gateId)}`, { nodeId }); const report = res.ok ? await res.json() as { id: string | null; @@ -306,7 +310,7 @@ export function DeployFeedbackProvider({ children }: { children: React.ReactNode return; } strikes = 0; - setHealthGate({ stackName, gateId, trigger, status: report.status, reason: report.reason, windowSeconds: report.windowSeconds, startedAt: report.startedAt }); + setHealthGate({ stackName, nodeId, gateId, trigger, status: report.status, reason: report.reason, windowSeconds: report.windowSeconds, startedAt: report.startedAt }); if (report.status !== 'observing') { settled = true; stopGatePolling(); @@ -414,7 +418,7 @@ export function DeployFeedbackProvider({ children }: { children: React.ReactNode errorMessage: result.ok ? undefined : result.errorMessage, })); if (result.ok && result.healthGateId && (params.action === 'update' || params.action === 'deploy')) { - startGatePolling(params.stackName, result.healthGateId, params.action, mySession); + startGatePolling(params.stackName, params.nodeId, result.healthGateId, params.action, mySession); } } diff --git a/frontend/src/context/__tests__/DeployFeedbackContext.test.tsx b/frontend/src/context/__tests__/DeployFeedbackContext.test.tsx index dbcef831..aa15d32a 100644 --- a/frontend/src/context/__tests__/DeployFeedbackContext.test.tsx +++ b/frontend/src/context/__tests__/DeployFeedbackContext.test.tsx @@ -5,6 +5,14 @@ import { DeployFeedbackProvider, useDeployFeedback } from '../DeployFeedbackCont import { DEPLOY_FEEDBACK_KEY } from '@/hooks/use-deploy-feedback-enabled'; import { DEPLOY_FEEDBACK_STYLE_KEY } from '@/hooks/use-deploy-feedback-style'; +// Only the health-gate poll touches the network; mock apiFetch so a poll can be +// asserted to target the captured node. Other exports stay real. +vi.mock('@/lib/api', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, apiFetch: vi.fn() }; +}); +import { apiFetch } from '@/lib/api'; + function wrapper({ children }: { children: ReactNode }) { return {children}; } @@ -12,6 +20,7 @@ function wrapper({ children }: { children: ReactNode }) { describe('DeployFeedbackContext', () => { beforeEach(() => { localStorage.setItem(DEPLOY_FEEDBACK_KEY, 'true'); + vi.mocked(apiFetch).mockReset(); }); afterEach(() => { localStorage.clear(); @@ -168,6 +177,38 @@ describe('DeployFeedbackContext', () => { await act(async () => { result.current.onTerminalReady(); await outer; }); }); + it('polls the health gate on the captured node and stamps it on the gate state', async () => { + vi.mocked(apiFetch).mockResolvedValue(new Response( + JSON.stringify({ id: 'g1', status: 'observing', reason: null, windowSeconds: 90, startedAt: Date.now() }), + { status: 200 }, + )); + const { result } = renderHook(() => useDeployFeedback(), { wrapper }); + + let outer: Promise | undefined; + await act(async () => { + outer = result.current.runWithLog({ stackName: 'web', action: 'update', nodeId: 7 }, async (started) => { + await started; + return { ok: true, healthGateId: 'g1' }; + }); + await Promise.resolve(); + }); + + await act(async () => { + result.current.onTerminalReady(); + await outer; + // Let the immediate gate tick's apiFetch resolve and its setHealthGate land. + await Promise.resolve(); + await Promise.resolve(); + }); + + const gateCall = vi.mocked(apiFetch).mock.calls.find((c) => String(c[0]).includes('/health-gate')); + expect(gateCall).toBeDefined(); + expect(gateCall?.[1]).toEqual(expect.objectContaining({ nodeId: 7 })); + expect(result.current.healthGate?.nodeId).toBe(7); + + act(() => { result.current.onPanelClose(); }); + }); + it('minimized is false before any session', () => { const { result } = renderHook(() => useDeployFeedback(), { wrapper }); expect(result.current.minimized).toBe(false); diff --git a/frontend/src/lib/__tests__/api.test.ts b/frontend/src/lib/__tests__/api.test.ts index 93b073f1..c505d871 100644 --- a/frontend/src/lib/__tests__/api.test.ts +++ b/frontend/src/lib/__tests__/api.test.ts @@ -75,6 +75,50 @@ describe('apiFetch header merge', () => { }); }); +describe('apiFetch nodeId override', () => { + it('targets the explicit node, overriding a different active node', async () => { + localStorage.setItem('sencho-active-node', '7'); + await apiFetch('/stacks/foo/deploy', { method: 'POST', nodeId: 3 }); + const init = lastFetchInit(); + expect((init.headers as Record)['x-node-id']).toBe('3'); + localStorage.removeItem('sencho-active-node'); + }); + + it('targets the local node (omits x-node-id) when nodeId is null, even with an active node set', async () => { + localStorage.setItem('sencho-active-node', '7'); + await apiFetch('/stacks/foo/deploy', { method: 'POST', nodeId: null }); + const init = lastFetchInit(); + expect((init.headers as Record)['x-node-id']).toBeUndefined(); + localStorage.removeItem('sencho-active-node'); + }); + + it('falls back to the active node when nodeId is undefined', async () => { + localStorage.setItem('sencho-active-node', '7'); + await apiFetch('/stacks/foo/deploy', { method: 'POST' }); + const init = lastFetchInit(); + expect((init.headers as Record)['x-node-id']).toBe('7'); + localStorage.removeItem('sencho-active-node'); + }); + + it('does not leak the nodeId option onto the outgoing fetch init', async () => { + await apiFetch('/stacks/foo/deploy', { method: 'POST', nodeId: 3 }); + const init = lastFetchInit() as RequestInit & { nodeId?: unknown }; + expect(init.nodeId).toBeUndefined(); + }); + + it('keeps an explicit nodeId authoritative over a caller-supplied x-node-id header', async () => { + await apiFetch('/stacks/foo/deploy', { method: 'POST', nodeId: 3, headers: { 'x-node-id': '99' } }); + const init = lastFetchInit(); + expect((init.headers as Record)['x-node-id']).toBe('3'); + }); + + it('drops a caller-supplied x-node-id when the explicit nodeId is null (local)', async () => { + await apiFetch('/stacks/foo/deploy', { method: 'POST', nodeId: null, headers: { 'x-node-id': '99' } }); + const init = lastFetchInit(); + expect((init.headers as Record)['x-node-id']).toBeUndefined(); + }); +}); + describe('withDeploySession', () => { it('uses the canonical header name (kept in sync with the backend)', () => { expect(DEPLOY_SESSION_HEADER).toBe('x-deploy-session-id'); diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index b2750d3d..a49fe994 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -4,6 +4,13 @@ export interface ApiFetchOptions extends RequestInit { /** When true, omits the x-node-id header so the request always targets * the local node regardless of which node is currently active in the UI. */ localOnly?: boolean; + /** Explicit node target, overriding the active-node read. A number targets + * that node; null omits x-node-id so the request hits the local node. Leave + * undefined for the default (read the active node from localStorage). Lets an + * operation stay bound to the node captured when it started, even if the + * active node changes mid-flight. Takes precedence over localOnly when both + * are set. */ + nodeId?: number | null; } /** Header carrying a deploy's progress-stream correlation id. Mirrors the @@ -31,17 +38,35 @@ export async function apiFetch( endpoint: string, options: ApiFetchOptions = {} ): Promise { - const { localOnly, ...fetchOptions } = options; + const { localOnly, nodeId: nodeIdOverride, ...fetchOptions } = options; const url = `${API_BASE}${endpoint}`; - const activeNodeId = localOnly ? null : localStorage.getItem('sencho-active-node'); + // An explicit nodeId (including null) wins over the active-node read so a + // captured operation node stays authoritative; null means target the local + // node, not the stored active node. + let activeNodeId: string | null; + if (nodeIdOverride !== undefined) { + activeNodeId = nodeIdOverride === null ? null : String(nodeIdOverride); + } else if (localOnly) { + activeNodeId = null; + } else { + activeNodeId = localStorage.getItem('sencho-active-node'); + } + const headers: Record = { + 'Content-Type': 'application/json', + ...(activeNodeId ? { 'x-node-id': activeNodeId } : {}), + ...(fetchOptions.headers as Record | undefined), + }; + // An explicit nodeId is authoritative: a caller-supplied x-node-id header must + // not silently override the captured operation node (it would for the default + // active-node read, which is the pre-existing behavior left unchanged). + if (nodeIdOverride !== undefined) { + if (activeNodeId) headers['x-node-id'] = activeNodeId; + else delete headers['x-node-id']; + } const defaultOptions: RequestInit = { credentials: 'include', - headers: { - 'Content-Type': 'application/json', - ...(activeNodeId ? { 'x-node-id': activeNodeId } : {}), - ...fetchOptions.headers, - }, + headers, }; // Drop headers from fetchOptions before the outer spread so the merged