Add git-backed builtin skill registry

docs/prd-builtin-skill-registry.md

@@ -0,0 +1,101 @@
+# PRD: Builtin Skill Registry And npx Skills Adapter
+
+## Overview / Problem Statement
+
+Ra can already discover and invoke project and personal `SKILL.md` files, but it does not yet have a first-class lifecycle for Ra-shipped builtin skills. It also does not expose the package-management workflow users expect from `npx skills`: adding, listing, finding, removing, and updating skills from remote or local sources.
+
+Ra should not reimplement the `npx skills` ecosystem manager. Instead, Ra should provide a small adapter that forwards skill-management commands to `npx skills`, while keeping Ra's runtime registry responsible for resolving which installed, configured, and builtin skills are active for a session.
+
+## Goals & Success Metrics
+
+- Ra ships a deterministic builtin skill registry that can be loaded without network access.
+- Ra can load a local clean git checkout as a skill registry and records its HEAD revision as skill provenance.
+- Project and global installed skills can shadow Ra builtin skills by command name.
+- Users can manage external skills through `ra skills ...` with behavior aligned to `npx skills`.
+- Project-level installs default to a path Ra already discovers.
+- Global installs are discoverable by Ra after installation.
+- Tests cover registry precedence, builtin disable/include/exclude behavior, and `npx skills` command forwarding argument construction.
+
+## User Personas & Stories
+
+- As a Ra user, I want useful builtin skills available on a fresh install without manually copying skill files.
+- As a project maintainer, I want committed project skills to override generic builtin guidance for the same workflow.
+- As a user of the open skills ecosystem, I want to run `ra skills add vercel-labs/agent-skills` and get the same installation semantics as `npx skills`.
+- As an operator, I want to disable builtin skills without deleting or modifying Ra-shipped files.
+
+## Functional Requirements
+
+| Priority | Requirement |
+| --- | --- |
+| Must | Add a builtin skill registry loaded from Ra-shipped skill definitions. |
+| Must | Treat builtin skills as virtual or embedded source entries, not as installed user files. |
+| Must | Resolve active skills with deterministic precedence: explicit config paths, project skills, global skills, local registry skills, then builtin skills. |
+| Must | Allow project or global skills to shadow builtin skills with the same slash command name. |
+| Must | Expose only the resolved active winner in the model-facing catalog and slash-command map. |
+| Must | Add config controls to enable/disable builtin skills and include/exclude builtin names. |
+| Must | Support an optional local registry path that must be the root of a git checkout and is skipped if it is not versioned by git. |
+| Must | Record the registry HEAD commit for each skill loaded from the local registry. |
+| Must | Skip registry loading when the registry checkout has uncommitted or untracked changes, so HEAD provenance describes the loaded content. |
+| Must | Preserve progressive disclosure: builtin skill bodies must not be dumped into the startup system prompt. |
+| Must | Support `ra skills add/list/find/remove/update/init` by forwarding to `npx skills`. |
+| Must | When forwarding `ra skills add` without an explicit `--agent`, default to `--agent codex` so installs land in `.agents/skills/` for project scope. |
+| Must | Add global Codex discovery for `~/.codex/skills/**/SKILL.md`, matching `npx skills` global Codex install location. |
+| Must | Keep explicit user `--agent` arguments intact and do not inject a default agent when the user already supplied one. |
+| Should | Pin the forwarded package invocation to a known `skills` npm version or make the version configurable. |
+| Should | Expose `ra skills list --builtin` to inspect Ra-shipped builtin skills and shadowing status. |
+| Should | Report shadowed builtin skills in diagnostics or JSON list output. |
+| Could | Provide `ra skills disable <name>` as a config-editing helper that writes a builtin exclude entry. |
+| Could | Support an environment variable equivalent to `INSTALL_INTERNAL_SKILLS` for internal builtin skills. |
+| Won't | Reimplement remote source parsing, package install, update, or removal logic already provided by `npx skills`. |
+
+## Non-Functional Requirements
+
+- Keep session startup deterministic and offline-capable.
+- Keep installed skill management separate from runtime skill resolution.
+- Avoid writing builtin skills into project or home directories unless explicitly requested by a future export command.
+- Preserve existing `.ra/skills`, `.agents/skills`, and `.claude/skills` discovery behavior.
+- Avoid surprising command rewriting: only add Ra defaults when the user did not specify the corresponding `npx skills` option.
+
+## Design Considerations
+
+`npx skills` is the ecosystem manager. It owns source formats, install/update/remove behavior, project/global scopes, agent target paths, and interactive flows. Ra should use it as a subprocess for management commands.
+
+Ra's runtime registry has two local sources. The builtin source is embedded with Ra and provides fallback skills. The local registry source is a clean git checkout root, analogous to the GitHub repositories used by `npx skills`, so skill versions are managed by commits, branches, tags, and ordinary git operations. Builtin and registry entries should behave like ordinary skills after resolution, including `disable-model-invocation`, `user-invocable`, runtime tool policy, hooks, model overrides, shell context, and fork behavior.
+
+## Technical Considerations
+
+The implementation should extend `src/skills.rs` with a `SkillSource` or equivalent provenance field so resolved entries can distinguish explicit, project, global, registry, and builtin sources. Registry-sourced skills should also carry the registry HEAD revision. `ResourceBundle::prompt_map` and `build_system_prompt` should operate on the resolved active list, not every discovered duplicate.
+
+Builtin definitions can live in a repo directory such as `skills/.system/<name>/SKILL.md` and be embedded at compile time with `include_str!`, or be loaded from a packaged runtime directory. Embedding is safer for single-binary installs; materializing to a read-only cache path may be useful so the existing `read` tool can load full builtin skill bodies during progressive disclosure.
+
+The optional local registry should default to `~/.ra/skill-registry` when the directory exists. It must be the root of a clean git checkout; Ra should not silently treat an unversioned directory, a subdirectory inside a larger repository, or a dirty worktree as a registry. Ra should not fetch, pull, checkout, or mutate the registry during normal startup. Version changes happen through git commands or future explicit registry-management commands.
+
+The `ra skills` adapter should shell out to:
+
+```bash
+npx --yes skills@<pinned-version> <subcommand> ...
+```
+
+For `ra skills add`, if no `--agent`/`-a` appears in user arguments, append `--agent codex`. For all other arguments, preserve ordering and values. The adapter should stream stdout/stderr and return the child exit status.
+
+## Timeline & Milestones
+
+| Milestone | Owner | Target |
+| --- | --- | --- |
+| PRD and issue draft | Agent | Before implementation |
+| Registry data model, git provenance, and precedence tests | Agent | First implementation PR |
+| Builtin config and discovery integration | Agent | First implementation PR |
+| `ra skills` passthrough adapter | Agent | Second implementation PR if scope grows |
+| Documentation and CLI examples | Agent | Before merge |
+
+## Open Questions & Risks
+
+- Whether builtin skill bodies should be embedded only, materialized to cache for `read`, or exposed through a virtual resource reader.
+- Whether future registry management should use `git` directly or redirect to a skills ecosystem command when the source is remote.
+- Whether `ra skills list` should default to Ra's resolved registry view or `npx skills list` project view. A pragmatic split is `ra skills list` passthrough and `ra skills registry` for Ra's resolved view.
+- `npx skills` is an npm dependency at command time. Users without Node/npm need a clear error and may still rely on builtin and manually installed skills.
+- A pinned `skills` version improves reproducibility but may lag ecosystem behavior. A config override can handle this without making default behavior unstable.
+
+## Appendix
+
+Reference behavior: `skills@1.5.9` (`npx skills`) supports `add`, `remove`, `list`, `find`, `update`, `init`, project/global scopes, `--agent`, `--skill`, `--copy`, `--all`, and Codex install paths of `.agents/skills/` for project scope and `~/.codex/skills/` for global scope.

