diff --git a/CHANGELOG.md b/CHANGELOG.md index 383d49c7..2279f092 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- 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 diff --git a/domains/coding/knowledge/.gitkeep b/domains/coding/knowledge/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/domains/coding/knowledge/cascade-refactoring.md b/domains/coding/knowledge/cascade-refactoring.md new file mode 100644 index 00000000..f70f6db9 --- /dev/null +++ b/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. diff --git a/domains/coding/knowledge/spike-integration-boundaries.md b/domains/coding/knowledge/spike-integration-boundaries.md new file mode 100644 index 00000000..c2f43def --- /dev/null +++ b/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. diff --git a/domains/coding/knowledge/type-level-assertions.md b/domains/coding/knowledge/type-level-assertions.md new file mode 100644 index 00000000..ba5d64e8 --- /dev/null +++ b/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 = 0 extends 1 & T ? true : false; +type IsNever = [T] extends [never] ? true : false; + +type IsEquivalent = + IsAny extends true ? IsAny + : IsAny extends true ? false + : [A, B] extends [B, A] ? true : false; + +type Expect< + X extends [X, V] extends [V, X] ? V : V & _, + V = true, +> = IsNever extends true ? X + : IsNever extends true ? Expect + : 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>, + Expect, 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` passes silently. Use `Expect>` 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 diff --git a/domains/coding/knowledge/typescript-intermediate-defaults.md b/domains/coding/knowledge/typescript-intermediate-defaults.md new file mode 100644 index 00000000..72fc3cf8 --- /dev/null +++ b/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, + 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 diff --git a/domains/coding/knowledge/typescript-strict-flag-pattern.md b/domains/coding/knowledge/typescript-strict-flag-pattern.md new file mode 100644 index 00000000..36d29f8f --- /dev/null +++ b/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 = /* branch on condition */; + +type StrictComponent = + ConditionA extends true + ? InferComponent + : ConditionB extends true + ? InferComponent + : 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 = InferComponent; +``` + +## 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` directly | +| Setting `Strict = true` as the default | Breaks callers not expecting the stricter behavior | diff --git a/domains/coding/skills/.gitkeep b/domains/coding/skills/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/domains/coding/skills/simulator-control/repos/metamask-mobile.md b/domains/coding/skills/simulator-control/repos/metamask-mobile.md new file mode 100644 index 00000000..53fc8286 --- /dev/null +++ b/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 ` 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://" --platform ios +yarn agent-device open "metamask://" --platform android +``` + +Check `app/core/AppConstants.js` and deeplink handler files for available routes. diff --git a/domains/coding/skills/simulator-control/skill.md b/domains/coding/skills/simulator-control/skill.md new file mode 100644 index 00000000..8fd5ad25 --- /dev/null +++ b/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. diff --git a/domains/coding/skills/ts2589-workaround/skill.md b/domains/coding/skills/ts2589-workaround/skill.md new file mode 100644 index 00000000..d09bd1cb --- /dev/null +++ b/domains/coding/skills/ts2589-workaround/skill.md @@ -0,0 +1,46 @@ +--- +maturity: experimental +name: ts2589-workaround +description: Fix TS2589 "type instantiation is excessively deep" in Expect<> constraints by pre-resolving complex conditionals to boolean before the constraint sees them. +--- + +# TS2589 Workaround: Pre-Resolve Before Expect + +## When To Use + +- A test file hits TS2589 on `Expect` +- The failing type involves union distribution, `infer` chains, or multi-step conditionals +- `Expect` fails even though the type itself resolves correctly + +## Do Not Use When + +- TS2589 is in production code, not test assertions — fix the type itself, don't work around it +- The type is simple enough that TypeScript resolves it without hitting the depth limit + +## Workflow + +1. Extract the complex type to a standalone alias +2. Manually flatten the equivalence check to `true`/`false` using an extends-pair +3. Feed the boolean result to `Expect<>`, not the original type + +```typescript +// 1. Extract to alias +type Resolved = ComplexConditional; + +// 2. Flatten to boolean +type Check = [Resolved] extends [Expected] + ? [Expected] extends [Resolved] + ? true + : false + : false; + +// 3. Feed boolean to Expect +Expect +``` + +## Common Pitfalls + +| Mistake | Correct Approach | +|---------|-----------------| +| Using `type Resolved = ComplexType` then `Expect` | TypeScript does not eagerly resolve aliases before constraint-checking — it re-expands inline. Use the manual extends-pair flatten. | +| Wrapping in a utility type | Any utility that re-introduces a conditional constraint re-triggers depth expansion. Flatten to `true`/`false` directly. | diff --git a/domains/general/knowledge/diagnose-blockers.md b/domains/general/knowledge/diagnose-blockers.md new file mode 100644 index 00000000..6f484298 --- /dev/null +++ b/domains/general/knowledge/diagnose-blockers.md @@ -0,0 +1,57 @@ +--- +name: diagnose-blockers +domain: general +description: When blocked, diagnose before acting. Stop, investigate, report findings with evidence, present options, then wait for approval before any destructive or scope-reducing action. +--- + +# Diagnose Before Acting + +When encountering a blocker, risk signal, or unexpected failure, the correct response is to diagnose — not to revert, reduce scope, or create workarounds. + +## Protocol + +1. **Stop.** Do not change anything yet. +2. **Diagnose.** What is actually failing or risky, and why? +3. **Report.** State the finding with evidence (logs, lockfile analysis, API docs, etc.). +4. **Present options.** Fix it / add safeguards / skip with stated tradeoffs / revert. +5. **Wait.** Get explicit approval before any spec modification, scope reduction, or destructive action. + +## Prohibited First Responses + +| Signal | Wrong | Right | +|--------|-------|-------| +| "This is untested" | Revert to safe version | Assess what would actually break, report finding | +| "This might not work" | Create simplified alternative | Investigate actual compatibility, report | +| "Tests are failing" | Weaken assertions | Diagnose root cause, present options | +| "Dependency conflict" | Remove the dependency | Analyze dependency trees, check isolation | +| Risk flagged by user | Undo the risky work | Investigate the risk, quantify it | + +## Key Distinction + +**"Untested" ≠ "broken."** An untested dependency or path is a risk to assess, not a trigger to revert. The investigation may reveal the risk is negligible (isolated dependency trees, separate processes) or real (shared runtime, incompatible APIs). Either finding is useful. Reverting without investigating wastes work and obscures the actual risk. + +## Destructive Actions Requiring Confirmation + +Before any of these, stop and get explicit approval: + +- Reverting a file to a previous state +- Deleting code or files +- Reducing scope of an implementation +- Adding skip/ignore flags +- Replacing an implementation with a "simpler" alternative + +## Evidence Standards + +A finding report must include: +- What was checked (commands run, files read, docs consulted) +- What was found (specific versions, error messages, dependency relationships) +- Confidence level (confirmed vs suspected) +- What remains unknown + +## Red Flags to Self-Monitor + +- "Let me revert this to be safe" — without diagnosing what's unsafe +- "Let me try a simpler approach" — after the specified approach hit a snag +- Creating new files when existing ones don't work +- Reporting "success" after silently excluding the hard parts +- Any destructive action taken without explicit user approval diff --git a/domains/general/skills/.gitkeep b/domains/general/skills/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/domains/general/skills/deploy-internal-vercel-app/repos/.gitkeep b/domains/general/skills/deploy-internal-vercel-app/repos/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/domains/general/skills/deploy-internal-vercel-app/skill.md b/domains/general/skills/deploy-internal-vercel-app/skill.md new file mode 100644 index 00000000..854c5053 --- /dev/null +++ b/domains/general/skills/deploy-internal-vercel-app/skill.md @@ -0,0 +1,300 @@ +--- +name: deploy-internal-vercel-app +description: Use when someone wants to deploy an internal Next.js app to the Consensys Vercel Enterprise account with Okta SSO. Automates GitHub repo creation, secret audit, NextAuth.js auth gate injection, and generates a TechOps email + Vercel deployment checklist. Works with both Pages Router and App Router. +maturity: stable +--- + +# Deploy Internal Vercel App + +Use this skill when someone wants to deploy an internal Next.js app to the Consensys Vercel Enterprise account with Okta SSO. + +## Announce at start + +"I'm using the deploy-internal-vercel-app skill to set up your project." + +## Step 1: Collect inputs + +Ask the following questions one at a time: + +1. **App name** — e.g. `money-movement-dashboard` (lowercase, hyphens only — used for repo name, Vercel project name, and domain) +2. **Local project path** — absolute path to the Next.js project on your machine +3. **Your name** — for the TechOps email sign-off +4. **One-line description** — for the GitHub repo description +5. **Does the app have a cron job?** (yes/no) — if yes, ask: what is the cron route path? (e.g. `/api/cron/rotation`) + +## Step 2: Preflight checks + +Run these before touching anything: + +```bash +# 1. Check gh CLI is authenticated +gh auth status +``` +If not authenticated: halt and tell user to run `gh auth login` first. + +```bash +# 2. Verify Next.js project exists +cat {project-path}/package.json | grep '"next"' +``` +If `next` not found: halt and tell user this skill only works with Next.js projects. + +```bash +# 3. Detect router type (check both root and src/ layouts) +ls {project-path}/app 2>/dev/null || ls {project-path}/src/app 2>/dev/null && echo "app-router" +ls {project-path}/pages 2>/dev/null || ls {project-path}/src/pages 2>/dev/null && echo "pages-router" +``` +- If `app/` or `src/app/` exists → App Router (set router root accordingly) +- If `pages/` or `src/pages/` exists (and no app dir) → Pages Router +- If both app and pages dirs exist → App Router (takes precedence as the newer standard) +- If neither → halt: "Cannot detect Next.js router type. Make sure your project has an `app/`, `src/app/`, `pages/`, or `src/pages/` directory." + +```bash +# 4. Check for existing git remotes +git -C {project-path} remote -v 2>/dev/null +``` +If remotes exist: show them to the user and confirm before proceeding. + +# 5. Check Consensys Vercel Enterprise team membership +Ask the user: "Are you already a member of the Consensys Vercel Enterprise account at vercel.com? (You can check by logging in to vercel.com and looking for a 'Consensys' team in the team switcher.)" + +- **Yes**: Continue. +- **No / unsure**: Warn the user: + > "⚠️ You need to be a member of the Consensys Vercel Enterprise account to import this project. The TechOps email in Step 6 will request membership — do not skip sending it, and wait for confirmation before trying to deploy on Vercel." + Continue (do not halt — the email handles the request). + +## Step 3: Repo hygiene + +### .gitignore +Check that these entries exist. Add any that are missing: +``` +node_modules/ +.next/ +.env.local +.env +.vercel +``` + +### .env.example +If it doesn't exist, create it. If it exists, scan for real secrets (tokens, webhook URLs). Flag any found and pause for the user to fix before continuing. + +Ensure these auth vars are present (no real values): +``` +AUTH_URL=https://{app-name}.vercel.app +AUTH_SECRET= +OKTA_CLIENT_ID= +OKTA_CLIENT_SECRET= +OKTA_ISSUER=https://your-domain.okta.com +``` + +If cron job: also add: +``` +CRON_SECRET= +``` + +### Secret audit +```bash +grep -rn \ + --include="*.js" --include="*.ts" --include="*.jsx" --include="*.tsx" --include="*.json" \ + -E "(xoxb-|xoxp-|hooks\.slack\.com|atlassian\.net/rest|Bearer |api_token|apiToken)" \ + {project-path} | grep -v node_modules | grep -v ".next" +``` +If any matches: show them to the user and pause. Do not proceed until the user confirms secrets are removed. + +## Step 4: Inject NextAuth.js auth gate + +### Check if auth already exists +- Pages Router: check for `{project-path}/pages/api/auth/[...nextauth].js` +- App Router: check for `{project-path}/app/api/auth/[...nextauth]/route.ts` + +If the file exists, read it. If it already uses NextAuth with Okta, skip injection. If it uses a different auth library, warn the user and skip injection. + +### If no auth handler exists — create it + +**Pages Router** → create `pages/api/auth/[...nextauth].js`: +```js +import NextAuth from 'next-auth'; +import OktaProvider from 'next-auth/providers/okta'; + +export const authOptions = { + providers: [ + OktaProvider({ + clientId: process.env.OKTA_CLIENT_ID, + clientSecret: process.env.OKTA_CLIENT_SECRET, + issuer: process.env.OKTA_ISSUER, + }), + ], +}; + +export default NextAuth(authOptions); +``` + +**App Router** → create `app/api/auth/[...nextauth]/route.ts`: +```ts +import NextAuth from 'next-auth'; +import OktaProvider from 'next-auth/providers/okta'; + +export const authOptions = { + providers: [ + OktaProvider({ + clientId: process.env.OKTA_CLIENT_ID!, + clientSecret: process.env.OKTA_CLIENT_SECRET!, + issuer: process.env.OKTA_ISSUER, + }), + ], +}; + +const handler = NextAuth(authOptions); +export { handler as GET, handler as POST }; +``` + +### Add auth gate to main page (commented out until Okta is ready) + +**Pages Router** — add to `pages/index.js` (or `pages/index.tsx`): +```js +import { getServerSession } from 'next-auth/next'; +import { authOptions } from './api/auth/[...nextauth]'; + +export async function getServerSideProps(context) { + // Auth gate — uncomment after adding OKTA_* env vars to Vercel + // const session = await getServerSession(context.req, context.res, authOptions); + // if (!session) { + // return { redirect: { destination: '/api/auth/signin', permanent: false } }; + // } + return { props: {} }; +} +``` + +**App Router** — add to top of `app/page.tsx`: +```ts +// Auth gate — uncomment after adding OKTA_* env vars to Vercel +// import { getServerSession } from 'next-auth/next'; +// import { authOptions } from './api/auth/[...nextauth]/route'; +// const session = await getServerSession(authOptions); +// if (!session) { redirect('/api/auth/signin'); } +``` + +### Install next-auth +Add `"next-auth": "^4.24.7"` to `dependencies` in `package.json`, then run: +```bash +npm install next-auth --prefix {project-path} +``` + +### Update README.md +Add an "Authentication (Okta SSO)" section documenting: +- How to request the Okta app from TechOps (`techops@consensys.net`) +- The callback URL format +- Which env vars are needed + +## Step 5: Create GitHub repo and push + +### Try Consensys org first +```bash +gh repo create Consensys/{app-name} \ + --private \ + --description "{description}" +``` + +If this fails (no permission): fall back to personal account: +```bash +gh repo create {app-name} \ + --private \ + --description "{description} (transfer to Consensys org pending)" +``` +Warn the user: "Created under your personal account — you'll need to transfer to Consensys org later via GitHub Settings." + +Store the actual repo URL for all downstream outputs. + +### Git init and commit +```bash +git -C {project-path} init +git -C {project-path} branch -M main +git -C {project-path} add . +git -C {project-path} commit -m "Initial commit: {app-name}" +``` + +### Push via GitHub API (avoids branch protection rules) +```bash +# Push to a temp branch first so the commit exists remotely +git -C {project-path} remote add origin https://github.com/{actual-repo-path}.git +git -C {project-path} push origin HEAD:refs/heads/tmp-init + +# Create main ref via API (repo is empty so main doesn't exist yet — use POST not PATCH) +gh api repos/{actual-repo-path}/git/refs \ + --method POST \ + --field ref=refs/heads/main \ + --field sha=$(git -C {project-path} rev-parse HEAD) + +# Clean up temp branch +gh api repos/{actual-repo-path}/git/refs/heads/tmp-init \ + --method DELETE +``` + +## Step 6: Output TechOps email + +Print this copy-pasteable email (substitute all placeholders): + +``` +To: techops@consensys.net +Subject: Internal App Setup Request — {app-name} + +Hi TechOps, + +I'm deploying an internal Next.js app and need a few things set up: + +1. Add me to the Consensys Vercel Enterprise team (if not already a member) + GitHub: {gh-username} + +2. Grant Vercel access to this private GitHub repo: + https://github.com/{actual-repo-path} + +3. Create an Okta OIDC Web Application: + App name: {app-name} + Sign-in redirect URI: https://{app-name}.vercel.app/api/auth/callback/okta + Sign-out redirect URI: https://{app-name}.vercel.app + + Note: if a custom domain is used instead of .vercel.app, update these URIs accordingly. + +Please share OKTA_CLIENT_ID, OKTA_CLIENT_SECRET, and OKTA_ISSUER when ready. + +Thanks, +{name} +``` + +## Step 7: Output Vercel deployment checklist + +``` +VERCEL DEPLOYMENT CHECKLIST +============================ +1. Go to vercel.com → switch to Consensys team → New Project +2. Import: https://github.com/{actual-repo-path} +3. Framework preset: Next.js (auto-detected — leave defaults) +4. Expand "Environment Variables" and add: + - AUTH_SECRET = +{if-cron} - CRON_SECRET = + [add any other app-specific vars from .env.example] + ⚠️ Skip OKTA_* and AUTH_URL for now +5. Click Deploy +6. Once deployed, note your URL (e.g. https://{app-name}.vercel.app) +7. Add AUTH_URL = https://{app-name}.vercel.app → Save → Redeploy +``` + +## Step 8: Output post-deploy checklist + +``` +POST-DEPLOY CHECKLIST (after TechOps responds with Okta credentials) +===================================================================== +0. Confirm AUTH_URL is already set in Vercel (from step 7 above) +1. Add to Vercel env vars: OKTA_CLIENT_ID, OKTA_CLIENT_SECRET, OKTA_ISSUER → Save +2. Uncomment the auth gate in {pages/index.js or app/page.tsx} +3. Push the change — Vercel will auto-deploy +4. Visit the app URL and confirm Okta login flow works end-to-end +{if-cron}5. Verify cron dry-run: + curl -X POST https://{app-name}.vercel.app/{user-provided-cron-route} \ + -H "Authorization: Bearer $CRON_SECRET" +{last-step}. Decommission any previous personal Vercel deployment +``` + +## Done + +Tell the user: +"Your app is on GitHub and ready to import into Vercel. Send the TechOps email above, then follow the Vercel checklist. Once TechOps responds with Okta credentials, follow the post-deploy checklist to enable the auth gate." diff --git a/domains/general/skills/scope-lock/skill.md b/domains/general/skills/scope-lock/skill.md new file mode 100644 index 00000000..9220d553 --- /dev/null +++ b/domains/general/skills/scope-lock/skill.md @@ -0,0 +1,69 @@ +--- +maturity: experimental +name: scope-lock +description: Enforce declared file scope for PRs — changes outside declared set require explicit justification +--- + +# Scope Lock + +## When To Use + +- Reviewing agent-authored PRs (automated runs via Cursor, Copilot, etc.) +- Any PR where the diff touches files outside the ticket's stated scope +- Pre-flight check before submitting a PR +- A targeted change starts pulling you across the dependency graph + +## Do Not Use When + +- Intentional refactoring PRs with broad, documented scope +- Release/version-bump PRs with expected wide file changes + +A PR should modify only files necessary for its stated goal. Every file outside that set creates review burden and risk. Run `diff-audit` to verify. + +## Scope Declaration + +Before starting work, declare the intended modification set: + +``` +Scope: app/scripts/controllers/perps/*.ts, ui/pages/perps/** +Reason: Fix decimal formatting in perps display logic +``` + +Changes outside this set require explicit justification. + +## Dependency Graph Traversal + +When a targeted change triggers type errors in distant files: + +- Only modify dependents that **fail to compile** — do not preemptively refactor consumers that still compile +- Type-level cosmetic cleanups (e.g., `Omit` swaps) are not required — don't propagate +- Infrastructure changes should not leak from feature tasks unless explicitly scoped + +## Specification Evasion Detection + +| Signal | Example | Correct Response | +|--------|---------|------------------| +| New files when existing ones fail | Simplified script instead of fixing original | Diagnose the original failure | +| CLI flags not in original spec | `--skip-X` to avoid errors | Fix the errors | +| Reduced scope without discussion | "Let's just test account switching" | Surface the choice to the user | +| Success reported after adding exclusions | "Tests pass now" (with new ignores) | Report what was skipped | + +When encountering a blocker: diagnose first, escalate second. Never modify spec without explicit approval. Distinguish "ran successfully" from "achieved stated goal." + +## Agent Anti-Patterns + +| Anti-pattern | Signal | Check | +|-------------|--------|-------| +| Porting without adapting | New utility/hook duplicating existing one | Grep codebase for existing equivalents | +| Blast radius blindness | Files modified outside feature directory | Justify each against ticket scope | +| Workaround cascade | Config changes for one test/feature | See `specifications-as-guardrails` cascade rule | + +## Common Pitfalls + +| Mistake | Correct Approach | +|---------|-----------------| +| package.json changes for undeclared deps | Revert — only declared deps belong | +| attribution.txt modified in feature PR | Revert — release artifacts are release-only | +| New hook that reimplements abstraction | Grep for existing utilities first | +| shared config e.g. jest/webpack changed for small number of tests | Find the scoped fix in the test itself | +| Dev artifacts committed | Revert — only commit files that should be tracked in repo | diff --git a/domains/general/skills/specifications-as-guardrails/skill.md b/domains/general/skills/specifications-as-guardrails/skill.md new file mode 100644 index 00000000..6b589842 --- /dev/null +++ b/domains/general/skills/specifications-as-guardrails/skill.md @@ -0,0 +1,80 @@ +--- +maturity: experimental +name: specifications-as-guardrails +description: Treat failing automated checks as signals to investigate, not obstacles to remove +--- + +# Specifications as Guardrails + +## When To Use + +- A failing CI test, TypeScript error, lint rule, or schema check is blocking work +- Considering disabling, weakening, or bypassing an automated check +- A pre-existing flaky test is tempting to delete rather than diagnose + +## Do Not Use When + +- The check is genuinely wrong and the team has explicitly agreed to update it +- The spec change is part of a documented requirement change + +Automated checks are the specification. When they fail, the code doesn't meet the spec — not the other way around. Never weaken a spec as the first response to failure. + +## Applies To + +| Check | Specification | Wrong Response | +|-------|--------------|----------------| +| CI tests | Expected behavior | Weaken assertions | +| TypeScript errors | Contract definitions | Add `any` or casts | +| Lint rules | Code standards | Disable the rule | +| Schema validation | Data contracts | Loosen schemas | +| Access controls | Security boundaries | Add bypass flags | +| Build/test config | Platform constraints | Add workarounds | + +## Triage Protocol + +Before touching the spec, answer in order: + +1. Is this failure caused by changes in this PR, or pre-existing? +2. Did this test pass on `main` recently? +3. Is this a known flaky test? +4. What did the PR change that could affect this? +5. Does the fix touch shared config? If so, why don't other consumers need this? + +## Workaround Cascade Rule + +When a fix requires changing shared config (jest.config, webpack, CI pipelines), you are working around a problem, not fixing it. + +Count hops between your fix and the failing line: + +| Hops | Fix location | Confidence | +|------|-------------|------------| +| 0 | Same file as failure | High — direct fix | +| 1 | Adjacent file (test helper, mock) | Moderate — verify necessity | +| 2+ | Config, build, CI | Low — almost certainly a workaround | + +If your fix chain crosses an abstraction boundary (test code -> test config -> build config), trace backwards to root cause. + +``` +Cascade: unmocked import -> ESM parse failure -> transformIgnorePatterns -> CI transpiles extra packages for all tests +Fix: "Why is Jest loading this module?" -> test doesn't mock it -> mock the module +``` + +**Agent corollary:** When an agent adds config-level workarounds, check how the codebase handles the same situation elsewhere. If other packages don't need the workaround, the agent's code is the problem. + +## When Spec Changes Are Appropriate + +- Requirements genuinely changed (explicit, documented decision) +- Spec itself has a confirmed bug (rare; requires evidence) +- User explicitly requests after understanding the tradeoff + +## Common Pitfalls + +| Mistake | Correct Approach | +|---------|-----------------| +| "Let me update the test to pass" | Diagnose why code doesn't meet the spec | +| Add `as any` to fix a type error | Find why the type is wrong | +| `// eslint-disable` on a violation | Understand and fix the violation | +| Arbitrary delay to fix flaky test | Investigate the actual race condition | +| Add package to `transformIgnorePatterns` | Ask why Jest is loading the real module | +| Change jest/webpack config for one test | Find the scoped fix — other consumers don't need this | +| "I'll try a different approach" after error | "The test expects X, but code does Y. Investigating why." | diff --git a/domains/pr-workflow/knowledge/.gitkeep b/domains/pr-workflow/knowledge/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/domains/pr-workflow/knowledge/epic-hierarchy.md b/domains/pr-workflow/knowledge/epic-hierarchy.md new file mode 100644 index 00000000..e0dd371b --- /dev/null +++ b/domains/pr-workflow/knowledge/epic-hierarchy.md @@ -0,0 +1,19 @@ +--- +name: epic-hierarchy +domain: pr-workflow +description: GitHub sub-issues are a structural relationship via the API, not parsed from body text — always create the link explicitly +--- + +# GitHub Epic Hierarchy + +Parent-child epic relationships are structural (GitHub sub-issues API), not body text. A `**Parent Epic:**` reference in the body is documentation only — the link won't exist without the API call. + +## Pattern +Always both steps: +1. Create the issue (body may reference the parent) +2. Establish the sub-issue link via the API (sub-issues endpoint) using the new issue ID + +Without step 2 the epic is orphaned — invisible in the parent's sub-issue list. + +## Also +If the parent epic has a manual sub-epics table in its body, update it to include the new child. diff --git a/domains/pr-workflow/knowledge/inline-comment-policy.md b/domains/pr-workflow/knowledge/inline-comment-policy.md new file mode 100644 index 00000000..1dacab46 --- /dev/null +++ b/domains/pr-workflow/knowledge/inline-comment-policy.md @@ -0,0 +1,21 @@ +--- +name: inline-comment-policy +domain: pr-workflow +description: Inline code comments are targeted documentation for future contributors, not line-by-line narration of what changed +--- + +# Inline Comment Policy + +Code comments are documentation for future contributors, not play-by-plays of a diff. + +## Leave out +- One-liners restating what the code obviously does (`// Moved to X`, `// Renamed from Y`) +- Narration of changes visible from the diff or commit message + +## Belongs +- TODOs for planned follow-ups with enough context to act on +- Comments clarifying motivation, architectural background, or constraints not apparent from the code +- Context that would otherwise require archaeology (why a workaround exists, what invariant a block depends on) + +## For reviewer clarity +Use the PR description / commit body / review threads first. Inline comments persist after merge — they must earn their place as documentation, not serve as transient review aids. diff --git a/domains/pr-workflow/knowledge/ticket-management.md b/domains/pr-workflow/knowledge/ticket-management.md new file mode 100644 index 00000000..fb4dfb3f --- /dev/null +++ b/domains/pr-workflow/knowledge/ticket-management.md @@ -0,0 +1,27 @@ +--- +name: ticket-management +domain: pr-workflow +description: MetaMask ticket conventions — never auto-assign, edit don't comment, priority distribution targets, consolidation methodology +--- + +# Ticket Management + +## Never auto-assign +**Never assign yourself or anyone when creating tickets unless explicitly directed.** Leave `assignees` empty by default. Assignment implies commitment — let humans decide who owns work. + +## Edit, don't comment +Don't add a comment explaining a change you made to the ticket itself ("updated the description…", "changed priority because…"). Edit the ticket directly; GitHub tracks edit history natively. Comments are for discussion. + +## MetaMask labels +| Pattern | Meaning | +|---|---| +| `team-{name}` | Team assignment (e.g. `team-extension-platform`) | +| `Epic` | Issue is an epic (has sub-issues) | +| `area-*` | Area (e.g. `area-performance`) | +| `priority-critical` / `-high` / `-medium` | P0 / P1 / P2 | + +## Priority distribution (target) +P0 10-15% · P1 40-50% · P2 30-40% · P3 5-10%. Everything marked P0/P1 = no prioritization. + +## Consolidation (epic review) +Target ~20-40% ticket reduction. Merge when: **scope overlap** (same code in same PR), **sequential dependency** (always done together), or **trivial standalone** (few lines, not a separate module). Confirm: the smaller ticket's criteria fit the larger, no separate assignee/timeline, work is truly sequential. diff --git a/domains/pr-workflow/skills/.gitkeep b/domains/pr-workflow/skills/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/domains/pr-workflow/skills/commit-discipline/skill.md b/domains/pr-workflow/skills/commit-discipline/skill.md new file mode 100644 index 00000000..27593248 --- /dev/null +++ b/domains/pr-workflow/skills/commit-discipline/skill.md @@ -0,0 +1,59 @@ +--- +maturity: experimental +name: commit-discipline +description: Atomic commits, granular commit chain construction, and pre-push lint discipline +--- + +# Commit Discipline + +## When To Use + +- Creating commits for a PR +- Splitting or reorganizing work before pushing +- Reviewing a commit chain before requesting review + +## Do Not Use When + +- The PR will be squash-merged via the GitHub UI (the merge bot owns final commit shape) +- WIP commits on a personal branch with no PR planned + +## Atomic Commit Rule + +Each commit must contain changes that share a single motivation. + +**Test:** Can you describe this commit's purpose in one sentence without `and`? +- "Fix selector memoization breaking cascade" → ✓ atomic +- "Fix selector and clean up test fixtures" → ✗ split it + +Exception: causally linked changes belong together (e.g., a type change that forces a test update). + +## Commit Chain Construction + +Build commits by working backward from desired final state: + +1. Verify the desired final state (all changes in place, tests pass) +2. Restore to base (`git stash` or `git reset HEAD~n`) +3. Build each commit in order, verifying state at each step +4. Confirm final committed state matches desired final state exactly + +Don't commit incrementally during development — categorize changes first, then assemble. + +**Mechanics:** use porcelain `git commit`. It runs hooks, honors signing config (`commit.gpgsign`), and applies commit templates. Don't hand-assemble commits with `git commit-tree` / `git write-tree` — that bypasses hooks and config. Leave commit trailers (`Co-authored-by`, `Signed-off-by`) in place; don't strip attribution. + +## Pre-Push Checklist + +- [ ] Each commit passes standalone (`git stash --keep-index && yarn test:unit`) +- [ ] No lint errors on changed files +- [ ] No test changes that weaken assertions (specs are guardrails, not obstacles) +- [ ] Commit messages describe motivation, not diff inventory + +## Common Pitfalls + +| Mistake | Correct Approach | +|---------|-----------------| +| Bundling orthogonal changes (bug fix + lint cleanup) | Separate commits — they don't share motivation | +| Committing incrementally during development | Organize after the work, not during | +| Message like "Fix tests" | "Fix `getAccounts` selector to stop returning new reference on equal input" | +| Modifying tests to make them pass | Fix the implementation; tests are the spec | +| Hand-building commits with `git commit-tree` | Use `git commit` — it honors hooks, signing, and templates | +| Stripping trailers (`Co-authored-by`, `Signed-off-by`) | Keep trailers — preserve attribution and sign-off | diff --git a/domains/pr-workflow/skills/diff-audit/skill.md b/domains/pr-workflow/skills/diff-audit/skill.md new file mode 100644 index 00000000..b9f5f589 --- /dev/null +++ b/domains/pr-workflow/skills/diff-audit/skill.md @@ -0,0 +1,70 @@ +--- +maturity: experimental +name: diff-audit +description: Systematic file-by-file PR diff review — categorize, justify, flag out-of-scope changes +--- + +# Diff Audit + +## When To Use + +- Reviewing any PR before approving — especially agent-authored PRs +- Self-reviewing before requesting review +- A PR diff looks disproportionately large for the stated goal + +## Do Not Use When + +- Automated release PRs with expected wide file changes (version bumps, changelog) +- Reviewing a single-file hotfix with obvious scope + +Concrete trigger: PR #41558 had 7 changes outside ticket scope (~2.5h reviewer time). A systematic diff audit would have caught all on first pass. + +## Workflow + +1. Run `git diff --stat ...HEAD` (or check the PR Files tab) +2. For each file, assign a category from the table below +3. Apply the category's check — if it fails, flag for revert or justification +4. Report: list of flagged files with category and reason + +## File Categories + +| Category | Examples | Check | +|----------|----------|-------| +| In-scope feature code | Files in ticket's target directories | Should match ticket scope declaration | +| Mechanically required | Import updates, type re-exports forced by in-scope changes | Would the in-scope change compile without this? | +| Test updates | Test files for changed code | Do assertions test new behavior, not weaken old? | +| Dependencies | package.json, yarn.lock | Only declared deps changed? No version downgrades? | +| Build/test config | jest.config, webpack, tsconfig, babel | Why does the feature need config changes? (Red flag — see `specifications-as-guardrails`) | +| Release artifacts | attribution.txt, CHANGELOG.md, version fields | Is this a release PR? If no, revert | +| Generated files | yarn.lock, lockfiles | Proportional to declared dep changes? | +| Dev artifacts | PR_DESC.md, .cursor/*, .vscode/*, local paths | Should not be committed — revert | + +## Triage Per File + +1. Is this file in the declared scope? +2. If not, is it mechanically required by an in-scope change? +3. If not, does it affect shared infrastructure? +4. If yes to #3, why don't other consumers need this change? +5. Would reverting this file break the feature? If no, revert it. + +## Agent-Specific Checks + +Agent-authored PRs (Cursor, Copilot, etc.) have predictable failure modes. Add these checks: + +| Check | What to look for | +|-------|-----------------| +| Platform convention violations | New patterns that duplicate existing utilities (`useAsyncResult`, `StorageService`) | +| Rebase damage | Dependency downgrades, removed packages not in ticket scope | +| Config pollution | `transformIgnorePatterns`, webpack aliases added for one feature | +| Stale references | Imports from moved/renamed files the agent found via outdated index | +| Dev artifact leakage | PR_DESC.md, .cursor/*, filesystem paths in committed files | + +## Common Pitfalls + +| Mistake | Correct Approach | +|---------|-----------------| +| Reviewing feature logic before checking scope | First pass is always scope audit — flag tangential files before reading code | +| Accepting config changes because "tests pass now" | Ask why other consumers don't need the same config change | +| Ignoring yarn.lock diff size | Large yarn.lock delta with small package.json delta = rebase damage | +| Treating agent anti-patterns as code quality issues | They're scope violations — the pattern exists, the agent didn't find it | +| Skipping generated file review | Generated files can encode dependency corruption silently | diff --git a/domains/pr-workflow/skills/pr-review-discipline/skill.md b/domains/pr-workflow/skills/pr-review-discipline/skill.md new file mode 100644 index 00000000..0fa344ab --- /dev/null +++ b/domains/pr-workflow/skills/pr-review-discipline/skill.md @@ -0,0 +1,41 @@ +--- +maturity: experimental +name: pr-review-discipline +description: Discipline for reviewing PRs — verify the mechanism before judging, triage findings before drafting comments, don't overcorrect after being wrong, challenge workarounds +--- + +# PR Review Discipline + +## When To Use + +- Reviewing a PR before drafting review comments +- Evaluating a bot / automated finding (Bugbot, cursor[bot]) +- Deciding whether a flagged issue is real + +## Do Not Use When + +- Authoring your own PR (see `pr-description`, `commit-discipline`) +- The change is trivial / docs-only + +## Principles + +### Verify the mechanism before judging +Don't conflate similar-sounding entities. Enumerate the distinct things, state what each does, **then** evaluate. (Failure mode: dismiss a finding by pattern-matching names, then overcorrect into an unnecessary fix — both from skipping this step.) + +### Don't overcorrect after being wrong +Being wrong about a mechanism doesn't mean the opposite fix is needed. (1) update understanding, (2) evaluate whether the corrected understanding implies a real **runtime** problem, (3) only then propose a fix. + +### Triage before drafting comments +After a full review, present a triaged list (worth raising vs not) and let the author confirm before drafting full comments with suggestions. Unfiltered findings add noise. + +### Challenge workarounds before accepting them +Question the premise of a workaround (e.g. a test-config exclusion) — it may be masking a deeper issue that doesn't need the workaround at all. + +## Common Pitfalls + +| Mistake | Correct approach | +|---|---| +| Pattern-match on names, then dismiss/accept a finding | Enumerate distinct entities first, then judge | +| Propose the opposite fix after being wrong | Re-evaluate whether a real problem exists at runtime | +| Draft every finding as a comment | Triage first; confirm what's worth raising | +| Accept a workaround as necessary | Challenge the premise — is it masking the real issue? |