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..35c99ab3d7c 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -58,6 +58,18 @@ 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 + +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, 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. 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. 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 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. The file shape and how a wrapper composes the layers are in [Features](docs/features.md). diff --git a/packages/mosaic/docs/migration.md b/packages/mosaic/docs/migration.md index a426c526c54..966154286b0 100644 --- a/packages/mosaic/docs/migration.md +++ b/packages/mosaic/docs/migration.md @@ -4,6 +4,8 @@ Read this when you move a legacy component (usually under `packages/ui/src/compo Worked example: `src/features/reverification/`. +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. + ## Rules - 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. @@ -15,7 +17,11 @@ 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. 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 | | --------------------------------- | -------------------------------------------------- | @@ -30,11 +36,30 @@ 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 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 +# sign-out-button parity + +Legacy: packages/react/src/components/SignOutButton.tsx + +## Inventory + +- `redirectUrl` defaults to `/` +- … +``` + ## 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 +81,11 @@ 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. +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. + +Tests only cover branches you remembered to write. Launch a read-only subagent with the prompt in [Parity audit](parity-audit.md). Give it the legacy and new revisions, file paths, shared types, and accepted decisions. Use the classifications and evidence columns there. 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`, next to the Phase 1 inventory. The table guides the work 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 40dc17d5df0..27eafe3d83c 100644 --- a/packages/mosaic/docs/parity-audit.md +++ b/packages/mosaic/docs/parity-audit.md @@ -1,55 +1,167 @@ -# Parity audit +# Parity audit — Phase 4 reference -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). +The parity audit is the confidence step of a Mosaic migration. It diffs the +legacy implementation and shared API contract against the new behavior, +including the helpers and Clerk runtime they call. -## Rules +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. -- 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. -- 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 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. +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 a read-only 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). +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. ``` -To find the legacy behaviors, start from the search in [Migration → Phase 1](migration.md#phase-1-inventory-the-legacy-behavior). - -## Output - -| 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` | - -## What it catches - -An audit of the organization domains section, migrated before Mosaic was its own package (that code is not in `packages/mosaic`), found three dropped behaviors, each tracked only by a `// TODO` and none failing a test or typecheck: +## Output table format + +| 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) + +```bash +rg -n 'useEffect|handleError|card\.setError|useReverification|revalidate|/ packages/react/src/components/.tsx +``` -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. +## Worked example — `OrganizationProfileDomainsSection` + +A historical example: the domains section was migrated before Mosaic became +its own package, and that code is not in `packages/mosaic` today. The audit +run against the finished migration surfaced three candidates for investigation, +each tracked only by a buried `// TODO`: + +1. **Per-field error mapping.** Legacy `handleError(err, [field])` mapped errors + to the specific field; the new add/verify flow collapses every failure to one + `errorMessage` string in the controller's machine context. +2. **Step-up reverification on domain delete.** Legacy `RemoveDomainForm` left a + TODO to wrap `domain.delete()` in `useReverification`; the new remove flow + omits the step-up entirely. +3. **Pending-invitations callout.** Legacy `useCalloutLabel` computed a count + from `totalPendingInvitations + totalPendingSuggestions`; the new view + receives the counts but never renders the callout. + +None of these failed a test or a typecheck. Establish the baseline contract before +classifying them. In particular, an unimplemented legacy reverification TODO +does not establish a regression. Require a fix only when the migration loses +behavior or violates an applicable rule. Record inherited improvements separately +and keep any approved deferral and ticket attached to its underlying finding. 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); + } + }, + ); +});