spec/ra-config.schema.json

@@ -174,6 +174,38 @@
       },
       "additionalProperties": false
     },
+    "BuiltinSkillsSection": {
+      "type": "object",
+      "properties": {
+        "enabled": {
+          "description": "Master switch for Ra-shipped builtin skills. Default true.",
+          "default": true,
+          "type": "boolean"
+        },
+        "exclude": {
+          "description": "Builtin command names to suppress. Always wins over `include`.",
+          "default": [],
+          "type": "array",
+          "items": {
+            "type": "string"
+          }
+        },
+        "include": {
+          "description": "Empty = load the curated builtin set. Non-empty = load only these builtin command names before applying `exclude`.",
+          "default": [],
+          "type": "array",
+          "items": {
+            "type": "string"
+          }
+        },
+        "include_internal": {
+          "description": "Include builtins marked as internal. Default false.",
+          "default": false,
+          "type": "boolean"
+        }
+      },
+      "additionalProperties": false
+    },
     "GraphifySection": {
       "description": "`[graphify]` — native [Graphify](https://github.com/safishamsi/graphify) support. When enabled, Ra targets a project's `graphify-out/graph.json` (walking cwd → git root), treats Graphify as an agent-owned R2A graph workflow, and registers ensure/impact/update plus native graph query tools.",
       "type": "object",
@@ -692,11 +724,54 @@
       },
       "additionalProperties": false
     },
