feat(cli): add archon skill install command#1445
Merged
Conversation
Adds a standalone `archon skill install [path]` subcommand that copies the bundled Archon skill files into `<target>/.claude/skills/archon/`, so users can install or refresh the skill outside the interactive setup wizard. Defaults to the current directory. Refactors `copyArchonSkill` out of `commands/setup.ts` into a new `commands/skill.ts` so the helper can be shared between the wizard and the new CLI command without pulling in `@clack/prompts`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new top-level Changes
Sequence DiagramsequenceDiagram
participant User as User/CLI
participant CLI as cli.ts
participant SkillCmd as skillInstallCommand
participant CopyFn as copyArchonSkill
participant Bundled as Bundled Skill
participant FS as File System
User->>CLI: archon skill install [path]
CLI->>CLI: parse args, resolve target path
CLI->>SkillCmd: skillInstallCommand(targetPath)
SkillCmd->>FS: check target directory exists
alt directory missing
FS-->>SkillCmd: error
SkillCmd-->>User: log "Directory does not exist"
SkillCmd-->>CLI: return 1
else directory exists
SkillCmd->>CopyFn: copyArchonSkill(targetPath)
CopyFn->>Bundled: dynamic import BUNDLED_SKILL_FILES
Bundled-->>CopyFn: file map
loop per bundled file
CopyFn->>FS: create dirs and write/overwrite file to .claude/skills/archon/
end
CopyFn-->>SkillCmd: success
SkillCmd-->>User: log completion + restart message
SkillCmd-->>CLI: return 0
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Collaborator
Author
🔍 Comprehensive PR ReviewPR: #1445 — feat(cli): add archon skill install command SummaryClean, well-structured PR that extracts Verdict:
🟠 High Issues (Documentation Gaps)1. CLAUDE.md Missing
|
| Issue | Location | Agent | Recommendation |
|---|---|---|---|
Double resolve() in skillInstallCommand |
skill.ts:42 |
Code Review | Keep — defensive, idempotent |
async without await |
skill.ts:41 |
Code Review | Keep — matches CLI convention |
Catch discards err.code |
skill.ts:55 |
Error Handling | Keep — err.message includes it |
| Partial write not communicated | skill.ts:22-31 |
Error Handling | Keep — idempotent re-run fixes |
| CLI dispatch untested | cli.ts:573-598 |
Test Coverage | Keep — matches codebase pattern |
| Docstring lists specific file types | skill.ts:4 |
Comment Quality | Keep — accurate today |
✅ What's Good
- Clean refactor —
copyArchonSkillextracted without behavioral changes - Follows existing patterns —
skilldispatch mirrorsvalidateexactly - Strong test coverage — install, overwrite, success, and error paths all tested
- Proper test isolation —
spyOnonly, correct batch grouping, no mock pollution - Fail-fast design —
existsSyncguard + try/catch with clear error messages - Idempotent — always overwrites, safe to re-run
- No silent failures — every error path reports to user with exit code 1
- Concise — 61 lines source, 85 lines tests
📋 Next Steps
- 📝 Add
skill installto CLAUDE.md CLI section (HIGH) - 📝 Add
skill installto docs-web CLI reference (HIGH) - 📝 Consider mentioning bundled skill in skills guide (MEDIUM)
- 🎯 Merge when documentation is updated
| Agent | Findings | Verdict |
|---|---|---|
| Code Review | 2 LOW | APPROVE |
| Error Handling | 2 LOW | APPROVE |
| Test Coverage | 2 LOW | APPROVE |
| Comment Quality | 1 LOW | APPROVE |
| Docs Impact | 2 HIGH + 1 MEDIUM | UPDATES_REQUIRED |
Reviewed by Archon comprehensive-pr-review workflow
- Add `skill install` command entries to CLAUDE.md CLI section - Add `skill install` section to docs-web CLI reference page - Add bundled Archon skill to Popular Skills table in skills guide Addresses HIGH findings from comprehensive PR review. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
Author
⚡ Auto-Fix ReportStatus: COMPLETE Fixes Applied
What Was Fixed
Commit
🟢 Low Issues (No Action Needed)All 6 LOW findings were independently assessed as "keep as-is" by their review agents. See the comprehensive review comment for details. Validation✅ Type check | ✅ Lint | ✅ Format Auto-fixed by Archon comprehensive-pr-review workflow |
Resolves conflict in packages/cli/src/commands/setup.ts: PR extracted copyArchonSkill into ./skill while dev (#1394) added a lazy-import fix inline. Kept PR's structure (function lives in skill.ts) and applied dev's lazy-import pattern there so archon --help no longer crashes when source skill files are missing on linked-source installs. - packages/cli/src/commands/skill.ts: dynamic-import bundled-skill; copyArchonSkill is now async - packages/cli/src/commands/setup.ts: await copyArchonSkill at the setup-wizard call site - packages/cli/src/commands/skill.test.ts: await the now-async helper Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/cli/src/cli.ts (1)
579-582: ⚡ Quick winResolve relative install paths against
--cwdfor consistency.When users pass both
--cwdand a relative[path],resolve(targetArg)is based onprocess.cwd(), not the overriddencwd. Considerresolve(cwd, targetArg)to keep path behavior consistent with global--cwd.Proposed diff
- const targetPath = targetArg ? resolve(targetArg) : cwd; + const targetPath = targetArg ? resolve(cwd, targetArg) : cwd;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/cli.ts` around lines 579 - 582, The positional install path is being resolved relative to process.cwd() instead of the overridden cwd, so update the resolution logic to use the resolved CLI cwd as the base: when computing targetPath from positionals[2] (targetArg), call resolve(cwd, targetArg) rather than resolve(targetArg), and then pass that targetPath into skillInstallCommand; keep the existing fallback of using cwd when no positional is provided and ensure symbols referenced are positionals, targetArg, targetPath, cwd and skillInstallCommand.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/skill.ts`:
- Around line 55-67: The import of BUNDLED_SKILL_FILES is outside the
error-handling path so failures there can crash the CLI; move or replicate the
dynamic import (await import('../bundled-skill')) into the same try block that
calls copyArchonSkill(absoluteTarget) (or wrap it in its own try/catch) and on
failure log the same styled error and return 1 (use the same error handling used
for copyArchonSkill), ensuring any references to BUNDLED_SKILL_FILES and the
computed skillRoot are only used after the guarded import succeeds.
---
Nitpick comments:
In `@packages/cli/src/cli.ts`:
- Around line 579-582: The positional install path is being resolved relative to
process.cwd() instead of the overridden cwd, so update the resolution logic to
use the resolved CLI cwd as the base: when computing targetPath from
positionals[2] (targetArg), call resolve(cwd, targetArg) rather than
resolve(targetArg), and then pass that targetPath into skillInstallCommand; keep
the existing fallback of using cwd when no positional is provided and ensure
symbols referenced are positionals, targetArg, targetPath, cwd and
skillInstallCommand.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 34f65baa-c111-4324-995a-01e9c3df6a1d
📒 Files selected for processing (9)
CLAUDE.mdpackages/cli/package.jsonpackages/cli/src/cli.tspackages/cli/src/commands/setup.test.tspackages/cli/src/commands/setup.tspackages/cli/src/commands/skill.test.tspackages/cli/src/commands/skill.tspackages/docs-web/src/content/docs/guides/skills.mdpackages/docs-web/src/content/docs/reference/cli.md
…lock
The dynamic `await import('../bundled-skill')` was outside the try/catch,
so a load failure crashed uncaught instead of returning exit code 1.
Move the import (and the success log + return) inside the try so import,
copy, and post-copy errors all flow through the same controlled path.
Addresses coderabbitai review on PR #1445.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
archon setupwizard..claude/skills/archon/so Claude Code picks it up.archon skill install [path](defaulting to cwd), and refactors the existingcopyArchonSkillhelper out ofcommands/setup.tsinto a dedicatedcommands/skill.tsso the helper is shared without dragging@clack/promptsinto the new code path.bundled-skill.tsstatic-imports module, or any other CLI command. No changes to@archon/workflows, server, web, or adapters.UX Journey
Before
After
Architecture Diagram
Before
After
Connection inventory:
cli.tscommands/skill.tsskill installdispatchcommands/setup.tscommands/skill.tscopyArchonSkillre-imported from new homecommands/skill.tsbundled-skill.tsBUNDLED_SKILL_FILEScommands/setup.tsbundled-skill.ts./skill)commands/setup.test.tscommands/skill.tscopyArchonSkillimport re-pointedLabel Snapshot
risk: lowsize: Sclicli:commandsChange Metadata
featurecliLinked Issue
(no linked issue — this is a small developer-experience addition)
Validation Evidence (required)
End-to-end smoke tests against the dev shim (
~/.archon/bin/archonexecsbun packages/cli/src/cli.ts):skill.test.tscovers happy path, missing-dir, and overwrite behavior usingmkdtempSync.bun run validate'scheck:bundledflagged the bundle as stale — that staleness comes from a pre-existing untracked.archon/workflows/defaults/archon-feedback.yamlthat was already on disk before this branch and is not part of this PR. With that file moved aside,bun run validateis green.Security Impact (required)
<target>/.claude/skills/archon/under a user-supplied path.copyArchonSkillhelper used byarchon setup. Bundled skill files are static text imported at compile time.Compatibility / Migration
copyArchonSkillexport fromcommands/setup.tsis removed, but it was an internal helper consumed only bysetup.test.ts(re-pointed in this PR).Human Verification (required)
What was personally validated beyond CI:
archon skill installwith no path arg installs into the cwd.archon skill install <path>installs into the given path.archon skill install --cwd <path>works via the global--cwdflag..claude/skills/archon/.skill.test.ts).archon skill(no subcommand) andarchon skill fooexit 1 with helpful messages.archon helpshows the new command and example invocations.archon setupinteractive wizard still installs the skill (helper relocation only).copyArchonSkillcreates intermediate dirs).bun build --compile). Static imports ofBUNDLED_SKILL_FILESare unchanged, so the binary path is expected to behave identically — flagging here for completeness.Side Effects / Blast Radius (required)
@archon/cli. No changes to workflow execution, isolation, server, adapters, web, or providers.setup.tsis thatcopyArchonSkillnow lives in./skill. Tests guard the import path, and the wizard's call site is unchanged.skill.test.tsexercise both the happy and error paths.archon helpnow exposes the command, so users will discover it via the standard help surface.Rollback Plan (required)
git revert <merge-sha>— single isolated commit with no data-model or storage impact.archon skill install ...exiting non-zero, or skill files not appearing under.claude/skills/archon/on a successful run.Risks and Mitigations
archon skill installagainst a project that already has a customised.claude/skills/archon/SKILL.md; the command will overwrite it.copyArchonSkill). The CLI prints the absolute target path before writing, so the destination is visible. Future PR could add--dry-runor--no-overwriteif maintainers want it.skill *) which future contributors must keep consistent (help text, examples, no-git list).workflowandisolationsubcommand patterns; covered by tests; one-line help/example entries land alongside the dispatch.Summary by CodeRabbit
New Features
Documentation
Tests