Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,8 @@ skillfish add owner/repo --force # Overwrite existing
skillfish add owner/repo --yes # Skip confirmation
skillfish add owner/repo --project # Project only (./)
skillfish add owner/repo --global # Global only (~/)
skillfish add owner/repo --agent Cursor # Install to one detected agent
skillfish add owner/repo --agent Cursor --agent Codex # Install to multiple agents
```

### init
Expand Down Expand Up @@ -324,7 +326,7 @@ In non-interactive mode, commands use default values where possible and error wi

| Command | Required | Defaults |
|---------|----------|----------|
| `add` | `<owner/repo>` + skill name, `--path`, or `--all` if repo has multiple skills | Location: global (`~/`), Agents: all detected |
| `add` | `<owner/repo>` + skill name, `--path`, or `--all` if repo has multiple skills | Location: global (`~/`), Agents: all detected unless one or more `--agent` flags are passed |
| `init` | `--name`, `--description` | Location: project (`./`), Agents: all detected |
| `list` | (none) | Both locations, all agents |
| `remove` | Skill name or `--all` | Both locations, all agents |
Expand Down Expand Up @@ -372,6 +374,9 @@ Exit codes are consistent across all commands:
# Install skills in CI (non-interactive, JSON output)
skillfish add owner/repo --yes --json

# Install to specific detected agents (repeat --agent as needed)
skillfish add owner/repo --agent "Claude Code" --agent Cursor --yes --json

# Create a skill template in CI
skillfish init --name my-skill --description "My skill" --project --json

Expand Down
157 changes: 157 additions & 0 deletions src/__tests__/add-agents.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
/**
* Command-level tests for `skillfish add --agent` routing.
*/

import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { Command } from 'commander';
import { addCommand } from '../commands/add.js';
import { getDetectedAgentsForLocation, type Agent } from '../lib/agents.js';
import { fetchDefaultBranch, fetchTreeSha } from '../lib/github.js';
import { installSkill } from '../lib/installer.js';

vi.mock('../lib/agents.js', async (importOriginal) => {
const actual = await importOriginal<typeof import('../lib/agents.js')>();
return { ...actual, getDetectedAgentsForLocation: vi.fn() };
});

vi.mock('../lib/github.js', async (importOriginal) => {
const actual = await importOriginal<typeof import('../lib/github.js')>();
return {
...actual,
fetchDefaultBranch: vi.fn(),
fetchTreeSha: vi.fn(),
};
});

vi.mock('../lib/installer.js', async (importOriginal) => {
const actual = await importOriginal<typeof import('../lib/installer.js')>();
return { ...actual, installSkill: vi.fn() };
});

vi.mock('../telemetry.js', () => ({
trackCommand: vi.fn(),
trackInstall: vi.fn(),
}));

const mockGetDetectedAgents = vi.mocked(getDetectedAgentsForLocation);
const mockFetchDefaultBranch = vi.mocked(fetchDefaultBranch);
const mockFetchTreeSha = vi.mocked(fetchTreeSha);
const mockInstallSkill = vi.mocked(installSkill);

class ProcessExit extends Error {
constructor(readonly code: number) {
super(`process.exit(${code})`);
}
}

const detectedAgents: readonly Agent[] = [
{ name: 'Claude Code', dir: '.claude/skills', detect: () => true },
{ name: 'Cursor', dir: '.cursor/skills', detect: () => true },
{ name: 'Codex', dir: '.codex/skills', detect: () => true },
];

const program = new Command().option('--json');
program.setOptionValue('json', true);
program.setOptionValue('version', 'test');
program.addCommand(addCommand);

async function runAdd(args: string[]): Promise<{ exitCode: number; output: unknown }> {
const output: string[] = [];
vi.spyOn(console, 'log').mockImplementation((message?: unknown) => {
output.push(String(message));
});
vi.spyOn(process, 'exit').mockImplementation((code) => {
throw new ProcessExit(typeof code === 'number' ? code : Number(code ?? 0));
});

for (const option of ['force', 'yes', 'all', 'project', 'global', 'path']) {
addCommand.setOptionValue(option, undefined);
}
addCommand.setOptionValue('agent', undefined);

let exitCode = -1;
try {
await program.parseAsync(['node', 'skillfish', 'add', ...args]);
} catch (error) {
if (!(error instanceof ProcessExit)) {
throw error;
}
exitCode = error.code;
}

return { exitCode, output: JSON.parse(output.at(-1) ?? '{}') };
}

describe('add command agent routing', () => {
beforeEach(() => {
vi.clearAllMocks();
mockGetDetectedAgents.mockReturnValue(detectedAgents);
mockFetchDefaultBranch.mockResolvedValue('main');
mockFetchTreeSha.mockResolvedValue('root-sha');
mockInstallSkill.mockResolvedValue({
installed: [],
skipped: [],
warnings: [],
failed: false,
});
});

afterEach(() => {
vi.restoreAllMocks();
});

it('passes only repeated requested agents to the installer', async () => {
const result = await runAdd([
'owner/repo',
'--path',
'skills/example',
'--global',
'--agent',
'claude code',
'--agent',
'Cursor',
'--agent',
'CLAUDE CODE',
]);

expect(result.exitCode).toBe(0);
expect(mockInstallSkill).toHaveBeenCalledOnce();
expect(mockInstallSkill.mock.calls[0][4].map((agent) => agent.name)).toEqual([
'Claude Code',
'Cursor',
]);
});

it('rejects mixed valid and unknown agents before installation', async () => {
const result = await runAdd([
'owner/repo',
'--path',
'skills/example',
'--global',
'--agent',
'Cursor',
'--agent',
'Unknown Agent',
]);

expect(result.exitCode).toBe(4);
expect(mockInstallSkill).not.toHaveBeenCalled();
expect(result.output).toMatchObject({
success: false,
exit_code: 4,
errors: [expect.stringContaining('Unknown Agent')],
});
});

it('passes all detected agents when --agent is omitted', async () => {
const result = await runAdd(['owner/repo', '--path', 'skills/example', '--global']);

expect(result.exitCode).toBe(0);
expect(mockInstallSkill).toHaveBeenCalledOnce();
expect(mockInstallSkill.mock.calls[0][4].map((agent) => agent.name)).toEqual([
'Claude Code',
'Cursor',
'Codex',
]);
});
});
8 changes: 8 additions & 0 deletions src/__tests__/add.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import { describe, it, expect } from 'vitest';
import { invokeCli } from './invoke-cli.js';
import { addCommand } from '../commands/add.js';

describe('add command', () => {
it('shows help with --help', () => {
Expand All @@ -13,6 +14,7 @@ describe('add command', () => {
expect(stdout).toContain('--force');
expect(stdout).toContain('--yes');
expect(stdout).toContain('--all');
expect(stdout).toContain('--agent');
});

it('requires a repository argument', () => {
Expand All @@ -21,6 +23,12 @@ describe('add command', () => {
expect(stderr).toContain("required argument 'repo'");
});

it('collects repeated --agent flags', () => {
addCommand.parseOptions(['owner/repo', '--agent', 'Claude Code', '--agent', 'Cursor']);

expect(addCommand.opts().agent).toEqual(['Claude Code', 'Cursor']);
});

it('exits with error for invalid repo format (single part)', () => {
const { exitCode, stderr } = invokeCli(['add', 'invalid']);
expect(exitCode).toBe(2);
Expand Down
31 changes: 31 additions & 0 deletions src/__tests__/agents.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {
detectAgentInProject,
getDetectedAgentsForLocation,
getAgentSkillDir,
resolveRequestedAgents,
type Agent,
type AgentConfig,
} from '../lib/agents.js';

Expand Down Expand Up @@ -268,4 +270,33 @@ describe('agents.ts', () => {
expect(getAgentSkillDir(agent, '/project')).toBe('/project/.cursor/skills');
});
});

describe('resolveRequestedAgents', () => {
const agents: readonly Agent[] = [
{ name: 'Claude Code', dir: '.claude/skills', detect: () => true },
{ name: 'Cursor', dir: '.cursor/skills', detect: () => true },
{ name: 'Codex', dir: '.codex/skills', detect: () => true },
];

it('matches multiple agent names case-insensitively', () => {
const result = resolveRequestedAgents(agents, ['claude code', 'CURSOR']);

expect(result.agents.map((agent) => agent.name)).toEqual(['Claude Code', 'Cursor']);
expect(result.unknownNames).toEqual([]);
});

it('deduplicates repeated agent names', () => {
const result = resolveRequestedAgents(agents, ['Codex', 'codex']);

expect(result.agents.map((agent) => agent.name)).toEqual(['Codex']);
expect(result.unknownNames).toEqual([]);
});

it('reports every unknown agent name without dropping valid selections', () => {
const result = resolveRequestedAgents(agents, ['Cursor', 'Unknown', 'Also Unknown']);

expect(result.agents.map((agent) => agent.name)).toEqual(['Cursor']);
expect(result.unknownNames).toEqual(['Unknown', 'Also Unknown']);
});
});
});
32 changes: 30 additions & 2 deletions src/commands/add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
} from '../utils.js';
import {
getDetectedAgentsForLocation,
resolveRequestedAgents,
type Agent,
type DetectionLocation,
AGENT_CONFIGS,
Expand Down Expand Up @@ -53,6 +54,7 @@ interface AddCommandOptions {
project?: boolean;
global?: boolean;
path?: string;
agent?: string[];
}

interface SkillMetadata {
Expand All @@ -62,6 +64,10 @@ interface SkillMetadata {
description: string; // From frontmatter or empty
}

function collectAgentName(value: string, previous?: string[]): string[] {
return [...(previous ?? []), value];
}

// === Command Definition ===

export const addCommand = new Command('add')
Expand All @@ -74,6 +80,7 @@ export const addCommand = new Command('add')
.option('--project', 'Install to current project (./.claude)')
.option('--global', 'Install to home directory (~/.claude)')
.option('--path <path>', 'Path to a specific skill in the repository')
.option('--agent <name>', 'Install to a specific detected agent (repeatable)', collectAgentName)
.helpOption('-h, --help', 'Display help for command')
.addHelpText(
'after',
Expand All @@ -84,7 +91,9 @@ Examples:
$ skillfish add owner/repo --all Install all skills in repo
$ skillfish add owner/repo/plugin/skill Install a specific skill by path
$ skillfish add owner/repo --path path/to Install skill at specific path
$ skillfish add owner/repo --project Install to current project only`,
$ skillfish add owner/repo --project Install to current project only
$ skillfish add owner/repo --agent Cursor Install to Cursor only
$ skillfish add owner/repo --agent Cursor --agent Codex Install to multiple agents`,
)
.action(
async (
Expand Down Expand Up @@ -142,6 +151,7 @@ Examples:
const installAll = options.all ?? false;
const projectFlag = options.project ?? false;
const globalFlag = options.global ?? false;
const requestedAgentNames = options.agent ?? [];
let explicitPath: string | null = options.path ?? null;

// Validate flag conflicts
Expand Down Expand Up @@ -279,7 +289,25 @@ Examples:

let targetAgents: readonly Agent[];

if (!isInputTTY() || jsonMode) {
if (requestedAgentNames.length > 0) {
const { agents, unknownNames } = resolveRequestedAgents(detected, requestedAgentNames);

if (unknownNames.length > 0) {
const noun = unknownNames.length === 1 ? 'Agent' : 'Agents';
exitWithError(
`${noun} not found: ${unknownNames.map((name) => `"${name}"`).join(', ')}. Detected agents: ${detected.map((agent) => agent.name).join(', ')}`,
EXIT_CODES.NOT_FOUND,
true,
);
}

targetAgents = agents;
if (!jsonMode) {
console.log(
`Installing to ${targetAgents.length} agent(s): ${targetAgents.map((agent) => agent.name).join(', ')}`,
);
}
} else if (!isInputTTY() || jsonMode) {
// Non-TTY or JSON mode: use all detected agents
if (!jsonMode) {
console.log(
Expand Down
38 changes: 38 additions & 0 deletions src/lib/agents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,11 @@ export interface Agent {
readonly detect: () => boolean;
}

export interface ResolvedAgents {
readonly agents: readonly Agent[];
readonly unknownNames: readonly string[];
}

/**
* Location context for agent detection.
*/
Expand Down Expand Up @@ -298,6 +303,39 @@ export function getDetectedAgentsForLocation(
}));
}

/**
* Resolve user-provided agent names against a detected agent list.
* Matching is case-insensitive and repeated names are returned only once.
*/
export function resolveRequestedAgents(
detectedAgents: readonly Agent[],
requestedNames: readonly string[],
): ResolvedAgents {
const detectedByName = new Map(
detectedAgents.map((agent) => [agent.name.toLowerCase(), agent] as const),
);
const agents: Agent[] = [];
const unknownNames: string[] = [];
const seenNames = new Set<string>();

for (const requestedName of requestedNames) {
const normalizedName = requestedName.toLowerCase();
if (seenNames.has(normalizedName)) {
continue;
}
seenNames.add(normalizedName);

const agent = detectedByName.get(normalizedName);
if (agent) {
agents.push(agent);
} else {
unknownNames.push(requestedName);
}
}

return { agents, unknownNames };
}

/**
* Get the skill directory path for an agent.
* Uses globalDir (if defined) when baseDir is the home directory.
Expand Down