From 7c84969b310b7c58c94ddab04854c47e63b607cf Mon Sep 17 00:00:00 2001 From: Anso Date: Sun, 24 May 2026 15:18:47 -0400 Subject: [PATCH] fix(editor): harden save-deploy, node-switch, delete, and stats reactivity (#1188) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(stacks): validate input, bound YAML parses, and reorder delete steps Backend hardening covering three editor-served routes: - `/:stackName/containers` GET adds an explicit `isValidStackName` guard so bad input is rejected at the call site even if the router-level param validator changes in future. - `MAX_COMPOSE_PARSE_BYTES` (1 MiB) bounds the two `YAML.parse` callsites (`resolveAllEnvFilePaths`, `/services`) so a malformed or oversize compose cannot exhaust heap during routine env/service lookups. - `DELETE /:stackName` is reordered to abort before any database cleanup if `FileSystemService.deleteStack` throws, keeping DB and FS in sync. Partial-failure responses now describe the resulting state in human terms instead of returning a generic 500. Adds debug-mode entry-point traces (`[Stacks:debug] ...`) on save / down / restart / delete handlers, all sanitised through `sanitizeForLog`. New vitest covers the containers validator, the YAML size guard, and the small-compose happy path. * fix(editor): gate save-and-deploy on save success, abort stale loads `saveFile` now returns a boolean: true on a successful PUT, false on any failure. `handleSaveAndDeploy` short-circuits when save fails so a backend 500 on the compose write no longer slips through to a deploy with the unsaved in-memory content. The diff-preview confirm path in ShellOverlays applies the same guard. `loadFile` now drives a per-hook `AbortController`. A stack switch, a node switch (via `resetEditorState`), or hook unmount aborts the in-flight GET chain so a late compose / env / containers / backup response from the previous selection never overwrites freshly-loaded state. `hasUnsavedChanges` is exported so EditorLayout can check it during the node-switch lifecycle. New unit tests cover the boolean save contract and the save-fail-blocks-deploy invariant. * fix(editor): prompt on node switch when the editor has unsaved changes Switching the active node previously called `resetEditorState()` without checking the editor's dirty state, silently dropping in-progress edits. The post-auth shell now intercepts the node-change effect: if the editor is dirty, the attempted node is stashed via the existing `pendingUnsavedNode` field, `pendingUnsavedLoad` is set to a sentinel that routes `discardAndLoadPending` to `setActiveNode`, and `activeNode` is reverted to the previous node so the dialog can be resolved without losing content. A re-entrant switch (clicking a third node while the dialog is still open) is now ignored — the second switch reverts silently so the dialog's anchor stays on the first attempt. When the previous node is no longer in the registry and cannot be reverted to, the operator gets a warning toast before the wipe so the loss is at least visible. * fix(editor): split delete and deploy permission gates in the action bar The action bar previously wrapped every affordance — including the Delete menu item — in a single `can('stack:deploy')` check, even though the backend route requires `stack:delete`. A user with `stack:deploy` only saw a Delete button that 403'd, and a user with `stack:delete` only saw no menu at all. Each affordance now renders against its own permission: deploy / stop / restart / update on `stack:deploy`, delete on `stack:delete`, rollback on `canDeploy + isPaid + backupInfo.exists`, scan on `isAdmin + trivy.available`. The overflow menu appears if any of {rollback, scan, delete} is granted, so a delete-only operator still has a way to remove the stack. Adds a Monaco model dispose on EditorView unmount via the existing editor ref, and a compact `Stats unavailable` chip in the CONTAINERS header that lights up when the live-stats WebSocket reports a persistent failure. * fix(editor): make container-stats hook reactive to the active node `useContainerStats` previously read the active node id from `localStorage` on each WebSocket open, with a deps array of `[containers]` only. After a node switch the stats stream stayed pointed at the previous node's `/ws` endpoint until the containers array refreshed. The hook now accepts `activeNodeId` as a second argument, depends on `[containers, activeNodeId]`, and drops the localStorage read. The return shape is `{ stats, error }`: the error field carries a string when the stream fails, surfaced by EditorView as a small chip in the CONTAINERS header. A per-WS `warnedOnce` set ensures a flaky daemon emits at most one console.warn per stream lifetime, never at message rate. Close codes 1000 / 1001 stay silent (normal teardown, navigation). The error reset (`setError(null)`) is split into its own effect keyed on `activeNodeId` so the banner does not flap on every containers-array refresh tick against a persistently-flaky daemon. Tests cover the new shape, the node-id reactivity, and the abnormal-close warn behaviour. * docs(editor): describe new gate split and add troubleshooting entries Updates the editor cockpit page to reflect that the action bar now gates each affordance on its own permission (deploy / delete), and that the bar appears for delete-only users so a stack can still be removed. Adds three troubleshooting accordions covering the new behaviours: a failed save that blocks the subsequent deploy, the unsaved-changes prompt on node switch, and the live-stats chip when the daemon is unreachable. Adds an E2E spec verifying that a forced PUT 500 on the compose write surfaces the failure toast and prevents the deploy POST from firing. * fix(stacks): use printf-style format for compose-down warn `console.warn` treats arg-1 as a printf format string when subsequent args follow. The template literal here interpolated a sanitized but not %-escaped stackName into arg-1 alongside an error argument, so a stackName containing a `%s` placeholder could theoretically swallow the error in the substitution. Switch to the file's established `'... %s ...', value, err` pattern. * test(editor): fix save-deploy spec; click both edit buttons, drop Monaco fill The editor has two edit affordances: a lowercase 'edit' in the Anatomy panel header that swaps the right column to the editor tabs, and a capital 'Edit' in the editor toolbar that flips Monaco from read-only into edit mode. The spec previously matched both with a case-insensitive regex and only fired one click, so Monaco never entered edit mode. It also tried to fill .monaco-editor textarea — that element is Monaco's IME accessibility helper, hard-coded readonly; the real editable surface is a contenteditable div. `saveFile()` does not gate on a dirty buffer, so the spec does not need to modify Monaco at all. Click both edit buttons with case-anchored regexes and drop the fill step. * test(editor): disambiguate Save & Deploy locator from sidebar row The TEST_STACK fixture is named 'e2e-save-deploy-stack'. The sidebar renders each stack into a div with role=button whose accessible name includes the stack slug, so the regex /save.*deploy/i matches both the sidebar row ('e2e-save-deploy-stack') and the editor toolbar's actual Save & Deploy button — strict-mode bails. Anchor the locator to the literal button text with exact:true. --- .../__tests__/stacks-edit-hardening.test.ts | 97 ++++++++++ backend/src/routes/stacks.ts | 144 ++++++++++----- docs/features/editor.mdx | 13 +- e2e/editor-save-deploy.spec.ts | 78 ++++++++ frontend/src/components/EditorLayout.tsx | 57 +++++- .../components/EditorLayout/EditorView.tsx | 126 ++++++++----- .../components/EditorLayout/ShellOverlays.tsx | 4 +- .../hooks/useContainerStats.test.ts | 63 +++++-- .../EditorLayout/hooks/useContainerStats.ts | 54 +++++- .../hooks/useStackActions.test.ts | 172 ++++++++++++++++++ .../EditorLayout/hooks/useStackActions.ts | 83 +++++++-- 11 files changed, 754 insertions(+), 137 deletions(-) create mode 100644 backend/src/__tests__/stacks-edit-hardening.test.ts create mode 100644 e2e/editor-save-deploy.spec.ts create mode 100644 frontend/src/components/EditorLayout/hooks/useStackActions.test.ts diff --git a/backend/src/__tests__/stacks-edit-hardening.test.ts b/backend/src/__tests__/stacks-edit-hardening.test.ts new file mode 100644 index 00000000..c9818904 --- /dev/null +++ b/backend/src/__tests__/stacks-edit-hardening.test.ts @@ -0,0 +1,97 @@ +import { describe, it, expect, beforeAll, afterAll, vi, beforeEach } from 'vitest'; +import request from 'supertest'; +import { setupTestDb, cleanupTestDb, loginAsTestAdmin } from './helpers/setupTestDb'; + +const { + mockGetContainersByStack, + mockGetStackContent, +} = vi.hoisted(() => ({ + mockGetContainersByStack: vi.fn(), + mockGetStackContent: vi.fn(), +})); + +vi.mock('../services/DockerController', async () => { + const actual = await vi.importActual( + '../services/DockerController', + ); + return { + ...actual, + default: { + ...actual.default, + getInstance: () => ({ + getContainersByStack: mockGetContainersByStack, + }), + }, + }; +}); + +vi.mock('../services/FileSystemService', () => ({ + FileSystemService: { + getInstance: () => ({ + getStacks: vi.fn().mockResolvedValue([]), + getBaseDir: () => '/tmp/compose', + hasComposeFile: vi.fn().mockResolvedValue(true), + getStackContent: mockGetStackContent, + }), + }, +})); + +let tmpDir: string; +let app: import('express').Express; +let authCookie: string; + +beforeAll(async () => { + tmpDir = await setupTestDb(); + ({ app } = await import('../index')); + authCookie = await loginAsTestAdmin(app); +}); + +afterAll(() => { + vi.restoreAllMocks(); + cleanupTestDb(tmpDir); +}); + +beforeEach(() => { + mockGetContainersByStack.mockReset(); + mockGetStackContent.mockReset(); +}); + +describe('GET /:stackName/containers — input validation', () => { + it('returns 400 for a stackName containing path traversal', async () => { + const res = await request(app) + .get('/api/stacks/..%2Fetc%2Fpasswd/containers') + .set('Cookie', authCookie); + expect(res.status).toBe(400); + expect(mockGetContainersByStack).not.toHaveBeenCalled(); + }); + + it('passes a valid stackName through to the DockerController', async () => { + mockGetContainersByStack.mockResolvedValue([]); + const res = await request(app) + .get('/api/stacks/myapp/containers') + .set('Cookie', authCookie); + expect(res.status).toBe(200); + expect(mockGetContainersByStack).toHaveBeenCalledWith('myapp'); + }); +}); + +describe('GET /:stackName/services — YAML parse size guard', () => { + it('returns 413 when the compose file exceeds the parse cap', async () => { + // 1 MiB cap; produce 1 MiB + 1 byte of content. + mockGetStackContent.mockResolvedValue('x'.repeat(1_048_577)); + const res = await request(app) + .get('/api/stacks/myapp/services') + .set('Cookie', authCookie); + expect(res.status).toBe(413); + expect(res.body.error).toMatch(/too large/i); + }); + + it('parses a small compose file normally', async () => { + mockGetStackContent.mockResolvedValue('services:\n web:\n image: nginx\n'); + const res = await request(app) + .get('/api/stacks/myapp/services') + .set('Cookie', authCookie); + expect(res.status).toBe(200); + expect(res.body).toEqual(['web']); + }); +}); diff --git a/backend/src/routes/stacks.ts b/backend/src/routes/stacks.ts index 0eb41b21..50b995a3 100644 --- a/backend/src/routes/stacks.ts +++ b/backend/src/routes/stacks.ts @@ -25,6 +25,11 @@ import { invalidateNodeCaches } from '../helpers/cacheInvalidation'; import { STACK_STATUSES_CACHE_TTL_MS } from '../helpers/constants'; import { getTerminalWs } from '../websocket/generic'; +// Authenticated users with edit permission can write arbitrarily large compose +// files. Refuse to YAML.parse anything beyond this bound so a malformed (or +// adversarial) file cannot exhaust heap during an env or service lookup. +const MAX_COMPOSE_PARSE_BYTES = 1_048_576; // 1 MiB + function notifyActionFailure(action: string, stackName: string, error: unknown): void { const message = getErrorMessage(error, `Failed to ${action} stack`); NotificationService.getInstance() @@ -113,6 +118,11 @@ export async function resolveAllEnvFilePaths(nodeId: number, stackName: string): if (!composeContent) return [defaultEnvPath]; + if (composeContent.length > MAX_COMPOSE_PARSE_BYTES) { + console.warn(`[Stacks] Compose for ${sanitizeForLog(stackName)} exceeds ${MAX_COMPOSE_PARSE_BYTES} bytes; skipping env_file resolution`); + return [defaultEnvPath]; + } + const parsed = YAML.parse(composeContent); if (!parsed?.services) return [defaultEnvPath]; @@ -284,6 +294,7 @@ stacksRouter.put('/:stackName', async (req: Request, res: Response) => { console.error('Content is not a string, got:', typeof content); return res.status(400).json({ error: 'Content must be a string' }); } + if (isDebugEnabled()) console.debug(`[Stacks:debug] Save starting`, { stackName: sanitizeForLog(stackName), bytes: content.length, nodeId: req.nodeId }); await FileSystemService.getInstance(req.nodeId).saveStackContent(stackName, content); invalidateNodeCaches(req.nodeId); console.log(`[Stacks] Compose file saved: ${sanitizeForLog(stackName)}`); @@ -590,80 +601,113 @@ stacksRouter.delete('/:stackName', async (req: Request, res: Response) => { const stackName = req.params.stackName as string; if (!requirePermission(req, res, 'stack:delete', 'stack', stackName)) return; const pruneVolumes = req.query.pruneVolumes === 'true'; + const debug = isDebugEnabled(); + const sanitizedName = sanitizeForLog(stackName); + if (debug) console.debug(`[Stacks:debug] Delete starting`, { stackName: sanitizedName, pruneVolumes, nodeId: req.nodeId }); + + // Step 1: compose down. Best-effort: a stack with corrupt compose files or + // a temporarily-unreachable daemon should still be removable. Logged so the + // operator can investigate orphaned containers separately. try { + await ComposeService.getInstance(req.nodeId).downStack(stackName); + if (debug) console.debug(`[Stacks:debug] Delete: down OK`, { stackName: sanitizedName }); + } catch (downErr) { + console.warn('[Stacks] Compose down failed or no-op for %s:', sanitizeForLog(stackName), downErr); + } + + // Step 2: volume prune (only if requested). Best-effort. + if (pruneVolumes) { try { - await ComposeService.getInstance(req.nodeId).downStack(stackName); - } catch (downErr) { - console.warn(`[Teardown] Docker down failed or nothing to clean up for ${sanitizeForLog(stackName)}`); + const result = await DockerController.getInstance().pruneManagedOnly('volumes', [stackName]); + console.log(`[Stacks] Pruned volumes for ${sanitizeForLog(stackName)}: ${result.reclaimedBytes} bytes reclaimed`); + } catch (pruneErr) { + console.warn('[Stacks] Volume prune failed for %s, continuing delete:', sanitizeForLog(stackName), pruneErr); } + } - if (pruneVolumes) { - try { - const result = await DockerController.getInstance().pruneManagedOnly('volumes', [stackName]); - console.log(`[Stacks] Pruned volumes for ${sanitizeForLog(stackName)}: ${result.reclaimedBytes} bytes reclaimed`); - } catch (pruneErr) { - console.warn('[Stacks] Volume prune failed for %s, continuing delete:', sanitizeForLog(stackName), pruneErr); - } - } - - let fsErr: unknown = null; - try { - await FileSystemService.getInstance(req.nodeId).deleteStack(stackName); - } catch (err) { - fsErr = err; - console.error('[Stacks] File deletion failed for %s, continuing with DB cleanup:', sanitizeForLog(stackName), err); - } + // Step 3: filesystem delete. If this fails the on-disk compose files are + // still present, so we abort BEFORE touching the database — keeping DB and + // FS in sync so the operator can retry. Otherwise a half-deleted stack + // (rows gone, files remain) becomes invisible to the UI. + try { + await FileSystemService.getInstance(req.nodeId).deleteStack(stackName); + if (debug) console.debug(`[Stacks:debug] Delete: fs OK`, { stackName: sanitizedName }); + } catch (fsErr) { + console.error('[Stacks] File deletion failed for %s; database state untouched:', sanitizeForLog(stackName), fsErr); + const message = getErrorMessage(fsErr, 'Failed to remove stack files'); + res.status(500).json({ + error: `${message}. Stack containers may have been stopped but on-disk files remain. Retry the delete or clean the files manually.`, + }); + return; + } + // Step 4: database cleanup. Per-call idempotent; safe to run sequentially. + try { DatabaseService.getInstance().clearStackUpdateStatus(req.nodeId, stackName); DatabaseService.getInstance().clearStackAutoUpdateSetting(req.nodeId, stackName); DatabaseService.getInstance().deleteRoleAssignmentsByResource('stack', stackName); DatabaseService.getInstance().deleteGitSource(stackName); - - // Cascade a mesh opt-out so the mesh_stacks row, override file on disk, - // and derived aliases do not outlive the stack. optOutStack is idempotent - // (no-op when the stack was never opted in). Best-effort: a mesh cleanup - // failure must not regress the delete itself. - try { - await MeshService.getInstance().optOutStack( - req.nodeId, - stackName, - req.user?.username ?? 'system', - ); - } catch (meshErr) { - console.warn( - '[Stacks] Mesh opt-out cascade failed for %s, continuing delete:', - sanitizeForLog(stackName), - meshErr, - ); - } - - if (fsErr) throw fsErr; - - invalidateNodeCaches(req.nodeId); - console.log(`[Stacks] Stack deleted: ${sanitizeForLog(stackName)}`); - res.json({ success: true }); - } catch (error: unknown) { - console.error('[Stacks] Failed to delete stack %s:', sanitizeForLog(stackName), error); - const message = getErrorMessage(error, 'Failed to delete stack'); - res.status(500).json({ error: message }); + if (debug) console.debug(`[Stacks:debug] Delete: db OK`, { stackName: sanitizedName }); + } catch (dbErr) { + console.error('[Stacks] Database cleanup failed for %s; files already removed:', sanitizeForLog(stackName), dbErr); + const message = getErrorMessage(dbErr, 'Failed to clear stack database state'); + res.status(500).json({ + error: `${message}. Stack files have been removed; some database rows for this stack may remain. Recreating the stack with the same name will reuse those rows.`, + }); + return; } + + // Step 5: mesh opt-out cascade. Idempotent; best-effort cleanup of derived + // aliases / override files. A mesh failure here must not flip the delete + // result, since the stack itself is already gone. + try { + await MeshService.getInstance().optOutStack( + req.nodeId, + stackName, + req.user?.username ?? 'system', + ); + } catch (meshErr) { + console.warn( + '[Stacks] Mesh opt-out cascade failed for %s, continuing delete:', + sanitizeForLog(stackName), + meshErr, + ); + } + + invalidateNodeCaches(req.nodeId); + console.log(`[Stacks] Stack deleted: ${sanitizeForLog(stackName)}`); + res.json({ success: true }); }); stacksRouter.get('/:stackName/containers', async (req: Request, res: Response) => { + const stackName = req.params.stackName as string; + if (!isValidStackName(stackName)) { + res.status(400).json({ error: 'Invalid stack name' }); + return; + } try { - const stackName = req.params.stackName as string; const dockerController = DockerController.getInstance(req.nodeId); const containers = await dockerController.getContainersByStack(stackName); res.json(containers); } catch (error) { + console.error('[Stacks] Failed to fetch containers for %s:', sanitizeForLog(stackName), error); res.status(500).json({ error: 'Failed to fetch containers' }); } }); stacksRouter.get('/:stackName/services', async (req: Request, res: Response) => { + const stackName = req.params.stackName as string; + if (!isValidStackName(stackName)) { + res.status(400).json({ error: 'Invalid stack name' }); + return; + } try { - const stackName = req.params.stackName as string; const content = await FileSystemService.getInstance(req.nodeId).getStackContent(stackName); + if (content.length > MAX_COMPOSE_PARSE_BYTES) { + console.warn(`[Stacks] Compose for ${sanitizeForLog(stackName)} exceeds ${MAX_COMPOSE_PARSE_BYTES} bytes; refusing to parse services`); + res.status(413).json({ error: 'Compose file too large to parse' }); + return; + } const parsed = YAML.parse(content); const services = parsed?.services ? Object.keys(parsed.services) : []; res.json(services); @@ -721,6 +765,7 @@ stacksRouter.post('/:stackName/down', async (req: Request, res: Response) => { // Lock held below. All early-returns must stay inside the try so finally fires. if (!tryAcquireStackOpLock(req, res, stackName, 'down')) return; try { + if (isDebugEnabled()) console.debug(`[Stacks:debug] Down starting`, { stackName: sanitizeForLog(stackName), nodeId: req.nodeId }); await ComposeService.getInstance(req.nodeId).runCommand(stackName, 'down', getTerminalWs()); invalidateNodeCaches(req.nodeId); console.log(`[Stacks] Down completed: ${sanitizeForLog(stackName)}`); @@ -778,6 +823,7 @@ async function bulkContainerOp( if (!tryAcquireStackOpLock(req, res, stackName, action)) return; try { const titleCase = action.charAt(0).toUpperCase() + action.slice(1); + if (isDebugEnabled()) console.debug(`[Stacks:debug] ${titleCase} starting`, { stackName: sanitizeForLog(stackName), nodeId: req.nodeId }); const outcome = await containerActionForStack(req.nodeId, stackName, action); if (outcome.kind === 'no-containers') { diff --git a/docs/features/editor.mdx b/docs/features/editor.mdx index 69ca0161..42d9baec 100644 --- a/docs/features/editor.mdx +++ b/docs/features/editor.mdx @@ -26,7 +26,7 @@ The top card on the left holds the stack's identity and primary controls. ### Stack action bar -The action bar runs every state transition for the whole stack. It is hidden for roles without the `stack:deploy` permission. +The action bar runs every state transition for the whole stack. The primary buttons (**Start**, **Restart**, **Stop**, **Update**) require the `stack:deploy` permission; the **Delete** entry in the kebab dropdown requires the `stack:delete` permission. The bar still appears when only **Delete** is authorised so the operator has a way to remove the stack. | Button | Behavior | |--------|----------| @@ -39,7 +39,7 @@ The kebab dropdown carries: - **Rollback ``** restores the previous deployment using the stored snapshot. Visible only when a backup exists. Skipper or Admiral. - **Scan config** runs Trivy against the compose configuration and surfaces misconfigurations inline. Visible only when Trivy is reachable. Admin role required. Skipper or Admiral. -- **Delete** stops the stack and removes its compose directory. Always available to the deploy role. +- **Delete** stops the stack and removes its compose directory. Requires the `stack:delete` permission. ## Containers list @@ -197,4 +197,13 @@ Sencho tries `/bin/bash` first and transparently falls back to `/bin/sh` if bash The WebSocket dropped silently. Close the modal and reopen it to start a fresh session; the bash modal does not reconnect automatically. + + The PUT to write the compose file failed (most often a permission or disk-space issue on the node). Save & Deploy stops at the save step in this case, so no deploy is attempted and your editor still holds the unsaved version. Resolve the underlying error on the node, then retry. + + + The editor blocks a silent loss of in-progress edits. Click **Cancel** to return to the original node with your edits intact; click **Discard** to abandon them and proceed to the other node. + + + The stats WebSocket failed to open, or closed unexpectedly. This usually means the Docker daemon on the node is unreachable. Container status, action buttons, and logs continue to work; only live CPU / memory / network rates pause. + diff --git a/e2e/editor-save-deploy.spec.ts b/e2e/editor-save-deploy.spec.ts new file mode 100644 index 00000000..fe39b4a8 --- /dev/null +++ b/e2e/editor-save-deploy.spec.ts @@ -0,0 +1,78 @@ +/** + * EditorView save-and-deploy: a failed PUT must abort the deploy. Verified by + * intercepting the PUT with a forced 500 and asserting that no POST to /deploy + * is observed, plus a "Failed to save file" toast surfaces. + */ +import { test, expect } from '@playwright/test'; +import { loginAs, waitForStacksLoaded } from './helpers'; + +const TEST_STACK = 'e2e-save-deploy-stack'; + +async function deleteTestStack(page: import('@playwright/test').Page) { + await page.evaluate(async (name) => { + await fetch(`/api/stacks/${name}`, { method: 'DELETE', credentials: 'include' }).catch(() => { }); + }, TEST_STACK); +} + +async function createTestStack(page: import('@playwright/test').Page) { + await page.getByRole('button', { name: 'Create Stack' }).click(); + await expect(page.getByRole('dialog')).toBeVisible({ timeout: 5_000 }); + await page.locator('#create-stack-name').fill(TEST_STACK); + await page.locator('[role="dialog"]').getByRole('button', { name: 'Create' }).click(); + await expect(page.getByRole('dialog')).toBeHidden({ timeout: 8_000 }); +} + +test.describe('EditorView save-and-deploy', () => { + test.beforeEach(async ({ page }) => { + await loginAs(page); + await waitForStacksLoaded(page); + await deleteTestStack(page); + await page.waitForTimeout(300); + await page.reload(); + await loginAs(page); + await waitForStacksLoaded(page); + await createTestStack(page); + // Open the new stack in the editor. + await page.locator('[role="listbox"]').getByText(TEST_STACK, { exact: true }).click(); + }); + + test.afterEach(async ({ page }) => { + await deleteTestStack(page); + }); + + test('does NOT deploy when the save PUT fails', async ({ page }) => { + // Force the compose-save PUT to fail. + await page.route(`**/api/stacks/${TEST_STACK}`, async (route, req) => { + if (req.method() === 'PUT') { + await route.fulfill({ status: 500, body: 'forced save failure' }); + return; + } + await route.continue(); + }); + + // Track whether the deploy POST is ever attempted. + let deployAttempts = 0; + await page.route(`**/api/stacks/${TEST_STACK}/deploy*`, async (route, req) => { + if (req.method() === 'POST') deployAttempts += 1; + await route.continue(); + }); + + // The editor surface has two distinct edit affordances. First click swaps + // the right panel from Anatomy to the editor tabs (Monaco mounts read-only); + // second click flips Monaco into edit mode and reveals Save & Deploy. + await page.getByRole('button', { name: /^edit$/ }).click(); + await page.getByRole('button', { name: /^Edit$/ }).click(); + + // No need to modify Monaco content: saveFile fires the PUT regardless of + // dirty state. The route interceptor forces it to 500; the gated handler + // then must not call POST /deploy. + await page.getByRole('button', { name: 'Save & Deploy', exact: true }).click(); + + // Failure toast must appear. + await expect(page.getByText(/failed to save file/i)).toBeVisible({ timeout: 5_000 }); + + // Give the UI a beat to (incorrectly) fire a deploy if the guard is broken. + await page.waitForTimeout(1_000); + expect(deployAttempts).toBe(0); + }); +}); diff --git a/frontend/src/components/EditorLayout.tsx b/frontend/src/components/EditorLayout.tsx index 5327ce3f..46b17e26 100644 --- a/frontend/src/components/EditorLayout.tsx +++ b/frontend/src/components/EditorLayout.tsx @@ -12,7 +12,7 @@ import { useEditorViewState } from './EditorLayout/hooks/useEditorViewState'; import { useStackListState } from './EditorLayout/hooks/useStackListState'; import { useViewNavigationState } from './EditorLayout/hooks/useViewNavigationState'; import { useOverlayState } from './EditorLayout/hooks/useOverlayState'; -import { useStackActions } from './EditorLayout/hooks/useStackActions'; +import { useStackActions, NODE_SWITCH_PENDING_TOKEN } from './EditorLayout/hooks/useStackActions'; import { useTheme } from './EditorLayout/hooks/useTheme'; import { useNotifications } from './EditorLayout/hooks/useNotifications'; import { useContainerStats } from './EditorLayout/hooks/useContainerStats'; @@ -150,7 +150,10 @@ export default function EditorLayout() { onImageUpdatesChange: fetchImageUpdates, }); - const containerStats = useContainerStats(containers); + const { stats: containerStats, error: containerStatsError } = useContainerStats( + containers, + activeNode?.id ?? null, + ); const stackActions = useStackActions({ editorState, @@ -223,10 +226,59 @@ export default function EditorLayout() { const { theme, setTheme, isDarkMode } = useTheme(); + // Track the last "committed" node id so the node-switch dirty guard can + // detect an actual switch (vs the initial mount or an internal revert). + const lastSeenNodeIdRef = useRef(activeNode?.id ?? null); + // Set true when we revert activeNode after stashing a pending switch, so the + // re-fire of this effect on the reverted id is a no-op. + const revertingNodeSwitchRef = useRef(false); + // Re-fetch stacks whenever the active node changes (or becomes available on mount). // Also clears any stale editor/container state that belonged to the previous node. useEffect(() => { + if (revertingNodeSwitchRef.current) { + revertingNodeSwitchRef.current = false; + return; + } if (!activeNode) return; + + const previousId = lastSeenNodeIdRef.current; + const isRealSwitch = previousId !== null && previousId !== activeNode.id; + + // A node-switch prompt is already open. Ignore any further activeNode + // changes until the user resolves the current dialog; revert silently so + // the dialog's pendingUnsavedNode stays anchored to the first attempt. + if (isRealSwitch && overlayState.pendingUnsavedLoad === NODE_SWITCH_PENDING_TOKEN) { + const previousNode = nodes.find(n => n.id === previousId); + if (previousNode) { + revertingNodeSwitchRef.current = true; + setActiveNode(previousNode); + } + return; + } + + if (isRealSwitch && stackActions.hasUnsavedChanges()) { + const previousNode = nodes.find(n => n.id === previousId); + if (previousNode) { + // Stash the attempted node + open the unsaved-changes dialog via the + // existing pendingUnsavedLoad/Node fields. Revert activeNode back to + // the previous node; the revertingNodeSwitchRef makes the resulting + // effect fire a no-op so dirty content survives. + overlayState.setPendingUnsavedNode(activeNode); + overlayState.setPendingUnsavedLoad(NODE_SWITCH_PENDING_TOKEN); + revertingNodeSwitchRef.current = true; + setActiveNode(previousNode); + return; + } + // Previous node is no longer reachable from the nodes list (deleted or + // dropped from the registry). We cannot revert, so the unsaved edits + // will be lost. Warn the operator before the wipe so the loss is at + // least visible. + toast.warning('Unsaved changes were discarded: the previous node is no longer available.'); + } + + lastSeenNodeIdRef.current = activeNode.id; + const pendingStack = pendingStackLoadRef.current; pendingStackLoadRef.current = null; @@ -420,6 +472,7 @@ export default function EditorLayout() { isDarkMode={isDarkMode} containers={containers} containerStats={containerStats} + containerStatsError={containerStatsError} content={content} envContent={envContent} envExists={envExists} diff --git a/frontend/src/components/EditorLayout/EditorView.tsx b/frontend/src/components/EditorLayout/EditorView.tsx index f10ec608..e3f6ddde 100644 --- a/frontend/src/components/EditorLayout/EditorView.tsx +++ b/frontend/src/components/EditorLayout/EditorView.tsx @@ -165,6 +165,7 @@ export interface EditorViewProps { // Stack data (raw; safe-wrapped locally for backwards-compat with prior idiom) containers: ContainerInfo[]; containerStats: Record; + containerStatsError: string | null; content: string; envContent: string; envExists: boolean; @@ -235,6 +236,7 @@ export function EditorView({ isDarkMode, containers, containerStats, + containerStatsError, content, envContent, envExists, @@ -282,6 +284,22 @@ export function EditorView({ }: EditorViewProps) { const monacoEditorRef = useRef(null); + // Dispose the underlying Monaco model when EditorView unmounts. The + // @monaco-editor/react wrapper reuses a single model per editor instance + // (we do not pass a `path`), so this catches the unmount case rather than + // a per-stack-switch leak. + useEffect(() => { + return () => { + const editor = monacoEditorRef.current; + if (!editor) return; + try { + editor.getModel()?.dispose(); + } catch { + // Editor already torn down by Monaco; nothing to do. + } + }; + }, []); + // Force Monaco to re-measure its container after the tab switch DOM settles. // Monaco's internal child is position:static with an explicit pixel height that // creates a circular CSS dependency (Monaco drives card height -> grid height -> Monaco). @@ -373,35 +391,45 @@ export function EditorView({ ); })()} - {/* Action Bar */} - {can('stack:deploy', 'stack', stackName) && ( -
- {isRunning ? ( - - ) : ( - - )} - {isRunning && ( - - )} - - {(() => { - const canRollback = isPaid && backupInfo.exists; - const canScan = trivy.available && isAdmin; - const hasOverflowExtras = canRollback || canScan; - return ( + {/* Action Bar — deploy / delete affordances render against + their own backend permissions so a delete-only or + deploy-only persona sees exactly what they can act on. */} + {(() => { + const canDeploy = can('stack:deploy', 'stack', stackName); + const canDelete = can('stack:delete', 'stack', stackName); + const canRollback = canDeploy && isPaid && backupInfo.exists; + const canScan = trivy.available && isAdmin; + const hasOverflowExtras = canRollback || canScan; + const hasOverflow = hasOverflowExtras || canDelete; + if (!canDeploy && !hasOverflow) return null; + return ( +
+ {canDeploy && ( + <> + {isRunning ? ( + + ) : ( + + )} + {isRunning && ( + + )} + + + )} + {hasOverflow && (
- )} + )} +
+ ); + })()} {/* Per-container health strip */}
-

CONTAINERS

+
+

CONTAINERS

+ {containerStatsError && safeContainers.length > 0 && ( + + Stats unavailable + + )} +
{safeContainers.length === 0 ? (
No containers running for this stack.
) : ( diff --git a/frontend/src/components/EditorLayout/ShellOverlays.tsx b/frontend/src/components/EditorLayout/ShellOverlays.tsx index d5d17baa..5de73521 100644 --- a/frontend/src/components/EditorLayout/ShellOverlays.tsx +++ b/frontend/src/components/EditorLayout/ShellOverlays.tsx @@ -148,8 +148,8 @@ export function ShellOverlays({ setDiffPreviewConfirming(true); try { if (snapshot?.mode === 'save-and-deploy') { - await stackActions.saveFile(); - await stackActions.deployStack(); + const saved = await stackActions.saveFile(); + if (saved) await stackActions.deployStack(); } else { await stackActions.saveFile(); } diff --git a/frontend/src/components/EditorLayout/hooks/useContainerStats.test.ts b/frontend/src/components/EditorLayout/hooks/useContainerStats.test.ts index ac195086..caa912f5 100644 --- a/frontend/src/components/EditorLayout/hooks/useContainerStats.test.ts +++ b/frontend/src/components/EditorLayout/hooks/useContainerStats.test.ts @@ -7,7 +7,7 @@ class MockWS { static instances: MockWS[] = []; onopen: (() => void) | null = null; onmessage: ((e: { data: string }) => void) | null = null; - onclose: (() => void) | null = null; + onclose: ((e: { code: number }) => void) | null = null; onerror: (() => void) | null = null; readyState = 1; send = vi.fn(); @@ -34,17 +34,18 @@ afterEach(() => { describe('useContainerStats', () => { it('returns empty stats for empty containers', () => { - const { result } = renderHook(() => useContainerStats([])); - expect(result.current).toEqual({}); + const { result } = renderHook(() => useContainerStats([], 1)); + expect(result.current.stats).toEqual({}); + expect(result.current.error).toBeNull(); }); it('opens one WebSocket per container', () => { - renderHook(() => useContainerStats([makeContainer('abc'), makeContainer('def')])); + renderHook(() => useContainerStats([makeContainer('abc'), makeContainer('def')], 1)); expect(MockWS.instances).toHaveLength(2); }); it('sends streamStats action on WS open', () => { - renderHook(() => useContainerStats([makeContainer('abc')])); + renderHook(() => useContainerStats([makeContainer('abc')], 1)); act(() => { MockWS.instances[0]?.onopen?.(); }); expect(MockWS.instances[0].send).toHaveBeenCalledWith( expect.stringContaining('"action":"streamStats"'), @@ -52,7 +53,7 @@ describe('useContainerStats', () => { }); it('flushes buffered stats into state after 1500ms', () => { - const { result } = renderHook(() => useContainerStats([makeContainer('c1')])); + const { result } = renderHook(() => useContainerStats([makeContainer('c1')], 1)); act(() => { MockWS.instances[0]?.onopen?.(); }); const statsMsg = { @@ -63,21 +64,57 @@ describe('useContainerStats', () => { }; act(() => { MockWS.instances[0]?.onmessage?.({ data: JSON.stringify(statsMsg) }); }); - expect(result.current['c1']).toBeUndefined(); + expect(result.current.stats['c1']).toBeUndefined(); act(() => { vi.advanceTimersByTime(1500); }); - expect(result.current['c1']).toBeDefined(); - expect(result.current['c1'].cpu).toContain('%'); - expect(result.current['c1'].ram).toContain('MB'); + expect(result.current.stats['c1']).toBeDefined(); + expect(result.current.stats['c1'].cpu).toContain('%'); + expect(result.current.stats['c1'].ram).toContain('MB'); }); it('closes all WebSockets when containers change', () => { const { rerender } = renderHook( - (containers: ContainerInfo[]) => useContainerStats(containers), - { initialProps: [makeContainer('old')] }, + ({ containers, nodeId }: { containers: ContainerInfo[]; nodeId: number }) => + useContainerStats(containers, nodeId), + { initialProps: { containers: [makeContainer('old')], nodeId: 1 } }, ); const firstWs = MockWS.instances[0]; - rerender([makeContainer('new')]); + rerender({ containers: [makeContainer('new')], nodeId: 1 }); expect(firstWs.close).toHaveBeenCalled(); }); + + it('reopens WebSockets when activeNodeId changes', () => { + const { rerender } = renderHook( + ({ containers, nodeId }: { containers: ContainerInfo[]; nodeId: number }) => + useContainerStats(containers, nodeId), + { initialProps: { containers: [makeContainer('abc')], nodeId: 1 } }, + ); + const firstWs = MockWS.instances[0]; + rerender({ containers: [makeContainer('abc')], nodeId: 2 }); + expect(firstWs.close).toHaveBeenCalled(); + expect(MockWS.instances).toHaveLength(2); + }); + + // These two cases run with real timers so React's scheduler can commit the + // setError call triggered from inside the WebSocket onclose handler. + it('warns on abnormal WebSocket close (code 1006)', () => { + vi.useRealTimers(); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + renderHook(() => useContainerStats([makeContainer('c1')], 1)); + act(() => { MockWS.instances[0]?.onclose?.({ code: 1006 } as CloseEvent); }); + expect(warnSpy).toHaveBeenCalledWith( + '[useContainerStats] stats stream failure:', + expect.objectContaining({ kind: 'ws-closed' }), + ); + warnSpy.mockRestore(); + }); + + it('does NOT warn on a normal close (code 1000)', () => { + vi.useRealTimers(); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + renderHook(() => useContainerStats([makeContainer('c1')], 1)); + act(() => { MockWS.instances[0]?.onclose?.({ code: 1000 } as CloseEvent); }); + expect(warnSpy).not.toHaveBeenCalled(); + warnSpy.mockRestore(); + }); }); diff --git a/frontend/src/components/EditorLayout/hooks/useContainerStats.ts b/frontend/src/components/EditorLayout/hooks/useContainerStats.ts index 952e3b0a..f73cc575 100644 --- a/frontend/src/components/EditorLayout/hooks/useContainerStats.ts +++ b/frontend/src/components/EditorLayout/hooks/useContainerStats.ts @@ -9,8 +9,17 @@ function formatBytes(bytes: number): string { return parseFloat((bytes / Math.pow(k, i)).toFixed(2)) + ' ' + sizes[i]; } -export function useContainerStats(containers: ContainerInfo[]): Record { +export interface ContainerStatsHookResult { + stats: Record; + error: string | null; +} + +export function useContainerStats( + containers: ContainerInfo[], + activeNodeId: number | null | undefined, +): ContainerStatsHookResult { const [containerStats, setContainerStats] = useState>({}); + const [error, setError] = useState(null); const pendingStatsRef = useRef>({}); + // Reset the error banner only on an actual node switch. Doing it inside the + // main effect would clear the banner on every containers-array identity + // change (which fires at the dashboard's refresh cadence) and cause the + // banner to visibly flap against a persistently-flaky daemon. + useEffect(() => { + setError(null); + }, [activeNodeId]); + useEffect(() => { const wsMap: Record = {}; + // Track which WSs have already emitted a console.warn for the lifetime of + // this effect, so a flaky daemon does not spam the console at message rate. + const warnedOnce = new Set(); + + const nodeQuery = activeNodeId != null ? `?nodeId=${activeNodeId}` : ''; (containers || []).forEach(container => { if (!container?.Id) return; + + const recordFailure = (kind: 'ws-error' | 'ws-closed' | 'parse-error' | 'ws-create') => { + if (!warnedOnce.has(container.Id)) { + warnedOnce.add(container.Id); + console.warn('[useContainerStats] stats stream failure:', { kind, containerId: container.Id }); + } + setError('Container stats unavailable. The node may be unreachable.'); + }; + try { const wsProtocol = window.location.protocol === 'https:' ? 'wss:' : 'ws:'; - const activeNodeId = localStorage.getItem('sencho-active-node') || ''; - const ws = new WebSocket(`${wsProtocol}//${window.location.host}/ws${activeNodeId ? `?nodeId=${activeNodeId}` : ''}`); + const ws = new WebSocket(`${wsProtocol}//${window.location.host}/ws${nodeQuery}`); wsMap[container.Id] = ws; ws.onopen = () => ws.send(JSON.stringify({ action: 'streamStats', containerId: container.Id, - nodeId: activeNodeId || undefined, + nodeId: activeNodeId ?? undefined, })); + ws.onerror = () => recordFailure('ws-error'); + ws.onclose = (event) => { + // 1000 normal, 1001 going-away are expected during cleanup or navigation. + if (event.code !== 1000 && event.code !== 1001) recordFailure('ws-closed'); + }; + ws.onmessage = (event) => { try { const data = JSON.parse(event.data); @@ -74,11 +110,11 @@ export function useContainerStats(containers: ContainerInfo[]): Record { clearInterval(flushInterval); pendingStatsRef.current = {}; - Object.values(wsMap).forEach(ws => { try { ws.close(); } catch { /* ignore */ } }); + Object.values(wsMap).forEach(ws => { try { ws.close(1000); } catch { /* ignore */ } }); }; - }, [containers]); // eslint-disable-line react-hooks/exhaustive-deps + }, [containers, activeNodeId]); // eslint-disable-line react-hooks/exhaustive-deps - return containerStats; + return { stats: containerStats, error }; } diff --git a/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts b/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts new file mode 100644 index 00000000..1b8ea583 --- /dev/null +++ b/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts @@ -0,0 +1,172 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { renderHook } from '@testing-library/react'; +import { useStackActions } from './useStackActions'; +import type { useEditorViewState } from './useEditorViewState'; +import type { useStackListState } from './useStackListState'; +import type { useViewNavigationState } from './useViewNavigationState'; +import type { OverlayState } from './useOverlayState'; + +vi.mock('@/lib/api', () => ({ apiFetch: vi.fn() })); +vi.mock('@/components/ui/toast-store', () => ({ + toast: { success: vi.fn(), error: vi.fn(), info: vi.fn() }, +})); + +import { apiFetch } from '@/lib/api'; + +type EditorState = ReturnType; +type StackListState = ReturnType; +type NavState = ReturnType; + +function makeEditorState(over: Partial = {}): EditorState { + const base = { + content: 'services: {}', + originalContent: 'services: {}\n', + envContent: '', + originalEnvContent: '', + activeTab: 'compose' as const, + selectedEnvFile: '', + setContent: vi.fn(), + setOriginalContent: vi.fn(), + setEnvContent: vi.fn(), + setOriginalEnvContent: vi.fn(), + setIsEditing: vi.fn(), + setEditingCompose: vi.fn(), + setActiveTab: vi.fn(), + setContainers: vi.fn(), + setEnvFiles: vi.fn(), + setSelectedEnvFile: vi.fn(), + setEnvExists: vi.fn(), + setBackupInfo: vi.fn(), + setIsFileLoading: vi.fn(), + setGitSourcePendingMap: vi.fn(), + }; + return { ...base, ...over } as unknown as EditorState; +} + +function makeStackListState(over: Partial = {}): StackListState { + const base = { + selectedFile: 'web.yml', + files: ['web.yml'], + stackStatuses: { 'web.yml': 'running' }, + setSelectedFile: vi.fn(), + setOptimisticStatus: vi.fn(), + setStackAction: vi.fn(), + clearStackAction: vi.fn(), + isStackBusy: vi.fn().mockReturnValue(false), + refreshStacks: vi.fn(), + setSearchQuery: vi.fn(), + }; + return { ...base, ...over } as unknown as StackListState; +} + +function makeOverlay(): OverlayState { + return { + setPendingUnsavedLoad: vi.fn(), + setPendingUnsavedNode: vi.fn(), + pendingUnsavedLoad: null, + pendingUnsavedNode: null, + setPolicyBlock: vi.fn(), + setPolicyBypassing: vi.fn(), + setDiffPreview: vi.fn(), + } as unknown as OverlayState; +} + +const runWithLog: Parameters[0]['runWithLog'] = async (_p, run) => + run(Promise.resolve()); + +function setup(over: { editorState?: Partial } = {}) { + const editorState = makeEditorState(over.editorState); + const stackListState = makeStackListState(); + const navState = { setActiveView: vi.fn() } as unknown as NavState; + const overlayState = makeOverlay(); + + const { result } = renderHook(() => + useStackActions({ + editorState, + stackListState, + navState, + overlayState, + activeNode: { id: 1, type: 'local' } as Parameters[0]['activeNode'], + setActiveNode: vi.fn(), + nodes: [], + isPaid: false, + runWithLog, + diffPreviewEnabled: false, + }), + ); + return { result, editorState, stackListState, overlayState }; +} + +describe('useStackActions.saveFile', () => { + beforeEach(() => { + vi.mocked(apiFetch).mockReset(); + }); + + it('returns true and stores originalContent when the PUT succeeds', async () => { + vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 })); + const { result, editorState } = setup({ + editorState: { content: 'new', originalContent: 'old' }, + }); + const ok = await result.current.saveFile(); + expect(ok).toBe(true); + expect(editorState.setOriginalContent).toHaveBeenCalledWith('new'); + }); + + it('returns false and leaves dirty state intact when the PUT fails', async () => { + vi.mocked(apiFetch).mockResolvedValue(new Response('boom', { status: 500 })); + const { result, editorState } = setup({ + editorState: { content: 'new', originalContent: 'old' }, + }); + const ok = await result.current.saveFile(); + expect(ok).toBe(false); + expect(editorState.setOriginalContent).not.toHaveBeenCalled(); + }); + + it('returns false when no stack is selected', async () => { + vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 })); + const editorState = makeEditorState(); + const stackListState = makeStackListState({ selectedFile: null }); + const { result } = renderHook(() => + useStackActions({ + editorState, + stackListState, + navState: { setActiveView: vi.fn() } as unknown as NavState, + overlayState: makeOverlay(), + activeNode: { id: 1, type: 'local' } as Parameters[0]['activeNode'], + setActiveNode: vi.fn(), + nodes: [], + isPaid: false, + runWithLog, + diffPreviewEnabled: false, + }), + ); + const ok = await result.current.saveFile(); + expect(ok).toBe(false); + expect(apiFetch).not.toHaveBeenCalled(); + }); +}); + +describe('useStackActions.handleSaveAndDeploy', () => { + beforeEach(() => { + vi.mocked(apiFetch).mockReset(); + }); + + it('does NOT call /deploy when saveFile fails', async () => { + // PUT returns 500 → saveFile resolves false → deploy must not be attempted. + vi.mocked(apiFetch).mockResolvedValueOnce(new Response('save broke', { status: 500 })); + const { result } = setup(); + await result.current.handleSaveAndDeploy({ preventDefault: vi.fn(), stopPropagation: vi.fn() } as unknown as React.MouseEvent); + const calls = vi.mocked(apiFetch).mock.calls.map(c => c[0]); + expect(calls.some(c => String(c).includes('/deploy'))).toBe(false); + }); + + it('calls /deploy when saveFile succeeds', async () => { + vi.mocked(apiFetch).mockResolvedValueOnce(new Response(null, { status: 200 })); // save OK + vi.mocked(apiFetch).mockResolvedValueOnce(new Response(null, { status: 200 })); // deploy OK + vi.mocked(apiFetch).mockResolvedValueOnce(new Response('[]', { status: 200 })); // containers refresh + const { result } = setup(); + await result.current.handleSaveAndDeploy({ preventDefault: vi.fn(), stopPropagation: vi.fn() } as unknown as React.MouseEvent); + const calls = vi.mocked(apiFetch).mock.calls.map(c => c[0]); + expect(calls.some(c => String(c).includes('/deploy'))).toBe(true); + }); +}); diff --git a/frontend/src/components/EditorLayout/hooks/useStackActions.ts b/frontend/src/components/EditorLayout/hooks/useStackActions.ts index 433e8cc7..686fef35 100644 --- a/frontend/src/components/EditorLayout/hooks/useStackActions.ts +++ b/frontend/src/components/EditorLayout/hooks/useStackActions.ts @@ -17,6 +17,12 @@ interface RunResult { rolledBack?: boolean; } +// Sentinel stored in overlayState.pendingUnsavedLoad to mark that the pending +// confirmation is a node switch (not a stack load). When the user confirms the +// discard, discardAndLoadPending calls setActiveNode(targetNode) and skips the +// stack-load branch. +export const NODE_SWITCH_PENDING_TOKEN = '__node-switch-pending__'; + type StackActionError = Error & { rolledBack?: boolean }; type StackOpAction = 'deploy' | 'down' | 'restart' | 'stop' | 'start' | 'update'; @@ -130,15 +136,24 @@ export function useStackActions(options: UseStackActionsOptions) { const pendingStackLoadRef = useRef(null); const pendingLogsRef = useRef<{ stackName: string; containerName: string } | null>(null); const checkUpdatesIntervalRef = useRef | null>(null); + // Aborts the most recent loadFile sequence (compose GET, envs GET, env content + // GET, containers GET, backup GET). A node switch, an unmount, or a second + // loadFile call before the first finishes all cancel the in-flight fetches so + // late responses never overwrite freshly-loaded state. + const loadFileAbortRef = useRef(null); useEffect(() => { return () => { if (checkUpdatesIntervalRef.current !== null) { clearInterval(checkUpdatesIntervalRef.current); } + loadFileAbortRef.current?.abort(); }; }, []); + const isAbortError = (err: unknown): boolean => + err instanceof Error && err.name === 'AbortError'; + const hasUnsavedChanges = () => editorState.content !== editorState.originalContent || editorState.envContent !== editorState.originalEnvContent; @@ -164,6 +179,11 @@ export function useStackActions(options: UseStackActionsOptions) { }; const resetEditorState = () => { + // Cancel any in-flight loadFile chain before wiping state; a late response + // arriving after the reset would otherwise repopulate the editor with the + // previous node's data. + loadFileAbortRef.current?.abort(); + loadFileAbortRef.current = null; stackListState.setSelectedFile(null); editorState.setContent(''); editorState.setOriginalContent(''); @@ -221,14 +241,16 @@ export function useStackActions(options: UseStackActionsOptions) { editorState.setEnvExists(false); }; - const loadEnvState = async (filename: string) => { + const loadEnvState = async (filename: string, signal?: AbortSignal) => { try { - const envsRes = await apiFetch(`/stacks/${filename}/envs`); + const envsRes = await apiFetch(`/stacks/${filename}/envs`, { signal }); + if (signal?.aborted) return; if (!envsRes.ok) { clearEnvState(); return; } const { envFiles } = await envsRes.json(); + if (signal?.aborted) return; if (envFiles && envFiles.length > 0) { editorState.setEnvFiles(envFiles); const firstFile = envFiles[0]; @@ -236,7 +258,9 @@ export function useStackActions(options: UseStackActionsOptions) { editorState.setEnvExists(true); const envContentRes = await apiFetch( `/stacks/${filename}/env?file=${encodeURIComponent(firstFile)}`, + { signal }, ); + if (signal?.aborted) return; if (envContentRes.ok) { const envText = await envContentRes.text(); editorState.setEnvContent(envText || ''); @@ -248,29 +272,34 @@ export function useStackActions(options: UseStackActionsOptions) { } else { clearEnvState(); } - } catch { + } catch (err) { + if (isAbortError(err)) return; clearEnvState(); } }; - const loadContainerState = async (filename: string) => { + const loadContainerState = async (filename: string, signal?: AbortSignal) => { try { - const containersRes = await apiFetch(`/stacks/${filename}/containers`); + const containersRes = await apiFetch(`/stacks/${filename}/containers`, { signal }); + if (signal?.aborted) return; const conts = await containersRes.json(); editorState.setContainers(Array.isArray(conts) ? conts : []); } catch (error) { + if (isAbortError(error)) return; console.error('Failed to load containers:', error); editorState.setContainers([]); } }; - const loadBackupState = async (filename: string) => { + const loadBackupState = async (filename: string, signal?: AbortSignal) => { if (!isPaid) return; try { - const backupRes = await apiFetch(`/stacks/${filename}/backup`); + const backupRes = await apiFetch(`/stacks/${filename}/backup`, { signal }); + if (signal?.aborted) return; if (backupRes.ok) editorState.setBackupInfo(await backupRes.json()); else editorState.setBackupInfo({ exists: false, timestamp: null }); - } catch { + } catch (err) { + if (isAbortError(err)) return; editorState.setBackupInfo({ exists: false, timestamp: null }); } }; @@ -285,21 +314,31 @@ export function useStackActions(options: UseStackActionsOptions) { overlayState.setPendingUnsavedLoad(filename); return; } + // Cancel any in-flight load before starting a new one. A late response + // from the previous stack must not overwrite the freshly-loaded one. + loadFileAbortRef.current?.abort(); + const controller = new AbortController(); + loadFileAbortRef.current = controller; + const { signal } = controller; + editorState.setIsFileLoading(true); editorState.setIsEditing(false); editorState.setEditingCompose(false); editorState.setActiveTab('compose'); try { - const res = await apiFetch(`/stacks/${filename}`); + const res = await apiFetch(`/stacks/${filename}`, { signal }); + if (signal.aborted) return; const text = await res.text(); + if (signal.aborted) return; stackListState.setSelectedFile(filename); navState.setActiveView('editor'); editorState.setContent(text || ''); editorState.setOriginalContent(text || ''); - await loadEnvState(filename); - await loadContainerState(filename); - await loadBackupState(filename); + await loadEnvState(filename, signal); + await loadContainerState(filename, signal); + await loadBackupState(filename, signal); } catch (error) { + if (isAbortError(error) || signal.aborted) return; console.error('Failed to load file:', error); stackListState.setSelectedFile(null); editorState.setContent(''); @@ -308,7 +347,9 @@ export function useStackActions(options: UseStackActionsOptions) { editorState.setOriginalEnvContent(''); editorState.setContainers([]); } finally { - editorState.setIsFileLoading(false); + if (!signal.aborted) { + editorState.setIsFileLoading(false); + } } }; @@ -355,9 +396,9 @@ export function useStackActions(options: UseStackActionsOptions) { } }; - const saveFile = async () => { - if (editorState.activeTab === 'files') return; - if (!stackListState.selectedFile) return; + const saveFile = async (): Promise => { + if (editorState.activeTab === 'files') return false; + if (!stackListState.selectedFile) return false; const currentContent = editorState.activeTab === 'compose' ? editorState.content || '' @@ -381,9 +422,11 @@ export function useStackActions(options: UseStackActionsOptions) { } editorState.setIsEditing(false); toast.success('File saved successfully!'); + return true; } catch (error) { console.error('Failed to save file:', error); toast.error(`Failed to save file: ${(error as Error).message}`); + return false; } }; @@ -523,7 +566,8 @@ export function useStackActions(options: UseStackActionsOptions) { }; const handleSaveAndDeploy = async (e: React.MouseEvent) => { - await saveFile(); + const saved = await saveFile(); + if (!saved) return; await deployStack(e); }; @@ -768,6 +812,10 @@ export function useStackActions(options: UseStackActionsOptions) { editorState.setEnvContent(editorState.originalEnvContent); overlayState.setPendingUnsavedLoad(null); overlayState.setPendingUnsavedNode(null); + if (target === NODE_SWITCH_PENDING_TOKEN) { + if (targetNode) setActiveNode(targetNode); + return; + } if (target) { if (targetNode) void loadFileOnNode(targetNode, target); else void loadFile(target); @@ -890,6 +938,7 @@ export function useStackActions(options: UseStackActionsOptions) { return { pendingStackLoadRef, pendingLogsRef, + hasUnsavedChanges, getStackMenuVisibility, openStackApp, resetEditorState,