feat: holodeck skill command (catalog + claude/cursor/codex/gemini installers)

[ArangoGutierrez]

Problem

Users adopting holodeck via an AI coding agent (Claude Code, Cursor,
Codex, Gemini CLI) have no first-class way to teach the agent the
CLI. Today the only path is "paste --help into the agent."

Approach

Adds a holodeck skill command with two subcommands:

• holodeck skill list — show the embedded catalog (table/json/yaml).
• holodeck skill add <name> --claude|--cursor|--codex|--gemini —
  install the skill in the agent's native format. --global writes
  to user config; --all installs every skill in the catalog;
  --all-agents is a shortcut for all four agent flags.

Skills are authored once as Claude Code SKILL.md files under
pkg/skill/data/skills/<name>/, embedded into the binary via
//go:embed. A Renderer interface translates per agent. Codex
and Gemini share a single-file install path (AGENTS.md /
GEMINI.md); the install is idempotent via section markers.

v1 ships one skill: using-holodeck.

Testing

• Table-driven unit tests for parser and all four renderers.
• Integration tests for the installer with t.TempDir():
  per-file atomic write, single-file section splice (append /
  replace / preserve other sections), overwrite-prompt /
  --force interaction.
• CLI subcommand action tests for flag validation.
• Manual end-to-end smoke per-agent in /tmp documented in the plan.

Spec / plan

• Spec: docs/superpowers/specs/2026-05-14-holodeck-skill-cmd-design.md
• Plan: docs/superpowers/plans/2026-05-14-holodeck-skill-cmd.md

Breaking changes

None. New command, no existing surface modified.

[ArangoGutierrez]

Principal Engineer review — PR #812

(Posted by Team Lead on behalf of principal-engineer subagent. See ~/.claude/CLAUDE.md PE-review protocol.)

Verdict

APPROVE

Architecture

• Renderer interface (pkg/skill/render.go:27-43) is cleanly factored: four methods, single responsibility, no leaky abstractions. The SingleFile() discriminator is the right shape for the two install models.
• Install() (pkg/skill/install.go:54-113) cleanly switches on r.SingleFile() and dispatches to installSingleFile vs the per-file atomic flow. Both paths share writeFileAtomic.
• cmd/cli/skill/ correctly mirrors cmd/cli/<cmd>/ pattern. Dependency direction is right: cmd/cli/skill → pkg/skill, no upward leaks.
• pkg/skill has zero non-stdlib deps beyond yaml.v2 (parser) and go-isatty (already in tree). No transitive churn.
• Mild scope concern: command struct (cmd/cli/skill/skill.go:30-52) is shared between list and add; outputFormat only meaningful for list, other 10 fields only for add. Follows existing patterns; acceptable.

Go conventions

• Errors wrapped at every call site (install.go:126,134,138,149,155,172, etc.) with %w. errors.Is(err, os.ErrNotExist) used correctly.
• Receivers correct (value receivers on stateless renderers; consistent).
• context.Context deliberately absent — appropriate for short synchronous CLI ops on local FS.
• "Accept interfaces, return structs" shape held: Renderer consumed where defined; *xRenderer returned from NewXRenderer.
• No init(), no globals, no any.

Security

• gosec G304 (os.ReadFile(dest) at install.go:165): False positive. dest is computed from r.InstallPath(...), which is bounded to known string constants (.codex/AGENTS.md, .gemini/GEMINI.md, ~/.codex/..., ~/.gemini/...).
• gosec G301 (MkdirAll(..., 0o755) at install.go:133): Acceptable. These are config dirs (.claude/skills/, .cursor/rules/, etc.) — user-readable is conventional and matches how Claude Code / Cursor / Codex / Gemini already create them. 0o750 would satisfy gosec; not worth a behavior change. Consider // #nosec G301 -- config dir convention annotation for clean lint output.
• Path-traversal — defense-in-depth gap (non-blocking): parseSkill (skill.go:43-75) does not validate the name field. s.Name flows unsanitized into filepath.Join(".claude", "skills", s.Name, "SKILL.md") and filepath.Join(".cursor", "rules", s.Name+".mdc"). Safe today because the catalog is //go:embed-only (compile-time, trusted). The moment a future PR adds --file <local-SKILL.md> or a remote catalog, a name: ../../etc/cron.d/evil becomes a traversal write. Recommend a ^[a-z0-9][a-z0-9-]{0,63}$ allowlist regex in parseSkill. Cheap, future-proofs the package.
• writeFileAtomic (install.go:132-159) — correct temp + rename with cleanup-on-err.
• installSingleFile has a benign concurrency race (read → splice → write, no flock). Two concurrent holodeck skill add against the same AGENTS.md would lose one update. Out of v1 scope; flag for follow-up.

