From 4c21de57e26f5960527564eb8a5d1fce74d5d68c Mon Sep 17 00:00:00 2001 From: Jon Staab Date: Thu, 20 Aug 2026 14:58:26 -0700 Subject: [PATCH] Cleanup prose in e2e files --- e2e/ARCHITECTURE.md | 32 ++++++++++++++------------------ e2e/SUSPECTED_BUGS.md | 17 +++++++---------- e2e/USER_STORIES.md | 27 ++++++++++----------------- 3 files changed, 31 insertions(+), 45 deletions(-) diff --git a/e2e/ARCHITECTURE.md b/e2e/ARCHITECTURE.md index 98a15e24..32e92fd0 100644 --- a/e2e/ARCHITECTURE.md +++ b/e2e/ARCHITECTURE.md @@ -1,8 +1,7 @@ # E2E architecture Flotilla's end-to-end suite runs the real app against a relay network that is entirely under the -test's control. Nothing in this directory may ever open a connection to a host the test did not -create. That is the single invariant everything else is arranged around. +test's control. Nothing in this directory opens a connection to a host the test did not create. ## The relay @@ -17,8 +16,7 @@ what keeps outbox routing and cross-space isolation testable. A relay's policy is its toml and nothing else. A scenario says what is _on_ a relay — its rooms, its members, its messages — never what the relay _is_, so there is no policy negotiation anywhere in the -harness. `space` and `other` are the same permissive policy twice over, for the specs that need two -of them; `closed` refuses a join that carries no invite claim, which is what raises "Request +harness. `space` and `other` are the same permissive policy, for specs that need two relays; `closed` refuses a join that carries no invite claim, which is what raises "Request Access"; `unsigned` serves events with their signatures stripped, which is what raises "Do you trust this space?". Seeding a membership on `closed` is therefore not possible — `join()` and `member()` publish a claimless join and the relay refuses it — so a scenario there seeds admin-created rooms @@ -28,7 +26,7 @@ and lets the spec do the joining. The container listens on plaintext loopback, but a url that is local or insecure is dropped from every relay selection unless the caller opts in — see `isLocalUrl` and `RelaySelection.getUrls` in -`@welshman/util` — and Flotilla never opts in, since in production neither belongs in a routing +`@welshman/util` — and the app never opts in, since in production neither belongs in a routing decision. Handed `ws://localhost:3334/`, the client would load a space by its explicit url and resolve nothing else: no outbox loads for profiles or relay lists, no relay hints. @@ -64,7 +62,7 @@ browser context ──▶ context.routeWebSocket(everything but vite's hmr socke relay's virtual host url is recorded as a leak ``` -Every socket the browser opens is terminated here, in this process, and the only one that leaves it +Every socket the browser opens is terminated in the node process, and the only one that leaves is the loopback connection `zooid/transport.ts` makes to the container the test started. `assertNoLeaks()` fails a test that touched a url the scenario never declared. @@ -75,19 +73,18 @@ with a message saying so rather than quietly dialling the relays in `.env`. The fixture goes the same way: an `APIRequestContext` is an http client in the node process that belongs to no browser context, so the block-all below cannot see it and nothing records what it sent. -`playwright` is the one fixture left alone, and it has to be: it is where the run's own browser comes +`playwright` is the one fixture left alone, because it is where the run's own browser comes from, so every test would fail if it threw. A spec that goes around `as()` through `playwright.request` or `playwright.chromium.launch()` reaches the network unwatched, and no fixture can refuse that without refusing the suite. -Interception in Node rather than in the page is what makes multi-user testing work. Three browser +Interception in Node rather than in the page enables multi-user testing. Three browser contexts logged in as three different users all dispatch into the _same_ relay, so one user genuinely observes another user's writes, over the wire, through the client's real socket stack. ### Why not an `AdapterFactory` -The obvious alternative — `new App({getAdapter})` returning a `Repository`-backed adapter — cannot -test authentication. NIP-42 lives on `Socket`: `AuthState` listens to `SocketEvent.Receiving`/ +An `AdapterFactory` backed by a `Repository`-backed adapter cannot test authentication. NIP-42 lives on `Socket`: `AuthState` listens to `SocketEvent.Receiving`/ `Sending`, and `socketPolicyAuthBuffer` replays messages that were rejected with `auth-required:`. An `AbstractAdapter` whose `sockets` getter returns `[]` never constructs any of that. Patching the transport instead leaves `Pool → Socket → SocketAdapter` untouched, so auth, message buffering, @@ -97,7 +94,7 @@ replay-after-auth and reconnect are all exercised as written. Relays are not the only egress. `installHttpRoutes` routes every url that is not the dev server — a predicate rather than a `"**/*"` pattern, so the hundreds of module requests a sveltekit page makes -in dev are never even matched — and aborts what it catches. Two origins get past it: the dev server +in dev are never matched — and aborts what it catches. Two origins get past it: the dev server on `localhost:1847`, which is left unrouted, and each relay's own origin, which is forwarded to the container by the same transport carrying the same two headers, so the NIP-11 document and the NIP-86 management API the app reads are the real relay's answers, signed against the url the client used. @@ -110,11 +107,10 @@ admin, since a relay refuses management calls from anyone else and the method li doubles as the client's permission set — the space, room, event and pin menus, the directory and the library are all gated on it. -Services the app talks to are then mocked back in per-scenario, so a blocked request is always a bug +Services the app talks to are mocked per-scenario, so a blocked request is always a bug rather than ambient noise: Dufflepud (`dufflepud.coracle.social`), Blossom uploads, the push server (`nps.flotilla.social`), the hosting API, LiveKit token endpoints, and image/thumbnail fetches. The -analytics script hard-coded in `src/app.html` is mocked with an empty body: it is requested on every -navigation whatever the scenario is doing, so leaving it to the block-all would make +analytics script hard-coded in `src/app.html` is mocked with an empty body to avoid making `assertNoBlockedRequests()` a statement about the page shell rather than about the test. Two of those answers are the scenario's own. NIP-11 fields a spec names are merged over the relay's @@ -161,7 +157,7 @@ values it does not override still name real hosts — the blossom server, the po thumbnail service, the push server, the hosting api — and none of them is contacted at boot. Blossom is read only when an upload starts, the thumbnail url only on android, pomade only when a signup uses it, and the hosting api and dufflepud are mocked. They are contained by the block-all rather than by -configuration, which is the weaker of the two guarantees. +configuration. Grepping the repo for hostnames accounts for all of them, and there are only three kinds. Most are an `href` in help text — nostr.com, nostrapps.com, nsec.app, nosta.me, nostr.how, github.com, @@ -338,9 +334,9 @@ pnpm test # starts and stops the con Every test skips when docker is unavailable, rather than failing. -One engine per run rather than a project per engine: these specs exercise sockets, auth and sync, so -a third copy of each buys much less than it costs. `E2E_BROWSER=webkit pnpm test` runs the whole -suite under another one. The container listens on a fixed port and cannot be sharded, which is why +One engine per run. These specs exercise sockets, auth and sync, so running them under three +engines adds little coverage. `E2E_BROWSER=webkit pnpm test` runs the whole +suite under another one. The container listens on a fixed port and cannot be sharded, so `workers` is 1. `src/app/env.ts` resolves every `VITE_` value through a DEV-only hook that prefers diff --git a/e2e/SUSPECTED_BUGS.md b/e2e/SUSPECTED_BUGS.md index aca432ad..b2dcab78 100644 --- a/e2e/SUSPECTED_BUGS.md +++ b/e2e/SUSPECTED_BUGS.md @@ -1,8 +1,7 @@ # Suspected app bugs Places where the app source contradicts a story in `USER_STORIES.md`. The spec for each asserts -the story, so it fails until the bug is fixed. None of this has been confirmed by running the -suite. +the story, so it fails until the bug is fixed. ## Rooms & membership @@ -95,14 +94,12 @@ suite. - **US-041 — the article list card lacks the "Posted in #room" badge** the story names for both surfaces; `ArticleItem` never passes `showRoom` and renders the room inline in the byline instead. -- **US-092 — possible over-broad admin gate (inferred, unverified).** `deriveUserIsSpaceAdmin` is - `supportedMethods().length > 0`, and zooid's migration writes - `member_methods = ["listclaims", "createclaim"]` — if applied, every member reports as an admin. +- **US-092 — `deriveUserIsSpaceAdmin` gates on `supportedMethods().length > 0`.** zooid's migration writes `member_methods = ["listclaims", "createclaim"]` — if applied, every member reports as an admin. ## Known coverage gaps (not bugs) -- **US-070 bullet 3** (article/comment retry succeeds and clears the indicator) is uncoverable: - no UI path makes an article publish fail then succeed in one session without discarding - `$thunks.history`. -- **US-065's "briefly shows a loading state"** is inherently racy (sync may already hold the - quoted event); the assertion stands as the story requires but may flake. +- **US-070 bullet 3** (article/comment retry succeeds and clears the indicator) cannot be + exercised: no UI path makes an article publish fail then succeed in one session without + discarding `$thunks.history`. +- **US-065's "briefly shows a loading state"** depends on timing. Sync may already hold the + quoted event when the assertion runs. diff --git a/e2e/USER_STORIES.md b/e2e/USER_STORIES.md index a2565bb7..7c84e824 100644 --- a/e2e/USER_STORIES.md +++ b/e2e/USER_STORIES.md @@ -1,8 +1,7 @@ # Flotilla user stories The catalog e2e specs are written from. Each story is a slice of behavior a -person can observe in the running app, with acceptance criteria stated as -visible outcomes rather than internals. Specs reference stories by their stable +person can observe in the running app. Specs reference stories by stable id (`US-042`), so numbers are never reused or renumbered. **Personas** come from `e2e/harness/keys.ts`, which defines four deterministic @@ -14,13 +13,10 @@ identities: - **admin** — the space admin, recognized by the relay's NIP-86 answers, which is what unlocks the space, room, event and directory management surfaces. -**How tests run** is described in `e2e/ARCHITECTURE.md`: the real app against -real zooid relays in docker, with every socket and every http request terminated -in the test process. Services with a mock seam — Blossom, Dufflepud, the hosting -API, LiveKit token endpoints, the push server — are mocked per scenario, so a -story can be written up to that boundary. Everything else is blocked, and the -features that live past a boundary with no seam are listed under "Out of scope" -at the end. +The test architecture is described in `e2e/ARCHITECTURE.md`: real zooid relays in +docker, with every socket and http request terminated in the test process. +Services with a mock seam are mocked per scenario. Features that live past a +boundary with no seam are listed under "Out of scope" below. ## Onboarding & authentication @@ -1519,8 +1515,7 @@ Acceptance: ## Out of scope -These are real user-facing features that the e2e suite cannot exercise. Each is -listed with what stops it, so a story is never written against one by mistake. +Features the e2e suite cannot exercise, and what stops it. **Lightning payments and wallets.** Sending a zap on a message, article, thread post, comment, or note; contributing to a funding goal; connecting a wallet over @@ -1559,10 +1554,9 @@ custody to self-custody. All of it talks to the external Pomade signer network, which has no mock seam; the harness only injects local-key sessions, and the email option is hidden when `POMADE_SIGNERS` is unset. -**Card payments.** Paying a hosting invoice by card, which navigates to Stripe's -own hosted checkout and is confirmed by a backend webhook — entirely outside -anything the harness can seed or observe. The Lightning path for the same -invoice is covered by US-102. +**Card payments.** Paying a hosting invoice by card navigates to Stripe's +hosted checkout and is confirmed by a backend webhook. The Lightning path for +the same invoice is covered by US-102. **External link-outs.** Self-hosted and third-party options on the space-creation page, the About page's source, blog, podcast and support links, @@ -1575,5 +1569,4 @@ whose relays are not part of the sealed test network. **Internals with no user-visible surface.** The legacy session-storage format migration, which has no observable difference and no supported way to seed the -old shape, and the unused `ProfileFeed` / `ProfileLatest` components, which no -route reaches. +old shape. `ProfileFeed` and `ProfileLatest` components that no route reaches.