src/commands/skills.ts (1)

281-290: Consider whether unavailable specified agents should error or warn.

When a user explicitly requests --agent claude but Claude Code is not installed, the current logic includes the agent anyway (line 282-284 comment: "If specific agent requested, include it even if not available"). However, if no agents end up being available, the user sees "No supported agents detected" which may be confusing.

Consider providing a more specific message when the requested agent is not installed, e.g., "Agent 'claude' (Claude Code) is not installed."

Suggested improvement for clearer error messaging

   // Filter to only available agents (unless specific agent requested)
   const availableAgents = agentId
     ? agents // If specific agent requested, include it even if not available
     : agents.filter((a) => a.available);

   if (availableAgents.length === 0) {
     console.log(`${YELLOW}No supported agents detected.${RESET}`);
     console.log(`${DIM}Install Claude Code, OpenCode, or Factory Droid to use skills.${RESET}`);
     return;
   }
+
+  // Warn if specific agent requested but not available
+  if (agentId && !availableAgents[0].available) {
+    console.log(`${YELLOW}Warning:${RESET} ${availableAgents[0].meta.name} is not installed.`);
+    console.log(`${DIM}Skills will be installed to the expected path, but the agent may not find them.${RESET}\n`);
+  }

src/setup/skill-installer.ts (1)

366-374: Consider whether overriding results for the same skill across directories is the intended behaviour.

The comment on lines 370-372 notes that when installing to multiple directories (personal then repo), later results override earlier ones in the allResults map. This means:

• If a skill succeeds in personal but fails in repo, the failure is reported
• If a skill is skipped in personal but succeeds in repo, success is reported

This may be intentional (repo is the "final" state), but could also mask partial failures. Consider whether aggregating results (e.g., storing { personal: result1, repo: result2 }) would be more useful for diagnostics.

website/content/docs/getting-started/installation.mdx (1)

191-195: Factory Droid example now included — previous feedback addressed.

The per-agent install examples now correctly include all three supported agents: claude, opencode, and droid.

website/content/docs/getting-started/installation.mdx (1)

171-177: Consider clarifying that repo-local paths also exist.

The table shows personal skill locations, but the PR objectives indicate each agent also supports repo-local paths (e.g., .claude/skills/, .opencode/skill/, .factory/skills/). If users might install skills at the repository level, briefly mentioning this could be helpful.

📄 Suggested documentation addition

 | Agent | Skills Location |
 |-------|-----------------|
 | Claude Code | `~/.claude/skills/` |
 | OpenCode | `~/.opencode/skill/` |
 | Factory Droid | `~/.factory/skills/` |
+
+> **Note:** Repository-local skill paths (e.g., `.claude/skills/`) are also supported for project-specific skills.

src/commands/skills.test.ts (1)

169-174: Fragile fallback logic weakens test coverage.

The conditional fallback silently passes when no agent paths are found by checking for the section header instead. This means the test never actually verifies agent paths in CI, defeating its purpose.

Consider mocking getSkillCapableAgents to ensure consistent test behaviour across environments.

Suggested approach: mock agent detection

// At top of file or in beforeEach
import { getSkillCapableAgents } from '../plugins/agents/registry.js';

// In test setup, mock to return at least one available agent
const mockAgents = [
  { id: 'claude', name: 'Claude Code', available: true, skillPaths: { personal: '~/.claude/skills/' } }
];
// Use spyOn or mock.module to inject mockAgents

Alternatively, if mocking is complex, skip the test explicitly when no agents are detected rather than using a silent fallback:

-    if (!hasAgentPaths) {
-      // Fallback: ensure at least the section structure is present
-      expect(allOutput).toContain('Installation Status by Agent');
-    } else {
-      expect(hasAgentPaths).toBe(true);
-    }
+    if (!hasAgentPaths) {
+      console.log('Skipping: No agent paths found in test environment');
+      return;
+    }
+    expect(hasAgentPaths).toBe(true);

src/commands/skills.test.ts (1)

157-179: The CI fallback addresses the previous failure, but the assertion logic is tautological.

The fallback pattern handles CI environments without agents. However, the else branch at line 177 (expect(hasAgentPaths).toBe(true)) is redundant — if execution reaches the else branch, hasAgentPaths is already truthy by definition.

♻️ Suggested simplification

     // In CI environments, the agent registry should still show paths for all
     // registered agents. If somehow no paths appear, at minimum we should
     // see the "Installation Status by Agent" section header.
     if (!hasAgentPaths) {
       // Fallback: ensure at least the section structure is present
       expect(allOutput).toContain('Installation Status by Agent');
-    } else {
-      expect(hasAgentPaths).toBe(true);
     }
+    // If hasAgentPaths is true, the test passes implicitly
   });

src/setup/skill-installer.test.ts (1)

465-570: Consider adding a personal+repo install test.
You cover default personal-only and repo-only flows, but not the combined case. Adding a test for personal: true + repo: true would catch result-merging edge cases.