feat: support multi-status filtering and includeAllStatuses for task listing

[devondragon]

Summary

• Add includeAllStatuses boolean parameter to the motion_tasks list operation, passing through to the Motion API to return tasks across all statuses (including completed/resolved) in one call
• Update status parameter to accept either a single string or an array of strings, enabling multi-status filtering (e.g., ["Todo", "Completed"])
• Add validation that status and includeAllStatuses cannot be combined (per API constraints)
• Improve status field description to clarify default filtering behavior and reference motion_statuses tool

Closes #68

Test plan

• npm run build passes cleanly
• Server starts successfully with timeout 3s npm run mcp
• List tasks with includeAllStatuses: true returns completed tasks
• List tasks with status: ["Todo", "Completed"] returns tasks matching either status
• List tasks with single status: "Todo" still works (backward compatible)
• Combining status and includeAllStatuses returns a validation error
• List tasks with no status filter still returns only active tasks (unchanged default)

[claude]

PR Review: feat: support multi-status filtering and includeAllStatuses for task listing

Overall this is a clean, well-scoped addition. The core logic is sound and the backward-compatibility story is good. I found a few issues worth addressing before merge.

---

Bugs / Issues

1. MotionTasksArgs type not updated (medium severity)

src/types/mcp-tool-args.ts still has status?: string and no includeAllStatuses field:

// src/types/mcp-tool-args.ts — unchanged by this PR
export interface MotionTasksArgs {
  // ...
  status?: string;          // still string, not string | string[]
  // includeAllStatuses missing entirely
}

The handleList cast params as ListTaskParams masks this at the call site, but TypeScript type safety is compromised. An LLM passing an array for status via the MCP protocol would have it land in MotionTasksArgs typed as string, silently mistyped before the cast. The fix is to update MotionTasksArgs to match the new shape (at minimum for the list operation).

2. Empty-array edge case in conflict validation (minor)

In TaskHandler.ts:

if (params.includeAllStatuses && params.status) {

An empty array [] is truthy in JavaScript, so status: [] combined with includeAllStatuses: true would incorrectly trigger the validation error even though an empty array means "no status filter." A more defensive check:

const hasStatusFilter = Array.isArray(params.status) ? params.status.length > 0 : !!params.status;
if (params.includeAllStatuses && hasStatusFilter) { ... }

The same edge case affects the statusDisplay ternary — [].join(', ') produces "" which would display as an empty status label.

3. Missing per-item validation for status array (minor)

The existing labels parameter validates each item:

// src/handlers/TaskHandler.ts:269
if (params.labels && (!Array.isArray(params.labels) || params.labels.some(label => !label || typeof label !== 'string'))) {

No equivalent guard exists for status when it is an array. An input like status: ["", "Todo"] would silently send an empty string to the Motion API. Consider adding:

if (Array.isArray(params.status) && params.status.some(s => !s || typeof s !== 'string')) {
  return this.handleError(new Error('status array must contain non-empty strings'));
}

---

Questions / Uncertainty

Motion API support for repeated status params — the implementation appends status multiple times to URLSearchParams (the correct pattern for array query params), but neither the PR description nor any code comment confirms the Motion API actually accepts status as a multi-value parameter. If the API only accepts a single value and silently picks one, multi-status filtering would appear to work while quietly dropping values. Worth verifying against the Motion API docs and adding a comment.

---

What looks good

• The oneOf schema definition is consistent with how duration is already handled — no new patterns introduced.
• URLSearchParams.append() in a loop is the correct approach for repeated query-string keys.
• The statusDisplay ternary covers all three cases (includeAllStatuses / array / string) cleanly.
• The guard against combining status and includeAllStatuses is the right thing to do and the error message is clear.
• Backward compatibility is preserved — a single string status value still works exactly as before.
• The test plan checklist covers the main scenarios well.

---

Summary

The critical fix before merge is updating MotionTasksArgs in mcp-tool-args.ts to add includeAllStatuses and widen status to string | string[]. The empty-array edge case and missing array-item validation are lower priority but cheap to fix. Confirming Motion API support for multi-value status params would give confidence the feature actually works end-to-end.

[Copilot]

Pull request overview

This PR adds support for multi-status filtering and the includeAllStatuses parameter to the motion_tasks list operation, addressing issue #68. The changes enable users to retrieve tasks across multiple statuses (e.g., both "Todo" and "Completed") or all statuses in a single API call, which was previously not possible.

Changes:

• Added includeAllStatuses boolean parameter to list tasks across all statuses including completed/resolved tasks
• Updated status parameter to accept either a single string or an array of strings for multi-status filtering
• Added validation to prevent combining status and includeAllStatuses parameters per API constraints

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
src/tools/ToolDefinitions.ts	Updated tool schema to support status as string or array using oneOf, and added includeAllStatuses boolean parameter
src/services/motionApi.ts	Modified GetTasksOptions interface and getTasks method to handle array status values and includeAllStatuses parameter
src/handlers/TaskHandler.ts	Updated ListTaskParams interface, added validation for parameter conflicts, and enhanced status display formatting

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The validation incorrectly rejects the combination of includeAllStatuses with an empty status array. An empty array is semantically equivalent to no status filter and should not conflict with includeAllStatuses. The condition should check for non-empty values: if (params.includeAllStatuses && params.status && (typeof params.status === 'string' || params.status.length > 0))

[Copilot]

Missing validation for status array elements. When status is an array, each element should be validated to ensure it's a non-empty string (similar to the labels validation on line 269). Without this, empty strings or non-string values in the array could be passed to the API. Add validation before the includeAllStatuses check: if (Array.isArray(params.status) && params.status.some(s => !s || typeof s !== 'string')) { return this.handleError(new Error('Status array must contain only non-empty strings')); }

Suggested change

	// Validate status array elements if provided
	if (Array.isArray(params.status) && params.status.some(s => !s || typeof s !== 'string')) {
	return this.handleError(new Error('Status array must contain only non-empty strings'));
	}

[Copilot]

The new multi-status filtering and includeAllStatuses functionality lacks test coverage. Based on the test plan in the PR description, there should be tests verifying: 1) status as an array works correctly, 2) includeAllStatuses parameter works, 3) combining status and includeAllStatuses returns a validation error, and 4) backward compatibility with single status strings. Consider adding tests for these scenarios in tests/handlers.task.spec.ts.

