From 16f2d4d4cf9a7156028e56d8ed1d78b44adcba86 Mon Sep 17 00:00:00 2001 From: Greg Pstrucha <875316+gricha@users.noreply.github.com> Date: Fri, 17 Apr 2026 11:20:45 -0700 Subject: [PATCH 1/2] fix(sync): Prune stale managed skills Use the local lockfile during sync to distinguish truly local orphaned skills from managed skills that were removed from agents.toml. This prevents collaborators from re-adopting stale installs after pulling config changes, and it respects wildcard exclude changes during offline sync. Add regression coverage for the direct prune path, wildcard excludes, and a collaborator-style clone/pull/sync flow so the behavior is exercised closer to end to end. Fixes #86 Co-Authored-By: Codex --- README.md | 2 +- docs/public/llms.txt | 2 +- docs/src/app/cli/page.tsx | 2 +- docs/src/app/guide/page.tsx | 3 +- specs/SPEC.md | 15 ++-- src/cli/commands/sync.test.ts | 147 ++++++++++++++++++++++++++++++++++ src/cli/commands/sync.ts | 45 ++++++++--- src/cli/index.ts | 2 +- 8 files changed, 194 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index d658f43..2b5b537 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,7 @@ npx @sentry/dotagents install | `remove [-y]` | Remove a skill or all skills from a source | | `install` | Install all dependencies from `agents.toml` | | `list` | Show installed skills and their status | -| `sync` | Reconcile gitignore, symlinks, and verify state | +| `sync` | Reconcile state offline: adopt local skills, prune stale managed ones, repair configs | | `mcp` | Manage MCP server declarations | | `trust` | Manage trusted sources | | `doctor` | Check project health and fix issues | diff --git a/docs/public/llms.txt b/docs/public/llms.txt index 51af614..ec3f97b 100644 --- a/docs/public/llms.txt +++ b/docs/public/llms.txt @@ -290,7 +290,7 @@ When the argument is a source specifier (e.g. `owner/repo`, a URL) instead of a npx @sentry/dotagents sync ``` -Reconcile project state without network access: adopt orphaned skills (installed but not declared), regenerate `.agents/.gitignore`, check for missing skills, repair symlinks, verify/repair MCP and hook configs. Reports issues as warnings or errors. +Reconcile project state without network access: adopt truly local orphaned skills, prune stale managed skills removed from config, regenerate `.agents/.gitignore`, check for missing skills, repair symlinks, and verify/repair MCP and hook configs. Reports issues as warnings or errors. ### mcp add diff --git a/docs/src/app/cli/page.tsx b/docs/src/app/cli/page.tsx index e729486..08381ff 100644 --- a/docs/src/app/cli/page.tsx +++ b/docs/src/app/cli/page.tsx @@ -186,7 +186,7 @@ export default function CliPage() {
    -
  • Adopts orphaned skills (installed but not declared)
    • +
  • Adopts truly local orphaned skills (installed but not declared)
    • +
  • Prunes stale managed skills removed from config
  • Regenerates .agents/.gitignore
    • diff --git a/specs/SPEC.md b/specs/SPEC.md index 5126c98..01b66d4 100644 --- a/specs/SPEC.md +++ b/specs/SPEC.md @@ -459,13 +459,14 @@ dotagents sync ``` **Behavior:** -1. Adopt orphaned skills (installed but not in `agents.toml`) into config -2. Regenerate `.agents/.gitignore` -3. Warn if `agents.lock` and `.agents/.gitignore` are not in the root `.gitignore` -4. Check for missing skills (in `agents.toml` but not installed) -5. Create/verify/repair symlinks -6. Verify and repair MCP config files for declared agents -7. Verify and repair hook config files for declared agents +1. Adopt orphaned local skills (installed but not in `agents.toml`, and not previously managed) into config +2. Prune stale managed skills that were removed from config but still exist on disk locally +3. Regenerate `.agents/.gitignore` +4. Warn if `agents.lock` and `.agents/.gitignore` are not in the root `.gitignore` +5. Check for missing skills (in `agents.toml` but not installed) +6. Create/verify/repair symlinks +7. Verify and repair MCP config files for declared agents +8. Verify and repair hook config files for declared agents ### `dotagents mcp` diff --git a/src/cli/commands/sync.test.ts b/src/cli/commands/sync.test.ts index 77ab9b8..2cb74c8 100644 --- a/src/cli/commands/sync.test.ts +++ b/src/cli/commands/sync.test.ts @@ -4,10 +4,13 @@ import { existsSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; import { runSync } from "./sync.js"; +import { runInstall } from "./install.js"; +import { runRemove } from "./remove.js"; import { writeLockfile } from "../../lockfile/writer.js"; import { loadLockfile } from "../../lockfile/loader.js"; import { loadConfig } from "../../config/loader.js"; import { resolveScope } from "../../scope.js"; +import { exec } from "../../utils/exec.js"; const SKILL_MD = (name: string) => `--- name: ${name} @@ -91,6 +94,150 @@ describe("runSync", () => { expect(result.issues).toHaveLength(0); }); + it("prunes stale managed skills removed from config instead of re-adopting them", async () => { + await writeFile( + join(projectRoot, "agents.toml"), + "version = 1\n", + ); + const managedDir = join(projectRoot, ".agents", "skills", "pdf"); + await mkdir(managedDir, { recursive: true }); + await writeFile(join(managedDir, "SKILL.md"), SKILL_MD("pdf")); + await writeLockfile(join(projectRoot, "agents.lock"), { + version: 1, + skills: { + pdf: { + source: "org/repo", + resolved_url: "https://github.com/org/repo.git", + resolved_path: "pdf", + }, + }, + }); + + const result = await runSync({ scope: resolveScope("project", projectRoot) }); + + expect(result.adopted).toHaveLength(0); + expect(result.pruned).toEqual(["pdf"]); + expect(existsSync(managedDir)).toBe(false); + + const config = await loadConfig(join(projectRoot, "agents.toml")); + expect(config.skills).toHaveLength(0); + + const lockfile = await loadLockfile(join(projectRoot, "agents.lock")); + expect(lockfile).not.toBeNull(); + expect(lockfile!.skills["pdf"]).toBeUndefined(); + }); + + it("prunes wildcard skills newly excluded from config", async () => { + await writeFile( + join(projectRoot, "agents.toml"), + `version = 1\n\n[[skills]]\nname = "*"\nsource = "org/repo"\nexclude = ["review"]\n`, + ); + const reviewDir = join(projectRoot, ".agents", "skills", "review"); + await mkdir(reviewDir, { recursive: true }); + await writeFile(join(reviewDir, "SKILL.md"), SKILL_MD("review")); + await writeLockfile(join(projectRoot, "agents.lock"), { + version: 1, + skills: { + review: { + source: "org/repo", + resolved_url: "https://github.com/org/repo.git", + resolved_path: "skills/review", + }, + }, + }); + + const result = await runSync({ scope: resolveScope("project", projectRoot) }); + + expect(result.adopted).toHaveLength(0); + expect(result.pruned).toEqual(["review"]); + expect(existsSync(reviewDir)).toBe(false); + }); + + it("prunes stale managed skills after a collaborator removes the dependency and another collaborator pulls", async () => { + const skillRepo = join(tmpDir, "skill-repo"); + const projectOrigin = join(tmpDir, "project-origin.git"); + const projectSeed = join(tmpDir, "project-seed"); + const aliceRepo = join(tmpDir, "alice"); + const bobRepo = join(tmpDir, "bob"); + const aliceStateDir = join(tmpDir, "alice-state"); + const bobStateDir = join(tmpDir, "bob-state"); + + const previousStateDir = process.env["DOTAGENTS_STATE_DIR"]; + + await mkdir(skillRepo, { recursive: true }); + await exec("git", ["init"], { cwd: skillRepo }); + await exec("git", ["config", "user.email", "test@example.com"], { cwd: skillRepo }); + await exec("git", ["config", "user.name", "Test User"], { cwd: skillRepo }); + await mkdir(join(skillRepo, "pdf"), { recursive: true }); + await writeFile(join(skillRepo, "pdf", "SKILL.md"), SKILL_MD("pdf")); + await exec("git", ["add", "."], { cwd: skillRepo }); + await exec("git", ["commit", "-m", "initial skill"], { cwd: skillRepo }); + + await exec("git", ["init", "--bare", projectOrigin], { cwd: tmpDir }); + await mkdir(projectSeed, { recursive: true }); + await exec("git", ["init", "-b", "main"], { cwd: projectSeed }); + await exec("git", ["config", "user.email", "test@example.com"], { cwd: projectSeed }); + await exec("git", ["config", "user.name", "Test User"], { cwd: projectSeed }); + await writeFile( + join(projectSeed, "agents.toml"), + `version = 1\n\n[[skills]]\nname = "pdf"\nsource = "git:${skillRepo}"\n`, + ); + await exec("git", ["add", "agents.toml"], { cwd: projectSeed }); + await exec("git", ["commit", "-m", "initial project config"], { cwd: projectSeed }); + await exec("git", ["remote", "add", "origin", projectOrigin], { cwd: projectSeed }); + await exec("git", ["push", "-u", "origin", "main"], { cwd: projectSeed }); + + await exec("git", ["clone", projectOrigin, aliceRepo], { cwd: tmpDir }); + await exec("git", ["clone", projectOrigin, bobRepo], { cwd: tmpDir }); + await exec("git", ["config", "user.email", "alice@example.com"], { cwd: aliceRepo }); + await exec("git", ["config", "user.name", "Alice"], { cwd: aliceRepo }); + await exec("git", ["config", "user.email", "bob@example.com"], { cwd: bobRepo }); + await exec("git", ["config", "user.name", "Bob"], { cwd: bobRepo }); + + try { + process.env["DOTAGENTS_STATE_DIR"] = aliceStateDir; + await runInstall({ scope: resolveScope("project", aliceRepo) }); + + process.env["DOTAGENTS_STATE_DIR"] = bobStateDir; + await runInstall({ scope: resolveScope("project", bobRepo) }); + + const bobSkillDir = join(bobRepo, ".agents", "skills", "pdf"); + expect(existsSync(bobSkillDir)).toBe(true); + + process.env["DOTAGENTS_STATE_DIR"] = aliceStateDir; + await runRemove({ scope: resolveScope("project", aliceRepo), skillName: "pdf" }); + await exec("git", ["add", "agents.toml"], { cwd: aliceRepo }); + await exec("git", ["commit", "-m", "remove pdf"], { cwd: aliceRepo }); + await exec("git", ["push", "origin", "main"], { cwd: aliceRepo }); + + await exec("git", ["pull", "--ff-only", "origin", "main"], { cwd: bobRepo }); + + const bobConfig = await loadConfig(join(bobRepo, "agents.toml")); + expect(bobConfig.skills).toHaveLength(0); + expect(existsSync(bobSkillDir)).toBe(true); + + const bobLockBeforeSync = await loadLockfile(join(bobRepo, "agents.lock")); + expect(bobLockBeforeSync!.skills["pdf"]).toBeDefined(); + + process.env["DOTAGENTS_STATE_DIR"] = bobStateDir; + const result = await runSync({ scope: resolveScope("project", bobRepo) }); + + expect(result.adopted).toHaveLength(0); + expect(result.pruned).toEqual(["pdf"]); + expect(existsSync(bobSkillDir)).toBe(false); + + const bobLockAfterSync = await loadLockfile(join(bobRepo, "agents.lock")); + expect(bobLockAfterSync).not.toBeNull(); + expect(bobLockAfterSync!.skills["pdf"]).toBeUndefined(); + } finally { + if (previousStateDir === undefined) { + delete process.env["DOTAGENTS_STATE_DIR"]; + } else { + process.env["DOTAGENTS_STATE_DIR"] = previousStateDir; + } + } + }); + it("detects missing skills", async () => { await writeFile( join(projectRoot, "agents.toml"), diff --git a/src/cli/commands/sync.ts b/src/cli/commands/sync.ts index 6e052bf..6643bb1 100644 --- a/src/cli/commands/sync.ts +++ b/src/cli/commands/sync.ts @@ -1,6 +1,6 @@ import { join, resolve } from "node:path"; import { existsSync } from "node:fs"; -import { readdir } from "node:fs/promises"; +import { readdir, rm } from "node:fs/promises"; import chalk from "chalk"; import { loadConfig } from "../../config/loader.js"; import { isWildcardDep } from "../../config/schema.js"; @@ -35,6 +35,7 @@ export interface SyncOptions { export interface SyncResult { issues: SyncIssue[]; adopted: string[]; + pruned: string[]; gitignoreUpdated: boolean; symlinksRepaired: number; mcpRepaired: number; @@ -46,26 +47,26 @@ export async function runSync(opts: SyncOptions): Promise { const { configPath, lockPath, agentsDir, skillsDir } = scope; let config = await loadConfig(configPath); - const lockfile = await loadLockfile(lockPath); + let lockfile = await loadLockfile(lockPath); // Build declared names from explicit entries + wildcard-expanded lockfile entries const declaredNames = new Set( config.skills.filter((s) => !isWildcardDep(s)).map((s) => s.name), ); if (lockfile) { - // Add concrete skill names from wildcard sources - const wildcardSources = new Set( - config.skills - .filter(isWildcardDep) - .map((s) => normalizeSource(s.source)), - ); for (const [name, locked] of Object.entries(lockfile.skills)) { - if (wildcardSources.has(normalizeSource(locked.source))) { + const wildcardDep = config.skills.find( + (s): s is Extract => + isWildcardDep(s) && + normalizeSource(s.source) === normalizeSource(locked.source), + ); + if (wildcardDep && !wildcardDep.exclude.includes(name)) { declaredNames.add(name); } } } const issues: SyncIssue[] = []; const adopted: string[] = []; + const pruned: string[] = []; // 1. Adopt orphaned skills (installed but not in agents.toml) if (existsSync(skillsDir)) { @@ -75,6 +76,14 @@ export async function runSync(opts: SyncOptions): Promise { if (!entry.isDirectory()) {continue;} if (declaredNames.has(entry.name)) {continue;} + const locked = lockfile?.skills[entry.name]; + if (locked && !isInPlaceSkill(locked.source)) { + await removeStaleManagedSkill(skillsDir, entry.name); + delete lockfile!.skills[entry.name]; + pruned.push(entry.name); + continue; + } + const sourcePrefix = scope.scope === "user" ? "path:skills/" : "path:.agents/skills/"; const source = `${sourcePrefix}${entry.name}`; await addSkillToConfig(configPath, entry.name, { source }); @@ -84,11 +93,14 @@ export async function runSync(opts: SyncOptions): Promise { adopted.push(entry.name); } - if (adopted.length > 0) { - await writeLockfile(lockPath, { + if (adopted.length > 0 || pruned.length > 0) { + lockfile = { version: 1, skills: { ...lockfile?.skills, ...adoptedLockEntries }, - }); + }; + await writeLockfile(lockPath, lockfile); + } + if (adopted.length > 0) { config = await loadConfig(configPath); } } @@ -218,6 +230,7 @@ export async function runSync(opts: SyncOptions): Promise { return { issues, adopted, + pruned, gitignoreUpdated, symlinksRepaired, mcpRepaired, @@ -225,6 +238,10 @@ export async function runSync(opts: SyncOptions): Promise { }; } +async function removeStaleManagedSkill(skillsDir: string, name: string): Promise { + await rm(join(skillsDir, name), { recursive: true, force: true }); +} + export default async function sync(_args: string[], flags?: { user?: boolean }): Promise { let scope: ScopeRoot; try { @@ -244,6 +261,10 @@ export default async function sync(_args: string[], flags?: { user?: boolean }): console.log(chalk.green(`Adopted ${result.adopted.length} orphan(s): ${result.adopted.join(", ")}`)); } + if (result.pruned.length > 0) { + console.log(chalk.green(`Pruned ${result.pruned.length} stale managed skill(s): ${result.pruned.join(", ")}`)); + } + if (scope.scope === "project" && result.gitignoreUpdated) { console.log(chalk.green("Regenerated .agents/.gitignore")); } diff --git a/src/cli/index.ts b/src/cli/index.ts index e1da29c..84cb7ee 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -31,7 +31,7 @@ Commands: install Install dependencies from agents.toml add Add a skill dependency remove Remove a skill or all skills from a source - sync Reconcile gitignore, symlinks, verify state + sync Reconcile state offline and repair generated config list Show installed skills mcp Manage MCP server declarations trust Manage trusted sources From 5ea494d882c135d5467142856ed952bf62a886c3 Mon Sep 17 00:00:00 2001 From: Greg Pstrucha <875316+gricha@users.noreply.github.com> Date: Fri, 17 Apr 2026 11:33:35 -0700 Subject: [PATCH 2/2] test(sync): Clone temp repos from main Make the collaborator sync regression test clone the temp project remote with an explicit main branch. This avoids CI-only failures caused by relying on the bare remote HEAD when creating Alice and Bob clones. Refs #86 Co-Authored-By: Codex --- src/cli/commands/sync.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cli/commands/sync.test.ts b/src/cli/commands/sync.test.ts index 2cb74c8..4b05d58 100644 --- a/src/cli/commands/sync.test.ts +++ b/src/cli/commands/sync.test.ts @@ -187,8 +187,8 @@ describe("runSync", () => { await exec("git", ["remote", "add", "origin", projectOrigin], { cwd: projectSeed }); await exec("git", ["push", "-u", "origin", "main"], { cwd: projectSeed }); - await exec("git", ["clone", projectOrigin, aliceRepo], { cwd: tmpDir }); - await exec("git", ["clone", projectOrigin, bobRepo], { cwd: tmpDir }); + await exec("git", ["clone", "--branch", "main", projectOrigin, aliceRepo], { cwd: tmpDir }); + await exec("git", ["clone", "--branch", "main", projectOrigin, bobRepo], { cwd: tmpDir }); await exec("git", ["config", "user.email", "alice@example.com"], { cwd: aliceRepo }); await exec("git", ["config", "user.name", "Alice"], { cwd: aliceRepo }); await exec("git", ["config", "user.email", "bob@example.com"], { cwd: bobRepo });