feat(perps): share an explicit static review checklist by abretonc7s · Pull Request #163 · MetaMask/skills

abretonc7s · 2026-09-17T01:40:57Z

Description

Perps static review is generated from the canonical Perps library (source revision and digests stamped): a small shared skill.md (855 words, including a "Signal over noise" row against AI-slop comments, ticket keys in source and padded text), one overlay per repository in repos/<repo>.md (Mobile 265, Extension 630, Core 427 words), each criterion's full rule in references/criteria/<family>/<slug>.md read only when the diff touches that family, and one complete template per client at review-pr/static-perps.<mobile|extension|core> for a control plane. No run mode anywhere. An optional analyzer copy can be produced with --analyzer-out --client.

The installer supports explicit-only invocation, preserves repository overlays, and removes managed copies of the retired PR-only QA skill when installing recipe-qa.

Type of Change

Skill improvement/update
Bug fix
Documentation update

Checklist

Required skill metadata is present and validates.
No secrets or private keys added.
Documentation and regeneration instructions included.
Unrelated skill selection behavior preserved.
Interactive runner behavior exercised. Native controls are checked through generated install artifacts; no model smoke runs were launched.

Testing

Full local skills suite passed, plus targeted installer update/retirement tests.
Canonical skill/template/analyzer equivalence and drift checks passed.
Shared-runtime materialization and completion-contract checks passed.
Independent code and architecture reviews completed against this commit.

Additional Context

Coordinated review: https://github.com/Consensys/skills/pull/74

Harness proof and tutorials (merged, published as 0.52.0): https://github.com/MetaMask/experimental-metamask-harness/pull/264

Farm wrapper: https://github.com/MetaMask/experimental-metamask-farm/pull/56

Raw source bundles also retain native explicit-invocation policy. Verified through the real support collector/installer for all three farms, with no bundled app harness or recipe-library requirement.

The generated review is now a small common body (about 850 words) with one overlay per client (repos/metamask-mobile.md, repos/metamask-extension.md, repos/core.md) and one file per criteria family under references/criteria, read only when the diff touches that family. A reviewer receives 1,100 to 1,450 words instead of 6,854. Each client has its own execution template, review-pr/static-perps.<client>, byte-identical to the installed skill; the test holds that for all three clients. The base review carries the same signal-over-noise row as the harness base review.

Harness 0.52.0 is published. Deploy the reviewed skills before applying the farm migration.

Release note: @metamask/skills 0.3.x runs the installer bundled in the npm package, so the explicit-invocation controls and the removal of the retired QA wrapper reach consumers only after this merges, a new @metamask/skills is published, and MetaMask Mobile (^0.3.1) and Extension (^0.1.0) bump to it. Until then yarn skills installs the new skill content without explicit-invocation flags. Live farm defaults are unchanged.

aganglada

Three fixes before merge:

aganglada · 2026-09-17T07:56:41Z

+    assert.match(readFileSync(path.join(out, 'agents/openai.yaml'), 'utf8'), /allow_implicit_invocation: false/);
+    const skill = readFileSync(path.join(out, 'skill.md'), 'utf8');
+    const template = readFileSync(path.join(out, 'references/templates/review-pr/static-perps.md'), 'utf8');
+    assert.equal(body(skill), body(template));


This only checks a temp library. Also assert the committed skill.md body equals references/templates/review-pr/static-perps.md so CI catches hand-edits.

Done in 3b031d1. New test asserts the committed skill.md and references/templates/review-pr/static-perps.md carry the same checklist. Checked it fails on a hand edit to the skill.

aganglada · 2026-09-17T07:56:41Z

+      echo 'policy:'
+      echo '  allow_implicit_invocation: false'
+    fi
+  } > "$dir/agents/openai.yaml"


This overwrites agents/openai.yaml for every user-scoped Codex skill. Write or replace it only when disable-model-invocation is set (or merge), so a reinstall does not clobber a custom policy.

Done in 3b031d1. The user-scoped agents/openai.yaml is written only for an explicit-only skill and carries an installer banner. On downgrade it is removed only when that banner is present, so a hand-written policy survives a reinstall. Test covers both; it fails if the banner check is dropped.

aganglada · 2026-09-17T07:56:41Z

+
+Set `disable-model-invocation: true` in source skill frontmatter to require explicit invocation. The installer emits the native Claude flag, a Codex `agents/openai.yaml` policy with `allow_implicit_invocation: false`, and a manual Cursor `.mdc` rule without description or glob matching. Other skills retain their existing selection behavior. Installation does not activate a workflow.
+
+Perps review carries a generated phased checklist and a catalog at `references/templates`. Its canonical rules remain in the Perps recipe library. Regenerate from a clean library revision with `node domains/perps/skills/perps-review-pr/scripts/materialize-review.mjs --library <library>`; add `--check` in validation. An explicitly configured analyzer can receive the same content with `--analyzer-out <consumer>/.ai-pr-analyzer/prompt-context.md`. Review that consumer change before enabling it.


--check against the recipe library is not runnable in this repo’s CI. Document that public validation is the committed skill/template equality check, and who regenerates from the library.

Done in 3b031d1. README now says the public check is the committed skill/template equality test, and that Perps skill maintainers regenerate from a clean library revision and run --check where the library is available.

… review A user-scoped reinstall rewrote ~/.codex/skills/<name>/agents/openai.yaml for every skill. Write it only for an explicit-only skill, mark it as installer-owned, and remove it on downgrade only when that mark is present, so a policy the engineer wrote survives. The generator test used a temporary library only. Assert that the committed skill and its execution template carry the same checklist, so a hand edit to either fails in CI, and say in the README that this equality is the public check while maintainers regenerate from the library with --check.

