docs(mosaic): refine agent guidance from PR reviews

.changeset/mosaic-review-guidance.md

@@ -0,0 +1,2 @@
+---
+---

.claude/skills/mosaic/SKILL.md

@@ -68,20 +68,11 @@ adding a new pattern. Check it before writing any of those by hand.
 
 ## Which reference to read
 
-| You are…                                                                                               | Read                                                                             |
-| ------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------- |
-| Building on / authoring a headless primitive (`src/primitives/`)                                       | `packages/mosaic/docs/headless.md`                                               |
-| Styling a component (tokens, `stylex.create`, `themeProps`, CSS build)                                 | `packages/mosaic/docs/stylex.md`                                                 |
-| Building an enter/exit transition, a row that expands and collapses, or any motion that reads as wrong | `packages/mosaic/docs/motion.md`                                                 |
-| Writing the model (the Clerk adapter, `status`, permissions)                                           | `packages/mosaic/docs/models.md`                                                 |
-| Writing the controller (local state, pending, action wrapping)                                         | `packages/mosaic/docs/controllers.md`                                            |
-| Authoring or debugging a state machine, or wiring one to React                                         | `packages/mosaic/docs/controllers.md` → "Machines", then `src/machine/README.md` |
-| Writing the view (rendering plain props)                                                               | `packages/mosaic/docs/views.md`                                                  |
-| Testing a feature (feature tests, unit tests)                                                          | `packages/mosaic/docs/testing.md`                                                |
-| Migrating a legacy component into Mosaic (the end-to-end workflow)                                     | `packages/mosaic/docs/migration.md`                                              |
-| Running the parity audit that guards a migration                                                       | `packages/mosaic/docs/parity-audit.md`                                           |
-
-The migration workflow (`migration.md`) ties the flow references together: it
+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`.
 
@@ -114,8 +105,6 @@ this axis.
 Which surface the dialog holds, and the geometry that comes with it.
 ```
 
-This is the opposite of the rule for **code**, where a comment earns its place by
-explaining what the code cannot say for itself — the cascade fight behind a
-`null`, the measured reason a duration is what it is. Keep that reasoning where a
-future maintainer will hit it, which is the source file. The trade you rejected
-belongs in a code comment or the PR description; the docs get the conclusion.
+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.

packages/mosaic/AGENTS.md

@@ -5,6 +5,23 @@ Mosaic is Clerk's next-generation component library. These rules apply to everyt
 - `ARCHITECTURE.md` (this folder) is the contract: tokens, the `.cl-<slot>` + `data-<axis>` styling API, the CSS build, and the model → controller → view split.
 - `docs/` is the how-to, one file per task: StyleX authoring, motion, headless primitives, each flow layer, testing, and migration.
 
+## Task guides
+
+Read the guide for the task you are doing. Paths below are relative to this package.
+
+| Task                                      | Read                                                                              |
+| ----------------------------------------- | --------------------------------------------------------------------------------- |
+| Author a headless primitive               | [Headless primitives](docs/headless.md)                                           |
+| Style a component or change the CSS build | [StyleX](docs/stylex.md)                                                          |
+| Add or debug motion                       | [Motion](docs/motion.md)                                                          |
+| Write a model                             | [Models](docs/models.md)                                                          |
+| Write a controller                        | [Controllers](docs/controllers.md)                                                |
+| Author or debug a state machine           | [Controllers](docs/controllers.md), then [Machine runtime](src/machine/README.md) |
+| Write a view                              | [Views](docs/views.md)                                                            |
+| Test a feature                            | [Testing](docs/testing.md)                                                        |
+| Migrate legacy behavior                   | [Migration](docs/migration.md)                                                    |
+| Review migration parity                   | [Parity audit](docs/parity-audit.md)                                              |
+
 ## Layers in one paragraph
 
 A flow lives in `src/features/<feature>/`. 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.
@@ -25,6 +42,7 @@ Most problems a feature hits already have a shared answer. Find the row for your
 | Moving focus after a list row is removed                                | `useListRemovalFocus` (`src/hooks/`)                                                                          |
 | A debounced, abortable async check as the user types                    | `useDebouncedAsync` (`src/hooks/`)                                                                            |
 | A spinner that should not flash or flicker                              | `useSpinDelay` (`src/hooks/`)                                                                                 |
