Tighten the e2e harness comments (#398)

Reviewed-on: https://gitea.coracle.social/coracle/flotilla/pulls/398
Co-authored-by: Coracle-Bot <npub1klq6260@nostr.local>

e2e/harness/app/boot.ts

@@ -7,13 +7,13 @@ import {injectEvents, injectSession} from "./session"
 // Must match TEST_ENV_KEY in src/lib/test/env.ts.
 const TEST_ENV_KEY = "__TEST_ENV__"
 
-// Set the first time the app reads a value out of the injected env, which is this side's only
+// Set the first time the app reads a value out of the injected env, the only evidence this side
-// evidence that the hook in src/app/env.ts ran at all.
+// has that the hook in src/app/env.ts ran.
 const TEST_ENV_READ_KEY = "__TEST_ENV_READ__"
 
 export type BootOptions = {
-  // Every relay list the app reads at startup is pointed here, so the urls it dials on its own
+  // Every relay list the app reads at startup is pointed here, so it can only dial relays the
-  // initiative can only ever be relays the scenario created.
+  // scenario created.
   relays: string[]
   spaces?: string[]
   user?: TestUser
@@ -21,7 +21,7 @@ export type BootOptions = {
   events?: TrustedEvent[]
   path?: string
   // VITE_ values the scenario sets for itself, applied over the relay-derived ones below. Anything
-  // named here still has to be something the test owns, or the app will reach for it.
+  // named here has to be something the test owns, or the test fails on a leak.
   env?: Record<string, string>
 }
 
@@ -73,16 +73,15 @@ export const boot = async (
   await page.goto(path)
 
   // The root layout renders nothing until its async setup block resolves, so the shell appearing
-  // is the first point at which the app is really running. With a session injected, wait for the
+  // is the first point at which the app is running. With a session injected, wait for the signed-in
-  // signed-in nav instead — one the app rejected renders the landing dialog, and failing on that
+  // nav instead. A session the app rejected renders the landing dialog, and failing on that here
-  // here is much easier to read than the assertions it would break later.
+  // reads far better than the assertions it would break later.
   await page
     .locator(user ? ".primary-nav" : ".fl")
     .waitFor({state: "attached", timeout: ms(int(1, MINUTE))})
 
-  // src/app/env.ts reads every VITE_ value as it is imported, so by now the app has either resolved
+  // src/app/env.ts reads every VITE_ value as it is imported, so by now the app has resolved them
-  // them against the env above or against .env's real relays — which would otherwise surface as
+  // against either the env above or .env's real relays.
-  // every assertion in the suite timing out.
   const usedTestEnv = await page.evaluate(
     key => Boolean(Reflect.get(window, key)),
     TEST_ENV_READ_KEY,

e2e/harness/app/cache.ts

@@ -7,9 +7,8 @@ const databaseName = (pubkey: string) => `flotilla-9gl-${pubkey}`
 
 /**
  * What this user's client has written to disk so far. Events reach indexeddb in three-second
- * batches and nothing in the ui says when one has landed, so a spec about what survives a restart
+ * batches with nothing in the ui to say when one has landed, so a spec about what survives a
- * has to read the cache to know the restart is testing anything: a reload before the batch would
+ * restart waits on this before it reloads.
- * fail whether or not the events were ever going to be persisted.
  */
 export const readCachedEvents = (page: Page, pubkey: string): Promise<TrustedEvent[]> =>
   page.evaluate(async name => {
@@ -21,7 +20,7 @@ export const readCachedEvents = (page: Page, pubkey: string): Promise<TrustedEve
     })
 
     // An unversioned open creates the database when it is missing, so a client that has not written
-    // anything yet answers with an empty one rather than with a store to read.
+    // anything yet has no store to read.
     if (!open.objectStoreNames.contains("events")) {
       open.close()
 

e2e/harness/app/nip07.ts

@@ -2,9 +2,8 @@ import type {BrowserContext} from "@playwright/test"
 import type {StampedEvent} from "@welshman/util"
 import type {TestUser} from "../keys"
 
-// The function playwright installs on window for the shim below to call into. A binding is the
+// The function playwright installs on window for the shim below to call into. The keys live in
-// only way across: the keys live in node, and a signer built in the page would be a different
+// node, so a signer built in the page would not be the one seeding signs with.
-// thing from the one seeding signs with.
 const TEST_NIP07_KEY = "__TEST_NIP07__"
 
 type Nip07Call =
@@ -14,10 +13,10 @@ type Nip07Call =
 
 /**
  * A NIP-07 provider backed by a test identity's real signer, so an extension login produces
- * signatures the relays accept — including the NIP-42 auth events a members-only relay demands.
+ * signatures the relays accept, including the NIP-42 auth events a members-only relay demands.
  *
- * Both halves have to be installed before the page navigates: LogIn.svelte reads `window.nostr`
+ * Both halves have to be installed before the page navigates. LogIn.svelte reads `window.nostr`
- * while it renders to decide whether to offer the button at all.
+ * while it renders, to decide whether to offer the button at all.
  */
 export const injectNip07 = async (context: BrowserContext, user: TestUser) => {
   await context.exposeBinding(TEST_NIP07_KEY, (source, call: Nip07Call) => {

e2e/harness/app/session.ts

@@ -8,9 +8,8 @@ const TEST_SESSION_KEY = "__TEST_SESSION__"
 const TEST_EVENTS_KEY = "__TEST_EVENTS__"
 
 // A nip01 session in the {method, data} shape @welshman/app's session handlers deserialize, so
-// restoreSession can build a signer from it without any of the storage encoding a real login
+// restoreSession can build a signer from it without the storage encoding a real login goes through.
-// would have gone through. addInitScript runs before any page script, so this must be called
+// addInitScript runs before any page script, so this has to be called before navigating.
-// before navigating, and it is installed on the context so every page in it boots as this user.
 export const injectSession = (context: BrowserContext, user: TestUser) =>
   context.addInitScript(
     ([key, session]) => {
@@ -19,8 +18,8 @@ export const injectSession = (context: BrowserContext, user: TestUser) =>
     [TEST_SESSION_KEY, {method: "nip01", data: {secret: user.secret}}] as const,
   )
 
-// The repository contents the app loads once the injected session is restored — the local cache
+// The repository contents the app loads once the injected session is restored, the local cache a
-// a user who had used the app before would boot with.
+// returning user would boot with.
 export const injectEvents = (context: BrowserContext, events: TrustedEvent[]) =>
   context.addInitScript(
     ([key, value]) => {

e2e/harness/index.ts

@@ -39,8 +39,8 @@ export {
 } from "./net/http"
 export type {DufflepudFixtures, HostingFixtures, HostingHandle, HostingRecord} from "./net/http"
 
-// Mirrors encodeRelay in src/app/relays.ts, which can't be imported here — it reaches the app's
+// Mirrors encodeRelay in src/app/relays.ts. Importing it reaches the app's module graph, and with
-// module graph, and with it sveltekit.
+// it sveltekit.
 const encodeRelay = (url: string) =>
   encodeURIComponent(
     normalizeRelayUrl(url)
@@ -54,19 +54,17 @@ export const roomPath = (url: string, h: string) => `${spacePath(url)}/${h}`
 
 // What a page is opened with, over and above the scenario's own relays.
 export type PageOptions = {
-  // Overrides the project's context options, for a spec that needs a viewport, a colour scheme or
+  // Overrides the project's context options, for a spec that needs a viewport or a permission of
-  // a permission of its own.
+  // its own.
   context?: BrowserContextOptions
-  // VITE_ values applied over the ones derived from the scenario's relays, e.g. a platform space
+  // VITE_ values applied over the ones derived from the scenario's relays, e.g. a platform space or
-  // or the domain hosted relays are created under. Whatever is named here has to be something the
+  // the domain hosted relays are created under. See BootOptions in app/boot.ts.
-  // scenario owns, or the app will reach for it and the test will fail on a leak.
   env?: Record<string, string>
   // A NIP-07 provider signing as this user, for a login that goes through an extension.
   nip07?: TestUser
-  // A blossom server, installed before the page boots. A spec whose server is one the app probes
+  // A blossom server, installed before the page boots. mockBlossom called on the page `as()`
-  // on load — a space's own url, which src/app/sync.ts asks about as soon as its page opens —
+  // returns arrives after src/app/sync.ts has probed and cached a space's own url, so a spec whose
-  // has to name it here: mockBlossom called on the page `as()` returns arrives after that probe
+  // server is one the app probes on load has to name it here instead.
-  // has already been answered and cached, and uploads go to the default server instead.
   blossom?: BlossomOptions
   // Fields merged over a relay's own nip-11 document, keyed by relay url.
   relayInfo?: RelayInfoOverrides
@@ -78,11 +76,11 @@ export type PageOptions = {
 export type Harness = {
   zooid: Zooid
   seed(build: (tools: SeedTools) => MaybeAsync<void>): Promise<Scenario>
-  // A logged-in page for a user: its own browser context, its own storage, its own sockets into
+  // A logged-in page for a user, in its own browser context, with its own storage and its own
-  // the relays every other user is talking to.
+  // sockets into the relays every other user is talking to.
   as(user: TestUser, path?: string, options?: PageOptions): Promise<Page>
-  // The same page with no session injected — the app as someone who has never logged in sees it,
+  // The same page with no session injected, which is the only way to watch a login or a logout
-  // and the only way to watch a login, a reload or a logout happen.
+  // happen.
   visit(path?: string, options?: PageOptions): Promise<Page>
 }
 
@@ -98,11 +96,8 @@ export type HarnessWorkerFixtures = {
 }
 
 export const test = base.extend<HarnessFixtures, HarnessWorkerFixtures>({
-  // Playwright's own context — and the `page` fixture built on it — is unrouted: no websocket
+  // Playwright's own context, and the `page` fixture built on it, is unrouted, so a page born
-  // interception, no http block-all, no injected env, and nothing collecting leaks from it. A page
+  // there boots the app against the relays baked into .env.
-  // born there boots the app against the relays baked into .env, which is the one thing this suite
-  // exists to prevent, so it is refused outright and `as()`/`visit()` are the only ways to get a
-  // page.
   context: async () => {
     throw new Error(
       "The built-in `context` and `page` fixtures reach the real network. Open a page with the " +
@@ -110,9 +105,8 @@ export const test = base.extend<HarnessFixtures, HarnessWorkerFixtures>({
         "they navigate.",
     )
   },
-  // Playwright builds this one with `playwright.request.newContext()`, so it is an http client in
+  // Playwright builds this one with `playwright.request.newContext()`, an http client in this
-  // this process that belongs to no browser context: `installHttpRoutes` cannot see it and nothing
+  // process that belongs to no browser context, so `installHttpRoutes` cannot see it.
-  // records what it sent.
   request: async () => {
     throw new Error(
       "The built-in `request` fixture makes http requests from node, where nothing intercepts " +
@@ -153,13 +147,12 @@ export const test = base.extend<HarnessFixtures, HarnessWorkerFixtures>({
 
     const open = async (path: string, options: PageOptions, user?: TestUser) => {
       const {urls, cache} = requireScenario()
-      // The project's own `use` first, so a viewport, colour scheme or device descriptor set in
+      // The project's own `use` first, so a viewport or device descriptor set in
-      // playwright.config.ts reaches the context rather than being silently dropped.
+      // playwright.config.ts reaches the context rather than being dropped.
       //
-      // A request a service worker makes is not seen by context.route, so a worker is the one
+      // context.route does not see a request a service worker makes, and sveltekit registers
-      // way out of the block-all below. Sveltekit registers src/service-worker.js on every
+      // src/service-worker.js on every navigation in dev, so registration is blocked rather than
-      // navigation in dev; it has no fetch handler today, and blocking registration is what keeps
+      // left as the thing containment rests on.
-      // that from being the thing containment rests on.
       const context = await browser.newContext({
         ...testInfo.project.use,
         serviceWorkers: "block",
@@ -168,9 +161,9 @@ export const test = base.extend<HarnessFixtures, HarnessWorkerFixtures>({
 
       contexts.push(context)
 
-      // Interception before navigation, and the block-all before the mocks — playwright matches
+      // Playwright matches the most recently registered route first and every mock falls through
-      // the most recently registered route first, and every mock falls through what it doesn't
+      // what it doesn't recognize, so the block-all goes in before the mocks, and all of it before
-      // recognize, so the mocks have to be registered last to be reachable at all.
+      // the page navigates.
       await installHttpRoutes(context)
       await installWebSocketRoutes(context, zooid)
       await mockRelayInfo(context, options.relayInfo ?? {})
@@ -188,10 +181,9 @@ export const test = base.extend<HarnessFixtures, HarnessWorkerFixtures>({
         await injectNip07(context, options.nip07)
       }
 
-      // Playwright grants the "notifications" permission at the browser level, but headless Chromium
+      // Headless Chromium reports `Notification.permission` as "denied" even where playwright has
-      // still reports `Notification.permission` as "denied", so a spec that opted into notifications
+      // granted the permission at the browser level, so the grant is reflected into the API the app
-      // would watch the app's push-enable refuse a permission it was given. Reflect the grant into
+      // reads.
-      // the Notification API the app actually reads.
       if (options.context?.permissions?.includes("notifications")) {
         await context.addInitScript(() => {
           Object.defineProperty(Notification, "permission", {

e2e/harness/keys.ts

@@ -21,9 +21,8 @@ const makeUser = (name: string, secret: string): TestUser => {
   return user
 }
 
-// The secrets are near-zero entropy on purpose: they never leave the test process, they are
+// The secrets never leave the test process. They are stable across runs, so a pubkey can be
-// stable across runs so a pubkey can be asserted on directly, and the leading nibbles make an
+// asserted on directly, and the leading nibbles name an author in a failed-assertion diff.
-// event's author recognizable at a glance in a failed-assertion diff.
 export const users = {
   alice: makeUser("alice", "a11ce00000000000000000000000000000000000000000000000000000000001"),
   bob: makeUser("bob", "b0b0000000000000000000000000000000000000000000000000000000000002"),
@@ -31,14 +30,14 @@ export const users = {
   admin: makeUser("admin", "ad31100000000000000000000000000000000000000000000000000000000004"),
 }
 
-// secp256k1's group order. A secret is a scalar in [1, n), and a hash lands outside that range
+// secp256k1's group order. A secret is a scalar in [1, n), so the digest below is reduced into
-// only astronomically rarely, but reducing rather than rejecting keeps the derivation total.
+// that range rather than rejected when it falls outside.
 const CURVE_ORDER = BigInt("0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141")
 
 /**
- * A fifth, sixth, hundredth identity, named rather than listed. The secret is derived from the
+ * An identity beyond the four above, named rather than listed. The secret is derived from the name,
- * name, so the pubkey is as stable across runs as the four above, and minting one registers it —
+ * so the pubkey is as stable across runs, and minting one registers it: a scenario can seed for it
- * a scenario can seed a profile, a message or a follow for it like any other test user.
+ * like any other test user.
  */
 export const makeTestUser = (name: string) => {
   const digest = BigInt("0x" + createHash("sha256").update(name).digest("hex"))

e2e/harness/net/context.ts

@@ -0,0 +1,23 @@
+import type {BrowserContext} from "@playwright/test"
+
+// State the harness keeps per browser context, outside the browser: recorded traffic, blocked
+// requests, the hosting fake's store. Reading one before the route installer that owns it has run
+// is a spec calling things out of order, which the error names.
+export const makeContextStore = <T>(installer: string) => {
+  const byContext = new WeakMap<BrowserContext, T>()
+
+  return {
+    set: (context: BrowserContext, value: T) => {
+      byContext.set(context, value)
+
+      return value
+    },
+    get: (context: BrowserContext) => {
+      const value = byContext.get(context)
+
+      if (value) return value
+
+      throw new Error(`${installer} was never called for this browser context`)
+    },
+  }
+}

e2e/harness/net/http.ts

@@ -5,10 +5,11 @@ import type {Handle} from "@welshman/util"
 import type {ZapperValues} from "@welshman/domain"
 import {tenantByUrl} from "../zooid/config"
 import {requestZooid} from "../zooid/transport"
+import {makeContextStore} from "./context"
 
-// Mirrors the service urls in src/app/env.ts and .env, which this process can't import: env.ts
+// Mirrors the service urls in src/app/env.ts and .env, which this process can't import because
-// reads import.meta.env and pulls in Capacitor. Nothing here is ever fetched — these are route
+// env.ts reads import.meta.env and pulls in Capacitor. These are route patterns rather than urls
-// patterns, and every handler answers from memory.
+// anything fetches, and every handler answers from memory.
 const DUFFLEPUD_ORIGIN = "https://dufflepud.coracle.social"
 const PUSH_SERVER_ORIGIN = "https://nps.flotilla.social"
 const HOSTING_ORIGIN = "https://api.hosting.coracle.social"
@@ -16,8 +17,8 @@ const HOSTING_ORIGIN = "https://api.hosting.coracle.social"
 // Hard-coded in src/app.html, so every navigation asks for it whatever the scenario is doing.
 const PLAUSIBLE_ORIGIN = "https://plausible.coracle.social"
 
-// Where the hosting api sends a browser to pay. Nothing serves it — `.test` resolves nowhere and
+// Where the hosting api sends a browser to pay. `.test` resolves nowhere and the block-all aborts
-// the block-all aborts the navigation — so a spec sees the redirect without one leaving.
+// the navigation, so a spec sees the redirect without one leaving.
 const CHECKOUT_ORIGIN = "https://checkout.test"
 
 // Relay-hosted livekit lives under a well-known path rather than an origin of its own.
@@ -30,8 +31,7 @@ const PNG = Buffer.from(
 )
 
 // The dev server from vite.config.ts. Traffic to it is the app loading itself rather than egress,
-// so it is the one host both layers here let past, websockets included — Vite's hmr socket has to
+// so it is the one host both layers here let past, websockets included for Vite's hmr socket.
-// keep working.
 export const isDevServerUrl = (url: URL) =>
   url.port === "1847" && ["localhost", "127.0.0.1", "[::1]"].includes(url.hostname)
 
@@ -47,31 +47,19 @@ export type BlockedRequest = {
   url: string
 }
 
-const blockedByContext = new WeakMap<BrowserContext, BlockedRequest[]>()
+const blockedStore = makeContextStore<BlockedRequest[]>("installHttpRoutes")
-
-export const getBlockedRequests = (context: BrowserContext) => {
-  const blocked = blockedByContext.get(context)
-
-  if (blocked) {
-    return blocked
-  }
-
-  throw new Error("installHttpRoutes was never called for this browser context")
-}
 
 /**
  * Blocks every http request the app makes, except to the dev server and to a relay's own origin,
  * which is forwarded to the container so that the nip-11 document and the nip-86 management api the
- * app reads are the real relay's. Install this first: the per-service mocks below are registered
+ * app reads are the real relay's. Install this first. The per-service mocks below are registered
  * later and therefore take priority, so a request that still reaches this handler is one nothing
  * has mocked.
  */
 export const installHttpRoutes = (context: BrowserContext) => {
-  const blocked: BlockedRequest[] = []
+  const blocked = blockedStore.set(context, [])
 
-  blockedByContext.set(context, blocked)
+  // The dev server is left unrouted rather than matched and continued. A sveltekit page in dev is
-
-  // The dev server is left unrouted rather than matched and continued: a sveltekit page in dev is
   // hundreds of module requests, and none of them is egress.
   return context.route(
     url => !isDevServerUrl(url),
@@ -102,12 +90,12 @@ export const installHttpRoutes = (context: BrowserContext) => {
 
 /**
  * Opt-in, for a spec that wants every request the app made to have been answered by something the
- * scenario stood up. Some of what a page asks for is meant to be refused — the blossom probe
+ * scenario stood up. Some of what a page asks for is meant to be refused, such as the blossom probe
- * against a relay with blossom off, for one — so call this from a spec that has mocked what it
+ * against a relay with blossom off, so call this from a spec that has mocked what it exercises
- * exercises rather than from teardown.
+ * rather than from teardown.
  */
 export const assertNoBlockedRequests = (context: BrowserContext) => {
-  const blocked = getBlockedRequests(context)
+  const blocked = blockedStore.get(context)
 
   if (blocked.length > 0) {
     throw new Error(
@@ -123,12 +111,12 @@ export const assertNoBlockedRequests = (context: BrowserContext) => {
 export type RelayInfoOverrides = Record<string, object>
 
 /**
- * A relay's real document with a few fields replaced: a `redirect_to`, a `limitation`, a NIP the
+ * A relay's real document with a few fields replaced, such as a `redirect_to`, a `limitation`, or a
- * relay does not implement. Merged rather than fabricated, because `self`, `pubkey`, `name` and
+ * NIP the relay does not implement. It is merged rather than fabricated because `self`, `pubkey`,
- * `supported_nips` are what room state is trusted from — a hand-written document breaks every
+ * `name` and `supported_nips` are what room state is trusted from, and a hand-written document
- * room on the page.
+ * breaks every room on the page.
  *
- * Install it before the page navigates: the document is read at startup and cached from then on.
+ * Install it before the page navigates. The document is read at startup and cached from then on.
  */
 export const mockRelayInfo = (context: BrowserContext, overrides: RelayInfoOverrides) => {
   const overrideByOrigin = new Map(
@@ -151,14 +139,14 @@ export const mockRelayInfo = (context: BrowserContext, overrides: RelayInfoOverr
       if (request.method() === "GET" && host && override) {
         const {status, headers, body} = await requestZooid(host, "GET", "/", {
           ...request.headers(),
-          // The merge has to read the document, and khatru will compress it if invited to.
+          // The merge has to read the document, and khatru compresses it when invited to.
           "accept-encoding": "identity",
         })
 
         return route.fulfill({
           status,
           // khatru's Access-Control-Allow-Origin is what makes the fetch legal, so the relay's own
-          // headers are kept — all but the length of a body that is about to change.
+          // headers are kept, all but the length of a body that is about to change.
           headers: omit(["content-length"], headers),
           body: JSON.stringify({...JSON.parse(body.toString()), ...override}),
         })
@@ -171,9 +159,8 @@ export const mockRelayInfo = (context: BrowserContext, overrides: RelayInfoOverr
 
 /**
  * The analytics script src/app.html loads on every page. Answering with an empty body leaves
- * `window.plausible` as the queueing shim src/app/analytics.ts installs, so pageviews accumulate
+ * `window.plausible` as the queueing shim src/app/analytics.ts installs, so pageviews accumulate in
- * in memory and nothing is ever sent — and `assertNoBlockedRequests` stays a statement about the
+ * memory and nothing is ever sent.
- * scenario rather than about the page shell.
  */
 export const mockAnalytics = (context: BrowserContext) =>
   context.route(`${PLAUSIBLE_ORIGIN}/**`, route =>
@@ -186,15 +173,15 @@ export type DufflepudFixtures = {
   // A link preview is only rendered when it carries a title or an image.
   preview?: {title?: string; description?: string; image?: string}
   handles?: {handle: string; info?: Handle}[]
-  // `lnurl` is hex here, not bech32 — that's the encoding dufflepud speaks.
+  // `lnurl` is hex here rather than bech32, which is the encoding dufflepud speaks.
   zappers?: {lnurl: string; info?: Omit<ZapperValues, "lnurl">}[]
 }
 
 /**
  * Dufflepud, whose origin is the one service url the app hard-codes rather than reading from a
- * `VITE_` value. `as()` installs it with no fixtures, so a spec that needs one — a zapper for a zap
+ * `VITE_` value. `as()` installs it with no fixtures, so a spec that needs one calls this again
- * receipt, a link preview — calls this again with its own: playwright matches the most recently
+ * with its own. Playwright matches the most recently registered route first, so the spec's answers
- * registered route first, so the spec's answers win over the empty defaults.
+ * win over the empty defaults.
  */
 export const mockDufflepud = (context: BrowserContext, fixtures: DufflepudFixtures = {}) =>
   context.route(`${DUFFLEPUD_ORIGIN}/**`, route => {
@@ -227,7 +214,7 @@ export type BlossomOptions = {
 }
 
 /**
- * A blossom server that keeps what it was given: an upload is hashed exactly as the real thing
+ * A blossom server that keeps what it was given. An upload is hashed exactly as the real thing
  * would be, so the descriptor it answers with points at a blob this mock can then serve back.
  */
 export const mockBlossom = (context: BrowserContext, {server}: BlossomOptions) => {
@@ -283,7 +270,7 @@ export const mockPushServer = (context: BrowserContext) =>
         return route.fulfill({json: {}})
       }
 
-      // Registration is only usable if it comes back with both, and the relay is told to post
+      // Registration is only usable if it comes back with both. The relay is told to post
       // notifications to the callback rather than the client ever fetching it.
       const key = "test-push-subscription"
 
@@ -294,8 +281,8 @@ export const mockPushServer = (context: BrowserContext) =>
   })
 
 // One record straight off the hosting api, whose shapes live in src/app/hosting.ts. They're left
-// as loose objects here so that mocking a payment flow doesn't pull the app's module graph — and
+// as loose objects here so that mocking a payment flow doesn't pull the app's module graph, and
-// with it import.meta.env — into the node process. A relay record may also carry `members` and
+// with it import.meta.env, into the node process. A relay record may also carry `members` and
 // `activity`, and an invoice `items` and `bolt11`, which is where those endpoints answer from.
 export type HostingRecord = Record<string, unknown>
 
@@ -311,31 +298,23 @@ export type HostingFixtures = {
   draftInvoice?: HostingRecord
 }
 
-// The backend changing its mind between two of the user's clicks — a custom domain that verifies,
+// The backend changing its mind between two of the user's clicks, such as a custom domain that
-// an invoice that gets paid — which is the half of those flows no click can reach.
+// verifies or an invoice that gets paid. It is the half of those flows no click can reach.
 export type HostingHandle = {
   setTenant(patch: HostingRecord): void
   setRelay(id: string, patch: HostingRecord): void
   setInvoice(id: string, patch: HostingRecord): void
 }
 
-const hostingByContext = new WeakMap<BrowserContext, HostingHandle>()
+const hostingStore = makeContextStore<HostingHandle>("mockHosting")
 
-export const getHosting = (context: BrowserContext) => {
+export const getHosting = (context: BrowserContext) => hostingStore.get(context)
-  const handle = hostingByContext.get(context)
-
-  if (handle) {
-    return handle
-  }
-
-  throw new Error("mockHosting was never called for this browser context")
-}
 
 /**
- * The hosting api as a small stateful fake: a write mutates the record it names and the next read
+ * The hosting api as a small stateful fake. A write mutates the record it names and the next read
  * sees it, which is what makes editing a space's details, deactivating it, changing its plan or
- * saving a custom domain observable at all. Each browser context gets its own store, so one user's
+ * saving a custom domain observable. Each browser context gets its own store, so one user's spaces
- * spaces are not another's.
+ * are not another's.
  */
 export const mockHosting = async (context: BrowserContext, fixtures: HostingFixtures = {}) => {
   const plans = fixtures.plans ?? []
@@ -363,7 +342,7 @@ export const mockHosting = async (context: BrowserContext, fixtures: HostingFixt
     },
   }
 
-  hostingByContext.set(context, handle)
+  hostingStore.set(context, handle)
 
   await context.route(`${HOSTING_ORIGIN}/**`, route => {
     const request = route.request()
@@ -487,8 +466,8 @@ export const mockHosting = async (context: BrowserContext, fixtures: HostingFixt
         return route.fulfill({json: {data: {url: `${CHECKOUT_ORIGIN}/invoices/${id}`}}})
       }
 
-      // GET and reconcile both answer with the invoice as it now stands, which is how a spec
+      // GET and reconcile both answer with the invoice as it now stands, which is how a spec marks
-      // marks one paid: flip `paid_at` with the handle and let the dialog's next poll find it.
+      // one paid. Flip `paid_at` with the handle and let the dialog's next poll find it.
       return route.fulfill({json: {data: invoice}})
     }
 
@@ -499,7 +478,7 @@ export const mockHosting = async (context: BrowserContext, fixtures: HostingFixt
 }
 
 export type LivekitOptions = {
-  // Where the client is told to connect. Point it at something the test owns — the token this
+  // Where the client is told to connect. Point it at something the test owns, since the token this
   // hands out is accepted by nothing else.
   serverUrl: string
   token?: string
@@ -523,8 +502,8 @@ export const mockLivekit = (context: BrowserContext, {serverUrl, token}: Livekit
   )
 
 /**
- * Serves a png for anything the browser is loading as an image — avatars, banners, blossom blobs,
+ * Serves a png for anything the browser is loading as an image, so a scenario's fixtures can
- * video posters — so a scenario's fixtures can reference urls without any of them being fetched.
+ * reference avatar, banner, blob and poster urls without any of them being fetched.
  */
 export const mockImages = (context: BrowserContext) =>
   context.route(

e2e/harness/net/websocket.ts

@@ -5,6 +5,7 @@ import {RelayMessageType, isClientEvent, isClientReq} from "@welshman/net"
 import type {ClientMessage, RelayMessage} from "@welshman/net"
 import type {RelayConnection} from "../zooid/types"
 import type {Zooid} from "../zooid/relay"
+import {makeContextStore} from "./context"
 import {isDevServerUrl} from "./http"
 
 export type Direction = "toRelay" | "toClient"
@@ -21,21 +22,10 @@ type Traffic = {
   forgotten: Set<string>
 }
 
-const trafficByContext = new WeakMap<BrowserContext, Traffic>()
+const trafficStore = makeContextStore<Traffic>("installWebSocketRoutes")
 
-const getTraffic = (context: BrowserContext) => {
+// A relay that holds nothing. REQs get an immediate EOSE and events are accepted and dropped, so a
-  const traffic = trafficByContext.get(context)
+// leak fails on the assertion that names it rather than on a timeout three layers away.
-
-  if (traffic) {
-    return traffic
-  }
-
-  throw new Error("installWebSocketRoutes was never called for this browser context")
-}
-
-// A relay that holds nothing: REQs get an immediate EOSE and events are accepted into the void.
-// Only urls the container does not serve get one, so a leak fails on the assertion that names it
-// rather than on a timeout three layers away.
 const openEmptyRelay = (): RelayConnection => {
   let listener: (message: RelayMessage) => void = () => undefined
 
@@ -92,16 +82,18 @@ const serve = (traffic: Traffic, zooid: Zooid, route: WebSocketRoute) => {
 /**
  * The single interception point for relay traffic. It goes on the context rather than a page, so
  * every page in it is covered including ones opened later, and it is safe to call before any page
- * exists — routing is the environment a page is born into, not a step in a sequence.
+ * exists.
  *
  * Vite's hmr socket is the one url left alone. Everything else is answered from this process, and
  * a url that is not one of the container's virtual relays is served by an empty relay and recorded
  * as a leak.
  */
 export const installWebSocketRoutes = (context: BrowserContext, zooid: Zooid) => {
-  const traffic: Traffic = {transcript: [], leaks: new Set(), forgotten: new Set()}
+  const traffic = trafficStore.set(context, {
-
+    transcript: [],
-  trafficByContext.set(context, traffic)
+    leaks: new Set(),
+    forgotten: new Set(),
+  })
 
   return context.routeWebSocket(
     url => !isDevServerUrl(url),
@@ -109,14 +101,13 @@ export const installWebSocketRoutes = (context: BrowserContext, zooid: Zooid) =>
   )
 }
 
-export const getTranscript = (context: BrowserContext) => getTraffic(context).transcript
+export const getTranscript = (context: BrowserContext) => trafficStore.get(context).transcript
 
-// Retention, as this context sees it: from here on the relay answers like one that never held
+// Makes a relay answer like one that never held anything, without its url becoming a leak. `serve`
-// anything, while staying a url the scenario declared rather than becoming a leak. Sockets already
+// resolves a relay once, at open, so sockets already open keep theirs and the drop takes effect on
-// open keep the relay they were opened against — `serve` resolves once, at open — so the drop takes
+// the next connection. A reload is what gives it one.
-// effect on the next connection, which is what a reload gives it.
 export const forgetRelay = (context: BrowserContext, url: string) =>
-  getTraffic(context).forgotten.add(normalizeRelayUrl(url))
+  trafficStore.get(context).forgotten.add(normalizeRelayUrl(url))
 
 // Every frame in both directions, oldest first. Attach it to a failing test to see what the client
 // actually said, and to whom.
@@ -129,7 +120,7 @@ export const formatTranscript = (context: BrowserContext) =>
     .join("\n")
 
 export const assertNoLeaks = (context: BrowserContext) => {
-  const {leaks} = getTraffic(context)
+  const {leaks} = trafficStore.get(context)
 
   if (leaks.size > 0) {
     throw new Error(

e2e/harness/seed/scenario.ts

@@ -9,13 +9,12 @@ import type {TestUser} from "../keys"
 import {seedSpace} from "./space"
 import type {SeededSpace} from "./space"
 
-// A fixture timestamp, as an offset from the moment the scenario started: `at(2, HOUR)` is two
+// A fixture timestamp, as an offset from the moment the scenario started. `at(2, HOUR)` is two
-// hours before the test began. Count-first, like int and ago.
+// hours before the test began, count-first like int and ago.
 export type At = (count: number, unit: number) => number
 
 export type SeedTools = {
-  // Names a relay the container already serves — its policy is its toml in zooid/docker/config, so
+  // Names a relay the container already serves. Its policy is its toml in zooid/docker/config.
-  // a scenario describes what is on a relay, never what the relay is.
   relay: (name: TenantName) => SeededSpace
   user: typeof users
   at: At
@@ -26,9 +25,9 @@ export type Scenario = {
   readonly at: At
   readonly urls: string[]
   space(name: TenantName): SeededSpace
-  // The events a returning user's client would already have on disk. Just the room list: the
+  // The events a returning user's client would already have on disk, which is just the room list.
-  // scenario's own relays stand in for the indexers, and a members-only relay won't serve the list
+  // A members-only relay won't serve the list that would tell authPolicy it may identify to it. See
-  // that would tell authPolicy it may identify to it. See ARCHITECTURE.md, "Users and sessions".
+  // ARCHITECTURE.md, "Users and sessions".
   cache(user: TestUser): SignedEvent[]
 }
 

e2e/harness/seed/space.ts

@@ -21,8 +21,8 @@ import {users} from "../keys"
 import type {TestUser} from "../keys"
 
 // @welshman/domain has no writer for NIP-29 kind-9 messages, and none of its readers describe one,
-// so this pairs the base writer with the base reader. The behavior tags it renders — `h` via
+// so this pairs the base writer with the base reader. The behavior tags it renders are everything a
-// setRoom, `q`/`p` via addQuote/addMention — are everything a room message carries.
+// room message carries: `h` via setRoom, `q` and `p` via addQuote and addMention.
 class MessageWriter extends EventWriter<BaseEventReader> {}
 
 // A handle to an event the scenario is going to publish. Seeding calls record what to write and
@@ -32,8 +32,8 @@ export type SeededEvent = {
   readonly id: string
 }
 
-// The kind-14 a direct message really is. It is never published — each participant gets it inside
+// The kind-14 a direct message really is. It is never published, since each participant gets it
-// a gift wrap — so this, rather than anything on the wire, is what a spec asserts on.
+// inside a gift wrap, so this is what a spec asserts on.
 export type SeededRumor = {
   readonly rumor: HashedEvent
   readonly id: string
@@ -51,8 +51,7 @@ export type ProfileValues = {
   nip05?: string
 }
 
-// A queued write. Seeding is ordered — a reply's parent has to exist first — so every write goes
+// A queued write, drained in declaration order by `seed` in scenario.ts.
-// through the scenario's queue rather than starting when it is declared.
 export type Enqueue = (write: () => Promise<void>) => void
 
 // A user's membership as their own client sees it, which the scenario turns into one room list
@@ -69,15 +68,15 @@ export type SeededSpace = {
   readonly memberships: SeededMembership[]
   room(h: string, options?: RoomOptions): void
   member(user: TestUser, h?: string): void
-  // Relay and room membership, plus a place in the user's own room list — what a user who joined
+  // Relay and room membership, plus a place in the user's own room list, which is what a user who
-  // this space through the ui would end up with.
+  // joined this space through the ui ends up with.
   join(user: TestUser, ...rooms: string[]): void
   message(user: TestUser, h: string, content: string, createdAt?: number): SeededEvent
   reply(user: TestUser, parent: SeededEvent, content: string, createdAt?: number): SeededEvent
   profile(user: TestUser, values: ProfileValues, createdAt?: number): SeededEvent
   event(user: TestUser, template: SeededTemplate, createdAt?: number): SeededEvent
-  // A nip-17 conversation: one kind-14 rumor, gift-wrapped once per participant — the sender
+  // A nip-17 conversation. One kind-14 rumor, gift-wrapped once per participant including the
-  // included, since their own copy is the half of the thread their client reads back.
+  // sender, whose own copy is the half of the thread their client reads back.
   dm(from: TestUser, to: TestUser[], content: string, createdAt?: number): SeededRumor
   // This space's domain kinds, bound to a resolver that answers with its url, so a writer built
   // here renders its relay hints as this space. For everything `event()` takes a template for:
@@ -91,7 +90,7 @@ export type SeedSpaceOptions = {
   zooid: Zooid
   enqueue: Enqueue
   // The moment the scenario began. A fixture declared without a timestamp is stamped with it
-  // rather than with the wall clock, so a run's events never drift apart from one another.
+  // rather than with the wall clock.
   startedAt: number
   name: TenantName
 }
@@ -181,8 +180,8 @@ export const seedSpace = ({zooid, enqueue, startedAt, name}: SeedSpaceOptions):
   const message = (user: TestUser, h: string, content: string, createdAt = startedAt) =>
     publish(() => relay().message(user, h, content, createdAt))
 
-  // Flotilla replies in a room by quoting: Content.svelte renders a quote from the nostr uri in the
+  // Flotilla replies in a room by quoting. Content.svelte renders a quote from the nostr uri in the
-  // content rather than from the q tag, so the uri has to be prepended, as prependParent does in
+  // content rather than from the q tag, so the uri is prepended as prependParent does in
   // src/app/rooms.ts.
   const reply = (user: TestUser, parent: SeededEvent, content: string, createdAt = startedAt) =>
     publishTemplate(user, async () => {
@@ -213,9 +212,9 @@ export const seedSpace = ({zooid, enqueue, startedAt, name}: SeedSpaceOptions):
   const kind = <R extends BaseEventReader, W extends EventWriter<R>>(factory: KindFactory<R, W>) =>
     factory.configure(context)
 
-  // Every wrap is published over the sender's own connection: a gift wrap is signed by an
+  // Every wrap is published over the sender's own connection, since a gift wrap's author is an
-  // ephemeral key, so its author is nobody this process can authenticate as. zooid stores it
+  // ephemeral key nobody in this process can authenticate as. zooid stores it anyway, authorizing a
-  // anyway, because it authorizes a kind-1059 by the member named in its p tag.
+  // kind-1059 by the member named in its p tag.
   const dm = (from: TestUser, to: TestUser[], content: string, createdAt = startedAt) => {
     const rumor = seeded(async () => {
       const writer = kind(DirectMessage).writer().setContent(content)

e2e/harness/zooid/config.ts

@@ -1,21 +1,20 @@
 /**
  * The virtual relays the container serves, and the only place their names are written down.
  *
- * zooid is multi-tenant: it binds a config to a Host header and serves any number of them from one
+ * zooid binds a config to a Host header and serves any number of them from one process, so a second
- * process, so a second space costs a toml in docker/config and nothing else. The names are a union
+ * space costs a toml in docker/config and nothing else. The names are a union rather than a string
- * rather than a string so that a scenario naming a relay that has no config fails to compile
+ * so that a scenario naming a relay that has no config fails to compile instead of hanging on a 404
- * instead of hanging on a 404 from the dispatcher.
+ * from the dispatcher.
  *
- * Each host is a name that resolves nowhere. The container is reached on loopback and told what to
+ * Each host resolves nowhere, and `.test` is reserved by rfc 2606, so a url that escapes this
- * call itself, so that what it calls itself is a url a relay selection will keep — see transport.ts
+ * process fails to connect rather than reaching a host. transport.ts covers why the container is
- * for why that matters, and `.test` is reserved by rfc 2606, so a url that escapes this process
+ * told to call itself this rather than its loopback address.
- * fails to connect rather than reaching a host.
  */
 export const tenants = {
   space: "space.test",
   other: "other.test",
-  // Policy that space.toml cannot express at the same time: one that refuses a join without an
+  // Policy space.toml cannot express at the same time. `closed` refuses a join without an invite,
-  // invite, and one that serves events with their signatures stripped.
+  // `unsigned` serves events with their signatures stripped.
   closed: "closed.test",
   unsigned: "unsigned.test",
 } as const
@@ -26,6 +25,5 @@ export const tenantUrl = (name: TenantName) => `wss://${tenants[name]}/`
 
 export const tenantNames = Object.keys(tenants) as TenantName[]
 
-// Which tenant a url belongs to, for the transport's Host header. A url the app opens that is not
+// Which tenant a url belongs to, for the transport's Host header.
-// one of these is a leak, and is reported as one rather than dialled.
 export const tenantByUrl = new Map(tenantNames.map(name => [tenantUrl(name), tenants[name]]))

e2e/harness/zooid/relay.ts

@@ -17,7 +17,7 @@ import {tenantNames, tenantUrl, tenants} from "./config"
 import type {TenantName} from "./config"
 import {makeTestRelay} from "./testRelay"
 import type {PublishOptions} from "./types"
-import {connectToZooid, requestZooid} from "./transport"
+import {connectToZooid, port, requestZooid} from "./transport"
 import type {ZooidConnection} from "./transport"
 
 const image = "gitea.coracle.social/coracle/zooid:latest"
@@ -27,32 +27,23 @@ const composeFile = fileURLToPath(new URL("docker/compose.yaml", import.meta.url
 const configSource = fileURLToPath(new URL("docker/config", import.meta.url))
 
 // What the container mounts as /app/config. zooid saves a relay's toml back whenever a nip-86 call
-// edits that relay's name, description or icon, so it is handed a copy rather than the repo's own
+// edits that relay's name, description or icon. A read-only mount fails those calls, and mounting
-// directory: a read-only mount fails those calls, and a writable one would leave the fixtures every
+// the repo's own directory would leave one test rewriting the fixtures the rest read, so it is
-// other test reads rewritten by whatever the last one did. The copy is staged here rather than in
+// handed a copy. The image is distroless, so the copy is staged here rather than in an entrypoint.
-// an entrypoint because the image is distroless — there is no shell in it to copy anything with.
 const configDir = join(tmpdir(), "flotilla-e2e-zooid-config")
 
 const execFileAsync = promisify(execFile)
 
-// execFile does not go through a shell, so a `docker` that exists only as an alias or a function —
+// execFile does not go through a shell, so a `docker` that exists only as an alias or a function is
-// podman, colima, a wrapper in a shell rc file — is invisible to it however well it works when
+// invisible to it however well it works when typed. Name the executable to drive with E2E_DOCKER in
-// typed. Name the executable to drive with E2E_DOCKER in that case.
+// that case.
 const dockerCommand = process.env.E2E_DOCKER ?? "docker"
 
-// The loopback port the container publishes and transport.ts dials. It is fixed rather than
+// Whether something already answers on the port the container publishes, asked before this run
-// configurable on purpose: the harness talks to whatever answers here, so a copy of the suite left
+// brings its own up.
-// running by anyone else on the machine — under another user, even — would be seeded, queried and
-// reset out from under this run. Both places have to agree, so this must match the published port
-// in docker/compose.yaml and `port` in transport.ts.
-const relayHostPort = 3334
-
-// Whether something already answers on the relay's loopback port. Used as a pre-flight: the port is
-// the harness's alone for the length of a run, so anything holding it before this run brings its own
-// container up is a foreign occupant we must refuse rather than quietly share.
 const isRelayPortTaken = () =>
   new Promise<boolean>(resolve => {
-    const socket = createConnection({port: relayHostPort, host: "127.0.0.1"})
+    const socket = createConnection({port, host: "127.0.0.1"})
 
     socket.setTimeout(1000)
     socket.once("connect", () => {
@@ -66,18 +57,17 @@ const isRelayPortTaken = () =>
     socket.once("error", () => resolve(false))
   })
 
-// Every invocation carries ZOOID_CONFIG, `down` included: the compose file names it without a
+// Every invocation carries ZOOID_CONFIG, `down` included. The compose file names it without a
-// default, so a call that left it out would fail to interpolate rather than quietly mounting
+// default, so a call that left it out fails to interpolate rather than quietly mounting something
-// something else.
+// else.
 const docker = (...args: string[]) =>
   execFileAsync(dockerCommand, args, {env: {...process.env, ZOOID_CONFIG: configDir}})
 
 const compose = (...args: string[]) => docker("compose", "-f", composeFile, ...args)
 
-// Why the container cannot be driven, or undefined when it can. The two failures need different
+// Why the container cannot be driven, or undefined when it can. A cli that is not on this process's
-// answers — a cli that is not on this process's PATH is usually a shell alias or an inherited
+// PATH and a daemon that is not running need different answers, so they are reported apart rather
-// environment, and telling someone to start docker sends them the wrong way — so they are reported
+// than as one boolean.
-// apart rather than as one boolean.
 const probeDocker = async () => {
   try {
     await docker("compose", "version")
@@ -102,8 +92,8 @@ const probeDocker = async () => {
 }
 
 // How far the container's clock is from this host's, in seconds, read off the Date header of the
-// relay's own http answer. Positive means the container is behind, which is what makes an event the
+// relay's own http answer. Positive means the container is behind, so an event the harness has just
-// harness has just signed look like it comes from the future.
+// signed looks like it comes from the future.
 const getClockDrift = async (host: string) => {
   const {headers} = await requestZooid(host, "GET", "/", {accept: "application/nostr+json"})
 
@@ -112,10 +102,9 @@ const getClockDrift = async (host: string) => {
   }
 }
 
-// nip-42 accepts an auth event within ten minutes of the relay's own clock (nip42.go), and the two
+// nip-42 accepts an auth event within ten minutes of the relay's own clock (nip42.go), and under a
-// clocks here belong to different machines: the harness signs with this host's, the container
+// container runtime's vm the two clocks belong to different machines. The relay's own one-line
-// validates with the vm's. A vm whose clock stopped while the machine slept is the usual reason
+// detail says nothing about how to fix that.
-// they diverge, and the relay's own one-line detail says nothing about how to fix it.
 const describeClockDrift = (drift: number) => {
   const [minutes, direction] =
     drift > 0 ? [drift / 60, "behind"] : [Math.abs(drift) / 60, "ahead of"]
@@ -146,9 +135,8 @@ const getTestUser = (pubkey: string) => {
 
 let problem: Maybe<Promise<Maybe<string>>>
 
-// Asked once a worker: docker does not come and go mid-run, and the answer is also written to the
+// Asked once a worker, since docker does not come and go mid-run. The answer is also written to the
-// terminal, because a skip reason otherwise reaches only the html report and a run that silently
+// terminal, because a skip reason otherwise reaches only the html report.
-// skips every test is the least useful thing this can do.
 export const describeDockerProblem = () =>
   (problem ??= probeDocker().then(reason => {
     if (reason) {
@@ -161,14 +149,13 @@ export const describeDockerProblem = () =>
   }))
 
 /**
- * The relay every test runs against: one zooid container on loopback, serving a virtual relay per
+ * The relay every test runs against. One zooid container on loopback serves a virtual relay per
- * entry in `tenants`. Each relay's policy is whatever its toml in docker/config says, which is the
+ * entry in `tenants`, and each relay's policy is its toml in docker/config, the only place policy
- * only place policy is written down — a scenario describes what is *on* a relay, never what the
+ * is written down. A scenario describes what is on a relay, never what the relay is.
- * relay is.
  */
 export class Zooid {
-  // Every socket into the container — the client's and seeding's alike — is one this process
+  // Every socket into the container, the client's and seeding's alike, is one this process opened,
-  // opened, so this map is also the definition of a url that is not a leak.
+  // so this map is also the definition of a url that is not a leak.
   relays = new Map(
     tenantNames.map(name => [
       tenantUrl(name),
@@ -180,8 +167,8 @@ export class Zooid {
 
   private started = false
 
-  // Verifying docker rather than bringing the container up: every test resets, and that is what
+  // Verifies docker rather than bringing the container up, which `reset` does. Repeat calls are
-  // starts it. Repeat calls are free, so the fixture can call this per test.
+  // free, so the fixture can call this per test.
   start = async () => {
     if (this.started) return
 
@@ -204,13 +191,9 @@ export class Zooid {
       )
     }
 
-    // Nothing should answer on the relay port yet: this run has not brought its container up. If
-    // something does, it is a leftover container from an aborted run or another copy of the suite —
-    // and because the harness dials this fixed port, sharing it silently corrupts both runs. Refuse
-    // loudly instead. `compose down` clears a leftover of this project's own.
     if (await isRelayPortTaken()) {
       throw new Error(
-        `127.0.0.1:${relayHostPort} is already in use, but the zooid relay container publishes ` +
+        `127.0.0.1:${port} is already in use, but the zooid relay container publishes ` +
           "exactly that port and the harness talks to whatever answers there. This is a leftover " +
           "container from an interrupted run, or another copy of this suite running on the machine " +
           `(under any user). Free it before running — \`${dockerCommand} compose -f ` +
@@ -221,7 +204,7 @@ export class Zooid {
     this.started = true
   }
 
-  relay = async (name: TenantName) =>
+  relay = (name: TenantName) =>
     makeTestRelay({
       name,
       url: tenantUrl(name),
@@ -273,13 +256,12 @@ export class Zooid {
     throw new Error(logs ? `${summary}\n\nzooid said:\n${logs}` : summary)
   }
 
-  // Recreating rather than restarting is what makes this a reset: with storage in tmpfs and no
+  // Storage is tmpfs with no volume mounted for it, so a new container is a new database and
-  // volume mounted for it, a new container is a new database.
+  // recreating is what makes this a reset.
   private up = async () => {
-    // A fresh copy on every recreate is what makes a reset restore pristine relay metadata after a
+    // A fresh copy on every recreate is what restores relay metadata a test edited through nip-86.
-    // test has edited some through nip-86. The modes are permissive because the container runs as
+    // The modes are permissive because the container runs as uid 65532 while the copy belongs to
-    // uid 65532 while the copy belongs to whoever ran the suite, and a runtime that keeps host
+    // whoever ran the suite, and a runtime that keeps host ownership leaves zooid unable to write
-    // ownership — rootless podman, docker on linux — would otherwise leave zooid unable to write
     // the file it was told to save.
     await rm(configDir, {recursive: true, force: true})
     await cp(configSource, configDir, {recursive: true})
@@ -331,9 +313,8 @@ export class Zooid {
   }
 
   // NIP-42 binds an identity to a connection, so each test user seeds over its own, per relay. The
-  // url signed into the auth event is the one the client uses, because that is the one khatru
+  // url signed into the auth event is the one the client uses, since that is what khatru rebuilds
-  // rebuilds from the headers transport.ts sends — a fixture is written over the same relay the app
+  // from the headers transport.ts sends.
-  // talks to.
   private authenticate = async (host: string, user: TestUser) => {
     const key = `${host} ${user.pubkey}`
     const session = this.sessions.get(key)
@@ -354,8 +335,8 @@ export class Zooid {
     const summary = `Failed to authenticate as ${user.name} on ${host}: ${detail}`
     const drift = await getClockDrift(host).catch(() => undefined)
 
-    // Nearly always the clock rather than the key: every identity here is one zooid's toml names,
+    // Nearly always the clock rather than the key. Every identity here is one zooid's toml names,
-    // and the signature is checked last, after the timestamp the relay compares against its own.
+    // and the signature is checked after the timestamp the relay compares against its own.
     if (drift !== undefined && Math.abs(drift) > int(10, MINUTE)) {
       throw new Error(`${summary}\n\n${describeClockDrift(drift)}`)
     }
@@ -363,8 +344,8 @@ export class Zooid {
     throw new Error(summary)
   }
 
-  // Both halves of seeding — the auth that opens a connection and every event written over it —
+  // Both halves of seeding, the auth that opens a connection and every event written over it, are
-  // are answered with an OK naming the event that was sent.
+  // answered with an OK naming the event that was sent.
   private send = async (connection: ZooidConnection, message: ClientMessage, id: string) => {
     connection.send(message)
 

e2e/harness/zooid/testRelay.ts

@@ -20,9 +20,8 @@ export type TestRelayOptions = {
 // Seeding signs the events a real client would send and publishes them over a socket. Room
 // administration is signed by `admin`, the only test identity the tomls grant can_manage.
 //
-// Every timestamp is the caller's: seeding runs off the scenario's clock, and reading the wall
+// Every timestamp is the caller's. Reading the wall clock here would stamp two fixtures that
-// clock here would leave two fixtures that describe the same thing — a room's membership, say —
+// describe the same thing seconds apart, which is enough to decide which of them wins.
-// stamped seconds apart, which is enough to decide which of them wins.
 export const makeTestRelay = ({name, url, publish}: TestRelayOptions): TestRelay => {
   const event = async (user: TestUser, template: StampedEvent) => {
     const signed = await user.signer.sign(template)

e2e/harness/zooid/transport.ts

@@ -5,22 +5,23 @@ import type {ClientMessage, RelayMessage} from "@welshman/net"
 import type {RelayConnection} from "./types"
 
 /**
- * How the harness reaches the zooid container, and the only place that knows the container has a
+ * How the harness reaches the zooid container, and the only place that knows it has a loopback
- * loopback address at all.
+ * address at all.
  *
- * The client is given `wss://<tenant>.test/` and never learns anything else. It has to be that
+ * The client is given `wss://<tenant>.test/` and never learns anything else. A relay selection
- * rather than the container's own `ws://localhost:3334/`, because a relay selection drops a url
+ * drops a url that is local or insecure unless the caller opts in, which Flotilla never does
- * that is local or insecure unless the caller opts in — see isLocalUrl and RelaySelection.getUrls
+ * (isLocalUrl and RelaySelection.getUrls in @welshman/util), so the container's own
- * in @welshman/util — which Flotilla never does, so the Router would resolve every outbox load,
+ * `ws://localhost:3334/` would leave the Router resolving every outbox load, profile and relay hint
- * profile and relay hint to nothing.
+ * to nothing.
  *
- * The two headers below are what makes the container answer to that name. zooid's dispatcher binds
+ * The two headers below make the container answer to that name, and are what a tls terminator adds
- * a config to a Host (cmd/relay/main.go), and khatru derives the url it checks nip-42 and nip-86
+ * in front of a real deployment. zooid's dispatcher binds a config to a Host (cmd/relay/main.go),
- * signatures against from that Host plus the forwarded proto (getBaseURL in khatru/relay.go), so
+ * and khatru derives the url it checks nip-42 and nip-86 signatures against from that Host plus the
- * both the relay the config describes and the url it expects to have been signed are the tenant's.
+ * forwarded proto (getBaseURL in khatru/relay.go).
- * They are exactly what a tls terminator adds in front of a real deployment.
  */
-const port = 3334
+
+// Must match the published port in zooid/docker/compose.yaml.
+export const port = 3334
 
 const headersFor = (host: string) => ({Host: host, "X-Forwarded-Proto": "https"})
 
@@ -31,7 +32,7 @@ export type ZooidConnection = RelayConnection & {
 }
 
 // One connection to the container, as one of its virtual relays. Node's own WebSocket cannot carry
-// a Host header — fetch forbids it — so this is the one thing in the harness that `ws` is here for.
+// a Host header, which fetch forbids, so the harness depends on `ws` for this alone.
 export const connectToZooid = (host: string): ZooidConnection => {
   const socket = new WebSocket(`ws://127.0.0.1:${port}/`, {headers: headersFor(host)})
   const listeners = new Set<(message: RelayMessage) => void>()
@@ -61,8 +62,8 @@ export const connectToZooid = (host: string): ZooidConnection => {
     onMessage: listener => {
       listeners.add(listener)
     },
-    // A client may speak before the container has finished its handshake — the page's socket is
+    // A client may speak before the container has finished its handshake. The page's socket is open
-    // open as soon as playwright has a route for it, which is well before this one is.
+    // as soon as playwright has a route for it, well before this one is.
     send: message => {
       if (socket.readyState === WebSocket.OPEN) {
         write(message)
@@ -91,8 +92,8 @@ export type ZooidResponse = {
   body: Buffer
 }
 
-// One http request to the container, carrying the same two headers, so that the nip-11 document
+// One http request to the container, carrying the same two headers, so the nip-11 document and the
-// and the nip-86 management api the browser reads are the real relay's rather than a mock's.
+// nip-86 management api the browser reads are the real relay's.
 export const requestZooid = (
   host: string,
   method: string,
@@ -121,8 +122,8 @@ export const requestZooid = (
         const responseHeaders: Record<string, string> = {}
 
         for (const [name, value] of Object.entries(response.headers)) {
-          // Hop-by-hop headers describe the connection this answer arrived on rather than the one
+          // Hop-by-hop headers describe the connection this answer arrived on, not the one the
-          // the browser is waiting on, which playwright frames itself.
+          // browser is waiting on, which playwright frames itself.
           if (value && !["connection", "keep-alive", "transfer-encoding"].includes(name)) {
             responseHeaders[name] = Array.isArray(value) ? value.join(", ") : value
           }

e2e/harness/zooid/types.ts

@@ -4,8 +4,8 @@ import type {TestUser} from "../keys"
 
 export type PublishOptions = {
   // Which identity the seeding connection authenticates as. Defaults to the event's own author,
-  // which then has to be a test identity — a gift wrap is signed by an ephemeral key, so nothing
+  // which then has to be a test identity. A gift wrap is signed by an ephemeral key nothing in this
-  // in this process can sign as its author and the connection has to belong to the sender.
+  // process can authenticate as, so its connection belongs to the sender instead.
   as?: TestUser
 }
 
@@ -19,8 +19,7 @@ export type RoomOptions = {
 
 // A handle to one relay, plus the seeding affordances scenarios build on. Every seeding call
 // publishes over a real socket rather than inserting into storage, so the relay stores exactly what
-// it would have stored for a real client. Each one is stamped by its caller rather than off the
+// it would have stored for a real client.
-// wall clock, so a scenario's fixtures share one clock.
 export type TestRelay = {
   readonly name: string
   readonly url: string
@@ -30,13 +29,13 @@ export type TestRelay = {
   member(user: TestUser, h: string | undefined, createdAt: number): Promise<void>
   // Escape hatch for kinds with no affordance of their own: profiles, reactions, threads, DMs.
   event(user: TestUser, event: StampedEvent): Promise<SignedEvent>
-  // An event this process did not sign, sent over `as`'s connection — a gift wrap, whose author
+  // An event this process did not sign, sent over `as`'s connection. A gift wrap, whose author is
-  // is the ephemeral key that wrapped it.
+  // the ephemeral key that wrapped it, is the case that needs this.
   publish(event: SignedEvent, options: PublishOptions): Promise<void>
 }
 
-// One client's connection to a relay. Every connection to a url reaches the same container, which
+// One client's connection to a relay. Every connection to a url reaches the same container, so one
-// is what lets one browser context observe another's writes over the wire.
+// browser context observes another's writes over the wire.
 export type RelayConnection = {
   onMessage(listener: (message: RelayMessage) => void): void
   send(message: ClientMessage): void