Merge pull request #1491 from itsmylife44/refactor/extract-formatCustomSkillsBlock

refactor(agents): extract formatCustomSkillsBlock to eliminate duplication

src/agents/atlas/utils.ts

@@ -6,7 +6,7 @@
  */
 
 import type { CategoryConfig } from "../../config/schema"
-import type { AvailableAgent, AvailableSkill } from "../dynamic-agent-prompt-builder"
+import { formatCustomSkillsBlock, type AvailableAgent, type AvailableSkill } from "../dynamic-agent-prompt-builder"
 import { DEFAULT_CATEGORIES, CATEGORY_DESCRIPTIONS } from "../../tools/delegate-task/constants"
 
 export const getCategoryDescription = (name: string, userCategories?: Record<string, CategoryConfig>) =>
@@ -70,7 +70,7 @@ export function buildSkillsSection(skills: AvailableSkill[]): string {
     return `| \`${s.name}\` | ${shortDesc} | ${source} |`
   })
 
-  const customSkillNames = customSkills.map((s) => `"${s.name}"`).join(", ")
+  const customSkillBlock = formatCustomSkillsBlock(customRows, customSkills, "**")
 
   let skillsTable: string
 
@@ -81,27 +81,9 @@ export function buildSkillsSection(skills: AvailableSkill[]): string {
 |-------|-------------|
 ${builtinRows.join("\n")}
 
-**User-Installed Skills (HIGH PRIORITY):**
+${customSkillBlock}`
-
-The user installed these for their workflow. They MUST be evaluated for EVERY delegation.
-
-| Skill | When to Use | Source |
-|-------|-------------|--------|
-${customRows.join("\n")}
-
-> **CRITICAL**: The user installed ${customSkillNames} for a reason — USE THEM when the task overlaps with their domain.
-> When in doubt, INCLUDE a user-installed skill rather than omit it.`
   } else if (customSkills.length > 0) {
-    skillsTable = `**User-Installed Skills (HIGH PRIORITY):**
+    skillsTable = customSkillBlock
-
-The user installed these for their workflow. They MUST be evaluated for EVERY delegation.
-
-| Skill | When to Use | Source |
-|-------|-------------|--------|
-${customRows.join("\n")}
-
-> **CRITICAL**: The user installed ${customSkillNames} for a reason — USE THEM when the task overlaps with their domain.
-> When in doubt, INCLUDE a user-installed skill rather than omit it.`
   } else {
     skillsTable = `| Skill | When to Use |
 |-------|-------------|

src/agents/dynamic-agent-prompt-builder.test.ts

@@ -4,6 +4,7 @@ import { describe, it, expect } from "bun:test"
 import {
   buildCategorySkillsDelegationGuide,
   buildUltraworkSection,
+  formatCustomSkillsBlock,
   type AvailableSkill,
   type AvailableCategory,
   type AvailableAgent,
@@ -159,3 +160,46 @@ describe("buildUltraworkSection", () => {
     expect(result).not.toContain("User-Installed Skills")
   })
 })
+
+describe("formatCustomSkillsBlock", () => {
+  const customSkills: AvailableSkill[] = [
+    { name: "react-19", description: "React 19 patterns", location: "user" },
+    { name: "tailwind-4", description: "Tailwind v4", location: "project" },
+  ]
+
+  const customRows = customSkills.map((s) => {
+    const source = s.location === "project" ? "project" : "user"
+    return `| \`${s.name}\` | ${s.description} | ${source} |`
+  })
+
+  it("should produce consistent output used by both builders", () => {
+    //#given: custom skills and rows
+    //#when: formatting with default header level
+    const result = formatCustomSkillsBlock(customRows, customSkills)
+
+    //#then: contains all expected elements
+    expect(result).toContain("User-Installed Skills (HIGH PRIORITY)")
+    expect(result).toContain("CRITICAL")
+    expect(result).toContain('"react-19"')
+    expect(result).toContain('"tailwind-4"')
+    expect(result).toContain("| user |")
+    expect(result).toContain("| project |")
+  })
+
+  it("should use #### header by default", () => {
+    //#given: default header level
+    const result = formatCustomSkillsBlock(customRows, customSkills)
+
+    //#then: uses markdown h4
+    expect(result).toContain("#### User-Installed Skills")
+  })
+
+  it("should use bold header when specified", () => {
+    //#given: bold header level (used by Atlas)
+    const result = formatCustomSkillsBlock(customRows, customSkills, "**")
+
+    //#then: uses bold instead of h4
+    expect(result).toContain("**User-Installed Skills (HIGH PRIORITY):**")
+    expect(result).not.toContain("#### User-Installed Skills")
+  })
+})

