Skip to content

FE-600: serialize authenticated user updates - #9242

Open
claude[bot] wants to merge 2 commits into
mainfrom
claude/fe-600-serialize-user-updates
Open

FE-600: serialize authenticated user updates#9242
claude[bot] wants to merge 2 commits into
mainfrom
claude/fe-600-serialize-user-updates

Conversation

@claude

@claude claude Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Requested by Ciaran Morinan · Slack thread

🌟 What is the purpose of this PR?

Before. Preference changes were sent and forgotten. Toggle a display setting in settings and expand a sidebar section a moment later, and the two saves overlapped on the same user record; the graph refused the second. The refusal came back inside a successful-looking response that nothing was reading, so the UI kept showing what you had just clicked while the server kept the old value, and your change was gone on the next page load. Even when both saves got through, each was assembled from a copy of your preferences taken before the other landed, so the later save wrote the earlier one back. In Playwright the failure was consistent rather than occasional, because tests click faster than a person.

After. Preference saves go one at a time, so two never collide and the graph never refuses one. Each save is assembled when it is sent rather than when it was clicked, and only the setting the caller actually changed is applied on top of what is already stored — so an expanded sidebar section and a switched display variant both stick, in either order. Favourites behave the same way. No call site changed.

🔗 Related links

🚫 Blocked by

  • Nothing

🔍 What does this change?

How. useUpdateAuthenticatedUser now runs the body of each update through a module-scoped serial queue (serial-queue.ts), so an update starts only once every update enqueued before it has settled — the queue is module-scoped rather than per hook instance because each component that saves preferences has its own instance of the hook and they do not coordinate. Everything the update needs (the authenticated user, getMe, updateEntity, refetch) is read from a ref when its turn comes rather than captured when it was enqueued, so it sees the state the updates ahead of it left behind; the callback consequently has an empty dependency array and is stable. Before sending, rebaseUserPreferences (rebase-preferences.ts) diffs the preferences the caller built its payload from against the payload, and re-applies just that difference on top of the preferences read back from the server in the same turn — recursing through nested objects, and treating the favourites array as a set, since its order is display-only. The queue swallows rejections on the chain it keeps internally, so one failed update cannot stall the ones behind it, while still rejecting for the caller that submitted it.

  • Scope note, called out for review: the ticket's approach 2 asked only for a queue. Serializing alone does not fix the user-visible symptom, because each call site builds its payload by spreading the preferences it last rendered — so the second queued save still overwrites the first's change. The rebase module is therefore included, and it is the larger part of the diff. Ciaran approved including it.
  • Amended in e38e8d6, after a reviewer was already requested: two cases were fixed and covered by tests, both in what the rebase treats as its base. The base was authenticatedUser?.preferences, which is undefined for a user with nothing stored, while every call site builds its payload from useUserPreferences, which substitutes the defaults — so with a concurrent change already on the server, every default was read as a caller edit and overwrote it, and two quick first-time saves still clobbered each other. Separately, a second toggle of the same field before the context refreshed produced a payload deep-equal to the base, so the rebase found no change to re-apply, returned the server's value and dropped the revert — a regression against main, where that payload was sent unrebased. The base now comes from preferencesToRebaseFrom, which reads the same useUserPreferences value the call sites build from, and prefers the payload the same hook instance still has in flight.
  • Latency: nothing a user waits on got slower. An update makes the same three round trips as before (getMe, updateEntity, refetch) — none were added — and all six preference call sites discard the promise and never read the hook's loading, so they never awaited the round trip in the first place. The only call site that awaits a save behind a disabled button is the profile modal, which can now wait out a preference save that is already in flight if you toggled a sidebar section a moment before pressing Save; that is bounded by one update, and it is exactly the overlap that previously failed.
  • Validation: 14 new unit tests pass; eslint and oxfmt clean; typecheck adds no new error attributable to this change (the package cannot typecheck cleanly on main either, because of an unbuilt Rust→wasm artifact and ungenerated GraphQL types). The hook's own wiring has no test — the queue and the rebase are tested as units, and the original race is not reproduced anywhere.

Pre-Merge Checklist 🚀

🚢 Has this modified a publishable library?

This PR:

  • does not modify any publishable blocks or libraries, or modifications do not need publishing

📜 Does this require a change to the docs?

The changes in this PR:

  • are internal and do not require a docs change

🕸️ Does this require a change to the Turbo Graph?

The changes in this PR:

  • do not affect the execution graph

⚠️ Known issues

  • The hook itself is not covered by a test, as noted above.
  • rebaseUserPreferences treats the favourites array as a set. That is right for favourites, and would be wrong for any future preference whose array order carries meaning.
  • Updates are serialized globally, across every kind of update to the authenticated user rather than only preferences. That is deliberate — they all patch the same entity — but it does mean a profile save queues behind a preference save.

