Skip to content

fix(planning): install planning skills on MetaMask Extension and narrow triggers - #219

Merged
mganchas merged 13 commits into
mainfrom
fix/narrow-mms-planning
Nov 6, 2026
Merged

mganchas merged 13 commits into
mainfrom
fix/narrow-mms-planning

Conversation

@mganchas

@mganchas mganchas commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Description

Make the planning skill work on MetaMask Extension as well as MetaMask Mobile, and narrow when it triggers.

  • planning, planning-spec and jira-brief install for MetaMask Extension with per-repo overlays. Shared body, same rules in both repos. Core stays out of scope.
  • planning, planning-spec and jira-brief are maturity: experimental with no base, so a default stable sync installs none of them. planning triggers only when scope is ambiguous, the change is large, the work is security-sensitive or performance-sensitive, a contract or API is being changed, or the engineer asks for it.
  • Security and performance stop for agreement before the edit, even when fully specified. A contract or API change, including a new public method, stops for a brief even when fully specified. A spec is only for a large change.
  • large is tightened to a new product area, or a cross-system design with undecided scope.
  • Cheap uncertainty (whether the result is kept, or whether an existing spec, plan or ticket covers the change) proceeds and ends with one confirmation line. Risk uncertainty stops for a short brief and does not recommend a spec.
  • Catalog step reads every installed mms-* description on every path, and follows a matching mms-* skill over a non-mms-* one. If a later step is reached before that read, the catalog runs first. The fixed mms-performance read is dropped, since that skill does not install for Extension. On Mobile, performance-sensitive work still reads mms-performance.
  • The line A Jira ticket would improve this context. is removed. jira-brief reads a ticket when one is identified; otherwise planning continues without asking.

Type of Change

  • Skill improvement/update

Testing

Latest check, on 1d402d6: 19 scenarios × 3 labs. The description repair is in that commit: the accepted suggestion had split the ambiguous-scope sentence and repeated the security line. The contract trigger and the catalog recovery sentence stayed. The only run comment has that table. The table below is the first 54 runs. Labs: Claude Sonnet 5.5 (high), GPT 5.6 Terra (medium), Grok 4.7 (high). Skills were installed locally in metamask-mobile and metamask-extension checkouts with tools/install --include planning/<skill>.

Outcome per scenario. Labels: "opened, proceeded" = skill read and triggered, then the agent continued without a pause; "note, proceeded" = continued with the closing note; "brief, no spec" = short brief, no spec recommended; "short brief, no spec" = one-line pause for agreement; "spec" = planning spec recommended; "did not open" = skill not triggered.

Scenario Claude GPT Grok
Typo, Mobile opened, proceeded did not open did not open
Typo, Extension did not open opened, proceeded did not open
Ambiguous send, Mobile brief, no spec brief, no spec brief, no spec
Ambiguous swap, Mobile brief, no spec brief, no spec brief, no spec
Vault, decision pending short brief, no spec brief, no spec brief, no spec
Keyring, decision pending brief, no spec brief, no spec brief, no spec
Explicit plan, Mobile brief, no spec brief, no spec brief, no spec
Explicit plan, Extension brief, no spec brief, no spec brief, no spec
Label, Mobile did not open opened, proceeded did not open
Label, Extension opened, proceeded opened, proceeded did not open
New area: prediction markets spec spec spec
Jest (mechanical) opened, proceeded opened, proceeded did not open
Vault, locked short brief, no spec short brief, no spec short brief, no spec
Public method on keyring short brief, no spec brief, no spec brief, no spec
New area: household, Mobile spec spec spec
New area: household, Extension spec spec spec
Cheap uncertainty, Mobile (send amount) brief, stopped note, proceeded note, proceeded
Ticket EXT-4401, Extension note, proceeded note, proceeded note, proceeded

Notes:

  • Security-sensitive scenarios paused every time, without a spec.
  • Over-triggering on mechanical work: the skill opened on typo, label and Jest scenarios in several runs, but then proceeded. No pause happened on those. Known gap: the Do not use this skill before every edit line does not fully stop this yet (7 of 15 mechanical runs opened the skill).
  • Claude stopped for a brief on cheap uncertainty (send amount, Mobile). GPT and Grok proceeded with the note. The request was ambiguous, so the brief is defensible. This is a lab divergence, not a rule-level change.
  • Claude used a short brief for the keyring public method. GPT and Grok wrote a full brief. None recommended a spec.
  • Large-scope judgement differs between labs on some borderline cases. The table records what each run did.

Validation run locally:

  • yarn test: 76/76 pass.
  • yarn lint:changelog: pass.
  • yarn audit:skills: 0 errors.

