From c2701bb7383080314a212a5aa4c9ae269359474a Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Mon, 31 Aug 2026 16:32:39 +0200 Subject: [PATCH 01/16] feat: add analytics skill with MetaMask Mobile overlay - Introduced a new `analytics` skill that provides a repo-agnostic base and integrates a MetaMask Mobile overlay for the canonical tracking API. This skill is marked as `mandatory: true`, ensuring it installs even when the `coding` domain is filtered out. - Updated documentation in `README.md` to clarify the behavior of `mandatory: true` in relation to domain filtering. --- CHANGELOG.md | 2 + README.md | 3 + .../skills/analytics/repos/metamask-mobile.md | 117 ++++++++++++++++++ domains/coding/skills/analytics/skill.md | 25 ++++ 4 files changed, 147 insertions(+) create mode 100644 domains/coding/skills/analytics/repos/metamask-mobile.md create mode 100644 domains/coding/skills/analytics/skill.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 1edf27fe..01c712aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Install a **base skill set by default**. Skills marked `base: true` in frontmatter install on every automatic `postinstall` regardless of domain selection, so a fresh clone lands a useful set with no configuration. `yarn skills` is unchanged and still installs every domain. Opt out with `SKILLS_AUTO_UPDATE=0`. ([#135](https://github.com/MetaMask/skills/pull/135)) - Lint rules for base skills: a `base: true` skill cannot also be `experimental`, and its `description` must be long enough to self-trigger (`BASE_DESCRIPTION_MIN`). Both are errors, so CI fails rather than warning invisibly. ([#135](https://github.com/MetaMask/skills/pull/135)) - Validate `--domain` / `SKILLS_DOMAINS` against the domains that actually exist. A typo previously installed the base set and exited 0, which reads as success. ([#135](https://github.com/MetaMask/skills/pull/135)) +- Add `analytics` skill with a repo-agnostic base and a MetaMask Mobile overlay for the canonical tracking API. Marked `mandatory: true` so it installs even when the `coding` domain is filtered out. + - Add `swaps-cpu-profile-audit` skill for MetaMask Mobile: parse a recorded Hermes / React Native Release Profiler `.cpuprofile` and audit slow frames in swaps/bridge. ([#123](https://github.com/MetaMask/skills/pull/123)) - Add opt-in stale project skill pruning via `--prune-stale` and `SKILLS_PRUNE_STALE=1`. - docs(testing): document CV stale-press flakiness plus high-leverage assert patterns (migration parity, filter both-sides example, loading/skeleton honesty, RefreshControl + flag overrides) in `mobile-testing` component-view and placement refs. diff --git a/README.md b/README.md index c1608837..d90ddcaf 100644 --- a/README.md +++ b/README.md @@ -386,6 +386,9 @@ Extra metadata blocks (e.g. OpenClaw-style `metadata:` with emoji and homepage) are preserved through install — only `name`, `description`, `maturity`, `base`, and `scope` are read by the CLI. +`mandatory: true` installs the skill even when its domain is filtered out +(`--exclude` / `SKILLS_EXCLUDE` still wins). + The 1,536-character ceiling is a repo budget rather than an operator limit — the description is always-on context for every installed skill, so it is capped deliberately. It is enforced by `yarn audit:skills` from diff --git a/domains/coding/skills/analytics/repos/metamask-mobile.md b/domains/coding/skills/analytics/repos/metamask-mobile.md new file mode 100644 index 00000000..3a41a193 --- /dev/null +++ b/domains/coding/skills/analytics/repos/metamask-mobile.md @@ -0,0 +1,117 @@ +--- +repo: metamask-mobile +parent: analytics +--- + +# Analytics — MetaMask Mobile + +Human-facing file map: `app/core/Analytics/README.md`. + +## Canonical API + +Two emission paths. Use one of them; do not add a third. + +| Role | Path | +|------|------| +| Imperative helper | `app/util/analytics/analytics.ts` → `analytics.trackEvent` | +| Controller messenger | `AnalyticsController:trackEvent` via the Engine / init messenger | +| React hook (same helper) | `app/components/hooks/useAnalytics/useAnalytics.ts` → `useAnalytics` | +| Event builder | `app/util/analytics/AnalyticsEventBuilder.ts` → `AnalyticsEventBuilder.createEventBuilder` | +| Catalog | `app/core/Analytics/` → `EVENT_NAME`, `MetaMetricsEvents` | +| Test factory | `app/util/test/analyticsMock.ts` → `createMockUseAnalyticsHook` | + +`useAnalytics()` is the React face of `analytics`. It returns `trackEvent`, +`createEventBuilder`, `identify`, `enable`, `isEnabled`, `getAnalyticsId`, +and data-deletion helpers. + +Controllers that already talk to Engine should call +`messenger.call('AnalyticsController:trackEvent', event)` with a built event. + +## Require + +- UI: platform `useAnalytics` from `app/components/hooks/useAnalytics/useAnalytics.ts` +- Non-React: `analytics.trackEvent`, or `AnalyticsController:trackEvent` on a messenger +- Event names from `EVENT_NAME` / `MetaMetricsEvents` +- Properties via `.addProperties(...).build()` +- Tests: `createMockUseAnalyticsHook` + +```ts +import { useAnalytics } from '../../hooks/useAnalytics/useAnalytics'; +import { EVENT_NAME } from '../../../core/Analytics'; + +const { trackEvent, createEventBuilder, identify } = useAnalytics(); + +trackEvent( + createEventBuilder(EVENT_NAME.RAMPS_BUTTON_CLICKED) + .addProperties({ location: 'AccountsMenu' }) + .build(), +); + +await identify({ /* traits */ }); +``` + +Non-React: + +```ts +import { analytics } from '../../util/analytics/analytics'; +import { AnalyticsEventBuilder } from '../../util/analytics/AnalyticsEventBuilder'; +import { EVENT_NAME } from '../../core/Analytics'; + +analytics.trackEvent( + AnalyticsEventBuilder.createEventBuilder(EVENT_NAME.APP_OPENED) + .addProperties({ source: 'cold_start' }) + .build(), +); +``` + +Messenger (controllers): + +```ts +initMessenger.call( + 'AnalyticsController:trackEvent', + AnalyticsEventBuilder.createEventBuilder(EVENT_NAME.APP_OPENED) + .addProperties({ source: 'cold_start' }) + .build(), +); +``` + +Prefer `EVENT_NAME.*` strings. `MetaMetricsEvents.*` wrappers (`IMetaMetricsEvent`) +are still valid; `createEventBuilder` copies only `category`. When migrating a +wrapper that used `generateOpt(name, action, description)`, re-apply +`properties.action` and `properties.name` with `addProperties`. + +`generateOpt` belongs in catalog modules: `app/core/Analytics/MetaMetrics.events.ts`, +`app/core/Analytics/events/`, and feature-local `/analytics/events.ts` +(see SampleFeature). Component files import catalog entries; they do not call +`generateOpt` themselves. + +Tests mock the hook with the factory, not a hand-built object: + +```ts +import { useAnalytics } from '../../hooks/useAnalytics/useAnalytics'; +import { createMockUseAnalyticsHook } from '../../../util/test/analyticsMock'; +import { AnalyticsEventBuilder } from '../../../util/analytics/AnalyticsEventBuilder'; + +jest.mock('../../hooks/useAnalytics/useAnalytics'); + +jest.mocked(useAnalytics).mockReturnValue( + createMockUseAnalyticsHook({ + trackEvent: mockTrackEvent, + createEventBuilder: AnalyticsEventBuilder.createEventBuilder, + }), +); +``` + +## Reject + +- `addSensitiveProperties` — deprecated. New tracking uses `addProperties` only. + When editing a call site that already uses `addSensitiveProperties`, stop and + review those fields: drop them, or move them to `addProperties`, whenever + that is safe. Do not add new sensitive properties to an existing event. +- A feature-owned tracking API between the call site and `analytics` / + `AnalyticsController:trackEvent` (a second `useAnalytics`, a typed event + map, an `*Analytics` module, a local `track*` helper). Call the platform + helper or messenger directly. Existing feature APIs stay; do not add another. +- Reintroducing `useMetrics` (removed) or MetaMetrics internals at call sites +- Dropping `generateOpt` `action` / `name` when migrating `IMetaMetricsEvent` call sites (until the catalog migration lands) +- Hand-built `useAnalytics` mock objects — use `createMockUseAnalyticsHook` diff --git a/domains/coding/skills/analytics/skill.md b/domains/coding/skills/analytics/skill.md new file mode 100644 index 00000000..da759641 --- /dev/null +++ b/domains/coding/skills/analytics/skill.md @@ -0,0 +1,25 @@ +--- +name: analytics +description: >- + Product analytics and event tracking. Use when adding, migrating, or + reviewing tracked events, or when writing tests for analytics call sites. +maturity: stable +mandatory: true +--- + +# Analytics + +Use this skill for product event tracking. + +## When to use + +- Adding or migrating event tracking in UI or non-UI code +- Writing or updating tests for analytics call sites +- Reviewing a PR that introduces or changes tracked events + +## Workflow + +1. Pick an event name from the catalog. +2. Attach properties on the event builder. +3. Send the built event through the tracking entry point. +4. In tests, mock the analytics hook with the test factory. From c8889c933aec5030df4fd0aa08dce8b827307134 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Thu, 3 Sep 2026 11:50:11 +0200 Subject: [PATCH 02/16] docs(analytics): update workflow and testing guidelines for event tracking - Revised the event tracking workflow to clarify the registration process in the catalog, emphasizing the reuse of existing catalog names only for identical interactions. - Enhanced UI testing instructions to specify wrapping `useAnalytics` with the test factory and asserting builder calls in non-React tests. - Updated documentation to reflect these changes and improve clarity on testing practices. --- .../skills/analytics/repos/metamask-mobile.md | 31 +++++++++++++------ domains/coding/skills/analytics/skill.md | 4 +-- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/domains/coding/skills/analytics/repos/metamask-mobile.md b/domains/coding/skills/analytics/repos/metamask-mobile.md index 3a41a193..1566a5ed 100644 --- a/domains/coding/skills/analytics/repos/metamask-mobile.md +++ b/domains/coding/skills/analytics/repos/metamask-mobile.md @@ -31,9 +31,10 @@ Controllers that already talk to Engine should call - UI: platform `useAnalytics` from `app/components/hooks/useAnalytics/useAnalytics.ts` - Non-React: `analytics.trackEvent`, or `AnalyticsController:trackEvent` on a messenger -- Event names from `EVENT_NAME` / `MetaMetricsEvents` +- Event names from `EVENT_NAME` / `MetaMetricsEvents`. New tracking: add the name in catalog modules, then import it. Reuse a catalog name only when this control is the same interaction as existing call sites (same event, same product meaning). - Properties via `.addProperties(...).build()` -- Tests: `createMockUseAnalyticsHook` +- UI tests: `createMockUseAnalyticsHook` wrapping `useAnalytics`, including when the file already mocks the hook +- Non-React tests: assert `AnalyticsEventBuilder.createEventBuilder` and `analytics.trackEvent` or `AnalyticsController:trackEvent` ```ts import { useAnalytics } from '../../hooks/useAnalytics/useAnalytics'; @@ -85,7 +86,14 @@ wrapper that used `generateOpt(name, action, description)`, re-apply (see SampleFeature). Component files import catalog entries; they do not call `generateOpt` themselves. -Tests mock the hook with the factory, not a hand-built object: +Tests mock the hook with the factory, not a hand-built object. +Call `createMockUseAnalyticsHook` again in `beforeEach` after +`jest.clearAllMocks()` / `jest.resetAllMocks()` — those wipe mock +implementations. Prefer `AnalyticsEventBuilder.createEventBuilder`. +When existing assertions inspect a simplified `{ event, properties }` +payload, pass a stub builder into the factory (`createMockEventBuilder` +in `analyticsMock.ts`, or a local stub); still wrap the hook with the +factory. ```ts import { useAnalytics } from '../../hooks/useAnalytics/useAnalytics'; @@ -94,12 +102,15 @@ import { AnalyticsEventBuilder } from '../../../util/analytics/AnalyticsEventBui jest.mock('../../hooks/useAnalytics/useAnalytics'); -jest.mocked(useAnalytics).mockReturnValue( - createMockUseAnalyticsHook({ - trackEvent: mockTrackEvent, - createEventBuilder: AnalyticsEventBuilder.createEventBuilder, - }), -); +beforeEach(() => { + jest.clearAllMocks(); + jest.mocked(useAnalytics).mockReturnValue( + createMockUseAnalyticsHook({ + trackEvent: mockTrackEvent, + createEventBuilder: AnalyticsEventBuilder.createEventBuilder, + }), + ); +}); ``` ## Reject @@ -115,3 +126,5 @@ jest.mocked(useAnalytics).mockReturnValue( - Reintroducing `useMetrics` (removed) or MetaMetrics internals at call sites - Dropping `generateOpt` `action` / `name` when migrating `IMetaMetricsEvent` call sites (until the catalog migration lands) - Hand-built `useAnalytics` mock objects — use `createMockUseAnalyticsHook` +- Attaching a new control to a catalog event whose live call sites are a different product (example: `VIEW_ALL_ASSETS_CLICKED` is wallet tokens/NFTs `asset_type`, not a homepage section) +- Firing an existing catalog event at a new lifecycle (example: `TOKEN_DETECTED` on controller init). Add a catalog name for that lifecycle. diff --git a/domains/coding/skills/analytics/skill.md b/domains/coding/skills/analytics/skill.md index da759641..7f28688c 100644 --- a/domains/coding/skills/analytics/skill.md +++ b/domains/coding/skills/analytics/skill.md @@ -19,7 +19,7 @@ Use this skill for product event tracking. ## Workflow -1. Pick an event name from the catalog. +1. Register this interaction in the catalog (`EVENT_NAME` + `generateOpt` in catalog modules). Reuse an existing catalog name only when this control is another instance of that same interaction (same dashboard event, same owners). 2. Attach properties on the event builder. 3. Send the built event through the tracking entry point. -4. In tests, mock the analytics hook with the test factory. +4. In UI tests, wrap `useAnalytics` with the test factory (including files that already mock the hook). In non-React tests, assert the builder and the helper or messenger call. From 416e7f9c6e252fbf15d99ba8e529be3815c145a7 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Thu, 3 Sep 2026 12:56:47 +0200 Subject: [PATCH 03/16] docs(analytics): clarify emission paths and requirements in tracking API documentation - Updated the documentation to specify the two emission paths for analytics: the `analytics` helper and `AnalyticsController:trackEvent` via `initMessenger`. - Enhanced clarity on the roles of different components in the analytics system, including the distinction between non-React and UI helpers. - Revised the requirements section to reflect the updated paths and usage guidelines for analytics tracking. --- .../skills/analytics/repos/metamask-mobile.md | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/domains/coding/skills/analytics/repos/metamask-mobile.md b/domains/coding/skills/analytics/repos/metamask-mobile.md index 1566a5ed..fcf289e8 100644 --- a/domains/coding/skills/analytics/repos/metamask-mobile.md +++ b/domains/coding/skills/analytics/repos/metamask-mobile.md @@ -9,28 +9,27 @@ Human-facing file map: `app/core/Analytics/README.md`. ## Canonical API -Two emission paths. Use one of them; do not add a third. +Two emission paths: the `analytics` helper, and `AnalyticsController:trackEvent` on the Engine / init messenger. `useAnalytics()` is the UI wrapper around the helper (`analytics.trackEvent`). | Role | Path | |------|------| -| Imperative helper | `app/util/analytics/analytics.ts` → `analytics.trackEvent` | -| Controller messenger | `AnalyticsController:trackEvent` via the Engine / init messenger | -| React hook (same helper) | `app/components/hooks/useAnalytics/useAnalytics.ts` → `useAnalytics` | +| Helper (non-React) | `app/util/analytics/analytics.ts` → `analytics.trackEvent` | +| Helper (UI) | `app/components/hooks/useAnalytics/useAnalytics.ts` → `useAnalytics` | +| Messenger | `AnalyticsController:trackEvent` via `initMessenger.call` | | Event builder | `app/util/analytics/AnalyticsEventBuilder.ts` → `AnalyticsEventBuilder.createEventBuilder` | | Catalog | `app/core/Analytics/` → `EVENT_NAME`, `MetaMetricsEvents` | | Test factory | `app/util/test/analyticsMock.ts` → `createMockUseAnalyticsHook` | -`useAnalytics()` is the React face of `analytics`. It returns `trackEvent`, -`createEventBuilder`, `identify`, `enable`, `isEnabled`, `getAnalyticsId`, -and data-deletion helpers. +`useAnalytics()` returns `trackEvent`, `createEventBuilder`, `identify`, `enable`, +`isEnabled`, `getAnalyticsId`, and data-deletion helpers. Controllers that already talk to Engine should call -`messenger.call('AnalyticsController:trackEvent', event)` with a built event. +`initMessenger.call('AnalyticsController:trackEvent', event)` with a built event. -## Require +## Requirements - UI: platform `useAnalytics` from `app/components/hooks/useAnalytics/useAnalytics.ts` -- Non-React: `analytics.trackEvent`, or `AnalyticsController:trackEvent` on a messenger +- Non-React: `analytics.trackEvent`, or `AnalyticsController:trackEvent` on `initMessenger` - Event names from `EVENT_NAME` / `MetaMetricsEvents`. New tracking: add the name in catalog modules, then import it. Reuse a catalog name only when this control is the same interaction as existing call sites (same event, same product meaning). - Properties via `.addProperties(...).build()` - UI tests: `createMockUseAnalyticsHook` wrapping `useAnalytics`, including when the file already mocks the hook From d3a7e402cd6e613733e07a913aed742d7d56722c Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Thu, 3 Sep 2026 14:47:06 +0200 Subject: [PATCH 04/16] docs(analytics): update CHANGELOG and README for analytics skill and platform domain - Added the `analytics` skill to the CHANGELOG, highlighting its repo-agnostic base and MetaMask Mobile overlay. - Updated the README to include the new `platform` domain, clarifying its purpose for product analytics and platform skills. - Adjusted the `analytics` skill's domain from `coding` to `platform` to better reflect its functionality. --- .github/CODEOWNERS | 1 + CHANGELOG.md | 4 ++-- README.md | 3 ++- .../skills/analytics/repos/metamask-mobile.md | 0 domains/{coding => platform}/skills/analytics/skill.md | 2 +- 5 files changed, 6 insertions(+), 4 deletions(-) rename domains/{coding => platform}/skills/analytics/repos/metamask-mobile.md (100%) rename domains/{coding => platform}/skills/analytics/skill.md (98%) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 20217c32..b610ca32 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -17,6 +17,7 @@ /domains/coding/ @MetaMask/extension-platform @MetaMask/mobile-platform @MetaMask/core-platform /domains/general/ @MetaMask/extension-platform @MetaMask/mobile-platform /domains/performance/ @MetaMask/extension-platform @MetaMask/mobile-platform +/domains/platform/ @MetaMask/extension-platform @MetaMask/mobile-platform @MetaMask/core-platform /domains/perps/ @MetaMask/perps /domains/pr-workflow/ @MetaMask/extension-platform @MetaMask/mobile-platform /domains/swaps/ @MetaMask/swaps-engineers diff --git a/CHANGELOG.md b/CHANGELOG.md index 01c712aa..6e7cf4b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,8 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Install a **base skill set by default**. Skills marked `base: true` in frontmatter install on every automatic `postinstall` regardless of domain selection, so a fresh clone lands a useful set with no configuration. `yarn skills` is unchanged and still installs every domain. Opt out with `SKILLS_AUTO_UPDATE=0`. ([#135](https://github.com/MetaMask/skills/pull/135)) - Lint rules for base skills: a `base: true` skill cannot also be `experimental`, and its `description` must be long enough to self-trigger (`BASE_DESCRIPTION_MIN`). Both are errors, so CI fails rather than warning invisibly. ([#135](https://github.com/MetaMask/skills/pull/135)) - Validate `--domain` / `SKILLS_DOMAINS` against the domains that actually exist. A typo previously installed the base set and exited 0, which reads as success. ([#135](https://github.com/MetaMask/skills/pull/135)) -- Add `analytics` skill with a repo-agnostic base and a MetaMask Mobile overlay for the canonical tracking API. Marked `mandatory: true` so it installs even when the `coding` domain is filtered out. - +- Add `analytics` skill 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. - Add `swaps-cpu-profile-audit` skill for MetaMask Mobile: parse a recorded Hermes / React Native Release Profiler `.cpuprofile` and audit slow frames in swaps/bridge. ([#123](https://github.com/MetaMask/skills/pull/123)) - Add opt-in stale project skill pruning via `--prune-stale` and `SKILLS_PRUNE_STALE=1`. - docs(testing): document CV stale-press flakiness plus high-leverage assert patterns (migration parity, filter both-sides example, loading/skeleton honesty, RefreshControl + flag overrides) in `mobile-testing` component-view and placement refs. @@ -23,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Breaking:** frontmatter key `mandatory` renamed to `base`. Update any skill still using `mandatory:`. ([#135](https://github.com/MetaMask/skills/pull/135)) - **Breaking:** `postinstall` now syncs by default. It previously required `SKILLS_AUTO_UPDATE=1`; the opt-out is now `SKILLS_AUTO_UPDATE=0`. An explicitly empty `SKILLS_AUTO_UPDATE=` keeps its old meaning (off) rather than being read as unset. ([#135](https://github.com/MetaMask/skills/pull/135)) - `base:` truthiness is consistent across all four implementations. The linter alone accepted `on`/`off`, so `base: on` linted clean while the installer skipped the skill. ([#135](https://github.com/MetaMask/skills/pull/135)) +- Move `analytics` from `coding` to a new `platform` domain (`platform/analytics`). - Rewrite `CONTRIBUTING.md` and skill template for MetaMask/skills layout. - `swaps-cpu-profile-audit` now audits the whole capture instead of swaps-owned files only: non-swaps frames that ran while the user was on a swaps screen are classified by their relation to the swaps call stacks (called by swaps, hosts the swaps screen, or concurrent with it), bucketed into named context areas, and reported alongside swaps rows. Every reported row carries an `Owned by swaps` column, and fix depth is gated on it. New `--context-min-pct` and `--swaps-only` analyzer flags. - `swaps-cpu-profile-audit` reports swaps-owned areas, non-swaps areas on the swaps path, and non-swaps areas running concurrently as separate tables, so the per-area swaps detail is no longer diluted by context rows. The swaps table gained an inclusive-time column, and the report explains that self time on a leaf means a screen can trigger heavy work while showing ~0 ms of its own. Non-swaps rows in the fix table are now capped to the few that matter. diff --git a/README.md b/README.md index d90ddcaf..fcbcb780 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,7 @@ tools/ | -------------- | ----------------- | ------------------------------------------- | | `web3-tools` | dApp builders | `gator-cli`, `smart-accounts-kit`, `oh-my-opencode` | | `coding` | MM product eng | Coding guidelines, controller patterns | +| `platform` | MM product eng | Product analytics and other platform skills | | `agentic` | MM product eng | Experimental recipe workflows and runtime proof tools | | `assets` | MM product eng | Assets domain skills | | `general` | All agents | `codex`, `gemini` CLI usage guides | @@ -386,7 +387,7 @@ Extra metadata blocks (e.g. OpenClaw-style `metadata:` with emoji and homepage) are preserved through install — only `name`, `description`, `maturity`, `base`, and `scope` are read by the CLI. -`mandatory: true` installs the skill even when its domain is filtered out +`base: true` installs the skill even when its domain is filtered out (`--exclude` / `SKILLS_EXCLUDE` still wins). The 1,536-character ceiling is a repo budget rather than an operator limit — the diff --git a/domains/coding/skills/analytics/repos/metamask-mobile.md b/domains/platform/skills/analytics/repos/metamask-mobile.md similarity index 100% rename from domains/coding/skills/analytics/repos/metamask-mobile.md rename to domains/platform/skills/analytics/repos/metamask-mobile.md diff --git a/domains/coding/skills/analytics/skill.md b/domains/platform/skills/analytics/skill.md similarity index 98% rename from domains/coding/skills/analytics/skill.md rename to domains/platform/skills/analytics/skill.md index 7f28688c..2febf503 100644 --- a/domains/coding/skills/analytics/skill.md +++ b/domains/platform/skills/analytics/skill.md @@ -4,7 +4,7 @@ description: >- Product analytics and event tracking. Use when adding, migrating, or reviewing tracked events, or when writing tests for analytics call sites. maturity: stable -mandatory: true +base: true --- # Analytics From 478dd8d93defe4cad5cf39cd83d71a92e111b793 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Fri, 4 Sep 2026 11:18:25 +0200 Subject: [PATCH 05/16] feat(platform): add feature-flags skill with MetaMask Mobile overlay --- CHANGELOG.md | 1 + .../feature-flags/repos/metamask-mobile.md | 71 +++++++++++++++++++ .../platform/skills/feature-flags/skill.md | 27 +++++++ 3 files changed, 99 insertions(+) create mode 100644 domains/platform/skills/feature-flags/repos/metamask-mobile.md create mode 100644 domains/platform/skills/feature-flags/skill.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e7cf4b0..ddcea606 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Install a **base skill set by default**. Skills marked `base: true` in frontmatter install on every automatic `postinstall` regardless of domain selection, so a fresh clone lands a useful set with no configuration. `yarn skills` is unchanged and still installs every domain. Opt out with `SKILLS_AUTO_UPDATE=0`. ([#135](https://github.com/MetaMask/skills/pull/135)) - Lint rules for base skills: a `base: true` skill cannot also be `experimental`, and its `description` must be long enough to self-trigger (`BASE_DESCRIPTION_MIN`). Both are errors, so CI fails rather than warning invisibly. ([#135](https://github.com/MetaMask/skills/pull/135)) - Validate `--domain` / `SKILLS_DOMAINS` against the domains that actually exist. A typo previously installed the base set and exited 0, which reads as success. ([#135](https://github.com/MetaMask/skills/pull/135)) +- 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. - Add `analytics` skill 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. - Add `swaps-cpu-profile-audit` skill for MetaMask Mobile: parse a recorded Hermes / React Native Release Profiler `.cpuprofile` and audit slow frames in swaps/bridge. ([#123](https://github.com/MetaMask/skills/pull/123)) - Add opt-in stale project skill pruning via `--prune-stale` and `SKILLS_PRUNE_STALE=1`. diff --git a/domains/platform/skills/feature-flags/repos/metamask-mobile.md b/domains/platform/skills/feature-flags/repos/metamask-mobile.md new file mode 100644 index 00000000..2fe7f377 --- /dev/null +++ b/domains/platform/skills/feature-flags/repos/metamask-mobile.md @@ -0,0 +1,71 @@ +--- +repo: metamask-mobile +parent: feature-flags +--- + +# Feature flags — MetaMask Mobile + +Human-facing file map: `docs/readme/version-gated-feature-flags.md`. + +## Canonical API + +| Role | Path | +|------|------| +| Helper | `app/util/remoteFeatureFlag/index.ts` → `validatedVersionGatedFeatureFlag`, `hasMinimumRequiredVersion` | +| Raw flags | `RemoteFeatureFlagController.remoteFeatureFlags` (no client version gating) | +| Selectors | `app/selectors/featureFlagController/` or `**/selectors/featureFlags/` | +| Names (when the flag is overrideable in dev tools) | `app/constants/featureFlags.ts` → `FeatureFlagNames` | + +`validatedVersionGatedFeatureFlag` compares against the native binary version from `react-native-device-info` `getVersion()`, not `package.json`. Progressive-rollout wrappers `{ name, value: { enabled, minimumVersion } }` are unwrapped in the helper. + +`hasMinimumRequiredVersion` is the lower-level compare used inside the helper. Import it from the util only for a standalone version check. + +Multi-version flags shaped `{ versions: { "7.53.0": value } }` are resolved at fetch time by the controller. Consumers read the processed value — no helper call for that shape. + +## Requirements + +- Selectors call `validatedVersionGatedFeatureFlag`. UI and hooks only `useSelector(selectXEnabled)`. +- Non-standard flag shapes (e.g. `active` instead of `enabled`) map to `{ enabled, minimumVersion }` before the helper. +- Remote flag wins when valid. Use `?? env/local` when the helper returns `undefined` (invalid shape, or `OVERRIDE_REMOTE_FEATURE_FLAGS=true`). + +```ts +import { createSelector } from 'reselect'; +import { selectRemoteFeatureFlags } from '../index'; +import { + validatedVersionGatedFeatureFlag, + type VersionGatedFeatureFlag, +} from '../../../util/remoteFeatureFlag'; + +export const selectMyFeatureEnabled = createSelector( + selectRemoteFeatureFlags, + (remoteFeatureFlags) => { + const localFlag = process.env.MM_MY_FEATURE_ENABLED === 'true'; + const remoteFlag = + remoteFeatureFlags?.myFeature as unknown as VersionGatedFeatureFlag; + + return validatedVersionGatedFeatureFlag(remoteFlag) ?? localFlag; + }, +); +``` + +Non-standard shape: + +```ts +validatedVersionGatedFeatureFlag({ + enabled: depositConfig.active ?? false, + minimumVersion: depositConfig.minimumVersion ?? '', +}) ?? false; +``` + +UI: + +```ts +const isEnabled = useSelector(selectMyFeatureEnabled); +``` + +## Reject + +- Local copies of `hasMinimumRequiredVersion` or `validatedVersionGatedFeatureFlag` +- Inline `compare-versions` + `getVersion` for feature-flag gating outside `app/util/remoteFeatureFlag` +- Version checks in hooks or components (`getVersion()`, `compare-versions`, or a local helper) +- Duplicate util files under `app/components/UI/**/utils/` or `app/core/redux/slices/**/` diff --git a/domains/platform/skills/feature-flags/skill.md b/domains/platform/skills/feature-flags/skill.md new file mode 100644 index 00000000..6f230054 --- /dev/null +++ b/domains/platform/skills/feature-flags/skill.md @@ -0,0 +1,27 @@ +--- +name: feature-flags +description: >- + Version-gated remote feature flags. Use when adding, migrating, or + reviewing a remote or version-gated feature flag, writing a flag + selector, or gating UI on a flag boolean. +maturity: stable +base: true +--- + +# Feature flags + +Use this skill for remote and version-gated feature flags. + +## When to use + +- Adding or migrating a remote / version-gated feature flag +- Writing or updating a flag selector +- Gating UI or hooks on a flag boolean +- Reviewing a PR that introduces or changes flag evaluation + +## Workflow + +1. Evaluate the flag in a selector. Return a boolean. +2. Map a non-standard remote shape to `{ enabled, minimumVersion }` before the shared helper. +3. Consume that boolean in UI or hooks. Do not re-run version math there. +4. Cover the selector with collocated tests (enabled, disabled, invalid, fallback). From 2d99fc0a85358017b1eb91af8b12368b84053202 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Wed, 9 Sep 2026 13:33:40 +0200 Subject: [PATCH 06/16] docs: add PR link to feature-flags changelog entry Co-authored-by: Cursor --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddcea606..01f44780 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Install a **base skill set by default**. Skills marked `base: true` in frontmatter install on every automatic `postinstall` regardless of domain selection, so a fresh clone lands a useful set with no configuration. `yarn skills` is unchanged and still installs every domain. Opt out with `SKILLS_AUTO_UPDATE=0`. ([#135](https://github.com/MetaMask/skills/pull/135)) - Lint rules for base skills: a `base: true` skill cannot also be `experimental`, and its `description` must be long enough to self-trigger (`BASE_DESCRIPTION_MIN`). Both are errors, so CI fails rather than warning invisibly. ([#135](https://github.com/MetaMask/skills/pull/135)) - Validate `--domain` / `SKILLS_DOMAINS` against the domains that actually exist. A typo previously installed the base set and exited 0, which reads as success. ([#135](https://github.com/MetaMask/skills/pull/135)) -- 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. +- 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 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. - Add `swaps-cpu-profile-audit` skill for MetaMask Mobile: parse a recorded Hermes / React Native Release Profiler `.cpuprofile` and audit slow frames in swaps/bridge. ([#123](https://github.com/MetaMask/skills/pull/123)) - Add opt-in stale project skill pruning via `--prune-stale` and `SKILLS_PRUNE_STALE=1`. From abc8798e03563d05e0f6f1087861d487f2195684 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 9 Sep 2026 10:29:25 -0400 Subject: [PATCH 07/16] Add the `metamask-extension` overlay for `feature-flags` The extension's helper is a direct port of mobile's and its header says so, so the exported names match. Three things do not, and they are what the overlay is for. The version basis inverts mobile's warning: mobile compares against the native binary version, the extension against `packageJson.version`. Two modules export `hasMinimumRequiredVersion`, `validatedVersionGatedFeatureFlag` and `VersionGatedFeatureFlag` with different semantics. Only one unwraps progressive-rollout wrappers, and an import of either name compiles against either module. `getBooleanFeatureFlag` is the house entry point rather than mobile's, at 17 production call sites against 6, and it takes the fallback as a required argument instead of a trailing `??`. --- .../feature-flags/repos/metamask-extension.md | 171 ++++++++++++++++++ 1 file changed, 171 insertions(+) create mode 100644 domains/platform/skills/feature-flags/repos/metamask-extension.md diff --git a/domains/platform/skills/feature-flags/repos/metamask-extension.md b/domains/platform/skills/feature-flags/repos/metamask-extension.md new file mode 100644 index 00000000..df3d2fe6 --- /dev/null +++ b/domains/platform/skills/feature-flags/repos/metamask-extension.md @@ -0,0 +1,171 @@ +--- +repo: metamask-extension +parent: feature-flags +--- + +# Feature flags — MetaMask Extension + +There is no human-facing doc for version gating. `docs/ab-testing.md` covers A/B flags only. +`AGENTS.md` "Working with Feature Flags" covers build flags and local overrides, not version +gating. + +## Canonical API + +| Role | Path | +|------|------| +| Helper | `shared/lib/remote-feature-flag-utils.ts` → `getBooleanFeatureFlag`, `validatedVersionGatedFeatureFlag`, `hasMinimumRequiredVersion` | +| Raw flags | `shared/lib/selectors/remote-feature-flags.ts` → `getRemoteFeatureFlags`, `getFeatureFlagThresholdGroups` | +| Selectors | A colocated `feature-flags.ts` per domain, under `ui/selectors/**` or `shared/lib/**` | +| Names | Exported string constants beside the selector that reads them | +| E2E registry | `test/e2e/feature-flags/feature-flag-registry.ts` → `FEATURE_FLAG_REGISTRY` | + +`shared/lib/remote-feature-flag-utils.ts` is a port of mobile's +`app/util/remoteFeatureFlag/index.ts` and its own header says so, so the exported names match +mobile. The evaluation differs in one respect that matters: `hasMinimumRequiredVersion` +compares against `packageJson.version`, read from the repo's `package.json` at build time. +There is no native binary version and no `react-native-device-info` equivalent. + +`getBooleanFeatureFlag(flagValue, defaultValue)` is the usual entry point, at 34 non-test call +sites against 11 for `validatedVersionGatedFeatureFlag`. It takes the fallback as a required +second argument rather than mobile's trailing `?? localFlag`, and it returns a plain boolean +flag unchanged, so one call covers both a boolean and a version-gated object. + +`validatedVersionGatedFeatureFlag` returns `boolean | undefined`. Reach for it when the caller +must tell an invalid or absent flag apart from a disabled one. + +Progressive rollout wrappers shaped `{ name?, value: { enabled, minimumVersion } }` are +unwrapped by `unwrapVersionGatedFeatureFlag` inside both helpers. + +### Two modules export the same three names + +`shared/lib/feature-flags/version-gating.ts` also exports `hasMinimumRequiredVersion`, +`validatedVersionGatedFeatureFlag` and the type `VersionGatedFeatureFlag`. An import of either +name compiles against either module, so read the import path before trusting the behavior. + +| | `shared/lib/remote-feature-flag-utils.ts` | `shared/lib/feature-flags/version-gating.ts` | +|---|---|---| +| Non-test importers | 14 | 4 | +| `minimumVersion` type | `string` | `string \| null` | +| Wrapper unwrapping | Yes | No | +| Also exports | `getBooleanFeatureFlag`, `isVersionGatedFeatureFlag` | `getBaseSemVerVersion` | + +Prefer `shared/lib/remote-feature-flag-utils.ts` for new code. The other module is live and its +four importers are not defects, but it does not unwrap rollout wrappers. + +Two further local reimplementations of the same version compare exist and take no new callers: +`isMultichainFeatureEnabled` in `shared/lib/multichain-feature-flags.ts`, and +`isPerpsRemoteConfigSatisfied` in `shared/lib/perps-feature-flags.ts`. + +## Requirements + +- Evaluate in a selector or a shared predicate. UI and hooks only call + `useSelector(selectXEnabled)`. +- Map a non-standard remote shape to `{ enabled, minimumVersion }` before calling the helper. +- Register every new remote flag in `FEATURE_FLAG_REGISTRY` with its production default. + +A shared predicate over the raw flag bag, when the background needs the same answer: + +```ts +import { validatedVersionGatedFeatureFlag } from '../remote-feature-flag-utils'; + +export const MY_FEATURE_FLAG_NAME = 'myFeature'; + +export function isMyFeatureEnabled( + remoteFeatureFlags: Record | undefined, +): boolean { + return ( + validatedVersionGatedFeatureFlag(remoteFeatureFlags?.[MY_FEATURE_FLAG_NAME]) ?? false + ); +} +``` + +The selector then composes that predicate over `getRemoteFeatureFlags`, which keeps one +version-gate interpretation shared between the UI and the background: + +```ts +import { createSelector } from 'reselect'; +import { getRemoteFeatureFlags } from '../../../shared/lib/selectors/remote-feature-flags'; +import { isMyFeatureEnabled } from '../../../shared/lib/my-domain/feature-flags'; + +export const selectMyFeatureEnabled = createSelector( + getRemoteFeatureFlags, + isMyFeatureEnabled, +); +``` + +Where no background caller needs the predicate, `getBooleanFeatureFlag` inline is the shorter +and more common form: + +```ts +export const selectMyFeatureEnabled = createSelector( + getRemoteFeatureFlags, + ({ myFeature }) => getBooleanFeatureFlag(myFeature, false), +); +``` + +UI: + +```ts +const isEnabled = useSelector(selectMyFeatureEnabled); +``` + +## Adding a flag + +1. Export the flag name as a constant beside the selector that reads it. +2. Write the predicate or selector against `getRemoteFeatureFlags`. +3. Add a `FEATURE_FLAG_REGISTRY` entry: `name`, `type`, `inProd`, `productionDefault` and + `status`. The registry is what `mock-e2e.js` serves, so an unregistered flag reads as + absent in E2E. +4. Cover the selector with a collocated test. + +`getRemoteFeatureFlags` merges manifest flags over controller state, manifest winning, so a +local override goes in `.manifest-overrides.json` under `_flags.remoteFeatureFlags` with +`MANIFEST_OVERRIDES` set in `.metamaskrc`. There is no `OVERRIDE_REMOTE_FEATURE_FLAGS` switch. + +## Testing + +Collocated unit tests import `packageJson` and build flag values around the real current +version rather than hardcoding one, since the gate reads `package.json`: + +```ts +import packageJson from '../../../package.json'; + +const CURRENT_VERSION = packageJson.version; +``` + +Cover enabled, disabled, below-minimum, rollout-wrapped, and invalid or absent. For E2E, seed +state with `withRemoteFeatureFlagController(...)` from +`test/e2e/fixtures/fixture-builder-v2.ts`, or override at runtime with +`manifestFlags.remoteFeatureFlags`. + +## Not present in the extension + +State these as gaps rather than substituting a near neighbor: + +- No `useRemoteFeatureFlag` hook. Consumption is `useSelector` over a selector. The only + flag-reading hook is `useABTest` in `ui/hooks/useABTest.ts`, which is for A/B assignment and + exposure events, not version gating. +- No central flag-name registry equivalent to mobile's `FeatureFlagNames`. An enum of that + name exists at `shared/lib/feature-flags.ts` holding a single member, and it is not where + flag names live. +- No handling for the multi-version shape `{ versions: { "7.53.0": value } }`. Neither helper + reads a `versions` key. + +## Extension only + +- `FEATURE_FLAG_REGISTRY` is the production-default source of truth for E2E. Mobile has no + counterpart. +- `getRemoteFeatureFlags` folds manifest overrides in at the selector, so override precedence + is a property of the read rather than of a build switch. +- Threshold and A/B flags carry a separate `featureFlagThresholdGroups` map, read through + `getFeatureFlagThresholdGroups`. + +## Reject + +- New local copies of `hasMinimumRequiredVersion`, or a `semver.gte` against + `packageJson.version` for flag gating, outside `shared/lib/remote-feature-flag-utils.ts` +- Version checks in hooks or components +- A new remote flag with no `FEATURE_FLAG_REGISTRY` entry +- Importing `validatedVersionGatedFeatureFlag` without checking which of the two modules it + resolves to + From 9d6c626acc9b4c39bb0861be075af5d099a6773b Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Mon, 31 Aug 2026 16:32:39 +0200 Subject: [PATCH 08/16] feat: add analytics skill with MetaMask Mobile overlay - Introduced a new `analytics` skill that provides a repo-agnostic base and integrates a MetaMask Mobile overlay for the canonical tracking API. This skill is marked as `mandatory: true`, ensuring it installs even when the `coding` domain is filtered out. - Updated documentation in `README.md` to clarify the behavior of `mandatory: true` in relation to domain filtering. --- CHANGELOG.md | 2 + README.md | 3 + .../skills/analytics/repos/metamask-mobile.md | 117 ++++++++++++++++++ domains/coding/skills/analytics/skill.md | 25 ++++ 4 files changed, 147 insertions(+) create mode 100644 domains/coding/skills/analytics/repos/metamask-mobile.md create mode 100644 domains/coding/skills/analytics/skill.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f4e618f..3416f9ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Install a **base skill set by default**. Skills marked `base: true` in frontmatter install on every automatic `postinstall` regardless of domain selection, so a fresh clone lands a useful set with no configuration. `yarn skills` is unchanged and still installs every domain. Opt out with `SKILLS_AUTO_UPDATE=0`. ([#135](https://github.com/MetaMask/skills/pull/135)) - Lint rules for base skills: a `base: true` skill cannot also be `experimental`, and its `description` must be long enough to self-trigger (`BASE_DESCRIPTION_MIN`). Both are errors, so CI fails rather than warning invisibly. ([#135](https://github.com/MetaMask/skills/pull/135)) - Validate `--domain` / `SKILLS_DOMAINS` against the domains that actually exist. A typo previously installed the base set and exited 0, which reads as success. ([#135](https://github.com/MetaMask/skills/pull/135)) +- Add `analytics` skill with a repo-agnostic base and a MetaMask Mobile overlay for the canonical tracking API. Marked `mandatory: true` so it installs even when the `coding` domain is filtered out. + - Add `swaps-cpu-profile-audit` skill for MetaMask Mobile: parse a recorded Hermes / React Native Release Profiler `.cpuprofile` and audit slow frames in swaps/bridge. ([#123](https://github.com/MetaMask/skills/pull/123)) - Add opt-in stale project skill pruning via `--prune-stale` and `SKILLS_PRUNE_STALE=1`. diff --git a/README.md b/README.md index c1608837..d90ddcaf 100644 --- a/README.md +++ b/README.md @@ -386,6 +386,9 @@ Extra metadata blocks (e.g. OpenClaw-style `metadata:` with emoji and homepage) are preserved through install — only `name`, `description`, `maturity`, `base`, and `scope` are read by the CLI. +`mandatory: true` installs the skill even when its domain is filtered out +(`--exclude` / `SKILLS_EXCLUDE` still wins). + The 1,536-character ceiling is a repo budget rather than an operator limit — the description is always-on context for every installed skill, so it is capped deliberately. It is enforced by `yarn audit:skills` from diff --git a/domains/coding/skills/analytics/repos/metamask-mobile.md b/domains/coding/skills/analytics/repos/metamask-mobile.md new file mode 100644 index 00000000..3a41a193 --- /dev/null +++ b/domains/coding/skills/analytics/repos/metamask-mobile.md @@ -0,0 +1,117 @@ +--- +repo: metamask-mobile +parent: analytics +--- + +# Analytics — MetaMask Mobile + +Human-facing file map: `app/core/Analytics/README.md`. + +## Canonical API + +Two emission paths. Use one of them; do not add a third. + +| Role | Path | +|------|------| +| Imperative helper | `app/util/analytics/analytics.ts` → `analytics.trackEvent` | +| Controller messenger | `AnalyticsController:trackEvent` via the Engine / init messenger | +| React hook (same helper) | `app/components/hooks/useAnalytics/useAnalytics.ts` → `useAnalytics` | +| Event builder | `app/util/analytics/AnalyticsEventBuilder.ts` → `AnalyticsEventBuilder.createEventBuilder` | +| Catalog | `app/core/Analytics/` → `EVENT_NAME`, `MetaMetricsEvents` | +| Test factory | `app/util/test/analyticsMock.ts` → `createMockUseAnalyticsHook` | + +`useAnalytics()` is the React face of `analytics`. It returns `trackEvent`, +`createEventBuilder`, `identify`, `enable`, `isEnabled`, `getAnalyticsId`, +and data-deletion helpers. + +Controllers that already talk to Engine should call +`messenger.call('AnalyticsController:trackEvent', event)` with a built event. + +## Require + +- UI: platform `useAnalytics` from `app/components/hooks/useAnalytics/useAnalytics.ts` +- Non-React: `analytics.trackEvent`, or `AnalyticsController:trackEvent` on a messenger +- Event names from `EVENT_NAME` / `MetaMetricsEvents` +- Properties via `.addProperties(...).build()` +- Tests: `createMockUseAnalyticsHook` + +```ts +import { useAnalytics } from '../../hooks/useAnalytics/useAnalytics'; +import { EVENT_NAME } from '../../../core/Analytics'; + +const { trackEvent, createEventBuilder, identify } = useAnalytics(); + +trackEvent( + createEventBuilder(EVENT_NAME.RAMPS_BUTTON_CLICKED) + .addProperties({ location: 'AccountsMenu' }) + .build(), +); + +await identify({ /* traits */ }); +``` + +Non-React: + +```ts +import { analytics } from '../../util/analytics/analytics'; +import { AnalyticsEventBuilder } from '../../util/analytics/AnalyticsEventBuilder'; +import { EVENT_NAME } from '../../core/Analytics'; + +analytics.trackEvent( + AnalyticsEventBuilder.createEventBuilder(EVENT_NAME.APP_OPENED) + .addProperties({ source: 'cold_start' }) + .build(), +); +``` + +Messenger (controllers): + +```ts +initMessenger.call( + 'AnalyticsController:trackEvent', + AnalyticsEventBuilder.createEventBuilder(EVENT_NAME.APP_OPENED) + .addProperties({ source: 'cold_start' }) + .build(), +); +``` + +Prefer `EVENT_NAME.*` strings. `MetaMetricsEvents.*` wrappers (`IMetaMetricsEvent`) +are still valid; `createEventBuilder` copies only `category`. When migrating a +wrapper that used `generateOpt(name, action, description)`, re-apply +`properties.action` and `properties.name` with `addProperties`. + +`generateOpt` belongs in catalog modules: `app/core/Analytics/MetaMetrics.events.ts`, +`app/core/Analytics/events/`, and feature-local `/analytics/events.ts` +(see SampleFeature). Component files import catalog entries; they do not call +`generateOpt` themselves. + +Tests mock the hook with the factory, not a hand-built object: + +```ts +import { useAnalytics } from '../../hooks/useAnalytics/useAnalytics'; +import { createMockUseAnalyticsHook } from '../../../util/test/analyticsMock'; +import { AnalyticsEventBuilder } from '../../../util/analytics/AnalyticsEventBuilder'; + +jest.mock('../../hooks/useAnalytics/useAnalytics'); + +jest.mocked(useAnalytics).mockReturnValue( + createMockUseAnalyticsHook({ + trackEvent: mockTrackEvent, + createEventBuilder: AnalyticsEventBuilder.createEventBuilder, + }), +); +``` + +## Reject + +- `addSensitiveProperties` — deprecated. New tracking uses `addProperties` only. + When editing a call site that already uses `addSensitiveProperties`, stop and + review those fields: drop them, or move them to `addProperties`, whenever + that is safe. Do not add new sensitive properties to an existing event. +- A feature-owned tracking API between the call site and `analytics` / + `AnalyticsController:trackEvent` (a second `useAnalytics`, a typed event + map, an `*Analytics` module, a local `track*` helper). Call the platform + helper or messenger directly. Existing feature APIs stay; do not add another. +- Reintroducing `useMetrics` (removed) or MetaMetrics internals at call sites +- Dropping `generateOpt` `action` / `name` when migrating `IMetaMetricsEvent` call sites (until the catalog migration lands) +- Hand-built `useAnalytics` mock objects — use `createMockUseAnalyticsHook` diff --git a/domains/coding/skills/analytics/skill.md b/domains/coding/skills/analytics/skill.md new file mode 100644 index 00000000..da759641 --- /dev/null +++ b/domains/coding/skills/analytics/skill.md @@ -0,0 +1,25 @@ +--- +name: analytics +description: >- + Product analytics and event tracking. Use when adding, migrating, or + reviewing tracked events, or when writing tests for analytics call sites. +maturity: stable +mandatory: true +--- + +# Analytics + +Use this skill for product event tracking. + +## When to use + +- Adding or migrating event tracking in UI or non-UI code +- Writing or updating tests for analytics call sites +- Reviewing a PR that introduces or changes tracked events + +## Workflow + +1. Pick an event name from the catalog. +2. Attach properties on the event builder. +3. Send the built event through the tracking entry point. +4. In tests, mock the analytics hook with the test factory. From 9eee97f0cd6edbcb3bbc20bcf3e3b70eae82fa1e Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Thu, 3 Sep 2026 11:50:11 +0200 Subject: [PATCH 09/16] docs(analytics): update workflow and testing guidelines for event tracking - Revised the event tracking workflow to clarify the registration process in the catalog, emphasizing the reuse of existing catalog names only for identical interactions. - Enhanced UI testing instructions to specify wrapping `useAnalytics` with the test factory and asserting builder calls in non-React tests. - Updated documentation to reflect these changes and improve clarity on testing practices. --- .../skills/analytics/repos/metamask-mobile.md | 31 +++++++++++++------ domains/coding/skills/analytics/skill.md | 4 +-- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/domains/coding/skills/analytics/repos/metamask-mobile.md b/domains/coding/skills/analytics/repos/metamask-mobile.md index 3a41a193..1566a5ed 100644 --- a/domains/coding/skills/analytics/repos/metamask-mobile.md +++ b/domains/coding/skills/analytics/repos/metamask-mobile.md @@ -31,9 +31,10 @@ Controllers that already talk to Engine should call - UI: platform `useAnalytics` from `app/components/hooks/useAnalytics/useAnalytics.ts` - Non-React: `analytics.trackEvent`, or `AnalyticsController:trackEvent` on a messenger -- Event names from `EVENT_NAME` / `MetaMetricsEvents` +- Event names from `EVENT_NAME` / `MetaMetricsEvents`. New tracking: add the name in catalog modules, then import it. Reuse a catalog name only when this control is the same interaction as existing call sites (same event, same product meaning). - Properties via `.addProperties(...).build()` -- Tests: `createMockUseAnalyticsHook` +- UI tests: `createMockUseAnalyticsHook` wrapping `useAnalytics`, including when the file already mocks the hook +- Non-React tests: assert `AnalyticsEventBuilder.createEventBuilder` and `analytics.trackEvent` or `AnalyticsController:trackEvent` ```ts import { useAnalytics } from '../../hooks/useAnalytics/useAnalytics'; @@ -85,7 +86,14 @@ wrapper that used `generateOpt(name, action, description)`, re-apply (see SampleFeature). Component files import catalog entries; they do not call `generateOpt` themselves. -Tests mock the hook with the factory, not a hand-built object: +Tests mock the hook with the factory, not a hand-built object. +Call `createMockUseAnalyticsHook` again in `beforeEach` after +`jest.clearAllMocks()` / `jest.resetAllMocks()` — those wipe mock +implementations. Prefer `AnalyticsEventBuilder.createEventBuilder`. +When existing assertions inspect a simplified `{ event, properties }` +payload, pass a stub builder into the factory (`createMockEventBuilder` +in `analyticsMock.ts`, or a local stub); still wrap the hook with the +factory. ```ts import { useAnalytics } from '../../hooks/useAnalytics/useAnalytics'; @@ -94,12 +102,15 @@ import { AnalyticsEventBuilder } from '../../../util/analytics/AnalyticsEventBui jest.mock('../../hooks/useAnalytics/useAnalytics'); -jest.mocked(useAnalytics).mockReturnValue( - createMockUseAnalyticsHook({ - trackEvent: mockTrackEvent, - createEventBuilder: AnalyticsEventBuilder.createEventBuilder, - }), -); +beforeEach(() => { + jest.clearAllMocks(); + jest.mocked(useAnalytics).mockReturnValue( + createMockUseAnalyticsHook({ + trackEvent: mockTrackEvent, + createEventBuilder: AnalyticsEventBuilder.createEventBuilder, + }), + ); +}); ``` ## Reject @@ -115,3 +126,5 @@ jest.mocked(useAnalytics).mockReturnValue( - Reintroducing `useMetrics` (removed) or MetaMetrics internals at call sites - Dropping `generateOpt` `action` / `name` when migrating `IMetaMetricsEvent` call sites (until the catalog migration lands) - Hand-built `useAnalytics` mock objects — use `createMockUseAnalyticsHook` +- Attaching a new control to a catalog event whose live call sites are a different product (example: `VIEW_ALL_ASSETS_CLICKED` is wallet tokens/NFTs `asset_type`, not a homepage section) +- Firing an existing catalog event at a new lifecycle (example: `TOKEN_DETECTED` on controller init). Add a catalog name for that lifecycle. diff --git a/domains/coding/skills/analytics/skill.md b/domains/coding/skills/analytics/skill.md index da759641..7f28688c 100644 --- a/domains/coding/skills/analytics/skill.md +++ b/domains/coding/skills/analytics/skill.md @@ -19,7 +19,7 @@ Use this skill for product event tracking. ## Workflow -1. Pick an event name from the catalog. +1. Register this interaction in the catalog (`EVENT_NAME` + `generateOpt` in catalog modules). Reuse an existing catalog name only when this control is another instance of that same interaction (same dashboard event, same owners). 2. Attach properties on the event builder. 3. Send the built event through the tracking entry point. -4. In tests, mock the analytics hook with the test factory. +4. In UI tests, wrap `useAnalytics` with the test factory (including files that already mock the hook). In non-React tests, assert the builder and the helper or messenger call. From 6f7422f2bef2d85a32081f3ecca100b798739590 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Thu, 3 Sep 2026 12:56:47 +0200 Subject: [PATCH 10/16] docs(analytics): clarify emission paths and requirements in tracking API documentation - Updated the documentation to specify the two emission paths for analytics: the `analytics` helper and `AnalyticsController:trackEvent` via `initMessenger`. - Enhanced clarity on the roles of different components in the analytics system, including the distinction between non-React and UI helpers. - Revised the requirements section to reflect the updated paths and usage guidelines for analytics tracking. --- .../skills/analytics/repos/metamask-mobile.md | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/domains/coding/skills/analytics/repos/metamask-mobile.md b/domains/coding/skills/analytics/repos/metamask-mobile.md index 1566a5ed..fcf289e8 100644 --- a/domains/coding/skills/analytics/repos/metamask-mobile.md +++ b/domains/coding/skills/analytics/repos/metamask-mobile.md @@ -9,28 +9,27 @@ Human-facing file map: `app/core/Analytics/README.md`. ## Canonical API -Two emission paths. Use one of them; do not add a third. +Two emission paths: the `analytics` helper, and `AnalyticsController:trackEvent` on the Engine / init messenger. `useAnalytics()` is the UI wrapper around the helper (`analytics.trackEvent`). | Role | Path | |------|------| -| Imperative helper | `app/util/analytics/analytics.ts` → `analytics.trackEvent` | -| Controller messenger | `AnalyticsController:trackEvent` via the Engine / init messenger | -| React hook (same helper) | `app/components/hooks/useAnalytics/useAnalytics.ts` → `useAnalytics` | +| Helper (non-React) | `app/util/analytics/analytics.ts` → `analytics.trackEvent` | +| Helper (UI) | `app/components/hooks/useAnalytics/useAnalytics.ts` → `useAnalytics` | +| Messenger | `AnalyticsController:trackEvent` via `initMessenger.call` | | Event builder | `app/util/analytics/AnalyticsEventBuilder.ts` → `AnalyticsEventBuilder.createEventBuilder` | | Catalog | `app/core/Analytics/` → `EVENT_NAME`, `MetaMetricsEvents` | | Test factory | `app/util/test/analyticsMock.ts` → `createMockUseAnalyticsHook` | -`useAnalytics()` is the React face of `analytics`. It returns `trackEvent`, -`createEventBuilder`, `identify`, `enable`, `isEnabled`, `getAnalyticsId`, -and data-deletion helpers. +`useAnalytics()` returns `trackEvent`, `createEventBuilder`, `identify`, `enable`, +`isEnabled`, `getAnalyticsId`, and data-deletion helpers. Controllers that already talk to Engine should call -`messenger.call('AnalyticsController:trackEvent', event)` with a built event. +`initMessenger.call('AnalyticsController:trackEvent', event)` with a built event. -## Require +## Requirements - UI: platform `useAnalytics` from `app/components/hooks/useAnalytics/useAnalytics.ts` -- Non-React: `analytics.trackEvent`, or `AnalyticsController:trackEvent` on a messenger +- Non-React: `analytics.trackEvent`, or `AnalyticsController:trackEvent` on `initMessenger` - Event names from `EVENT_NAME` / `MetaMetricsEvents`. New tracking: add the name in catalog modules, then import it. Reuse a catalog name only when this control is the same interaction as existing call sites (same event, same product meaning). - Properties via `.addProperties(...).build()` - UI tests: `createMockUseAnalyticsHook` wrapping `useAnalytics`, including when the file already mocks the hook From 3140f968b836fad06a0767b8678bb5010849bfd8 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Thu, 3 Sep 2026 14:47:06 +0200 Subject: [PATCH 11/16] docs(analytics): update CHANGELOG and README for analytics skill and platform domain - Added the `analytics` skill to the CHANGELOG, highlighting its repo-agnostic base and MetaMask Mobile overlay. - Updated the README to include the new `platform` domain, clarifying its purpose for product analytics and platform skills. - Adjusted the `analytics` skill's domain from `coding` to `platform` to better reflect its functionality. --- .github/CODEOWNERS | 1 + CHANGELOG.md | 4 ++-- README.md | 3 ++- .../skills/analytics/repos/metamask-mobile.md | 0 domains/{coding => platform}/skills/analytics/skill.md | 2 +- 5 files changed, 6 insertions(+), 4 deletions(-) rename domains/{coding => platform}/skills/analytics/repos/metamask-mobile.md (100%) rename domains/{coding => platform}/skills/analytics/skill.md (98%) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 20217c32..b610ca32 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -17,6 +17,7 @@ /domains/coding/ @MetaMask/extension-platform @MetaMask/mobile-platform @MetaMask/core-platform /domains/general/ @MetaMask/extension-platform @MetaMask/mobile-platform /domains/performance/ @MetaMask/extension-platform @MetaMask/mobile-platform +/domains/platform/ @MetaMask/extension-platform @MetaMask/mobile-platform @MetaMask/core-platform /domains/perps/ @MetaMask/perps /domains/pr-workflow/ @MetaMask/extension-platform @MetaMask/mobile-platform /domains/swaps/ @MetaMask/swaps-engineers diff --git a/CHANGELOG.md b/CHANGELOG.md index 3416f9ce..e03e218a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,8 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Install a **base skill set by default**. Skills marked `base: true` in frontmatter install on every automatic `postinstall` regardless of domain selection, so a fresh clone lands a useful set with no configuration. `yarn skills` is unchanged and still installs every domain. Opt out with `SKILLS_AUTO_UPDATE=0`. ([#135](https://github.com/MetaMask/skills/pull/135)) - Lint rules for base skills: a `base: true` skill cannot also be `experimental`, and its `description` must be long enough to self-trigger (`BASE_DESCRIPTION_MIN`). Both are errors, so CI fails rather than warning invisibly. ([#135](https://github.com/MetaMask/skills/pull/135)) - Validate `--domain` / `SKILLS_DOMAINS` against the domains that actually exist. A typo previously installed the base set and exited 0, which reads as success. ([#135](https://github.com/MetaMask/skills/pull/135)) -- Add `analytics` skill with a repo-agnostic base and a MetaMask Mobile overlay for the canonical tracking API. Marked `mandatory: true` so it installs even when the `coding` domain is filtered out. - +- Add `analytics` skill 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. - Add `swaps-cpu-profile-audit` skill for MetaMask Mobile: parse a recorded Hermes / React Native Release Profiler `.cpuprofile` and audit slow frames in swaps/bridge. ([#123](https://github.com/MetaMask/skills/pull/123)) - Add opt-in stale project skill pruning via `--prune-stale` and `SKILLS_PRUNE_STALE=1`. @@ -46,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Breaking:** frontmatter key `mandatory` renamed to `base`. Update any skill still using `mandatory:`. ([#135](https://github.com/MetaMask/skills/pull/135)) - **Breaking:** `postinstall` now syncs by default. It previously required `SKILLS_AUTO_UPDATE=1`; the opt-out is now `SKILLS_AUTO_UPDATE=0`. An explicitly empty `SKILLS_AUTO_UPDATE=` keeps its old meaning (off) rather than being read as unset. ([#135](https://github.com/MetaMask/skills/pull/135)) - `base:` truthiness is consistent across all four implementations. The linter alone accepted `on`/`off`, so `base: on` linted clean while the installer skipped the skill. ([#135](https://github.com/MetaMask/skills/pull/135)) +- Move `analytics` from `coding` to a new `platform` domain (`platform/analytics`). - Rewrite `CONTRIBUTING.md` and skill template for MetaMask/skills layout. - `swaps-cpu-profile-audit` now audits the whole capture instead of swaps-owned files only: non-swaps frames that ran while the user was on a swaps screen are classified by their relation to the swaps call stacks (called by swaps, hosts the swaps screen, or concurrent with it), bucketed into named context areas, and reported alongside swaps rows. Every reported row carries an `Owned by swaps` column, and fix depth is gated on it. New `--context-min-pct` and `--swaps-only` analyzer flags. - `swaps-cpu-profile-audit` reports swaps-owned areas, non-swaps areas on the swaps path, and non-swaps areas running concurrently as separate tables, so the per-area swaps detail is no longer diluted by context rows. The swaps table gained an inclusive-time column, and the report explains that self time on a leaf means a screen can trigger heavy work while showing ~0 ms of its own. Non-swaps rows in the fix table are now capped to the few that matter. diff --git a/README.md b/README.md index d90ddcaf..fcbcb780 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,7 @@ tools/ | -------------- | ----------------- | ------------------------------------------- | | `web3-tools` | dApp builders | `gator-cli`, `smart-accounts-kit`, `oh-my-opencode` | | `coding` | MM product eng | Coding guidelines, controller patterns | +| `platform` | MM product eng | Product analytics and other platform skills | | `agentic` | MM product eng | Experimental recipe workflows and runtime proof tools | | `assets` | MM product eng | Assets domain skills | | `general` | All agents | `codex`, `gemini` CLI usage guides | @@ -386,7 +387,7 @@ Extra metadata blocks (e.g. OpenClaw-style `metadata:` with emoji and homepage) are preserved through install — only `name`, `description`, `maturity`, `base`, and `scope` are read by the CLI. -`mandatory: true` installs the skill even when its domain is filtered out +`base: true` installs the skill even when its domain is filtered out (`--exclude` / `SKILLS_EXCLUDE` still wins). The 1,536-character ceiling is a repo budget rather than an operator limit — the diff --git a/domains/coding/skills/analytics/repos/metamask-mobile.md b/domains/platform/skills/analytics/repos/metamask-mobile.md similarity index 100% rename from domains/coding/skills/analytics/repos/metamask-mobile.md rename to domains/platform/skills/analytics/repos/metamask-mobile.md diff --git a/domains/coding/skills/analytics/skill.md b/domains/platform/skills/analytics/skill.md similarity index 98% rename from domains/coding/skills/analytics/skill.md rename to domains/platform/skills/analytics/skill.md index 7f28688c..2febf503 100644 --- a/domains/coding/skills/analytics/skill.md +++ b/domains/platform/skills/analytics/skill.md @@ -4,7 +4,7 @@ description: >- Product analytics and event tracking. Use when adding, migrating, or reviewing tracked events, or when writing tests for analytics call sites. maturity: stable -mandatory: true +base: true --- # Analytics From cda130cd78d30e378c25092a4a64db62e6a2d901 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Fri, 11 Sep 2026 11:13:06 +0200 Subject: [PATCH 12/16] docs(changelog): keep analytics entries under Unreleased after 0.3.1 Co-authored-by: Cursor --- CHANGELOG.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e03e218a..67127913 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `analytics` skill 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. + +### Changed + +- Move `analytics` from `coding` to a new `platform` domain (`platform/analytics`). + ## [0.3.1] ### Fixed @@ -32,7 +40,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Install a **base skill set by default**. Skills marked `base: true` in frontmatter install on every automatic `postinstall` regardless of domain selection, so a fresh clone lands a useful set with no configuration. `yarn skills` is unchanged and still installs every domain. Opt out with `SKILLS_AUTO_UPDATE=0`. ([#135](https://github.com/MetaMask/skills/pull/135)) - Lint rules for base skills: a `base: true` skill cannot also be `experimental`, and its `description` must be long enough to self-trigger (`BASE_DESCRIPTION_MIN`). Both are errors, so CI fails rather than warning invisibly. ([#135](https://github.com/MetaMask/skills/pull/135)) - Validate `--domain` / `SKILLS_DOMAINS` against the domains that actually exist. A typo previously installed the base set and exited 0, which reads as success. ([#135](https://github.com/MetaMask/skills/pull/135)) -- Add `analytics` skill 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. - Add `swaps-cpu-profile-audit` skill for MetaMask Mobile: parse a recorded Hermes / React Native Release Profiler `.cpuprofile` and audit slow frames in swaps/bridge. ([#123](https://github.com/MetaMask/skills/pull/123)) - Add opt-in stale project skill pruning via `--prune-stale` and `SKILLS_PRUNE_STALE=1`. @@ -45,7 +52,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Breaking:** frontmatter key `mandatory` renamed to `base`. Update any skill still using `mandatory:`. ([#135](https://github.com/MetaMask/skills/pull/135)) - **Breaking:** `postinstall` now syncs by default. It previously required `SKILLS_AUTO_UPDATE=1`; the opt-out is now `SKILLS_AUTO_UPDATE=0`. An explicitly empty `SKILLS_AUTO_UPDATE=` keeps its old meaning (off) rather than being read as unset. ([#135](https://github.com/MetaMask/skills/pull/135)) - `base:` truthiness is consistent across all four implementations. The linter alone accepted `on`/`off`, so `base: on` linted clean while the installer skipped the skill. ([#135](https://github.com/MetaMask/skills/pull/135)) -- Move `analytics` from `coding` to a new `platform` domain (`platform/analytics`). - Rewrite `CONTRIBUTING.md` and skill template for MetaMask/skills layout. - `swaps-cpu-profile-audit` now audits the whole capture instead of swaps-owned files only: non-swaps frames that ran while the user was on a swaps screen are classified by their relation to the swaps call stacks (called by swaps, hosts the swaps screen, or concurrent with it), bucketed into named context areas, and reported alongside swaps rows. Every reported row carries an `Owned by swaps` column, and fix depth is gated on it. New `--context-min-pct` and `--swaps-only` analyzer flags. - `swaps-cpu-profile-audit` reports swaps-owned areas, non-swaps areas on the swaps path, and non-swaps areas running concurrently as separate tables, so the per-area swaps detail is no longer diluted by context rows. The swaps table gained an inclusive-time column, and the report explains that self time on a leaf means a screen can trigger heavy work while showing ~0 ms of its own. Non-swaps rows in the fix table are now capped to the few that matter. From 8aeb8e3b18610378908dbe1ed9907cafdebcb388 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Fri, 11 Sep 2026 13:28:38 +0200 Subject: [PATCH 13/16] fix(analytics): align Mobile overlay with live Engine tracking Address review on the analytics skill: App Opened type/source, Engine trackEvent over raw messenger, drop-only sensitive properties, MetaMetricsEvents at existing sites, typed *Tracking helpers, and the test factory default build(). --- CHANGELOG.md | 1 + .../skills/analytics/repos/metamask-mobile.md | 148 ++++++++++++------ domains/platform/skills/analytics/skill.md | 2 +- 3 files changed, 101 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67127913..fe841624 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Move `analytics` from `coding` to a new `platform` domain (`platform/analytics`). +- Align the Mobile analytics overlay with Engine `trackEvent` / `buildAndTrackEvent`, App Opened `type`/`source`, `MetaMetricsEvents` at existing sites, typed `*Tracking.ts` helpers, and the test factory default `build()`. ## [0.3.1] diff --git a/domains/platform/skills/analytics/repos/metamask-mobile.md b/domains/platform/skills/analytics/repos/metamask-mobile.md index fcf289e8..f9033093 100644 --- a/domains/platform/skills/analytics/repos/metamask-mobile.md +++ b/domains/platform/skills/analytics/repos/metamask-mobile.md @@ -5,49 +5,59 @@ parent: analytics # Analytics — MetaMask Mobile -Human-facing file map: `app/core/Analytics/README.md`. +Human-facing file map: `app/core/Analytics/README.md`. A/B enrichment SSOT: `docs/ab-testing.md`. ## Canonical API -Two emission paths: the `analytics` helper, and `AnalyticsController:trackEvent` on the Engine / init messenger. `useAnalytics()` is the UI wrapper around the helper (`analytics.trackEvent`). - | Role | Path | |------|------| | Helper (non-React) | `app/util/analytics/analytics.ts` → `analytics.trackEvent` | | Helper (UI) | `app/components/hooks/useAnalytics/useAnalytics.ts` → `useAnalytics` | -| Messenger | `AnalyticsController:trackEvent` via `initMessenger.call` | +| Engine (controllers) | `app/core/Engine/utils/analytics.ts` → `trackEvent`, `buildAndTrackEvent` | | Event builder | `app/util/analytics/AnalyticsEventBuilder.ts` → `AnalyticsEventBuilder.createEventBuilder` | -| Catalog | `app/core/Analytics/` → `EVENT_NAME`, `MetaMetricsEvents` | -| Test factory | `app/util/test/analyticsMock.ts` → `createMockUseAnalyticsHook` | +| Catalog | `app/core/Analytics/` → `MetaMetricsEvents` (existing sites), `EVENT_NAME` (new catalog names) | +| Typed helpers | `app/util/analytics/actionButtonTracking.ts` (and sibling `*Tracking.ts` files) | +| Test factory | `app/util/test/analyticsMock.ts` → `createMockUseAnalyticsHook`, `createMockEventBuilder` | `useAnalytics()` returns `trackEvent`, `createEventBuilder`, `identify`, `enable`, `isEnabled`, `getAnalyticsId`, and data-deletion helpers. -Controllers that already talk to Engine should call -`initMessenger.call('AnalyticsController:trackEvent', event)` with a built event. +Controllers that already talk to Engine use `trackEvent` / `buildAndTrackEvent` +from `app/core/Engine/utils/analytics.ts` (A/B enrichment + try/catch). Raw +`initMessenger.call('AnalyticsController:trackEvent', …)` skips enrichment: +attach `active_ab_tests` with `createActiveABTestAssignment()` from +`app/util/analytics/activeABTestAssignments.ts`, and keep the Engine-util cast. + +`createMockEventBuilder()` default `build()` is +`{ name: 'mock-event', properties: {}, sensitiveProperties: {} }`. ## Requirements - UI: platform `useAnalytics` from `app/components/hooks/useAnalytics/useAnalytics.ts` -- Non-React: `analytics.trackEvent`, or `AnalyticsController:trackEvent` on `initMessenger` -- Event names from `EVENT_NAME` / `MetaMetricsEvents`. New tracking: add the name in catalog modules, then import it. Reuse a catalog name only when this control is the same interaction as existing call sites (same event, same product meaning). +- Non-React: `analytics.trackEvent` +- Controllers: `trackEvent` / `buildAndTrackEvent` from `app/core/Engine/utils/analytics.ts` +- When a typed helper exists in `app/util/analytics/` (`*Tracking.ts`) for this event, call it (do not invent a new feature-local layer) +- Existing call sites keep `MetaMetricsEvents.*`. `EVENT_NAME.*` is for brand-new catalog names. New tracking: add the name in catalog modules, then import it. Reuse a catalog name only when this control is the same interaction as existing call sites (same event, same product meaning). - Properties via `.addProperties(...).build()` -- UI tests: `createMockUseAnalyticsHook` wrapping `useAnalytics`, including when the file already mocks the hook -- Non-React tests: assert `AnalyticsEventBuilder.createEventBuilder` and `analytics.trackEvent` or `AnalyticsController:trackEvent` +- UI tests: `createMockUseAnalyticsHook` wrapping `useAnalytics`, including when the file already mocks the hook; `createEventBuilder: jest.fn(() => createMockEventBuilder())` +- Non-React tests: assert `AnalyticsEventBuilder.createEventBuilder` and `analytics.trackEvent` or Engine `trackEvent` / `buildAndTrackEvent` ```ts import { useAnalytics } from '../../hooks/useAnalytics/useAnalytics'; -import { EVENT_NAME } from '../../../core/Analytics'; - -const { trackEvent, createEventBuilder, identify } = useAnalytics(); - -trackEvent( - createEventBuilder(EVENT_NAME.RAMPS_BUTTON_CLICKED) - .addProperties({ location: 'AccountsMenu' }) - .build(), -); - -await identify({ /* traits */ }); +import { + ActionButtonType, + ActionLocation, + trackActionButtonClick, +} from '../../../../util/analytics/actionButtonTracking'; + +const { trackEvent, createEventBuilder } = useAnalytics(); + +trackActionButtonClick(trackEvent, createEventBuilder, { + action_name: ActionButtonType.SEND, + action_position: actionPosition, + button_label: label, + location: ActionLocation.HOME, +}); ``` Non-React: @@ -55,29 +65,66 @@ Non-React: ```ts import { analytics } from '../../util/analytics/analytics'; import { AnalyticsEventBuilder } from '../../util/analytics/AnalyticsEventBuilder'; -import { EVENT_NAME } from '../../core/Analytics'; +import { MetaMetricsEvents } from '../../core/Analytics'; analytics.trackEvent( - AnalyticsEventBuilder.createEventBuilder(EVENT_NAME.APP_OPENED) - .addProperties({ source: 'cold_start' }) + AnalyticsEventBuilder.createEventBuilder(MetaMetricsEvents.APP_OPENED) + .addProperties({ type: 'cold_start', source: 'direct' }) .build(), ); ``` -Messenger (controllers): +Controllers: ```ts -initMessenger.call( +import { buildAndTrackEvent } from '../../core/Engine/utils/analytics'; +import { MetaMetricsEvents } from '../../core/Analytics'; + +buildAndTrackEvent( + initMessenger, + MetaMetricsEvents.PROFILE_ACTIVITY_UPDATED.category, + { + profile_id: profileId, + feature_name: 'Contacts Sync', + action: 'Contacts Sync Contact Updated', + }, +); +``` + +Messenger escape hatch (skips Engine-util enrichment): + +```ts +import type { AnalyticsTrackingEvent as PackageAnalyticsTrackingEvent } from '@metamask/analytics-controller'; +import { createActiveABTestAssignment } from '../../util/analytics/activeABTestAssignments'; +import { AnalyticsEventBuilder } from '../../util/analytics/AnalyticsEventBuilder'; +import { MetaMetricsEvents } from '../../core/Analytics'; + +const event = AnalyticsEventBuilder.createEventBuilder( + MetaMetricsEvents.APP_OPENED, +) + .addProperties({ + type: 'cold_start', + source: 'direct', + active_ab_tests: [createActiveABTestAssignment('flagKey', 'treatment')], + }) + .build(); + +// Cast needed until @metamask/analytics-controller removes saveDataRecording from its AnalyticsTrackingEvent +( + initMessenger as typeof initMessenger & { + call: ( + action: 'AnalyticsController:trackEvent', + event: PackageAnalyticsTrackingEvent, + ) => void; + } +).call( 'AnalyticsController:trackEvent', - AnalyticsEventBuilder.createEventBuilder(EVENT_NAME.APP_OPENED) - .addProperties({ source: 'cold_start' }) - .build(), + event as unknown as PackageAnalyticsTrackingEvent, ); ``` -Prefer `EVENT_NAME.*` strings. `MetaMetricsEvents.*` wrappers (`IMetaMetricsEvent`) -are still valid; `createEventBuilder` copies only `category`. When migrating a -wrapper that used `generateOpt(name, action, description)`, re-apply +`createEventBuilder` copies only `category` from `IMetaMetricsEvent`. When +migrating a wrapper that used `generateOpt(name, action, description)`, re-apply `properties.action` and `properties.name` with `addProperties`. `generateOpt` belongs in catalog modules: `app/core/Analytics/MetaMetrics.events.ts`, @@ -88,16 +135,14 @@ wrapper that used `generateOpt(name, action, description)`, re-apply Tests mock the hook with the factory, not a hand-built object. Call `createMockUseAnalyticsHook` again in `beforeEach` after `jest.clearAllMocks()` / `jest.resetAllMocks()` — those wipe mock -implementations. Prefer `AnalyticsEventBuilder.createEventBuilder`. -When existing assertions inspect a simplified `{ event, properties }` -payload, pass a stub builder into the factory (`createMockEventBuilder` -in `analyticsMock.ts`, or a local stub); still wrap the hook with the -factory. +implementations. ```ts import { useAnalytics } from '../../hooks/useAnalytics/useAnalytics'; -import { createMockUseAnalyticsHook } from '../../../util/test/analyticsMock'; -import { AnalyticsEventBuilder } from '../../../util/analytics/AnalyticsEventBuilder'; +import { + createMockUseAnalyticsHook, + createMockEventBuilder, +} from '../../../util/test/analyticsMock'; jest.mock('../../hooks/useAnalytics/useAnalytics'); @@ -106,7 +151,7 @@ beforeEach(() => { jest.mocked(useAnalytics).mockReturnValue( createMockUseAnalyticsHook({ trackEvent: mockTrackEvent, - createEventBuilder: AnalyticsEventBuilder.createEventBuilder, + createEventBuilder: jest.fn(() => createMockEventBuilder()), }), ); }); @@ -114,16 +159,21 @@ beforeEach(() => { ## Reject -- `addSensitiveProperties` — deprecated. New tracking uses `addProperties` only. - When editing a call site that already uses `addSensitiveProperties`, stop and - review those fields: drop them, or move them to `addProperties`, whenever - that is safe. Do not add new sensitive properties to an existing event. +- `addSensitiveProperties` on new tracking. Existing call sites: drop those + fields only. Moving the last sensitive field into `addProperties` flips + `isAnonymous` (true iff `sensitiveProperties` is nonempty). Do not relocate + without human sign-off. - A feature-owned tracking API between the call site and `analytics` / - `AnalyticsController:trackEvent` (a second `useAnalytics`, a typed event - map, an `*Analytics` module, a local `track*` helper). Call the platform - helper or messenger directly. Existing feature APIs stay; do not add another. + Engine `trackEvent` (a second `useAnalytics`, a typed event map, an + `*Analytics` module, a local `track*` helper). Files matching `*Tracking.ts` + under `app/util/analytics/` are the platform typed-helper layer — use them; + do not add another feature-local one. Existing feature APIs stay. +- Replacing `MetaMetricsEvents.*` at an existing call site with `EVENT_NAME.*` + unless that site is taking a brand-new catalog name - Reintroducing `useMetrics` (removed) or MetaMetrics internals at call sites - Dropping `generateOpt` `action` / `name` when migrating `IMetaMetricsEvent` call sites (until the catalog migration lands) - Hand-built `useAnalytics` mock objects — use `createMockUseAnalyticsHook` +- Raw `initMessenger.call('AnalyticsController:trackEvent', …)` when Engine + `trackEvent` / `buildAndTrackEvent` is available (skips A/B enrichment) - Attaching a new control to a catalog event whose live call sites are a different product (example: `VIEW_ALL_ASSETS_CLICKED` is wallet tokens/NFTs `asset_type`, not a homepage section) - Firing an existing catalog event at a new lifecycle (example: `TOKEN_DETECTED` on controller init). Add a catalog name for that lifecycle. diff --git a/domains/platform/skills/analytics/skill.md b/domains/platform/skills/analytics/skill.md index 2febf503..b1149b43 100644 --- a/domains/platform/skills/analytics/skill.md +++ b/domains/platform/skills/analytics/skill.md @@ -22,4 +22,4 @@ Use this skill for product event tracking. 1. Register this interaction in the catalog (`EVENT_NAME` + `generateOpt` in catalog modules). Reuse an existing catalog name only when this control is another instance of that same interaction (same dashboard event, same owners). 2. Attach properties on the event builder. 3. Send the built event through the tracking entry point. -4. In UI tests, wrap `useAnalytics` with the test factory (including files that already mock the hook). In non-React tests, assert the builder and the helper or messenger call. +4. In UI tests, wrap `useAnalytics` with the test factory (including files that already mock the hook). In non-React tests, assert the builder and the helper or Engine tracking util. From b08eed3285a7ad82781ade4889c7153b0fb0c3fd Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Mon, 14 Sep 2026 13:26:24 +0200 Subject: [PATCH 14/16] fix(analytics): copy live Mobile fences and drop messenger A/B hatch Ground UI and non-React examples on one live file each, qualify Engine A/B enrichment, and document base:true install caveats. --- CHANGELOG.md | 7 +- README.md | 7 +- .../skills/analytics/repos/metamask-mobile.md | 167 ++++++++++-------- 3 files changed, 97 insertions(+), 84 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe841624..4404ecac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,12 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Add `analytics` skill 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. - -### Changed - -- Move `analytics` from `coding` to a new `platform` domain (`platform/analytics`). -- Align the Mobile analytics overlay with Engine `trackEvent` / `buildAndTrackEvent`, App Opened `type`/`source`, `MetaMetricsEvents` at existing sites, typed `*Tracking.ts` helpers, and the test factory default `build()`. +- 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)) ## [0.3.1] diff --git a/README.md b/README.md index fcbcb780..1e82a148 100644 --- a/README.md +++ b/README.md @@ -387,8 +387,11 @@ Extra metadata blocks (e.g. OpenClaw-style `metadata:` with emoji and homepage) are preserved through install — only `name`, `description`, `maturity`, `base`, and `scope` are read by the CLI. -`base: true` installs the skill even when its domain is filtered out -(`--exclude` / `SKILLS_EXCLUDE` still wins). +`base: true` installs the skill even when its domain is filtered out. +`--exclude` / `SKILLS_EXCLUDE` still wins. The maturity filter runs before the +base bypass, so `--maturity stable` drops a `base: true` experimental skill. +A skill with a `repos/` directory and no overlay for `--repo` is skipped +(this `analytics` skill installs for Mobile and is skipped for Extension). The 1,536-character ceiling is a repo budget rather than an operator limit — the description is always-on context for every installed skill, so it is capped diff --git a/domains/platform/skills/analytics/repos/metamask-mobile.md b/domains/platform/skills/analytics/repos/metamask-mobile.md index f9033093..68462864 100644 --- a/domains/platform/skills/analytics/repos/metamask-mobile.md +++ b/domains/platform/skills/analytics/repos/metamask-mobile.md @@ -15,61 +15,106 @@ Human-facing file map: `app/core/Analytics/README.md`. A/B enrichment SSOT: `doc | Helper (UI) | `app/components/hooks/useAnalytics/useAnalytics.ts` → `useAnalytics` | | Engine (controllers) | `app/core/Engine/utils/analytics.ts` → `trackEvent`, `buildAndTrackEvent` | | Event builder | `app/util/analytics/AnalyticsEventBuilder.ts` → `AnalyticsEventBuilder.createEventBuilder` | -| Catalog | `app/core/Analytics/` → `MetaMetricsEvents` (existing sites), `EVENT_NAME` (new catalog names) | -| Typed helpers | `app/util/analytics/actionButtonTracking.ts` (and sibling `*Tracking.ts` files) | -| Test factory | `app/util/test/analyticsMock.ts` → `createMockUseAnalyticsHook`, `createMockEventBuilder` | +| Catalog | `app/core/Analytics/` → `MetaMetricsEvents` at call sites; `EVENT_NAME` in catalog modules | +| Typed helpers | `app/util/analytics/actionButtonTracking.ts` (sibling files matching `*Tracking.ts`) | +| A/B registry | `app/util/analytics/abTestAnalyticsRegistry.ts` (feature-local `abTestConfig.ts`, e.g. `app/components/Views/Homepage/abTestConfig.ts`) | +| Test factory | `app/util/test/analyticsMock.ts` → `createMockUseAnalyticsHook` (default); `createMockEventBuilder` (optional standalone double) | `useAnalytics()` returns `trackEvent`, `createEventBuilder`, `identify`, `enable`, `isEnabled`, `getAnalyticsId`, and data-deletion helpers. -Controllers that already talk to Engine use `trackEvent` / `buildAndTrackEvent` -from `app/core/Engine/utils/analytics.ts` (A/B enrichment + try/catch). Raw -`initMessenger.call('AnalyticsController:trackEvent', …)` skips enrichment: -attach `active_ab_tests` with `createActiveABTestAssignment()` from -`app/util/analytics/activeABTestAssignments.ts`, and keep the Engine-util cast. +Controllers that already talk to Engine import `trackEvent` / `buildAndTrackEvent` +from `app/core/Engine/utils/analytics.ts`. Those helpers always wrap the +messenger call in try/catch. `enrichWithABTests` runs only when the event name +is registered in `app/util/analytics/abTestAnalyticsRegistry.ts` (fed by +feature-local `abTestConfig.ts`). New experiment events: follow +`docs/ab-testing.md`. Do not copy Engine-util internals. `createMockEventBuilder()` default `build()` is -`{ name: 'mock-event', properties: {}, sensitiveProperties: {} }`. +`{ name: 'mock-event', properties: {}, sensitiveProperties: {} }`. Use it only +as a standalone builder double, wrapped in `jest.fn(() => createMockEventBuilder())`. ## Requirements - UI: platform `useAnalytics` from `app/components/hooks/useAnalytics/useAnalytics.ts` - Non-React: `analytics.trackEvent` - Controllers: `trackEvent` / `buildAndTrackEvent` from `app/core/Engine/utils/analytics.ts` -- When a typed helper exists in `app/util/analytics/` (`*Tracking.ts`) for this event, call it (do not invent a new feature-local layer) -- Existing call sites keep `MetaMetricsEvents.*`. `EVENT_NAME.*` is for brand-new catalog names. New tracking: add the name in catalog modules, then import it. Reuse a catalog name only when this control is the same interaction as existing call sites (same event, same product meaning). +- When a typed helper exists in `app/util/analytics/` (files matching `*Tracking.ts`) for this event, call it +- Call sites (new and existing) import `MetaMetricsEvents.*`. Register new names as `EVENT_NAME` + `generateOpt` in catalog modules, then emit via `MetaMetricsEvents`. Reuse a catalog name only when this control is the same interaction as existing call sites (same event, same product meaning). - Properties via `.addProperties(...).build()` -- UI tests: `createMockUseAnalyticsHook` wrapping `useAnalytics`, including when the file already mocks the hook; `createEventBuilder: jest.fn(() => createMockEventBuilder())` +- UI tests: `createMockUseAnalyticsHook` wrapping `useAnalytics`, including when the file already mocks the hook. Default: `createMockUseAnalyticsHook({ trackEvent: mockTrackEvent })`. Tests that assert `addProperties` keep `AnalyticsEventBuilder.createEventBuilder` - Non-React tests: assert `AnalyticsEventBuilder.createEventBuilder` and `analytics.trackEvent` or Engine `trackEvent` / `buildAndTrackEvent` +Generic UI (`app/components/UI/BalanceEmptyState/BalanceEmptyState.tsx`): + ```ts +import React from 'react'; +import { MetaMetricsEvents } from '../../../core/Analytics'; import { useAnalytics } from '../../hooks/useAnalytics/useAnalytics'; + +const BalanceEmptyState: React.FC = ({ + testID = 'balance-empty-state', + ...props +}) => { + const { trackEvent, createEventBuilder } = useAnalytics(); + + const handleAction = () => { + trackEvent( + createEventBuilder(MetaMetricsEvents.RAMPS_BUTTON_CLICKED) + .addProperties({ + button_text: 'Add funds', + location: 'BalanceEmptyState', + ramp_type: 'UNIFIED_BUY_2', + }) + .build(), + ); + }; +``` + +Typed helper (`app/components/Views/Homepage/components/HomepageActionButtonsGrid/buttons/SendButton.tsx`): + +```ts +import React, { useCallback } from 'react'; +import { useAnalytics } from '../../../../../hooks/useAnalytics/useAnalytics'; import { ActionButtonType, ActionLocation, trackActionButtonClick, -} from '../../../../util/analytics/actionButtonTracking'; - -const { trackEvent, createEventBuilder } = useAnalytics(); - -trackActionButtonClick(trackEvent, createEventBuilder, { - action_name: ActionButtonType.SEND, - action_position: actionPosition, - button_label: label, - location: ActionLocation.HOME, -}); +} from '../../../../../../util/analytics/actionButtonTracking'; + +const SendButton = ({ + actionPosition, + allowTwoLineLabel, + onSend, +}: SendButtonProps) => { + const { trackEvent, createEventBuilder } = useAnalytics(); + + const handlePress = useCallback(() => { + trackActionButtonClick(trackEvent, createEventBuilder, { + action_name: ActionButtonType.SEND, + action_position: actionPosition, + button_label: label, + location: ActionLocation.HOME, + }); + onSend(); + }, [actionPosition, createEventBuilder, label, onSend, trackEvent]); ``` -Non-React: +Non-React (`app/util/analytics/accountAccessTracking.ts`): ```ts -import { analytics } from '../../util/analytics/analytics'; -import { AnalyticsEventBuilder } from '../../util/analytics/AnalyticsEventBuilder'; -import { MetaMetricsEvents } from '../../core/Analytics'; +import { MetaMetricsEvents } from '../../core/Analytics/MetaMetrics.events'; +import { analytics } from './analytics'; +import { AnalyticsEventBuilder } from './AnalyticsEventBuilder'; analytics.trackEvent( - AnalyticsEventBuilder.createEventBuilder(MetaMetricsEvents.APP_OPENED) - .addProperties({ type: 'cold_start', source: 'direct' }) + AnalyticsEventBuilder.createEventBuilder( + MetaMetricsEvents.APP_UNLOCKED_FAILED, + ) + .addProperties({ + unlock_error_type: unlockErrorType, + forced_reset: forcedReset, + }) .build(), ); ``` @@ -91,38 +136,6 @@ buildAndTrackEvent( ); ``` -Messenger escape hatch (skips Engine-util enrichment): - -```ts -import type { AnalyticsTrackingEvent as PackageAnalyticsTrackingEvent } from '@metamask/analytics-controller'; -import { createActiveABTestAssignment } from '../../util/analytics/activeABTestAssignments'; -import { AnalyticsEventBuilder } from '../../util/analytics/AnalyticsEventBuilder'; -import { MetaMetricsEvents } from '../../core/Analytics'; - -const event = AnalyticsEventBuilder.createEventBuilder( - MetaMetricsEvents.APP_OPENED, -) - .addProperties({ - type: 'cold_start', - source: 'direct', - active_ab_tests: [createActiveABTestAssignment('flagKey', 'treatment')], - }) - .build(); - -// Cast needed until @metamask/analytics-controller removes saveDataRecording from its AnalyticsTrackingEvent -( - initMessenger as typeof initMessenger & { - call: ( - action: 'AnalyticsController:trackEvent', - event: PackageAnalyticsTrackingEvent, - ) => void; - } -).call( - 'AnalyticsController:trackEvent', - event as unknown as PackageAnalyticsTrackingEvent, -); -``` - `createEventBuilder` copies only `category` from `IMetaMetricsEvent`. When migrating a wrapper that used `generateOpt(name, action, description)`, re-apply `properties.action` and `properties.name` with `addProperties`. @@ -134,46 +147,48 @@ migrating a wrapper that used `generateOpt(name, action, description)`, re-apply Tests mock the hook with the factory, not a hand-built object. Call `createMockUseAnalyticsHook` again in `beforeEach` after -`jest.clearAllMocks()` / `jest.resetAllMocks()` — those wipe mock -implementations. +`jest.resetAllMocks()` — that wipes mock implementations. `jest.clearAllMocks()` +does not. ```ts import { useAnalytics } from '../../hooks/useAnalytics/useAnalytics'; -import { - createMockUseAnalyticsHook, - createMockEventBuilder, -} from '../../../util/test/analyticsMock'; +import { createMockUseAnalyticsHook } from '../../../util/test/analyticsMock'; jest.mock('../../hooks/useAnalytics/useAnalytics'); beforeEach(() => { - jest.clearAllMocks(); + jest.resetAllMocks(); jest.mocked(useAnalytics).mockReturnValue( createMockUseAnalyticsHook({ trackEvent: mockTrackEvent, - createEventBuilder: jest.fn(() => createMockEventBuilder()), }), ); }); ``` +Standalone builder double (only when the test needs one): + +```ts +createEventBuilder: jest.fn(() => createMockEventBuilder()), +``` + ## Reject - `addSensitiveProperties` on new tracking. Existing call sites: drop those fields only. Moving the last sensitive field into `addProperties` flips `isAnonymous` (true iff `sensitiveProperties` is nonempty). Do not relocate without human sign-off. -- A feature-owned tracking API between the call site and `analytics` / - Engine `trackEvent` (a second `useAnalytics`, a typed event map, an - `*Analytics` module, a local `track*` helper). Files matching `*Tracking.ts` - under `app/util/analytics/` are the platform typed-helper layer — use them; - do not add another feature-local one. Existing feature APIs stay. -- Replacing `MetaMetricsEvents.*` at an existing call site with `EVENT_NAME.*` - unless that site is taking a brand-new catalog name +- A new feature-local tracker that is not a file matching `*Tracking.ts` under + `app/util/analytics/`, a feature-local `abTestConfig.ts`, or a catalog + `generateOpt` module (`app/core/Analytics/MetaMetrics.events.ts`, + `app/core/Analytics/events/`, `/analytics/events.ts`) +- Replacing `MetaMetricsEvents.*` at a call site with `EVENT_NAME.*` - Reintroducing `useMetrics` (removed) or MetaMetrics internals at call sites - Dropping `generateOpt` `action` / `name` when migrating `IMetaMetricsEvent` call sites (until the catalog migration lands) - Hand-built `useAnalytics` mock objects — use `createMockUseAnalyticsHook` - Raw `initMessenger.call('AnalyticsController:trackEvent', …)` when Engine - `trackEvent` / `buildAndTrackEvent` is available (skips A/B enrichment) + `trackEvent` / `buildAndTrackEvent` is available +- Defaulting UI tests to `createEventBuilder: jest.fn(() => createMockEventBuilder())` + when `createMockUseAnalyticsHook()` already stubs the builder - Attaching a new control to a catalog event whose live call sites are a different product (example: `VIEW_ALL_ASSETS_CLICKED` is wallet tokens/NFTs `asset_type`, not a homepage section) - Firing an existing catalog event at a new lifecycle (example: `TOKEN_DETECTED` on controller init). Add a catalog name for that lifecycle. From 9790948733e33a3aad8125fa6d4bb3a1857f5b07 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Fri, 4 Sep 2026 11:18:25 +0200 Subject: [PATCH 15/16] feat(platform): add feature-flags skill with MetaMask Mobile overlay --- CHANGELOG.md | 1 + .../feature-flags/repos/metamask-mobile.md | 71 +++++++++++++++++++ .../platform/skills/feature-flags/skill.md | 27 +++++++ 3 files changed, 99 insertions(+) create mode 100644 domains/platform/skills/feature-flags/repos/metamask-mobile.md create mode 100644 domains/platform/skills/feature-flags/skill.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 4404ecac..ffb60f88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- 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)) ## [0.3.1] diff --git a/domains/platform/skills/feature-flags/repos/metamask-mobile.md b/domains/platform/skills/feature-flags/repos/metamask-mobile.md new file mode 100644 index 00000000..2fe7f377 --- /dev/null +++ b/domains/platform/skills/feature-flags/repos/metamask-mobile.md @@ -0,0 +1,71 @@ +--- +repo: metamask-mobile +parent: feature-flags +--- + +# Feature flags — MetaMask Mobile + +Human-facing file map: `docs/readme/version-gated-feature-flags.md`. + +## Canonical API + +| Role | Path | +|------|------| +| Helper | `app/util/remoteFeatureFlag/index.ts` → `validatedVersionGatedFeatureFlag`, `hasMinimumRequiredVersion` | +| Raw flags | `RemoteFeatureFlagController.remoteFeatureFlags` (no client version gating) | +| Selectors | `app/selectors/featureFlagController/` or `**/selectors/featureFlags/` | +| Names (when the flag is overrideable in dev tools) | `app/constants/featureFlags.ts` → `FeatureFlagNames` | + +`validatedVersionGatedFeatureFlag` compares against the native binary version from `react-native-device-info` `getVersion()`, not `package.json`. Progressive-rollout wrappers `{ name, value: { enabled, minimumVersion } }` are unwrapped in the helper. + +`hasMinimumRequiredVersion` is the lower-level compare used inside the helper. Import it from the util only for a standalone version check. + +Multi-version flags shaped `{ versions: { "7.53.0": value } }` are resolved at fetch time by the controller. Consumers read the processed value — no helper call for that shape. + +## Requirements + +- Selectors call `validatedVersionGatedFeatureFlag`. UI and hooks only `useSelector(selectXEnabled)`. +- Non-standard flag shapes (e.g. `active` instead of `enabled`) map to `{ enabled, minimumVersion }` before the helper. +- Remote flag wins when valid. Use `?? env/local` when the helper returns `undefined` (invalid shape, or `OVERRIDE_REMOTE_FEATURE_FLAGS=true`). + +```ts +import { createSelector } from 'reselect'; +import { selectRemoteFeatureFlags } from '../index'; +import { + validatedVersionGatedFeatureFlag, + type VersionGatedFeatureFlag, +} from '../../../util/remoteFeatureFlag'; + +export const selectMyFeatureEnabled = createSelector( + selectRemoteFeatureFlags, + (remoteFeatureFlags) => { + const localFlag = process.env.MM_MY_FEATURE_ENABLED === 'true'; + const remoteFlag = + remoteFeatureFlags?.myFeature as unknown as VersionGatedFeatureFlag; + + return validatedVersionGatedFeatureFlag(remoteFlag) ?? localFlag; + }, +); +``` + +Non-standard shape: + +```ts +validatedVersionGatedFeatureFlag({ + enabled: depositConfig.active ?? false, + minimumVersion: depositConfig.minimumVersion ?? '', +}) ?? false; +``` + +UI: + +```ts +const isEnabled = useSelector(selectMyFeatureEnabled); +``` + +## Reject + +- Local copies of `hasMinimumRequiredVersion` or `validatedVersionGatedFeatureFlag` +- Inline `compare-versions` + `getVersion` for feature-flag gating outside `app/util/remoteFeatureFlag` +- Version checks in hooks or components (`getVersion()`, `compare-versions`, or a local helper) +- Duplicate util files under `app/components/UI/**/utils/` or `app/core/redux/slices/**/` diff --git a/domains/platform/skills/feature-flags/skill.md b/domains/platform/skills/feature-flags/skill.md new file mode 100644 index 00000000..6f230054 --- /dev/null +++ b/domains/platform/skills/feature-flags/skill.md @@ -0,0 +1,27 @@ +--- +name: feature-flags +description: >- + Version-gated remote feature flags. Use when adding, migrating, or + reviewing a remote or version-gated feature flag, writing a flag + selector, or gating UI on a flag boolean. +maturity: stable +base: true +--- + +# Feature flags + +Use this skill for remote and version-gated feature flags. + +## When to use + +- Adding or migrating a remote / version-gated feature flag +- Writing or updating a flag selector +- Gating UI or hooks on a flag boolean +- Reviewing a PR that introduces or changes flag evaluation + +## Workflow + +1. Evaluate the flag in a selector. Return a boolean. +2. Map a non-standard remote shape to `{ enabled, minimumVersion }` before the shared helper. +3. Consume that boolean in UI or hooks. Do not re-run version math there. +4. Cover the selector with collocated tests (enabled, disabled, invalid, fallback). From bb1173bb9d065f2e7513cf4187204756e8b4001b Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 14 Sep 2026 08:57:55 -0400 Subject: [PATCH 16/16] Say which users never fetch flags, and how to test the fetch path in E2E A flag reaches only builds whose code reads it, and the controller is off until onboarding completes and while basic functionality is off. Seeded fixture state skips fetching and validating the response, which `testSpecificMock` exercises, and the registry drift check runs weekly, not on pull requests. --- .../feature-flags/repos/metamask-extension.md | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/domains/platform/skills/feature-flags/repos/metamask-extension.md b/domains/platform/skills/feature-flags/repos/metamask-extension.md index df3d2fe6..b137978f 100644 --- a/domains/platform/skills/feature-flags/repos/metamask-extension.md +++ b/domains/platform/skills/feature-flags/repos/metamask-extension.md @@ -23,7 +23,9 @@ gating. `app/util/remoteFeatureFlag/index.ts` and its own header says so, so the exported names match mobile. The evaluation differs in one respect that matters: `hasMinimumRequiredVersion` compares against `packageJson.version`, read from the repo's `package.json` at build time. -There is no native binary version and no `react-native-device-info` equivalent. +There is no native binary version and no `react-native-device-info` equivalent. A flag change +reaches only installs running a build whose code reads the flag, so it cannot change behavior +on an older build. `getBooleanFeatureFlag(flagValue, defaultValue)` is the usual entry point, at 34 non-test call sites against 11 for `validatedVersionGatedFeatureFlag`. It takes the fallback as a required @@ -79,6 +81,10 @@ export function isMyFeatureEnabled( } ``` +Background code reads the bag through the `RemoteFeatureFlagController:getState` messenger +action. Until this session's fetch completes, that state holds the flags persisted from the +previous fetch. + The selector then composes that predicate over `getRemoteFeatureFlags`, which keeps one version-gate interpretation shared between the UI and the background: @@ -133,10 +139,18 @@ import packageJson from '../../../package.json'; const CURRENT_VERSION = packageJson.version; ``` -Cover enabled, disabled, below-minimum, rollout-wrapped, and invalid or absent. For E2E, seed -state with `withRemoteFeatureFlagController(...)` from +Cover enabled, disabled, below-minimum, rollout-wrapped, and invalid or absent. +`RemoteFeatureFlagController` is disabled until onboarding completes and while basic +functionality (`useExternalServices`) is off, so those users never fetch and read absent or +last-persisted flags. A failed fetch is only logged, in +`app/scripts/lib/update-remote-feature-flags.ts`. + +For E2E, seed state with `withRemoteFeatureFlagController(...)` from `test/e2e/fixtures/fixture-builder-v2.ts`, or override at runtime with -`manifestFlags.remoteFeatureFlags`. +`manifestFlags.remoteFeatureFlags`. Seeded state never passes through fetching and validating +a flag response. To test that path, serve the response from `testSpecificMock`, which +`test/e2e/mock-e2e.js` registers ahead of its registry default for the `client-config` flags +route. ## Not present in the extension @@ -154,7 +168,9 @@ State these as gaps rather than substituting a near neighbor: ## Extension only - `FEATURE_FLAG_REGISTRY` is the production-default source of truth for E2E. Mobile has no - counterpart. + counterpart. `.github/workflows/check-feature-flag-registry-drift.yml` checks it against + production on a weekly schedule (`cron: '0 1 * * 2'`) and not on pull requests, so a new + flag's `productionDefault` can merge unchecked. - `getRemoteFeatureFlags` folds manifest overrides in at the selector, so override precedence is a property of the read rather than of a build switch. - Threshold and A/B flags carry a separate `featureFlagThresholdGroups` map, read through