🐾 Next steps

  • If the loading flag ever needs to reflect queue position rather than just this instance's own in-flight updates, that would be a separate change.

🛡 What tests cover this?

New unit tests, 14 in total:

  • serial-queue.test.ts (4) — a task does not start while an earlier one is running; a task runs when its turn comes rather than when it was enqueued, so it reads what the one before it left; a failing task does not stop the tasks behind it; tasks resolve in the order they were enqueued.
  • rebase-preferences.test.ts (10) — eight for rebaseUserPreferences: a change that landed while the caller's update was queued is kept; the caller's change wins when the server changed the same field; the server's value is taken for anything the caller did not change; a favourite is added, and separately removed, without dropping one added in the meantime; the caller's preferences are sent as-is when the server has none; fields the caller never saw are kept; and another caller's first-ever change survives when neither caller had stored preferences, rather than the defaults substituted for the caller overwriting it. Two for preferencesToRebaseFrom: what the caller rendered is used when it has nothing in flight; and a second change to the same field, made before the first landed, is kept rather than lost to the server's value.

❓ How to test this?

  1. Check out the branch / view the deployment.
  2. Open settings → personalization and switch a sidebar section's display variant, then immediately expand a different sidebar section.
  3. Reload. Both changes should still be there — on main, one of the two is lost.
  4. Repeat with favourites: star and unstar items in quick succession, then reload.

Updates to the authenticated user all patch the same entity, and the graph
rejects a patch which overlaps another. Preference changes were sent and
forgotten, so two which overlapped left the UI showing a change the server
had refused.

Run these updates one at a time through a module-scoped serial queue, and
build each one when it is sent rather than when it was clicked. Because
callers construct their payload by spreading the preferences they last
rendered, only the setting the caller actually changed is re-applied on top
of the preferences on the server now.
@vercel

vercel Bot commented Aug 19, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hash Ready Ready Preview Aug 19, 2026 10:46am
3 Skipped Deployments
Project Deployment Actions Updated (UTC)
hashdotdesign-tokens Ignored Ignored Preview Aug 19, 2026 10:46am
petrinaut-docs Ignored Ignored Preview Aug 19, 2026 10:46am
petrinaut Skipped Skipped Aug 19, 2026 10:46am

@github-actions github-actions Bot added area/apps > hash* Affects HASH (a `hash-*` app) type/eng > frontend Owned by the @frontend team area/apps labels Aug 19, 2026
@claude
claude Bot marked this pull request as ready for review August 19, 2026 10:22
@cursor

cursor Bot commented Aug 19, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes how all authenticated-user entity patches are ordered and how preference blobs are merged; behavior is well-tested for queue/rebase but the hook itself is untested and profile saves now queue behind preference saves.

Overview
Fixes lost preference saves when multiple UI actions update the same user record in quick succession (graph RaceConditionOnUpdate and full-object overwrites).

useUpdateAuthenticatedUser now runs each update through a module-scoped serial queue so patches to the authenticated user entity never overlap. When a task runs, it reads the latest user, getMe, refetch, and updateEntity from a ref (not from the closure at enqueue time), and loading tracks only this hook instance’s in-flight work.

For preferences, saves call rebaseUserPreferences: diff the caller’s payload against what they built it from (preferencesToRebaseFrom, including an in-flight payload for double-toggles), then merge only those deltas onto the preferences fetched from the server in that turn—so concurrent sidebar/settings/favorites changes compose instead of clobbering each other.

Adds serial-queue and rebase-preferences helpers with unit tests; no call-site API changes.

Reviewed by Cursor Bugbot for commit e38e8d6. Bugbot is set up for automated code reviews on this repo. Configure here.

@claude
claude Bot requested a review from CiaranMn August 19, 2026 10:22

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 52116dc. Configure here.

Comment thread apps/hash-frontend/src/components/hooks/use-update-authenticated-user.ts Outdated
…yload from

Two cases where the base the rebase compared a payload with was not the
preferences the caller had built it from, so its diff was wrong:

- A caller with no stored preferences renders the defaults which
  `useUserPreferences` substitutes, but the base was read straight from
  `authenticatedUser.preferences` and so was `undefined`. Every default the
  caller sent then looked like a change of its own, overwriting another
  first-time change which had landed in the meantime. The base is now read
  from `useUserPreferences`, and `rebaseUserPreferences` requires one.

- A caller which changes the same setting twice before the first update lands
  builds its second payload from what it rendered, which does not include its
  own first change yet, so the second payload matched the base exactly and the
  rebase treated the revert as no change at all. Each hook instance now rebases
  from the payload it still has in flight, while it has one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/apps > hash* Affects HASH (a `hash-*` app) area/apps type/eng > frontend Owned by the @frontend team

Development

Successfully merging this pull request may close these issues.

2 participants