From 699492903cb938c9b8982ad63618567a46c10bdb Mon Sep 17 00:00:00 2001 From: Arthur Breton Date: Wed, 10 Jun 2026 19:17:23 +0800 Subject: [PATCH 1/4] feat(perps): add multiproject validation skill --- .../perps/knowledge/review-antipatterns.md | 8 ++ .../repos/metamask-extension.md | 2 + domains/perps/skills/review-perps-pr/skill.md | 7 +- .../validate-perps-multiproject/skill.md | 136 ++++++++++++++++++ 4 files changed, 151 insertions(+), 2 deletions(-) create mode 100644 domains/perps/skills/validate-perps-multiproject/skill.md diff --git a/domains/perps/knowledge/review-antipatterns.md b/domains/perps/knowledge/review-antipatterns.md index d0266453..ddab98b7 100644 --- a/domains/perps/knowledge/review-antipatterns.md +++ b/domains/perps/knowledge/review-antipatterns.md @@ -105,3 +105,11 @@ PRs that touch UI components must include testIDs so agentic recipes and E2E tes - **testID not in `Perps.testIds.ts`** — testIDs defined as inline strings instead of exported constants from `app/components/UI/Perps/Perps.testIds.ts`. All testIDs must be centralized so recipes can reference them by constant name. - **testID missing from the element that holds the value** — adding testID to a wrapper View instead of the `TextInput` or Text that actually contains the value. CDP fiber-walk reads `value` from the React element with the matching testID — the testID must be on the element that owns the state. - **TP/SL price inputs without testID** — the trigger price `TextInput` components in `PerpsTPSLView` (and similar order-form screens) frequently lack testIDs, making it impossible to assert the accepted decimal precision agentically. Any PR touching these screens must add `testID` to both the Take Profit and Stop Loss price inputs. + +## Component View Test Coverage + +Perps page/view tests should use the component-view test framework when they are exercising screen behavior through rendered UI and app state. Broad unit tests that render a page and mock hooks/selectors are a review smell. + +- **Component-view behavior tested as a unit test** — files such as `ui/pages/perps/**/index.test.tsx` or `app/components/UI/Perps/**/*.test.tsx` that render a whole page/view and assert UI behavior should be converted to `*.view.test.tsx` using the component-view test framework/skill. Keep simple unit tests only for focused pure logic, local helper functions, or narrow component contracts. +- **Hook/selector mocking in a page behavior test** — mocking selectors, hooks, or service modules to force page state bypasses the real state wiring. Drive behavior through framework state presets/renderers instead. If the framework cannot cover the case yet, keep the unit test focused and link a follow-up for the missing framework support. +- **Coverage drops during conversion** — converting to component-view tests must preserve the same coverage intent. If a scenario cannot move to component-view, document why and retain the smallest focused unit test needed to keep coverage. diff --git a/domains/perps/skills/review-perps-pr/repos/metamask-extension.md b/domains/perps/skills/review-perps-pr/repos/metamask-extension.md index 4e363fbf..a9d5ecfd 100644 --- a/domains/perps/skills/review-perps-pr/repos/metamask-extension.md +++ b/domains/perps/skills/review-perps-pr/repos/metamask-extension.md @@ -30,6 +30,8 @@ ui/hooks/perps/usePerpsOrderForm.ts -- formatCu **Hook consolidation** — extension merges several mobile hooks into one. When reviewing hook changes, check that the consolidated hook still covers all cases the separate mobile hooks handle. +**Component-view test fit** — Perps page tests under `ui/pages/perps/**/*.test.tsx` that render full pages or exercise UI behavior should be reviewed as component-view test candidates. Ask for conversion to the component-view test framework/skill while preserving coverage. Keep ordinary unit tests only for pure helpers, narrow rendering contracts, or cases the framework cannot cover yet; require that exception to be stated. + **Missing screens** — close-all, cancel-all, withdraw, order book, order details. Don't block PRs for these, but note if a PR introduces partial implementations that conflict with future full implementations. ## Validation diff --git a/domains/perps/skills/review-perps-pr/skill.md b/domains/perps/skills/review-perps-pr/skill.md index 7966c339..ddd0a811 100644 --- a/domains/perps/skills/review-perps-pr/skill.md +++ b/domains/perps/skills/review-perps-pr/skill.md @@ -59,7 +59,9 @@ loop, run each CLI as a live tmux pane instead of one-shot, and `/clear` between From the perps `knowledge/` dir: **review-antipatterns** (core checklist), architecture, connection-architecture, caching-architecture, formatting-rules, mobile-extension-map, shared-package-analysis, feature-flags, screens. Check both repos when a shared util/screen -changes. +changes. For page/view test changes, also enforce the component-view test guidance: broad +rendered UI behavior tests belong in the component-view framework/skill unless a focused unit +test is explicitly justified. ## Reviewer prompt (force a fresh full review every round) @@ -67,7 +69,8 @@ changes. Fresh full review of perps changes in at vs . No prior context. Load perps knowledge (review-antipatterns + the rest). You gate this before any human sees it. Every perps anti-pattern AND every nit (naming, magic number, missing testID, weak test, -.toFixed, fallback-display vs 0) = BLOCKER. APPROVE only if nothing is left to improve. +component-view behavior left as broad unit tests, .toFixed, fallback-display vs 0) = BLOCKER. +APPROVE only if nothing is left to improve. Return: VERDICT: APPROVE | REQUEST_CHANGES COMMIT: diff --git a/domains/perps/skills/validate-perps-multiproject/skill.md b/domains/perps/skills/validate-perps-multiproject/skill.md new file mode 100644 index 00000000..d84d55cc --- /dev/null +++ b/domains/perps/skills/validate-perps-multiproject/skill.md @@ -0,0 +1,136 @@ +--- +name: validate-perps-multiproject +description: Interactively validate perps changes across local MetaMask Core, Mobile, and Extension checkouts. Use when a Core @metamask/perps-controller change must be checked in Mobile/Extension, or when a Mobile/Extension perps change needs parity validation in the other client. Defaults to current checkout as owner, yalc for Core package transport, read-only validation targets, and the smallest meaningful proof; asks the user only when required folders or proof level cannot be resolved. +maturity: stable +--- + +# Validate Perps Multiproject + +Validate one perps change across multiple local MetaMask repo checkouts. + +## Defaults + +- **Owner checkout**: current repo/cwd unless a path is provided. +- **Folder layout**: assume Core/controller, Mobile, and Extension are sibling folders under one workspace. +- **Targets**: + - Core/controller owner -> validate in Mobile and Extension when available; + - Mobile owner -> validate parity in Extension; + - Extension owner -> validate parity in Mobile. +- **Transport**: `yalc` for `@metamask/perps-controller`; none for client parity. +- **Proof**: smallest meaningful proof: build/package smoke first; recipe/e2e or real UI flow when behavior changes. +- **Target edits**: forbidden unless explicitly allowed. +- **Cleanup**: required; restore validation checkouts to pre-state. + +## Step 0 — resolve folders or ask + +Do not guess missing folders. Discover likely local checkouts, then ask only for unresolved choices. + +```bash +HERE=$(git rev-parse --show-toplevel 2>/dev/null || pwd) +PARENT=$(dirname "$HERE") + +# Prefer sibling folders in the same workspace. +for name in core metamask-core controller metamask-mobile mobile metamask-extension extension; do + [ -d "$PARENT/$name" ] && echo "$PARENT/$name" +done + +# Optional fallback if repos are not siblings. +ROOT="${METAMASK_REPOS_DIR:-$HOME/dev/metamask}" +find "$ROOT" -maxdepth 1 -type d \ + \( -name 'core*' -o -name 'controller*' -o -name 'metamask-mobile*' -o -name 'mobile*' -o -name 'metamask-extension*' -o -name 'extension*' \) -print 2>/dev/null +``` + +If needed, ask one concise question using the runtime's interactive question tool when available: + +```text +I need the validation folders: +- Owner checkout: +- Validation target(s): +- Proof level: transport-only | type/import | recipe/e2e | real UI flow +- May validation targets be edited? default no +``` + +Echo the resolved contract before changing anything. + +## Worker context to inject + +```md +## Perps multiproject validation + +Owner: `` on `` +Targets: +- `` — — purpose: +- `` — — purpose: + +Rules: +1. Capture `git status --short --branch` in every checkout first. +2. Edit only the owner checkout unless a target is explicitly editable. +3. For Core package validation, build first, then publish via yalc; never publish stale `dist/`. +4. Run the smallest proof that reaches the changed perps behavior. +5. Label package transport-only checks as `transport-only`; do not call them E2E. +6. Cleanup yalc/package-manager changes and prove final target status matches pre-state. +``` + +## Core -> clients via yalc + +```bash +# Pre-state +for repo in /path/to/core /path/to/mobile /path/to/extension; do + echo "--- $repo" + git -C "$repo" status --short --branch + (cd "$repo" && yarn --version) +done + +# Build package; stop if this fails +cd /path/to/core +yarn workspace @metamask/perps-controller build + +# Publish package +export P="/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:$HOME/.yarn/bin:$HOME/.npm-global/bin:$HOME/.local/bin:$HOME/.asdf/shims" +cd /path/to/core/packages/perps-controller +PATH="$P" yalc publish --private + +# Install in each client; Yarn 4 uses singular skip-build +cd /path/to/client +PATH="$P" yalc add @metamask/perps-controller +yarn install --mode=skip-build + +grep -n "@metamask/perps-controller" package.json yalc.lock yarn.lock +ls -la .yalc/@metamask/perps-controller/dist +``` + +Run the selected proof in each client, then cleanup: + +```bash +cd /path/to/client +PATH="$P" yalc remove @metamask/perps-controller || true +git checkout -- package.json yarn.lock 2>/dev/null || true +rm -rf .yalc yalc.lock +git status --short --branch +``` + +## Client parity + +For Mobile <-> Extension checks: + +1. Load relevant perps knowledge: `mobile-extension-map`, `screens`, `shared-package-analysis`, architecture docs as needed. +2. Find the equivalent screen/hook/flow in the other client. +3. Validate with real flow evidence; do not inject UI state. +4. Report semantic differences separately from regressions. + +## Stop conditions + +Stop and report when the owner package cannot build, target checkout has unexpected dirt, cleanup would delete user work, target source edits are needed but not allowed, or required device/browser/credential context is missing. + +## Final answer + +```md +Validated against . +- Owner: @, status +- Targets: , +- Transport: +- Proof: => +- Artifacts: +- Cleanup: +- Follow-ups/blockers: +``` From f9044d6d3757a8620b987bf28f91e6c5cc0283d9 Mon Sep 17 00:00:00 2001 From: Arthur Breton Date: Wed, 10 Jun 2026 19:36:14 +0800 Subject: [PATCH 2/4] fix(skills): install domain knowledge with skills --- README.md | 2 +- domains/perps/skills/fix-perps-bug/skill.md | 2 +- domains/perps/skills/review-perps-pr/skill.md | 4 +-- .../validate-perps-multiproject/skill.md | 2 +- tools/install | 33 +++++++++++++++++-- 5 files changed, 36 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index b94fbe3a..8a237019 100644 --- a/README.md +++ b/README.md @@ -125,7 +125,7 @@ domains// scripts/ # optional helper scripts adapters/ # optional runtime payloads used by scripts repos/.md # optional repo-specific overlay - knowledge/ # optional shared domain reference + knowledge/ # optional shared domain reference, installed beside each domain skill tools/ install # core writer (mms- prefix, multi-operator output) sync # Flow 2: `yarn skills` wrapper for engineers diff --git a/domains/perps/skills/fix-perps-bug/skill.md b/domains/perps/skills/fix-perps-bug/skill.md index e37ae018..ab5ce99f 100644 --- a/domains/perps/skills/fix-perps-bug/skill.md +++ b/domains/perps/skills/fix-perps-bug/skill.md @@ -22,7 +22,7 @@ maturity: stable - Check if the bug involves a duplicated utility (see shared-package-analysis) -- if so, check both codebases 3. **Check formatting.** If the bug involves number display: - - Read formatting-rules knowledge + - Read installed `knowledge/formatting-rules.md` - Fix must follow the sig-dig rules, not hardcode decimals 4. **Check stream hooks.** If the bug involves stale/missing data: diff --git a/domains/perps/skills/review-perps-pr/skill.md b/domains/perps/skills/review-perps-pr/skill.md index ddd0a811..cf0c4b06 100644 --- a/domains/perps/skills/review-perps-pr/skill.md +++ b/domains/perps/skills/review-perps-pr/skill.md @@ -56,7 +56,7 @@ loop, run each CLI as a live tmux pane instead of one-shot, and `/clear` between ## Perps standard (reviewers must load and enforce as blockers) -From the perps `knowledge/` dir: **review-antipatterns** (core checklist), architecture, +From the installed `knowledge/` dir: **review-antipatterns** (core checklist), architecture, connection-architecture, caching-architecture, formatting-rules, mobile-extension-map, shared-package-analysis, feature-flags, screens. Check both repos when a shared util/screen changes. For page/view test changes, also enforce the component-view test guidance: broad @@ -67,7 +67,7 @@ test is explicitly justified. ``` Fresh full review of perps changes in at vs . No prior context. -Load perps knowledge (review-antipatterns + the rest). You gate this before any human sees it. +Load installed perps knowledge (`knowledge/`, review-antipatterns + the rest). You gate this before any human sees it. Every perps anti-pattern AND every nit (naming, magic number, missing testID, weak test, component-view behavior left as broad unit tests, .toFixed, fallback-display vs 0) = BLOCKER. APPROVE only if nothing is left to improve. diff --git a/domains/perps/skills/validate-perps-multiproject/skill.md b/domains/perps/skills/validate-perps-multiproject/skill.md index d84d55cc..fe9e869a 100644 --- a/domains/perps/skills/validate-perps-multiproject/skill.md +++ b/domains/perps/skills/validate-perps-multiproject/skill.md @@ -113,7 +113,7 @@ git status --short --branch For Mobile <-> Extension checks: -1. Load relevant perps knowledge: `mobile-extension-map`, `screens`, `shared-package-analysis`, architecture docs as needed. +1. Load relevant installed perps knowledge from `knowledge/`: `mobile-extension-map`, `screens`, `shared-package-analysis`, architecture docs as needed. 2. Find the equivalent screen/hook/flow in the other client. 3. Validate with real flow evidence; do not inject UI state. 4. Report semantic differences separately from regressions. diff --git a/tools/install b/tools/install index 22bb83b6..fcd91dc1 100755 --- a/tools/install +++ b/tools/install @@ -360,17 +360,46 @@ copy_bundle_dirs() { done } +copy_domain_knowledge() { + local skill_dir="$1" dest_dir="$2" label="$3" + local domain_dir; domain_dir=$(cd "$skill_dir/../.." && pwd) + local knowledge_dir="$domain_dir/knowledge" + + if [[ -d "$knowledge_dir" ]]; then + action "$label/knowledge/" + $DRY_RUN && return + mkdir -p "$dest_dir" + rm -rf "$dest_dir/knowledge" + cp -R "$knowledge_dir" "$dest_dir/knowledge" + else + if $DRY_RUN; then + [[ -e "$dest_dir/knowledge" ]] && action "$label/knowledge/ (remove stale)" + return + fi + rm -rf "$dest_dir/knowledge" + fi +} + copy_project_bundles() { local skill_dir="$1" out_name="$2" copy_bundle_dirs "$skill_dir" "$CLAUDE_DIR/$out_name" ".claude/skills/$out_name" + copy_domain_knowledge "$skill_dir" "$CLAUDE_DIR/$out_name" ".claude/skills/$out_name" copy_bundle_dirs "$skill_dir" "$CURSOR_DIR/$out_name" ".cursor/rules/$out_name" + copy_domain_knowledge "$skill_dir" "$CURSOR_DIR/$out_name" ".cursor/rules/$out_name" copy_bundle_dirs "$skill_dir" "$AGENTS_DIR/$out_name" ".agents/skills/$out_name" + copy_domain_knowledge "$skill_dir" "$AGENTS_DIR/$out_name" ".agents/skills/$out_name" } copy_user_bundles() { local skill_dir="$1" out_name="$2" - [[ -d "$HOME/.claude" ]] && copy_bundle_dirs "$skill_dir" "$USER_CLAUDE_DIR/$out_name" "~/.claude/skills/$out_name" - [[ -d "$HOME/.codex" ]] && copy_bundle_dirs "$skill_dir" "$USER_CODEX_DIR/$out_name" "~/.codex/skills/$out_name" + if [[ -d "$HOME/.claude" ]]; then + copy_bundle_dirs "$skill_dir" "$USER_CLAUDE_DIR/$out_name" "~/.claude/skills/$out_name" + copy_domain_knowledge "$skill_dir" "$USER_CLAUDE_DIR/$out_name" "~/.claude/skills/$out_name" + fi + if [[ -d "$HOME/.codex" ]]; then + copy_bundle_dirs "$skill_dir" "$USER_CODEX_DIR/$out_name" "~/.codex/skills/$out_name" + copy_domain_knowledge "$skill_dir" "$USER_CODEX_DIR/$out_name" "~/.codex/skills/$out_name" + fi } process_skill() { From 88670483ca7b106802b4970c1f20907898d49bf1 Mon Sep 17 00:00:00 2001 From: Arthur Breton Date: Wed, 10 Jun 2026 23:27:20 +0800 Subject: [PATCH 3/4] docs(validate-perps): enhance multiproject validation skill with detailed usage instructions and error handling guidelines --- .../validate-perps-multiproject/skill.md | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/domains/perps/skills/validate-perps-multiproject/skill.md b/domains/perps/skills/validate-perps-multiproject/skill.md index fe9e869a..68d83c40 100644 --- a/domains/perps/skills/validate-perps-multiproject/skill.md +++ b/domains/perps/skills/validate-perps-multiproject/skill.md @@ -73,37 +73,57 @@ Rules: ## Core -> clients via yalc +Use the repo toolchain and capture it in the evidence. Do not assume a Node manager. If your local checkout uses asdf/nvm/volta, activate it explicitly before running build commands. + ```bash +# Optional examples only; use the manager configured for this checkout. +# asdf users: export PATH="$HOME/.asdf/shims:$HOME/.asdf/bin:$PATH" +# nvm users: nvm use +# volta users usually need no shell change. +YALC_BIN="${YALC_BIN:-yalc}" + # Pre-state for repo in /path/to/core /path/to/mobile /path/to/extension; do echo "--- $repo" git -C "$repo" status --short --branch - (cd "$repo" && yarn --version) + (cd "$repo" && printf 'node=%s yarn=%s\n' "$(node -v)" "$(yarn -v)") done -# Build package; stop if this fails +# Build the exact package. This is the freshness gate for yalc. cd /path/to/core yarn workspace @metamask/perps-controller build -# Publish package -export P="/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:$HOME/.yarn/bin:$HOME/.npm-global/bin:$HOME/.local/bin:$HOME/.asdf/shims" +# Publish only after the package build succeeds. cd /path/to/core/packages/perps-controller -PATH="$P" yalc publish --private +"$YALC_BIN" publish --private -# Install in each client; Yarn 4 uses singular skip-build +# Install in each client; Yarn 4 uses singular skip-build. cd /path/to/client -PATH="$P" yalc add @metamask/perps-controller +"$YALC_BIN" add @metamask/perps-controller yarn install --mode=skip-build grep -n "@metamask/perps-controller" package.json yalc.lock yarn.lock ls -la .yalc/@metamask/perps-controller/dist ``` +Yalc path handling: + +- Prefer plain `yalc` from the developer shell. +- If `yalc` resolves to a broken version-manager shim, set `YALC_BIN=/opt/homebrew/bin/yalc`, `YALC_BIN=/usr/local/bin/yalc`, or another explicit binary path for that machine. + +Build failure handling: + +- If the package build fails, do **not** use an older `~/.yalc` package and do **not** hand-edit `dist/`. Report that client validation is blocked before transport. +- If errors mention `TS6305`, `unknown`, `never`, or a Core workspace dependency cycle, the package graph is stale or cyclic. Do not run `yarn workspaces foreach --from @metamask/perps-controller -R ...`; it can fail on the account-tree/multichain/perps/snap cycle and still leave `dist/` deleted. +- If a full Core rebuild is acceptable, ask first, then run it gently: `nice -n 10 yarn build`. Otherwise stop at the build blocker with the log path. +- Treat `yalc publish` success as meaningful only when the current run produced fresh `packages/perps-controller/dist` first. +- If the user still needs a client runtime smoke despite a Core build blocker, use an explicit **runtime-only yalc overlay**: restore the target client's installed package `dist` as a baseline, transpile only changed package source files into that `dist`, publish with yalc, and label the proof `runtime-only overlay, not publish-quality build`. Do not present this as a successful Core package build. + Run the selected proof in each client, then cleanup: ```bash cd /path/to/client -PATH="$P" yalc remove @metamask/perps-controller || true +"$YALC_BIN" remove @metamask/perps-controller || true git checkout -- package.json yarn.lock 2>/dev/null || true rm -rf .yalc yalc.lock git status --short --branch From 3f52dd8b220a02d784c56fa7c92c594d39c42675 Mon Sep 17 00:00:00 2001 From: Arthur Breton Date: Fri, 12 Jun 2026 11:54:37 +0800 Subject: [PATCH 4/4] feat(validate-perps): introduce perps validation script for multiproject checks - Added `scripts/perps-validate.sh` to automate the validation process for Core changes in client checkouts (Mobile and Extension). - Enhanced documentation in `skill.md` with detailed usage instructions and error handling guidelines. - Emphasized the importance of pre-state awareness and yalc resolution in the validation workflow. --- .../scripts/perps-validate.sh | 262 ++++++++++++++++++ .../validate-perps-multiproject/skill.md | 190 +++++++++---- 2 files changed, 402 insertions(+), 50 deletions(-) create mode 100755 domains/perps/skills/validate-perps-multiproject/scripts/perps-validate.sh diff --git a/domains/perps/skills/validate-perps-multiproject/scripts/perps-validate.sh b/domains/perps/skills/validate-perps-multiproject/scripts/perps-validate.sh new file mode 100755 index 00000000..ebc834a3 --- /dev/null +++ b/domains/perps/skills/validate-perps-multiproject/scripts/perps-validate.sh @@ -0,0 +1,262 @@ +#!/usr/bin/env bash +# +# perps-validate.sh — deterministic helper for validating a Core +# @metamask/perps-controller change inside a client checkout (Mobile or +# Extension) via yalc. +# +# Direction is always: a CLIENT validates a CORE controller change. +# owner = the Core checkout that holds the perps-controller change +# client = the Mobile/Extension checkout that consumes it +# +# Run subcommands in this order: +# 1. prestate [client-dir...] # snapshot before touching anything +# 2. build [--full] # build the package (freshness gate) +# 3. push [...] # yalc publish + push into clients +# 4. verify # confirm version + new symbols landed +# 5. restore [client-dir...] # put the client back to its snapshot +# +# Helper subcommands: +# resolve-yalc # print the resolved yalc invocation +# doctor # quick environment sanity check +# +# Design goals: +# - No assumption about the Node manager (asdf / nvm / volta / brew / none). +# - No hardcoded paths. Everything is derived or passed in. +# - Pre-state aware: if a client was ALREADY on a yalc link, restore brings +# that exact link back instead of nuking the user's dev setup. +# +# Env overrides: +# YALC_BIN explicit yalc invocation (e.g. "/opt/homebrew/bin/yalc" +# or "node /path/to/yalc/src/yalc.js"). Skips auto-resolution. +# PKG package name (default: @metamask/perps-controller) +# STATE_DIR where snapshots live (default: /tmp/.perps-validate) + +set -euo pipefail + +PKG="${PKG:-@metamask/perps-controller}" +# perps-controller lives at packages/ in the Core monorepo. +PKG_LEAF="${PKG##*/}" # perps-controller +PKG_SCOPE="${PKG%/*}" # @metamask + +# --------------------------------------------------------------------------- +# yalc resolution — the single most fragile thing across machines. +# +# A version-manager shim (notably asdf) can SUCCEED with exit 0 yet do nothing, +# printing "No version is set for command yalc". So we never trust exit code +# alone: a working yalc must print a semver to stdout. If the shim is broken we +# fall back to locating yalc's own yalc.js and running it through a real node. +# --------------------------------------------------------------------------- +_looks_like_version() { printf '%s' "$1" | grep -Eq '^[0-9]+\.[0-9]+'; } + +resolve_yalc() { + if [ -n "${YALC_BIN:-}" ]; then printf '%s' "$YALC_BIN"; return 0; fi + + # 1. plain yalc on PATH, but only if it actually reports a version. + # Use `command yalc` so we hit the real binary, never the run_yalc wrapper. + if command -v yalc >/dev/null 2>&1; then + local v; v="$(command yalc --version 2>/dev/null || true)" + if _looks_like_version "$v"; then printf 'yalc'; return 0; fi + fi + + # 2. a real node to run yalc.js with (any working node is fine). + local node_bin; node_bin="$(command -v node || true)" + [ -z "$node_bin" ] && { echo "ERROR: no node on PATH to run yalc" >&2; return 1; } + + # 3. hunt for yalc's entrypoint across the common install layouts. + local cand + for cand in \ + "$(npm root -g 2>/dev/null)/yalc/src/yalc.js" \ + "$(npm root -g 2>/dev/null)/yalc/yalc.js" \ + "$HOME"/.asdf/installs/nodejs/*/lib/node_modules/yalc/src/yalc.js \ + "$HOME"/.nvm/versions/node/*/lib/node_modules/yalc/src/yalc.js \ + "$HOME"/.volta/tools/image/packages/yalc/lib/node_modules/yalc/src/yalc.js \ + /opt/homebrew/lib/node_modules/yalc/src/yalc.js \ + /usr/local/lib/node_modules/yalc/src/yalc.js ; do + [ -f "$cand" ] && { printf '%s %s' "$node_bin" "$cand"; return 0; } + done + + echo "ERROR: could not resolve yalc. Install it (npm i -g yalc) or set YALC_BIN." >&2 + return 1 +} + +# Run yalc regardless of how it resolved (binary or "node yalc.js"). +# NOT named `yalc` on purpose — a function named `yalc` would shadow the real +# binary and make resolve_yalc recurse forever. +run_yalc() { local y; y="$(resolve_yalc)" || return 1; eval "$y \"\$@\""; } + +# --------------------------------------------------------------------------- +state_dir() { printf '%s/tmp/.perps-validate' "$1"; } + +# =========================================================================== +cmd_resolve_yalc() { + local y; y="$(resolve_yalc)" || exit 1 + echo "yalc => $y" + eval "$y --version" +} + +# --------------------------------------------------------------------------- +cmd_prestate() { + [ "$#" -ge 1 ] || { echo "usage: prestate [client-dir...]" >&2; exit 2; } + for client in "$@"; do + client="$(cd "$client" && pwd)" + local sd; sd="$(state_dir "$client")"; mkdir -p "$sd" + echo "=== prestate: $client ===" + git -C "$client" status --short --branch | tee "$sd/git-status.txt" >/dev/null + cp "$client/package.json" "$sd/package.json.bak" 2>/dev/null || true + cp "$client/yalc.lock" "$sd/yalc.lock.bak" 2>/dev/null || true + + local linkdir="$client/.yalc/$PKG" + if [ -d "$linkdir" ]; then + # Client was ALREADY on a yalc link — back it up byte-for-byte so restore + # reproduces the exact pre-existing dev setup, not a clean registry state. + echo "mode=PREEXISTING_YALC" > "$sd/mode" + tar -czf "$sd/yalc-pkg.tgz" -C "$client/.yalc/$PKG_SCOPE" "$PKG_LEAF" + cat "$linkdir/yalc.sig" 2>/dev/null > "$sd/yalc.sig" || true + echo " was already yalc-linked: version=$(node -p "require('$linkdir/package.json').version" 2>/dev/null) sig=$(cat "$sd/yalc.sig" 2>/dev/null)" + else + echo "mode=REGISTRY" > "$sd/mode" + echo " no prior yalc link (registry baseline)" + fi + echo " snapshot -> $sd" + done +} + +# --------------------------------------------------------------------------- +# Build the controller package. The package CANNOT build standalone in a fresh +# Core checkout: its referenced packages have no dist yet and tsc fails with +# TS6305. That is expected — the supported fix is a full monorepo build first. +cmd_build() { + local core="${1:?usage: build [--full]}"; shift || true + local full=0; [ "${1:-}" = "--full" ] && full=1 + core="$(cd "$core" && pwd)" + local log="$core/tmp/perps-build.log"; mkdir -p "$core/tmp" + + if [ "$full" -eq 1 ]; then + echo "=== full monorepo build (nice) — builds all referenced dists ===" + ( cd "$core" && nice -n 10 yarn build ) 2>&1 | tee "$log" + else + echo "=== package build: yarn workspace $PKG build ===" + if ! ( cd "$core" && yarn workspace "$PKG" build ) 2>&1 | tee "$log"; then + : + fi + if grep -q "TS6305" "$log"; then + echo "" + echo "BLOCKED: TS6305 — referenced package dists are missing (fresh checkout)." + echo "Do NOT use 'workspaces foreach -R' (cycle can delete dist)." + echo "Re-run with --full to build the whole monorepo first:" + echo " perps-validate.sh build $core --full" + exit 3 + fi + fi + + # Freshness gate: prove the built dist actually carries this change. + local dist="$core/packages/$PKG_LEAF/dist" + [ -f "$dist/index.cjs" ] || { echo "ERROR: no dist/index.cjs produced" >&2; exit 3; } + echo "" + echo "built version: $(node -p "require('$core/packages/$PKG_LEAF/package.json').version")" + echo "dist OK -> $dist" +} + +# --------------------------------------------------------------------------- +cmd_push() { + local core="${1:?usage: push [client-dir...]}"; shift + [ "$#" -ge 1 ] || { echo "need at least one client dir" >&2; exit 2; } + core="$(cd "$core" && pwd)" + local pkgdir="$core/packages/$PKG_LEAF" + + echo "=== yalc publish $PKG from $pkgdir ===" + ( cd "$pkgdir" && run_yalc publish --private ) + + for client in "$@"; do + client="$(cd "$client" && pwd)" + echo "=== push into client: $client ===" + if [ -d "$client/.yalc/$PKG" ] || grep -q "$PKG" "$client/yalc.lock" 2>/dev/null; then + ( cd "$client" && run_yalc update "$PKG" ) # advance an existing link + else + ( cd "$client" && run_yalc add "$PKG" && yarn install --mode=skip-build ) + fi + done +} + +# --------------------------------------------------------------------------- +cmd_verify() { + local client="${1:?usage: verify }" + client="$(cd "$client" && pwd)" + local inst="$client/node_modules/$PKG" + echo "=== verify $PKG in $client ===" + echo "installed version: $(node -p "require('$inst/package.json').version" 2>/dev/null || echo MISSING)" + echo "yalc link version: $(node -p "require('$client/.yalc/$PKG/package.json').version" 2>/dev/null || echo none)" + echo "yalc sig: $(cat "$client/.yalc/$PKG/yalc.sig" 2>/dev/null || echo none)" + echo "--- next: run the client's own proof (type-check + the affected tests) ---" +} + +# --------------------------------------------------------------------------- +# Restore is pre-state aware. PREEXISTING_YALC clients get their exact link +# back; REGISTRY clients are fully un-yalc'd. +cmd_restore() { + [ "$#" -ge 1 ] || { echo "usage: restore [client-dir...]" >&2; exit 2; } + for client in "$@"; do + client="$(cd "$client" && pwd)" + local sd; sd="$(state_dir "$client")" + local mode; mode="$(cat "$sd/mode" 2>/dev/null | cut -d= -f2 || echo UNKNOWN)" + echo "=== restore: $client (mode=$mode) ===" + case "$mode" in + PREEXISTING_YALC) + rm -rf "$client/.yalc/$PKG" + tar -xzf "$sd/yalc-pkg.tgz" -C "$client/.yalc/$PKG_SCOPE" + cp "$sd/yalc.lock.bak" "$client/yalc.lock" 2>/dev/null || true + cp "$sd/package.json.bak" "$client/package.json" 2>/dev/null || true + echo " restored prior link: sig now=$(cat "$client/.yalc/$PKG/yalc.sig" 2>/dev/null) expected=$(cat "$sd/yalc.sig" 2>/dev/null)" + echo " run 'yarn install --mode=skip-build' if node_modules needs to match" + ;; + REGISTRY) + ( cd "$client" && run_yalc remove "$PKG" || true ) + git -C "$client" checkout -- package.json yarn.lock 2>/dev/null || true + rm -rf "$client/.yalc" "$client/yalc.lock" + echo " un-yalc'd; package.json/yarn.lock reverted" + ;; + *) + echo " no snapshot found ($sd) — nothing to restore. Run prestate next time." ;; + esac + git -C "$client" status --short --branch | sed 's/^/ /' + done +} + +# --------------------------------------------------------------------------- +cmd_doctor() { + local core="${1:?usage: doctor }"; local client="${2:?need client dir}" + echo "=== doctor ===" + echo "node: $(command -v node) $(node -v 2>/dev/null)" + echo -n "yalc: "; cmd_resolve_yalc || true + echo "core: $core ($(git -C "$core" rev-parse --abbrev-ref HEAD 2>/dev/null))" + echo "client: $client ($(git -C "$client" rev-parse --abbrev-ref HEAD 2>/dev/null))" + echo "package: $PKG ($core/packages/$PKG_LEAF $( [ -d "$core/packages/$PKG_LEAF" ] && echo found || echo MISSING ))" + echo "client links pkg already: $( [ -d "$client/.yalc/$PKG" ] && echo yes || echo no )" +} + +# =========================================================================== +sub="${1:-}"; shift || true +case "$sub" in + resolve-yalc) cmd_resolve_yalc "$@" ;; + prestate) cmd_prestate "$@" ;; + build) cmd_build "$@" ;; + push) cmd_push "$@" ;; + verify) cmd_verify "$@" ;; + restore) cmd_restore "$@" ;; + doctor) cmd_doctor "$@" ;; + *) cat >&2 < [client-dir...] + perps-validate.sh build [--full] + perps-validate.sh push [client-dir...] + perps-validate.sh verify + perps-validate.sh restore [client-dir...] + perps-validate.sh resolve-yalc + perps-validate.sh doctor + +Env: YALC_BIN, PKG (default @metamask/perps-controller), STATE_DIR +EOF + exit 2 ;; +esac diff --git a/domains/perps/skills/validate-perps-multiproject/skill.md b/domains/perps/skills/validate-perps-multiproject/skill.md index 68d83c40..cea2ebcd 100644 --- a/domains/perps/skills/validate-perps-multiproject/skill.md +++ b/domains/perps/skills/validate-perps-multiproject/skill.md @@ -8,6 +8,32 @@ maturity: stable Validate one perps change across multiple local MetaMask repo checkouts. +**Canonical direction: a CLIENT validates a CORE controller change.** The +`@metamask/perps-controller` package lives in Core; Mobile and Extension consume +it. The owner is wherever the controller change is; the targets are the clients +that must not break. (Client↔client parity is a secondary mode — see below.) + +## Deterministic layer — use `scripts/perps-validate.sh` + +Do **not** re-derive the per-folder commands each run. `scripts/perps-validate.sh` +encodes them and is portable across machines and Node managers. Run it in order: + +```bash +SC="$(dirname "$0")/scripts/perps-validate.sh" # or the skill's scripts/ path + +perps-validate.sh doctor # sanity: branches, yalc, links +perps-validate.sh prestate [client-dir...] # snapshot BEFORE touching anything +perps-validate.sh build [--full] # build pkg; freshness gate +perps-validate.sh push [...] # yalc publish + push into clients +perps-validate.sh verify # version + new symbols landed? +# -> then run the client's own proof (type-check + the affected tests) +perps-validate.sh restore [client-dir...] # pre-state-aware restore +``` + +The script makes no assumption about the Node manager and resolves `yalc` +robustly (see "yalc resolution" below). Everything else in this doc explains the +*why* so you can adapt when a step deviates. + ## Defaults - **Owner checkout**: current repo/cwd unless a path is provided. @@ -17,9 +43,13 @@ Validate one perps change across multiple local MetaMask repo checkouts. - Mobile owner -> validate parity in Extension; - Extension owner -> validate parity in Mobile. - **Transport**: `yalc` for `@metamask/perps-controller`; none for client parity. -- **Proof**: smallest meaningful proof: build/package smoke first; recipe/e2e or real UI flow when behavior changes. +- **Proof**: smallest meaningful proof. For a client validating a Core change, + the high-value proof is the **client type-check** (catches removed/renamed/ + changed exports) plus **the client tests that exercise the changed surface**. + Add recipe/e2e or real UI flow only when runtime behavior changes. - **Target edits**: forbidden unless explicitly allowed. -- **Cleanup**: required; restore validation checkouts to pre-state. +- **Cleanup**: required, and **pre-state aware** — a client that was already on a + yalc link must be restored to that exact link, not wiped to a registry baseline. ## Step 0 — resolve folders or ask @@ -40,12 +70,16 @@ find "$ROOT" -maxdepth 1 -type d \ \( -name 'core*' -o -name 'controller*' -o -name 'metamask-mobile*' -o -name 'mobile*' -o -name 'metamask-extension*' -o -name 'extension*' \) -print 2>/dev/null ``` +Note: a workspace may hold many numbered clones (`core-1..core-6`, +`metamask-mobile-1..6`). Confirm WHICH core holds the change and WHICH client to +validate — do not assume the first match. + If needed, ask one concise question using the runtime's interactive question tool when available: ```text I need the validation folders: -- Owner checkout: -- Validation target(s): +- Owner checkout (Core with the change): +- Validation target client(s): - Proof level: transport-only | type/import | recipe/e2e | real UI flow - May validation targets be edited? default no ``` @@ -60,75 +94,128 @@ Echo the resolved contract before changing anything. Owner: `` on `` Targets: - `` — — purpose: -- `` — — purpose: Rules: -1. Capture `git status --short --branch` in every checkout first. +1. Capture `git status --short --branch` in every checkout first (`prestate`). 2. Edit only the owner checkout unless a target is explicitly editable. 3. For Core package validation, build first, then publish via yalc; never publish stale `dist/`. -4. Run the smallest proof that reaches the changed perps behavior. -5. Label package transport-only checks as `transport-only`; do not call them E2E. -6. Cleanup yalc/package-manager changes and prove final target status matches pre-state. +4. Prove the built `dist/` actually carries the change (version bump + new symbols) before publishing. +5. Run the smallest proof that reaches the changed perps behavior. +6. Restore each client to its snapshot; a pre-existing yalc link must come back byte-for-byte. ``` ## Core -> clients via yalc -Use the repo toolchain and capture it in the evidence. Do not assume a Node manager. If your local checkout uses asdf/nvm/volta, activate it explicitly before running build commands. +`scripts/perps-validate.sh` runs all of this. The manual equivalents below are +for when you must deviate. + +### yalc resolution (do not assume asdf/nvm/brew) + +`yalc` is the single most fragile dependency across machines. A version-manager +**shim can exit 0 yet do nothing** — e.g. asdf prints `No version is set for +command yalc` and returns success, so `yalc publish` silently no-ops. Never trust +the exit code alone: a working `yalc` prints a semver to stdout. + +Resolution order (what the script does): +1. `YALC_BIN` env override, if set. +2. `command yalc --version` — accept only if it prints a semver. +3. Otherwise locate yalc's own `yalc.js` and run it through any working `node`. + Searched layouts: `$(npm root -g)/yalc`, `~/.asdf/installs/nodejs/*/lib/...`, + `~/.nvm/versions/node/*/lib/...`, `~/.volta/...`, `/opt/homebrew/lib/...`, + `/usr/local/lib/...`. + +If all else fails: `npm i -g yalc`, or pass an explicit +`YALC_BIN="node /abs/path/to/yalc/src/yalc.js"`. ```bash -# Optional examples only; use the manager configured for this checkout. -# asdf users: export PATH="$HOME/.asdf/shims:$HOME/.asdf/bin:$PATH" -# nvm users: nvm use -# volta users usually need no shell change. -YALC_BIN="${YALC_BIN:-yalc}" - -# Pre-state -for repo in /path/to/core /path/to/mobile /path/to/extension; do - echo "--- $repo" - git -C "$repo" status --short --branch +# Pre-state for every checkout (the script's `prestate` does this + a byte-for-byte +# backup of any existing .yalc link). +for repo in /path/to/core /path/to/client; do + echo "--- $repo"; git -C "$repo" status --short --branch (cd "$repo" && printf 'node=%s yarn=%s\n' "$(node -v)" "$(yarn -v)") done +``` -# Build the exact package. This is the freshness gate for yalc. +### Building the package — expect a full monorepo build + +The perps-controller package **cannot build standalone in a fresh Core checkout.** +Its `tsconfig.build.json` uses project references, so `yarn workspace +@metamask/perps-controller build` fails with `TS6305: Output file ... has not been +built from source file` for every dependency package whose `dist/` is missing +(network-controller, transaction-controller, controller-utils, +profile-sync-controller, remote-feature-flag-controller, messenger, …). This is +the normal state of a fresh checkout, **not** a perps-controller bug. + +The supported fix is a full monorepo build first: + +```bash cd /path/to/core -yarn workspace @metamask/perps-controller build +nice -n 10 yarn build # builds all referenced dists in dependency order +``` -# Publish only after the package build succeeds. -cd /path/to/core/packages/perps-controller -"$YALC_BIN" publish --private +This takes several minutes (40+ packages) but is clean and leaves the checkout +buildable for later runs. Do **not** use `yarn workspaces foreach --from +@metamask/perps-controller -R ...` — it can fail on the +account-tree/multichain/perps/snap cycle and delete `dist/` on the way out. +`perps-validate.sh build --full` wraps `nice -n 10 yarn build`; the +plain `build` form detects TS6305 and tells you to re-run with `--full`. -# Install in each client; Yarn 4 uses singular skip-build. -cd /path/to/client -"$YALC_BIN" add @metamask/perps-controller -yarn install --mode=skip-build +### Freshness gate + +`yalc publish` success is meaningful only when the **current** run produced a +fresh `packages/perps-controller/dist`, and that dist carries the change. Verify +before trusting it: -grep -n "@metamask/perps-controller" package.json yalc.lock yarn.lock -ls -la .yalc/@metamask/perps-controller/dist +```bash +D=/path/to/core/packages/perps-controller +node -p "require('$D/package.json').version" # expected bump +ls "$D"/dist/index.cjs # dist exists +grep -l "" "$D"/dist/index.d.cts # new surface present ``` -Yalc path handling: +### Publish + push (advancing an existing link) -- Prefer plain `yalc` from the developer shell. -- If `yalc` resolves to a broken version-manager shim, set `YALC_BIN=/opt/homebrew/bin/yalc`, `YALC_BIN=/usr/local/bin/yalc`, or another explicit binary path for that machine. +A client may **already** be on a yalc link (e.g. mid-feature dev). In that case +`yalc add` is wrong — use `yalc update` to advance the existing link in place. +`perps-validate.sh push` picks the right one automatically. -Build failure handling: +```bash +cd /path/to/core/packages/perps-controller && yalc publish --private --push +# `--push` propagates to every linked project. WATCH THE OUTPUT: it will touch +# *other* repos that link the same package (e.g. a sibling clone), not just your +# target. That is a side effect to note, not a failure. +``` -- If the package build fails, do **not** use an older `~/.yalc` package and do **not** hand-edit `dist/`. Report that client validation is blocked before transport. -- If errors mention `TS6305`, `unknown`, `never`, or a Core workspace dependency cycle, the package graph is stale or cyclic. Do not run `yarn workspaces foreach --from @metamask/perps-controller -R ...`; it can fail on the account-tree/multichain/perps/snap cycle and still leave `dist/` deleted. -- If a full Core rebuild is acceptable, ask first, then run it gently: `nice -n 10 yarn build`. Otherwise stop at the build blocker with the log path. -- Treat `yalc publish` success as meaningful only when the current run produced fresh `packages/perps-controller/dist` first. -- If the user still needs a client runtime smoke despite a Core build blocker, use an explicit **runtime-only yalc overlay**: restore the target client's installed package `dist` as a baseline, transpile only changed package source files into that `dist`, publish with yalc, and label the proof `runtime-only overlay, not publish-quality build`. Do not present this as a successful Core package build. +### Run the proof, then restore -Run the selected proof in each client, then cleanup: +Client proof for "controller won't break": ```bash cd /path/to/client -"$YALC_BIN" remove @metamask/perps-controller || true -git checkout -- package.json yarn.lock 2>/dev/null || true -rm -rf .yalc yalc.lock -git status --short --branch +# 1. type-check — the strongest signal for export/type regressions. +NODE_OPTIONS='--max-old-space-size=8192' npx tsc --noEmit --incremental \ + --tsBuildInfoFile .tsbuildinfo --project ./tsconfig.json +# 2. the client tests that exercise the changed surface (NOT the whole suite, +# NOT --findRelatedTests). +yarn jest --no-coverage ``` +### Cleanup — pre-state aware (critical) + +The old "rm -rf .yalc yalc.lock && git checkout package.json yarn.lock" recipe is +**destructive** when the client was already on a yalc link before you started: it +wipes the user's active dev setup. Restore to the snapshot taken by `prestate`: + +- **Client was already yalc-linked** → restore the backed-up `.yalc/` + byte-for-byte, restore `yalc.lock`/`package.json`, re-run + `yarn install --mode=skip-build` if node_modules must match. +- **Client had no prior link (registry baseline)** → `yalc remove `, + `git checkout -- package.json yarn.lock`, `rm -rf .yalc yalc.lock`. + +`perps-validate.sh restore ` does the right one based on the recorded +mode. If you advanced a client deliberately and the user wants to keep the new +version, say so explicitly and keep the snapshot for a later restore. + ## Client parity For Mobile <-> Extension checks: @@ -140,17 +227,20 @@ For Mobile <-> Extension checks: ## Stop conditions -Stop and report when the owner package cannot build, target checkout has unexpected dirt, cleanup would delete user work, target source edits are needed but not allowed, or required device/browser/credential context is missing. +Stop and report when the owner package cannot build (after `--full`), a target +checkout has unexpected dirt, cleanup would delete user work, target source edits +are needed but not allowed, or required device/browser/credential context is +missing. ## Final answer ```md Validated against . - Owner: @, status -- Targets: , -- Transport: -- Proof: => +- Targets: +- Transport: (built via full monorepo build / package build) +- Proof: => - Artifacts: -- Cleanup: +- Cleanup: - Follow-ups/blockers: ```