feat: new skills system

[pedronauck]

Summary by CodeRabbit

• New Features

  • CLI: new skill management commands — list, view, info, create — for bundled and custom skills.
  • Skills integrated into agent prompts via a catalog; skills are auto-loaded and polled for changes.
  • New agent skill bundle added (fix-coderabbit-review) with helper scripts.

• Configuration

  • New skills config section with enable/disable toggle, disabled-skills list, and poll interval.

• Tests

  • Extensive test coverage added for skills, loader, watcher, registry, catalog, and CLI.

• Chores

  • Added YAML dependency and updated .gitignore.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1e80c43c-c93a-4757-8bf6-2dfac31ba0f6

📥 Commits

Reviewing files that changed from the base of the PR and between 9a8d817 and 3d1be0d.

⛔ Files ignored due to path filters (3)

• .agents/skills/fix-coderabbit-review/SKILL.md is excluded by !**/*.md
• .compozy/docs/rfcs/agent-md-with-skills-memory.md is excluded by !**/*.md
• .compozy/docs/rfcs/skills-system-final.md is excluded by !**/*.md

📒 Files selected for processing (16)

• .agents/skills/fix-coderabbit-review/agents/openai.yaml
• .agents/skills/fix-coderabbit-review/scripts/pr-review.ts
• .agents/skills/fix-coderabbit-review/scripts/resolve_pr_issues.sh
• .gitignore
• internal/cli/skill.go
• internal/cli/skill_test.go
• internal/skills/catalog.go
• internal/skills/catalog_test.go
• internal/skills/loader.go
• internal/skills/registry.go
• internal/skills/registry_test.go
• internal/skills/verify.go
• internal/skills/verify_test.go
• internal/skills/watcher.go
• internal/skills/watcher_test.go
• skills-lock.json

---

Walkthrough

Adds a new skills subsystem: skill types, loader, verifier, registry with workspace-aware caching, polling watcher, CLI commands (list/view/info/create), bundled skill embedding, daemon integration via composable prompt providers, config/home plumbing, and comprehensive tests across modules.

Changes

Cohort / File(s)	Summary
Config & Home /.gitignore, go.mod, internal/config/... internal/config/config.go, internal/config/config_test.go, internal/config/home.go, internal/config/home_test.go, internal/config/merge.go, internal/config/merge_test.go	Add [skills] config (enabled, disabled_skills, poll_interval), merge overlay support, home paths for skills, default values, tests; add gopkg.in/yaml.v3 dependency and new .gitignore entries.
CLI internal/cli/root.go, internal/cli/skill.go, internal/cli/skill_test.go	Introduce skill command group with list, view, info, create subcommands, safe resource access, source filtering, multiple output formats, creation scaffolding, and extensive integration tests including traversal/symlink protections.
Skills Core (types, loader, verify) internal/skills/types.go, internal/skills/loader.go, internal/skills/loader_test.go, internal/skills/verify.go, internal/skills/verify_test.go	Define skill models and registry config, add SKILL.md frontmatter parsing, directory scanning with snapshotting/limits, content verification patterns and severity, and comprehensive unit tests.
Registry & Watcher internal/skills/registry.go, internal/skills/registry_test.go, internal/skills/watcher.go, internal/skills/watcher_test.go	Implement in-memory registry with global/workspace merging, cloning, caching with TTL, file-snapshot diffs, refresh semantics, and a polling Watcher that triggers global refreshes; extensive concurrency and behavior tests.
Catalog & Bundled Assets internal/skills/catalog.go, internal/skills/catalog_test.go, internal/skills/bundled/embed.go, internal/skills/bundled/bundled_test.go	Add embedded bundled-skills FS, catalog builder emitting XML-like available-skills block with escaping/truncation, and tests validating embedded assets and deterministic catalog rendering.
Daemon & Prompt Composition internal/daemon/..., internal/memory/assembler.go, internal/session/prompt_provider.go internal/daemon/daemon.go, internal/daemon/composed_assembler.go, internal/daemon/..._test.go, internal/memory/assembler_test.go, internal/session/prompt_provider.go	Introduce PromptProvider interface and ComposedAssembler; wire skills registry and catalog into daemon boot when enabled, start/stop watcher, adapt memory assembler to provider interface, and add integration tests covering various feature flags and watcher lifecycle.
Skills Tests & Helpers internal/skills/*_test.go	Large suite of new tests for loader, registry, catalog, watcher, and bundled assets to validate parsing, verification, caching, refresh, and concurrency behaviors.
Agents Skill Pack .agents/skills/fix-coderabbit-review/..., skills-lock.json	Add new skill package with agent config, Node script (pr-review.ts) and resolver script, and update skills-lock.json to pin the new skill; scripts export and optionally resolve PR review issues.

Sequence Diagram(s)

sequenceDiagram
    participant Daemon
    participant Registry
    participant Loader
    participant Watcher
    participant Disk

    Daemon->>Registry: NewRegistry(cfg)
    Daemon->>Registry: LoadAll(ctx)
    Registry->>Loader: ParseSkillFile(path)
    Loader->>Disk: Read SKILL.md
    Disk-->>Loader: frontmatter + content
    Loader-->>Registry: Skill
    Registry-->>Daemon: skills loaded

    Daemon->>Watcher: Start(ctx)

    rect rgba(100, 200, 255, 0.5)
    Note over Watcher: Polling loop
    Watcher->>Disk: scanDirectory()
    Disk-->>Watcher: file snapshots
    Watcher->>Watcher: detectChanges()
    alt Changes detected
        Watcher->>Registry: RefreshGlobal(ctx)
        Registry->>Loader: rescan skills
        Loader-->>Registry: updated skills
    end
    end

Loading

sequenceDiagram
    participant Client
    participant CLI
    participant Registry
    participant Disk

    Client->>CLI: skill list --source bundled
    CLI->>Registry: List() / ForWorkspace(ctx, ws)
    Registry-->>CLI: []*Skill
    CLI->>CLI: filter/format output
    CLI-->>Client: JSON/human output

    Client->>CLI: skill view <name> --file <resource>
    CLI->>Registry: Get(name)
    Registry-->>CLI: *Skill
    CLI->>Disk: Read resource (validated path)
    Disk-->>CLI: file content
    CLI-->>Client: skill content

    Client->>CLI: skill create <name>
    CLI->>Disk: create workspace skill dir + SKILL.md
    Disk-->>CLI: created
    CLI->>Loader: ParseSkillFile(SKILL.md)
    Loader-->>CLI: validated Skill
    CLI-->>Client: creation success

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• feat: add cy-idea-factory skill and improve planning skills DX compozy#35 — touches bundled skills embedding and tests; likely overlaps bundled FS and SKILL.md fixtures.
• feat: new skills system #1 — appears to implement or mirror the core skills subsystem changes (types, registry, loader, CLI, daemon wiring).
• feat: add config.toml compozy#40 — adds the same gopkg.in/yaml.v3 dependency in go.mod; related at dependency level.

Poem

🐰 I found SKILL.md in a cozy nook,
I parsed frontmatter and gave it a look.
I watched and I cached, kept prompts in a row,
I listed and viewed with a soft little "go!"
Hooray for new skills — hop, hum, and show! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 8.49% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat: new skills system' directly describes the main feature added across the entire changeset—a comprehensive new skills subsystem including CLI commands, registry management, skill loading, catalog generation, and daemon integration.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch pn/skills

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 9

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.gitignore:
- Line 19: Update the .gitignore entry for the skills runtime artifact from
".compozy/runs" to include a trailing slash for consistency with other directory
patterns; locate the line containing ".compozy/runs" and replace it with
".compozy/runs/" so it clearly denotes a directory like ".tmp/", ".cache/", and
".turbo/".

In `@go.mod`:
- Line 13: The project now depends on two YAML libraries (gopkg.in/yaml.v3 and
github.com/goccy/go-yaml) which overlap; decide whether to consolidate or keep
both and then implement the change: either refactor internal/skills/loader.go to
use github.com/goccy/go-yaml's Node/AST-equivalent APIs (or its
UnmarshalWithOptions) so only github.com/goccy/go-yaml is required, or update
the modules and code that use github.com/goccy/go-yaml (memory/unmarshal paths)
to use gopkg.in/yaml.v3's Node API and strict decoding so only yaml.v3 is
required; update imports in the affected files (e.g., internal/skills/loader.go
and any memory unmarshaling utilities), remove the unused module from go.mod,
and run tests/formatting to ensure compatibility.

In `@internal/cli/skill.go`:
- Around line 582-612: normalizeSkillName currently allows control characters
and YAML-sensitive input which can break filesystem paths and the unquoted name
written by defaultSkillTemplate; update normalizeSkillName to strictly validate
and/or sanitize the input (reject names with control chars, newlines, colons,
YAML anchors/indicators, path separators, and any characters outside an allowed
pattern such as [A-Za-z0-9._-]) and return an error for invalid names, and
update defaultSkillTemplate (or the code that writes SKILL.md) to use a safe
representation (either a validated name or a properly quoted/escaped YAML
string) so the directory created and the name field in SKILL.md cannot diverge;
reference normalizeSkillName, defaultSkillTemplate, and titleizeSkillName when
locating where to apply validation and safe YAML quoting.
- Around line 503-524: The current lexical check uses filepath.Rel on
absRoot/absTarget but doesn't resolve symlinks, so a symlink inside the skill
dir can point outside; call filepath.EvalSymlinks (or equivalent) on absRoot and
absTarget before computing relativeToRoot and performing the boundary check,
then use the evaluated absTarget when calling os.ReadFile (update error messages
that reference paths like cleanPath/absTarget as needed); specifically modify
the sequence around variables absRoot, absTarget, relativeToRoot and the final
os.ReadFile to operate on symlink-resolved paths to ensure real filesystem
boundaries are enforced.

In `@internal/skills/catalog.go`:
- Around line 17-21: The XML replacer currently named catalogXMLReplacer doesn’t
escape double quotes so inserting entry.name into a quoted attribute can break
the XML; update catalogXMLReplacer to also replace `"` with `"` (and ensure
the same change is applied to the other replacer/usage referenced around the
entry writing code at the other occurrence) and make sure all places that write
name="..." (where entry.name is used) use this replacer so skill names like foo"
bar are properly encoded.

In `@internal/skills/loader.go`:
- Around line 182-200: The function decodeSkillMeta currently unmarshals the
frontmatter twice; instead, unmarshal once into a yaml.Node (the existing
document variable), call warnUnknownFields(&document) as before, then use
document.Decode(&meta) to populate the SkillMeta struct (handle and return any
decode error), followed by the existing trimming of meta.Name, meta.Description,
and meta.Version; this removes the second yaml.Unmarshal and preserves the same
validation and error handling.

In `@internal/skills/registry.go`:
- Around line 231-252: The loadBundledSkills function currently parses and
overlays bundled skills without running safety verification; update it to run
the same verification gate used for directory-backed skills (call
VerifyContent(ctx, skill) or the registry's verification method) after
parseBundledSkill and before r.overlaySkill, and if VerifyContent reports a
critical finding drop the skill (do not call r.overlaySkill) while still
respecting r.applyDisabled; ensure errors from verification are handled the same
way as for directory skills.

In `@internal/skills/verify.go`:
- Around line 39-44: The current rule with pattern "role-hijack-you-are-now"
uses a broad regex `(?i)\byou\s+are\s+now\b` that causes false positives; update
the regex in that rule (the map entry with keys pattern, regex, severity,
message) to require explicit role indicators such as "a" or "an" or common role
nouns (e.g., assistant|agent|bot|system) — for example use a case-insensitive
pattern like `(?i)\byou\s+are\s+now\s+(?:a|an|the|assistant|agent|bot|system)\b`
to tighten matching while keeping SeverityCritical and the existing message.
Ensure you replace only the regex value for that rule in verify.go and keep the
rest of the entry intact.

In `@internal/skills/watcher.go`:
- Around line 136-139: The first watcher scan unconditionally seeds w.snapshots
and returns without triggering RefreshGlobal, allowing races between
Registry.LoadAll() and the first scan to leave the registry stale; instead seed
the watcher from the same snapshot used by LoadAll or run a refresh on the first
successful scan: when w.initialized is false in the watch loop, compare the
current snapshot to the registry snapshot (or call RefreshGlobal once) before
setting w.initialized and assigning w.snapshots, ensuring either you initialize
w.snapshots from the LoadAll snapshot or invoke RefreshGlobal() once so the
registry is updated on the first scan.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0aac5ddf-1875-461e-9c82-046e9bd1ccc5

📥 Commits

Reviewing files that changed from the base of the PR and between c274676 and 9a8d817.

⛔ Files ignored due to path filters (34)

• .agents/skills/cy-create-prd/SKILL.md is excluded by !**/*.md
• .agents/skills/cy-idea-factory/SKILL.md is excluded by !**/*.md
• .agents/skills/cy-idea-factory/references/council.md is excluded by !**/*.md
• .agents/skills/cy-idea-factory/references/idea-template.md is excluded by !**/*.md
• .agents/skills/cy-idea-factory/references/question-protocol.md is excluded by !**/*.md
• .compozy/tasks/skills-system/_idea.md is excluded by !**/*.md
• .compozy/tasks/skills-system/_meta.md is excluded by !**/*.md
• .compozy/tasks/skills-system/_tasks.md is excluded by !**/*.md
• .compozy/tasks/skills-system/memory/MEMORY.md is excluded by !**/*.md
• .compozy/tasks/skills-system/memory/task_01.md is excluded by !**/*.md
• .compozy/tasks/skills-system/memory/task_02.md is excluded by !**/*.md
• .compozy/tasks/skills-system/memory/task_03.md is excluded by !**/*.md
• .compozy/tasks/skills-system/memory/task_04.md is excluded by !**/*.md
• .compozy/tasks/skills-system/memory/task_05.md is excluded by !**/*.md
• .compozy/tasks/skills-system/memory/task_06.md is excluded by !**/*.md
• .compozy/tasks/skills-system/memory/task_07.md is excluded by !**/*.md
• .compozy/tasks/skills-system/memory/task_08.md is excluded by !**/*.md
• .compozy/tasks/skills-system/memory/task_09.md is excluded by !**/*.md
• .compozy/tasks/skills-system/memory/task_10.md is excluded by !**/*.md
• .compozy/tasks/skills-system/memory/task_11.md is excluded by !**/*.md
• .compozy/tasks/skills-system/task_01.md is excluded by !**/*.md
• .compozy/tasks/skills-system/task_02.md is excluded by !**/*.md
• .compozy/tasks/skills-system/task_03.md is excluded by !**/*.md
• .compozy/tasks/skills-system/task_04.md is excluded by !**/*.md
• .compozy/tasks/skills-system/task_05.md is excluded by !**/*.md
• .compozy/tasks/skills-system/task_06.md is excluded by !**/*.md
• .compozy/tasks/skills-system/task_07.md is excluded by !**/*.md
• .compozy/tasks/skills-system/task_08.md is excluded by !**/*.md
• .compozy/tasks/skills-system/task_09.md is excluded by !**/*.md
• .compozy/tasks/skills-system/task_10.md is excluded by !**/*.md
• .compozy/tasks/skills-system/task_11.md is excluded by !**/*.md
• internal/skills/bundled/skills/agh-agent-setup/SKILL.md is excluded by !**/*.md
• internal/skills/bundled/skills/agh-memory-guide/SKILL.md is excluded by !**/*.md
• internal/skills/bundled/skills/agh-session-guide/SKILL.md is excluded by !**/*.md

📒 Files selected for processing (32)

• .gitignore
• go.mod
• internal/cli/root.go
• internal/cli/skill.go
• internal/cli/skill_test.go
• internal/config/config.go
• internal/config/config_test.go
• internal/config/home.go
• internal/config/home_test.go
• internal/config/merge.go
• internal/config/merge_test.go
• internal/daemon/composed_assembler.go
• internal/daemon/composed_assembler_test.go
• internal/daemon/daemon.go
• internal/daemon/daemon_integration_test.go
• internal/daemon/daemon_test.go
• internal/memory/assembler.go
• internal/memory/assembler_test.go
• internal/session/prompt_provider.go
• internal/skills/bundled/bundled_test.go
• internal/skills/bundled/embed.go
• internal/skills/catalog.go
• internal/skills/catalog_test.go
• internal/skills/loader.go
• internal/skills/loader_test.go
• internal/skills/registry.go
• internal/skills/registry_test.go
• internal/skills/types.go
• internal/skills/verify.go
• internal/skills/verify_test.go
• internal/skills/watcher.go
• internal/skills/watcher_test.go