fix(tasks): add assignee filtering to list_all_uncompleted

[devondragon]

Summary

• Add assignee filtering to list_all_uncompleted: The operation previously silently ignored assignee/assigneeId params, so cross-workspace queries like "show me my tasks" returned everyone's uncompleted tasks. Now supports the same assignee: 'me' shortcut and name resolution as the list operation, with server-side filtering via the Motion API.
• Fix pre-existing TypeScript build error: Remove invalid quiet option from dotenv.config() call in mcp-server.ts (not a valid DotenvConfigOptions property).

Closes #59

Changes

• src/handlers/TaskHandler.ts — Add assigneeId/assignee to ListAllUncompletedParams; add assignee resolution logic to handleListAllUncompleted (including 'me' shortcut and cross-workspace name lookup); pass resolved assignee to API and response formatter
• src/services/motionApi.ts — Add optional assigneeId param to getAllUncompletedTasks and pass it through to per-workspace getTasks() calls
• src/tools/ToolDefinitions.ts — Update assigneeId/assignee parameter descriptions to indicate they work with both list and list_all_uncompleted
• src/mcp-server.ts — Remove invalid { quiet: true } from dotenv.config()

Test plan

• npm run build compiles with zero errors
• timeout 3s npm run mcp starts cleanly
• motion_tasks with operation: 'list_all_uncompleted' and assignee: 'me' returns only the current user's uncompleted tasks
• motion_tasks with operation: 'list_all_uncompleted' without assignee params still returns all uncompleted tasks (no regression)

[claude]

Code Review

Summary

This PR correctly fixes a real usability bug (assignee filtering silently ignored on list_all_uncompleted) and removes an invalid dotenv option. The logic is sound, but there are some issues worth addressing.

---

Issues

1. Code Duplication — Medium severity

The assignee resolution block in handleListAllUncompleted is nearly identical to the one already in handleList (lines 265–294 of TaskHandler.ts). Both methods independently define the same two nested functions:

// Duplicated in both handleList and handleListAllUncompleted:
const normalizeDisplayFromUser = (user: { name?: string; email?: string; id: string }) => {
  return user.name || user.email || user.id;
};

const resolveCurrentUser = async () => {
  const currentUser = await this.motionService.getCurrentUser();
  resolvedAssigneeId = currentUser.id;
  assigneeDisplay = normalizeDisplayFromUser(currentUser);
};

This violates DRY and means future changes to the resolution logic need to be applied in two places. Both helpers (and the ~20 lines of if/else branching that follow) should be extracted to private methods on TaskHandler. For example:

private normalizeDisplayFromUser(user: { name?: string; email?: string; id: string }): string {
  return user.name || user.email || user.id;
}

private async resolveAssigneeParams(
  assigneeId?: string,
  assignee?: string,
  workspaceIds?: string[]
): Promise<{ resolvedAssigneeId?: string; assigneeDisplay?: string }> {
  // shared resolution logic
}

2. Mutation via closure — Low severity

The resolveCurrentUser closure mutates outer let variables by side effect rather than returning values. This works but is an unusual pattern in TypeScript that makes the control flow harder to follow. Returning the resolved values and assigning them at the call site would be cleaner and more testable.

3. assigneeId: 'me' semantically incorrect — Low severity (pre-existing)

Both handleList and this new code accept assigneeId: 'me'. Since assigneeId is documented as an ID field, accepting the string 'me' there is semantically surprising. This is pre-existing behavior so it's not a regression, but worth a comment in the code or schema description clarifying the special case.

---

What's correct

• Cross-workspace user resolution is the right approach for list_all_uncompleted — searching all workspaces is more appropriate here than in handleList (which is scoped to one workspace). The error message is now accurate.
• dotenv.config() fix is correct. quiet is not a valid DotenvConfigOptions property in the dotenv type definitions, so this was a real build error, not just a style choice.
• getAllUncompletedTasks API change is minimal and clean — adding the optional assigneeId param and threading it through to getTasks() is the right place for this.
• ToolDefinitions.ts update is accurate and helpful.
• The limit/adaptive pagination interaction is unchanged and still works correctly; server-side filtering by assignee should reduce the data volume returned per workspace.

---

Minor notes

• assigneeDisplay || resolvedAssigneeId when both are undefined passes undefined to formatTaskList's assigneeName. That is fine if the formatter handles optional fields (which it appears to), but worth a quick check.
• No test coverage exists (per CLAUDE.md), so manual testing per the PR test plan is critical here. The four scenarios in the test plan are the right ones to verify.

---

Overall this is a good fix for a real bug. The main ask would be to extract the shared assignee resolution logic rather than duplicating it — the rest of the changes look solid.

[Copilot]

Pull request overview

This PR adds assignee filtering support to the list_all_uncompleted operation and fixes a TypeScript build error related to dotenv configuration. The operation previously did not support assignee filtering, which meant cross-workspace queries returned all users' uncompleted tasks. The fix brings feature parity with the list operation by implementing the same assignee resolution logic, including support for the 'me' shortcut and name-based user lookup across workspaces.