Test quality

Spot-checked every test file. No theater tests detected.

• TestParseSkill (skill_test.go): table-driven, six cases (valid + 5 error paths). Literal expected strings derived independently of impl. Fails if parseSkill body deleted.
• TestClaudeRendererRender (render_claude_test.go:36): asserts against fully-derived literal "---\nname: demo\ndescription: A demo skill\n---\n\n# Body\n\nDo the thing.\n".
• TestSpliceSection_* (install_test.go:27-72): three cases (append, replace, only-touch-named-skill) with literal substrings, count checks (bytes.Count(... == 1), surrounding-content preservation. Catches splice regressions.
• TestInstall_SingleFile_* / TestInstall_PerFile_*: invoke production functions on t.TempDir() and read back. Real I/O.
• TestRunAdd_*: validation tests assert specific error substrings; TestRunAdd_DryRun_ClaudeOnly and TestRunAdd_AllAgents_SetsAllFour go through the full Catalog+Install pipeline.
• TestBuildAddCommand_DescriptionUsesFlagFirstExamples (add_test.go:161-174) is a particularly nice doc-regression guard for the urfave/cli/v2 quirk fixed in ad00a640.
• Tests are deterministic: no time.Now, no rand, no map-iteration ordering reliance. Catalog is sorted before assertions touch it.
• Minor caveat: TestRunAdd_DryRun_ClaudeOnly and TestRunAdd_AllAgents_SetsAllFour use os.Chdir(t.TempDir()) and are not t.Parallel()-safe. Currently fine; flag if anyone adds t.Parallel() later.

YAGNI / scope

Implementation honored the deferral. No Remove(), no tag filter, no TUI picker, no remote catalog plumbing. Renderer interface is exactly what the four shipped agents need.

SKILL.md content

• Frontmatter description (SKILL.md:3) is well-shaped for Claude Code's skill-triggering matcher: starts with "Use when...", names concrete user intents, names the trigger noun ("holodeck CLI"), lists scope. Matcher-friendly.
• Aspirational content (list, status, ssh, scp, cleanup, os list) is per Team Lead direction; flagged for separate reconciliation.

Blocking issues

none

Non-blocking suggestions

• Add a name regex validator in parseSkill for defense-in-depth.
• Suppress G301 with // #nosec G301 -- config dir convention or downgrade to 0o750.
• Add a brief comment on installSingleFile documenting the concurrent-write race.

Follow-up issues to file

1. Parser fix (ad00a640 follow-up): urfave/cli/v2 → v3 migration or interspersion shim. The in-binary --help showing holodeck skill add --claude using-holodeck will confuse users who reach for the natural holodeck skill add using-holodeck --claude and silently get the "must specify at least one of --claude/..." error.
2. SKILL.md aspirational content reconciliation: trim to commands that exist on main (create, delete, dryrun) OR ship the referenced subcommands before v0.3.0 cut. v0.3.0 is in bug-fix sprint per .claude/CLAUDE.md, so cutting the aspirational sections is the lower-risk path.
3. Skill-name validation regex: low-priority hardening before opening the catalog source to anything non-embed.

[coveralls]

Coverage Report for CI Build 26209564134

Coverage increased (+0.9%) to 48.745%

Details

• Coverage increased (+0.9%) from the base build.
• Patch coverage: 150 uncovered changes across 9 files (342 of 492 lines covered, 69.51%).
• No coverage regressions found.

Uncovered Changes

File	Changed	Covered	%
pkg/skill/install.go	151	58	38.41%
cmd/cli/skill/add.go	122	103	84.43%
cmd/cli/skill/list.go	50	37	74.0%
pkg/skill/embed.go	25	17	68.0%
pkg/skill/render_cursor.go	20	15	75.0%
pkg/skill/render_claude.go	21	17	80.95%
pkg/skill/render_codex.go	19	16	84.21%
pkg/skill/render_gemini.go	15	12	80.0%
pkg/skill/skill.go	35	33	94.29%

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	11558
Covered Lines:	5634
Line Coverage:	48.75%
Coverage Strength:	0.54 hits per line

---

💛 - Coveralls