src/agents/dynamic-agent-prompt-builder.ts

@@ -166,6 +166,33 @@ export function buildDelegationTable(agents: AvailableAgent[]): string {
   return rows.join("\n")
 }
 
+/**
+ * Renders the "User-Installed Skills (HIGH PRIORITY)" block used across multiple agent prompts.
+ * Extracted to avoid duplication between buildCategorySkillsDelegationGuide, buildSkillsSection, etc.
+ */
+export function formatCustomSkillsBlock(
+  customRows: string[],
+  customSkills: AvailableSkill[],
+  headerLevel: "####" | "**" = "####"
+): string {
+  const customSkillNames = customSkills.map((s) => `"${s.name}"`).join(", ")
+  const header = headerLevel === "####"
+    ? `#### User-Installed Skills (HIGH PRIORITY)`
+    : `**User-Installed Skills (HIGH PRIORITY):**`
+
+  return `${header}
+
+**The user has installed these custom skills. They MUST be evaluated for EVERY delegation.**
+Subagents are STATELESS — they lose all custom knowledge unless you pass these skills via \`load_skills\`.
+
+| Skill | Expertise Domain | Source |
+|-------|------------------|--------|
+${customRows.join("\n")}
+
+> **CRITICAL**: Ignoring user-installed skills when they match the task domain is a failure.
+> The user installed ${customSkillNames} for a reason — USE THEM when the task overlaps with their domain.`
+}
+
 export function buildCategorySkillsDelegationGuide(categories: AvailableCategory[], skills: AvailableSkill[]): string {
   if (categories.length === 0 && skills.length === 0) return ""
 
@@ -188,7 +215,7 @@ export function buildCategorySkillsDelegationGuide(categories: AvailableCategory
     return `| \`${s.name}\` | ${desc} | ${source} |`
   })
 
-  const customSkillNames = customSkills.map((s) => `"${s.name}"`).join(", ")
+  const customSkillBlock = formatCustomSkillsBlock(customRows, customSkills)
 
   let skillsSection: string
 
@@ -199,29 +226,9 @@ export function buildCategorySkillsDelegationGuide(categories: AvailableCategory
 |-------|------------------|
 ${builtinRows.join("\n")}
 
-#### User-Installed Skills (HIGH PRIORITY)
+${customSkillBlock}`
-
-**The user has installed these custom skills. They MUST be evaluated for EVERY delegation.**
-Subagents are STATELESS — they lose all custom knowledge unless you pass these skills via \`load_skills\`.
-
-| Skill | Expertise Domain | Source |
-|-------|------------------|--------|
-${customRows.join("\n")}
-
-> **CRITICAL**: Ignoring user-installed skills when they match the task domain is a failure.
-> The user installed ${customSkillNames} for a reason — USE THEM when the task overlaps with their domain.`
   } else if (customSkills.length > 0) {
-    skillsSection = `#### User-Installed Skills (HIGH PRIORITY)
+    skillsSection = customSkillBlock
-
-**The user has installed these custom skills. They MUST be evaluated for EVERY delegation.**
-Subagents are STATELESS — they lose all custom knowledge unless you pass these skills via \`load_skills\`.
-
-| Skill | Expertise Domain | Source |
-|-------|------------------|--------|
-${customRows.join("\n")}
-
-> **CRITICAL**: Ignoring user-installed skills when they match the task domain is a failure.
-> The user installed ${customSkillNames} for a reason — USE THEM when the task overlaps with their domain.`
   } else {
     skillsSection = `#### Available Skills (Domain Expertise Injection)