-
-
Notifications
You must be signed in to change notification settings - Fork 77
feat(agent-skills): Add --skills command to install Sentry agent skills #1187
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.
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
base: master
Are you sure you want to change the base?
Conversation
This adds a new wizard feature that installs Sentry agent skills for various AI coding assistants: - Claude Code (.claude/skills/) - OpenAI Codex (.codex/skills/) - OpenCode (.opencode/skill/) - Cursor (.cursor/skills/) - GitHub Copilot (.github/skills/) - Factory Droid (.factory/skills/) Features: - Interactive mode with scope and editor selection prompts - Headless mode: --skills claude-code opencode - Project-scoped (default) or user-scoped installation: --scope user - Smart update logic: updates Sentry skills, preserves user-created skills - Downloads skills from getsentry/sentry-agent-skills GitHub releases Usage: npx @sentry/wizard --skills # Interactive npx @sentry/wizard --skills claude-code cursor # Headless npx @sentry/wizard --skills claude-code --scope user # User profile
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Features
🤖 This preview updates automatically when you update the PR. |
Lms24
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I think the new flow is a good starting point. We could even potentially call it from SDK setup flows (if we want to) but this makes sense standalone as well (as discussed on Slack)
A general question: IIUC for now, this downloads the latest skill version at the time. Is there some way users can easily update skills without having to periodically rerun the wizard? This is more a general "skill ecosystem" question. My only concern is these skills becoming outdated and users not being aware that there are improvements available.
| chalk.red( | ||
| 'Failed to download Sentry agent skills from GitHub.\n' + | ||
| 'Please check your internet connection and try again.\n\n' + | ||
| 'You can also install skills manually from:\n' + | ||
| chalk.cyan('https://github.com/getsentry/sentry-agent-skills'), | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: We usually don't color entire error messages. clack.log.error already marks the output visually as errored. Feel free to disregard though, no strong feelings :) IF you change it, let's also use template strings over concatinating line-by-line. Makes it a bit more readable IMHO.
| const tempDir = path.join(os.tmpdir(), `sentry-skills-${Date.now()}`); | ||
| await fs.promises.mkdir(tempDir, { recursive: true }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l: let's use
| const tempDir = path.join(os.tmpdir(), `sentry-skills-${Date.now()}`); | |
| await fs.promises.mkdir(tempDir, { recursive: true }); | |
| const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'sentry-skills-')); |
instead for a truly random temp dir (fs.mkdtemp will automatically append a random sequence to the last - specified in the name).
| } catch { | ||
| // Network error on HEAD, try archive | ||
| tarballUrl = SKILLS_REPO_ARCHIVE_URL; | ||
| isArchive = true; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
q: if we land here, something went wrong, correct? Should we emit a log/warning?
| const skillsExist = await fs.promises | ||
| .access(extractPath) | ||
| .then(() => true) | ||
| .catch(() => false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l: can we use fs.existsSync(extractPath) here instead?
| const skillsExist = await fs.promises | |
| .access(extractPath) | |
| .then(() => true) | |
| .catch(() => false); | |
| const skillsExist = fs.existsSync(extractPath); |
No strong feelings but I think fs.access is nice for high throughput envs but in case of the wizard the sync operation is totally fine
| const targetExists = await fs.promises | ||
| .access(targetPath) | ||
| .then(() => true) | ||
| .catch(() => false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same q about existsSync here
|
One more thing: Let's make sure to add an e2e test for this flow, to have it covered. Thanks! |
Summary
This PR adds a new wizard feature that installs Sentry agent skills for various AI coding assistants.
Supported Editors
.claude/skills/).codex/skills/).opencode/skill/).cursor/skills/).github/skills/).factory/skills/)Features
--skills claude-code opencode--scope userUsage
Changes
bin.ts- Added--skillsand--scopeCLI argumentssrc/agent-skills/agent-skills-wizard.ts- Main wizard implementationsrc/agent-skills/editor-configs.ts- Editor configuration and path helperssrc/run.ts- Route to agent skills wizard when--skillsflag is presentTesting