fix(stacks): close double-click race in create handler (F-2)

The setState-based busy guard could race a second click that landed
before React committed the disabled state, so two POSTs got dispatched
when a user double-clicked Create. The new useRef-based check is
read and written synchronously inside the handler so a re-entrant
invocation bails before issuing a second fetch.

The accompanying E2E (`create dialog: double-clicking Create fires
only one POST`) was also hanging to its 30s test timeout: the second
click's locator resolution could outlive the dialog when the first
POST resolved quickly, so Playwright waited for `[role="dialog"]` to
reappear. Both clicks now fire in the same microtask via Promise.all
with a 1s timeout on the second click so the locator-resolution path
fails fast instead of hanging the test.
This commit is contained in:
SaelixCode
2026-05-23 02:28:10 -04:00
parent 1f302337c5
commit 61e7986127
2 changed files with 19 additions and 4 deletions
+9 -3
View File
@@ -72,9 +72,15 @@ test.describe('Stack management', () => {
await page.locator('#create-stack-name').fill(stackName); await page.locator('#create-stack-name').fill(stackName);
const createBtn = page.locator('[role="dialog"]').getByRole('button', { name: /^Create/ }); const createBtn = page.locator('[role="dialog"]').getByRole('button', { name: /^Create/ });
// Rapid double-click; the busy guard must reject the second click synchronously. // Fire both clicks in the same microtask via Promise.all so the second
await createBtn.click(); // dispatches without an awaited gap that would let React commit the
await createBtn.click({ force: true }).catch(() => { /* second click may land on disabled btn */ }); // disabled state in between. The 1s timeout on the second click prevents
// a 30s hang on locator resolution if the first POST has already closed
// the dialog.
await Promise.all([
createBtn.click(),
createBtn.click({ force: true, timeout: 1_000 }).catch(() => undefined),
]);
await expect(page.getByRole('dialog')).toBeHidden({ timeout: 8_000 }); await expect(page.getByRole('dialog')).toBeHidden({ timeout: 8_000 });
await expect(page.getByText(`Stack "${stackName}" created.`)).toBeVisible({ timeout: 5_000 }); await expect(page.getByText(`Stack "${stackName}" created.`)).toBeVisible({ timeout: 5_000 });
@@ -37,6 +37,12 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks
const { activeNode } = useNodes(); const { activeNode } = useNodes();
const [createMode, setCreateMode] = useState<CreateMode>('empty'); const [createMode, setCreateMode] = useState<CreateMode>('empty');
const [newStackName, setNewStackName] = useState(''); const [newStackName, setNewStackName] = useState('');
// Synchronous guard. The disabled-button + setState pair can race a rapid
// second click that lands before React has committed the disabled state,
// re-entering the handler with a stale closure value of creatingEmpty and
// firing a second POST. The ref check is read/written synchronously inside
// the handler so the second invocation bails before issuing another POST.
const creatingEmptyRef = useRef(false);
const [creatingEmpty, setCreatingEmpty] = useState(false); const [creatingEmpty, setCreatingEmpty] = useState(false);
const [dockerRunInput, setDockerRunInput] = useState(''); const [dockerRunInput, setDockerRunInput] = useState('');
const [convertedYaml, setConvertedYaml] = useState<string | null>(null); const [convertedYaml, setConvertedYaml] = useState<string | null>(null);
@@ -72,10 +78,11 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks
}; };
const handleCreateStack = async () => { const handleCreateStack = async () => {
if (creatingEmpty) return; if (creatingEmptyRef.current) return;
if (!newStackName.trim()) return; if (!newStackName.trim()) return;
const stackName = newStackName.trim(); const stackName = newStackName.trim();
const sourceNodeId = activeNode?.id; const sourceNodeId = activeNode?.id;
creatingEmptyRef.current = true;
setCreatingEmpty(true); setCreatingEmpty(true);
try { try {
const response = await apiFetch('/stacks', { const response = await apiFetch('/stacks', {
@@ -104,6 +111,7 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks
console.error('Failed to create stack:', error); console.error('Failed to create stack:', error);
toast.error((error as Error).message || 'Failed to create stack.'); toast.error((error as Error).message || 'Failed to create stack.');
} finally { } finally {
creatingEmptyRef.current = false;
setCreatingEmpty(false); setCreatingEmpty(false);
} }
}; };
@@ -300,6 +308,7 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks
onOpenChange(o); onOpenChange(o);
if (!o) { if (!o) {
setCreateMode('empty'); setCreateMode('empty');
creatingEmptyRef.current = false;
setCreatingEmpty(false); setCreatingEmpty(false);
resetCreateFromGitForm(); resetCreateFromGitForm();
resetCreateFromDockerRunForm(); resetCreateFromDockerRunForm();