From 0a58debd92f283455147e357ffee424fe21e931c Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Thu, 19 Feb 2026 17:41:29 +0900 Subject: [PATCH] refactor(agents): remove dead code and update to compact skill format - Remove formatCustomSkillsBlock function (dead code) - Remove unused truncateDescription import - Update buildCategorySkillsDelegationGuide to compact format - Update tests to match new compact output Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus --- .../dynamic-agent-prompt-builder.test.ts | 94 +++++++----------- src/agents/dynamic-agent-prompt-builder.ts | 95 ++++++------------- 2 files changed, 59 insertions(+), 130 deletions(-) diff --git a/src/agents/dynamic-agent-prompt-builder.test.ts b/src/agents/dynamic-agent-prompt-builder.test.ts index fcc99bad..f105542b 100644 --- a/src/agents/dynamic-agent-prompt-builder.test.ts +++ b/src/agents/dynamic-agent-prompt-builder.test.ts @@ -4,7 +4,6 @@ import { describe, it, expect } from "bun:test" import { buildCategorySkillsDelegationGuide, buildUltraworkSection, - formatCustomSkillsBlock, type AvailableSkill, type AvailableCategory, type AvailableAgent, @@ -30,40 +29,39 @@ describe("buildCategorySkillsDelegationGuide", () => { { name: "our-design-system", description: "Internal design system components", location: "project" }, ] - it("should separate builtin and custom skills into distinct sections", () => { + it("should list builtin and custom skills in compact format", () => { //#given: mix of builtin and custom skills const allSkills = [...builtinSkills, ...customUserSkills] //#when: building the delegation guide const result = buildCategorySkillsDelegationGuide(categories, allSkills) - //#then: should have separate sections - expect(result).toContain("Built-in Skills") - expect(result).toContain("User-Installed Skills") - expect(result).toContain("HIGH PRIORITY") + //#then: should use compact format with both sections + expect(result).toContain("**Built-in**: playwright, frontend-ui-ux") + expect(result).toContain("YOUR SKILLS (PRIORITY)") + expect(result).toContain("react-19 (user)") + expect(result).toContain("tailwind-4 (user)") }) - it("should list custom skills and keep CRITICAL warning", () => { - //#given: custom skills installed + it("should point to skill tool as source of truth", () => { + //#given: skills present const allSkills = [...builtinSkills, ...customUserSkills] //#when: building the delegation guide const result = buildCategorySkillsDelegationGuide(categories, allSkills) - //#then: should mention custom skills by name and include warning - expect(result).toContain("`react-19`") - expect(result).toContain("`tailwind-4`") - expect(result).toContain("CRITICAL") + //#then: should reference the skill tool for full descriptions + expect(result).toContain("`skill` tool") }) - it("should show source column for custom skills (user vs project)", () => { + it("should show source tags for custom skills (user vs project)", () => { //#given: both user and project custom skills const allSkills = [...builtinSkills, ...customUserSkills, ...customProjectSkills] //#when: building the delegation guide const result = buildCategorySkillsDelegationGuide(categories, allSkills) - //#then: should show source for each custom skill + //#then: should show source tag for each custom skill expect(result).toContain("(user)") expect(result).toContain("(project)") }) @@ -76,8 +74,8 @@ describe("buildCategorySkillsDelegationGuide", () => { const result = buildCategorySkillsDelegationGuide(categories, allSkills) //#then: should not contain custom skill emphasis - expect(result).not.toContain("User-Installed Skills") - expect(result).not.toContain("HIGH PRIORITY") + expect(result).not.toContain("YOUR SKILLS") + expect(result).toContain("**Built-in**:") expect(result).toContain("Available Skills") }) @@ -88,10 +86,9 @@ describe("buildCategorySkillsDelegationGuide", () => { //#when: building the delegation guide const result = buildCategorySkillsDelegationGuide(categories, allSkills) - //#then: should show custom skills with emphasis, no builtin section - expect(result).toContain("User-Installed Skills") - expect(result).toContain("HIGH PRIORITY") - expect(result).not.toContain("Built-in Skills") + //#then: should show custom skills with emphasis, no builtin line + expect(result).toContain("YOUR SKILLS (PRIORITY)") + expect(result).not.toContain("**Built-in**:") }) it("should include priority note for custom skills in evaluation step", () => { @@ -103,7 +100,7 @@ describe("buildCategorySkillsDelegationGuide", () => { //#then: evaluation section should mention user-installed priority expect(result).toContain("User-installed skills get PRIORITY") - expect(result).toContain("INCLUDE it rather than omit it") + expect(result).toContain("INCLUDE rather than omit") }) it("should NOT include priority note when no custom skills", () => { @@ -125,6 +122,20 @@ describe("buildCategorySkillsDelegationGuide", () => { //#then: should return empty string expect(result).toBe("") }) + + it("should include category descriptions", () => { + //#given: categories with descriptions + const allSkills = [...builtinSkills] + + //#when: building the delegation guide + const result = buildCategorySkillsDelegationGuide(categories, allSkills) + + //#then: should list categories with their descriptions + expect(result).toContain("`visual-engineering`") + expect(result).toContain("Frontend, UI/UX") + expect(result).toContain("`quick`") + expect(result).toContain("Trivial tasks") + }) }) describe("buildUltraworkSection", () => { @@ -161,45 +172,4 @@ describe("buildUltraworkSection", () => { }) }) -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") - }) -}) diff --git a/src/agents/dynamic-agent-prompt-builder.ts b/src/agents/dynamic-agent-prompt-builder.ts index c192054f..ad44bc71 100644 --- a/src/agents/dynamic-agent-prompt-builder.ts +++ b/src/agents/dynamic-agent-prompt-builder.ts @@ -1,5 +1,4 @@ import type { AgentPromptMetadata } from "./types" -import { truncateDescription } from "../shared/truncate-description" export interface AvailableAgent { name: string @@ -158,29 +157,6 @@ 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 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\`. - -${customRows.join("\n")} - -> **CRITICAL**: Ignoring user-installed skills when they match the task domain is a failure. -> The user installed custom skills 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 "" @@ -193,35 +169,37 @@ export function buildCategorySkillsDelegationGuide(categories: AvailableCategory const builtinSkills = skills.filter((s) => s.location === "plugin") const customSkills = skills.filter((s) => s.location !== "plugin") - const builtinRows = builtinSkills.map((s) => { - const desc = truncateDescription(s.description) - return `- \`${s.name}\` — ${desc}` - }) - - const customRows = customSkills.map((s) => { - const desc = truncateDescription(s.description) - const source = s.location === "project" ? "project" : "user" - return `- \`${s.name}\` (${source}) — ${desc}` - }) - - const customSkillBlock = formatCustomSkillsBlock(customRows, customSkills) + const builtinNames = builtinSkills.map((s) => s.name).join(", ") + const customNames = customSkills.map((s) => { + const source = s.location === "project" ? "project" : "user" + return `${s.name} (${source})` + }).join(", ") let skillsSection: string if (customSkills.length > 0 && builtinSkills.length > 0) { - skillsSection = `#### Built-in Skills + skillsSection = `#### Available Skills (via \`skill\` tool) -${builtinRows.join("\n")} +**Built-in**: ${builtinNames} +**⚡ YOUR SKILLS (PRIORITY)**: ${customNames} -${customSkillBlock}` +> User-installed skills OVERRIDE built-in defaults. ALWAYS prefer YOUR SKILLS when domain matches. +> Full skill descriptions → use the \`skill\` tool to check before EVERY delegation.` } else if (customSkills.length > 0) { - skillsSection = customSkillBlock + skillsSection = `#### Available Skills (via \`skill\` tool) + +**⚡ YOUR SKILLS (PRIORITY)**: ${customNames} + +> User-installed skills OVERRIDE built-in defaults. ALWAYS prefer YOUR SKILLS when domain matches. +> Full skill descriptions → use the \`skill\` tool to check before EVERY delegation.` + } else if (builtinSkills.length > 0) { + skillsSection = `#### Available Skills (via \`skill\` tool) + +**Built-in**: ${builtinNames} + +> Full skill descriptions → use the \`skill\` tool to check before EVERY delegation.` } else { - skillsSection = `#### Available Skills (Domain Expertise Injection) - -Skills inject specialized instructions into the subagent. Read the description to understand when each skill applies. - -${builtinRows.join("\n")}` + skillsSection = "" } return `### Category + Skills Delegation System @@ -245,33 +223,14 @@ ${skillsSection} - Match task requirements to category domain - Select the category whose domain BEST fits the task -**STEP 2: Evaluate ALL Skills (Built-in AND User-Installed)** -For EVERY skill listed above, ask yourself: +**STEP 2: Evaluate ALL Skills** +Check the \`skill\` tool for available skills and their descriptions. For EVERY skill, ask: > "Does this skill's expertise domain overlap with my task?" - If YES → INCLUDE in \`load_skills=[...]\` -- If NO → You MUST justify why (see below) +- If NO → OMIT (no justification needed) ${customSkills.length > 0 ? ` -> **User-installed skills get PRIORITY.** The user explicitly installed them for their workflow. -> When in doubt about a user-installed skill, INCLUDE it rather than omit it.` : ""} - -**STEP 3: Justify Omissions** - -If you choose NOT to include a skill that MIGHT be relevant, you MUST provide: - -\`\`\` -SKILL EVALUATION for "[skill-name]": -- Skill domain: [what the skill description says] -- Task domain: [what your task is about] -- Decision: OMIT -- Reason: [specific explanation of why domains don't overlap] -\`\`\` - -**WHY JUSTIFICATION IS MANDATORY:** -- Forces you to actually READ skill descriptions -- Prevents lazy omission of potentially useful skills -- Subagents are STATELESS - they only know what you tell them -- Missing a relevant skill = suboptimal output +> **User-installed skills get PRIORITY.** When in doubt, INCLUDE rather than omit.` : ""} ---