feat(stability): add memory-leak skill

.github/CODEOWNERS

@@ -18,6 +18,7 @@
 /domains/performance/    @MetaMask/extension-platform @MetaMask/mobile-platform
 /domains/perps/          @MetaMask/perps
 /domains/pr-workflow/    @MetaMask/extension-platform @MetaMask/mobile-platform
+/domains/stability/      @MetaMask/extension-platform @MetaMask/mobile-platform
 /domains/swaps/          @MetaMask/swaps-engineers
 /domains/testing/        @MetaMask/qa
 /domains/ui/             @MetaMask/design-system-engineers

domains/stability/skills/memory-leak/references/heap-investigation.md

@@ -0,0 +1,67 @@
+# Phase 2 — runtime investigation
+
+Reach for this **only** when Phase 1 (static pairing) leaves an introduced primitive it
+cannot pair, or when the claim is explicitly about magnitude ("retained heap grows across N
+cycles", "detached nodes accumulate"). A snapshot confirms a *suspected* leak and shows its size and retainer chain — but a
+non-leak is also worth demonstrating: a flat retained-heap curve across N cycles is positive
+evidence, valid **only beside a positive control** (a known-leaking arm that grows), because a
+measurement that cannot detect a leak cannot prove its absence. See `scripts/heap-over-cycles.example.ts`
+for a two-arm driver (real code flat vs control grows) over the real module.
+
+## Order of escalation (cheapest first)
+
+### 1. Falsifying lifecycle test (preferred)
+
+Deterministic, fast, and it lives in the suite as a regression guard. Force the boundary the
+primitive should release at, then assert the release directly:
+
+- listener: assert `emitter.listenerCount(ev)` returns to its pre-acquire value after the
+  boundary (stream close, `destroy()`, instance replacement).
+- singleton / cache: assert the reference is nulled / the entry evicted.
+- pending registry: assert the map is empty after the flow (all requests settled or rejected).
+- subscription: assert the unsubscribe was called (spy) and no further dispatches land.
+
+The test **fails on the leaking code and passes on the fix** — that falsifiability is the
+point. A test that passes on both proves nothing.
+
+### 2. Heap-over-a-flow (when a unit test can't reach it)
+
+One snapshot shows occupancy, not a leak. You need the **delta across repetition**:
+
+1. Drive the flow once to warm caches; take a baseline snapshot.
+2. Run N cycles of the suspected flow (open/close, mount/unmount, connect/disconnect).
+3. Force GC, take a second snapshot.
+4. Compare **retained size**, **detached DOM nodes**, and **listener count** — a leak grows
+   roughly linearly in N. A flat delta refutes the leak.
+
+Capture (Chrome, extension context):
+- DevTools Memory panel → *Allocation instrumentation on timeline* or two heap snapshots
+  with *Comparison* view; or
+- CDP: `HeapProfiler.takeHeapSnapshot` before/after, diff the node counts. `mm cdp` drives
+  the extension's contexts (page, service worker) over the protocol.
+
+### 3. Retainer graph — must match the static argument
+
+Select a surviving object in the post-flow snapshot and read its **retainer chain** (why it
+is still reachable). That chain must name the **same holder → held → boundary** the Phase-1
+read named. If the profiler says the object is retained by a path the static argument did not
+predict, the static argument is incomplete — reconcile before concluding. Agreement between
+the two independently-derived paths is what makes the finding trustworthy; either alone is
+weaker.
+
+## Trust gate
+
+- **A single snapshot is not evidence of a leak** — it is occupancy. Only the delta across N
+  cycles is.
+- **GC must be forced** before the comparison snapshot, or you measure collection lag, not
+  retention.
+- **The retainer chain is the discriminator** — "retained size went up" without a chain
+  naming the culprit is a symptom, not a diagnosis.
+- **Warm the caches first** — the first cycle populates legitimate one-time caches that would
+  otherwise read as a leak.
+
+## Scrub before sharing
+
+Heap snapshots and retainer graphs can contain live application state (URLs, account
+identifiers, in-flight request payloads). Scrub or crop before any snapshot leaves the
+machine; never attach a raw `.heapsnapshot` to a public surface.

domains/stability/skills/memory-leak/scripts/heap-over-cycles.example.ts

@@ -0,0 +1,54 @@
+// Phase-2 runtime evidence — PR #40684 introduced pending-request Map does not leak.
+// Drives the REAL PatchStoreSubstreamConnection over N cycles, measures retained V8
+// heap. A no-leak result is meaningful only beside a control that grows: ARM B drains
+// the request stream but withholds responses, so entries accumulate.
+import v8 from 'node:v8';
+import ObjectMultiplex from '@metamask/object-multiplex';
+import { PATCH_STORE_SUBSTREAM_METHODS } from '../../shared/constants/patch-store-substream-methods';
+import { PatchStoreSubstreamConnection } from './patch-store-substream-connection';
+
+function pair() {
+  const uiMux = new ObjectMultiplex(); const bgMux = new ObjectMultiplex();
+  uiMux.pipe(bgMux).pipe(uiMux);
+  return { uiStream: uiMux.createStream('patch-store'), bgStream: bgMux.createStream('patch-store') };
+}
+const flush = () => new Promise((r) => setImmediate(r));
+function usedMB() { global.gc!(); global.gc!(); return v8.getHeapStatistics().used_heap_size / 1048576; }
+const N = 100000;
+
+async function main() {
+  console.log(`PR #40684 · PatchStoreSubstreamConnection · pending-request Map · ${N} request cycles`);
+  console.log('='.repeat(74));
+
+  // ARM A — head code: every request answered → entry .delete on response
+  { const { uiStream, bgStream } = pair();
+    bgStream.on('data', (m: any) => { if (m?.method === PATCH_STORE_SUBSTREAM_METHODS.GetStatePatches) bgStream.write({ id: m.id, jsonrpc: '2.0', result: [] }); });
+    const conn = new PatchStoreSubstreamConnection(uiStream, { handleSendUpdate: () => undefined });
+    let got = 0; await conn.getStatePatches();
+    const before = usedMB();
+    for (let i = 0; i < N; i++) { const r = await conn.getStatePatches(); got += r.length === 0 ? 1 : 0; }
+    await flush();
+    const after = usedMB();
+    console.log(`\nARM A  head code — all ${N} requests answered  (${got} responses consumed)`);
+    console.log(`       retained heap   ${before.toFixed(1)} -> ${after.toFixed(1)} MB      Δ ${(after - before >= 0 ? '+' : '') + (after - before).toFixed(1)} MB   ── FLAT`);
+    console.log(`       every .set(id) on request is matched by .delete(id) on response; the Map returns to empty`);
+  }
+
+  // ARM B — control: requests consumed but never answered → Map accumulates N entries
+  { const { uiStream, bgStream } = pair();
+    bgStream.on('data', () => { /* consume the request, send no response */ });
+    const conn = new PatchStoreSubstreamConnection(uiStream, { handleSendUpdate: () => undefined });
+    const held: Promise<unknown>[] = [];
+    const before = usedMB();
+    for (let i = 0; i < N; i++) held.push(conn.getStatePatches().catch(() => {}));
+    await flush();
+    const after = usedMB();
+    console.log(`\nARM B  control — same code, ${N} requests, none answered (${held.length} promises pending)`);
+    console.log(`       retained heap   ${before.toFixed(1)} -> ${after.toFixed(1)} MB      Δ +${(after - before).toFixed(1)} MB   ── GROWS`);
+    console.log(`       the .set(id) has no matching .delete; entries pile up — proving the measurement catches a leak`);
+  }
+
+  console.log(`\nVERDICT: the pending-request Map introduced by #40684 does not retain across ${N} cycles —`);
+  console.log(`         confirmed at runtime, beside a control that does. The static pairing is corroborated, not merely asserted.`);
+}
+main().then(() => process.exit(0));

domains/stability/skills/memory-leak/scripts/retention-scan.py

@@ -0,0 +1,96 @@
+#!/usr/bin/env python3
+"""Retention review, scoped to a diff. For each file+patch: find every retention
+primitive, pair each acquire with its release IN THE SAME FILE, and mark each
+NEW (line appears in the patch's added lines) or pre-existing. Charge only NEW
+un-paired primitives; report pre-existing separately. Re-runnable: inputs are
+the head files and the PR patch, both fetched from the repo by ref."""
+import re, sys
+
+def added_lines(patch_path):
+    out=set()
+    try:
+        for l in open(patch_path):
+            if l.startswith('+') and not l.startswith('+++'):
+                out.add(l[1:].strip())
+    except FileNotFoundError:
+        pass
+    return out
+
+def scan(src_path, patch_path):
+    src=open(src_path).read(); lines=src.split('\n'); added=added_lines(patch_path)
+    rows=[]
+    # listeners: pair .on(ev,handler) with .removeListener(ev,handler)
+    for m in re.finditer(r'(\w+)\.(?:on|addListener)\(\s*[\'"](\w+)[\'"]\s*,\s*(\w+)', src):
+        emitter,ev,handler=m.groups(); ln=src[:m.start()].count('\n')+1
+        acquire_line=lines[ln-1].strip()
+        new = acquire_line in added
+        rem=re.search(r'\.(?:removeListener|off)\(\s*[\'"]'+ev+r'[\'"]\s*,\s*'+handler, src)
+        if rem:
+            rln=src[:rem.start()].count('\n')+1
+            ctx=src[max(0,rem.start()-140):rem.start()]
+            onclose='Closed' in ctx or 'close' in ctx
+            rows.append((new,'ok',f"{emitter}.on('{ev}', {handler})",ln,
+                         f"removeListener L{rln}"+(" on stream close" if onclose else "")))
+        else:
+            rows.append((new,'OPEN',f"{emitter}.on('{ev}', {handler})",ln,"no removeListener in file"))
+    # named-subscription listeners: onXxx(handler) / subscribe(handler) / addXxxListener(handler).
+    # The quoted-event form above cannot see these — the method name carries the event, and
+    # there is no event-name argument to pair on. Missing them yields a clean verdict over a
+    # real unpaired listener (observed: background.onNotification on extension#42823).
+    for m in re.finditer(
+        r'(\w+)\.(on[A-Z]\w*|subscribe|addEventListener|add[A-Z]\w*Listener)\(\s*([\w.]+)\s*[,)]',
+        src):
+        emitter, method, handler = m.groups()
+        if method in ('on', 'addListener'):
+            continue                      # already covered by the quoted-event pass
+        ln = src[:m.start()].count('\n') + 1
+        new = lines[ln - 1].strip() in added
+        # Release forms that correspond to this acquire form.
+        if method.startswith('on'):
+            rel = ['remove' + method[0].upper() + method[1:], 'off' + method[2:]]
+        elif method == 'subscribe':
+            rel = ['unsubscribe']
+        elif method == 'addEventListener':
+            rel = ['removeEventListener']
+        else:
+            rel = ['remove' + method[3:]]
+        found = None
+        for r in rel:
+            rm = re.search(re.escape(r) + r'\(', src)
+            if rm:
+                found = (r, src[:rm.start()].count('\n') + 1)
+                break
+        label = f"{emitter}.{method}({handler})"
+        if found:
+            rows.append((new, 'ok', label, ln, f"{found[0]} L{found[1]}"))
+        else:
+            rows.append((new, 'OPEN', label, ln,
+                         "no " + "/".join(rel) + " in file"))
+
+    # pending registries: Map with set paired with delete
+    for m in re.finditer(r'(#?\w*[Pp]ending\w*|#?\w*[Rr]equests?\w*)\s*[=:][^\n]*new Map', src):
+        name=m.group(1); ln=src[:m.start()].count('\n')+1
+        new=lines[ln-1].strip() in added
+        setm=re.search(re.escape(name)+r'\.set\(', src); delm=re.search(re.escape(name)+r'\.delete\(', src)
+        if setm:
+            sln=src[:setm.start()].count('\n')+1
+            if delm:
+                rows.append((new,'ok',f"{name}  (.set L{sln})",ln,f".delete L{src[:delm.start()].count(chr(10))+1}"))
+            else:
+                rows.append((new,'OPEN',f"{name}  (.set L{sln})",ln,"no .delete — entries accumulate"))
+    return rows
+
+print("RETENTION REVIEW — scoped to the supplied diff  (re-run: retention-scan.py <file>:<patch> [...])")
+print("="*74)
+new_open=0
+for pair in sys.argv[1:]:
+    f,patch=pair.split(':')
+    print(f"\n{f.split('/')[-1]}")
+    for new,mark,what,ln,status in sorted(scan(f,patch), key=lambda r:(not r[0], r[3])):
+        tag='NEW' if new else 'pre-exist'
+        if new and mark=='OPEN': new_open+=1
+        print(f"  [{tag:9}] {mark:4} L{ln}: {what}")
+        print(f"                   -> {status}")
+print()
+print("VERDICT:", "no retention path INTRODUCED — every NEW primitive is torn down; no heap snapshot warranted"
+      if new_open==0 else f"{new_open} NEW un-paired primitive(s) → escalate to a heap snapshot (Phase 2)")