Skip to content

Pass Claude skills through Pi#79

Merged
glittercowboy merged 2 commits intomainfrom
pi-skill-paths-and-list
Apr 28, 2026
Merged

Pass Claude skills through Pi#79
glittercowboy merged 2 commits intomainfrom
pi-skill-paths-and-list

Conversation

@glittercowboy
Copy link
Copy Markdown
Contributor

@glittercowboy glittercowboy commented Apr 28, 2026

Summary

  • discover Claude-compatible skills from ~/.claude/skills and project ancestor .claude/skills roots
  • pass discovered SKILL.md paths to Pi via repeated --skill flags while keeping --no-extensions and --no-prompt-templates
  • add daemon listSkills handling and advertise the skills capability
  • bump protocol-go to v0.22.0

Verification

  • go test ./...
  • go build -o /tmp/gsd-cloud-pi-skill-paths-and-list .
  • git diff --check

Merge order

  1. protocol-go v0.22.0 is merged and tagged.
  2. Merge this daemon PR.
  3. Tag daemon/v0.3.14 to rebuild daemon binaries.
  4. Merge the cloud-app PR that consumes listSkills for the composer UI.

Summary by CodeRabbit

  • New Features

    • Automatic discovery of Claude skills from project and home scopes, with nearest-project preference and de-duplication.
    • Client now advertises skills support capability.
    • Daemon responds to skill-listing requests and returns discovered skills.
    • Configured skill paths are forwarded to the PI/CLI at startup and logged.
  • Tests

    • Added unit tests covering skill discovery behavior and CLI argument forwarding.
  • Chores

    • Updated a Go module dependency.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 28, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b95272fa-76ed-4c84-98fa-0d6f4c831469

📥 Commits

Reviewing files that changed from the base of the PR and between c0ed02b and c42619a.

📒 Files selected for processing (1)
  • internal/skills/discovery.go

📝 Walkthrough

Walkthrough

Adds Claude skills discovery and wiring: new DiscoverClaudeSkills(cwd) scans home and ancestor .claude/skills; daemon handles ListSkills; relay advertises Skills: true; PI executor accepts SkillPaths (emits --skill args); session actor discovers skills at startup; tests validate argument wiring.

Changes

Cohort / File(s) Summary
Dependency Update
go.mod
Bumped github.com/gsd-build/protocol-go from v0.21.0 to v0.22.0.
Skills Discovery
internal/skills/discovery.go, internal/skills/discovery_test.go
Added exported DiscoverClaudeSkills(cwd string) to find .claude/skills in home and ancestor dirs, parse SKILL.md frontmatter (name/description) with limits, deduplicate by path/name, scope and stable-sort results; tests cover discovery, nearest-duplicate preference, and name fallback.
Daemon List Handler
internal/loop/daemon.go
Handle protocol.ListSkills by resolving effective CWD, calling skills.DiscoverClaudeSkills, and returning protocol.ListSkillsResult with skills and error string on failure.
Relay Hello Capabilities
internal/relay/conn.go
Advertise Skills: true in the protocol.Hello capabilities during connect.
PI Executor Options
internal/pi/executor.go
Added SkillPaths []string to Options; Executor.Run appends one --skill <path> arg per non-empty path and logs configured skillCount.
Session Actor Integration & Tests
internal/session/actor.go, internal/session/actor_test.go
runPiExecutor performs pre-run discovery using actor CWD, logs discovery failures, supplies discovered skill paths to pi.NewExecutor; unit test verifies --skill <abs-path> appears in PI invocation args.

Sequence Diagram

sequenceDiagram
    participant Client as Client
    participant Daemon as Daemon
    participant FS as Filesystem
    participant Session as SessionActor
    participant PI as PIExecutor

    Client->>Daemon: ListSkills request (cwd)
    activate Daemon
    Daemon->>FS: DiscoverClaudeSkills(cwd) — read home + ancestor `.claude/skills`
    FS-->>Daemon: skills metadata (name, desc, path, scope)
    Daemon-->>Client: ListSkillsResult (skills, error)
    deactivate Daemon

    Session->>FS: DiscoverClaudeSkills(cwd)
    FS-->>Session: list of skill paths
    Session->>PI: NewExecutor(..., SkillPaths: paths)
    PI->>PI: Build args (one `--skill <path>` per entry)
    PI-->>Session: executor ready
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly Related PRs

Poem

🐰 I nosed through .claude in night and day,
Found SKILL.md breadcrumbs along the way,
Names tidy, paths trimmed neat,
The PI hums — arguments complete! 🎩

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title "Pass Claude skills through Pi" directly and clearly describes the main change: enabling Claude skills to be discovered and passed to the Pi executor.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 pi-skill-paths-and-list

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

@glittercowboy glittercowboy force-pushed the pi-skill-paths-and-list branch from 64257b8 to c0ed02b Compare April 28, 2026 15:35
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/skills/discovery.go`:
- Around line 137-141: The deferred file.Close() call is ignored; replace defer
file.Close() with a deferred closure that captures and checks the Close error
(e.g., defer func() { if err := file.Close(); err != nil { log.Printf("failed to
close %s: %v", path, err) } }()), or otherwise handle the error (propagate or
log) so the error return from file.Close() is not silently dropped; locate the
os.Open(path) call and the defer file.Close() in the same function and update
there.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c9dde1c0-5c7a-49d0-90f2-1f292ed140e1

📥 Commits

Reviewing files that changed from the base of the PR and between 2029756 and 64257b8.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (8)
  • go.mod
  • internal/loop/daemon.go
  • internal/pi/executor.go
  • internal/relay/conn.go
  • internal/session/actor.go
  • internal/session/actor_test.go
  • internal/skills/discovery.go
  • internal/skills/discovery_test.go

Comment thread internal/skills/discovery.go Outdated
@glittercowboy glittercowboy merged commit 7078d8f into main Apr 28, 2026
1 check passed
@glittercowboy glittercowboy deleted the pi-skill-paths-and-list branch April 28, 2026 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant