Skip to content

fix(agent): stop double-loading .claude/skills as synthetic plugins #3351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MattPua wants to merge 1 commit into from
Closed

fix(agent): stop double-loading .claude/skills as synthetic plugins #3351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/workspace-server/src/services/agent/agent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
190 changes: 6 additions & 184 deletions packages/workspace-server/src/services/agent/discover-plugins.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 });
Expand All @@ -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-/);
});
});

Expand Down
64 changes: 13 additions & 51 deletions packages/workspace-server/src/services/agent/discover-plugins.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -13,7 +12,6 @@ import type { AgentScopedLogger } from "./ports";

interface DiscoverPluginsOptions {
userDataDir: string;
repoPath?: string;
/**
* The bundled PostHog skills dir (`<plugin>/skills`). Used only to dedupe the
* user's Codex skills against names PostHog Code already provides.
Expand All @@ -28,26 +26,24 @@ const noopLogger: AgentScopedLogger = {
error() {},
};

/**
* `~/.claude/skills` and `<repoPath>/.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<SdkPluginConfig[]> {
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];
}

/**
Expand Down Expand Up @@ -77,45 +73,11 @@ async function discoverCodexSkills(
);
}

async function discoverUserSkills(
userDataDir: string,
log: AgentScopedLogger,
): Promise<SdkPluginConfig[]> {
return buildSyntheticPlugin(
getUserSkillsDir(),
path.join(userDataDir, "plugins", "user-skills"),
"user-skills",
"User Claude skills",
log,
);
}

async function discoverMarketplacePlugins(): Promise<SdkPluginConfig[]> {
const paths = await getMarketplaceInstallPaths();
return paths.map((p) => ({ type: "local" as const, path: p }));
}

async function discoverRepoSkills(
userDataDir: string,
repoPath: string,
log: AgentScopedLogger,
): Promise<SdkPluginConfig[]> {
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,
Expand Down
Loading