test(e2e): local-backend Playwright suite for workplace, with advisory CI - #476
Open
camreeves wants to merge 12 commits into
Open
test(e2e): local-backend Playwright suite for workplace, with advisory CI#476camreeves wants to merge 12 commits into
camreeves wants to merge 12 commits into
Conversation
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
- 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) <noreply@anthropic.com>
- 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) <noreply@anthropic.com>
- 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
|
Deployment failed with the following error: Learn More: https://vercel.com/placeos?upgradeToPro=build-rate-limit |
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) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds an end-to-end test suite for workplace that runs only against a local backend, the
isolated stack to run it against, and an advisory CI job on a self-hosted runner.
Nothing here gates merges. See Not a gate.
What's in it
e2e/support/storageState, a real-login driver, worker-scoped auth fixtures, idempotent API-driven seeding, stack preflighte2e/stack/apps/workplace/e2e/local/subsurviving refresh), a non-admin booking a desk through the full UIE2E_USER_STORIES.md.github/workflows/e2e-advisory.ymlconfig/proxy.conf.js7 passing, ~3 min in CI, ~26s locally. Verified green from a genuine cold start
(
up.sh --fresh, volumes destroyed) and on the runner.Local backend only
assertLocalOnly()throws at config load ifE2E_BACKEND_URL/E2E_APP_URLresolve to anythingbut a loopback host — an allowlist, not a prod blocklist, because a suite that creates and deletes
real data should not be one typo from doing it to a deployment.
The only surface needing anything external is room/calendar events, which is marked out of scope
and must never enter a gate. Desks, lockers, parking and visitors all work against a placeholder
tenant with no outbound calls.
Not a gate
Deliberately advisory until it has earned trust:
pull_requesttrigger. This repo is public and the runner is self-hosted, so a fork PRcould run arbitrary code on a machine on an internal network. Every trigger requires write access.
build.ymlruns onubuntu-latest, a different runner pool, andActions has no cross-workflow
needs.misread.
e2e/**— nobody sees a new checkon their commits. Stage 2, once the nightly has a record: add
push: develop. Three-line change.Note the nightly only starts once this is on
develop—schedulefires from the default branchonly. That's what breaks the chicken-and-egg of proving it before merging it.
Two real bugs found, both filed
POST /bookingspermanently poisons staff-api's connection pool; every later booking-createreturns 500 until the service restarts. Reproducer kept at
e2e/support/repro/reg09-concurrent-bookings.ts.typed input while still initialising (title / All Day / Require locker revert).
Neither is fixed here: PPT-2642's likely fix is in
pg-orm, shared by every service in atransactional path; PPT-2643's fix embeds a UX decision in a service used by every booking flow.
bookDeskViaUIworks around PPT-2643, so this suite no longer detects it. When it's fixed,remove the workaround and replace it with a spec asserting input survives init. Flagged in the
contract.
Review notes
Worth a look at
config/proxy.conf.js(shared dev config) and the workflow. The rest is new files.Things learned the hard way, all documented in code comments rather than lost:
GET /bookingsis caller-scoped — an admin sees none of another user's bookings, so a leakcheck run as admin proves nothing.
type, notbooking_type.login_url, which ts-client resolves without the port —dead-ends the login redirect on any non-443 deployment.
seed.tspatches it.frontend-loaderis required even though the dev server serves the SPA — it supplies/login.🤖 Generated with Claude Code