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/settings.json b/.claude/settings.json index d6148f47f42..4c1a9826d9d 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -35,6 +35,19 @@ ] } ], + "PreToolUse": [ + { + "matcher": "Write", + "hooks": [ + { + "type": "command", + "command": "node \"$(git rev-parse --show-toplevel)/scripts/mosaic-parity-gate.mjs\" write", + "timeout": 10, + "if": "Write(/packages/mosaic/src/features/**)" + } + ] + } + ], "Stop": [ { "hooks": [ @@ -47,6 +60,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 \"$(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 5cdb17f0017..8b281808837 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -38,6 +38,17 @@ Before you change a file that matches a pattern below, read its guide, even for | `src/**/*.view.tsx` | [Views](docs/views.md) | | `src/**/__tests__/**`, `src/**/*.test.ts(x)`, `src/**/*.test-d.ts` | [Testing](docs/testing.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 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 e71beb3c410..ba864df8766 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 | @@ -45,13 +47,26 @@ 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 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 @@ -102,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: @@ -114,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 93dbd7b9fdf..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 @@ -36,6 +37,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 @@ -50,7 +56,7 @@ by a `// TODO` (these are the regressions at risk of shipping). ```bash rg -n 'useEffect|handleError|card\.setError|useReverification|revalidate|/ + packages/ui/src/components// packages/react/src/components/.tsx ``` ## Worked example — `OrganizationProfileDomainsSection` 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(); + }); +});