…overlays The generated review skill was one 6,854-word file that every client received, so a Mobile reviewer read 2,163 words of Extension and Core criteria only to mark them not applicable. The repo's own design is repo overlays merged at install. Generate a common body of about 850 words, one overlay per client that ends with the verdict rows, and one file per criteria family under references/criteria read only when the diff touches that family. A reviewer now receives 1,100 to 1,450 words. Each client gets its own execution template, review-pr/static-perps.<client>, byte-identical to what the installer produces, and a test holds that for all three. The base review gains the same signal-over-noise row as the harness base review. The analyzer output requires --client and inlines the full text.

abretonc7s · 2026-09-17T16:04:26Z

Pre-review pass (17 Sept)

A full read of the end state of this branch before asking for review.

Checked, nothing to change:

No run-mode language (runMode, autonomous/interactive, --mode) in the generated skill, overlays, criteria, templates, installer or tests. The only "interactive" hits are the README's domain picker and a testID rule about interactive UI elements.
No control-plane mentions in the Perps skill, tools/install or the README.
The three requested changes are in 3b031d1 and answered inline: committed skill/template equality test, user-scope agents/openai.yaml written only for explicit-only skills and removed only when it carries the installer banner, README naming the public check and who regenerates. The client-agnostic static-perps.md the test mentions is asserted absent; the per-client set replaced it in 6e453f8.
Word counts: skill.md 855 (main 622: the base grew by the phase structure and the Signal-over-noise row; the repo-specific families moved out to repos/metamask-mobile.md 265, repos/metamask-extension.md 630, repos/core.md 427; 30 criteria files, largest 451 words, read on demand).

Fixed in 5003f6d: materialize-review.mjs --check against the library reported the committed files stale because the library main had moved from c991d06 to cac185c (a recipe fix; review/ unchanged). Regenerated: 5 files, 5 lines, revision stamps only, which also shows the committed output is reproducible from a clean library revision.

Not changed on purpose: nothing else was found that is a factual or text error; structural or test suggestions are out of scope for this PR. PR description refreshed to describe the small body + overlays + per-client templates.

… review

abretonc7s · 2026-09-18T02:24:38Z

91a4cd3: tools/install now prunes every retired installed name on sync (mms-recipe-pr-qa-review, mms-recipe-evidence, mms-recipe-fixbug, mms-recipe-quality), once before the install loop, keeping the managed-banner guard. Before this, only the QA review name was removed, so the renames in Consensys/skills#74 left the old directories installed beside the new ones on every checkout that had synced earlier; an agent could still invoke /mms-recipe-fixbug. yarn skills execs into this installer, which is why the same list in the Consensys copy did not take effect. Explicit-invocation test passes.

abretonc7s added 2 commits September 17, 2026 09:30

feat(perps): share an explicit static review checklist

1e61e18

fix(perps): retain explicit invocation in raw skill bundles

47362fe

aganglada approved these changes Sep 17, 2026

View reviewed changes

aganglada requested changes Sep 17, 2026

View reviewed changes

abretonc7s added 2 commits September 17, 2026 16:04

abretonc7s mentioned this pull request Sep 17, 2026

refactor(templates)!: remove run mode as a template property deeeed/farmslot#660

Merged

chore(perps): regenerate the review from library cac185c

5003f6d

fix(install): prune every retired skill name on sync, not only the QA…

91a4cd3

… review

Navigation Menu

Uh oh!

feat(perps): share an explicit static review checklist - #163

feat(perps): share an explicit static review checklist#163
abretonc7s wants to merge 6 commits into
mainfrom
feat/explicit-review-template

abretonc7s commented Sep 17, 2026 •

edited

Loading

Uh oh!

aganglada left a comment

Uh oh!

aganglada Sep 17, 2026

Uh oh!

abretonc7s Sep 17, 2026

Uh oh!

aganglada Sep 17, 2026

Uh oh!

abretonc7s Sep 17, 2026

Uh oh!

aganglada Sep 17, 2026

Uh oh!

abretonc7s Sep 17, 2026

Uh oh!

abretonc7s commented Sep 17, 2026

Uh oh!

abretonc7s commented Sep 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		Set `disable-model-invocation: true` in source skill frontmatter to require explicit invocation. The installer emits the native Claude flag, a Codex `agents/openai.yaml` policy with `allow_implicit_invocation: false`, and a manual Cursor `.mdc` rule without description or glob matching. Other skills retain their existing selection behavior. Installation does not activate a workflow.

		Perps review carries a generated phased checklist and a catalog at `references/templates`. Its canonical rules remain in the Perps recipe library. Regenerate from a clean library revision with `node domains/perps/skills/perps-review-pr/scripts/materialize-review.mjs --library <library>`; add `--check` in validation. An explicitly configured analyzer can receive the same content with `--analyzer-out <consumer>/.ai-pr-analyzer/prompt-context.md`. Review that consumer change before enabling it.

Uh oh!

Conversation

abretonc7s commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of Change

Checklist

Testing

Additional Context

Uh oh!

aganglada left a comment

Choose a reason for hiding this comment

Uh oh!

aganglada Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

abretonc7s Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

aganglada Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

abretonc7s Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

aganglada Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

abretonc7s Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

abretonc7s commented Sep 17, 2026

Pre-review pass (17 Sept)

Uh oh!

abretonc7s commented Sep 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

abretonc7s commented Sep 17, 2026 •

edited

Loading