From 2821e87d3f5e95185c5e9d6845e2287c83d66b28 Mon Sep 17 00:00:00 2001 From: Anso Date: Thu, 18 Jun 2026 23:29:20 -0400 Subject: [PATCH] fix: gate fleet stop-by-label on a resolved preview and validate remote stop responses (#1392) * fix: gate fleet stop-by-label on a resolved preview and validate remote stop responses The destructive "Stop fleet by label" action could run before its blast radius was known, and a malformed remote response could be rendered as a successful zero-stack stop. Both are release blockers for the fleet stop-by-label flow. Gate the "Stop fleet" button on a resolved blast radius: the live match-preview resolving to at least one matching stack, or a dry run of the current label when the preview endpoint is unavailable. Carry the resolved node and stack list into the confirm modal so the operator confirms against the concrete targets rather than a label name alone. Editing the label invalidates a prior dry-run snapshot, so a stale blast radius cannot re-enable the action. Validate the remote local-stop 200 body before trusting it. A body that is not the local-stop contract (missing matched flag, non-array results, or a malformed result element) now fails that node with a clear error, consistent with the non-ok and unreachable paths, instead of defaulting matched to true and results to empty. * fix: ignore stale fleet stop-by-label preview responses after the label changes The debounced match-preview callback set the preview state unconditionally, so a request issued for one label that resolved after the operator switched to another label would mark the preview ready with the old label's blast radius. That re-enabled the destructive Stop and showed stale nodes/stacks in the confirm modal for a label that no longer matched them. Guard the in-flight request with a per-run cancelled flag flipped in the effect cleanup, so a response for a label that is no longer current cannot set the preview. This mirrors the cancellation pattern already used by the suggestions effect. Add a test that holds a preview request in-flight, switches the label, then resolves the stale response and asserts Stop stays disabled and the old targets do not leak. --- .../fleet-action-card-endpoints.test.ts | 66 ++++++- backend/src/helpers/fleetLabelStop.ts | 22 +++ backend/src/routes/fleet.ts | 19 +- docs/features/fleet-actions.mdx | 5 +- .../cards/LabelFleetStopCard.test.tsx | 184 ++++++++++++++++-- .../FleetActions/cards/LabelFleetStopCard.tsx | 87 ++++++++- 6 files changed, 359 insertions(+), 24 deletions(-) diff --git a/backend/src/__tests__/fleet-action-card-endpoints.test.ts b/backend/src/__tests__/fleet-action-card-endpoints.test.ts index 6ca415b8..45198341 100644 --- a/backend/src/__tests__/fleet-action-card-endpoints.test.ts +++ b/backend/src/__tests__/fleet-action-card-endpoints.test.ts @@ -609,7 +609,37 @@ describe('POST /api/fleet/labels/fleet-stop remote leg', () => { } }); - it('degrades a malformed 200 body (non-array results) to empty instead of forwarding it', async () => { + it('accepts a legitimate empty stop (matched:true, results:[]) without flagging it malformed', async () => { + const remoteId = db.addNode({ + name: 'remote-empty', type: 'remote', api_url: 'http://remote-empty.example:1852', + api_token: 'tok', compose_dir: '/app/compose', is_default: false, + }); + try { + const label = db.createLabel(remoteId, `remote-empty-${++labelCounter}`, 'teal'); + db.setStackLabels('alpha', remoteId, [label.id]); + // The local-stop receiver emits exactly this when the label exists with no + // assigned stacks. The guard must let it through, not over-reject it. + vi.spyOn(globalThis, 'fetch').mockResolvedValue({ + ok: true, status: 200, json: async () => ({ matched: true, results: [] }), + } as unknown as Response); + + const res = await request(app) + .post('/api/fleet/labels/fleet-stop') + .set('Authorization', authHeader) + .send({ labelName: label.name }); + + expect(res.status).toBe(200); + const remoteRow = res.body.results.find((r: { nodeId: number }) => r.nodeId === remoteId); + expect(remoteRow.reachable).toBe(true); + expect(remoteRow.matched).toBe(true); + expect(remoteRow.stackResults).toEqual([]); + expect(remoteRow.error).toBeUndefined(); + } finally { + db.deleteNode(remoteId); + } + }); + + it('fails a node whose 200 body has a non-array results, never rendering it as zero-stack success', async () => { const remoteId = db.addNode({ name: 'remote-malformed', type: 'remote', api_url: 'http://remote-malformed.example:1852', api_token: 'tok', compose_dir: '/app/compose', is_default: false, @@ -628,7 +658,41 @@ describe('POST /api/fleet/labels/fleet-stop remote leg', () => { expect(res.status).toBe(200); const remoteRow = res.body.results.find((r: { nodeId: number }) => r.nodeId === remoteId); + // A malformed contract is a node failure: reachable:false + error, not the + // matched:true/empty-results shape the UI reads as a successful no-op. + expect(remoteRow.reachable).toBe(false); + expect(remoteRow.matched).toBe(false); expect(remoteRow.stackResults).toEqual([]); + expect(remoteRow.error).toMatch(/malformed/i); + } finally { + db.deleteNode(remoteId); + } + }); + + it('fails a node whose 200 body omits the matched flag instead of defaulting it to true', async () => { + const remoteId = db.addNode({ + name: 'remote-no-matched', type: 'remote', api_url: 'http://remote-no-matched.example:1852', + api_token: 'tok', compose_dir: '/app/compose', is_default: false, + }); + try { + const label = db.createLabel(remoteId, `remote-no-matched-${++labelCounter}`, 'teal'); + db.setStackLabels('alpha', remoteId, [label.id]); + // The previous default (`matched: remote.matched ?? true`) turned this body + // into a matched:true/zero-stack success; the guard must fail the node. + vi.spyOn(globalThis, 'fetch').mockResolvedValue({ + ok: true, status: 200, json: async () => ({ results: [] }), + } as unknown as Response); + + const res = await request(app) + .post('/api/fleet/labels/fleet-stop') + .set('Authorization', authHeader) + .send({ labelName: label.name }); + + expect(res.status).toBe(200); + const remoteRow = res.body.results.find((r: { nodeId: number }) => r.nodeId === remoteId); + expect(remoteRow.reachable).toBe(false); + expect(remoteRow.matched).toBe(false); + expect(remoteRow.error).toMatch(/malformed/i); } finally { db.deleteNode(remoteId); } diff --git a/backend/src/helpers/fleetLabelStop.ts b/backend/src/helpers/fleetLabelStop.ts index 3933703e..45458af8 100644 --- a/backend/src/helpers/fleetLabelStop.ts +++ b/backend/src/helpers/fleetLabelStop.ts @@ -28,6 +28,28 @@ export interface LabelLocalStopResponse { results: StackStopResult[]; } +function isStackStopResult(value: unknown): value is StackStopResult { + if (typeof value !== 'object' || value === null) return false; + const r = value as Record; + return typeof r.stackName === 'string' && typeof r.success === 'boolean'; +} + +/** + * Validate a remote node's `local-stop` 200 body before the control trusts it. + * A response that is not this exact shape (a missing/non-boolean `matched`, a + * non-array `results`, or a malformed result element) is a remote contract + * failure, not an empty stop. The control-side fan-out fails that node rather + * than defaulting `matched` to true and `results` to [], which the UI would + * otherwise render as a successful zero-stack node and hide the failure. + */ +export function isLabelLocalStopResponse(value: unknown): value is LabelLocalStopResponse { + if (typeof value !== 'object' || value === null) return false; + const r = value as Record; + return typeof r.matched === 'boolean' + && Array.isArray(r.results) + && r.results.every(isStackStopResult); +} + /** * Run a label-name-matched container stop against one node's own local Docker. * diff --git a/backend/src/routes/fleet.ts b/backend/src/routes/fleet.ts index 370603b5..63c96eb2 100644 --- a/backend/src/routes/fleet.ts +++ b/backend/src/routes/fleet.ts @@ -37,7 +37,7 @@ import { CloudBackupService } from '../services/CloudBackupService'; import { NotificationService } from '../services/NotificationService'; import { invalidateNodeCaches, invalidateRemoteMetaCache } from '../helpers/cacheInvalidation'; import { activeBulkActions } from './labels'; -import { runLocalLabelStop, type LabelLocalStopResponse, type StackStopResult } from '../helpers/fleetLabelStop'; +import { runLocalLabelStop, isLabelLocalStopResponse, type StackStopResult } from '../helpers/fleetLabelStop'; import { collectFleetLabelSummaries } from '../helpers/fleetLabelSummary'; import { buildLocalConfigurationStatus, type ConfigurationStatus } from './dashboard'; import { buildLocalGraph, mergeFleetGraph, isLocalDependencyGraph, type FleetNodeGraphResult } from '../services/DependencyGraphService'; @@ -1320,14 +1320,19 @@ fleetRouter.post('/labels/fleet-stop', authMiddleware, async (req: Request, res: const err = (await response.json().catch(() => ({}))) as { error?: string }; return { nodeId: node.id, nodeName: node.name, reachable: false, matched: false, stackResults: [], error: err.error || `Remote returned ${response.status}` }; } - // Trust the remote's own matched flag. Guard results as an array so a - // malformed 200 body degrades to empty rather than flowing a non-array - // into the per-stack renderers. - const remote = (await response.json()) as Partial; + // A 200 with a body that isn't the local-stop contract is a remote + // failure, not an empty stop. Fail the node (matching the non-ok and + // unreachable paths above) instead of defaulting matched to true and + // results to [], which the UI renders as a successful zero-stack node + // and hides the remote contract break. + const remote = await response.json().catch(() => null); + if (!isLabelLocalStopResponse(remote)) { + return { nodeId: node.id, nodeName: node.name, reachable: false, matched: false, stackResults: [], error: 'Remote returned a malformed response' }; + } return { nodeId: node.id, nodeName: node.name, reachable: true, - matched: remote.matched ?? true, - stackResults: Array.isArray(remote.results) ? remote.results : [], + matched: remote.matched, + stackResults: remote.results, }; } catch (err) { return { nodeId: node.id, nodeName: node.name, reachable: false, matched: false, stackResults: [], error: getErrorMessage(err, 'Failed to reach remote node') }; diff --git a/docs/features/fleet-actions.mdx b/docs/features/fleet-actions.mdx index c22161f7..04bde43b 100644 --- a/docs/features/fleet-actions.mdx +++ b/docs/features/fleet-actions.mdx @@ -47,8 +47,9 @@ Stop every stack assigned a given **stack label** on every node where that stack 1. Open **Fleet → Actions**. 2. Type a name in the **Stack label** field. The picker queries each reachable node for its own stack labels and lists each name once with its combined stack and node counts (and the carrying node names), so the scope is unmistakable. A stack label that exists only on a remote node still appears. When a node cannot be reached, the picker notes that the suggestions may be incomplete. You can also type a name by hand. -3. Click **Stop fleet**. -4. A confirmation appears with the kicker **Fleet stop** and the title `Stop all stacks with the stack label ""?`. Click **Stop fleet** to commit. +3. As you type, a live preview resolves the blast radius across the fleet and the card lists which nodes and stacks match. **Stop fleet** stays disabled until that preview resolves to at least one matching stack, so the action can never run before you can see what it will hit. If the live preview is unavailable, **Dry run** rehearses the same fan-out without stopping anything and resolves the blast radius the same way. +4. Click **Stop fleet**. +5. A confirmation appears with the kicker **Fleet stop**, the title `Stop all stacks with the stack label ""?`, and the resolved list of nodes and stacks that will be stopped. Review it, then click **Stop fleet** to commit. Fleet stop confirmation dialog. Kicker 'Fleet stop' in red mono, italic title 'Stop all stacks with the stack label "docs-preview"?', Cancel and Stop fleet buttons in the footer. diff --git a/frontend/src/components/fleet/FleetActions/cards/LabelFleetStopCard.test.tsx b/frontend/src/components/fleet/FleetActions/cards/LabelFleetStopCard.test.tsx index e2a1a642..d9644e33 100644 --- a/frontend/src/components/fleet/FleetActions/cards/LabelFleetStopCard.test.tsx +++ b/frontend/src/components/fleet/FleetActions/cards/LabelFleetStopCard.test.tsx @@ -10,7 +10,7 @@ * zero-stack preview, and a non-ok suggestions response is non-fatal. */ import { it, expect, vi, beforeEach } from 'vitest'; -import { render, screen, waitFor, within } from '@testing-library/react'; +import { render, screen, waitFor, within, act } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; vi.mock('@/lib/api', () => ({ @@ -56,12 +56,14 @@ it('disables both actions until a label name is entered', () => { expect(screen.getByRole('button', { name: 'Dry run' })).toBeDisabled(); }); -it('enables the actions once a label is typed', async () => { +it('enables Dry run on a label name but gates Stop fleet until the blast radius resolves', async () => { const user = userEvent.setup(); + // The default mocks 404 every endpoint, so the match-preview never resolves. render(); await user.type(screen.getByPlaceholderText('e.g. production'), 'prod'); - expect(screen.getByRole('button', { name: 'Stop fleet' })).toBeEnabled(); expect(screen.getByRole('button', { name: 'Dry run' })).toBeEnabled(); + // No resolved preview (or dry run) yet, so the destructive Stop stays disabled. + expect(screen.getByRole('button', { name: 'Stop fleet' })).toBeDisabled(); }); it('labels the target as a stack label and explains node labels are excluded', () => { @@ -132,7 +134,7 @@ it('populates the input when a suggestion is selected', async () => { await user.click(input); await user.click(await screen.findByRole('button', { name: /production/ })); expect(input.value).toBe('production'); - expect(screen.getByRole('button', { name: 'Stop fleet' })).toBeEnabled(); + expect(screen.getByRole('button', { name: 'Dry run' })).toBeEnabled(); }); it('shows a zero-stack preview when a node-only name is typed', async () => { @@ -161,7 +163,7 @@ it('keeps the card usable when the suggestions endpoint returns 403', async () = render(); // No crash, no suggestions, and the operator can still type a name by hand. await user.type(screen.getByPlaceholderText('e.g. production'), 'prod'); - expect(screen.getByRole('button', { name: 'Stop fleet' })).toBeEnabled(); + expect(screen.getByRole('button', { name: 'Dry run' })).toBeEnabled(); }); it('dry run calls fleet-stop with dryRun:true and never opens the confirm modal', async () => { @@ -187,9 +189,15 @@ it('dry run calls fleet-stop with dryRun:true and never opens the confirm modal' await waitFor(() => expect(toastSuccess).toHaveBeenCalled()); }); -it('stop fleet opens the confirm modal, and confirming runs a real stop that renders per-node results', async () => { +it('gates Stop fleet on a resolved preview, carries the node/stack list into the modal, then runs a real stop', async () => { const user = userEvent.setup(); mockedFetch.mockImplementation((url: string) => { + if (url === '/fleet/labels/match-preview') { + return Promise.resolve(jsonResponse(200, { + matchedNodes: 1, matchedStacks: 1, unreachableNodes: 0, + perNode: [{ nodeId: 1, nodeName: 'central', reachable: true, labelExists: true, stackCount: 1, stackNames: ['web'] }], + })); + } if (url === '/fleet/labels/fleet-stop') { return Promise.resolve(jsonResponse(200, { results: [ @@ -202,12 +210,20 @@ it('stop fleet opens the confirm modal, and confirming runs a real stop that ren }); render(); await user.type(screen.getByPlaceholderText('e.g. production'), 'prod'); - await user.click(screen.getByRole('button', { name: 'Stop fleet' })); + + // Stop fleet only enables once the live preview resolves the blast radius. + const stopBtn = screen.getByRole('button', { name: 'Stop fleet' }); + await waitFor(() => expect(stopBtn).toBeEnabled(), { timeout: 2000 }); + await user.click(stopBtn); const dialog = await screen.findByRole('alertdialog'); expect(within(dialog).getByText('Stop all stacks with the stack label "prod"?')).toBeInTheDocument(); + // The modal carries the resolved node/stack list, not just the label name. + expect(within(dialog).getByText(/Will stop 1 stack across 1 node/)).toBeInTheDocument(); + expect(within(dialog).getByText('central')).toBeInTheDocument(); + expect(within(dialog).getByText('web')).toBeInTheDocument(); - // The real stop must not have fired yet. + // The real stop must not have fired yet (only the preview hit the network). expect(mockedFetch.mock.calls.find(c => c[0] === '/fleet/labels/fleet-stop')).toBeFalsy(); await user.click(within(dialog).getByRole('button', { name: 'Stop fleet' })); @@ -216,10 +232,41 @@ it('stop fleet opens the confirm modal, and confirming runs a real stop that ren const stopCall = mockedFetch.mock.calls.find(c => c[0] === '/fleet/labels/fleet-stop'); expect(JSON.parse(stopCall![1].body)).toEqual({ labelName: 'prod', dryRun: false }); }); - expect(await screen.findByText('web')).toBeInTheDocument(); + // Per-node results render in the card (the "· 1 stack" label is unique to the + // results list, distinguishing it from the preview well). + expect(await screen.findByText('central · 1 stack')).toBeInTheDocument(); await waitFor(() => expect(toastSuccess).toHaveBeenCalled()); }); +it('lists every node and stack in the confirm modal and sums them across nodes', async () => { + const user = userEvent.setup(); + mockedFetch.mockImplementation((url: string) => { + if (url === '/fleet/labels/match-preview') { + return Promise.resolve(jsonResponse(200, { + matchedNodes: 2, matchedStacks: 3, unreachableNodes: 0, + perNode: [ + { nodeId: 1, nodeName: 'central', reachable: true, labelExists: true, stackCount: 1, stackNames: ['web'] }, + { nodeId: 2, nodeName: 'edge-1', reachable: true, labelExists: true, stackCount: 2, stackNames: ['api', 'db'] }, + ], + })); + } + return Promise.resolve(jsonResponse(404, {})); + }); + render(); + await user.type(screen.getByPlaceholderText('e.g. production'), 'prod'); + const stopBtn = screen.getByRole('button', { name: 'Stop fleet' }); + await waitFor(() => expect(stopBtn).toBeEnabled(), { timeout: 2000 }); + await user.click(stopBtn); + + const dialog = await screen.findByRole('alertdialog'); + // The reduce + pluralization across nodes is the only computed display logic. + expect(within(dialog).getByText(/Will stop 3 stacks across 2 nodes/)).toBeInTheDocument(); + expect(within(dialog).getByText('central')).toBeInTheDocument(); + expect(within(dialog).getByText('edge-1')).toBeInTheDocument(); + expect(within(dialog).getByText('web')).toBeInTheDocument(); + expect(within(dialog).getByText('api, db')).toBeInTheDocument(); +}); + it('surfaces an error toast when fleet-stop returns a non-ok response', async () => { const user = userEvent.setup(); mockedFetch.mockImplementation((url: string) => { @@ -259,8 +306,123 @@ it('degrades to "preview unavailable" when match-preview returns a malformed 200 render(); await user.type(screen.getByPlaceholderText('e.g. production'), 'prod'); expect(await screen.findByText('preview endpoint did not respond', undefined, { timeout: 2000 })).toBeInTheDocument(); - // The card stays interactive; no crash from the missing perNode array. - expect(screen.getByRole('button', { name: 'Stop fleet' })).toBeEnabled(); + // The card stays interactive (no crash from the missing perNode array), but the + // destructive Stop stays gated while the preview is unavailable; Dry run is the + // escape hatch that can still resolve the blast radius. + expect(screen.getByRole('button', { name: 'Dry run' })).toBeEnabled(); + expect(screen.getByRole('button', { name: 'Stop fleet' })).toBeDisabled(); +}); + +it('keeps Stop fleet disabled when the preview resolves to zero matching stacks', async () => { + const user = userEvent.setup(); + mockedFetch.mockImplementation((url: string) => { + if (url === '/fleet/labels/match-preview') { + return Promise.resolve(jsonResponse(200, { matchedNodes: 0, matchedStacks: 0, unreachableNodes: 0, perNode: [] })); + } + return Promise.resolve(jsonResponse(404, {})); + }); + render(); + await user.type(screen.getByPlaceholderText('e.g. production'), 'edge'); + await waitFor(() => expect(screen.getByText('0 matching stacks')).toBeInTheDocument(), { timeout: 2000 }); + // A resolved-but-empty preview is nothing to stop, so the destructive Stop + // stays disabled even though the blast radius is fully resolved. + expect(screen.getByRole('button', { name: 'Stop fleet' })).toBeDisabled(); +}); + +it('lets a successful dry run unblock Stop fleet when the match-preview endpoint is unavailable', async () => { + const user = userEvent.setup(); + mockedFetch.mockImplementation((url: string) => { + if (url === '/fleet/labels/fleet-stop') { + return Promise.resolve(jsonResponse(200, { + results: [{ nodeId: 1, nodeName: 'central', reachable: true, matched: true, stackResults: [{ stackName: 'web', success: true, dryRun: true }] }], + })); + } + // match-preview (and everything else) is unavailable. + return Promise.resolve(jsonResponse(404, {})); + }); + render(); + await user.type(screen.getByPlaceholderText('e.g. production'), 'prod'); + const stopBtn = screen.getByRole('button', { name: 'Stop fleet' }); + expect(stopBtn).toBeDisabled(); + + // A dry run resolves the blast radius even with no live preview. + await user.click(screen.getByRole('button', { name: 'Dry run' })); + await waitFor(() => expect(stopBtn).toBeEnabled()); + + // And the confirm modal it opens carries the dry-run-resolved targets. + await user.click(stopBtn); + const dialog = await screen.findByRole('alertdialog'); + expect(within(dialog).getByText('central')).toBeInTheDocument(); + expect(within(dialog).getByText('web')).toBeInTheDocument(); +}); + +it('invalidates a dry-run snapshot when the label is edited, even back to the same name', async () => { + const user = userEvent.setup(); + mockedFetch.mockImplementation((url: string) => { + if (url === '/fleet/labels/fleet-stop') { + return Promise.resolve(jsonResponse(200, { + results: [{ nodeId: 1, nodeName: 'central', reachable: true, matched: true, stackResults: [{ stackName: 'web', success: true, dryRun: true }] }], + })); + } + // match-preview stays unavailable, so only the dry-run snapshot can resolve. + return Promise.resolve(jsonResponse(404, {})); + }); + render(); + const input = screen.getByPlaceholderText('e.g. production'); + await user.type(input, 'prod'); + const stopBtn = screen.getByRole('button', { name: 'Stop fleet' }); + + // Resolve via dry run, then edit away and back to the same label. + await user.click(screen.getByRole('button', { name: 'Dry run' })); + await waitFor(() => expect(stopBtn).toBeEnabled()); + await user.clear(input); + await user.type(input, 'prod'); + + // The stale snapshot must not re-enable Stop; a fresh dry run/preview is required. + await waitFor(() => expect(stopBtn).toBeDisabled()); +}); + +it('ignores a stale in-flight preview response that resolves after the label changed', async () => { + const user = userEvent.setup(); + let resolveProd: ((r: Response) => void) | null = null; + mockedFetch.mockImplementation((url: string, init?: { body?: string }) => { + if (url === '/fleet/labels/match-preview') { + const label = JSON.parse(init?.body ?? '{}').labelName; + if (label === 'prod') { + // Hold the prod preview open so it resolves AFTER the label changes. + return new Promise((resolve) => { resolveProd = resolve; }); + } + if (label === 'qa') { + return Promise.resolve(jsonResponse(200, { matchedNodes: 0, matchedStacks: 0, unreachableNodes: 0, perNode: [] })); + } + } + return Promise.resolve(jsonResponse(404, {})); + }); + render(); + const input = screen.getByPlaceholderText('e.g. production'); + + // Type prod and wait for its debounced preview request to actually go in-flight. + await user.type(input, 'prod'); + await waitFor(() => expect(resolveProd).not.toBeNull(), { timeout: 2000 }); + + // Switch to qa; its preview resolves to zero matches, so Stop is disabled. + await user.clear(input); + await user.type(input, 'qa'); + await waitFor(() => expect(screen.getByText('0 matching stacks')).toBeInTheDocument(), { timeout: 2000 }); + expect(screen.getByRole('button', { name: 'Stop fleet' })).toBeDisabled(); + + // The stale prod preview now resolves with a matching stack. It must not gate + // Stop for qa, nor leak prod's targets into the readout/preview. + await act(async () => { + resolveProd!(jsonResponse(200, { + matchedNodes: 1, matchedStacks: 1, unreachableNodes: 0, + perNode: [{ nodeId: 1, nodeName: 'central', reachable: true, labelExists: true, stackCount: 1, stackNames: ['web'] }], + })); + await new Promise(r => setTimeout(r, 0)); + }); + expect(screen.getByRole('button', { name: 'Stop fleet' })).toBeDisabled(); + expect(screen.getByText('0 matching stacks')).toBeInTheDocument(); + expect(screen.queryByText('web')).not.toBeInTheDocument(); }); it('does not crash when fleet-stop returns a malformed 200 body', async () => { diff --git a/frontend/src/components/fleet/FleetActions/cards/LabelFleetStopCard.tsx b/frontend/src/components/fleet/FleetActions/cards/LabelFleetStopCard.tsx index 2667c2ca..0b00a92c 100644 --- a/frontend/src/components/fleet/FleetActions/cards/LabelFleetStopCard.tsx +++ b/frontend/src/components/fleet/FleetActions/cards/LabelFleetStopCard.tsx @@ -33,6 +33,11 @@ type PreviewState = | { kind: 'unavailable' } | { kind: 'ready'; data: MatchPreviewResponse }; +// One node and the stacks the resolved blast radius would stop on it. The +// destructive Stop stays disabled until this is known, so the operator always +// confirms against a concrete node/stack list rather than a label name alone. +interface ResolvedTarget { nodeName: string; stackNames: string[] } + const KICKER = 'font-mono text-[10px] uppercase tracking-[0.18em]'; const PREVIEW_ROW_LIMIT = 6; @@ -75,6 +80,10 @@ export function LabelFleetStopCard() { const [running, setRunning] = useState(false); const [results, setResults] = useState([]); const [preview, setPreview] = useState({ kind: 'idle' }); + // Snapshot of the most recent dry run's resolved targets, keyed by the label + // it ran for. Stands in for the live preview when the match-preview endpoint is + // unavailable, so a successful dry run still unblocks the real stop. + const [dryRunResolved, setDryRunResolved] = useState<{ label: string; targets: ResolvedTarget[] } | null>(null); // Stack-label suggestions for the target picker. The fleet endpoint queries // every node authoritatively (local DB + live remote reads), so node labels @@ -107,7 +116,17 @@ export function LabelFleetStopCard() { const debounceRef = useRef | null>(null); useEffect(() => { const trimmed = labelName.trim(); + // `cancelled` guards against an out-of-order response: once the debounce + // timer fires, the request is in-flight and clearing the timer no longer + // stops it. If the label changes before it resolves, this effect's cleanup + // flips `cancelled` so the stale response cannot set `preview` for a label + // that is no longer current and gate Stop against the wrong blast radius. + let cancelled = false; if (debounceRef.current) clearTimeout(debounceRef.current); + // Any edit to the label invalidates a prior dry run's snapshot, so editing + // away and back to the same name cannot re-enable Stop from a stale blast + // radius. A fresh preview or dry run must resolve the current label again. + setDryRunResolved(null); if (trimmed.length === 0) { setPreview({ kind: 'idle' }); return; @@ -119,6 +138,7 @@ export function LabelFleetStopCard() { method: 'POST', body: JSON.stringify({ labelName: trimmed }), }); + if (cancelled) return; if (res.status === 404) { setPreview({ kind: 'unavailable' }); return; @@ -128,6 +148,7 @@ export function LabelFleetStopCard() { return; } const data = await res.json().catch(() => null); + if (cancelled) return; if (!isMatchPreviewResponse(data)) { // A 200 with a shape we can't render is a server/contract bug, not a // transport miss; log it like the catch path so it's diagnosable. @@ -137,6 +158,7 @@ export function LabelFleetStopCard() { } setPreview({ kind: 'ready', data }); } catch (err) { + if (cancelled) return; // Non-destructive readout: the operator can still type a name and run the // stop, so we degrade to "unavailable" rather than toasting, but leave a // console trail so a recurring preview failure is diagnosable. @@ -145,6 +167,7 @@ export function LabelFleetStopCard() { } }, 500); return () => { + cancelled = true; if (debounceRef.current) clearTimeout(debounceRef.current); }; }, [labelName]); @@ -223,6 +246,14 @@ export function LabelFleetStopCard() { }; }); setResults(rows); + if (opts.dryRun) { + // Record what this dry run resolved so the real stop can be confirmed + // against it even when the live preview endpoint is down. + const targets = apiResults + .filter(n => n.reachable && n.matched && Array.isArray(n.stackResults) && n.stackResults.length > 0) + .map(n => ({ nodeName: n.nodeName, stackNames: n.stackResults.map(s => s.stackName) })); + setDryRunResolved({ label: trimmed, targets }); + } const reachable = apiResults.filter(n => n.reachable); const unreachableCount = apiResults.length - reachable.length; const matchedNodes = reachable.filter(n => n.matched).length; @@ -248,6 +279,28 @@ export function LabelFleetStopCard() { } const trimmed = labelName.trim(); + + // The resolved blast radius the operator confirms against. Sourced from the + // live preview, or from a dry run of the *current* label when the live preview + // has not resolved. Null until one resolves, which keeps the destructive Stop + // disabled until the affected nodes/stacks are known. + const resolvedTargets = useMemo(() => { + if (preview.kind === 'ready') { + return preview.data.perNode + .filter(n => n.reachable && n.stackCount > 0) + .map(n => ({ nodeName: n.nodeName, stackNames: n.stackNames })); + } + if (dryRunResolved && dryRunResolved.label === trimmed && trimmed.length > 0) { + return dryRunResolved.targets; + } + return null; + }, [preview, dryRunResolved, trimmed]); + + // The real Stop is enabled only once the blast radius is resolved to at least + // one stack, and never while a run is in flight. Loading/unavailable previews, + // 0-match results, and an invalidated dry-run snapshot all leave it disabled. + const canStopFleet = !running && resolvedTargets !== null && resolvedTargets.some(t => t.stackNames.length > 0); + const previewSection = renderPreviewSection(preview, trimmed); // Freshness placeholder: until a run-record store exists, the footer carries @@ -273,7 +326,7 @@ export function LabelFleetStopCard() { label: 'Stop fleet', onClick: () => setConfirmOpen(true), variant: 'destructive', - disabled: running || trimmed.length === 0, + disabled: !canStopFleet, }} footerContext={footerContext} > @@ -311,13 +364,16 @@ export function LabelFleetStopCard() { open={confirmOpen} onOpenChange={(open) => { if (!open) setConfirmOpen(false); }} variant="destructive" + size="md" kicker="Fleet stop" title={`Stop all stacks with the stack label "${trimmed}"?`} - description="Sencho will stop every stack assigned this stack label on every reachable node at execution time. Node labels are not used by this action. Services will be unavailable until restarted." + description="Sencho will stop the stacks listed below. Node labels are not used by this action. Services will be unavailable until restarted." confirmLabel="Stop fleet" confirming={running} onConfirm={() => run({ dryRun: false })} - /> + > + {resolvedTargets && resolvedTargets.length > 0 && } + ); } @@ -370,6 +426,31 @@ function renderPreviewSection(preview: PreviewState, trimmed: string) { return null; } +// The resolved node/stack list carried into the destructive confirm modal, so +// the operator confirms against the concrete blast radius rather than a label +// name. Final membership is re-resolved per node at execution time (state can +// drift between preview and confirm), which the header states plainly. +function ResolvedTargetsList({ targets }: { targets: ResolvedTarget[] }) { + const stackCount = targets.reduce((n, t) => n + t.stackNames.length, 0); + return ( +
+
+ Will stop {stackCount} stack{stackCount === 1 ? '' : 's'} across {targets.length} node{targets.length === 1 ? '' : 's'}. The final set is re-resolved on each node at execution time. +
+
+
    + {targets.map(t => ( +
  • + {t.nodeName} + {t.stackNames.join(', ')} +
    • + ))} +
+
+
+ ); +} + // Muted list of nodes Sencho could not query, shown beneath the preview so a // partial blast radius is observable rather than silently dropped. function UnreachableList({ nodes }: { nodes: MatchPreviewNode[] }) {