Skip to content

Inconsistent batch-failure handling across tools (throw vs. structured failures) #505

Description

@engfragui

Summary

Batch tools handle "some/all items failed" inconsistently. Some throw (surfacing to the caller as a Tool execution failed error), others return a structured result with a per-item failures[] array. Callers — and the LLM — can't rely on a predictable shape, and a thrown error flattens the per-item reasons into one opaque string.

The three patterns in the codebase today

A — Structured, never throws (returns failures[] even when every item fails):
complete-tasks, uncomplete-tasks, complete-goals, link-goal-tasks

npx tsx scripts/run-tool.ts complete-tasks '{"ids":["XXX","YYY"]}'
Text output:
Completed tasks: 0/2 successful.
Failed (2):
    XXX (Error: HTTP 400: Bad Request)
    YYY (Error: HTTP 400: Bad Request).

Structured output:
{
  "completed": [],
  "failures": [
    {
      "item": "XXX",
      "error": "HTTP 400: Bad Request"
    },
    {
      "item": "YYY",
      "error": "HTTP 400: Bad Request"
    }
  ],
  "totalRequested": 2,
  "successCount": 0,
  "failureCount": 2
}

B — Per-item failures[], but throws on total failure (All N … failed):
add-tasksthrow new Error('All N task(s) failed to create: …')

npx tsx scripts/run-tool.ts add-tasks '{"tasks":[
  {"content":"Task A that fails","duration":"banana"},
  {"content":"Task B that fails","duration":"99h"}
]}'
Tool execution failed: Error: All 2 task(s) failed to create: "Task A that fails": Task "Task A that fails": Invalid duration format "banana": Use format like "2h", "30m", "2h30m", or "1.5h"; "Task B that fails": Task "Task B that fails": Invalid duration format "99h": Duration cannot exceed 24 hours (1440 minutes)
    at Object.execute (/Users/francesca/conductor/workspaces/todoist-mcp/adelaide-v1/src/tools/add-tasks.ts:146:19)
    at async main (/Users/francesca/conductor/workspaces/todoist-mcp/adelaide-v1/scripts/run-tool.ts:209:24)

C — All-or-nothing Promise.all (throws on any item failure, no failures[] at all):
add-sections, add-projects, add-goals, add-labels, add-comments, add-reminders, update-sections, update-projects, update-goals, update-labels, update-comments, update-reminders, update-tasks

npx tsx scripts/run-tool.ts update-tasks '{"tasks":[
  {"id":"XXX","projectId":"ZZZ"},
  {"id":"YYY","projectId":"ZZZ"}
]}'
Tool execution failed: Error
    at request (file:///Users/francesca/conductor/workspaces/todoist-mcp/adelaide-v1/node_modules/@doist/todoist-sdk/dist/esm/transport/http-client.js:68:27)
    at TaskClient.moveTask (file:///Users/francesca/conductor/workspaces/todoist-mcp/adelaide-v1/node_modules/@doist/todoist-sdk/dist/esm/clients/task-client.js:203:32)
    at TodoistApi.moveTask (file:///Users/francesca/conductor/workspaces/todoist-mcp/adelaide-v1/node_modules/@doist/todoist-sdk/dist/esm/todoist-api.js:231:32)
    at <anonymous> (/Users/francesca/conductor/workspaces/todoist-mcp/adelaide-v1/src/tools/update-tasks.ts:220:44)
    at async Promise.all (index 1)
    at async Object.execute (/Users/francesca/conductor/workspaces/todoist-mcp/adelaide-v1/src/tools/update-tasks.ts:228:31)
    at async main (/Users/francesca/conductor/workspaces/todoist-mcp/adelaide-v1/scripts/run-tool.ts:209:24) {
  httpStatusCode: 400,
  responseData: {
    error: 'Invalid argument value',
    error_code: 20,
    error_extra: {
      argument: 'task_id',
      event_id: '4da4ddc97e6b4e7aae6a7da82f5c692e',
      expected: 'Value error, Incorrect padding',
      retry_after: 9
    },
    error_tag: 'INVALID_ARGUMENT_VALUE',
    http_code: 400
  },
  isAuthenticationError: [Function (anonymous)]
}

Plus two outliers: manage-assignments (atomic rollback, throws All N … failed) and reschedule-tasks (single sync batch — the SDK throws on the first failing command, discarding any commands that already applied).

Why it matters

  • Predictability: for some tools a partial/total failure is an inspectable payload; for others it's a thrown error. Callers must special-case per tool.
  • LLM actionability: a structured "0 succeeded, N failed, here's why each" is far more useful than one opaque Tool execution failed.
  • Lost information: the throw path collapses individual reasons into a single string and discards any partial successes.

Context

PR #502 (update-tasks) moves that tool from Pattern C to Pattern A (which seems to be the one that makes the most sense to me) — settle each item, always return failures[], never throw (total failures included). Before aligning everything else, we should agree on the canonical pattern.

Decision needed

  1. Which pattern is canonical for batch write tools? (Leaning A: always return structured, never throw on per-item/total failure.)
  2. If A: align add-tasks (drop the total-failure throw) and bring the Pattern-C tools up to failures[].
  3. Scope question: should single-object tools (delete-object, project-move, project-management) and the atomic/sync ones (manage-assignments, reschedule-tasks) also avoid throwing, or is throwing acceptable for non-batch / all-or-nothing-by-design operations?

Related

Metadata

Metadata

Labels

enhancementNew feature or requestquestionFurther information is requested

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions