From 2a9d1989b66666172f81473fc95a993a9cde5087 Mon Sep 17 00:00:00 2001 From: anti Date: Sat, 13 Jun 2026 00:19:39 -0400 Subject: [PATCH] fix(web): stop runaway deploy-success loop in DeployWizard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After a successful deploy the wizard hammered GET /deckies and stacked "DEPLOYED" toasts unbounded (~1.4/s, forever). Two compounding causes: 1. The auto-close effect's dep array includes onComplete, which the parent passes as an inline arrow (new reference every render). onComplete calls refresh(), re-rendering the parent, producing a new onComplete ref, re-running the effect, which reschedules onComplete — a feedback loop. 2. DeployWizard stayed permanently mounted (open only toggled a child overlay), so its hooks kept running with lifecycleDone===true after close. Fix both: a completedRef guard makes the auto-close fire exactly once per deploy regardless of effect re-runs (reset in startDeploy), and the parent now mounts the wizard only while open so closing tears down its hooks and clears the latent "reopen re-completes stale rows" path. Lifecycle polling itself was never the runaway — it stops cleanly at terminal status; the bounded ~25-poll build window is expected. Adds a regression test asserting onComplete fires once across a simulated re-render storm. --- decnet_web/src/components/DeckyFleet.tsx | 36 +++++++------ .../DeckyFleet/DeployWizard.test.tsx | 50 ++++++++++++++++++- .../components/DeckyFleet/DeployWizard.tsx | 13 ++++- 3 files changed, 80 insertions(+), 19 deletions(-) diff --git a/decnet_web/src/components/DeckyFleet.tsx b/decnet_web/src/components/DeckyFleet.tsx index 772ea8ac..ec9d90a7 100644 --- a/decnet_web/src/components/DeckyFleet.tsx +++ b/decnet_web/src/components/DeckyFleet.tsx @@ -237,21 +237,27 @@ const DeckyFleet: React.FC = ({ searchQuery = '' }) => { )} - setShowDeploy(false)} - onComplete={(count) => { - setShowDeploy(false); - void refresh(); - push({ - text: `DEPLOYED · ${count} DECK${count === 1 ? 'Y' : 'IES'}`, - tone: 'matrix', - icon: 'check-circle', - }); - }} - /> + {/* Mounted only while open so closing tears down the wizard's hooks + (lifecycle polling, the auto-close effect). Leaving it permanently + mounted kept those effects alive after close and, combined with the + inline onComplete below, drove a runaway /deckies + toast loop. */} + {showDeploy && ( + setShowDeploy(false)} + onComplete={(count) => { + setShowDeploy(false); + void refresh(); + push({ + text: `DEPLOYED · ${count} DECK${count === 1 ? 'Y' : 'IES'}`, + tone: 'matrix', + icon: 'check-circle', + }); + }} + /> + )} ({ + default: { post: vi.fn(), get: vi.fn() }, +})); + // ServiceConfigFields fetches the per-service schema; replace with a stub // so the wizard tests don't need MSW handlers for that side-channel. vi.mock('../ServiceConfigFields', async () => { @@ -82,4 +87,45 @@ describe('DeployWizard', () => { await user.click(screen.getByText('CANCEL')); expect(onClose).toHaveBeenCalled(); }); + + it('fires onComplete exactly once after a successful deploy, even across re-renders', async () => { + // Regression: onComplete is an inline arrow in the parent (new ref every + // render) and it triggers a parent refresh -> re-render. Without the + // completedRef guard the auto-close effect re-ran on every re-render and + // rescheduled onComplete forever (runaway /deckies + toast loop). + (api.post as Mock).mockResolvedValue({ + data: { lifecycle_ids: ['lc-1'], message: 'ok', mode: 'unihost' }, + }); + (api.get as Mock).mockResolvedValue({ + data: { rows: [{ + id: 'lc-1', decky_name: 'qa-01', host_uuid: null, operation: 'deploy', + status: 'succeeded', error: null, + started_at: '2026-01-01T00:00:00', updated_at: '2026-01-01T00:00:00', + completed_at: '2026-01-01T00:00:00', + }] }, + }); + + const onComplete = vi.fn(); + const user = userEvent.setup(); + const props = { open: true, onClose: () => {}, archetypes, fleetSize: 0 }; + // Fresh arrow each render mirrors the parent's unstable onComplete ref. + const { rerender } = render( + onComplete()} />, + ); + + await user.click(screen.getByText('Web Server')); + await user.click(screen.getByText('NEXT →')); + await user.click(screen.getByText('NEXT →')); + await user.click(screen.getByText('NEXT →')); + await user.click(screen.getByText('ESTABLISH FLEET')); + + await waitFor(() => expect(onComplete).toHaveBeenCalledTimes(1), { timeout: 3000 }); + + // Simulate the parent re-render storm with fresh onComplete refs. + for (let i = 0; i < 5; i++) { + rerender( onComplete()} />); + } + await new Promise((r) => setTimeout(r, 1200)); + expect(onComplete).toHaveBeenCalledTimes(1); + }); }); diff --git a/decnet_web/src/components/DeckyFleet/DeployWizard.tsx b/decnet_web/src/components/DeckyFleet/DeployWizard.tsx index 5204e947..45f56c42 100644 --- a/decnet_web/src/components/DeckyFleet/DeployWizard.tsx +++ b/decnet_web/src/components/DeckyFleet/DeployWizard.tsx @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-or-later -import React, { useEffect, useMemo, useState } from 'react'; +import React, { useEffect, useMemo, useRef, useState } from 'react'; import { PlusCircle } from '../../icons'; import { useLifecyclePolling } from '../../hooks/useLifecyclePolling'; import api from '../../utils/api'; @@ -197,11 +197,19 @@ export const DeployWizard: React.FC = ({ ); const deployOk = lifecycleDone && deployFailures.length === 0; + // Fire onComplete exactly once per deploy. onComplete is an inline arrow in + // the parent (new ref every render) and it triggers a parent refresh, so + // without this guard the effect re-runs on every re-render and reschedules + // onComplete forever — a runaway loop of /deckies refetches + toasts. Reset + // in startDeploy so a subsequent deploy can complete again. + const completedRef = useRef(false); + // When every row reaches terminal status, auto-close on full success // (or stay open so the operator can read failures). useEffect(() => { - if (!lifecycleDone) return; + if (!lifecycleDone || completedRef.current) return; if (deployFailures.length === 0) { + completedRef.current = true; const t = window.setTimeout(() => onComplete(count), 700); return () => window.clearTimeout(t); } @@ -215,6 +223,7 @@ export const DeployWizard: React.FC = ({ setDeployErr(null); setLog([]); setLifecycleIds([]); + completedRef.current = false; setDeploying(true); // Roll the per-service forms into the compact payload the server // expects — empty values dropped, types coerced where the schema