Changes:

• Add assignee filtering to list_all_uncompleted operation with support for assigneeId and assignee parameters, including 'me' shortcut and cross-workspace name resolution
• Fix TypeScript build error by removing invalid quiet option from dotenv.config() call
• Update parameter descriptions in tool definitions to reflect that assignee filtering works with both list and list_all_uncompleted operations

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/handlers/TaskHandler.ts	Added assigneeId/assignee parameters to ListAllUncompletedParams interface; implemented assignee resolution logic with 'me' shortcut and cross-workspace user lookup; passed resolved assignee to API calls and response formatter
src/services/motionApi.ts	Added optional assigneeId parameter to getAllUncompletedTasks method and passed it through to per-workspace getTasks calls
src/tools/ToolDefinitions.ts	Updated assigneeId and assignee parameter descriptions to indicate they work with both list and list_all_uncompleted operations
src/mcp-server.ts	Removed invalid { quiet: true } option from dotenv.config() call to fix TypeScript build error

---

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

[Copilot]

The new assignee filtering functionality for list_all_uncompleted lacks test coverage. Consider adding tests that verify: (1) assignee filtering with 'me' shortcut works correctly, (2) assignee name resolution across workspaces functions as expected, (3) the assigneeId parameter is properly passed to getAllUncompletedTasks, and (4) appropriate error handling when an assignee is not found. The existing tests in handlers.task.spec.ts provide examples of how to structure these tests.

[Copilot]

This assignee resolution logic is duplicated from the handleList method (lines 268-293). Consider extracting this into a reusable private method to improve maintainability. The only difference is that handleListAllUncompleted iterates through all workspaces for user lookup (lines 461-466), while handleList uses a single workspace. This could be parameterized with an optional workspaceId parameter.

[claude]

PR Review: fix(tasks): add assignee filtering to list_all_uncompleted

Overall: This is a clean, well-focused fix that resolves a genuine usability gap and opportunistically reduces code duplication. The changes are appropriately scoped. A few observations below.

---

Positive

• Good refactoring: Extracting the inline assignee resolution into resolveAssignee() eliminates a copy-paste hazard and gives list_all_uncompleted the same resolution capability as list without duplicating ~30 lines of logic.
• Correct error-propagation: resolveAssignee now throws instead of returning an error response, which is appropriate for a private helper — the outer handle() catch block converts thrown errors into MCP-compliant responses.
• dotenv fix: Removing the non-existent quiet option is the right call; this was a latent TypeScript error.
• Consistent logging: assigneeId is included in the getAllUncompletedTasks debug log, matching the existing style.

---

Issues / Suggestions

1. Sequential cross-workspace user lookup (performance)

In resolveAssignee, when no workspaceId is supplied and a name/email is given, workspaces are searched one at a time:

const workspaces = await this.motionService.getWorkspaces();
for (const ws of workspaces) {
  const user = await this.motionService.resolveUserIdentifier({ userName: input }, ws.id);
  if (user) { return ...; }
}

list_all_uncompleted already fans out to every workspace for task fetching; adding a sequential user-lookup loop before it means the total workspace round-trips can be N + N in the worst case. Consider parallelising with Promise.all and taking the first non-null result:

const workspaces = await this.motionService.getWorkspaces();
const results = await Promise.all(
  workspaces.map(ws => this.motionService.resolveUserIdentifier({ userName: input }, ws.id))
);
const user = results.find(Boolean);

Whether the service supports parallel calls (rate-limiting, connection pool) is a consideration, but it's worth noting.

2. display is undefined when a raw assigneeId is passed without assignee

if (assigneeId) {
  // ... 'me' case omitted ...
  return { resolvedId: assigneeId, display: assignee };  // assignee may be undefined
}

If a caller passes only assigneeId: "user-abc" (no assignee), display is undefined. The formatter then falls back to the raw ID:

assigneeName: display || resolvedId,

This is functionally correct, but the response will show a UUID rather than a human-readable name. A small improvement would be to fetch the user and return their name, or at minimum document this limitation in the JSDoc.

3. No parameter-level validation in ToolDefinitions.ts for list_all_uncompleted

The assigneeId / assignee parameters are listed at the top-level tool schema without being scoped to specific operations. Since the schema is shared, callers targeting other operations (e.g. create) won't accidentally break — but an LLM could infer that assigneeId is valid for every operation. This is a pre-existing issue and out of scope here, but worth noting for future schema versioning.

---

Minor

• The JSDoc for resolveAssignee says "omit for cross-workspace search" for workspaceId, which is accurate — good documentation.
• The test plan in the PR description is reasonable. A regression test (no assignee params → all tasks returned) is explicitly listed, which is important.

---

Summary: Approve with the performance note on sequential lookups as the main thing to consider in a follow-up. The display fallback to ID is a minor polish item. The core fix is correct.