diff --git a/.changeset/new-seals-go.md b/.changeset/new-seals-go.md new file mode 100644 index 00000000..b2084ee7 --- /dev/null +++ b/.changeset/new-seals-go.md @@ -0,0 +1,5 @@ +--- +"@bunny.net/cli": patch +--- + +fix(skills): always write .agents/skills, refuse home installs diff --git a/AGENTS.md b/AGENTS.md index 5e0f772c..e3ebe1a3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -174,7 +174,7 @@ bunny-cli/ │ ├── cli.ts # Root yargs instance, global flags, command registration │ │ │ ├── core/ -│ │ ├── agent-skill.ts # Generic project skill installer/remover: marked AGENTS.md block upsert/remove + skill file writes (Claude-gated for projects; ~/.agents/skills + ~/.claude/skills for --global); project writes refuse symlink escapes; SKILL.md is a completion sentinel (removed first, written last per root) and the installed check requires every global root, so partial installs and failed refreshes re-offer +│ │ ├── agent-skill.ts # Generic project skill installer/remover: marked AGENTS.md block upsert/remove + skill file writes (projects always write .agents/skills, adding .claude/skills when detected; ~/.agents/skills + ~/.claude/skills for --global); installing into the home directory is refused; project writes refuse symlink escapes; SKILL.md is a completion sentinel (boundary-checked, then removed first and written last per root) and the installed check requires every global root, so partial installs and failed refreshes re-offer │ │ ├── agent-skill.test.ts # Tests for install/upsert idempotency, marker scoping, Claude gating │ │ ├── client-options.ts # clientOptions() helper — builds ClientOptions from ResolvedConfig │ │ ├── define-command.ts # Command factory (see "Command Pattern" below) @@ -427,7 +427,7 @@ bunny-cli/ │ │ │ ├── index.ts # defineNamespace("skills", ...) registers skills commands │ │ │ ├── content.ts # BUNNY_CLI_SKILL: embeds skills/bunny-cli/** at bundle time via Bun text imports (single source of truth) + compact AGENTS.md section │ │ │ ├── content.test.ts # Guards: every reference SKILL.md routes to is embedded; section stays compact -│ │ │ ├── install.ts # bunny skills install [--global]: project (AGENTS.md + Claude-gated .claude/skills) or global (~/.agents/skills + ~/.claude/skills) +│ │ │ ├── install.ts # bunny skills install [--global]: project (AGENTS.md + .agents/skills, plus .claude/skills when detected) or global (~/.agents/skills + ~/.claude/skills) │ │ │ ├── offer.ts # One-time global-install nudge: interactive offer after bunny login + passive post-command stderr hint for users who never log in (shared marker in the XDG cache dir) │ │ │ └── remove.ts # bunny skills remove [--global] [--force]: strips the AGENTS.md block and deletes the skill dirs for either scope │ │ └── scripts/ @@ -1167,7 +1167,7 @@ bunny │ ├── upgrade-router [site] [--link] Republish the site's router script with the CLI's current source │ └── delete [site] [--force] [--keep-storage] Delete pull zone → router → storage zone (typed-name confirmation, so unattended runs need --force; best-effort so re-runs finish a partial delete) ├── skills -│ ├── install (aliases: add, update) [--global] Install the bunny agent skill: marked AGENTS.md block + .claude/skills/bunny-cli/ when the project uses Claude Code; --global writes ~/.agents/skills/bunny-cli/ and ~/.claude/skills/bunny-cli/ for every project +│ ├── install (aliases: add, update) [--global] Install the bunny agent skill: marked AGENTS.md block + .agents/skills/bunny-cli/, plus .claude/skills/bunny-cli/ when the project uses Claude Code; --global writes ~/.agents/skills/bunny-cli/ and ~/.claude/skills/bunny-cli/ for every project │ └── remove (aliases: rm, uninstall) [--global] [--force] Remove the skill: strips the AGENTS.md block (deleting the file when only the installer's scaffold heading remains) and deletes the skill dirs; confirmed unless --force ├── docs Open bunny.net documentation in browser ├── open [--print] Open bunny.net dashboard in browser (or print URL) @@ -1367,7 +1367,7 @@ handler: async ({ output, profile, apiKey }) => { So coding agents discover the CLI at all, `bunny skills install` writes the shipped `skills/bunny-cli/` skill into the user's environment. The generic machinery lives in `packages/cli/src/core/agent-skill.ts` so future per-resource skills can reuse it: -- **Project install (default)**: upserts a marked block (``) into the project's `AGENTS.md` (created if missing, replaced in place on reinstall; markers are per-skill so multiple blocks coexist; a malformed block, meaning a missing, reversed, or duplicated marker, errors instead of guessing). When the project uses Claude Code (`.claude/` or `CLAUDE.md` exists) it also writes the full skill with all references to `.claude/skills/bunny-cli/`. Writes that a symlink would redirect outside the project are refused, so a checkout can't plant links that make the installer overwrite unrelated files (symlinks resolving inside the project, e.g. `AGENTS.md -> CLAUDE.md`, are followed). +- **Project install (default)**: upserts a marked block (``) into the project's `AGENTS.md` (created if missing, replaced in place on reinstall; markers are per-skill so multiple blocks coexist; a malformed block, meaning a missing, reversed, or duplicated marker, errors instead of guessing). The full skill with all references is always written to `.agents/skills/bunny-cli/`, and additionally to `.claude/skills/bunny-cli/` when the project uses Claude Code (`.claude/` or `CLAUDE.md` exists). Installing into the home directory is refused: per-user config dirs like `~/.claude` would otherwise read as project markers, and files there would apply to every directory you work in. Removal is still allowed there, so a stray `AGENTS.md` from an earlier version can be cleaned up, but it strips only that block: the skill directories under home are the global install, and only `--global` deletes those. Writes that a symlink would redirect outside the project are refused, so a checkout can't plant links that make the installer overwrite unrelated files (symlinks resolving inside the project, e.g. `AGENTS.md -> CLAUDE.md`, are followed). - **Global install (`--global`)**: writes the skill to `~/.agents/skills/bunny-cli/` (the cross-tool Agent Skills directory read by Cursor, Codex, OpenCode, Copilot, and others) and `~/.claude/skills/bunny-cli/` so AI coding tools pick it up in every project; nothing project-local is touched. - **Removal**: `bunny skills remove [--global]` undoes either scope; it strips the marked block (deleting AGENTS.md only when the installer's own scaffold heading is all that remains) and deletes the skill directories. Everything removed is regenerable with `bunny skills install`, and `update` is an install alias since reinstalling refreshes in place. - **Single source of truth**: `packages/cli/src/commands/skills/content.ts` embeds `skills/bunny-cli/**` at bundle time via Bun text imports (`with { type: "text" }`), so the installed skill is always the shipped one; only the compact AGENTS.md section is authored separately. `content.test.ts` fails if SKILL.md routes to a reference that isn't embedded. diff --git a/README.md b/README.md index 72e97674..d308fa1b 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,7 @@ bun ny # Examples bun ny login # offers to install the agent skill after authenticating; --install-skill/--no-install-skill decides without prompting bun ny db list -bun ny skills install # install the bunny agent skill into this project (AGENTS.md block + .claude/skills when Claude Code is used) so AI coding tools know how to use the CLI; alias: skills update +bun ny skills install # install the bunny agent skill into this project (AGENTS.md block + .agents/skills, plus .claude/skills when Claude Code is used) so AI coding tools know how to use the CLI; alias: skills update bun ny skills install --global # install to ~/.agents/skills and ~/.claude/skills for every project bun ny skills remove # remove the skill from this project (or --global); everything is regenerable with skills install bun ny apps deploy ghcr.io/me/api:v1.2 # deploy a pre-built image diff --git a/packages/cli/src/commands/skills/install.ts b/packages/cli/src/commands/skills/install.ts index e9ad968e..a9f776ed 100644 --- a/packages/cli/src/commands/skills/install.ts +++ b/packages/cli/src/commands/skills/install.ts @@ -20,10 +20,12 @@ interface InstallArgs { * * Project install (default) maintains a marked block in AGENTS.md, which most * coding agents read, and writes the full skill with references under - * .claude/skills/bunny-cli/ when the project uses Claude Code. A global install - * writes the skill to ~/.agents/skills/bunny-cli/ (the cross-tool directory) - * and ~/.claude/skills/bunny-cli/ so AI coding tools pick it up in every - * project. Reinstalling refreshes the same files, so `update` is an alias. + * .agents/skills/bunny-cli/, plus .claude/skills/bunny-cli/ when the project + * uses Claude Code. Installing into the home directory is refused, since files + * there are not project scoped. A global install writes the skill to + * ~/.agents/skills/bunny-cli/ (the cross-tool directory) and + * ~/.claude/skills/bunny-cli/ so AI coding tools pick it up in every project. + * Reinstalling refreshes the same files, so `update` is an alias. * * @example * ```bash @@ -38,7 +40,7 @@ export const skillsInstallCommand = defineCommand({ examples: [ [ "$0 skills install", - "Install into this project (AGENTS.md and .claude/skills)", + "Install into this project (AGENTS.md and .agents/skills)", ], [ "$0 skills install --global", diff --git a/packages/cli/src/commands/skills/remove.ts b/packages/cli/src/commands/skills/remove.ts index 057cac2d..f2a1ab3b 100644 --- a/packages/cli/src/commands/skills/remove.ts +++ b/packages/cli/src/commands/skills/remove.ts @@ -21,9 +21,9 @@ interface RemoveArgs { * Remove the bunny-cli agent skill. * * Project removal (default) strips the marked block from AGENTS.md and deletes - * .claude/skills/bunny-cli/. A global removal deletes the skill from - * ~/.agents/skills/ and ~/.claude/skills/. Everything removed is regenerable - * with `bunny skills install`. + * .agents/skills/bunny-cli/ and .claude/skills/bunny-cli/. A global removal + * deletes the skill from ~/.agents/skills/ and ~/.claude/skills/. Everything + * removed is regenerable with `bunny skills install`. * * @example * ```bash @@ -38,7 +38,7 @@ export const skillsRemoveCommand = defineCommand({ examples: [ [ "$0 skills remove", - "Remove from this project (AGENTS.md and .claude/skills)", + "Remove from this project (AGENTS.md and .agents/skills)", ], [ "$0 skills remove --global", diff --git a/packages/cli/src/core/agent-skill.test.ts b/packages/cli/src/core/agent-skill.test.ts index cafb1f28..5eacb7ae 100644 --- a/packages/cli/src/core/agent-skill.test.ts +++ b/packages/cli/src/core/agent-skill.test.ts @@ -3,6 +3,7 @@ import { existsSync, mkdirSync, mkdtempSync, + readdirSync, readFileSync, realpathSync, rmSync, @@ -85,7 +86,7 @@ describe("upsertMarkedBlock", () => { describe("installProjectSkill", () => { test("creates AGENTS.md with a marked block when missing", () => { const files = installProjectSkill(cwd, SKILL); - expect(files).toEqual([AGENTS_FILE]); + expect(files[0]).toBe(AGENTS_FILE); const content = readFileSync(join(cwd, AGENTS_FILE), "utf8"); expect(content).toContain(agentsMarkers("bunny-test").start); expect(content).toContain("Use `bunny test` for testing."); @@ -123,24 +124,33 @@ describe("installProjectSkill", () => { expect(content).toContain(agentsMarkers("bunny-other").start); }); - test("skips .claude/skills when the project does not use Claude", () => { - installProjectSkill(cwd, SKILL); - expect(existsSync(join(cwd, ".claude"))).toBe(false); + test("writes the full skill to .agents/skills", () => { + expect(installProjectSkill(cwd, SKILL)).toEqual([ + AGENTS_FILE, + ".agents/skills/bunny-test/references/extra.md", + ".agents/skills/bunny-test/SKILL.md", + ]); + expect( + readFileSync(join(cwd, ".agents/skills/bunny-test/SKILL.md"), "utf8"), + ).toContain("name: bunny-test"); }); - test("writes all skill files when .claude/ exists", () => { + test("adds .claude/skills on top when .claude/ exists", () => { mkdirSync(join(cwd, ".claude")); - const files = installProjectSkill(cwd, SKILL); - expect(files).toEqual([ + expect(installProjectSkill(cwd, SKILL)).toEqual([ AGENTS_FILE, + ".agents/skills/bunny-test/references/extra.md", + ".agents/skills/bunny-test/SKILL.md", ".claude/skills/bunny-test/references/extra.md", ".claude/skills/bunny-test/SKILL.md", ]); - const skill = readFileSync( - join(cwd, ".claude/skills/bunny-test/SKILL.md"), - "utf8", + }); + + test("refuses to install into the home directory", () => { + expect(() => installProjectSkill(cwd, SKILL, cwd)).toThrow( + "Refusing to install into your home directory", ); - expect(skill).toContain("name: bunny-test"); + expect(readdirSync(cwd)).toEqual([]); }); test("writes skill files when CLAUDE.md exists", () => { @@ -172,15 +182,22 @@ describe("installProjectSkill", () => { ); }); - test("refuses skill writes when .claude/skills escapes via a symlink", () => { + // Planting a SKILL.md outside proves the boundary check runs before the sentinel delete, not just before the writes. + test.each([ + ".agents", + ".claude", + ])("refuses skill writes when %s/skills escapes via a symlink", (parent) => { const outside = mkdtempSync(join(tmpdir(), "bunny-agent-skill-escape-")); + const planted = join(outside, "bunny-test/SKILL.md"); try { - mkdirSync(join(cwd, ".claude")); - symlinkSync(outside, join(cwd, ".claude/skills")); + mkdirSync(join(outside, "bunny-test")); + writeFileSync(planted, "precious\n"); + mkdirSync(join(cwd, parent)); + symlinkSync(outside, join(cwd, parent, "skills")); expect(() => installProjectSkill(cwd, SKILL)).toThrow( "resolves outside the project", ); - expect(existsSync(join(outside, "bunny-test/SKILL.md"))).toBe(false); + expect(readFileSync(planted, "utf8")).toBe("precious\n"); } finally { rmSync(outside, { recursive: true, force: true }); } @@ -248,7 +265,11 @@ describe("removeProjectSkill", () => { mkdirSync(join(cwd, ".claude")); installProjectSkill(cwd, SKILL); const removed = removeProjectSkill(cwd, "bunny-test"); - expect(removed).toEqual([AGENTS_FILE, ".claude/skills/bunny-test"]); + expect(removed).toEqual([ + AGENTS_FILE, + ".agents/skills/bunny-test", + ".claude/skills/bunny-test", + ]); const content = readFileSync(join(cwd, AGENTS_FILE), "utf8"); expect(content).toContain("Use tabs."); expect(content).not.toContain(agentsMarkers("bunny-test").start); @@ -273,6 +294,16 @@ describe("removeProjectSkill", () => { expect(content).not.toContain(agentsMarkers("bunny-test").start); expect(content).toContain(agentsMarkers("bunny-other").start); }); + test("in the home directory strips AGENTS.md but leaves the global skill dirs", () => { + installGlobalSkill(SKILL, cwd); + writeFileSync( + join(cwd, AGENTS_FILE), + upsertMarkedBlock(null, "bunny-test", SKILL.agentsSection), + ); + const removed = removeProjectSkill(cwd, "bunny-test", cwd); + expect(removed).toEqual([AGENTS_FILE]); + expect(isGlobalSkillInstalled("bunny-test", cwd)).toBe(true); + }); }); describe("removeGlobalSkill", () => { diff --git a/packages/cli/src/core/agent-skill.ts b/packages/cli/src/core/agent-skill.ts index 5cb42879..1983154d 100644 --- a/packages/cli/src/core/agent-skill.ts +++ b/packages/cli/src/core/agent-skill.ts @@ -33,11 +33,38 @@ export function isProjectSkillInstalled(cwd: string, name: string): boolean { return readFileSync(path, "utf8").includes(agentsMarkers(name).start); } -/** True when the project shows Claude Code usage, gating .claude/skills writes. */ +/** True when the project shows Claude Code usage, adding .claude/skills to the project install. */ export function usesClaude(cwd: string): boolean { return existsSync(join(cwd, ".claude")) || existsSync(join(cwd, "CLAUDE.md")); } +/** Cross-tool skills directory, written by every project install so the skill lands whatever the agent. */ +const UNIVERSAL_SKILL_DIR = ".agents/skills"; + +/** Skill roots a project install writes, relative to cwd: the cross-tool directory always, Claude Code's when detected. */ +function projectSkillRoots(cwd: string, name: string): string[] { + const roots = [`${UNIVERSAL_SKILL_DIR}/${name}`]; + if (usesClaude(cwd)) roots.push(`.claude/skills/${name}`); + return roots; +} + +/** True when two paths resolve to the same directory, falling back to a literal match when either cannot be resolved. */ +function isSamePath(a: string, b: string): boolean { + try { + return realpathSync(a) === realpathSync(b); + } catch { + return a === b; + } +} + +/** Throw when cwd is the user's home directory, where per-user config dirs like ~/.claude would read as project markers. */ +function assertNotHome(cwd: string, home: string): void { + if (!isSamePath(cwd, home)) return; + throw new UserError( + "Refusing to install into your home directory: it is not a project, and files here apply to every directory you work in. Run `bunny skills install --global` for a machine-wide install, or rerun this from a project directory.", + ); +} + /** Heading written when the installer creates AGENTS.md from scratch. */ const DEFAULT_AGENTS_HEADING = "# Agent instructions"; @@ -139,7 +166,10 @@ function writeSkillFiles( boundary?: string, ): string[] { // SKILL.md is the completion sentinel: removed first and written last, so an interrupted install or refresh reads as incomplete. - rmSync(join(root, "SKILL.md"), { force: true }); + const sentinel = join(root, "SKILL.md"); + // Validated before the delete, so a symlinked root cannot get an external SKILL.md removed on the way to being rejected. + if (boundary) assertWriteWithin(boundary, sentinel, "SKILL.md"); + rmSync(sentinel, { force: true }); const entries = Object.entries(skill.files).sort( ([a], [b]) => Number(a === "SKILL.md") - Number(b === "SKILL.md"), ); @@ -152,16 +182,17 @@ function writeSkillFiles( return entries.map(([relPath]) => relPath); } -/** Install or update a skill in the project (AGENTS.md block always, .claude/skills// when the project uses Claude Code), returning the cwd-relative paths written. */ +/** Install or update a skill in the project (AGENTS.md block, .agents/skills//, plus .claude/skills// when the project uses Claude Code), returning the cwd-relative paths written. */ export function installProjectSkill( cwd: string, skill: ProjectSkill, + home = homedir(), ): string[] { + assertNotHome(cwd, home); const written: string[] = [ upsertAgentsFile(cwd, skill.name, skill.agentsSection), ]; - if (usesClaude(cwd)) { - const skillRoot = `.claude/skills/${skill.name}`; + for (const skillRoot of projectSkillRoots(cwd, skill.name)) { for (const relPath of writeSkillFiles(join(cwd, skillRoot), skill, cwd)) { written.push(`${skillRoot}/${relPath}`); } @@ -169,8 +200,12 @@ export function installProjectSkill( return written; } -/** Undo installProjectSkill: strip the AGENTS.md block (deleting the file when only the scaffold heading remains) and delete .claude/skills//, returning the changed paths. */ -export function removeProjectSkill(cwd: string, name: string): string[] { +/** Undo installProjectSkill: strip the AGENTS.md block (deleting the file when only the scaffold heading remains) and delete both skill directories, returning the changed paths. Unlike install, this runs in the home directory, but there it touches only AGENTS.md, since the skill directories under home are the global install and only `--global` may delete those. */ +export function removeProjectSkill( + cwd: string, + name: string, + home = homedir(), +): string[] { const removed: string[] = []; const path = join(cwd, AGENTS_FILE); if (existsSync(path)) { @@ -185,9 +220,15 @@ export function removeProjectSkill(cwd: string, name: string): string[] { removed.push(AGENTS_FILE); } } - const skillRoot = `.claude/skills/${name}`; - const dir = join(cwd, skillRoot); - if (existsSync(dir)) { + // In home these paths are the global install, not a project one, so a scopeless remove leaves them to `--global`. + if (isSamePath(cwd, home)) return removed; + // Both roots are cleaned whatever this project now detects as, so installs made under older detection rules still come out. + for (const skillRoot of [ + `${UNIVERSAL_SKILL_DIR}/${name}`, + `.claude/skills/${name}`, + ]) { + const dir = join(cwd, skillRoot); + if (!existsSync(dir)) continue; assertWriteWithin(cwd, dir, skillRoot); rmSync(dir, { recursive: true, force: true }); removed.push(skillRoot);