feat(setup): install agent skills for detected roots#747
Conversation
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛Init
Resolve
Upgrade
Other
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
|
Codecov Results 📊✅ 134 passed | Total: 134 | Pass Rate: 100% | Execution Time: 0ms 📊 Comparison with Base Branch
✨ No test changes detected All tests are passing successfully. ✅ Patch coverage is 100.00%. Project has 1628 uncovered lines. Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 95.28% 95.31% +0.03%
==========================================
Files 234 234 —
Lines 34644 34676 +32
Branches 0 0 —
==========================================
+ Hits 33008 33048 +40
- Misses 1636 1628 -8
- Partials 0 0 —Generated by Codecov Action |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed:
getSkillInstallPathis now unused in production code- Removed the unused getSkillInstallPath function and its test, as production code now constructs paths directly in writeSkillFiles.
- ✅ Fixed: Fresh Claude install notification suppressed by stale agents result
- Modified installAgentSkills to merge the created flags when both targets succeed, ensuring fresh Claude installs are reported even when .agents already existed.
Or push these changes by commenting:
@cursor push 78e6498ad6
Preview (78e6498ad6)
diff --git a/src/lib/agent-skills.ts b/src/lib/agent-skills.ts
--- a/src/lib/agent-skills.ts
+++ b/src/lib/agent-skills.ts
@@ -38,14 +38,6 @@
}
/**
- * Get the installation path for the Sentry CLI skill in the cross-agent
- * standard location (~/.agents/skills/).
- */
-export function getSkillInstallPath(homeDir: string): string {
- return join(homeDir, ".agents", "skills", "sentry-cli", "SKILL.md");
-}
-
-/**
* Write skill files to a target directory.
*
* Checks that the parent directory is writable before attempting writes.
@@ -156,6 +148,14 @@
}
}
- // Prefer the cross-agent standard path as the reported location
+ // Prefer the cross-agent standard path as the reported location.
+ // If both succeeded, prefer agents but merge the `created` flag so
+ // fresh Claude installs are reported even when .agents already existed.
+ if (agentsResult && claudeResult) {
+ return {
+ ...agentsResult,
+ created: agentsResult.created || claudeResult.created,
+ };
+ }
return agentsResult ?? claudeResult;
}
diff --git a/test/lib/agent-skills.test.ts b/test/lib/agent-skills.test.ts
--- a/test/lib/agent-skills.test.ts
+++ b/test/lib/agent-skills.test.ts
@@ -11,7 +11,6 @@
import { join } from "node:path";
import {
detectClaudeCode,
- getSkillInstallPath,
installAgentSkills,
} from "../../src/lib/agent-skills.js";
@@ -41,13 +40,6 @@
});
});
- describe("getSkillInstallPath", () => {
- test("returns cross-agent standard path under ~/.agents/skills", () => {
- const path = getSkillInstallPath("/home/user");
- expect(path).toBe("/home/user/.agents/skills/sentry-cli/SKILL.md");
- });
- });
-
describe("installAgentSkills", () => {
let testDir: string;
@@ -164,6 +156,34 @@
expect(second!.path).toBe(first!.path);
});
+ test("reports created: true when Claude is freshly installed but .agents already exists", async () => {
+ // First run without Claude
+ const first = await installAgentSkills(testDir);
+ expect(first!.created).toBe(true);
+ expect(first!.path).toBe(
+ join(testDir, ".agents", "skills", "sentry-cli", "SKILL.md")
+ );
+
+ // Install Claude Code
+ mkdirSync(join(testDir, ".claude"), { recursive: true });
+
+ // Second run with Claude
+ const second = await installAgentSkills(testDir);
+ // Should report created: true because Claude skills are fresh
+ expect(second!.created).toBe(true);
+ expect(second!.path).toBe(
+ join(testDir, ".agents", "skills", "sentry-cli", "SKILL.md")
+ );
+
+ // Both locations should exist
+ expect(
+ existsSync(join(testDir, ".agents", "skills", "sentry-cli", "SKILL.md"))
+ ).toBe(true);
+ expect(
+ existsSync(join(testDir, ".claude", "skills", "sentry-cli", "SKILL.md"))
+ ).toBe(true);
+ });
+
test("claude install succeeds even if ~/.agents is not writable", async () => {
mkdirSync(join(testDir, ".claude"), { recursive: true });
mkdirSync(join(testDir, ".agents"), { recursive: true, mode: 0o444 });This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 352019d. Configure here.
Only install shared agent skills when a compatible agent root already exists, so setup stays silent for users without agent tooling and avoids creating ~/.agents unconditionally. When a new target is added later, report the path that was actually created and cover the detected-root behavior with focused setup tests. Made-with: Cursor
Collapse the agent skill follow-up back toward the original Claude-only shape so installAgentSkills owns the control flow again and getSkillInstallPath is the single source of truth for Claude and shared agent paths. Keep the detected-root behavior and fresh-path reporting, but drop the extra wrapper helpers that made the review diff larger than necessary. Made-with: Cursor
5a1cdd5 to
ab573d1
Compare
BYK
left a comment
There was a problem hiding this comment.
Love it!
We may wanna consider making the .claude one a symlink if both exists but that's a very minor and hard-to-get optimization. We can consider if there are more targets in the future (hope not! :D)


Summary
This updates the existing review branch with the smaller change built from
maininstead of the earlier unconditional.agentsinstaller. It keeps the Claude-style flow, adds shared~/.agentssupport only when that root already exists, and keeps setup output aligned with the file that was actually created.Changes
~/.agentsand~/.clauderoots instead of creating a top-level shared root~/.agentsinstalls, and fresh-path reportingTest Plan
bun test test/lib/agent-skills.test.tsbun test test/commands/cli/setup.test.tsbun run typecheckbun run lint(passes with an existing unrelated warning insrc/lib/formatters/markdown.ts)