feat(skills): refuse reloading attached and already-loaded skills

Author: TheGreatAxios

src/agent/use-skill.test.ts

@@ -3,6 +3,7 @@ import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { describe, expect, test } from "bun:test";
 
+import { withMockedModuleDuring } from "../../tests/helpers/mock-module.js";
 import {
   createUseSkillTool,
   useSkillDefinition,
@@ -66,3 +67,65 @@ describe("createUseSkillTool allowedNames", () => {
     expect(out).toContain("Create worktree recipe.");
   });
 });
+
+describe("createUseSkillTool already-in-context", () => {
+  test("attached name is refused without resolving the body", async () => {
+    const cwd = await fixtureWithHiddenSkill();
+    let resolveCalls = 0;
+    await withMockedModuleDuring(
+      import.meta.resolve("../extensions/skills.js"),
+      (real: typeof import("../extensions/skills.js")) => ({
+        ...real,
+        resolveSkillBody: async (
+          ...args: Parameters<typeof real.resolveSkillBody>
+        ) => {
+          resolveCalls += 1;
+          return real.resolveSkillBody(...args);
+        },
+      }),
+      async () => {
+        const tool = createUseSkillTool(
+          cwd,
+          [],
+          undefined,
+          undefined,
+          useSkillDefinition,
+          ["git-worktrees"],
+        );
+        const out = await call(tool, { name: "git-worktrees" });
+        expect(out).toBe(
+          'Skill "git-worktrees" is already attached / already in context.',
+        );
+        expect(out).not.toContain("Create worktree recipe.");
+      },
+    );
+    expect(resolveCalls).toBe(0);
+  });
+
+  test("second use_skill of the same name is refused without returning the body", async () => {
+    const cwd = await fixtureWithHiddenSkill();
+    const tool = createUseSkillTool(cwd);
+    const first = await call(tool, { name: "git-worktrees" });
+    expect(first).toContain("Create worktree recipe.");
+    const second = await call(tool, { name: "git-worktrees" });
+    expect(second).toBe(
+      'Skill "git-worktrees" is already attached / already in context.',
+    );
+    expect(second).not.toContain("Create worktree recipe.");
+  });
+
+  test("first load still returns the body when the name is not attached", async () => {
+    const cwd = await fixtureWithHiddenSkill();
+    const tool = createUseSkillTool(
+      cwd,
+      [],
+      undefined,
+      undefined,
+      useSkillDefinition,
+      ["style"],
+    );
+    const out = await call(tool, { name: "git-worktrees" });
+    expect(out).toContain("Create worktree recipe.");
+    expect(out).toContain('Skill "git-worktrees"');
+  });
+});

src/agent/use-skill.ts

@@ -12,7 +12,9 @@ import { captureSkillUsed } from "../telemetry/product-events.js";
 // model decides one applies. There is no operator invocation — discovery and
 // loading are entirely model-driven. Primary copy is on-demand catalog
 // (Skywalker has no attached skills). Workers mount workerUseSkillDefinition
