Add pr-workflow, coding, and general skills

CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+- Add `coding` TypeScript knowledge: `derive-dont-define-types` (derive from authoritative types; ad-hoc types duplicate, run too wide, and drift) and `avoid-any` (`any` disables type checking — substitute by position: assignee -> `unknown`, assigned -> `never`)
+
+- Add `pr-workflow` skills (commit-discipline, diff-audit, pr-review-discipline + knowledge), `coding` skills (ts2589-workaround + TypeScript knowledge), and `general` skills (scope-lock, specifications-as-guardrails, diagnose-blockers)
+
 ## [0.1.0]
 
 ### Added

domains/coding/knowledge/avoid-any.md

@@ -0,0 +1,52 @@
+---
+maturity: experimental
+name: avoid-any
+description: Handle `any` correctly — it is not a type but a directive that disables type checking; substitute by position (assignee → `unknown`, assigned → `never`)
+---
+
+# Handling `any`
+
+Grounded in `MetaMask/contributor-docs` `docs/typescript.md` (§ Avoid `any`, PR #74).
+
+## Conceptualize it correctly — `any` is not a type
+
+`any` is the most dangerous explicit type declaration and should be completely avoided. The mental model matters more than the ESLint rule, because `no-explicit-any` alone does not stop the reasoning that reaches for it:
+
+- **`any` is not a type. It is a compiler directive that _disables_ type checking** for the value it's assigned to. It is not "the widest type" — that is `unknown`.
+- **It suppresses every error about its assignee** — the equivalent of applying `@ts-ignore` to every use of that variable. The errors still affect the code; `any` only makes them invisible.
+- **It subsumes everything it touches.** Any type in a union, intersection, or property relationship with `any` becomes `any` — an unmitigated loss of type information.
+- **It infects downstream code.** One `any` at a source (e.g. a library type that resolves to `any`) propagates silently through every consumer. This is its most dangerous property: it converts compile-time errors into **silent runtime failures**, defeating the purpose of a statically-typed language.
+
+## Substitute by POSITION — assignee vs assigned
+
+When tempted to write `any`, first identify which side of an assignment it sits on:
+
+- **Assignee `any`** — the variable, parameter, or return that _receives_ a value ("it could be anything"). **Try `unknown` first**, then narrow to a supertype of the assigned type. `unknown` is the true universal supertype: everything is assignable to it, but it forces a type guard before use. In assignee position `any` and `unknown` are interchangeable, so this is almost always a safe swap.
+  - 🚫 `type Fn = () => any; const xs: any[]`
+  - ✅ `type Fn = () => unknown; const xs: unknown[]`
+- **Assigned `any`** — the value that _flows into_ a slot. **Try `never` first**, then widen to a subtype of the assignee type. `unknown` cannot substitute here (it is only assignable to `unknown`); `never` is the bottom type, assignable to everything.
+
+## Avoidance case — a generic default of `any`
+
+Some generic types default a parameter to `any` (e.g. `@types/jest` v27's `jest.fn()` → `Mock<any, any>`). Always supply explicit type arguments so the default can't silently poison the instantiation — and **derive** them from the target (`ReturnType<Fn>`, `Parameters<Fn>`), don't hand-write them.
+
+## The one acceptable exception — generic *constraints*
+
+`any` is acceptable in a generic **constraint**, and only there. It bounds a type parameter without being assigned to a value, so it does not pollute or infect.
+
+```typescript
+class BaseController<
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  Messenger extends RestrictedMessenger<N, any, any, string, string>,
+> // ...
+```
+
+The guideline attaches three conditions:
+
+- **Declare it explicitly.** The `no-explicit-any` rule is `error`, so a constraint `any` needs an inline `// eslint-disable-next-line @typescript-eslint/no-explicit-any` at the site — it is a deliberate, visible exception, not a silent one.
+- **Constraints only — never a generic _argument_.** Passing `any` as an argument (`ControllerMessenger<any, any>`) is 🚫 not acceptable: that assigns `any` and infects. Constraint-vs-argument is the entire distinction.
+- **Prefer a narrower constraint anyway.** A specific constraint gives better type safety and intellisense; reach for `any` here only when the narrower bound is genuinely unavailable.
+
+## When it still seems unavoidable
+
+Prefer the narrower, visible escape hatches over `any`: `as unknown as` as a documented, commented last resort, or `@ts-expect-error` with a TODO. Both are scoped and greppable; `any` infects silently. Never reach for `any` to unblock feature work "to fix later" — that is exactly the tech-debt pattern the guideline exists to prevent.

domains/coding/knowledge/cascade-refactoring.md

@@ -0,0 +1,30 @@
+---
+name: cascade-refactoring
+domain: coding
+description: Only modify files that would fail to compile without the change. Verify with a blast-radius check before committing.
+---
+
+# Cascade Refactoring Anti-Pattern
+
+When making a targeted change in one layer of a system, do not follow type/lint feedback across the dependency graph into "while I'm here" cleanups.
+
+## Rule
+
+Only modify files that would **fail to compile** without the change. If a consumer still compiles with the old types, leave it alone.
+
+## Pattern
+
+A change to one layer triggers type errors in adjacent layers. The correct response is to fix only what breaks. The wrong response is to follow each error transitively and refactor downstream consumers.
+
+Those downstream changes compile, but they're cosmetic refactoring disguised as necessary fixes — expanding scope without expanding value.
+
+## Blast-Radius Check
+
+Before committing, list every modified file and justify it against the task description:
+
+- Does this file fail to compile without the change? → Keep
+- Does it compile fine with the old types? → Revert
+
+## Signal
+
+If a diff touches files from multiple unrelated layers for a task scoped to one layer, something leaked. Common pattern: a "collection" file change pulling in "reporting" or "display" file changes.

domains/coding/knowledge/derive-dont-define-types.md

@@ -0,0 +1,23 @@
+---
+maturity: experimental
+name: derive-dont-define-types
+description: Derive types from authoritative sources — never hand-write an ad-hoc type that restates one that already exists
+---
+
+# Derive Types From Authoritative Sources
+
+When a type already exists at an authoritative source — a controller's state type, a function's return, a library's exported type, a schema — **derive from it** rather than hand-writing a fresh type that restates it. Derive with indexed access (`State['field']`), `typeof`, `ReturnType` / `Parameters`, and utility types (`Pick` / `Omit` / `Partial`), and let inference carry the rest.
+
+This is the structural form of the contributor-docs rule *Prefer type inference over annotations and assertions* (`MetaMask/contributor-docs` `docs/typescript.md`, PR #74): inferred and derived types are "responsive to changes in code, always reflecting up-to-date type information," while hand-written declarations "rely on hard-coding, making them brittle against code drift."
+
+## Why an ad-hoc type is a liability
+
+An ad-hoc type — one hand-defined to describe a value that an authoritative type already describes — carries three dangers:
+
+- **Duplication.** The same shape is now stated in two places. Every reader has to reconcile them, and every change has to touch both.
+- **Incorrect, and usually too wide.** A hand-written type is a *guess* at the source's shape, and the guess is almost always looser than the real type — it admits values the authoritative type would reject, so invalid data still type-checks. This is the same failure the guideline warns about in *Avoid unintentionally widening an inferred type*: a wider declaration loses information instead of adding it.
+- **Drift.** The source type evolves; the ad-hoc copy does not. It silently goes stale — and because it is hand-written rather than derived, the compiler cannot flag the divergence. The bug surfaces at runtime, not at build.
+
+## Rule
+
+Before writing a type, ask where the value comes from and whether that source already types it. If it does, **derive**. Define a fresh type only when no authoritative source exists — a genuinely new shape at a boundary you own.

domains/coding/knowledge/spike-integration-boundaries.md

@@ -0,0 +1,37 @@
+---
+name: spike-integration-boundaries
+domain: coding
+description: Test the riskiest integration seam in isolation with a 10-line spike before building the full multi-system script.
+---
+
+# Spike Hard Integrations Before Committing
+
+When building automation that combines 3+ systems or APIs, test the hardest integration point in isolation before writing the full script.
+
+## Rule
+
+Multi-system integrations fail at boundaries — not in any single system but at the seam between them. A 10-line spike that tests the hardest seam saves hours debugging a full script that can't work.
+
+## Procedure
+
+1. **Identify the single hardest unknown.** Ask: "Which integration point am I least confident about?"
+2. **Write a 10-line throwaway test** for just that one thing.
+3. **Run it.** If it fails, you've saved hours. If it passes, proceed.
+
+## Cost Analysis
+
+| | Skip spike | Do spike |
+|---|---|---|
+| Spike succeeds | N/A | +5 min |
+| Spike fails | Hours debugging dead-end script | 5 min + pivot |
+| Expected value | Negative (boundary failures are the norm) | Positive |
+
+## Common Integration Unknowns
+
+- Can API A connect to service B in this environment/process?
+- Does state persist across the lifecycle event I'm relying on?
+- Will the runtime I'm using support the protocol I'm assuming?
+
+## Anti-Pattern
+
+Building the full 400-line script first, then discovering the core integration is impossible — requiring full restart after multiple debug iterations across unrelated failure modes.

domains/coding/knowledge/type-level-assertions.md

@@ -0,0 +1,61 @@
+---
+name: type-level-assertions
+domain: coding
+description: Compile-time type tests using branded uninhabitable types, IsEquivalent guards against any, and mutual-assignability checks.
+---
+
+# Type-Level Compile-Time Assertions
+
+Pattern for writing type-level tests that produce compiler errors on failure rather than runtime assertions.
+
+## Core Types
+
+```typescript
+type _ = { readonly _: unique symbol };
+
+type IsAny<T> = 0 extends 1 & T ? true : false;
+type IsNever<T> = [T] extends [never] ? true : false;
+
+type IsEquivalent<A, B> =
+  IsAny<A> extends true ? IsAny<B>
+  : IsAny<B> extends true ? false
+  : [A, B] extends [B, A] ? true : false;
+
+type Expect<
+  X extends [X, V] extends [V, X] ? V : V & _,
+  V = true,
+> = IsNever<V> extends true ? X
+  : IsNever<X> extends true ? Expect<X, V>
+  : X;
+```
+
+## Usage
+
+Organize assertions in named tuple types. Each element either compiles or produces a type error at the failing assertion.
+
+```typescript
+type Describe_MyFeature = [
+  Expect<IsEquivalent<Actual, Expected>>,
+  Expect<IsEquivalent<EdgeCase, EdgeExpected>, false>,
+];
+```
+
+## Design Decisions
+
+| Decision | Rationale |
+|----------|-----------|
+| `_` branded uninhabitable type | `V & _` is unsatisfiable, forcing a type error when `X` doesn't match `V` |
+| `IsEquivalent` guards against `any` | Naive `[A, B] extends [B, A]` returns `true` if either side is `any` |
+| `IsNever` wraps in tuple | Prevents distributive conditional evaluation over `never` |
+| `Expect` uses naive `& _` constraint, not `IsEquivalent` | Referencing `IsEquivalent` in `Expect`'s own bound creates a circular constraint (TS2313) |
+
+## Known Limitation
+
+`Expect<any, string>` passes silently. Use `Expect<IsEquivalent<any, string>>` when `any`-safety matters.
+
+## ESLint Configuration
+
+Type-level spec files (`*.spec.ts`) need relaxed rules:
+- Disable `no-unused-vars` (tuple types are never "used")
+- Allow `Describe_` prefixed type names
+- Exclude E2E test files from these overrides

domains/coding/knowledge/typescript-intermediate-defaults.md

@@ -0,0 +1,36 @@
+---
+name: typescript-intermediate-defaults
+domain: coding
+description: Break complex conditional types into a chain of named intermediate defaulted type parameters instead of nesting conditionals.
+---
+
+# Intermediate Defaulted Type Parameters
+
+When a conditional type needs to resolve multiple aspects of a generic input, chain intermediate defaulted parameters instead of nesting conditionals.
+
+## Pattern
+
+```typescript
+type InferResult<
+  Input extends Record<PropertyKey, unknown>,
+  Step1 = Input extends { key: infer V } ? V : never,
+  Step2 = [Step1] extends [never] ? false : true,
+  Step3 = Step1 extends { nested: infer V } ? V : never,
+  Result = Step2 extends true ? Step1 : Fallback,
+> = Result;
+```
+
+## Why
+
+| Benefit | Detail |
+|---------|--------|
+| Transparent resolution chain | Each step is named and hoverable in IDE |
+| Flat debugging | Hover any parameter to see its resolved value without tracing nested branches |
+| Extensible | Adding resolution steps doesn't increase nesting depth |
+| Compatible with `import()` inference | Works where deferred conditional evaluation in generics prevents resolution |
+
+## When To Use
+
+- Complex conditional types with 3+ resolution steps
+- Types handling multiple structural patterns (default exports, named exports, nested namespaces)
+- When deferred conditional evaluation blocks resolution in generic type parameters

domains/coding/knowledge/typescript-strict-flag-pattern.md

@@ -0,0 +1,51 @@
+---
+name: typescript-strict-flag-pattern
+domain: coding
+description: Fold a wrapper type's stricter constraint into the base type behind a Strict boolean flag parameter instead of duplicating branching logic.
+---
+
+# Strict-Flag DRY Pattern
+
+When a wrapper type duplicates branching logic from a base type to add a stricter constraint, fold the constraint into the base type behind a boolean flag.
+
+## Before (duplicated logic)
+
+```typescript
+type InferComponent<Module, ...> = /* branch on condition */;
+
+type StrictComponent<Module, ...> =
+  ConditionA extends true
+    ? InferComponent<Module>
+    : ConditionB extends true
+      ? InferComponent<Module>
+      : never;
+```
+
+## After (consolidated)
+
+```typescript
+type InferComponent<
+  Module,
+  Strict extends boolean = false,
+  FromNamedExport = Strict extends true
+    ? ConditionB extends true
+      ? ValidResult
+      : never
+    : ValidResult,
+> = /* single branch on primary condition */;
+
+type StrictComponent<Module> = InferComponent<Module, true>;
+```
+
+## When To Apply
+
+- Wrapper type delegates to base type in all passing branches
+- Wrapper only adds a `never` gate for certain input shapes
+- `Strict = false` default preserves backward compatibility for existing callers
+
+## Common Pitfalls
+
+| Mistake | Correct Approach |
+|---------|-----------------|
+| Keeping the wrapper type after consolidation | Delete it — callers use `Base<Input, true>` directly |
+| Setting `Strict = true` as the default | Breaks callers not expecting the stricter behavior |

domains/coding/skills/simulator-control/repos/metamask-mobile.md

@@ -0,0 +1,61 @@
+---
+repo: metamask-mobile
+parent: simulator-control
+---
+
+# Simulator Control — MetaMask Mobile
+
+`agent-device` is installed as a local dependency. All device commands run via `yarn agent-device <command>` in the shell.
+
+## Step 1 — Version Check
+
+```bash
+yarn agent-device --version
+```
+
+Require `version >= 0.14.0`. If the version is lower, ask the user to bump the `agent-device` dependency in `package.json`.
+
+## Step 2 — Prerequisites
+
+Before any agent-device command:
+
+1. Confirm Metro is running by checking the terminals folder for an active `yarn watch:clean` process. Do NOT start Metro yourself — if it is not running, stop and ask the user to run `yarn watch:clean` in a separate terminal, then wait for confirmation.
+2. Confirm a simulator is booted:
+   ```bash
+   yarn agent-device devices --platform ios
+   ```
+   If none are booted, ask the user to boot one via Xcode Simulator, then wait for confirmation.
+
+## App Identifiers
+
+| Platform | App identifier              |
+|----------|-----------------------------|
+| iOS      | `io.metamask.MetaMask`      |
+| Android  | `io.metamask`               |
+
+```bash
+yarn agent-device open io.metamask.MetaMask --platform ios
+yarn agent-device open io.metamask --platform android
+```
+
+## Core Loop
+
+```bash
+yarn agent-device devices --platform ios
+yarn agent-device open io.metamask.MetaMask --platform ios --device "iPhone 17"
+yarn agent-device snapshot -i
+yarn agent-device screenshot
+```
+
+Run `yarn agent-device --help` for the full list of interaction, validation, and evidence commands.
+
+## Deep Link Navigation
+
+To navigate directly to a screen, use the `metamask://` URL scheme instead of tapping through the UI:
+
+```bash
+yarn agent-device open "metamask://<route>" --platform ios
+yarn agent-device open "metamask://<route>" --platform android
+```
+
+Check `app/core/AppConstants.js` and deeplink handler files for available routes.

domains/coding/skills/simulator-control/skill.md

@@ -0,0 +1,11 @@
+---
+name: simulator-control
+description: Controls iOS/Android simulators to verify UI changes. Use when opening the app, navigating to a screen, taking a screenshot, checking what the UI looks like, tapping/typing/scrolling, verifying an implementation on device, or collecting visual evidence for a PR.
+maturity: experimental
+---
+
+# Simulator Control
+
+This skill only applies to MetaMask Mobile.
+
+If `agent-device` is not listed in the project's `package.json`, stop and tell the user this skill is not supported for their project.