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", ],