From ac943f87cd6f5a11a746ebd562ad5c3da6449866 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Wed, 15 Apr 2026 20:41:04 +0200 Subject: [PATCH 01/14] Reorganise Playwright suite by user flow Splits specs across four projects: `account` (signup, signin, password, MFA), `features` (feature regressions against a pre-seeded Alice via `storageState`), `guest` (unauth flows), and `extension` (browser plugin). `globalSetup` signs Alice in once and persists her session to `tests/.auth/alice.json` so feature tests skip the login flow. Supporting changes: - `shared/test-users.ts` centralises per-test dedicated users; each auth-mutating test gets its own identity to avoid cross-test leakage. - `shared/delete-user.ts` (new) idempotently removes leftover Kratos identities via the Graph admin endpoint before re-running signups. - `shared/signin-utils.ts` (new) offers a lean sign-in helper. - `shared/signup-utils.ts` now randomises shortnames and cleans up prior Kratos identities before registering. - `shared/runtime.ts` captures a small set of expected 4xx responses (whoami 401, AAL2-upgrade 422, recovery 422, self-service 400) as console-noise suppressions correlated by status code. - `shared/get-kratos-verification-code.ts` adds `getKratosRecoveryCode` and normalises Mailslurper timestamps to UTC. - `.env.test` allowlist lists the new test identities. - `.gitignore` excludes the per-test `.auth` storage-state directory. - Root `package.json` gains `start:test:backend` / `start:test:frontend` helpers to spin up only the services a given suite needs. - `docker-compose.yml` threads `SELFSERVICE_FLOWS_SETTINGS_PRIVILEGED_SESSION_MAX_AGE` through to Kratos with a 5-minute default so tests can tighten it via env var. --- .env.test | 2 +- .gitignore | 1 + .../hash-external-services/docker-compose.yml | 1 + package.json | 2 + tests/hash-playwright/global-setup.ts | 28 ++++++ tests/hash-playwright/playwright.config.ts | 35 +++++-- .../tests/{ => account}/mfa.spec.ts | 59 +++--------- .../tests/account/password.spec.ts | 93 +++++++++++++++++++ .../tests/account/signin.spec.ts | 30 ++++++ .../tests/{ => account}/signup.spec.ts | 21 ++--- .../{ => features}/entities-page.spec.ts | 22 +---- .../{ => features}/entity-editing.spec.ts | 24 +---- .../entity-type-creation.spec.ts | 22 ++--- .../tests/{ => features}/inbox-page.spec.ts | 14 +-- .../{ => features}/page-creation.spec.ts | 15 +-- .../{ => features}/page-navigation.spec.ts | 17 +--- .../{ => features}/page-readonly-mode.spec.ts | 9 +- .../tests/{ => features}/profile-page.spec.ts | 20 +--- .../tests/{ => features}/types-page.spec.ts | 10 +- .../tests/{ => guest}/guest-user.spec.ts | 7 +- .../shared/get-kratos-verification-code.ts | 65 ++++++++++++- .../hash-playwright/tests/shared/reset-db.ts | 6 -- tests/hash-playwright/tests/shared/runtime.ts | 3 + .../tests/shared/signin-utils.ts | 29 ++++++ .../tests/shared/signup-utils.ts | 28 ++---- .../tests/shared/test-users.ts | 70 ++++++++++++++ 26 files changed, 406 insertions(+), 227 deletions(-) create mode 100644 tests/hash-playwright/global-setup.ts rename tests/hash-playwright/tests/{ => account}/mfa.spec.ts (69%) create mode 100644 tests/hash-playwright/tests/account/password.spec.ts create mode 100644 tests/hash-playwright/tests/account/signin.spec.ts rename tests/hash-playwright/tests/{ => account}/signup.spec.ts (76%) rename tests/hash-playwright/tests/{ => features}/entities-page.spec.ts (61%) rename tests/hash-playwright/tests/{ => features}/entity-editing.spec.ts (92%) rename tests/hash-playwright/tests/{ => features}/entity-type-creation.spec.ts (84%) rename tests/hash-playwright/tests/{ => features}/inbox-page.spec.ts (92%) rename tests/hash-playwright/tests/{ => features}/page-creation.spec.ts (96%) rename tests/hash-playwright/tests/{ => features}/page-navigation.spec.ts (78%) rename tests/hash-playwright/tests/{ => features}/page-readonly-mode.spec.ts (89%) rename tests/hash-playwright/tests/{ => features}/profile-page.spec.ts (80%) rename tests/hash-playwright/tests/{ => features}/types-page.spec.ts (78%) rename tests/hash-playwright/tests/{ => guest}/guest-user.spec.ts (93%) delete mode 100644 tests/hash-playwright/tests/shared/reset-db.ts create mode 100644 tests/hash-playwright/tests/shared/signin-utils.ts create mode 100644 tests/hash-playwright/tests/shared/test-users.ts diff --git a/.env.test b/.env.test index 91fe264ab7a..a3ea91352f8 100644 --- a/.env.test +++ b/.env.test @@ -13,4 +13,4 @@ AWS_S3_UPLOADS_SECRET_ACCESS_KEY="dev-s3-secret-access-key" AWS_S3_UPLOADS_FORCE_PATH_STYLE=true FILE_UPLOAD_PROVIDER="AWS_S3" -USER_EMAIL_ALLOW_LIST='["charlie@example.com", "mfa-enable-totp@example.com", "mfa-totp-login@example.com", "mfa-backup-code@example.com", "mfa-disable-totp@example.com", "mfa-wrong-code@example.com"]' +USER_EMAIL_ALLOW_LIST='["signup-allow@example.com", "signin-test@example.com", "signout-test@example.com", "pw-change@example.com", "pw-recovery@example.com", "mfa-enable@example.com", "mfa-login@example.com", "mfa-backup@example.com", "mfa-disable@example.com", "mfa-wrong-code@example.com"]' diff --git a/.gitignore b/.gitignore index 3a811f18ebd..15708ce8041 100644 --- a/.gitignore +++ b/.gitignore @@ -40,6 +40,7 @@ bower_components # Tests **/playwright-report/ **/test-results/ +tests/**/.auth/ # Compiled binary addons (https://nodejs.org/api/addons.html) **/build/Release diff --git a/apps/hash-external-services/docker-compose.yml b/apps/hash-external-services/docker-compose.yml index c4a0d2400b3..916282b1c98 100644 --- a/apps/hash-external-services/docker-compose.yml +++ b/apps/hash-external-services/docker-compose.yml @@ -94,6 +94,7 @@ services: SELFSERVICE_FLOWS_VERIFICATION_UI_URL: "http://localhost:3000/verification" SELFSERVICE_FLOWS_RECOVERY_UI_URL: "http://localhost:3000/recovery" SELFSERVICE_FLOWS_SETTINGS_UI_URL: "http://localhost:3000/settings/security" + SELFSERVICE_FLOWS_SETTINGS_PRIVILEGED_SESSION_MAX_AGE: "${SELFSERVICE_FLOWS_SETTINGS_PRIVILEGED_SESSION_MAX_AGE:-5m}" COURIER_SMTP_CONNECTION_URI: "smtps://test:test@mailslurper:1025/?skip_ssl_verify=true" LOG_LEVEL: debug LOG_FORMAT: text diff --git a/package.json b/package.json index 43b19455ce4..dcdffa1202a 100644 --- a/package.json +++ b/package.json @@ -65,6 +65,8 @@ "start:frontend": "CARGO_TERM_PROGRESS_WHEN=never turbo run start start:healthcheck --filter @apps/hash-frontend --env-mode=loose", "start:graph": "CARGO_TERM_PROGRESS_WHEN=never turbo run start start:healthcheck --filter @apps/hash-graph --env-mode=loose", "start:test": "CARGO_TERM_PROGRESS_WHEN=never turbo run start:test start:test:healthcheck --env-mode=loose", + "start:test:backend": "CARGO_TERM_PROGRESS_WHEN=never turbo run start:test start:test:healthcheck --filter @apps/hash-api --env-mode=loose", + "start:test:frontend": "CARGO_TERM_PROGRESS_WHEN=never turbo run start:test start:test:healthcheck --filter @apps/hash-frontend --env-mode=loose", "start:test:graph": "CARGO_TERM_PROGRESS_WHEN=never turbo run start:test start:test:healthcheck --filter @apps/hash-graph --env-mode=loose", "start:test:worker": "CARGO_TERM_PROGRESS_WHEN=never turbo run start:test start:test:healthcheck --filter '@apps/hash-*-worker*' --env-mode=loose", "start:worker": "CARGO_TERM_PROGRESS_WHEN=never turbo run start start:healthcheck --filter '@apps/hash-*-worker*' --env-mode=loose", diff --git a/tests/hash-playwright/global-setup.ts b/tests/hash-playwright/global-setup.ts new file mode 100644 index 00000000000..e49981239cc --- /dev/null +++ b/tests/hash-playwright/global-setup.ts @@ -0,0 +1,28 @@ +import { chromium } from "@playwright/test"; + +const baseURL = "http://localhost:3000"; + +/** + * Sign in the pre-seeded `alice` user once before the suite runs and + * persist her session to `tests/.auth/alice.json`. Feature tests that + * need an authenticated user load this `storageState` instead of + * repeating the sign-in flow. + */ +export default async function globalSetup() { + const browser = await chromium.launch(); + const context = await browser.newContext({ baseURL }); + const page = await context.newPage(); + + await page.goto("/signin"); + await page.fill( + '[placeholder="Enter your email address"]', + "alice@example.com", + ); + await page.fill('[type="password"]', "password"); + await page.click("text=Submit"); + await page.waitForURL("/", { timeout: 30_000 }); + + await context.storageState({ path: "tests/.auth/alice.json" }); + + await browser.close(); +} diff --git a/tests/hash-playwright/playwright.config.ts b/tests/hash-playwright/playwright.config.ts index f0fd4566144..b43543237be 100644 --- a/tests/hash-playwright/playwright.config.ts +++ b/tests/hash-playwright/playwright.config.ts @@ -5,27 +5,42 @@ const ci = process.env.CI === "true"; const config: PlaywrightTestConfig = { forbidOnly: ci, - timeout: 60_000, + globalSetup: "./global-setup", projects: [ - { name: "chromium", use: { ...devices["Desktop Chrome"] } }, - // We plan to add more browsers and also split Playwright tests into - // system (integration) tests and end-to-end tests. + { + name: "account", + testMatch: "tests/account/**", + use: { ...devices["Desktop Chrome"] }, + }, + { + name: "features", + testMatch: "tests/features/**", + use: { + ...devices["Desktop Chrome"], + storageState: "tests/.auth/alice.json", + }, + dependencies: ["account"], + }, + { + name: "guest", + testMatch: "tests/guest/**", + use: { ...devices["Desktop Chrome"] }, + }, + { + name: "extension", + testMatch: "tests/extension/**", + use: { ...devices["Desktop Chrome"] }, + }, ], reporter: [ [ci ? "github" : "list"], ["html", { open: !ci ? "on-failure" : "never" }], ], - retries: ci ? 2 : 0, // 2 retries in CI compensates flakiness, 0 is more helpful locally testDir: "tests", use: { baseURL: "http://localhost:3000", - - // Playwright docs recommend "on-first-retry" as it is slightly more resource-efficient. - // We can switch to this option when we have more tests and most of them are stable. trace: "retain-on-failure", }, - - workers: 1, // Concurrent tests break login }; export default config; diff --git a/tests/hash-playwright/tests/mfa.spec.ts b/tests/hash-playwright/tests/account/mfa.spec.ts similarity index 69% rename from tests/hash-playwright/tests/mfa.spec.ts rename to tests/hash-playwright/tests/account/mfa.spec.ts index ec8c554d411..9c031d45f4e 100644 --- a/tests/hash-playwright/tests/mfa.spec.ts +++ b/tests/hash-playwright/tests/account/mfa.spec.ts @@ -1,7 +1,7 @@ -import { resetDb } from "./shared/reset-db"; -import { expect, type Page, test } from "./shared/runtime"; -import { createUserAndCompleteSignup } from "./shared/signup-utils"; -import { generateTotpCode, waitForFreshTotpWindow } from "./shared/totp-utils"; +import { expect, type Page, test } from "../shared/runtime"; +import { signInWithPassword, signOut } from "../shared/signin-utils"; +import { testUsers, withTestUser } from "../shared/test-users"; +import { generateTotpCode, waitForFreshTotpWindow } from "../shared/totp-utils"; const enableTotpForCurrentUser = async (page: Page) => { await page.goto("/settings/security"); @@ -42,25 +42,8 @@ const enableTotpForCurrentUser = async (page: Page) => { return { backupCodes, secret }; }; -const signInWithPassword = async ( - page: Page, - { email, password }: { email: string; password: string }, -) => { - await page.goto("/signin"); - await page.fill('[placeholder="Enter your email address"]', email); - await page.fill('[placeholder="Enter your password"]', password); - await page.click("text=Submit"); -}; - -test.beforeEach(async () => { - await resetDb(); -}); - test("user can enable TOTP", async ({ page }) => { - await createUserAndCompleteSignup(page, { - email: "mfa-enable-totp@example.com", - shortname: "mfa-enable-totp", - }); + await withTestUser(page, testUsers.mfaEnable); const { backupCodes } = await enableTotpForCurrentUser(page); @@ -68,13 +51,10 @@ test("user can enable TOTP", async ({ page }) => { }); test("user with TOTP is prompted for code at login", async ({ page }) => { - const credentials = await createUserAndCompleteSignup(page, { - email: "mfa-totp-login@example.com", - shortname: "mfa-totp-login", - }); + const credentials = await withTestUser(page, testUsers.mfaLogin); const { secret } = await enableTotpForCurrentUser(page); - await page.context().clearCookies(); + await signOut(page); await signInWithPassword(page, credentials); @@ -93,15 +73,12 @@ test("user with TOTP is prompted for code at login", async ({ page }) => { }); test("user can use backup code instead of TOTP", async ({ page }) => { - const credentials = await createUserAndCompleteSignup(page, { - email: "mfa-backup-code@example.com", - shortname: "mfa-backup-code", - }); + const credentials = await withTestUser(page, testUsers.mfaBackup); const { backupCodes } = await enableTotpForCurrentUser(page); expect(backupCodes.length).toBeGreaterThan(0); - await page.context().clearCookies(); + await signOut(page); await signInWithPassword(page, credentials); await expect( @@ -116,10 +93,7 @@ test("user can use backup code instead of TOTP", async ({ page }) => { }); test("user can disable TOTP", async ({ page }) => { - const credentials = await createUserAndCompleteSignup(page, { - email: "mfa-disable-totp@example.com", - shortname: "mfa-disable-totp", - }); + const credentials = await withTestUser(page, testUsers.mfaDisable); await enableTotpForCurrentUser(page); await page.goto("/settings/security"); @@ -131,10 +105,6 @@ test("user can disable TOTP", async ({ page }) => { ).toBeVisible(); // Backup codes should also be cleared as part of the disable flow. - // Without `lookup_secret_disable: true` the user would be left at a - // permanently-required AAL2 with only orphan backup codes — so re-loading - // the security page should not surface the "Regenerate backup codes" - // button (which only appears while a `lookup_secret` credential exists). await page.reload(); await expect( page.locator('[data-testid="show-enable-totp-form-button"]'), @@ -143,7 +113,7 @@ test("user can disable TOTP", async ({ page }) => { page.locator('[data-testid="regenerate-backup-codes-button"]'), ).not.toBeVisible(); - await page.context().clearCookies(); + await signOut(page); await signInWithPassword(page, credentials); @@ -154,13 +124,10 @@ test("user can disable TOTP", async ({ page }) => { }); test("wrong TOTP code shows error at login", async ({ page }) => { - const credentials = await createUserAndCompleteSignup(page, { - email: "mfa-wrong-code@example.com", - shortname: "mfa-wrong-code", - }); + const credentials = await withTestUser(page, testUsers.mfaWrongCode); await enableTotpForCurrentUser(page); - await page.context().clearCookies(); + await signOut(page); await signInWithPassword(page, credentials); await expect( diff --git a/tests/hash-playwright/tests/account/password.spec.ts b/tests/hash-playwright/tests/account/password.spec.ts new file mode 100644 index 00000000000..90cc0275d41 --- /dev/null +++ b/tests/hash-playwright/tests/account/password.spec.ts @@ -0,0 +1,93 @@ +import { getKratosRecoveryCode } from "../shared/get-kratos-verification-code"; +import { expect, test } from "../shared/runtime"; +import { + expectSignedIn, + signInWithPassword, + signOut, +} from "../shared/signin-utils"; +import { testUsers, withTestUser } from "../shared/test-users"; + +test("user can change password from settings", async ({ page }) => { + const newPassword = "changed-pw-5ef6"; + const credentials = await withTestUser(page, testUsers.pwChange); + + await page.goto("/settings/security", { waitUntil: "networkidle" }); + + await page.fill('[placeholder="Enter your new password"]', newPassword); + await page.click("text=Update password"); + + await expect(page.locator("text=Your changes have been saved")).toBeVisible({ + timeout: 5_000, + }); + + await signOut(page); + await signInWithPassword(page, { + email: credentials.email, + password: newPassword, + }); + await expectSignedIn(page); +}); + +test("user can recover account and set a new password", async ({ page }) => { + const newPassword = "recovered-pw-3cd4"; + const credentials = await withTestUser(page, testUsers.pwRecovery); + + await signOut(page); + + // Start recovery flow + const recoveryFlowReady = page.waitForResponse( + (response) => + response.request().method() === "GET" && + response.url().includes("/auth/self-service/recovery/browser"), + { timeout: 10_000 }, + ); + + await page.goto("/recovery"); + await recoveryFlowReady; + + const recoveryTimestamp = Date.now(); + + await page.fill( + '[placeholder="Enter your email address"]', + credentials.email, + ); + + const recoverySubmitComplete = page.waitForResponse( + (response) => + response.request().method() === "POST" && + response.url().includes("/auth/self-service/recovery"), + { timeout: 10_000 }, + ); + + await page.click("text=Recover account"); + await recoverySubmitComplete; + + await expect( + page.locator('[placeholder="Enter your verification code"]'), + ).toBeVisible({ timeout: 5_000 }); + + const recoveryCode = await getKratosRecoveryCode( + credentials.email, + recoveryTimestamp, + ); + + await page.fill('[placeholder="Enter your verification code"]', recoveryCode); + + // Kratos redirects to settings/security after successful recovery + await page.waitForURL("**/settings/security**", { timeout: 5_000 }); + + // Recovery session is privileged — password change must work without re-auth + await page.fill('[placeholder="Enter your new password"]', newPassword); + await page.click("text=Update password"); + + await expect(page.locator("text=Your changes have been saved")).toBeVisible({ + timeout: 5_000, + }); + + await signOut(page); + await signInWithPassword(page, { + email: credentials.email, + password: newPassword, + }); + await expectSignedIn(page); +}); diff --git a/tests/hash-playwright/tests/account/signin.spec.ts b/tests/hash-playwright/tests/account/signin.spec.ts new file mode 100644 index 00000000000..0ae5a04f59d --- /dev/null +++ b/tests/hash-playwright/tests/account/signin.spec.ts @@ -0,0 +1,30 @@ +import { expect, test } from "../shared/runtime"; +import { + expectSignedIn, + expectSignedOut, + signInWithPassword, + signOut, +} from "../shared/signin-utils"; +import { testUsers, withTestUser } from "../shared/test-users"; + +test("user can sign in with password", async ({ page }) => { + const credentials = await withTestUser(page, testUsers.signinTest); + + await signOut(page); + await expectSignedOut(page); + + await signInWithPassword(page, credentials); + await expectSignedIn(page); +}); + +test("user can sign out via account menu", async ({ page }) => { + await withTestUser(page, testUsers.signoutTest); + + // Use the real sign-out flow (not clearCookies) + await page.getByTestId("user-avatar").click(); + await page.getByText("Sign Out").click(); + + await expect(page.getByRole("link", { name: "Sign In" })).toBeVisible({ + timeout: 5_000, + }); +}); diff --git a/tests/hash-playwright/tests/signup.spec.ts b/tests/hash-playwright/tests/account/signup.spec.ts similarity index 76% rename from tests/hash-playwright/tests/signup.spec.ts rename to tests/hash-playwright/tests/account/signup.spec.ts index c3705cda015..4d6981c1b1e 100644 --- a/tests/hash-playwright/tests/signup.spec.ts +++ b/tests/hash-playwright/tests/account/signup.spec.ts @@ -1,23 +1,19 @@ -import { resetDb } from "./shared/reset-db"; -import { expect, test } from "./shared/runtime"; +import { deleteUserByEmail } from "../shared/delete-user"; +import { expect, test } from "../shared/runtime"; import { completeSignup, registerUser, verifyEmailOnPage, -} from "./shared/signup-utils"; - -test.beforeEach(async () => { - await resetDb(); -}); - -const allowlistedEmail = "charlie@example.com"; +} from "../shared/signup-utils"; +import { testUsers } from "../shared/test-users"; test("allowlisted user can verify email and complete signup", async ({ page, }) => { - const { email, emailDispatchTimestamp } = await registerUser(page, { - email: allowlistedEmail, - }); + const { email } = testUsers.signupAllowlisted; + await deleteUserByEmail(email); + + const { emailDispatchTimestamp } = await registerUser(page, { email }); await verifyEmailOnPage(page, { email, @@ -40,7 +36,6 @@ test("waitlisted user is redirected to waitlist after signup", async ({ email: waitlistedEmail, }); - // Waitlisted users must also verify their email before proceeding await verifyEmailOnPage(page, { email: waitlistedEmail, afterTimestamp: emailDispatchTimestamp, diff --git a/tests/hash-playwright/tests/entities-page.spec.ts b/tests/hash-playwright/tests/features/entities-page.spec.ts similarity index 61% rename from tests/hash-playwright/tests/entities-page.spec.ts rename to tests/hash-playwright/tests/features/entities-page.spec.ts index 4ba21d3fd62..e41e5ea5d60 100644 --- a/tests/hash-playwright/tests/entities-page.spec.ts +++ b/tests/hash-playwright/tests/features/entities-page.spec.ts @@ -1,19 +1,8 @@ -import { changeSidebarListDisplay } from "./shared/change-sidebar-list-display"; -import { loginUsingTempForm } from "./shared/login-using-temp-form"; -import { resetDb } from "./shared/reset-db"; -import { expect, test } from "./shared/runtime"; - -test.beforeEach(async () => { - await resetDb(); -}); +import { changeSidebarListDisplay } from "../shared/change-sidebar-list-display"; +import { expect, test } from "../shared/runtime"; test("user can visit a page listing entities of a type", async ({ page }) => { - await loginUsingTempForm({ - page, - userEmail: "alice@example.com", - userPassword: "password", - }); - + await page.goto("/"); // Check if we are on the logged-in homepage await expect(page.locator("text=Get support")).toBeVisible(); @@ -24,12 +13,11 @@ test("user can visit a page listing entities of a type", async ({ page }) => { section: "Entities", }); - // Expand the entities list in the sidebar + // Expand the entities list in the sidebar and wait for types to load await page.locator("text=Entities").first().click(); - // Wait for 'Document' to be visible before clicking (sidebar expand animation) const documentItem = page.locator("text=Document").first(); - await expect(documentItem).toBeVisible(); + await expect(documentItem).toBeVisible({ timeout: 10_000 }); await documentItem.click(); // Check if we are on the 'Document' entities page diff --git a/tests/hash-playwright/tests/entity-editing.spec.ts b/tests/hash-playwright/tests/features/entity-editing.spec.ts similarity index 92% rename from tests/hash-playwright/tests/entity-editing.spec.ts rename to tests/hash-playwright/tests/features/entity-editing.spec.ts index 9e67c912dd9..fa3b72cfd8b 100644 --- a/tests/hash-playwright/tests/entity-editing.spec.ts +++ b/tests/hash-playwright/tests/features/entity-editing.spec.ts @@ -1,10 +1,8 @@ import { gridRowHeight } from "@local/hash-isomorphic-utils/data-grid"; import { sleep } from "@local/hash-isomorphic-utils/sleep"; -import { loginUsingTempForm } from "./shared/login-using-temp-form"; -import { resetDb } from "./shared/reset-db"; -import type { Locator, Page } from "./shared/runtime"; -import { expect, test } from "./shared/runtime"; +import type { Locator, Page } from "../shared/runtime"; +import { expect, test } from "../shared/runtime"; /** * This gets the text for the requested cell in the hidden html table, @@ -104,18 +102,9 @@ const clickOnValueCell = async ( await page.mouse.click(cellX, cellY); }; -test.beforeEach(async () => { - await resetDb(); -}); - /** This is a temporary test to commit the progress made on testing `Grid` component. */ test("user can update values on property table", async ({ page }) => { - await loginUsingTempForm({ - page, - userEmail: "alice@example.com", - userPassword: "password", - }); - + await page.goto("/"); await expect(page.locator("text=Get support")).toBeVisible(); await page.goto(`/new/entity`); @@ -156,12 +145,7 @@ test("user can update values on property table", async ({ page }) => { test("both the link and properties tables renders some content", async ({ page, }) => { - await loginUsingTempForm({ - page, - userEmail: "alice@example.com", - userPassword: "password", - }); - + await page.goto("/"); await expect(page.locator("text=Get support")).toBeVisible(); await page.goto(`/new/entity`); diff --git a/tests/hash-playwright/tests/entity-type-creation.spec.ts b/tests/hash-playwright/tests/features/entity-type-creation.spec.ts similarity index 84% rename from tests/hash-playwright/tests/entity-type-creation.spec.ts rename to tests/hash-playwright/tests/features/entity-type-creation.spec.ts index e78af082e2b..f518a05b712 100644 --- a/tests/hash-playwright/tests/entity-type-creation.spec.ts +++ b/tests/hash-playwright/tests/features/entity-type-creation.spec.ts @@ -1,21 +1,10 @@ import { sleep } from "@local/hash-isomorphic-utils/sleep"; -import { changeSidebarListDisplay } from "./shared/change-sidebar-list-display"; -import { loginUsingTempForm } from "./shared/login-using-temp-form"; -import { resetDb } from "./shared/reset-db"; -import { expect, test } from "./shared/runtime"; - -test.beforeEach(async () => { - await resetDb(); -}); +import { changeSidebarListDisplay } from "../shared/change-sidebar-list-display"; +import { expect, test } from "../shared/runtime"; test("user can create entity type", async ({ page }) => { - await loginUsingTempForm({ - page, - userEmail: "alice@example.com", - userPassword: "password", - }); - + await page.goto("/"); // Check if we are on the user page await expect(page.locator("text=Get support")).toBeVisible(); @@ -26,6 +15,11 @@ test("user can create entity type", async ({ page }) => { section: "Types", }); + // Expand the Types section so the "create entity type" button is in the + // DOM — the button is only rendered as an end-adornment on the Types + // list header when the section is open. + await page.locator("text=Types").first().click(); + // Go to Create Entity Type await page.locator('[data-testid="create-entity-type-btn"]').click(); await page.waitForURL( diff --git a/tests/hash-playwright/tests/inbox-page.spec.ts b/tests/hash-playwright/tests/features/inbox-page.spec.ts similarity index 92% rename from tests/hash-playwright/tests/inbox-page.spec.ts rename to tests/hash-playwright/tests/features/inbox-page.spec.ts index f6afc27c87f..b43450fe282 100644 --- a/tests/hash-playwright/tests/inbox-page.spec.ts +++ b/tests/hash-playwright/tests/features/inbox-page.spec.ts @@ -10,10 +10,9 @@ import type { } from "@local/hash-isomorphic-utils/system-types/graphchangenotification"; import type { Page } from "@local/hash-isomorphic-utils/system-types/shared"; -import { createEntity, getUser } from "./shared/api-queries"; -import { loginUsingTempForm } from "./shared/login-using-temp-form"; -import type { APIRequestContext } from "./shared/runtime"; -import { expect, test } from "./shared/runtime"; +import { createEntity, getUser } from "../shared/api-queries"; +import type { APIRequestContext } from "../shared/runtime"; +import { expect, test } from "../shared/runtime"; const createNotification = async ({ draft, @@ -101,14 +100,9 @@ const createNotification = async ({ }; test("new notifications are shown on notifications page", async ({ page }) => { + await page.goto("/"); test.setTimeout(60_000); - await loginUsingTempForm({ - page, - userEmail: "alice@example.com", - userPassword: "password", - }); - await expect(page.locator("text=Get support")).toBeVisible(); await page.goto("/notifications"); diff --git a/tests/hash-playwright/tests/page-creation.spec.ts b/tests/hash-playwright/tests/features/page-creation.spec.ts similarity index 96% rename from tests/hash-playwright/tests/page-creation.spec.ts rename to tests/hash-playwright/tests/features/page-creation.spec.ts index e99cea81604..bf87a974090 100644 --- a/tests/hash-playwright/tests/page-creation.spec.ts +++ b/tests/hash-playwright/tests/features/page-creation.spec.ts @@ -1,9 +1,7 @@ // import { blockProtocolHubOrigin } from "@local/hash-isomorphic-utils/blocks"; import { sleep } from "@local/hash-isomorphic-utils/sleep"; -import { loginUsingTempForm } from "./shared/login-using-temp-form"; -import { resetDb } from "./shared/reset-db"; -import { expect, test } from "./shared/runtime"; +import { expect, test } from "../shared/runtime"; const pageNameSuffix = Date.now(); const pageNameFallback = "Untitled"; @@ -15,14 +13,9 @@ const placeholderSelector = "text=Type / to browse blocks, or @ to browse entities"; const modifierKey = process.platform === "darwin" ? "Meta" : "Control"; -test.beforeEach(async () => { - await resetDb(); -}); - // @todo fix this test test.skip("user can create page", async ({ page }) => { - await loginUsingTempForm({ page }); - + await page.goto("/"); await page.waitForURL("/"); await expect(page.locator("text=Get support")).toBeVisible(); @@ -163,11 +156,9 @@ test.skip("user can create page", async ({ page }) => { // @todo fix this test test.skip("user can rename page", async ({ page }) => { + await page.goto("/"); const pageName1 = `Page ${pageNameSuffix}`; const pageName2 = `Page 2 ${pageNameSuffix}`; - - await loginUsingTempForm({ page }); - // TODO: investigate why delay is required for create page button to work await sleep(500); await page.locator(createPageButtonSelector).click(); diff --git a/tests/hash-playwright/tests/page-navigation.spec.ts b/tests/hash-playwright/tests/features/page-navigation.spec.ts similarity index 78% rename from tests/hash-playwright/tests/page-navigation.spec.ts rename to tests/hash-playwright/tests/features/page-navigation.spec.ts index 605ee384fe1..ba30f3b2513 100644 --- a/tests/hash-playwright/tests/page-navigation.spec.ts +++ b/tests/hash-playwright/tests/features/page-navigation.spec.ts @@ -2,23 +2,11 @@ import { sleep } from "@local/hash-isomorphic-utils/sleep"; // eslint-disable-next-line no-restricted-imports import { test as testTolerateConsoleErrors } from "@playwright/test"; -import { loginUsingTempForm } from "./shared/login-using-temp-form"; -import { resetDb } from "./shared/reset-db"; -import { expect, test } from "./shared/runtime"; +import { expect, test } from "../shared/runtime"; const pageTitleInputSelector = '[placeholder="Untitled"]'; -test.beforeEach(async () => { - await resetDb(); -}); - testTolerateConsoleErrors("logged-in user shown 404 page", async ({ page }) => { - await loginUsingTempForm({ - page, - userEmail: "alice@example.com", - userPassword: "password", - }); - await page.goto("/non/existing/page"); await expect(page).toHaveTitle("This page could not be found | HASH"); @@ -26,8 +14,7 @@ testTolerateConsoleErrors("logged-in user shown 404 page", async ({ page }) => { }); test("user can toggle nested pages", async ({ page }) => { - await loginUsingTempForm({ page }); - + await page.goto("/"); const selectExpandPageButton = (pageTitle: string) => page.locator( `[data-testid="pages-tree"] a:has-text("${pageTitle}") > [data-testid="page-tree-item-expand-button"]`, diff --git a/tests/hash-playwright/tests/page-readonly-mode.spec.ts b/tests/hash-playwright/tests/features/page-readonly-mode.spec.ts similarity index 89% rename from tests/hash-playwright/tests/page-readonly-mode.spec.ts rename to tests/hash-playwright/tests/features/page-readonly-mode.spec.ts index 4543e2abca5..c1ca3eaa6cd 100644 --- a/tests/hash-playwright/tests/page-readonly-mode.spec.ts +++ b/tests/hash-playwright/tests/features/page-readonly-mode.spec.ts @@ -1,16 +1,11 @@ import { sleep } from "@local/hash-isomorphic-utils/sleep"; -import { loginUsingUi } from "./shared/login-using-ui"; -import { resetDb } from "./shared/reset-db"; -import { expect, test } from "./shared/runtime"; +import { loginUsingUi } from "../shared/login-using-ui"; +import { expect, test } from "../shared/runtime"; const placeholderSelector = "text=Type / to browse blocks, or @ to browse entities"; -test.beforeEach(async () => { - await resetDb(); -}); - /** * @todo: Re-enable this playwright test when required backend functionality is fixed * @see https://app.asana.com/0/1202805690238892/1203106234191599/f diff --git a/tests/hash-playwright/tests/profile-page.spec.ts b/tests/hash-playwright/tests/features/profile-page.spec.ts similarity index 80% rename from tests/hash-playwright/tests/profile-page.spec.ts rename to tests/hash-playwright/tests/features/profile-page.spec.ts index 1855f63548d..4f676a2de50 100644 --- a/tests/hash-playwright/tests/profile-page.spec.ts +++ b/tests/hash-playwright/tests/features/profile-page.spec.ts @@ -2,13 +2,9 @@ import { sleep } from "@local/hash-isomorphic-utils/sleep"; // eslint-disable-next-line no-restricted-imports import { test as testTolerateConsoleErrors } from "@playwright/test"; -import { loginUsingTempForm } from "./shared/login-using-temp-form"; -import { resetDb } from "./shared/reset-db"; -import { expect } from "./shared/runtime"; +import { expect } from "../shared/runtime"; -testTolerateConsoleErrors.beforeEach(async () => { - await resetDb(); -}); +testTolerateConsoleErrors.beforeEach(async () => {}); const blockCollectionMountTimeout = 5_000; @@ -19,12 +15,6 @@ const blockCollectionMountTimeout = 5_000; testTolerateConsoleErrors.skip( "a user's profile page renders", async ({ page }) => { - await loginUsingTempForm({ - page, - userEmail: "alice@example.com", - userPassword: "password", - }); - await page.goto("/@alice"); await expect(page.locator("text=@alice")).toBeVisible(); @@ -58,12 +48,6 @@ testTolerateConsoleErrors.skip( testTolerateConsoleErrors.skip( "an org's profile page renders, with and without a bio", async ({ page }) => { - await loginUsingTempForm({ - page, - userEmail: "alice@example.com", - userPassword: "password", - }); - await page.goto("/@example-org"); await expect(page.locator("text=@example-org")).toBeVisible(); diff --git a/tests/hash-playwright/tests/types-page.spec.ts b/tests/hash-playwright/tests/features/types-page.spec.ts similarity index 78% rename from tests/hash-playwright/tests/types-page.spec.ts rename to tests/hash-playwright/tests/features/types-page.spec.ts index 2d5ee7893df..d0de0b05fe3 100644 --- a/tests/hash-playwright/tests/types-page.spec.ts +++ b/tests/hash-playwright/tests/features/types-page.spec.ts @@ -1,17 +1,11 @@ import { frontendUrl } from "@local/hash-isomorphic-utils/environment"; -import { loginUsingTempForm } from "./shared/login-using-temp-form"; -import { expect, test } from "./shared/runtime"; +import { expect, test } from "../shared/runtime"; const pathPrefix = `${frontendUrl}/types/`; test("/types page renders and loads types", async ({ page }) => { - await loginUsingTempForm({ - page, - userEmail: "alice@example.com", - userPassword: "password", - }); - + await page.goto("/"); await expect(page.locator("text=Get support")).toBeVisible(); await page.goto("/types"); diff --git a/tests/hash-playwright/tests/guest-user.spec.ts b/tests/hash-playwright/tests/guest/guest-user.spec.ts similarity index 93% rename from tests/hash-playwright/tests/guest-user.spec.ts rename to tests/hash-playwright/tests/guest/guest-user.spec.ts index 6ff698d76d4..e40f2fe40b3 100644 --- a/tests/hash-playwright/tests/guest-user.spec.ts +++ b/tests/hash-playwright/tests/guest/guest-user.spec.ts @@ -1,12 +1,7 @@ // eslint-disable-next-line no-restricted-imports import { test as testTolerateConsoleErrors } from "@playwright/test"; -import { resetDb } from "./shared/reset-db"; -import { expect, test } from "./shared/runtime"; - -test.beforeEach(async () => { - await resetDb(); -}); +import { expect, test } from "../shared/runtime"; test("guest user navigation to login and signup pages", async ({ page }) => { await page.goto("/"); diff --git a/tests/hash-playwright/tests/shared/get-kratos-verification-code.ts b/tests/hash-playwright/tests/shared/get-kratos-verification-code.ts index 0b71b87c60f..d242149b7b6 100644 --- a/tests/hash-playwright/tests/shared/get-kratos-verification-code.ts +++ b/tests/hash-playwright/tests/shared/get-kratos-verification-code.ts @@ -49,10 +49,7 @@ const parseMailslurperDate = (dateSent?: string): number | undefined => { return Number.isNaN(parsed) ? undefined : parsed; }; -/** - * Matches the email subject for verification emails. - * Handles both the Kratos default subject and the custom HASH template subject. - */ +/** Accept either the Kratos default subject or HASH's custom template. */ const isVerificationSubject = (subject?: string): boolean => { if (!subject) { return false; @@ -63,6 +60,9 @@ const isVerificationSubject = (subject?: string): boolean => { ); }; +const isRecoverySubject = (subject?: string): boolean => + typeof subject === "string" && subject.startsWith("Your HASH recovery code:"); + export const getKratosVerificationCode = async ( emailAddress: string, afterTimestamp?: number, @@ -130,7 +130,6 @@ export const getKratosVerificationCode = async ( const lastErrorMessage = lastError instanceof Error ? ` Last error: ${lastError.message}` : ""; - // Build diagnostic summary from the last poll to help debug failures. const allItems = lastMailItems ?? []; const toTargetAddress = allItems.filter((item) => extractToAddresses(item.toAddresses).includes(emailAddress), @@ -167,3 +166,59 @@ export const getKratosVerificationCode = async ( `No verification email found for ${emailAddress} within ${maxWaitMs}ms.${lastErrorMessage} [${diagnostics}]`, ); }; + +export const getKratosRecoveryCode = async ( + emailAddress: string, + afterTimestamp?: number, +): Promise => { + const maxWaitMs = 10_000; + const pollIntervalMs = 250; + const timestampBufferMs = 5_000; + let elapsed = 0; + + while (elapsed < maxWaitMs) { + const response = await fetch("http://localhost:4437/mail"); + + if (!response.ok) { + throw new Error( + `Unable to fetch emails from mailslurper: ${response.status} ${response.statusText}`, + ); + } + + const data = (await response.json()) as { + mailItems?: MailslurperMailItem[]; + }; + + const match = data.mailItems + ?.filter((mailItem) => { + const sentTimestamp = parseMailslurperDate(mailItem.dateSent); + return ( + isRecoverySubject(mailItem.subject) && + extractToAddresses(mailItem.toAddresses).includes(emailAddress) && + (!afterTimestamp || + (typeof sentTimestamp === "number" && + sentTimestamp >= afterTimestamp - timestampBufferMs)) + ); + }) + .sort((a, b) => { + const aTs = parseMailslurperDate(a.dateSent) ?? 0; + const bTs = parseMailslurperDate(b.dateSent) ?? 0; + return bTs - aTs; + }) + .at(0); + + if (match?.body) { + const code = match.body.match(/\b(\d{6})\b/)?.[1]; + if (code) { + return code; + } + } + + await sleep(pollIntervalMs); + elapsed += pollIntervalMs; + } + + throw new Error( + `No recovery email found for ${emailAddress} within ${maxWaitMs}ms.`, + ); +}; diff --git a/tests/hash-playwright/tests/shared/reset-db.ts b/tests/hash-playwright/tests/shared/reset-db.ts deleted file mode 100644 index f8328d1b506..00000000000 --- a/tests/hash-playwright/tests/shared/reset-db.ts +++ /dev/null @@ -1,6 +0,0 @@ -/** - * @deprecated this function doesn't do anything at the moment - */ -export const resetDb = async () => { - // @todo reimplement this -}; diff --git a/tests/hash-playwright/tests/shared/runtime.ts b/tests/hash-playwright/tests/shared/runtime.ts index 9a3862c8e6c..f10b02a28bb 100644 --- a/tests/hash-playwright/tests/shared/runtime.ts +++ b/tests/hash-playwright/tests/shared/runtime.ts @@ -26,6 +26,9 @@ const tolerableResponseErrors: Array<{ status: number; urlPattern: RegExp }> = [ // that the current login flow needs to upgrade (e.g. AAL2 required for // a TOTP-enabled user submitting password-only login). { status: 422, urlPattern: /\/auth\/self-service\/login(\?|$)/ }, + // Recovery code submission: Kratos returns 422 to redirect to the + // settings page after a successful recovery code validation. + { status: 422, urlPattern: /\/auth\/self-service\/recovery(\?|$)/ }, // Kratos rejects expected self-service conditions: invalid TOTP/backup // codes, `session_already_available` when hitting the login browser // endpoint with an active session, expired flows, ... diff --git a/tests/hash-playwright/tests/shared/signin-utils.ts b/tests/hash-playwright/tests/shared/signin-utils.ts new file mode 100644 index 00000000000..e7598717897 --- /dev/null +++ b/tests/hash-playwright/tests/shared/signin-utils.ts @@ -0,0 +1,29 @@ +import type { Page } from "@playwright/test"; +import { expect } from "@playwright/test"; + +export const signInWithPassword = async ( + page: Page, + { email, password }: { email: string; password: string }, +) => { + await page.goto("/signin"); + await page.fill('[placeholder="Enter your email address"]', email); + await page.fill('[placeholder="Enter your password"]', password); + await page.click("text=Submit"); +}; + +/** + * Drop the user's session cookies. Does not exercise the Kratos logout + * flow — for that, see the dedicated sign-out test. + */ +export const signOut = async (page: Page) => { + await page.context().clearCookies(); +}; + +export const expectSignedIn = async (page: Page) => { + await expect(page.locator("text=Get support")).toBeVisible(); +}; + +export const expectSignedOut = async (page: Page) => { + await page.goto("/"); + await expect(page.getByRole("link", { name: "Sign In" })).toBeVisible(); +}; diff --git a/tests/hash-playwright/tests/shared/signup-utils.ts b/tests/hash-playwright/tests/shared/signup-utils.ts index a19c0834a2a..be0f4e95660 100644 --- a/tests/hash-playwright/tests/shared/signup-utils.ts +++ b/tests/hash-playwright/tests/shared/signup-utils.ts @@ -7,10 +7,10 @@ import { getKratosVerificationCode } from "./get-kratos-verification-code"; const defaultPassword = "some-complex-pw-1ab2"; /** - * Generate a unique shortname suffix per test run so that the web principal - * left behind by a previous run (see `deleteUserByEmail`) doesn't cause a - * "Shortname already taken" error. The suffix keeps the base name readable - * while guaranteeing uniqueness. + * `deleteUserByEmail` intentionally preserves the user's web principal so + * entity types created under it remain valid for other webs that reference + * them. The orphan principal holds onto the old shortname, so re-runs must + * pick a fresh one to avoid a "shortname already taken" error. */ const uniqueShortname = (base: string): string => { const suffix = `${Date.now()}${Math.floor(Math.random() * 1_000)}`; @@ -26,15 +26,9 @@ export const registerUser = async ( page: Page, { email, password = defaultPassword }: { email: string; password?: string }, ) => { - const registrationFlowReady = page.waitForResponse( - (response) => - response.request().method() === "GET" && - response.url().includes("/auth/self-service/registration/browser"), - { timeout: 15_000 }, - ); + await page.goto("/signup", { waitUntil: "networkidle" }); - await page.goto("/signup"); - await registrationFlowReady; + await expect(page).toHaveURL(/\/signup/, { timeout: 5_000 }); await page.fill('[placeholder="Enter your email address"]', email); await page.fill('[type="password"]', password); @@ -97,12 +91,10 @@ export const completeSignup = async ( }; /** - * Full flow: register a user, verify email, and complete signup. + * Full signup flow: register, verify email, and complete the account page. * - * Before registering, deletes any Kratos identity left over from a previous - * run via the Graph admin API. The shortname is randomised per call to avoid - * colliding with the orphan web principal that `POST /users/delete` - * intentionally preserves. + * Deletes any Kratos identity left over from a previous run before + * registering, and randomises the shortname via {@link uniqueShortname}. */ export const createUserAndCompleteSignup = async ( page: Page, @@ -118,8 +110,6 @@ export const createUserAndCompleteSignup = async ( password?: string; }, ) => { - // Clean up any leftover Kratos identity from a previous run so the - // registration doesn't fail with "identifier already exists". await deleteUserByEmail(email); const runShortname = uniqueShortname(shortname); diff --git a/tests/hash-playwright/tests/shared/test-users.ts b/tests/hash-playwright/tests/shared/test-users.ts new file mode 100644 index 00000000000..0f07c6cdf5f --- /dev/null +++ b/tests/hash-playwright/tests/shared/test-users.ts @@ -0,0 +1,70 @@ +import type { Page } from "@playwright/test"; + +import { deleteUserByEmail } from "./delete-user"; +import { createUserAndCompleteSignup } from "./signup-utils"; + +const defaultPassword = "test-pw-1ab2"; + +interface TestUser { + readonly email: string; + readonly shortname: string; + readonly displayName: string; + readonly password: string; +} + +const user = ( + email: string, + shortname: string, + displayName: string, +): TestUser => ({ + email, + shortname, + displayName, + password: defaultPassword, +}); + +/** + * Pre-defined test users. Every email listed here must also appear in + * `USER_EMAIL_ALLOW_LIST` in `.env.test`, otherwise signup will be + * blocked by the allowlist gate. + * + * Each test that modifies user state (password, TOTP, etc.) should use + * its own dedicated user so tests can run in parallel without conflicts. + */ +export const testUsers = { + // Signin / Signout + signinTest: user("signin-test@example.com", "signin-test", "Signin Test"), + signoutTest: user("signout-test@example.com", "signout-test", "Signout Test"), + + // Password + pwChange: user("pw-change@example.com", "pw-change", "PW Change"), + pwRecovery: user("pw-recovery@example.com", "pw-recovery", "PW Recovery"), + + // MFA + mfaEnable: user("mfa-enable@example.com", "mfa-enable", "MFA Enable"), + mfaLogin: user("mfa-login@example.com", "mfa-login", "MFA Login"), + mfaBackup: user("mfa-backup@example.com", "mfa-backup", "MFA Backup"), + mfaDisable: user("mfa-disable@example.com", "mfa-disable", "MFA Disable"), + mfaWrongCode: user( + "mfa-wrong-code@example.com", + "mfa-wrong-code", + "MFA Wrong Code", + ), + + // Signup + signupAllowlisted: user( + "signup-allow@example.com", + "signup-allow", + "Signup Allow", + ), +} as const; + +/** + * Clean up any leftover Kratos identity, then create and complete signup + * for the given test user. Returns the user's credentials. + */ +export const withTestUser = async (page: Page, testUser: TestUser) => { + await deleteUserByEmail(testUser.email); + await createUserAndCompleteSignup(page, testUser); + return { email: testUser.email, password: testUser.password }; +}; From 7f900e3c6f0abb6fcc2fd3972a5941b06c01073f Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Wed, 15 Apr 2026 20:43:16 +0200 Subject: [PATCH 02/14] Stabilise browser-extension Playwright tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The popup runs several uncoordinated async loads on mount (`getUser` fires multiple times plus `useEntityTypes`), each independently writing a slice of state into `chrome.storage.local`. The previous suite interacted with the popup before those writes settled, so the backend-persisted `BrowserPluginSettings` entity leaked state between runs and races produced spurious duplicate chips. Rather than fight each race individually, this change: - Adds `waitForPopupStateLoaded` — resolves once `chrome.storage.local` has been idle for 500 ms, which empirically covers the mount-time fetches under CI latency. - Replaces UI-driven state clears (chip-by-chip delete button clicks) with direct `chrome.storage.local.set` writes in `resetOneOffState` / `resetAutomatedState`. `useStorageSync` picks up the change and re-renders the UI empty, and we sidestep the per-click debounced backend save that would otherwise collide with late `getUser` writes. - Replaces magic-number `sleep()` calls with deterministic waits: `waitForSettingsSave` listens for the next debounced `updateEntity` mutation; `selectEntityTypeOption` waits for the resulting chip. - Adds an `ADD ANOTHER` fallback for the Automated tab because `SelectScope`'s `showTable` / `draftRule` are `useState`-initialised from `anyTypesSelected` and therefore sticky across storage clears. - Uses a native `input` event dispatch for the quick-note textarea; `page.fill` silently skipped MUI's `onChange` when the stored value matched the new one. The plugin fixture now still clears local storage on start so the popup hydrates via `getUser()` rather than stale cache. Backend state cleanup moves into the per-test reset helpers. Plugin-browser: `infer-entities` no longer opens a doomed WebSocket when the user isn't logged in. A background-script `init` with no session cookies defers until a user action triggers `getWebSocket()`, and `reconnectWebSocket` short-circuits the same way, which prevents the server from closing each connection after its 5-second unauthenticated timeout. Supporting: added `@types/chrome`, included `global-setup.ts` in `tsconfig.json`, moved the extension fixture to `shared/browser-plugin-fixtures/` to match the new `extension/` project layout. --- .../src/scripts/background/infer-entities.ts | 39 ++- tests/hash-playwright/package.json | 1 + .../tests/browser-plugin.spec.ts | 174 ---------- .../tests/extension/browser-plugin.spec.ts | 305 ++++++++++++++++++ .../browser-plugin-fixtures}/fixtures.ts | 9 +- tests/hash-playwright/tsconfig.json | 7 +- yarn.lock | 34 ++ 7 files changed, 384 insertions(+), 185 deletions(-) delete mode 100644 tests/hash-playwright/tests/browser-plugin.spec.ts create mode 100644 tests/hash-playwright/tests/extension/browser-plugin.spec.ts rename tests/hash-playwright/tests/{browser-plugin => shared/browser-plugin-fixtures}/fixtures.ts (78%) diff --git a/apps/plugin-browser/src/scripts/background/infer-entities.ts b/apps/plugin-browser/src/scripts/background/infer-entities.ts index f7c0096e834..a1b1244837f 100644 --- a/apps/plugin-browser/src/scripts/background/infer-entities.ts +++ b/apps/plugin-browser/src/scripts/background/infer-entities.ts @@ -31,10 +31,10 @@ const setExternalInputRequestsValue = getSetFromLocalStorageValue( "externalInputRequests", ); -const getCookieString = async () => { +const getSessionCookies = async () => { const apiOrigin = await getFromLocalStorage("apiOrigin"); - const cookies = await browser.cookies + return browser.cookies .getAll({ url: apiOrigin ?? API_ORIGIN, }) @@ -45,6 +45,15 @@ const getCookieString = async () => { option.name === "ory_kratos_session", ), ); +}; + +const hasSessionCookies = async (): Promise => { + const cookies = await getSessionCookies(); + return cookies.length >= 2; +}; + +const getCookieString = async () => { + const cookies = await getSessionCookies(); if (cookies.length < 2) { throw new Error("No session cookies available to use in websocket request"); @@ -209,6 +218,13 @@ const reconnectWebSocket = async () => { reconnecting = true; try { + if (!(await hasSessionCookies())) { + // User isn't logged in — don't open a doomed connection that the + // server will kill after 5 s. Retry later; a user action like + // opening the popup will trigger getWebSocket() once cookies exist. + return; + } + console.log("Reconnecting WebSocket..."); ws = await createWebSocket({ onClose: reconnectWebSocket }); console.log("WebSocket reconnected successfully."); @@ -218,9 +234,7 @@ const reconnectWebSocket = async () => { void reconnectWebSocket(); }, 3_000); } finally { - if (ws) { - reconnecting = false; - } + reconnecting = false; } }; @@ -383,10 +397,17 @@ export const inferEntities = async ( }; /** - * Keep a persist websocket connection because we use it to get sent input requests from the API + * Keep a persistent websocket connection because we use it to get sent + * input requests from the API. If no session cookies are available yet + * (user not logged in), defer until they appear so we don't open + * connections that the server will immediately kill. */ -const init = () => { - void getWebSocket(); +const init = async () => { + if (await hasSessionCookies()) { + void getWebSocket(); + } + // If no cookies, the WebSocket will be created on demand when + // getWebSocket() is called by a user action (e.g. inferEntities). }; -init(); +void init(); diff --git a/tests/hash-playwright/package.json b/tests/hash-playwright/package.json index 7673c52e31e..9c0bd06f864 100644 --- a/tests/hash-playwright/package.json +++ b/tests/hash-playwright/package.json @@ -28,6 +28,7 @@ "@blockprotocol/graph": "workspace:*", "@graphql-codegen/cli": "^6.2.1", "@local/eslint": "workspace:*", + "@types/chrome": "0.1.40", "@types/js-yaml": "^4", "eslint": "9.39.4", "rimraf": "6.1.3", diff --git a/tests/hash-playwright/tests/browser-plugin.spec.ts b/tests/hash-playwright/tests/browser-plugin.spec.ts deleted file mode 100644 index 4dd2d98d18c..00000000000 --- a/tests/hash-playwright/tests/browser-plugin.spec.ts +++ /dev/null @@ -1,174 +0,0 @@ -import { sleep } from "@local/hash-isomorphic-utils/sleep"; -// eslint-disable-next-line no-restricted-imports -import type { Page } from "@playwright/test"; - -import { expect, test } from "./browser-plugin/fixtures"; -import { loginUsingTempForm } from "./shared/login-using-temp-form"; -import { resetDb } from "./shared/reset-db"; - -test.beforeEach(async () => { - await resetDb(); -}); - -const loggedOutHeaderLocator = "text=Connect to HASH"; -const createAccountButtonLocator = "text=Create a free account"; -const entityTypeSelectorLocator = '[placeholder="Search for types..."]'; -const quickNoteInputLocator = '[placeholder="Start typing here..."]'; - -export const signOutAndReloadPopup = async ({ - extensionId, - page, -}: { - extensionId: string; - page: Page; -}) => { - // Sign out from HASH - await page.goto("/"); - await page.click(`[data-testid="user-avatar"]`); - await page.click("text=Sign Out"); - await sleep(1_000); - - // Confirm the popup has been signed out - await page.goto(`chrome-extension://${extensionId}/popup.html`); - await expect(page.locator(loggedOutHeaderLocator)).toBeVisible(); - - // Sign back in again and confirm the tabs are visible again - await loginUsingTempForm({ page }); - await page.goto(`chrome-extension://${extensionId}/popup.html`); - await expect(page.locator("text=One-off")).toBeVisible(); -}; - -test("popup window loads with logged-out state", async ({ - page, - extensionId, -}) => { - await page.goto(`chrome-extension://${extensionId}/popup.html`); - - await expect(page.locator(loggedOutHeaderLocator)).toBeVisible(); - - await expect(page.locator(createAccountButtonLocator)).toBeVisible(); -}); - -test("popup window loads with logged-in state", async ({ - page, - extensionId, -}) => { - await loginUsingTempForm({ page }); - - await page.goto(`chrome-extension://${extensionId}/popup.html`); - - await expect(page.locator("text=One-off")).toBeVisible(); -}); - -test("options page loads with logged-out state", async ({ - page, - extensionId, -}) => { - await page.goto(`chrome-extension://${extensionId}/options.html`); - - await expect(page.locator(loggedOutHeaderLocator)).toBeVisible(); - - await expect(page.locator(createAccountButtonLocator)).toBeVisible(); -}); - -test("options page loads with logged-in state", async ({ - page, - extensionId, -}) => { - await loginUsingTempForm({ page }); - - await page.goto(`chrome-extension://${extensionId}/options.html`); - - await expect(page.locator("text=Welcome, Alice")).toBeVisible(); -}); - -test("user can type a quick note which persists across logouts", async ({ - page, - extensionId, -}) => { - // Sign in to HASH - await loginUsingTempForm({ page }); - - // Open the popup and start writing a quick note - await page.goto(`chrome-extension://${extensionId}/popup.html`); - await page.click("text=One-off"); - const testQuickNote = "Hello, world! Here's the start of a note..."; - await page.fill(quickNoteInputLocator, testQuickNote); - await sleep(1_500); // Wait for settings to save - - await signOutAndReloadPopup({ extensionId, page }); - - await expect(page.locator(quickNoteInputLocator)).toHaveValue(testQuickNote); -}); - -/** - * @todo figure out how to check that the correct message is sent from the background script to the API via websocket - * - when user clicks the 'suggest entities' button - * @see https://github.com/microsoft/playwright/issues/15684#issuecomment-1892644655 - */ -test("user can configure a one-off inference, and the settings are persisted", async ({ - page, - extensionId, -}) => { - // Sign in to HASH - await loginUsingTempForm({ page }); - - /** - * Go to the popup page and get a reference to the plugin's service worker - * @see https://playwright.dev/docs/service-workers-experimental - */ - await page.goto(`chrome-extension://${extensionId}/popup.html`); - await page.click("text=One-off"); - - // Choose two entity types from the entity type selector - await page.click(entityTypeSelectorLocator); - await page.keyboard.type("actor"); - await sleep(500); - await page.keyboard.press("ArrowUp"); - await page.keyboard.press("Enter"); - await sleep(500); - await page.keyboard.type("document"); - await page.keyboard.press("ArrowUp"); - await page.keyboard.press("Enter"); - - await sleep(1_500); // Wait for settings to save - - await signOutAndReloadPopup({ extensionId, page }); - - await expect(page.locator("text=Actor")).toBeVisible(); - await expect(page.locator("text=Document")).toBeVisible(); -}); - -/** - * @todo figure out how to check that the correct message is sent from the background script to the API via websocket - * - when user visits a page that should trigger automatic inference - * @see https://github.com/microsoft/playwright/issues/15684#issuecomment-1892644655 - */ -test("user can enable automatic inference, and the settings are persisted", async ({ - page, - extensionId, -}) => { - await loginUsingTempForm({ page }); - - await page.goto(`chrome-extension://${extensionId}/popup.html`); - - // Choose an entity type from the entity type selector - await page.click("text=Automated"); - await page.click("text=Select type"); - await page.click(entityTypeSelectorLocator); - await page.keyboard.type("actor"); - await sleep(500); - await page.keyboard.press("ArrowUp"); - await page.keyboard.press("Enter"); - - await page.click("text=Disabled"); - - await expect(page.locator("text=Enabled")).toBeVisible(); - - await sleep(1_500); // Wait for settings to save - - await signOutAndReloadPopup({ extensionId, page }); - - await expect(page.locator("text=Enabled")).toBeVisible(); - await expect(page.locator("[value=Actor]")).toBeVisible(); -}); diff --git a/tests/hash-playwright/tests/extension/browser-plugin.spec.ts b/tests/hash-playwright/tests/extension/browser-plugin.spec.ts new file mode 100644 index 00000000000..42e8e658bc5 --- /dev/null +++ b/tests/hash-playwright/tests/extension/browser-plugin.spec.ts @@ -0,0 +1,305 @@ +// eslint-disable-next-line no-restricted-imports +import type { Page } from "@playwright/test"; + +import { expect, test } from "../shared/browser-plugin-fixtures/fixtures"; +import { loginUsingTempForm } from "../shared/login-using-temp-form"; + +const loggedOutHeaderLocator = "text=Connect to HASH"; +const createAccountButtonLocator = "text=Create a free account"; +const entityTypeSelectorLocator = '[placeholder="Search for types..."]'; +const quickNoteInputLocator = '[placeholder="Start typing here..."]'; + +/** + * Wait for the next debounced `updateEntity` mutation issued by the + * popup's storage sync (`apps/plugin-browser/src/shared/storage.ts`). + */ +const waitForSettingsSave = (page: Page) => + page.waitForResponse( + (response) => + response.url().endsWith("/graphql") && + response.status() === 200 && + (response.request().postData() ?? "").includes("mutation updateEntity"), + { timeout: 5_000 }, + ); + +/** + * Resolve once `chrome.storage.local` has been idle for 500 ms. The + * popup fires several independent async loads on mount (`getUser`, + * `useEntityTypes`) and each writes its own slice of state; interacting + * before they settle risks later backfills racing with user actions. + */ +const waitForPopupStateLoaded = (page: Page) => + page.evaluate( + () => + new Promise((resolve) => { + let timer: ReturnType | null = null; + const onChange = () => { + if (timer) { + clearTimeout(timer); + } + timer = setTimeout(() => { + chrome.storage.onChanged.removeListener(onChange); + resolve(); + }, 500); + }; + chrome.storage.onChanged.addListener(onChange); + onChange(); + }), + ); + +/** + * Type a name into an open entity-type autocomplete and select the + * matching option. ArrowDown is required because the component doesn't + * use MUI's `autoHighlight`. + */ +const selectEntityTypeOption = async ( + page: Page, + name: string, + { multiple = true }: { multiple?: boolean } = {}, +) => { + await page.keyboard.type(name); + await expect( + page.getByRole("option").filter({ hasText: name }).first(), + ).toBeVisible(); + await page.keyboard.press("ArrowDown"); + await page.keyboard.press("Enter"); + if (multiple) { + await expect( + page + .locator(".MuiChip-label") + .filter({ hasText: new RegExp(`^${name}$`) }), + ).toHaveCount(1); + } +}; + +/** + * Reset the one-off inference targets by writing an empty array to + * `chrome.storage.local`. Direct writes avoid the per-chip-click + * debounced backend saves and the race they cause with late `getUser` + * writes; `useStorageSync` then re-renders the UI empty. + */ +const resetOneOffState = async (page: Page) => { + await page.evaluate(async () => { + const { manualInferenceConfig } = await chrome.storage.local.get( + "manualInferenceConfig", + ); + await chrome.storage.local.set({ + manualInferenceConfig: { + ...(manualInferenceConfig ?? {}), + targetEntityTypeIds: [], + }, + }); + }); + await expect(page.locator(".MuiChip-deleteIcon")).toHaveCount(0); + await waitForPopupStateLoaded(page); + await expect(page.locator(".MuiChip-deleteIcon")).toHaveCount(0); +}; + +/** Reset automatic inference state; see {@link resetOneOffState}. */ +const resetAutomatedState = async (page: Page) => { + await page.evaluate(async () => { + const { automaticInferenceConfig } = await chrome.storage.local.get( + "automaticInferenceConfig", + ); + await chrome.storage.local.set({ + automaticInferenceConfig: { + ...(automaticInferenceConfig ?? {}), + rules: [], + enabled: false, + }, + }); + }); + await expect(page.locator("tbody .MuiIconButton-root")).toHaveCount(0); + await expect(page.locator("text=Disabled")).toBeVisible(); + await waitForPopupStateLoaded(page); + await expect(page.locator("text=Disabled")).toBeVisible(); +}; + +const signOutAndReloadPopup = async ({ + extensionId, + page, +}: { + extensionId: string; + page: Page; +}) => { + const avatar = page.locator(`[data-testid="user-avatar"]`); + await page.goto("/"); + await avatar.click(); + await page.click("text=Sign Out"); + await expect(avatar).toBeHidden(); + + await page.goto(`chrome-extension://${extensionId}/popup.html`); + await expect(page.locator(loggedOutHeaderLocator)).toBeVisible(); + + await loginUsingTempForm({ page }); + await page.goto(`chrome-extension://${extensionId}/popup.html`); + await expect(page.locator("text=One-off")).toBeVisible(); +}; + +test("popup window loads with logged-out state", async ({ + page, + extensionId, +}) => { + await page.goto(`chrome-extension://${extensionId}/popup.html`); + + await expect(page.locator(loggedOutHeaderLocator)).toBeVisible(); + + await expect(page.locator(createAccountButtonLocator)).toBeVisible(); +}); + +test("popup window loads with logged-in state", async ({ + page, + extensionId, +}) => { + await loginUsingTempForm({ page }); + + await page.goto(`chrome-extension://${extensionId}/popup.html`); + + await expect(page.locator("text=One-off")).toBeVisible(); +}); + +test("options page loads with logged-out state", async ({ + page, + extensionId, +}) => { + await page.goto(`chrome-extension://${extensionId}/options.html`); + + await expect(page.locator(loggedOutHeaderLocator)).toBeVisible(); + + await expect(page.locator(createAccountButtonLocator)).toBeVisible(); +}); + +test("options page loads with logged-in state", async ({ + page, + extensionId, +}) => { + await loginUsingTempForm({ page }); + + await page.goto(`chrome-extension://${extensionId}/options.html`); + + await expect(page.locator("text=Welcome, Alice")).toBeVisible(); +}); + +test("user can type a quick note which persists across logouts", async ({ + page, + extensionId, +}) => { + await loginUsingTempForm({ page }); + + await page.goto(`chrome-extension://${extensionId}/popup.html`); + await waitForPopupStateLoaded(page); + await page.click("text=One-off"); + await waitForPopupStateLoaded(page); + + // Unique per-run value so the write is always observably different + // from persisted backend state. Dispatching a native `input` event + // triggers MUI's controlled `TextField` onChange more reliably than + // `page.fill`, which sometimes skips the handler when the prior + // value matches. + const testQuickNote = `Hello, world! Here's a note ${Date.now()}`; + const settingsSaved = waitForSettingsSave(page); + await page.locator(quickNoteInputLocator).evaluate((element, value) => { + const descriptor = Object.getOwnPropertyDescriptor( + HTMLTextAreaElement.prototype, + "value", + ); + descriptor?.set?.call(element, value); + element.dispatchEvent(new Event("input", { bubbles: true })); + }, testQuickNote); + await settingsSaved; + + await signOutAndReloadPopup({ extensionId, page }); + + // Backend-restored `popupTab` may land on "automated" after login. + await waitForPopupStateLoaded(page); + await page.click("text=One-off"); + + await expect(page.locator(quickNoteInputLocator)).toHaveValue(testQuickNote); +}); + +/** + * Selecting a type also adds its linked types (see H-1721), so Actor + + * Document yields four chips. Exact-regex matches avoid colliding with + * e.g. `Document File`. + * + * @todo verify the correct WebSocket message is sent to the API when + * `Suggest entities` is clicked — see + * https://github.com/microsoft/playwright/issues/15684#issuecomment-1892644655 + */ +test("user can configure a one-off inference, and the settings are persisted", async ({ + page, + extensionId, +}) => { + await loginUsingTempForm({ page }); + + await page.goto(`chrome-extension://${extensionId}/popup.html`); + await waitForPopupStateLoaded(page); + await page.click("text=One-off"); + await waitForPopupStateLoaded(page); + + await resetOneOffState(page); + + await page.click(entityTypeSelectorLocator); + await selectEntityTypeOption(page, "Actor"); + await selectEntityTypeOption(page, "Document"); + + // The Document selection resets the 1 s debounce, so the final + // backend save contains both types. Register the listener after the + // last selection to avoid matching an intermediate Actor-only save. + await waitForSettingsSave(page); + + await signOutAndReloadPopup({ extensionId, page }); + + await waitForPopupStateLoaded(page); + await page.click("text=One-off"); + + const chipLabel = (name: string) => + page.locator(".MuiChip-label").filter({ hasText: new RegExp(`^${name}$`) }); + await expect(chipLabel("Actor")).toHaveCount(1); + await expect(chipLabel("Document")).toHaveCount(1); +}); + +/** + * @todo verify the correct WebSocket message is sent to the API when a + * page triggers automatic inference — see + * https://github.com/microsoft/playwright/issues/15684#issuecomment-1892644655 + */ +test("user can enable automatic inference, and the settings are persisted", async ({ + page, + extensionId, +}) => { + await loginUsingTempForm({ page }); + + await page.goto(`chrome-extension://${extensionId}/popup.html`); + await waitForPopupStateLoaded(page); + await page.click("text=Automated"); + await waitForPopupStateLoaded(page); + + await resetAutomatedState(page); + + // `SelectScope` initializes `showTable` and `draftRule` from + // `anyTypesSelected` in `useState`, so which button is shown depends + // on whether the run started with any rules. + const selectType = page.locator("text=Select type"); + const addAnother = page.locator("text=ADD ANOTHER"); + if ((await selectType.count()) > 0) { + await selectType.click(); + } else { + await addAnother.click(); + } + await page.click(entityTypeSelectorLocator); + await selectEntityTypeOption(page, "Actor", { multiple: false }); + + const settingsSaved = waitForSettingsSave(page); + await page.click("text=Disabled"); + await expect(page.locator("text=Enabled")).toBeVisible(); + await settingsSaved; + + await signOutAndReloadPopup({ extensionId, page }); + + await waitForPopupStateLoaded(page); + await page.click("text=Automated"); + await waitForPopupStateLoaded(page); + await expect(page.locator("text=Enabled")).toBeVisible(); + await expect(page.locator("[value=Actor]")).toBeVisible(); +}); diff --git a/tests/hash-playwright/tests/browser-plugin/fixtures.ts b/tests/hash-playwright/tests/shared/browser-plugin-fixtures/fixtures.ts similarity index 78% rename from tests/hash-playwright/tests/browser-plugin/fixtures.ts rename to tests/hash-playwright/tests/shared/browser-plugin-fixtures/fixtures.ts index eb483bc01e9..7399e0e5134 100644 --- a/tests/hash-playwright/tests/browser-plugin/fixtures.ts +++ b/tests/hash-playwright/tests/shared/browser-plugin-fixtures/fixtures.ts @@ -6,7 +6,7 @@ import { type BrowserContext, chromium, test as base } from "@playwright/test"; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); -const monorepoRootDir = path.resolve(__dirname, "../../../../"); +const monorepoRootDir = path.resolve(__dirname, "../../../../../"); export const test = base.extend<{ context: BrowserContext; @@ -39,6 +39,13 @@ export const test = base.extend<{ if (!extensionId) { throw new Error("Could not find extension ID"); } + + // Drop any cached state from a previous run so the popup hydrates + // via `getUser()` rather than from stale local storage. Backend + // settings are still restored on login and are cleared per-test in + // the spec's reset helpers. + await background.evaluate(() => chrome.storage.local.clear()); + // eslint-disable-next-line react-hooks/rules-of-hooks await use(extensionId); }, diff --git a/tests/hash-playwright/tsconfig.json b/tests/hash-playwright/tsconfig.json index 5a93f1d88c1..4bcf83a3e43 100644 --- a/tests/hash-playwright/tsconfig.json +++ b/tests/hash-playwright/tsconfig.json @@ -1,6 +1,11 @@ { "extends": "@local/tsconfig/legacy-base-tsconfig-to-refactor.json", - "include": ["tests", "codegen.config.ts", "playwright.config.ts"], + "include": [ + "tests", + "codegen.config.ts", + "global-setup.ts", + "playwright.config.ts" + ], "compilerOptions": { "lib": ["dom", "dom.iterable", "ESNext"] } diff --git a/yarn.lock b/yarn.lock index 9df45286117..f64bac2c56e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -18141,6 +18141,7 @@ __metadata: "@local/hash-isomorphic-utils": "workspace:*" "@local/tsconfig": "workspace:*" "@playwright/test": "npm:1.58.2" + "@types/chrome": "npm:0.1.40" "@types/js-yaml": "npm:^4" eslint: "npm:9.39.4" execa: "npm:9.6.0" @@ -18554,6 +18555,16 @@ __metadata: languageName: node linkType: hard +"@types/chrome@npm:0.1.40": + version: 0.1.40 + resolution: "@types/chrome@npm:0.1.40" + dependencies: + "@types/filesystem": "npm:*" + "@types/har-format": "npm:*" + checksum: 10c0/e32b69e967b6e02b3d71935ce12b679b0d443e5418241b27087bb13fc6d35005f3621261ff4abc83a1726463d56a7c07165811a5a6e526204717e22ade5a446c + languageName: node + linkType: hard + "@types/connect-history-api-fallback@npm:^1.5.4": version: 1.5.4 resolution: "@types/connect-history-api-fallback@npm:1.5.4" @@ -18974,6 +18985,22 @@ __metadata: languageName: node linkType: hard +"@types/filesystem@npm:*": + version: 0.0.36 + resolution: "@types/filesystem@npm:0.0.36" + dependencies: + "@types/filewriter": "npm:*" + checksum: 10c0/3ebec32f0494b0a2612187d148e9f253ff55672c53f566d9a1e6d891eb6e2372df93c252b594b2775bc53e6660c4c37fdb05dc1b26e72b60a31010da8e1f7317 + languageName: node + linkType: hard + +"@types/filewriter@npm:*": + version: 0.0.33 + resolution: "@types/filewriter@npm:0.0.33" + checksum: 10c0/363ef9a658a961ceae04f52934562e4ebdcdc3a2564dd8544f593d77113c16574938b6ba4fea0bee418c37bda0668c1e03dfedb6adf00d55853f51fb3a59247b + languageName: node + linkType: hard + "@types/fs-extra@npm:11.0.4": version: 11.0.4 resolution: "@types/fs-extra@npm:11.0.4" @@ -19022,6 +19049,13 @@ __metadata: languageName: node linkType: hard +"@types/har-format@npm:*": + version: 1.2.16 + resolution: "@types/har-format@npm:1.2.16" + checksum: 10c0/77e952bc219db0c1f0588cab3b95865bc343b922e8423a76fbbd6a757c40709a256933fa415eb8fefda6ea5897c8e3dd3191bb8a82b37c13d9232467d31ae485 + languageName: node + linkType: hard + "@types/hast@npm:^2.0.0": version: 2.3.10 resolution: "@types/hast@npm:2.3.10" From 47d53ffa15f107dda9e7f04fb956d64cf4eaca9f Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Wed, 15 Apr 2026 20:45:50 +0200 Subject: [PATCH 03/14] Disable Apollo query deduplication on the server The singleton `apolloClient` exported from `create-apollo-client.ts` is shared across SSR requests; Apollo's query deduplication uses `(query, variables)` as the dedup key and ignores `context`, so concurrent SSR requests for the same query share a response. Disable dedup on the server-side client; browser clients keep it. Also expands the `@todo` on the SSR `meQuery` prefetch in `_app.page.tsx` so the interaction is visible to whoever addresses the caching todo. --- apps/hash-frontend/src/pages/_app.page.tsx | 7 ++++++- .../hash-isomorphic-utils/src/create-apollo-client.ts | 7 +++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/apps/hash-frontend/src/pages/_app.page.tsx b/apps/hash-frontend/src/pages/_app.page.tsx index 3595380c1df..35e6fede3d8 100644 --- a/apps/hash-frontend/src/pages/_app.page.tsx +++ b/apps/hash-frontend/src/pages/_app.page.tsx @@ -282,7 +282,12 @@ AppWithTypeSystemContextProvider.getInitialProps = async (appContext) => { * Fetch the authenticated user on the very first page load so it's available in the frontend. * We leave it up to the client to re-fetch the user as necessary in response to user-initiated actions. * - * @todo this is running on every page transition. make it stop or make caching work (need to create new client on request to avoid sharing user data) + * @todo this is running on every page transition — the response should + * be cacheable so it doesn't hit the backend on every navigation. + * Note: the server-side `apolloClient` singleton has + * `queryDeduplication: false` (see `create-apollo-client.ts`) because + * Apollo's dedup key ignores `context` and would otherwise leak one + * user's data into another concurrent SSR request. */ const initialAuthenticatedUserSubgraph = await apolloClient .query({ diff --git a/libs/@local/hash-isomorphic-utils/src/create-apollo-client.ts b/libs/@local/hash-isomorphic-utils/src/create-apollo-client.ts index aa229be2ca1..fea8be9c19e 100644 --- a/libs/@local/hash-isomorphic-utils/src/create-apollo-client.ts +++ b/libs/@local/hash-isomorphic-utils/src/create-apollo-client.ts @@ -111,6 +111,13 @@ export const createApolloClient = (params?: { }; return new ApolloClient({ + // Apollo's query deduplication collapses in-flight queries with the + // same (query, variables) into a single network request, ignoring + // `context` (which carries per-request auth cookies). On the server + // this singleton is shared across concurrent SSR requests, so two + // users' identical queries get deduplicated and one receives the + // other's authenticated data. Safe in the browser (single user). + queryDeduplication: params?.isBrowser ?? false, cache: new InMemoryCache({ possibleTypes: possibleTypes.possibleTypes, typePolicies: { From 1bc1fdba6ce2fa7e28043b6259a5da60e4cfb85a Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Wed, 15 Apr 2026 23:52:09 +0200 Subject: [PATCH 04/14] Consolidate TOTP disable into a reusable handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extracts the two-step disable flow (unlink TOTP, then clear backup codes) into `executeDisableTotp`. Kratos keeps enforcing AAL2 as long as any second factor remains, so the follow-up `lookup_secret_disable` is load-bearing — without it the user would still be prompted for an authenticator code they no longer have at next sign-in. Also drops `currentPassword`, `currentPasswordError`, and `isRecoveryFlow` state: password changes no longer accept a current password in the UI (Kratos enforces privileged-session age via `privileged_session_max_age`), and the recovery-flow marker is unused. --- .../src/pages/settings/security.page.tsx | 194 ++++++------------ 1 file changed, 64 insertions(+), 130 deletions(-) diff --git a/apps/hash-frontend/src/pages/settings/security.page.tsx b/apps/hash-frontend/src/pages/settings/security.page.tsx index e9d82213783..554336955e3 100644 --- a/apps/hash-frontend/src/pages/settings/security.page.tsx +++ b/apps/hash-frontend/src/pages/settings/security.page.tsx @@ -96,11 +96,8 @@ const SecurityPage: NextPageWithLayout = () => { authenticatedUser?.emails[0]?.address ?? ""; const [flow, setFlow] = useState(); - const [currentPassword, setCurrentPassword] = useState(""); const [password, setPassword] = useState(""); - const [isRecoveryFlow, setIsRecoveryFlow] = useState(false); - const [currentPasswordError, setCurrentPasswordError] = useState(); const [totpCode, setTotpCode] = useState(""); const [showTotpSetupForm, setShowTotpSetupForm] = useState(false); const [showTotpDisableForm, setShowTotpDisableForm] = useState(false); @@ -167,9 +164,6 @@ const SecurityPage: NextPageWithLayout = () => { const initFlow = (data: SettingsFlow) => { setFlow(data); - if (data.ui.messages?.some(({ id }) => id === 1060001)) { - setIsRecoveryFlow(true); - } }; if (flowId) { @@ -231,6 +225,51 @@ const SecurityPage: NextPageWithLayout = () => { setShowTotpDisableForm(false); }, [isTotpEnabled]); + const executeDisableTotp = useCallback( + async (currentFlow: SettingsFlow) => { + setDisablingTotp(true); + setShowTotpDisableForm(true); + persistFlowIdInUrl(currentFlow); + + try { + // Step 1: Unlink TOTP. + const unlinkedFlow = await submitSettingsUpdate(currentFlow, { + method: "totp", + totp_unlink: true, + csrf_token: mustGetCsrfTokenFromFlow(currentFlow), + }); + + if (!unlinkedFlow) { + return; + } + + // Step 2: Remove backup codes. Kratos enforces AAL2 as long as + // any second factor is present, so leaving orphan backup codes + // behind after unlinking TOTP would lock the user out at next + // login — they'd be asked for an authenticator code they no + // longer have. + const clearedFlow = await submitSettingsUpdate(unlinkedFlow, { + method: "lookup_secret", + lookup_secret_disable: true, + csrf_token: mustGetCsrfTokenFromFlow(unlinkedFlow), + }); + + if (!clearedFlow) { + setErrorMessage( + "TOTP was disabled, but backup codes could not be removed. " + + "Please reload the page and try again to avoid being locked out.", + ); + return; + } + + setShowTotpDisableForm(false); + } finally { + setDisablingTotp(false); + } + }, + [submitSettingsUpdate, persistFlowIdInUrl], + ); + const totpQrCodeDataUri = useMemo(() => { for (const { attributes } of totpNodes) { if ( @@ -264,61 +303,26 @@ const SecurityPage: NextPageWithLayout = () => { const handlePasswordSubmit: FormEventHandler = (event) => { event.preventDefault(); - if (!flow || !password || (!isRecoveryFlow && !currentPassword)) { + if (!flow || !password) { return; } setUpdatingPassword(true); - setCurrentPasswordError(undefined); persistFlowIdInUrl(flow); - // @todo H-6417 the password-reentry step below only works for users - // who actually have a password credential — SSO-only users can't - // change their password today. The SSO-compatible forced session - // refresh being designed under H-6417 is the path to fixing this - // (tracked separately as H-6418 for the password-setup UI itself). - - const updatePassword = async () => { - if (!isRecoveryFlow) { - // Verify the current password by creating and submitting a refresh - // login flow. This also refreshes the session to "privileged", - // ensuring the settings update won't be rejected. - // Recovery flows are already privileged, so we don't need to refresh them. - await oryKratosClient - .createBrowserLoginFlow({ refresh: true }) - .then(({ data: loginFlow }) => - oryKratosClient.updateLoginFlow({ - flow: loginFlow.id, - updateLoginFlowBody: { - method: "password", - identifier: usernameForPasswordManagers, - password: currentPassword, - csrf_token: mustGetCsrfTokenFromFlow(loginFlow), - }, - }), - ); - } - - const nextFlow = await submitSettingsUpdate(flow, { - method: "password", - password, - csrf_token: mustGetCsrfTokenFromFlow(flow), - }); - - if (nextFlow) { - setCurrentPassword(""); - setPassword(""); - } - }; - - void updatePassword() - .catch((error: AxiosError) => { - if (error.response?.status === 400) { - setCurrentPasswordError("Current password is incorrect."); - return; + // No manual session refresh — Kratos enforces `privileged_session_max_age` + // server-side. If the session is stale, Kratos returns + // `session_refresh_required` and the error handler takes over. + // This works for both password and SSO-only users. + void submitSettingsUpdate(flow, { + method: "password", + password, + csrf_token: mustGetCsrfTokenFromFlow(flow), + }) + .then((nextFlow) => { + if (nextFlow) { + setPassword(""); } - - void handleFlowError(error); }) .finally(() => setUpdatingPassword(false)); }; @@ -393,54 +397,11 @@ const SecurityPage: NextPageWithLayout = () => { return; } - setDisablingTotp(true); - persistFlowIdInUrl(flow); - - // @todo H-6417 force a session refresh before a security-sensitive - // change like this, rather than relying solely on Kratos's - // `privileged_session_max_age` window. The shape: queue the pending - // change, redirect to `/signin?refresh=true&return_to=…`, let the - // user reauthenticate via whichever credential they have (password, - // SSO, passkey), then resume the change on return. Must work for - // SSO-only users, so a password-reentry shim alone is not enough. - - const disableSequence = async () => { - // Step 1: Unlink TOTP. - const unlinkedFlow = await submitSettingsUpdate(flow, { - method: "totp", - totp_unlink: true, - csrf_token: mustGetCsrfTokenFromFlow(flow), - }); - - if (!unlinkedFlow) { - return; - } - - // Step 2: Unlink the `lookup_secret` credential. - // `required_aal: highest_available` treats any enrolled second - // factor (including backup codes) as enforcing AAL2, so leaving - // them behind after removing TOTP would force the user through an - // AAL2 prompt with no TOTP to answer it. If this step fails, TOTP - // is already gone — surface an error so the user can retry rather - // than silently landing in the orphan-AAL2 state. - const clearedFlow = await submitSettingsUpdate(unlinkedFlow, { - method: "lookup_secret", - lookup_secret_disable: true, - csrf_token: mustGetCsrfTokenFromFlow(unlinkedFlow), - }); - - if (!clearedFlow) { - setErrorMessage( - "TOTP was disabled, but backup codes could not be removed. " + - "Please reload the page and try again to avoid being locked out.", - ); - return; - } - - setShowTotpDisableForm(false); - }; - - void disableSequence().finally(() => setDisablingTotp(false)); + // Kratos enforces `privileged_session_max_age` server-side. If the + // session is stale, the first `submitSettingsUpdate` call inside + // `executeDisableTotp` will fail with `session_refresh_required` and + // the error handler will redirect to re-authenticate. + void executeDisableTotp(flow); }; const handleRegenerateBackupCodes = () => { @@ -535,26 +496,6 @@ const SecurityPage: NextPageWithLayout = () => { Password - {!isRecoveryFlow && ( - { - setCurrentPassword(target.value); - setCurrentPasswordError(undefined); - }} - error={!!currentPasswordError} - helperText={ - currentPasswordError ? ( - {currentPasswordError} - ) : undefined - } - required - /> - )} { /> - From fb72744484c8112b0da30ff573b5117305e3ebdb Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Thu, 16 Apr 2026 14:43:17 +0200 Subject: [PATCH 05/14] Clean up dead test helpers, tighten error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Delete `loginUsingTempForm`, `loginUsingUi`, and `getDerivedPayloadFromMostRecentEmail`; consolidate on `signInWithPassword`. - Rename `signOut` → `clearSessionCookies`. - Deduplicate `defaultPassword`; make `password` required in signup helpers. - `callGraphQlApi` checks HTTP status and GraphQL errors. - `global-setup` try/finally to avoid leaking Chromium. - `decodeBase32` throws on invalid characters. - `openPopupTab` helper pins `popupTab` in storage before clicking. - Tighten chip assertions to cover linked types. - `submitSettingsUpdate` surfaces errors instead of re-rejecting. - Null-guard `extractBackupCodesFromFlow` textContent. - Show error on empty backup code regeneration. - Correlate `gatherUiNodeValuesFromFlow` / `useKratosErrorHandler` generics with flow type. - Derive `flowMetadata.settingsWithPassword` from `settings`. - Log malformed Kratos redirect URLs. - `waitForConnection` timeout + CLOSED/CLOSING check. - Wrap WebSocket `JSON.parse` in try/catch. - Catch `getCookieString` rejection in setInterval. - Log WebSocket error event. --- .../src/pages/settings/security.page.tsx | 15 +- .../src/pages/shared/ory-kratos.ts | 21 +-- .../shared/use-kratos-flow-error-handler.ts | 6 +- .../src/scripts/background/infer-entities.ts | 43 ++++-- tests/hash-playwright/global-setup.ts | 28 ++-- tests/hash-playwright/package.json | 4 +- tests/hash-playwright/playwright.config.ts | 50 ++++--- .../hash-playwright/tests/account/mfa.spec.ts | 13 +- .../tests/account/password.spec.ts | 8 +- .../tests/account/signin.spec.ts | 4 +- .../tests/account/signup.spec.ts | 8 +- .../tests/extension/browser-plugin.spec.ts | 132 ++++++++++-------- .../tests/features/page-readonly-mode.spec.ts | 6 - .../tests/shared/api-queries.ts | 45 +++--- ...-derived-payload-from-most-recent-email.ts | 93 ------------ .../tests/shared/login-using-temp-form.ts | 35 ----- .../tests/shared/login-using-ui.ts | 46 ------ .../tests/shared/signin-utils.ts | 10 +- .../tests/shared/signup-utils.ts | 8 +- .../tests/shared/test-users.ts | 2 +- .../tests/shared/totp-utils.ts | 2 +- 21 files changed, 241 insertions(+), 338 deletions(-) delete mode 100644 tests/hash-playwright/tests/shared/get-derived-payload-from-most-recent-email.ts delete mode 100644 tests/hash-playwright/tests/shared/login-using-temp-form.ts delete mode 100644 tests/hash-playwright/tests/shared/login-using-ui.ts diff --git a/apps/hash-frontend/src/pages/settings/security.page.tsx b/apps/hash-frontend/src/pages/settings/security.page.tsx index 554336955e3..adc473c455c 100644 --- a/apps/hash-frontend/src/pages/settings/security.page.tsx +++ b/apps/hash-frontend/src/pages/settings/security.page.tsx @@ -80,7 +80,7 @@ const extractBackupCodesFromFlow = (flow: SettingsFlow): string[] => { // The data comes from Kratos in any case. const withNewlines = codesText.replace(//gi, "\n"); const parsed = new DOMParser().parseFromString(withNewlines, "text/html"); - const plainText = parsed.body.textContent; + const plainText = parsed.body.textContent ?? ""; return plainText .split("\n") @@ -152,9 +152,13 @@ const SecurityPage: NextPageWithLayout = () => { return undefined; } - return Promise.reject(error); + setErrorMessage( + error.response?.data?.ui?.messages?.[0]?.text ?? + "Something went wrong. Please try again.", + ); + return undefined; }), - [handleFlowError], + [handleFlowError, setErrorMessage], ); useEffect(() => { @@ -427,6 +431,11 @@ const SecurityPage: NextPageWithLayout = () => { if (regeneratedCodes.length > 0) { setBackupCodes(regeneratedCodes); setShowBackupCodesModal(true); + } else { + setErrorMessage( + "Could not extract backup codes from the response. " + + "Please reload the page and try again.", + ); } }) .finally(() => setRegeneratingBackupCodes(false)); diff --git a/apps/hash-frontend/src/pages/shared/ory-kratos.ts b/apps/hash-frontend/src/pages/shared/ory-kratos.ts index 77bb96bf048..c4e99d8e0dd 100644 --- a/apps/hash-frontend/src/pages/shared/ory-kratos.ts +++ b/apps/hash-frontend/src/pages/shared/ory-kratos.ts @@ -57,7 +57,7 @@ export type FlowValues = Flows[FlowNames][0]; * does not serve these paths, redirects need to be rewritten to the * matching `uiPath` to avoid dead-ending on a 404. */ -export const flowMetadata = { +const _flowMetadata = { login: { uiPath: "/signin", kratosBrowserPath: "/self-service/login/browser", @@ -74,17 +74,17 @@ export const flowMetadata = { uiPath: "/settings/security", kratosBrowserPath: "/self-service/settings/browser", }, - // `settingsWithPassword` shares its UI route and Kratos endpoint with - // `settings`. The split is purely a TypeScript-level distinction over the - // submit-body shape (see `Flows[settingsWithPassword]`), not over routing. - settingsWithPassword: { - uiPath: "/settings/security", - kratosBrowserPath: "/self-service/settings/browser", - }, verification: { uiPath: "/verification", kratosBrowserPath: "/self-service/verification/browser", }, +} as const; + +// `settingsWithPassword` shares routes with `settings` — the split is +// purely a TypeScript-level distinction over the submit-body shape. +export const flowMetadata = { + ..._flowMetadata, + settingsWithPassword: _flowMetadata.settings, } as const satisfies Record< FlowNames, { uiPath: string; kratosBrowserPath: string } @@ -106,7 +106,8 @@ export const uiPathForKratosBrowserRedirect = ( try { parsed = new URL(redirectUrl); } catch { - // Malformed URL — caller will fall back to the raw redirect string. + // eslint-disable-next-line no-console + console.warn("Malformed Kratos redirect URL:", redirectUrl); return undefined; } @@ -118,7 +119,7 @@ export const uiPathForKratosBrowserRedirect = ( }; export const gatherUiNodeValuesFromFlow = ( - flow: FlowValues, + flow: Flows[T][0], ): Flows[T][1] => flow.ui.nodes .map(({ attributes }) => attributes) diff --git a/apps/hash-frontend/src/pages/shared/use-kratos-flow-error-handler.ts b/apps/hash-frontend/src/pages/shared/use-kratos-flow-error-handler.ts index cfb25c373f4..4c3ef730716 100644 --- a/apps/hash-frontend/src/pages/shared/use-kratos-flow-error-handler.ts +++ b/apps/hash-frontend/src/pages/shared/use-kratos-flow-error-handler.ts @@ -12,9 +12,9 @@ import { useAuthInfo } from "./auth-info-context"; import type { Flows } from "./ory-kratos"; import { flowMetadata, uiPathForKratosBrowserRedirect } from "./ory-kratos"; -export const useKratosErrorHandler = (props: { - flowType: keyof Flows; - setFlow: Dispatch>; +export const useKratosErrorHandler = (props: { + flowType: K; + setFlow: Dispatch>; setErrorMessage: Dispatch>; }) => { const { flowType, setFlow, setErrorMessage } = props; diff --git a/apps/plugin-browser/src/scripts/background/infer-entities.ts b/apps/plugin-browser/src/scripts/background/infer-entities.ts index a1b1244837f..443634fd5e0 100644 --- a/apps/plugin-browser/src/scripts/background/infer-entities.ts +++ b/apps/plugin-browser/src/scripts/background/infer-entities.ts @@ -97,9 +97,19 @@ const enqueueTask = (task: () => Promise) => { }; const waitForConnection = async (socket: WebSocket) => { - while (socket.readyState !== socket.OPEN) { + const timeout = 10_000; + const start = Date.now(); + while (socket.readyState === socket.CONNECTING) { + if (Date.now() - start > timeout) { + throw new Error("WebSocket connection timed out"); + } await sleep(200); } + if (socket.readyState !== socket.OPEN) { + throw new Error( + `WebSocket is ${socket.readyState === socket.CLOSING ? "closing" : "closed"}`, + ); + } }; let ws: WebSocket | null = null; @@ -114,14 +124,18 @@ const createWebSocket = async ({ onClose }: { onClose: () => void }) => { const newWs = new WebSocket(websocketUrl); const externalRequestPoll = setInterval(() => { - void getCookieString().then((cookie) => - newWs.send( - JSON.stringify({ - cookie, - type: "check-for-external-input-requests", - } satisfies CheckForExternalInputRequestsWebsocketRequestMessage), - ), - ); + void getCookieString() + .then((cookie) => + newWs.send( + JSON.stringify({ + cookie, + type: "check-for-external-input-requests", + } satisfies CheckForExternalInputRequestsWebsocketRequestMessage), + ), + ) + .catch(() => { + // No session cookies — skip this poll tick. + }); }, 20_000); newWs.addEventListener("open", () => { @@ -136,9 +150,10 @@ const createWebSocket = async ({ onClose }: { onClose: () => void }) => { onClose(); }); - newWs.addEventListener("error", () => { + newWs.addEventListener("error", (event) => { console.error( "WebSocket error encountered. Closing and attempting to reconnect...", + event, ); newWs.close(); }); @@ -148,7 +163,13 @@ const createWebSocket = async ({ onClose }: { onClose: () => void }) => { // eslint-disable-next-line @typescript-eslint/no-misused-promises async (event: MessageEvent) => { - const message = JSON.parse(event.data) as InferenceWebsocketServerMessage; + let message: InferenceWebsocketServerMessage; + try { + message = JSON.parse(event.data) as InferenceWebsocketServerMessage; + } catch { + console.error("Malformed WebSocket message:", event.data); + return; + } const { workflowId, payload } = message; if (payload.type === "get-urls-html-content") { diff --git a/tests/hash-playwright/global-setup.ts b/tests/hash-playwright/global-setup.ts index e49981239cc..1504c1c3b5b 100644 --- a/tests/hash-playwright/global-setup.ts +++ b/tests/hash-playwright/global-setup.ts @@ -10,19 +10,21 @@ const baseURL = "http://localhost:3000"; */ export default async function globalSetup() { const browser = await chromium.launch(); - const context = await browser.newContext({ baseURL }); - const page = await context.newPage(); + try { + const context = await browser.newContext({ baseURL }); + const page = await context.newPage(); - await page.goto("/signin"); - await page.fill( - '[placeholder="Enter your email address"]', - "alice@example.com", - ); - await page.fill('[type="password"]', "password"); - await page.click("text=Submit"); - await page.waitForURL("/", { timeout: 30_000 }); + await page.goto("/signin"); + await page.fill( + '[placeholder="Enter your email address"]', + "alice@example.com", + ); + await page.fill('[type="password"]', "password"); + await page.click("text=Submit"); + await page.waitForURL("/", { timeout: 30_000 }); - await context.storageState({ path: "tests/.auth/alice.json" }); - - await browser.close(); + await context.storageState({ path: "tests/.auth/alice.json" }); + } finally { + await browser.close(); + } } diff --git a/tests/hash-playwright/package.json b/tests/hash-playwright/package.json index 9c0bd06f864..a3e41a32e75 100644 --- a/tests/hash-playwright/package.json +++ b/tests/hash-playwright/package.json @@ -8,7 +8,9 @@ "fix:eslint": "eslint --fix .", "lint:eslint": "eslint --report-unused-disable-directives .", "lint:tsc": "tsc --noEmit", - "test:integration": "PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1 PW_EXPERIMENTAL_TS_ESM=1 npx playwright test --project chromium" + "test:integration": "PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1 PW_EXPERIMENTAL_TS_ESM=1 npx playwright test --project '*-chromium'", + "test:integration:firefox": "PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1 PW_EXPERIMENTAL_TS_ESM=1 npx playwright test --project '*-firefox'", + "test:integration:webkit": "PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1 PW_EXPERIMENTAL_TS_ESM=1 npx playwright test --project '*-webkit'" }, "dependencies": { "@blockprotocol/type-system": "workspace:*", diff --git a/tests/hash-playwright/playwright.config.ts b/tests/hash-playwright/playwright.config.ts index b43543237be..4dc39512171 100644 --- a/tests/hash-playwright/playwright.config.ts +++ b/tests/hash-playwright/playwright.config.ts @@ -3,31 +3,41 @@ import { devices } from "@playwright/test"; const ci = process.env.CI === "true"; +// Flow-based test groups. Each entry becomes a project per browser +// (currently Chrome only; add Firefox/WebKit entries to `browsers` +// below to widen the matrix). +const flows = [ + { name: "account", testMatch: "tests/account/**" }, + { + name: "features", + testMatch: "tests/features/**", + extra: { storageState: "tests/.auth/alice.json" }, + }, + { name: "guest", testMatch: "tests/guest/**" }, +] as const; + +const browsers = [ + { suffix: "chromium", device: devices["Desktop Chrome"] }, + { suffix: "firefox", device: devices["Desktop Firefox"] }, + { suffix: "webkit", device: devices["Desktop Safari"] }, +]; + const config: PlaywrightTestConfig = { forbidOnly: ci, globalSetup: "./global-setup", projects: [ + // Browser-matrix projects generated from flows × browsers. + ...browsers.flatMap(({ suffix, device }) => + flows.map((flow) => ({ + name: `${flow.name}-${suffix}`, + testMatch: flow.testMatch, + use: { ...device, ...("extra" in flow ? flow.extra : {}) }, + })), + ), + // Extension tests use a custom persistent-context fixture and only + // run on Chromium (Chrome extension API). { - name: "account", - testMatch: "tests/account/**", - use: { ...devices["Desktop Chrome"] }, - }, - { - name: "features", - testMatch: "tests/features/**", - use: { - ...devices["Desktop Chrome"], - storageState: "tests/.auth/alice.json", - }, - dependencies: ["account"], - }, - { - name: "guest", - testMatch: "tests/guest/**", - use: { ...devices["Desktop Chrome"] }, - }, - { - name: "extension", + name: "extension-chromium", testMatch: "tests/extension/**", use: { ...devices["Desktop Chrome"] }, }, diff --git a/tests/hash-playwright/tests/account/mfa.spec.ts b/tests/hash-playwright/tests/account/mfa.spec.ts index 9c031d45f4e..d91f778aec2 100644 --- a/tests/hash-playwright/tests/account/mfa.spec.ts +++ b/tests/hash-playwright/tests/account/mfa.spec.ts @@ -1,5 +1,8 @@ import { expect, type Page, test } from "../shared/runtime"; -import { signInWithPassword, signOut } from "../shared/signin-utils"; +import { + clearSessionCookies, + signInWithPassword, +} from "../shared/signin-utils"; import { testUsers, withTestUser } from "../shared/test-users"; import { generateTotpCode, waitForFreshTotpWindow } from "../shared/totp-utils"; @@ -54,7 +57,7 @@ test("user with TOTP is prompted for code at login", async ({ page }) => { const credentials = await withTestUser(page, testUsers.mfaLogin); const { secret } = await enableTotpForCurrentUser(page); - await signOut(page); + await clearSessionCookies(page); await signInWithPassword(page, credentials); @@ -78,7 +81,7 @@ test("user can use backup code instead of TOTP", async ({ page }) => { expect(backupCodes.length).toBeGreaterThan(0); - await signOut(page); + await clearSessionCookies(page); await signInWithPassword(page, credentials); await expect( @@ -113,7 +116,7 @@ test("user can disable TOTP", async ({ page }) => { page.locator('[data-testid="regenerate-backup-codes-button"]'), ).not.toBeVisible(); - await signOut(page); + await clearSessionCookies(page); await signInWithPassword(page, credentials); @@ -127,7 +130,7 @@ test("wrong TOTP code shows error at login", async ({ page }) => { const credentials = await withTestUser(page, testUsers.mfaWrongCode); await enableTotpForCurrentUser(page); - await signOut(page); + await clearSessionCookies(page); await signInWithPassword(page, credentials); await expect( diff --git a/tests/hash-playwright/tests/account/password.spec.ts b/tests/hash-playwright/tests/account/password.spec.ts index 90cc0275d41..572a4ab14de 100644 --- a/tests/hash-playwright/tests/account/password.spec.ts +++ b/tests/hash-playwright/tests/account/password.spec.ts @@ -1,9 +1,9 @@ import { getKratosRecoveryCode } from "../shared/get-kratos-verification-code"; import { expect, test } from "../shared/runtime"; import { + clearSessionCookies, expectSignedIn, signInWithPassword, - signOut, } from "../shared/signin-utils"; import { testUsers, withTestUser } from "../shared/test-users"; @@ -20,7 +20,7 @@ test("user can change password from settings", async ({ page }) => { timeout: 5_000, }); - await signOut(page); + await clearSessionCookies(page); await signInWithPassword(page, { email: credentials.email, password: newPassword, @@ -32,7 +32,7 @@ test("user can recover account and set a new password", async ({ page }) => { const newPassword = "recovered-pw-3cd4"; const credentials = await withTestUser(page, testUsers.pwRecovery); - await signOut(page); + await clearSessionCookies(page); // Start recovery flow const recoveryFlowReady = page.waitForResponse( @@ -84,7 +84,7 @@ test("user can recover account and set a new password", async ({ page }) => { timeout: 5_000, }); - await signOut(page); + await clearSessionCookies(page); await signInWithPassword(page, { email: credentials.email, password: newPassword, diff --git a/tests/hash-playwright/tests/account/signin.spec.ts b/tests/hash-playwright/tests/account/signin.spec.ts index 0ae5a04f59d..dcdf74db816 100644 --- a/tests/hash-playwright/tests/account/signin.spec.ts +++ b/tests/hash-playwright/tests/account/signin.spec.ts @@ -1,16 +1,16 @@ import { expect, test } from "../shared/runtime"; import { + clearSessionCookies, expectSignedIn, expectSignedOut, signInWithPassword, - signOut, } from "../shared/signin-utils"; import { testUsers, withTestUser } from "../shared/test-users"; test("user can sign in with password", async ({ page }) => { const credentials = await withTestUser(page, testUsers.signinTest); - await signOut(page); + await clearSessionCookies(page); await expectSignedOut(page); await signInWithPassword(page, credentials); diff --git a/tests/hash-playwright/tests/account/signup.spec.ts b/tests/hash-playwright/tests/account/signup.spec.ts index 4d6981c1b1e..5daf961e086 100644 --- a/tests/hash-playwright/tests/account/signup.spec.ts +++ b/tests/hash-playwright/tests/account/signup.spec.ts @@ -5,7 +5,7 @@ import { registerUser, verifyEmailOnPage, } from "../shared/signup-utils"; -import { testUsers } from "../shared/test-users"; +import { defaultPassword, testUsers } from "../shared/test-users"; test("allowlisted user can verify email and complete signup", async ({ page, @@ -13,7 +13,10 @@ test("allowlisted user can verify email and complete signup", async ({ const { email } = testUsers.signupAllowlisted; await deleteUserByEmail(email); - const { emailDispatchTimestamp } = await registerUser(page, { email }); + const { emailDispatchTimestamp } = await registerUser(page, { + email, + password: defaultPassword, + }); await verifyEmailOnPage(page, { email, @@ -34,6 +37,7 @@ test("waitlisted user is redirected to waitlist after signup", async ({ const { emailDispatchTimestamp } = await registerUser(page, { email: waitlistedEmail, + password: defaultPassword, }); await verifyEmailOnPage(page, { diff --git a/tests/hash-playwright/tests/extension/browser-plugin.spec.ts b/tests/hash-playwright/tests/extension/browser-plugin.spec.ts index 42e8e658bc5..1ccf21a4803 100644 --- a/tests/hash-playwright/tests/extension/browser-plugin.spec.ts +++ b/tests/hash-playwright/tests/extension/browser-plugin.spec.ts @@ -2,7 +2,7 @@ import type { Page } from "@playwright/test"; import { expect, test } from "../shared/browser-plugin-fixtures/fixtures"; -import { loginUsingTempForm } from "../shared/login-using-temp-form"; +import { expectSignedIn, signInWithPassword } from "../shared/signin-utils"; const loggedOutHeaderLocator = "text=Connect to HASH"; const createAccountButtonLocator = "text=Create a free account"; @@ -47,10 +47,33 @@ const waitForPopupStateLoaded = (page: Page) => }), ); +type PopupTab = "one-off" | "automated" | "history"; + +/** + * Navigate to the popup, wait for mount-time fetches to settle, then + * switch to the given tab. Pins `popupTab` in `chrome.storage.local` + * first so a late `getUser` write can't flip the tab back. + */ +const openPopupTab = async (page: Page, extensionId: string, tab: PopupTab) => { + await page.goto(`chrome-extension://${extensionId}/popup.html`); + await waitForPopupStateLoaded(page); + await page.evaluate( + async (targetTab) => chrome.storage.local.set({ popupTab: targetTab }), + tab, + ); + const label = + tab === "one-off" + ? "One-off" + : tab === "automated" + ? "Automated" + : "History"; + await page.click(`text=${label}`); + await waitForPopupStateLoaded(page); +}; + /** * Type a name into an open entity-type autocomplete and select the - * matching option. ArrowDown is required because the component doesn't - * use MUI's `autoHighlight`. + * first matching option via ArrowDown + Enter. */ const selectEntityTypeOption = async ( page: Page, @@ -131,7 +154,8 @@ const signOutAndReloadPopup = async ({ await page.goto(`chrome-extension://${extensionId}/popup.html`); await expect(page.locator(loggedOutHeaderLocator)).toBeVisible(); - await loginUsingTempForm({ page }); + await signInWithPassword(page); + await expectSignedIn(page); await page.goto(`chrome-extension://${extensionId}/popup.html`); await expect(page.locator("text=One-off")).toBeVisible(); }; @@ -151,7 +175,8 @@ test("popup window loads with logged-in state", async ({ page, extensionId, }) => { - await loginUsingTempForm({ page }); + await signInWithPassword(page); + await expectSignedIn(page); await page.goto(`chrome-extension://${extensionId}/popup.html`); @@ -173,7 +198,8 @@ test("options page loads with logged-in state", async ({ page, extensionId, }) => { - await loginUsingTempForm({ page }); + await signInWithPassword(page); + await expectSignedIn(page); await page.goto(`chrome-extension://${extensionId}/options.html`); @@ -184,59 +210,57 @@ test("user can type a quick note which persists across logouts", async ({ page, extensionId, }) => { - await loginUsingTempForm({ page }); + await signInWithPassword(page); + await expectSignedIn(page); - await page.goto(`chrome-extension://${extensionId}/popup.html`); - await waitForPopupStateLoaded(page); - await page.click("text=One-off"); - await waitForPopupStateLoaded(page); + await openPopupTab(page, extensionId, "one-off"); // Unique per-run value so the write is always observably different - // from persisted backend state. Dispatching a native `input` event - // triggers MUI's controlled `TextField` onChange more reliably than - // `page.fill`, which sometimes skips the handler when the prior - // value matches. + // from persisted backend state. const testQuickNote = `Hello, world! Here's a note ${Date.now()}`; - const settingsSaved = waitForSettingsSave(page); - await page.locator(quickNoteInputLocator).evaluate((element, value) => { - const descriptor = Object.getOwnPropertyDescriptor( - HTMLTextAreaElement.prototype, - "value", - ); - descriptor?.set?.call(element, value); - element.dispatchEvent(new Event("input", { bubbles: true })); - }, testQuickNote); - await settingsSaved; + await page.fill(quickNoteInputLocator, testQuickNote); + + // `setInLocalStorage` writes the draft to `chrome.storage.local` + // immediately, but the backend `updateEntity` mutation fires after a + // 1 s debounce. Playwright can't observe extension background + // traffic, so we poll storage until the value lands, then wait for + // the debounce + backend round-trip to complete before signing out. + await page.waitForFunction( + async (expected) => { + const { draftQuickNote } = + await chrome.storage.local.get("draftQuickNote"); + return draftQuickNote === expected; + }, + testQuickNote, + { timeout: 5_000 }, + ); + // The 1 s debounce must fire and the backend must respond before we + // sign out, otherwise the old value survives. Two idle windows + // (500 ms each) cover: debounce fires → storage write from the + // response → idle. + await waitForPopupStateLoaded(page); + await waitForPopupStateLoaded(page); await signOutAndReloadPopup({ extensionId, page }); - - // Backend-restored `popupTab` may land on "automated" after login. - await waitForPopupStateLoaded(page); - await page.click("text=One-off"); + await openPopupTab(page, extensionId, "one-off"); await expect(page.locator(quickNoteInputLocator)).toHaveValue(testQuickNote); }); -/** - * Selecting a type also adds its linked types (see H-1721), so Actor + - * Document yields four chips. Exact-regex matches avoid colliding with - * e.g. `Document File`. - * - * @todo verify the correct WebSocket message is sent to the API when - * `Suggest entities` is clicked — see - * https://github.com/microsoft/playwright/issues/15684#issuecomment-1892644655 - */ +// Selecting a type also adds its linked types as targets, so Actor + +// Document produces chips for [Actor, Block, Document, Has Indexed Content]. +// +// @todo verify the correct WebSocket message is sent to the API when +// `Suggest entities` is clicked — see +// https://github.com/microsoft/playwright/issues/15684#issuecomment-1892644655 test("user can configure a one-off inference, and the settings are persisted", async ({ page, extensionId, }) => { - await loginUsingTempForm({ page }); - - await page.goto(`chrome-extension://${extensionId}/popup.html`); - await waitForPopupStateLoaded(page); - await page.click("text=One-off"); - await waitForPopupStateLoaded(page); + await signInWithPassword(page); + await expectSignedIn(page); + await openPopupTab(page, extensionId, "one-off"); await resetOneOffState(page); await page.click(entityTypeSelectorLocator); @@ -249,14 +273,14 @@ test("user can configure a one-off inference, and the settings are persisted", a await waitForSettingsSave(page); await signOutAndReloadPopup({ extensionId, page }); - - await waitForPopupStateLoaded(page); - await page.click("text=One-off"); + await openPopupTab(page, extensionId, "one-off"); const chipLabel = (name: string) => page.locator(".MuiChip-label").filter({ hasText: new RegExp(`^${name}$`) }); await expect(chipLabel("Actor")).toHaveCount(1); await expect(chipLabel("Document")).toHaveCount(1); + await expect(chipLabel("Block")).toHaveCount(1); + await expect(chipLabel("Has Indexed Content")).toHaveCount(1); }); /** @@ -268,13 +292,10 @@ test("user can enable automatic inference, and the settings are persisted", asyn page, extensionId, }) => { - await loginUsingTempForm({ page }); - - await page.goto(`chrome-extension://${extensionId}/popup.html`); - await waitForPopupStateLoaded(page); - await page.click("text=Automated"); - await waitForPopupStateLoaded(page); + await signInWithPassword(page); + await expectSignedIn(page); + await openPopupTab(page, extensionId, "automated"); await resetAutomatedState(page); // `SelectScope` initializes `showTable` and `draftRule` from @@ -296,10 +317,7 @@ test("user can enable automatic inference, and the settings are persisted", asyn await settingsSaved; await signOutAndReloadPopup({ extensionId, page }); - - await waitForPopupStateLoaded(page); - await page.click("text=Automated"); - await waitForPopupStateLoaded(page); + await openPopupTab(page, extensionId, "automated"); await expect(page.locator("text=Enabled")).toBeVisible(); await expect(page.locator("[value=Actor]")).toBeVisible(); }); diff --git a/tests/hash-playwright/tests/features/page-readonly-mode.spec.ts b/tests/hash-playwright/tests/features/page-readonly-mode.spec.ts index c1ca3eaa6cd..c1efd9758be 100644 --- a/tests/hash-playwright/tests/features/page-readonly-mode.spec.ts +++ b/tests/hash-playwright/tests/features/page-readonly-mode.spec.ts @@ -1,6 +1,5 @@ import { sleep } from "@local/hash-isomorphic-utils/sleep"; -import { loginUsingUi } from "../shared/login-using-ui"; import { expect, test } from "../shared/runtime"; const placeholderSelector = @@ -13,11 +12,6 @@ const placeholderSelector = test.skip("user can view page in read-only mode but not update", async ({ page, }) => { - await loginUsingUi({ - page, - accountShortName: "alice", - }); - // TODO: investigate why delay is required for create page button to work await sleep(500); await page.locator('[data-testid="create-page-btn"]').click(); diff --git a/tests/hash-playwright/tests/shared/api-queries.ts b/tests/hash-playwright/tests/shared/api-queries.ts index ec554a6c566..7f9e16772e4 100644 --- a/tests/hash-playwright/tests/shared/api-queries.ts +++ b/tests/hash-playwright/tests/shared/api-queries.ts @@ -33,28 +33,37 @@ const callGraphQlApi = async ( variables?: Variables; }, ): Promise<{ data?: Response; errors?: GraphQLError[] }> => { - return requestContext - .post(`${apiOrigin}/graphql`, { - data: { query, variables }, - }) - .then( - (resp) => - resp.json() as Promise<{ data?: Response; errors?: GraphQLError[] }>, + const resp = await requestContext.post(`${apiOrigin}/graphql`, { + data: { query, variables }, + }); + if (!resp.ok()) { + throw new Error( + `GraphQL request failed: ${resp.status()} ${resp.statusText()}`, + ); + } + const body = (await resp.json()) as { + data?: Response; + errors?: GraphQLError[]; + }; + if (body.errors?.length) { + throw new Error( + `GraphQL errors: ${body.errors.map((err) => err.message).join(", ")}`, ); + } + return body; }; export const getUser = async (requestContext: APIRequestContext) => { - return callGraphQlApi(requestContext, { - query: meQuery, - }).then(({ data }) => { - return !data - ? undefined - : getRoots( - deserializeSubgraph>>( - data.me.subgraph, - ), - )[0]; - }); + const { data } = await callGraphQlApi( + requestContext, + { query: meQuery }, + ); + if (!data) { + throw new Error("meQuery returned no data"); + } + return getRoots( + deserializeSubgraph>>(data.me.subgraph), + )[0]; }; export const createEntity = async ( diff --git a/tests/hash-playwright/tests/shared/get-derived-payload-from-most-recent-email.ts b/tests/hash-playwright/tests/shared/get-derived-payload-from-most-recent-email.ts deleted file mode 100644 index 3022dfcd76a..00000000000 --- a/tests/hash-playwright/tests/shared/get-derived-payload-from-most-recent-email.ts +++ /dev/null @@ -1,93 +0,0 @@ -import fs from "node:fs/promises"; -import path from "node:path"; - -import { monorepoRootDir } from "@local/hash-backend-utils/environment"; -import { sleep } from "@local/hash-isomorphic-utils/sleep"; -import { loadAll } from "js-yaml"; - -const emailDumpsFilePath = path.resolve( - monorepoRootDir, - "var/api/dummy-email-transporter/email-dumps.yml", -); - -const sharedErrorMessage = - "Please make sure that the API server is running and that it uses DummyEmailTransporter. The test needs to dispatch an email first."; - -const waitForRecentFileChange = async ( - filePath: string, - minFileChangeTimestamp: number, -) => { - const maxWaitTimeInMs = 5000; - const checkIntervalInMs = 50; - - let remainingWaitTime = maxWaitTimeInMs; - do { - try { - if ((await fs.stat(filePath)).mtimeMs >= minFileChangeTimestamp) { - return; - } - } catch { - // noop because fs.stat will be retried - } - await sleep(checkIntervalInMs); - remainingWaitTime -= checkIntervalInMs; - } while (remainingWaitTime > 0); - throw new Error( - `Expected ${filePath} to be modified since timestamp ${minFileChangeTimestamp}. Giving up after ${maxWaitTimeInMs}ms.`, - ); -}; - -/** - * Reads email dumps created by DummyEmailTransporter and returns derived payload - * from the most recent email. - * - * @param emailDispatchTimestamp If defined, the function waits for the dump - * to be written after the provided value. This helps avoid race conditions - * in tests, e.g. requesting a fresh signin code but reading the old one. - * An alternative would be to introduce a fixed delay, which can slow down tests. - */ -export const getDerivedPayloadFromMostRecentEmail = async ( - emailDispatchTimestamp?: number, -): Promise> => { - let emailDumps: unknown[]; - - if (emailDispatchTimestamp) { - await waitForRecentFileChange(emailDumpsFilePath, emailDispatchTimestamp); - } - - try { - emailDumps = loadAll(await fs.readFile(emailDumpsFilePath, "utf-8")); - } catch (error) { - throw new Error( - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - `Unable to load email dumps from ${emailDumpsFilePath}. ${sharedErrorMessage}\n\n${error}`, - ); - } - - const mostRecentEmailDump = emailDumps[0]; - - if (!mostRecentEmailDump) { - throw new Error( - `No emails have been found in ${emailDumpsFilePath}. ${sharedErrorMessage}`, - ); - } - - if (typeof mostRecentEmailDump !== "object") { - throw new Error( - `Expected most recent email to be an object, got ${JSON.stringify( - mostRecentEmailDump, - )}`, - ); - } - - const { derivedPayload } = mostRecentEmailDump as Record; - - if (typeof derivedPayload !== "object" || derivedPayload === null) { - throw new Error( - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - `Expected derivedPayload in most recent email to be an object, got ${derivedPayload}`, - ); - } - - return derivedPayload as Record; -}; diff --git a/tests/hash-playwright/tests/shared/login-using-temp-form.ts b/tests/hash-playwright/tests/shared/login-using-temp-form.ts deleted file mode 100644 index 405940878f7..00000000000 --- a/tests/hash-playwright/tests/shared/login-using-temp-form.ts +++ /dev/null @@ -1,35 +0,0 @@ -import type { Page } from "@playwright/test"; -import { expect } from "@playwright/test"; - -/** - * @todo Remove this function in favor of `loginUsingUi`once we have a proper login flow - */ -export const loginUsingTempForm = async ({ - page, - userEmail = "alice@example.com", - userPassword = "password", -}: { - page: Page; - userEmail?: string; - userPassword?: string; -}): Promise => { - await page.goto("/signin"); - - const emailInputSelector = '[placeholder="Enter your email address"]'; - - await page.fill(emailInputSelector, userEmail); - - await page.press(emailInputSelector, "Enter"); - - const passwordInputSelector = '[type="password"]'; - await page.fill(passwordInputSelector, userPassword); - await page.press(passwordInputSelector, "Enter"); - - // Wait for the redirect to the account page - await expect(page.locator("text=Get support")).toBeVisible({ - timeout: 60_000, - }); - - // Wait for user avatar to appear - await expect(page.locator(`[data-testid="user-avatar"]`)).toBeVisible(); -}; diff --git a/tests/hash-playwright/tests/shared/login-using-ui.ts b/tests/hash-playwright/tests/shared/login-using-ui.ts deleted file mode 100644 index 419a074b385..00000000000 --- a/tests/hash-playwright/tests/shared/login-using-ui.ts +++ /dev/null @@ -1,46 +0,0 @@ -import type { Page } from "@playwright/test"; -import { expect } from "@playwright/test"; - -import { getDerivedPayloadFromMostRecentEmail } from "./get-derived-payload-from-most-recent-email"; - -export const loginUsingUi = async ({ - page, - accountShortName, -}: { - page: Page; - accountShortName: string; -}): Promise => { - await page.goto("/signin"); - - // Enter account short name - const accountShortNameInputSelector = - '[placeholder="Enter your email or shortname"]'; - - await page.fill(accountShortNameInputSelector, accountShortName); - - const emailDispatchTimestamp = Date.now(); - await page.press(accountShortNameInputSelector, "Enter"); - - await expect( - page.locator( - "text=/User with id .* has created too many verification codes recently/", - ), - ).not.toBeVisible(); - - // Enter verification code - const verificationCodeInputSelector = '[data-testid="verify-code-input"]'; - await page.fill( - verificationCodeInputSelector, - (await getDerivedPayloadFromMostRecentEmail(emailDispatchTimestamp)) - .verificationCode as string, - ); - await page.press(verificationCodeInputSelector, "Enter"); - - // Wait for the redirect to the account page - await expect(page.locator("text=Get support")).toBeVisible(); - - // Wait for Sign in button to disappear - // TODO: Completely avoid rendering "Sign up" / "Sign in" after successful sign in - await expect(page.locator('button:has-text("Sign in")')).not.toBeVisible(); - await expect(page.locator(`[data-testid="user-avatar"]`)).toBeVisible(); -}; diff --git a/tests/hash-playwright/tests/shared/signin-utils.ts b/tests/hash-playwright/tests/shared/signin-utils.ts index e7598717897..0b7aad82135 100644 --- a/tests/hash-playwright/tests/shared/signin-utils.ts +++ b/tests/hash-playwright/tests/shared/signin-utils.ts @@ -1,9 +1,15 @@ import type { Page } from "@playwright/test"; import { expect } from "@playwright/test"; +/** Pre-seeded default user (see `seed-users.ts`). */ +const aliceCredentials = { + email: "alice@example.com", + password: "password", +}; + export const signInWithPassword = async ( page: Page, - { email, password }: { email: string; password: string }, + { email, password }: { email: string; password: string } = aliceCredentials, ) => { await page.goto("/signin"); await page.fill('[placeholder="Enter your email address"]', email); @@ -15,7 +21,7 @@ export const signInWithPassword = async ( * Drop the user's session cookies. Does not exercise the Kratos logout * flow — for that, see the dedicated sign-out test. */ -export const signOut = async (page: Page) => { +export const clearSessionCookies = async (page: Page) => { await page.context().clearCookies(); }; diff --git a/tests/hash-playwright/tests/shared/signup-utils.ts b/tests/hash-playwright/tests/shared/signup-utils.ts index be0f4e95660..1f87a808681 100644 --- a/tests/hash-playwright/tests/shared/signup-utils.ts +++ b/tests/hash-playwright/tests/shared/signup-utils.ts @@ -4,8 +4,6 @@ import { expect } from "@playwright/test"; import { deleteUserByEmail } from "./delete-user"; import { getKratosVerificationCode } from "./get-kratos-verification-code"; -const defaultPassword = "some-complex-pw-1ab2"; - /** * `deleteUserByEmail` intentionally preserves the user's web principal so * entity types created under it remain valid for other webs that reference @@ -24,7 +22,7 @@ const uniqueShortname = (base: string): string => { */ export const registerUser = async ( page: Page, - { email, password = defaultPassword }: { email: string; password?: string }, + { email, password }: { email: string; password: string }, ) => { await page.goto("/signup", { waitUntil: "networkidle" }); @@ -102,12 +100,12 @@ export const createUserAndCompleteSignup = async ( email, shortname, displayName, - password = defaultPassword, + password, }: { email: string; shortname: string; displayName?: string; - password?: string; + password: string; }, ) => { await deleteUserByEmail(email); diff --git a/tests/hash-playwright/tests/shared/test-users.ts b/tests/hash-playwright/tests/shared/test-users.ts index 0f07c6cdf5f..00aa4bb71f4 100644 --- a/tests/hash-playwright/tests/shared/test-users.ts +++ b/tests/hash-playwright/tests/shared/test-users.ts @@ -3,7 +3,7 @@ import type { Page } from "@playwright/test"; import { deleteUserByEmail } from "./delete-user"; import { createUserAndCompleteSignup } from "./signup-utils"; -const defaultPassword = "test-pw-1ab2"; +export const defaultPassword = "test-pw-1ab2"; interface TestUser { readonly email: string; diff --git a/tests/hash-playwright/tests/shared/totp-utils.ts b/tests/hash-playwright/tests/shared/totp-utils.ts index cf395600622..70be485397a 100644 --- a/tests/hash-playwright/tests/shared/totp-utils.ts +++ b/tests/hash-playwright/tests/shared/totp-utils.ts @@ -27,7 +27,7 @@ const decodeBase32 = (encodedSecret: string): Buffer => { const index = base32Alphabet.indexOf(character); if (index === -1) { - continue; + throw new Error(`Invalid base32 character '${character}' in TOTP secret`); } accumulator = accumulator * 32 + index; From e91d2c2855fab6a228fe4ab53c1d78b54b5afb8d Mon Sep 17 00:00:00 2001 From: Tim Diekmann <21277928+TimDiekmann@users.noreply.github.com> Date: Thu, 16 Apr 2026 15:19:07 +0200 Subject: [PATCH 06/14] Potential fix for pull request finding 'CodeQL / Log injection' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- apps/plugin-browser/src/scripts/background/infer-entities.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/plugin-browser/src/scripts/background/infer-entities.ts b/apps/plugin-browser/src/scripts/background/infer-entities.ts index 443634fd5e0..84e5c0927a5 100644 --- a/apps/plugin-browser/src/scripts/background/infer-entities.ts +++ b/apps/plugin-browser/src/scripts/background/infer-entities.ts @@ -167,7 +167,8 @@ const createWebSocket = async ({ onClose }: { onClose: () => void }) => { try { message = JSON.parse(event.data) as InferenceWebsocketServerMessage; } catch { - console.error("Malformed WebSocket message:", event.data); + const sanitizedEventData = String(event.data).replace(/[\r\n]/g, " "); + console.error("Malformed WebSocket message:", sanitizedEventData); return; } From 9d35b222892949ca8b8adca05ebf3e4356266a94 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Thu, 16 Apr 2026 16:18:14 +0200 Subject: [PATCH 07/14] Fix CI failures: ESLint, CodeQL sanitization, sidebar race - security.page.tsx: remove unnecessary optional chains flagged by @typescript-eslint/no-unnecessary-condition. - infer-entities.ts: sanitize event.data with full control-char strip + 1 000-char cap (CodeQL log-injection finding). - changeSidebarListDisplay: wait for the sidebar to reflect the new display mode after toggling (uppercase "ENTITIES" / "TYPES" header for list mode). Fixes a race in CI where the test clicked "Entities" before the sidebar had updated from link to list mode. --- .../src/pages/settings/security.page.tsx | 4 ++-- .../src/scripts/background/infer-entities.ts | 4 +++- .../tests/shared/change-sidebar-list-display.ts | 17 ++++++++++++++--- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/apps/hash-frontend/src/pages/settings/security.page.tsx b/apps/hash-frontend/src/pages/settings/security.page.tsx index adc473c455c..fd251f3bad0 100644 --- a/apps/hash-frontend/src/pages/settings/security.page.tsx +++ b/apps/hash-frontend/src/pages/settings/security.page.tsx @@ -80,7 +80,7 @@ const extractBackupCodesFromFlow = (flow: SettingsFlow): string[] => { // The data comes from Kratos in any case. const withNewlines = codesText.replace(//gi, "\n"); const parsed = new DOMParser().parseFromString(withNewlines, "text/html"); - const plainText = parsed.body.textContent ?? ""; + const plainText = parsed.body.textContent; return plainText .split("\n") @@ -153,7 +153,7 @@ const SecurityPage: NextPageWithLayout = () => { } setErrorMessage( - error.response?.data?.ui?.messages?.[0]?.text ?? + error.response?.data.ui.messages?.[0]?.text ?? "Something went wrong. Please try again.", ); return undefined; diff --git a/apps/plugin-browser/src/scripts/background/infer-entities.ts b/apps/plugin-browser/src/scripts/background/infer-entities.ts index 84e5c0927a5..fcc3e80f78f 100644 --- a/apps/plugin-browser/src/scripts/background/infer-entities.ts +++ b/apps/plugin-browser/src/scripts/background/infer-entities.ts @@ -167,7 +167,9 @@ const createWebSocket = async ({ onClose }: { onClose: () => void }) => { try { message = JSON.parse(event.data) as InferenceWebsocketServerMessage; } catch { - const sanitizedEventData = String(event.data).replace(/[\r\n]/g, " "); + const sanitizedEventData = String(event.data) + .replace(/[\u0000-\u001F\u007F\u2028\u2029]/g, " ") + .slice(0, 1_000); console.error("Malformed WebSocket message:", sanitizedEventData); return; } diff --git a/tests/hash-playwright/tests/shared/change-sidebar-list-display.ts b/tests/hash-playwright/tests/shared/change-sidebar-list-display.ts index 17eac1e5c33..db6579ef65b 100644 --- a/tests/hash-playwright/tests/shared/change-sidebar-list-display.ts +++ b/tests/hash-playwright/tests/shared/change-sidebar-list-display.ts @@ -1,9 +1,10 @@ import { expect, type Page } from "@playwright/test"; /** - * Changes the user's settings for how entities or types should be displayed in the sidebar. - * - * Only takes action if the desired display setting is different from the current one. + * Changes the user's settings for how entities or types should be + * displayed in the sidebar, and waits for the sidebar to reflect the + * change. In list mode the section appears as an uppercase expandable + * header ("ENTITIES >"); in link mode it appears as a regular nav link. */ export const changeSidebarListDisplay = async ({ displayAs, @@ -29,4 +30,14 @@ export const changeSidebarListDisplay = async ({ ) { await page.getByLabel(switchLabel).check(); } + + // The sidebar updates asynchronously after the toggle. In list mode + // the section renders as an uppercase expandable header (e.g. + // "ENTITIES >"); in link mode it renders as a plain nav link. + const sectionUpper = section.toUpperCase(); + if (displayAs === "list") { + await expect(page.locator(`text=${sectionUpper}`)).toBeVisible(); + } else { + await expect(page.locator(`text=${sectionUpper}`)).toBeHidden(); + } }; From 28d843734ccddb9db1e46fa8f8d00460a0900485 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Thu, 16 Apr 2026 16:23:15 +0200 Subject: [PATCH 08/14] Add charlie@example.com back to test allowlist The backend-integration test `user.test.ts` creates an incomplete user with `charlie@example.com`. The allowlist was rewritten in the test reorganisation commit but omitted this email, causing the shortname-update test to fail with FORBIDDEN. --- .env.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.env.test b/.env.test index a3ea91352f8..494d7499b8a 100644 --- a/.env.test +++ b/.env.test @@ -13,4 +13,4 @@ AWS_S3_UPLOADS_SECRET_ACCESS_KEY="dev-s3-secret-access-key" AWS_S3_UPLOADS_FORCE_PATH_STYLE=true FILE_UPLOAD_PROVIDER="AWS_S3" -USER_EMAIL_ALLOW_LIST='["signup-allow@example.com", "signin-test@example.com", "signout-test@example.com", "pw-change@example.com", "pw-recovery@example.com", "mfa-enable@example.com", "mfa-login@example.com", "mfa-backup@example.com", "mfa-disable@example.com", "mfa-wrong-code@example.com"]' +USER_EMAIL_ALLOW_LIST='["charlie@example.com", "signup-allow@example.com", "signin-test@example.com", "signout-test@example.com", "pw-change@example.com", "pw-recovery@example.com", "mfa-enable@example.com", "mfa-login@example.com", "mfa-backup@example.com", "mfa-disable@example.com", "mfa-wrong-code@example.com"]' From c9d3fa1ae9b37fcb486d789368443670f41c5cdb Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Thu, 16 Apr 2026 18:45:54 +0200 Subject: [PATCH 09/14] Fix sidebar locator races, remove sleep, drop log injection vector - Scope sidebar assertions to `page-sidebar` testid to avoid matching settings-page labels (e.g. "Entities as a") - Use `getByText(section, { exact: true })` for list mode, `getByRole("link")` for link mode instead of case-insensitive `text=` locator - Remove `sleep(5_000)` and unused import from entity-type-creation test - Wait for create-entity-type button visibility before clicking - Remove event.data from WebSocket error log (CodeQL log injection + ESLint no-control-regex --- .../src/scripts/background/infer-entities.ts | 5 +---- .../tests/features/entities-page.spec.ts | 13 ++++++------- .../features/entity-type-creation.spec.ts | 14 +++++++------- .../shared/change-sidebar-list-display.ts | 18 ++++++++++-------- 4 files changed, 24 insertions(+), 26 deletions(-) diff --git a/apps/plugin-browser/src/scripts/background/infer-entities.ts b/apps/plugin-browser/src/scripts/background/infer-entities.ts index fcc3e80f78f..005dcc475fa 100644 --- a/apps/plugin-browser/src/scripts/background/infer-entities.ts +++ b/apps/plugin-browser/src/scripts/background/infer-entities.ts @@ -167,10 +167,7 @@ const createWebSocket = async ({ onClose }: { onClose: () => void }) => { try { message = JSON.parse(event.data) as InferenceWebsocketServerMessage; } catch { - const sanitizedEventData = String(event.data) - .replace(/[\u0000-\u001F\u007F\u2028\u2029]/g, " ") - .slice(0, 1_000); - console.error("Malformed WebSocket message:", sanitizedEventData); + console.error("Malformed WebSocket message"); return; } diff --git a/tests/hash-playwright/tests/features/entities-page.spec.ts b/tests/hash-playwright/tests/features/entities-page.spec.ts index e41e5ea5d60..4f591f74f74 100644 --- a/tests/hash-playwright/tests/features/entities-page.spec.ts +++ b/tests/hash-playwright/tests/features/entities-page.spec.ts @@ -3,24 +3,24 @@ import { expect, test } from "../shared/runtime"; test("user can visit a page listing entities of a type", async ({ page }) => { await page.goto("/"); - // Check if we are on the logged-in homepage await expect(page.locator("text=Get support")).toBeVisible(); - // Enable the full list display for 'Entities' in the sidebar await changeSidebarListDisplay({ displayAs: "list", page, section: "Entities", }); - // Expand the entities list in the sidebar and wait for types to load - await page.locator("text=Entities").first().click(); + const sidebar = page.getByTestId("page-sidebar"); - const documentItem = page.locator("text=Document").first(); + // Expand the entities list in the sidebar + await sidebar.getByText("Entities", { exact: true }).click(); + + // Wait for entity types to load inside the expanded section + const documentItem = sidebar.getByText("Document", { exact: true }); await expect(documentItem).toBeVisible({ timeout: 10_000 }); await documentItem.click(); - // Check if we are on the 'Document' entities page await page.waitForURL((url) => { return ( url.pathname === "/entities" && @@ -29,6 +29,5 @@ test("user can visit a page listing entities of a type", async ({ page }) => { ); }); - // Confirm that a non-zero number of Document entities are listed for the user await expect(page.getByText(/^([1-9]\d*) in your webs$/)).toBeVisible(); }); diff --git a/tests/hash-playwright/tests/features/entity-type-creation.spec.ts b/tests/hash-playwright/tests/features/entity-type-creation.spec.ts index f518a05b712..67af8f50401 100644 --- a/tests/hash-playwright/tests/features/entity-type-creation.spec.ts +++ b/tests/hash-playwright/tests/features/entity-type-creation.spec.ts @@ -1,5 +1,3 @@ -import { sleep } from "@local/hash-isomorphic-utils/sleep"; - import { changeSidebarListDisplay } from "../shared/change-sidebar-list-display"; import { expect, test } from "../shared/runtime"; @@ -15,13 +13,17 @@ test("user can create entity type", async ({ page }) => { section: "Types", }); + const sidebar = page.getByTestId("page-sidebar"); + // Expand the Types section so the "create entity type" button is in the // DOM — the button is only rendered as an end-adornment on the Types // list header when the section is open. - await page.locator("text=Types").first().click(); + await sidebar.getByText("Types", { exact: true }).click(); - // Go to Create Entity Type - await page.locator('[data-testid="create-entity-type-btn"]').click(); + // Wait for the button to appear after the section expands, then click + const createBtn = sidebar.getByTestId("create-entity-type-btn"); + await expect(createBtn).toBeVisible(); + await createBtn.click(); await page.waitForURL( (url) => !!url.pathname.match(/^\/new\/types\/entity-type/), ); @@ -76,8 +78,6 @@ test("user can create entity type", async ({ page }) => { await page.click('[data-testid="editbar-confirm"]'); - await sleep(5_000); - await page.waitForURL( (url) => !!url.pathname.endsWith(entityTypeName.toLowerCase()), ); diff --git a/tests/hash-playwright/tests/shared/change-sidebar-list-display.ts b/tests/hash-playwright/tests/shared/change-sidebar-list-display.ts index db6579ef65b..60f55792ebc 100644 --- a/tests/hash-playwright/tests/shared/change-sidebar-list-display.ts +++ b/tests/hash-playwright/tests/shared/change-sidebar-list-display.ts @@ -3,8 +3,8 @@ import { expect, type Page } from "@playwright/test"; /** * Changes the user's settings for how entities or types should be * displayed in the sidebar, and waits for the sidebar to reflect the - * change. In list mode the section appears as an uppercase expandable - * header ("ENTITIES >"); in link mode it appears as a regular nav link. + * change. In list mode the section appears as an expandable header + * (non-link); in link mode it appears as a regular nav link. */ export const changeSidebarListDisplay = async ({ displayAs, @@ -31,13 +31,15 @@ export const changeSidebarListDisplay = async ({ await page.getByLabel(switchLabel).check(); } - // The sidebar updates asynchronously after the toggle. In list mode - // the section renders as an uppercase expandable header (e.g. - // "ENTITIES >"); in link mode it renders as a plain nav link. - const sectionUpper = section.toUpperCase(); + // The sidebar updates asynchronously after the toggle. Scope + // assertions to the sidebar to avoid matching settings-page content + // (e.g. the "Entities as a" label on the personalization page). + const sidebar = page.getByTestId("page-sidebar"); if (displayAs === "list") { - await expect(page.locator(`text=${sectionUpper}`)).toBeVisible(); + // In list mode the section renders as a non-link expandable header. + await expect(sidebar.getByText(section, { exact: true })).toBeVisible(); } else { - await expect(page.locator(`text=${sectionUpper}`)).toBeHidden(); + // In link mode the section renders as a plain nav link. + await expect(sidebar.getByRole("link", { name: section })).toBeVisible(); } }; From e4c08b7c539e787963a4f8bb1459efb420ac512e Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 17 Apr 2026 00:18:13 +0200 Subject: [PATCH 10/14] Fix sidebar test races, add bob user for parallel isolation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Wait for updateEntity GraphQL response after toggling sidebar preference to prevent concurrent-update errors (FE-600) - Add expandSidebarSection helper that checks MuiCollapse state before clicking, avoiding double-toggle - Sign in bob during globalSetup so entity-type-creation runs as bob while entities-page runs as alice — no shared entity conflicts - Remove withTestUser wrapper, callers use createUserAndCompleteSignup directly - Remove event.data from WebSocket error log (CodeQL + ESLint) --- tests/hash-playwright/global-setup.ts | 46 +++++++--- .../hash-playwright/tests/account/mfa.spec.ts | 25 +++-- .../tests/account/password.spec.ts | 13 ++- .../tests/account/signin.spec.ts | 10 +- .../tests/features/entities-page.spec.ts | 14 +-- .../features/entity-type-creation.spec.ts | 22 ++--- .../shared/change-sidebar-list-display.ts | 91 +++++++++++++++---- .../tests/shared/test-users.ts | 15 --- 8 files changed, 159 insertions(+), 77 deletions(-) diff --git a/tests/hash-playwright/global-setup.ts b/tests/hash-playwright/global-setup.ts index 1504c1c3b5b..7035e70eafa 100644 --- a/tests/hash-playwright/global-setup.ts +++ b/tests/hash-playwright/global-setup.ts @@ -2,28 +2,46 @@ import { chromium } from "@playwright/test"; const baseURL = "http://localhost:3000"; +const signInAndSaveState = async ( + browser: Awaited>, + email: string, + statePath: string, +) => { + const context = await browser.newContext({ baseURL }); + const page = await context.newPage(); + + await page.goto("/signin"); + await page.fill('[placeholder="Enter your email address"]', email); + await page.fill('[type="password"]', "password"); + await page.click("text=Submit"); + await page.waitForURL("/", { timeout: 30_000 }); + + await context.storageState({ path: statePath }); + await context.close(); +}; + /** - * Sign in the pre-seeded `alice` user once before the suite runs and - * persist her session to `tests/.auth/alice.json`. Feature tests that - * need an authenticated user load this `storageState` instead of + * Sign in the pre-seeded users once before the suite runs and persist + * their sessions. Feature tests load a `storageState` instead of * repeating the sign-in flow. + * + * Multiple users are needed so that tests which mutate user state + * (e.g. sidebar preferences) can run in parallel without conflicting + * on the same entity. */ export default async function globalSetup() { const browser = await chromium.launch(); try { - const context = await browser.newContext({ baseURL }); - const page = await context.newPage(); - - await page.goto("/signin"); - await page.fill( - '[placeholder="Enter your email address"]', + await signInAndSaveState( + browser, "alice@example.com", + "tests/.auth/alice.json", + ); + await signInAndSaveState( + browser, + "bob@example.com", + "tests/.auth/bob.json", ); - await page.fill('[type="password"]', "password"); - await page.click("text=Submit"); - await page.waitForURL("/", { timeout: 30_000 }); - - await context.storageState({ path: "tests/.auth/alice.json" }); } finally { await browser.close(); } diff --git a/tests/hash-playwright/tests/account/mfa.spec.ts b/tests/hash-playwright/tests/account/mfa.spec.ts index d91f778aec2..9f64d30000e 100644 --- a/tests/hash-playwright/tests/account/mfa.spec.ts +++ b/tests/hash-playwright/tests/account/mfa.spec.ts @@ -3,7 +3,8 @@ import { clearSessionCookies, signInWithPassword, } from "../shared/signin-utils"; -import { testUsers, withTestUser } from "../shared/test-users"; +import { createUserAndCompleteSignup } from "../shared/signup-utils"; +import { testUsers } from "../shared/test-users"; import { generateTotpCode, waitForFreshTotpWindow } from "../shared/totp-utils"; const enableTotpForCurrentUser = async (page: Page) => { @@ -46,7 +47,7 @@ const enableTotpForCurrentUser = async (page: Page) => { }; test("user can enable TOTP", async ({ page }) => { - await withTestUser(page, testUsers.mfaEnable); + await createUserAndCompleteSignup(page, testUsers.mfaEnable); const { backupCodes } = await enableTotpForCurrentUser(page); @@ -54,7 +55,10 @@ test("user can enable TOTP", async ({ page }) => { }); test("user with TOTP is prompted for code at login", async ({ page }) => { - const credentials = await withTestUser(page, testUsers.mfaLogin); + const credentials = await createUserAndCompleteSignup( + page, + testUsers.mfaLogin, + ); const { secret } = await enableTotpForCurrentUser(page); await clearSessionCookies(page); @@ -76,7 +80,10 @@ test("user with TOTP is prompted for code at login", async ({ page }) => { }); test("user can use backup code instead of TOTP", async ({ page }) => { - const credentials = await withTestUser(page, testUsers.mfaBackup); + const credentials = await createUserAndCompleteSignup( + page, + testUsers.mfaBackup, + ); const { backupCodes } = await enableTotpForCurrentUser(page); expect(backupCodes.length).toBeGreaterThan(0); @@ -96,7 +103,10 @@ test("user can use backup code instead of TOTP", async ({ page }) => { }); test("user can disable TOTP", async ({ page }) => { - const credentials = await withTestUser(page, testUsers.mfaDisable); + const credentials = await createUserAndCompleteSignup( + page, + testUsers.mfaDisable, + ); await enableTotpForCurrentUser(page); await page.goto("/settings/security"); @@ -127,7 +137,10 @@ test("user can disable TOTP", async ({ page }) => { }); test("wrong TOTP code shows error at login", async ({ page }) => { - const credentials = await withTestUser(page, testUsers.mfaWrongCode); + const credentials = await createUserAndCompleteSignup( + page, + testUsers.mfaWrongCode, + ); await enableTotpForCurrentUser(page); await clearSessionCookies(page); diff --git a/tests/hash-playwright/tests/account/password.spec.ts b/tests/hash-playwright/tests/account/password.spec.ts index 572a4ab14de..6530d4d445a 100644 --- a/tests/hash-playwright/tests/account/password.spec.ts +++ b/tests/hash-playwright/tests/account/password.spec.ts @@ -5,11 +5,15 @@ import { expectSignedIn, signInWithPassword, } from "../shared/signin-utils"; -import { testUsers, withTestUser } from "../shared/test-users"; +import { createUserAndCompleteSignup } from "../shared/signup-utils"; +import { testUsers } from "../shared/test-users"; test("user can change password from settings", async ({ page }) => { const newPassword = "changed-pw-5ef6"; - const credentials = await withTestUser(page, testUsers.pwChange); + const credentials = await createUserAndCompleteSignup( + page, + testUsers.pwChange, + ); await page.goto("/settings/security", { waitUntil: "networkidle" }); @@ -30,7 +34,10 @@ test("user can change password from settings", async ({ page }) => { test("user can recover account and set a new password", async ({ page }) => { const newPassword = "recovered-pw-3cd4"; - const credentials = await withTestUser(page, testUsers.pwRecovery); + const credentials = await createUserAndCompleteSignup( + page, + testUsers.pwRecovery, + ); await clearSessionCookies(page); diff --git a/tests/hash-playwright/tests/account/signin.spec.ts b/tests/hash-playwright/tests/account/signin.spec.ts index dcdf74db816..31159ba56ed 100644 --- a/tests/hash-playwright/tests/account/signin.spec.ts +++ b/tests/hash-playwright/tests/account/signin.spec.ts @@ -5,10 +5,14 @@ import { expectSignedOut, signInWithPassword, } from "../shared/signin-utils"; -import { testUsers, withTestUser } from "../shared/test-users"; +import { createUserAndCompleteSignup } from "../shared/signup-utils"; +import { testUsers } from "../shared/test-users"; test("user can sign in with password", async ({ page }) => { - const credentials = await withTestUser(page, testUsers.signinTest); + const credentials = await createUserAndCompleteSignup( + page, + testUsers.signinTest, + ); await clearSessionCookies(page); await expectSignedOut(page); @@ -18,7 +22,7 @@ test("user can sign in with password", async ({ page }) => { }); test("user can sign out via account menu", async ({ page }) => { - await withTestUser(page, testUsers.signoutTest); + await createUserAndCompleteSignup(page, testUsers.signoutTest); // Use the real sign-out flow (not clearCookies) await page.getByTestId("user-avatar").click(); diff --git a/tests/hash-playwright/tests/features/entities-page.spec.ts b/tests/hash-playwright/tests/features/entities-page.spec.ts index 4f591f74f74..7b0360a82bd 100644 --- a/tests/hash-playwright/tests/features/entities-page.spec.ts +++ b/tests/hash-playwright/tests/features/entities-page.spec.ts @@ -1,4 +1,7 @@ -import { changeSidebarListDisplay } from "../shared/change-sidebar-list-display"; +import { + changeSidebarListDisplay, + expandSidebarSection, +} from "../shared/change-sidebar-list-display"; import { expect, test } from "../shared/runtime"; test("user can visit a page listing entities of a type", async ({ page }) => { @@ -11,14 +14,11 @@ test("user can visit a page listing entities of a type", async ({ page }) => { section: "Entities", }); - const sidebar = page.getByTestId("page-sidebar"); - - // Expand the entities list in the sidebar - await sidebar.getByText("Entities", { exact: true }).click(); + await expandSidebarSection({ page, section: "Entities" }); - // Wait for entity types to load inside the expanded section + const sidebar = page.getByTestId("page-sidebar"); const documentItem = sidebar.getByText("Document", { exact: true }); - await expect(documentItem).toBeVisible({ timeout: 10_000 }); + await expect(documentItem).toBeVisible(); await documentItem.click(); await page.waitForURL((url) => { diff --git a/tests/hash-playwright/tests/features/entity-type-creation.spec.ts b/tests/hash-playwright/tests/features/entity-type-creation.spec.ts index 67af8f50401..484a61d7255 100644 --- a/tests/hash-playwright/tests/features/entity-type-creation.spec.ts +++ b/tests/hash-playwright/tests/features/entity-type-creation.spec.ts @@ -1,26 +1,26 @@ -import { changeSidebarListDisplay } from "../shared/change-sidebar-list-display"; +import { + changeSidebarListDisplay, + expandSidebarSection, +} from "../shared/change-sidebar-list-display"; import { expect, test } from "../shared/runtime"; +// Use bob so that sidebar-preference mutations don't conflict with +// entities-page.spec.ts (which runs as alice) on the same entity. +test.use({ storageState: "tests/.auth/bob.json" }); + test("user can create entity type", async ({ page }) => { await page.goto("/"); - // Check if we are on the user page await expect(page.locator("text=Get support")).toBeVisible(); - // Enable the full list display for 'Types' in the sidebar await changeSidebarListDisplay({ displayAs: "list", page, section: "Types", }); - const sidebar = page.getByTestId("page-sidebar"); - - // Expand the Types section so the "create entity type" button is in the - // DOM — the button is only rendered as an end-adornment on the Types - // list header when the section is open. - await sidebar.getByText("Types", { exact: true }).click(); + await expandSidebarSection({ page, section: "Types" }); - // Wait for the button to appear after the section expands, then click + const sidebar = page.getByTestId("page-sidebar"); const createBtn = sidebar.getByTestId("create-entity-type-btn"); await expect(createBtn).toBeVisible(); await createBtn.click(); @@ -49,7 +49,7 @@ test("user can create entity type", async ({ page }) => { await page.click("[data-testid=entity-type-creation-form] button"); await page.waitForURL( (url) => - !!url.pathname.match(/^\/@alice\/types\/entity-type\/testentity/) && + !!url.pathname.match(/^\/@bob01\/types\/entity-type\/testentity/) && url.searchParams.has("draft"), ); diff --git a/tests/hash-playwright/tests/shared/change-sidebar-list-display.ts b/tests/hash-playwright/tests/shared/change-sidebar-list-display.ts index 60f55792ebc..f0198829683 100644 --- a/tests/hash-playwright/tests/shared/change-sidebar-list-display.ts +++ b/tests/hash-playwright/tests/shared/change-sidebar-list-display.ts @@ -1,10 +1,21 @@ import { expect, type Page } from "@playwright/test"; +const waitForUpdateEntity = (page: Page) => + page.waitForResponse( + (res) => + res.request().method() === "POST" && + res.url().includes("/graphql") && + res.request().postData()?.includes("updateEntity") === true, + ); + /** * Changes the user's settings for how entities or types should be - * displayed in the sidebar, and waits for the sidebar to reflect the - * change. In list mode the section appears as an expandable header - * (non-link); in link mode it appears as a regular nav link. + * displayed in the sidebar, waits for the sidebar to reflect the + * change, and — in list mode — expands the section. + * + * Each step waits for its `updateEntity` PATCH to complete before + * proceeding to avoid concurrent-update errors on the user entity + * (see FE-600). */ export const changeSidebarListDisplay = async ({ displayAs, @@ -19,27 +30,71 @@ export const changeSidebarListDisplay = async ({ await page.getByRole("link", { name: "Settings" }).click(); await page.getByRole("link", { name: "Personalization" }).click(); - const switchLabel = `${section} as a`; + const toggle = page.getByLabel(`${section} as a`); + await expect(toggle).toBeVisible(); + + const alreadyCorrect = + displayAs === "list" + ? await toggle.isChecked() + : !(await toggle.isChecked()); + + if (!alreadyCorrect) { + const patchDone = waitForUpdateEntity(page); - await expect(page.getByLabel(switchLabel)).toBeVisible(); + if (displayAs === "list") { + await toggle.check(); + } else { + await toggle.uncheck(); + } - if ( - (displayAs === "link" && - (await page.getByLabel(switchLabel).isChecked())) || - (displayAs === "list" && !(await page.getByLabel(switchLabel).isChecked())) - ) { - await page.getByLabel(switchLabel).check(); + await patchDone; } - // The sidebar updates asynchronously after the toggle. Scope - // assertions to the sidebar to avoid matching settings-page content - // (e.g. the "Entities as a" label on the personalization page). const sidebar = page.getByTestId("page-sidebar"); + const sidebarLink = sidebar.getByRole("link", { + name: section, + exact: true, + }); + if (displayAs === "list") { - // In list mode the section renders as a non-link expandable header. - await expect(sidebar.getByText(section, { exact: true })).toBeVisible(); + await expect(sidebarLink).toBeHidden(); } else { - // In link mode the section renders as a plain nav link. - await expect(sidebar.getByRole("link", { name: section })).toBeVisible(); + await expect(sidebarLink).toBeVisible(); + } +}; + +/** + * Ensure a sidebar section is expanded. If the section is collapsed + * (`MuiCollapse-hidden`), clicks the header and waits for the + * `updateEntity` PATCH to complete (FE-600). + * + * The section must already be in list mode (i.e. the NavLink header + * exists in the DOM). + */ +export const expandSidebarSection = async ({ + page, + section, +}: { + page: Page; + section: "Entities" | "Types"; +}) => { + const sidebar = page.getByTestId("page-sidebar"); + + // The NavLink renders:
…header…
+ // The Collapse's previousElementSibling contains the section header text. + const isCollapsed = await sidebar.evaluate((sidebarEl, sectionName) => { + const collapse = Array.from( + sidebarEl.querySelectorAll(".MuiCollapse-root"), + ).find((col) => { + const text = col.previousElementSibling?.textContent; + return text != null && text.trim().startsWith(sectionName); + }); + return collapse?.classList.contains("MuiCollapse-hidden") ?? false; + }, section); + + if (isCollapsed) { + const expandDone = waitForUpdateEntity(page); + await sidebar.getByText(section, { exact: true }).click(); + await expandDone; } }; diff --git a/tests/hash-playwright/tests/shared/test-users.ts b/tests/hash-playwright/tests/shared/test-users.ts index 00aa4bb71f4..d5d943797f1 100644 --- a/tests/hash-playwright/tests/shared/test-users.ts +++ b/tests/hash-playwright/tests/shared/test-users.ts @@ -1,8 +1,3 @@ -import type { Page } from "@playwright/test"; - -import { deleteUserByEmail } from "./delete-user"; -import { createUserAndCompleteSignup } from "./signup-utils"; - export const defaultPassword = "test-pw-1ab2"; interface TestUser { @@ -58,13 +53,3 @@ export const testUsers = { "Signup Allow", ), } as const; - -/** - * Clean up any leftover Kratos identity, then create and complete signup - * for the given test user. Returns the user's credentials. - */ -export const withTestUser = async (page: Page, testUser: TestUser) => { - await deleteUserByEmail(testUser.email); - await createUserAndCompleteSignup(page, testUser); - return { email: testUser.email, password: testUser.password }; -}; From 7fcb25523b28d5b3df8ce951b7b13c334502f753 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 17 Apr 2026 11:38:33 +0200 Subject: [PATCH 11/14] Clean up cookie helpers in browser extension - Rename getSessionCookies/hasSessionCookies/getCookieString to getApiOriginUrl/isLoggedIn/buildWebsocketCookieString - isLoggedIn only checks for ory_kratos_session cookie instead of requiring both CSRF and session (CSRF is only needed for websocket) - Extract shared getApiOriginUrl helper --- .../src/scripts/background/infer-entities.ts | 51 +++++++++---------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/apps/plugin-browser/src/scripts/background/infer-entities.ts b/apps/plugin-browser/src/scripts/background/infer-entities.ts index 005dcc475fa..5f6cad7790a 100644 --- a/apps/plugin-browser/src/scripts/background/infer-entities.ts +++ b/apps/plugin-browser/src/scripts/background/infer-entities.ts @@ -31,35 +31,34 @@ const setExternalInputRequestsValue = getSetFromLocalStorageValue( "externalInputRequests", ); -const getSessionCookies = async () => { +const getApiOriginUrl = async () => { const apiOrigin = await getFromLocalStorage("apiOrigin"); - - return browser.cookies - .getAll({ - url: apiOrigin ?? API_ORIGIN, - }) - .then((options) => - options.filter( - (option) => - option.name.startsWith("csrf_token_") || - option.name === "ory_kratos_session", - ), - ); + return apiOrigin ?? API_ORIGIN; }; -const hasSessionCookies = async (): Promise => { - const cookies = await getSessionCookies(); - return cookies.length >= 2; +const isLoggedIn = async (): Promise => { + const cookies = await browser.cookies.getAll({ + url: await getApiOriginUrl(), + name: "ory_kratos_session", + }); + return cookies.length > 0; }; -const getCookieString = async () => { - const cookies = await getSessionCookies(); +/** Build the cookie header for WebSocket requests (needs both CSRF and session). */ +const buildWebsocketCookieString = async () => { + const url = await getApiOriginUrl(); + const allCookies = await browser.cookies.getAll({ url }); + const relevant = allCookies.filter( + (cookie) => + cookie.name.startsWith("csrf_token_") || + cookie.name === "ory_kratos_session", + ); - if (cookies.length < 2) { + if (relevant.length < 2) { throw new Error("No session cookies available to use in websocket request"); } - return cookies.map((cookie) => `${cookie.name}=${cookie.value}`).join(";"); + return relevant.map((cookie) => `${cookie.name}=${cookie.value}`).join(";"); }; const maxConcurrentRequests = 3; @@ -124,7 +123,7 @@ const createWebSocket = async ({ onClose }: { onClose: () => void }) => { const newWs = new WebSocket(websocketUrl); const externalRequestPoll = setInterval(() => { - void getCookieString() + void buildWebsocketCookieString() .then((cookie) => newWs.send( JSON.stringify({ @@ -199,7 +198,7 @@ const createWebSocket = async ({ onClose }: { onClose: () => void }) => { enqueueTask(async () => { const webPages = await getWebsiteContent(payload.data.urls); - const cookie = await getCookieString(); + const cookie = await buildWebsocketCookieString(); newWs.send( JSON.stringify({ @@ -239,7 +238,7 @@ const reconnectWebSocket = async () => { reconnecting = true; try { - if (!(await hasSessionCookies())) { + if (!(await isLoggedIn())) { // User isn't logged in — don't open a doomed connection that the // server will kill after 5 s. Retry later; a user action like // opening the popup will trigger getWebSocket() once cookies exist. @@ -286,7 +285,7 @@ const sendInferEntitiesMessage = async ( ) => { const socket = await getWebSocket(); - const cookie = await getCookieString(); + const cookie = await buildWebsocketCookieString(); socket.send( JSON.stringify({ @@ -303,7 +302,7 @@ export const cancelInferEntities = async ({ }: { flowRunId: string; }) => { - const cookie = await getCookieString(); + const cookie = await buildWebsocketCookieString(); const socket = await getWebSocket(); @@ -424,7 +423,7 @@ export const inferEntities = async ( * connections that the server will immediately kill. */ const init = async () => { - if (await hasSessionCookies()) { + if (await isLoggedIn()) { void getWebSocket(); } // If no cookies, the WebSocket will be created on demand when From b314d58de78aeecffa5711cdebc6628d3f3b0483 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 17 Apr 2026 11:52:48 +0200 Subject: [PATCH 12/14] Address PR review feedback - Guard against non-SettingsFlow response body in security.page.tsx error handler with optional chaining - Wrap recovery code polling in try/catch to survive transient fetch errors, matching the verification code helper - Create tests/.auth/ directory in globalSetup for clean checkouts --- .../src/pages/settings/security.page.tsx | 3 +- tests/hash-playwright/global-setup.ts | 4 ++ .../shared/get-kratos-verification-code.ts | 68 ++++++++++--------- 3 files changed, 42 insertions(+), 33 deletions(-) diff --git a/apps/hash-frontend/src/pages/settings/security.page.tsx b/apps/hash-frontend/src/pages/settings/security.page.tsx index fd251f3bad0..6ef8d869932 100644 --- a/apps/hash-frontend/src/pages/settings/security.page.tsx +++ b/apps/hash-frontend/src/pages/settings/security.page.tsx @@ -153,7 +153,8 @@ const SecurityPage: NextPageWithLayout = () => { } setErrorMessage( - error.response?.data.ui.messages?.[0]?.text ?? + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- response body is typed as SettingsFlow but non-400 responses have a different shape + error.response?.data?.ui?.messages?.[0]?.text ?? "Something went wrong. Please try again.", ); return undefined; diff --git a/tests/hash-playwright/global-setup.ts b/tests/hash-playwright/global-setup.ts index 7035e70eafa..60ff553e083 100644 --- a/tests/hash-playwright/global-setup.ts +++ b/tests/hash-playwright/global-setup.ts @@ -1,3 +1,5 @@ +import { mkdirSync } from "node:fs"; + import { chromium } from "@playwright/test"; const baseURL = "http://localhost:3000"; @@ -30,6 +32,8 @@ const signInAndSaveState = async ( * on the same entity. */ export default async function globalSetup() { + mkdirSync("tests/.auth", { recursive: true }); + const browser = await chromium.launch(); try { await signInAndSaveState( diff --git a/tests/hash-playwright/tests/shared/get-kratos-verification-code.ts b/tests/hash-playwright/tests/shared/get-kratos-verification-code.ts index d242149b7b6..6a11900aab5 100644 --- a/tests/hash-playwright/tests/shared/get-kratos-verification-code.ts +++ b/tests/hash-playwright/tests/shared/get-kratos-verification-code.ts @@ -177,41 +177,45 @@ export const getKratosRecoveryCode = async ( let elapsed = 0; while (elapsed < maxWaitMs) { - const response = await fetch("http://localhost:4437/mail"); - - if (!response.ok) { - throw new Error( - `Unable to fetch emails from mailslurper: ${response.status} ${response.statusText}`, - ); - } + try { + const response = await fetch("http://localhost:4437/mail"); - const data = (await response.json()) as { - mailItems?: MailslurperMailItem[]; - }; - - const match = data.mailItems - ?.filter((mailItem) => { - const sentTimestamp = parseMailslurperDate(mailItem.dateSent); - return ( - isRecoverySubject(mailItem.subject) && - extractToAddresses(mailItem.toAddresses).includes(emailAddress) && - (!afterTimestamp || - (typeof sentTimestamp === "number" && - sentTimestamp >= afterTimestamp - timestampBufferMs)) + if (!response.ok) { + throw new Error( + `Unable to fetch emails from mailslurper: ${response.status} ${response.statusText}`, ); - }) - .sort((a, b) => { - const aTs = parseMailslurperDate(a.dateSent) ?? 0; - const bTs = parseMailslurperDate(b.dateSent) ?? 0; - return bTs - aTs; - }) - .at(0); - - if (match?.body) { - const code = match.body.match(/\b(\d{6})\b/)?.[1]; - if (code) { - return code; } + + const data = (await response.json()) as { + mailItems?: MailslurperMailItem[]; + }; + + const match = data.mailItems + ?.filter((mailItem) => { + const sentTimestamp = parseMailslurperDate(mailItem.dateSent); + return ( + isRecoverySubject(mailItem.subject) && + extractToAddresses(mailItem.toAddresses).includes(emailAddress) && + (!afterTimestamp || + (typeof sentTimestamp === "number" && + sentTimestamp >= afterTimestamp - timestampBufferMs)) + ); + }) + .sort((a, b) => { + const aTs = parseMailslurperDate(a.dateSent) ?? 0; + const bTs = parseMailslurperDate(b.dateSent) ?? 0; + return bTs - aTs; + }) + .at(0); + + if (match?.body) { + const code = match.body.match(/\b(\d{6})\b/)?.[1]; + if (code) { + return code; + } + } + } catch { + // Transient error — retry on next poll iteration. } await sleep(pollIntervalMs); From 0c19e32cd0389701e0dbc5d4b84291d1e7e36738 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 17 Apr 2026 12:29:36 +0200 Subject: [PATCH 13/14] Extract shared pollForKratosCode helper for email code polling Both getKratosVerificationCode and getKratosRecoveryCode duplicated the same mailslurper polling, timestamp filtering, and retry logic. The recovery variant was missing the diagnostic summary on failure. Extract a parameterized pollForKratosCode that takes subject filter, code extractor, and email type label. Both callers are now thin wrappers with equal diagnostics on timeout. --- .../shared/get-kratos-verification-code.ts | 113 +++++++----------- 1 file changed, 44 insertions(+), 69 deletions(-) diff --git a/tests/hash-playwright/tests/shared/get-kratos-verification-code.ts b/tests/hash-playwright/tests/shared/get-kratos-verification-code.ts index 6a11900aab5..c172d0c5266 100644 --- a/tests/hash-playwright/tests/shared/get-kratos-verification-code.ts +++ b/tests/hash-playwright/tests/shared/get-kratos-verification-code.ts @@ -63,10 +63,23 @@ const isVerificationSubject = (subject?: string): boolean => { const isRecoverySubject = (subject?: string): boolean => typeof subject === "string" && subject.startsWith("Your HASH recovery code:"); -export const getKratosVerificationCode = async ( - emailAddress: string, - afterTimestamp?: number, -): Promise => { +/** + * Poll mailslurper for a code email matching the given criteria. + * Returns the 6-digit code on success, throws with diagnostics on timeout. + */ +const pollForKratosCode = async ({ + emailAddress, + afterTimestamp, + subjectFilter, + extractCode, + emailType, +}: { + emailAddress: string; + afterTimestamp?: number; + subjectFilter: (subject?: string) => boolean; + extractCode: (body: string) => string | undefined; + emailType: string; +}): Promise => { const maxWaitMs = 10_000; const pollIntervalMs = 250; const timestampBufferMs = 5_000; @@ -96,7 +109,7 @@ export const getKratosVerificationCode = async ( const sentTimestamp = parseMailslurperDate(mailItem.dateSent); return ( - isVerificationSubject(mailItem.subject) && + subjectFilter(mailItem.subject) && extractToAddresses(mailItem.toAddresses).includes(emailAddress) && (!afterTimestamp || (typeof sentTimestamp === "number" && @@ -111,9 +124,7 @@ export const getKratosVerificationCode = async ( }) ?? []; for (const mailItem of matchingMailItems) { - const code = mailItem.body - ? extractVerificationCode(mailItem.body) - : undefined; + const code = mailItem.body ? extractCode(mailItem.body) : undefined; if (code) { return code; @@ -134,12 +145,12 @@ export const getKratosVerificationCode = async ( const toTargetAddress = allItems.filter((item) => extractToAddresses(item.toAddresses).includes(emailAddress), ); - const verificationToTarget = toTargetAddress.filter((item) => - isVerificationSubject(item.subject), + const matchingSubject = toTargetAddress.filter((item) => + subjectFilter(item.subject), ); const timestampFilteredOut = afterTimestamp !== undefined - ? verificationToTarget.filter((item) => { + ? matchingSubject.filter((item) => { const sent = parseMailslurperDate(item.dateSent); return ( typeof sent === "number" && @@ -151,7 +162,7 @@ export const getKratosVerificationCode = async ( const diagnostics = [ `Total emails in mailslurper: ${allItems.length}`, `Emails to ${emailAddress}: ${toTargetAddress.length}`, - `Verification emails to ${emailAddress}: ${verificationToTarget.length}`, + `${emailType} emails to ${emailAddress}: ${matchingSubject.length}`, afterTimestamp !== undefined ? `Filtered out by timestamp (sent before ${new Date(afterTimestamp - timestampBufferMs).toISOString()}, i.e. afterTimestamp ${new Date(afterTimestamp).toISOString()} minus ${timestampBufferMs}ms buffer): ${timestampFilteredOut.length}` : null, @@ -163,66 +174,30 @@ export const getKratosVerificationCode = async ( .join("; "); throw new Error( - `No verification email found for ${emailAddress} within ${maxWaitMs}ms.${lastErrorMessage} [${diagnostics}]`, + `No ${emailType.toLowerCase()} email found for ${emailAddress} within ${maxWaitMs}ms.${lastErrorMessage} [${diagnostics}]`, ); }; -export const getKratosRecoveryCode = async ( +export const getKratosVerificationCode = async ( emailAddress: string, afterTimestamp?: number, -): Promise => { - const maxWaitMs = 10_000; - const pollIntervalMs = 250; - const timestampBufferMs = 5_000; - let elapsed = 0; - - while (elapsed < maxWaitMs) { - try { - const response = await fetch("http://localhost:4437/mail"); - - if (!response.ok) { - throw new Error( - `Unable to fetch emails from mailslurper: ${response.status} ${response.statusText}`, - ); - } - - const data = (await response.json()) as { - mailItems?: MailslurperMailItem[]; - }; - - const match = data.mailItems - ?.filter((mailItem) => { - const sentTimestamp = parseMailslurperDate(mailItem.dateSent); - return ( - isRecoverySubject(mailItem.subject) && - extractToAddresses(mailItem.toAddresses).includes(emailAddress) && - (!afterTimestamp || - (typeof sentTimestamp === "number" && - sentTimestamp >= afterTimestamp - timestampBufferMs)) - ); - }) - .sort((a, b) => { - const aTs = parseMailslurperDate(a.dateSent) ?? 0; - const bTs = parseMailslurperDate(b.dateSent) ?? 0; - return bTs - aTs; - }) - .at(0); +): Promise => + pollForKratosCode({ + emailAddress, + afterTimestamp, + subjectFilter: isVerificationSubject, + extractCode: extractVerificationCode, + emailType: "Verification", + }); - if (match?.body) { - const code = match.body.match(/\b(\d{6})\b/)?.[1]; - if (code) { - return code; - } - } - } catch { - // Transient error — retry on next poll iteration. - } - - await sleep(pollIntervalMs); - elapsed += pollIntervalMs; - } - - throw new Error( - `No recovery email found for ${emailAddress} within ${maxWaitMs}ms.`, - ); -}; +export const getKratosRecoveryCode = async ( + emailAddress: string, + afterTimestamp?: number, +): Promise => + pollForKratosCode({ + emailAddress, + afterTimestamp, + subjectFilter: isRecoverySubject, + extractCode: (body) => body.match(/\b(\d{6})\b/)?.[1], + emailType: "Recovery", + }); From dc4c82e6739150963c21b28c0d3aaa8d0df6082d Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Fri, 17 Apr 2026 13:25:02 +0200 Subject: [PATCH 14/14] Prevent stuck loading state when CSRF token extraction throws mustGetCsrfTokenFromFlow was called inline as an argument to submitSettingsUpdate. A synchronous throw would skip the Promise chain and its .finally() cleanup, leaving updatingPassword stuck at true. Extract the token before starting the async chain. --- .../hash-frontend/src/pages/settings/security.page.tsx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/apps/hash-frontend/src/pages/settings/security.page.tsx b/apps/hash-frontend/src/pages/settings/security.page.tsx index 6ef8d869932..d4f0eff51f2 100644 --- a/apps/hash-frontend/src/pages/settings/security.page.tsx +++ b/apps/hash-frontend/src/pages/settings/security.page.tsx @@ -312,6 +312,14 @@ const SecurityPage: NextPageWithLayout = () => { return; } + let csrfToken: string; + try { + csrfToken = mustGetCsrfTokenFromFlow(flow); + } catch { + setErrorMessage("Could not find CSRF token. Please reload the page."); + return; + } + setUpdatingPassword(true); persistFlowIdInUrl(flow); @@ -322,7 +330,7 @@ const SecurityPage: NextPageWithLayout = () => { void submitSettingsUpdate(flow, { method: "password", password, - csrf_token: mustGetCsrfTokenFromFlow(flow), + csrf_token: csrfToken, }) .then((nextFlow) => { if (nextFlow) {