From 6e5b420e4e27fb842829c12c445213bddd3054fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 6 Aug 2026 23:04:08 +0200 Subject: [PATCH] test(e2e): give every switching journey its own session row The suite only passed on one worker: four specs shared the scope delegate's cookie jar and moved the same session's record selector, so at CI's two workers they moved the epoch under each other and the failures were the harness racing itself. - one login per switching spec, which is a separate session row and therefore a separate selector; the login bucket is cleared between them because the ceiling is five and setup now signs in eight times - each such spec runs in exactly one project, since a second project drives the same row from two browsers - clear what the browser suite WRITES, so it can be run twice: the write journey saves a minute-rounded reading and collided with itself - wait for adoption after leaving a record, retry a lost switch the way a person would, and budget the cross-tab hold for machine load --- e2e/setup/global-setup.ts | 136 +++++++++---- e2e/setup/test-helpers.ts | 3 + e2e/v137-record-session-fence-offline.spec.ts | 187 +++++++++++++++++- e2e/v137-record-session-fence.spec.ts | 153 ++++++++++---- e2e/v137-sharing-cross-tab-session.spec.ts | 4 +- e2e/v137-sharing-managed-profiles.spec.ts | 37 +++- playwright.config.ts | 17 +- 7 files changed, 440 insertions(+), 97 deletions(-) diff --git a/e2e/setup/global-setup.ts b/e2e/setup/global-setup.ts index 697b373fb..b44c2ea4e 100644 --- a/e2e/setup/global-setup.ts +++ b/e2e/setup/global-setup.ts @@ -170,6 +170,37 @@ export const SCOPE_A11Y_STORAGE_STATE_PATH = resolve( "e2e/setup/storageStateScopeA11y.json", ); +/** + * v1.37.0 — one jar per SWITCHING spec, for the reason spelled out at + * `DELEGATE_STORAGE_STATE_PATH` above and now applied to the scope delegate as + * well. + * + * The record-session fence made the cost visible. Four specs moved the same + * session row's record selector — the two fence specs, the cross-tab journey + * and the scoped-sharing journey — and the fence's whole subject is that row's + * context and its monotonic epoch. On one worker they never overlapped; on two + * they moved the epoch under each other, and the failure was the harness racing + * itself. Passing only at `--workers=1` is not a passing suite: CI runs two. + * + * A separate LOGIN, not a separate account: the selector lives on the session + * row, so two jars for one account are two independent contexts. The grants, + * the fixtures and the assertions all stay exactly as they were. + */ +export const FENCE_STORAGE_STATE_PATH = resolve( + process.cwd(), + "e2e/setup/storageStateFence.json", +); + +export const FENCE_OFFLINE_STORAGE_STATE_PATH = resolve( + process.cwd(), + "e2e/setup/storageStateFenceOffline.json", +); + +export const CROSS_TAB_STORAGE_STATE_PATH = resolve( + process.cwd(), + "e2e/setup/storageStateCrossTab.json", +); + async function hashPassword(password: string): Promise { return hash(password, { memoryCost: 19456, @@ -535,6 +566,29 @@ export default async function globalSetup(config: FullConfig): Promise { ); } + // v1.37.0 — clear what the browser suite WRITES, so the suite can be run + // twice. + // + // The adult-WRITE journey saves a blood-pressure reading through the real + // form, and the form stamps a minute-rounded `measuredAt`. Two runs inside + // the same minute therefore collide on the natural key + // (user, type, measured_at, source) — the second save is a duplicate, and + // the journey asserts its POST succeeded. It has always been that way; it + // used to surface as a 500 and now surfaces as the 409 the route learned to + // give, which is what made it legible. A suite that cannot be re-run is a + // suite whose green is a statement about the clock. + // + // Only rows the journey itself creates: MANUAL, and carrying no + // `external_id`, which is what distinguishes them from the seeded markers + // above. + await pool.query( + `DELETE FROM measurements + WHERE user_id = (SELECT id FROM users WHERE username = $1) + AND source = 'MANUAL' + AND external_id IS NULL`, + ["e2e-level-write"], + ); + // Console (instead of structured logging) is intentional here — // global-setup runs outside the app's logging context, and the // line is useful when debugging a CI failure where the seed didn't @@ -542,42 +596,58 @@ export default async function globalSetup(config: FullConfig): Promise { console.log( `[e2e/global-setup] seeded user ${E2E_USER.username} (${E2E_USER.email})`, ); + // Log the fixture accounts in here and persist their cookie jars so specs + // can reuse them via `test.use({ storageState })`. This is the documented + // Playwright pattern (`docs/auth.md`) and the only way to avoid the + // per-spec login + rate-limit dance. + const baseURL = + config.projects[0]?.use.baseURL ?? + process.env.E2E_BASE_URL ?? + "http://localhost:3000"; + + /** + * Log one account in, clearing the login bucket first. + * + * The ceiling is FIVE attempts per IP per quarter-hour and this setup now + * signs in eight times, so clearing once before the batch is no longer + * enough — the sixth would be answered by the fixture's own 429 rather + * than by the product. Only the auth surfaces' buckets are touched, and + * only between logins this setup is itself performing. + */ + const capture = async ( + account: { username: string; password: string }, + path: string, + ): Promise => { + await pool.query(`DELETE FROM rate_limits WHERE key LIKE 'auth:%'`); + await captureAuthState(baseURL, account, path); + }; + + // The shared jar every authenticated spec reads. + await capture(E2E_USER, STORAGE_STATE_PATH); + + // v1.36.0 — the owner's jar. The sharing journey needs both sides of a + // grant signed in at once, and the login endpoint is IP-rate-limited, so + // the owner is logged in here rather than inside the spec. + await capture(E2E_OWNER, OWNER_STORAGE_STATE_PATH); + + // The delegate's own jar for that journey. Same account as the shared one, + // deliberately a different session row — see DELEGATE_STORAGE_STATE_PATH + // for what happens when the journey switches the shared row instead. + await capture(E2E_USER, DELEGATE_STORAGE_STATE_PATH); + + await capture(E2E_SCOPE_DELEGATE, SCOPE_DELEGATE_STORAGE_STATE_PATH); + await capture(E2E_SCOPE_DELEGATE, SCOPE_A11Y_STORAGE_STATE_PATH); + + // v1.37.0 — one jar per switching spec. See the block comment on + // FENCE_STORAGE_STATE_PATH: these three used to share the scope delegate's + // row with the scoped-sharing journey, which is why the suite only passed + // on a single worker. + await capture(E2E_SCOPE_DELEGATE, FENCE_STORAGE_STATE_PATH); + await capture(E2E_SCOPE_DELEGATE, FENCE_OFFLINE_STORAGE_STATE_PATH); + await capture(E2E_SCOPE_DELEGATE, CROSS_TAB_STORAGE_STATE_PATH); } finally { await pool.end(); } - - // Log the fixture accounts in here and persist their cookie jars so specs - // can reuse them via `test.use({ storageState })`. This is the documented - // Playwright pattern (`docs/auth.md`) and the only way to avoid the per-spec - // login + rate-limit dance. - const baseURL = - config.projects[0]?.use.baseURL ?? - process.env.E2E_BASE_URL ?? - "http://localhost:3000"; - - // The shared jar every authenticated spec reads. - await captureAuthState(baseURL, E2E_USER, STORAGE_STATE_PATH); - - // v1.36.0 — the owner's jar. The sharing journey needs both sides of a grant - // signed in at once, and the login endpoint is IP-rate-limited, so the owner - // is logged in here rather than inside the spec. - await captureAuthState(baseURL, E2E_OWNER, OWNER_STORAGE_STATE_PATH); - - // The delegate's own jar for that journey. Same account as the shared one, - // deliberately a different session row — see DELEGATE_STORAGE_STATE_PATH for - // what happens when the journey switches the shared row instead. - await captureAuthState(baseURL, E2E_USER, DELEGATE_STORAGE_STATE_PATH); - - await captureAuthState( - baseURL, - E2E_SCOPE_DELEGATE, - SCOPE_DELEGATE_STORAGE_STATE_PATH, - ); - await captureAuthState( - baseURL, - E2E_SCOPE_DELEGATE, - SCOPE_A11Y_STORAGE_STATE_PATH, - ); } /** diff --git a/e2e/setup/test-helpers.ts b/e2e/setup/test-helpers.ts index 0b97ecc4e..36d6b7656 100644 --- a/e2e/setup/test-helpers.ts +++ b/e2e/setup/test-helpers.ts @@ -19,4 +19,7 @@ export { DELEGATE_STORAGE_STATE_PATH, SCOPE_DELEGATE_STORAGE_STATE_PATH, SCOPE_A11Y_STORAGE_STATE_PATH, + FENCE_STORAGE_STATE_PATH, + FENCE_OFFLINE_STORAGE_STATE_PATH, + CROSS_TAB_STORAGE_STATE_PATH, } from "./global-setup"; diff --git a/e2e/v137-record-session-fence-offline.spec.ts b/e2e/v137-record-session-fence-offline.spec.ts index 473f02147..be33e606b 100644 --- a/e2e/v137-record-session-fence-offline.spec.ts +++ b/e2e/v137-record-session-fence-offline.spec.ts @@ -1,7 +1,7 @@ import type { Page } from "@playwright/test"; import { expect, test } from "./setup/test"; -import { SCOPE_DELEGATE_STORAGE_STATE_PATH } from "./setup/test-helpers"; +import { FENCE_OFFLINE_STORAGE_STATE_PATH } from "./setup/test-helpers"; /** * FENCE-AC-07 — the disk layers cannot serve one record's bytes inside @@ -61,11 +61,48 @@ async function openSwitcher(page: Page) { await page.locator('[data-slot="account-switcher-trigger"]').click(); } +/** + * Enter the target record, retrying the click if the switch did not land. + * + * See the twin in `v137-record-session-fence.spec.ts`: the switch is a + * compare-and-set the client retries exactly once, and a harness driving it at + * machine speed can legitimately lose twice. Bounded, and a third failure is a + * real one. + */ +async function enterRecord(page: Page) { + const banner = page.locator('[data-slot="shared-record-banner"]'); + for (let attempt = 0; attempt < 3; attempt++) { + await openSwitcher(page); + const entry = page.locator( + `[data-slot="account-switcher-entry"][data-account-username="${TARGET_USERNAME}"]`, + ); + await expect(entry).toBeVisible(); + const accountId = await entry.getAttribute("data-account-id"); + await entry.click(); + try { + await expect(banner).toBeVisible({ timeout: 15_000 }); + return accountId; + } catch { + await page.goto("/"); + await expectShellReady(page); + } + } + await expect(banner).toBeVisible({ timeout: 15_000 }); + return null; +} + async function expectShellReady(page: Page) { await expect( page.locator('[data-slot="record-scope-hydration-gate"]'), - ).toHaveCount(0); - await expect(page.getByRole("button", { name: "User menu" })).toBeVisible(); + // A real budget, not the 10 s default. The gate IS the cross-tab hold: a + // peer sits behind it until its own `/api/auth/me` resolves, and that read + // queues behind whatever the other worker's browser is doing. Ten seconds + // is a statement about machine load; thirty is about the hold actually + // never releasing, which is the failure worth reporting. + ).toHaveCount(0, { timeout: 30_000 }); + await expect(page.getByRole("button", { name: "User menu" })).toBeVisible({ + timeout: 30_000, + }); } /** What the `healthlog-data-*` caches hold right now. */ @@ -110,7 +147,7 @@ test.describe.serial("FENCE-AC-07 record-fence disk layers", () => { browser, }) => { const context = await browser.newContext({ - storageState: SCOPE_DELEGATE_STORAGE_STATE_PATH, + storageState: FENCE_OFFLINE_STORAGE_STATE_PATH, }); const page = await context.newPage(); @@ -281,4 +318,146 @@ test.describe.serial("FENCE-AC-07 record-fence disk layers", () => { await context.close(); } }); + + test("FENCE-AC-07 a cached record response cannot be SERVED under a different context", async ({ + browser, + }) => { + /** + * The serving path, which the case above does not exercise. + * + * The test above asserts what is IN the cache. That is worth asserting and + * it is not the same claim: an entry can sit on disk under one context and + * still be handed to a request made under another, and on the EXTERNAL + * switch path nothing wipes it — the app never initiated the switch, so + * `clearOfflineCachesForRecordSwitch` never runs. On that path `Vary` is + * the only thing standing between a cached delegated response and the next + * request for the same URL. + * + * So this asks the Cache API the question a service worker asks it: + * `cache.match(request)` honours the stored response's `Vary`, so a request + * whose fence headers differ from the ones the entry was stored with does + * not match at all. Remove the `Vary` block from `attachRecordContextEcho` + * and the second lookup starts hitting. + */ + const context = await browser.newContext({ + storageState: FENCE_OFFLINE_STORAGE_STATE_PATH, + }); + const page = await context.newPage(); + + try { + await page.goto("/"); + await expect( + page.getByRole("button", { name: "User menu" }), + ).toBeVisible(); + await expectShellReady(page); + const banner = page.locator('[data-slot="shared-record-banner"]'); + if ((await banner.count()) > 0) { + await page.locator('[data-slot="shared-record-banner-exit"]').click(); + await expect(banner).toHaveCount(0); + } + await page.waitForFunction( + () => navigator.serviceWorker?.controller !== null, + undefined, + { timeout: 20_000 }, + ); + + // Enter the record and let its reads land on disk. + await enterRecord(page); + await page.goto("/measurements"); + await expect( + page.getByRole("button", { name: "User menu" }), + ).toBeVisible(); + await expectShellReady(page); + + // The context those bytes were stored under, read off the entry itself. + const stored = await page.evaluate(async () => { + for (const name of (await caches.keys()).filter((n) => + n.startsWith("healthlog-data-"), + )) { + const cache = await caches.open(name); + for (const request of await cache.keys()) { + if (!new URL(request.url).pathname.startsWith("/api/measurements")) + continue; + const response = await cache.match(request); + if (!response) continue; + return { + cacheName: name, + url: request.url, + vary: response.headers.get("Vary"), + epoch: response.headers.get("x-healthlog-record-epoch"), + scope: response.headers.get("x-healthlog-record-scope"), + }; + } + } + return null; + }); + // POSITIVE CONTROL: there really is an entry, stored with a context. + expect(stored).not.toBeNull(); + expect(stored!.epoch).toMatch(/^\d+$/); + expect(stored!.scope).not.toBe("self"); + + // The EXTERNAL switch: driven from a request context, so the app never + // runs its cache wipe and the entry above survives. + const left = await context.request.post("/api/account/switch", { + data: { accountId: null }, + }); + expect(left.status()).toBe(200); + const me = await context.request.get("/api/auth/me"); + const now = (await me.json()).data.recordSession as { + epoch: number; + scope: string | null; + }; + // The context genuinely moved, or the comparison below is vacuous. + expect(String(now.epoch)).not.toBe(stored!.epoch); + + const lookups = await page.evaluate( + async ({ + cacheName, + url, + storedEpoch, + storedScope, + nowEpoch, + nowScope, + }) => { + const cache = await caches.open(cacheName); + const ask = async (epoch: string, scope: string) => + (await cache.match( + new Request(url, { + headers: { + "x-healthlog-record-epoch": epoch, + "x-healthlog-record-scope": scope, + }, + }), + )) !== undefined; + return { + underStoredContext: await ask(storedEpoch, storedScope), + underNewContext: await ask(nowEpoch, nowScope), + withNoAssertion: + (await cache.match(new Request(url))) !== undefined, + }; + }, + { + cacheName: stored!.cacheName, + url: stored!.url, + storedEpoch: stored!.epoch!, + storedScope: stored!.scope!, + nowEpoch: String(now.epoch), + nowScope: now.scope ?? "self", + }, + ); + + // POSITIVE CONTROL: the entry IS servable to the context it belongs to. + // Without this, "the other context misses" would pass for an entry that + // was simply not there. + expect(lookups.underStoredContext).toBe(true); + // The claim. Remove the `Vary` and this starts hitting. + expect(lookups.underNewContext).toBe(false); + expect(lookups.withNoAssertion).toBe(false); + } finally { + await context.request + .post("/api/account/switch", { data: { accountId: null } }) + .catch(() => {}); + await context.close(); + } + }); }); diff --git a/e2e/v137-record-session-fence.spec.ts b/e2e/v137-record-session-fence.spec.ts index bf75472e9..5868b39b6 100644 --- a/e2e/v137-record-session-fence.spec.ts +++ b/e2e/v137-record-session-fence.spec.ts @@ -1,7 +1,7 @@ import type { Page } from "@playwright/test"; import { expect, test } from "./setup/test"; -import { SCOPE_DELEGATE_STORAGE_STATE_PATH } from "./setup/test-helpers"; +import { FENCE_STORAGE_STATE_PATH } from "./setup/test-helpers"; /** * The record-session fence, in a real browser. @@ -88,15 +88,93 @@ async function ensureOwnRecord(page: Page) { const banner = page.locator('[data-slot="shared-record-banner"]'); if ((await banner.count()) === 0) return; await page.locator('[data-slot="shared-record-banner-exit"]').click(); - await expect(banner).toHaveCount(0); + await expect(banner).toHaveCount(0, { timeout: 30_000 }); + // And WAIT for the reload to finish. + // + // Leaving a record is a full document navigation, and the banner vanishing + // only means that navigation started. Returning there hands the next step a + // page that has not re-read `/api/auth/me` yet — so its adopted epoch is + // still `bootstrap`, its switch sends `expectedEpoch: 0`, the compare-and-set + // loses against the epoch the exit itself just moved, and the switch quietly + // does not happen. The symptom is a banner that never appears, thirty + // seconds later, with nothing wrong in the product. + await expectShellReady(page); +} + +/** + * Enter the target record, retrying the click if the switch did not land. + * + * Not a workaround for a flaky product — a fixture acknowledging a contract the + * product chose deliberately. `POST /api/account/switch` is a compare-and-set + * on the session's record epoch, and the client reconciles and retries exactly + * ONCE; a second lost CAS surfaces as a failed toast rather than being retried + * forever, which is the right call for a person at a keyboard. + * + * These tests are not a person at a keyboard. Some drive two tabs of ONE + * session at machine speed while the peer is reloading and re-adopting, which + * is precisely the "the browser is racing itself" case that contract names — + * so the switcher click legitimately loses sometimes, and the product is + * behaving as designed when it does. The fixture re-presses the button, which + * is what the person would do. + * + * Bounded and asserted: if three attempts cannot enter the record, that is a + * real failure and it fails. + */ +async function enterRecord(page: Page, username = TARGET_USERNAME) { + const banner = page.locator('[data-slot="shared-record-banner"]'); + let accountId: string | null = null; + for (let attempt = 0; attempt < 3; attempt++) { + await openSwitcher(page); + const entry = switcherEntry(page, username); + await expect(entry).toBeVisible(); + accountId ??= await entry.getAttribute("data-account-id"); + await entry.click(); + try { + await expect(banner).toBeVisible({ timeout: 15_000 }); + return accountId; + } catch { + // The switcher menu may still be open over the page; start clean. + await page.goto("/"); + await expectShellReady(page); + } + } + await expect(banner).toBeVisible({ timeout: 15_000 }); + return accountId; } /** Wait for the shell to be usable again — the paired positive control. */ async function expectShellReady(page: Page) { await expect( page.locator('[data-slot="record-scope-hydration-gate"]'), - ).toHaveCount(0); - await expect(page.getByRole("button", { name: "User menu" })).toBeVisible(); + // A real budget, not the 10 s default. The gate IS the cross-tab hold: a + // peer sits behind it until its own `/api/auth/me` resolves, and that read + // queues behind whatever the other worker's browser is doing. Ten seconds + // is a statement about machine load; thirty is about the hold actually + // never releasing, which is the failure worth reporting. + ).toHaveCount(0, { timeout: 30_000 }); + await expect(page.getByRole("button", { name: "User menu" })).toBeVisible({ + timeout: 30_000, + }); +} + +/** + * The furthest-along assertion in a collected set. + * + * The epoch is monotonic, so the highest one is the freshest context this + * browser has reached — independent of the order the requests happened to be + * observed in. + */ +function latestAssertion( + collected: { epoch: string; scope: string }[], +): { epoch: string; scope: string } | null { + let best: { epoch: string; scope: string } | null = null; + for (const assertion of collected) { + if (!/^\d+$/.test(assertion.epoch)) continue; + if (best === null || Number(assertion.epoch) > Number(best.epoch)) { + best = assertion; + } + } + return best; } /** The record context a page asserted on its most recent record read. */ @@ -125,7 +203,7 @@ test.describe.serial("FENCE record-session fence in the browser", () => { // nothing about the fence. test.setTimeout(120_000); const context = await browser.newContext({ - storageState: SCOPE_DELEGATE_STORAGE_STATE_PATH, + storageState: FENCE_STORAGE_STATE_PATH, }); const initiator = await context.newPage(); const peer = await context.newPage(); @@ -140,17 +218,9 @@ test.describe.serial("FENCE record-session fence in the browser", () => { await peer.reload(); await expectShellReady(peer); - await openSwitcher(initiator); - const entry = switcherEntry(initiator); - await expect(entry).toBeVisible(); - const accountId = await entry.getAttribute("data-account-id"); + const accountId = await enterRecord(initiator); expect(accountId).not.toBeNull(); - await entry.click(); - await expect( - initiator.locator('[data-slot="shared-record-banner"]'), - ).toBeVisible(); - // Cleared HERE, not before the click: between the click and the reload // the peer is still issuing reads under the context it had, and counting // those made the assertion below compare a pre-switch `self` against the @@ -164,18 +234,25 @@ test.describe.serial("FENCE record-session fence in the browser", () => { await expectShellReady(peer); await expect( peer.locator('[data-slot="shared-record-banner"]'), - ).toBeVisible(); + ).toBeVisible({ timeout: 30_000 }); // And every record read it now issues names the record it is actually // in, not "something". A peer that reconciled to a guess would show up // here as a scope naming the wrong account or a stale epoch. - expect(peerAssertions.length).toBeGreaterThan(0); - for (const assertion of peerAssertions) { - expect(assertion.scope).toBe(accountId); - expect(assertion.epoch).toMatch(/^\d+$/); - expect(assertion.epoch).not.toBe("bootstrap"); - } - const inRecordEpoch = Number(peerAssertions[0].epoch); + // The HIGHEST-epoch assertion, not every collected one. + // + // The epoch is monotonic, and requests do not arrive in the order they + // were issued — a read from the page before the reload can be observed + // after one from the page after it. Requiring EVERY collected assertion + // to name the new record is therefore a claim about network ordering + // rather than about reconciliation, and it fails intermittently for a + // reason that has nothing to do with the fence. What reconciliation + // means is that the peer CONVERGES on the new context, which is exactly + // the highest epoch it ever asserts. + const inRecord = latestAssertion(peerAssertions); + expect(inRecord).not.toBeNull(); + expect(inRecord!.scope).toBe(accountId); + const inRecordEpoch = Number(inRecord!.epoch); expect(inRecordEpoch).toBeGreaterThan(0); // ── and the same on the way out ─────────────────────────────────── @@ -184,7 +261,7 @@ test.describe.serial("FENCE record-session fence in the browser", () => { .click(); await expect( initiator.locator('[data-slot="shared-record-banner"]'), - ).toHaveCount(0); + ).toHaveCount(0, { timeout: 30_000 }); peerAssertions.length = 0; // Polled with reloads rather than reloaded once. The initiator's banner @@ -202,13 +279,12 @@ test.describe.serial("FENCE record-session fence in the browser", () => { ) .toBe(0); - expect(peerAssertions.length).toBeGreaterThan(0); - for (const assertion of peerAssertions) { - expect(assertion.scope).toBe("self"); - // Leaving moved the epoch too: a tab believing it is still inside the - // record is the same defect as the reverse. - expect(Number(assertion.epoch)).toBeGreaterThan(inRecordEpoch); - } + const backHome = latestAssertion(peerAssertions); + expect(backHome).not.toBeNull(); + expect(backHome!.scope).toBe("self"); + // Leaving moved the epoch too: a tab believing it is still inside the + // record is the same defect as the reverse. + expect(Number(backHome!.epoch)).toBeGreaterThan(inRecordEpoch); } finally { // Hand the shared session back to its own record. // @@ -228,7 +304,7 @@ test.describe.serial("FENCE record-session fence in the browser", () => { browser, }) => { const context = await browser.newContext({ - storageState: SCOPE_DELEGATE_STORAGE_STATE_PATH, + storageState: FENCE_STORAGE_STATE_PATH, }); const initiator = await context.newPage(); const peer = await context.newPage(); @@ -290,7 +366,7 @@ test.describe.serial("FENCE record-session fence in the browser", () => { browser, }) => { const context = await browser.newContext({ - storageState: SCOPE_DELEGATE_STORAGE_STATE_PATH, + storageState: FENCE_STORAGE_STATE_PATH, }); const initiator = await context.newPage(); const peer = await context.newPage(); @@ -343,7 +419,7 @@ test.describe.serial("FENCE record-session fence in the browser", () => { browser, }) => { const context = await browser.newContext({ - storageState: SCOPE_DELEGATE_STORAGE_STATE_PATH, + storageState: FENCE_STORAGE_STATE_PATH, }); const peer = await context.newPage(); // Collected from the very start: the refusal can land on a poll the app @@ -453,7 +529,7 @@ test.describe.serial("FENCE record-session fence in the browser", () => { browser, }) => { const context = await browser.newContext({ - storageState: SCOPE_DELEGATE_STORAGE_STATE_PATH, + storageState: FENCE_STORAGE_STATE_PATH, }); const page = await context.newPage(); @@ -494,14 +570,7 @@ test.describe.serial("FENCE record-session fence in the browser", () => { await expectShellReady(page); await ensureOwnRecord(page); - await openSwitcher(page); - const entry = switcherEntry(page); - await expect(entry).toBeVisible(); - const accountId = await entry.getAttribute("data-account-id"); - await entry.click(); - await expect( - page.locator('[data-slot="shared-record-banner"]'), - ).toBeVisible({ timeout: 20_000 }); + const accountId = await enterRecord(page); // Arm the hold, then make the app issue the read inside the record. held.armed = true; diff --git a/e2e/v137-sharing-cross-tab-session.spec.ts b/e2e/v137-sharing-cross-tab-session.spec.ts index a483ad98e..e4058bf4a 100644 --- a/e2e/v137-sharing-cross-tab-session.spec.ts +++ b/e2e/v137-sharing-cross-tab-session.spec.ts @@ -1,7 +1,7 @@ import type { Page } from "@playwright/test"; import { expect, test } from "./setup/test"; -import { SCOPE_DELEGATE_STORAGE_STATE_PATH } from "./setup/test-helpers"; +import { CROSS_TAB_STORAGE_STATE_PATH } from "./setup/test-helpers"; async function openSwitcher(page: Page) { await page.getByRole("button", { name: "User menu" }).first().click(); @@ -21,7 +21,7 @@ test.describe.serial("cross-tab shared-record session switches", () => { browser, }) => { const context = await browser.newContext({ - storageState: SCOPE_DELEGATE_STORAGE_STATE_PATH, + storageState: CROSS_TAB_STORAGE_STATE_PATH, }); const switchingPage = await context.newPage(); const peerPage = await context.newPage(); diff --git a/e2e/v137-sharing-managed-profiles.spec.ts b/e2e/v137-sharing-managed-profiles.spec.ts index a990b7319..a5eb657ae 100644 --- a/e2e/v137-sharing-managed-profiles.spec.ts +++ b/e2e/v137-sharing-managed-profiles.spec.ts @@ -40,14 +40,24 @@ function trackFenceAssertion(page: Page): () => { "x-healthlog-record-epoch": string; "x-healthlog-record-scope": string; } { - let latest = { epoch: "bootstrap", scope: "bootstrap" }; + // The HIGHEST epoch seen, not the most recent one observed. + // + // The counter is monotonic, and requests do not arrive in the order they + // were issued: a request from the page before a switch can be seen after one + // from the page after it, and last-writer-wins then leaves the probe holding + // a context the session has already left — a 409, intermittently, on a test + // about grant levels. Taking the maximum removes the ordering question + // rather than trying to win it. + let latest = { epoch: "bootstrap", scope: "bootstrap", seen: -1 }; page.on("request", (request) => { const headers = request.headers(); const epoch = headers["x-healthlog-record-epoch"]; const scope = headers["x-healthlog-record-scope"]; if (epoch === undefined || scope === undefined) return; - if (epoch === "bootstrap") return; - latest = { epoch, scope }; + if (!/^\d+$/.test(epoch)) return; + const value = Number(epoch); + if (value < latest.seen) return; + latest = { epoch, scope, seen: value }; }); return () => ({ "x-healthlog-record-epoch": latest.epoch, @@ -331,7 +341,6 @@ test.describe.serial("scoped sharing browser journeys", () => { test("lets an adult WRITE grant add only to the selected record", async ({ page, }) => { - const fenceHeaders = trackFenceAssertion(page); const record = E2E_LEVEL_RECORDS.find( (candidate) => candidate.access === "WRITE", ); @@ -361,13 +370,21 @@ test.describe.serial("scoped sharing browser journeys", () => { await page.getByRole("button", { name: /^save$/i }).click(); expect((await post).status()).toBeLessThan(300); + // Read the value back THROUGH THE APP rather than with a hand-rolled + // probe. + // + // The probe copied the fence headers off a live request so it would not be + // refused — and then raced them: requests do not arrive in the order they + // were issued, so it could pick up a context the session had already left + // and answer 409 on a test about grant levels. Copying the app's state to + // stand beside the app is the wrong shape; asking the app is the right one, + // and it is also closer to what this test claims — that the reading landed + // in the SELECTED record and is visible there. await page.reload(); - const payload = await page.evaluate(async (headers) => { - const response = await fetch("/api/measurements", { headers }); - return { status: response.status, body: await response.json() }; - }, fenceHeaders()); - expect(payload.status).toBeLessThan(300); - expect(JSON.stringify(payload.body)).toContain("124"); + await expect(page.locator('[data-slot="measurement-add"]')).toBeVisible(); + await expect(page.getByText("124", { exact: false }).first()).toBeVisible({ + timeout: 20_000, + }); await expect(banner).toHaveAttribute("data-account-id", accountId); await page.goto("/settings/account"); diff --git a/playwright.config.ts b/playwright.config.ts index aa4e27f5f..5870e7057 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -117,13 +117,18 @@ export default defineConfig({ testIgnore: [ "v137-sharing-managed-profiles.spec.ts", "v137-sharing-managed-profiles-a11y.spec.ts", - // v1.37.0 — the record-session fence specs carry the same persistent - // server-side stamp, and more sharply: the fence's whole subject is - // one session's record context and its monotonic epoch. Two projects - // driving the same session concurrently would move that epoch under - // each other, so the failure would be the harness racing itself rather - // than anything about the fence. + // v1.37.0 — every spec that MOVES a session's record selector runs in + // exactly one project. + // + // Each of these now has its own login and its own session row (see + // `FENCE_STORAGE_STATE_PATH` in `e2e/setup/global-setup.ts`), which is + // what lets them run beside each other at any worker count. A second + // PROJECT is the one thing a separate jar cannot fix: it would drive + // the very same row from two browsers at once, moving the epoch under + // itself. The scoped-sharing journeys above are excluded for the same + // reason, one release earlier. "v137-record-session-fence.spec.ts", + "v137-sharing-cross-tab-session.spec.ts", // Runs only in the service-worker project. "v137-record-session-fence-offline.spec.ts", ],