refactor(agents): extract formatCustomSkillsBlock to eliminate duplication
Address review feedback (P3): The User-Installed Skills block was duplicated verbatim in two if/else branches in both buildCategorySkillsDelegationGuide() and Atlas buildSkillsSection(). Extract shared formatCustomSkillsBlock() with configurable header level (#### vs **) so both builders reference a single source of truth. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -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):**
|
||||
|
||||
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.`
|
||||
${customSkillBlock}`
|
||||
} else if (customSkills.length > 0) {
|
||||
skillsTable = `**User-Installed Skills (HIGH PRIORITY):**
|
||||
|
||||
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.`
|
||||
skillsTable = customSkillBlock
|
||||
} else {
|
||||
skillsTable = `| Skill | When to Use |
|
||||
|-------|-------------|
|
||||
|
||||
@@ -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")
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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)
|
||||
|
||||
**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.`
|
||||
${customSkillBlock}`
|
||||
} else if (customSkills.length > 0) {
|
||||
skillsSection = `#### User-Installed Skills (HIGH PRIORITY)
|
||||
|
||||
**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.`
|
||||
skillsSection = customSkillBlock
|
||||
} else {
|
||||
skillsSection = `#### Available Skills (Domain Expertise Injection)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user