From 67dbff7e4929d7e765e5ce37744a8aac77a21a16 Mon Sep 17 00:00:00 2001 From: Matt Pua Date: Fri, 10 Jul 2026 11:40:36 -0400 Subject: [PATCH] fix(agent): stop double-loading .claude/skills as synthetic plugins The Claude Agent SDK already auto-loads ~/.claude/skills and /.claude/skills natively as @skills-dir plugins. Wrapping them again in synthetic user-skills/repo-skills- plugins made every skill show up twice in the session's skill list. Generated-By: PostHog Code Task-Id: bd56bbcd-cc4e-4d96-9913-0ff3c22968a5 --- .../src/services/agent/agent.ts | 1 - .../services/agent/discover-plugins.test.ts | 190 +----------------- .../src/services/agent/discover-plugins.ts | 64 ++---- 3 files changed, 19 insertions(+), 236 deletions(-) diff --git a/packages/workspace-server/src/services/agent/agent.ts b/packages/workspace-server/src/services/agent/agent.ts index e0852eae1d..3b776847b5 100644 --- a/packages/workspace-server/src/services/agent/agent.ts +++ b/packages/workspace-server/src/services/agent/agent.ts @@ -924,7 +924,6 @@ If a repository IS genuinely required, attach one in this priority order: externalPlugins = await discoverExternalPlugins( { userDataDir: this.storagePaths.appDataPath, - repoPath, bundledSkillsDir, }, this.log, diff --git a/packages/workspace-server/src/services/agent/discover-plugins.test.ts b/packages/workspace-server/src/services/agent/discover-plugins.test.ts index bbf390f446..23ca42e0c0 100644 --- a/packages/workspace-server/src/services/agent/discover-plugins.test.ts +++ b/packages/workspace-server/src/services/agent/discover-plugins.test.ts @@ -46,91 +46,6 @@ describe("discoverExternalPlugins", () => { expect(result).toEqual([]); }); - describe("user skills", () => { - it("discovers user skills from ~/.claude/skills/", async () => { - createSkillDir(USER_SKILLS_DIR, "my-skill"); - - const result = await discoverExternalPlugins({ - userDataDir: USER_DATA_DIR, - }); - - expect(result).toHaveLength(1); - expect(result[0]).toEqual({ - type: "local", - path: `${USER_DATA_DIR}/plugins/user-skills`, - }); - }); - - it("creates a synthetic plugin.json for user skills", async () => { - createSkillDir(USER_SKILLS_DIR, "my-skill"); - - await discoverExternalPlugins({ userDataDir: USER_DATA_DIR }); - - const pluginJson = JSON.parse( - vol.readFileSync( - `${USER_DATA_DIR}/plugins/user-skills/plugin.json`, - "utf-8", - ) as string, - ); - expect(pluginJson).toEqual({ - name: "user-skills", - description: "User Claude skills", - version: "1.0.0", - }); - }); - - it("symlinks each skill directory into the synthetic plugin", async () => { - createSkillDir(USER_SKILLS_DIR, "skill-a"); - createSkillDir(USER_SKILLS_DIR, "skill-b"); - - await discoverExternalPlugins({ userDataDir: USER_DATA_DIR }); - - const syntheticSkillsDir = `${USER_DATA_DIR}/plugins/user-skills/skills`; - const entries = vol.readdirSync(syntheticSkillsDir); - expect(entries).toContain("skill-a"); - expect(entries).toContain("skill-b"); - }); - - it("ignores directories without SKILL.md", async () => { - vol.mkdirSync(`${USER_SKILLS_DIR}/not-a-skill`, { recursive: true }); - vol.writeFileSync(`${USER_SKILLS_DIR}/not-a-skill/README.md`, "nope"); - - const result = await discoverExternalPlugins({ - userDataDir: USER_DATA_DIR, - }); - - expect(result).toEqual([]); - }); - - it("ignores regular files in the skills directory", async () => { - vol.mkdirSync(USER_SKILLS_DIR, { recursive: true }); - vol.writeFileSync(`${USER_SKILLS_DIR}/random-file.txt`, "hello"); - - const result = await discoverExternalPlugins({ - userDataDir: USER_DATA_DIR, - }); - - expect(result).toEqual([]); - }); - - it("cleans stale symlinks before creating new ones", async () => { - createSkillDir(USER_SKILLS_DIR, "fresh-skill"); - - // First run - await discoverExternalPlugins({ userDataDir: USER_DATA_DIR }); - - // Manually add a stale entry to simulate leftover from previous run - const syntheticSkillsDir = `${USER_DATA_DIR}/plugins/user-skills/skills`; - vol.mkdirSync(`${syntheticSkillsDir}/stale-skill`, { recursive: true }); - - // Second run should clean stale and only have fresh-skill - await discoverExternalPlugins({ userDataDir: USER_DATA_DIR }); - - const entries = vol.readdirSync(syntheticSkillsDir); - expect(entries).toEqual(["fresh-skill"]); - }); - }); - describe("marketplace plugins", () => { it("discovers installed marketplace plugins", async () => { const installPath = "/mock/plugins/my-plugin"; @@ -293,97 +208,8 @@ describe("discoverExternalPlugins", () => { }); }); - describe("repo skills", () => { - const REPO_PATH = "/mock/repo"; - - it("discovers skills from repo .claude/skills/", async () => { - createSkillDir(`${REPO_PATH}/.claude/skills`, "repo-skill"); - - const result = await discoverExternalPlugins({ - userDataDir: USER_DATA_DIR, - repoPath: REPO_PATH, - }); - - expect(result).toHaveLength(1); - expect(result[0]?.type).toBe("local"); - expect(result[0]?.path).toMatch( - /\/mock\/userData\/plugins\/repo-skills-[a-f0-9]{8}$/, - ); - }); - - it("creates a synthetic plugin.json with repo name in description", async () => { - createSkillDir(`${REPO_PATH}/.claude/skills`, "repo-skill"); - - await discoverExternalPlugins({ - userDataDir: USER_DATA_DIR, - repoPath: REPO_PATH, - }); - - // Find the generated plugin dir - const pluginEntries = vol.readdirSync( - `${USER_DATA_DIR}/plugins`, - ) as string[]; - const repoPluginDir = pluginEntries.find((e) => - e.startsWith("repo-skills-"), - ); - expect(repoPluginDir).toBeDefined(); - - const pluginJson = JSON.parse( - vol.readFileSync( - `${USER_DATA_DIR}/plugins/${repoPluginDir}/plugin.json`, - "utf-8", - ) as string, - ); - expect(pluginJson.description).toBe("Repo skills for repo"); - }); - - it("returns empty when repoPath has no .claude/skills dir", async () => { - vol.mkdirSync(REPO_PATH, { recursive: true }); - - const result = await discoverExternalPlugins({ - userDataDir: USER_DATA_DIR, - repoPath: REPO_PATH, - }); - - expect(result).toEqual([]); - }); - - it("skips repo skills when repoPath is not provided", async () => { - createSkillDir(`${REPO_PATH}/.claude/skills`, "repo-skill"); - - const result = await discoverExternalPlugins({ - userDataDir: USER_DATA_DIR, - }); - - // Only user skills and marketplace plugins are checked - expect(result).toEqual([]); - }); - - it("uses deterministic hash for repo plugin dir name", async () => { - createSkillDir(`${REPO_PATH}/.claude/skills`, "repo-skill"); - - const result1 = await discoverExternalPlugins({ - userDataDir: USER_DATA_DIR, - repoPath: REPO_PATH, - }); - - vol.reset(); - createSkillDir(`${REPO_PATH}/.claude/skills`, "repo-skill"); - - const result2 = await discoverExternalPlugins({ - userDataDir: USER_DATA_DIR, - repoPath: REPO_PATH, - }); - - expect(result1[0]?.path).toBe(result2[0]?.path); - }); - }); - describe("combined sources", () => { - it("merges all three sources together", async () => { - // User skills - createSkillDir(USER_SKILLS_DIR, "user-skill"); - + it("merges marketplace and codex skills together", async () => { // Marketplace plugin const marketplacePath = "/mock/plugins/marketplace-plugin"; vol.mkdirSync(marketplacePath, { recursive: true }); @@ -404,26 +230,22 @@ describe("discoverExternalPlugins", () => { }), ); - // Repo skills - const repoPath = "/mock/repo"; - createSkillDir(`${repoPath}/.claude/skills`, "repo-skill"); + // Codex skills + createSkillDir("/mock/home/.agents/skills", "codex-skill"); const result = await discoverExternalPlugins({ userDataDir: USER_DATA_DIR, - repoPath, }); - expect(result).toHaveLength(3); + expect(result).toHaveLength(2); expect(result[0]).toEqual({ type: "local", - path: `${USER_DATA_DIR}/plugins/user-skills`, + path: marketplacePath, }); expect(result[1]).toEqual({ type: "local", - path: marketplacePath, + path: `${USER_DATA_DIR}/plugins/codex-skills`, }); - expect(result[2]?.type).toBe("local"); - expect(result[2]?.path).toMatch(/repo-skills-/); }); }); diff --git a/packages/workspace-server/src/services/agent/discover-plugins.ts b/packages/workspace-server/src/services/agent/discover-plugins.ts index 3e5cfe2dfc..7d56b85046 100644 --- a/packages/workspace-server/src/services/agent/discover-plugins.ts +++ b/packages/workspace-server/src/services/agent/discover-plugins.ts @@ -1,4 +1,3 @@ -import * as crypto from "node:crypto"; import * as fs from "node:fs"; import * as path from "node:path"; import type { SdkPluginConfig } from "@anthropic-ai/claude-agent-sdk"; @@ -13,7 +12,6 @@ import type { AgentScopedLogger } from "./ports"; interface DiscoverPluginsOptions { userDataDir: string; - repoPath?: string; /** * The bundled PostHog skills dir (`/skills`). Used only to dedupe the * user's Codex skills against names PostHog Code already provides. @@ -28,26 +26,24 @@ const noopLogger: AgentScopedLogger = { error() {}, }; +/** + * `~/.claude/skills` and `/.claude/skills` are already auto-loaded + * natively by the Claude Agent SDK as `@skills-dir` plugins, so they're not + * wrapped here — doing so double-registered every skill under both its bare + * name and a synthetic plugin prefix. Only sources the SDK can't see on its + * own (marketplace-installed plugins, the user's Codex skills dir) need + * manual discovery. + */ export async function discoverExternalPlugins( options: DiscoverPluginsOptions, log: AgentScopedLogger = noopLogger, ): Promise { - const [globalSkills, marketplacePlugins, repoSkills, codexSkills] = - await Promise.all([ - discoverUserSkills(options.userDataDir, log), - discoverMarketplacePlugins(), - options.repoPath - ? discoverRepoSkills(options.userDataDir, options.repoPath, log) - : Promise.resolve([]), - discoverCodexSkills(options.userDataDir, options.bundledSkillsDir, log), - ]); + const [marketplacePlugins, codexSkills] = await Promise.all([ + discoverMarketplacePlugins(), + discoverCodexSkills(options.userDataDir, options.bundledSkillsDir, log), + ]); - return [ - ...globalSkills, - ...marketplacePlugins, - ...repoSkills, - ...codexSkills, - ]; + return [...marketplacePlugins, ...codexSkills]; } /** @@ -77,45 +73,11 @@ async function discoverCodexSkills( ); } -async function discoverUserSkills( - userDataDir: string, - log: AgentScopedLogger, -): Promise { - return buildSyntheticPlugin( - getUserSkillsDir(), - path.join(userDataDir, "plugins", "user-skills"), - "user-skills", - "User Claude skills", - log, - ); -} - async function discoverMarketplacePlugins(): Promise { const paths = await getMarketplaceInstallPaths(); return paths.map((p) => ({ type: "local" as const, path: p })); } -async function discoverRepoSkills( - userDataDir: string, - repoPath: string, - log: AgentScopedLogger, -): Promise { - const skillsDir = path.join(repoPath, ".claude", "skills"); - const hash = crypto - .createHash("md5") - .update(repoPath) - .digest("hex") - .slice(0, 8); - - return buildSyntheticPlugin( - skillsDir, - path.join(userDataDir, "plugins", `repo-skills-${hash}`), - `repo-skills-${hash}`, - `Repo skills for ${path.basename(repoPath)}`, - log, - ); -} - async function buildSyntheticPlugin( sourceSkillsDir: string, pluginDir: string,