feat(skills): add --list and --all flags to skills add

[christso]

Summary

Adds two flags to allagents skills add to bring it closer to npx skills parity:

• --list / -l — discover skills at a --from source without installing. Reads SKILL.md frontmatter for each skill and prints name/description. Works with GitHub URLs, owner/repo shorthand, and marketplace sources. JSON mode emits a structured listing.
• --all — install every discovered skill from a --from source. Sets the plugin's skill allowlist to the full set and syncs. For marketplace sources, iterates all marketplace plugins and enables every skill in each.

Validation:

• --list requires --from; cannot be combined with --all.
• --all requires --from; cannot be combined with a positional skill name.
• Without either flag, the skill positional remains required (existing behavior).

Closes #368
Closes #369

Test plan

• bun test — 1183 pass / 0 fail
• bun run typecheck — clean
• bun run lint — clean
• Red E2E: skills add --list --from rstackjs/agent-skills fails on main with "Unknown arguments"
• Green E2E (--list, no workspace required):

  ./dist/index.js skills add --list --from rstackjs/agent-skills

  Prints 21 skills with descriptions. JSON mode (--json) emits structured data with isMarketplace, skills[].name, skills[].description.
• Green E2E (--all):

  mkdir /tmp/test && cd /tmp/test
  ./dist/index.js workspace init --client claude
  ./dist/index.js skills add --all --from rstackjs/agent-skills
  ls .claude/skills/ | wc -l   # → 21

  .allagents/workspace.yaml records the source with an explicit skills: allowlist containing all 21 skills.
• Error cases:
  • skills add --list (no --from) → exit 1 with clear error
  • skills add --all (no --from) → exit 1
  • skills add --list --all --from ... → exit 1
  • skills add foo --all --from ... → exit 1

🤖 Generated with Claude Code

[cloudflare-workers-and-pages]

Deploying allagents with    Cloudflare Pages

Latest commit:	1cdc55c
Status:	✅  Deploy successful!
Preview URL:	https://ff18b0c1.allagents.pages.dev
Branch Preview URL:	https://feat-368-369-skills-add-list.allagents.pages.dev

View logs

[christso]

Code Review — feat(skills): --list and --all flags

Overall the feature is well-structured and the happy paths work. Five issues worth addressing before merge, ranging from a latent bug to meaningful test gaps.

Summary of findings

1. Guard/resolve ordering bug in discoverSkillsFromSource — not a crash today, but semantically wrong
2. Double fetch for marketplace --all path — fetchPlugin is called twice
3. Positional skill arg silently ignored when --list is supplied with a skill name
4. Sync output regression — --all drops per-plugin detail that the rest of the codebase shows via formatSyncHeader/formatSyncSummary
5. Test coverage gaps — handler-level validation, JSON output, and the marketplace branch of discoverSkillsFromSource are all untested

Details inline below.

[christso]

Issue 1 — Guard/resolve ordering is inverted

resolvePluginSourcePath is called on line 680 before the typeof plugin.source === 'object' guard on line 682. When source is an object (a remote URL ref), resolvePluginSourcePath returns source.url — a raw URL string — and then existsSync(url) is called on it. existsSync silently returns false for a URL string, so it happens not to crash, but the ordering is wrong and could break if resolvePluginSourcePath ever throws on object sources.

Move the guard before the resolve call:

for (const plugin of manifestResult.data.plugins) {
  // Skip remote URL sources first — listing would need extra fetches
  if (typeof plugin.source === 'object') continue;
  const resolved = resolvePluginSourcePath(plugin.source, fetchResult.cachePath);
  if (!existsSync(resolved)) continue;
  const skills = await discoverSkillsWithMetadata(resolved, plugin.name);
  all.push(...skills);
}

[christso]

Issue 2 — Double fetch on marketplace --all path

installAllSkillsFromSource calls fetchPlugin on line 713 to detect whether the source is a marketplace, then on the marketplace branch (line 723) delegates to installAllViaMarketplace, which calls addMarketplace or updateMarketplace — both of which call fetchPlugin again internally.

The fetchResult.cachePath is already available here. Pass it through so installAllViaMarketplace can skip the refetch:

if (manifestResult.success) {
  return installAllViaMarketplace({ from, isUser, workspacePath, cachedPath: fetchResult.cachePath });
}

And accept cachedPath in installAllViaMarketplace to use instead of re-fetching. For non-large repos this is just a perf nuisance, but for users on slow connections running --all on a large marketplace it doubles the wait time.

[christso]

Issue 3 — Positional skill arg silently ignored when --list is used

If a user runs allagents skills add my-skill --list --from repo, the skillArg is silently dropped because the if (list) branch short-circuits before any check for skillArg. The user likely made a mistake (maybe they expected filtered output), but they get no feedback.

Add a validation guard at the top of the list branch:

if (list) {
  if (skillArg) {
    const error = 'Cannot combine a skill argument with --list. Use --list alone to discover available skills.';
    // emit error + exit(1)
  }
  // ... rest of --list logic
}

[christso]

Issue 4 — --all drops sync detail that the rest of the codebase shows

After --all completes, this prints only 'Sync complete.'. Every other call site in this file (regular add, plugin install, plugin uninstall) uses formatSyncHeader/formatSyncSummary from src/cli/format-sync.ts to print per-plugin file counts and failure detail.

Users running --all on a large marketplace want to know how many files were actually copied and whether anything failed. This is also where a partial failure (some plugins synced, some didn't) would go unnoticed.

Replace the bare console.log('Sync complete.') with:

// use the full SyncResult from syncWorkspace — not the stripped { copied, failed } shape
for (const line of formatSyncHeader(fullSyncResult)) console.log(line);
for (const line of formatSyncSummary(fullSyncResult)) console.log(line);

This means threading the full SyncResult object up through installAllSkillsFromSource / installAllViaMarketplace rather than the current stripped { copied, failed } shape.

[christso]

Issue 5 — Test coverage gaps

The tests only cover discoverSkillsWithMetadata. The actual paths users will hit are untested:

Missing test cases:

• Handler validation: --list without --from → should exit 1 with error message
• Handler validation: --all without --from → same
• Handler validation: --list --all together → should exit 1
• Handler validation: no skill + no flag → should exit 1 with hint message
• discoverSkillsFromSource — the marketplace branch (mocked manifest with plugins) is never exercised
• JSON output shape for --list — isMarketplace, skills[].plugin field only appears for marketplace sources

The root-layout fallback in resolveSkillMdPath (line 631 — returning join(pluginPath, 'SKILL.md') when neither standard nor flat path exists) is also untested. This is the path a single-skill root-layout repo would hit.

Handler-level validation tests can use the existing test pattern for other command handlers in the project rather than invoking the CLI binary.