fix(skills): prune skills removed from source on sync

packages/workspace-server/src/services/posthog-plugin/posthog-plugin.test.ts

@@ -90,7 +90,7 @@ import type { IAppMeta } from "@posthog/platform/app-meta";
 import type { IBundledResources } from "@posthog/platform/bundled-resources";
 import type { IStoragePaths } from "@posthog/platform/storage-paths";
 import { PosthogPluginService } from "./posthog-plugin";
-import { syncCodexSkills } from "./update-skills-saga";
+import { overlayDownloadedSkills, syncCodexSkills } from "./update-skills-saga";
 
 /** Expose private members for testing without `as any`. */
 interface TestablePluginService {
@@ -481,6 +481,80 @@ describe("PosthogPluginService", () => {
 
       expect(vol.existsSync(CODEX_SKILLS_DIR)).toBe(false);
     });
+
+    it("prunes skills removed from the plugin but keeps foreign skills", async () => {
+      // First sync: plugin ships two skills.
+      vol.mkdirSync(`${BUNDLED_PLUGIN_DIR}/skills/skill-a`, {
+        recursive: true,
+      });
+      vol.writeFileSync(`${BUNDLED_PLUGIN_DIR}/skills/skill-a/SKILL.md`, "# A");
+      vol.mkdirSync(`${BUNDLED_PLUGIN_DIR}/skills/skill-b`, {
+        recursive: true,
+      });
+      vol.writeFileSync(`${BUNDLED_PLUGIN_DIR}/skills/skill-b/SKILL.md`, "# B");
+
+      // A skill placed in the Codex dir by another tool — must be preserved.
+      vol.mkdirSync(`${CODEX_SKILLS_DIR}/foreign-skill`, { recursive: true });
+      vol.writeFileSync(
+        `${CODEX_SKILLS_DIR}/foreign-skill/SKILL.md`,
+        "# Foreign",
+      );
+
+      await syncCodexSkills(BUNDLED_PLUGIN_DIR, CODEX_SKILLS_DIR);
+
+      expect(vol.existsSync(`${CODEX_SKILLS_DIR}/skill-a/SKILL.md`)).toBe(true);
+      expect(vol.existsSync(`${CODEX_SKILLS_DIR}/skill-b/SKILL.md`)).toBe(true);
+
+      // Second sync: skill-b has been removed from the plugin.
+      vol.rmSync(`${BUNDLED_PLUGIN_DIR}/skills/skill-b`, { recursive: true });
+
+      await syncCodexSkills(BUNDLED_PLUGIN_DIR, CODEX_SKILLS_DIR);
+
+      expect(vol.existsSync(`${CODEX_SKILLS_DIR}/skill-a/SKILL.md`)).toBe(true);
+      // Removed from the source → pruned from the Codex dir.
+      expect(vol.existsSync(`${CODEX_SKILLS_DIR}/skill-b`)).toBe(false);
+      // Never managed by us → left untouched.
+      expect(vol.existsSync(`${CODEX_SKILLS_DIR}/foreign-skill/SKILL.md`)).toBe(
+        true,
+      );
+    });
+  });
+
+  describe("overlayDownloadedSkills", () => {
+    it("does nothing when the skills cache does not exist", async () => {
+      await overlayDownloadedSkills("/nonexistent", RUNTIME_PLUGIN_DIR);
+
+      expect(vol.existsSync(`${RUNTIME_PLUGIN_DIR}/skills`)).toBe(false);
+    });
+
+    it("prunes skills removed from the cache since the last overlay", async () => {
+      // First overlay: cache holds two skills.
+      vol.mkdirSync(`${RUNTIME_SKILLS_DIR}/skill-a`, { recursive: true });
+      vol.writeFileSync(`${RUNTIME_SKILLS_DIR}/skill-a/SKILL.md`, "# A");
+      vol.mkdirSync(`${RUNTIME_SKILLS_DIR}/skill-b`, { recursive: true });
+      vol.writeFileSync(`${RUNTIME_SKILLS_DIR}/skill-b/SKILL.md`, "# B");
+
+      await overlayDownloadedSkills(RUNTIME_SKILLS_DIR, RUNTIME_PLUGIN_DIR);
+
+      expect(
+        vol.existsSync(`${RUNTIME_PLUGIN_DIR}/skills/skill-a/SKILL.md`),
+      ).toBe(true);
+      expect(
+        vol.existsSync(`${RUNTIME_PLUGIN_DIR}/skills/skill-b/SKILL.md`),
+      ).toBe(true);
+
+      // Second overlay: skill-b was removed from the cache (atomic swap).
+      vol.rmSync(`${RUNTIME_SKILLS_DIR}/skill-b`, { recursive: true });
+
+      await overlayDownloadedSkills(RUNTIME_SKILLS_DIR, RUNTIME_PLUGIN_DIR);
+
+      expect(
+        vol.existsSync(`${RUNTIME_PLUGIN_DIR}/skills/skill-a/SKILL.md`),
+      ).toBe(true);
+      expect(vol.existsSync(`${RUNTIME_PLUGIN_DIR}/skills/skill-b`)).toBe(
+        false,
+      );
+    });
   });
 
   describe("copyBundledPlugin", () => {

packages/workspace-server/src/services/posthog-plugin/update-skills-saga.ts

@@ -12,9 +12,81 @@ import { basename, dirname, join } from "node:path";
 import { Saga } from "@posthog/shared";
 import { extractZip, unzipAsync } from "./extract-zip";
 
+/**
+ * Tracks which skill directories a sync wrote into a destination, so a later
+ * sync can remove the ones that have since disappeared from the source without
+ * touching skills it never managed (e.g. skills another tool placed in the
+ * shared Codex dir). Mirrors the `.sync-manifest` approach used by the
+ * ai-plugin skill-sync workflow.
+ */
+const SYNC_MANIFEST_FILE = ".sync-manifest";
+
+async function readSyncManifest(destDir: string): Promise<string[]> {
+  try {
+    const content = await readFile(join(destDir, SYNC_MANIFEST_FILE), "utf-8");
+    return content
+      .split("\n")
+      .map((line) => line.trim())
+      .filter(Boolean);
+  } catch {
+    return [];
+  }
+}
+
+async function writeSyncManifest(
+  destDir: string,
+  names: string[],
+): Promise<void> {
+  const sorted = [...names].sort();
+  await writeFile(
+    join(destDir, SYNC_MANIFEST_FILE),
+    sorted.length > 0 ? `${sorted.join("\n")}\n` : "",
+  );
+}
+
+/**
+ * Mirrors the skill directories from `sourceDir` into `destDir`:
+ * - copies/overwrites each source skill into the destination, and
+ * - removes any skill this sync previously wrote (tracked in `.sync-manifest`)
+ *   that is no longer present in the source.
+ *
+ * Skills in `destDir` that were never written by a previous sync are left
+ * untouched, so this is safe to run against a directory shared with other tools.
+ */
+async function syncSkillDirs(
+  sourceDir: string,
+  destDir: string,
+): Promise<void> {
+  await mkdir(destDir, { recursive: true });
+
+  const sourceEntries = await readdir(sourceDir, { withFileTypes: true });
+  const sourceNames = sourceEntries
+    .filter((entry) => entry.isDirectory())
+    .map((entry) => entry.name);
+  const sourceSet = new Set(sourceNames);
+
+  // Remove skills we previously synced that have since vanished from the source.
+  const previouslySynced = await readSyncManifest(destDir);
+  for (const name of previouslySynced) {
+    if (!sourceSet.has(name)) {
+      await rm(join(destDir, name), { recursive: true, force: true });
+    }
+  }
+
+  // Overlay the current source skills.
+  for (const name of sourceNames) {
+    const dest = join(destDir, name);
+    await rm(dest, { recursive: true, force: true });
+    await cp(join(sourceDir, name), dest, { recursive: true });
+  }
+
+  await writeSyncManifest(destDir, sourceNames);
+}
+
 /**
  * Overlays previously-downloaded skills on top of the runtime plugin dir.
- * Each skill directory in the cache replaces the same-named one in the plugin.
+ * Each skill directory in the cache replaces the same-named one in the plugin,
+ * and skills removed from the cache since the last overlay are pruned.
  */
 export async function overlayDownloadedSkills(
   runtimeSkillsDir: string,
@@ -24,22 +96,12 @@ export async function overlayDownloadedSkills(
     return;
   }
 
-  const destSkillsDir = join(runtimePluginDir, "skills");
-  await mkdir(destSkillsDir, { recursive: true });
-
-  const entries = await readdir(runtimeSkillsDir, { withFileTypes: true });
-  for (const entry of entries) {
-    if (entry.isDirectory()) {
-      const src = join(runtimeSkillsDir, entry.name);
-      const dest = join(destSkillsDir, entry.name);
-      await rm(dest, { recursive: true, force: true });
-      await cp(src, dest, { recursive: true });
-    }
-  }
+  await syncSkillDirs(runtimeSkillsDir, join(runtimePluginDir, "skills"));
 }
 
 /**
- * Syncs skills from the effective plugin dir to `codexSkillsDir` for Codex.
+ * Syncs skills from the effective plugin dir to `codexSkillsDir` for Codex,
+ * pruning skills removed from the plugin since the last sync.
  */
 export async function syncCodexSkills(
   pluginPath: string,
@@ -51,17 +113,7 @@ export async function syncCodexSkills(
   }
 
   try {
-    await mkdir(codexSkillsDir, { recursive: true });
-
-    const entries = await readdir(effectiveSkillsDir, { withFileTypes: true });
-    for (const entry of entries) {
-      if (entry.isDirectory()) {
-        const src = join(effectiveSkillsDir, entry.name);
-        const dest = join(codexSkillsDir, entry.name);
-        await rm(dest, { recursive: true, force: true });
-        await cp(src, dest, { recursive: true });
-      }
-    }
+    await syncSkillDirs(effectiveSkillsDir, codexSkillsDir);
   } catch {
     // Fire-and-forget — don't block startup or updates on Codex sync
   }