fix(web): stop runaway deploy-success loop in DeployWizard
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.
This commit is contained in:
@@ -237,21 +237,27 @@ const DeckyFleet: React.FC<FleetProps> = ({ searchQuery = '' }) => {
|
|||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<DeployWizard
|
{/* Mounted only while open so closing tears down the wizard's hooks
|
||||||
open={showDeploy}
|
(lifecycle polling, the auto-close effect). Leaving it permanently
|
||||||
archetypes={archetypes}
|
mounted kept those effects alive after close and, combined with the
|
||||||
fleetSize={deckies.length}
|
inline onComplete below, drove a runaway /deckies + toast loop. */}
|
||||||
onClose={() => setShowDeploy(false)}
|
{showDeploy && (
|
||||||
onComplete={(count) => {
|
<DeployWizard
|
||||||
setShowDeploy(false);
|
open={showDeploy}
|
||||||
void refresh();
|
archetypes={archetypes}
|
||||||
push({
|
fleetSize={deckies.length}
|
||||||
text: `DEPLOYED · ${count} DECK${count === 1 ? 'Y' : 'IES'}`,
|
onClose={() => setShowDeploy(false)}
|
||||||
tone: 'matrix',
|
onComplete={(count) => {
|
||||||
icon: 'check-circle',
|
setShowDeploy(false);
|
||||||
});
|
void refresh();
|
||||||
}}
|
push({
|
||||||
/>
|
text: `DEPLOYED · ${count} DECK${count === 1 ? 'Y' : 'IES'}`,
|
||||||
|
tone: 'matrix',
|
||||||
|
icon: 'check-circle',
|
||||||
|
});
|
||||||
|
}}
|
||||||
|
/>
|
||||||
|
)}
|
||||||
|
|
||||||
<IntervalEditor
|
<IntervalEditor
|
||||||
key={intervalEditor?.name ?? 'closed'}
|
key={intervalEditor?.name ?? 'closed'}
|
||||||
|
|||||||
@@ -1,10 +1,15 @@
|
|||||||
// SPDX-License-Identifier: AGPL-3.0-or-later
|
// SPDX-License-Identifier: AGPL-3.0-or-later
|
||||||
import { describe, it, expect, vi } from 'vitest';
|
import { describe, it, expect, vi, type Mock } from 'vitest';
|
||||||
import { render, screen } from '@testing-library/react';
|
import { render, screen, waitFor } from '@testing-library/react';
|
||||||
import userEvent from '@testing-library/user-event';
|
import userEvent from '@testing-library/user-event';
|
||||||
import { DeployWizard } from './DeployWizard';
|
import { DeployWizard } from './DeployWizard';
|
||||||
|
import api from '../../utils/api';
|
||||||
import type { Archetype } from './types';
|
import type { Archetype } from './types';
|
||||||
|
|
||||||
|
vi.mock('../../utils/api', () => ({
|
||||||
|
default: { post: vi.fn(), get: vi.fn() },
|
||||||
|
}));
|
||||||
|
|
||||||
// ServiceConfigFields fetches the per-service schema; replace with a stub
|
// ServiceConfigFields fetches the per-service schema; replace with a stub
|
||||||
// so the wizard tests don't need MSW handlers for that side-channel.
|
// so the wizard tests don't need MSW handlers for that side-channel.
|
||||||
vi.mock('../ServiceConfigFields', async () => {
|
vi.mock('../ServiceConfigFields', async () => {
|
||||||
@@ -82,4 +87,45 @@ describe('DeployWizard', () => {
|
|||||||
await user.click(screen.getByText('CANCEL'));
|
await user.click(screen.getByText('CANCEL'));
|
||||||
expect(onClose).toHaveBeenCalled();
|
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(
|
||||||
|
<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'));
|
||||||
|
|
||||||
|
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(<DeployWizard {...props} onComplete={() => onComplete()} />);
|
||||||
|
}
|
||||||
|
await new Promise((r) => setTimeout(r, 1200));
|
||||||
|
expect(onComplete).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
// SPDX-License-Identifier: AGPL-3.0-or-later
|
// 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 { PlusCircle } from '../../icons';
|
||||||
import { useLifecyclePolling } from '../../hooks/useLifecyclePolling';
|
import { useLifecyclePolling } from '../../hooks/useLifecyclePolling';
|
||||||
import api from '../../utils/api';
|
import api from '../../utils/api';
|
||||||
@@ -197,11 +197,19 @@ 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
|
||||||
|
// 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
|
// 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).
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!lifecycleDone) return;
|
if (!lifecycleDone || completedRef.current) return;
|
||||||
if (deployFailures.length === 0) {
|
if (deployFailures.length === 0) {
|
||||||
|
completedRef.current = true;
|
||||||
const t = window.setTimeout(() => onComplete(count), 700);
|
const t = window.setTimeout(() => onComplete(count), 700);
|
||||||
return () => window.clearTimeout(t);
|
return () => window.clearTimeout(t);
|
||||||
}
|
}
|
||||||
@@ -215,6 +223,7 @@ export const DeployWizard: React.FC<Props> = ({
|
|||||||
setDeployErr(null);
|
setDeployErr(null);
|
||||||
setLog([]);
|
setLog([]);
|
||||||
setLifecycleIds([]);
|
setLifecycleIds([]);
|
||||||
|
completedRef.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
|
||||||
|
|||||||
Reference in New Issue
Block a user