diff --git a/.gitignore b/.gitignore index 611a9907..74f888d6 100644 --- a/.gitignore +++ b/.gitignore @@ -23,3 +23,7 @@ infra/step-ca/dev-password # traversal error: ... .timestamp-*.mjs: No such file or directory" when the # file vanished mid-scan. Ignoring them removes the race. *.timestamp-*.mjs + +# Playwright run artifacts (#1445, P6 E2E gate) +apps/web/test-results/ +apps/web/playwright-report/ diff --git a/.woodpecker/ci.yml b/.woodpecker/ci.yml index 34fb9235..4eeafacf 100644 --- a/.woodpecker/ci.yml +++ b/.woodpecker/ci.yml @@ -254,6 +254,23 @@ steps: depends_on: - typecheck + # Canonical verify:release stage `build` (#1445, P6): every PR proves the + # full workspace build — including the SPA `vite build` — before merge, + # instead of leaving build breakage to surface post-merge in publish.yml's + # verify step. Same canonical command the publish pipeline's build step runs. + build: + image: *node_image + commands: + - *enable_pnpm + - pnpm build + depends_on: + # after test, not typecheck: turbo gives `test` a ^build dependency, so + # running this step concurrently with test would put two independent + # turbo builds on the same shared-workspace dist/ and turbo cache with + # no cross-process locking — the same serialization invariant + # publish.yml documents for #1411. + - test + services: ci-postgres: image: pgvector/pgvector:pg17 diff --git a/.woodpecker/publish.yml b/.woodpecker/publish.yml index 0b1afdc4..b656c5db 100644 --- a/.woodpecker/publish.yml +++ b/.woodpecker/publish.yml @@ -407,6 +407,96 @@ steps: - build - verify + # #1445 (P6): headless Playwright E2E gate on every trunk merge. Boots the + # real gateway on the embedded PGlite path (no DATABASE_URL, no services) + # serving the built SPA bundle via WEB_DIST_DIR — the exact serving path the + # gateway image ships (docker/gateway.Dockerfile sets WEB_DIST_DIR to the + # baked bundle), which keeps #1407's parity guarantee: the image build steps + # below depend on this gate, so a bundle that fails E2E never publishes. + # + # Image pinned to the @playwright/test version in pnpm-lock.yaml so the + # image's bundled browsers match the workspace driver exactly (bump the two + # together). The step installs no workspace packages (corepack does fetch + # the pinned pnpm itself): it reuses the workspace node_modules + # from `install` and the dist outputs from `build` — the gateway's runtime + # dependency path is pure JS/WASM (PGlite is WASM, postgres-js is pure JS), + # so the alpine-installed modules run unchanged under this glibc image. + # depends_on publish-next-npm per the #1411 serialization invariant: this + # step reads the workspace and must never run inside the manifest-transform + # window. + e2e: + image: mcr.microsoft.com/playwright:v1.58.2-noble + environment: + GATEWAY_PORT: '14242' + PLAYWRIGHT_BASE_URL: http://localhost:14242 + # The database is seeded by Playwright's globalSetup in this step, so + # login failures are real failures: without this flag the suite's + # skip-when-login-fails guards (a live-environment affordance) could + # skip every authenticated spec and go green while proving nothing. + E2E_REQUIRE_SEEDED_AUTH: '1' + commands: + - corepack enable + - | + # Throwaway signing secret for this step's ephemeral embedded database + # (the gateway refuses to boot without one). Generated per run so no + # usable literal lives in the tree. + export BETTER_AUTH_SECRET="$(head -c 32 /dev/urandom | base64)" + export WEB_DIST_DIR="$(pwd)/apps/web/dist" + if [ ! -f "$WEB_DIST_DIR/index.html" ]; then + echo "[e2e] FATAL: $WEB_DIST_DIR/index.html missing — did the build step run?" >&2 + exit 1 + fi + # Boot the gateway from the built dist, cwd- AND HOME-isolated: the + # local-tier PGlite database lives under $HOME/.config/mosaic/gateway/ + # (database.module.ts), not under cwd, so HOME must point at the + # throwaway dir too or the run would share a database with anything + # else in the container's home. + GATEWAY_RUN_DIR="$(mktemp -d /tmp/e2e-gateway.XXXXXX)" + (cd "$GATEWAY_RUN_DIR" && export HOME="$GATEWAY_RUN_DIR" && exec node "$OLDPWD/apps/gateway/dist/main.js") > /tmp/gateway.log 2>&1 & + GATEWAY_PID=$! + ready=0 + for i in $(seq 1 90); do + if node -e "fetch('http://localhost:' + process.env.GATEWAY_PORT + '/health', { signal: AbortSignal.timeout(2000) }).then((r) => process.exit(r.ok ? 0 : 1), () => process.exit(1))"; then + ready=1 + break + fi + if ! kill -0 "$GATEWAY_PID" 2>/dev/null; then + echo "[e2e] FATAL: gateway process exited during startup" >&2 + cat /tmp/gateway.log >&2 + exit 1 + fi + echo "[e2e] waiting for gateway ($i/90)..." + sleep 1 + done + if [ "$ready" -ne 1 ]; then + echo "[e2e] FATAL: gateway did not become ready in 90s" >&2 + cat /tmp/gateway.log >&2 + exit 1 + fi + echo "[e2e] gateway ready; running Playwright suite" + set +e + pnpm --filter @mosaicstack/web exec playwright test + E2E_EXIT=$? + set -e + kill "$GATEWAY_PID" 2>/dev/null || true + if [ "$E2E_EXIT" -ne 0 ]; then + echo "[e2e] FATAL: Playwright suite failed (exit $E2E_EXIT); gateway log follows" >&2 + tail -100 /tmp/gateway.log >&2 + echo "[e2e] browser-side traces/screenshots are under apps/web/test-results/ in the step workspace (not persisted past the pod)" >&2 + fi + exit "$E2E_EXIT" + # Same filter as the image builds it gates: a merge that publishes no + # image (docs-only on main) pays no browser suite, and a skipped e2e does + # not block anything (skipped-dependency semantics, same as + # publish-next-npm on tag events). + when: *image_build_when + depends_on: + - build + - verify + # #1411: never read the workspace inside publish-next-npm's + # manifest-transform window. + - publish-next-npm + # TODO: Uncomment when ready to publish to npmjs.org # publish-npmjs: # image: *node_image @@ -466,6 +556,8 @@ steps: # ERR_PNPM_OUTDATED_LOCKFILE despite a clean restore. This edge is the # serialization invariant; add it to every new workspace consumer. - publish-next-npm + # #1445 (P6): a bundle that fails the E2E gate never publishes an image. + - e2e build-appservice: image: gcr.io/kaniko-project/executor:debug @@ -510,3 +602,5 @@ steps: # ERR_PNPM_OUTDATED_LOCKFILE despite a clean restore. This edge is the # serialization invariant; add it to every new workspace consumer. - publish-next-npm + # #1445 (P6): a bundle that fails the E2E gate never publishes an image. + - e2e diff --git a/apps/gateway/src/spa/serve-spa.e2e.spec.ts b/apps/gateway/src/spa/serve-spa.e2e.spec.ts new file mode 100644 index 00000000..486182da --- /dev/null +++ b/apps/gateway/src/spa/serve-spa.e2e.spec.ts @@ -0,0 +1,192 @@ +/** + * E2E integration test — SPA static serving (Phase P5 cutover, #1444; tests + * added in P6, #1445, review follow-up SF1 on PR #1453). + * + * Boots a real Nest+Fastify app the way main.ts does (mountSpaStatic after the + * controllers) against a fixture dist directory, and pins the serving + * contract: + * + * 1. `/` and client-side deep links fall back to index.html. + * 2. Declared API routes win over the catch-all. + * 3. Unknown backend paths (/api, /mcp, /socket.io) are JSON 404s, never the + * SPA page — including with a query string (`/api?x=1`). + * 4. Static files are served exactly; hashed /assets/ files get immutable + * cache headers, everything else revalidates (max-age=0), and a missing + * /assets/ file is a 404 — never the SPA fallback. + * 5. WEB_DIST_DIR unset disables SPA serving entirely. + * 6. WEB_DIST_DIR pointing at a directory without index.html fails at boot. + */ + +import 'reflect-metadata'; +import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { describe, it, expect, afterAll, beforeAll } from 'vitest'; +import { Test } from '@nestjs/testing'; +import { Controller, Get, type INestApplication } from '@nestjs/common'; +import { FastifyAdapter, type NestFastifyApplication } from '@nestjs/platform-fastify'; +import request from 'supertest'; +import { mountSpaStatic } from './serve-spa.js'; + +const INDEX_HTML = 'mosaic spa fixture\n'; +const ASSET_JS = 'console.log("hashed asset");\n'; + +@Controller('api/spa-test') +class SpaTestController { + @Get('ping') + ping(): { ok: boolean } { + return { ok: true }; + } +} + +async function createApp(): Promise { + const moduleRef = await Test.createTestingModule({ + controllers: [SpaTestController], + }).compile(); + + const app = moduleRef.createNestApplication(new FastifyAdapter()); + await app.init(); + // Mirror main.ts ordering: SPA mounting happens after the app (and its + // controllers) exist, before listen. + await mountSpaStatic(app as NestFastifyApplication); + await (app as NestFastifyApplication).getHttpAdapter().getInstance().ready(); + return app; +} + +describe('SPA static serving — fixture dist dir', () => { + let app: INestApplication; + let distDir: string; + let previousWebDistDir: string | undefined; + + beforeAll(async () => { + distDir = await mkdtemp(path.join(tmpdir(), 'serve-spa-fixture-')); + await writeFile(path.join(distDir, 'index.html'), INDEX_HTML); + await writeFile(path.join(distDir, 'favicon.svg'), '\n'); + await mkdir(path.join(distDir, 'assets'), { recursive: true }); + await writeFile(path.join(distDir, 'assets', 'app-abc123.js'), ASSET_JS); + + previousWebDistDir = process.env['WEB_DIST_DIR']; + process.env['WEB_DIST_DIR'] = distDir; + app = await createApp(); + }); + + afterAll(async () => { + if (previousWebDistDir === undefined) { + delete process.env['WEB_DIST_DIR']; + } else { + process.env['WEB_DIST_DIR'] = previousWebDistDir; + } + await app.close(); + await rm(distDir, { recursive: true, force: true }); + }); + + it('serves index.html at /', async () => { + const res = await request(app.getHttpServer()).get('/'); + expect(res.status).toBe(200); + expect(res.text).toBe(INDEX_HTML); + expect(res.headers['content-type']).toContain('text/html'); + }); + + it('falls back to index.html for client-side deep links', async () => { + for (const deepLink of ['/chat', '/projects/42', '/settings']) { + const res = await request(app.getHttpServer()).get(deepLink); + expect(res.status, deepLink).toBe(200); + expect(res.text, deepLink).toBe(INDEX_HTML); + } + }); + + it('declared API routes win over the SPA catch-all', async () => { + const res = await request(app.getHttpServer()).get('/api/spa-test/ping'); + expect(res.status).toBe(200); + expect(res.body).toEqual({ ok: true }); + }); + + it('unknown backend paths are JSON 404s, never the SPA page', async () => { + for (const backendPath of ['/api/nope', '/api', '/mcp/nope', '/socket.io/nope']) { + const res = await request(app.getHttpServer()).get(backendPath); + expect(res.status, backendPath).toBe(404); + expect(res.headers['content-type'], backendPath).toContain('application/json'); + expect(res.body, backendPath).toMatchObject({ error: 'Not Found', statusCode: 404 }); + } + }); + + it('a backend path with a query string is still a backend 404 (/api?x=1)', async () => { + const res = await request(app.getHttpServer()).get('/api?x=1'); + expect(res.status).toBe(404); + expect(res.headers['content-type']).toContain('application/json'); + }); + + it('serves static files exactly', async () => { + const res = await request(app.getHttpServer()).get('/favicon.svg'); + expect(res.status).toBe(200); + // supertest buffers image/svg+xml as a Buffer body, not res.text. + const body = res.text || (res.body as Buffer).toString('utf8'); + expect(body).toBe('\n'); + }); + + it('hashed /assets/ files get immutable cache headers', async () => { + const res = await request(app.getHttpServer()).get('/assets/app-abc123.js'); + expect(res.status).toBe(200); + expect(res.text).toBe(ASSET_JS); + expect(res.headers['cache-control']).toBe('public, max-age=31536000, immutable'); + }); + + it('missing /assets/ files are 404s, never the SPA page with an immutable header', async () => { + // The exact request a browser with a stale index.html makes after a + // deploy: the old hashed filename. Serving index.html here would poison + // caches with a year-long immutable entry whose body is HTML. + for (const missingAsset of ['/assets/app-old999.js', '/assets/app-old999.js?v=1']) { + const res = await request(app.getHttpServer()).get(missingAsset); + expect(res.status, missingAsset).toBe(404); + expect(res.text, missingAsset).not.toContain('mosaic spa fixture'); + // The 404 carries no cache-control at all; ?? '' keeps the assertion valid. + expect(res.headers['cache-control'] ?? '', missingAsset).not.toContain('immutable'); + } + }); + + it('index.html and non-asset files revalidate (no immutable caching)', async () => { + for (const revalidating of ['/', '/chat', '/favicon.svg']) { + const res = await request(app.getHttpServer()).get(revalidating); + expect(res.headers['cache-control'], revalidating).not.toContain('immutable'); + } + }); + + it('non-GET unmatched requests keep the stock 404 (catch-all is GET/HEAD only)', async () => { + const res = await request(app.getHttpServer()).post('/chat'); + expect(res.status).toBe(404); + expect(res.text).not.toContain('mosaic spa fixture'); + }); +}); + +describe('SPA static serving — configuration edges', () => { + it('WEB_DIST_DIR unset disables SPA serving', async () => { + const previous = process.env['WEB_DIST_DIR']; + delete process.env['WEB_DIST_DIR']; + try { + const app = await createApp(); + const res = await request(app.getHttpServer()).get('/chat'); + expect(res.status).toBe(404); + await app.close(); + } finally { + if (previous !== undefined) { + process.env['WEB_DIST_DIR'] = previous; + } + } + }); + + it('WEB_DIST_DIR without index.html fails at boot', async () => { + const emptyDir = await mkdtemp(path.join(tmpdir(), 'serve-spa-empty-')); + const previous = process.env['WEB_DIST_DIR']; + process.env['WEB_DIST_DIR'] = emptyDir; + try { + await expect(createApp()).rejects.toThrow(/index\.html.*does not exist/); + } finally { + if (previous === undefined) { + delete process.env['WEB_DIST_DIR']; + } else { + process.env['WEB_DIST_DIR'] = previous; + } + await rm(emptyDir, { recursive: true, force: true }); + } + }); +}); diff --git a/apps/gateway/src/spa/serve-spa.ts b/apps/gateway/src/spa/serve-spa.ts index bda3617f..dc3f2cfd 100644 --- a/apps/gateway/src/spa/serve-spa.ts +++ b/apps/gateway/src/spa/serve-spa.ts @@ -8,7 +8,12 @@ import type { NestFastifyApplication } from '@nestjs/platform-fastify'; const BACKEND_PREFIXES = ['/api', '/mcp', '/socket.io'] as const; function isBackendPath(url: string): boolean { - return BACKEND_PREFIXES.some((prefix) => url === prefix || url.startsWith(`${prefix}/`)); + // Match on the path only: `/api?x=1` is a backend request, and the query + // string must never turn it into an SPA fallback. + const pathOnly = url.split('?', 1)[0] ?? url; + return BACKEND_PREFIXES.some( + (prefix) => pathOnly === prefix || pathOnly.startsWith(`${prefix}/`), + ); } /** @@ -39,8 +44,7 @@ export async function mountSpaStatic(app: NestFastifyApplication): Promise // Default cache semantics: public, max-age=0 with ETag/Last-Modified, so // every response revalidates (304 when unchanged). Always correct, including - // for index.html after a deploy; immutable caching for hashed /assets/ files - // is a P6 optimization. + // for index.html after a deploy. await app.register( fastifyStatic as never, { @@ -50,13 +54,28 @@ export async function mountSpaStatic(app: NestFastifyApplication): Promise } as never, ); + const fastify = app.getHttpAdapter().getInstance(); + + // Files under /assets/ carry a content hash in their name (Vite emits them + // that way), so they get long-lived immutable caching: a changed file is a + // new URL, never a stale cache hit. An onSend hook rather than the plugin's + // `setHeaders` option, because @fastify/static applies its own computed + // cache-control (reply.headers) after calling setHeaders, overriding it. + fastify.addHook('onSend', (req, reply, payload, done) => { + const pathOnly = (req.raw.url ?? '').split('?', 1)[0] ?? ''; + if (reply.statusCode === 200 && pathOnly.startsWith('/assets/')) { + void reply.header('cache-control', 'public, max-age=31536000, immutable'); + } + done(null, payload); + }); + // A wildcard route, not setNotFoundHandler: Nest installs its own not-found // handler during init and Fastify allows only one. find-my-way matches // most-specific-first, so every declared route (API, static files) wins over // this catch-all; non-GET unmatched requests keep Fastify's stock 404. - const fastify = app.getHttpAdapter().getInstance(); fastify.get('/*', (req, reply) => { const url = req.raw.url ?? ''; + const pathOnly = url.split('?', 1)[0] ?? url; if (isBackendPath(url)) { // An unknown backend path is an API 404, never the SPA page. void reply.code(404).send({ @@ -66,6 +85,18 @@ export async function mountSpaStatic(app: NestFastifyApplication): Promise }); return; } + if (pathOnly === '/assets' || pathOnly.startsWith('/assets/')) { + // A missing hashed asset — typically a browser holding a stale + // index.html after a deploy — must 404. Falling through to the SPA + // fallback would return index.html as the asset body, and the onSend + // hook above would stamp it with a year-long immutable cache-control. + void reply.code(404).send({ + message: `Asset ${pathOnly} not found`, + error: 'Not Found', + statusCode: 404, + }); + return; + } // sendFile is decorated by @fastify/static; its type augmentation targets // a different fastify copy in the pnpm tree than the Nest adapter's. (reply as unknown as { sendFile: (file: string) => unknown }).sendFile('index.html'); diff --git a/apps/web/e2e/admin.spec.ts b/apps/web/e2e/admin.spec.ts index 9b0d8c16..b0dbafe8 100644 --- a/apps/web/e2e/admin.spec.ts +++ b/apps/web/e2e/admin.spec.ts @@ -1,11 +1,14 @@ import { test, expect } from '@playwright/test'; -import { loginAs, ADMIN_USER, TEST_USER } from './helpers/auth.js'; +import { loginAs, ADMIN_USER, REQUIRE_SEEDED_AUTH, TEST_USER } from './helpers/auth.js'; test.describe('Admin page — admin user', () => { test.beforeEach(async ({ page }) => { await loginAs(page, ADMIN_USER.email, ADMIN_USER.password); const url = page.url(); - test.skip(!url.includes('/chat'), 'No seeded admin user — skipping admin tests'); + test.skip( + !REQUIRE_SEEDED_AUTH && !url.includes('/chat'), + 'No seeded admin user — skipping admin tests', + ); }); test('admin page loads with the Admin Panel heading', async ({ page }) => { @@ -31,15 +34,11 @@ test.describe('Admin page — admin user', () => { await page.goto('/admin'); await page.getByRole('button', { name: /system health/i }).click(); // Health cards or loading indicator should appear - const hasLoading = await page + const loadingOrCard = page .getByText(/loading health/i) - .isVisible() - .catch(() => false); - const hasCard = await page - .getByText(/database/i) - .isVisible() - .catch(() => false); - expect(hasLoading || hasCard).toBe(true); + .or(page.getByText(/database/i)) + .first(); + await expect(loadingOrCard).toBeVisible({ timeout: 10_000 }); }); }); @@ -47,26 +46,19 @@ test.describe('Admin page — non-admin user', () => { test.beforeEach(async ({ page }) => { await loginAs(page, TEST_USER.email, TEST_USER.password); const url = page.url(); - test.skip(!url.includes('/chat'), 'No seeded test user — skipping non-admin tests'); + test.skip( + !REQUIRE_SEEDED_AUTH && !url.includes('/chat'), + 'No seeded test user — skipping non-admin tests', + ); }); - test('non-admin visiting /admin sees access denied or is redirected', async ({ page }) => { + test('non-admin visiting /admin never sees the admin panel', async ({ page }) => { await page.goto('/admin'); - // Either redirected away or shown an access-denied message - const onAdmin = page.url().includes('/admin'); - if (onAdmin) { - // Should show some access-denied content rather than the full admin panel - const hasPanel = await page - .getByRole('heading', { name: /admin panel/i }) - .isVisible() - .catch(() => false); - // If heading is visible, the guard allowed access (user may have admin role in this env) - // — not a failure, just informational - if (!hasPanel) { - // access denied message, redirect, or guard placeholder - const url = page.url(); - expect(url).toBeTruthy(); // environment-dependent — no hard assertion - } - } + // Wait for the app shell to render (redirect and access-denied views both + // keep the sidebar), then assert the panel itself is absent. globalSetup + // seeds TEST_USER with role 'member', so this is a real authorization + // assertion, not environment-dependent. + await expect(page.getByRole('img', { name: /mosaic logo/i })).toBeVisible({ timeout: 10_000 }); + await expect(page.getByRole('heading', { name: /admin panel/i })).not.toBeVisible(); }); }); diff --git a/apps/web/e2e/auth.spec.ts b/apps/web/e2e/auth.spec.ts index 93915a38..3fd19e5b 100644 --- a/apps/web/e2e/auth.spec.ts +++ b/apps/web/e2e/auth.spec.ts @@ -1,5 +1,5 @@ import { test, expect } from '@playwright/test'; -import { TEST_USER } from './helpers/auth.js'; +import { REQUIRE_SEEDED_AUTH, TEST_USER } from './helpers/auth.js'; // ── Login page ──────────────────────────────────────────────────────────────── @@ -49,18 +49,14 @@ test.describe('Login page', () => { }); test('redirects to /chat after successful login', async ({ page }) => { + // Only meaningful with known-good credentials; against a live environment + // this would just probe someone else's user table. + test.skip(!REQUIRE_SEEDED_AUTH, 'needs seeded credentials (E2E_REQUIRE_SEEDED_AUTH=1)'); await page.goto('/login'); await page.getByLabel('Email').fill(TEST_USER.email); await page.getByLabel('Password').fill(TEST_USER.password); await page.getByRole('button', { name: /sign in/i }).click(); - // Either reaches /chat or shows an error (if credentials are wrong in this env). - // We assert a navigation away from /login, or the alert is shown. - await Promise.race([ - expect(page).toHaveURL(/\/chat/, { timeout: 10_000 }), - expect(page.getByRole('alert')).toBeVisible({ timeout: 10_000 }), - ]).catch(() => { - // Acceptable — environment may not have seeded credentials - }); + await expect(page).toHaveURL(/\/chat/, { timeout: 10_000 }); }); }); diff --git a/apps/web/e2e/chat.spec.ts b/apps/web/e2e/chat.spec.ts index d908d73d..6b6f99ab 100644 --- a/apps/web/e2e/chat.spec.ts +++ b/apps/web/e2e/chat.spec.ts @@ -1,45 +1,38 @@ import { test, expect } from '@playwright/test'; -import { loginAs, TEST_USER } from './helpers/auth.js'; +import { loginAs, REQUIRE_SEEDED_AUTH, TEST_USER } from './helpers/auth.js'; test.describe('Chat page', () => { test.beforeEach(async ({ page }) => { await loginAs(page, TEST_USER.email, TEST_USER.password); // If login failed (no seeded user in env) we may be on /login — skip const url = page.url(); - test.skip(!url.includes('/chat'), 'No seeded test user — skipping authenticated tests'); + test.skip( + !REQUIRE_SEEDED_AUTH && !url.includes('/chat'), + 'No seeded test user — skipping authenticated tests', + ); }); - test('chat page loads and shows the welcome message or conversation list', async ({ page }) => { + test('chat page loads and shows the conversation area', async ({ page }) => { await page.goto('/chat'); - // Either there are conversations listed or the welcome empty-state is shown - const hasWelcome = await page - .getByRole('heading', { name: /welcome to mosaic chat/i }) - .isVisible() - .catch(() => false); - const hasConversationPanel = await page - .locator('[data-testid="conversation-list"], nav, aside') - .first() - .isVisible() - .catch(() => false); - - expect(hasWelcome || hasConversationPanel).toBe(true); + await expect(page.getByRole('heading', { level: 1, name: /chat/i })).toBeVisible({ + timeout: 10_000, + }); + await expect(page.getByRole('log', { name: /conversation/i })).toBeVisible(); }); - test('new conversation button is visible', async ({ page }) => { + test('message composer input is visible', async ({ page }) => { await page.goto('/chat'); - // "Start new conversation" button or a "+" button in the sidebar - const newConvButton = page.getByRole('button', { name: /new conversation|start new/i }).first(); - await expect(newConvButton).toBeVisible({ timeout: 10_000 }); + await expect(page.getByLabel('Message')).toBeVisible({ timeout: 10_000 }); }); - test('clicking new conversation shows a chat input area', async ({ page }) => { + test('command panel lists /new and exposes the run controls', async ({ page }) => { await page.goto('/chat'); - // Find any button that creates a new conversation - const newBtn = page.getByRole('button', { name: /new conversation|start new/i }).first(); - await newBtn.click(); - // After creating, a text input for sending messages should appear - const chatInput = page.getByRole('textbox').or(page.locator('textarea')).first(); - await expect(chatInput).toBeVisible({ timeout: 10_000 }); + // Conversations are command-driven: /new starts one via the commands panel. + const commandList = page.getByRole('list', { name: /available commands/i }); + await expect(commandList).toBeVisible({ timeout: 10_000 }); + await expect(commandList.getByText('/new', { exact: true })).toBeVisible(); + await expect(page.getByLabel('Command name')).toBeVisible(); + await expect(page.getByRole('button', { name: /run command/i })).toBeVisible(); }); test('sidebar navigation is present on chat page', async ({ page }) => { diff --git a/apps/web/e2e/global-setup.ts b/apps/web/e2e/global-setup.ts new file mode 100644 index 00000000..02a4ee12 --- /dev/null +++ b/apps/web/e2e/global-setup.ts @@ -0,0 +1,95 @@ +import type { FullConfig } from '@playwright/test'; +import { ADMIN_USER, REQUIRE_SEEDED_AUTH, TEST_USER } from './helpers/auth.js'; + +/** + * Seed the E2E users through the gateway's real APIs (#1445, P6). + * + * On a fresh database (CI boots the gateway on the embedded PGlite path): + * 1. POST /api/bootstrap/setup creates ADMIN_USER as the first admin. + * 2. The admin signs in and creates TEST_USER via the better-auth admin API. + * + * Against an environment that already has users (needsSetup=false), seeding is + * skipped entirely: the specs keep their own skip-when-login-fails guards, so + * a live environment stays usable as a test target without mutation. Under + * E2E_REQUIRE_SEEDED_AUTH=1 (CI) that state is instead a hard failure and the + * guards are disabled — see helpers/auth.ts. + * + * On a fresh database, any seeding failure throws and fails the whole run: an + * E2E gate whose authenticated suites silently skip would pass while proving + * nothing. + */ +export default async function globalSetup(config: FullConfig): Promise { + const baseURL = config.projects[0]?.use?.baseURL ?? 'http://localhost:14242'; + + const statusRes = await fetch(`${baseURL}/api/bootstrap/status`); + if (!statusRes.ok) { + throw new Error(`GET /api/bootstrap/status returned ${statusRes.status} — is the gateway up?`); + } + const status = (await statusRes.json()) as { needsSetup: boolean }; + if (!status.needsSetup) { + if (REQUIRE_SEEDED_AUTH) { + // CI boots the gateway on a fresh HOME-isolated database, so an + // already-populated one means the isolation regressed — refuse to run + // against unknown data rather than skip-and-pass. + throw new Error( + 'E2E_REQUIRE_SEEDED_AUTH=1 but the database already has users — gateway HOME isolation regressed?', + ); + } + console.info('[e2e setup] users already exist; skipping seed'); + return; + } + + const setupRes = await fetch(`${baseURL}/api/bootstrap/setup`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ + name: ADMIN_USER.name, + email: ADMIN_USER.email, + password: ADMIN_USER.password, + }), + }); + if (!setupRes.ok) { + throw new Error( + `POST /api/bootstrap/setup failed (${setupRes.status}): ${await setupRes.text()}`, + ); + } + console.info(`[e2e setup] bootstrap admin created: ${ADMIN_USER.email}`); + + // better-auth's CSRF protection rejects requests without an Origin header + // (403 MISSING_OR_NULL_ORIGIN), so the server-side fetches here send the + // gateway's own origin — the same value a browser tab on the SPA would send. + const authHeaders = { 'content-type': 'application/json', origin: baseURL }; + + const signInRes = await fetch(`${baseURL}/api/auth/sign-in/email`, { + method: 'POST', + headers: authHeaders, + body: JSON.stringify({ email: ADMIN_USER.email, password: ADMIN_USER.password }), + }); + if (!signInRes.ok) { + throw new Error(`admin sign-in failed (${signInRes.status}): ${await signInRes.text()}`); + } + const cookies = signInRes.headers + .getSetCookie() + .map((cookie) => cookie.split(';', 1)[0]) + .join('; '); + if (!cookies) { + throw new Error('admin sign-in returned no session cookie'); + } + + const createRes = await fetch(`${baseURL}/api/auth/admin/create-user`, { + method: 'POST', + headers: { ...authHeaders, cookie: cookies }, + body: JSON.stringify({ + name: TEST_USER.name, + email: TEST_USER.email, + password: TEST_USER.password, + role: 'member', + }), + }); + if (!createRes.ok) { + throw new Error( + `POST /api/auth/admin/create-user failed (${createRes.status}): ${await createRes.text()}`, + ); + } + console.info(`[e2e setup] test user created: ${TEST_USER.email}`); +} diff --git a/apps/web/e2e/helpers/auth.ts b/apps/web/e2e/helpers/auth.ts index b1b1428f..98113b08 100644 --- a/apps/web/e2e/helpers/auth.ts +++ b/apps/web/e2e/helpers/auth.ts @@ -13,11 +13,28 @@ export const ADMIN_USER = { }; /** - * Fill the login form and submit. Waits for navigation after success. + * Set when the database was seeded by global-setup (CI sets it in the + * publish.yml e2e step). Seeded credentials MUST work, so login failures are + * hard failures and the skip-when-login-fails guards are disabled — otherwise + * a login regression would skip every authenticated suite and the gate would + * pass while proving nothing. Unset (a live environment used as a test + * target), the guards stay on and unseeded credentials skip their suites. + */ +export const REQUIRE_SEEDED_AUTH = process.env['E2E_REQUIRE_SEEDED_AUTH'] === '1'; + +/** + * Fill the login form and submit, then wait for the post-login redirect to + * /chat. Under REQUIRE_SEEDED_AUTH a missed redirect throws (failing the + * test). Otherwise the timeout is swallowed: the page stays on /login and the + * callers' `test.skip(...)` guards see that. Without this wait, every guard + * read page.url() before the redirect happened and skipped its suite even + * when login succeeded (#1445). */ export async function loginAs(page: Page, email: string, password: string): Promise { await page.goto('/login'); await page.getByLabel('Email').fill(email); await page.getByLabel('Password').fill(password); await page.getByRole('button', { name: /sign in/i }).click(); + const redirect = page.waitForURL(/\/chat/, { timeout: 10_000 }); + await (REQUIRE_SEEDED_AUTH ? redirect : redirect.catch(() => {})); } diff --git a/apps/web/e2e/navigation.spec.ts b/apps/web/e2e/navigation.spec.ts index 58cbc819..60122f3a 100644 --- a/apps/web/e2e/navigation.spec.ts +++ b/apps/web/e2e/navigation.spec.ts @@ -1,16 +1,22 @@ import { test, expect } from '@playwright/test'; -import { loginAs, TEST_USER } from './helpers/auth.js'; +import { loginAs, REQUIRE_SEEDED_AUTH, TEST_USER } from './helpers/auth.js'; test.describe('Sidebar navigation', () => { test.beforeEach(async ({ page }) => { await loginAs(page, TEST_USER.email, TEST_USER.password); const url = page.url(); - test.skip(!url.includes('/chat'), 'No seeded test user — skipping authenticated tests'); + test.skip( + !REQUIRE_SEEDED_AUTH && !url.includes('/chat'), + 'No seeded test user — skipping authenticated tests', + ); }); - test('sidebar shows Mosaic brand link', async ({ page }) => { + test('sidebar shows the Mosaic brand', async ({ page }) => { await page.goto('/chat'); - await expect(page.getByRole('link', { name: /mosaic/i }).first()).toBeVisible(); + // The brand block is a logo image plus "Mosaic / Mission Control" text, + // not a link. + await expect(page.getByRole('img', { name: /mosaic logo/i })).toBeVisible(); + await expect(page.getByText('Mission Control')).toBeVisible(); }); test('Chat nav link navigates to /chat', async ({ page }) => { @@ -48,11 +54,12 @@ test.describe('Sidebar navigation', () => { test('active link is visually highlighted', async ({ page }) => { await page.goto('/chat'); - // The active link should have a distinct class — check that the Chat link - // has the active style class (bg-blue-600/20 text-blue-400) + // The sidebar marks the active item with `font-medium` (plus an inline + // primary-color style); inactive items get the hover class instead. const chatLink = page.getByRole('link', { name: /^chat$/i }).first(); - const cls = await chatLink.getAttribute('class'); - expect(cls).toContain('blue'); + const projectsLink = page.getByRole('link', { name: /^projects$/i }).first(); + await expect(chatLink).toHaveClass(/font-medium/); + await expect(projectsLink).not.toHaveClass(/font-medium/); }); }); @@ -60,18 +67,23 @@ test.describe('Route transitions', () => { test.beforeEach(async ({ page }) => { await loginAs(page, TEST_USER.email, TEST_USER.password); const url = page.url(); - test.skip(!url.includes('/chat'), 'No seeded test user — skipping authenticated tests'); + test.skip( + !REQUIRE_SEEDED_AUTH && !url.includes('/chat'), + 'No seeded test user — skipping authenticated tests', + ); }); test('navigating chat → projects → settings → chat works without errors', async ({ page }) => { await page.goto('/chat'); await expect(page).toHaveURL(/\/chat/); + // level: 1 — empty-state h2s ("No projects yet") also match the loose + // patterns, and a two-element match is a strict-mode violation. await page.goto('/projects'); - await expect(page.getByRole('heading', { name: /projects/i })).toBeVisible(); + await expect(page.getByRole('heading', { level: 1, name: /projects/i })).toBeVisible(); await page.goto('/settings'); - await expect(page.getByRole('heading', { name: /settings/i })).toBeVisible(); + await expect(page.getByRole('heading', { level: 1, name: /settings/i })).toBeVisible(); await page.goto('/chat'); await expect(page).toHaveURL(/\/chat/); diff --git a/apps/web/e2e/projects.spec.ts b/apps/web/e2e/projects.spec.ts index 6059d770..b7deacac 100644 --- a/apps/web/e2e/projects.spec.ts +++ b/apps/web/e2e/projects.spec.ts @@ -1,16 +1,23 @@ import { test, expect } from '@playwright/test'; -import { loginAs, TEST_USER } from './helpers/auth.js'; +import { loginAs, REQUIRE_SEEDED_AUTH, TEST_USER } from './helpers/auth.js'; test.describe('Projects page', () => { test.beforeEach(async ({ page }) => { await loginAs(page, TEST_USER.email, TEST_USER.password); const url = page.url(); - test.skip(!url.includes('/chat'), 'No seeded test user — skipping authenticated tests'); + test.skip( + !REQUIRE_SEEDED_AUTH && !url.includes('/chat'), + 'No seeded test user — skipping authenticated tests', + ); }); test('projects page loads with heading', async ({ page }) => { await page.goto('/projects'); - await expect(page.getByRole('heading', { name: /projects/i })).toBeVisible({ timeout: 10_000 }); + // level: 1 — the "No projects yet" empty-state h2 also matches /projects/i + // and a two-element match is a strict-mode violation. + await expect(page.getByRole('heading', { level: 1, name: /projects/i })).toBeVisible({ + timeout: 10_000, + }); }); test('shows empty state or project cards when loaded', async ({ page }) => { @@ -18,23 +25,11 @@ test.describe('Projects page', () => { // Wait for loading state to clear await expect(page.getByText(/loading projects/i)).not.toBeVisible({ timeout: 10_000 }); - const hasProjects = await page + const cardsOrEmpty = page .locator('[class*="grid"]') - .isVisible() - .catch(() => false); - const hasEmpty = await page - .getByText(/no projects yet/i) - .isVisible() - .catch(() => false); - - expect(hasProjects || hasEmpty).toBe(true); - }); - - test('shows Active Mission section', async ({ page }) => { - await page.goto('/projects'); - await expect(page.getByRole('heading', { name: /active mission/i })).toBeVisible({ - timeout: 10_000, - }); + .or(page.getByText(/no projects yet/i)) + .first(); + await expect(cardsOrEmpty).toBeVisible({ timeout: 10_000 }); }); test('sidebar navigation is present', async ({ page }) => { diff --git a/apps/web/e2e/settings.spec.ts b/apps/web/e2e/settings.spec.ts index 143b435e..d07a84c3 100644 --- a/apps/web/e2e/settings.spec.ts +++ b/apps/web/e2e/settings.spec.ts @@ -1,11 +1,14 @@ import { test, expect } from '@playwright/test'; -import { loginAs, TEST_USER } from './helpers/auth.js'; +import { loginAs, REQUIRE_SEEDED_AUTH, TEST_USER } from './helpers/auth.js'; test.describe('Settings page', () => { test.beforeEach(async ({ page }) => { await loginAs(page, TEST_USER.email, TEST_USER.password); const url = page.url(); - test.skip(!url.includes('/chat'), 'No seeded test user — skipping authenticated tests'); + test.skip( + !REQUIRE_SEEDED_AUTH && !url.includes('/chat'), + 'No seeded test user — skipping authenticated tests', + ); }); test('settings page loads with heading', async ({ page }) => { diff --git a/apps/web/playwright.config.ts b/apps/web/playwright.config.ts index aaf04204..72bd3762 100644 --- a/apps/web/playwright.config.ts +++ b/apps/web/playwright.config.ts @@ -1,23 +1,30 @@ import { defineConfig, devices } from '@playwright/test'; /** - * Playwright E2E configuration for Mosaic web app. + * Playwright E2E configuration for the Mosaic web SPA. * - * Assumes: - * - Next.js web app running on http://localhost:3000 - * - NestJS gateway running on http://localhost:14242 + * Assumes the NestJS gateway is already running on http://localhost:14242 and + * serving the built SPA bundle (WEB_DIST_DIR pointing at apps/web/dist) — the + * same serving path production uses (Phase P5, #1444). Override the target + * with PLAYWRIGHT_BASE_URL. + * + * global-setup seeds the E2E users through the real bootstrap and admin APIs + * when the database is empty; against an already-populated environment it + * seeds nothing. * * Run with: pnpm --filter @mosaicstack/web test:e2e */ export default defineConfig({ testDir: './e2e', + globalSetup: './e2e/global-setup.ts', fullyParallel: true, forbidOnly: !!process.env['CI'], retries: process.env['CI'] ? 2 : 0, workers: process.env['CI'] ? 1 : undefined, - reporter: 'html', + // CI needs the verdict in the step log; the html report is a local tool. + reporter: process.env['CI'] ? 'list' : 'html', use: { - baseURL: process.env['PLAYWRIGHT_BASE_URL'] ?? 'http://localhost:3000', + baseURL: process.env['PLAYWRIGHT_BASE_URL'] ?? 'http://localhost:14242', trace: 'on-first-retry', screenshot: 'only-on-failure', }, @@ -27,6 +34,6 @@ export default defineConfig({ use: { ...devices['Desktop Chrome'] }, }, ], - // Do NOT auto-start the dev server — tests assume it is already running. + // Do NOT auto-start a server — tests assume the gateway is already running. // webServer is intentionally omitted so tests can run against a live env. }); diff --git a/apps/web/src/spa/pages/settings.tsx b/apps/web/src/spa/pages/settings.tsx index 2e8ee779..098a0a48 100644 --- a/apps/web/src/spa/pages/settings.tsx +++ b/apps/web/src/spa/pages/settings.tsx @@ -57,6 +57,16 @@ function prefValue(prefs: Preference[], key: string, fallback: T): T { return p.value as T; } +// The reset must not outlive the tab: an uncleared setTimeout fires into a +// torn-down environment (unmount, or jsdom teardown under vitest). +function useSavedBadgeReset(saveState: SaveState, setSaveState: (s: SaveState) => void): void { + useEffect(() => { + if (saveState !== 'saved') return undefined; + const timer = setTimeout(() => setSaveState('idle'), 2000); + return () => clearTimeout(timer); + }, [saveState, setSaveState]); +} + // ─── Main Page ──────────────────────────────────────────────────────────────── export function SettingsPage(): React.ReactElement { @@ -111,6 +121,7 @@ function ProfileTab({ const [image, setImage] = useState(session?.user.image ?? ''); const [saveState, setSaveState] = useState('idle'); const [errorMsg, setErrorMsg] = useState(''); + useSavedBadgeReset(saveState, setSaveState); // Sync from session when it loads useEffect(() => { @@ -131,7 +142,6 @@ function ProfileTab({ return; } setSaveState('saved'); - setTimeout(() => setSaveState('idle'), 2000); } catch (err: unknown) { const message = err instanceof Error ? err.message : 'Failed to update profile'; setErrorMsg(message); @@ -194,6 +204,7 @@ function AppearanceTab(): React.ReactElement { const [defaultModel, setDefaultModel] = useState(''); const [saveState, setSaveState] = useState('idle'); const [errorMsg, setErrorMsg] = useState(''); + useSavedBadgeReset(saveState, setSaveState); useEffect(() => { api('/api/memory/preferences?category=appearance') @@ -239,7 +250,6 @@ function AppearanceTab(): React.ReactElement { : []), ]); setSaveState('saved'); - setTimeout(() => setSaveState('idle'), 2000); } catch (err: unknown) { const message = err instanceof Error ? err.message : 'Failed to save preferences'; setErrorMsg(message); @@ -323,6 +333,7 @@ function NotificationsTab(): React.ReactElement { const [emailDigest, setEmailDigest] = useState(false); const [saveState, setSaveState] = useState('idle'); const [errorMsg, setErrorMsg] = useState(''); + useSavedBadgeReset(saveState, setSaveState); useEffect(() => { api('/api/memory/preferences?category=communication') @@ -369,7 +380,6 @@ function NotificationsTab(): React.ReactElement { }), ]); setSaveState('saved'); - setTimeout(() => setSaveState('idle'), 2000); } catch (err: unknown) { const message = err instanceof Error ? err.message : 'Failed to save preferences'; setErrorMsg(message); diff --git a/docs/guides/dev-guide.md b/docs/guides/dev-guide.md index 09830639..4615e28d 100644 --- a/docs/guides/dev-guide.md +++ b/docs/guides/dev-guide.md @@ -212,6 +212,20 @@ Woodpecker `.woodpecker/publish.yml` keeps stable and integration-line artifacts `next` never publishes npm `latest` or Docker `latest`. The next npm publish step verifies that `@mosaicstack/mosaic@next` resolves to the computed prerelease before the pipeline can pass. +### E2E Gate (#1445, P6) + +Trunk publish pipelines run a headless Playwright suite (`e2e` step) before any image publishes: the built gateway `dist` boots on a throwaway embedded PGlite database (isolated via a fresh `HOME`), serves the built SPA bundle through `WEB_DIST_DIR` — the same serving path the gateway image ships — and the suite runs against it inside the pinned `mcr.microsoft.com/playwright` image. `E2E_REQUIRE_SEEDED_AUTH=1` makes login failures hard failures (the skip-when-login-fails guards are a live-environment affordance only). Both image build steps depend on this gate. + +Reproduce locally (Ubuntu-based environments; Fedora's headless-shell rendering is broken): + +```bash +pnpm build +BETTER_AUTH_SECRET="$(head -c 32 /dev/urandom | base64)" GATEWAY_PORT=14242 \ + WEB_DIST_DIR="$PWD/apps/web/dist" HOME="$(mktemp -d)" node apps/gateway/dist/main.js & +E2E_REQUIRE_SEEDED_AUTH=1 PLAYWRIGHT_BASE_URL=http://localhost:14242 \ + pnpm --filter @mosaicstack/web exec playwright test +``` + --- ## Adding New Agent Tools diff --git a/eslint.config.mjs b/eslint.config.mjs index acfed05f..3af1ad2e 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -7,7 +7,6 @@ export default tseslint.config( ignores: [ '**/dist/**', '**/node_modules/**', - '**/.next/**', '**/coverage/**', '**/drizzle.config.ts', '**/framework/**', diff --git a/scripts/verify-release.mjs b/scripts/verify-release.mjs index f35edf14..aca6957b 100644 --- a/scripts/verify-release.mjs +++ b/scripts/verify-release.mjs @@ -21,10 +21,10 @@ // lint | lint | pnpm lint // format | format | pnpm format:check // test | test | pnpm test -// build | publish.yml build | pnpm build +// build | build (#1445, P6) | pnpm build (also publish.yml build) // quality-rails | (canonical-only) | the TS quality-rails evaluator -// | | (RI-N4, QC-19 monorepo subject). Like -// | | `build`, this stage has no ci.yml +// | | (RI-N4, QC-19 monorepo subject). +// | | The one stage with no ci.yml // | | mirror; it is implemented by // | | importing the evaluator CLI rather // | | than duplicating its presence logic. @@ -106,8 +106,8 @@ export const STAGES = [ { // RI-N4 (QC-19, card RI-3-002): the typed quality-rails evaluator, invoked // as the implementation of the check it owns instead of a duplicated - // presence loop here. Canonical-only stage (no ci.yml mirror — same shape - // as `build`); runs AFTER build so the evaluator's dist/ exists. Subject + // presence loop here. Canonical-only stage (no ci.yml mirror; `build` + // gained one in #1445); runs AFTER build so the evaluator's dist/ exists. Subject // is this repository (`.` → monorepo subject kind, per-subject check set). name: 'quality-rails', commands: ['node packages/quality-rails/dist/cli.js quality-rails evaluate --project .'], diff --git a/scripts/verify-release.test.mjs b/scripts/verify-release.test.mjs index c614a7c9..96dcf13c 100644 --- a/scripts/verify-release.test.mjs +++ b/scripts/verify-release.test.mjs @@ -228,9 +228,10 @@ steps: function assertStagesMirrorCi(stages, ci) { const canonical = Object.fromEntries(stages.map((stage) => [stage.name, stage.commands])); - // The complete mandatory set, in gate order. `quality-rails` is a - // canonical-only stage (RI-N4, QC-19): like `build`, it has no ci.yml - // mirror to match — its contract is asserted separately below. + // The complete mandatory set, in gate order. `quality-rails` is the one + // canonical-only stage (RI-N4, QC-19) with no ci.yml mirror to match — its + // contract is asserted separately below. `build` gained a ci.yml mirror in + // #1445 (P6) and is enforced with the other pnpm stages. assert.deepEqual( stages.map((stage) => stage.name), [ @@ -257,7 +258,7 @@ function assertStagesMirrorCi(stages, ci) { // pnpm stages: ci.yml commands minus `corepack enable` must be exactly the // canonical stage commands. - for (const stepName of ['typecheck', 'lint', 'format']) { + for (const stepName of ['typecheck', 'lint', 'format', 'build']) { assert.deepEqual( ci.steps[stepName].commands.filter((command) => command !== 'corepack enable'), canonical[stepName],