FE-1171, FE-1172, FE-1174: fix flaky Playwright tests#8992
Conversation
The signin page fetches its Kratos login flow asynchronously after mount, and submitting before the flow has loaded throws without retrying, so the test's waitForURL never resolves. - global-setup: wait for the /self-service/login flow response (registered before page.goto so it can't be missed) before clicking Submit - signin page: disable the Submit button until the flow has loaded, so Playwright's actionability checks also cover this Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQLxSZtnPp3HfxFycFLwLY
…1174) The tab counts only render once the types contexts finish loading all types (a heavy all-versions query); until then the tabs show just their title with a spinner. The default 5s expect timeout races that query – give each tab-count assertion an explicit 30s timeout instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQLxSZtnPp3HfxFycFLwLY
getOutgoingLinkAndTargetEntities / getIncomingLinkAndSourceEntities return `undefined` for rightEntity / leftEntity when the link's has-right-entity / has-left-entity edge is not resolved into the subgraph, but LinkEntityAndRightEntity / LinkEntityAndLeftEntity declared those properties as non-optional and an `as` cast hid the mismatch. Consumers indexing `rightEntity[0]` then crash at runtime – most visibly constructOrg, which runs via the auth context on every page and turned a partially-consistent subgraph into an app-wide crash loop. - declare rightEntity / leftEntity as possibly undefined - guard every consumer flagged by the compiler, skipping links whose target/source entity array is missing or empty instead of throwing - add a changeset for @blockprotocol/graph Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQLxSZtnPp3HfxFycFLwLY
github-actions
Bot
added
area/apps > hash*
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8992 +/- ##
=======================================
Coverage 59.73% 59.73%
=======================================
Files 1371 1371
Lines 135461 135461
Branches 6066 6066
=======================================
Hits 80911 80911
Misses 53618 53618
Partials 932 932
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…tities (FE-1172) Where a link's target is a public entity whose presence in the subgraph is guaranteed by the query (user/org membership targets, parent pages, linear-sync web targets), a missing entity now throws a descriptive error referencing BE-644 instead of being silently skipped. The root cause (subgraph reads not being snapshot-consistent) is being fixed server-side; these errors keep the invariant violation loud if it ever recurs. Genuinely-optional absences (notifications, mentions, links tables, permission-filterable targets) remain graceful.
…ng test timeout (FE-1174) The /types page previously derived its entity type tab counts from the entity types context, which fetches every version of every entity type (including archived ones) with deep constraint resolution - a heavy query that could take longer than the Playwright default expect timeout to resolve. The page now runs its own latestOnly query with constraint resolve depths zeroed (only the inheritance chain is needed to classify link types), and classifies link vs non-link types locally. The 30s expect timeout previously added to the types-page spec is reverted: the default 5s timeout is the page's load budget.
…variables Remove ticket references from invariant-error messages and comments - the errors describe what is wrong, not which ticket tracks it. In constructUser, collect the checked link/target entity revisions instead of re-indexing the revision arrays with non-null assertions afterwards.
PR SummaryMedium Risk Overview Link targets ( Sign-in (FE-1171) — Playwright global setup waits for the Kratos Types page (FE-1174) — The Reviewed by Cursor Bugbot for commit 2203c6b. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Pull request overview
This PR addresses three sources of Playwright flakiness in the HASH frontend test suite by fixing underlying races and hardening client-side handling of partially-resolved subgraphs (rather than adding retries).
Changes:
- FE-1171: Prevent submitting the sign-in form before the Kratos login flow has loaded (both in global setup and the Signin page UI).
- FE-1172: Make
LinkEntityAndRightEntity/LinkEntityAndLeftEntityreflect runtime reality by allowingrightEntity/leftEntityto beundefined, and update consumers to either handle absence gracefully or throw invariant violations where absence should be impossible. - FE-1174: Decouple
/typestab counts from the heavy “all versions” entity types context by issuing a page-scopedlatestOnlyquery with narrowed resolve depths.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/hash-playwright/tests/features/types-page.spec.ts | Keeps the 5s assertion budget and validates non-zero tab counts. |
| tests/hash-playwright/global-setup.ts | Waits for the Kratos login-flow response before submitting sign-in. |
| libs/@local/hash-isomorphic-utils/src/service-usage.ts | Guards against missing link target entity and throws a descriptive error. |
| libs/@local/hash-isomorphic-utils/src/save.ts | Updates link-target indexing to tolerate rightEntity being absent. |
| libs/@blockprotocol/graph/src/types/entity.ts | Makes rightEntity / leftEntity optional in link+endpoint helper types. |
| libs/@blockprotocol/graph/src/stdlib/subgraph/edge/link-entity.ts | Documents undefined link endpoints and clarifies the generic cast usage. |
| apps/plugin-browser/src/shared/get-user.ts | Avoids unsafe indexing into possibly-missing link target entities; throws invariants where required. |
| apps/hash-frontend/src/pages/types/[[...type-kind]].page.tsx | Adds a lighter page-scoped entity-types query for counts/classification. |
| apps/hash-frontend/src/pages/signin.page.tsx | Disables Submit until the login flow is available. |
| apps/hash-frontend/src/pages/shared/use-flow-runs-usage.ts | Handles missing rightEntity when reading incurred-in relationships. |
| apps/hash-frontend/src/pages/shared/entity/get-entity-multi-type-dependencies.ts | Handles missing link endpoints when collecting dependency type ids. |
| apps/hash-frontend/src/pages/shared/entity/entity-editor/links-section/outgoing-links-section/use-rows.ts | Skips outgoing links whose target entity isn’t resolved into the subgraph. |
| apps/hash-frontend/src/pages/shared/entity/entity-editor/links-section/outgoing-links-section/readonly-outgoing-links-table.tsx | Skips rows when right-entity is missing to avoid crashing the links table. |
| apps/hash-frontend/src/pages/shared/entity/entity-editor/links-section/incoming-links-section/incoming-links-table.tsx | Skips rows when left-entity is missing to avoid crashing the links table. |
| apps/hash-frontend/src/pages/shared/entity/entity-editor/links-section/incoming-links-section.tsx | Updates incoming link filtering to tolerate optional leftEntity. |
| apps/hash-frontend/src/pages/shared/claims-table.tsx | Avoids unsafe indexing into optional rightEntity for claims rendering. |
| apps/hash-frontend/src/pages/shared/block-collection/shared/mention-suggester.tsx | Defaults missing right-entity revisions to an empty list during suggestion building. |
| apps/hash-frontend/src/pages/shared/block-collection/mention-view/mention-display.tsx | Avoids unsafe indexing into possibly-missing mention targets. |
| apps/hash-frontend/src/pages/shared/block-collection/block-context-menu/block-select-data-modal.tsx | Filters out links with missing query target entities before mapping. |
| apps/hash-frontend/src/pages/shared/block-collection-contents.ts | Updates block child entity lookup to tolerate missing target. |
| apps/hash-frontend/src/pages/settings/integrations/linear/use-linear-integrations.ts | Throws invariant violations when a public link target is unexpectedly missing. |
| apps/hash-frontend/src/pages/notifications.page/notifications-with-links-context.tsx | Avoids unsafe indexing into optional link targets for notification context. |
| apps/hash-frontend/src/lib/user-and-org.ts | Splits “legal absence” vs “invariant violation” handling for user/org graph walking. |
| apps/hash-frontend/src/components/hooks/use-account-pages.ts | Throws invariant violation if a resolved has-parent link target is missing. |
| apps/hash-api/src/graphql/resolvers/knowledge/org/shared.ts | Updates org invitation resolution to tolerate optional leftEntity. |
| .changeset/fe-1172-optional-link-targets.md | Adds a changeset describing the @blockprotocol/graph type correction. |
| // The cast is only needed for the caller-provided generic – the mapped | ||
| // value is a valid `LinkEntityAndRightEntity[]`. | ||
| // @todo consider fixing generics in functions called within | ||
| ) as LinkAndRightEntities; |
There was a problem hiding this comment.
The generic parameter and cast are pre-existing API design, not introduced here — this PR only changed the default element types so that, absent an explicit type argument, callers see rightEntity?: Entity[] / leftEntity?: Entity[] and must handle undefined. Supplying a narrower type argument is an explicit caller assertion about their subgraph (e.g. that link-target resolve depths were requested), the same opt-in escape hatch it has always been. Removing the generic/cast would be a breaking redesign of the public signature, already flagged by the existing @todo and out of scope for this fix.
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| "@blockprotocol/graph": minor | |||
There was a problem hiding this comment.
@blockprotocol/graph is at 0.4.3, i.e. pre-1.0. Under semver's 0.x convention — and changesets' handling of 0.x packages — a breaking change is expressed as a minor bump (0.4.x → 0.5.0); a major bump would promote the package to 1.0.0, which is not intended here. Note also that runtime behavior is unchanged: these functions could always return undefined targets, and the previous types overpromised — this change surfaces that pre-existing unsoundness at compile time rather than introducing new behavior.
Requested by Tim Diekmann · Slack thread
🌟 What is the purpose of this PR?
Fixes the three flaky Playwright failures tracked in FE-1171, FE-1172, and FE-1174. Each one is a race, and each is fixed at the source rather than by retrying the test.
FE-1171 — global-setup sign-in race
Before: the suite's global setup sometimes timed out on
waitForURL("/")after clicking Submit on/signin. The signin page fetches its Kratos login flow asynchronously after mount; if setup clicked Submit before the flow response arrived,handleSubmitthrew "No sign in flow available" with no retry, so no navigation ever happened and the whole run failed in setup.After: global setup registers a wait for the
/self-service/loginflow response before navigating, and awaits it before clicking Submit. The signin page's Submit button is also disabled until the flow has loaded, so Playwright's own actionability checks (and real users) can no longer submit into the void.FE-1172 — app crash loop from unguarded link-target indexing
Before: specs intermittently found the frontend stuck in an error state on every page.
getOutgoingLinkAndTargetEntities/getIncomingLinkAndSourceEntitiesin@blockprotocol/graphreturnundefinedforrightEntity/leftEntitywhen the link'shas-right-entity/has-left-entityedge isn't resolved into the subgraph, but theLinkEntityAndRightEntity/LinkEntityAndLeftEntitytypes declared those properties non-optional, with anascast hiding the mismatch. Consumers indexingrightEntity[0]then crashed with aTypeError— most visiblyconstructOrg, which runs via the auth context on every page, turning one bad subgraph into an app-wide crash loop. The flakiness trigger is parallel specs acting as the same user: concurrent writes can produce partially-consistent subgraphs in which a link entity is present but its target entity is not.After: the types are honest —
rightEntity/leftEntityare declared possiblyundefined— and every consumer the compiler flagged now handles the absence explicitly, split by whether absence is legal:Invariant violation: …error instead of silently skipping: org membership walking inconstructOrg(user-and-org.ts), parent-page resolution inuse-account-pages.ts, org memberships in the browser plugin'sget-user.ts, andsyncLinearDataWithweb targets inuse-linear-integrations.ts. (The billing path inservice-usage.tsand the block-contents paths insave.ts/block-collection-contents.ts/org/shared.tsalready threw and keep doing so.) The Playwright console fixture fails tests on console errors, so if the server race ever recurs the failure stays loud rather than silently degrading.Behaviour is unchanged when subgraph data is complete. The root cause — subgraph reads not being snapshot-consistent under concurrent writes — is fixed server-side in #8996 (BE-644); this PR deliberately does not silence the error client-side. A changeset is included for
@blockprotocol/graph.FE-1174 — tab-count assertion racing a heavy query
Before: the
/typespage spec asserted each tab shows a non-zero count (e.g.Entity Types12) with the default 5s expect timeout, but the entity type counts only rendered after the entity types context finished loading every version of every entity type (including archived ones) with deep constraint resolution — a heavy query. Under load the tabs still showed just their title with a spinner at the 5s mark, failing the assertion.After: the test's default 5s expect timeout is the page's load budget, so the fix is in the page, not the test. The
/typespage no longer waits on the heavy entity types context for its counts: it runs its ownlatestOnly: truequery with the constraint resolve depths zeroed (only the inheritance chain is needed to classify link types) and classifies link vs non-link types locally. The spec keeps the default expect timeout and the non-zero-count regexes.Remaining latency risk: test artifacts show navigation + app hydration consume roughly 4s of the budget before any query fires, and the heavy all-versions context query is still triggered in the background by the types table (though it no longer blocks the counts). If the budget is still tight in CI, the next lever is app-shell hydration cost, which is out of scope here.
🔗 Related links
🔍 What does this change?
tests/hash-playwright/global-setup.ts(await the Kratos login-flow response before clicking Submit),apps/hash-frontend/src/pages/signin.page.tsx(disable Submit until the flow is loaded)libs/@blockprotocol/graph/src/types/entity.ts(makerightEntity/leftEntityoptional),libs/@blockprotocol/graph/src/stdlib/subgraph/edge/link-entity.ts(document the now-honest return values), plus explicit handling in every consumer the compiler flagged — loud invariant errors where presence is guaranteed (apps/hash-frontend/src/lib/user-and-org.ts,apps/hash-frontend/src/components/hooks/use-account-pages.ts,use-linear-integrations.ts,apps/plugin-browser/src/shared/get-user.ts), graceful skips where absence is legal:notifications-with-links-context.tsx,block-collection-contents.ts,block-select-data-modal.tsx,mention-display.tsx,mention-suggester.tsx,claims-table.tsx, entity-editor links-section (incoming-links-section.tsx,incoming-links-table.tsx,readonly-outgoing-links-table.tsx,use-rows.ts),get-entity-multi-type-dependencies.ts,use-flow-runs-usage.ts,apps/hash-api/src/graphql/resolvers/knowledge/org/shared.ts,apps/plugin-browser/src/shared/get-user.ts,libs/@local/hash-isomorphic-utils/src/save.ts,libs/@local/hash-isomorphic-utils/src/service-usage.ts; changeset in.changeset/fe-1172-optional-link-targets.mdapps/hash-frontend/src/pages/types/[[...type-kind]].page.tsx(page-scopedlatestOnlyentity types query with narrowed resolve depths, replacing the page's dependence on the heavy all-versions entity types context),tests/hash-playwright/tests/features/types-page.spec.ts(default expect timeout kept — the 5s budget is the point of the test)Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
LinkEntityAndRightEntity-shaped values with their own explicit generic instantiations (e.g.save.ts,block-collection-contents.ts) still assert non-optional target arrays; the compiler accepts these and their existing runtime guards were left as-is.🐾 Next steps
sleep(500)in thetests/hash-playwright/tests/features/entity-editing.spec.tshelpers is the same class of bug — a fixed delay racing async rendering — and should be replaced with a condition-based wait.🛡 What tests cover this?
global-setup.ts,types-page.spec.ts) exercises FE-1171 and FE-1174 directly; the types-page spec now enforces the 5s load budget with the default expect timeout.tests/hash-backend-integration/src/tests/subgraph/friendship.test.tsalready assertsrightEntity: undefined/rightEntity: []for unresolved and archived targets, which the FE-1172 type change now reflects honestly.lint:tsc) passes for@blockprotocol/graph,@local/hash-isomorphic-utils,@local/hash-backend-utils,@apps/hash-frontend,@apps/hash-api,@apps/plugin-browser,@tests/hash-playwright, and@tests/hash-backend-integration; eslint and oxfmt clean on changed files.❓ How to test this?
Invariant violation …error (the read inconsistency itself is fixed in BE-644: Run subgraph reads in a single READ ONLY, REPEATABLE READ transaction #8996) rather than silently rendering incomplete data; genuinely-optional targets (notifications, links tables, mentions) are still skipped gracefully📹 Demo
N/A — test-stability and defensive-typing changes with no intended UI change (the signin Submit button is now briefly disabled while the login flow loads).
🤖 Generated with Claude Code
https://claude.ai/code/session_01VQLxSZtnPp3HfxFycFLwLY
Generated by Claude Code