feat: embed AI agent skills in extension packages

[stack72]

Summary

Extension authors can now ship AI agent skills alongside their extensions. Skills follow the Agent Skills open standard and can contain SKILL.md, references, scripts, assets, and evals.

How it works

Push side: Authors add skills: ["my-skill"] to their manifest.yaml. Push resolves the skill from the project-local skill directory (e.g., .claude/skills/my-skill/) first, then falls back to the global ~/.claude/skills/my-skill/. All files in the directory are packaged recursively — no file type restrictions.

Pull side: Skills are extracted to the correct AI tool directory based on .swamp.yaml:

• Claude → .claude/skills/
• Cursor → .cursor/skills/
• Kiro → .kiro/skills/
• OpenCode/Codex → .agents/skills/
• none → .swamp/pulled-extensions/skills/ (fallback)

Skills keep their original directory name — no namespacing. Collisions are handled by the existing conflict detection.

Cleanup: Skill directories (not individual files) are tracked in upstream_extensions.json. extension rm deletes them via recursive removal.

Security

Skills are prompt content loaded into AI agent context and can include executable scripts. The pull renderer displays a prominent warning:

WARNING: This extension includes AI agent skills (3 files).
WARNING: Skills are loaded into your AI agent's context and may contain executable scripts.
WARNING: Review ALL skill files before use:
WARNING:   .claude/skills/aws-ec2/SKILL.md
WARNING:   .claude/skills/aws-ec2/scripts/setup.sh

If scripts/ is detected, an escalated warning is shown.

Design tradeoffs

1. Skills-only extensions are invalid — the existing .refine() that requires at least one model/workflow/vault/driver/datastore/report is unchanged. Skills must accompany real functionality.

2. No bundling — skills are plain files (markdown, scripts, JSON). No deno bundle step, no cached bundles.

3. Directory-level tracking — upstream_extensions.json tracks the skill directory root, not individual files. This simplifies cleanup (one recursive delete) at the cost of not tracking exactly which files were installed.

4. Tool switching is out of scope — if a user switches from Claude to Cursor, skills stay in the old location. extension pull --force re-installs to the new location. A migration step could be added to repo upgrade later.

5. Registry metadata is sent but not consumed yet — push sends ExtractedSkill[] in content metadata so swamp-club can display skill badges. Server-side tracked in systeminit/swamp-club#354.

6. Push-time validation — SKILL.md must exist with valid frontmatter (name + description). Size limits enforced (500KB per file, 2MB total). No file type restrictions since the standard allows scripts, assets, evals, etc.

Files changed

Area	Files	What
Schema	extension_manifest.ts	Add skills field
Validation	extension_skill_validator.ts (new)	Push-time SKILL.md + size validation
Constants	skill_dirs.ts (new), paths.ts	Shared SKILL_DIRS, pulledSkills path
Push	push.ts, extension_push.ts, resolve_extension_files.ts	Package skills into archive
Pull	pull.ts, extension_pull.ts, extension_install.ts, extension_update.ts, extension_search.ts	Extract skills, wire skillsDir
Rm	rm.ts	Recursive removal for directory entries
Rendering	extension_pull.ts, extension_push.ts (renderers)	Skill warnings + counts
Registry	extension_content.ts, extension_content_extractor.ts	ExtractedSkill metadata
Tests	5 test files updated	Add missing skills fields to type constructions

Test plan

• deno check passes
• deno lint passes
• deno fmt passes
• All 4184 tests pass
• Binary compiles
• E2E: swamp repo init + create skill + swamp extension push --dry-run shows "Skills (1): test-skill (3 files)"
• E2E: Archive contains extension/skills/test-skill/ with SKILL.md, references/, scripts/
• E2E: Pull from registry installs skills to .claude/skills/ (requires published extension)
• E2E: extension rm removes skill directory

🤖 Generated with Claude Code

[github-actions]

Code Review

Blocking Issues

None.

Suggestions

1. Missing unit test for resolveSkillsDir (src/domain/repo/skill_dirs.ts): This function has branching logic (tool-aware resolution with fallback) but no dedicated test file. The validator tests cover validateExtensionSkills well, but resolveSkillsDir is used across 5+ CLI commands and deserves its own skill_dirs_test.ts to verify known tools resolve to tool-specific paths and unknown/"none" falls back to the swamp pulled-skills directory.

2. ExtractedSkill.description is always empty string (src/libswamp/extensions/push.ts:518): The validator already parses SKILL.md frontmatter and has access to the description field, but ValidatedSkill doesn't carry it through, so push hardcodes description: "" in the registry metadata. Since the PR description notes registry metadata is "sent but not consumed yet," this is fine for now, but worth a follow-up to thread the description through ValidatedSkill before the registry starts consuming it.

3. Duplicate recursive file collection: collectFiles() in extension_skill_validator.ts and the inline collectSkillFiles() in resolve_extension_files.ts (lines 302-311) do the same recursive directory walk. Minor duplication — could share the helper, but not worth blocking on.

