Skip to content

Commit 34adbb3

Browse files
committed
fix(skills): claim use_skill names before resolve so parallel loads do not double-dump
1 parent 7cf1628 commit 34adbb3

2 files changed

Lines changed: 44 additions & 3 deletions

File tree

‎src/agent/use-skill.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,38 @@ describe("createUseSkillTool already-in-context", () => {
114114
expect(second).not.toContain("Create worktree recipe.");
115115
});
116116

117+
test("parallel use_skill of the same name dumps the body only once", async () => {
118+
const cwd = await fixtureWithHiddenSkill();
119+
let resolveCalls = 0;
120+
await withMockedModuleDuring(
121+
import.meta.resolve("../extensions/skills.js"),
122+
(real: typeof import("../extensions/skills.js")) => ({
123+
...real,
124+
resolveSkillBody: async (
125+
...args: Parameters<typeof real.resolveSkillBody>
126+
) => {
127+
resolveCalls += 1;
128+
await Promise.resolve();
129+
return real.resolveSkillBody(...args);
130+
},
131+
}),
132+
async () => {
133+
const tool = createUseSkillTool(cwd);
134+
const [a, b] = await Promise.all([
135+
call(tool, { name: "git-worktrees" }),
136+
call(tool, { name: "git-worktrees" }),
137+
]);
138+
const bodies = [a, b].filter((s) => s.includes("Create worktree recipe."));
139+
const refused = [a, b].filter((s) =>
140+
s.includes("already attached / already in context"),
141+
);
142+
expect(bodies).toHaveLength(1);
143+
expect(refused).toHaveLength(1);
144+
},
145+
);
146+
expect(resolveCalls).toBe(1);
147+
});
148+
117149
test("first load still returns the body when the name is not attached", async () => {
118150
const cwd = await fixtureWithHiddenSkill();
119151
const tool = createUseSkillTool(

‎src/agent/use-skill.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,22 @@ export function createUseSkillTool(
7070
return `No skill named "${name}" is available.`;
7171
}
7272
if (loaded.has(name)) return alreadyInContextMessage(name);
73-
const body = await resolveSkillBody(cwd, name, skillDirs);
74-
if (body === undefined) return `No skill named "${name}" is available.`;
73+
loaded.add(name);
74+
let body: string | undefined;
75+
try {
76+
body = await resolveSkillBody(cwd, name, skillDirs);
77+
} catch (err) {
78+
loaded.delete(name);
79+
throw err;
80+
}
81+
if (body === undefined) {
82+
loaded.delete(name);
83+
return `No skill named "${name}" is available.`;
84+
}
7585
// Skill names are project- or plugin-authored, so an unrecognised
7686
// name never leaves the process: first-party `corbits-skills` names
7787
// are reported by name, everything else as `custom`.
7888
captureSkillUsed(telemetry, name);
79-
loaded.add(name);
8089
return `Skill "${name}" — follow these instructions for this task:\n\n${body}`;
8190
},
8291
});

0 commit comments

Comments
 (0)