-// so they do not reload bodies already injected as attached.
+// so they do not reload bodies already injected as attached. The handler
+// refuses attached names and names already loaded this session so the body
+// is never dumped twice.
 const USE_SKILL_INPUT_SCHEMA = {
   type: "object",
   properties: {
@@ -40,15 +42,21 @@ export const workerUseSkillDefinition: ToolDefinition = {
 
 const UseSkillArgs = type({ name: "string" });
 
+function alreadyInContextMessage(name: string): string {
+  return `Skill "${name}" is already attached / already in context.`;
+}
+
 export function createUseSkillTool(
   cwd: string,
   skillDirs: string[] = [],
   telemetry: Telemetry = NOOP_TELEMETRY,
   allowedNames?: readonly string[],
   definition: ToolDefinition = useSkillDefinition,
+  attachedNames?: readonly string[],
 ): AgentTool {
   const allowed =
     allowedNames === undefined ? undefined : new Set(allowedNames);
+  const loaded = new Set(attachedNames ?? []);
   return stringTool({
     definition,
     handler: async (rawArgs: Record<string, unknown>): Promise<string> => {
@@ -61,12 +69,14 @@ export function createUseSkillTool(
       if (allowed !== undefined && !allowed.has(name)) {
         return `No skill named "${name}" is available.`;
       }
+      if (loaded.has(name)) return alreadyInContextMessage(name);
       const body = await resolveSkillBody(cwd, name, skillDirs);
       if (body === undefined) return `No skill named "${name}" is available.`;
       // Skill names are project- or plugin-authored, so an unrecognised
       // name never leaves the process: first-party `corbits-skills` names
       // are reported by name, everything else as `custom`.
       captureSkillUsed(telemetry, name);
+      loaded.add(name);
       return `Skill "${name}" — follow these instructions for this task:\n\n${body}`;
     },
   });

src/subagent/run-skill-scope.test.ts

@@ -359,7 +359,6 @@ describe("runSubAgent worker skill mounts (CL-7668)", () => {
               await run({
                 ...baseParams(cwd, join(cwd, ".ctx"), baseURL),
                 skillDirs: [pluginRoot],
-                attachedSkills: ["style"],
               }).catch(() => {
                 // Inference fails by design; mount decisions run first.
               });
@@ -377,6 +376,70 @@ describe("runSubAgent worker skill mounts (CL-7668)", () => {
     expect(loaded).toContain("Follow the style guide.");
   }, 15_000);
 
+  test("threads attachedSkills into use_skill and refuses those names without returning the body", async () => {
+    const cwd = await tmpCwd();
+    await writeSkill(
+      cwd,
+      "style",
+      "Code style rules.",
+      "Follow the style guide.",
+    );
+
+    let useSkillArgs: readonly unknown[] | undefined;
+    let useSkillTool:
+      | {
+          kind: string;
+          handler: (
+            args: Record<string, unknown>,
+            signal: AbortSignal,
+          ) => Promise<string>;
+        }
+      | undefined;
+
+    await runWithFailingInference((baseURL) =>
+      withMockedModuleDuring(
+        import.meta.resolve("../agent/use-skill.js"),
+        (real: typeof import("../agent/use-skill.js")) => ({
+          ...real,
+          createUseSkillTool: (...args: unknown[]) => {
+            useSkillArgs = args;
+            const tool = (
+              real.createUseSkillTool as (...a: never[]) => unknown
+            )(...(args as never[]));
+            if (
+              typeof tool !== "object" ||
+              tool === null ||
+              (tool as { kind: string }).kind !== "string"
+            )
+              throw new Error("expected string tool");
+            useSkillTool = tool as typeof useSkillTool & {};
+            return tool;
+          },
+        }),
+        async () => {
+          const { runSubAgent: run } = await import("./run.js");
+          await run({
+            ...baseParams(cwd, join(cwd, ".ctx"), baseURL),
+            attachedSkills: ["style"],
+          }).catch(() => {
+            // Inference fails by design; mount decisions run first.
+          });
+        },
+      ),
+    );
+
+    expect(useSkillArgs?.[5]).toEqual(["style"]);
+    expect(useSkillTool).toBeDefined();
+    const refused = await useSkillTool?.handler(
+      { name: "style" },
+      new AbortController().signal,
+    );
+    expect(refused).toBe(
+      'Skill "style" is already attached / already in context.',
+    );
+    expect(refused).not.toContain("Follow the style guide.");
+  }, 15_000);
+
   test("injects attached skill bodies into the worker prompt and notes misses without parking", async () => {
     const cwd = await tmpCwd();
     const pluginRoot = join(cwd, "plugin");

src/subagent/run.ts

@@ -782,7 +782,8 @@ async function runSubAgentInner(
     // (union of pkg.attachedSkills and optionalSkills). Mounted before the
     // capability filter so worker allowlists keep them like any other named
     // tool; the scope cannot widen — use_skill refuses names outside the
-    // allowlist. Plugin skill dirs match the primary so bundled
+    // allowlist and refuses attached/already-loaded names without dumping the
+    // body again. Plugin skill dirs match the primary so bundled
     // corbits-skills (style/philosophy) resolve.
     const modelFamilyPolicy = resolveModelFamilyPolicy({
       providerName: params.provider.providerName,
@@ -806,6 +807,7 @@ async function runSubAgentInner(
         liveTelemetry,
         params.allowedSkillNames,
         workerUseSkillDefinition,
+        params.attachedSkills,
       ),
     ];