feat(perps): distribute generated review checklists

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Add `performance/profiling-regression-proposal`: an evidence-only skill that proposes follow-up actions after MetaMask Mobile CI has already classified a Hermes CPU-profile regression.
 - Support explicit-only workflow skills through native invocation controls, preserve repository overlays, and prune managed retired skill names during sync.
+- Distribute the Perps static review as generated per-client checklists with source-tracked criteria, repository overlays, and optional analyzer context export.
 - Add `navigation` skill with a repo-agnostic base and a MetaMask Mobile overlay for `Routes` and `NavigationService`. Marked `base: true` so it installs even when its domain is filtered out.
 - Add `feature-flags` skill with a repo-agnostic base and a MetaMask Mobile overlay for version-gated remote flags. Marked `base: true` so it installs even when its domain is filtered out. ([#147](https://github.com/MetaMask/skills/pull/147))
 - Add `analytics` skill (`platform/analytics`, moved from `coding`) with a repo-agnostic base and a MetaMask Mobile overlay for the canonical tracking API. Marked `base: true` so it installs even when its domain is filtered out. ([#140](https://github.com/MetaMask/skills/pull/140))

domains/perps/skills/perps-review-pr/references/criteria/core/ensureunifiedaccountenabled-retry-vs-permanent-failure-cache-semantics.md

@@ -0,0 +1,17 @@
+# `#ensureUnifiedAccountEnabled` — Retry vs Permanent-Failure Cache Semantics
+
+An attempted unified-account setup that fails either sets the retry flag (`#unifiedAccountSetupNeedsRetry`, transient) or caches `{ attempted: true, enabled: false }` in `TradingReadinessCache` (permanent), never both; the deferred-signing, feature-disabled, and unknown-mode paths intentionally return without touching either.
+
+- **Permanent account-shape condition cached as retryable** — if a condition that can never
+  resolve (e.g. the account is already a confirmed multi-sig) sets the retry flag instead
+  of caching `{ attempted: true, enabled: false }`, the client re-runs the failing path on
+  every Perps tab entry indefinitely. This is the root cause of the recurring Perps-tab
+  error: set the retry flag only for transient failures that a subsequent attempt might
+  recover from; cache permanent failures as final without setting the retry flag.
+- **Retryable flag + permanent condition** — verify that each attempted-setup path through
+  `#ensureUnifiedAccountEnabled` that returns without enabling the account either (a) sets
+  the retry flag and returns without caching, or (b) caches `{ attempted: true, enabled:
+  false }` and does *not* set the retry flag. Both flags active on the same path is a loop.
+- **Deferred or skipped path treated as a failure** — the defer-until-action, feature-disabled,
+  and unknown-mode returns leave the cache untouched on purpose so the next entry re-evaluates;
+  caching them as attempted suppresses the migration when the user later trades.

domains/perps/skills/perps-review-pr/references/criteria/core/evidence-expected-before-core-perps-review.md

@@ -0,0 +1,3 @@
+# Evidence Expected Before Core Perps Review
+
+The PR carries a contract impact matrix (state, methods, events, exports, constants), a Mobile/Extension compatibility note or paired PR links, provider abstraction and fallback tests, and grep evidence that no client import or environment global entered the package.

domains/perps/skills/perps-review-pr/references/criteria/core/hyperliquid-multi-sig-account-handling-in-hyperliquidprovider.md

@@ -0,0 +1,16 @@
+# HyperLiquid Multi-Sig Account Handling in HyperLiquidProvider
+
+Every user-scoped exchange write in `HyperLiquidProvider` needs both a proactive info probe placed right before the write and a message classifier in its `catch`; HyperLiquid rejects every single-signer write for a multi-sig account, and neither guard alone is sufficient.
+
+- **Catch-path classifier without proactive probe** — burns a doomed write on every entry
+  for a multi-sig account. The error is caught, but the round-trip and any side-effects
+  (recording premature state, logging) have already occurred.
+- **Proactive probe without catch-path classifier** — can race the multi-sig conversion
+  window and fails open: the probe returns normal, the write fires during the transition,
+  and the error is unhandled.
+- **Probe placed too early** — placing the probe immediately after `userAbstraction` (rather
+  than immediately before the write) means already-unified multi-sig accounts are probed on
+  every call, and the probe result can cause the account to be recorded as `enabled: false`
+  before the unified path has had a chance to short-circuit. The correct placement is
+  **after** the already-compatible short-circuit, the defer branch, and the unknown-mode
+  bail — right before the write that would otherwise fail.

domains/perps/skills/perps-review-pr/references/criteria/core/package-release-metadata-must-match-contract-impact.md

@@ -0,0 +1,7 @@
+# Package Release Metadata Must Match Contract Impact
+
+The changelog entry and the semver bump match the contract impact, and a bump PR names the package version and the consumers it was checked against.
+
+- **Public API/state change without changelog** — clients need migration context.
+- **Breaking change released as minor/patch** — semver must match impact.
+- **Controller package and client integration out of sync** — sync/bump PRs should state package version and consumer compatibility.

domains/perps/skills/perps-review-pr/references/criteria/core/public-controller-contracts-must-be-versioned-and-migration-aware.md

@@ -0,0 +1,8 @@
+# Public Controller Contracts Must Be Versioned and Migration-Aware
+
+A change to controller state shape, method signatures, event names or payloads, or package exports ships with a client migration plan and package-level consumer-style tests.
+
+- **State shape changes without client migration** — Mobile/Extension selectors and hooks may break.
+- **Method signature changes without compatibility plan** — exported controller methods need backward compatibility or coordinated client changes.
+- **Event name/payload drift** — clients and metrics rely on stable events.
+- **Package export changes without package-level tests** — changing exports must include consumer-style assertions.

domains/perps/skills/perps-review-pr/references/criteria/extension/analytics-wiring-patterns.md

@@ -0,0 +1,8 @@
+# Analytics Wiring Patterns
+
+Screen views are emitted once, from the screen or modal that renders them, and attribution for controller-owned events is merged in `createPerpsInfrastructure`, not in UI code.
+
+- **Screen-view double-emission on normal+error page pairs** — any page that renders both a normal screen view and an error screen view must gate the normal view on the subject existing (`Boolean(market)`) and give the error view a `resetKey`. Without this, one rendered error screen emits two events, and consecutive bad symbols each emit one instead of resetting cleanly.
+- **Modal screen view at trigger site instead of in the modal** — screen views for a modal belong in the modal itself, not at its trigger sites. A modal with many triggers (e.g. a geo-block notice with 17 triggers across 11 hosts) needs one declarative `usePerpsEventTracking({conditions: isOpen})` in the modal, not 17 scattered call sites.
+- **Removing client `track()` calls without checking background API** — when migrating analytics from client to controller, verify the matching background API actually accepts `trackingData`. Some APIs (e.g. `UpdateMarginParams`) do not — no `trackingData` field is needed for those.
+- **Attribution split** — UI `trackingData` carries entry/discovery/hlFeeRate; stored UTM context must be merged in `createPerpsInfrastructure` via `mergeAttributionContext` for controller-emitted lifecycle events. Do not merge attribution in UI code for controller-owned events.

domains/perps/skills/perps-review-pr/references/criteria/extension/backend-routing-and-controller-preload-caches.md

@@ -0,0 +1,7 @@
+# Backend Routing and Controller Preload Caches
+
+A backend route or provider endpoint change updates the preload and cache-prime paths (`cachedMarketDataByProvider`, `PerpsStreamBridge`'s `startMarketDataPreload`) and the reconnect fallback, not only the explicit UI fetch, or warm restarts keep serving the stale route.
+
+- **Preload cache not updated alongside explicit fetch path** — whenever a backend route or provider endpoint changes, grep for all preload and cache-prime call sites (`cachedMarketDataByProvider`, `PerpsStreamBridge`'s `startMarketDataPreload`) and verify they resolve through the same updated path.
+- **Reconnect fallback bypasses cache invalidation** — reconnect handlers that re-init from cache without invalidating first will restore the old route after a network interruption.
+- **Cache TTL assumes a route that no longer exists** — if the TTL or stale-while-revalidate window is longer than the rollout window for a backend routing change, the cache will serve the old route to users who reconnected within that window.

domains/perps/skills/perps-review-pr/references/criteria/extension/batch-action-and-analytics-error-path-parity.md

@@ -0,0 +1,7 @@
+# Batch-Action and Analytics Error-Path Parity
+
+Sibling batch-action handlers (`handleCloseAllPositions`, `handleCancelAllOrders`) share one error contract: the same catch and soft-failure analytics in every sibling, each new branch covered by a test.
+
+- **Asymmetric catch blocks across sibling handlers** — if one batch-action handler emits `PerpsError` + `trackPerpsErrorScreenViewed` on transport throw, every parallel handler in the same component must do the same. Diff all `catch` branches in the file before declaring analytics parity.
+- **Soft-failure branch without error-screen-view** — a `{ success: false }` (or equivalent `result?.success` check) branch that fires `batchActionError` but not an error-screen-view event is incomplete. Before signing off on error analytics, grep sibling components that own the same `{ success: boolean }` shape and verify their soft-failure branches are symmetric (`git grep -l 'success.*boolean\|{ success:' -- '*.tsx' '*.ts'`).
+- **New analytics branch ships without a test** — every new `catch` block or `if (!result?.success)` branch that emits an analytics event must have a corresponding unit test covering that branch. Missing coverage surfaces as a hard gate failure later; add the test in the same commit as the analytics change.

domains/perps/skills/perps-review-pr/references/criteria/extension/cdp-e2e-proof-surfaces.md

@@ -0,0 +1,5 @@
+# CDP / E2E Proof Surfaces
+
+A Perps tab screenshot proves market data only when a non-zero price or position value is visible or a CDP state assertion confirms live data; a navigated route over a loading skeleton is not proof.
+
+- **Perps tab screenshot treated as market-data-loaded proof** — a screenshot of the Perps tab can show a navigated route (e.g., `/perps/market/BTC`) while the page body is still a loading skeleton. Route navigation is *not* evidence that prices, positions, or market data have loaded. Before citing a screenshot as market-data proof, confirm a non-zero price or position value is visible in the image, or pair it with a CDP state assertion that confirms live data is present.

domains/perps/skills/perps-review-pr/references/criteria/extension/charts-and-ctas-need-feature-parity-evidence.md

@@ -0,0 +1,10 @@
+# Charts and CTAs Need Feature-Parity Evidence
+
+- **Loading and loaded section order differ**: Reserve space for every conditional section above stable controls, including a populated watchlist. Compare loading and loaded layouts with that data present.
+- **Measurements ignore rendered state**: Invalidate cached widths when selection, icons or labels change. Keep measurement out of unused layouts and live-data render loops; prove the clear/overflow control stays reachable at the smallest supported width.
+
+A chart or CTA change keeps the old chart context, gates the CTA by capability, and ships event coverage or an explicit deferral.
+
+- **Advanced chart drops volume or realtime signal** — preserve old chart context unless intentionally removed.
+- **CTA shown for unsupported asset/context** — gate by capability, not generic asset presence.
+- **New CTA lacks analytics** — action buttons need event coverage or explicit deferral.

domains/perps/skills/perps-review-pr/references/criteria/extension/controller-mock-must-be-kept-current.md

@@ -0,0 +1,6 @@
+# Controller Mock Must Be Kept Current
+
+Every new contract value that product code reads is added to the hand-maintained `test/mocks/metamask-perps-controller.js` in the same PR, otherwise tests silently see `undefined`.
+
+- **New event name / property used in product code but absent from mock** — grep the new symbol in `test/mocks/metamask-perps-controller.js` before merging. If missing, add it.
+- **Mock drift goes unnoticed** — tests do not warn when a mock returns `undefined`; they silently fail on downstream assertions. Do not assume the mock is up to date after a controller version bump.

domains/perps/skills/perps-review-pr/references/criteria/extension/evidence-expected-before-extension-perps-review.md

@@ -0,0 +1,3 @@
+# Evidence Expected Before Extension Perps Review
+
+The PR carries a controller package version and contract compatibility note, a state-flow matrix for the selectors and hooks touched, a market data source matrix across stream, detail, order, chart and fallback paths, recordings for order form toggles and submitted params, and, for backend-routing changes, the preload cache and reconnect fallback paths explicitly addressed.

domains/perps/skills/perps-review-pr/references/criteria/extension/extension-must-consume-the-published-controller-contract.md

@@ -0,0 +1,10 @@
+# Extension Must Consume the Published Controller Contract
+
+A controller bump or a `perps-events.ts` merge is proven against the shipped `@metamask/perps-controller` bundle and its `.d.cts`, not against manifests or Mobile assumptions.
+
+- **Client patch compensates for controller state gap** — fix the shared controller contract or use explicit client-owned state.
+- **Assumes Mobile-only initialization semantics** — Extension background/controller init may differ.
+- **Package bump without compatibility check** — controller version changes need state/method/event compatibility validation.
+- **Package bump proved only from manifests or `node_modules`** — those checks can pass while `dist` is stale. After building, verify a symbol introduced by the target controller version is present in the shipped bundle.
+- **New constant accepted from main without contract check** — when resolving a merge conflict in `perps-events.ts` or similar, verify every constant added by main against `@metamask/perps-controller`'s `.d.cts` before accepting. Some constants are already supplied by the controller spread with identical string values (no-op to add); others are Extension-only aliases that must stay in the local alias layer. A constant that exists on neither side but has live consumers will cause a compile break if it is accidentally dropped.
+- **Extension-only alias keys added as inline snake_case** — Extension-only analytics property keys (e.g. `query_count`, `time_in_search_ms`) must go in the alias layer of `shared/constants/perps-events.ts`, not as inline snake_case object keys. Inline snake_case keys trip `@typescript-eslint/naming-convention`.

domains/perps/skills/perps-review-pr/references/criteria/extension/hook-import-boundaries.md

@@ -0,0 +1,7 @@
+# Hook Import Boundaries
+
+Shared perps hooks are imported from their module file, not the `hooks/perps` barrel, stream-module mocks list every hook a component uses, and no hook mutates a caller's ref.
+
+- **Import shared hooks from their module, not the `hooks/perps` barrel** — components rendered by many hosts must import shared hooks (e.g. `usePerpsEventTracking`) directly from their module file, not from the `hooks/perps` barrel. Several test suites partially mock the barrel, so a barrel import surfaces as `usePerpsEventTracking is not a function` at render in unrelated tests.
+- **Stream-module mocks are explicit whitelists** — when a covered component imports another stream hook, update the test's stream-module mock object too. A missing hook otherwise fails behind the React Router error boundary as an unrelated `is not a function` render error.
+- **`react-compiler` forbids mutating a hook argument** — a hook cannot reset a caller's `hasCommittedRef`. The reset belongs in the caller's own open/reset effect. Mobile's version does mutate the ref — do not copy that part.

domains/perps/skills/perps-review-pr/references/criteria/extension/market-data-source-and-provider-behavior-must-be-consistent-across-paths.md

@@ -0,0 +1,7 @@
+# Market Data Source and Provider Behavior Must Be Consistent Across Paths
+
+A preferred market data source or provider applies to every fetch path (stream, market detail, order form, charts, fallback) through a typed, visible selection with tested fallback.
+
+- **Preferred source wired only to stream path** — detail/order/chart fetches still use old/default source.
+- **Source choice hidden in unchanged params** — make source/provider selection typed and visible.
+- **Fallback path lacks evidence** — source/provider fallback should be tested and documented.

domains/perps/skills/perps-review-pr/references/criteria/extension/order-forms-must-preserve-user-input-across-toggles.md

@@ -0,0 +1,6 @@
+# Order Forms Must Preserve User Input Across Toggles
+
+A TP/SL sign or percent toggle transforms the existing value, and the submitted order params equal what the form displays.
+
+- **TP/SL sign or percent toggle drops value** — toggles should transform existing state, not reset it unexpectedly.
+- **Displayed value differs from submit params** — submitted order must match what user sees.

domains/perps/skills/perps-review-pr/references/criteria/perps/agentic-testability-testids.md

@@ -0,0 +1,8 @@
+# Agentic Testability (testIDs)
+
+PRs that touch UI components must include testIDs so agentic recipes and E2E tests can navigate and assert on the app without manual interaction.
+
+- **Missing testID on interactive elements** — any `TextInput`, `Pressable`, `Button`, or touchable in a new or modified component without a `testID` prop. Agentic recipes use `app-state.sh press <testID>` and `eval_sync` fiber-walk queries to interact with and assert on UI. If the element has no testID, the recipe cannot press it or read its value — the fix is untestable agentically.
+- **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.

domains/perps/skills/perps-review-pr/references/criteria/perps/connection-websocket-architecture.md

@@ -0,0 +1,13 @@
+# Connection & WebSocket Architecture
+
+- **Cleanup has no owner for in-flight setup**: Register the owner before asynchronous initialization starts. Timeout, unmount and feature disable must retire that owner and prevent late activation. Cover pre-registration and post-ready paths separately; successful UI disposal must preserve an explicitly owned reuse/grace policy.
+
+A single `PerpsAlwaysOnProvider` at the wallet root owns connect/disconnect; `PerpsConnectionProvider` only exposes connection state (`isEnabled`, `isFullScreen`, `suppressErrorView`) through the singleton connection manager.
+
+- **A second lifecycle owner** — a provider, hook, or screen that calls connect/disconnect itself (or a `PerpsConnectionProvider` variant that tries to) creates reference-count bugs. Only `PerpsAlwaysOnProvider` manages the lifecycle.
+- **Unthrottled WS → setState** — every WS tick triggers state update. Must use `useLivePrices` with appropriate `throttleMs` (100ms for charts, 2s for lists, 10s for order forms).
+- **Per-component WS subscription** — creating a new WebSocket connection per component instead of using `PerpsStreamManager` shared subscriptions with reference counting.
+- **WS subscription leak** — subscribing on mount without unsubscribing on unmount or market switch. `PerpsStreamManager` handles ref counting but custom subscriptions must clean up.
+- **Stale data after async gap** — reading position/order state, awaiting something, then using the stale read. WS updates change state between awaits. Re-read after async boundaries.
+- **Static WebView work coupled to live ticks** — a payload containing both `currentPrice` and static overlays can resend teardown/recreate work on every tick. Compare the static subset before mutating chart lines, do not force autoscale on a no-op update, and cover skip/clear behavior with executable helper tests rather than source-string assertions.
+- **Missing cache invalidation** — after trade/withdrawal/position change, not calling `PerpsCacheInvalidator.invalidate()` for affected cache types (`positions`, `accountState`). Standalone queries on token detail pages show stale data.