diff --git a/e2e/stacks.spec.ts b/e2e/stacks.spec.ts index fa0db5c9..d2c4e106 100644 --- a/e2e/stacks.spec.ts +++ b/e2e/stacks.spec.ts @@ -35,18 +35,71 @@ test.describe('Stack management', () => { await page.locator('#create-stack-name').fill(TEST_STACK); await page.locator('[role="dialog"]').getByRole('button', { name: 'Create' }).click(); - // Wait for dialog to close (success) or error message to appear (failure) - await Promise.race([ - page.getByRole('dialog').waitFor({ state: 'hidden', timeout: 8_000 }), - page.getByText(/already exists/i).waitFor({ state: 'visible', timeout: 8_000 }), - ]).catch(() => { }); + // F-2: the dialog MUST close on a successful create. No silent .catch swallow. + await expect(page.getByRole('dialog')).toBeHidden({ timeout: 8_000 }); - // The stack should now exist - refresh and verify via the sidebar + // F-2: a success toast MUST appear so the click does not feel like a no-op. + await expect(page.getByText(`Stack "${TEST_STACK}" created.`)).toBeVisible({ timeout: 5_000 }); + + // F-2: the new stack appears in the sidebar without a manual reload. + await expect( + page.locator('[role="listbox"]').getByText(TEST_STACK, { exact: true }) + ).toBeVisible({ timeout: 5_000 }); + }); + + test('create dialog: double-clicking Create fires only one POST', async ({ page }) => { + const stackName = 'e2e-double-click-stack'; + + // Clean any prior leftover and reload so we start from a known sidebar state. + await page.evaluate(async (name) => { + await fetch(`/api/stacks/${name}`, { method: 'DELETE', credentials: 'include' }).catch(() => { }); + }, stackName); await page.reload(); await loginAs(page); await waitForStacksLoaded(page); - await expect(page.getByText(TEST_STACK).first()).toBeVisible({ timeout: 5_000 }); + // Count POST /api/stacks calls and hold the response briefly so the + // disabled-button window is observable from the test. Awaiting + // route.continue() avoids the fire-and-forget pattern that can subtly + // stall a request on a busy CI runner. + let postCount = 0; + await page.route('**/api/stacks', async (route) => { + if (route.request().method() === 'POST') { + postCount += 1; + await new Promise((resolve) => setTimeout(resolve, 400)); + } + await route.continue(); + }); + + try { + await page.getByRole('button', { name: 'Create Stack' }).click(); + await expect(page.getByRole('dialog')).toBeVisible({ timeout: 5_000 }); + await page.locator('#create-stack-name').fill(stackName); + + const createBtn = page.locator('[role="dialog"]').getByRole('button', { name: /^Create/ }); + await createBtn.click(); + + // The synchronous useRef guard plus React's disabled re-render must + // surface a disabled Create button while the POST is in flight. If this + // assertion ever times out, the busy-state path has regressed. + await expect(createBtn).toBeDisabled({ timeout: 1_500 }); + + // Best-effort second click against the now-disabled native button. A real + // user mash translates to a no-op at the browser level on a disabled + // submit button; the synchronous ref guard catches it even if Playwright + // managed to dispatch the click anyway. + await createBtn.click({ force: true, timeout: 500 }).catch(() => undefined); + + await expect(page.getByRole('dialog')).toBeHidden({ timeout: 8_000 }); + await expect(page.getByText(`Stack "${stackName}" created.`)).toBeVisible({ timeout: 5_000 }); + expect(postCount).toBe(1); + } finally { + await page.unroute('**/api/stacks'); + // Cleanup + await page.evaluate(async (name) => { + await fetch(`/api/stacks/${name}`, { method: 'DELETE', credentials: 'include' }).catch(() => { }); + }, stackName); + } }); test('delete the test stack', async ({ page }) => { diff --git a/frontend/src/components/EditorLayout.tsx b/frontend/src/components/EditorLayout.tsx index bc49040f..80c832ab 100644 --- a/frontend/src/components/EditorLayout.tsx +++ b/frontend/src/components/EditorLayout.tsx @@ -33,6 +33,7 @@ import { useTrivyStatus } from '@/hooks/useTrivyStatus'; import { StackSidebar } from '@/components/sidebar/StackSidebar'; import type { StackRowStatus } from '@/components/sidebar/stack-status-utils'; import { useComposeDiffPreviewEnabled } from '@/hooks/use-compose-diff-preview-enabled'; +import { toast } from '@/components/ui/toast-store'; export default function EditorLayout() { const { isAdmin, can } = useAuth(); @@ -91,6 +92,15 @@ export default function EditorLayout() { const { nodes, activeNode, setActiveNode } = useNodes(); + // Mirror activeNode.id in a ref so async handlers (e.g. CreateStackDialog's + // post-create handoff) can detect a node switch that happened mid-flight. + // Closure capture of activeNode would always match the value at handler-creation + // time and miss the switch. + const activeNodeIdRef = useRef(activeNode?.id ?? null); + useEffect(() => { + activeNodeIdRef.current = activeNode?.id ?? null; + }, [activeNode?.id]); + const overlayState = useOverlayState(); const { createDialogOpen, setCreateDialogOpen, @@ -227,8 +237,16 @@ export default function EditorLayout() { { + onStackCreated={async (sName, sourceNodeId) => { await refreshStacks(); + // loadFile keeps its own unsaved-changes overlay (intentional safety, + // shared with every other "switch to a different stack" code path). + // Skip the load if the user switched nodes mid-create so we do not + // 404 against a stack name that lives on the previous node. + if (sourceNodeId != null && activeNodeIdRef.current !== sourceNodeId) { + toast.info(`Stack "${sName}" created on the previous node.`); + return; + } await stackActions.loadFile(sName); }} onStacksChanged={async () => { await refreshStacks(); }} diff --git a/frontend/src/components/EditorLayout/CreateStackDialog.tsx b/frontend/src/components/EditorLayout/CreateStackDialog.tsx index 3ebe660a..ab434ff5 100644 --- a/frontend/src/components/EditorLayout/CreateStackDialog.tsx +++ b/frontend/src/components/EditorLayout/CreateStackDialog.tsx @@ -1,4 +1,4 @@ -import { useRef, useState, type KeyboardEvent } from 'react'; +import { useRef, useState, type FormEvent, type KeyboardEvent } from 'react'; import { Plus, GitBranch, FileCode2, Loader2, type LucideIcon } from 'lucide-react'; import { Modal, ModalHeader, ModalBody, ModalFooter } from '../ui/modal'; import { Button } from '../ui/button'; @@ -9,12 +9,16 @@ import { Checkbox } from '../ui/checkbox'; import { GitSourceFields, type ApplyMode } from '../stack/GitSourceFields'; import { apiFetch } from '@/lib/api'; import { toast } from '@/components/ui/toast-store'; +import { useNodes } from '@/context/NodeContext'; import { cn } from '@/lib/utils'; export interface CreateStackDialogProps { open: boolean; onOpenChange: (open: boolean) => void; - onStackCreated: (stackName: string) => void | Promise; + // sourceNodeId is the active node ID captured at the moment the user clicked + // Create. Parent compares against the current active node before navigating + // so a mid-flight node switch does not land the user on a 404. + onStackCreated: (stackName: string, sourceNodeId: number | null | undefined) => void | Promise; onStacksChanged: () => void | Promise; } @@ -30,8 +34,16 @@ const tabId = (m: CreateMode) => `create-stack-tab-${m}`; const panelId = (m: CreateMode) => `create-stack-panel-${m}`; export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacksChanged }: CreateStackDialogProps) { + const { activeNode } = useNodes(); const [createMode, setCreateMode] = useState('empty'); 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 [dockerRunInput, setDockerRunInput] = useState(''); const [convertedYaml, setConvertedYaml] = useState(null); const [isConverting, setIsConverting] = useState(false); @@ -66,8 +78,12 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks }; const handleCreateStack = async () => { + if (creatingEmptyRef.current) return; if (!newStackName.trim()) return; const stackName = newStackName.trim(); + const sourceNodeId = activeNode?.id; + creatingEmptyRef.current = true; + setCreatingEmpty(true); try { const response = await apiFetch('/stacks', { method: 'POST', @@ -75,21 +91,37 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks }); if (!response.ok) { if (response.status === 409) { - throw new Error('Stack already exists'); - } else if (response.status === 400) { - throw new Error('Invalid stack name (use alphanumeric characters and hyphens only)'); + throw new Error('Stack already exists.'); } - throw new Error('Failed to create stack'); + if (response.status === 400) { + throw new Error('Invalid stack name (use alphanumeric characters and hyphens only).'); + } + const body = await response.json().catch(() => ({})); + const backendError = (body as { error?: string })?.error; + if (response.status === 403) { + throw new Error(backendError || 'You do not have permission to create stacks.'); + } + throw new Error(backendError || 'Failed to create stack.'); } onOpenChange(false); setNewStackName(''); - await onStackCreated(stackName); + setCreateMode('empty'); + toast.success(`Stack "${stackName}" created.`); + await onStackCreated(stackName, sourceNodeId); } catch (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 { + creatingEmptyRef.current = false; + setCreatingEmpty(false); } }; + const handleEmptyFormSubmit = (e: FormEvent) => { + e.preventDefault(); + void handleCreateStack(); + }; + const handleCreateStackFromGit = async () => { const stackName = newStackName.trim(); if (!stackName) { @@ -104,6 +136,7 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks toast.error('Only HTTPS repository URLs are supported.'); return; } + const sourceNodeId = activeNode?.id; setCreatingFromGit(true); const loadingId = toast.loading(gitDeployNow ? 'Fetching, creating, and deploying...' : 'Fetching and creating stack...'); try { @@ -154,7 +187,7 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks } onOpenChange(false); resetCreateFromGitForm(); - await onStackCreated(stackName); + await onStackCreated(stackName, sourceNodeId); } catch (error) { console.error('Failed to create stack from Git:', error); toast.error((error as Error)?.message || 'Failed to create stack from Git.'); @@ -209,6 +242,7 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks toast.error('Convert the command before creating the stack.'); return; } + const sourceNodeId = activeNode?.id; setCreatingFromDockerRun(true); const loadingId = toast.loading('Creating stack from converted YAML...'); let createdStack = false; @@ -245,7 +279,7 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks onOpenChange(false); resetCreateFromDockerRunForm(); setNewStackName(''); - await onStackCreated(stackName); + await onStackCreated(stackName, sourceNodeId); } catch (error) { console.error('Failed to create stack from docker run:', error); const err = error as { message?: string; error?: string; data?: { error?: string } }; @@ -265,7 +299,7 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks } }; - const busy = creatingFromGit || creatingFromDockerRun; + const busy = creatingEmpty || creatingFromGit || creatingFromDockerRun; return ( @@ -289,31 +328,43 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks {createMode === 'empty' && (
- -
- - setNewStackName(e.target.value)} - autoFocus - /> -
- - onOpenChange(false)}> - Cancel - - } - primary={ - - } - /> +
+ +
+ + setNewStackName(e.target.value)} + disabled={creatingEmpty} + autoFocus + /> +
+ + onOpenChange(false)} + disabled={creatingEmpty} + > + Cancel + + } + primary={ + + } + /> +
)}