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 eafc2715b1c..2d8562d83c0 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -35,6 +35,18 @@ ] } ], + "PreToolUse": [ + { + "matcher": "Write|Edit", + "hooks": [ + { + "type": "command", + "command": "node \"$(git rev-parse --show-toplevel)/scripts/mosaic-parity-gate.mjs\" write", + "timeout": 10 + } + ] + } + ], "Stop": [ { "hooks": [ diff --git a/.codex/hooks.json b/.codex/hooks.json index 9c06bbed754..fe1494fe991 100644 --- a/.codex/hooks.json +++ b/.codex/hooks.json @@ -15,6 +15,18 @@ } ] } + ], + "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/.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/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index 04fbe3de9e7..dd5d0e4e581 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -25,7 +25,7 @@ The docs describe the Mosaic we are building toward. Code that differs from them Read every line that matches your task. Paths below are relative to this package. -- Add a feature, or a file to one → [Features](docs/features.md), then the guide for each layer below. If the repo's `packages/ui` has a legacy version, [Migration](docs/migration.md) first +- Add a feature, or a file to one → [Features](docs/features.md), then the guide for each layer below. If `packages/ui` or `packages/react` has a legacy version, [Migration](docs/migration.md) first, even for a headless port - Write or change a model, or add a Clerk read, mutation or permission check → [Models](docs/models.md) - Build a feature from a design before Clerk's API can back it → [Models → Before the API exists](docs/models.md#before-the-api-exists) - Write or change a controller, or how a feature handles pending state, errors, dialogs or removal focus → [Controllers](docs/controllers.md) @@ -41,7 +41,7 @@ Read every line that matches your task. Paths below are relative to this package - Write, move or review a test → [Testing](docs/testing.md) - Fix or silence a lint error → [Lint](docs/lint.md) - Move a legacy component's logic or styles into Mosaic → [Migration](docs/migration.md) -- Check that a migration kept every legacy behavior → [Parity audit](docs/parity-audit.md) +- Implement or review a migration, or check that it kept every legacy behavior → [Parity audit](docs/parity-audit.md) - Add or change a component's Swingset page → [Documenting components](docs/documenting-components.md) - Prove a change in a browser → [verify-mosaic](.claude/skills/verify-mosaic/SKILL.md) - Remove or rename a public export, prop, `--cl-*` token or styling hook, or change a public type or default → [Breaking changes](docs/breaking-changes.md) diff --git a/packages/mosaic/docs/features.md b/packages/mosaic/docs/features.md index 019b598af6a..56ab559f205 100644 --- a/packages/mosaic/docs/features.md +++ b/packages/mosaic/docs/features.md @@ -6,7 +6,7 @@ Worked example: `src/features/reverification/` (model, controller, view, types, ## Rules -- Before you add a feature, check `packages/ui/src/components/` for a legacy version. If one exists, this is a migration: read [Migration](migration.md) first. +- Before you add a feature, check `packages/ui/src/components/` and `packages/react/src/components/` for a legacy version, including for a headless port. If one exists, this is a migration: read [Migration](migration.md) first. - Only a `*.model.ts(x)`, or a shared Clerk-reading hook in `src/hooks/` (`useMosaicEnvironment`, `useMosaicRouter`, `useMosaicSupportEmail`), imports Clerk hooks or calls Clerk resource methods. - What crosses each boundary is plain data: no Clerk resource reaches the controller, and no machine snapshot reaches the view. - A feature's machine lives in its `*.controller.ts(x)`, not in `src/machine/` and not in a `*.machine.ts`. diff --git a/packages/mosaic/docs/migration.md b/packages/mosaic/docs/migration.md index a426c526c54..21ffced7cf8 100644 --- a/packages/mosaic/docs/migration.md +++ b/packages/mosaic/docs/migration.md @@ -1,6 +1,6 @@ # Migration -Read this when you move a legacy component (usually under `packages/ui/src/components//`) into Mosaic, whether its flow logic or only its styles. It covers the five phases from inventory to ship; the finished shape is in [Features](features.md), and the parity check is in [Parity audit](parity-audit.md). +Read this when you move a legacy component (under `packages/ui/src/components//` or `packages/react/src/components/`) into Mosaic, including a headless port framed as a new feature, whether its flow logic or only its styles. It covers the five phases from inventory to ship; the finished shape is in [Features](features.md), and the parity check is in [Parity audit](parity-audit.md). Worked example: `src/features/reverification/`. @@ -8,6 +8,8 @@ Worked example: `src/features/reverification/`. - The legacy component is the spec. A legacy file fuses rendering, data fetching and flow logic, and splitting it three ways drops behavior that was only implicit (a per-field error map, a step-up reverification, an empty-state gate, a derived callout) without failing a test or a type check. - Inventory the legacy behavior first, then make each layer account for a row. Do not write the layers first and ask what you missed. +- Apply this workflow to the current task, not every Mosaic change already on the branch. +- Write the inventory to `tmp/mosaic-parity/.md` before the model, with the legacy path and the features in scope. Run the [Parity audit](parity-audit.md) into the same file before you call the work done, review it or open a PR. - Every inventory row lands in exactly one layer, and later in exactly one parity bucket. A row with no home is a behavior you are about to drop. - A deferred behavior gets a tracked ticket, never a buried `// TODO`. - Share a business-rule helper when legacy and Mosaic must behave the same. Do not extract similar presentation code only to remove duplication, since the designs may diverge. @@ -15,7 +17,7 @@ Worked example: `src/features/reverification/`. ## Phase 1: inventory the legacy behavior -Grep the legacy files for the primitives that carry hidden logic. Every hit is a row you must place or consciously drop: +Locate the legacy files: the prebuilt component under `packages/ui/src/components//`, the `@clerk/react` component under `packages/react/src/components/`, or both. Grep them for the primitives that carry hidden logic. Every hit is a row you must place or consciously drop: | Primitive | The behavior it usually hides | | --------------------------------- | -------------------------------------------------- | @@ -30,11 +32,24 @@ Grep the legacy files for the primitives that carry hidden logic. Every hit is a ```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 or loading state, permission gate, revalidate, reset-on-close and derived label. The table does not cover every behavior; read the files too. For shared answers (`save`, `useReverificationFlow`), use [Reuse before you write](../AGENTS.md#reuse-before-you-write). +Include shared prop types and mode-dependent options, defaults, callback order and error delivery; the full list is in [Parity audit → Rules](parity-audit.md#rules). Write the inventory to `tmp/mosaic-parity/.md`: + +```md +# checkout-button parity + +Legacy: packages/react/src/components/CheckoutButton.tsx + +## Inventory + +- `for="organization"` throws without an active organization +- … +``` + ## Phase 2: map every row to a layer - Clerk reads, mutations, permission gates, revalidate timing, first-load empty states and capability flags go in the **model** ([Models](models.md)). @@ -56,7 +71,7 @@ Test with one `*.feature.test.tsx` that uses the feature the way a user would ([ ## Phase 4: verify parity -Tests cover only the branches you remembered. Run the [Parity audit](parity-audit.md) against the legacy files and the new layers, and place every Phase 1 row as Migrated, Deliberately changed (such as infinite scroll becoming a "Load more" button) or Deferred. +Tests cover only the branches you remembered. Run the [Parity audit](parity-audit.md) against the legacy files and the new layers, and place every Phase 1 row as Migrated, Deliberately changed (such as infinite scroll becoming a "Load more" button) or Deferred, under `## Parity audit` in `tmp/mosaic-parity/.md`. ## Phase 5: ship diff --git a/packages/mosaic/docs/parity-audit.md b/packages/mosaic/docs/parity-audit.md index 40dc17d5df0..9b9f1da7968 100644 --- a/packages/mosaic/docs/parity-audit.md +++ b/packages/mosaic/docs/parity-audit.md @@ -1,45 +1,61 @@ # Parity audit -Read this when you review whether a migrated feature kept every legacy behavior, or before you open a migration PR. It covers how to run the audit and how to report it; the migration itself is in [Migration](migration.md). +Read this when you implement or review a migration of a `packages/ui` or `packages/react` component, change behavior a migration moved, or open a migration PR. It covers how to run the audit, how to classify what it finds and how to report it; the migration itself is in [Migration](migration.md). + +Worked example: [What it catches](#what-it-catches). ## Rules -- Classify every legacy behavior: each effect, guard, error path, empty or loading state, permission gate, revalidate or reload call, reset-on-close, derived label or count, and pagination trigger. +- Audit the current task, not every Mosaic change on the branch. Record the features in scope and the legacy and new revisions before you compare. When you review earlier feedback, compare against the commit the reviewer saw. +- Classify every legacy behavior: each prop and mode-dependent option, default, callback order, effect, guard, error path, empty or loading state, permission gate, revalidate or reload call, reset-on-close, derived label or count, and pagination trigger. Follow shared prop types, helpers and Clerk runtime calls until the observable outcome is clear; matching method names is not parity. - Each one is **Migrated** (points at a state, transition, context field or prop in the new code), **Deliberately changed** (names the new behavior and why) or **Deferred** (not implemented; say whether a ticket tracks it). +- A Migrated row cites the test or check that shows the behavior, not only the code that should produce it. A row with no such evidence is not Migrated yet; name the missing check. +- A Deliberately changed row cites the user's decision or the Mosaic rule behind it. If no one decided, mark it "decision needed"; do not infer intent from the code. - A Deferred item tracked only by a `// TODO` is a regression at risk of shipping. Surface it as a tracked item. -- Run it as a read-only subagent that returns a conclusion, not file dumps. The table guides the work and the PR discussion; nothing is committed. +- A defect the legacy component already had is not a parity row. List it below the table as an optional improvement. +- A difference that breaks a rule in [AGENTS.md](../AGENTS.md) or `docs/` is fixed or raised with the user, not shipped as intentional. +- An error the legacy component let reach the app, by a throw, a rejection or the console, stays observable. Losing it is a gap to fix, not a decision to ask about. Check that whatever owns the error still renders after the action fails: a failed sign-out can unmount its own button. +- For each async action in scope, check success, pending and a representative rejection through the feature's fake FAPI harness ([Testing](testing.md)). A mock of the Clerk method cannot prove that method's runtime effect. +- Run it as a read-only subagent that returns a conclusion, not file dumps. Put its table under `## Parity audit` in `tmp/mosaic-parity/.md`, beside the [Phase 1 inventory](migration.md#phase-1-inventory-the-legacy-behavior). `tmp/` is ignored by git, so nothing is committed. +- A filled table is not proof. Read the results of the checks it cites before you claim parity. +- List every Deliberately changed and Deferred row in the PR description, with its effect on consumers. ## The subagent prompt -Fill in the two file lists and hand this to a read-only agent: +Fill in the blanks and hand this to a read-only 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, -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` +Follow packages/mosaic/docs/parity-audit.md → Rules. Enumerate every behavior +it lists, find where each lives in the new files, and classify it. Read the +shared prop types and follow helpers and Clerk runtime calls to the observable +outcome. For callbacks, compare order and awaiting with docs/headless.md before +demanding legacy order. For redirects, check explicit URLs and omitted props +with a non-default provider setting. 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). +guard). Never dump whole files. Return the table below, most severe gaps first, +with the test command or case behind each Migrated row. Then list Deferred +items tracked only by a `// TODO`, legacy defects as optional improvements, and +every row that breaks a rule in packages/mosaic/AGENTS.md or +packages/mosaic/docs/, quoting the rule. ``` To find the legacy behaviors, start from the search in [Migration → Phase 1](migration.md#phase-1-inventory-the-legacy-behavior). +For a read-only review, `node scripts/mosaic-parity-gate.mjs review [feature ...]` prints the same reminder the edit hooks give. It does not run the audit. + ## Output | Legacy behavior | Layer it should live in | Status | Evidence (legacy → new) | @@ -53,3 +69,5 @@ An audit of the organization domains section, migrated before Mosaic was its own 1. Per-field error mapping: every add or verify failure collapsed to one `errorMessage`. 2. Step-up reverification on domain delete was omitted. 3. The pending-invitations callout count reached the view but was never rendered. + +An audit of the headless auth buttons (#10588) found a rejected action whose error `usePendingAction` captured but nothing showed, an `unsafeMetadata` option the shared props type allows only in modal mode, and a sign-out path a method mock hid from the test. It also found missing MetaMask error mapping, which the legacy button lacked too; that is an optional improvement, not a parity row. diff --git a/scripts/mosaic-parity-gate.mjs b/scripts/mosaic-parity-gate.mjs new file mode 100644 index 00000000000..1235f7abdcb --- /dev/null +++ b/scripts/mosaic-parity-gate.mjs @@ -0,0 +1,88 @@ +import { spawnSync } from 'node:child_process'; +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(' '); +} + +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; + } + if (mode !== 'write' || args.length > 0) { + return; + } + + let input; + try { + input = JSON.parse(readFileSync(0, 'utf8')); + } catch { + return; + } + if ( + !input || + typeof input.cwd !== 'string' || + !isAbsolute(input.cwd) || + input.cwd.includes('\0') || + !input.tool_input + ) { + return; + } + + 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 (paths.length === 0 || paths.some(path => path.includes('\0'))) { + return; + } + + const git = spawnSync('git', ['rev-parse', '--show-cdup'], { cwd: input.cwd, encoding: 'utf8' }); + if (git.status !== 0) { + return; + } + 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) }, + }), + ); + } +} + +main(); diff --git a/scripts/mosaic-parity-gate.test.mjs b/scripts/mosaic-parity-gate.test.mjs new file mode 100644 index 00000000000..2bdf207d9fa --- /dev/null +++ b/scripts/mosaic-parity-gate.test.mjs @@ -0,0 +1,268 @@ +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'; + +const script = fileURLToPath(new URL('./mosaic-parity-gate.mjs', import.meta.url)); +const features = 'packages/mosaic/src/features'; +let root; + +function write(path, text = 'export {};') { + mkdirSync(dirname(join(root, path)), { recursive: true }); + writeFileSync(join(root, path), text); +} + +function git(...args) { + const result = spawnSync('git', args, { cwd: root, encoding: 'utf8' }); + expect(result.status, result.stderr).toBe(0); + return result.stdout; +} + +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`); +}); + +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('advises on new models without requiring notes or counterpart matches', () => { + advisory(invoke(edit(`${features}/renamed-feature/renamed-feature.model.ts`, 'Write')), ['renamed-feature']); + }); + + 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('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('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') })); + }); +}); + +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 lint'))).toBe(true); + expect(stop.some(hook => hook.command.includes('packages/mosaic'))).toBe(true); + expect(stop.some(hook => hook.command.includes('packages/swingset'))).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).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); + expect(config.hooks.SessionStart).toHaveLength(1); + } + }, + ); +});