From 8ba88755b1e85aa60a8b08c03da4a0cfa2801d30 Mon Sep 17 00:00:00 2001 From: Anso Date: Sun, 24 May 2026 01:06:22 -0400 Subject: [PATCH] fix(stacks): default Empty template ships ports block commented out (#1189) The Empty branch of the Create Stack flow wrote a compose.yaml whose first service bound the host's port 8080 by default. On any workstation already running something on 8080 (traefik, caddy, librespeed, another nginx, etc.) the very first deploy failed at the docker compose networking step with "Bind for 0.0.0.0:8080 failed: port is already allocated", which made the day-one experience feel broken right after F-2 (PR #1168) tightened the dialog itself. The boilerplate in FileSystemService.createStack now emits the ports block commented out plus a one-line hint above it. A fresh deploy binds no host port, so the container starts cleanly on any host; the user uncomments the two-line block when they're ready to expose the container. The deterministic shape (no probe-and-write, no random port, no preflight scan) avoids the TOCTOU race that a free-port probe would have left between template creation and the actual compose up. Adds backend/src/__tests__/file-system-service-create-stack.test.ts (8 cases): directory + file creation, structural YAML assertions via yaml.parse to lock in the no-live-ports invariant, raw-text regex to lock in the commented hint, and the already-exists rejection path. FileSystemService.createStack had no coverage before this change. Adds e2e/default-stack-template-no-fixed-port.spec.ts (1 case): drives the dialog through Create, reads the resulting compose via the in-browser apiFetch, and asserts the live + commented invariants end-to-end. docs/features/stack-management.mdx Empty bullet rewritten to describe the minimal skeleton and the commented ports block instead of calling it "blank". Resolves: F-3 in the v1.0 audit tracker. --- .../file-system-service-create-stack.test.ts | 98 +++++++++++++++++++ backend/src/services/FileSystemService.ts | 5 +- docs/features/stack-management.mdx | 2 +- ...fault-stack-template-no-fixed-port.spec.ts | 62 ++++++++++++ 4 files changed, 164 insertions(+), 3 deletions(-) create mode 100644 backend/src/__tests__/file-system-service-create-stack.test.ts create mode 100644 e2e/default-stack-template-no-fixed-port.spec.ts diff --git a/backend/src/__tests__/file-system-service-create-stack.test.ts b/backend/src/__tests__/file-system-service-create-stack.test.ts new file mode 100644 index 00000000..da474896 --- /dev/null +++ b/backend/src/__tests__/file-system-service-create-stack.test.ts @@ -0,0 +1,98 @@ +/** + * Tests for FileSystemService.createStack(). + * + * Runs against a real tmpdir-backed compose root so the template is read + * from actual on-disk bytes (not a writeFile spy). Locks in the default + * Empty template shape: a minimal nginx skeleton with the host port + * binding commented out so a fresh deploy never collides with whatever + * already owns the host's port 8080. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import os from 'os'; +import { parse as parseYaml } from 'yaml'; + +const mockState = { composeDir: '' }; + +vi.mock('../services/NodeRegistry', () => ({ + NodeRegistry: { + getInstance: () => ({ + getComposeDir: () => mockState.composeDir, + getDefaultNodeId: () => 1, + }), + }, +})); + +import { FileSystemService } from '../services/FileSystemService'; + +describe('FileSystemService.createStack', () => { + let tmpBase: string; + + beforeEach(async () => { + tmpBase = await fs.mkdtemp(path.join(os.tmpdir(), 'sencho-create-stack-')); + mockState.composeDir = tmpBase; + }); + + afterEach(async () => { + await fs.rm(tmpBase, { recursive: true, force: true }); + }); + + async function createAndRead(name: string): Promise<{ text: string; parsed: unknown }> { + await FileSystemService.getInstance().createStack(name); + const text = await fs.readFile(path.join(tmpBase, name, 'compose.yaml'), 'utf-8'); + return { text, parsed: parseYaml(text) }; + } + + it('creates the stack directory under the compose root', async () => { + await FileSystemService.getInstance().createStack('alpha'); + const stat = await fs.stat(path.join(tmpBase, 'alpha')); + expect(stat.isDirectory()).toBe(true); + }); + + it('writes a compose.yaml file inside the stack directory', async () => { + await FileSystemService.getInstance().createStack('bravo'); + const stat = await fs.stat(path.join(tmpBase, 'bravo', 'compose.yaml')); + expect(stat.isFile()).toBe(true); + }); + + it('writes a YAML document that parses as a service map with image and restart', async () => { + const { parsed } = await createAndRead('charlie'); + expect(parsed).toMatchObject({ + services: { + app: { + image: 'nginx:latest', + restart: 'always', + }, + }, + }); + }); + + it('ships the ports block commented out (no live host port binding)', async () => { + const { parsed } = await createAndRead('delta'); + const services = (parsed as { services?: { app?: { ports?: unknown } } }).services; + expect(services?.app?.ports).toBeUndefined(); + }); + + it('preserves a commented ports hint so the user can uncomment without re-typing the structure', async () => { + const { text } = await createAndRead('echo'); + expect(text).toMatch(/^[ \t]*#[ \t]*ports:[ \t]*$/m); + expect(text).toMatch(/^[ \t]*#[ \t]*-[ \t]*"8080:80"[ \t]*$/m); + }); + + it('includes a one-line hint above the commented block explaining what to uncomment', async () => { + const { text } = await createAndRead('foxtrot'); + expect(text).toMatch(/^[ \t]*#[ \t]+Uncomment to expose a host port:[ \t]*$/m); + }); + + it('contains live (uncommented) configuration so the template is not entirely commented out', async () => { + const { text } = await createAndRead('golf'); + expect(text).toMatch(/^[ \t]*image:[ \t]+nginx:latest[ \t]*$/m); + expect(text).toMatch(/^[ \t]*restart:[ \t]+always[ \t]*$/m); + }); + + it('rejects creation when the stack directory already exists', async () => { + await FileSystemService.getInstance().createStack('hotel'); + await expect(FileSystemService.getInstance().createStack('hotel')).rejects.toThrow(/already exists/); + }); +}); diff --git a/backend/src/services/FileSystemService.ts b/backend/src/services/FileSystemService.ts index a9b5c7da..c2b16b50 100644 --- a/backend/src/services/FileSystemService.ts +++ b/backend/src/services/FileSystemService.ts @@ -234,9 +234,10 @@ export class FileSystemService { const boilerplate = `services: app: image: nginx:latest - ports: - - "8080:80" restart: always + # Uncomment to expose a host port: + # ports: + # - "8080:80" `; try { await fsPromises.writeFile(path.join(stackDir, 'compose.yaml'), boilerplate, 'utf-8'); diff --git a/docs/features/stack-management.mdx b/docs/features/stack-management.mdx index fcf007ab..433c9df0 100644 --- a/docs/features/stack-management.mdx +++ b/docs/features/stack-management.mdx @@ -13,7 +13,7 @@ A **stack** in Sencho is a Docker Compose project: a directory inside your `COMP Click **Create Stack** in the left sidebar to open the **New stack** dialog. Pick a source from the tabs at the top: -- **Empty**: start with a blank `compose.yaml` and write it from scratch. +- **Empty**: start with a minimal `compose.yaml` skeleton. The default service uses `nginx:latest` with the host port binding commented out, so a fresh deploy never collides with an existing service on the host. Uncomment and adjust the `ports:` block when you're ready to expose the container. - **From Git**: clone a Git repository so its compose file becomes the stack source. Future webhook pulls keep it in sync. See [Git Sources](/features/git-sources) for the full sync flow. - **From Docker Run**: paste a `docker run` command and let Sencho convert it to compose YAML. diff --git a/e2e/default-stack-template-no-fixed-port.spec.ts b/e2e/default-stack-template-no-fixed-port.spec.ts new file mode 100644 index 00000000..cb78990f --- /dev/null +++ b/e2e/default-stack-template-no-fixed-port.spec.ts @@ -0,0 +1,62 @@ +/** + * F-3 regression: creating an Empty stack via the dialog must produce a + * compose.yaml whose host port binding is commented out, so the first + * deploy never collides with whatever already owns the host's port 8080. + */ +import { test, expect } from '@playwright/test'; +import { loginAs, waitForStacksLoaded } from './helpers'; + +const TEST_STACK = `e2e-default-template-${Date.now()}`; + +async function deleteStackViaApi(page: import('@playwright/test').Page, name: string) { + await page.evaluate(async (stackName) => { + await fetch(`/api/stacks/${stackName}`, { method: 'DELETE', credentials: 'include' }).catch(() => { }); + }, name); +} + +async function readComposeViaApi(page: import('@playwright/test').Page, name: string): Promise { + return page.evaluate(async (stackName) => { + const res = await fetch( + `/api/stacks/${stackName}/files/content?path=compose.yaml`, + { credentials: 'include', cache: 'no-store' } + ); + if (!res.ok) throw new Error(`read compose failed: HTTP ${res.status}`); + const body = (await res.json()) as { content?: string }; + if (typeof body.content !== 'string') throw new Error('compose response missing content field'); + return body.content; + }, name); +} + +test.describe('Default Empty-stack template', () => { + test.beforeEach(async ({ page }) => { + await loginAs(page); + await waitForStacksLoaded(page); + await deleteStackViaApi(page, TEST_STACK); + }); + + test.afterEach(async ({ page }) => { + await deleteStackViaApi(page, TEST_STACK); + }); + + test('ships with the ports block commented out so first deploy never collides on 8080', async ({ 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 }); + await expect(page.getByText(`Stack "${TEST_STACK}" created.`)).toBeVisible({ timeout: 5_000 }); + + const compose = await readComposeViaApi(page, TEST_STACK); + + expect(compose, 'no uncommented ports: line').not.toMatch(/^(?![ \t]*#)[ \t]*ports:/m); + expect(compose, 'no uncommented host-port mapping').not.toMatch(/^(?![ \t]*#)[ \t]*-[ \t]+["']?8080:80["']?/m); + + expect(compose, 'commented ports: hint present').toMatch(/^[ \t]*#[ \t]*ports:[ \t]*$/m); + expect(compose, 'commented port mapping present').toMatch(/^[ \t]*#[ \t]*-[ \t]*"8080:80"[ \t]*$/m); + expect(compose, 'commented hint sentence above the block').toMatch(/^[ \t]*#[ \t]+Uncomment to expose a host port:[ \t]*$/m); + + expect(compose, 'live image line preserved').toMatch(/^[ \t]*image:[ \t]+nginx:latest[ \t]*$/m); + expect(compose, 'live restart line preserved').toMatch(/^[ \t]*restart:[ \t]+always[ \t]*$/m); + }); +});