From ce3afe166be151cec0e31f66c6d47c1e48346c8b Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Tue, 3 Nov 2026 19:45:27 -0500 Subject: [PATCH 01/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/**)" } ] } From 719ef140a27dfec1d9b3b030ab4d29c58ae6749d Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Thu, 5 Nov 2026 11:04:39 -0700 Subject: [PATCH 11/12] fix(repo): scope Mosaic parity reminders to current edits --- .claude/settings.json | 10 +- .codex/hooks.json | 5 - packages/mosaic/AGENTS.md | 11 +- packages/mosaic/docs/migration.md | 43 +++-- packages/mosaic/docs/parity-audit.md | 145 +++++++++++--- scripts/mosaic-parity-gate.mjs | 228 +++++++--------------- scripts/mosaic-parity-gate.test.mjs | 278 +++++++++++++++++++++++---- 7 files changed, 459 insertions(+), 261 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index 4c1a9826d9d..0a9d3b445a8 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -37,13 +37,12 @@ ], "PreToolUse": [ { - "matcher": "Write", + "matcher": "Write|Edit", "hooks": [ { "type": "command", "command": "node \"$(git rev-parse --show-toplevel)/scripts/mosaic-parity-gate.mjs\" write", - "timeout": 10, - "if": "Write(/packages/mosaic/src/features/**)" + "timeout": 10 } ] } @@ -60,11 +59,6 @@ "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 } ] } diff --git a/.codex/hooks.json b/.codex/hooks.json index a1890188fe4..36dc7d89188 100644 --- a/.codex/hooks.json +++ b/.codex/hooks.json @@ -12,11 +12,6 @@ "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 } ] } diff --git a/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index 8b281808837..89268449de3 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -40,14 +40,15 @@ Before you change a file that matches a pattern below, read its guide, even for ## 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: +For a current task that implements or reviews a Mosaic migration, or changes its migrated behavior, search `packages/react/src/components/` and `packages/ui/src/components/` for the counterpart. A headless port framed as a new feature is a [migration](docs/migration.md) too. Unrelated work does not acquire this workflow merely because the branch already contains Mosaic changes. -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. +1. Read the counterpart, shared prop types, and relevant helper/runtime calls. Write the Phase 1 inventory to `tmp/mosaic-parity/.md` before the model. Record the task scope, exact revisions, accepted differences, and exclusions alongside the legacy path and `## 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. +3. Use the audit's classifications to separate parity gaps, intentional differences needing disclosure, missing verification, and optional improvements. List intentional differences and approved deferrals in the PR description. Do not require inherited improvements or refactors as parity fixes. +4. Listing a difference does not make it acceptable. Cite any conflicting rule in this file or `docs/`, and fix the gap or raise it with the user. Do not infer intentional behavior from the implementation. Preserve decisions the user already made, and leave unresolved product choices open. +5. Inspect the actual results of focused type and behavior checks. For relevant async actions, include a representative failure through the existing fake FAPI or supported browser setup. A populated note is process bookkeeping, not proof of parity. -`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. +`tmp/` is ignored by git, so the file stays out of the PR. The hooks in `.claude/settings.json` and `.codex/hooks.json` (`scripts/mosaic-parity-gate.mjs`) give conditional guidance for the feature paths in the current edit. They do not inspect branch history or notes, deny edits, or block completion. For a read-only parity review, run the explicit scoped command in [Parity audit](docs/parity-audit.md). These reminders do not run the audit or verify its evidence. ## Layers in one paragraph diff --git a/packages/mosaic/docs/migration.md b/packages/mosaic/docs/migration.md index ba864df8766..ca0c7b425fd 100644 --- a/packages/mosaic/docs/migration.md +++ b/packages/mosaic/docs/migration.md @@ -18,6 +18,11 @@ Do not write the new layers first and then ask "did I get everything?" — that means proving a negative. Invert it: enumerate the legacy behavior first, then make each layer account for a specific row. +Apply this workflow to the current requested Mosaic work, not every Mosaic change +already on the branch. Record the feature scope, original revisions, accepted +differences, and exclusions. When reviewing earlier feedback, use the commit the +reviewer saw, not a later implementation that may already have addressed it. + --- ## Phase 1 — Inventory the legacy behavior (the spec) @@ -54,6 +59,12 @@ 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. +Include shared prop contracts and mode-dependent options, callback ordering and +awaiting, defaults, and error delivery. Follow helpers and Clerk runtime calls +when they determine the outcome. Record inherited limitations separately from +introduced gaps. A grep hit list or comparison of component files alone is not +the complete contract. See [Parity audit](parity-audit.md) for the checks. + Write it to `tmp/mosaic-parity/.md` (ignored by git) before the model: ```md @@ -112,26 +123,28 @@ row into a test there by default, and reach for a smaller test only where highest-risk layer, so make sure its rows (revalidation, permission gates, empty states) each have one. +For each affected async action, exercise a representative rejection as well as +success and pending work. Verify error delivery and the final rendered/resource +state, including recovery where promised. Do not replace a Clerk mutation with a +method mock when the existing fake FAPI can drive it. Check mode-dependent prop +contracts with focused type checks and verify forwarding in the relevant modes. + ## Phase 4 — Verify parity (the confidence step) -Tests only cover branches you remembered to write. To catch the -ones you didn't, run an automated diff of legacy against new. +Tests only cover branches you remembered to write. Have an independent reader +trace the contract and identify missing checks as well as behavioral differences. 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: - -- **Migrated** — points at a specific state / transition / context field. -- **Deliberately changed** — names the new behavior and why (e.g. infinite - scroll → "Load more" button). -- **Deferred** — a real tracked ticket, **not** a `// TODO` buried in a - controller or model. A buried TODO is invisible at review time; that is exactly - 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. Put the table +legacy and new revisions, file paths, shared types, and accepted decisions. +Use the classifications and evidence columns in `parity-audit.md`. Distinguish +parity gaps, intentional differences needing disclosure, missing verification, +and optional improvements. Leave unresolved intent as a decision for the user. + +Every inventory row from Phase 1 must have a conclusion and evidence. Put the table under `## Parity audit` in `tmp/mosaic-parity/.md`. It drives the work -and the PR discussion, and is never committed. +and the PR discussion, and is never committed. Read the results of its referenced +checks before claiming parity. A filled table, a tracked deferral, or a hook +reminder is not proof that the behavior works. ## Phase 5 — Ship diff --git a/packages/mosaic/docs/parity-audit.md b/packages/mosaic/docs/parity-audit.md index f97cced06b3..27eafe3d83c 100644 --- a/packages/mosaic/docs/parity-audit.md +++ b/packages/mosaic/docs/parity-audit.md @@ -1,56 +1,140 @@ # Parity audit — Phase 4 reference The parity audit is the confidence step of a Mosaic migration. It diffs the -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. +legacy implementation and shared API contract against the new behavior, +including the helpers and Clerk runtime they call. + +Use this workflow when the current task implements or reviews a Mosaic +migration or changes its migrated behavior. Do not start it for unrelated work +because the branch already contains Mosaic changes. Record the task's component +scope and exact legacy/new revisions before comparing. A renamed counterpart +still counts; a same-name match is only a discovery shortcut. +A shared-library task does not trigger this full workflow solely because Mosaic +is a downstream consumer. + +For a requested read-only parity review, name the current task's feature folders +explicitly from the repository root: + +```bash +node scripts/mosaic-parity-gate.mjs review sign-out-button sign-up-button +``` + +This command prints scoped guidance without changing files or registering scope +for later requests. It does not run an audit. Use feature names from the request, +not every changed feature on the branch. A renamed or historical feature can be +named even when its folder is absent from the current checkout. + +Edit hooks provide the same guidance for current feature paths. A style-only edit +does not require a behavioral audit. The reminder is conditional on changing +migrated behavior, and emits at most once per tool action. Shell-only edits and +read-only reviews without the explicit command have no automatic reminder. +There is no parity Stop hook or completion gate. Follow this workflow whenever +the current task calls for it, whether a reminder appeared or not. 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. +A populated note proves neither parity nor execution of the checks. Inspect the +source citations and actual test results, and distinguish measured outcomes from +source-only conclusions and checks that could not run. ## The subagent prompt -Fill in the two file lists and hand this to an Explore agent: +Fill in the scope, revisions, and file lists and hand this to an Explore agent: ``` In the repo at , audit a Mosaic migration for behavioral parity. +CURRENT TASK: +REVISIONS: +ACCEPTED DIFFERENCES: + LEGACY component files (the spec — behavior must be preserved or consciously changed): NEW Mosaic files (model / controller / view / wrapper): -Enumerate EVERY behavior in the legacy files — each: effect, guard, error path, +Read the shared public prop types, relevant callers, and Mosaic rules as well. +Follow calls into shared helpers and Clerk runtime until you can establish the +observable outcome. Do not stop at matching method names or argument objects. + +Enumerate EVERY relevant behavior — each: prop and mode-dependent option, default, +callback order/awaiting, effect, guard, error path, empty/loading state, permission gate, revalidate/reload call, reset-on-close, derived label/count, pagination trigger. For each one, find where it lives in -the new files and classify it: - - - Migrated → points at a specific new state / transition / context field / prop - - Deliberately changed → the new behavior differs on purpose; name the new behavior and why - - Deferred → not implemented in the new code; note whether a tracked ticket exists - or it is only a buried `// TODO` +the new implementation and classify it: + + - Preserved: same observable contract, with source and verification evidence. + - Parity gap: introduced loss or incompatible contract; identify the failing condition. + - Intentional difference: cite the user decision or Mosaic rule, and required disclosure. + If intent is not established, mark Decision needed rather than inventing approval. + - Verification gap: relevant outcome is untested or blocked; name the missing probe. + - Optional improvement: inherited defect or refactor outside the migration contract. + Keep it separate; do not require a fix to complete parity work. + +For each in-scope action check success, pending, and a representative rejection. +Use the existing real-Clerk/fake-FAPI feature harness where it can exercise the +behavior. Assert final DOM, error delivery, resource state, and navigation as +relevant. A mock of the Clerk method cannot prove that method's runtime effects. +Use a controlled boundary failure for a wallet or external modal when the harness +cannot model it, and state what that leaves unverified. + +Use focused type checks for mode-dependent contracts, including options accepted +but silently ignored. Verify runtime forwarding separately. Check loading both +before Clerk loads and after a failed mutation, since runtime state changes may +unmount the controller that owns the pending state or error. + +For callbacks, record internal/consumer order, awaiting, and throw/rejection +behavior. Compare them with headless.md before demanding legacy ordering. +For redirects, exercise explicit URLs and omitted props with a non-default +provider setting; follow runtime defaults through to the navigation destination. 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. +guard). Never dump whole files. Return the table below, gaps first. Include the +test command/case, expected and observed outcome, and any limitation. An existing +test that asserts only re-enablement does not verify error delivery or recovery. + +Check differences against packages/mosaic/AGENTS.md and the relevant docs. +Use models.md for public contracts, controllers.md for error ownership, +headless.md for callback composition, and testing.md for test boundaries. +Quote the applicable rule and distinguish its requirement from your suggested +implementation. Lost errors require an observable error path; that does not +automatically require a new onError API or a particular message. +Raise unresolved rule conflicts and product choices with the user. A ticket, +TODO, or difference label does not resolve a parity gap or verification gap. ``` ## Output table format -| Legacy behavior | Layer it should live in | Status | Evidence (legacy → new) | -| ------------------------------------------------------- | ----------------------- | --------------------------- | -------------------------------------------------------------------------- | -| Per-field error mapping via `handleError(err, [field])` | controller/view | Deferred (only a `// TODO`) | `AddDomainForm.tsx` handleError → `*.controller.tsx` single `errorMessage` | - -`Status` is one of: **Migrated** · **Deliberately changed** · **Deferred**. +| Contract / condition | Classification | Source evidence (legacy → new, types/rules/runtime) | Verification (expected → observed; command/case) | Disclosure / decision / next step | +| ----------------------------------- | -------------- | ------------------------------------------------------ | ------------------------------------------------ | ---------------------------------------------- | +| Rejected mutation delivers an error | Parity gap | Legacy rejection → controller captures and drops error | Fake FAPI rejects; no observable error | Restore an error path; API choice remains open | + +List intentional differences in the PR description, including their consumer +impact. Keep optional improvements separate from required fixes. Record deferred +work with its approval and ticket without changing its underlying classification. + +## Calibration: headless auth buttons (#10588) + +At the originally reviewed commit `ecb1a5508614aad8614e74ba53554864afdfd824`: + +- Trace rejected actions through `usePendingAction` to their observable error + destination. Capturing an error and returning only `isPending` loses it. +- Compare `SignUpButtonProps` in shared types, not just `SignUpProps`. Check that + `unsafeMetadata` is accepted only in modal mode and forwarded there. +- Exercise sign-out through fake FAPI, including rejection. Follow `clerk.signOut` + state emission and `useUser` loading state; a method mock hides this path. +- Disclose async `onClick` behavior under `mergeProps(internal, consumer)`, initial + fallback rendering, and the changed sign-out redirect default. The original + review requested documentation for these, not restoration of legacy behavior. +- Inspect modal `appearance` support as an API difference. Establish the intended + Mosaic contract before removing or restoring it. +- Do not require missing-MetaMask message mapping or sign-up continuation as + parity fixes. Both limitations existed in the legacy button. Sharing the four + buttons, deduplicating option lists, and the requested pending presentation are + optional improvements unless a separately established contract requires them. ## Grep list (shared with Phase 1) @@ -63,9 +147,8 @@ rg -n 'useEffect|handleError|card\.setError|useReverification|revalidate| 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'; +import { readFileSync } from 'node:fs'; +import { isAbsolute, relative, resolve } from 'node:path'; + +const FEATURE_NAME = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; +const FEATURE_FILE = /^packages\/mosaic\/src\/features\/([a-z0-9]+(?:-[a-z0-9]+)*)\/.+/; + +function reminder(features, review) { + return [ + `Mosaic parity scope: ${features.join(', ')}.`, + review ? 'For this requested review, follow' : 'If this work changes migrated behavior, follow', + 'packages/mosaic/docs/migration.md and packages/mosaic/docs/parity-audit.md.', + 'Use actual focused type and runtime results, and preserve the accepted decisions and classifications in those guides.', + 'This reminder is not parity evidence.', + ].join(' '); } -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())); +function main() { + const [mode, ...args] = process.argv.slice(2); + if (mode === 'review') { + if (args.length === 0 || args.some(name => !FEATURE_NAME.test(name))) { + process.stderr.write('Usage: node scripts/mosaic-parity-gate.mjs review [feature-name ...]\n'); + process.exitCode = 1; + return; + } + process.stdout.write(`${reminder([...new Set(args)].sort(), true)}\n`); + return; } - return []; -} - -function run(args, cwd) { - return spawnSync('git', args, { cwd, encoding: 'utf8' }).stdout ?? ''; -} - -function readText(path) { - try { - return readFileSync(path, 'utf8'); - } catch { - return ''; + if (mode !== 'write' || args.length > 0) { + return; } -} -function readInput() { + let input; try { - return JSON.parse(readFileSync(0, 'utf8')); + input = 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) { + if ( + !input || + typeof input.cwd !== 'string' || + !isAbsolute(input.cwd) || + input.cwd.includes('\0') || + !input.tool_input + ) { return; } - let state; - try { - state = JSON.parse(readFileSync(statePath, 'utf8')); - } catch { - state = { blocks: 0 }; + const tool = input.tool_input; + let paths; + if (input.tool_name === 'Write' || input.tool_name === 'Edit') { + paths = typeof tool.file_path === 'string' ? [tool.file_path] : []; + } else if (input.tool_name === 'apply_patch' && typeof tool.command === 'string') { + const patch = tool.command.trim(); + if (!patch.startsWith('*** Begin Patch\n') || !patch.endsWith('\n*** End Patch')) { + return; + } + paths = [...patch.matchAll(/^\*\*\* (?:Add File|Update File|Delete File|Move to): (.+)$/gm)].map(match => match[1]); + } else { + return; } - if (state.blocks >= MAX_BLOCKS) { + if (paths.length === 0 || paths.some(path => path.includes('\0'))) { 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) { + const git = spawnSync('git', ['rev-parse', '--show-cdup'], { cwd: input.cwd, encoding: 'utf8' }); + if (git.status !== 0) { return; } - if (process.argv[2] === 'write') { - beforeWrite(input, root); - } else if (process.argv[2] === 'stop') { - beforeStop(input, root); + const root = resolve(input.cwd, git.stdout.trim()); + const features = [ + ...new Set( + paths.flatMap(path => { + const feature = relative(root, resolve(input.cwd, path)).match(FEATURE_FILE)?.[1]; + return feature ? [feature] : []; + }), + ), + ].sort(); + if (features.length > 0) { + process.stdout.write( + JSON.stringify({ + hookSpecificOutput: { hookEventName: 'PreToolUse', additionalContext: reminder(features, false) }, + }), + ); } } -if (import.meta.url === pathToFileURL(process.argv[1] ?? '').href) { - main(); -} +main(); diff --git a/scripts/mosaic-parity-gate.test.mjs b/scripts/mosaic-parity-gate.test.mjs index dcc5d23bbaf..521aeabe874 100644 --- a/scripts/mosaic-parity-gate.test.mjs +++ b/scripts/mosaic-parity-gate.test.mjs @@ -1,56 +1,262 @@ -import { describe, expect, test } from 'vitest'; +import { execFile, spawnSync } from 'node:child_process'; +import { mkdtempSync, mkdirSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { promisify } from 'node:util'; +import { fileURLToPath } from 'node:url'; +import { afterEach, beforeEach, describe, expect, test } from 'vitest'; -import { auditProblem, createdFiles, inventoryProblem } from './mosaic-parity-gate.mjs'; +const script = fileURLToPath(new URL('./mosaic-parity-gate.mjs', import.meta.url)); +const features = 'packages/mosaic/src/features'; +let root; -const root = '/repo'; -const legacy = ['packages/react/src/components/SignOutButton.tsx']; +function write(path, text = 'export {};') { + mkdirSync(dirname(join(root, path)), { recursive: true }); + writeFileSync(join(root, path), text); +} -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']); - }); +function git(...args) { + const result = spawnSync('git', args, { cwd: root, encoding: 'utf8' }); + expect(result.status, result.stderr).toBe(0); + return result.stdout; +} - 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', - ]); +function invoke(input, args = ['write']) { + return spawnSync(process.execPath, [script, ...args], { + cwd: root, + input: typeof input === 'string' ? input : JSON.stringify(input), + encoding: 'utf8', + env: { ...process.env, TMPDIR: root }, }); +} + +function edit(path, tool_name = 'Edit', cwd = root) { + return { cwd, session_id: 'same-session', tool_name, tool_input: { file_path: path } }; +} + +function patch(lines, cwd = root) { + return { + cwd, + tool_name: 'apply_patch', + tool_input: { command: ['*** Begin Patch', ...lines, '*** End Patch'].join('\n') }, + }; +} + +function advisory(result, names) { + expect(result.status, result.stderr).toBe(0); + expect(result.stderr).toBe(''); + const output = JSON.parse(result.stdout); + expect(Object.keys(output)).toEqual(['hookSpecificOutput']); + expect(Object.keys(output.hookSpecificOutput).sort()).toEqual(['additionalContext', 'hookEventName']); + expect(output.hookSpecificOutput.hookEventName).toBe('PreToolUse'); + const text = output.hookSpecificOutput.additionalContext; + expect(text).toContain(names.join(', ')); + expect(text).toMatch(/if.*migrated behavior/i); + expect(text).toContain('packages/mosaic/docs/migration.md'); + expect(text).toContain('packages/mosaic/docs/parity-audit.md'); + expect(text).not.toMatch(/passed|parity verified|retry|must.*audit/i); + return text; +} + +function silent(result) { + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toBe(''); + expect(result.stderr).toBe(''); +} + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'mosaic-reminder-test-')); + git('init', '-q'); + git('config', 'user.email', 'test@example.com'); + git('config', 'user.name', 'Test'); + write('README.md', 'fixture'); + write('packages/react/src/components/SignOutButton.tsx'); + git('add', '.'); + git('commit', '-qm', 'base'); + git('update-ref', 'refs/remotes/origin/main', 'HEAD'); + write(`${features}/sign-out-button/sign-out-button.model.ts`); + git('add', '.'); + git('commit', '-qm', 'prior Mosaic work'); + write(`${features}/sign-up-button/sign-up-button.model.ts`); }); -describe('inventoryProblem', () => { - test('requires the proof file', () => { - expect(inventoryProblem('', legacy)).toBe('missing'); +afterEach(() => rmSync(root, { recursive: true, force: true })); + +describe('current action scope', () => { + test('ignores prior committed and untracked Mosaic work during unrelated edits and Stop', () => { + silent(invoke(edit('packages/shared/src/utils.ts'))); + silent(invoke({ cwd: root, session_id: 'same-session' }, ['stop'])); + }); + + test.each(['Edit', 'Write'])('advises on existing feature %s without blocking', tool => { + advisory(invoke(edit(`${features}/sign-out-button/sign-out-button.model.ts`, tool)), ['sign-out-button']); }); - test('requires the legacy path', () => { - expect(inventoryProblem('## Inventory\n- redirectUrl defaults to /', legacy)).toMatch(/does not name/); + test('advises on new models without requiring notes or counterpart matches', () => { + advisory(invoke(edit(`${features}/renamed-feature/renamed-feature.model.ts`, 'Write')), ['renamed-feature']); }); - test('requires a non-empty inventory', () => { - expect(inventoryProblem(`Legacy: ${legacy[0]}\n## Inventory\n\n## Parity audit\n`, legacy)).toMatch(/Inventory/); + test.each(['view.tsx', 'styles.ts', 'test.ts'])('keeps guidance conditional for feature %s', file => { + advisory(invoke(edit(`${features}/sign-out-button/${file}`)), ['sign-out-button']); }); - test('passes a filled inventory', () => { - expect(inventoryProblem(`Legacy: ${legacy[0]}\n## Inventory\n- redirectUrl defaults to /`, legacy)).toBeUndefined(); + test('does not carry scope into later unrelated actions in the same session', () => { + advisory(invoke(edit(`${features}/sign-out-button/view.tsx`)), ['sign-out-button']); + silent(invoke(edit('README.md'))); + silent(invoke(edit('packages/mosaic/src/primitives/button.tsx'))); + }); + + test('normalizes absolute, relative, and nested-cwd paths', () => { + const path = `${features}/sign-out-button/view.tsx`; + const first = advisory(invoke(edit(path)), ['sign-out-button']); + expect(advisory(invoke(edit(join(root, path))), ['sign-out-button'])).toBe(first); + expect( + advisory(invoke(edit('src/features/sign-out-button/view.tsx', 'Edit', join(root, 'packages/mosaic'))), [ + 'sign-out-button', + ]), + ).toBe(first); + }); + + test('collects add, update, delete and move scopes in one sorted unique advisory', () => { + const text = advisory( + invoke( + patch([ + `*** Add File: ${features}/new-feature/model.ts`, + '+export {};', + `*** Update File: ${join(root, features)}/sign-out-button/view.tsx`, + `*** Move to: ${features}/renamed-feature/view.tsx`, + '@@', + '-old', + '+new', + `*** Delete File: ${features}/deleted-feature/model.ts`, + '*** Add File: README.md', + '+unrelated', + `*** Update File: ${features}/new-feature/view.tsx`, + '@@', + '-old', + '+new', + '*** Delete File: /elsewhere/packages/mosaic/src/features/outside/view.tsx', + ]), + ), + ['deleted-feature', 'new-feature', 'renamed-feature', 'sign-out-button'], + ); + expect(text).not.toContain('outside'); + }); + + test.each([ + '/elsewhere/packages/mosaic/src/features/outside/view.tsx', + '../packages/mosaic/src/features/outside/view.tsx', + 'packages/mosaic/src/features/../../shared.ts', + 'packages/mosaic/src/features-extra/outside/view.tsx', + ])('ignores paths outside feature scope: %s', path => silent(invoke(edit(path)))); + + test('populated notes neither suppress guidance nor prove success', () => { + write( + 'tmp/mosaic-parity/sign-out-button.md', + 'packages/react/src/components/SignOutButton.tsx\n## Inventory\nComplete\n## Parity audit\nPassed', + ); + advisory(invoke(edit(`${features}/sign-out-button/new.model.ts`, 'Write')), ['sign-out-button']); }); }); -describe('auditProblem', () => { - test('requires a non-empty audit section', () => { - expect(auditProblem('## Inventory\n- a\n## Parity audit\n')).toMatch(/Parity audit/); +describe('invalid hook input', () => { + test.each(['{', 'null', '[]', '"text"', '{}'])('silently ignores %s', input => silent(invoke(input))); + test.each([ + { tool_name: 'Edit', tool_input: null }, + { tool_name: 'Edit', tool_input: { file_path: 1 } }, + { tool_name: 'Edit', tool_input: { file_path: `${features}/x/view\0.ts` } }, + { + tool_name: 'apply_patch', + tool_input: { command: `*** Begin Patch\n*** Add File: ${features}/x/view\0.ts\n+text\n*** End Patch` }, + }, + { tool_name: 'Bash', tool_input: { file_path: `${features}/x/x.ts` } }, + { tool_name: 'Bash', tool_input: { command: `*** Begin Patch\n*** Add File: ${features}/x/x.ts\n*** End Patch` } }, + { tool_name: 'apply_patch', tool_input: { command: `echo hi\n*** Add File: ${features}/x/x.ts` } }, + { tool_name: 'apply_patch', tool_input: { command: `*** Begin Patch\n*** Add File: ${features}/x/x.ts` } }, + ])('ignores malformed or unsupported payload %#', input => silent(invoke({ cwd: root, ...input }))); + + test('requires a valid cwd and Git root', () => { + const input = edit(`${features}/x/x.ts`); + delete input.cwd; + silent(invoke(input)); + silent(invoke({ ...input, cwd: 42 })); + silent(invoke({ ...input, cwd: `${root}\0` })); + silent(invoke({ ...input, cwd: tmpdir() })); + silent(invoke({ ...input, cwd: join(root, 'absent') })); }); +}); - test('passes a filled audit section', () => { - expect(auditProblem('## Inventory\n- a\n## Parity audit\n| row | Migrated |')).toBeUndefined(); +describe('explicit read-only review', () => { + test('prints sorted scope without modifying files or registering future scope', () => { + const before = readdirSync(root, { recursive: true }).sort(); + const status = git('status', '--porcelain', '--untracked-files=all'); + const result = invoke('', ['review', 'sign-up-button', 'historical-feature', 'sign-up-button']); + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toContain('historical-feature, sign-up-button'); + expect(result.stdout).toContain('packages/mosaic/docs/parity-audit.md'); + expect(result.stdout).not.toMatch(/if.*migrated behavior|passed|verified/i); + expect(readdirSync(root, { recursive: true }).sort()).toEqual(before); + expect(git('status', '--porcelain', '--untracked-files=all')).toBe(status); + silent(invoke(edit('README.md'))); }); + + test.each([[], ['../x'], ['X'], ['x/y'], ['--help'], ['x', ''], ['-x'], ['x--y']])( + 'rejects invalid review arguments %j', + (...args) => { + const result = invoke('', ['review', ...args]); + expect(result.status).toBe(1); + expect(result.stderr).toContain('Usage:'); + expect(result.stdout).toBe(''); + }, + ); + + test('repeated and concurrent invocations stay independent and create no state', async () => { + const before = readdirSync(root, { recursive: true }).sort(); + const inputs = ['alpha', 'beta', 'alpha']; + const results = await Promise.all( + inputs.map(name => { + const run = promisify(execFile)(process.execPath, [script, 'review', name], { + cwd: root, + env: { ...process.env, TMPDIR: root }, + timeout: 2000, + }); + run.child.stdin.end(); + return run; + }), + ); + results.forEach((result, index) => { + expect(result.stdout).toContain(inputs[index]); + expect(result.stdout).not.toContain(inputs[index] === 'alpha' ? 'beta' : 'alpha'); + }); + for (let count = 0; count < 4; count++) { + advisory(invoke(edit(`${features}/sign-out-button/view.tsx`)), ['sign-out-button']); + } + expect(readdirSync(root, { recursive: true }).sort()).toEqual(before); + }); +}); + +describe('hook registration', () => { + test.each(['.claude/settings.json', '.codex/hooks.json'])( + '%s retains lint hooks and uses only current-action parity advice', + path => { + const config = JSON.parse(readFileSync(new URL(`../${path}`, import.meta.url), 'utf8')); + const stop = config.hooks.Stop.flatMap(entry => entry.hooks); + expect(stop).toHaveLength(2); + expect(stop.every(hook => hook.timeout === 180 && hook.command.includes('pnpm exec eslint'))).toBe(true); + expect(stop.some(hook => hook.command.includes('packages/mosaic'))).toBe(true); + expect(stop.some(hook => hook.command.includes('packages/swingset/src'))).toBe(true); + expect(config.hooks.UserPromptSubmit).toBeUndefined(); + expect(config.hooks.PostToolUse).toBeUndefined(); + const pre = config.hooks.PreToolUse; + expect(pre).toHaveLength(1); + expect(pre[0].matcher).toBe(path.startsWith('.claude') ? 'Write|Edit' : 'apply_patch|Edit|Write'); + expect(pre[0].hooks[0].if).toBeUndefined(); + expect(pre[0].hooks[0].command).toContain('mosaic-parity-gate.mjs'); + if (path.startsWith('.claude')) { + expect(config.disableBypassPermissionsMode).toBe(true); + expect(config.permissions.deny).toHaveLength(20); + expect(config.hooks.SessionStart).toHaveLength(1); + } + }, + ); }); From 21701ca533d009c12be19ec6c42cf94163898866 Mon Sep 17 00:00:00 2001 From: austincalvelage Date: Thu, 5 Nov 2026 11:17:17 -0700 Subject: [PATCH 12/12] test(repo): run Mosaic parity checks in CI --- .github/workflows/ci.yml | 2 +- scripts/mosaic-parity-gate.test.mjs | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a8f50d993e5..d16c7f4d7e6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -381,7 +381,7 @@ jobs: - name: Run verify script tests if: matrix.node-version == '24.15.0' - run: pnpm vitest run --root scripts verify-attach verify-flaky + run: pnpm vitest run --root scripts verify-attach verify-flaky mosaic-parity-gate - name: Upload Turbo Summary uses: actions/upload-artifact@v4.6.2 diff --git a/scripts/mosaic-parity-gate.test.mjs b/scripts/mosaic-parity-gate.test.mjs index 521aeabe874..8a1427affe3 100644 --- a/scripts/mosaic-parity-gate.test.mjs +++ b/scripts/mosaic-parity-gate.test.mjs @@ -251,7 +251,13 @@ describe('hook registration', () => { expect(pre).toHaveLength(1); expect(pre[0].matcher).toBe(path.startsWith('.claude') ? 'Write|Edit' : 'apply_patch|Edit|Write'); expect(pre[0].hooks[0].if).toBeUndefined(); - expect(pre[0].hooks[0].command).toContain('mosaic-parity-gate.mjs'); + expect(pre[0].hooks).toEqual([ + { + type: 'command', + command: 'node "$(git rev-parse --show-toplevel)/scripts/mosaic-parity-gate.mjs" write', + timeout: 10, + }, + ]); if (path.startsWith('.claude')) { expect(config.disableBypassPermissionsMode).toBe(true); expect(config.permissions.deny).toHaveLength(20);