fix(skills): preserve disable-model-invocation prompt filtering

altaywtf · altaywtf · commit 2b48b3a45596 · 2026-04-05T01:25:54.000+03:00
diff --git a/src/agents/skills.buildworkspaceskillsnapshot.test.ts b/src/agents/skills.buildworkspaceskillsnapshot.test.ts
@@ -104,10 +104,8 @@ describe("buildWorkspaceSkillSnapshot", () => {
 
     expect(snapshot.prompt).toContain("visible-skill");
     expect(snapshot.prompt).not.toContain("hidden-skill");
-    expect(snapshot.skills.map((skill) => skill.name).toSorted()).toEqual([
-      "hidden-skill",
-      "visible-skill",
-    ]);
+    expect(snapshot.skills.map((skill) => skill.name)).toContain("hidden-skill");
+    expect(snapshot.skills.map((skill) => skill.name)).toContain("visible-skill");
   });
 
   it("keeps prompt output aligned with buildWorkspaceSkillsPrompt", async () => {
diff --git a/src/agents/skills.loadworkspaceskillentries.test.ts b/src/agents/skills.loadworkspaceskillentries.test.ts
@@ -149,12 +149,12 @@ describe("loadWorkspaceSkillEntries", () => {
     expect(entries.map((entry) => entry.skill.name)).toContain("fallback-name");
   });
 
-  it("keeps disable-model-invocation skills visible in exposure metadata for newly loaded entries", async () => {
+  it("marks disable-model-invocation skills as hidden in exposure metadata for newly loaded entries", async () => {
     const workspaceDir = await createTempWorkspaceDir();
     await writeSkill({
       dir: path.join(workspaceDir, "skills", "hidden-skill"),
       name: "hidden-skill",
-      description: "Visible prompt entry",
+      description: "Hidden prompt entry",
       frontmatterExtra: "disable-model-invocation: true",
     });
 
@@ -166,7 +166,7 @@ describe("loadWorkspaceSkillEntries", () => {
     const hiddenEntry = entries.find((entry) => entry.skill.name === "hidden-skill");
 
     expect(hiddenEntry?.invocation?.disableModelInvocation).toBe(true);
-    expect(hiddenEntry?.exposure?.includeInAvailableSkillsPrompt).toBe(true);
+    expect(hiddenEntry?.exposure?.includeInAvailableSkillsPrompt).toBe(false);
   });
 
   it("inherits agents.defaults.skills when an agent omits skills", async () => {
diff --git a/src/agents/skills.test.ts b/src/agents/skills.test.ts
@@ -333,22 +333,22 @@ describe("buildWorkspaceSkillsPrompt", () => {
     expect(prompt).toContain(path.join(skillDir, "SKILL.md"));
   });
 
-  it("keeps runtime-eligible skills visible in available_skills when disable-model-invocation is set", async () => {
+  it("omits disable-model-invocation skills from available_skills for freshly loaded entries", async () => {
     const workspaceDir = await makeWorkspace();
     const skillDir = path.join(workspaceDir, "skills", "hidden-skill");
 
     await writeSkill({
       dir: skillDir,
       name: "hidden-skill",
-      description: "Still visible in the prompt",
+      description: "Hidden from the prompt",
       frontmatterExtra: "disable-model-invocation: true",
     });
 
     const prompt = buildWorkspaceSkillsPrompt(workspaceDir, resolveTestSkillDirs(workspaceDir));
 
-    expect(prompt).toContain("hidden-skill");
-    expect(prompt).toContain("Still visible in the prompt");
-    expect(prompt).toContain(path.join(skillDir, "SKILL.md"));
+    expect(prompt).not.toContain("hidden-skill");
+    expect(prompt).not.toContain("Hidden from the prompt");
+    expect(prompt).not.toContain(path.join(skillDir, "SKILL.md"));
   });
 });
 
diff --git a/src/agents/skills/skill-contract.ts b/src/agents/skills/skill-contract.ts
@@ -36,9 +36,10 @@ function escapeXml(str: string): string {
 }
 
 /**
- * Keep this formatter byte-for-byte aligned with the upstream Agent Skills XML
- * layout so we can avoid importing the full pi-coding-agent package root on the
- * cold skills path.
+ * Keep this formatter's XML layout byte-for-byte aligned with the upstream
+ * Agent Skills formatter so we can avoid importing the full pi-coding-agent
+ * package root on the cold skills path. Visibility policy is applied upstream
+ * before calling this helper.
  */
 export function formatSkillsForPrompt(skills: Skill[]): string {
   if (skills.length === 0) {
diff --git a/src/agents/skills/workspace.ts b/src/agents/skills/workspace.ts
@@ -487,9 +487,10 @@ function loadSkillEntries(
       invocation,
       exposure: {
         includeInRuntimeRegistry: true,
-        // Prompt visibility now follows runtime inclusion for freshly loaded skills.
-        // Legacy entries without exposure metadata still use the fallback below.
-        includeInAvailableSkillsPrompt: true,
+        // Freshly loaded entries preserve the documented disable-model-invocation
+        // contract, while legacy entries without exposure metadata still use the
+        // fallback in isSkillVisibleInAvailableSkillsPrompt().
+        includeInAvailableSkillsPrompt: invocation.disableModelInvocation !== true,
         userInvocable: invocation.userInvocable !== false,
       },
     };
