Skip to content

fix: normalize --agent flag, migrate 6 commands to onboarding helpers #99

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rohitg00 merged 3 commits into from
Apr 7, 2026
Merged

fix: normalize --agent flag, migrate 6 commands to onboarding helpers #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
64944a1
fix: normalize --agent flag, migrate 6 commands to onboarding helpers
rohitg00 Apr 7, 2026
d9142c8
fix: simplify review — error styling, double-wrap, aliases
rohitg00 Apr 7, 2026
f714a39
fix: add deprecation warning when --agents is used
rohitg00 Apr 7, 2026
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
34 changes: 17 additions & 17 deletions packages/cli/src/commands/enable.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import chalk from 'chalk';
import { colors, warn, success, error } from '../onboarding/index.js';
import { Command, Option } from 'clipanion';
import { setSkillEnabled, findSkill } from '@skillkit/core';
import { getSearchDirs } from '../helpers.js';
Expand All @@ -18,36 +18,36 @@ export class EnableCommand extends Command {

async execute(): Promise<number> {
const searchDirs = getSearchDirs();
let success = 0;
let successCount = 0;
let failed = 0;

for (const skillName of this.skills) {
const skill = findSkill(skillName, searchDirs);

if (!skill) {
console.log(chalk.red(`Skill not found: ${skillName}`));
error(`Skill not found: ${skillName}`);
failed++;
continue;
}

if (skill.enabled) {
console.log(chalk.dim(`Already enabled: ${skillName}`));
console.log(colors.muted(`Already enabled: ${skillName}`));
continue;
}

const result = setSkillEnabled(skill.path, true);

if (result) {
console.log(chalk.green(`Enabled: ${skillName}`));
success++;
success(`Enabled: ${skillName}`);
successCount++;
} else {
console.log(chalk.red(`Failed to enable: ${skillName}`));
error(`Failed to enable: ${skillName}`);
failed++;
}
}

if (success > 0) {
console.log(chalk.dim('\nRun `skillkit sync` to update your agent config'));
if (successCount > 0) {
console.log(colors.muted('\nRun `skillkit sync` to update your agent config'));
}

return failed > 0 ? 1 : 0;
Expand All @@ -69,36 +69,36 @@ export class DisableCommand extends Command {

async execute(): Promise<number> {
const searchDirs = getSearchDirs();
let success = 0;
let successCount = 0;
let failed = 0;

for (const skillName of this.skills) {
const skill = findSkill(skillName, searchDirs);

if (!skill) {
console.log(chalk.red(`Skill not found: ${skillName}`));
error(`Skill not found: ${skillName}`);
failed++;
continue;
}

if (!skill.enabled) {
console.log(chalk.dim(`Already disabled: ${skillName}`));
console.log(colors.muted(`Already disabled: ${skillName}`));
continue;
}

const result = setSkillEnabled(skill.path, false);

if (result) {
console.log(chalk.yellow(`Disabled: ${skillName}`));
success++;
warn(`Disabled: ${skillName}`);
successCount++;
} else {
console.log(chalk.red(`Failed to disable: ${skillName}`));
error(`Failed to disable: ${skillName}`);
failed++;
}
}

if (success > 0) {
console.log(chalk.dim('\nRun `skillkit sync` to update your agent config'));
if (successCount > 0) {
console.log(colors.muted('\nRun `skillkit sync` to update your agent config'));
}

return failed > 0 ? 1 : 0;
Expand Down
7 changes: 5 additions & 2 deletions packages/cli/src/commands/generate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export class GenerateCommand extends Command {
['Interactive wizard', '$0 generate'],
['Use specific provider', '$0 generate --provider openai'],
['Compose from existing skills', '$0 generate --compose "testing patterns for vitest"'],
['Target specific agents', '$0 generate --agents claude-code,cursor'],
['Target specific agents', '$0 generate --agent claude-code,cursor'],
['Skip memory context', '$0 generate --no-memory'],
],
});
Expand All @@ -50,7 +50,7 @@ export class GenerateCommand extends Command {
description: 'Natural language search to find skills to compose',
});

agents = Option.String('--agents,-a', {
agents = Option.String('--agent,--agents,-a', {
description: 'Target agents (comma-separated)',
});

Expand Down Expand Up @@ -536,6 +536,9 @@ ${trustScore.warnings.length > 0 ? `\nWarnings:\n${trustScore.warnings.map((w) =
let targetAgents: string[];

if (this.agents) {
if (process.argv.includes('--agents')) {
console.warn('Warning: --agents is deprecated, use --agent instead (--agents will be removed in v2.0)');
}
targetAgents = this.agents.split(',').map((a) => a.trim());
} else {
const agentOptions = [
Expand Down
21 changes: 10 additions & 11 deletions packages/cli/src/commands/list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import chalk from 'chalk';
import { Command, Option } from 'clipanion';
import { findAllSkills, evaluateSkillDirectory } from '@skillkit/core';
import { getSearchDirs } from '../helpers.js';
import { formatQualityBadge, colors } from '../onboarding/index.js';
import { formatQualityBadge, colors, warn } from '../onboarding/index.js';

export class ListCommand extends Command {
static override paths = [['list'], ['ls'], ['l']];
Expand Down Expand Up @@ -65,26 +64,26 @@ export class ListCommand extends Command {
}

if (skills.length === 0) {
console.log(chalk.yellow('No skills installed'));
console.log(chalk.dim('Install skills with: skillkit install <source>'));
warn('No skills installed');
console.log(colors.muted('Install skills with: skillkit install <source>'));
return 0;
}

console.log(chalk.cyan(`Installed skills (${skills.length}):\n`));
console.log(colors.cyan(`Installed skills (${skills.length}):\n`));

const projectSkills = skillsWithQuality.filter(s => s.location === 'project');
const globalSkills = skillsWithQuality.filter(s => s.location === 'global');

if (projectSkills.length > 0) {
console.log(chalk.blue('Project skills:'));
console.log(colors.info('Project skills:'));
for (const skill of projectSkills) {
printSkill(skill, this.quality);
}
console.log();
}

if (globalSkills.length > 0) {
console.log(chalk.dim('Global skills:'));
console.log(colors.muted('Global skills:'));
for (const skill of globalSkills) {
printSkill(skill, this.quality);
}
Expand All @@ -95,7 +94,7 @@ export class ListCommand extends Command {
const disabledCount = skills.length - enabledCount;

console.log(
chalk.dim(
colors.muted(
`${projectSkills.length} project, ${globalSkills.length} global` +
(disabledCount > 0 ? `, ${disabledCount} disabled` : '')
)
Expand All @@ -117,9 +116,9 @@ function printSkill(
skill: { name: string; description: string; enabled: boolean; location: string; quality: number | null },
showQuality = false
) {
const status = skill.enabled ? chalk.green('✓') : chalk.red('○');
const name = skill.enabled ? skill.name : chalk.dim(skill.name);
const desc = chalk.dim(truncate(skill.description, 50));
const status = skill.enabled ? colors.success('✓') : colors.error('○');
const name = skill.enabled ? skill.name : colors.muted(skill.name);
const desc = colors.muted(truncate(skill.description, 50));
const qualityBadge = showQuality && skill.quality !== null ? ` ${formatQualityBadge(skill.quality)}` : '';

console.log(` ${status} ${name}${qualityBadge}`);
Expand Down
5 changes: 4 additions & 1 deletion packages/cli/src/commands/manifest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export class ManifestAddCommand extends Command {
description: 'Specific skills to include (comma-separated)',
});

agents = Option.String('--agents,-a', {
agents = Option.String('--agent,--agents,-a', {
description: 'Target agents (comma-separated)',
});
Comment on lines +150 to 152
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 7, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add required deprecation warning for --agents.

This adds alias compatibility, but the command still doesn’t emit the required warning message when users pass --agents (Use --agent instead (--agents will be removed in v2.0)), so issue #94 acceptance criteria is incomplete.

Suggested patch 
  async execute(): Promise<number> {
+    if (process.argv.includes('--agents')) {
+      warn('Use --agent instead (--agents will be removed in v2.0)');
+    }
+
     const options: { skills?: string[]; agents?: string[] } = {};
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/manifest.ts` around lines 150 - 152, The
Option.String definition for agents (agents =
Option.String('--agent,--agents,-a', {...})) adds the deprecated alias but
doesn't emit a deprecation warning; update the command handling where the parsed
options are processed (the code path that reads the agents option) to detect if
the user explicitly provided the deprecated flag `--agents` and log the exact
message "Use --agent instead (--agents will be removed in v2.0)" (e.g., via the
command logger or console.warn), while still honoring the value, so users see
the warning whenever the `--agents` alias is used; reference the agents
Option.String symbol to locate the option and add the conditional deprecation
notice near the option parsing/validation logic.


Expand All @@ -159,6 +159,9 @@ export class ManifestAddCommand extends Command {
}

if (this.agents) {
if (process.argv.includes('--agents')) {
console.warn('Warning: --agents is deprecated, use --agent instead (--agents will be removed in v2.0)');
}
options.agents = this.agents.split(',').map(s => s.trim());
}

Expand Down
22 changes: 11 additions & 11 deletions packages/cli/src/commands/remove.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { existsSync, rmSync, readFileSync, writeFileSync } from 'node:fs';
import { join } from 'node:path';
import chalk from 'chalk';
import { colors, warn, success, error } from '../onboarding/index.js';
import { Command, Option } from 'clipanion';
import { findSkill, AgentsMdParser, AgentsMdGenerator } from '@skillkit/core';
import { getSearchDirs } from '../helpers.js';
Expand Down Expand Up @@ -32,22 +32,22 @@ export class RemoveCommand extends Command {
const skill = findSkill(skillName, searchDirs);

if (!skill) {
console.log(chalk.yellow(`Skill not found: ${skillName}`));
warn(`Skill not found: ${skillName}`);
continue;
}

if (!existsSync(skill.path)) {
console.log(chalk.yellow(`Path not found: ${skill.path}`));
warn(`Path not found: ${skill.path}`);
continue;
}

try {
rmSync(skill.path, { recursive: true, force: true });
console.log(chalk.green(`Removed: ${skillName}`));
success(`Removed: ${skillName}`);
removed++;
} catch (error) {
console.log(chalk.red(`Failed to remove: ${skillName}`));
console.error(chalk.dim(error instanceof Error ? error.message : String(error)));
} catch (err) {
error(`Failed to remove: ${skillName}`);
console.error(colors.muted(err instanceof Error ? err.message : String(err)));
failed++;
}
}
Expand All @@ -65,11 +65,11 @@ export class RemoveCommand extends Command {
writeFileSync(agentsMdPath, updated, 'utf-8');
}
}
} catch (error) {
console.log(chalk.yellow('Warning: Failed to update AGENTS.md'));
console.error(chalk.dim(error instanceof Error ? error.message : String(error)));
} catch (err) {
warn('Warning: Failed to update AGENTS.md');
console.error(colors.muted(err instanceof Error ? err.message : String(err)));
}
console.log(chalk.dim('\nRun `skillkit sync` to update your agent config'));
console.log(colors.muted('\nRun `skillkit sync` to update your agent config'));
}

return failed > 0 ? 1 : 0;
Expand Down
7 changes: 4 additions & 3 deletions packages/cli/src/commands/scan.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Command, Option } from 'clipanion';
import { resolve } from 'node:path';
import { existsSync } from 'node:fs';
import { SkillScanner, formatResult, Severity } from '@skillkit/core';
import { error } from '../onboarding/index.js';
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 7, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In @clack/prompts v0.11.x, does clack.log.error write directly to process streams, and is there an officially supported way to route it through custom output streams (like Clipanion this.context.stderr)?

💡 Result:

Yes—in @clack/prompts v0.11.x, log.error() writes directly to the Node process stream (specifically process.stdout.write, not process.stderr). log.error() just delegates to log.message(), and log.message() hard-codes process.stdout.write(...). [1]

No—v0.11.x does not provide an officially supported way to route clack.log.* output to custom streams (e.g. this.context.stderr in Clipanion). There’s no output/stderr option on log.* in v0.11.x; you’d have to wrap/replace it yourself (or monkey-patch process.stdout.write). [1]

(For context: newer @clack/prompts versions added an output option to log.message(...) that lets you pass a custom Writable, but that’s not present in v0.11.x.) [2]

Sources
[1] @clack/prompts@0.11.0 packages/prompts/src/index.ts (log implementation) (github.com)
[2] Current packages/prompts/src/log.ts (shows output = process.stdout option) (github.com)

Citations:

🏁 Script executed:

# First, find and examine the scan.ts file to see the context
fd -t f "scan.ts" --path "*packages/cli*"

Repository: rohitg00/skillkit

Length of output: 294

🏁 Script executed:

# Get the exact lines mentioned in the review - lines 54, 60, 70
sed -n '1,80p' packages/cli/src/commands/scan.ts | cat -n

Repository: rohitg00/skillkit

Length of output: 3301

🏁 Script executed:

# Examine the error() import source
cat -n packages/cli/src/onboarding/index.js | head -50

Repository: rohitg00/skillkit

Length of output: 129

🏁 Script executed:

# Search for what clack is and how it's used in onboarding
rg "clack|log\.error" packages/cli/src/onboarding/ -A 2 -B 2

Repository: rohitg00/skillkit

Length of output: 8204

🏁 Script executed:

# Find the onboarding index file
fd -t f "index" packages/cli/src/onboarding/

Repository: rohitg00/skillkit

Length of output: 97

🏁 Script executed:

# Check the onboarding index content
cat -n packages/cli/src/onboarding/index.ts

Repository: rohitg00/skillkit

Length of output: 7486

🏁 Script executed:

# Check package.json for clack version
rg "@clack/prompts" packages/cli/package.json

Repository: rohitg00/skillkit

Length of output: 93

🏁 Script executed:

# Search for this.context.stderr usage in the codebase
rg "this\.context\.stderr" packages/cli/

Repository: rohitg00/skillkit

Length of output: 12940

🏁 Script executed:

# Check if ScanCommand has direct access to context
rg "class ScanCommand" -A 20 packages/cli/src/commands/scan.ts

Repository: rohitg00/skillkit

Length of output: 919

Use this.context.stderr.write(...) for validation error messages in ScanCommand.

Lines 54, 60, and 70 use error() which delegates to clack.log.error(). In @clack/prompts v0.11.x, log.error() writes directly to process.stdout with no support for custom stream routing. This bypasses this.context.stderr and breaks stderr capture in tests and scripted callers.

Adopt the pattern used in other commands (e.g., team.ts, plugin.ts, methodology.ts): replace error(...) with this.context.stderr.write(chalk.red(...)) for these pre-scan validation checks.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/scan.ts` at line 5, ScanCommand currently calls the
shared error() helper (imported from ../onboarding/index.js) for pre-scan
validation messages which writes to stdout; replace those calls inside the
ScanCommand (the validation branches that call error(...)) with
this.context.stderr.write(chalk.red(<same message>)) so messages go to stderr
and keep the original text, add an import for chalk if missing, and remove the
now-unused error import; ensure usage matches other commands (team.ts,
plugin.ts) so tests and scripted callers can capture stderr correctly.


const SEVERITY_MAP: Record<string, Severity> = {
critical: Severity.CRITICAL,
Expand Down Expand Up @@ -50,13 +51,13 @@ export class ScanCommand extends Command {
const targetPath = resolve(this.skillPath);

if (!existsSync(targetPath)) {
this.context.stderr.write(`Path not found: ${targetPath}\n`);
error(`Path not found: ${targetPath}`);
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot Apr 7, 2026

Choose a reason for hiding this comment

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

🔴 Scan command error messages redirected from stderr to stdout

The scan command previously wrote validation errors to stderr via this.context.stderr.write(...) (Clipanion's stderr stream, which defaults to process.stderr). The new code uses the error() helper which calls clack.log.error(), which writes to stdout. The scan command is designed for CI/pipeline integration (supports --format sarif for GitHub Code Scanning and --format json), where the convention is that machine-readable output goes to stdout and diagnostic/error messages go to stderr. This change breaks that contract — downstream tools that check stderr for error diagnostics will no longer see them.

Affected lines in scan.ts

Lines 54, 60, and 70 all changed from this.context.stderr.write(...) to error(...) which writes to stdout.

Prompt for agents 
The scan command (packages/cli/src/commands/scan.ts) is designed for CI/pipeline use and supports machine-readable output formats (json, sarif). It previously wrote error messages to stderr via this.context.stderr.write(), maintaining the Unix convention that stdout carries data and stderr carries diagnostics. The migration to the error() helper from onboarding/prompts.js routes these messages to stdout instead (via @clack/prompts log.error which writes to process.stdout). To fix, either: (1) keep using this.context.stderr.write() for the scan command's error messages (lines 54, 60, 70), or (2) create a stderrError() helper that wraps process.stderr.write with the same styling as error(). The scan command needs different behavior than interactive commands because its stdout is consumed by machines.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

return 1;
}

const validFormats = ['summary', 'json', 'table', 'sarif'];
if (!validFormats.includes(this.format)) {
this.context.stderr.write(`Invalid format: "${this.format}". Must be one of: ${validFormats.join(', ')}\n`);
error(`Invalid format: "${this.format}". Must be one of: ${validFormats.join(', ')}`);
return 1;
}

Expand All @@ -66,7 +67,7 @@ export class ScanCommand extends Command {
if (this.failOn) {
failOnSeverity = SEVERITY_MAP[this.failOn.toLowerCase()];
if (!failOnSeverity) {
this.context.stderr.write(`Invalid --fail-on value: "${this.failOn}". Must be one of: ${Object.keys(SEVERITY_MAP).join(', ')}\n`);
error(`Invalid --fail-on value: "${this.failOn}". Must be one of: ${Object.keys(SEVERITY_MAP).join(', ')}`);
return 1;
}
}
Expand Down
Loading
Loading