From afd554b2d9135322ccae9a3c414db8b71a9b70d9 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sun, 5 Apr 2026 15:46:24 +0900 Subject: [PATCH] fix(delegate-task): validate description parameter and handle undefined in notifications OpenCode's fromPlugin wrapper skips Zod validation for plugin tools, so LLMs can omit required args like description without getting an error. When Atlas orchestrates and the model omits description, it flows through as undefined to manager.launch() and background task notifications show 'undefined' for all completed tasks. Two fixes: - Add runtime validation for description in delegate-task tool (matches existing run_in_background and load_skills validation pattern) - Defensive fallback in notification template: use task ID when description is missing instead of rendering 'undefined' --- ...kground-task-notification-template.test.ts | 48 +++++++++++++++++++ .../background-task-notification-template.ts | 9 ++-- src/tools/delegate-task/tools.test.ts | 29 +++++++++++ src/tools/delegate-task/tools.ts | 3 ++ 4 files changed, 85 insertions(+), 4 deletions(-) diff --git a/src/features/background-agent/background-task-notification-template.test.ts b/src/features/background-agent/background-task-notification-template.test.ts index 5555e909a..37b416570 100644 --- a/src/features/background-agent/background-task-notification-template.test.ts +++ b/src/features/background-agent/background-task-notification-template.test.ts @@ -127,4 +127,52 @@ Use \`background_output(task_id="")\` to retrieve each result. expect(notification).toBe(expectedNotification) }) }) + + describe("#given all tasks completed with undefined descriptions", () => { + test("#when building the final notification #then it uses task ID as fallback instead of 'undefined'", () => { + // given + const notification = buildBackgroundTaskNotificationText({ + task: { + id: "bg_abc123", + description: undefined as unknown as string, + status: "completed", + }, + duration: "5s", + statusText: "COMPLETED", + allComplete: true, + remainingCount: 0, + completedTasks: [ + { id: "bg_abc123", description: undefined as unknown as string, status: "completed" }, + { id: "bg_def456", description: undefined as unknown as string, status: "completed" }, + ], + }) + + // then + expect(notification).not.toContain(": undefined") + expect(notification).toContain("bg_abc123") + expect(notification).toContain("bg_def456") + }) + }) + + describe("#given a single task notification with undefined description", () => { + test("#when building the partial notification #then it uses task ID as fallback", () => { + // given + const notification = buildBackgroundTaskNotificationText({ + task: { + id: "bg_xyz789", + description: undefined as unknown as string, + status: "completed", + }, + duration: "3s", + statusText: "COMPLETED", + allComplete: false, + remainingCount: 2, + completedTasks: [], + }) + + // then + expect(notification).not.toContain("undefined") + expect(notification).toContain("bg_xyz789") + }) + }) }) diff --git a/src/features/background-agent/background-task-notification-template.ts b/src/features/background-agent/background-task-notification-template.ts index 240efe9c0..ad6769fac 100644 --- a/src/features/background-agent/background-task-notification-template.ts +++ b/src/features/background-agent/background-task-notification-template.ts @@ -19,6 +19,7 @@ export function buildBackgroundTaskNotificationText(input: { }): string { const { task, duration, statusText, allComplete, remainingCount, completedTasks } = input + const safeDescription = (t: BackgroundTaskNotificationTask): string => t.description || t.id const errorInfo = task.error ? `\n**Error:** ${task.error}` : "" if (allComplete) { @@ -26,10 +27,10 @@ export function buildBackgroundTaskNotificationText(input: { const failedTasks = completedTasks.filter((t) => t.status !== "completed") const succeededText = succeededTasks.length > 0 - ? succeededTasks.map((t) => `- \`${t.id}\`: ${t.description}`).join("\n") + ? succeededTasks.map((t) => `- \`${t.id}\`: ${safeDescription(t)}`).join("\n") : "" const failedText = failedTasks.length > 0 - ? failedTasks.map((t) => `- \`${t.id}\`: ${t.description} [${t.status.toUpperCase()}]${t.error ? ` - ${t.error}` : ""}`).join("\n") + ? failedTasks.map((t) => `- \`${t.id}\`: ${safeDescription(t)} [${t.status.toUpperCase()}]${t.error ? ` - ${t.error}` : ""}`).join("\n") : "" const hasFailures = failedTasks.length > 0 @@ -45,7 +46,7 @@ export function buildBackgroundTaskNotificationText(input: { body += `\n**Failed:**\n${failedText}\n` } if (!body) { - body = `- \`${task.id}\`: ${task.description} [${task.status.toUpperCase()}]${task.error ? ` - ${task.error}` : ""}\n` + body = `- \`${task.id}\`: ${safeDescription(task)} [${task.status.toUpperCase()}]${task.error ? ` - ${task.error}` : ""}\n` } return ` @@ -62,7 +63,7 @@ Use \`background_output(task_id="")\` to retrieve each result.${hasFailures return ` [BACKGROUND TASK ${statusText}] **ID:** \`${task.id}\` -**Description:** ${task.description} +**Description:** ${safeDescription(task)} **Duration:** ${duration}${errorInfo} **${remainingCount} task${remainingCount === 1 ? "" : "s"} still in progress.** You WILL be notified when ALL complete. diff --git a/src/tools/delegate-task/tools.test.ts b/src/tools/delegate-task/tools.test.ts index ac2fc1b25..33e338c1b 100644 --- a/src/tools/delegate-task/tools.test.ts +++ b/src/tools/delegate-task/tools.test.ts @@ -1366,6 +1366,35 @@ describe("sisyphus-task", () => { )).rejects.toThrow("Invalid arguments: 'run_in_background' parameter is REQUIRED") }) + test("#given category without description #when executing #then throws required parameter error", async () => { + // given + const { createDelegateTask } = require("./tools") + const mockManager = { launch: async () => ({}) } + const mockClient = { + app: { agents: async () => ({ data: [] }) }, + config: { get: async () => ({ data: { model: SYSTEM_DEFAULT_MODEL } }) }, + session: { + create: async () => ({ data: { id: "test-session" } }), + prompt: async () => ({ data: {} }), + promptAsync: async () => ({ data: {} }), + messages: async () => ({ data: [] }), + }, + } + const tool = createDelegateTask({ manager: mockManager, client: mockClient }) + + // when + // then + await expect(tool.execute( + { + prompt: "Do something", + category: "quick", + run_in_background: false, + load_skills: [], + }, + { sessionID: "parent-session", messageID: "parent-message", agent: "sisyphus", abort: new AbortController().signal } + )).rejects.toThrow("Invalid arguments: 'description' parameter is REQUIRED") + }) + test("#given explicit run_in_background=false #when executing #then sync execution succeeds", async () => { // given const { createDelegateTask } = require("./tools") diff --git a/src/tools/delegate-task/tools.ts b/src/tools/delegate-task/tools.ts index c149f8025..46822bfcd 100644 --- a/src/tools/delegate-task/tools.ts +++ b/src/tools/delegate-task/tools.ts @@ -122,6 +122,9 @@ export function createDelegateTask(options: DelegateTaskToolOptions): ToolDefini title: args.description, }) + if (!args.description || typeof args.description !== "string") { + throw new Error(`Invalid arguments: 'description' parameter is REQUIRED. Provide a short (3-5 words) task description.`) + } if (args.run_in_background === undefined) { throw new Error(`Invalid arguments: 'run_in_background' parameter is REQUIRED. Specify run_in_background=false for task delegation, or run_in_background=true for parallel exploration.`) }