[Copilot]

The statusDisplay logic doesn't handle empty arrays correctly. If params.status is an empty array, params.status.join(', ') returns an empty string, which would result in the display text showing with status "". Consider adding a check: Array.isArray(params.status) && params.status.length > 0 ? params.status.join(', ') : params.status

Suggested change

	: Array.isArray(params.status)
	: Array.isArray(params.status) && params.status.length > 0

[Copilot]

Empty strings in the status array are not filtered before appending to query parameters. Unlike the labels handling (lines 628-629) which checks if (label) before appending, status values are appended without validation. If an empty string exists in the status array, it will be sent to the API as status=. Consider adding validation: if (s) { params.append('status', s); }

Suggested change

	params.append('status', s);
	}
	} else {
	if (s) {
	params.append('status', s);
	}
	}
	} else if (status) {

[Copilot]

The implementation doesn't check for or filter duplicate status values in the array. While not necessarily wrong (the API may handle duplicates gracefully), it could result in redundant query parameters like status=Todo&status=Todo. Consider deduplicating the array before iteration for cleaner API calls, though this is a minor optimization.

Suggested change

	for (const s of status) {
	const uniqueStatuses = Array.from(new Set(status));
	for (const s of uniqueStatuses) {

[claude]

PR Review: feat: support multi-status filtering and includeAllStatuses for task listing

The prior claude review was written against an earlier version of this PR. The current diff has already resolved all three issues it raised:

• mcp-tool-args.ts IS updated — status is widened to string | string[] and includeAllStatuses?: boolean is added.
• Empty-array edge case IS handled — the hasStatusFilter guard uses params.status.length > 0 rather than raw truthiness.
• Per-item array validation IS present — params.status.some(s => !s || typeof s !== 'string') catches empty/non-string elements.

Good to see those addressed. Here's what remains.

---

One real concern: Motion API multi-value status support

The implementation appends status multiple times via URLSearchParams.append():

for (const s of uniqueStatuses) {
  if (s) {
    params.append('status', s);
  }
}

This produces ?status=Todo&status=Completed, which is the standard HTTP convention for repeated query parameters. However, neither the PR description nor any code comment confirms the Motion API actually accepts status as a multi-value parameter. If the API only honours the first (or last) occurrence, multi-status filtering would silently return incomplete results.

Recommendation: Verify against Motion API docs and add a comment like:

// Motion API accepts repeated status= params for multi-value filtering (confirmed in docs)

If the API does NOT support this, the feature needs a different implementation approach (e.g., multiple serial requests merged client-side).

---

Minor: empty array is truthy in the service layer

In motionApi.ts:

if (status) {         // ← truthy check; empty array [] passes this
  if (Array.isArray(status)) {
    const uniqueStatuses = Array.from(new Set(status));
    for (const s of uniqueStatuses) { ... }   // ← loop runs 0 times for []
  }
}

The behaviour is correct (nothing gets appended for []), but the guard is misleading. The handler validates before this point, so an empty array won't realistically arrive here, but a more explicit guard would be clearer:

if (status && (!Array.isArray(status) || status.length > 0)) {

Low priority — doesn't affect correctness given handler validation.

---

What looks good

• All three issues from the prior review are addressed.
• CreateTaskParams getting status?: string | string[] with an early-return guard for the array case is correct and preserves type consistency.
• The statusDisplay ternary correctly handles all four states: includeAllStatuses, non-empty array, single string, and undefined.
• Deduplication of status values via Set before appending is a nice defensive touch.
• Backward compatibility is fully preserved — single string status works exactly as before.
• The improved status description referencing motion_statuses is a helpful discoverability aid.

---

Summary

The main thing blocking confidence in this PR is confirmation that the Motion API supports repeated status= query parameters. If that's verified, this is in good shape to merge. The empty-array truthy issue in the service layer is cosmetic.

[devondragon]

Verified: Motion API supports repeated status= query parameters

Tested directly against the live Motion API to confirm multi-status filtering works with the repeated parameter approach used in this PR.

Test Results

Test	Query	Task Count	Statuses Returned
Single status=Todo	baseline	35	Todo: 35
Single status=In Progress	baseline	1	In Progress: 1
Repeated status=Todo&status=In Progress	PR approach	36	Todo: 35, In Progress: 1
Three statuses Todo + In Progress + Backlog	PR approach	36	Todo: 35, In Progress: 1 (0 Backlog tasks exist)
Comma-separated status=Todo,In Progress	alternative	0	No match

Conclusions

• Repeated status= params work correctly — the API returns the union of tasks matching any provided status (35 + 1 = 36, exactly matching individual baselines).
• Comma-separated values do NOT work — returns 0 results, confirming URLSearchParams.append() in a loop is the correct approach.
• The status=Todo&status=In%20Progress pattern is the right way to do multi-status filtering with the Motion API.