From ce3afe166be151cec0e31f66c6d47c1e48346c8b Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Tue, 3 Nov 2026 19:45:27 -0500 Subject: [PATCH 01/10] fix(mosaic): keep submit buttons pending, not disabled, while a form submits --- .changeset/mosaic-submit-button-pending.md | 5 + .../__tests__/use-form.submit-button.test.tsx | 51 ++++++++++ .../form/__tests__/use-form.test.ts | 4 +- .../src/components/form/form.machine.ts | 99 ++++++++++++------- .../mosaic/src/components/form/use-form.ts | 9 +- .../user-profile-add-email.controller.ts | 4 +- .../__tests__/waitlist.feature.test.tsx | 2 +- 7 files changed, 131 insertions(+), 43 deletions(-) create mode 100644 .changeset/mosaic-submit-button-pending.md create mode 100644 packages/mosaic/src/components/form/__tests__/use-form.submit-button.test.tsx diff --git a/.changeset/mosaic-submit-button-pending.md b/.changeset/mosaic-submit-button-pending.md new file mode 100644 index 00000000000..5ea57c07e3a --- /dev/null +++ b/.changeset/mosaic-submit-button-pending.md @@ -0,0 +1,5 @@ +--- +'@clerk/mosaic': patch +--- + +Form submit buttons now show their pending state and keep focus while the form submits, instead of looking disabled. diff --git a/packages/mosaic/src/components/form/__tests__/use-form.submit-button.test.tsx b/packages/mosaic/src/components/form/__tests__/use-form.submit-button.test.tsx new file mode 100644 index 00000000000..4555c39b0e1 --- /dev/null +++ b/packages/mosaic/src/components/form/__tests__/use-form.submit-button.test.tsx @@ -0,0 +1,51 @@ +import { act, render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import React from 'react'; +import { describe, expect, it } from 'vitest'; + +import { deferred } from '../../../__tests__/async'; +import { Form } from '../../../primitives/form'; +import { SubmitButton } from '../../button'; +import { useForm } from '../use-form'; + +const atoms = (el: Element) => el.className.split(' ').filter(Boolean); + +function SaveForm({ onSubmit }: { onSubmit: () => Promise }) { + const form = useForm({ initialValues: { name: 'alex' }, onSubmit }); + return ( +
+ + Save + +
+ ); +} + +describe('useForm with SubmitButton', () => { + it('shows the submit button as pending, not disabled, while the form submits', async () => { + const { unmount } = render(Save); + const pendingAtoms = atoms(screen.getByRole('button')); + unmount(); + + const request = deferred(); + render( request.promise} />); + const button = screen.getByRole('button', { name: 'Save' }); + + await userEvent.click(button); + + expect(button).toHaveAttribute('aria-busy', 'true'); + expect(button).not.toHaveAttribute('disabled'); + expect(atoms(button)).toEqual(pendingAtoms); + + await act(async () => { + request.resolve(); + await request.promise; + }); + }); +}); diff --git a/packages/mosaic/src/components/form/__tests__/use-form.test.ts b/packages/mosaic/src/components/form/__tests__/use-form.test.ts index 03c73c8f369..865faf36365 100644 --- a/packages/mosaic/src/components/form/__tests__/use-form.test.ts +++ b/packages/mosaic/src/components/form/__tests__/use-form.test.ts @@ -56,7 +56,7 @@ describe('useForm', () => { act(() => result.current.setValue('username', 'alexc')); act(() => result.current.submit()); expect(result.current.isSubmitting).toBe(true); - expect(result.current.canSubmit).toBe(false); + expect(result.current.canSubmit).toBe(true); act(() => result.current.submit()); expect(onSubmit).toHaveBeenCalledTimes(1); expect(onSubmit).toHaveBeenCalledWith({ username: 'alexc' }); @@ -584,7 +584,7 @@ describe('useForm', () => { act(() => result.current.submit()); expect(onSubmit).not.toHaveBeenCalled(); expect(result.current.isSubmitting).toBe(true); - expect(result.current.canSubmit).toBe(false); + expect(result.current.canSubmit).toBe(true); await act(async () => { check.resolve(undefined); await tick(); diff --git a/packages/mosaic/src/components/form/form.machine.ts b/packages/mosaic/src/components/form/form.machine.ts index 31d36005528..4d933124c6e 100644 --- a/packages/mosaic/src/components/form/form.machine.ts +++ b/packages/mosaic/src/components/form/form.machine.ts @@ -53,7 +53,6 @@ export interface FormContext extends FormDeps { touched: Partial>; async: Partial>; error: FormError | undefined; - submitQueued: boolean; } export type FormEvent = @@ -117,12 +116,12 @@ function submitOrStay( ): TransitionResult, FormState> { const next = { ...context, ...patch }; if (!isValid(next)) { - return { context: { ...patch, submitQueued: false } }; + return { target: 'editing', context: patch }; } if (isValidating(next)) { - return { context: { ...patch, submitQueued: true } }; + return { target: 'queued', context: patch }; } - return { target: 'submitting', context: { ...patch, submitQueued: false } }; + return { target: 'submitting', context: patch }; } function displayableFields( @@ -195,7 +194,7 @@ function asyncStateFor( return { value, feedback: context.async[name]?.feedback, pending: true }; } -type FormState = 'editing' | 'submitting'; +type FormState = 'editing' | 'queued' | 'submitting'; export function createFormMachine(deps: FormDeps) { const { createMachine, assign, fromPromise } = setup, FormEvent>(); @@ -210,41 +209,29 @@ export function createFormMachine(deps: FormDeps ({ - context: { - values: { ...context.values, [event.name]: event.value }, - async: { ...context.async, [event.name]: asyncStateFor(context, event.name, event.value) }, - error: withoutField(context.error, event.name), - submitQueued: false, - }, - }), - TOUCH: ({ context, event }) => ({ context: { touched: { ...context.touched, [event.name]: true } } }), + CHANGE: ({ context, event }) => ({ context: changed(context, event) }), + TOUCH: ({ context, event }) => ({ context: touched(context, event) }), VALIDATED: ({ context, event }) => { - if (context.async[event.name]?.value !== event.value) { - return undefined; - } - const async = { - ...context.async, - [event.name]: { value: event.value, feedback: event.feedback, pending: false }, - }; - return context.submitQueued ? submitOrStay(context, { async }) : { context: { async } }; + const async = validated(context, event); + return async === undefined ? undefined : { context: { async } }; }, SUBMIT: ({ context }) => submitOrStay(context, { touched: mapKeys(context.values, (): true => true) }), - RESET: ({ context, event }) => ({ - context: { - values: event.values ?? context.initialValues, - baseline: event.values, - touched: {}, - async: {}, - error: undefined, - submitQueued: false, - }, - }), + RESET: ({ context, event }) => ({ context: resetTo(context, event) }), + }, + }, + queued: { + on: { + CHANGE: ({ context, event }) => ({ target: 'editing', context: changed(context, event) }), + TOUCH: ({ context, event }) => ({ context: touched(context, event) }), + VALIDATED: ({ context, event }) => { + const async = validated(context, event); + return async === undefined ? undefined : submitOrStay(context, { async }); + }, + RESET: ({ context, event }) => ({ target: 'editing', context: resetTo(context, event) }), }, }, submitting: { @@ -269,3 +256,49 @@ export function createFormMachine(deps: FormDeps['type']> = Extract< + FormEvent, + { type: TType } +>; + +function changed( + context: FormContext, + event: EventOf, +): Partial> { + return { + values: { ...context.values, [event.name]: event.value }, + async: { ...context.async, [event.name]: asyncStateFor(context, event.name, event.value) }, + error: withoutField(context.error, event.name), + }; +} + +function touched( + context: FormContext, + event: EventOf, +): Partial> { + return { touched: { ...context.touched, [event.name]: true } }; +} + +function validated( + context: FormContext, + event: EventOf, +): FormContext['async'] | undefined { + if (context.async[event.name]?.value !== event.value) { + return undefined; + } + return { ...context.async, [event.name]: { value: event.value, feedback: event.feedback, pending: false } }; +} + +function resetTo( + context: FormContext, + event: EventOf, +): Partial> { + return { + values: event.values ?? context.initialValues, + baseline: event.values, + touched: {}, + async: {}, + error: undefined, + }; +} diff --git a/packages/mosaic/src/components/form/use-form.ts b/packages/mosaic/src/components/form/use-form.ts index 0a3616b4259..29ac9d17b4c 100644 --- a/packages/mosaic/src/components/form/use-form.ts +++ b/packages/mosaic/src/components/form/use-form.ts @@ -127,9 +127,9 @@ export function useForm( return; } const settle = (feedback: FieldFeedback | undefined) => { - const { submitQueued } = actor.getSnapshot().context; + const wasQueued = actor.getSnapshot().value === 'queued'; send({ type: 'VALIDATED', name, value, feedback }); - if (submitQueued) { + if (wasQueued) { focusFirstInvalid(); } }; @@ -183,7 +183,6 @@ export function useForm( ref: refFor(name), }); - const isSubmitting = snapshot.value === 'submitting' || context.submitQueued; const initial = initialOf(context); const fields = mapKeys(values, (name): FormField => { const feedback = fieldFeedback(context, name); @@ -203,10 +202,10 @@ export function useForm( error: context.error?.message, get isSubmitting() { const current = actor.getSnapshot(); - return current.value === 'submitting' || current.context.submitQueued; + return current.value !== 'editing'; }, isDirty: keysOf(values).some(name => fields[name].isDirty), - canSubmit: !isSubmitting && isValid(context), + canSubmit: isValid(context), register, registerValue, setValue, diff --git a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.controller.ts index be357744086..6b50fd93fdf 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-email-section/user-profile-add-email.controller.ts @@ -224,9 +224,9 @@ export function useUserProfileAddEmailController({ step, reverification, emailAddress: state === 'email' ? emailForm.values.emailAddress : snapshot.context.emailAddress, - canSubmitEmail: emailForm.canSubmit || emailForm.isSubmitting, + canSubmitEmail: emailForm.canSubmit, code: codeForm.values.code, - canSubmitCode: codeForm.canSubmit || codeForm.isSubmitting, + canSubmitCode: codeForm.canSubmit, errorMessage: formError?.message ?? form.error ?? machineError, isPending: form.isSubmitting || state === 'redirecting', onOpenChange: open => { diff --git a/packages/mosaic/src/features/waitlist/__tests__/waitlist.feature.test.tsx b/packages/mosaic/src/features/waitlist/__tests__/waitlist.feature.test.tsx index c41d17d3e08..6de6f450f8a 100644 --- a/packages/mosaic/src/features/waitlist/__tests__/waitlist.feature.test.tsx +++ b/packages/mosaic/src/features/waitlist/__tests__/waitlist.feature.test.tsx @@ -45,7 +45,7 @@ describe('Waitlist', () => { await waitFor(() => expect(request.requests).toHaveLength(1)); expect(emailField()).toHaveAttribute('aria-disabled', 'true'); - await userEvent.setup().click(screen.getByRole('button', { name: /Join the waitlist/ })); + await userEvent.setup({ pointerEventsCheck: 0 }).click(screen.getByRole('button', { name: /Join the waitlist/ })); expect(request.requests).toHaveLength(1); request.release(); From 9a8ac7507fe4b3f7f342d1a79a35af366c20453e Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Tue, 3 Nov 2026 19:45:56 -0500 Subject: [PATCH 02/10] fix(mosaic): keep confirm dialogs busy until the action settles --- .changeset/mosaic-confirm-pending.md | 5 ++ .../__tests__/confirm.controller.test.ts | 57 +++++++++++++++++-- .../src/blocks/confirm/confirm.controller.ts | 16 ++++-- 3 files changed, 68 insertions(+), 10 deletions(-) create mode 100644 .changeset/mosaic-confirm-pending.md diff --git a/.changeset/mosaic-confirm-pending.md b/.changeset/mosaic-confirm-pending.md new file mode 100644 index 00000000000..ffcd43bd761 --- /dev/null +++ b/.changeset/mosaic-confirm-pending.md @@ -0,0 +1,5 @@ +--- +'@clerk/mosaic': patch +--- + +Confirm dialogs now stay busy until their action finishes, even after you cancel a reverification prompt, and they can't be reopened or confirmed a second time while the request is in flight. diff --git a/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts b/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts index e6ec9447bfa..7fc0e8d8704 100644 --- a/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts +++ b/packages/mosaic/src/blocks/confirm/__tests__/confirm.controller.test.ts @@ -38,20 +38,43 @@ describe('confirmMachine', () => { expect(actor.getSnapshot().status).toBe('active'); }); - it('abandons the run when cancelled while pending, ignoring how it settles', async () => { + it('closes when cancelled while pending, and stays pending until the run settles', async () => { const run = deferred(); const actor = start(); actor.send({ type: 'CONFIRM', run: () => run.promise }); actor.send({ type: 'CANCEL' }); - expect(actor.getSnapshot().value).toBe('idle'); + expect(actor.getSnapshot().value).toBe('pending'); + expect(actor.getSnapshot().context.open).toBe(false); + + run.resolve(); + await vi.waitFor(() => expect(actor.getSnapshot().value).toBe('idle')); + }); + + it('ends idle without a reason when a cancelled run fails', async () => { + const run = deferred(); + const actor = start(); + actor.send({ type: 'CONFIRM', run: () => run.promise }); + actor.send({ type: 'CANCEL' }); run.reject(blocked); - await run.promise.catch(() => undefined); - expect(actor.getSnapshot().value).toBe('idle'); + + await vi.waitFor(() => expect(actor.getSnapshot().value).toBe('idle')); expect(actor.getSnapshot().context.error).toBeUndefined(); }); + it('stays closed when reopened while a cancelled run is still pending', () => { + const run = deferred(); + const actor = start(); + actor.send({ type: 'CONFIRM', run: () => run.promise }); + actor.send({ type: 'CANCEL' }); + + actor.send({ type: 'OPEN' }); + + expect(actor.getSnapshot().value).toBe('pending'); + expect(actor.getSnapshot().context.open).toBe(false); + }); + it('lands back on confirming with the reason when the action fails', async () => { const actor = start(); actor.send({ type: 'CONFIRM', run: () => Promise.reject(blocked) }); @@ -121,4 +144,30 @@ describe('useConfirmController', () => { }); await waitFor(() => expect(result.current.isOpen).toBe(false)); }); + + it('closes on a cancelled reverification but reports pending until the action settles', async () => { + const pending = deferred(); + const onCancel = vi.fn(); + const { result } = renderHook(() => + useConfirmController({ + onConfirm: () => pending.promise, + reverification: { status: 'loading', onCancel, visible: false }, + }), + ); + act(() => result.current.onOpenChange(true)); + act(() => result.current.confirm()); + + act(() => result.current.onOpenChange(false)); + + expect(onCancel).toHaveBeenCalledOnce(); + expect(result.current.isOpen).toBe(false); + expect(result.current.isPending).toBe(true); + + await act(async () => { + pending.reject(cancelled); + await pending.promise.catch(() => undefined); + }); + await waitFor(() => expect(result.current.isPending).toBe(false)); + expect(result.current.isOpen).toBe(false); + }); }); diff --git a/packages/mosaic/src/blocks/confirm/confirm.controller.ts b/packages/mosaic/src/blocks/confirm/confirm.controller.ts index 6924f103b5b..c0df19156fc 100644 --- a/packages/mosaic/src/blocks/confirm/confirm.controller.ts +++ b/packages/mosaic/src/blocks/confirm/confirm.controller.ts @@ -14,6 +14,7 @@ import { toLocalizableError } from '../../utils/errors'; export interface ConfirmContext { run: () => Promise; error: ErrorDescription | undefined; + open: boolean; } export type ConfirmEvent = { type: 'OPEN' } | { type: 'CONFIRM'; run: () => Promise } | { type: 'CANCEL' }; @@ -30,29 +31,32 @@ export const confirmMachine = createMachine({ context: { run: notSeated, error: undefined, + open: false, }, states: { idle: { on: { - OPEN: { target: 'confirming', actions: assign(() => ({ error: undefined })) }, + OPEN: { target: 'confirming', actions: assign(() => ({ error: undefined, open: true })) }, }, }, confirming: { on: { CONFIRM: { target: 'pending', actions: assign((_, event) => ({ run: event.run, error: undefined })) }, - CANCEL: { target: 'idle', actions: assign(() => ({ error: undefined })) }, + CANCEL: { target: 'idle', actions: assign(() => ({ error: undefined, open: false })) }, }, }, pending: { on: { - CANCEL: { target: 'idle', actions: assign(() => ({ error: undefined })) }, + CANCEL: { actions: assign(() => ({ open: false })) }, }, invoke: fromPromise(context => context.run(), { - onDone: { target: 'idle', actions: assign(() => ({ error: undefined })) }, + onDone: { target: 'idle', actions: assign(() => ({ error: undefined, open: false })) }, onError: [ { - guard: (_: ConfirmContext, event: ErrorInvokeEvent) => isReverificationCancelledError(event.error), + guard: (context: ConfirmContext, event: ErrorInvokeEvent) => + !context.open || isReverificationCancelledError(event.error), target: 'idle', + actions: assign(() => ({ open: false })), }, { target: 'confirming', @@ -117,7 +121,7 @@ export function useConfirmController({ return { handle, - isOpen: snapshot.value !== 'idle', + isOpen: snapshot.value !== 'idle' && snapshot.context.open, isPending, canClose, errorMessage: error ? errorText(error, errorFallback) : undefined, From e4ed58d54f847038063d0be1614c2368846ac5fe Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Tue, 3 Nov 2026 19:45:57 -0500 Subject: [PATCH 03/10] refactor(mosaic): patch suggestions on accept and fold one-rule styles --- .changeset/mosaic-small-cleanups.md | 2 ++ .../features/reverification/reverification.styles.ts | 3 +++ .../steps/reverification-method-picker.tsx | 8 +------- .../mosaic/src/features/switcher/switcher.model.tsx | 10 +++++++--- packages/mosaic/src/utils/truncatable-label.tsx | 10 +--------- 5 files changed, 14 insertions(+), 19 deletions(-) create mode 100644 .changeset/mosaic-small-cleanups.md diff --git a/.changeset/mosaic-small-cleanups.md b/.changeset/mosaic-small-cleanups.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/mosaic-small-cleanups.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/mosaic/src/features/reverification/reverification.styles.ts b/packages/mosaic/src/features/reverification/reverification.styles.ts index 506678bc8bb..a3f4ee4b5de 100644 --- a/packages/mosaic/src/features/reverification/reverification.styles.ts +++ b/packages/mosaic/src/features/reverification/reverification.styles.ts @@ -6,6 +6,9 @@ const stackedFooter = '@container card (max-width: 20rem)' as const; const half = `calc(50% - ${space['1']})`; export const styles = stylex.create({ + helpText: { + textAlign: 'center', + }, pending: { paddingBlock: space['12'], alignItems: 'center', diff --git a/packages/mosaic/src/features/reverification/steps/reverification-method-picker.tsx b/packages/mosaic/src/features/reverification/steps/reverification-method-picker.tsx index 97076446acc..1f397897d9c 100644 --- a/packages/mosaic/src/features/reverification/steps/reverification-method-picker.tsx +++ b/packages/mosaic/src/features/reverification/steps/reverification-method-picker.tsx @@ -1,5 +1,4 @@ import { useMergeRefs } from '@floating-ui/react'; -import * as stylex from '@stylexjs/stylex'; import type { Ref } from 'react'; import { Button } from '../../../components/button'; @@ -12,12 +11,7 @@ import { Spinner } from '../../../components/spinner'; import { Text } from '../../../components/text'; import { ChevronRight } from '../../../icons/glyphs/chevron-right'; import { rtl } from '../../../styles/rtl.styles'; - -const styles = stylex.create({ - helpText: { - textAlign: 'center', - }, -}); +import { styles } from '../reverification.styles'; export interface ReverificationMethod { id: string; diff --git a/packages/mosaic/src/features/switcher/switcher.model.tsx b/packages/mosaic/src/features/switcher/switcher.model.tsx index b12cbd6fb3d..93b93b93941 100644 --- a/packages/mosaic/src/features/switcher/switcher.model.tsx +++ b/packages/mosaic/src/features/switcher/switcher.model.tsx @@ -401,11 +401,15 @@ export function useSwitcherModel(options?: SwitcherModelOptions, modals?: Switch onAcceptSuggestion: async suggestionId => { const suggestion = suggestionData.find(s => s.id === suggestionId); try { - await suggestion?.accept(); - } finally { - // We always revalidate, a failed accept might be because of old state + const accepted = await suggestion?.accept(); + if (accepted) { + await userSuggestions.setData?.(pages => replaceInPages(pages ?? [], accepted)); + } + } catch (error) { + // A failed accept might be because of old state // Using allSettled since it never throws and we don't want failed revalidates to look like failed accepts await Promise.allSettled([userSuggestions.revalidate?.()]); + throw error; } }, onAcceptInvitation: async invitationId => { diff --git a/packages/mosaic/src/utils/truncatable-label.tsx b/packages/mosaic/src/utils/truncatable-label.tsx index 86f222e6673..7149501bc7d 100644 --- a/packages/mosaic/src/utils/truncatable-label.tsx +++ b/packages/mosaic/src/utils/truncatable-label.tsx @@ -4,14 +4,6 @@ import React from 'react'; import { reset } from '../styles/reset.styles'; import { truncationStyles } from '../styles/typography.styles'; -// The ellipsis comes from `truncationStyles.singleLine`; this releases the flex-item min-width -// floor, without which the box never shrinks enough to clip. -const styles = stylex.create({ - label: { - minWidth: 0, - }, -}); - // Wrap the text children so they have a box of their own to truncate against — a bare text // child is laid out in an anonymous flex item that no selector can reach. A whole run of // adjacent text shares one box, or `Delete {name}` would split into two flex items with the @@ -28,7 +20,7 @@ export function withTruncatableLabel(children: React.ReactNode): React.ReactNode result.push( {run} , From c5bfad8b51fac59f04335ada3ff88b08e23c38a2 Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Tue, 3 Nov 2026 19:45:59 -0500 Subject: [PATCH 04/10] docs(mosaic): align skills and docs with the code --- .changeset/mosaic-docs-align.md | 2 + .claude/skills/mosaic/SKILL.md | 13 +++-- packages/mosaic/AGENTS.md | 16 ++++++ packages/mosaic/ARCHITECTURE.md | 69 ++++++++++++++----------- packages/mosaic/docs/controllers.md | 39 +++++++------- packages/mosaic/docs/models.md | 20 ++++--- packages/mosaic/docs/skeletons.md | 2 +- packages/mosaic/docs/views.md | 6 +-- packages/mosaic/src/machine/ADOPTION.md | 4 +- packages/mosaic/src/machine/README.md | 6 +-- 10 files changed, 109 insertions(+), 68 deletions(-) create mode 100644 .changeset/mosaic-docs-align.md diff --git a/.changeset/mosaic-docs-align.md b/.changeset/mosaic-docs-align.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/mosaic-docs-align.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/.claude/skills/mosaic/SKILL.md b/.claude/skills/mosaic/SKILL.md index 88844ed63a1..e059b9e1d6c 100644 --- a/.claude/skills/mosaic/SKILL.md +++ b/.claude/skills/mosaic/SKILL.md @@ -63,16 +63,21 @@ canonical contract for the whole design system — the `--cl-*` tokens, the architecture" section that defines the split. Read it for the _what_; this skill is the _how-to_. -`packages/mosaic/src/features/user-button/` is the fullest worked example of the split -in the repo — model, controller, view, wrapper, types, messages, and a feature -test. Copy from it. Tests go in its `__tests__/` folder, as -`packages/mosaic/docs/testing.md` describes under "Test placement". +`packages/mosaic/src/features/switcher/` is the fullest worked example of the split +in the repo — model, controller, view, wrapper, types, layout and styles. Copy from +it. `UserButton` wraps it, and its feature test is in +`packages/mosaic/src/features/user-button/__tests__/`. Tests go in a `__tests__/` +folder, as `packages/mosaic/docs/testing.md` describes under "Test placement". `packages/mosaic/AGENTS.md` loads for any work in the package. It maps common problems (errors, pending state, focus after removal, debouncing, confirm steps) to the shared helper that already solves them, and says to ask the user before adding a new pattern. Check it before writing any of those by hand. +Its "Shaping a fix" section covers bug fixes: fix the shared helper rather than +wrapping it, and treat a changed shared signature as a new pattern. Read it +before any `fix` PR. + ## Which reference to read Use the "Task guides" table in `packages/mosaic/AGENTS.md`. It is the shared diff --git a/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index b94bb4ef1de..aadb5c752fd 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -84,6 +84,22 @@ Stop and check the table if you are writing any of these in a feature: The form rule applies when you add or rewrite a form. Some existing forms predate it. +## Shaping a fix + +A bug in a feature is often a gap in a shared helper. These rules apply to `fix` PRs as much as to features. + +- **Fix the owner, don't wrap it.** When a shared helper (`src/hooks/`, `src/blocks/`, `src/components/`, `src/machine/`, or `@clerk/shared`) behaves wrongly, change the helper and its tests, then delete the workarounds. Do not spread a helper's return value to swap one method, copy a `||` that undoes one of its fields, or add a second hook beside it for one caller. +- **A changed shared signature is a new pattern.** A new parameter, a return type that goes from `void` to a promise, or a returned field whose meaning changes all go through "When nothing fits: ask first". +- **One source of truth for busy.** When a machine, `usePendingAction` or `useForm` already knows an action is running, derive the flag from it: `isDisabled: actions.isPending || remove.isPending`. Do not mirror it into a second lock, or turn machine state into a promise to feed one. +- **Handle staleness through lifecycle.** A new identity remounts through a React `key`. A busy state ignores close (Controllers → Rules). Do not thread run ids or `AbortSignal`s through the model, controller and machine. +- **Extend a contract with an options object.** Make a field required when every caller has it: `connect(strategy, { walletName, onTarget })`, not `connect(strategy, walletName?, onTarget?)`. +- **Pass shared results through whole.** The view gets `form`, not an object that renames `form.id`, `form.isSubmitting` and `form.handleSubmit`. +- **Keep controller state off model types.** Pending and error flags the controller sets go on a row type the controller builds, not as optional fields on the type the model returns. +- **Narrow by mounting.** A hook that needs loaded data takes it non-null. Mount its component once the model is ready instead of checking for `null` inside (Controllers → Composition and dialog lifetime). +- **Machines:** `assign` stays pure. An effect the machine starts runs in `invoke`. The runtime has no spawned actors, so effects that run side by side, like `useForm`'s per-field `validateAsync`, run in the hook and report back by event, and the machine drops stale results. A phase of an action the user can see (queued, pending, failed) is a named state, not a context field plus a predicate. Screens that can sit on top of any state, like reverification's method picker and help, may be a stack in context, because the runtime has no parallel or history states. + +If code or a test contradicts a rule in these docs, do not copy the code. Stop and ask which one is right, and fix the loser in the same PR. + ## Nested radii A part inset from a rounded surface gets its radius from that surface: `max(0px, calc(outer - border - padding))`. Do not pick a fixed step, or it stops matching when `--cl-radius-*` or `--cl-spacing` is overridden. This applies to small insets. When the inset is about as large as the outer radius or larger (Card content, TagInput tags), the part keeps its own radius. diff --git a/packages/mosaic/ARCHITECTURE.md b/packages/mosaic/ARCHITECTURE.md index c9b4c5e9c30..ebf92ef40c7 100644 --- a/packages/mosaic/ARCHITECTURE.md +++ b/packages/mosaic/ARCHITECTURE.md @@ -59,10 +59,15 @@ A theme styles a part from CSS, against that class and its attributes: } ``` -A part exposes no `className` or `style` prop. Inside `@clerk/mosaic`, code that renders a part and needs to nudge it passes StyleX atoms through `xstyle`, declared in its own `stylex.create` and composed last so they win: +A part exposes no `className` or `style` prop. Inside `@clerk/mosaic`, code that renders a part and needs to nudge it passes StyleX atoms through `xstyle`, declared in its own `*.styles.ts` file and composed last so they win: + +```ts +// reverification.styles.ts +export const styles = stylex.create({ helpText: { textAlign: 'center' } }); +``` ```tsx -const styles = stylex.create({ helpText: { textAlign: 'center' } }); +import { styles } from './reverification.styles'; /` and is split by role, one file per layer, prefixed with the feature name. Use `.ts` unless the file renders JSX: ```text -user-button.model.ts(x) // Clerk adapter — the only file that imports Clerk -user-button.controller.ts(x) // local state (React state or a machine) + action wrapping -user-button.view.tsx // rendering only -user-button.tsx // composition wrapper and the public props type +switcher.model.ts(x) // Clerk adapter — the only file that imports Clerk +switcher.controller.ts(x) // local state (React state or a machine) + action wrapping +switcher.view.tsx // rendering only +switcher.tsx // composition wrapper and the public props type ``` Supporting files carry the parts that would otherwise bloat those four: ```text -user-button.types.ts // the data contract the model and view both agree on -user-button.messages.ts // every string the surface renders, as one `as const` object -user-button.layout.ts // pure derivation (which affordance goes in which slot) -user-button.utils.ts // pure helpers -user-button.styles.ts // `stylex.create` atoms (see the StyleX authoring rules) +switcher.types.ts // the data contract the model and view both agree on +switcher.messages.ts // every string the surface renders, as one `as const` object +switcher.layout.ts // pure derivation (which affordance goes in which slot) +switcher.utils.ts // pure helpers +switcher.styles.ts // `stylex.create` atoms (see the StyleX authoring rules) .dialog.tsx // a dialog the view opens, when it would crowd the view file ``` @@ -270,10 +275,10 @@ Two shapes, chosen by whether the slice fetches its own data. wrapper resolves the model, hands it to the controller, and branches on `status`: ```tsx -export function UserButton(props: UserButtonProps = {}) { +export function Switcher(props: SwitcherProps = {}) { const { showTriggerLabel, mode, fallback, ...options } = props; - const model = useUserButtonModel(options); - const controller = useUserButtonController(model, { mode }); + const model = useSwitcherModel(options); + const controller = useSwitcherController(model, { mode }); if (controller.status === 'loading') { return <>{fallback}; @@ -285,7 +290,7 @@ export function UserButton(props: UserButtonProps = {}) { const { status: _status, ...viewController } = controller; return ( - @@ -313,12 +318,12 @@ branches on one value rather than on a scatter of `isLoaded` flags. Every callba it exposes is a plain function over plain ids — never a Clerk resource: ```tsx -export type UserButtonModel = +export type SwitcherModel = | { status: 'loading' } | { status: 'hidden' } - | (UserButtonData & UserButtonCallbacks & { status: 'ready'; organizationsEnabled: boolean }); + | (SwitcherData & SwitcherCallbacks & { status: 'ready'; organizationsEnabled: boolean }); -export function useUserButtonModel(options?: UserButtonModelOptions): UserButtonModel { +export function useSwitcherModel(options?: SwitcherModelOptions): SwitcherModel { const { isLoaded: isUserLoaded, user } = useUser(); const { isLoaded: isSessionLoaded, session } = useSession(); const clerk = useClerk(); @@ -353,19 +358,19 @@ local state, wraps the model's actions to drive pending state, keeps the surface stable while an action runs, and closes the surface on the actions that should close it. -How it holds that state is a complexity call. `UserButton` earns a machine — an +How it holds that state is a complexity call. `Switcher` earns a machine — an action is async, `open` and `pendingKey` constrain each other, and dismissing must not abandon an in-flight invoke — so the machine is declared in the same file with `setup()`, and its context carries the injected effect. Abridged from `features/switcher/switcher.controller.tsx`: ```tsx -const { createMachine, assign, fromPromise } = setup(); +const { createMachine, assign, fromPromise } = setup(); const settled = { pendingKey: null, frozenModel: null }; -const userButtonMachine = createMachine({ - id: 'userButton', +const switcherMachine = createMachine({ + id: 'switcher', initial: 'idle', context: { open: false, pendingKey: null, frozenModel: null, run: () => Promise.resolve(), closeOnSuccess: false }, states: { @@ -400,14 +405,20 @@ const userButtonMachine = createMachine({ }, { target: 'idle', actions: assign(() => settled) }, ], - onError: { target: 'idle', actions: assign(() => settled) }, + onError: { + target: 'idle', + actions: [ + assign(() => settled), + (_, event) => console.error('[Clerk] User button action failed', event.error), + ], + }, }), }, }, }); -export function useUserButtonController(model: UserButtonModel, options = {}): UserButtonController { - const [{ context }, send] = useMachine(userButtonMachine); +export function useSwitcherController(model: SwitcherModel, options = {}): SwitcherController { + const [{ context }, send] = useMachine(switcherMachine); const displayPendingKey = useSpinDelay(context.pendingKey, { delay: 0 }); // While an action runs, render the model it started from, not the live one. @@ -433,7 +444,7 @@ export function useUserButtonController(model: UserButtonModel, options = {}): U open: context.open, onOpenChange: next => send(next ? { type: 'OPEN' } : { type: 'CLOSE' }), pendingKey: displayPendingKey, - onSelectOrganization: runAction(userButtonBusyKeys.selectOrganization, resolvedModel.onSelectOrganization, true), + onSelectOrganization: runAction(switcherBusyKeys.selectOrganization, resolvedModel.onSelectOrganization, true), // … }; } @@ -474,7 +485,7 @@ export function useUserProfileEditUsernameController({ username = '', required = ``` Reaching for a machine here would produce a two-state machine with one event, -which is a boolean spelled long. Reaching for `useState` in `UserButton` would +which is a boolean spelled long. Reaching for `useState` in `Switcher` would produce the flag soup the machine exists to prevent. `src/machine/ADOPTION.md` has the criteria and worked before/afters for the calls in between. @@ -485,7 +496,7 @@ derived — `open`, `pendingKey`, an absent callback — never on a machine snap so the view needs neither the machine nor Clerk: ```tsx -export function UserButtonView({ open, onOpenChange, pendingKey, onSignOutAll, ...data }: UserButtonProps) { +export function SwitcherView({ open, onOpenChange, pendingKey, onSignOutAll, ...data }: SwitcherViewProps) { return ( {m.accounts.signOutAll} diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index 94f05e37503..f07c2c0cf8e 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -65,7 +65,7 @@ error and releases the lock. It is not for: `useConfirmController` earns a machine: the confirm is async, a failure must land back on the confirming step with a reason, and a cancelled reverification must -close without one. `useUserButtonController` earns one because `open` and +close without one. `useSwitcherController` earns one because `open` and `pendingKey` constrain each other and dismissing must not abandon an in-flight invoke. @@ -161,34 +161,35 @@ it. The runtime is documented next to the code: are the full API. Shared async test helpers are in `src/__tests__/async.ts`. -A machine never calls Clerk. The effect it invokes arrives through context as a -plain function, seated by `useMachine(machine, { context })`, which re-seats -context every render so the machine always invokes the latest one. From -`user-profile-rename-passkey.controller.ts`: +A machine never calls Clerk. The effect it runs arrives as a plain function. An +effect that differs per action rides on the event and is assigned to context. +From `blocks/confirm/confirm.controller.ts`: ```ts -saving: { - invoke: fromPromise(async context => context.onRename?.(context.passkeyId, context.name), { - onDone: { target: 'idle' }, +confirming: { + on: { + CONFIRM: { target: 'pending', actions: assign((_, event) => ({ run: event.run, error: undefined })) }, + }, +}, +pending: { + invoke: fromPromise(context => context.run(), { + onDone: { target: 'idle', actions: assign(() => ({ error: undefined, open: false })) }, onError: { - target: 'editing', + target: 'confirming', actions: assign((_, event) => ({ error: toLocalizableError(event.error) })), }, }), -} - -const [snapshot, send] = useMachine(userProfileRenamePasskeyMachine, { - context: { passkeyId: id, savedName: name, onRename }, -}); +}, ``` -An effect that differs per action can ride on the event instead and be assigned -to context, as `confirmationMachine` does with `CONFIRM` -(`blocks/confirmation/confirmation.controller.ts`). +An effect fixed for the controller's lifetime is seated by +`useMachine(machine, { context })`, which re-seats context every render so the +machine always runs the latest one (`onRedirect` in +`features/waitlist/waitlist.controller.ts`). Context holds the `ErrorDescription`, never copy (see "Errors"). A machine whose surface shows no error still logs it, so a failure is never silent -(`user-button.controller.tsx`). +(`features/switcher/switcher.controller.tsx`). ## Responsibilities @@ -334,7 +335,7 @@ One path, so every error a user sees goes through the `errors.*` catalog: dialogs), or `useForm` (forms). The first two call `toLocalizableError(cause)` from `utils/errors.ts`. Use one of them instead of catching in a feature. A controller with its own machine maps in the machine's - `onError` (`user-profile-rename-passkey.controller.ts`). + `onError` (`organization-list.controller.ts`). - **Machines that display all errors use `toLocalizableErrors(cause)`.** It preserves each error and its parameter name. Reverification stores this array, routes matching parameters to the active field and everything else to the diff --git a/packages/mosaic/docs/models.md b/packages/mosaic/docs/models.md index 545dfb49553..4fca659dae8 100644 --- a/packages/mosaic/docs/models.md +++ b/packages/mosaic/docs/models.md @@ -14,10 +14,10 @@ A discriminated union on `status`, so consumers branch on one value instead of a scatter of `isLoaded` flags: ```tsx -export type UserButtonModel = +export type SwitcherModel = | { status: 'loading' } | { status: 'hidden' } - | (UserButtonData & UserButtonCallbacks & { status: 'ready'; organizationsEnabled: boolean }); + | (SwitcherData & SwitcherCallbacks & { status: 'ready'; organizationsEnabled: boolean }); ``` `hidden` is an **answer**, not an absence: signed out is settled, so the wrapper @@ -53,17 +53,23 @@ drops the fallback instead of holding the space open. Keep the two apart. The view hides the affordance an absent callback drives, so the model never has to describe UI. -- **Own revalidate timing.** Call `.revalidate()` / `.reload()` after a mutation, - from inside the callback that made it. Deciding _when_ is model logic: +- **Own cache updates.** Update the cache after a mutation, from inside the + callback that made it. Deciding _how_ is model logic: patch the returned + resource in with `replaceInPages` on success, and revalidate on failure, since + a failed mutation might mean stale state: ```tsx onAcceptSuggestion: async suggestionId => { try { - await suggestionData.find(s => s.id === suggestionId)?.accept(); - } finally { - // Always revalidate — a failed accept might be stale state. allSettled never throws, + const accepted = await suggestionData.find(s => s.id === suggestionId)?.accept(); + if (accepted) { + await userSuggestions.setData?.(pages => replaceInPages(pages ?? [], accepted)); + } + } catch (error) { + // A failed accept might be stale state. allSettled never throws, // so a failed revalidate doesn't look like a failed accept. await Promise.allSettled([userSuggestions.revalidate?.()]); + throw error; } }, ``` diff --git a/packages/mosaic/docs/skeletons.md b/packages/mosaic/docs/skeletons.md index 0bb0bfe3300..a605af4ab72 100644 --- a/packages/mosaic/docs/skeletons.md +++ b/packages/mosaic/docs/skeletons.md @@ -109,7 +109,7 @@ peaking in the middle, sweeps left to right across two bone widths, over 1.6s Not decided by the skeleton. The component that owns the loading state shows it for as long as the data is loading. The planned gate is -`useSpinDelay(loading, { delay: 150, minDuration: 500 })` with the skeleton mounted +`useSpinDelay(loading || null, { delay: 150, minDuration: 500 }) !== null` with the skeleton mounted but `visibility: hidden` until the delay passes, so a fast load never flashes it and nothing shifts when it appears. diff --git a/packages/mosaic/docs/views.md b/packages/mosaic/docs/views.md index bc5c08a00d9..3025faadcaf 100644 --- a/packages/mosaic/docs/views.md +++ b/packages/mosaic/docs/views.md @@ -13,7 +13,7 @@ The view renders plain props and calls plain callbacks. Nothing else. machine guard or works out for itself whether an action is allowed. ```tsx -export function UserButtonView({ open, onOpenChange, pendingKey, onSignOutAll, ...data }: UserButtonProps) { +export function SwitcherView({ open, onOpenChange, pendingKey, onSignOutAll, ...data }: SwitcherViewProps) { return ( {m.accounts.signOutAll} @@ -37,7 +37,7 @@ export function UserButtonView({ open, onOpenChange, pendingKey, onSignOutAll, . Two shapes, chosen by whether the slice fetches its own data: - A **wrapper composes** model + controller + view, and the view is a pure - function of props (`user-button.tsx`). + function of props (`switcher.tsx`). - A **leaf view owns its controller** and takes the effect as a prop (`UserProfileUsernameRowView` calls `useUserProfileEditUsernameController` with its `onSubmit`). Still no Clerk — the effect arrives from above. diff --git a/packages/mosaic/src/machine/ADOPTION.md b/packages/mosaic/src/machine/ADOPTION.md index dee1600a84c..710a375ff47 100644 --- a/packages/mosaic/src/machine/ADOPTION.md +++ b/packages/mosaic/src/machine/ADOPTION.md @@ -794,8 +794,8 @@ collocated with the value that drives it. The controllers that already made this call are the best reference: -- `blocks/confirmation/confirmation.controller.ts`: a machine -- `blocks/destructive/destructive.controller.ts`: one `useState` holding a status union +- `blocks/confirm/confirm.controller.ts`: a machine +- `features/user-profile/user-profile-profile-section/user-profile-edit-username.controller.ts`: one `useState` beside `useForm` - `features/switcher/switcher.controller.tsx`: a machine - `hooks/use-pending-action.ts`: keyed `useState` plus a lock diff --git a/packages/mosaic/src/machine/README.md b/packages/mosaic/src/machine/README.md index 6bad81cb237..f4205427a94 100644 --- a/packages/mosaic/src/machine/README.md +++ b/packages/mosaic/src/machine/README.md @@ -83,7 +83,7 @@ const loader = createMachine({ }, onError: { target: 'failure', - actions: assign((_ctx, event) => ({ error: String(event.error) })), + actions: assign((_ctx, event) => ({ error: toLocalizableError(event.error) })), }, }), }, @@ -137,7 +137,7 @@ const loader = createMachine({ }, onError: { target: 'failure', - actions: assign((_, e) => ({ error: String(e.error) })), // e: ErrorInvokeEvent + actions: assign((_, e) => ({ error: toLocalizableError(e.error) })), // e: ErrorInvokeEvent }, }), }, @@ -250,7 +250,7 @@ you can't easily click to (a `deleting` state hidden behind a 2-second mutation, a guard-gated wizard step), `mockActor` drops you straight in: ```ts -const actor = mockActor(loader, { value: 'failure', context: { error: 'Network error' } }); +const actor = mockActor(loader, { value: 'failure', context: { error: { cause: new Error('Network error') } } }); render(); // snapshot a state you'd otherwise have to provoke ``` From 9fc5b1f445e4921c45b08b2ab760452e1d34cc7f Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Tue, 3 Nov 2026 20:00:29 -0500 Subject: [PATCH 05/10] docs(mosaic): use a generic prefix for supporting feature files --- packages/mosaic/ARCHITECTURE.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/mosaic/ARCHITECTURE.md b/packages/mosaic/ARCHITECTURE.md index ebf92ef40c7..a743247fff0 100644 --- a/packages/mosaic/ARCHITECTURE.md +++ b/packages/mosaic/ARCHITECTURE.md @@ -201,11 +201,11 @@ switcher.tsx // composition wrapper and the public props type Supporting files carry the parts that would otherwise bloat those four: ```text -switcher.types.ts // the data contract the model and view both agree on -switcher.messages.ts // every string the surface renders, as one `as const` object -switcher.layout.ts // pure derivation (which affordance goes in which slot) -switcher.utils.ts // pure helpers -switcher.styles.ts // `stylex.create` atoms (see the StyleX authoring rules) +.types.ts // the data contract the model and view both agree on +.messages.ts // every string the surface renders, as one `as const` object +.layout.ts // pure derivation (which affordance goes in which slot) +.utils.ts // pure helpers +.styles.ts // `stylex.create` atoms (see the StyleX authoring rules) .dialog.tsx // a dialog the view opens, when it would crowd the view file ``` From c1cd382d91ddae5e5a12996b998e2878fd15379f Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Tue, 3 Nov 2026 20:10:19 -0500 Subject: [PATCH 06/10] docs(mosaic): remove the mosaic skill and keep its rules in AGENTS.md --- .changeset/mosaic-remove-skill.md | 2 + .claude/skills/mosaic/SKILL.md | 144 ------------------------------ packages/mosaic/AGENTS.md | 18 ++++ packages/mosaic/docs/migration.md | 4 +- 4 files changed, 22 insertions(+), 146 deletions(-) create mode 100644 .changeset/mosaic-remove-skill.md delete mode 100644 .claude/skills/mosaic/SKILL.md diff --git a/.changeset/mosaic-remove-skill.md b/.changeset/mosaic-remove-skill.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/mosaic-remove-skill.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/.claude/skills/mosaic/SKILL.md b/.claude/skills/mosaic/SKILL.md deleted file mode 100644 index e059b9e1d6c..00000000000 --- a/.claude/skills/mosaic/SKILL.md +++ /dev/null @@ -1,144 +0,0 @@ ---- -name: mosaic -description: >- - Work on Mosaic UI: styling a component with StyleX (`stylex.create`, `--cl-*` - tokens, `themeProps`), or building a flow — writing the model (the Clerk - adapter), the controller (local state, in React state or a state machine: - `setup`, states/guards/`invoke`, wired to React with `useMachine`/`useActor`/ - `useSelector`), or the view (rendering, including moving between screens with - the `Flow` component), testing a feature, or - migrating a legacy / pre-Mosaic component into the model / controller / view - split. Use when building, styling, debugging, testing, or migrating anything - Mosaic. `packages/mosaic/ARCHITECTURE.md` holds the design-system - contract; this skill is the how-to layer. ---- - -# Mosaic UI - -Two things live under Mosaic, and this skill covers the how-to for both: - -- **Styled components** are authored with **StyleX** — `stylex.create` declares - the styles, `themeProps` emits the part's public identity (the `.cl-` - class plus `data-` attrs), and `mergeStyleProps` fuses the two with the - props the part was called with. A part takes `xstyle` (StyleX atoms for its - root), never `className`/`style`. -- **Flows** follow a **model → controller → view** split — _where the data comes - from_ → _what the user is doing to it_ → _what that looks like_. What crosses - each boundary is plain data: no Clerk resource reaches the controller, no - machine snapshot reaches the view. - -"Flow" means two things here. A feature's model, controller and view are its -flow. `Flow` from `src/components/flow/` is the component that moves between -screens on one card or dialog. If a screen replaces another on the same surface, -use the component: a conditional makes the new screen pop in. See "Moving between -screens on one surface" in `packages/mosaic/docs/views.md`. - -```text -model Clerk adapter: reads Clerk hooks and resources, resolves the - environment, gates permissions, and answers with plain data plus - plain callbacks under an explicit `status`. The only layer that may - import Clerk hooks or call Clerk resource methods. - -controller Local state: what is open, what is in flight, what the view may do - next — held in React state or a state machine, whichever the - interaction's complexity calls for. Wraps the model's callbacks so - an action can report pending, hold the surface still while it runs, - and close on success. No Clerk imports. - -view Rendering: takes plain props and callbacks, renders UI, calls them - back. No Clerk imports. No data-fetching. No machine snapshot. -``` - -A machine is **not a fourth layer**, and not a requirement. It is one of the two -ways a controller can hold its state, and picking one is a complexity call: -`useState` for a boolean that never touches async, a machine once the -interaction has an async lifecycle or two values that must change together, and -sometimes both in one controller. Either way the controller returns plain props, -so the view cannot tell and neither can its tests. Criteria and worked -before/afters: `packages/mosaic/src/machine/ADOPTION.md`. - -`packages/mosaic/ARCHITECTURE.md` is the -canonical contract for the whole design system — the `--cl-*` tokens, the -`.cl-` + `data-` styling API, the CSS build, and the "Flow and data -architecture" section that defines the split. Read it for the _what_; this skill -is the _how-to_. - -`packages/mosaic/src/features/switcher/` is the fullest worked example of the split -in the repo — model, controller, view, wrapper, types, layout and styles. Copy from -it. `UserButton` wraps it, and its feature test is in -`packages/mosaic/src/features/user-button/__tests__/`. Tests go in a `__tests__/` -folder, as `packages/mosaic/docs/testing.md` describes under "Test placement". - -`packages/mosaic/AGENTS.md` loads for any work in the package. It maps common -problems (errors, pending state, focus after removal, debouncing, confirm steps) -to the shared helper that already solves them, and says to ask the user before -adding a new pattern. Check it before writing any of those by hand. - -Its "Shaping a fix" section covers bug fixes: fix the shared helper rather than -wrapping it, and treat a changed shared signature as a new pattern. Read it -before any `fix` PR. - -## Which reference to read - -Use the "Task guides" table in `packages/mosaic/AGENTS.md`. It is the shared -task index for agents, including those that do not discover this skill. Read -the guide for the layer or behavior you are changing. - -The migration workflow (`packages/mosaic/docs/migration.md`) ties the flow references together: it -treats the legacy component as the spec and drives you through the model, -controller, and view layers, then verifies parity with `parity-audit.md`. - -## Breaking changes - -While `@clerk/mosaic` is on 0.x, a breaking change is a `minor` changeset. Start -its body with `**Breaking:**`, then give an old-to-new table and a search that -finds affected usage, so an agent can migrate from the changelog alone. The -changelog ships in the npm package at -`node_modules/@clerk/mosaic/CHANGELOG.md`. - -```md ---- -'@clerk/mosaic': minor ---- - -**Breaking:** Rename a `UserButton` prop. Values and defaults are unchanged; TypeScript flags the old name. - -| Before | After | -| -------------------- | ------------------ | -| `renderTriggerLabel` | `showTriggerLabel` | - -Find usages with `git grep -n renderTriggerLabel`. -``` - -## Documenting a component in swingset - -`packages/swingset/CLAUDE.md` is the house style — archetypes, required section -order, `meta` conventions. One rule on top of it, because it is the one agents -get wrong: - -**The docs describe the API as it is. They do not carry the reasoning that -produced it.** No "not a `size`, because…", no rejected alternatives, no history -of what the prop used to be. A reader is there to learn what the thing does, and -every sentence of rationale is a sentence they have to skim past to find it. -State the behaviour plainly and briefly, then stop. - -```mdx - - -### Variant - -Which surface the dialog holds, and so the geometry it is given. Not a `size`, -because these are different surfaces rather than one surface at two widths — a -second card width would be a size of the `card` variant, with nowhere to sit on -this axis. - - - -### Variant - -Which surface the dialog holds, and the geometry that comes with it. -``` - -Follow the root `AGENTS.md` rule for code comments. Put implementation rationale -and rejected alternatives in the PR description. Component docs describe the -resulting API. diff --git a/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index aadb5c752fd..0deb7c80030 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -133,6 +133,24 @@ Docs say what the code cannot: rules, reasons, and which tool fits which problem A PR that adds, renames or changes the signature of a shared helper in `src/hooks/`, `src/utils/`, `src/blocks/`, `src/components/form/` or `src/machine/` updates the table above, and any doc that names it, in the same PR. +## Breaking changes + +While `@clerk/mosaic` is on 0.x, a breaking change is a `minor` changeset. Start its body with `**Breaking:**`, then give an old-to-new table and a search that finds affected usage, so an agent can migrate from the changelog alone. The changelog ships in the npm package at `node_modules/@clerk/mosaic/CHANGELOG.md`. + +```md +--- +'@clerk/mosaic': minor +--- + +**Breaking:** Rename a `UserButton` prop. Values and defaults are unchanged; TypeScript flags the old name. + +| Before | After | +| -------------------- | ------------------ | +| `renderTriggerLabel` | `showTriggerLabel` | + +Find usages with `git grep -n renderTriggerLabel`. +``` + ## Lint Mosaic runs stricter lint rules than the rest of the repo: the `packages/mosaic` block in the root `eslint.config.mjs`, plus the layer boundaries, test rules and effect rules in `packages/mosaic/eslint/config.mjs`, which only load through `packages/mosaic/eslint.config.mjs`. Lint with `pnpm lint` from `packages/mosaic`. Running `eslint` from the repo root skips them. diff --git a/packages/mosaic/docs/migration.md b/packages/mosaic/docs/migration.md index 83e46a9edbc..e71beb3c410 100644 --- a/packages/mosaic/docs/migration.md +++ b/packages/mosaic/docs/migration.md @@ -1,8 +1,8 @@ # Migrating a component into Mosaic Migrating a legacy component means taking logic that was fused into one file and -pulling it apart into the model / controller / view layers (see the skill -overview and `packages/mosaic/ARCHITECTURE.md` → "Flow and data architecture"). +pulling it apart into the model / controller / view layers (see +`packages/mosaic/ARCHITECTURE.md` → "Flow and data architecture"). `packages/mosaic/src/features/reverification/` is a compact worked example of the finished shape: model, controller, view, types, messages, wrapper, and a feature test. From d703838235452876bac89ed5efb92012ec0f9a0f Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 4 Nov 2026 11:55:27 -0700 Subject: [PATCH 07/10] docs(mosaic): make agents read the legacy component and run the parity audit for ports Ports of @clerk/react and @clerk/ui components are migrations. A PreToolUse hook blocks writing a new feature model until its legacy namesake has been read, and a Stop hook blocks finishing once until the parity audit subagent has run. AGENTS.md, migration.md and parity-audit.md say the same for agents that don't run Claude hooks. --- .changeset/mosaic-parity-gates.md | 2 + .claude/hooks/mosaic-hook-utils.mjs | 41 ++++++++++++++++++ .claude/hooks/require-mosaic-legacy-read.mjs | 41 ++++++++++++++++++ .claude/hooks/require-mosaic-parity-audit.mjs | 43 +++++++++++++++++++ .claude/settings.json | 17 ++++++++ packages/mosaic/AGENTS.md | 9 ++++ packages/mosaic/docs/migration.md | 6 ++- packages/mosaic/docs/parity-audit.md | 5 +++ 8 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 .changeset/mosaic-parity-gates.md create mode 100644 .claude/hooks/mosaic-hook-utils.mjs create mode 100644 .claude/hooks/require-mosaic-legacy-read.mjs create mode 100644 .claude/hooks/require-mosaic-parity-audit.mjs diff --git a/.changeset/mosaic-parity-gates.md b/.changeset/mosaic-parity-gates.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/mosaic-parity-gates.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/.claude/hooks/mosaic-hook-utils.mjs b/.claude/hooks/mosaic-hook-utils.mjs new file mode 100644 index 00000000000..0005625964d --- /dev/null +++ b/.claude/hooks/mosaic-hook-utils.mjs @@ -0,0 +1,41 @@ +import { existsSync, readFileSync } from 'node:fs'; +import { join } from 'node:path'; + +export function read(path) { + try { + return readFileSync(path, 'utf8'); + } catch { + return ''; + } +} + +function parse(text) { + try { + return JSON.parse(text); + } catch { + return undefined; + } +} + +export function readInput() { + return parse(read(0)) ?? {}; +} + +export function toolCalls(transcript) { + return transcript.split('\n').flatMap(line => { + const entry = parse(line); + const content = entry?.type === 'assistant' ? entry.message?.content : undefined; + return Array.isArray(content) ? content.filter(part => part?.type === 'tool_use') : []; + }); +} + +function pascalCase(kebab) { + return kebab.replace(/(^|-)([a-z0-9])/g, (_, __, char) => char.toUpperCase()); +} + +export function legacyCounterparts(repoRoot, feature) { + const name = pascalCase(feature); + return [join('packages/react/src/components', `${name}.tsx`), join('packages/ui/src/components', name)].filter(path => + existsSync(join(repoRoot, path)), + ); +} diff --git a/.claude/hooks/require-mosaic-legacy-read.mjs b/.claude/hooks/require-mosaic-legacy-read.mjs new file mode 100644 index 00000000000..9c8e27b32ac --- /dev/null +++ b/.claude/hooks/require-mosaic-legacy-read.mjs @@ -0,0 +1,41 @@ +import { existsSync } from 'node:fs'; +import { basename } from 'node:path'; + +import { legacyCounterparts, read, readInput, toolCalls } from './mosaic-hook-utils.mjs'; + +const input = readInput(); +const filePath = input.tool_input?.file_path ?? ''; +const newModel = filePath.match(/^(.*)\/packages\/mosaic\/src\/features\/([a-z0-9-]+)\/\2\.model\.tsx?$/); + +if (input.tool_name !== 'Write' || !newModel || existsSync(filePath)) { + process.exit(0); +} + +const transcript = read(input.agent_transcript_path || input.transcript_path || ''); + +if (!transcript) { + process.exit(0); +} + +const touched = toolCalls(transcript) + .filter(call => call.name !== 'Agent') + .map(call => JSON.stringify(call.input ?? {})); +const unread = legacyCounterparts(newModel[1], newModel[2]).filter( + path => !touched.some(input => input.includes(path)), +); + +if (unread.length > 0) { + process.stdout.write( + JSON.stringify({ + hookSpecificOutput: { + hookEventName: 'PreToolUse', + permissionDecision: 'deny', + permissionDecisionReason: [ + `\`${basename(filePath)}\` has a legacy counterpart: ${unread.join(', ')}.`, + 'This is a migration. Read the counterpart and write the Phase 1 behavior inventory from packages/mosaic/docs/migration.md', + '(one row per prop, default, guard, error path, loading state and click behavior) before writing the model, then retry.', + ].join(' '), + }, + }), + ); +} diff --git a/.claude/hooks/require-mosaic-parity-audit.mjs b/.claude/hooks/require-mosaic-parity-audit.mjs new file mode 100644 index 00000000000..5de327b6406 --- /dev/null +++ b/.claude/hooks/require-mosaic-parity-audit.mjs @@ -0,0 +1,43 @@ +import { legacyCounterparts, read, readInput, toolCalls } from './mosaic-hook-utils.mjs'; + +const input = readInput(); + +if (input.stop_hook_active) { + process.exit(0); +} + +const calls = toolCalls(read(input.transcript_path || '')); +const features = new Map(); + +for (const call of calls) { + if (!['Write', 'Edit', 'MultiEdit'].includes(call.name)) { + continue; + } + const match = String(call.input?.file_path ?? '').match(/^(.*)\/packages\/mosaic\/src\/features\/([a-z0-9-]+)\//); + if (match) { + features.set(match[2], match[1]); + } +} + +const migrations = [...features].flatMap(([feature, repoRoot]) => + legacyCounterparts(repoRoot, feature).map(legacy => ({ feature, legacy })), +); + +const audited = calls.some(call => call.name === 'Agent' && /parity/i.test(JSON.stringify(call.input ?? {}))); + +if (migrations.length === 0 || audited) { + process.exit(0); +} + +const list = migrations.map(({ feature, legacy }) => `${feature} (legacy: ${legacy})`).join(', '); + +process.stdout.write( + JSON.stringify({ + decision: 'block', + reason: [ + `This session changed Mosaic features with a legacy counterpart: ${list}.`, + 'Before finishing, launch the Explore subagent with the prompt in packages/mosaic/docs/parity-audit.md.', + 'Fix or raise with the user every row it reports as breaking a Mosaic rule, and list the remaining differences in the PR description.', + ].join(' '), + }), +); diff --git a/.claude/settings.json b/.claude/settings.json index d6148f47f42..f5e032091e2 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -35,6 +35,18 @@ ] } ], + "PreToolUse": [ + { + "matcher": "Write", + "hooks": [ + { + "type": "command", + "command": "node \"$CLAUDE_PROJECT_DIR/.claude/hooks/require-mosaic-legacy-read.mjs\"", + "timeout": 10 + } + ] + } + ], "Stop": [ { "hooks": [ @@ -47,6 +59,11 @@ "type": "command", "command": "cd \"$(git rev-parse --show-toplevel)\" && { git diff --name-only --diff-filter=d HEAD -- packages/swingset/src; git ls-files --others --exclude-standard -- packages/swingset/src; } | grep -E '\\.tsx?$' | xargs -r pnpm exec eslint --quiet --no-warn-ignored --rule 'import/no-unresolved: off' >&2 || exit 2", "timeout": 180 + }, + { + "type": "command", + "command": "node \"$CLAUDE_PROJECT_DIR/.claude/hooks/require-mosaic-parity-audit.mjs\"", + "timeout": 10 } ] } diff --git a/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index 0deb7c80030..c1df7bd339d 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -24,6 +24,15 @@ Read the guide for the task you are doing. Paths below are relative to this pack | Review migration parity | [Parity audit](docs/parity-audit.md) | | Add a public prop that shows or hides UI | [Public visibility props](docs/public-props.md) | +## Ports of legacy components are migrations + +Before you write a new feature, search `packages/react/src/components/` and `packages/ui/src/components/` for its counterpart. If one exists, the work is a [migration](docs/migration.md), including a headless port of a `@clerk/react` button framed as a new feature: + +1. Read the counterpart and write the Phase 1 inventory before the model. +2. Before you call the work done, review it, or open a PR, launch the Explore subagent with the prompt in [Parity audit](docs/parity-audit.md). Reading the doc is not running the audit. +3. List every **Deliberately changed** and **Deferred** row in the PR description. A difference that is not listed is a regression. +4. Listing a difference does not make it acceptable. If one breaks a rule in this file or in `docs/`, such as a failure that nobody sees, fix it or ask the user before you finish. Do not label it deliberate. + ## Layers in one paragraph A flow lives in `src/features//`. The **model** (`*.model.ts(x)`) is the only file that reads Clerk hooks or calls Clerk resources, and answers with plain data, plain callbacks and a `status`. The **controller** (`*.controller.ts(x)`) holds local state and wraps the model's callbacks for pending, errors and closing. The **view** (`*.view.tsx`) renders plain props. No Clerk resource reaches the controller and no machine snapshot reaches the view. diff --git a/packages/mosaic/docs/migration.md b/packages/mosaic/docs/migration.md index e71beb3c410..41eff902a08 100644 --- a/packages/mosaic/docs/migration.md +++ b/packages/mosaic/docs/migration.md @@ -22,8 +22,10 @@ make each layer account for a specific row. ## Phase 1 — Inventory the legacy behavior (the spec) -Locate the legacy files (usually under `packages/ui/src/components//`). -Then grep them for the primitives that carry hidden logic. Every +Locate the legacy files: the prebuilt component under +`packages/ui/src/components//`, the `@clerk/react` component under +`packages/react/src/components/`, or both. A headless port of a `@clerk/react` +button is a migration too. Then grep them for the primitives that carry hidden logic. Every hit is a row you must consciously place or drop later: | Primitive | The behavior it usually hides | diff --git a/packages/mosaic/docs/parity-audit.md b/packages/mosaic/docs/parity-audit.md index 93dbd7b9fdf..5ba7980ac57 100644 --- a/packages/mosaic/docs/parity-audit.md +++ b/packages/mosaic/docs/parity-audit.md @@ -36,6 +36,11 @@ Quote ONLY the relevant branch from each file (the conditional, the effect, the guard). Never dump whole files. Return the result as the table below, most severe gaps first, then a short list of any Deferred items that are tracked only by a `// TODO` (these are the regressions at risk of shipping). + +Then check every row that is not Migrated against packages/mosaic/AGENTS.md and +packages/mosaic/docs/. List each one that breaks a rule there, with the rule it +breaks, such as a failure that is never shown or logged. These must be fixed or +raised with the user; they cannot ship as Deliberately changed. ``` ## Output table format From 7317272cd816dba5978249f0d21986c5703563a0 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 4 Nov 2026 12:37:56 -0700 Subject: [PATCH 08/10] docs(mosaic): count only real reads and require an audit per ported feature The legacy-read hook only counts Read, Grep, Glob and Bash calls, so a write that mentions the legacy path no longer passes. The Stop hook requires a parity audit that names each ported feature. The Phase 1 search command in migration.md and parity-audit.md now includes the @clerk/react component. --- .claude/hooks/require-mosaic-legacy-read.mjs | 15 +++++++++++---- .claude/hooks/require-mosaic-parity-audit.mjs | 12 +++++++++--- packages/mosaic/docs/migration.md | 2 +- packages/mosaic/docs/parity-audit.md | 2 +- 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/.claude/hooks/require-mosaic-legacy-read.mjs b/.claude/hooks/require-mosaic-legacy-read.mjs index 9c8e27b32ac..0f6ec630b33 100644 --- a/.claude/hooks/require-mosaic-legacy-read.mjs +++ b/.claude/hooks/require-mosaic-legacy-read.mjs @@ -17,11 +17,18 @@ if (!transcript) { process.exit(0); } -const touched = toolCalls(transcript) - .filter(call => call.name !== 'Agent') - .map(call => JSON.stringify(call.input ?? {})); +const READ_TARGETS = { + Read: input => input.file_path, + Grep: input => input.path, + Glob: input => `${input.path ?? ''}/${input.pattern ?? ''}`, + Bash: input => input.command, +}; + +const reads = toolCalls(transcript) + .filter(call => call.name in READ_TARGETS) + .map(call => String(READ_TARGETS[call.name](call.input ?? {}) ?? '')); const unread = legacyCounterparts(newModel[1], newModel[2]).filter( - path => !touched.some(input => input.includes(path)), + path => !reads.some(target => target.includes(path)), ); if (unread.length > 0) { diff --git a/.claude/hooks/require-mosaic-parity-audit.mjs b/.claude/hooks/require-mosaic-parity-audit.mjs index 5de327b6406..61f8910eda4 100644 --- a/.claude/hooks/require-mosaic-parity-audit.mjs +++ b/.claude/hooks/require-mosaic-parity-audit.mjs @@ -23,13 +23,19 @@ const migrations = [...features].flatMap(([feature, repoRoot]) => legacyCounterparts(repoRoot, feature).map(legacy => ({ feature, legacy })), ); -const audited = calls.some(call => call.name === 'Agent' && /parity/i.test(JSON.stringify(call.input ?? {}))); +const audits = calls + .filter(call => call.name === 'Agent') + .map(call => JSON.stringify(call.input ?? {})) + .filter(input => /parity/i.test(input)); +const unaudited = migrations.filter( + ({ feature, legacy }) => !audits.some(input => input.includes(legacy) || input.includes(`/features/${feature}/`)), +); -if (migrations.length === 0 || audited) { +if (unaudited.length === 0) { process.exit(0); } -const list = migrations.map(({ feature, legacy }) => `${feature} (legacy: ${legacy})`).join(', '); +const list = unaudited.map(({ feature, legacy }) => `${feature} (legacy: ${legacy})`).join(', '); process.stdout.write( JSON.stringify({ diff --git a/packages/mosaic/docs/migration.md b/packages/mosaic/docs/migration.md index 41eff902a08..1fb86345aab 100644 --- a/packages/mosaic/docs/migration.md +++ b/packages/mosaic/docs/migration.md @@ -47,7 +47,7 @@ implementation in Phase 2. ```bash rg -n 'useEffect|handleError|card\.setError|useReverification|revalidate|/ + packages/ui/src/components// packages/react/src/components/.tsx ``` Turn the hits into a **behavior inventory** — one row per: effect, guard, error diff --git a/packages/mosaic/docs/parity-audit.md b/packages/mosaic/docs/parity-audit.md index 5ba7980ac57..6addf828116 100644 --- a/packages/mosaic/docs/parity-audit.md +++ b/packages/mosaic/docs/parity-audit.md @@ -55,7 +55,7 @@ raised with the user; they cannot ship as Deliberately changed. ```bash rg -n 'useEffect|handleError|card\.setError|useReverification|revalidate|/ + packages/ui/src/components// packages/react/src/components/.tsx ``` ## Worked example — `OrganizationProfileDomainsSection` From d93517f8cfc193765ad661e9fb30ff50c0b36433 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 4 Nov 2026 14:28:52 -0700 Subject: [PATCH 09/10] docs(mosaic): check parity with a proof file that Claude and Codex hooks can both read --- .claude/hooks/mosaic-hook-utils.mjs | 41 ---- .claude/hooks/require-mosaic-legacy-read.mjs | 48 ----- .claude/hooks/require-mosaic-parity-audit.mjs | 49 ----- .claude/settings.json | 6 +- .codex/hooks.json | 17 ++ packages/mosaic/AGENTS.md | 6 +- packages/mosaic/docs/migration.md | 21 +- packages/mosaic/docs/parity-audit.md | 7 +- scripts/mosaic-parity-gate.mjs | 184 ++++++++++++++++++ scripts/mosaic-parity-gate.test.mjs | 56 ++++++ 10 files changed, 285 insertions(+), 150 deletions(-) delete mode 100644 .claude/hooks/mosaic-hook-utils.mjs delete mode 100644 .claude/hooks/require-mosaic-legacy-read.mjs delete mode 100644 .claude/hooks/require-mosaic-parity-audit.mjs create mode 100644 scripts/mosaic-parity-gate.mjs create mode 100644 scripts/mosaic-parity-gate.test.mjs diff --git a/.claude/hooks/mosaic-hook-utils.mjs b/.claude/hooks/mosaic-hook-utils.mjs deleted file mode 100644 index 0005625964d..00000000000 --- a/.claude/hooks/mosaic-hook-utils.mjs +++ /dev/null @@ -1,41 +0,0 @@ -import { existsSync, readFileSync } from 'node:fs'; -import { join } from 'node:path'; - -export function read(path) { - try { - return readFileSync(path, 'utf8'); - } catch { - return ''; - } -} - -function parse(text) { - try { - return JSON.parse(text); - } catch { - return undefined; - } -} - -export function readInput() { - return parse(read(0)) ?? {}; -} - -export function toolCalls(transcript) { - return transcript.split('\n').flatMap(line => { - const entry = parse(line); - const content = entry?.type === 'assistant' ? entry.message?.content : undefined; - return Array.isArray(content) ? content.filter(part => part?.type === 'tool_use') : []; - }); -} - -function pascalCase(kebab) { - return kebab.replace(/(^|-)([a-z0-9])/g, (_, __, char) => char.toUpperCase()); -} - -export function legacyCounterparts(repoRoot, feature) { - const name = pascalCase(feature); - return [join('packages/react/src/components', `${name}.tsx`), join('packages/ui/src/components', name)].filter(path => - existsSync(join(repoRoot, path)), - ); -} diff --git a/.claude/hooks/require-mosaic-legacy-read.mjs b/.claude/hooks/require-mosaic-legacy-read.mjs deleted file mode 100644 index 0f6ec630b33..00000000000 --- a/.claude/hooks/require-mosaic-legacy-read.mjs +++ /dev/null @@ -1,48 +0,0 @@ -import { existsSync } from 'node:fs'; -import { basename } from 'node:path'; - -import { legacyCounterparts, read, readInput, toolCalls } from './mosaic-hook-utils.mjs'; - -const input = readInput(); -const filePath = input.tool_input?.file_path ?? ''; -const newModel = filePath.match(/^(.*)\/packages\/mosaic\/src\/features\/([a-z0-9-]+)\/\2\.model\.tsx?$/); - -if (input.tool_name !== 'Write' || !newModel || existsSync(filePath)) { - process.exit(0); -} - -const transcript = read(input.agent_transcript_path || input.transcript_path || ''); - -if (!transcript) { - process.exit(0); -} - -const READ_TARGETS = { - Read: input => input.file_path, - Grep: input => input.path, - Glob: input => `${input.path ?? ''}/${input.pattern ?? ''}`, - Bash: input => input.command, -}; - -const reads = toolCalls(transcript) - .filter(call => call.name in READ_TARGETS) - .map(call => String(READ_TARGETS[call.name](call.input ?? {}) ?? '')); -const unread = legacyCounterparts(newModel[1], newModel[2]).filter( - path => !reads.some(target => target.includes(path)), -); - -if (unread.length > 0) { - process.stdout.write( - JSON.stringify({ - hookSpecificOutput: { - hookEventName: 'PreToolUse', - permissionDecision: 'deny', - permissionDecisionReason: [ - `\`${basename(filePath)}\` has a legacy counterpart: ${unread.join(', ')}.`, - 'This is a migration. Read the counterpart and write the Phase 1 behavior inventory from packages/mosaic/docs/migration.md', - '(one row per prop, default, guard, error path, loading state and click behavior) before writing the model, then retry.', - ].join(' '), - }, - }), - ); -} diff --git a/.claude/hooks/require-mosaic-parity-audit.mjs b/.claude/hooks/require-mosaic-parity-audit.mjs deleted file mode 100644 index 61f8910eda4..00000000000 --- a/.claude/hooks/require-mosaic-parity-audit.mjs +++ /dev/null @@ -1,49 +0,0 @@ -import { legacyCounterparts, read, readInput, toolCalls } from './mosaic-hook-utils.mjs'; - -const input = readInput(); - -if (input.stop_hook_active) { - process.exit(0); -} - -const calls = toolCalls(read(input.transcript_path || '')); -const features = new Map(); - -for (const call of calls) { - if (!['Write', 'Edit', 'MultiEdit'].includes(call.name)) { - continue; - } - const match = String(call.input?.file_path ?? '').match(/^(.*)\/packages\/mosaic\/src\/features\/([a-z0-9-]+)\//); - if (match) { - features.set(match[2], match[1]); - } -} - -const migrations = [...features].flatMap(([feature, repoRoot]) => - legacyCounterparts(repoRoot, feature).map(legacy => ({ feature, legacy })), -); - -const audits = calls - .filter(call => call.name === 'Agent') - .map(call => JSON.stringify(call.input ?? {})) - .filter(input => /parity/i.test(input)); -const unaudited = migrations.filter( - ({ feature, legacy }) => !audits.some(input => input.includes(legacy) || input.includes(`/features/${feature}/`)), -); - -if (unaudited.length === 0) { - process.exit(0); -} - -const list = unaudited.map(({ feature, legacy }) => `${feature} (legacy: ${legacy})`).join(', '); - -process.stdout.write( - JSON.stringify({ - decision: 'block', - reason: [ - `This session changed Mosaic features with a legacy counterpart: ${list}.`, - 'Before finishing, launch the Explore subagent with the prompt in packages/mosaic/docs/parity-audit.md.', - 'Fix or raise with the user every row it reports as breaking a Mosaic rule, and list the remaining differences in the PR description.', - ].join(' '), - }), -); diff --git a/.claude/settings.json b/.claude/settings.json index f5e032091e2..0f518f8161b 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -41,7 +41,7 @@ "hooks": [ { "type": "command", - "command": "node \"$CLAUDE_PROJECT_DIR/.claude/hooks/require-mosaic-legacy-read.mjs\"", + "command": "node \"$(git rev-parse --show-toplevel)/scripts/mosaic-parity-gate.mjs\" write", "timeout": 10 } ] @@ -62,8 +62,8 @@ }, { "type": "command", - "command": "node \"$CLAUDE_PROJECT_DIR/.claude/hooks/require-mosaic-parity-audit.mjs\"", - "timeout": 10 + "command": "node \"$(git rev-parse --show-toplevel)/scripts/mosaic-parity-gate.mjs\" stop", + "timeout": 30 } ] } diff --git a/.codex/hooks.json b/.codex/hooks.json index 2462021dc7f..a1890188fe4 100644 --- a/.codex/hooks.json +++ b/.codex/hooks.json @@ -12,6 +12,23 @@ "type": "command", "command": "cd \"$(git rev-parse --show-toplevel)\" && { git diff --name-only --diff-filter=d HEAD -- packages/swingset/src; git ls-files --others --exclude-standard -- packages/swingset/src; } | grep -E '\\.tsx?$' | xargs -r pnpm exec eslint --quiet --no-warn-ignored --rule 'import/no-unresolved: off' >&2 || exit 2", "timeout": 180 + }, + { + "type": "command", + "command": "node \"$(git rev-parse --show-toplevel)/scripts/mosaic-parity-gate.mjs\" stop", + "timeout": 30 + } + ] + } + ], + "PreToolUse": [ + { + "matcher": "apply_patch|Edit|Write", + "hooks": [ + { + "type": "command", + "command": "node \"$(git rev-parse --show-toplevel)/scripts/mosaic-parity-gate.mjs\" write", + "timeout": 10 } ] } diff --git a/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index c1df7bd339d..5f50303e43e 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -28,11 +28,13 @@ Read the guide for the task you are doing. Paths below are relative to this pack Before you write a new feature, search `packages/react/src/components/` and `packages/ui/src/components/` for its counterpart. If one exists, the work is a [migration](docs/migration.md), including a headless port of a `@clerk/react` button framed as a new feature: -1. Read the counterpart and write the Phase 1 inventory before the model. -2. Before you call the work done, review it, or open a PR, launch the Explore subagent with the prompt in [Parity audit](docs/parity-audit.md). Reading the doc is not running the audit. +1. Read the counterpart and write the Phase 1 inventory to `tmp/mosaic-parity/.md` before the model. The file names the legacy path and has an `## Inventory` section. +2. Before you call the work done, review it, or open a PR, run the [parity audit](docs/parity-audit.md) in a read-only subagent and put its table under `## Parity audit` in the same file. Reading the doc is not running the audit. 3. List every **Deliberately changed** and **Deferred** row in the PR description. A difference that is not listed is a regression. 4. Listing a difference does not make it acceptable. If one breaks a rule in this file or in `docs/`, such as a failure that nobody sees, fix it or ask the user before you finish. Do not label it deliberate. +`tmp/` is ignored by git, so the file stays out of the PR. Hooks in `.claude/settings.json` and `.codex/hooks.json` (`scripts/mosaic-parity-gate.mjs`) check it: one blocks writing a new feature's model until the inventory exists, and one blocks finishing until the audit is in. + ## Layers in one paragraph A flow lives in `src/features//`. The **model** (`*.model.ts(x)`) is the only file that reads Clerk hooks or calls Clerk resources, and answers with plain data, plain callbacks and a `status`. The **controller** (`*.controller.ts(x)`) holds local state and wraps the model's callbacks for pending, errors and closing. The **view** (`*.view.tsx`) renders plain props. No Clerk resource reaches the controller and no machine snapshot reaches the view. diff --git a/packages/mosaic/docs/migration.md b/packages/mosaic/docs/migration.md index 1fb86345aab..ba864df8766 100644 --- a/packages/mosaic/docs/migration.md +++ b/packages/mosaic/docs/migration.md @@ -54,6 +54,19 @@ Turn the hits into a **behavior inventory** — one row per: effect, guard, erro path, empty/loading state, permission gate, revalidate, reset-on-close, derived label. This list is finite and is the contract the migration must satisfy. +Write it to `tmp/mosaic-parity/.md` (ignored by git) before the model: + +```md +# sign-out-button parity + +Legacy: packages/react/src/components/SignOutButton.tsx + +## Inventory + +- `redirectUrl` defaults to `/` +- … +``` + ## Phase 2 — Design the split (map every row to a layer) Assign each inventory row to exactly one layer. A row with no home is a behavior @@ -104,7 +117,7 @@ empty states) each have one. Tests only cover branches you remembered to write. To catch the ones you didn't, run an automated diff of legacy against new. -Launch an **Explore subagent** with the prompt in `parity-audit.md`. Give it the +Launch a read-only subagent (Explore in Claude Code) with the prompt in `parity-audit.md`. Give it the legacy file paths and the new model/controller/view paths. It returns a table classifying every legacy behavior as: @@ -116,9 +129,9 @@ classifying every legacy behavior as: how an earlier domains-section migration shipped three regressions (see `parity-audit.md`). -Every inventory row from Phase 1 must land in exactly one bucket. The table is -**ephemeral**: it drives the work and the PR discussion, then is discarded. It is -not committed. +Every inventory row from Phase 1 must land in exactly one bucket. Put the table +under `## Parity audit` in `tmp/mosaic-parity/.md`. It drives the work +and the PR discussion, and is never committed. ## Phase 5 — Ship diff --git a/packages/mosaic/docs/parity-audit.md b/packages/mosaic/docs/parity-audit.md index 6addf828116..f97cced06b3 100644 --- a/packages/mosaic/docs/parity-audit.md +++ b/packages/mosaic/docs/parity-audit.md @@ -5,9 +5,10 @@ legacy component against the new model/controller/view and classifies every legacy behavior, so behavior that lived implicitly in the old blob can't be silently dropped. -Run it as an **Explore subagent** (read-only, returns a conclusion, not file -dumps). It is **ephemeral** — the table guides the work and the PR discussion, -then is discarded. Nothing is committed. +Run it as a read-only subagent (Explore in Claude Code) that returns a +conclusion, not file dumps. Put its table under `## Parity audit` in +`tmp/mosaic-parity/.md`, next to the Phase 1 inventory. The table guides +the work and the PR discussion. `tmp/` is ignored by git, so nothing is committed. ## The subagent prompt diff --git a/scripts/mosaic-parity-gate.mjs b/scripts/mosaic-parity-gate.mjs new file mode 100644 index 00000000000..2da84a798aa --- /dev/null +++ b/scripts/mosaic-parity-gate.mjs @@ -0,0 +1,184 @@ +import { spawnSync } from 'node:child_process'; +import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { isAbsolute, join, relative } from 'node:path'; +import { pathToFileURL } from 'node:url'; + +export const PROOF_DIR = 'tmp/mosaic-parity'; +export const MAX_BLOCKS = 3; + +const FEATURE_MODEL = /^packages\/mosaic\/src\/features\/([a-z0-9-]+)\/\1\.model\.tsx?$/; +const FEATURE_FILE = /^packages\/mosaic\/src\/features\/([a-z0-9-]+)\//; + +function pascalCase(kebab) { + return kebab.replace(/(^|-)([a-z0-9])/g, (_, __, char) => char.toUpperCase()); +} + +export function legacyCounterparts(root, feature) { + const name = pascalCase(feature); + return [`packages/react/src/components/${name}.tsx`, `packages/ui/src/components/${name}`].filter(path => + existsSync(join(root, path)), + ); +} + +export function proofPath(feature) { + return `${PROOF_DIR}/${feature}.md`; +} + +function section(text, heading) { + const match = text.match(new RegExp(`^##\\s+${heading}\\s*$([\\s\\S]*?)(?=^##\\s|(?![\\s\\S]))`, 'im')); + return match ? match[1].trim() : ''; +} + +export function inventoryProblem(proof, legacy) { + if (!proof) { + return 'missing'; + } + const missing = legacy.filter(path => !proof.includes(path)); + if (missing.length > 0) { + return `does not name ${missing.join(', ')}`; + } + return section(proof, 'Inventory') ? undefined : 'has no `## Inventory` section'; +} + +export function auditProblem(proof) { + if (!proof) { + return 'missing'; + } + return section(proof, 'Parity audit') ? undefined : 'has no `## Parity audit` section'; +} + +export function createdFiles(input, root) { + const tool = input.tool_input ?? {}; + const cwd = input.cwd ?? root; + const toRepo = path => relative(root, isAbsolute(path) ? path : join(cwd, path)); + + if (typeof tool.file_path === 'string') { + return existsSync(isAbsolute(tool.file_path) ? tool.file_path : join(cwd, tool.file_path)) + ? [] + : [toRepo(tool.file_path)]; + } + if (typeof tool.command === 'string') { + return [...tool.command.matchAll(/^\*\*\* Add File: (.+)$/gm)].map(match => toRepo(match[1].trim())); + } + return []; +} + +function run(args, cwd) { + return spawnSync('git', args, { cwd, encoding: 'utf8' }).stdout ?? ''; +} + +function readText(path) { + try { + return readFileSync(path, 'utf8'); + } catch { + return ''; + } +} + +function readInput() { + try { + return JSON.parse(readFileSync(0, 'utf8')); + } catch { + return {}; + } +} + +function newFeatures(root) { + const base = run(['merge-base', 'HEAD', 'origin/main'], root).trim() || 'HEAD'; + const changed = `${run(['diff', '--name-only', '--diff-filter=d', base, '--', 'packages/mosaic/src/features'], root)}\n${run(['ls-files', '--others', '--exclude-standard', '--', 'packages/mosaic/src/features'], root)}`; + const features = new Set( + changed + .split('\n') + .map(file => file.match(FEATURE_FILE)?.[1]) + .filter(Boolean), + ); + return [...features].filter( + feature => !run(['ls-tree', '--name-only', base, `packages/mosaic/src/features/${feature}`], root).trim(), + ); +} + +function beforeWrite(input, root) { + const blocked = createdFiles(input, root).flatMap(file => { + const feature = file.match(FEATURE_MODEL)?.[1]; + const legacy = feature ? legacyCounterparts(root, feature) : []; + const problem = legacy.length > 0 ? inventoryProblem(readText(join(root, proofPath(feature))), legacy) : undefined; + return problem ? [{ feature, legacy, problem }] : []; + }); + + if (blocked.length === 0) { + return; + } + + const reason = blocked + .map( + ({ feature, legacy, problem }) => + `\`${feature}\` ports ${legacy.join(' and ')}, and ${proofPath(feature)} ${problem}. Read the legacy component, then write ${proofPath(feature)} with the legacy path and an \`## Inventory\` section (one row per prop, default, guard, error path, loading state and click behavior), as packages/mosaic/docs/migration.md describes. Then retry this edit.`, + ) + .join(' '); + + process.stdout.write( + JSON.stringify({ + hookSpecificOutput: { hookEventName: 'PreToolUse', permissionDecision: 'deny', permissionDecisionReason: reason }, + }), + ); +} + +function beforeStop(input, root) { + const unaudited = newFeatures(root).flatMap(feature => { + const legacy = legacyCounterparts(root, feature); + const problem = legacy.length > 0 ? auditProblem(readText(join(root, proofPath(feature)))) : undefined; + return problem ? [{ feature, legacy, problem }] : []; + }); + + const stateDir = join(tmpdir(), 'clerk-mosaic-parity-gate'); + const statePath = join(stateDir, `${String(input.session_id ?? 'unknown').replace(/[^\w-]/g, '_')}.json`); + + if (unaudited.length === 0) { + return; + } + + let state; + try { + state = JSON.parse(readFileSync(statePath, 'utf8')); + } catch { + state = { blocks: 0 }; + } + if (state.blocks >= MAX_BLOCKS) { + return; + } + mkdirSync(stateDir, { recursive: true }); + writeFileSync(statePath, JSON.stringify({ blocks: state.blocks + 1 })); + + const list = unaudited.map( + ({ feature, legacy, problem }) => `${feature} (legacy: ${legacy.join(', ')}; ${proofPath(feature)} ${problem})`, + ); + + process.stdout.write( + JSON.stringify({ + decision: 'block', + reason: [ + `This branch adds Mosaic features that port a legacy component: ${list.join('; ')}.`, + 'Before finishing, run the parity audit in packages/mosaic/docs/parity-audit.md and put its table under `## Parity audit` in that file.', + 'Fix or raise with the user every row that breaks a Mosaic rule, and list the remaining differences in the PR description.', + ].join(' '), + }), + ); +} + +function main() { + const input = readInput(); + const root = run(['rev-parse', '--show-toplevel'], input.cwd ?? process.cwd()).trim(); + if (!root) { + return; + } + if (process.argv[2] === 'write') { + beforeWrite(input, root); + } else if (process.argv[2] === 'stop') { + beforeStop(input, root); + } +} + +if (import.meta.url === pathToFileURL(process.argv[1] ?? '').href) { + main(); +} diff --git a/scripts/mosaic-parity-gate.test.mjs b/scripts/mosaic-parity-gate.test.mjs new file mode 100644 index 00000000000..dcc5d23bbaf --- /dev/null +++ b/scripts/mosaic-parity-gate.test.mjs @@ -0,0 +1,56 @@ +import { describe, expect, test } from 'vitest'; + +import { auditProblem, createdFiles, inventoryProblem } from './mosaic-parity-gate.mjs'; + +const root = '/repo'; +const legacy = ['packages/react/src/components/SignOutButton.tsx']; + +describe('createdFiles', () => { + test('reads a Claude Write of a new file', () => { + const input = { tool_input: { file_path: '/repo/packages/mosaic/src/features/x/x.model.ts' } }; + expect(createdFiles(input, root)).toEqual(['packages/mosaic/src/features/x/x.model.ts']); + }); + + test('reads every added file in a Codex apply_patch', () => { + const command = [ + '*** Begin Patch', + '*** Add File: packages/mosaic/src/features/x/x.model.ts', + '+export {};', + '*** Update File: packages/mosaic/src/localization/registry.ts', + '*** Add File: /repo/packages/mosaic/src/features/x/x.view.tsx', + '*** End Patch', + ].join('\n'); + expect(createdFiles({ cwd: root, tool_input: { command } }, root)).toEqual([ + 'packages/mosaic/src/features/x/x.model.ts', + 'packages/mosaic/src/features/x/x.view.tsx', + ]); + }); +}); + +describe('inventoryProblem', () => { + test('requires the proof file', () => { + expect(inventoryProblem('', legacy)).toBe('missing'); + }); + + test('requires the legacy path', () => { + expect(inventoryProblem('## Inventory\n- redirectUrl defaults to /', legacy)).toMatch(/does not name/); + }); + + test('requires a non-empty inventory', () => { + expect(inventoryProblem(`Legacy: ${legacy[0]}\n## Inventory\n\n## Parity audit\n`, legacy)).toMatch(/Inventory/); + }); + + test('passes a filled inventory', () => { + expect(inventoryProblem(`Legacy: ${legacy[0]}\n## Inventory\n- redirectUrl defaults to /`, legacy)).toBeUndefined(); + }); +}); + +describe('auditProblem', () => { + test('requires a non-empty audit section', () => { + expect(auditProblem('## Inventory\n- a\n## Parity audit\n')).toMatch(/Parity audit/); + }); + + test('passes a filled audit section', () => { + expect(auditProblem('## Inventory\n- a\n## Parity audit\n| row | Migrated |')).toBeUndefined(); + }); +}); From 30ed6008e93c1901b610ce821ca3b2db45b77d0d Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Wed, 4 Nov 2026 14:41:09 -0700 Subject: [PATCH 10/10] docs(mosaic): only start the Claude write check for Mosaic feature files --- .claude/settings.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.claude/settings.json b/.claude/settings.json index 0f518f8161b..4c1a9826d9d 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -42,7 +42,8 @@ { "type": "command", "command": "node \"$(git rev-parse --show-toplevel)/scripts/mosaic-parity-gate.mjs\" write", - "timeout": 10 + "timeout": 10, + "if": "Write(/packages/mosaic/src/features/**)" } ] }