Overall: Clean, well-structured feature. Good security posture with size limits, path validation, and prominent skill warnings on pull. DDD layering is correct (domain → infrastructure import tracked by ratchet). Test coverage is solid for the validator.

[github-actions]

CLI UX Review

Blocking

None.

Suggestions

1. Pull renderer log mode — warning placement: Skill security warnings appear after Extracted N files: (which may be a long list), meaning they scroll off screen. The existing pattern puts warnings before the success/extracted output for other signals (e.g. safety warnings come before Pulled X@Y). Consider hoisting the skill warning above the file list so it's less likely to be missed on a large pull. Not blocking — the warnings are present and use logger.warn correctly.

2. Pull renderer log mode — escalated scripts warning ordering: The escalated EXECUTABLE SCRIPTS warning is placed after the file list rather than immediately after the first warning sentence. A user scanning the output might miss the escalation. Minor wording suggestion: fold the scripts escalation into the first warning line itself (includes AI agent skills (N files), including EXECUTABLE SCRIPTS) so it's impossible to miss.

3. Push dry-run — fileCount per skill vs. skillCount in summary: Dry-run shows Skills (1): aws-ec2 (3 files) while the success summary shows Skills: 1. This mixed representation (directory count vs. file count) is consistent with how models work (dry-run lists names, summary shows count), so it's fine as-is.

Verified

• copyDir returns relative(repoDir, destPath) paths — skill file paths in warnings are repo-relative (e.g. .claude/skills/aws-ec2/SKILL.md), not absolute. Good.
• JSON mode: skillWarning follows the existing multi-object pattern (same as safetyWarnings, platforms, missingSourceFiles). Consistent.
• Push JSON: ExtensionPushResolvedData.skills and skillCount in ExtensionPushSuccessData are both serialized automatically by JsonExtensionPushRenderer. Complete.
• Error messages are actionable: missing skill dir includes looked-in paths; unsupported tool error includes the fix command.
• extension rm recursive removal is handled correctly via stat.isDirectory check.

Verdict

PASS — no blocking issues. New skill UX is consistent with existing extension commands.

[github-actions]

Adversarial Review

Critical / High

No critical or high severity findings.

Medium

1. src/libswamp/extensions/push.ts:889-892 — startsWith path matching double-counts skill files when one skill name is a prefix of another

   buildResolvedData computes per-skill fileCount using:

   input.allSkillFiles.filter((f) => f.startsWith(s.absolutePath)).length

   If two skills exist at /repo/.claude/skills/foo and /repo/.claude/skills/foobar, the string /repo/.claude/skills/foobar/SKILL.md matches startsWith("/repo/.claude/skills/foo") — so files from foobar are also counted under foo.

   Breaking example: manifest.yaml with skills: ["aws", "aws-ec2"]. The dry-run preview shows aws (5 files) when it really has 2, because aws-ec2's 3 files also match the startsWith check.

   Fix: f.startsWith(s.absolutePath + "/") (or use sep from @std/path for Windows compat).

2. src/libswamp/extensions/push.ts:498-510 — ExtractedSkill.description is always "" in registry metadata

   The validator reads and validates SKILL.md frontmatter (ensuring name and description exist), but ValidatedSkill doesn't carry the parsed description back. So contentMetadata.skills always gets description: "". The PR body acknowledges the registry metadata "is sent but not consumed yet," so this may be intentional incompleteness — but the ExtractedSkill type promises a description field that's always empty, which could mislead downstream consumers when they do start reading it.

   Fix: Add description: string to ValidatedSkill in the validator, populate it from parsed frontmatter, then use validated?.description ?? "" in push.ts.

Low

1. src/libswamp/extensions/rm.ts:337-339 — removeFile now unconditionally stats before removal

   The change from Deno.remove(path) to stat(path) + remove(path, { recursive: stat.isDirectory }) applies to ALL tracked entries, not just skills. If a previously-tracked file path has been replaced on disk with a directory (e.g., user accidentally creates a dir at that path), extension rm now recursively deletes it instead of failing. The blast radius broadened slightly. This is by-design for the skill directory use case but worth noting — the old behavior was a safe failure; the new behavior silently succeeds with recursive delete. In practice, the tracked file list is tightly controlled by the pull process so this is unlikely to cause harm.

2. src/domain/extensions/extension_skill_validator.ts:93-99 — parseFrontmatter accepts any YAML, not just mappings

   If the frontmatter YAML is a valid scalar like ---\ntrue\n---, parseYaml returns a boolean. The typeof parsed === "object" && parsed !== null check correctly rejects this, returning null, which triggers a "missing frontmatter" error. This works, but the error message ("must start with --- delimiters") is misleading when the delimiters are present but the content isn't a mapping. Minor UX nit, not blocking.

Verdict

PASS — The code is solid overall. The safePathString validation on manifest skills, the NotFound guards on readDir, and the recursive-copy approach are all correct. The startsWith path-prefix bug (Medium #1) is a real logic error but only affects dry-run display counts, not the actual archive. The empty description (Medium #2) is incomplete metadata, not corruption. Ship it.