Limitations:

  • The table is the first 54 runs, taken before the Jira-line removal. Later checks are summarised here: on 9d2bc38, spec and Jira paths still fired and no ticket was requested; on 7613162, the clear performance case and the borderline selector case paused, the plain UI change did not, and ticket and no-ticket paths behaved on Mobile and Extension. The only run comment is the latest, on 1d402d6: 19 scenarios times 3 labs.

Checklist

  • I have read the CONTRIBUTING.md guidelines
  • My skill follows the SKILL_TEMPLATE.md format
  • I have tested this skill with an AI agent
  • My skill does not contain any secrets, private keys, or sensitive data
  • I have added appropriate documentation
  • My changes don't break existing skills

…ow triggers

- Install planning, planning-spec and jira-brief for MetaMask Extension with per-repo overlays. Core stays out of scope.
- planning is experimental with no base; it triggers only for ambiguous, large, security-sensitive, performance-sensitive or explicitly requested work.
- Security and performance stop for agreement before the edit, with a spec only when also large.
- Cheap uncertainty proceeds with a closing note; risk uncertainty stops for a short brief.
- Catalog reads every installed mms-* description and follows the matching skill; drop the fixed mms-performance read.
- Remove the Jira nudge line; jira-brief reads a ticket when one is identified and planning continues otherwise.
… performance trigger

Narrow the performance-sensitive trigger from any screen to a screen's
render or data path. Mark planning-spec and jira-brief experimental so
they no longer install without planning. Fold the planning changelog
entries and log the dropped -pt.md instruction.
…nsitive

Reading an existing selector was pausing fully specified UI that only
displays data already in the store.
A screen render still counted as performance-sensitive when it only
showed a value already in the store. A new or changed selector still stops.
Treat uncertainty about performance as risk uncertainty. On Mobile, read
mms-performance when the work is performance-sensitive.
@NicolasMassart

Copy link
Copy Markdown
Contributor

Explanation of my above comments:

issue (blocking): Contract/API changes should explicitly trigger planning

The workflow already correctly states that contract/API changes require a brief, even when fully specified, but don't require a spec unless the change is also large.

However, contract/API changes are missing from the skill's trigger conditions, meaning mms-planning might never activate for these cases.

I'd suggest adding contract/API changes to both the frontmatter description and the When to use section.

This would make the trigger conditions consistent with the existing workflow without broadening the scope unnecessarily or requiring further changes.

mganchas and others added 6 commits November 6, 2026 13:47
Co-authored-by: Nico MASSART <NicolasMassart@users.noreply.github.com>
Co-authored-by: Nico MASSART <NicolasMassart@users.noreply.github.com>
Co-authored-by: Nico MASSART <NicolasMassart@users.noreply.github.com>
Co-authored-by: Nico MASSART <NicolasMassart@users.noreply.github.com>
The accepted description edit dropped the ambiguous-scope sentence and
repeated the security line. Contract or API changes stay a trigger and
stop for a brief. The catalog still runs first if a later step starts early.
@mganchas

mganchas commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Re-check on 1d402d6

This is the only run comment. Earlier evidence comments were removed. 19 scenarios × 3 labs. The first 18 are the original set. The last one is new: a fully specified public method that is not security-sensitive.

Scenario Claude GPT Grok
Typo, Mobile did not open opened, proceeded did not open
Typo, Extension did not open opened, proceeded did not open
Ambiguous send, Mobile full brief, no spec full brief, no spec full brief, no spec
Ambiguous swap, Extension full brief, no spec full brief, no spec full brief, no spec
Vault, shape not decided full brief, no spec full brief, no spec full brief, no spec
Keyring, decision pending full brief, no spec full brief, no spec full brief, no spec
Explicit plan, Mobile full brief, no spec full brief, no spec full brief, no spec
Explicit plan, Extension full brief, no spec full brief, no spec short brief, no spec
Label, Mobile did not open opened, proceeded did not open
Label, Extension did not open opened, proceeded did not open
New area: prediction markets brief, spec spec spec
Jest, routine CI opened, proceeded opened, proceeded did not open
Vault, re-encrypt, fully specified short brief, no spec short brief, no spec short brief, no spec
Public method on keyring full brief, no spec full brief, no spec full brief, no spec
New area: household, Mobile brief, spec spec brief, spec
New area: household, Extension brief, spec brief, spec brief, spec
Cheap uncertainty, keep the result full brief, no spec proceeded cheap note, proceeded
Ticket might cover the change cheap note, proceeded cheap note, proceeded cheap note, proceeded
Public method, not security, fully specified full brief, no spec full brief, no spec full brief, no spec

The new contract trigger works: the non-security public method stopped for a brief with no spec in all three labs. Security still stops without a spec. Large areas still recommend a spec. Mechanical work still opens on GPT, then proceeds. That known gap is unchanged.

@mganchas
mganchas merged commit 25d814d into main Nov 6, 2026
28 checks passed
@mganchas
mganchas deleted the fix/narrow-mms-planning branch November 6, 2026 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants