You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Setting an email address, phone number, or wallet as primary now keeps its menu open with a pending indicator, then updates the Primary badge without moving rows. If the change needs session verification, the menu closes when the verification dialog opens. A shared section lock prevents primary changes and removal from running at the same time. Failed removals keep their confirmation dialog open with the error.
Each section controller disables its controls while either its pending action or its removal confirmation is pending, and the rows it returns carry their own pending state, so views no longer compare pending keys. A confirmation closed during session verification now stays pending until the interrupted action settles, so a second removal cannot start in that window.
Fixes CORE-4051.
Checklist
pnpm test runs as expected.
pnpm build runs as expected.
(If applicable) JSDoc comments have been added or updated for any package exports
It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.
No actionable comments were generated in the recent review.
Walkthrough
ConfirmController.confirm now returns a boolean result. usePendingAction can guard confirmations and serialize them with direct actions. Email, phone, and wallet sections expose pending state on individual rows, disable section controls during actions, and retain their initial row order. The changes also update confirmation focus coverage, cache removal-trigger refs, and document the shared action pattern.
clerk/javascript#10446: This change replaces the pendingVerifyId contract with per-row verification state.
clerk/javascript#10520: This follow-up uses the shared guard and disabled-state contracts introduced here.
Suggested reviewers:ephem
Merge Risk: Low · previous review c724d
A narrow, user-visible row-order issue remains after primary changes, but profile actions remain usable.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Title check
The title "fix(mosaic): coordinate pending identifier actions" names the primary change. The changeset adds a shared per-section pending-action lock for email, phone, and wallet rows. The lock covers …
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Description check
The description directly matches the changes. It explains coordinated pending actions, shared locking, confirmation behavior, row-level pending state, and the CORE-4051 bug fix.
Full details: Title check
Explanation
The title "fix(mosaic): coordinate pending identifier actions" names the primary change. The changeset adds a shared per-section pending-action lock for email, phone, and wallet rows. The lock covers primary changes and removal, so the two cannot run at the same time. The title is concise, specific, and uses the conventional commit format.
Full details: Docstring Coverage
Explanation
Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 13 functions across 25 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Linked repositories: Some configured repositories could not be included in this review. Cross-repository analysis may be incomplete. Please check linked-repository access and settings.
Included review availability: This review used your included allowance. 3 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.
Before, the menu closed with no pending sign and bob jumped above alice once saved; after, the menu stays open with a spinner while saving, then bob gets the Primary badge in place.
Merge-base
This PR a78e3c5c
Also checked · 6 match
Wallet set-as-primary holds menu and order Reverification closes the menu Lock blocks removal during primary change Failed removal keeps dialog with error Phone set-as-primary Cancelled confirm stays pending until run settles
Proof on a78e3c5c (from 74bd0892) · Did Proof get it right? Yes · No
The goal here is right: set-primary and remove must not run at the same time. But I don't think guard is the shape we want on a shared primitive.
Why
It decorates another controller from outside.guard(remove) spreads the confirm controller and swaps its confirm. The returned object's isPending and canClose still come only from the confirm machine, so the shared lock state lives in two places and views have to know to read actions.isPending instead.
It forces a contract change on useConfirmController.confirm goes from void to Promise<boolean>, and the snapshot.value !== 'confirming' check is dropped in favour of completion ?? Promise.resolve(false). That only works if the machine calls run synchronously inside send, which ties the confirm controller's return value to a runtime detail of create-actor.
It couples a generic hook to one controller's shape (PendingConfirm<TPayload>, a method literally named confirm).
The name clashes.guard already means a transition guard in our machine runtime (machine/create-actor.ts, and the confirm machine's own onError).
The underlying gap is in useConfirmController, and it exists on main today. When reverification exposes onCancel, closing the dialog sends CANCEL from pending to idle and abandons the invoke while onConfirm is still running. As a result:
The dialog can be reopened and confirmed again, which starts a secondonConfirm while the first is still in flight.
isPending is false while that first run is still settling.
These two tests fail on main (packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts):
it('does not start a second run while a cancelled run is still settling',async()=>{constfirst=deferred<void>();constonConfirm=vi.fn(()=>first.promise);constreverification={status: 'loading'asconst,onCancel: vi.fn(),visible: false};const{ result }=renderHook(()=>useConfirmController({ onConfirm, reverification }));act(()=>result.current.onOpenChange(true));act(()=>result.current.confirm());act(()=>result.current.onOpenChange(false));act(()=>result.current.onOpenChange(true));act(()=>result.current.confirm());expect(onConfirm).toHaveBeenCalledOnce();// fails on main: called 2 times// ...});it('stays pending until a cancelled run settles',async()=>{// same setup, confirm, then closeexpect(result.current.isOpen).toBe(false);expect(result.current.isPending).toBe(true);// fails on main: false// ...});
Proposal
Fix the confirm controller in its own layer, then combine pending state where the section reads it. No new API on usePendingAction:
useConfirmController tracks the in-flight run separately from the dialog being open (for example a cancelling state that holds until the run settles), so isPending stays true and a second confirm is ignored until then. Whether that should stay isPending or become a separate flag is open for discussion.
usePendingAction stays as it is on main: no guard, claim, or external pending kind. confirm goes back to returning void.
The other changes in this PR (menu stays open with a pending indicator, stable row order, row-level pending state) are independent of this and can stay.
Edge cases worth covering in the section tests: set-primary in flight blocks remove; remove in flight disables set-primary; a cancelled reverification keeps set-primary disabled until the removal settles; reopening during that window doesn't start a second removal; a failed removal keeps the dialog open with its error and unlocks the section; a double-click on set-primary runs once.
Thanks, agreed on all points. Done in the last four commits:
1ac7778 drops guard. usePendingAction, confirm-shell.tsx and the active devices controller match main again, and confirm returns void. The email, phone and wallet sections use isDisabled: actions.isPending || remove.isPending and pass remove through unchanged.
0a6f7c2 adds your two cases as one it.each (the run resolves or rejects after the dialog closes). It fails at that commit with isPendingfalse.
b218cb3 adds a cancelling state to the confirm machine. Closing the dialog while a run is in flight closes it but keeps isPending true, and OPEN and CONFIRM are ignored until the run settles. The run starts once on confirming → pending, so pending and cancelling wait on the same promise. The existing machine test that expected CANCEL to abandon the run now expects cancelling.
a78e3c5 adds section coverage: a removal in flight disables the Manage buttons and Add, a failed removal keeps its error in the dialog and unlocks the section, and a double-click on Set as primary sends one request. The wallets failed-removal test checks the same lock.
On isPending versus a separate flag: I kept isPending. Every caller wants "something is still running," and the only consumer that cares about the dialog uses isOpen, which is false during cancelling. Happy to split it if you'd rather.
One case has no section-level test: a cancelled reverification keeping set-primary disabled until the removal settles. In the real flow that window lasts only a moment, so a feature test can't hold it open without a fake request. The controller test covers it.
usePendingAction gains guard(), which runs a confirmation's confirm under
the section lock, replacing runExternal and the wrapper each section
copied. Section controllers put verify and primary pending flags on the
rows they return, so views no longer compare pending keys. Confirmation
drops the disabled prop, which no reachable state could set.
Set as primary stayed pending while the session verification dialog was
open, so its menu stayed open with a spinner behind the dialog. The
pending status now ends when the dialog appears, as ActionMenu expects.
Email, phone and web3 wallet rows list the primary first when the
section loads, then keep their positions when a new primary is set, so
only the Primary badge moves. The new primary leads on the next load.
…te in sections
Identifier sections disable their controls from both the pending action and
the removal confirmation instead of decorating the confirm controller, and
confirm returns void again.
Closing the dialog during reverification now moves the confirm machine to a
cancelling state that waits for the in-flight run, so isPending stays true and
a reopened dialog cannot start a second run until it settles.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
austincalvelage
changed the title
Description
Setting an email address, phone number, or wallet as primary now keeps its menu open with a pending indicator, then updates the Primary badge without moving rows. If the change needs session verification, the menu closes when the verification dialog opens. A shared section lock prevents primary changes and removal from running at the same time. Failed removals keep their confirmation dialog open with the error.
Each section controller disables its controls while either its pending action or its removal confirmation is pending, and the rows it returns carry their own pending state, so views no longer compare pending keys. A confirmation closed during session verification now stays pending until the interrupted action settles, so a second removal cannot start in that window.
Fixes CORE-4051.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change