feat: support Pi Desktop subagents and Selesai skills - #176
yourconscience wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Sorry @yourconscience, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 5 hours and 26 minutes by commenting @sourcery-ai review. Upgrade to get a review now.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideThe PR replaces the unsupported Pi Desktop plugin path with native global skills and Markdown subagents, adds Selesai support that filters bundled skill names while retaining user skills, and updates detection, tests, setup behavior, and capability documentation accordingly. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f34f6a191d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // renderPiDesktopAgentRole emits the Markdown frontmatter consumed by | ||
| // Pi Desktop's global ~/.agents/subagents directory. | ||
| func renderPiDesktopAgentRole(role agentRole) string { | ||
| return renderClaudeAgentRole(role) |
There was a problem hiding this comment.
Render Pi roles without Claude-specific overrides
When a canonical role defines claude.model, this delegates to renderClaudeAgentRole, which prioritizes that Claude-only override over the generic model. Pi Desktop therefore receives a model selection intended exclusively for Claude Code, potentially pinning the wrong provider/model or making the subagent unusable; render Pi frontmatter independently from role.Claude.
AGENTS.md reference: AGENTS.md:L21-L21
Useful? React with 👍 / 👎.
| func detectPiDesktop(executable string) bool { | ||
| // Pi Desktop is a GUI application. Detect by checking if the app bundle exists. | ||
| // The executable might be 'pi' from the PATH, but we check for the desktop app. | ||
| info, err := os.Stat("/Applications/PI-Desktop.app") |
There was a problem hiding this comment.
Detect Pi Desktop in user-local installations
On macOS, applications may be installed under ~/Applications, but this detector checks only the system-wide /Applications directory. In that installation context both automatic setup and explicit --agents pi-desktop selection reject the installed harness because both paths still call isDetected, so the new skills and subagent integration cannot be configured.
AGENTS.md reference: AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
| | Pi Desktop¶ | yes | yes | -- | -- | -- | | ||
| | Selesai Code** | yes, filtered | -- | -- | -- | -- | |
There was a problem hiding this comment.
Add the new harnesses to the dotagents skill documentation
These new capability rows leave skills/dotagents/SKILL.md inconsistent: its description and capability matrix still enumerate only the previous harnesses, its role targets omit Pi Desktop, and it still states that Pi has skills only without distinguishing Desktop. Agents using the bundled dotagents skill will consequently give stale guidance about the newly advertised integrations.
AGENTS.md reference: AGENTS.md:L49-L51
Useful? React with 👍 / 👎.
Add Selesai Code integration: - New selesai harness with dynamic bundled-skill discovery - Filters bundled skills from sync to avoid conflicts - Uses ~/.selesai/agent/skills directory - Detects Selesai via version output - Tests for harness capabilities and skill filtering Add Pi Desktop harness foundation: - Harness definition with skills-only support - Detection via /Applications/PI-Desktop.app presence - Uses ~/.pi/agent/skills directory - Tests for basic capabilities Update isDetected to support GUI-only apps: - Allows harness Detect function when no executable specified - Enables Pi Desktop detection without CLI dependency Note: Pi Desktop plugin/subagent sync pending captain decision on manual vs automated approach (see needs-decision status)
Add Selesai Code integration: - New selesai harness with dynamic bundled-skill discovery - Filters bundled skills from sync to avoid conflicts - Uses ~/.selesai/agent/skills directory - Detects Selesai via version output - Tests for harness capabilities and skill filtering Add Pi Desktop plugin-based integration: - Generates loadable Pi Desktop plugin from canonical skills and roles - Plugin manifest.json contributes skills to Pi Desktop - Skill files copied from canonical dotagents skills - Agent roles converted to skill markdown format - Plugin directory at .pi-desktop-plugin/ in repo root - Config-driven integration (not simple symlink duplication) - Requires one-time manual loading via Pi Desktop GUI Update isDetected to support GUI-only apps: - Allows harness Detect function when no executable specified - Enables Pi Desktop detection without CLI dependency
c292396 to
51e6b58
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51e6b58baf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| selesaiPath, err := exec.LookPath("selesai") | ||
| if err != nil { | ||
| // Selesai not installed, return empty set | ||
| return make(map[string]struct{}), nil |
There was a problem hiding this comment.
Honor the configured Selesai executable when filtering skills
When a Selesai agent uses a custom detect command or absolute path, isDetected validates that executable but this helper independently searches for the literal selesai. If that name is absent—or resolves to a different installation—the helper returns an empty bundled set, so canonical skills that collide with Selesai's bundled names are no longer filtered and sync can abort on those conflicts. Pass the detected executable through to bundled-skill discovery instead of performing a second hard-coded lookup.
Useful? React with 👍 / 👎.
Summary
Verification
Full repository tests were not used as the merge gate because this machine has Pi Desktop installed and the existing environment-sensitive detection tests expect no GUI app; the focused integration checks pass.
Summary by Sourcery
Add native Pi Desktop and Selesai integrations with safe skill handling and documented capabilities.
New Features:
Enhancements:
Documentation:
Tests: