fix(web): duplicated deploy log lines + cancelled auto-close in DeployWizard

Two defects exposed after the deploy-success loop fix (verified live):

1. Duplicated / skipped transcript lines. The placeholder-log interval did
   `setLog(prev => [...prev, msgs[i]])` then `i++`. React 18 auto-batches
   setInterval updaters, so the updater ran after i had advanced and read the
   wrong index — skipping some lines ([NET], [SENSE]) and duplicating others
   ([TLS]). Fixed by capturing `const line = msgs[i]` before scheduling the
   update. A placeholderStartedRef also gates the effect to one run per deploy
   (reset in startDeploy) as defense-in-depth against re-render churn.

2. Wizard never closed on success. The completedRef guard combined with the
   auto-close effect's cleanup was self-defeating: a re-render inside the
   700ms window (e.g. the [OK] terminal-log append) ran the cleanup, clearing
   the pending close, and the guard then blocked rescheduling — so onComplete
   never fired. The timer now lives in a ref cleared only on unmount, so a
   scheduled close always fires exactly once regardless of re-renders.

Adds a regression test that a re-render during the close countdown does not
cancel the close. Verified end-to-end against the live instance: all 8 log
lines render once in order, the wizard auto-closes, and /deckies does not
storm.
This commit is contained in:
2026-06-13 00:34:21 -04:00
parent 2a9d1989b6
commit 207494f41e
2 changed files with 70 additions and 8 deletions

View File

@@ -128,4 +128,44 @@ describe('DeployWizard', () => {
await new Promise((r) => setTimeout(r, 1200)); await new Promise((r) => setTimeout(r, 1200));
expect(onComplete).toHaveBeenCalledTimes(1); expect(onComplete).toHaveBeenCalledTimes(1);
}); });
it('still closes when re-renders land during the 700ms close countdown', async () => {
// Regression: the close timer must survive re-renders inside its window.
// A naive completedRef guard whose effect cleanup cleared the timer would
// cancel the pending onComplete on the first in-window re-render and the
// wizard would never close. The timer lives in a ref to prevent that.
(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 };
const { rerender } = render(
<DeployWizard {...props} onComplete={() => 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'));
// Hammer fresh-onComplete re-renders across the close countdown window.
for (let i = 0; i < 8; i++) {
rerender(<DeployWizard {...props} onComplete={() => onComplete()} />);
await new Promise((r) => setTimeout(r, 40));
}
// The close must still fire exactly once despite the in-window churn.
await waitFor(() => expect(onComplete).toHaveBeenCalledTimes(1), { timeout: 3000 });
});
}); });

View File

@@ -177,12 +177,24 @@ export const DeployWizard: React.FC<Props> = ({
// Atmospheric backdrop (one-shot, decoupled from real progress now // Atmospheric backdrop (one-shot, decoupled from real progress now
// that the lifecycle rows carry truth). Runs once when DEPLOYING // that the lifecycle rows carry truth). Runs once when DEPLOYING
// begins so the operator sees activity before the first poll lands. // begins so the operator sees activity before the first poll lands.
// The guard is load-bearing: effectiveServices is recomputed each render
// (and lifecycle polling re-renders every 2s during deploy), so without it
// the effect re-ran and restarted the line sequence mid-stream, duplicating
// the transcript. Reset in startDeploy. (Effect deps kept for lint; the ref
// is what enforces once-per-deploy.)
const placeholderStartedRef = useRef(false);
useEffect(() => { useEffect(() => {
if (step !== 3 || !deploying || lifecycleIds.length === 0) return; if (step !== 3 || !deploying || lifecycleIds.length === 0) return;
if (placeholderStartedRef.current) return;
placeholderStartedRef.current = true;
const msgs = PLACEHOLDER_LINES(effectiveArchetypeName, effectiveServices, count, fleetSize); const msgs = PLACEHOLDER_LINES(effectiveArchetypeName, effectiveServices, count, fleetSize);
let i = 0; let i = 0;
const t = window.setInterval(() => { const t = window.setInterval(() => {
setLog((prev) => [...prev, msgs[i]]); // Capture the line NOW. React 18 auto-batches setInterval updaters, so a
// `prev => [...prev, msgs[i]]` updater runs after i++ has advanced i,
// reading the wrong index — skipping some lines and duplicating others.
const line = msgs[i];
setLog((prev) => [...prev, line]);
i++; i++;
if (i >= msgs.length) window.clearInterval(t); if (i >= msgs.length) window.clearInterval(t);
}, 420); }, 420);
@@ -197,12 +209,16 @@ export const DeployWizard: React.FC<Props> = ({
); );
const deployOk = lifecycleDone && deployFailures.length === 0; const deployOk = lifecycleDone && deployFailures.length === 0;
// Fire onComplete exactly once per deploy. onComplete is an inline arrow in // Schedule the auto-close exactly once per deploy. onComplete is an inline
// the parent (new ref every render) and it triggers a parent refresh, so // arrow in the parent (new ref every render) and it triggers a parent
// without this guard the effect re-runs on every re-render and reschedules // refresh, so the effect re-runs on every re-render. completedRef gates
// onComplete forever — a runaway loop of /deckies refetches + toasts. Reset // re-entry so we never reschedule (which would loop /deckies refetches +
// in startDeploy so a subsequent deploy can complete again. // toasts). Crucially, the timer lives in a ref — NOT the effect's cleanup —
// so a re-render inside the 700ms window (e.g. the [OK] terminal-log
// append) can't clear the pending close and leave the wizard stuck open.
// Reset in startDeploy so a subsequent deploy can complete again.
const completedRef = useRef(false); const completedRef = useRef(false);
const closeTimerRef = useRef<number | undefined>(undefined);
// When every row reaches terminal status, auto-close on full success // When every row reaches terminal status, auto-close on full success
// (or stay open so the operator can read failures). // (or stay open so the operator can read failures).
@@ -210,11 +226,16 @@ export const DeployWizard: React.FC<Props> = ({
if (!lifecycleDone || completedRef.current) return; if (!lifecycleDone || completedRef.current) return;
if (deployFailures.length === 0) { if (deployFailures.length === 0) {
completedRef.current = true; completedRef.current = true;
const t = window.setTimeout(() => onComplete(count), 700); closeTimerRef.current = window.setTimeout(() => onComplete(count), 700);
return () => window.clearTimeout(t);
} }
}, [lifecycleDone, deployFailures.length, count, onComplete]); }, [lifecycleDone, deployFailures.length, count, onComplete]);
// Clear a pending auto-close only on unmount (e.g. CANCEL mid-countdown),
// never on the re-renders that would otherwise cancel a successful close.
useEffect(() => () => {
if (closeTimerRef.current !== undefined) window.clearTimeout(closeTimerRef.current);
}, []);
const canNext = step === 0 const canNext = step === 0
? (pickMode === 'archetype' ? !!archetype : selectedServices.length > 0) ? (pickMode === 'archetype' ? !!archetype : selectedServices.length > 0)
: true; : true;
@@ -224,6 +245,7 @@ export const DeployWizard: React.FC<Props> = ({
setLog([]); setLog([]);
setLifecycleIds([]); setLifecycleIds([]);
completedRef.current = false; completedRef.current = false;
placeholderStartedRef.current = false;
setDeploying(true); setDeploying(true);
// Roll the per-service forms into the compact payload the server // Roll the per-service forms into the compact payload the server
// expects — empty values dropped, types coerced where the schema // expects — empty values dropped, types coerced where the schema