From 4d0931dd6268bd3df836a2614a674b4b7d5126ed Mon Sep 17 00:00:00 2001 From: Arthur Breton Date: Mon, 21 Sep 2026 20:37:00 +0800 Subject: [PATCH] feat(installer): support explicit-only workflow invocation --- CHANGELOG.md | 1 + README.md | 5 ++ test/explicit-invocation.test.mjs | 90 +++++++++++++++++++++++++++++++ tools/install | 89 ++++++++++++++++++++++++++++-- tools/skill-schema.mjs | 6 +-- 5 files changed, 184 insertions(+), 7 deletions(-) create mode 100644 test/explicit-invocation.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d552444..e7367690 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Support explicit-only workflow skills through native invocation controls, preserve repository overlays, and prune managed retired skill names during sync. - Add `navigation` skill with a repo-agnostic base and a MetaMask Mobile overlay for `Routes` and `NavigationService`. Marked `base: true` so it installs even when its domain is filtered out. - Add `feature-flags` skill with a repo-agnostic base and a MetaMask Mobile overlay for version-gated remote flags. Marked `base: true` so it installs even when its domain is filtered out. ([#147](https://github.com/MetaMask/skills/pull/147)) - Add `analytics` skill (`platform/analytics`, moved from `coding`) with a repo-agnostic base and a MetaMask Mobile overlay for the canonical tracking API. Marked `base: true` so it installs even when its domain is filtered out. ([#140](https://github.com/MetaMask/skills/pull/140)) diff --git a/README.md b/README.md index 1e82a148..79cf7749 100644 --- a/README.md +++ b/README.md @@ -432,3 +432,8 @@ MIT — see [LICENSE](LICENSE). See [SECURITY.md](SECURITY.md) for how to report issues with skills or the installer. + + +### Explicit-only workflows + +Set `disable-model-invocation: true` in source skill frontmatter to require explicit invocation. The installer emits the native invocation controls for each supported runner. Other skills retain their existing selection behavior. Installation does not activate a workflow. diff --git a/test/explicit-invocation.test.mjs b/test/explicit-invocation.test.mjs new file mode 100644 index 00000000..bb7e3486 --- /dev/null +++ b/test/explicit-invocation.test.mjs @@ -0,0 +1,90 @@ +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, mkdirSync, readFileSync, writeFileSync, existsSync, rmSync } from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { test } from 'node:test'; + +const installer = process.env.SKILLS_INSTALLER || fileURLToPath(new URL('../tools/install', import.meta.url)); + +test('explicit-only policy survives install, overlay and reinstall on every target', () => { + const root = mkdtempSync(path.join(os.tmpdir(), 'skills-invocation-')); + try { + const source = path.join(root, 'source'); + const target = path.join(root, 'target'); + const userRoot = path.join(root, 'user'); + for (const dir of [target, `${userRoot}/.claude`, `${userRoot}/.codex`]) mkdirSync(dir, { recursive: true }); + // Isolate home destinations without changing HOME or writing operator skills. + const isolatedInstaller = path.join(root, 'install'); + writeFileSync(isolatedInstaller, readFileSync(installer, 'utf8').replaceAll('$HOME/.claude', `${userRoot}/.claude`).replaceAll('$HOME/.codex', `${userRoot}/.codex`)); + function skill(name, explicit, scope = '') { + const dir = path.join(source, 'domains/test/skills', name); + mkdirSync(dir, { recursive: true }); + writeFileSync(path.join(dir, 'skill.md'), `---\nname: ${name}\ndescription: Test workflow\nmaturity: stable\n${scope ? `scope: ${scope}\n` : ''}${explicit ? 'disable-model-invocation: true\n' : ''}---\n# Workflow\nDo the requested work.\n`); + return dir; + } + const workflow = skill('workflow', false); + skill('reference', false); + skill('personal', true, 'user'); + skill('recipe-pr-qa-review', false); + mkdirSync(path.join(workflow, 'repos')); + writeFileSync(path.join(workflow, 'repos/core.md'), '# Core context\nUse the configured target.\n'); + function install(...extra) { + const result = spawnSync('/bin/bash', [isolatedInstaller, '--source', source, '--target', target, '--repo', 'core', '--include-user', ...extra], { encoding: 'utf8' }); + assert.equal(result.status, 0, result.stdout + result.stderr); + } + const agentFile = `${target}/.agents/skills/mms-workflow/agents/openai.yaml`; + install(); + const oldSkill = `${target}/.agents/skills/mms-recipe-pr-qa-review`; + assert.ok(existsSync(oldSkill)); + rmSync(path.join(source, 'domains/test/skills/recipe-pr-qa-review'), { recursive: true }); + skill('recipe-qa', true); + install('--dry-run'); + assert.ok(existsSync(oldSkill)); + install(); + for (const base of ['.agents/skills', '.claude/skills', '.cursor/rules']) { + assert.equal(existsSync(`${target}/${base}/mms-recipe-pr-qa-review`), false); + } + mkdirSync(oldSkill); + writeFileSync(path.join(oldSkill, 'SKILL.md'), '# Hand-authored custom skill'); + install(); + assert.ok(existsSync(oldSkill), 'never delete hand-authored skills'); + assert.ok(!readFileSync(agentFile, 'utf8').includes('allow_implicit_invocation: false')); + skill('workflow', true); + install('--dry-run'); + assert.ok(!readFileSync(agentFile, 'utf8').includes('allow_implicit_invocation: false')); + install(); + for (const base of ['.claude/skills', '.agents/skills']) { + const text = readFileSync(`${target}/${base}/mms-workflow/SKILL.md`, 'utf8'); + assert.match(text, /^disable-model-invocation: true$/m); + assert.match(text, /Core context/); + assert.doesNotMatch(readFileSync(`${target}/${base}/mms-reference/SKILL.md`, 'utf8'), /disable-model-invocation/); + } + assert.match(readFileSync(agentFile, 'utf8'), /policy:\n allow_implicit_invocation: false/); + const cursor = `${target}/.cursor/rules/mms-workflow`; + const frontmatter = readFileSync(`${cursor}/mms-workflow.mdc`, 'utf8').split('---')[1]; + assert.match(frontmatter, /alwaysApply: false/); + assert.doesNotMatch(frontmatter, /description:|globs:/); + assert.equal(existsSync(`${cursor}/RULE.md`), false); + assert.match(readFileSync(`${userRoot}/.claude/skills/mms-personal/SKILL.md`, 'utf8'), /disable-model-invocation: true/); + assert.match(readFileSync(`${userRoot}/.codex/skills/mms-personal/agents/openai.yaml`, 'utf8'), /allow_implicit_invocation: false/); + skill('workflow', false); + skill('personal', false, 'user'); + install(); + assert.equal(existsSync(`${cursor}/mms-workflow.mdc`), false); + assert.ok(existsSync(`${cursor}/RULE.md`)); + assert.doesNotMatch(readFileSync(agentFile, 'utf8'), /allow_implicit_invocation: false/); + // The installer removes the policy file it wrote once the skill is no longer explicit-only. + const userPolicy = `${userRoot}/.codex/skills/mms-personal/agents/openai.yaml`; + assert.equal(existsSync(userPolicy), false); + // A policy the engineer wrote by hand has no installer banner and survives a reinstall. + const custom = 'policy:\n allow_implicit_invocation: true\n'; + mkdirSync(path.dirname(userPolicy), { recursive: true }); + writeFileSync(userPolicy, custom); + install(); + assert.equal(readFileSync(userPolicy, 'utf8'), custom); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); diff --git a/tools/install b/tools/install index 85cbd959..acb05934 100755 --- a/tools/install +++ b/tools/install @@ -129,6 +129,8 @@ USER_CLAUDE_DIR="$HOME/.claude/skills" USER_CODEX_DIR="$HOME/.codex/skills" PREFIX="mms-" +# YAML files cannot carry the HTML banner; this line marks the ones the installer owns. +MANAGED_YAML_BANNER='# DO NOT EDIT: generated by MetaMask skills tools/install.' MANAGED_BANNER="" log() { echo " $1"; } @@ -176,6 +178,18 @@ body_after_frontmatter() { awk 'BEGIN{n=0} /^---$/{n++; next} n>=2{print}' "$1" } +# Body of an overlay file. Overlays normally have NO frontmatter (they start +# straight into Markdown), so strip a leading `---` block only when present; +# otherwise emit the whole file. Without this, a no-frontmatter overlay would be +# dropped entirely by body_after_frontmatter (which only prints after a 2nd `---`). +overlay_body() { + if [[ "$(head -1 "$1")" == "---" ]]; then + body_after_frontmatter "$1" + else + cat "$1" + fi +} + domain_allowed() { local domain="$1" # `none` selects no domains at all, leaving only `base: true` skills (which @@ -260,6 +274,7 @@ write_claude() { echo '---' echo "name: ${name}" yaml_field description "$description" + if is_truthy "${explicit_only:-false}"; then echo 'disable-model-invocation: true'; fi echo '---' echo "${MANAGED_BANNER}" echo "${content}" @@ -270,12 +285,19 @@ write_cursor() { local out_name="$1" description="$2" content="$3" local dir="$CURSOR_DIR/$out_name" local file="$dir/RULE.md" - action "cursor: .cursor/rules/$out_name/RULE.md" + # Explicit-only rules use Cursor's manual .mdc format, without matching metadata. + if is_truthy "${explicit_only:-false}"; then file="$dir/$out_name.mdc"; fi + action "cursor: .cursor/rules/$out_name/$(basename "$file")" $DRY_RUN && return mkdir -p "$dir" + if is_truthy "${explicit_only:-false}"; then + rm -f "$dir/RULE.md" + else + rm -f "$dir/$out_name.mdc" + fi { echo '---' - yaml_field description "$description" + if ! is_truthy "${explicit_only:-false}"; then yaml_field description "$description"; fi echo 'alwaysApply: false' echo '---' echo "${MANAGED_BANNER}" @@ -297,6 +319,7 @@ write_agents() { echo '---' echo "name: ${name}" yaml_field description "$description" + if is_truthy "${explicit_only:-false}"; then echo 'disable-model-invocation: true'; fi echo '---' echo "${MANAGED_BANNER}" echo "${content}" @@ -306,6 +329,10 @@ write_agents() { printf ' display_name: '; yaml_quoted "$name"; echo yaml_field ' short_description' "$description" printf ' default_prompt: '; yaml_quoted "Use \$${out_name} for this task."; echo + if is_truthy "${explicit_only:-false}"; then + echo 'policy:' + echo ' allow_implicit_invocation: false' + fi } > "$yaml" } @@ -324,6 +351,7 @@ write_user_claude() { echo '---' echo "name: ${name}" yaml_field description "$description" + if is_truthy "${explicit_only:-false}"; then echo 'disable-model-invocation: true'; fi echo '---' echo "${MANAGED_BANNER}" echo "${content}" @@ -342,10 +370,29 @@ write_user_codex() { echo '---' echo "name: ${name}" yaml_field description "$description" + if is_truthy "${explicit_only:-false}"; then echo 'disable-model-invocation: true'; fi echo '---' echo "${MANAGED_BANNER}" echo "${content}" } > "$file" + # ~/.codex belongs to the engineer. Write the policy file only for an + # explicit-only skill, and remove it only when this installer wrote it, so a + # reinstall never clobbers a policy the engineer authored. + local yaml="$dir/agents/openai.yaml" + if is_truthy "${explicit_only:-false}"; then + mkdir -p "$dir/agents" + { + echo "$MANAGED_YAML_BANNER" + echo 'interface:' + printf ' display_name: '; yaml_quoted "$name"; echo + yaml_field ' short_description' "$description" + printf ' default_prompt: '; yaml_quoted "Use \${out_name} for this task."; echo + echo 'policy:' + echo ' allow_implicit_invocation: false' + } > "$yaml" + elif [[ -f "$yaml" ]] && grep -Fqx -- "$MANAGED_YAML_BANNER" "$yaml"; then + rm -f "$yaml" + fi } copy_bundle_dirs() { @@ -414,7 +461,10 @@ expected_project_skill_contains() { is_managed_project_skill() { local dir="$1" label="$2" marker case "$label" in - .cursor/rules) marker="$dir/RULE.md" ;; + .cursor/rules) + marker="$dir/RULE.md" + [[ ! -f "$dir/$(basename "$dir").mdc" ]] || marker="$dir/$(basename "$dir").mdc" + ;; *) marker="$dir/SKILL.md" ;; esac [[ -f "$marker" ]] && grep -Fqx -- "$MANAGED_BANNER" "$marker" @@ -451,6 +501,33 @@ copy_user_bundles() { fi } +# recipe-qa replaces the old PR-only entry point. Leave hand-authored skills alone. +RETIRED_SKILLS=( + "mms-recipe-pr-qa-review|replaced by mms-recipe-qa" + "mms-recipe-evidence|renamed mms-recipe-pretty-pr" + "mms-recipe-fixbug|renamed mms-recipe-fix-bug" + "mms-recipe-quality|folded into the harness" +) + +remove_retired_skills() { + local entry name reason parent dir marker managed + for entry in "${RETIRED_SKILLS[@]}"; do + name="${entry%%|*}"; reason="${entry#*|}" + for parent in "$CLAUDE_DIR" "$CURSOR_DIR" "$AGENTS_DIR"; do + dir="$parent/$name" + [[ -d "$dir" ]] || continue + managed=false + for marker in "$dir/SKILL.md" "$dir/RULE.md" "$dir/$name.mdc"; do + if [[ -f "$marker" ]] && grep -Fqx -- "$MANAGED_BANNER" "$marker"; then managed=true; fi + done + if $managed; then + action "$dir ($reason)" + $DRY_RUN || rm -rf "$dir" + fi + done + done +} + process_skill() { local skill_dir="$1" domain_name="$2" local skill_name; skill_name=$(basename "$skill_dir") @@ -469,6 +546,7 @@ process_skill() { # source has migrated. [[ -z "$is_base" ]] && is_base=$(frontmatter_value "$base" "mandatory") local scope; scope=$(frontmatter_value "$base" "scope") + local explicit_only; explicit_only=$(frontmatter_value "$base" "disable-model-invocation") if skill_excluded "$domain_name" "$skill_name"; then log "skipped (explicitly excluded by --exclude/SKILLS_EXCLUDE)" @@ -538,7 +616,7 @@ process_skill() { local base_content; base_content=$(body_after_frontmatter "$base") local merged="$base_content" if [[ -f "$overlay" ]]; then - local overlay_content; overlay_content=$(body_after_frontmatter "$overlay") + local overlay_content; overlay_content=$(overlay_body "$overlay") merged="${merged} ${overlay_content}" @@ -662,6 +740,9 @@ if (( FAILED_SKILL_COUNT > 0 )); then exit 1 fi +# Prune before install so removed names never shadow a rename this run. +remove_retired_skills + current_domain="" for ((i = 0; i < ${#RESOLVED_KEYS[@]}; i++)); do domain_name="${RESOLVED_DOMAINS[$i]}" diff --git a/tools/skill-schema.mjs b/tools/skill-schema.mjs index 5ce8b65b..a2bd27aa 100644 --- a/tools/skill-schema.mjs +++ b/tools/skill-schema.mjs @@ -5,7 +5,7 @@ // in tools/install (Bash) mirrors BUNDLE_DIRS; keep the two in sync. export const REQUIRED_FRONTMATTER = ['name', 'description']; -export const OPTIONAL_FRONTMATTER = ['maturity', 'base', 'scope', 'metadata']; +export const OPTIONAL_FRONTMATTER = ['maturity', 'base', 'scope', 'metadata', 'disable-model-invocation']; // Pre-rename spelling of `base`, still READ by tools/install so that content from a // source on a different release cadence (the origin/main cache, a private overlay) @@ -37,10 +37,10 @@ export const BUNDLE_DIRS = ['references', 'scripts', 'assets', 'adapters', 'work // Directories allowed beside skill.md: the bundle dirs plus the repo-overlay // dir. Anything else is rejected, because the installer does not ship it and any -// reference to it would dangle post-install. `knowledge/` is not listed here +// reference to it would dangle post-install. Native agents/ policy is allowed for direct source consumers; the installer generates its own runner metadata. `knowledge/` is not listed here // because it is a per-DOMAIN directory, never a skill sibling; the installer // delivers it via copy_domain_knowledge. -export const ALLOWED_SIBLING_DIRS = [...BUNDLE_DIRS, 'repos']; +export const ALLOWED_SIBLING_DIRS = [...BUNDLE_DIRS, 'repos', 'agents']; export const KNOWN_REPOS = ['metamask-extension', 'metamask-mobile', 'core'];