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.
2026-08-20 15:22:59 +00:00 · 2026-06-18 23:29:20 -04:00
parent ba09e6f69e
commit 2821e87d3f
6 changed files with 359 additions and 24 deletions
@@ -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(<LabelFleetStopCard />);
  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(<LabelFleetStopCard />);
  // 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(<LabelFleetStopCard />);
  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(<LabelFleetStopCard />);
+  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(<LabelFleetStopCard />);
  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(<LabelFleetStopCard />);
+  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(<LabelFleetStopCard />);
+  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(<LabelFleetStopCard />);
+  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<Response>((resolve) => { resolveProd = resolve; });
+      }
+      if (label === 'qa') {
+        return Promise.resolve(jsonResponse(200, { matchedNodes: 0, matchedStacks: 0, unreachableNodes: 0, perNode: [] }));
+      }
+    }
+    return Promise.resolve(jsonResponse(404, {}));
+  });
+  render(<LabelFleetStopCard />);
+  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 () => {
@@ -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<ResultRow[]>([]);
  const [preview, setPreview] = useState<PreviewState>({ 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<ReturnType<typeof setTimeout> | 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<ResolvedTarget[] | null>(() => {
+    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 && <ResolvedTargetsList targets={resolvedTargets} />}
+      </ConfirmModal>
    </>
  );
 }
@@ -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 (
+    <div>
+      <div className={cn(KICKER, 'text-stat-icon mb-2 normal-case tracking-normal text-[11px]')}>
+        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.
+      </div>
+      <div className="max-h-[180px] overflow-y-auto rounded border border-card-border/60 bg-card/40 p-2">
+        <ul className="space-y-1.5">
+          {targets.map(t => (
+            <li key={t.nodeName} className="flex items-start gap-2">
+              <span className={cn(KICKER, 'shrink-0 pt-0.5 text-stat-subtitle')}>{t.nodeName}</span>
+              <span className="min-w-0 flex-1 break-words font-mono text-[11px] text-stat-value">{t.stackNames.join(', ')}</span>
+            </li>
+          ))}
+        </ul>
+      </div>
+    </div>
+  );
+}
+
 // 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[] }) {