From aa859f8cdddbe91b30c2be9e8b23a4d01738db79 Mon Sep 17 00:00:00 2001 From: justsisyphus Date: Fri, 16 Jan 2026 13:12:48 +0900 Subject: [PATCH] feat(sisyphus-task): require explicit skills parameter - reject empty array [] - Change skills type from string[] to string[] | null - Empty array [] now returns error with available skills list - null is allowed for tasks that genuinely need no skills - Updated tests to use skills: null instead of skills: [] - Forces explicit decision: either specify skills or justify with null --- src/tools/sisyphus-task/constants.ts | 2 +- src/tools/sisyphus-task/tools.test.ts | 116 ++++++++++++++++++++++++-- src/tools/sisyphus-task/tools.ts | 22 +++-- src/tools/sisyphus-task/types.ts | 2 +- 4 files changed, 127 insertions(+), 15 deletions(-) diff --git a/src/tools/sisyphus-task/constants.ts b/src/tools/sisyphus-task/constants.ts index 4919b655..f84e344a 100644 --- a/src/tools/sisyphus-task/constants.ts +++ b/src/tools/sisyphus-task/constants.ts @@ -244,7 +244,7 @@ MUTUALLY EXCLUSIVE: Provide EITHER category OR agent, not both (unless resuming) - agent: Use specific agent directly (e.g., "oracle", "explore") - background: true=async (returns task_id), false=sync (waits for result). Default: false. Use background=true ONLY for parallel exploration with 5+ independent queries. - resume: Session ID to resume (from previous task output). Continues agent with FULL CONTEXT PRESERVED - saves tokens, maintains continuity. -- skills: Array of skill names to prepend to prompt (e.g., ["playwright", "frontend-ui-ux"]). Skills will be resolved and their content prepended with a separator. Empty array = no prepending. +- skills: Array of skill names to prepend to prompt (e.g., ["playwright", "frontend-ui-ux"]). Skills will be resolved and their content prepended with a separator. Empty array [] is NOT allowed - use null if no skills needed. **WHEN TO USE resume:** - Task failed/incomplete → resume with "fix: [specific issue]" diff --git a/src/tools/sisyphus-task/tools.test.ts b/src/tools/sisyphus-task/tools.test.ts index bf18c3b6..fd208d4a 100644 --- a/src/tools/sisyphus-task/tools.test.ts +++ b/src/tools/sisyphus-task/tools.test.ts @@ -305,7 +305,7 @@ describe("sisyphus-task", () => { prompt: "Do something", category: "ultrabrain", run_in_background: true, - skills: [], + skills: null, }, toolContext ) @@ -320,10 +320,12 @@ describe("sisyphus-task", () => { }) describe("skills parameter", () => { - test("SISYPHUS_TASK_DESCRIPTION documents skills parameter", () => { + test("SISYPHUS_TASK_DESCRIPTION documents skills parameter with null option", () => { // #given / #when / #then expect(SISYPHUS_TASK_DESCRIPTION).toContain("skills") expect(SISYPHUS_TASK_DESCRIPTION).toContain("Array of skill names") + expect(SISYPHUS_TASK_DESCRIPTION).toContain("Empty array [] is NOT allowed") + expect(SISYPHUS_TASK_DESCRIPTION).toContain("null if no skills needed") }) test("skills parameter is required - returns error when not provided", async () => { @@ -368,6 +370,104 @@ describe("sisyphus-task", () => { expect(result).toContain("skills") expect(result).toContain("REQUIRED") }) + + test("empty array [] returns error with available skills list", async () => { + // #given + const { createSisyphusTask } = require("./tools") + + const mockManager = { launch: async () => ({}) } + const mockClient = { + app: { agents: async () => ({ data: [] }) }, + config: { get: async () => ({}) }, + session: { + create: async () => ({ data: { id: "test-session" } }), + prompt: async () => ({ data: {} }), + messages: async () => ({ data: [] }), + }, + } + + const tool = createSisyphusTask({ + manager: mockManager, + client: mockClient, + }) + + const toolContext = { + sessionID: "parent-session", + messageID: "parent-message", + agent: "Sisyphus", + abort: new AbortController().signal, + } + + // #when - empty array passed + const result = await tool.execute( + { + description: "Test task", + prompt: "Do something", + category: "ultrabrain", + run_in_background: false, + skills: [], + }, + toolContext + ) + + // #then - should return error about empty array with guidance + expect(result).toContain("❌") + expect(result).toContain("Empty array []") + expect(result).toContain("not allowed") + expect(result).toContain("skills=null") + }) + + test("null skills is allowed and proceeds without skill content", async () => { + // #given + const { createSisyphusTask } = require("./tools") + let promptBody: any + + const mockManager = { launch: async () => ({}) } + const mockClient = { + app: { agents: async () => ({ data: [] }) }, + config: { get: async () => ({}) }, + session: { + get: async () => ({ data: { directory: "/project" } }), + create: async () => ({ data: { id: "test-session" } }), + prompt: async (input: any) => { + promptBody = input.body + return { data: {} } + }, + messages: async () => ({ + data: [{ info: { role: "assistant" }, parts: [{ type: "text", text: "Done" }] }] + }), + status: async () => ({ data: {} }), + }, + } + + const tool = createSisyphusTask({ + manager: mockManager, + client: mockClient, + }) + + const toolContext = { + sessionID: "parent-session", + messageID: "parent-message", + agent: "Sisyphus", + abort: new AbortController().signal, + } + + // #when - null skills passed + await tool.execute( + { + description: "Test task", + prompt: "Do something", + category: "ultrabrain", + run_in_background: false, + skills: null, + }, + toolContext + ) + + // #then - should proceed without system content from skills + expect(promptBody).toBeDefined() + // system should not contain skill content (only category prompt append if any) + }, { timeout: 20000 }) }) describe("resume with background parameter", () => { @@ -426,7 +526,7 @@ describe("sisyphus-task", () => { prompt: "Continue the task", resume: "ses_resume_test", run_in_background: false, - skills: [], + skills: null, }, toolContext ) @@ -481,7 +581,7 @@ describe("sisyphus-task", () => { prompt: "Continue in background", resume: "ses_bg_resume", run_in_background: true, - skills: [], + skills: null, }, toolContext ) @@ -536,7 +636,7 @@ describe("sisyphus-task", () => { prompt: "Do something", category: "ultrabrain", run_in_background: false, - skills: [], + skills: null, }, toolContext ) @@ -597,7 +697,7 @@ describe("sisyphus-task", () => { prompt: "Do something", category: "ultrabrain", run_in_background: false, - skills: [], + skills: null, }, toolContext ) @@ -650,7 +750,7 @@ describe("sisyphus-task", () => { prompt: "Do something", category: "ultrabrain", run_in_background: false, - skills: [], + skills: null, }, toolContext ) @@ -705,7 +805,7 @@ describe("sisyphus-task", () => { prompt: "test", category: "custom-cat", run_in_background: false, - skills: [] + skills: null }, toolContext) // #then diff --git a/src/tools/sisyphus-task/tools.ts b/src/tools/sisyphus-task/tools.ts index e6ee744f..a1e1730f 100644 --- a/src/tools/sisyphus-task/tools.ts +++ b/src/tools/sisyphus-task/tools.ts @@ -182,7 +182,7 @@ export function createSisyphusTask(options: SisyphusTaskToolOptions): ToolDefini subagent_type: tool.schema.string().optional().describe("Agent name directly (e.g., 'oracle', 'explore'). Mutually exclusive with category."), run_in_background: tool.schema.boolean().describe("Run in background. MUST be explicitly set. Use false for task delegation, true only for parallel exploration."), resume: tool.schema.string().optional().describe("Session ID to resume - continues previous agent session with full context"), - skills: tool.schema.array(tool.schema.string()).describe("Array of skill names to prepend to the prompt. Use [] if no skills needed."), + skills: tool.schema.array(tool.schema.string()).nullable().describe("Array of skill names to prepend to the prompt. Use null if no skills needed. Empty array [] is NOT allowed."), }, async execute(args: SisyphusTaskArgs, toolContext) { const ctx = toolContext as ToolContextWithMetadata @@ -190,12 +190,24 @@ export function createSisyphusTask(options: SisyphusTaskToolOptions): ToolDefini return `❌ Invalid arguments: 'run_in_background' parameter is REQUIRED. Use run_in_background=false for task delegation, run_in_background=true only for parallel exploration.` } if (args.skills === undefined) { - return `❌ Invalid arguments: 'skills' parameter is REQUIRED. Use skills=[] if no skills needed.` + return `❌ Invalid arguments: 'skills' parameter is REQUIRED. Use skills=null if no skills are needed, or provide an array of skill names.` + } + if (Array.isArray(args.skills) && args.skills.length === 0) { + const allSkills = await discoverSkills({ includeClaudeCodePaths: true }) + const availableSkillsList = allSkills.map(s => ` - ${s.name}`).slice(0, 15).join("\n") + return `❌ Invalid arguments: Empty array [] is not allowed for 'skills' parameter. + +Use skills=null if this task genuinely requires no specialized skills. +Otherwise, select appropriate skills from available options: + +${availableSkillsList}${allSkills.length > 15 ? `\n ... and ${allSkills.length - 15} more` : ""} + +If you believe no skills are needed, you MUST explicitly explain why to the user before using skills=null.` } const runInBackground = args.run_in_background === true let skillContent: string | undefined - if (args.skills.length > 0) { + if (args.skills !== null && args.skills.length > 0) { const { resolved, notFound } = await resolveMultipleSkillsAsync(args.skills, { gitMasterConfig }) if (notFound.length > 0) { const allSkills = await discoverSkills({ includeClaudeCodePaths: true }) @@ -515,7 +527,7 @@ ${textContent || "(No text output)"}` parentModel, parentAgent, model: categoryModel, - skills: args.skills, + skills: args.skills ?? undefined, skillContent: systemContent, }) @@ -579,7 +591,7 @@ System notifies on completion. Use \`background_output\` with task_id="${task.id description: args.description, agent: agentToUse, isBackground: false, - skills: args.skills, + skills: args.skills ?? undefined, modelInfo, }) } diff --git a/src/tools/sisyphus-task/types.ts b/src/tools/sisyphus-task/types.ts index f60bbece..90ba2485 100644 --- a/src/tools/sisyphus-task/types.ts +++ b/src/tools/sisyphus-task/types.ts @@ -5,5 +5,5 @@ export interface SisyphusTaskArgs { subagent_type?: string run_in_background: boolean resume?: string - skills: string[] + skills: string[] | null }