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
2026-01-16 13:12:48 +09:00 · 2026-01-16 13:12:48 +09:00 · aa859f8cdd
commit aa859f8cdd
parent c282244439
4 changed files with 127 additions and 15 deletions
--- 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]"
--- 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
--- 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,
          })
        }
--- 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
 }