From 576c38e21dc01f13e88541a6ef4a40c6d1b3ca7c Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 30 Jul 2026 11:46:49 +1000 Subject: [PATCH 01/17] test(e2e): local-backend Playwright suite for workplace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an end-to-end suite that runs only against a local PlaceOS backend, plus an isolated stack to run it against and an advisory CI workflow. - e2e/support: headless PKCE mint + storageState, real-login driver, worker-scoped auth fixtures, idempotent API-driven seeding, stack preflight - e2e/stack: self-contained 10-service PlaceOS deployment on its own compose project and ports, so it coexists with a developer's own stack - apps/workplace/e2e/local: boot, real login (incl. scope/sub surviving refresh), and a non-admin desk booking driven through the full UI - E2E_USER_STORIES.md: the coverage contract - config/proxy.conf.js: make the dev-server proxy target env-driven (defaults unchanged) The suite refuses any non-loopback backend via an allowlist that throws before a browser launches. CI is ADVISORY and must not gate merges until the suite has a track record — see the CI section of E2E_USER_STORIES.md. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/e2e-advisory.yml | 147 +++++++++ E2E_USER_STORIES.md | 128 ++++++++ apps/workplace/e2e/local/boot.spec.ts | 81 +++++ apps/workplace/e2e/local/desk-booking.spec.ts | 101 ++++++ apps/workplace/e2e/local/login.spec.ts | 105 ++++++ apps/workplace/playwright.config.ts | 102 +++--- config/proxy.conf.js | 15 +- e2e/.env.example | 25 ++ e2e/.gitignore | 9 + e2e/README.md | 276 ++++++++++++++++ e2e/stack/ci-summary.mjs | 106 +++++++ e2e/stack/docker-compose.yml | 255 +++++++++++++++ e2e/stack/down.sh | 5 + e2e/stack/up.sh | 97 ++++++ e2e/support/api.ts | 172 ++++++++++ e2e/support/auth.ts | 205 ++++++++++++ e2e/support/env.ts | 127 ++++++++ e2e/support/fixtures.ts | 137 ++++++++ e2e/support/flows.ts | 142 +++++++++ e2e/support/global-setup.ts | 15 + e2e/support/index.ts | 7 + e2e/support/load-env.ts | 13 + e2e/support/login.ts | 72 +++++ e2e/support/preflight.ts | 33 ++ e2e/support/seed.ts | 298 ++++++++++++++++++ 25 files changed, 2630 insertions(+), 43 deletions(-) create mode 100644 .github/workflows/e2e-advisory.yml create mode 100644 E2E_USER_STORIES.md create mode 100644 apps/workplace/e2e/local/boot.spec.ts create mode 100644 apps/workplace/e2e/local/desk-booking.spec.ts create mode 100644 apps/workplace/e2e/local/login.spec.ts create mode 100644 e2e/.env.example create mode 100644 e2e/.gitignore create mode 100644 e2e/README.md create mode 100644 e2e/stack/ci-summary.mjs create mode 100644 e2e/stack/docker-compose.yml create mode 100755 e2e/stack/down.sh create mode 100755 e2e/stack/up.sh create mode 100644 e2e/support/api.ts create mode 100644 e2e/support/auth.ts create mode 100644 e2e/support/env.ts create mode 100644 e2e/support/fixtures.ts create mode 100644 e2e/support/flows.ts create mode 100644 e2e/support/global-setup.ts create mode 100644 e2e/support/index.ts create mode 100644 e2e/support/load-env.ts create mode 100644 e2e/support/login.ts create mode 100644 e2e/support/preflight.ts create mode 100644 e2e/support/seed.ts diff --git a/.github/workflows/e2e-advisory.yml b/.github/workflows/e2e-advisory.yml new file mode 100644 index 0000000000..d54583cb2e --- /dev/null +++ b/.github/workflows/e2e-advisory.yml @@ -0,0 +1,147 @@ +name: 'E2E (advisory)' + +# ADVISORY ONLY — this must NOT gate merges yet. +# +# The suite is new. Before it is allowed to block anyone it needs a track record: +# no false positives, specs reviewed by humans, and a known flake rate. Until then +# this workflow exists to BUILD that record, not to enforce anything. +# +# Concretely, that means: do not add this job to branch protection / required +# status checks. Nothing in this file can make it required — that is a repository +# setting — so keeping it advisory is a deliberate human decision, not a default. +# The job still reports honest pass/fail; it just does not block. +# +# The nightly run is the point: it accumulates evidence on a fixed codebase, which +# is the only way to tell a real regression from an unstable suite. +# +# Known instability at time of writing (see E2E_USER_STORIES.md REG-09): +# staff-api intermittently 500s with `DB::ConnectionLost` on concurrent +# POST /bookings. Playwright's retries absorb it and mark the spec `flaky` — a +# flaky-but-passing run is reported in the summary rather than hidden. + +on: + workflow_dispatch: + # Lets this workflow be exercised on its own branch before it lives on + # `develop` (where `workflow_dispatch` first becomes available). The prefix is + # deliberately outside the `feat/*`, `fix/*`, `custom/*`, `refactor/*` set that + # PR Flow watches, so iterating on CI here never triggers a build or a + # `build//` deploy. + push: + branches: ['e2e/**'] + schedule: + # 15:10 UTC daily — off-peak for AU-hours contributors, so a nightly + # failure is waiting to be read rather than interrupting anyone. + - cron: '10 15 * * *' + pull_request: + branches: [develop] + +concurrency: + group: e2e-${{ github.ref }} + cancel-in-progress: true + +jobs: + workplace: + name: 'workplace e2e (local stack)' + runs-on: ubuntu-latest + timeout-minutes: 45 + env: + E2E_BACKEND_URL: https://localhost:9443 + # Deliberately lower than the local default of 4. + # + # A GitHub runner has 4 cores and is simultaneously running ~10 + # containers, an Angular dev server and the browsers. Fewer workers + # means less contention, which means fewer failures that are about the + # runner rather than about the code — and avoiding false positives + # matters more here than shaving a minute off the run. + E2E_WORKERS: '2' + TZ: 'Etc/UTC' + steps: + - uses: actions/checkout@v6 + + - name: Cache node modules + id: cache + uses: actions/cache@v5 + with: + path: node_modules + key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/bun.lock') }} + restore-keys: | + ${{ runner.os }}-build-${{ env.cache-name }}- + ${{ runner.os }}-build- + ${{ runner.os }}- + + - name: Setup Node.js environment + uses: actions/setup-node@v6 + with: + node-version: '24' + + - name: Setup Bun + uses: oven-sh/setup-bun@v2 + with: + bun-version: '1.3.11' + + - name: Install dependencies + if: steps.cache.outputs.cache-hit != 'true' + run: bun install --frozen-lockfile + + - name: Setup version + run: bun run postinstall + + # Elasticsearch refuses to start with the runner's default + # vm.max_map_count (65530). Docker Desktop's VM is already above the + # threshold, which is why this is only needed in CI. + - name: Raise vm.max_map_count for Elasticsearch + run: sudo sysctl -w vm.max_map_count=262144 + + - name: Install Playwright browser + run: bunx playwright install --with-deps chromium + + # Brings up an isolated PlaceOS stack (its own compose project, own + # volumes, ports 9443/9080) and seeds it. Needs network access to + # GitHub once, to clone PlaceOS/www-core into the www volume — that is + # where the platform /login page comes from. + - name: Bring up the local PlaceOS stack + run: e2e/stack/up.sh + + - name: Run workplace e2e + run: bunx playwright test --config apps/workplace/playwright.config.ts + + - name: Summarise results + if: ${{ always() }} + run: node e2e/stack/ci-summary.mjs >> "$GITHUB_STEP_SUMMARY" + + - name: Collect backend logs + if: ${{ failure() }} + run: | + mkdir -p e2e-logs + cd e2e/stack + for svc in auth rest-api staff-api nginx frontend-loader search-ingest postgres; do + docker compose -p placeos-e2e logs "$svc" > "../../e2e-logs/$svc.log" 2>&1 || true + done + + - name: Upload Playwright report + if: ${{ always() }} + uses: actions/upload-artifact@v4 + with: + name: playwright-report-workplace + path: | + reports/e2e/workplace + reports/e2e/workplace-results.json + retention-days: 14 + if-no-files-found: warn + + # Traces/videos/screenshots for the failures. Without these a red + # nightly run is unactionable the next morning. + - name: Upload failure artifacts + if: ${{ failure() }} + uses: actions/upload-artifact@v4 + with: + name: playwright-failures-workplace + path: | + dist/.playwright + e2e-logs + retention-days: 14 + if-no-files-found: warn + + - name: Tear down the stack + if: ${{ always() }} + run: e2e/stack/down.sh --volumes diff --git a/E2E_USER_STORIES.md b/E2E_USER_STORIES.md new file mode 100644 index 0000000000..8b98df2ed5 --- /dev/null +++ b/E2E_USER_STORIES.md @@ -0,0 +1,128 @@ +# PlaceOS E2E — coverage contract + +The single source of truth for what the e2e suite covers, what it does not, and why. +Harness, conventions and gotchas live in [`e2e/README.md`](e2e/README.md). + +## CI status: advisory, deliberately + +`.github/workflows/e2e-advisory.yml` runs the suite on PRs into `develop`, nightly, and +on demand. **It must not gate merges yet**, and nothing in the workflow file can make it +do so — required status checks are a repository setting, so keeping this advisory is an +explicit human decision rather than a default. + +That is on purpose. Before this suite is allowed to block anyone it needs a track record: +no false positives, specs reviewed by humans, and a known flake rate. The nightly run +exists to build exactly that — a fixed codebase run repeatedly is the only way to +separate a real regression from an unstable suite. + +The job still reports honest pass/fail and writes a summary that leads on **flaky** +rather than passed, because a spec that only passes on retry is the signal that says +"not yet". + +**Before proposing this as a required check**, expect to be able to say: N consecutive +nightly runs green, every `flaky` occurrence explained, and REG-09 either fixed or +consciously accepted. + +## How this document works + +**It is the mechanism that keeps the suite current.** A suite decays the moment nobody +can tell what it covers, so: + +- **Every new feature** adds a story here and a spec, in the same PR. +- **Every bug fix** adds a `REG-*` row citing the ticket or changelog line, plus a spec + that fails before the fix and passes after. A regression spec that was never seen red + is a guess, not a guard. +- **Every spec that gets skipped or deleted** updates its row to say so, with a reason. +- A row without a status is a bug in this document. + +Priorities: **P0** = smoke gate, must always pass · **P1** = core regression · **P2** = breadth. + +Status: **done** = a green spec exists · **partial** = covered in part, gap named · +**todo** = not written · **blocked** = needs data, access or a fix first. + +Everything runs against a **local backend only** — see `e2e/README.md`. Rows that would +require an external service are marked **out of scope (external)** and must never enter +the PR gate. + +--- + +## 1. Workplace — core flows + +| ID | P | Story | Status | +|----|---|-------|--------| +| WP-E2E-01 | P0 | An authenticated user lands on the workplace home; the shell renders and org data resolves (**not** `/misconfigured`). | **done** — `local/boot.spec.ts` | +| WP-E2E-02 | P0 | An unauthenticated visit redirects to the authority's login page, real credentials sign in, and the app loads authenticated. | **done** — `local/login.spec.ts` | +| WP-E2E-05 | P0 | A **non-admin** books a desk through the full UI; the backend stores it with the right asset, title and zones. | **done** — `local/desk-booking.spec.ts` | +| WP-E2E-06 | P1 | A deleted booking disappears from the listing (teardown really tears down). | **done** — `local/desk-booking.spec.ts` | +| WP-E2E-03 | P1 | Building/level selectors are populated from seeded zones, and changing them re-scopes what is bookable. | todo | +| WP-E2E-07 | P1 | "Your bookings" lists the user's own booking; cancelling it moves it out of the upcoming list. | todo | +| WP-E2E-08 | P1 | A booking made by one user is **not** visible in another user's "your bookings" (per-user scoping). | todo — see AUTH-E2E-05 | +| WP-E2E-09 | P1 | Booking a **locker** end to end. Same metadata + per-worker-asset + sweep pattern as desks. | todo | +| WP-E2E-10 | P1 | Booking a **parking** space end to end. | todo | +| WP-E2E-11 | P2 | Inviting a **visitor** end to end. | todo | +| WP-E2E-12 | P2 | Directory / colleagues search returns seeded users. | todo | +| WP-E2E-13 | P2 | The explore/map view renders for a seeded level and reflects availability. | todo — needs map metadata seeded | +| WP-E2E-14 | P2 | Search validation and empty states: no blank page, no console error. | todo | +| WP-E2E-15 | P1 | **Room/meeting** booking end to end. | **out of scope (external)** — the only surface needing a real Microsoft/Google tenant. Opt-in project, never in the PR gate. | +| WP-E2E-04 | P2 | Mock mode still renders the landing page with no backend at all. | **done** — `landing.spec.ts` (project `mock`) | + +## 2. Auth & session + +Grounded in the auth.cr work (PPT-2536), where every production failure was an +environment, data or real-client gap that unit specs could not see. + +| ID | P | Story | Status | +|----|---|-------|--------| +| AUTH-E2E-01 | P0 | Authorization-code + PKCE exchange in a real browser: no `client_secret` anywhere, `S256` challenge, token is a JWT. | **partial** — `login.spec.ts` asserts the exchange and token shape; the explicit no-secret / challenge-recomputation assertions still live in `tasks/PPT-2536/e2e/backoffice-login.spec.js` and should move here. | +| AUTH-E2E-02 | P0 | A refreshed token keeps its scope, is rotated, preserves `sub`, and is still accepted by rest-api. | **done** — `login.spec.ts`. This is the exact 2026-07-23 revert (403 on `/oauth_apps` after refresh). | +| AUTH-E2E-03 | P1 | A refresh chain survives N sequential refreshes without degrading scope or access. | todo — covered API-only by `tasks/PPT-2536/integration/` (RF-03); wanted in-browser. | +| AUTH-E2E-04 | P1 | A stale/incompatible session cookie from a previous auth implementation does not break sign-in. | todo — verified manually (SC-01); needs automating. | +| AUTH-E2E-05 | P1 | A non-admin cannot read or mutate another user's bookings; an admin's own listing does not leak others'. | todo — **and it matters**: `GET /bookings` is caller-scoped, which we only learned by getting a leak check wrong. | +| AUTH-E2E-06 | P2 | Token expiry mid-session recovers without stranding the SPA. | todo | +| AUTH-E2E-07 | P2 | Malformed and hostile `/auth/*` requests return 4xx, never 5xx and never a backtrace. | todo — covered by auth.cr unit specs (SEC-01); browser-level coverage optional. | +| AUTH-E2E-08 | P1 | `SameSite` behaviour in a genuine third-party/iframe context. | **blocked** — Playwright Chromium cannot create a true third-party context. Known untested incident class (B.7). | + +## 3. Regression coverage + +Each row maps to something that actually broke. Citations are the changelog line or the +task that found it, so the row can be traced. + +| ID | P | Story | Source | Status | +|----|---|-------|--------|--------| +| REG-01 | P0 | Scope is not lost on token refresh; downstream authorisation still passes. | PPT-2536, 2026-07-23 revert | **done** — AUTH-E2E-02 | +| REG-02 | P1 | An overlapping desk booking is rejected rather than silently accepted. | `2607.1` "Fix rejecting overlapping bookings on desk assignment" | todo | +| REG-03 | P1 | A clash check uses the **current** `booking_end`, not a stale one. | `2607.1` "Fix stale booking_end being used for clash check" | todo | +| REG-04 | P1 | Desk booking status displays correctly in the booking list. | `2606.1` "Fix status display for desk bookings" | todo | +| REG-05 | P2 | The authorised-user check has no race on boot (no flash of unauthorised). | `2607.1` "Fix race condition for authorised check" | todo | +| REG-06 | P2 | Timezone parsing does not error for a building with an unusual timezone. | "Fix error when parsing timezones" | todo | +| REG-07 | P2 | Level selection does not persist once the selector is hidden/disabled. | "Fix level selections persisting when selector is disabled/hidden" | todo | +| REG-08 | P1 | An authority with a **relative** `login_url` still reaches a usable login page. | Found 2026-07-30, this suite | **blocked** — currently worked around in `seed.ts`; ts-client resolves a relative `login_url` against the authority host **without its port**, so any non-443 deployment dead-ends. Needs a ts-client/init fix before a spec can assert the good behaviour. | +| REG-09 | P1 | Concurrent `POST /bookings` do not 500. | Found 2026-07-30, this suite | **blocked** — staff-api raises `DB::ConnectionLost` under concurrency (~1 in 8 at 4 workers). Currently absorbed by CI retries and reported as `flaky`. Needs a staff-api fix. | + +## 4. Platform & configuration + +Config gaps caused several production incidents, and they are invisible to UI specs. + +| ID | P | Story | Status | +|----|---|-------|--------| +| CFG-01 | P0 | The suite refuses to run against any non-loopback backend. | **done** — `assertLocalOnly`, throws at config load | +| CFG-02 | P0 | A cold stack seeds to a working state in one command. | **done** — `e2e/stack/up.sh --fresh`, verified from destroyed volumes | +| CFG-03 | P1 | staff-api has a tenant for the domain, or every `/bookings` call 500s. | **done** — `seed.ts`; asserted implicitly by WP-E2E-05 | +| CFG-04 | P1 | A missing `org`/parented-`level` zone is caught as `/misconfigured`, not as a blank page. | **partial** — WP-E2E-01 asserts the healthy path; the negative case is unasserted | +| CFG-05 | P2 | Deployment-shaped run: the app served by nginx at `/workplace/` behind the `verified` cookie gate. | todo — a second project; `mintToken` already captures the cookie | + +--- + +## Notes & blockers + +- **Room/calendar events are the only genuinely external surface.** A placeholder tenant + unblocks every PlaceOS-native booking type (desks, lockers, parking, visitors) with no + outbound call. `/calendars` and `/events` do call Microsoft and fail `AADSTS900023`, so + WP-E2E-15 stays opt-in and out of the gate. +- **Two rows are blocked on product fixes, not on test effort** (REG-08, REG-09). Both were + found by this suite. Leaving them visible here is the point — a blocked row is coverage + information, a deleted row is not. +- **AUTH-E2E-08 may never be automatable** with Playwright Chromium. Say so rather than + quietly dropping it. +- The PPT-2536 harnesses (`tasks/PPT-2536/{e2e,integration}`) still hold assertions that + belong in this suite. Folding them in is tracked as AUTH-E2E-01 and -03. diff --git a/apps/workplace/e2e/local/boot.spec.ts b/apps/workplace/e2e/local/boot.spec.ts new file mode 100644 index 0000000000..237523c9a9 --- /dev/null +++ b/apps/workplace/e2e/local/boot.spec.ts @@ -0,0 +1,81 @@ +/** + * WP-E2E-01 — the Phase 0 gate. + * + * Proves the whole chain works against a LOCAL backend with no mocks: + * a headlessly-minted PKCE bearer is accepted by the SPA, the SPA boots + * authenticated, and the org data it needs actually resolves. + * + * The `/misconfigured` assertion is the load-bearing one. OrganisationService + * routes there when it cannot find a zone tagged `org`, or cannot find any zone + * tagged `level` that has a parent_id (libs/common/src/lib/org/organisation.service.ts + * :522, :640). A stack whose seed data is too thin renders a perfectly healthy- + * looking page that is in fact the failure state, so asserting "not misconfigured" + * is what separates "the app booted" from "the app booted and has data". + */ +import { test, expect } from '../../../../e2e/support/fixtures'; +import { APP_URL } from '../../../../e2e/support/env'; +import { clientId, redirectUriFor } from '../../../../e2e/support/auth'; +import { currentUser, zonesWithTag } from '../../../../e2e/support/api'; + +test.describe('workplace boots against the local backend', () => { + test('the seeded token authenticates the SPA and org data resolves', async ({ page }) => { + const failed_api: string[] = []; + page.on('response', (r) => { + const url = r.url(); + if ((url.includes('/api/') || url.includes('/auth/')) && r.status() >= 400) { + failed_api.push(`${r.status()} ${r.request().method()} ${new URL(url).pathname}`); + } + }); + + await page.goto('/#/'); + + // The SPA must not bounce to a login redirect — that means the seeded + // localStorage token was not picked up (wrong client_id, or expired). + await expect + .poll(() => page.url(), { timeout: 30_000 }) + .not.toMatch(/\/auth\/login|\/login\?/); + + // ...and it must not land on the misconfigured page. + await expect + .poll(() => page.url(), { timeout: 30_000 }) + .not.toContain('misconfigured'); + + // The shell renders. + await expect(page.locator('topbar')).toBeVisible({ timeout: 30_000 }); + + // The token really is in the store ts-client reads from, under the key + // derived from THIS app's redirect_uri. + const cid = clientId(redirectUriFor(APP_URL)); + const stored = await page.evaluate((k) => localStorage.getItem(k), `${cid}_access_token`); + expect(stored, `localStorage[${cid}_access_token] should hold the bearer`).toBeTruthy(); + + if (failed_api.length) { + console.log(' 4xx/5xx API calls during boot:\n ', failed_api.join('\n ')); + } + }); + + // Deliberately asserted as the NON-admin worker identity: a sys_admin can see + // zones a normal user cannot, so proving the hierarchy exists for an ordinary + // user is the assertion that actually matters. + test('the backend has the org hierarchy workplace requires', async ({ staffApi: api }) => { + const user = await currentUser(api); + expect(user.email, 'the token resolves to a real user').toBeTruthy(); + expect(user.sys_admin, 'this assertion runs as a NON-admin').toBeFalsy(); + + const orgs = await zonesWithTag(api, 'org'); + expect(orgs.length, 'at least one zone tagged `org` (else -> /misconfigured)').toBeGreaterThan(0); + + const levels = await zonesWithTag(api, 'level'); + const parented = levels.filter((z) => z.parent_id); + expect( + parented.length, + 'at least one zone tagged `level` WITH a parent_id (else -> /misconfigured)', + ).toBeGreaterThan(0); + + const buildings = await zonesWithTag(api, 'building'); + console.log( + ` org data: ${orgs.length} org, ${buildings.length} building, ` + + `${levels.length} level (${parented.length} parented)`, + ); + }); +}); diff --git a/apps/workplace/e2e/local/desk-booking.spec.ts b/apps/workplace/e2e/local/desk-booking.spec.ts new file mode 100644 index 0000000000..3ec388763c --- /dev/null +++ b/apps/workplace/e2e/local/desk-booking.spec.ts @@ -0,0 +1,101 @@ +/** + * WP-E2E-05 — book a desk through the full UI, as a NON-ADMIN, against a real + * local backend. + * + * This is the first spec that makes the stack do actual work rather than just + * answer 200s, and the first to lean on the placeholder-tenant finding: staff-api + * rejects every /bookings call until a tenant exists, but desks need no calendar + * credentials, so this flow needs nothing external. + * + * Two isolation rules make it safe in parallel: + * - Each worker books its OWN desk (a desk is exclusive for a time range). + * - Each test SWEEPS that desk before booking, so a previous crashed run cannot + * poison it. The bookings are all-day, so a leaked one would otherwise hold + * the desk for the remainder of the day and every later run would fail with + * something that looks unrelated. + */ +import { test, expect } from '../../../../e2e/support/fixtures'; +import { deskFor } from '../../../../e2e/support/env'; +import { bookDeskViaUI } from '../../../../e2e/support/flows'; +import { + deleteBooking, + getBooking, + listBookings, + releaseAsset, + uniqueTitle, +} from '../../../../e2e/support/api'; + +/** Window wide enough to cover an all-day booking made in any timezone. */ +const DAY = 86_400; +const window_from = () => Math.floor(Date.now() / 1000) - 2 * DAY; +const window_to = () => Math.floor(Date.now() / 1000) + 2 * DAY; + +test.describe('desk booking', () => { + test('a non-admin books a desk in the UI and the backend stores it', async ({ + staffPage, + staffApi, + }, testInfo) => { + const desk = deskFor(testInfo.parallelIndex); + const title = uniqueTitle('E2E Desk'); + let booking_id: number | undefined; + + const swept = await releaseAsset(staffApi, 'desk', desk.id, window_from(), window_to()); + if (swept) console.log(` swept ${swept} stale booking(s) off ${desk.id}`); + + try { + const created = await bookDeskViaUI(staffPage, desk.name, title); + booking_id = created.id; + + expect(created.id, 'the API returned a booking id').toBeTruthy(); + expect(created.asset_id, 'the booking is against this worker’s desk').toBe(desk.id); + expect(created.title, 'the title we typed reached the backend').toBe(title); + + // The UI reached its success state — proof the app believed it too, + // not merely that a POST happened. + await expect(staffPage).toHaveURL(/#\/book\/desk\/success/); + + // Read it back independently of the response we just parsed. + const stored = await getBooking(staffApi, booking_id); + expect(stored.booking_type, 'stored as a desk booking').toBe('desk'); + expect(stored.asset_id).toBe(desk.id); + expect(stored.title).toBe(title); + expect(stored.deleted, 'not soft-deleted').toBeFalsy(); + expect(stored.rejected, 'not rejected').toBeFalsy(); + expect( + stored.zones.length, + 'carries its zone hierarchy (org/building/level)', + ).toBeGreaterThan(0); + + // ...and that it is discoverable through the listing the app uses, + // not only by direct id lookup. + const listed = await listBookings(staffApi, 'desk', window_from(), window_to()); + expect( + listed.map((b) => b.id), + 'the new booking appears in the desk listing', + ).toContain(booking_id); + } finally { + if (booking_id != null) await deleteBooking(staffApi, booking_id); + } + }); + + test('a deleted desk booking leaves the listing', async ({ staffPage, staffApi }, testInfo) => { + const desk = deskFor(testInfo.parallelIndex); + const title = uniqueTitle('E2E Desk Cleanup'); + + await releaseAsset(staffApi, 'desk', desk.id, window_from(), window_to()); + + const created = await bookDeskViaUI(staffPage, desk.name, title); + expect( + (await listBookings(staffApi, 'desk', window_from(), window_to())).map((b) => b.id), + 'precondition: the booking is in the listing before we delete it', + ).toContain(created.id); + + await deleteBooking(staffApi, created.id); + + expect( + (await listBookings(staffApi, 'desk', window_from(), window_to())).map((b) => b.id), + 'a deleted booking must not come back in the listing — otherwise every ' + + 'spec teardown silently leaks state into the next run', + ).not.toContain(created.id); + }); +}); diff --git a/apps/workplace/e2e/local/login.spec.ts b/apps/workplace/e2e/local/login.spec.ts new file mode 100644 index 0000000000..842ac8abcd --- /dev/null +++ b/apps/workplace/e2e/local/login.spec.ts @@ -0,0 +1,105 @@ +/** + * WP-E2E-02 — the REAL login flow, no injected token. + * + * This is the spec that a synthesised-storageState suite structurally cannot + * write, and it covers the path that has actually regressed in production: the + * redirect handshake, the authorization-code exchange, PKCE, and the shape of + * the token that comes back. PPT-2536 was exactly this — a token that parsed + * fine but lost its scope, so every downstream authorisation check 403'd. + * + * Everything else in the suite injects a token for speed. This spec is what + * earns that shortcut. + */ +import { test, expect } from '../../../../e2e/support/fixtures'; +import { loginViaUI } from '../../../../e2e/support/login'; +import { APP_URL, roleFor } from '../../../../e2e/support/env'; +import { clientId, redirectUriFor } from '../../../../e2e/support/auth'; + +// The subject IS authentication, so start with no credentials at all. +test.use({ storageState: undefined }); + +const CLIENT_ID = clientId(redirectUriFor(APP_URL)); + +/** Read a JWT payload without verifying — we are asserting on claims, not trust. */ +function claims(token: string): Record { + const payload = token.split('.')[1]; + return JSON.parse(Buffer.from(payload, 'base64url').toString('utf8')); +} + +test.describe('real login', () => { + test('an unauthenticated visit signs in through the authority and lands in the app', async ({ + page, + }) => { + const { token } = await loginViaUI(page, roleFor('admin')); + + // The browser really performed a code exchange... + expect(token.access_token, 'the SPA received an access token').toBeTruthy(); + expect(token.access_token!.startsWith('eyJ'), 'access token is a JWT').toBeTruthy(); + + // ...with the scope intact. An empty scope here is the 2026-07-23 revert + // bug: the token authenticates but every authorisation check 403s. + expect(token.scope, 'token carries the public scope').toBe('public'); + expect(token.refresh_token, 'a refresh token was issued').toBeTruthy(); + + // And the app is genuinely usable, not merely rendered. + await expect(page).toHaveURL(/#\/landing/); + await expect(page.locator('topbar')).toBeVisible(); + }); + + test('the issued token is accepted by rest-api and survives a refresh', async ({ page }) => { + const { token } = await loginViaUI(page, roleFor('admin')); + expect(token.refresh_token, 'need a refresh token for this spec').toBeTruthy(); + + const call = (t?: string) => + page.evaluate( + async (bearer) => + ( + await fetch('/api/engine/v2/users/current', { + headers: { Authorization: `Bearer ${bearer}` }, + }) + ).status, + t, + ); + + expect(await call(token.access_token), 'freshly issued token is accepted').toBe(200); + + // Refresh with no client_secret — PlaceOS SPAs are public clients. + const refreshed = await page.evaluate( + async ({ cid, rt }) => { + const r = await fetch('/auth/oauth/token', { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams({ + grant_type: 'refresh_token', + client_id: cid, + refresh_token: rt, + }), + }); + const j = await r.json().catch(() => ({}) as Record); + return { status: r.status, access_token: j.access_token, scope: j.scope }; + }, + { cid: CLIENT_ID, rt: token.refresh_token! }, + ); + + expect(refreshed.status, 'refresh -> 200').toBe(200); + expect(refreshed.access_token, 'refresh returned a token').toBeTruthy(); + expect(refreshed.access_token, 'the token was rotated').not.toBe(token.access_token); + // The regression that caused the revert: scope silently lost on refresh. + expect(refreshed.scope, 'scope survives the refresh').toBe('public'); + + // Identity must survive too. A refresh that quietly changes `sub` would + // hand the session to a different user — the token still validates, so + // nothing downstream would notice. + const before_claims = claims(token.access_token!); + const after_claims = claims(refreshed.access_token!); + expect(after_claims.sub, 'the refreshed token keeps the same subject').toBe( + before_claims.sub, + ); + expect(before_claims.sub, 'the subject is a real user id').toBeTruthy(); + + expect( + await call(refreshed.access_token), + 'the REFRESHED token is still accepted by rest-api', + ).toBe(200); + }); +}); diff --git a/apps/workplace/playwright.config.ts b/apps/workplace/playwright.config.ts index 89f2eb5dd5..4651459dd8 100644 --- a/apps/workplace/playwright.config.ts +++ b/apps/workplace/playwright.config.ts @@ -3,62 +3,84 @@ import { defineConfig, devices } from '@playwright/test'; import { workspaceRoot } from '@nx/devkit'; -const baseURL = process.env['BASE_URL'] || 'http://localhost:4214'; +// env.ts self-loads e2e/.env before reading any E2E_* var, so importing it here +// is enough to get .env applied. +import { APP_URL, BACKEND_URL, WORKERS, assertLocalOnly } from '../../e2e/support/env'; + +// Fail fast, loudly, before a single browser launches, if anything points at a +// backend that is not local. This suite creates and deletes real data. +assertLocalOnly(BACKEND_URL, APP_URL); + +const CI = !!process.env.CI; +const PORT = new URL(APP_URL).port || '4214'; export default defineConfig({ ...nxE2EPreset(__filename, { testDir: './e2e' }), + globalSetup: require.resolve('../../e2e/support/global-setup'), + timeout: 90_000, + expect: { timeout: 15_000 }, + // We own the whole stack, so isolation is a seeding problem, not a reason to + // serialise: seed.ts provisions one non-admin identity per worker and the + // auth fixtures mint per worker. Keep `workers` <= E2E_WORKERS. + fullyParallel: true, + workers: WORKERS, + forbidOnly: CI, + retries: CI ? 2 : 0, reporter: [ ['list'], - [ - 'html', - { outputFolder: '../../reports/e2e/workplace', open: 'never' }, - ], + ['html', { outputFolder: '../../reports/e2e/workplace', open: 'never' }], + // `json` feeds the CI job summary (counts, and which specs were flaky — + // a flaky pass is the signal we care about most while confidence is + // still being built). `github` adds inline annotations on failures. + ...(CI + ? ([ + ['json', { outputFile: '../../reports/e2e/workplace-results.json' }], + ['github'], + ] as const) + : []), ], use: { - baseURL, + baseURL: APP_URL, + // The local stack terminates TLS with a self-signed cert. + ignoreHTTPSErrors: true, trace: 'on-first-retry', - }, - webServer: { - command: 'bunx nx serve workplace --port=4214', - url: 'http://localhost:4214', - reuseExistingServer: !process.env.CI, - cwd: workspaceRoot, - timeout: 120000, + screenshot: 'only-on-failure', + video: 'retain-on-failure', + actionTimeout: 20_000, + navigationTimeout: 45_000, }, projects: [ + // --- real local backend, no mocks ---------------------------------- + // Auth comes from the worker-scoped fixtures in e2e/support/fixtures.ts, + // so there is no setup project to serialise behind. { - name: 'chromium', + name: 'local', + testDir: './e2e/local', use: { ...devices['Desktop Chrome'] }, }, + // --- mock mode: no backend at all, fast render regression ---------- + // Kept deliberately. It costs no infrastructure and catches pure UI + // breakage, so it stays as its own project rather than being replaced. { - name: 'firefox', - use: { ...devices['Desktop Firefox'] }, + name: 'mock', + testDir: './e2e', + testIgnore: ['**/local/**'], + use: { ...devices['Desktop Chrome'] }, }, - - { - name: 'webkit', - use: { ...devices['Desktop Safari'] }, + ], + webServer: { + command: `bunx nx serve workplace --port=${PORT}`, + url: APP_URL, + reuseExistingServer: !CI, + cwd: workspaceRoot, + timeout: 180_000, + // Point the dev server's API proxy at the local stack instead of its + // default (the shared dev deployment). See config/proxy.conf.js. + env: { + PLACE_PROXY_DOMAIN: new URL(BACKEND_URL).host, + PLACE_PROXY_SECURE: String(new URL(BACKEND_URL).protocol === 'https:'), + PLACE_PROXY_VALID_SSL: 'false', }, - - // Uncomment for mobile browsers support - /* { - name: 'Mobile Chrome', - use: { ...devices['Pixel 5'] }, }, - { - name: 'Mobile Safari', - use: { ...devices['iPhone 12'] }, - }, */ - - // Uncomment for branded browsers - /* { - name: 'Microsoft Edge', - use: { ...devices['Desktop Edge'], channel: 'msedge' }, - }, - { - name: 'Google Chrome', - use: { ...devices['Desktop Chrome'], channel: 'chrome' }, - } */ - ], }); diff --git a/config/proxy.conf.js b/config/proxy.conf.js index 98978b1c36..d80a67f42d 100644 --- a/config/proxy.conf.js +++ b/config/proxy.conf.js @@ -1,9 +1,18 @@ +/** + * Defaults target the shared dev deployment. Override via env to point a dev + * server at another backend — e.g. the local PlaceOS stack, which the e2e suite + * requires: + * + * PLACE_PROXY_DOMAIN=localhost:8443 PLACE_PROXY_VALID_SSL=false nx serve workplace + * + * (`PLACE_PROXY_VALID_SSL=false` is needed for the local stack's self-signed cert.) + */ /** FQDN to proxy requests. i.e. No protocol and path should be in the value */ -const domain = 'placeos-dev.aca.im'; +const domain = process.env.PLACE_PROXY_DOMAIN || 'placeos-dev.aca.im'; /** Whether the proxied endpoints use SSL */ -const secure = true; +const secure = process.env.PLACE_PROXY_SECURE !== 'false'; /** Whether the SSL certificate used is valid on the internet */ -const valid_ssl = true; +const valid_ssl = process.env.PLACE_PROXY_VALID_SSL !== 'false'; const PROXY_CONFIG = {}; diff --git a/e2e/.env.example b/e2e/.env.example new file mode 100644 index 0000000000..4dd83d851d --- /dev/null +++ b/e2e/.env.example @@ -0,0 +1,25 @@ +# Copy to e2e/.env (gitignored) and adjust. Secrets never get committed. +# +# THIS SUITE IS LOCAL-BACKEND ONLY. Both URLs below are checked against a +# loopback allowlist before any browser launches; anything else is refused. + +# The PlaceOS stack under test. Default matches e2e/stack (`e2e/stack/up.sh`); +# use https://localhost:8443 to target a PlaceOS/local stack instead. +E2E_BACKEND_URL=https://localhost:9443 + +# Where the SPA under test is served (the Angular dev server). +E2E_APP_URL=http://localhost:4214 + +# The stack's seeded sys_admin. +E2E_ADMIN_EMAIL=support@place.tech +E2E_ADMIN_PASSWORD=development + +# Shared password for the per-worker non-admin users. Their addresses are derived, +# not configured — `e2e-staff-@place.tech`, one per worker, seeded by +# e2e/support/seed.ts. A non-admin is required for anything permission-gated: a +# sys_admin bypasses those checks, which makes the assertion vacuous. +E2E_STAFF_PASSWORD=e2e-staff-development + +# How many parallel workers to provision for. Must match (or exceed) the config's +# `workers`; re-run seed.ts after changing it. CI uses 2. +E2E_WORKERS=4 diff --git a/e2e/.gitignore b/e2e/.gitignore new file mode 100644 index 0000000000..e38f08eca9 --- /dev/null +++ b/e2e/.gitignore @@ -0,0 +1,9 @@ +# Auth artifacts — impersonation-grade bearer tokens. NEVER commit. +.auth/ +.env + +# Playwright outputs +test-results/ +playwright-report/ +blob-report/ +.secrets/ diff --git a/e2e/README.md b/e2e/README.md new file mode 100644 index 0000000000..43fa09a5ae --- /dev/null +++ b/e2e/README.md @@ -0,0 +1,276 @@ +# PlaceOS e2e — local backend only + +Playwright end-to-end tests that run against a **local PlaceOS stack**. There is no +"which environment am I pointed at today" mode: `assertLocalOnly()` refuses any +non-loopback host before a browser launches. A suite that creates, mutates and +deletes real data must never be one typo away from doing it to a deployment. + +Currently wired up for **workplace**. Other apps follow the same shape. + +> **What is covered, and what is not:** [`../E2E_USER_STORIES.md`](../E2E_USER_STORIES.md). +> Adding a feature or fixing a bug means adding a row there and a spec, in the same PR — +> that document is what stops the suite decaying. + +## Run it + +```bash +# 1. an isolated backend, seeded and ready (see "The stack" below) +e2e/stack/up.sh # --fresh to destroy its volumes first + +# 2. the tests +export E2E_BACKEND_URL=https://localhost:9443 +bunx playwright test --config apps/workplace/playwright.config.ts # everything +bunx playwright test --config apps/workplace/playwright.config.ts --project=local # real backend +bunx playwright test --config apps/workplace/playwright.config.ts --project=mock # no backend +bunx playwright test --config apps/workplace/playwright.config.ts --ui # watch it +``` + +Playwright starts the Angular dev server itself (`nx serve workplace --port=4214`) +and points its API proxy at the stack. Nothing reaches the network. + +If a run dies with "Executable doesn't exist", the workspace's pinned Playwright +needs its browser: `bunx playwright install chromium`. + +To target a PlaceOS/local stack instead, set `E2E_BACKEND_URL=https://localhost:8443` +and run `bunx tsx e2e/support/seed.ts` once. Note that seeding mutates that stack +(it adds an OAuth app, a tenant, test users, and makes `login_url` absolute), so +prefer the isolated stack if someone else is using yours. + +## The stack + +`e2e/stack/` is a self-contained PlaceOS deployment for testing — **not** +PlaceOS/local. It runs as its own compose project (`placeos-e2e`) with its own +network, volumes and secrets, on shifted ports (9443/9080), so it coexists with a +developer's own stack and cannot disturb it. + +```bash +e2e/stack/up.sh # bring up + seed (reuses volumes) +e2e/stack/up.sh --fresh # destroy volumes first — a genuine cold start +e2e/stack/down.sh # stop (--volumes to wipe) +``` + +It is trimmed to what the e2e path exercises: postgres, elasticsearch, redis, +search-ingest, frontend-loader, auth, rest-api, staff-api, nginx, init. Dropped +from PlaceOS/local: core, edge, triggers, dispatch, source, influx, chronograf, +mosquitto, minio and the loki/grafana profile — roughly half the containers. + +Two things a cold start taught us that a long-lived stack hides: + +- **`frontend-loader` is not optional**, even though the SPA is served by the dev + server. On startup it clones `PlaceOS/www-core` into the shared `www` volume, + and that is where the platform's `/login` page lives. Without it nginx serves a + bare 404 at `/login`. `up.sh` gates on `/login` returning 200 before seeding. + (This is the one point where bring-up needs network access to GitHub.) +- **A fresh authority gets a RELATIVE `login_url`** (`/login?continue={{url}}`), + and ts-client resolves that against the authority's `domain` column — a bare + host with **no port** — so on any non-default port it navigates to a dead URL + (`http://localhost/login`) and the app dead-ends on a browser error page. + `seed.ts` patches `login_url` to an absolute URL. A long-lived stack usually has + an absolute value already, which is why this never shows up locally. + +## Layout + +``` +e2e/ shared engine, all apps + stack/ isolated PlaceOS deployment (compose + up.sh/down.sh) + support/ + load-env.ts dotenv side-effect loader + env.ts config + per-worker roles + assertLocalOnly() + auth.ts headless PKCE mint -> storageState + login.ts drives the REAL login form + fixtures.ts worker-scoped auth fixtures — import test/expect from here + api.ts engine/staff-api helpers + asset sweep + flows.ts multi-step UI flows (bookDeskViaUI) + preflight.ts "is the stack up?" — fails in 1s, not 90 + global-setup.ts runs preflight once per suite + seed.ts idempotent API-driven seeding + .env.example copy to .env to override anything +apps/workplace/ + playwright.config.ts projects: local, mock + e2e/ + local/*.spec.ts real-backend specs + *.spec.ts mock-mode specs (no backend) +``` + +## Two origins, not one + +Under `nx serve` the app and the backend are **different origins**, and conflating +them is the fastest way to a mysteriously unauthenticated SPA: + +| | | +|---|---| +| `E2E_BACKEND_URL` | `https://localhost:8443` — the stack (nginx TLS entrypoint) | +| `E2E_APP_URL` | `http://localhost:4214` — the dev server serving the SPA | + +The dev server proxies `/api`, `/auth`, `/control` through to the backend, so the +browser only ever sees `E2E_APP_URL` — and that is the origin localStorage (and +therefore the token) belongs to. + +## Auth: two paths, and when to use which + +**The real login flow is the primary path.** `login.ts` drives it: unauthenticated +visit → redirect to the authority's login page (a cross-origin hop, with `continue` +carrying the return URL) → credentials → back into the app. Use it for anything +whose *subject* is authentication. `apps/workplace/e2e/local/login.spec.ts` is the +worked example, and it guards the exact class of regression that has bitten +production: scope surviving a refresh, rotation, and rest-api still accepting the +refreshed token. + +**Injected tokens are a speed optimisation, not the default way in.** The worker +fixtures mint a bearer over PKCE and write a `storageState` seeding +`localStorage[${cid}_access_token]`, so a spec that merely needs to *be* logged in +skips ~5s of form driving. Every spec gets this automatically. Specs that test auth +opt out: + +```ts +test.use({ storageState: undefined }); +``` + +Things that will bite you: + +- **After a real login, the access token is NOT in localStorage.** ts-client only + persists it when the device is trusted — `_storeTokenDetails` gates the + `setItem` on `isTrusted()` (ts-client/src/auth/functions.ts:1042); otherwise the + bearer lives in memory and only `${cid}_expires_at` is written. So "the token is + in localStorage" is true of a seeded state and **false of a real login**. Assert + on the token exchange or on `/users/current`, never on the key. +- **`cid` is derived from where the app is served.** ts-client computes + `redirect_uri = ${location.origin}${route}oauth-resp.html` at runtime, so the dev + server and a stack-deployed build are *different clients* and each needs its own + registered application. `seed.ts` registers the dev-server one. +- **`_expires_at` must be in the future** in a seeded state, or ts-client treats the + token as stale and triggers a re-auth redirect mid-test. +- **Do not inject `sessionStorage['ENGINE.auth.params']`.** The app deletes it on + init; re-adding a stale `code` makes ts-client try to re-exchange it and auth breaks. +- **nginx gates static assets behind an HMAC `verified` cookie.** Irrelevant while + the dev server serves the SPA, but a suite pointed at a stack-served build + (`https://localhost:8443/workplace/`) will 302 to `/auth/login` unless the cookie + is carried across. `mintToken` already captures it. + +## Parallelism + +The suite runs `fullyParallel` across `E2E_WORKERS` (default 4). We own the whole +stack, so isolation is a **seeding** problem rather than a reason to serialise: +`seed.ts` provisions one non-admin identity per worker (`e2e-staff-0@place.tech` …) +and `fixtures.ts` mints per worker, keyed on `parallelIndex`. + +Fixtures available: + +| Fixture | What | +|---|---| +| `page` | authenticated as this worker's **admin** (the default `storageState`) | +| `staffPage` | a page authenticated as this worker's **non-admin** | +| `staffApi` | an `APIRequestContext` as this worker's non-admin | + +**Assert permission-gated behaviour as `staffPage`/`staffApi`.** A `sys_admin` +bypasses the checks, so the same assertion made as admin passes whether or not the +permission logic works. + +Raise `E2E_WORKERS` and re-run `seed.ts` to scale out; keep the config's `workers` +at or below it. + +## What `seed.ts` sets up, and why + +| Step | Why | +|---|---| +| OAuth application for the dev-server `redirect_uri` | ts-client's `client_id` is `Md5(redirect_uri)`; authorize refuses without a matching `uid`. | +| An absolute authority `login_url` | A relative one resolves without the port and dead-ends the login redirect (see "The stack"). | +| A **tenant** for the backend domain | staff-api rejects *every* `/bookings` and `/events` call with "domain does not have a tenant configured" until one exists. | +| One **non-admin** user per worker | Permission gating needs a non-admin; parallel mutation needs distinct identities. | + +`seed.ts` polls for the authority rather than reading it once: `/domains` is +served from Elasticsearch, so on a cold stack the row exists in Postgres before +the API can see it. Failing fast there is the single most likely way a CI run +breaks, and the error looks nothing like the cause. + +It bootstraps through the `backoffice` application, which `init` always creates — +otherwise registering an OAuth app would require a token that requires an OAuth app. + +### The tenant credentials are placeholders on purpose + +staff-api only dereferences tenant credentials when it instantiates a +PlaceCalendar client, which happens on the calendar-backed routes and nowhere else. +So a placeholder tenant unblocks the **entire PlaceOS-native booking surface** — +desks, lockers, parking, visitors — with zero external calls. Verified: + +``` +GET /bookings?type=desk|locker|parking|visitor -> 200 (placeholder tenant) +GET /calendars, GET /events -> 500 (need real credentials) +``` + +**Room/calendar events are the only surface that needs a real Microsoft/Google +tenant.** Those specs are therefore opt-in, must live under a separate project, and +must never be part of the PR gate — wiring real credentials in would make the suite +depend on an external service, which is the one thing it is designed not to do. + +## CI + +`.github/workflows/e2e-advisory.yml` — runs on PRs into `develop`, nightly at 15:10 UTC, +and via **Run workflow**. It brings up the isolated stack, seeds it, runs the suite, writes +a step summary, and uploads the HTML report plus (on failure) traces, videos and backend +container logs. + +**It is advisory and must stay that way for now** — do not add it to branch protection or +required status checks until the suite has a track record. See the CI section of +[`../E2E_USER_STORIES.md`](../E2E_USER_STORIES.md) for the bar it needs to clear first. + +CI-specific choices worth knowing: + +| | | +|---|---| +| `E2E_WORKERS: 2` | Lower than the local default of 4. A runner has 4 cores and is also hosting ~10 containers, a dev server and browsers. Less contention means fewer failures that are about the runner rather than the code — avoiding false positives matters more than run time here. | +| `sudo sysctl -w vm.max_map_count=262144` | Elasticsearch will not start on a runner's default (65530). Docker Desktop's VM is already above it, which is why this is CI-only. | +| `bunx playwright install --with-deps chromium` | The workspace's pinned Playwright needs its browser and system libs. | +| Network access to GitHub | `up.sh` clones `PlaceOS/www-core` into the `www` volume once — that is where the platform `/login` page comes from. | + +## Booking specs: what the backend actually does + +Learned the hard way while building the desk specs. Each of these produced a +failure that looked like something else entirely. + +- **`GET /bookings` is scoped to the CALLER.** Listing as an admin does not show + other users' bookings, so "I checked for leaks as admin and found none" proves + nothing. Verify against the database, or list as the owning user. Deleting is + caller-scoped too. +- **The list query parameter is `type`, not `booking_type`.** The latter is the + model field and appears in the controller's `PARAMS`, which makes the source + misleading; the request 422s with `missing required parameter 'type'`. +- **Desks come from Zone METADATA, not systems** — `metadata.desks.details[]` on + a *level* zone. `groups: []` leaves them unrestricted; a populated `groups` + gates them behind membership. +- **A desk is exclusive for its time range.** Each worker books its own desk, and + each spec **sweeps that desk before booking** (`releaseAsset`). A post-test + `finally` alone is not enough: if a run dies mid-flight the leftover holds the + desk, and because these are all-day bookings it holds it for the rest of the + day. Every later run then fails as "desk not offered" or a 422 with an empty + `failures` array — nothing that points at the real cause. +- **`POST /bookings` occasionally 500s with `DB::ConnectionLost`** under + concurrent writes — roughly 1 run in 8 at 4 workers, on stock connection + settings (which PlaceOS/local ships too). This is a **staff-api defect, not a + rig artifact**; sizing the connection pool in compose was tried and did not + demonstrably help. CI's `retries: 2` absorbs it and reports the test as `flaky`, + which is the right outcome: the gate stays usable and the instability stays + visible. Worth fixing in staff-api on its own merits. +- **Calendar routes DO reach the internet.** With the placeholder tenant, + `/calendars` and `/events` call `login.microsoftonline.com` and fail with + `AADSTS900023`. So "zero external calls" is true of the routes these specs + exercise, not of the whole app — the boot spec logs those 500s as expected noise. + +## Conventions + +- **One spec owns its own data.** Create it, assert it, delete it in a `finally`. + Use `uniqueTitle()` so concurrent workers cannot collide. +- **Sweep before you create, not just after.** Post-test cleanup handles the happy + path; a pre-test sweep is what lets a spec recover from a previous crashed run. +- **Never blindly toggle a control** — ensure the state you want. Toggling a + checkbox that happened to start in the target state silently inverts your + intent, and the flow then dead-ends in a way that reads as a selector bug. +- **Assert through the API, not only UI text.** A green screen reflecting stale + state is worse than a red test. +- **Every bugfix PR adds a regression spec** carrying the ticket ID, and a `REG-*` row + in `E2E_USER_STORIES.md`. A regression spec that was never seen red is a guess, + not a guard. +- **Red-check new assertions.** Break the expected value once and confirm the test + fails for the reason you think. An assertion that never observed real data is + worse than no assertion. +- `.auth/` and `.env` are gitignored — those tokens are impersonation-grade. diff --git a/e2e/stack/ci-summary.mjs b/e2e/stack/ci-summary.mjs new file mode 100644 index 0000000000..7a787788dd --- /dev/null +++ b/e2e/stack/ci-summary.mjs @@ -0,0 +1,106 @@ +/** + * Turn Playwright's JSON report into a GitHub step summary. + * + * The important column is FLAKY, not passed. While the suite is advisory we are + * trying to learn whether it can be trusted, and a spec that only passes on retry + * is the thing that tells us it cannot yet — a plain "all green" hides exactly the + * signal we are collecting. + * + * Never exits non-zero: the test step already decided pass/fail, and a broken + * summary must not turn a green run red. + */ +import { readFileSync } from 'fs'; + +const REPORT = 'reports/e2e/workplace-results.json'; + +const line = (s = '') => process.stdout.write(`${s}\n`); + +let report; +try { + report = JSON.parse(readFileSync(REPORT, 'utf8')); +} catch (e) { + line('## workplace e2e'); + line(); + line(`No JSON report at \`${REPORT}\` — the run probably failed before Playwright started.`); + line(); + line(`> ${e.message}`); + process.exit(0); +} + +/** Playwright nests suites arbitrarily deep; flatten to specs. */ +function* specsOf(suite) { + for (const spec of suite.specs ?? []) yield spec; + for (const child of suite.suites ?? []) yield* specsOf(child); +} + +const specs = []; +for (const suite of report.suites ?? []) specs.push(...specsOf(suite)); + +const rows = specs.map((spec) => { + const tests = spec.tests ?? []; + // A spec is flaky when Playwright says so, or when it needed more than one + // attempt to end up passing. + const flaky = tests.some( + (t) => t.status === 'flaky' || (t.results?.length ?? 0) > 1 && t.status === 'expected', + ); + const failed = tests.some((t) => t.status === 'unexpected'); + const skipped = tests.every((t) => t.status === 'skipped'); + const attempts = Math.max(1, ...tests.map((t) => t.results?.length ?? 1)); + return { + title: spec.title, + file: spec.file, + state: failed ? 'failed' : skipped ? 'skipped' : flaky ? 'flaky' : 'passed', + attempts, + }; +}); + +const count = (s) => rows.filter((r) => r.state === s).length; +const failed = count('failed'); +const flaky = count('flaky'); + +line('## workplace e2e — advisory'); +line(); +line('| result | count |'); +line('|---|---|'); +line(`| passed | ${count('passed')} |`); +line(`| flaky (passed on retry) | ${flaky} |`); +line(`| failed | ${failed} |`); +line(`| skipped | ${count('skipped')} |`); +line(); +line(`Duration: ${((report.stats?.duration ?? 0) / 1000).toFixed(1)}s`); +line(); + +if (flaky) { + line('### Flaky'); + line(); + line('Passed, but not first time — these are what stop the suite being trusted yet.'); + line(); + for (const r of rows.filter((r) => r.state === 'flaky')) { + line(`- \`${r.file}\` — ${r.title} (${r.attempts} attempts)`); + } + line(); + line( + 'Known cause: staff-api 500s with `DB::ConnectionLost` on concurrent ' + + '`POST /bookings` (E2E_USER_STORIES.md → REG-09). If a flake here is *not* that, ' + + 'it needs investigating before this suite is made a required check.', + ); + line(); +} + +if (failed) { + line('### Failed'); + line(); + for (const r of rows.filter((r) => r.state === 'failed')) { + line(`- \`${r.file}\` — ${r.title}`); + } + line(); + line('Traces, videos and backend logs are attached as run artifacts.'); + line(); +} + +line('---'); +line(); +line( + '**This check is advisory and does not gate merges.** It is here to build a track ' + + 'record. See `E2E_USER_STORIES.md` for what is covered and what is knowingly unstable.', +); diff --git a/e2e/stack/docker-compose.yml b/e2e/stack/docker-compose.yml new file mode 100644 index 0000000000..f95ec712cc --- /dev/null +++ b/e2e/stack/docker-compose.yml @@ -0,0 +1,255 @@ +# Isolated PlaceOS stack for e2e. +# +# Deliberately NOT the PlaceOS/local stack. Two reasons: +# +# 1. Isolation. PlaceOS/local hardcodes `container_name` on every service and +# pins its network to a fixed name + subnet, so a second copy of it collides +# with a running one. Everything here is project-scoped (no container_name, +# no fixed network name), so it can run alongside a developer's own stack +# without touching it. +# 2. It is what CI needs. This is a trimmed stack — only what the workplace e2e +# path actually exercises. Dropped from PlaceOS/local: core, edge, triggers, +# dispatch, source, frontend-loader, influx, chronograf, mosquitto, minio and +# the loki/promtail/grafana profile. That is roughly half the containers and +# most of the RAM. +# +# frontend-loader IS required, despite the SPA being served by the Angular dev +# server. It does not only deploy app builds: on startup it clones +# PlaceOS/www-core into the shared `www` volume, and that is where the platform's +# own /login page, /scripts and /styles come from. Without it nginx serves a bare +# 404 at /login and the real-login spec dead-ends. (Consequence for CI: bringing +# this stack up needs network access to GitHub once, to populate the volume.) +# +# Bring it up with ./up.sh — it generates secrets and seeds before returning. + +volumes: + postgres-data: + elastic-data: + redis-data: + nginx-data: + www: + +x-deployment-env: &deployment-env + PLACE_LOG_FORMAT: JSON + LOG_LEVEL: ${LOG_LEVEL:-warn} + ENV: development + SG_ENV: development + TZ: ${TZ:-UTC} + +x-postgresdb-client-env: &postgresdb-client-env + PG_HOST: postgres + PG_PORT: 5432 + PG_DB: placeos + PG_USER: placeos + PG_PASSWORD: development + PG_DATABASE: placeos + PG_DATABASE_URL: postgresql://placeos:development@postgres:5432/placeos + +x-elastic-client-env: &elastic-client-env + ELASTIC_HOST: elastic + ELASTIC_PORT: 9200 + ES_HOST: elastic + ES_PORT: 9200 + +x-redis-client-env: &redis-client-env + REDIS_URL: redis://redis:6379 + +x-search-ingest-client-env: &search-ingest-client-env + PLACE_SEARCH_INGEST_URI: http://search-ingest:3000 + +x-logging: &std-logging + logging: + driver: json-file + options: + max-size: 25m + +services: + postgres: + image: postgres:18-alpine # 18: migrations use uuidv7(), a PG18 builtin + hostname: postgres + restart: unless-stopped + <<: *std-logging + healthcheck: + test: ['CMD-SHELL', 'pg_isready -U placeos -d placeos'] + interval: 5s + timeout: 5s + retries: 20 + volumes: + - postgres-data:/var/lib/postgresql/data + environment: + <<: *postgresdb-client-env + PGDATA: /var/lib/postgresql/data/pgdata + POSTGRES_USER: placeos + POSTGRES_PASSWORD: development + POSTGRES_DB: placeos + TZ: ${TZ:-UTC} + + elastic: + image: elasticsearch:7.17.6 + hostname: elastic + restart: always + <<: *std-logging + healthcheck: + test: curl --silent --fail localhost:9200/_cat/health + interval: 10s + start_period: 60s + retries: 20 + volumes: + - elastic-data:/usr/share/elasticsearch/data + environment: + bootstrap.memory_lock: 'true' + cluster.routing.allocation.disk.threshold_enabled: 'false' + discovery.type: single-node + ES_JAVA_OPTS: -Xms512m -Xmx512m + TZ: ${TZ:-UTC} + + redis: + image: eqalpha/keydb + hostname: redis + restart: always + <<: *std-logging + healthcheck: + test: keydb-cli ping + volumes: + - redis-data:/data + environment: + TZ: ${TZ:-UTC} + + search-ingest: + image: placeos/search-ingest:${PLACEOS_TAG:-latest} + hostname: search-ingest + restart: always + <<: *std-logging + depends_on: + elastic: { condition: service_healthy } + postgres: { condition: service_healthy } + environment: + <<: [*deployment-env, *postgresdb-client-env, *elastic-client-env] + + # Populates the shared `www` volume from PlaceOS/www-core — the source of the + # platform /login page that the real-login spec drives. + frontend-loader: + image: placeos/frontend-loader:${PLACEOS_TAG:-latest} + hostname: frontend-loader + restart: always + <<: *std-logging + depends_on: + postgres: { condition: service_healthy } + volumes: + - www:/app/www + env_file: + - .secrets/.env.secret_key + environment: + <<: [*deployment-env, *postgresdb-client-env] + PLACE_LOADER_WWW: www + + auth: + # Overridable so the suite can be run against a specific auth build — the + # image path is fixed because nginx/rest-api resolve it by hostname only. + image: ${E2E_AUTH_IMAGE:-placeos/auth:${PLACEOS_TAG:-latest}} + hostname: auth + restart: always + <<: *std-logging + depends_on: + postgres: { condition: service_healthy } + redis: { condition: service_started } + env_file: + - .secrets/.env.secret_key + environment: + <<: [*postgresdb-client-env, *redis-client-env] + COAUTH_NO_SSL: 'true' + TZ: ${TZ:-UTC} + PLACE_URI: https://${E2E_DOMAIN:-localhost:9443} + + rest-api: + image: placeos/rest-api:${PLACEOS_TAG:-latest} + hostname: api # load-bearing: nginx upstream is `api` + restart: always + <<: *std-logging + depends_on: + postgres: { condition: service_healthy } + elastic: { condition: service_healthy } + redis: { condition: service_started } + env_file: + - .secrets/.env.public_key + - .secrets/.env.secret_key + environment: + <<: + [ + *deployment-env, + *postgresdb-client-env, + *elastic-client-env, + *redis-client-env, + *search-ingest-client-env, + ] + + staff-api: + image: placeos/staff-api:${PLACEOS_TAG:-latest} + hostname: staff # load-bearing: nginx upstream is `staff` + restart: unless-stopped + <<: *std-logging + depends_on: + postgres: { condition: service_healthy } + env_file: + - .secrets/.env.public_key + - .secrets/.env.secret_key + environment: + <<: [*postgresdb-client-env, *redis-client-env] + SG_ENV: production + STAFF_TIME_ZONE: ${TZ:-UTC} + PLACE_URI: 'https://nginx' + SSL_VERIFY_NONE: 'true' + # NOTE: under concurrent `POST /bookings` staff-api occasionally raises + # `DB::ConnectionLost` (a 500). Reproduced roughly 1 run in 8 with 4 + # workers, on stock connection settings — which PlaceOS/local also ships. + # Left at the defaults deliberately: tuning the pool here was tried and did + # not demonstrably help, and hiding a real backend behaviour behind rig + # config would be worse than knowing about it. The specs sweep and retry + # instead. Worth investigating in staff-api on its own merits. + + nginx: + image: placeos/nginx:${PLACEOS_TAG:-latest} + hostname: nginx + restart: always + <<: *std-logging + # Shifted off 8080/8443 so this can run beside a developer's own stack. + ports: + - '${E2E_HTTP_PORT:-9080}:80' + - '${E2E_HTTPS_PORT:-9443}:443' + healthcheck: + test: "bash -c ': &>/dev/null /dev/null; then + printf 'PLACE_EMAIL=%s\nPLACE_PASSWORD=%s\n' \ + "${E2E_ADMIN_EMAIL:-support@place.tech}" \ + "${E2E_ADMIN_PASSWORD:-development}" > .secrets/.env + docker run --rm -w /tmp/secrets -v "$PWD/.secrets:/tmp/secrets" \ + -e PLACE_EMAIL="${E2E_ADMIN_EMAIL:-support@place.tech}" \ + -e PLACE_PASSWORD="${E2E_ADMIN_PASSWORD:-development}" \ + "placeos/init:${PLACEOS_TAG:-latest}" generate-secrets >/dev/null + # SECRET_KEY_BASE must be exactly 30 chars — nginx's lua HMAC and auth.cr + # both key off it, and they disagree with anything longer. + awk -F'=' '{ if ($1 == "SECRET_KEY_BASE") print $1"="substr($2,1,30); else print }' \ + .secrets/.env.secret_key > .secrets/.tmp && mv .secrets/.tmp .secrets/.env.secret_key + echo " generated" +else + echo " present (not rotated)" +fi + +# --------------------------------------------------------------------------- +step "starting services" +dc up -d --wait postgres elastic redis +dc up -d search-ingest frontend-loader auth rest-api staff-api nginx + +step "waiting for the API" +for i in $(seq 1 60); do + code=$(curl -sk -o /dev/null -w '%{http_code}' "https://localhost:${HTTPS_PORT}/api/engine/v2/" || true) + [[ -n "$code" && "$code" != "000" ]] && break + sleep 2 +done +[[ -n "${code:-}" && "$code" != "000" ]] || { echo "backend never answered on :${HTTPS_PORT}"; exit 1; } +echo " https://localhost:${HTTPS_PORT} -> HTTP $code" + +# --------------------------------------------------------------------------- +# frontend-loader clones PlaceOS/www-core into the shared volume on startup. +# Until it lands, nginx serves a bare 404 at /login and the real-login spec +# dead-ends on a browser error page rather than a form. +step "waiting for the platform login page (frontend-loader -> www volume)" +for i in $(seq 1 60); do + lcode=$(curl -sk -o /dev/null -w '%{http_code}' "https://localhost:${HTTPS_PORT}/login" || true) + [[ "$lcode" == "200" ]] && break + sleep 2 +done +[[ "${lcode:-}" == "200" ]] || { echo "/login never became available (got ${lcode:-none}); check: docker compose -p ${PROJECT} logs frontend-loader"; exit 1; } +echo " /login -> 200" + +# `init start` creates the authority, admin user, backoffice OAuth app and the +# placeholder zone hierarchy (org -> building -> level) that workplace needs to +# avoid /misconfigured. +step "seeding platform entities (init)" +dc run --rm init start + +step "seeding e2e fixtures" +cd ../.. +E2E_BACKEND_URL="https://localhost:${HTTPS_PORT}" bunx tsx e2e/support/seed.ts + +printf '\n\033[32mstack ready\033[0m backend=https://localhost:%s project=%s\n' "$HTTPS_PORT" "$PROJECT" +printf 'run the suite with:\n E2E_BACKEND_URL=https://localhost:%s bunx playwright test --config apps/workplace/playwright.config.ts\n' "$HTTPS_PORT" diff --git a/e2e/support/api.ts b/e2e/support/api.ts new file mode 100644 index 0000000000..28666f8ff3 --- /dev/null +++ b/e2e/support/api.ts @@ -0,0 +1,172 @@ +/** + * Backend helpers for deterministic setup/teardown and for asserting state + * directly rather than only through UI text. Specs should create and clean up + * their own data through these. + */ +import * as fs from 'fs'; +import { request as pwRequest, APIRequestContext } from '@playwright/test'; +import { BACKEND_URL, RoleName, roleFor } from './env'; + +export const ENGINE_API = '/api/engine/v2'; +export const STAFF_API = '/api/staff/v1'; + +/** Read the raw bearer a worker's auth fixture wrote for a role. */ +export function readToken(role: RoleName, workerIndex = 0): string { + const { tokenPath } = roleFor(role, workerIndex); + if (!fs.existsSync(tokenPath)) { + throw new Error( + `No auth token for "${role}" (worker ${workerIndex}) at ${tokenPath}. ` + + `Use the fixtures from e2e/support/fixtures.ts — they mint per worker.`, + ); + } + const { accessToken } = JSON.parse(fs.readFileSync(tokenPath, 'utf8')); + if (!accessToken) throw new Error(`Token file ${tokenPath} has no accessToken.`); + return accessToken; +} + +/** An APIRequestContext authenticated as the given role, pointed at the backend. */ +export async function apiFor(role: RoleName, workerIndex = 0): Promise { + return pwRequest.newContext({ + baseURL: BACKEND_URL, + ignoreHTTPSErrors: true, + extraHTTPHeaders: { Authorization: `Bearer ${readToken(role, workerIndex)}` }, + }); +} + +export interface CurrentUser { + id: string; + email: string; + name?: string; + sys_admin?: boolean; + support?: boolean; + [k: string]: unknown; +} + +export async function currentUser(api: APIRequestContext): Promise { + const res = await api.get(`${ENGINE_API}/users/current`); + if (!res.ok()) { + throw new Error(`GET /users/current failed: HTTP ${res.status()} ${await res.text()}`); + } + return res.json(); +} + +export interface Zone { + id: string; + name: string; + tags: string[]; + parent_id?: string; + [k: string]: unknown; +} + +/** Zones by tag — the org hierarchy workplace's OrganisationService walks. */ +export async function zonesWithTag(api: APIRequestContext, tag: string): Promise { + const res = await api.get(`${ENGINE_API}/zones`, { params: { tags: tag, limit: 500 } }); + if (!res.ok()) { + throw new Error(`GET /zones?tags=${tag} failed: HTTP ${res.status()} ${await res.text()}`); + } + const body = await res.json(); + return Array.isArray(body) ? body : (body.results ?? []); +} + +export interface Booking { + id: number; + booking_type: string; + asset_id: string; + booking_start: number; + booking_end: number; + title: string; + user_email: string; + approved: boolean; + rejected: boolean; + deleted?: boolean; + zones: string[]; + [k: string]: unknown; +} + +/** + * Bookings of a type in a window. + * + * The query parameter is `type`, NOT `booking_type` — the latter is the model + * field name and the controller's internal PARAMS list, which makes the source + * misleading. staff-api rejects the request with a 422 "missing required + * parameter 'type'" otherwise. Both the window and the type are mandatory; there + * is no "list everything" form. + */ +export async function listBookings( + api: APIRequestContext, + type: string, + from: number, + to: number, +): Promise { + const res = await api.get(`${STAFF_API}/bookings`, { + params: { + type, + period_start: String(from), + period_end: String(to), + include_checked_out: 'true', + }, + }); + if (!res.ok()) { + throw new Error(`GET /bookings failed: HTTP ${res.status()} ${await res.text()}`); + } + const body = await res.json(); + return Array.isArray(body) ? body : (body.results ?? []); +} + +export async function getBooking(api: APIRequestContext, id: number): Promise { + const res = await api.get(`${STAFF_API}/bookings/${id}`); + if (!res.ok()) { + throw new Error(`GET /bookings/${id} failed: HTTP ${res.status()} ${await res.text()}`); + } + return res.json(); +} + +/** Best-effort cleanup — never throws, so teardown cannot fail a passing test. */ +export async function deleteBooking(api: APIRequestContext, id: number): Promise { + try { + await api.delete(`${STAFF_API}/bookings/${id}`); + } catch { + /* swallow: teardown must not mask the actual result */ + } +} + +/** + * Free an asset before using it: delete every live booking against `asset_id` in + * the window. + * + * A post-test `finally` sweep is not enough on its own. If a run dies between + * creating a booking and cleaning it up — a crash, a timeout, someone hitting + * ctrl-c — the leftover holds the asset, and for an all-day booking that blocks + * it for the rest of the day. Every subsequent run then fails with something that + * looks nothing like the cause ("desk not offered", a 422 with no failures + * listed). Sweeping FIRST makes a spec recover from its own past failures instead + * of inheriting them. + */ +export async function releaseAsset( + api: APIRequestContext, + type: string, + asset_id: string, + from: number, + to: number, +): Promise { + let removed = 0; + try { + const existing = await listBookings(api, type, from, to); + for (const b of existing) { + if (b.asset_id === asset_id && !b.deleted) { + await deleteBooking(api, b.id); + removed++; + } + } + } catch { + /* a sweep that cannot run must not fail the test it is protecting */ + } + return removed; +} + +/** A title guaranteed unique per run so specs never collide on shared state. */ +export function uniqueTitle(prefix = 'E2E'): string { + const stamp = new Date().toISOString().replace(/[:.]/g, '-'); + const rand = Math.random().toString(36).slice(2, 7); + return `${prefix} ${stamp} ${rand}`; +} diff --git a/e2e/support/auth.ts b/e2e/support/auth.ts new file mode 100644 index 0000000000..5e9d48fbc6 --- /dev/null +++ b/e2e/support/auth.ts @@ -0,0 +1,205 @@ +/** + * Headless authentication for the PlaceOS e2e suite. + * + * Replays the OAuth2 authorization-code + PKCE flow that @placeos/ts-client + * performs in the browser, using a Playwright APIRequestContext (its own cookie + * jar), then materialises the result as a Playwright `storageState` so the SPA + * boots already authenticated. No login form is driven, no SSO round-trip. + * + * Why a synthetic storageState rather than clicking through /login: + * - ts-client stores its bearer in `localStorage[${cid}_access_token]` / + * `_expires_at`, where `cid = Md5(redirect_uri)` + * (ts-client/src/auth/functions.ts:306, :175-177). + * - workplace leaves `storage` unset, so ts-client resolves it to localStorage + * (functions.ts:303) — and Playwright's storageState DOES persist localStorage. + * Seeding those two keys is all the SPA needs for `hasToken()` to be true, so + * it never bounces to the login redirect. + * - The `_expires_at` we write must be in the FUTURE or ts-client treats the + * token as stale and triggers a re-auth redirect mid-test. + */ +import { createHash, randomBytes } from 'crypto'; +import { request as pwRequest, APIRequestContext } from '@playwright/test'; + +/** + * client_id as computed by ts-client: Md5(redirect_uri), 32 hex chars. + * `Md5.hashStr(_options.redirect_uri, false)` — ts-client/src/auth/functions.ts:306. + * The same value is what `init` stores as the application's `uid` + * (`Digest::MD5.hexdigest(redirect_uri)` — init/src/tasks/entities.cr:118), so an + * application row MUST exist for this exact redirect_uri or authorize will refuse. + */ +export function clientId(redirectUri: string): string { + return createHash('md5').update(redirectUri).digest('hex').slice(0, 32); +} + +/** + * The redirect_uri ts-client derives at runtime: + * `${location.origin}${route}oauth-resp.html`, route = (pathname + '/').replace('//','/') + * (libs/common/src/lib/placeos.ts, `setupPlace`). For an app served at the root of + * a dev server that is `${origin}/oauth-resp.html`. + */ +export function redirectUriFor(appUrl: string): string { + const { origin, pathname } = new URL(appUrl); + const route = (pathname + '/').replace('//', '/'); + return `${origin}${route}oauth-resp.html`; +} + +function base64url(buf: Buffer): string { + return buf.toString('base64').replace(/=/g, '').replace(/\+/g, '-').replace(/\//g, '_'); +} + +export interface MintResult { + accessToken: string; + refreshToken?: string; + expiresIn: number; + clientId: string; + redirectUri: string; + /** Cookies the backend set during signin, in Playwright storageState shape. */ + cookies: StorageStateCookie[]; +} + +export interface StorageStateCookie { + name: string; + value: string; + domain: string; + path: string; + expires: number; + httpOnly: boolean; + secure: boolean; + sameSite: 'Strict' | 'Lax' | 'None'; +} + +/** + * Run the 3-step PKCE flow against the local stack: + * POST /auth/signin -> 202 + session cookie (+ the `verified` cookie) + * GET /auth/oauth/authorize -> 302 with ?code= (do NOT follow the redirect) + * POST /auth/oauth/token -> access_token + */ +export async function mintToken( + backendUrl: string, + appUrl: string, + email: string, + password: string, +): Promise { + if (!password) { + throw new Error( + `Missing password for ${email}. Set the relevant E2E_*_PASSWORD env var ` + + `(see e2e/.env.example).`, + ); + } + const redirectUri = redirectUriFor(appUrl); + const cid = clientId(redirectUri); + const verifier = base64url(randomBytes(32)).slice(0, 43); + const challenge = base64url(createHash('sha256').update(verifier).digest()); + + const ctx: APIRequestContext = await pwRequest.newContext({ + baseURL: backendUrl, + ignoreHTTPSErrors: true, // the local stack uses a self-signed cert + }); + try { + // Step 1 — local password sign-in. auth.cr accepts JSON or form-encoded + // (auth.cr/src/placeos-auth/controllers/sessions.cr:17-22); JSON is used + // here because it is unambiguous. + const signin = await ctx.post('/auth/signin', { + data: { email, password }, + headers: { 'content-type': 'application/json' }, + }); + if (signin.status() !== 202) { + throw new Error( + `signin failed for ${email}: HTTP ${signin.status()} ${await safeText(signin)}\n` + + `(is the local stack up, and does this user have a password set?)`, + ); + } + + // Step 2 — authorization code. Read it out of Location; don't follow. + const authorize = await ctx.get('/auth/oauth/authorize', { + params: { + response_type: 'code', + client_id: cid, + redirect_uri: redirectUri, + scope: 'public', + state: 'e2e', + code_challenge: challenge, + code_challenge_method: 'S256', + }, + maxRedirects: 0, + }); + const location = authorize.headers()['location']; + if (!location || !location.includes('code=')) { + throw new Error( + `authorize did not return a code for ${email}: HTTP ${authorize.status()} ` + + `location=${location}\n` + + `(is there an OAuth application registered for redirect_uri ` + + `"${redirectUri}" (client_id ${cid})? See e2e/README.md.)`, + ); + } + const code = new URL(location, backendUrl).searchParams.get('code'); + if (!code) throw new Error(`could not parse code from location: ${location}`); + + // Step 3 — exchange the code. Public client: a verifier, never a secret. + const tokenRes = await ctx.post('/auth/oauth/token', { + form: { + grant_type: 'authorization_code', + client_id: cid, + redirect_uri: redirectUri, + code, + code_verifier: verifier, + }, + }); + if (!tokenRes.ok()) { + throw new Error( + `token exchange failed for ${email}: HTTP ${tokenRes.status()} ${await safeText(tokenRes)}`, + ); + } + const body = (await tokenRes.json()) as { + access_token?: string; + refresh_token?: string; + expires_in?: number; + }; + if (!body.access_token) { + throw new Error(`no access_token returned for ${email}: ${JSON.stringify(body)}`); + } + + // Carry the backend's cookies across. Not needed when the SPA is served + // by the dev server, but nginx gates static assets behind an HMAC + // `verified` cookie (nginx/config/nginx.conf.template, access_by_lua_block), + // so a suite pointed at a stack-served app would 302 to /auth/login + // without them. + const state = await ctx.storageState(); + return { + accessToken: body.access_token, + refreshToken: body.refresh_token, + expiresIn: body.expires_in ?? 3600, + clientId: cid, + redirectUri, + cookies: (state.cookies ?? []) as StorageStateCookie[], + }; + } finally { + await ctx.dispose(); + } +} + +/** + * Build a Playwright storageState that seeds ts-client's localStorage token keys + * on the APP origin (which is where the SPA runs, and is NOT necessarily the + * backend origin — under `nx serve` they differ). + */ +export function buildStorageState(mint: MintResult, appUrl: string) { + const origin = new URL(appUrl).origin; + const expiresAt = Date.now() + mint.expiresIn * 1000; + const localStorage = [ + { name: `${mint.clientId}_access_token`, value: mint.accessToken }, + { name: `${mint.clientId}_expires_at`, value: String(expiresAt) }, + ]; + if (mint.refreshToken) { + localStorage.push({ name: `${mint.clientId}_refresh_token`, value: mint.refreshToken }); + } + return { cookies: mint.cookies, origins: [{ origin, localStorage }] }; +} + +async function safeText(res: { text(): Promise }): Promise { + try { + return (await res.text()).slice(0, 300); + } catch { + return ''; + } +} diff --git a/e2e/support/env.ts b/e2e/support/env.ts new file mode 100644 index 0000000000..401b2228f6 --- /dev/null +++ b/e2e/support/env.ts @@ -0,0 +1,127 @@ +/** + * Environment + role configuration for the PlaceOS e2e suite. + * + * THIS SUITE RUNS AGAINST A LOCAL BACKEND ONLY. There is no "which env am I on + * today" mode: the guard below is an ALLOWLIST, and anything that is not a + * loopback host is refused before a browser is ever launched. A suite that + * creates, mutates and deletes data must never be one typo away from doing it to + * a real deployment. + * + * Two distinct origins, which are NOT the same thing under `nx serve`: + * BACKEND_URL — the PlaceOS stack (nginx TLS entrypoint), where /api + /auth live. + * APP_URL — where the SPA under test is served. The dev server proxies + * /api,/auth,/control from APP_URL through to BACKEND_URL, so the + * browser only ever sees APP_URL. localStorage (and therefore the + * token) belongs to APP_URL. + */ + +// Must be first: loads .env into process.env before any var below is read. +import './load-env'; +import * as path from 'path'; + +/** + * Auth artifacts are resolved against this file, not the cwd — Nx, Playwright + * and a bare `npx playwright test` all disagree about what the working directory + * is, and a relative path silently writes the storageState somewhere the test + * projects don't read from. + */ +export const AUTH_DIR = path.resolve(__dirname, '..', '.auth'); + +/** The only hosts this suite may ever touch. */ +const LOCAL_HOSTS = ['localhost', '127.0.0.1', '::1', '[::1]', 'host.docker.internal']; + +/** The PlaceOS stack under test. */ +export const BACKEND_URL = process.env.E2E_BACKEND_URL ?? 'https://localhost:8443'; + +/** Where the SPA is served (the Angular dev server). */ +export const APP_URL = process.env.E2E_APP_URL ?? 'http://localhost:4214'; + +/** + * How many parallel workers the suite is provisioned for. Each worker gets its + * own non-admin identity so that state-mutating specs cannot collide — we own + * the whole stack, so isolation is a seeding problem, not a reason to serialise. + * Must match (or exceed) the config's `workers`. + */ +export const WORKERS = Number(process.env.E2E_WORKERS ?? 4); + +/** + * Hard guard: refuse anything that is not a local backend, however we got + * pointed there. Called at config module scope so it throws before Playwright + * launches anything. + */ +export function assertLocalOnly(...urls: string[]): void { + for (const url of urls) { + let hostname: string; + try { + hostname = new URL(url).hostname; + } catch { + throw new Error(`E2E refused: invalid URL "${url}".`); + } + if (!LOCAL_HOSTS.includes(hostname)) { + throw new Error( + `E2E refused: "${url}" is not a local backend (host "${hostname}").\n` + + `This suite must never target a deployed environment — it creates and ` + + `deletes real data. Set E2E_BACKEND_URL / E2E_APP_URL to a loopback host ` + + `and run the local stack (cd local && ./placeos start).`, + ); + } + } +} + +export type RoleName = 'admin' | 'staff'; + +export interface Role { + name: RoleName; + email: string; + password: string; + /** storageState file for this role on this worker. */ + storagePath: string; + /** sidecar file holding the raw bearer for API setup/teardown. */ + tokenPath: string; +} + +const ADMIN_EMAIL = process.env.E2E_ADMIN_EMAIL ?? 'support@place.tech'; +const ADMIN_PASSWORD = process.env.E2E_ADMIN_PASSWORD ?? 'development'; +const STAFF_PASSWORD = process.env.E2E_STAFF_PASSWORD ?? 'e2e-staff-development'; + +/** Per-worker non-admin address. Seeded by seed.ts for 0..WORKERS-1. */ +export function staffEmail(workerIndex: number): string { + return `e2e-staff-${workerIndex}@place.tech`; +} + +/** Desk asset-id prefix. seed.ts creates `${DESK_PREFIX}0..WORKERS`. */ +export const DESK_PREFIX = 'e2e-desk-'; + +/** + * The desk this worker owns. Desks are exclusive for a time range, so giving + * each worker its own removes the only real contention in a parallel run. + */ +export function deskFor(workerIndex: number): { id: string; name: string } { + return { id: `${DESK_PREFIX}${workerIndex}`, name: `E2E Desk ${workerIndex}` }; +} + +/** + * Resolve a role for a given parallel worker. + * + * `admin` is a single shared account — its specs are read-mostly and independent + * bearers for the same user do not interfere. `staff` is one distinct user per + * worker, because those are the specs that create and delete bookings. + */ +export function roleFor(name: RoleName, workerIndex = 0): Role { + if (name === 'admin') { + return { + name, + email: ADMIN_EMAIL, + password: ADMIN_PASSWORD, + storagePath: path.join(AUTH_DIR, `admin-${workerIndex}.json`), + tokenPath: path.join(AUTH_DIR, `admin-${workerIndex}.token.json`), + }; + } + return { + name, + email: staffEmail(workerIndex), + password: STAFF_PASSWORD, + storagePath: path.join(AUTH_DIR, `staff-${workerIndex}.json`), + tokenPath: path.join(AUTH_DIR, `staff-${workerIndex}.token.json`), + }; +} diff --git a/e2e/support/fixtures.ts b/e2e/support/fixtures.ts new file mode 100644 index 0000000000..1d45ec1e91 --- /dev/null +++ b/e2e/support/fixtures.ts @@ -0,0 +1,137 @@ +/** + * Worker-scoped auth fixtures. + * + * Each parallel worker mints its own bearer once and reuses it for every test it + * runs. That is what makes parallelism safe here: we own the whole stack, so + * isolation is a seeding problem (one non-admin identity per worker), not a + * reason to serialise the suite. + * + * Import `test`/`expect` from this module rather than from @playwright/test. + */ +import { test as base, expect, APIRequestContext, Page, request as pwRequest } from '@playwright/test'; +import * as fs from 'fs'; +import * as path from 'path'; +import { + APP_URL, + AUTH_DIR, + BACKEND_URL, + RoleName, + assertLocalOnly, + roleFor, +} from './env'; +import { mintToken, buildStorageState } from './auth'; +import { ENGINE_API } from './api'; + +interface WorkerFixtures { + /** storageState path for the admin identity of THIS worker. */ + adminStorageState: string; + /** Raw bearer for the per-worker non-admin identity. */ + staffToken: string; +} + +interface TestFixtures { + /** An API context authenticated as this worker's non-admin user. */ + staffApi: APIRequestContext; + /** A browser page authenticated as this worker's non-admin user. */ + staffPage: Page; +} + +/** Mint for a role on a worker, writing storageState + token sidecar. */ +async function mintForWorker(role: RoleName, workerIndex: number) { + assertLocalOnly(BACKEND_URL, APP_URL); + const r = roleFor(role, workerIndex); + const mint = await mintToken(BACKEND_URL, APP_URL, r.email, r.password); + + // A token that parses but is rejected downstream is the failure mode that + // caused a production revert once already — assert it here, once, loudly. + const api = await pwRequest.newContext({ + baseURL: BACKEND_URL, + ignoreHTTPSErrors: true, + extraHTTPHeaders: { Authorization: `Bearer ${mint.accessToken}` }, + }); + try { + const res = await api.get(`${ENGINE_API}/users/current`); + if (!res.ok()) { + throw new Error( + `minted token for ${r.email} was rejected by rest-api: HTTP ${res.status()}`, + ); + } + const user = await res.json(); + if (role === 'staff' && (user.sys_admin || user.support)) { + throw new Error( + `${r.email} is admin/support — permission checks would be bypassed, making ` + + `any gating assertion vacuous. Re-run e2e/support/seed.ts.`, + ); + } + } finally { + await api.dispose(); + } + + fs.mkdirSync(AUTH_DIR, { recursive: true }); + fs.writeFileSync(r.storagePath, JSON.stringify(buildStorageState(mint, APP_URL), null, 2)); + fs.writeFileSync( + r.tokenPath, + JSON.stringify( + { + accessToken: mint.accessToken, + refreshToken: mint.refreshToken, + clientId: mint.clientId, + redirectUri: mint.redirectUri, + expiresIn: mint.expiresIn, + }, + null, + 2, + ), + ); + return { role: r, mint }; +} + +export const test = base.extend({ + // Default every test to the admin identity of its own worker. Specs whose + // subject IS authentication opt out with `test.use({ storageState: undefined })` + // and drive loginViaUI instead. + storageState: ({ adminStorageState }, use) => use(adminStorageState), + + adminStorageState: [ + async ({}, use) => { + const { role } = await mintForWorker('admin', test.info().parallelIndex); + await use(role.storagePath); + }, + { scope: 'worker' }, + ], + + staffToken: [ + async ({}, use) => { + const { mint } = await mintForWorker('staff', test.info().parallelIndex); + await use(mint.accessToken); + }, + { scope: 'worker' }, + ], + + staffApi: async ({ staffToken }, use) => { + const api = await pwRequest.newContext({ + baseURL: BACKEND_URL, + ignoreHTTPSErrors: true, + extraHTTPHeaders: { Authorization: `Bearer ${staffToken}` }, + }); + await use(api); + await api.dispose(); + }, + + // Depends on staffToken purely for its side effect: that worker fixture is + // what mints and writes the staff storageState file this context loads. + staffPage: async ({ browser, staffToken }, use) => { + void staffToken; + const storagePath = path.join(AUTH_DIR, `staff-${test.info().parallelIndex}.json`); + const ctx = await browser.newContext({ + storageState: storagePath, + ignoreHTTPSErrors: true, + baseURL: APP_URL, + }); + const page = await ctx.newPage(); + await use(page); + await ctx.close(); + }, +}); + +export { expect }; diff --git a/e2e/support/flows.ts b/e2e/support/flows.ts new file mode 100644 index 0000000000..ddc4f1faa2 --- /dev/null +++ b/e2e/support/flows.ts @@ -0,0 +1,142 @@ +/** + * Multi-step UI flows shared by specs. + * + * Selector notes, all confirmed against the running app rather than guessed: + * + * - The app DOES carry stable `name` attributes on the load-bearing controls + * (`add-desk`, `select-desk`, `toggle-desk`, `open-desk-confirm`, + * `confirm-desk`). Prefer those over text or role. + * - Do NOT match these by accessible name. Buttons render a Material icon as a + * text ligature inside the button, so "Select Desk" has the accessible name + * "done Select Desk" and "Add Desk" is "search Add Desk". A `getByRole` with + * an anchored name never matches. + * - Form control names are generated with a form index (`ng.form0.title`), so + * match the suffix (`input[name$=".title"]`) rather than the whole thing. + * - The final confirm dialog is NOT a `mat-dialog-container`; it lives directly + * in `.cdk-overlay-container`. Scope to the overlay, not to the dialog. + */ +import { Page, expect } from '@playwright/test'; + +/** + * Force an Angular Material checkbox to unchecked, reading the real state off the + * hidden native input rather than assuming a default. The visible `mat-checkbox` + * is the click target; the `input` is where `checked` actually lives. + */ +async function setCheckbox(page: Page, label: string, want: boolean): Promise { + const box = page.locator(`mat-checkbox:has-text("${label}")`); + if (!(await box.count())) return; + const input = box.first().locator('input[type="checkbox"]'); + const is = await input.isChecked().catch(() => false); + if (is !== want) { + await box.first().click(); + const check = expect(input, `"${label}" should be ${want ? 'checked' : 'unchecked'}`); + await (want ? check.toBeChecked() : check.not.toBeChecked()); + } +} + +export interface CreatedBooking { + id: number; + asset_id: string; + approved: boolean; + title: string; +} + +/** + * Book a desk through the full UI and return the booking the API created. + * + * The booking id comes from the real `POST /api/staff/v1/bookings` response, not + * from anything the UI renders — so the assertion is about what the backend + * actually stored. + */ +export async function bookDeskViaUI( + page: Page, + deskName: string, + title: string, +): Promise { + await page.goto('/#/book/desk/form'); + + const title_input = page.locator('input[name$=".title"]').first(); + await expect(title_input).toBeVisible({ timeout: 30_000 }); + + // "Require locker" defaults to CHECKED, and no lockers are seeded, so leaving + // it on makes the form unsatisfiable — the confirm dialog simply never opens, + // with no error shown. + // + // ENSURE the state; never blindly toggle. A toggle is only correct if you + // already know the starting state, and here it is not guaranteed: toggling a + // checkbox that happened to start unchecked switches the requirement back ON + // and the flow dead-ends in a way that looks like a selector problem. + await setCheckbox(page, 'Require locker', false); + + // Book the whole day rather than the form's default slot. + // + // That default is the next 5-minute boundary, which makes the booking + // implicitly time-sensitive: a run slow enough to cross the boundary ends up + // with a start time in the past, the form silently becomes invalid, and the + // confirm dialog never opens. It reproduced at roughly 1-in-6 and only on the + // slow runs. An all-day booking has no rolling window, so the flake cannot + // occur at all — the same reasoning as pinning fixed times instead of relative + // ones anywhere else in a suite. + await setCheckbox(page, 'All Day', true); + + await page.locator('button[name="add-desk"]').click(); + const desk = page + .locator('button[name="select-desk"]') + .filter({ hasText: deskName }) + .first(); + await expect( + desk, + `desk "${deskName}" should be offered — is it seeded and bookable?`, + ).toBeVisible({ timeout: 20_000 }); + await desk.click(); + + // Enabled only once a desk is selected. + const confirm_selection = page.locator('button[name="toggle-desk"]'); + await expect(confirm_selection).toBeEnabled({ timeout: 10_000 }); + await confirm_selection.click(); + + // Type the title LAST, deliberately. Attaching a desk rebuilds the form and + // resets the title to its default ("Booking"), so anything typed earlier is + // silently discarded — the booking still succeeds, just under the wrong name, + // which reads as a passing test unless you assert the value. Setting it after + // desk selection sidesteps the rebuild entirely. + await title_input.fill(title); + await expect(title_input, 'title survived to submission').toHaveValue(title); + + await page.locator('button[name="open-desk-confirm"]').click(); + + // The final confirm lives directly in the cdk overlay, not in a + // mat-dialog-container. If it never appears the form was rejected silently — + // say so, rather than reporting a bare selector timeout. + const confirm = page.locator('.cdk-overlay-container button[name="confirm-desk"]'); + await expect( + confirm, + 'the confirm dialog did not open — the form was silently invalid ' + + '(check "Require locker", the desk selection, and the date/time fields)', + ).toBeVisible({ timeout: 20_000 }); + + const [response] = await Promise.all([ + page.waitForResponse( + (r) => + r.url().includes('/api/staff/v1/bookings') && r.request().method() === 'POST', + { timeout: 30_000 }, + ), + confirm.click(), + ]); + + // Read the body ONCE, before asserting. Building the failure message with an + // inline `await response.text()` evaluates on every run, including the happy + // path, and Playwright cannot always re-read a body it has already handed + // over — which surfaces as an unrelated "Network.getResponseBody: No data + // found" flake instead of the assertion you wrote. + const status = response.status(); + const body = await response.text().catch(() => ''); + expect(response.ok(), `POST /bookings should succeed, got ${status}: ${body}`).toBeTruthy(); + const created = JSON.parse(body); + return { + id: created.id, + asset_id: created.asset_id, + approved: created.approved, + title: created.title, + }; +} diff --git a/e2e/support/global-setup.ts b/e2e/support/global-setup.ts new file mode 100644 index 0000000000..caf7b959d3 --- /dev/null +++ b/e2e/support/global-setup.ts @@ -0,0 +1,15 @@ +/** + * Runs once before the whole suite: prove the local stack is actually there. + * + * The stack is a precondition, not something Playwright starts — it is ~20 + * containers and Elasticsearch alone wants several GB, so a `webServer` block + * would be the wrong tool. Failing here costs a second and says what to do; + * failing later costs every spec a navigation timeout and says nothing. + */ +import { APP_URL, BACKEND_URL, assertLocalOnly } from './env'; +import { assertStackUp } from './preflight'; + +export default async function globalSetup(): Promise { + assertLocalOnly(BACKEND_URL, APP_URL); + await assertStackUp(); +} diff --git a/e2e/support/index.ts b/e2e/support/index.ts new file mode 100644 index 0000000000..9d18721d1c --- /dev/null +++ b/e2e/support/index.ts @@ -0,0 +1,7 @@ +export * from './env'; +export * from './auth'; +export * from './api'; +export * from './login'; +export * from './flows'; +export * from './preflight'; +export * from './fixtures'; diff --git a/e2e/support/load-env.ts b/e2e/support/load-env.ts new file mode 100644 index 0000000000..07d75971cf --- /dev/null +++ b/e2e/support/load-env.ts @@ -0,0 +1,13 @@ +/** + * Side-effect module: load `e2e/.env` before anything reads process.env. + * Imported FIRST by env.ts so the order holds regardless of who imports what. + * No hard dependency on dotenv — if it is absent, real env vars / CI secrets + * still work. + */ +import * as path from 'path'; + +try { + require('dotenv').config({ path: path.resolve(__dirname, '..', '.env') }); +} catch { + /* dotenv not installed — rely on exported env vars */ +} diff --git a/e2e/support/login.ts b/e2e/support/login.ts new file mode 100644 index 0000000000..e6f3a35bf9 --- /dev/null +++ b/e2e/support/login.ts @@ -0,0 +1,72 @@ +/** + * Driving the REAL login flow. + * + * This is the primary auth path for anything whose subject is authentication. + * Injecting a synthesised token (see auth.ts) is faster, and is the right choice + * for specs that merely need to *be* logged in — but it skips the code exchange, + * the redirect handshake and the token-storage logic entirely, which is exactly + * where the auth regressions have actually happened. + * + * One behaviour worth knowing before you assert on storage: after a normal login + * ts-client persists ONLY `${cid}_expires_at`. The access token stays in memory + * unless the device is marked trusted — `_storeTokenDetails` gates the + * `setItem(access_token)` call on `isTrusted()` (ts-client/src/auth/functions.ts:1042). + * So "the token is in localStorage" is true of a seeded storageState and false of + * a real login. Assert on the token exchange or on `/users/current`, not on the key. + */ +import { Page, expect } from '@playwright/test'; +import { APP_URL, Role } from './env'; + +export interface LoginResult { + /** The token-endpoint response the browser actually received. */ + token: { access_token?: string; refresh_token?: string; scope?: string; expires_in?: number }; +} + +/** + * Land on the app unauthenticated, follow the redirect to the login page, sign + * in, and wait to arrive back in the app. + * + * We wait for the login FORM, not for a URL. Where the login page lives depends + * on the authority's `login_url` column, and that genuinely differs between + * stacks: a freshly-initialised one gets a relative `/login?continue={{url}}` + * (so the dev server proxies it same-origin), while an older or hand-edited + * authority may carry an absolute `https://host:port/login?...` and send the + * browser cross-origin. Both are correct. Asserting on the URL shape couples the + * suite to a config detail that has nothing to do with what is being tested. + */ +export async function loginViaUI(page: Page, role: Role): Promise { + let token: LoginResult['token'] = {}; + page.on('response', async (res) => { + if (res.url().includes('/oauth/token') && res.status() === 200) { + try { + token = await res.json(); + } catch { + /* not JSON — leave token empty and let the caller's assert fail */ + } + } + }); + + await page.goto('/#/'); + + // Unauthenticated: the app must bounce to a login page — wherever it lives. + const email = page.locator('input[type="email"]'); + await expect( + email, + `expected a login form after visiting the app unauthenticated, but none appeared ` + + `(landed on ${page.url()}). Is the authority's login_url set?`, + ).toBeVisible({ timeout: 30_000 }); + + await email.fill(role.email); + await page.locator('input[type="password"]').fill(role.password); + await page.getByRole('button', { name: /log ?in/i }).click(); + + // ...and back into the app, authenticated. + await page.waitForURL(new RegExp(escapeRe(new URL(APP_URL).host)), { timeout: 30_000 }); + await expect(page.locator('topbar')).toBeVisible({ timeout: 30_000 }); + + return { token }; +} + +function escapeRe(s: string): string { + return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} diff --git a/e2e/support/preflight.ts b/e2e/support/preflight.ts new file mode 100644 index 0000000000..4c50f34685 --- /dev/null +++ b/e2e/support/preflight.ts @@ -0,0 +1,33 @@ +/** + * Is the local PlaceOS stack actually up? + * + * The stack is a PRECONDITION, not something Playwright starts — it is ~20 + * containers and Elasticsearch alone wants several GB, so a `webServer` block + * would be the wrong tool. Instead fail in about a second with a message that + * says what to do, rather than after 90s of unexplained navigation timeouts. + */ +import { request as pwRequest } from '@playwright/test'; +import { BACKEND_URL } from './env'; + +export async function assertStackUp(): Promise { + const ctx = await pwRequest.newContext({ + baseURL: BACKEND_URL, + ignoreHTTPSErrors: true, + timeout: 10_000, + }); + try { + const res = await ctx.get('/api/engine/v2/'); + // Any HTTP answer proves nginx + rest-api are reachable; the route itself + // may legitimately 401/404 depending on version. + if (res.status() >= 500) { + throw new Error(`backend returned HTTP ${res.status()}`); + } + } catch (e) { + throw new Error( + `Local PlaceOS stack is not reachable at ${BACKEND_URL} (${(e as Error).message}).\n` + + `Start it with: cd local && ./placeos start`, + ); + } finally { + await ctx.dispose(); + } +} diff --git a/e2e/support/seed.ts b/e2e/support/seed.ts new file mode 100644 index 0000000000..2538f4571e --- /dev/null +++ b/e2e/support/seed.ts @@ -0,0 +1,298 @@ +/** + * Idempotent, API-driven seeding of the local stack. + * + * Everything here goes through the real API rather than SQL: seeding straight + * into Postgres bypasses model validation, callbacks and derived columns (e.g. + * rest-api looks users up by `email_digest = MD5(lower(email))`, not `email`), + * which produces rows that look right and behave wrong. + * + * Bootstrap order matters. Minting a token needs a registered OAuth application, + * but registering one needs a token — so we bootstrap through the `backoffice` + * application, which `init` always creates when the stack is first started + * (init/src/tasks/initialization.cr:38-42), and use that token to register + * everything else. + * + * Safe to re-run: every step is an upsert keyed on a natural identifier. + */ +import { request as pwRequest, APIRequestContext } from '@playwright/test'; +import { + APP_URL, + BACKEND_URL, + DESK_PREFIX, + WORKERS, + assertLocalOnly, + roleFor, + staffEmail, +} from './env'; +import { mintToken, clientId, redirectUriFor } from './auth'; +import { ENGINE_API, STAFF_API } from './api'; + +/** The app `init` guarantees exists — our way in before anything else is registered. */ +const BOOTSTRAP_APP_URL = `${BACKEND_URL}/backoffice`; + +async function adminApi(): Promise { + const admin = roleFor('admin'); + const mint = await mintToken(BACKEND_URL, BOOTSTRAP_APP_URL, admin.email, admin.password); + return pwRequest.newContext({ + baseURL: BACKEND_URL, + ignoreHTTPSErrors: true, + extraHTTPHeaders: { Authorization: `Bearer ${mint.accessToken}` }, + }); +} + +/** Does this error body mean "the row is already there"? */ +function alreadyExists(body: string): boolean { + return /already (exists|taken)|has already been taken|must be unique|should be unique|duplicate/i.test( + body, + ); +} + +async function json(api: APIRequestContext, path: string, params?: Record) { + const res = await api.get(path, { params }); + if (!res.ok()) throw new Error(`GET ${path} failed: HTTP ${res.status()} ${await res.text()}`); + return res.json(); +} + +/** + * The authority (domain) this stack serves. + * + * Polls, because `/domains` is SEARCH-backed: on a cold stack the row exists in + * Postgres the moment `init` finishes, but the API reads it out of Elasticsearch + * and returns an empty list until search-ingest has indexed it. Failing here + * immediately is the single most likely way a CI run breaks, and the error looks + * nothing like the cause. + */ +async function authority(api: APIRequestContext, timeoutMs = 90_000) { + const deadline = Date.now() + timeoutMs; + let last: unknown[] = []; + for (;;) { + const domains = await json(api, `${ENGINE_API}/domains`); + last = Array.isArray(domains) ? domains : (domains.results ?? []); + if (last.length) return last[0] as { id: string }; + if (Date.now() > deadline) { + throw new Error( + `no authority visible via ${ENGINE_API}/domains after ${timeoutMs / 1000}s.\n` + + `The row is created by \`init start\`, but this endpoint is served from ` + + `Elasticsearch — check that search-ingest is running and has built its ` + + `indices (docker compose logs search-ingest).`, + ); + } + await new Promise((r) => setTimeout(r, 2000)); + } +} + +/** + * Register an OAuth application for the SPA's redirect_uri. + * ts-client derives `client_id = Md5(redirect_uri)` at runtime, so authorize + * only succeeds if a row exists whose `uid` is exactly that hash. Under + * `nx serve` the app origin is the dev server, NOT the stack, so this is a + * different application from the deployed one. + */ +async function ensureOAuthApp(api: APIRequestContext, appUrl: string, name: string) { + const redirect_uri = redirectUriFor(appUrl); + const uid = clientId(redirect_uri); + + const existing = await json(api, `${ENGINE_API}/oauth_apps`, { limit: '500' }); + const list = Array.isArray(existing) ? existing : (existing.results ?? []); + if (list.some((a: { uid?: string }) => a.uid === uid)) { + return { uid, redirect_uri, created: false }; + } + + const owner_id = (await authority(api)).id; + const res = await api.post(`${ENGINE_API}/oauth_apps`, { + data: { name, redirect_uri, scopes: 'public', owner_id, confidential: false }, + }); + if (!res.ok()) { + const body = await res.text(); + // The existence check above reads from Elasticsearch, so a seed re-run + // within a second or two of the first can miss a row that Postgres + // already has. A uniqueness rejection means it is there — not an error. + if (alreadyExists(body)) return { uid, redirect_uri, created: false }; + throw new Error(`create oauth_app failed: HTTP ${res.status()} ${body}`); + } + return { uid, redirect_uri, created: true }; +} + +/** + * Make the authority's `login_url` absolute. + * + * A freshly-initialised authority gets a RELATIVE `/login?continue={{url}}`. + * ts-client resolves a relative login_url against the authority's `domain` + * column — which stores a bare host with no port (`localhost`) — and prepends + * `location.protocol`. On any deployment that is not on the default port that + * produces a dead URL: here, `http://localhost/login` → ERR_CONNECTION_REFUSED, + * and the app dead-ends on a browser error page instead of a login form. + * + * A long-lived local stack usually has an absolute value already and so never + * shows this. It only appears on a cold start — which is the whole reason to + * test against one. + */ +async function ensureAbsoluteLoginUrl(api: APIRequestContext) { + const auth = (await authority(api)) as { id: string; login_url?: string }; + const current = auth.login_url ?? ''; + if (/^https?:\/\//i.test(current)) return { login_url: current, changed: false }; + + const login_url = `${BACKEND_URL}/login?continue={{url}}`; + const res = await api.patch(`${ENGINE_API}/domains/${auth.id}`, { data: { login_url } }); + if (!res.ok()) { + throw new Error(`patch domain login_url failed: HTTP ${res.status()} ${await res.text()}`); + } + return { login_url, changed: true }; +} + +/** + * staff-api refuses EVERY /bookings and /events call with + * "domain does not have a tenant configured" until a tenant row exists for the + * request hostname (staff-api/src/controllers/utilities/multi_tenant.cr). + * + * The credentials below are deliberately PLACEHOLDERS. They are only ever + * dereferenced when staff-api instantiates a PlaceCalendar client, which happens + * for the calendar-backed routes (/calendars, /events) and nothing else — so a + * placeholder tenant unblocks the whole PlaceOS-native booking surface (desks, + * lockers, parking, visitors) with ZERO external calls. That is what keeps this + * suite genuinely local. Real calendar credentials are opt-in; see e2e/README.md. + */ +async function ensureTenant(api: APIRequestContext) { + const domain = new URL(BACKEND_URL).hostname; + const existing = await json(api, `${STAFF_API}/tenants`); + const list = Array.isArray(existing) ? existing : (existing.results ?? []); + if (list.some((t: { domain?: string }) => t.domain === domain)) { + return { domain, created: false }; + } + const res = await api.post(`${STAFF_API}/tenants`, { + data: { + name: 'E2E Local', + domain, + platform: 'office365', + credentials: { + tenant: 'e2e-local-placeholder', + client_id: 'e2e-local-placeholder', + client_secret: 'e2e-local-placeholder', + }, + }, + }); + if (!res.ok()) { + throw new Error(`create tenant failed: HTTP ${res.status()} ${await res.text()}`); + } + return { domain, created: true }; +} + +/** + * Bookable desks. + * + * Desks are not systems — they live in Zone METADATA under the name `desks`, on + * a LEVEL zone, as `metadata.desks.details[]` + * (libs/bookings/src/lib/booking-form.service.ts, `listChildMetadata(building, {name:'desks'})`). + * `groups: []` keeps them unrestricted so a non-admin can book them; a populated + * `groups` array would gate them behind group membership. + * + * One desk per worker plus a shared spare, so parallel specs never contend for + * the same asset (a desk is exclusive for a given time range). + */ +async function ensureDesks(api: APIRequestContext) { + const levels = await json(api, `${ENGINE_API}/zones`, { tags: 'level', limit: '100' }); + const level_list = Array.isArray(levels) ? levels : (levels.results ?? []); + const level = level_list.find((z: { parent_id?: string }) => z.parent_id) ?? level_list[0]; + if (!level) { + throw new Error( + 'no `level` zone with a parent — workplace needs org -> building -> level. ' + + 'Did `init start` run?', + ); + } + + const details = Array.from({ length: WORKERS + 1 }, (_, i) => ({ + id: `${DESK_PREFIX}${i}`, + name: `E2E Desk ${i}`, + bookable: true, + groups: [], + features: [], + images: [], + })); + + const res = await api.put(`${ENGINE_API}/metadata/${level.id}`, { + data: { name: 'desks', details, description: 'e2e bookable desks' }, + }); + if (!res.ok()) { + throw new Error(`put desks metadata failed: HTTP ${res.status()} ${await res.text()}`); + } + return { zone: level.id, count: details.length }; +} + +/** + * One genuinely non-admin user PER PARALLEL WORKER. + * + * Two reasons it is per-worker rather than shared. Permission-gated behaviour + * must be asserted as a non-admin, because a sys_admin bypasses the checks and + * the assertion would pass whether or not the logic works. And state-mutating + * specs (create a booking, cancel it, assert "your bookings") need identities + * that cannot see each other's rows — which is what lets the suite run in + * parallel instead of serialising on one shared account. + */ +async function ensureStaffUsers(api: APIRequestContext) { + const authority_id = (await authority(api)).id; + const created: string[] = []; + const present: string[] = []; + + for (let i = 0; i < WORKERS; i++) { + const role = roleFor('staff', i); + const found = await json(api, `${ENGINE_API}/users`, { q: role.email, limit: '50' }); + const list = Array.isArray(found) ? found : (found.results ?? []); + if ( + list.some((u: { email?: string }) => u.email?.toLowerCase() === role.email.toLowerCase()) + ) { + present.push(role.email); + continue; + } + const res = await api.post(`${ENGINE_API}/users`, { + data: { + name: `E2E Staff ${i} (non-admin)`, + email: role.email, + password: role.password, + authority_id, + sys_admin: false, + support: false, + }, + }); + if (!res.ok()) { + const body = await res.text(); + if (alreadyExists(body)) { + present.push(role.email); + continue; + } + throw new Error(`create user ${role.email} failed: HTTP ${res.status()} ${body}`); + } + created.push(role.email); + } + return { created, present }; +} + +export async function seed(): Promise { + assertLocalOnly(BACKEND_URL, APP_URL); + const api = await adminApi(); + try { + const app = await ensureOAuthApp(api, APP_URL, 'workplace (e2e dev server)'); + console.log( + ` oauth app ${app.created ? 'created' : 'present'} ${app.uid} ${app.redirect_uri}`, + ); + const login = await ensureAbsoluteLoginUrl(api); + console.log(` login_url ${login.changed ? 'patched' : 'ok'} ${login.login_url}`); + const tenant = await ensureTenant(api); + console.log(` tenant ${tenant.created ? 'created' : 'present'} domain=${tenant.domain}`); + const desks = await ensureDesks(api); + console.log(` desks ${desks.count} on ${desks.zone}`); + const users = await ensureStaffUsers(api); + console.log( + ` staff users ${users.created.length} created, ${users.present.length} present ` + + `(${WORKERS} workers: ${staffEmail(0)} … ${staffEmail(WORKERS - 1)})`, + ); + } finally { + await api.dispose(); + } +} + +if (require.main === module) { + seed().catch((e) => { + console.error(e.message); + process.exit(1); + }); +} From 18dd1a66f62c84af87eddcbd3735462bf7ca26b3 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 30 Jul 2026 11:53:56 +1000 Subject: [PATCH 02/17] fix(e2e): make the stack start on a CI runner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first GitHub Actions run failed at bring-up with a single line — "container placeos-e2e-elastic-1 is unhealthy" — and no way to see why. - Elasticsearch had `bootstrap.memory_lock: true`, which requires an unlimited memlock rlimit or ES refuses to boot and the container exits. Docker Desktop grants that by default, so it only failed in CI. Turned off (production tuning, worthless to a throwaway test stack) and the ulimits set either way. - up.sh now dumps `compose ps` and logs for every service when bring-up fails, so the step output explains itself instead of needing an artifact. `set -E` is required for that ERR trap to be inherited by shell functions — verified by breaking a service on purpose. - The workflow's log collection enumerated a hand-picked service list that did not include elastic, so the one log that mattered was the one not captured. It now collects every service plus `compose ps`. - Bounded the health wait (`--wait-timeout 300`) so a stuck container fails clearly rather than running to the job timeout. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/e2e-advisory.yml | 10 ++++++++-- e2e/stack/docker-compose.yml | 18 +++++++++++++++++- e2e/stack/up.sh | 27 +++++++++++++++++++++++++-- 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/.github/workflows/e2e-advisory.yml b/.github/workflows/e2e-advisory.yml index d54583cb2e..66a9765fa1 100644 --- a/.github/workflows/e2e-advisory.yml +++ b/.github/workflows/e2e-advisory.yml @@ -109,13 +109,19 @@ jobs: if: ${{ always() }} run: node e2e/stack/ci-summary.mjs >> "$GITHUB_STEP_SUMMARY" + # Collect EVERY service, not a hand-picked list. The first run failed + # in Elasticsearch — which was not on the list — so the log that + # explained the failure was the one log not captured. - name: Collect backend logs if: ${{ failure() }} run: | mkdir -p e2e-logs cd e2e/stack - for svc in auth rest-api staff-api nginx frontend-loader search-ingest postgres; do - docker compose -p placeos-e2e logs "$svc" > "../../e2e-logs/$svc.log" 2>&1 || true + docker compose -p placeos-e2e ps --all > ../../e2e-logs/_ps.txt 2>&1 || true + docker compose -p placeos-e2e logs --no-color > ../../e2e-logs/_all.log 2>&1 || true + for svc in $(docker compose -p placeos-e2e config --services 2>/dev/null); do + docker compose -p placeos-e2e logs --no-color "$svc" \ + > "../../e2e-logs/$svc.log" 2>&1 || true done - name: Upload Playwright report diff --git a/e2e/stack/docker-compose.yml b/e2e/stack/docker-compose.yml index f95ec712cc..4a8416dfc6 100644 --- a/e2e/stack/docker-compose.yml +++ b/e2e/stack/docker-compose.yml @@ -96,8 +96,24 @@ services: retries: 20 volumes: - elastic-data:/usr/share/elasticsearch/data + # `bootstrap.memory_lock` is OFF deliberately. + # + # PlaceOS/local enables it, but it requires an unlimited `memlock` rlimit or + # Elasticsearch refuses to boot ("memory locking requested ... but memory is + # not locked") — the container exits, and `compose --wait` reports it as + # unhealthy within seconds rather than after its retry budget. Docker Desktop + # grants that rlimit by default, so this only bites in CI: it is what failed + # the first GitHub Actions run. Memory locking is production tuning and buys + # a throwaway test stack nothing. The ulimits below make it work either way. + ulimits: + memlock: + soft: -1 + hard: -1 + nofile: + soft: 65536 + hard: 65536 environment: - bootstrap.memory_lock: 'true' + bootstrap.memory_lock: 'false' cluster.routing.allocation.disk.threshold_enabled: 'false' discovery.type: single-node ES_JAVA_OPTS: -Xms512m -Xmx512m diff --git a/e2e/stack/up.sh b/e2e/stack/up.sh index 9dc41f228a..4af8e58c5c 100755 --- a/e2e/stack/up.sh +++ b/e2e/stack/up.sh @@ -8,7 +8,10 @@ # ./up.sh bring up (reuses volumes if present) # ./up.sh --fresh destroy volumes first — a genuine cold start # ./down.sh stop -set -euo pipefail +# -E (errtrace) is load-bearing: without it the ERR trap below is NOT inherited by +# shell functions, so a failure inside `dc()` would exit silently and the +# diagnostics would never run. Verified by breaking a service on purpose. +set -euEo pipefail cd "$(dirname "${BASH_SOURCE[0]}")" @@ -57,8 +60,28 @@ else fi # --------------------------------------------------------------------------- +# Self-diagnose on failure. +# +# `compose --wait` reports only "container X is unhealthy" and exits. On a CI +# runner that is the whole of the evidence unless something dumps more, and the +# first GitHub Actions run failed with exactly that one line — the reason +# (Elasticsearch refusing to boot without a memlock rlimit) was in a container log +# nobody had collected. Print state and logs for EVERY service here, so the step +# output alone explains the failure. +diagnose() { + echo + echo "=== compose ps ===" + dc ps --all || true + echo + echo "=== logs (all services, last 60 lines each) ===" + dc logs --tail 60 || true +} +trap 'rc=$?; [[ $rc -ne 0 ]] && diagnose; exit $rc' ERR + step "starting services" -dc up -d --wait postgres elastic redis +# Bounded so a stuck container fails with a clear message rather than hanging +# until the job timeout. +dc up -d --wait --wait-timeout 300 postgres elastic redis dc up -d search-ingest frontend-loader auth rest-api staff-api nginx step "waiting for the API" From afcd25240833401b4bfa47cac318fda48b80fafc Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 30 Jul 2026 12:00:25 +1000 Subject: [PATCH 03/17] fix(e2e): elasticsearch 7.17.28 for cgroup v2 compatibility 7.17.6 (what PlaceOS/local pins) bundles a JDK with the cgroup v2 NPE bug. Its launcher dies in JvmOptionsParser -> DefaultSystemMemoryInfo with "Cannot invoke CgroupInfo.getMountPoint() because anyController is null" before the JVM starts, so no ES_JAVA_OPTS workaround is possible. GitHub runners use cgroup v2; Docker Desktop's VM does not, which is why this only failed in CI. Staying on the 7.x line keeps client compatibility with rest-api and search-ingest. Verified locally on 7.17.28: search-ingest builds its indices, suite 7/7. Co-Authored-By: Claude Opus 5 (1M context) --- e2e/stack/docker-compose.yml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/e2e/stack/docker-compose.yml b/e2e/stack/docker-compose.yml index 4a8416dfc6..59915a98e1 100644 --- a/e2e/stack/docker-compose.yml +++ b/e2e/stack/docker-compose.yml @@ -85,7 +85,17 @@ services: TZ: ${TZ:-UTC} elastic: - image: elasticsearch:7.17.6 + # A LATE 7.17 patch, not the 7.17.6 that PlaceOS/local pins. + # + # 7.17.6 bundles a JDK with the cgroup v2 NPE bug: its launcher dies in + # `JvmOptionsParser` -> `DefaultSystemMemoryInfo` with "Cannot invoke + # CgroupInfo.getMountPoint() because anyController is null" before the JVM even + # starts, so no ES_JAVA_OPTS setting can work around it. GitHub runners use + # cgroup v2; Docker Desktop's VM does not, which is why this passes locally and + # failed the first two CI runs. Staying on the 7.x line keeps client + # compatibility with rest-api and search-ingest, which is what PlaceOS/local + # exercises. + image: elasticsearch:${E2E_ELASTIC_VERSION:-7.17.28} hostname: elastic restart: always <<: *std-logging From 60deee1de8702b8691e6b3c83d498704f6b701c8 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 30 Jul 2026 12:09:30 +1000 Subject: [PATCH 04/17] fix(e2e): converge on desk-form state instead of assuming a set sticks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first green CI run was green but reported both desk specs as flaky — failing on attempt 1 with "the confirm dialog did not open", passing on retry. The booking form is rebuilt when its async initialisation finishes, and the rebuild restores defaults: title -> "Booking", All Day -> off, Require locker -> on. It is a RACE, not a step. On a warm run it lands before we touch anything; on a cold one it lands mid-flow and silently discards our input. Locally that surfaced as a booking under the wrong title; in CI as an unopenable confirm dialog, because a reverted All Day leaves the default slot, which on a slow run has already passed and makes the form invalid with no visible error. Probing showed the values sometimes survive desk attachment and sometimes do not, so re-ordering cannot fix it. Re-applying inside a retrying block converges whenever the rebuild fires, without depending on an internal ready signal. Note this mitigates a race in the app, it does not fix one. Co-Authored-By: Claude Opus 5 (1M context) --- e2e/support/flows.ts | 65 ++++++++++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 24 deletions(-) diff --git a/e2e/support/flows.ts b/e2e/support/flows.ts index ddc4f1faa2..946e70b46a 100644 --- a/e2e/support/flows.ts +++ b/e2e/support/flows.ts @@ -58,25 +58,23 @@ export async function bookDeskViaUI( const title_input = page.locator('input[name$=".title"]').first(); await expect(title_input).toBeVisible({ timeout: 30_000 }); - // "Require locker" defaults to CHECKED, and no lockers are seeded, so leaving - // it on makes the form unsatisfiable — the confirm dialog simply never opens, - // with no error shown. + // Set the two controls that gate submission BEFORE picking a desk, so the + // availability list is computed against the window we intend to book. They are + // re-affirmed after desk selection, because the form can be rebuilt underneath + // us — see the converging block below. // - // ENSURE the state; never blindly toggle. A toggle is only correct if you - // already know the starting state, and here it is not guaranteed: toggling a - // checkbox that happened to start unchecked switches the requirement back ON - // and the flow dead-ends in a way that looks like a selector problem. - await setCheckbox(page, 'Require locker', false); - - // Book the whole day rather than the form's default slot. + // - "Require locker" defaults to CHECKED and no lockers are seeded, so leaving + // it on makes the form unsatisfiable, with no error shown. + // - "All Day" replaces the default slot, which is the next 5-minute boundary. + // That default makes the booking implicitly time-sensitive: a slow run + // crosses the boundary, the start time falls into the past, and the form + // silently becomes invalid. Same reasoning as pinning fixed times rather + // than relative ones anywhere else in a suite. // - // That default is the next 5-minute boundary, which makes the booking - // implicitly time-sensitive: a run slow enough to cross the boundary ends up - // with a start time in the past, the form silently becomes invalid, and the - // confirm dialog never opens. It reproduced at roughly 1-in-6 and only on the - // slow runs. An all-day booking has no rolling window, so the flake cannot - // occur at all — the same reasoning as pinning fixed times instead of relative - // ones anywhere else in a suite. + // ENSURE state; never blindly toggle. Toggling a checkbox that happened to + // start in the target state inverts the intent, and the flow then dead-ends in + // a way that reads as a selector problem. + await setCheckbox(page, 'Require locker', false); await setCheckbox(page, 'All Day', true); await page.locator('button[name="add-desk"]').click(); @@ -95,13 +93,32 @@ export async function bookDeskViaUI( await expect(confirm_selection).toBeEnabled({ timeout: 10_000 }); await confirm_selection.click(); - // Type the title LAST, deliberately. Attaching a desk rebuilds the form and - // resets the title to its default ("Booking"), so anything typed earlier is - // silently discarded — the booking still succeeds, just under the wrong name, - // which reads as a passing test unless you assert the value. Setting it after - // desk selection sidesteps the rebuild entirely. - await title_input.fill(title); - await expect(title_input, 'title survived to submission').toHaveValue(title); + // Converge on the form state instead of assuming a set sticks. + // + // The form is rebuilt when its async initialisation (org data, settings, + // resource lists) completes, and that rebuild restores defaults — title back + // to "Booking", All Day back off, Require locker back on. Crucially it is a + // RACE, not a step: on a warm run it lands before we touch anything and + // nothing is lost, on a cold one it lands mid-flow and silently discards our + // input. Locally that showed up as a booking created under the wrong title; in + // CI it showed up as "the confirm dialog did not open", because a reverted + // All Day leaves the default slot, which on a slow run has already passed and + // makes the form invalid with no visible error. + // + // Re-applying inside a retrying block converges whenever the rebuild fires, + // without needing to know the app's internal ready signal. + const all_day = page.locator('mat-checkbox:has-text("All Day") input[type="checkbox"]'); + const locker_input = page.locator( + 'mat-checkbox:has-text("Require locker") input[type="checkbox"]', + ); + await expect(async () => { + await setCheckbox(page, 'Require locker', false); + await setCheckbox(page, 'All Day', true); + await title_input.fill(title); + expect(await title_input.inputValue()).toBe(title); + expect(await all_day.isChecked()).toBe(true); + if (await locker_input.count()) expect(await locker_input.isChecked()).toBe(false); + }).toPass({ timeout: 30_000 }); await page.locator('button[name="open-desk-confirm"]').click(); From 79251e2cdc856a2de573aa41fd5c9666506562c2 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 30 Jul 2026 12:17:24 +1000 Subject: [PATCH 05/17] docs(e2e): record the CI track record and two app-side defects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Log all five CI runs honestly, including that run 1's diagnosis was wrong and that run 3 was "green" while reporting two flaky specs. - REG-09: note that DB::ConnectionLost has NOT been seen in CI, but CI runs 2 workers vs 4 locally, so the quiet record may just be lower concurrency rather than the problem being absent. - REG-10 (new): the booking form discards input while still initialising. A real user can hit this. bookDeskViaUI now works around it, which means the suite no longer detects it — so it needs a row of its own rather than being buried in a test helper. Co-Authored-By: Claude Opus 5 (1M context) --- E2E_USER_STORIES.md | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/E2E_USER_STORIES.md b/E2E_USER_STORIES.md index 8b98df2ed5..57995c85d4 100644 --- a/E2E_USER_STORIES.md +++ b/E2E_USER_STORIES.md @@ -23,6 +23,20 @@ rather than passed, because a spec that only passes on retry is the signal that nightly runs green, every `flaky` occurrence explained, and REG-09 either fixed or consciously accepted. +### Track record so far + +Honest log, because "it went green once" is not a track record. + +| Run | Result | What it taught us | +|---|---|---| +| 1 | failed | Elasticsearch would not start. Diagnosed as `memory_lock` — **wrong**, but the fix was harmless. Exposed that log collection used a hand-picked service list which omitted elastic, so the one useful log was the one not captured. | +| 2 | failed | With diagnostics in place: ES 7.17.6 bundles a JDK with the **cgroup v2** NPE bug and its launcher dies before the JVM starts. GitHub runners use cgroup v2; Docker Desktop does not — unreproducible locally. Fixed by moving to 7.17.28. | +| 3 | success, **2 flaky** | Both desk specs failed attempt 1 with "the confirm dialog did not open". Root cause: the booking form is rebuilt when async init completes and restores defaults — a **race**, not a step. Mitigated by converging on the form state in a retrying block. | +| 4 | success, 0 flaky | — | +| 5 (re-run of 4's commit) | success, 0 flaky | Same code twice with no flakes. | + +Two clean runs is a start, not a track record. The nightly is what accumulates one. + ## How this document works **It is the mechanism that keeps the suite current.** A suite decays the moment nobody @@ -97,7 +111,8 @@ task that found it, so the row can be traced. | REG-06 | P2 | Timezone parsing does not error for a building with an unusual timezone. | "Fix error when parsing timezones" | todo | | REG-07 | P2 | Level selection does not persist once the selector is hidden/disabled. | "Fix level selections persisting when selector is disabled/hidden" | todo | | REG-08 | P1 | An authority with a **relative** `login_url` still reaches a usable login page. | Found 2026-07-30, this suite | **blocked** — currently worked around in `seed.ts`; ts-client resolves a relative `login_url` against the authority host **without its port**, so any non-443 deployment dead-ends. Needs a ts-client/init fix before a spec can assert the good behaviour. | -| REG-09 | P1 | Concurrent `POST /bookings` do not 500. | Found 2026-07-30, this suite | **blocked** — staff-api raises `DB::ConnectionLost` under concurrency (~1 in 8 at 4 workers). Currently absorbed by CI retries and reported as `flaky`. Needs a staff-api fix. | +| REG-09 | P1 | Concurrent `POST /bookings` do not 500. | Found 2026-07-30, this suite | **blocked** — staff-api raises `DB::ConnectionLost` under concurrency. Observed ~1 run in 8 **locally at 4 workers**; **not yet observed in CI**, which runs 2 workers, so halving the concurrency may simply be avoiding it rather than the problem being absent. Needs a staff-api fix; do not treat the quiet CI record as evidence it is gone. | +| REG-10 | P1 | The booking form does not discard user input while it is still initialising. | Found 2026-07-30, this suite | **blocked** — the form is rebuilt when async init completes and restores defaults (title, All Day, Require locker), silently dropping anything typed before that. A real user can hit this; they would just see their title or options revert. `bookDeskViaUI` converges on the state to work around it, which means **the suite no longer detects it** — hence this row. Needs a fix in the app, then a spec that asserts input survives. | ## 4. Platform & configuration From 3cac12b18712678a9e5c61608f4ee9a649ae6e94 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 30 Jul 2026 13:18:04 +1000 Subject: [PATCH 06/17] ci(e2e): run on a self-hosted macOS runner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - runs-on: [self-hosted, placeos-e2e] - Drop the `pull_request` trigger. This repo is PUBLIC and the runner is a machine on an internal network, so a fork PR could run arbitrary code on it. Remaining triggers (schedule, dispatch, push to e2e/**) all require write access. Costs nothing today since the check is advisory and not a PR gate. - Add a reclaim step: a self-hosted machine is not a fresh VM, and a previous aborted run can leave the stack up or port 4214 held. - Drop the vm.max_map_count bump — required on GitHub-hosted Linux, meaningless on macOS where the value lives inside Docker Desktop's VM. - Move the nightly to 01:10 UTC and note that `schedule` only fires from the default branch, so the track record does not start until this is on develop. - Add e2e/stack/SELF_HOSTED_RUNNER.md: the runbook, led by the fact that no inbound access is needed, with a pre-flight connectivity check and the macOS gotchas (Docker Desktop needs a GUI session, sleep kills nightlies, service PATH). Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/e2e-advisory.yml | 53 +++++-- E2E_USER_STORIES.md | 18 ++- e2e/README.md | 11 +- e2e/stack/SELF_HOSTED_RUNNER.md | 226 +++++++++++++++++++++++++++++ 4 files changed, 286 insertions(+), 22 deletions(-) create mode 100644 e2e/stack/SELF_HOSTED_RUNNER.md diff --git a/.github/workflows/e2e-advisory.yml b/.github/workflows/e2e-advisory.yml index 66a9765fa1..e5a60fcf26 100644 --- a/.github/workflows/e2e-advisory.yml +++ b/.github/workflows/e2e-advisory.yml @@ -19,21 +19,27 @@ name: 'E2E (advisory)' # POST /bookings. Playwright's retries absorb it and mark the spec `flaky` — a # flaky-but-passing run is reported in the summary rather than hidden. +# NOTE: there is deliberately NO `pull_request` trigger. +# +# This repository is PUBLIC and this job runs on a SELF-HOSTED runner. A +# `pull_request` trigger would let anyone's fork execute arbitrary code on that +# machine — which sits on an internal network — so the only triggers here are ones +# that require write access to this repo: a schedule, a manual dispatch, and pushes +# to branches that live here. Do not add `pull_request` back without moving the job +# to a GitHub-hosted runner (`runs-on: ubuntu-latest`) or making the repo private. on: workflow_dispatch: # Lets this workflow be exercised on its own branch before it lives on - # `develop` (where `workflow_dispatch` first becomes available). The prefix is - # deliberately outside the `feat/*`, `fix/*`, `custom/*`, `refactor/*` set that - # PR Flow watches, so iterating on CI here never triggers a build or a - # `build//` deploy. + # `develop`. The prefix is deliberately outside the `feat/*`, `fix/*`, + # `custom/*`, `refactor/*` set that PR Flow watches, so iterating on CI here + # never triggers a build or a `build//` deploy. push: branches: ['e2e/**'] schedule: - # 15:10 UTC daily — off-peak for AU-hours contributors, so a nightly - # failure is waiting to be read rather than interrupting anyone. - - cron: '10 15 * * *' - pull_request: - branches: [develop] + # 01:10 UTC daily. NB scheduled workflows only run from the DEFAULT branch, + # so the nightly track record does not start accumulating until this file is + # merged to `develop`. + - cron: '10 1 * * *' concurrency: group: e2e-${{ github.ref }} @@ -42,7 +48,11 @@ concurrency: jobs: workplace: name: 'workplace e2e (local stack)' - runs-on: ubuntu-latest + # Self-hosted macOS runner, targeted by its custom label rather than by + # `macOS`/arch labels so the machine can be swapped without editing this. + # To fall back to GitHub-hosted, set `runs-on: ubuntu-latest` and restore + # the `Raise vm.max_map_count` step (see below). + runs-on: [self-hosted, placeos-e2e] timeout-minutes: 45 env: E2E_BACKEND_URL: https://localhost:9443 @@ -86,11 +96,24 @@ jobs: - name: Setup version run: bun run postinstall - # Elasticsearch refuses to start with the runner's default - # vm.max_map_count (65530). Docker Desktop's VM is already above the - # threshold, which is why this is only needed in CI. - - name: Raise vm.max_map_count for Elasticsearch - run: sudo sysctl -w vm.max_map_count=262144 + # A self-hosted machine is NOT a fresh VM. A previous aborted run can + # leave the stack up or the dev-server port held, and the failure that + # causes ("port already in use", or specs hitting a half-torn-down + # stack) looks nothing like its cause. Reclaim both before starting. + - name: Reclaim the machine from any previous run + run: | + e2e/stack/down.sh --volumes || true + # Playwright refuses to start its webServer if 4214 is held. + lsof -nP -iTCP:4214 -sTCP:LISTEN -t 2>/dev/null | xargs -r kill -9 || true + docker system df || true + + # NOTE: no `sysctl -w vm.max_map_count=262144` here. + # + # It is required on a GitHub-hosted Linux runner (default 65530 is below + # Elasticsearch's threshold) but is meaningless on macOS: the value lives + # inside Docker Desktop's VM, not the host, and Docker Desktop already + # sets it high enough. Restore that step if this job moves back to + # ubuntu-latest. - name: Install Playwright browser run: bunx playwright install --with-deps chromium diff --git a/E2E_USER_STORIES.md b/E2E_USER_STORIES.md index 57995c85d4..2fceaa922d 100644 --- a/E2E_USER_STORIES.md +++ b/E2E_USER_STORIES.md @@ -5,10 +5,20 @@ Harness, conventions and gotchas live in [`e2e/README.md`](e2e/README.md). ## CI status: advisory, deliberately -`.github/workflows/e2e-advisory.yml` runs the suite on PRs into `develop`, nightly, and -on demand. **It must not gate merges yet**, and nothing in the workflow file can make it -do so — required status checks are a repository setting, so keeping this advisory is an -explicit human decision rather than a default. +`.github/workflows/e2e-advisory.yml` runs the suite nightly, on demand, and on pushes to +`e2e/**`, on a **self-hosted macOS runner** (see +[`e2e/stack/SELF_HOSTED_RUNNER.md`](e2e/stack/SELF_HOSTED_RUNNER.md)). +**It must not gate merges yet**, and nothing in the workflow file can make it do so — +required status checks are a repository setting, so keeping this advisory is an explicit +human decision rather than a default. + +There is deliberately **no `pull_request` trigger**: this repo is public and the runner is +self-hosted, so a fork PR could execute arbitrary code on a machine on an internal +network. Every remaining trigger requires write access to this repo. Do not add it back +without moving the job to `ubuntu-latest` or making the repo private. + +Note also that GitHub only runs `schedule` triggers from the **default branch**, so the +nightly track record does not begin until this workflow is merged to `develop`. That is on purpose. Before this suite is allowed to block anyone it needs a track record: no false positives, specs reviewed by humans, and a known flake rate. The nightly run diff --git a/e2e/README.md b/e2e/README.md index 43fa09a5ae..658980b565 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -214,13 +214,18 @@ container logs. required status checks until the suite has a track record. See the CI section of [`../E2E_USER_STORIES.md`](../E2E_USER_STORIES.md) for the bar it needs to clear first. +It runs on a **self-hosted macOS runner** (label `placeos-e2e`). Setup, the network +requirements, and the macOS-specific gotchas are in +[`stack/SELF_HOSTED_RUNNER.md`](stack/SELF_HOSTED_RUNNER.md). + CI-specific choices worth knowing: | | | |---|---| -| `E2E_WORKERS: 2` | Lower than the local default of 4. A runner has 4 cores and is also hosting ~10 containers, a dev server and browsers. Less contention means fewer failures that are about the runner rather than the code — avoiding false positives matters more than run time here. | -| `sudo sysctl -w vm.max_map_count=262144` | Elasticsearch will not start on a runner's default (65530). Docker Desktop's VM is already above it, which is why this is CI-only. | -| `bunx playwright install --with-deps chromium` | The workspace's pinned Playwright needs its browser and system libs. | +| `E2E_WORKERS: 2` | Lower than the local default of 4. The runner is also hosting ~10 containers, a dev server and browsers. Less contention means fewer failures that are about the machine rather than the code — avoiding false positives matters more than run time here. | +| No `pull_request` trigger | This repo is public and the runner is self-hosted, so a fork PR could run arbitrary code on an internal machine. Triggers are limited to ones requiring write access. Do not add it back without moving to `ubuntu-latest`. | +| Reclaim step | A self-hosted machine is not a fresh VM; a previous aborted run can leave the stack up or port 4214 held. | +| No `vm.max_map_count` bump | Needed on GitHub-hosted Linux, meaningless on macOS (the value lives in Docker Desktop's VM). Restore it if reverting to `ubuntu-latest`. | | Network access to GitHub | `up.sh` clones `PlaceOS/www-core` into the `www` volume once — that is where the platform `/login` page comes from. | ## Booking specs: what the backend actually does diff --git a/e2e/stack/SELF_HOSTED_RUNNER.md b/e2e/stack/SELF_HOSTED_RUNNER.md new file mode 100644 index 0000000000..419209cc08 --- /dev/null +++ b/e2e/stack/SELF_HOSTED_RUNNER.md @@ -0,0 +1,226 @@ +# Running the e2e suite on a self-hosted macOS runner + +Runbook for pointing `.github/workflows/e2e-advisory.yml` at a Mac on the internal +network instead of a GitHub-hosted runner. + +--- + +## First: no inbound access is needed + +The common worry with a machine on a corporate wifi is "how will GitHub reach it". +It doesn't. **The runner opens an outbound HTTPS long-poll to GitHub and waits for +work.** No port forwarding, no inbound firewall rule, no static IP, no VPN, no +tunnel. + +What it *does* need is **outbound 443** to a handful of hosts. If the network +filters or proxies outbound traffic, that is the only thing to sort out. + +### Step 0 — pre-flight, before installing anything + +Run this on the runner machine. It answers the network question in about ten +seconds, and if it fails you have saved yourself the whole setup. + +```bash +for host in github.com api.github.com codeload.github.com \ + objects.githubusercontent.com pipelines.actions.githubusercontent.com \ + registry-1.docker.io auth.docker.io production.cloudflare.docker.com; do + printf '%-46s ' "$host" + curl -sS -o /dev/null -m 10 -w '%{http_code}\n' "https://$host" 2>&1 | tail -1 +done +``` + +Any HTTP status (including 401/404) means the host is reachable — you are looking +for connection failures and timeouts, not for 200s. + +- **`github.com` / `api.github.com` / `codeload.github.com`** — runner registration, + job dispatch, checkout, and cloning `PlaceOS/www-core` during stack bring-up. +- **`objects.githubusercontent.com` / `pipelines.actions.githubusercontent.com`** — + action downloads, log upload, artifact upload. +- **`registry-1.docker.io` / `auth.docker.io` / `production.cloudflare.docker.com`** — + pulling the PlaceOS images. This is the one people forget; without it the stack + cannot start. + +**If there is an outbound proxy**, the runner supports it. Set these before +`config.sh` *and* make sure the service sees them (see Step 3): + +```bash +export HTTPS_PROXY=http://proxy.internal:8080 +export HTTP_PROXY=http://proxy.internal:8080 +export NO_PROXY=localhost,127.0.0.1 +``` + +Docker Desktop needs the proxy configured separately, in +**Settings → Resources → Proxies**. + +--- + +## Step 1 — prerequisites + +| | | +|---|---| +| **Docker Desktop** | Installed, running, and set to **start on login**. Give it **≥ 8 GB memory and ≥ 4 CPUs** (Settings → Resources) — the stack is 10 containers including Elasticsearch and Postgres, and the job also runs an Angular build and a browser. | +| **Xcode Command Line Tools** | `xcode-select --install` | +| **git** | Comes with the CLT. | +| Node / Bun / Playwright browsers | **Not needed up front** — the workflow installs them (`setup-node`, `setup-bun`, `playwright install`). | + +Sanity check: + +```bash +docker run --rm hello-world # proves Docker works AND the registry is reachable +sw_vers && uname -m # note the arch for Step 2 +``` + +--- + +## Step 2 — register the runner + +You need **admin on the repo** to add a runner. + +```bash +# 1. Get a registration token (valid ~1 hour) +gh api -X POST repos/PlaceOS/user-interfaces/actions/runners/registration-token --jq .token + +# 2. Download the runner. Pick the arch from `uname -m`: +# arm64 -> osx-arm64 x86_64 -> osx-x64 +mkdir -p ~/actions-runner && cd ~/actions-runner +RUNNER_VERSION=2.336.0 +RUNNER_ARCH=osx-arm64 # or osx-x64 +curl -o runner.tar.gz -L \ + "https://github.com/actions/runner/releases/download/v${RUNNER_VERSION}/actions-runner-${RUNNER_ARCH}-${RUNNER_VERSION}.tar.gz" +tar xzf runner.tar.gz && rm runner.tar.gz + +# 3. Configure. The LABEL IS LOAD-BEARING — the workflow targets `placeos-e2e`. +./config.sh \ + --url https://github.com/PlaceOS/user-interfaces \ + --token \ + --name placeos-e2e-mac \ + --labels placeos-e2e \ + --work _work +``` + +(Or generate the token via **Settings → Actions → Runners → New self-hosted +runner**, which shows the same commands pre-filled.) + +The default `self-hosted` label is added automatically; `runs-on` in the workflow +is `[self-hosted, placeos-e2e]`. + +--- + +## Step 3 — run it as a service, and the three macOS gotchas + +```bash +cd ~/actions-runner +./svc.sh install +./svc.sh start +./svc.sh status +``` + +Then confirm it shows **Idle** under Settings → Actions → Runners. + +**These are the things that actually catch people out on macOS:** + +1. **Docker Desktop needs a logged-in GUI session.** The runner service is a + launchd *agent*, and Docker Desktop is a GUI app — neither runs when nobody is + logged in. If the Mac is headless, enable automatic login (System Settings → + Users & Groups → Automatic login) and leave the session logged in. A locked + screen is fine; a logged-out one is not. + +2. **A sleeping Mac runs no nightlies.** Prevent system sleep: + + ```bash + sudo pmset -a sleep 0 # never sleep the system + sudo pmset -a disablesleep 1 # laptops: also ignore lid close + pmset -g | grep -E 'sleep|disablesleep' + ``` + + Display sleep is fine and worth leaving on. + +3. **The service gets a minimal environment.** If a job reports `docker: command + not found`, the service cannot see Docker's CLI. Fix by adding it to the + runner's env file, then restarting the service: + + ```bash + cd ~/actions-runner + echo 'PATH=/usr/local/bin:/opt/homebrew/bin:/usr/bin:/bin:/usr/sbin:/sbin' >> .env + # add proxy vars here too, if you needed them in Step 0 + ./svc.sh stop && ./svc.sh start + ``` + +Prefer to watch it interactively while setting up? Skip the service and run +`./run.sh` in a terminal — same behaviour, logs in front of you, stops on Ctrl-C. + +--- + +## Step 4 — trigger a run + +The workflow already targets the runner. Nothing to edit. + +```bash +# From a clone of the repo, on the e2e branch: +git commit --allow-empty -m "ci: exercise the self-hosted runner" && git push +gh run watch +``` + +The `push: ['e2e/**']` trigger fires on any branch under `e2e/`. +`workflow_dispatch` also works, but only once this file is on `develop`. + +**Expected on a first run:** ~10–15 min, most of it pulling ~10 Docker images. Then +~4–5 min per run once the images are cached locally. + +--- + +## Step 5 — the nightly needs a merge to `develop` + +> **GitHub only runs `schedule` triggers from the default branch.** The nightly +> will not fire from `e2e/ci-verify` no matter what the cron says. + +So the track record does not start accumulating until this workflow is merged to +`develop`. Until then, use pushes or manual dispatch. Worth knowing, because "the +nightly silently never ran" is an easy thing to not notice for a fortnight. + +--- + +## Security posture — why there is no `pull_request` trigger + +`PlaceOS/user-interfaces` is a **public** repo, and GitHub's own guidance is not to +use self-hosted runners with public repositories: a `pull_request` trigger lets +anyone's fork run arbitrary code on the runner. Here that runner is a machine on an +internal network, so the blast radius is the network, not just a throwaway VM. + +The workflow therefore triggers **only** on `schedule`, `workflow_dispatch`, and +pushes to `e2e/**` — all of which require write access to this repo. There is no +path for fork code to reach the machine. + +**Do not add `pull_request` back** without either moving the job to +`runs-on: ubuntu-latest` or making the repo private. + +Worth doing as defence in depth, in case someone adds it later: + +- **Settings → Actions → General → Fork pull request workflows** → require approval + for **all** outside collaborators. +- Run the runner under a **dedicated macOS user account** with no access to + credentials or source you care about, and keep no secrets on the machine. The + workflow needs none. + +--- + +## Troubleshooting + +| Symptom | Cause / fix | +|---|---| +| Runner shows **Offline** | Service not running (`./svc.sh status`), machine asleep, or outbound 443 blocked — re-run Step 0. | +| `docker: command not found` | Service PATH — gotcha 3 above. | +| Job hangs on **Bring up the local PlaceOS stack** | Docker Desktop not running (no GUI session — gotcha 1), or registry unreachable. `up.sh` prints `compose ps` plus every service's logs on failure, so read the step output first. | +| `port is already allocated` / webServer port in use | A previous run died. The **Reclaim the machine** step handles this automatically; if it recurs, run `e2e/stack/down.sh --volumes` by hand. | +| `elastic-1 is unhealthy` | Give Docker Desktop more memory. Elasticsearch is pinned to 7.17.28 specifically to avoid the cgroup v2 JDK crash — do not downgrade it. | +| Disk filling up | Volumes accumulate across runs. `docker system prune -a --volumes` (safe — the stack is rebuilt from nothing each run). | +| Two runs at once | One runner takes one job at a time; the second queues. That is the desired behaviour: the stack binds fixed ports and cannot run twice. | + +## Reverting to GitHub-hosted + +In `.github/workflows/e2e-advisory.yml`: + +1. `runs-on: [self-hosted, placeos-e2e]` → `runs-on: ubuntu-latest` +2. Restore the `sysctl -w vm.max_map_count=262144` step (required on Linux, a no-op + on macOS — see the comment in the workflow). +3. The **Reclaim the machine** step becomes unnecessary but is harmless. From 5594e149b446f22e268cc1441381c612c6395af7 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 30 Jul 2026 13:21:00 +1000 Subject: [PATCH 07/17] docs(e2e): clarify runner network reachability and SSH-only setup limits - Nobody needs to reach the runner: committers push to GitHub, the runner collects work over its own outbound connection. The machine can sit on a network most of the team cannot reach and CI still works for everyone. - Setting up over SSH alone cannot complete two steps: Docker Desktop is a GUI app that will not start without an active GUI session, and svc.sh installs a launchd agent that belongs to one. Do those at the machine once, then SSH is fine. - Troubleshooting: being unable to ping/SSH the box is expected (macOS Remote Login off by default, ICMP dropped, possible wifi client isolation) and unrelated to CI. Co-Authored-By: Claude Opus 5 (1M context) --- e2e/stack/SELF_HOSTED_RUNNER.md | 34 ++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/e2e/stack/SELF_HOSTED_RUNNER.md b/e2e/stack/SELF_HOSTED_RUNNER.md index 419209cc08..97cd97c06d 100644 --- a/e2e/stack/SELF_HOSTED_RUNNER.md +++ b/e2e/stack/SELF_HOSTED_RUNNER.md @@ -54,6 +54,37 @@ Docker Desktop needs the proxy configured separately, in --- +### Who needs to be on the runner's network? Nobody. + +Worth stating plainly, because it is the natural thing to worry about and it is not a +constraint: + +- **Committers do not.** A push from anywhere triggers the workflow on GitHub, which + queues a job, which the runner collects over its own outbound connection. Where the + author sits is irrelevant. +- **GitHub does not.** It never initiates a connection to the machine. +- **You only do** if you want to administer the box directly — install it, read logs, + restart the service. + +So the runner can live on an internal network that most of the team cannot reach, and +CI still works for all of them. + +### Setting it up over SSH: read this first + +macOS ships with Remote Login **off**, so SSH may need enabling +(System Settings → General → Sharing → Remote Login). But even with SSH working, +**two of the steps below cannot be completed from an SSH-only session**: + +- **Docker Desktop is a GUI app.** `open -a Docker` over SSH only succeeds if that + user already has an active GUI session (logged in at the console). Over SSH alone + it will not start, and the stack step will fail with the daemon unreachable. +- **`svc.sh install` creates a launchd *agent***, which belongs to a GUI session. Run + from a bare SSH login it may install but not run when you expect. + +Practical order: do the GUI-dependent parts once at the machine (enable auto-login, +set Docker Desktop to start on login, start it), after which SSH is fine for +everything else — registering the runner, service control, logs. + ## Step 1 — prerequisites | | | @@ -208,7 +239,8 @@ Worth doing as defence in depth, in case someone adds it later: | Symptom | Cause / fix | |---|---| -| Runner shows **Offline** | Service not running (`./svc.sh status`), machine asleep, or outbound 443 blocked — re-run Step 0. | +| Runner shows **Offline** | Service not running (`./svc.sh status`), machine asleep, or outbound 443 blocked — re-run Step 0. Note this is about the runner's *outbound* connection; whether anyone can reach the machine is unrelated. | +| Can't ping / SSH the machine from elsewhere on the network | Expected and harmless for CI. macOS has Remote Login off by default and its firewall drops ICMP; some wifi also enforces client isolation. Only affects direct administration. | | `docker: command not found` | Service PATH — gotcha 3 above. | | Job hangs on **Bring up the local PlaceOS stack** | Docker Desktop not running (no GUI session — gotcha 1), or registry unreachable. `up.sh` prints `compose ps` plus every service's logs on failure, so read the step output first. | | `port is already allocated` / webServer port in use | A previous run died. The **Reclaim the machine** step handles this automatically; if it recurs, run `e2e/stack/down.sh --volumes` by hand. | From 1049354df9ed6555a9d6dfb7c10f75715f61c53e Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 30 Jul 2026 14:08:08 +1000 Subject: [PATCH 08/17] ci(e2e): drop remote caching, provision the toolchain on the runner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first self-hosted run spent 9.1 min uploading node_modules to GitHub's cache and 3.1 min saving the bun cache, out of 19.2 min total — while the suite itself took 1.0 min. actions/cache, setup-node and setup-bun are the right answer on a hosted runner that starts from a bare VM; on a persistent machine the workspace and toolchain are already present, so shipping them to a remote cache over a slow uplink costs more than everything else combined. Toolchain is now provisioned once on the machine (brew: colima, docker, docker-compose, bun, node@24), all resolvable from the runner service PATH. Also: - Fix a real concurrency bug: a scheduled run and a develop push share github.ref, so with a ref-only group plus cancel-in-progress a push would have CANCELLED an in-flight nightly, destroying the run the track record depends on. Keyed on github.event_name as well. - Name the job "advisory — does not block builds": the job name is what appears in the Checks list beside the build jobs, so a red X cannot be misread. - Stage the trigger rollout. `develop` is deliberately NOT enabled yet — it is the high-value trigger but it also puts a new check on everyone's commits, which should be earned once the nightly has a record. Three-line change when ready. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/e2e-advisory.yml | 79 +++++++++++++++++++----------- E2E_USER_STORIES.md | 36 ++++++++++++++ 2 files changed, 86 insertions(+), 29 deletions(-) diff --git a/.github/workflows/e2e-advisory.yml b/.github/workflows/e2e-advisory.yml index e5a60fcf26..b51f20e93c 100644 --- a/.github/workflows/e2e-advisory.yml +++ b/.github/workflows/e2e-advisory.yml @@ -29,25 +29,54 @@ name: 'E2E (advisory)' # to a GitHub-hosted runner (`runs-on: ubuntu-latest`) or making the repo private. on: workflow_dispatch: - # Lets this workflow be exercised on its own branch before it lives on - # `develop`. The prefix is deliberately outside the `feat/*`, `fix/*`, - # `custom/*`, `refactor/*` set that PR Flow watches, so iterating on CI here - # never triggers a build or a `build//` deploy. push: - branches: ['e2e/**'] + branches: + # For iterating on CI itself. Deliberately outside the `feat/*`, + # `fix/*`, `custom/*`, `refactor/*` set PR Flow watches, so this never + # triggers a build or a `build//` deploy. + - 'e2e/**' + # + # STAGE 2 — not yet. Add `develop` (and `release/**`, `rc/**`) once the + # nightly has a track record. Deliberately deferred: `develop` is the + # high-value trigger — it catches a regression where it lands, when + # bisecting is cheapest — but turning it on also puts a new check on + # everyone's commits, and that should be earned rather than assumed. + # Until then `workflow_dispatch` covers any one-off. + # + # It cannot block builds when it is enabled (different runner pool, no + # cross-workflow `needs`, no required status checks on develop) — see + # E2E_USER_STORIES.md. The reason to wait is noise and trust, not risk. schedule: # 01:10 UTC daily. NB scheduled workflows only run from the DEFAULT branch, # so the nightly track record does not start accumulating until this file is # merged to `develop`. - cron: '10 1 * * *' +# NOT triggered by feature branches, on purpose. +# +# There is ONE self-hosted runner and the stack binds fixed host ports +# (9443/9080/4214), so runs cannot overlap — they serialise. A busy trigger set +# would not just burn time, it would build a queue that delays or starves the +# nightly, and the nightly is what produces the confidence record. Anyone who +# wants the suite against a feature branch can use "Run workflow". + +# `github.event_name` is in the group deliberately. +# +# A scheduled run and a push run both have `github.ref` = refs/heads/develop, so +# with a ref-only group plus cancel-in-progress a push to develop would CANCEL an +# in-flight nightly — destroying the very run the track record depends on. Keying +# on the event as well keeps the two apart: pushes still supersede each other, +# nightlies are never collateral. concurrency: - group: e2e-${{ github.ref }} + group: e2e-${{ github.workflow }}-${{ github.event_name }}-${{ github.ref }} cancel-in-progress: true jobs: workplace: - name: 'workplace e2e (local stack)' + # The job name is what appears in the commit's Checks list, next to the + # build jobs. Carrying "advisory" there means a red X cannot be misread as + # "the build broke" at a glance. + name: 'workplace e2e (advisory — does not block builds)' # Self-hosted macOS runner, targeted by its custom label rather than by # `macOS`/arch labels so the machine can be swapped without editing this. # To fall back to GitHub-hosted, set `runs-on: ubuntu-latest` and restore @@ -68,29 +97,21 @@ jobs: steps: - uses: actions/checkout@v6 - - name: Cache node modules - id: cache - uses: actions/cache@v5 - with: - path: node_modules - key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/bun.lock') }} - restore-keys: | - ${{ runner.os }}-build-${{ env.cache-name }}- - ${{ runner.os }}-build- - ${{ runner.os }}- - - - name: Setup Node.js environment - uses: actions/setup-node@v6 - with: - node-version: '24' - - - name: Setup Bun - uses: oven-sh/setup-bun@v2 - with: - bun-version: '1.3.11' - + # No actions/cache, no setup-node, no setup-bun — on purpose. + # + # Those three are the right answer on a GitHub-hosted runner, where every + # job starts from a bare VM. Here they were the single biggest cost in + # the job: the first self-hosted run spent 9.1 min uploading node_modules + # to GitHub's cache and 3.1 min saving the bun cache, out of 19.2 min + # total — while the suite itself took 1.0 min. On a persistent machine + # the workspace and the toolchain are already there, so shipping them to + # a remote cache over a slow uplink buys nothing and costs more than + # everything else combined. + # + # The toolchain is provisioned once on the machine instead: + # brew install colima docker docker-compose oven-sh/bun/bun node@24 + # See e2e/stack/SELF_HOSTED_RUNNER.md. - name: Install dependencies - if: steps.cache.outputs.cache-hit != 'true' run: bun install --frozen-lockfile - name: Setup version diff --git a/E2E_USER_STORIES.md b/E2E_USER_STORIES.md index 2fceaa922d..e8bbcf2064 100644 --- a/E2E_USER_STORIES.md +++ b/E2E_USER_STORIES.md @@ -17,6 +17,42 @@ self-hosted, so a fork PR could execute arbitrary code on a machine on an intern network. Every remaining trigger requires write access to this repo. Do not add it back without moving the job to `ubuntu-latest` or making the repo private. +### It cannot block or delay builds + +Asked for by the frontend team, and true structurally rather than by convention: + +- **Different runner pool.** `build.yml` runs on `ubuntu-latest` (GitHub-hosted); this job + runs on the self-hosted Mac. They never compete for a runner, so an e2e run cannot + delay a build even when both fire on the same push to `develop`. +- **No dependency, and none possible.** `build.yml`'s jobs only `needs: install_deps`. + Actions has no cross-workflow `needs`, so this suite can never gate a build or a deploy. +- **Not a required check.** Nothing here can make it one; that is a repository setting. +- The job is named *"workplace e2e (advisory — does not block builds)"* so a red X in the + Checks list next to the build jobs cannot be misread as a broken build. + +### Trigger rollout, in two stages + +**Stage 1 (now):** nightly, `workflow_dispatch`, and pushes to `e2e/**`. Nobody sees a new +check on their commits, and the nightly starts accumulating the record — which it can only +do from the default branch, so this is what breaks the chicken-and-egg of "prove it before +merging it". + +**Stage 2 (once the record is good):** add `push: develop`, and `release/**` / `rc/**` to +match the branch set `build.yml` deploys from. `develop` is the high-value trigger — it +catches a regression at the moment it lands, when bisecting is cheapest — and with no +`pull_request` trigger it is also how merges get covered. Deferred because enabling it puts +a new check on everyone's commits, which should be earned rather than assumed. It is a +three-line change. + +**Never:** feature branches. There is one self-hosted runner and the stack binds fixed host +ports, so runs serialise; a busy trigger set would build a queue that delays or starves the +nightly. Use **Run workflow** for a one-off. + +No path filters, on purpose: most real changes touch `libs/**` which workplace depends on, +and the genuinely dangerous ones (`bun.lock`, `tsconfig.base.json`, `config/`) are the +easiest to leave off an include-list. A filter that is 95% right silently skips the run +that mattered. Revisit if `develop` volume makes the queue a problem. + Note also that GitHub only runs `schedule` triggers from the **default branch**, so the nightly track record does not begin until this workflow is merged to `develop`. From 6c9b80f174a124c9a924b4b5fe264b0aa466d194 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 30 Jul 2026 14:28:26 +1000 Subject: [PATCH 09/17] docs(e2e): file REG-09 and REG-10 as bugs, keep the reproducer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both findings investigated to root cause and filed against PPT, assigned to Cam. PPT-2642 (REG-09) — worse than the flake suggested. One burst of concurrent POST /bookings permanently poisons staff-api's connection pool; every later booking-create returns 500 with "There is an existing transaction in this connection" until the service restarts. Verified: serial requests are always clean (including 409/422 raised inside the transaction), a concurrent burst looks healthy itself, and everything after it fails. Reproducer kept at e2e/support/repro/reg09-concurrent-bookings.ts rather than only in the ticket. PPT-2643 (REG-10) — booking-form.service.ts newForm() defers itself until the current user loads, then resets the form, discarding anything typed in the meantime. One race, two symptoms: locally it ate the title, in CI it ate All Day (leaving the default 5-minute slot, already past on a slow run, silently invalidating the form). Neither is a PR: REG-09's likely fix is in pg-orm, shared by every PlaceOS service in a transactional path; REG-10's fix embeds a UX decision (preserve dirty input vs gate the form until ready) in a service used by every booking flow. Noted in the contract that the suite no longer detects REG-10 — bookDeskViaUI works around it — so the workaround must be removed when it is fixed. Co-Authored-By: Claude Opus 5 (1M context) --- E2E_USER_STORIES.md | 14 +++- .../repro/reg09-concurrent-bookings.ts | 74 +++++++++++++++++++ 2 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 e2e/support/repro/reg09-concurrent-bookings.ts diff --git a/E2E_USER_STORIES.md b/E2E_USER_STORIES.md index e8bbcf2064..f372908bb4 100644 --- a/E2E_USER_STORIES.md +++ b/E2E_USER_STORIES.md @@ -157,8 +157,8 @@ task that found it, so the row can be traced. | REG-06 | P2 | Timezone parsing does not error for a building with an unusual timezone. | "Fix error when parsing timezones" | todo | | REG-07 | P2 | Level selection does not persist once the selector is hidden/disabled. | "Fix level selections persisting when selector is disabled/hidden" | todo | | REG-08 | P1 | An authority with a **relative** `login_url` still reaches a usable login page. | Found 2026-07-30, this suite | **blocked** — currently worked around in `seed.ts`; ts-client resolves a relative `login_url` against the authority host **without its port**, so any non-443 deployment dead-ends. Needs a ts-client/init fix before a spec can assert the good behaviour. | -| REG-09 | P1 | Concurrent `POST /bookings` do not 500. | Found 2026-07-30, this suite | **blocked** — staff-api raises `DB::ConnectionLost` under concurrency. Observed ~1 run in 8 **locally at 4 workers**; **not yet observed in CI**, which runs 2 workers, so halving the concurrency may simply be avoiding it rather than the problem being absent. Needs a staff-api fix; do not treat the quiet CI record as evidence it is gone. | -| REG-10 | P1 | The booking form does not discard user input while it is still initialising. | Found 2026-07-30, this suite | **blocked** — the form is rebuilt when async init completes and restores defaults (title, All Day, Require locker), silently dropping anything typed before that. A real user can hit this; they would just see their title or options revert. `bookDeskViaUI` converges on the state to work around it, which means **the suite no longer detects it** — hence this row. Needs a fix in the app, then a spec that asserts input survives. | +| REG-09 | P1 | Concurrent `POST /bookings` do not 500. | **[PPT-2642](https://acaprojects.atlassian.net/browse/PPT-2642)** | **blocked** — staff-api raises `DB::ConnectionLost` under concurrency. Observed ~1 run in 8 **locally at 4 workers**; **not yet observed in CI**, which runs 2 workers, so halving the concurrency may simply be avoiding it rather than the problem being absent. Needs a staff-api fix; do not treat the quiet CI record as evidence it is gone. | +| REG-10 | P1 | The booking form does not discard user input while it is still initialising. | **[PPT-2643](https://acaprojects.atlassian.net/browse/PPT-2643)** | **blocked** — the form is rebuilt when async init completes and restores defaults (title, All Day, Require locker), silently dropping anything typed before that. A real user can hit this; they would just see their title or options revert. `bookDeskViaUI` converges on the state to work around it, which means **the suite no longer detects it** — hence this row. Needs a fix in the app, then a spec that asserts input survives. | ## 4. Platform & configuration @@ -180,9 +180,15 @@ Config gaps caused several production incidents, and they are invisible to UI sp unblocks every PlaceOS-native booking type (desks, lockers, parking, visitors) with no outbound call. `/calendars` and `/events` do call Microsoft and fail `AADSTS900023`, so WP-E2E-15 stays opt-in and out of the gate. -- **Two rows are blocked on product fixes, not on test effort** (REG-08, REG-09). Both were - found by this suite. Leaving them visible here is the point — a blocked row is coverage +- **Three rows are blocked on product fixes, not on test effort** (REG-08, REG-09, REG-10). All + were found by this suite. Leaving them visible here is the point — a blocked row is coverage information, a deleted row is not. +- **REG-09 is worse than a flake.** Filed as PPT-2642: one burst of concurrent booking POSTs + permanently poisons staff-api's connection pool, so booking creation returns 500 for everyone + until the service restarts. Reproducer kept at `e2e/support/repro/reg09-concurrent-bookings.ts`. +- **REG-10 is invisible to this suite by design.** `bookDeskViaUI` works around it, so nothing + here will catch it regressing. Filed as PPT-2643; when it is fixed, remove the workaround and + replace it with a spec asserting input survives initialisation. - **AUTH-E2E-08 may never be automatable** with Playwright Chromium. Say so rather than quietly dropping it. - The PPT-2536 harnesses (`tasks/PPT-2536/{e2e,integration}`) still hold assertions that diff --git a/e2e/support/repro/reg09-concurrent-bookings.ts b/e2e/support/repro/reg09-concurrent-bookings.ts new file mode 100644 index 0000000000..ab95a3fb43 --- /dev/null +++ b/e2e/support/repro/reg09-concurrent-bookings.ts @@ -0,0 +1,74 @@ +/** + * REG-09 reproducer — PPT-2642. + * + * Concurrent `POST /api/staff/v1/bookings` permanently poison staff-api's Postgres + * connection pool: after one burst, every later booking-create returns 500 with + * "There is an existing transaction in this connection", until staff-api restarts. + * + * Not part of the suite — it is a diagnostic kept next to the finding, because a + * reproducer that lives only in a ticket description rots. + * + * e2e/stack/up.sh # clean stack + * N=1 ROUNDS=3 bunx tsx e2e/support/repro/reg09-concurrent-bookings.ts # healthy + * N=6 ROUNDS=1 bunx tsx e2e/support/repro/reg09-concurrent-bookings.ts # the burst + * N=1 ROUNDS=4 bunx tsx e2e/support/repro/reg09-concurrent-bookings.ts # now 500s + * + * Restart staff-api to recover: + * cd e2e/stack && docker compose -p placeos-e2e restart staff-api + * + * Targets desks 0..4 on DIFFERENT time slots so nothing is a legitimate clash; + * occasional 409s across repeated runs are real clashes with leftovers, not the bug. + */ +import { request } from '@playwright/test'; +import { mintToken } from './e2e/support/auth'; + +const B = 'https://localhost:9443'; +const CONCURRENCY = Number(process.env.N ?? 6); +const ROUNDS = Number(process.env.ROUNDS ?? 6); + +(async () => { + // one token, but each request targets a DIFFERENT desk so nothing is a real clash + const m = await mintToken(B, `${B}/backoffice`, 'support@place.tech', 'development'); + const api = await request.newContext({ baseURL: B, ignoreHTTPSErrors: true, + extraHTTPHeaders: { Authorization: `Bearer ${m.accessToken}` } }); + + const zones = (await (await api.get('/api/engine/v2/zones', { params: { limit: '100' } })).json()) + .map((z: any) => z.id); + const me = await (await api.get('/api/engine/v2/users/current')).json(); + + const status: Record = {}; + let created: number[] = []; + + for (let round = 0; round < ROUNDS; round++) { + const base = Math.floor(Date.now() / 1000) + 86400 * (round + 2); // future, distinct per round + const reqs = Array.from({ length: CONCURRENCY }, (_, i) => + api.post('/api/staff/v1/bookings', { + data: { + booking_type: 'desk', + asset_id: `e2e-desk-${i % 5}`, + booking_start: base + i * 7200, + booking_end: base + i * 7200 + 3600, + timezone: 'Etc/UTC', + user_email: me.email, + user_id: me.id, + user_name: me.name, + title: `REG09 r${round} i${i}`, + zones, + }, + }).then(async r => { + const s = r.status(); + status[s] = (status[s] ?? 0) + 1; + if (s === 201) created.push((await r.json()).id); + else if (s >= 500) console.log(` ${s}: ${(await r.text()).slice(0,120)}`); + return s; + }).catch(e => { status['throw'] = (status['throw'] ?? 0) + 1; return 0; }) + ); + await Promise.all(reqs); + } + + console.log(`\n ${CONCURRENCY} concurrent x ${ROUNDS} rounds = ${CONCURRENCY*ROUNDS} POSTs`); + console.log(' status counts:', JSON.stringify(status)); + for (const id of created) await api.delete(`/api/staff/v1/bookings/${id}`).catch(()=>{}); + console.log(` cleaned up ${created.length}`); + await api.dispose(); +})(); From 15c577ff9baee9743a6dc13f4f3a2c6b4a354728 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 30 Jul 2026 14:54:47 +1000 Subject: [PATCH 10/17] docs(e2e): rewrite the runner runbook from the working install MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runbook described the setup I planned, not the one that exists. Rewritten from what actually works, with the corrections that cost time: - Colima, not Docker Desktop (no GUI session, startable over SSH) - brew's docker-compose needs cliPluginsExtraDirs or `docker compose` never resolves - osx-x64 (the machine is Intel), /usr/local not /opt/homebrew - svc.sh is generated by config.sh, not shipped in the tarball - a slow link looks exactly like a blocked host — retry before blaming the network - records the measured 2.8 min job time and why remote caching was removed - documents the reboot limitation honestly rather than implying it is handled Also fix the REG-09 reproducer's import, broken when it moved into e2e/support/repro/ — verified it runs from the repo root. Co-Authored-By: Claude Opus 5 (1M context) --- e2e/stack/SELF_HOSTED_RUNNER.md | 295 +++++++----------- .../repro/reg09-concurrent-bookings.ts | 2 +- 2 files changed, 115 insertions(+), 182 deletions(-) diff --git a/e2e/stack/SELF_HOSTED_RUNNER.md b/e2e/stack/SELF_HOSTED_RUNNER.md index 97cd97c06d..56a37e57bc 100644 --- a/e2e/stack/SELF_HOSTED_RUNNER.md +++ b/e2e/stack/SELF_HOSTED_RUNNER.md @@ -1,258 +1,191 @@ # Running the e2e suite on a self-hosted macOS runner -Runbook for pointing `.github/workflows/e2e-advisory.yml` at a Mac on the internal -network instead of a GitHub-hosted runner. +How `.github/workflows/e2e-advisory.yml` is set up. Written from an actual working +install rather than from documentation, so the gotchas below are ones that really bit. + +**Current runner:** `placeos-e2e-mac` — macOS 26.1, Intel x86_64, 12 CPU / 16 GB. +Labels `self-hosted, macOS, X64, placeos-e2e`. Docker via **Colima**, not Docker Desktop. --- -## First: no inbound access is needed +## No inbound access is needed -The common worry with a machine on a corporate wifi is "how will GitHub reach it". +The usual worry with a machine on a corporate network is "how will GitHub reach it". It doesn't. **The runner opens an outbound HTTPS long-poll to GitHub and waits for -work.** No port forwarding, no inbound firewall rule, no static IP, no VPN, no -tunnel. +work.** No port forwarding, no inbound firewall rule, no static IP, no VPN required +for CI to function. -What it *does* need is **outbound 443** to a handful of hosts. If the network -filters or proxies outbound traffic, that is the only thing to sort out. +Nor does anyone else need to reach it: -### Step 0 — pre-flight, before installing anything +- **Committers don't.** A push from anywhere triggers the workflow on GitHub, which + queues a job the runner collects over its own outbound connection. +- **GitHub doesn't.** It never initiates a connection to the machine. +- **You only do** to administer it — and only then is something like Tailscale useful. -Run this on the runner machine. It answers the network question in about ten -seconds, and if it fails you have saved yourself the whole setup. +What it *does* need is outbound 443. Check that first: ```bash for host in github.com api.github.com codeload.github.com \ - objects.githubusercontent.com pipelines.actions.githubusercontent.com \ + objects.githubusercontent.com release-assets.githubusercontent.com \ + pipelines.actions.githubusercontent.com \ registry-1.docker.io auth.docker.io production.cloudflare.docker.com; do - printf '%-46s ' "$host" + printf '%-48s ' "$host" curl -sS -o /dev/null -m 10 -w '%{http_code}\n' "https://$host" 2>&1 | tail -1 done ``` -Any HTTP status (including 401/404) means the host is reachable — you are looking -for connection failures and timeouts, not for 200s. - -- **`github.com` / `api.github.com` / `codeload.github.com`** — runner registration, - job dispatch, checkout, and cloning `PlaceOS/www-core` during stack bring-up. -- **`objects.githubusercontent.com` / `pipelines.actions.githubusercontent.com`** — - action downloads, log upload, artifact upload. -- **`registry-1.docker.io` / `auth.docker.io` / `production.cloudflare.docker.com`** — - pulling the PlaceOS images. This is the one people forget; without it the stack - cannot start. +Any HTTP status (401/403/404 included) means reachable — you are looking for +timeouts, not 200s. -**If there is an outbound proxy**, the runner supports it. Set these before -`config.sh` *and* make sure the service sees them (see Step 3): +- **github.com / api / codeload / release-assets** — registration, job dispatch, + checkout, the runner tarball, and cloning `PlaceOS/www-core` during bring-up. +- **registry-1.docker.io / auth.docker.io / production.cloudflare.docker.com** — + pulling the PlaceOS images. The one people forget; without it the stack cannot start. -```bash -export HTTPS_PROXY=http://proxy.internal:8080 -export HTTP_PROXY=http://proxy.internal:8080 -export NO_PROXY=localhost,127.0.0.1 -``` +> A slow link looks like a block. Colima's first VM image download failed here with a +> timeout that read like a firewall; the host was fine, just slow. Retry before +> concluding the network is at fault. -Docker Desktop needs the proxy configured separately, in -**Settings → Resources → Proxies**. +**Behind a proxy:** set `HTTPS_PROXY` / `HTTP_PROXY` / `NO_PROXY` before `config.sh` +and in `~/actions-runner/.env`. Colima needs it separately, in its own config. --- -### Who needs to be on the runner's network? Nobody. +## Setup -Worth stating plainly, because it is the natural thing to worry about and it is not a -constraint: +### 1. Docker, without Docker Desktop -- **Committers do not.** A push from anywhere triggers the workflow on GitHub, which - queues a job, which the runner collects over its own outbound connection. Where the - author sits is irrelevant. -- **GitHub does not.** It never initiates a connection to the machine. -- **You only do** if you want to administer the box directly — install it, read logs, - restart the service. +Docker Desktop is a GUI app, which drags in a login-session requirement for no +benefit on a headless box. **Colima** gives a normal `docker` + `docker compose` +against a Lima VM, entirely from the CLI, startable over SSH. -So the runner can live on an internal network that most of the team cannot reach, and -CI still works for all of them. +```bash +brew install colima docker docker-compose oven-sh/bun/bun node@24 +``` -### Setting it up over SSH: read this first +**Register the compose plugin** — brew installs it but does not wire it up, and every +script here uses `docker compose` (v2 subcommand syntax). Docker Desktop bundled this, +so it is easy to miss: -macOS ships with Remote Login **off**, so SSH may need enabling -(System Settings → General → Sharing → Remote Login). But even with SSH working, -**two of the steps below cannot be completed from an SSH-only session**: +```bash +mkdir -p ~/.docker +cat > ~/.docker/config.json <<'EOF' +{ "cliPluginsExtraDirs": ["/usr/local/lib/docker/cli-plugins"] } +EOF +``` -- **Docker Desktop is a GUI app.** `open -a Docker` over SSH only succeeds if that - user already has an active GUI session (logged in at the console). Over SSH alone - it will not start, and the stack step will fail with the daemon unreachable. -- **`svc.sh install` creates a launchd *agent***, which belongs to a GUI session. Run - from a bare SSH login it may install but not run when you expect. +(`/usr/local` is the Intel brew prefix. On Apple Silicon use `/opt/homebrew`.) -Practical order: do the GUI-dependent parts once at the machine (enable auto-login, -set Docker Desktop to start on login, start it), after which SSH is fine for -everything else — registering the runner, service control, logs. +```bash +colima start --cpu 6 --memory 10 --disk 80 +brew services start colima # launchd agent, so it comes back on login + +docker version && docker compose version && docker run --rm hello-world +``` -## Step 1 — prerequisites +Verified on this VM, so no extra provisioning is needed: | | | |---|---| -| **Docker Desktop** | Installed, running, and set to **start on login**. Give it **≥ 8 GB memory and ≥ 4 CPUs** (Settings → Resources) — the stack is 10 containers including Elasticsearch and Postgres, and the job also runs an Angular build and a browser. | -| **Xcode Command Line Tools** | `xcode-select --install` | -| **git** | Comes with the CLT. | -| Node / Bun / Playwright browsers | **Not needed up front** — the workflow installs them (`setup-node`, `setup-bun`, `playwright install`). | +| `vm.max_map_count` | **1048576** — above Elasticsearch's 262144, so no sysctl step | +| cgroup version | **v2** — the condition that crashes ES 7.17.6; the stack pins 7.17.28 for exactly this reason. Do not downgrade it. | +| Published ports | forwarded to the host, so `localhost:9443` works | -Sanity check: +### 2. Register the runner -```bash -docker run --rm hello-world # proves Docker works AND the registry is reachable -sw_vers && uname -m # note the arch for Step 2 -``` - ---- - -## Step 2 — register the runner - -You need **admin on the repo** to add a runner. +Needs **admin on the repo**. ```bash -# 1. Get a registration token (valid ~1 hour) gh api -X POST repos/PlaceOS/user-interfaces/actions/runners/registration-token --jq .token -# 2. Download the runner. Pick the arch from `uname -m`: -# arm64 -> osx-arm64 x86_64 -> osx-x64 mkdir -p ~/actions-runner && cd ~/actions-runner -RUNNER_VERSION=2.336.0 -RUNNER_ARCH=osx-arm64 # or osx-x64 -curl -o runner.tar.gz -L \ - "https://github.com/actions/runner/releases/download/v${RUNNER_VERSION}/actions-runner-${RUNNER_ARCH}-${RUNNER_VERSION}.tar.gz" +V=2.336.0 +ARCH=osx-x64 # Intel. Apple Silicon: osx-arm64 (check with `uname -m`) +curl -sS -o runner.tar.gz -L --retry 3 --retry-all-errors \ + "https://github.com/actions/runner/releases/download/v${V}/actions-runner-${ARCH}-${V}.tar.gz" tar xzf runner.tar.gz && rm runner.tar.gz -# 3. Configure. The LABEL IS LOAD-BEARING — the workflow targets `placeos-e2e`. -./config.sh \ +# The LABEL IS LOAD-BEARING — the workflow targets `placeos-e2e`. +./config.sh --unattended --replace \ --url https://github.com/PlaceOS/user-interfaces \ - --token \ - --name placeos-e2e-mac \ - --labels placeos-e2e \ - --work _work + --token --name placeos-e2e-mac --labels placeos-e2e --work _work ``` -(Or generate the token via **Settings → Actions → Runners → New self-hosted -runner**, which shows the same commands pre-filled.) - -The default `self-hosted` label is added automatically; `runs-on` in the workflow -is `[self-hosted, placeos-e2e]`. +> `svc.sh` is **not** in the tarball — `config.sh` generates it. Register before you +> try to install the service. ---- - -## Step 3 — run it as a service, and the three macOS gotchas +### 3. Install the service ```bash cd ~/actions-runner -./svc.sh install -./svc.sh start -./svc.sh status +# The launchd agent gets a minimal environment; brew's bin dir must be on PATH or +# jobs fail with `docker: command not found`. +echo 'PATH=/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin' >> .env +./svc.sh install && ./svc.sh start && ./svc.sh status ``` -Then confirm it shows **Idle** under Settings → Actions → Runners. - -**These are the things that actually catch people out on macOS:** +Confirm it shows **Idle** under Settings → Actions → Runners. -1. **Docker Desktop needs a logged-in GUI session.** The runner service is a - launchd *agent*, and Docker Desktop is a GUI app — neither runs when nobody is - logged in. If the Mac is headless, enable automatic login (System Settings → - Users & Groups → Automatic login) and leave the session logged in. A locked - screen is fine; a logged-out one is not. +Watching it interactively while setting up? Use `./run.sh` instead — same behaviour, +logs in front of you, Ctrl-C to stop. -2. **A sleeping Mac runs no nightlies.** Prevent system sleep: - - ```bash - sudo pmset -a sleep 0 # never sleep the system - sudo pmset -a disablesleep 1 # laptops: also ignore lid close - pmset -g | grep -E 'sleep|disablesleep' - ``` +--- - Display sleep is fine and worth leaving on. +## What the workflow does differently because it is self-hosted -3. **The service gets a minimal environment.** If a job reports `docker: command - not found`, the service cannot see Docker's CLI. Fix by adding it to the - runner's env file, then restarting the service: +| Choice | Why | +|---|---| +| **No `actions/cache`, `setup-node` or `setup-bun`** | Those are right on a hosted runner that starts from a bare VM. Here they were the biggest cost in the job: the first run spent **9.1 min** uploading `node_modules` and **3.1 min** saving the bun cache out of 19.2 min total, while the suite itself took 1.0 min. The workspace and toolchain persist on this machine, so the toolchain is provisioned once (step 1) and nothing is shipped to a remote cache. | +| **Reclaim step** | A self-hosted machine is not a fresh VM. A previous aborted run can leave the stack up or port 4214 held, and that failure looks nothing like its cause. | +| **No `vm.max_map_count` bump** | Required on hosted Linux, meaningless here — the value lives in Colima's VM and is already high. Restore it if reverting to `ubuntu-latest`. | +| **`E2E_WORKERS: 2`** | Lower than the local default of 4. The machine also hosts ~10 containers, a dev server and browsers; less contention means fewer failures that are about the machine rather than the code. | +| **No `pull_request` trigger** | This repo is **public**. A fork PR would run arbitrary code on a machine on an internal network. Every trigger requires write access. Do not add it back without moving to `ubuntu-latest` or making the repo private. | - ```bash - cd ~/actions-runner - echo 'PATH=/usr/local/bin:/opt/homebrew/bin:/usr/bin:/bin:/usr/sbin:/sbin' >> .env - # add proxy vars here too, if you needed them in Step 0 - ./svc.sh stop && ./svc.sh start - ``` +**Trade-off worth knowing:** dropping remote caching means runs are no longer hermetic +— machine state matters. `bun install --frozen-lockfile` still runs every job and will +repair a broken `node_modules`, and the reclaim step handles stale containers, but if +something inexplicable happens, machine state is the first thing to suspect. -Prefer to watch it interactively while setting up? Skip the service and run -`./run.sh` in a terminal — same behaviour, logs in front of you, stops on Ctrl-C. +**Measured:** ~2.8 min job execution, ~3 min wall clock. The suite is ~60s of that. --- -## Step 4 — trigger a run - -The workflow already targets the runner. Nothing to edit. +## Operating it ```bash -# From a clone of the repo, on the e2e branch: -git commit --allow-empty -m "ci: exercise the self-hosted runner" && git push -gh run watch +cd ~/actions-runner +./svc.sh status # is it running +tail -f _diag/Runner_*.log # runner logs +cd /e2e/stack && ./down.sh --volumes # reset the stack by hand +docker system prune -a --volumes # reclaim disk (stack rebuilds from nothing) ``` -The `push: ['e2e/**']` trigger fires on any branch under `e2e/`. -`workflow_dispatch` also works, but only once this file is on `develop`. - -**Expected on a first run:** ~10–15 min, most of it pulling ~10 Docker images. Then -~4–5 min per run once the images are cached locally. - ---- - -## Step 5 — the nightly needs a merge to `develop` - -> **GitHub only runs `schedule` triggers from the default branch.** The nightly -> will not fire from `e2e/ci-verify` no matter what the cron says. +### Known limitation: it will not survive a reboot unattended -So the track record does not start accumulating until this workflow is merged to -`develop`. Until then, use pushes or manual dispatch. Worth knowing, because "the -nightly silently never ran" is an easy thing to not notice for a fortnight. +Colima and the runner are both launchd **agents**, which need a logged-in user +session, and there is currently **no auto-login**. After a reboot nothing starts until +someone logs in. Accepted for now because the machine stays on; the fix when needed is +either to enable auto-login or to convert both to system-level LaunchDaemons. ---- - -## Security posture — why there is no `pull_request` trigger - -`PlaceOS/user-interfaces` is a **public** repo, and GitHub's own guidance is not to -use self-hosted runners with public repositories: a `pull_request` trigger lets -anyone's fork run arbitrary code on the runner. Here that runner is a machine on an -internal network, so the blast radius is the network, not just a throwaway VM. - -The workflow therefore triggers **only** on `schedule`, `workflow_dispatch`, and -pushes to `e2e/**` — all of which require write access to this repo. There is no -path for fork code to reach the machine. - -**Do not add `pull_request` back** without either moving the job to -`runs-on: ubuntu-latest` or making the repo private. - -Worth doing as defence in depth, in case someone adds it later: - -- **Settings → Actions → General → Fork pull request workflows** → require approval - for **all** outside collaborators. -- Run the runner under a **dedicated macOS user account** with no access to - credentials or source you care about, and keep no secrets on the machine. The - workflow needs none. - ---- +There is also no ordering between the two agents, so "runner online but Docker not +ready yet" is possible on a cold start — untested, and it would present as a +stack-bring-up failure. ## Troubleshooting | Symptom | Cause / fix | |---|---| -| Runner shows **Offline** | Service not running (`./svc.sh status`), machine asleep, or outbound 443 blocked — re-run Step 0. Note this is about the runner's *outbound* connection; whether anyone can reach the machine is unrelated. | -| Can't ping / SSH the machine from elsewhere on the network | Expected and harmless for CI. macOS has Remote Login off by default and its firewall drops ICMP; some wifi also enforces client isolation. Only affects direct administration. | -| `docker: command not found` | Service PATH — gotcha 3 above. | -| Job hangs on **Bring up the local PlaceOS stack** | Docker Desktop not running (no GUI session — gotcha 1), or registry unreachable. `up.sh` prints `compose ps` plus every service's logs on failure, so read the step output first. | -| `port is already allocated` / webServer port in use | A previous run died. The **Reclaim the machine** step handles this automatically; if it recurs, run `e2e/stack/down.sh --volumes` by hand. | -| `elastic-1 is unhealthy` | Give Docker Desktop more memory. Elasticsearch is pinned to 7.17.28 specifically to avoid the cgroup v2 JDK crash — do not downgrade it. | -| Disk filling up | Volumes accumulate across runs. `docker system prune -a --volumes` (safe — the stack is rebuilt from nothing each run). | -| Two runs at once | One runner takes one job at a time; the second queues. That is the desired behaviour: the stack binds fixed ports and cannot run twice. | +| Runner **Offline** | Service stopped (`./svc.sh status`), machine asleep or rebooted (see above), or outbound 443 blocked. Unrelated to whether anyone can reach the machine. | +| `docker: command not found` | Service PATH — step 3. | +| `docker compose` not a valid command | The compose plugin is not registered — step 1. | +| Hangs on **Bring up the local PlaceOS stack** | `up.sh` prints `compose ps` plus every service's logs on failure; read the step output first. Usually Colima not running (`colima status`) or the registry unreachable. | +| `port is already allocated` | Previous run died. The reclaim step handles it; otherwise `./down.sh --volumes`. | +| `elastic-1 is unhealthy` | Give Colima more memory. ES is pinned to 7.17.28 to avoid the cgroup v2 JDK crash — do not downgrade. | +| Can't SSH the machine from the LAN | Expected and irrelevant to CI. It sits on a different subnet with inter-VLAN traffic filtered; use Tailscale for admin. | ## Reverting to GitHub-hosted -In `.github/workflows/e2e-advisory.yml`: - 1. `runs-on: [self-hosted, placeos-e2e]` → `runs-on: ubuntu-latest` -2. Restore the `sysctl -w vm.max_map_count=262144` step (required on Linux, a no-op - on macOS — see the comment in the workflow). -3. The **Reclaim the machine** step becomes unnecessary but is harmless. +2. Restore the `sudo sysctl -w vm.max_map_count=262144` step. +3. Restore `actions/cache` + `setup-node` + `setup-bun` (a bare VM needs them). +4. The reclaim step becomes unnecessary but harmless. diff --git a/e2e/support/repro/reg09-concurrent-bookings.ts b/e2e/support/repro/reg09-concurrent-bookings.ts index ab95a3fb43..cdc1cefab1 100644 --- a/e2e/support/repro/reg09-concurrent-bookings.ts +++ b/e2e/support/repro/reg09-concurrent-bookings.ts @@ -20,7 +20,7 @@ * occasional 409s across repeated runs are real clashes with leftovers, not the bug. */ import { request } from '@playwright/test'; -import { mintToken } from './e2e/support/auth'; +import { mintToken } from '../auth'; const B = 'https://localhost:9443'; const CONCURRENCY = Number(process.env.N ?? 6); From 6e7b6169d91421bc03ac6562e4d75229e6973ff7 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 30 Jul 2026 15:06:47 +1000 Subject: [PATCH 11/17] docs(e2e): correct a stale Docker Desktop reference to Colima MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Caught reviewing my own diff — the README still attributed vm.max_map_count to Docker Desktop's VM after the switch to Colima. Co-Authored-By: Claude Opus 5 (1M context) --- e2e/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/README.md b/e2e/README.md index 658980b565..aaa8cdb688 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -225,7 +225,7 @@ CI-specific choices worth knowing: | `E2E_WORKERS: 2` | Lower than the local default of 4. The runner is also hosting ~10 containers, a dev server and browsers. Less contention means fewer failures that are about the machine rather than the code — avoiding false positives matters more than run time here. | | No `pull_request` trigger | This repo is public and the runner is self-hosted, so a fork PR could run arbitrary code on an internal machine. Triggers are limited to ones requiring write access. Do not add it back without moving to `ubuntu-latest`. | | Reclaim step | A self-hosted machine is not a fresh VM; a previous aborted run can leave the stack up or port 4214 held. | -| No `vm.max_map_count` bump | Needed on GitHub-hosted Linux, meaningless on macOS (the value lives in Docker Desktop's VM). Restore it if reverting to `ubuntu-latest`. | +| No `vm.max_map_count` bump | Needed on GitHub-hosted Linux, meaningless on macOS — the value lives inside Colima's VM, which already sets it to 1048576. Restore it if reverting to `ubuntu-latest`. | | Network access to GitHub | `up.sh` clones `PlaceOS/www-core` into the `www` volume once — that is where the platform `/login` page comes from. | ## Booking specs: what the backend actually does From 2854817e2fe17170dc600676fe249bcbe6623803 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Fri, 31 Jul 2026 13:49:45 +1000 Subject: [PATCH 12/17] ci(e2e): notify on failure, and flag a flaky-but-green run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A nightly result nobody sees is not a result. Adds a failure notification using the same chat integration PR Flow already uses, so there is nothing new to configure — repoint CHAT_URL to change the recipient. Only fires on failure. A green run stays silent, or the signal gets tuned out within a fortnight. continue-on-error so a broken webhook can never turn a green run red. Flaky runs pass by conclusion, so nothing notifies. The step summary now says so explicitly instead — that is the number that tells us whether the suite can be trusted yet, and the easiest one to quietly stop looking at. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/e2e-advisory.yml | 16 ++++++++++++++++ e2e/stack/ci-summary.mjs | 9 +++++++++ 2 files changed, 25 insertions(+) diff --git a/.github/workflows/e2e-advisory.yml b/.github/workflows/e2e-advisory.yml index b51f20e93c..033188d6d1 100644 --- a/.github/workflows/e2e-advisory.yml +++ b/.github/workflows/e2e-advisory.yml @@ -195,3 +195,19 @@ jobs: - name: Tear down the stack if: ${{ always() }} run: e2e/stack/down.sh --volumes + + # Tell someone. A nightly result nobody sees is not a result. + # + # Only fires on failure — a green run should be silent, or the signal + # gets tuned out within a fortnight. Uses the same chat integration as + # PR Flow, so there is nothing new to configure; change the recipient by + # repointing the CHAT_URL secret, or swap this step for email/Slack. + # + # `continue-on-error` so a broken webhook can never turn a green run red. + - name: Notify on failure + if: ${{ failure() }} + uses: fjogeleit/http-request-action@master + continue-on-error: true + with: + url: ${{ secrets.STATUS_URL }} + data: '{ "chat_url": "${{ secrets.CHAT_URL }}", "name": "${{ github.repository }}", "commit": "${{ github.sha }}", "pipeline_name": "e2e (advisory) — FAILED", "branch": "${{ github.ref_name }}", "status": "failure", "url": "https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" }' diff --git a/e2e/stack/ci-summary.mjs b/e2e/stack/ci-summary.mjs index 7a787788dd..2e0c884469 100644 --- a/e2e/stack/ci-summary.mjs +++ b/e2e/stack/ci-summary.mjs @@ -98,6 +98,15 @@ if (failed) { line(); } +// A flaky run is a PASS by conclusion, so nothing notifies. Make it visible in +// the summary instead — this is the number that says whether the suite can be +// trusted yet, and it is the easiest one to quietly stop looking at. +if (flaky && !failed) { + line('> **This run passed, but not cleanly.** Someone should look at the flaky'); + line('> spec above before it becomes background noise.'); + line(); +} + line('---'); line(); line( From a06d1bbe7a85754f6c631dc18e539fc5fa0a7556 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Mon, 3 Aug 2026 15:41:25 +1000 Subject: [PATCH 13/17] ci(e2e): health check after idle period Co-Authored-By: Claude Opus 5 (1M context) From c0c4c695fe6802a0e42dc054d09610e3eb8fd1bc Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Wed, 5 Aug 2026 14:40:20 +1000 Subject: [PATCH 14/17] =?UTF-8?q?test(e2e):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20least-privilege=20CI,=20backend-free=20mock=20project,=20boo?= =?UTF-8?q?t=20assertions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merges develop to pick up #478 (the PPT-2643 fix) and applies the review feedback on #476. CI hardening. The job now declares `permissions: contents: read` and checks out without persisting credentials: the repository default is `write`, and this is the only job on a self-hosted machine, where a token written to disk outlives the run. The failure notification drops `fjogeleit/http-request-action@master` for plain curl — same request, but a mutable ref should not execute on a box inside the internal network. The six other call sites are the same pattern on GitHub-hosted runners and are left for a repo-wide change. Preflight becomes a setup project that only `local` depends on, instead of a `globalSetup` that ran for every invocation. `--project=mock` is documented as needing no backend and now genuinely does not — verified by running it with the stack down. A missing stack also reports as a named failing test rather than a runner crash. The boot smoke test asserts on unexpected server errors instead of only logging them; a required endpoint could previously start failing while the test stayed green. The tolerated set is derived from an observed CI run rather than from documentation, scoped to 5xx and to same-origin API paths. Records the backend inputs (resolved image IDs, www-core HEAD) in the job summary so a changed nightly is attributable, and pins keydb by multi-arch digest. The PlaceOS services stay on ${PLACEOS_TAG} deliberately: catching backend regressions is what an advisory nightly is for, and PLACEOS_TAG is already the knob for pinning when bisecting. Documents that mock is Chromium-only on purpose, and corrects the CI section of the README, which described a `pull_request` trigger that deliberately does not exist. Co-Authored-By: Claude Fable 5 --- .github/workflows/e2e-advisory.yml | 90 +++++++++++++++++++++++++-- E2E_USER_STORIES.md | 14 +++-- apps/workplace/e2e/local/boot.spec.ts | 41 +++++++++++- apps/workplace/playwright.config.ts | 23 ++++++- e2e/README.md | 16 +++-- e2e/stack/docker-compose.yml | 11 +++- e2e/support/flows.ts | 2 +- e2e/support/global-setup.ts | 15 ----- e2e/support/preflight.setup.ts | 18 ++++++ 9 files changed, 194 insertions(+), 36 deletions(-) delete mode 100644 e2e/support/global-setup.ts create mode 100644 e2e/support/preflight.setup.ts diff --git a/.github/workflows/e2e-advisory.yml b/.github/workflows/e2e-advisory.yml index 033188d6d1..954a865544 100644 --- a/.github/workflows/e2e-advisory.yml +++ b/.github/workflows/e2e-advisory.yml @@ -71,6 +71,14 @@ concurrency: group: e2e-${{ github.workflow }}-${{ github.event_name }}-${{ github.ref }} cancel-in-progress: true +# The repository default is `write`. This job only reads the code and uploads +# artifacts (upload-artifact uses the Actions runtime token, not this one), so +# it has no reason to hold a token that can push. It matters more here than in +# the other workflows: this is the only job on a self-hosted machine, and that +# machine is persistent — a token written to its disk outlives the run. +permissions: + contents: read + jobs: workplace: # The job name is what appears in the commit's Checks list, next to the @@ -95,7 +103,11 @@ jobs: E2E_WORKERS: '2' TZ: 'Etc/UTC' steps: + # persist-credentials: false — checkout otherwise leaves the token in + # .git/config. Nothing here pushes, and this runner is not a fresh VM. - uses: actions/checkout@v6 + with: + persist-credentials: false # No actions/cache, no setup-node, no setup-bun — on purpose. # @@ -146,6 +158,42 @@ jobs: - name: Bring up the local PlaceOS stack run: e2e/stack/up.sh + # What backend did this run actually test? + # + # The PlaceOS services track ${PLACEOS_TAG:-latest} on purpose — an + # advisory nightly against current backends is worth having, and + # pinning them would make the suite blind to the regressions it is + # best placed to catch. The cost is that a result can change with no + # frontend commit, so record the inputs: with these IDs a changed + # nightly is attributable in seconds instead of guessed at. To pin + # deliberately (e.g. while bisecting a frontend regression), set + # PLACEOS_TAG in this job's `env` above — it is already threaded + # through every service. + - name: Record backend inputs + run: | + { + echo '### Backend inputs' + echo + echo '| image | id |' + echo '|---|---|' + docker compose -p placeos-e2e -f e2e/stack/docker-compose.yml images \ + --format json 2>/dev/null \ + | node -e ' + let raw = ""; + process.stdin.on("data", (d) => (raw += d)); + process.stdin.on("end", () => { + let rows = []; + try { rows = JSON.parse(raw); } catch { return; } + for (const r of rows) { + const tag = `${r.Repository}:${r.Tag}`; + console.log(`| ${tag} | \`${(r.ID || "").slice(0, 19)}\` |`); + } + }); + ' + echo + echo "www-core HEAD: \`$(git ls-remote https://github.com/PlaceOS/www-core HEAD | cut -f1)\`" + } >> "$GITHUB_STEP_SUMMARY" + - name: Run workplace e2e run: bunx playwright test --config apps/workplace/playwright.config.ts @@ -204,10 +252,44 @@ jobs: # repointing the CHAT_URL secret, or swap this step for email/Slack. # # `continue-on-error` so a broken webhook can never turn a green run red. + # + # Plain curl rather than the `fjogeleit/http-request-action@master` + # step the other workflows use. Same request, no third-party code: + # a mutable `@master` ref means whatever that repository holds at the + # moment the job runs executes here, and unlike build.yml and + # pull-request.yml, "here" is a persistent machine on an internal + # network. The secrets go through `env` rather than into the command + # line so they are not exposed in the process list. + # + # The other six call sites are the same pattern on GitHub-hosted + # runners; worth pinning too, but that is a repo-wide change and does + # not belong in this PR. - name: Notify on failure if: ${{ failure() }} - uses: fjogeleit/http-request-action@master continue-on-error: true - with: - url: ${{ secrets.STATUS_URL }} - data: '{ "chat_url": "${{ secrets.CHAT_URL }}", "name": "${{ github.repository }}", "commit": "${{ github.sha }}", "pipeline_name": "e2e (advisory) — FAILED", "branch": "${{ github.ref_name }}", "status": "failure", "url": "https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" }' + env: + STATUS_URL: ${{ secrets.STATUS_URL }} + CHAT_URL: ${{ secrets.CHAT_URL }} + REPO: ${{ github.repository }} + COMMIT: ${{ github.sha }} + BRANCH: ${{ github.ref_name }} + RUN_ID: ${{ github.run_id }} + run: | + # node, not jq — jq is not on a stock macOS runner, node is + # provisioned (see SELF_HOSTED_RUNNER.md). + node -e ' + process.stdout.write(JSON.stringify({ + chat_url: process.env.CHAT_URL, + name: process.env.REPO, + commit: process.env.COMMIT, + branch: process.env.BRANCH, + url: `https://github.com/${process.env.REPO}/actions/runs/${process.env.RUN_ID}`, + pipeline_name: "e2e (advisory) — FAILED", + status: "failure", + })); + ' \ + | curl --silent --show-error --fail \ + --max-time 30 \ + --header 'Content-Type: application/json' \ + --data @- \ + "$STATUS_URL" diff --git a/E2E_USER_STORIES.md b/E2E_USER_STORIES.md index f372908bb4..32b83b64c2 100644 --- a/E2E_USER_STORIES.md +++ b/E2E_USER_STORIES.md @@ -158,7 +158,7 @@ task that found it, so the row can be traced. | REG-07 | P2 | Level selection does not persist once the selector is hidden/disabled. | "Fix level selections persisting when selector is disabled/hidden" | todo | | REG-08 | P1 | An authority with a **relative** `login_url` still reaches a usable login page. | Found 2026-07-30, this suite | **blocked** — currently worked around in `seed.ts`; ts-client resolves a relative `login_url` against the authority host **without its port**, so any non-443 deployment dead-ends. Needs a ts-client/init fix before a spec can assert the good behaviour. | | REG-09 | P1 | Concurrent `POST /bookings` do not 500. | **[PPT-2642](https://acaprojects.atlassian.net/browse/PPT-2642)** | **blocked** — staff-api raises `DB::ConnectionLost` under concurrency. Observed ~1 run in 8 **locally at 4 workers**; **not yet observed in CI**, which runs 2 workers, so halving the concurrency may simply be avoiding it rather than the problem being absent. Needs a staff-api fix; do not treat the quiet CI record as evidence it is gone. | -| REG-10 | P1 | The booking form does not discard user input while it is still initialising. | **[PPT-2643](https://acaprojects.atlassian.net/browse/PPT-2643)** | **blocked** — the form is rebuilt when async init completes and restores defaults (title, All Day, Require locker), silently dropping anything typed before that. A real user can hit this; they would just see their title or options revert. `bookDeskViaUI` converges on the state to work around it, which means **the suite no longer detects it** — hence this row. Needs a fix in the app, then a spec that asserts input survives. | +| REG-10 | P1 | The booking form does not discard user input while it is still initialising. | **[PPT-2643](https://acaprojects.atlassian.net/browse/PPT-2643)** | **blocked** — the form is rebuilt when async init completes and restores defaults (title, All Day, Require locker), silently dropping anything typed before that. A real user can hit this; they would just see their title or options revert. `bookDeskViaUI` converges on the state to work around it, which means **the suite no longer detects it** — hence this row. **PPT-2643's fix (#478, `a0360486`) is NOT sufficient for this route.** Measured on 2026-08-05 with that fix present: removing the converging block turns desk-booking RED at `all_day.isChecked()`, the same assertion and the same symptom as before the fix. The same experiment against pre-fix code fails identically, so the flow *is* a genuine detector — the app behaviour simply has not changed on this path. The workaround therefore stays until the remaining path is fixed. | ## 4. Platform & configuration @@ -186,9 +186,15 @@ Config gaps caused several production incidents, and they are invisible to UI sp - **REG-09 is worse than a flake.** Filed as PPT-2642: one burst of concurrent booking POSTs permanently poisons staff-api's connection pool, so booking creation returns 500 for everyone until the service restarts. Reproducer kept at `e2e/support/repro/reg09-concurrent-bookings.ts`. -- **REG-10 is invisible to this suite by design.** `bookDeskViaUI` works around it, so nothing - here will catch it regressing. Filed as PPT-2643; when it is fixed, remove the workaround and - replace it with a spec asserting input survives initialisation. +- **REG-10 is invisible to this suite by design, and is still not fixed.** `bookDeskViaUI` works + around it, so nothing here will catch it regressing. PPT-2643's fix landed in #478 and does not + cover the desk-booking route — verified by deleting the workaround with the fix present and + watching desk-booking fail on the original symptom (2026-08-05). The removal is a two-line + change and the flow is a proven detector, so the moment the app path is genuinely fixed this + becomes trivial: delete the converging block, confirm green, delete this note. + A synthetic "type during init and assert it survives" spec was tried and deliberately NOT kept — + it passed with *and* without the fix, because the rebuild is a race that a warm local run wins. + A spec that cannot fail is worse than no spec. - **AUTH-E2E-08 may never be automatable** with Playwright Chromium. Say so rather than quietly dropping it. - The PPT-2536 harnesses (`tasks/PPT-2536/{e2e,integration}`) still hold assertions that diff --git a/apps/workplace/e2e/local/boot.spec.ts b/apps/workplace/e2e/local/boot.spec.ts index 237523c9a9..050484c39c 100644 --- a/apps/workplace/e2e/local/boot.spec.ts +++ b/apps/workplace/e2e/local/boot.spec.ts @@ -17,13 +17,40 @@ import { APP_URL } from '../../../../e2e/support/env'; import { clientId, redirectUriFor } from '../../../../e2e/support/auth'; import { currentUser, zonesWithTag } from '../../../../e2e/support/api'; +/** + * Boot-time 5xx this suite tolerates, derived from an observed run rather than + * from documentation — the CI log for this spec listed exactly one entry. + * + * `/calendars` (and `/events`, which boot does not currently reach) are the + * routes that genuinely call Microsoft/Google. The stack seeds a staff-api + * tenant with placeholder credentials, which is what lets every PlaceOS-native + * booking route work locally; these two 500 on `AADSTS900023` by design. Making + * them real would mean putting live Microsoft credentials in the default suite. + * + * Add to this list only with the failing URL in hand and a reason next to it. + */ +const TOLERATED_5XX = ['/api/staff/v1/calendars', '/api/staff/v1/events']; + test.describe('workplace boots against the local backend', () => { test('the seeded token authenticates the SPA and org data resolves', async ({ page }) => { const failed_api: string[] = []; + const unexpected_5xx: string[] = []; page.on('response', (r) => { - const url = r.url(); - if ((url.includes('/api/') || url.includes('/auth/')) && r.status() >= 400) { - failed_api.push(`${r.status()} ${r.request().method()} ${new URL(url).pathname}`); + const url = new URL(r.url()); + // Same-origin only. A substring test for `/api/` would also match a + // third-party URL that happens to contain that segment. + const ours = url.origin === new URL(APP_URL).origin; + const backend_path = + url.pathname.startsWith('/api/') || url.pathname.startsWith('/auth/'); + if (!ours || !backend_path || r.status() < 400) return; + + const entry = `${r.status()} ${r.request().method()} ${url.pathname}`; + failed_api.push(entry); + // Only 5xx is asserted on. A 4xx during boot is frequently benign — + // probes for optional resources, permission-shaped answers for a + // non-admin — whereas a 5xx means a backend route is broken. + if (r.status() >= 500 && !TOLERATED_5XX.some((p) => url.pathname.startsWith(p))) { + unexpected_5xx.push(entry); } }); @@ -52,6 +79,14 @@ test.describe('workplace boots against the local backend', () => { if (failed_api.length) { console.log(' 4xx/5xx API calls during boot:\n ', failed_api.join('\n ')); } + + // The list above used to be logged and nothing more, so a required + // endpoint could start 500ing and this stayed green as long as the shell + // still rendered. Anything outside TOLERATED_5XX now fails. + expect( + unexpected_5xx, + 'server errors during boot outside the known-tolerated set', + ).toEqual([]); }); // Deliberately asserted as the NON-admin worker identity: a sys_admin can see diff --git a/apps/workplace/playwright.config.ts b/apps/workplace/playwright.config.ts index 4651459dd8..d497f2671f 100644 --- a/apps/workplace/playwright.config.ts +++ b/apps/workplace/playwright.config.ts @@ -16,7 +16,6 @@ const PORT = new URL(APP_URL).port || '4214'; export default defineConfig({ ...nxE2EPreset(__filename, { testDir: './e2e' }), - globalSetup: require.resolve('../../e2e/support/global-setup'), timeout: 90_000, expect: { timeout: 15_000 }, // We own the whole stack, so isolation is a seeding problem, not a reason to @@ -50,18 +49,38 @@ export default defineConfig({ navigationTimeout: 45_000, }, projects: [ + // --- preflight: prove the stack is up, for `local` only ------------- + // A setup project rather than `globalSetup`, which would also run for + // `mock` and make a backend-free project need a backend. + { + name: 'preflight', + testDir: '../../e2e/support', + testMatch: /preflight\.setup\.ts$/, + use: { ...devices['Desktop Chrome'] }, + }, + // --- real local backend, no mocks ---------------------------------- // Auth comes from the worker-scoped fixtures in e2e/support/fixtures.ts, - // so there is no setup project to serialise behind. + // so the only thing to wait on is the stack itself. { name: 'local', testDir: './e2e/local', + dependencies: ['preflight'], use: { ...devices['Desktop Chrome'] }, }, // --- mock mode: no backend at all, fast render regression ---------- // Kept deliberately. It costs no infrastructure and catches pure UI // breakage, so it stays as its own project rather than being replaced. + // + // Chromium only. The projects this replaced also listed Firefox and + // WebKit, but that was untouched Nx generator scaffold that no script, + // workflow or human ever invoked, and landing.spec.ts only asserts that + // Angular components instantiate — the same code path in every engine. + // Paying two more browser downloads and ~2x runtime on one serialised + // self-hosted runner for that is a bad trade. If cross-browser earns its + // place later, add `mock-firefox`/`mock-webkit` behind an env flag so the + // nightly is unaffected. { name: 'mock', testDir: './e2e', diff --git a/e2e/README.md b/e2e/README.md index aaa8cdb688..ef6414643b 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -82,11 +82,11 @@ e2e/ shared engine, all apps api.ts engine/staff-api helpers + asset sweep flows.ts multi-step UI flows (bookDeskViaUI) preflight.ts "is the stack up?" — fails in 1s, not 90 - global-setup.ts runs preflight once per suite + preflight.setup.ts setup project — `local` depends on it, `mock` does not seed.ts idempotent API-driven seeding .env.example copy to .env to override anything apps/workplace/ - playwright.config.ts projects: local, mock + playwright.config.ts projects: preflight, local, mock e2e/ local/*.spec.ts real-backend specs *.spec.ts mock-mode specs (no backend) @@ -205,10 +205,14 @@ depend on an external service, which is the one thing it is designed not to do. ## CI -`.github/workflows/e2e-advisory.yml` — runs on PRs into `develop`, nightly at 15:10 UTC, -and via **Run workflow**. It brings up the isolated stack, seeds it, runs the suite, writes -a step summary, and uploads the HTML report plus (on failure) traces, videos and backend -container logs. +`.github/workflows/e2e-advisory.yml` — runs nightly at **01:10 UTC**, on pushes to `e2e/**`, +and via **Run workflow**. There is deliberately **no `pull_request` trigger** (see the table +below — it is a security decision, not an oversight). It brings up the isolated stack, seeds +it, runs the suite, writes a step summary, and uploads the HTML report plus (on failure) +traces, videos and backend container logs. + +`schedule` only fires from the default branch, so the nightly does not start accumulating a +track record until this lands on `develop`. **It is advisory and must stay that way for now** — do not add it to branch protection or required status checks until the suite has a track record. See the CI section of diff --git a/e2e/stack/docker-compose.yml b/e2e/stack/docker-compose.yml index 59915a98e1..d86ed4a56b 100644 --- a/e2e/stack/docker-compose.yml +++ b/e2e/stack/docker-compose.yml @@ -130,7 +130,16 @@ services: TZ: ${TZ:-UTC} redis: - image: eqalpha/keydb + # Pinned: keydb is not the system under test, so a moving tag here can only + # ever add noise. The PlaceOS services deliberately stay on ${PLACEOS_TAG} + # (see up.sh) because catching backend regressions is the point of the + # nightly — but that argument does not apply to infrastructure. + # + # By digest, not tag: upstream publishes only arch-specific version tags + # (`x86_64_v6.3.4`, `arm64_v6.3.4`), and pinning one would break either the + # Intel runner or an Apple Silicon laptop. This is the multi-arch manifest + # list for `latest` as of 2026-08-05 — which upstream last moved in 2023. + image: eqalpha/keydb@sha256:6537505c42355ca1f571276bddf83f5b750f760f07b2a185a676481791e388ac hostname: redis restart: always <<: *std-logging diff --git a/e2e/support/flows.ts b/e2e/support/flows.ts index 946e70b46a..0458db09fa 100644 --- a/e2e/support/flows.ts +++ b/e2e/support/flows.ts @@ -22,7 +22,7 @@ import { Page, expect } from '@playwright/test'; * hidden native input rather than assuming a default. The visible `mat-checkbox` * is the click target; the `input` is where `checked` actually lives. */ -async function setCheckbox(page: Page, label: string, want: boolean): Promise { +export async function setCheckbox(page: Page, label: string, want: boolean): Promise { const box = page.locator(`mat-checkbox:has-text("${label}")`); if (!(await box.count())) return; const input = box.first().locator('input[type="checkbox"]'); diff --git a/e2e/support/global-setup.ts b/e2e/support/global-setup.ts deleted file mode 100644 index caf7b959d3..0000000000 --- a/e2e/support/global-setup.ts +++ /dev/null @@ -1,15 +0,0 @@ -/** - * Runs once before the whole suite: prove the local stack is actually there. - * - * The stack is a precondition, not something Playwright starts — it is ~20 - * containers and Elasticsearch alone wants several GB, so a `webServer` block - * would be the wrong tool. Failing here costs a second and says what to do; - * failing later costs every spec a navigation timeout and says nothing. - */ -import { APP_URL, BACKEND_URL, assertLocalOnly } from './env'; -import { assertStackUp } from './preflight'; - -export default async function globalSetup(): Promise { - assertLocalOnly(BACKEND_URL, APP_URL); - await assertStackUp(); -} diff --git a/e2e/support/preflight.setup.ts b/e2e/support/preflight.setup.ts new file mode 100644 index 0000000000..0890128fed --- /dev/null +++ b/e2e/support/preflight.setup.ts @@ -0,0 +1,18 @@ +/** + * Stack preflight, as a Playwright setup project. + * + * This used to be `globalSetup`, which runs for every invocation regardless of + * `--project`. That made `--project=mock` — documented as needing no backend — + * fail when the stack was absent. A setup project is the mechanism for "only + * the projects that depend on this", so only `local` waits on it. + * + * It also reports better: a missing stack is now a named failing test with the + * message attached, rather than a runner crash before any test exists. + */ +import { test as setup } from '@playwright/test'; + +import { assertStackUp } from './preflight'; + +setup('local PlaceOS stack is up', async () => { + await assertStackUp(); +}); From be3ce21b76890da47e0199176d167920b6fbc80e Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 5 Aug 2026 15:18:12 +1000 Subject: [PATCH 15/17] test(e2e): settle boot traffic before asserting, correct preflight hint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on #476: - The boot spec asserted on collected 5xx as soon as the topbar rendered, but the shell renders before the org/zone calls finish — a slow 500 could land after the check and leave the test green. Wait for networkidle first so in-flight boot requests are counted. WebSockets do not count against networkidle, so the realtime channel cannot hang it; the 90s test timeout backstops the wait regardless. - preflight's "stack is down" message still pointed at `cd local && ./placeos start` — a developer's personal stack, not this suite's. Now points at `e2e/stack/up.sh`. Co-Authored-By: Claude Fable 5 --- apps/workplace/e2e/local/boot.spec.ts | 6 ++++++ e2e/support/preflight.ts | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/apps/workplace/e2e/local/boot.spec.ts b/apps/workplace/e2e/local/boot.spec.ts index 050484c39c..d36d969ec1 100644 --- a/apps/workplace/e2e/local/boot.spec.ts +++ b/apps/workplace/e2e/local/boot.spec.ts @@ -76,6 +76,12 @@ test.describe('workplace boots against the local backend', () => { const stored = await page.evaluate((k) => localStorage.getItem(k), `${cid}_access_token`); expect(stored, `localStorage[${cid}_access_token] should hold the bearer`).toBeTruthy(); + // Let in-flight boot requests land before asserting on them — the shell + // renders before the org/zone calls finish, so without this a slow 500 + // could arrive after the check and leave the test green. WebSockets do + // not count against networkidle, so the realtime channel cannot hang it. + await page.waitForLoadState('networkidle'); + if (failed_api.length) { console.log(' 4xx/5xx API calls during boot:\n ', failed_api.join('\n ')); } diff --git a/e2e/support/preflight.ts b/e2e/support/preflight.ts index 4c50f34685..6eead301f1 100644 --- a/e2e/support/preflight.ts +++ b/e2e/support/preflight.ts @@ -25,7 +25,7 @@ export async function assertStackUp(): Promise { } catch (e) { throw new Error( `Local PlaceOS stack is not reachable at ${BACKEND_URL} (${(e as Error).message}).\n` + - `Start it with: cd local && ./placeos start`, + `Start it with: e2e/stack/up.sh`, ); } finally { await ctx.dispose(); From aeb3dd907a4f0cd735d899c43bc1b6959cba31f7 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Wed, 5 Aug 2026 16:16:00 +1000 Subject: [PATCH 16/17] =?UTF-8?q?docs(e2e):=20correct=20the=20REG-10=20rec?= =?UTF-8?q?ord=20=E2=80=94=20no=20evidence=20#478=20is=20incomplete?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An earlier note claimed PPT-2643's fix did not cover the desk-booking route, on the strength of the suite going red when the converging block in bookDeskViaUI was deleted. That was wrong twice over, and both errors are recorded so the next person does not repeat them. The first red was a Playwright strict-mode violation rather than a reverted value: opening the desk-select modal puts a second "All Day" checkbox in the DOM, bound to the same form field, so an unscoped locator matched two elements and threw — with both of them checked. Scoping the locator to desk-flow-form then broke it a second way, because setCheckbox returns silently when its locator matches nothing, so a narrower scope became a no-op and the form really was invalid. The shipped helper is unchanged and stable: six consecutive full runs, 8/8, at --retries=0. Whether the fix is complete remains unproven in either direction — the race needs a slower machine than this one — so the block stays and REG-10 stays blocked, now with the two dead ends written down and a note that settling it needs throttled CPU or a unit test, not another e2e attempt. Co-Authored-By: Claude Fable 5 --- E2E_USER_STORIES.md | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/E2E_USER_STORIES.md b/E2E_USER_STORIES.md index 32b83b64c2..b411fa9c90 100644 --- a/E2E_USER_STORIES.md +++ b/E2E_USER_STORIES.md @@ -158,7 +158,7 @@ task that found it, so the row can be traced. | REG-07 | P2 | Level selection does not persist once the selector is hidden/disabled. | "Fix level selections persisting when selector is disabled/hidden" | todo | | REG-08 | P1 | An authority with a **relative** `login_url` still reaches a usable login page. | Found 2026-07-30, this suite | **blocked** — currently worked around in `seed.ts`; ts-client resolves a relative `login_url` against the authority host **without its port**, so any non-443 deployment dead-ends. Needs a ts-client/init fix before a spec can assert the good behaviour. | | REG-09 | P1 | Concurrent `POST /bookings` do not 500. | **[PPT-2642](https://acaprojects.atlassian.net/browse/PPT-2642)** | **blocked** — staff-api raises `DB::ConnectionLost` under concurrency. Observed ~1 run in 8 **locally at 4 workers**; **not yet observed in CI**, which runs 2 workers, so halving the concurrency may simply be avoiding it rather than the problem being absent. Needs a staff-api fix; do not treat the quiet CI record as evidence it is gone. | -| REG-10 | P1 | The booking form does not discard user input while it is still initialising. | **[PPT-2643](https://acaprojects.atlassian.net/browse/PPT-2643)** | **blocked** — the form is rebuilt when async init completes and restores defaults (title, All Day, Require locker), silently dropping anything typed before that. A real user can hit this; they would just see their title or options revert. `bookDeskViaUI` converges on the state to work around it, which means **the suite no longer detects it** — hence this row. **PPT-2643's fix (#478, `a0360486`) is NOT sufficient for this route.** Measured on 2026-08-05 with that fix present: removing the converging block turns desk-booking RED at `all_day.isChecked()`, the same assertion and the same symptom as before the fix. The same experiment against pre-fix code fails identically, so the flow *is* a genuine detector — the app behaviour simply has not changed on this path. The workaround therefore stays until the remaining path is fixed. | +| REG-10 | P1 | The booking form does not discard user input while it is still initialising. | **[PPT-2643](https://acaprojects.atlassian.net/browse/PPT-2643)** | **blocked** — the form is rebuilt when async init completes and restores defaults (title, All Day, Require locker), silently dropping anything typed before that. A real user can hit this; they would just see their title or options revert. `bookDeskViaUI` converges on the state to work around it, which means **the suite no longer detects it** — hence this row. Investigated 2026-08-05 against #478 (`a0360486`); **no evidence the fix is incomplete**, and the shipped suite is stable — 6 consecutive full runs, 8/8, at `--retries=0`. Removing the block could not be shown to be safe *or* unsafe here: the race needs a slow machine, and this one wins it every time. Two failed attempts to prove otherwise, both recorded so nobody repeats them: (1) a synthetic "type during init, assert it survives" spec passed with *and* without the fix, even with the API responses held to widen the window; (2) removing the block appeared to prove the bug survived — it did not. That red was a Playwright **strict-mode violation**, not a reverted value: opening the desk-select modal puts a second "All Day" checkbox in the DOM (`desk-filters`, bound to the same field), so an unscoped locator matched two elements and threw, with both checked. Scoping the locator to `desk-flow-form` then broke it a second way, because `setCheckbox` silently returns when its locator matches nothing, turning a narrower scope into a no-op and a genuinely invalid form. Both experiments were reverted. **The block stays, this row stays blocked, and settling it needs artificial slowness (throttled CPU) or a unit test driving the deferred branch — not another e2e attempt on fast hardware.** | ## 4. Platform & configuration @@ -186,15 +186,20 @@ Config gaps caused several production incidents, and they are invisible to UI sp - **REG-09 is worse than a flake.** Filed as PPT-2642: one burst of concurrent booking POSTs permanently poisons staff-api's connection pool, so booking creation returns 500 for everyone until the service restarts. Reproducer kept at `e2e/support/repro/reg09-concurrent-bookings.ts`. -- **REG-10 is invisible to this suite by design, and is still not fixed.** `bookDeskViaUI` works - around it, so nothing here will catch it regressing. PPT-2643's fix landed in #478 and does not - cover the desk-booking route — verified by deleting the workaround with the fix present and - watching desk-booking fail on the original symptom (2026-08-05). The removal is a two-line - change and the flow is a proven detector, so the moment the app path is genuinely fixed this - becomes trivial: delete the converging block, confirm green, delete this note. - A synthetic "type during init and assert it survives" spec was tried and deliberately NOT kept — - it passed with *and* without the fix, because the rebuild is a race that a warm local run wins. - A spec that cannot fail is worse than no spec. +- **REG-10 is invisible to this suite by design.** `bookDeskViaUI` converges on the form state, + so nothing here will catch it regressing. PPT-2643's fix landed in #478; whether it is + *complete* is genuinely unresolved, and the honest summary is that this suite cannot answer it + on hardware this fast. Deleting the converging block passes serially and fails about one full + parallel run in three, on the symptom rather than on the mechanism. + Nothing was changed in the flow helper in the end, and that is the finding: two attempts to + replace the workaround with a real assertion both produced red runs that looked like the app + bug and were not. The lesson is procedural — when a test goes red, confirm the *shipped* code + is green in the same environment before concluding anything about the app. Running the + unmodified helper three times (8/8 each) is what separated "the fix is incomplete" from "my + edit is wrong", and only the second was true. + **To settle it properly**, the next step is not another e2e attempt — it is to reproduce under + artificial slowness (throttled CPU, or a unit test that drives the deferred branch directly) + so the mechanism is observed rather than inferred from a timing-dependent symptom. - **AUTH-E2E-08 may never be automatable** with Playwright Chromium. Say so rather than quietly dropping it. - The PPT-2536 harnesses (`tasks/PPT-2536/{e2e,integration}`) still hold assertions that From 69c529f3f9c96bc3b4e19320e55a5e09cfbbb117 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Wed, 5 Aug 2026 16:22:34 +1000 Subject: [PATCH 17/17] =?UTF-8?q?docs(e2e):=20PPT-2643=20is=20live=20after?= =?UTF-8?q?=20all=20=E2=80=94=20found=20in=20code,=20fixed=20in=20#479?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous note said there was no evidence the fix was incomplete. There is; it just was not going to come from this suite. `newForm`'s protected branch is never taken by the flows, because the current user is restored from cache long before org data lands, and `loadForm` — which the flows call first — had no capture at all. The suite passing 8/8 either way is the finding, not a reassurance: REG-10 stays blocked once #479 lands, because this hardware cannot lose the race. Co-Authored-By: Claude Fable 5 --- E2E_USER_STORIES.md | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/E2E_USER_STORIES.md b/E2E_USER_STORIES.md index b411fa9c90..93e5ec1f16 100644 --- a/E2E_USER_STORIES.md +++ b/E2E_USER_STORIES.md @@ -158,7 +158,7 @@ task that found it, so the row can be traced. | REG-07 | P2 | Level selection does not persist once the selector is hidden/disabled. | "Fix level selections persisting when selector is disabled/hidden" | todo | | REG-08 | P1 | An authority with a **relative** `login_url` still reaches a usable login page. | Found 2026-07-30, this suite | **blocked** — currently worked around in `seed.ts`; ts-client resolves a relative `login_url` against the authority host **without its port**, so any non-443 deployment dead-ends. Needs a ts-client/init fix before a spec can assert the good behaviour. | | REG-09 | P1 | Concurrent `POST /bookings` do not 500. | **[PPT-2642](https://acaprojects.atlassian.net/browse/PPT-2642)** | **blocked** — staff-api raises `DB::ConnectionLost` under concurrency. Observed ~1 run in 8 **locally at 4 workers**; **not yet observed in CI**, which runs 2 workers, so halving the concurrency may simply be avoiding it rather than the problem being absent. Needs a staff-api fix; do not treat the quiet CI record as evidence it is gone. | -| REG-10 | P1 | The booking form does not discard user input while it is still initialising. | **[PPT-2643](https://acaprojects.atlassian.net/browse/PPT-2643)** | **blocked** — the form is rebuilt when async init completes and restores defaults (title, All Day, Require locker), silently dropping anything typed before that. A real user can hit this; they would just see their title or options revert. `bookDeskViaUI` converges on the state to work around it, which means **the suite no longer detects it** — hence this row. Investigated 2026-08-05 against #478 (`a0360486`); **no evidence the fix is incomplete**, and the shipped suite is stable — 6 consecutive full runs, 8/8, at `--retries=0`. Removing the block could not be shown to be safe *or* unsafe here: the race needs a slow machine, and this one wins it every time. Two failed attempts to prove otherwise, both recorded so nobody repeats them: (1) a synthetic "type during init, assert it survives" spec passed with *and* without the fix, even with the API responses held to widen the window; (2) removing the block appeared to prove the bug survived — it did not. That red was a Playwright **strict-mode violation**, not a reverted value: opening the desk-select modal puts a second "All Day" checkbox in the DOM (`desk-filters`, bound to the same field), so an unscoped locator matched two elements and threw, with both checked. Scoping the locator to `desk-flow-form` then broke it a second way, because `setCheckbox` silently returns when its locator matches nothing, turning a narrower scope into a no-op and a genuinely invalid form. Both experiments were reverted. **The block stays, this row stays blocked, and settling it needs artificial slowness (throttled CPU) or a unit test driving the deferred branch — not another e2e attempt on fast hardware.** | +| REG-10 | P1 | The booking form does not discard user input while it is still initialising. | **[PPT-2643](https://acaprojects.atlassian.net/browse/PPT-2643)** | **blocked** — the form is rebuilt when async init completes and restores defaults (title, All Day, Require locker), silently dropping anything typed before that. A real user can hit this; they would just see their title or options revert. `bookDeskViaUI` converges on the state to work around it, which means **the suite no longer detects it** — hence this row. Investigated 2026-08-05 against #478 (`a0360486`): **the bug is still live**, established by reading the code rather than by running this suite, and fixed in **PR #479**. `newForm`'s protected branch is never taken by the flows — the current user is restored from cache ~50ms after bootstrap, while `NewDeskFlowComponent.ngOnInit` calls `loadForm` then `newForm` only after org init plus a 300ms settle — and `loadForm` had no capture at all. The shipped e2e suite meanwhile is stable at 6 consecutive full runs, 8/8, `--retries=0`, which is precisely the problem: **it passes either way**. Removing the block could not be shown to be safe *or* unsafe from here: the race needs initialisation to be slow relative to typing, and this machine wins it every time. Two failed attempts to prove otherwise, both recorded so nobody repeats them: (1) a synthetic "type during init, assert it survives" spec passed with *and* without the fix, even with the API responses held to widen the window; (2) removing the block appeared to prove the bug survived — it did not. That red was a Playwright **strict-mode violation**, not a reverted value: opening the desk-select modal puts a second "All Day" checkbox in the DOM (`desk-filters`, bound to the same field), so an unscoped locator matched two elements and threw, with both checked. Scoping the locator to `desk-flow-form` then broke it a second way, because `setCheckbox` silently returns when its locator matches nothing, turning a narrower scope into a no-op and a genuinely invalid form. Both experiments were reverted. **The block stays and this row stays blocked even once #479 lands** — not because the app is unfixed, but because this suite cannot tell either way on fast hardware. The guard for REG-10 is the unit specs in `libs/bookings/src/test/booking-form.service.spec.ts`; unblocking this row needs artificial slowness (throttled CPU), not another e2e attempt. | ## 4. Platform & configuration @@ -191,12 +191,15 @@ Config gaps caused several production incidents, and they are invisible to UI sp *complete* is genuinely unresolved, and the honest summary is that this suite cannot answer it on hardware this fast. Deleting the converging block passes serially and fails about one full parallel run in three, on the symptom rather than on the mechanism. - Nothing was changed in the flow helper in the end, and that is the finding: two attempts to - replace the workaround with a real assertion both produced red runs that looked like the app - bug and were not. The lesson is procedural — when a test goes red, confirm the *shipped* code - is green in the same environment before concluding anything about the app. Running the - unmodified helper three times (8/8 each) is what separated "the fix is incomplete" from "my - edit is wrong", and only the second was true. + Nothing was changed in the flow helper in the end. Two attempts to replace the workaround with + a real assertion both produced red runs that looked like the app bug and were not — one a + Playwright strict-mode violation, one a silently no-op `setCheckbox` after over-narrowing a + locator. The lesson is procedural: when a test goes red, confirm the *shipped* code is green in + the same environment before concluding anything about the app. Running the unmodified helper + three times (8/8 each) is what separated "the fix is incomplete" from "my edit is wrong". + The bug was then found by reading `desk-flow.component.ts` and `booking-form.service.ts` + instead — which is where this should have started, given the symptom is timing-dependent and + the hardware is fast. **To settle it properly**, the next step is not another e2e attempt — it is to reproduce under artificial slowness (throttled CPU, or a unit test that drives the deferred branch directly) so the mechanism is observed rather than inferred from a timing-dependent symptom.