+| Current time for display labels                                         | `useNow` + `MosaicProvider` ([#10098](https://github.com/clerk/javascript/pull/10098), once available)        |
 | An inline message row that animates open and closed                     | `useHeldMessage`, `useMessageHeight`, `FeedbackBody` (`src/utils/feedback.tsx`) + `styles/feedback.styles.ts` |
 | Siblings that slide when an item is added or removed                    | `useLayoutAnimation` (`src/primitives/hooks/`)                                                                |
 | An interaction with an async lifecycle or two values that move together | A machine in the controller file, via `setup()` (`src/machine/`)                                              |
@@ -41,6 +59,14 @@ Stop and check the table if you are writing any of these in a feature:
 - `setTimeout` to debounce or to delay a spinner
 - `stylex.create` inside a `.tsx` file
 
+## Readability before complexity scores
+
+Do not extract a helper or introduce a machine only to reduce a lint score.
+Avoid turning a component into a hook solely to work around React's children
+typing. An abstraction must clarify a responsibility or hide useful complexity. When complexity is inherent, keep the relevant logic
+together and explain the tradeoff in review. Follow repository rules for any lint
+exception; this guidance does not authorize a suppression.
+
 ## When nothing fits: ask first
 
 If no row fits, or a shared helper almost fits but would need a change, **stop and ask the user before writing code**. Say:

packages/mosaic/ARCHITECTURE.md

@@ -88,19 +88,27 @@ import { MosaicProvider } from './mosaic-provider';
 
 Styles are declared once per component with `stylex.create`, keyed off the same axes the component exposes as props, then fused with `themeProps` output and the props the component was called with:
 
-```tsx
-const styles = stylex.create({
+Declare the styles in `src/components/button/button.styles.ts`:
+
+```ts
+export const styles = stylex.create({
   base: { display: 'inline-flex', borderRadius: radiusVars['--cl-radius-md'] },
 });
 
 // `color` and `variant` are independent props, but every pair resolves to one style, so they are
 // keyed compositely rather than merged at render time.
-const variants = stylex.create({
+export const variants = stylex.create({
   'filled-primary': { backgroundColor: colorVars['--cl-color-brand'], borderColor: 'transparent' },
   'filled-negative': { backgroundColor: colorVars['--cl-color-negative'], borderColor: 'transparent' },
   'outline-primary': { backgroundColor: 'transparent', borderColor: colorVars['--cl-color-brand'] },
   'outline-negative': { backgroundColor: 'transparent', borderColor: colorVars['--cl-color-negative'] },
 });
+```
+
+Import them in `src/components/button/button.tsx`:
+
+```tsx
+import { styles, variants } from './button.styles';
 
 export const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(function Button(
   { color = 'primary', variant = 'filled', size = 'md', disabled, xstyle, ...rest },
@@ -168,7 +176,7 @@ once the interaction has an async lifecycle or two values that must change
 together. A controller can also do both — a machine for the coordinated subset,
 `useState` for the UI-only flags beside it. The choice is invisible from the
 outside: the controller returns plain props either way, so the view cannot tell
-and neither can its tests. `machine/ADOPTION.md` holds the criteria.
+and neither can its tests. `src/machine/ADOPTION.md` holds the criteria.
 
 ### File shape
 
@@ -450,7 +458,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
-produce the flag soup the machine exists to prevent. `machine/ADOPTION.md` has
+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.
 
 ### Views

packages/mosaic/docs/controllers.md

@@ -177,6 +177,31 @@ surface shows no error still logs it, so a failure is never silent
   `useSpinDelay`, a `mode` forced by a capability flag — anything the view would
   otherwise have to re-derive.
 
+## Composition and dialog lifetime
+
+When a wrapper composes the controller above the view, prefer one
+controller-derived contract, forwarding the model's display data through the
+controller when needed. Where practical, mount a
+separate component only once the model is ready, so its controller receives the
+narrowed ready model instead of repeating loading branches. Keep the supported
+injected-effect leaf composition described in `views.md`.
+
+Decide which dialog state survives closing and reopening. Verify that the next
+interaction does not retain an unintended error or draft, and keep reset behavior
+consistent with the pending action's lifetime. Handle rejected dynamic imports
+through an appropriate error owner rather than leaving unhandled rejections.
+
+For a dialog that clears its draft and error on close, test a failed submit,
+close it, and reopen it. Assert that the error is gone and the field has its
+initial value. If the draft is meant to survive, assert the saved draft instead.
+Hold the submit request while dismissing to verify that closing does not unlock
+a second mutation or abandon the first. When a feature uses a dynamic import,
+reject the loader and assert the error UI and recovery behavior it promises.
+
+When introducing lazy loading or Suspense, consider a boundary around dialog
+content that leaves the trigger and parent available. Choose that boundary from
+the feature's loading behavior; it does not require restructuring every dialog.
+
 ## Rules
 
 - **No Clerk imports.** If a controller needs a Clerk fact, the model supplies it
@@ -226,11 +251,16 @@ One path, so every error a user sees goes through the `errors.*` catalog:
   password section maps its own errors (`user-profile-password-errors.ts`).
 - **`errorFallback` is the feature's copy for faults Clerk cannot describe**
   (code bugs, non-Clerk errors). An `UnlocalizableError` renders the fallback,
-  never its cause's `.message`. A network failure has its own catalog entry,
-  `network_error`.
+  never its cause's `.message`. A failure carrying the Clerk code `network_error`
+  has catalog copy, but network failures may also arrive as raw errors and remain
+  unlocalizable. Preserve the cause and check the unknown-error fallback too.
 - In tests, reject with `clerkApiError(code, message, { paramName })` from
   `src/__tests__/clerk-errors.ts` to assert catalog copy. Reject with a plain
   `Error` (silence `console.error`) to assert the fallback.
+  For example, reject with `new Error('socket disconnected')`, assert the
+  feature's fallback is visible, and assert that `socket disconnected` is not
+  rendered. Test `clerkApiError('network_error', 'server detail')` separately
+  to verify catalog copy rather than assuming every network fault has a code.
 
 ## Testing
 

packages/mosaic/docs/headless.md

@@ -46,7 +46,9 @@ params are documented on `UseRenderParams*` in `utils/use-render.tsx`.
 - **`mergeProps(internal, consumer)` — internal first, consumer second.** Event
   handlers chain (internal fires, then consumer), `style` shallow-merges,
   `className` concatenates, everything else the consumer overwrites. This lets
-  consumers extend behavior without breaking ARIA/handlers the primitive owns.
+  consumers extend event handlers while retaining the internal handlers. ARIA
+  attributes can be overwritten. A render function must apply the computed props
+  and ref to preserve the primitive's behavior.
 - **Positioners pass `enabled: mounted`**, so the floating DOM doesn't exist
   until the first frame (`select/select-positioner.tsx`).
 

packages/mosaic/docs/migration.md

@@ -34,13 +34,14 @@ hit is a row you must consciously place or drop later:
 | `revalidate` / `.reload()`        | cache invalidation timing after a mutation         |
 | `<Protect` / `checkAuthorization` | permission gating and section visibility           |
 | `useCalloutLabel`                 | derived, computed labels / counts                  |
+| `useFetch` / `useOrganization`    | data loading, pagination, loading/empty states     |
+| `useInView`                       | infinite-scroll / intersection triggers            |
 
-Most of these already have a Mosaic home: `packages/mosaic/AGENTS.md` →
-"Reuse before you write" maps each problem to its helper (`handleError` →
-`save`, `useReverification` → `useReverificationFlow`, and so on). Use that
-table when you place a behavior in Phase 2.
-| `useFetch` / `useOrganization` | data loading, pagination, loading/empty states |
-| `useInView` | infinite-scroll / intersection triggers |
+Use `packages/mosaic/AGENTS.md` → "Reuse before you write" for shared helpers
+such as `save` and `useReverificationFlow`. The table does not cover every
+legacy behavior. Place revalidation and loading rules in the model, and inspect
+existing features for pagination and derived callouts before choosing their
+implementation in Phase 2.
 
 ```bash
 rg -n 'useEffect|handleError|card\.setError|useReverification|revalidate|<Protect|checkAuthorization|useCalloutLabel|useFetch|useInView' \
@@ -71,6 +72,11 @@ Two rows deserve extra care because they have no obvious home:
 - **Pure derivation** (slot layout, ordering a consumer's list) belongs in
   `*.layout.ts` / `*.utils.ts` beside the view, where it gets its own test.
 
+Share business-rule helpers when legacy and Mosaic must keep the same behavior.
+Do not extract similar presentation code solely to remove duplication when the
+designs may diverge. Inspect the existing helper and its callers before deciding
+where shared logic belongs.
+
 ## Phase 3 — Implement and test
 
 File shape: `<feature>.model.ts(x)` · `<feature>.controller.ts(x)` ·

packages/mosaic/docs/models.md

@@ -96,6 +96,25 @@ drops the fallback instead of holding the space open. Keep the two apart.
 - No React state machinery beyond the Clerk hooks themselves; the model is a
   derivation of what Clerk currently says.
 
+## Time-dependent display data
+
+Use the shared `useNow` hook and `MosaicProvider` clock support introduced in
+[#10098](https://github.com/clerk/javascript/pull/10098). These APIs must be
+available in the consuming branch before following this guidance.
+
+`MosaicProvider` supplies a shared initial `Date`. Read it with `useNow` from
+`src/hooks/use-now.ts` in the consuming component or controller, then pass the
+value to pure formatting helpers. For relative labels that refresh each minute,
+use `useNow({ updateInterval: 60_000 })`. Without an interval, the hook retains
+its initial value. Reuse this API instead of adding a feature-specific clock
+provider or timer.
+
+In tests, supply a fixed `Date` through `MosaicNowProvider`, assert the displayed
+label, then advance fake timers across a label boundary and assert the update.
+If a feature supports SSR, also verify matching initial times during server
+rendering and hydration. Sharing time within one provider tree does not by
+itself guarantee agreement between server and client.
+
 ## Testing
 
 The feature test covers the model by default, running it against a real Clerk

packages/mosaic/docs/testing.md

@@ -94,6 +94,10 @@ it('makes the selected organization active and closes', async () => {
   await user.click(screen.getByRole('button', { name: /Open account menu/ }));
   await user.click(await screen.findByRole('button', { name: 'Other' }));
   await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull());
+
+  await user.click(screen.getByRole('button', { name: /Open account menu/ }));
+  expect(await screen.findByRole('button', { name: 'Acme' })).toBeInTheDocument();
+  expect(screen.queryByRole('button', { name: 'Other' })).toBeNull();
 });
 ```