+    "SkillRegistrySection": {
+      "type": "object",
+      "properties": {
+        "enabled": {
+          "description": "Enable loading from a local git-backed skill registry. Default true; no registry is loaded unless `path` is set or the default checkout exists.",
+          "default": true,
+          "type": "boolean"
+        },
+        "exclude": {
+          "description": "Registry command names to suppress. Always wins over `include`.",
+          "default": [],
+          "type": "array",
+          "items": {
+            "type": "string"
+          }
+        },
+        "include": {
+          "description": "Empty = load every discovered registry skill. Non-empty = load only these command names before applying `exclude`.",
+          "default": [],
+          "type": "array",
+          "items": {
+            "type": "string"
+          }
+        },
+        "path": {
+          "description": "Local clean git checkout root containing SKILL.md files. If omitted, Ra checks `~/.ra/skill-registry` and skips it when absent.",
+          "default": null,
+          "type": [
+            "string",
+            "null"
+          ]
+        }
+      },
+      "additionalProperties": false
+    },
     "SkillsSection": {
       "type": "object",
       "properties": {
+        "builtin": {
+          "description": "Ra-shipped builtin skill controls. Builtins are the lowest-priority source and can be shadowed by explicit, project, global, or registry skills.",
+          "allOf": [
+            {
+              "$ref": "#/definitions/BuiltinSkillsSection"
+            }
+          ]
+        },
         "discover": {
-          "description": "Auto-discover skills from Ra-native, universal/cross-agent, and Claude Code layouts: `./.ra/skills/`, `~/.ra/skills/`, `./.agents/skills/`, `~/.agents/skills/`, `./.claude/skills/`, and `~/.claude/skills/`. Default true so project and personal skills work without explicit paths.",
+          "description": "Auto-discover skills from Ra-native, universal/cross-agent, Codex, and Claude Code layouts: `./.ra/skills/`, `~/.ra/skills/`, `./.agents/skills/`, `~/.agents/skills/`, `~/.codex/skills/`, `./.claude/skills/`, and `~/.claude/skills/`. Default true so project and personal skills work without explicit paths.",
           "default": true,
           "type": "boolean"
         },
@@ -712,6 +787,14 @@
           "items": {
             "type": "string"
           }
+        },
+        "registry": {
+          "description": "Optional local skill registry checkout. The registry must be a clean git worktree root; Ra records its HEAD revision as skill provenance.",
+          "allOf": [
+            {
+              "$ref": "#/definitions/SkillRegistrySection"
+            }
+          ]
         }
       },
       "additionalProperties": false

spec/ra.toml.example

@@ -73,6 +73,7 @@ builtin = []
 #   ~/.agents/skills/             — cross-agent global layout
 #   ./.claude/skills/             — Claude Code project skills
 #   ~/.claude/skills/             — Claude Code personal skills
+#   ~/.codex/skills/              — Codex global skills installed by `npx skills`
 # Other catalog-style layouts (`skills/.curated/`, …) go in `paths`
 # explicitly.
 [skills]
@@ -81,6 +82,22 @@ discover = true
 paths = []
 # paths = ["./skills/.curated/**/SKILL.md"]  # opt-in extra layouts
 
+[skills.registry]
+# Optional clean git checkout root for versioned skill catalogs. Ra loads
+# SKILL.md files from this checkout and records HEAD as registry provenance.
+enabled = true
+# path = "~/.ra/skill-registry"
+# include = []  # empty = all registry skills
+# exclude = []
+
+[skills.builtin]
+# Ra-shipped fallback skills. Project/global/registry skills with the same
+# command name shadow builtins.
+enabled = true
+# include = []  # empty = curated builtin set
+# exclude = []
+# include_internal = false
+
 # ─── Prompt templates → slash commands ──────────────────────────────
 [prompts]
 enabled = true

src/config.rs

@@ -146,18 +146,66 @@ pub struct SkillsSection {
     /// Master switch. Default true.
     #[serde(default = "default_true")]
     pub enabled: bool,
-    /// Auto-discover skills from Ra-native, universal/cross-agent, and
-    /// Claude Code layouts: `./.ra/skills/`, `~/.ra/skills/`,
-    /// `./.agents/skills/`, `~/.agents/skills/`, `./.claude/skills/`,
-    /// and `~/.claude/skills/`. Default true so project and personal
-    /// skills work without explicit paths.
+    /// Auto-discover skills from Ra-native, universal/cross-agent, Codex,
+    /// and Claude Code layouts: `./.ra/skills/`, `~/.ra/skills/`,
+    /// `./.agents/skills/`, `~/.agents/skills/`, `~/.codex/skills/`,
+    /// `./.claude/skills/`, and `~/.claude/skills/`. Default true so
+    /// project and personal skills work without explicit paths.
     #[serde(default = "default_true")]
     pub discover: bool,
     /// Extra glob patterns expanded against `~` and the cwd. Each match
     /// is loaded as a SKILL.md, on top of (and de-duplicated against)
     /// whatever `discover` finds.
     #[serde(default)]
     pub paths: Vec<String>,
+    /// Ra-shipped builtin skill controls. Builtins are the lowest-priority
+    /// source and can be shadowed by explicit, project, global, or registry
+    /// skills.
+    #[serde(default)]
+    pub builtin: BuiltinSkillsSection,
+    /// Optional local skill registry checkout. The registry must be a clean git
+    /// worktree root; Ra records its HEAD revision as skill provenance.
+    #[serde(default)]
+    pub registry: SkillRegistrySection,
+}
+
+#[derive(Debug, Clone, Deserialize, JsonSchema)]
+#[serde(deny_unknown_fields)]
+pub struct BuiltinSkillsSection {
+    /// Master switch for Ra-shipped builtin skills. Default true.
+    #[serde(default = "default_true")]
+    pub enabled: bool,
+    /// Empty = load the curated builtin set. Non-empty = load only these
+    /// builtin command names before applying `exclude`.
+    #[serde(default)]
+    pub include: Vec<String>,
+    /// Builtin command names to suppress. Always wins over `include`.
+    #[serde(default)]
+    pub exclude: Vec<String>,
+    /// Include builtins marked as internal. Default false.
+    #[serde(default)]
+    pub include_internal: bool,
+}
+
+#[derive(Debug, Clone, Deserialize, JsonSchema)]
+#[serde(deny_unknown_fields)]
+pub struct SkillRegistrySection {
+    /// Enable loading from a local git-backed skill registry. Default true;
+    /// no registry is loaded unless `path` is set or the default checkout
+    /// exists.
+    #[serde(default = "default_true")]
+    pub enabled: bool,
+    /// Local clean git checkout root containing SKILL.md files. If omitted, Ra
+    /// checks `~/.ra/skill-registry` and skips it when absent.
+    #[serde(default)]
+    pub path: Option<String>,
+    /// Empty = load every discovered registry skill. Non-empty = load only
+    /// these command names before applying `exclude`.
+    #[serde(default)]
+    pub include: Vec<String>,
+    /// Registry command names to suppress. Always wins over `include`.
+    #[serde(default)]
+    pub exclude: Vec<String>,
 }
 
 impl Default for SkillsSection {
@@ -166,6 +214,30 @@ impl Default for SkillsSection {
             enabled: true,
             discover: true,
             paths: Vec::new(),
+            builtin: BuiltinSkillsSection::default(),
+            registry: SkillRegistrySection::default(),
+        }
+    }
+}
+
+impl Default for BuiltinSkillsSection {
+    fn default() -> Self {
+        Self {
+            enabled: true,
+            include: Vec::new(),
+            exclude: Vec::new(),
+            include_internal: false,
+        }
+    }
+}
+
+impl Default for SkillRegistrySection {
+    fn default() -> Self {
+        Self {
+            enabled: true,
+            path: None,
+            include: Vec::new(),
+            exclude: Vec::new(),
         }
     }
 }
@@ -741,6 +813,17 @@ builtin = ["read", "bash"]
 [skills]
 paths = ["./skills/**/SKILL.md"]
 
+[skills.registry]
+path = "~/.ra/skill-registry"
+include = ["review"]
+exclude = ["internal-only"]
+
+[skills.builtin]
+enabled = true
+include = ["review"]
+exclude = ["demo"]
+include_internal = true
+
 [prompts]
 paths = ["./prompts/*.md"]
 
@@ -770,6 +853,16 @@ timeout = 2.0
         assert_eq!(cfg.models.len(), 1);
         assert_eq!(cfg.models[0].name, "pi");
         assert_eq!(cfg.tools.builtin, vec!["read", "bash"]);
+        assert_eq!(
+            cfg.skills.registry.path.as_deref(),
+            Some("~/.ra/skill-registry")
+        );
+        assert_eq!(cfg.skills.registry.include, vec!["review"]);
+        assert_eq!(cfg.skills.registry.exclude, vec!["internal-only"]);
+        assert!(cfg.skills.builtin.enabled);
+        assert_eq!(cfg.skills.builtin.include, vec!["review"]);
+        assert_eq!(cfg.skills.builtin.exclude, vec!["demo"]);
+        assert!(cfg.skills.builtin.include_internal);
         assert_eq!(cfg.a2a.serve.http_port, Some(3000));
         assert_eq!(cfg.a2a.remote_agents.len(), 1);
         assert_eq!(cfg.mcp.servers.len(), 1);