Repository navigation
feat(cards): add pos parameter to move_card and update_card_details - #85
Conversation
Allows callers to specify card position ("top", "bottom", or a numeric
value) when moving or updating a card, passing it through to the Trello
PUT /cards/{id} endpoint.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for the 'pos' parameter in the update_card_details and move_card tools, enabling users to set card positions as 'top', 'bottom', or a specific number. The changes span the Zod schemas in src/index.ts and the TrelloClient implementation in src/trello-client.ts. Reviewers suggested refining the 'pos' parameter's validation and type definitions to use specific enums and literal types ('top' | 'bottom') instead of generic strings for better type safety and consistency.
| .describe('Mark the due date as complete (true) or incomplete (false)'), | ||
| labels: z.array(z.string()).optional().describe('New array of label IDs for the card'), | ||
| pos: z | ||
| .union([z.string(), z.number()]) |
There was a problem hiding this comment.
The 'pos' parameter validation can be more restrictive to ensure only valid values are passed to the Trello API. Using z.enum(['top', 'bottom']) instead of z.string() prevents invalid strings from being sent. This also maintains consistency with the update_checklist_item tool defined later in this file (line 1110).
| .union([z.string(), z.number()]) | |
| .union([z.number(), z.enum(['top', 'bottom'])]) |
| cardId: z.string().describe('ID of the card to move'), | ||
| listId: z.string().describe('ID of the target list'), | ||
| pos: z | ||
| .union([z.string(), z.number()]) |
There was a problem hiding this comment.
Similar to update_card_details, the 'pos' parameter here should use a more restrictive schema for better validation and consistency with other tools in this server.
| .union([z.string(), z.number()]) | |
| .union([z.number(), z.enum(['top', 'bottom'])]) |
| start?: string; | ||
| dueComplete?: boolean; | ||
| labels?: string[]; | ||
| pos?: string | number; |
There was a problem hiding this comment.
The type for 'pos' can be more specific to reflect the valid values accepted by Trello ('top', 'bottom', or a number). This aligns with the TrelloCheckItemUpdate interface already defined in src/types.ts.
| pos?: string | number; | |
| pos?: number | 'top' | 'bottom'; |
| } | ||
|
|
||
| async moveCard(boardId: string | undefined, cardId: string, listId: string): Promise<TrelloCard> { | ||
| async moveCard(boardId: string | undefined, cardId: string, listId: string, pos?: string | number): Promise<TrelloCard> { |
There was a problem hiding this comment.
Updating the type signature for 'pos' to be more specific, consistent with the updateCard method and existing project types.
| async moveCard(boardId: string | undefined, cardId: string, listId: string, pos?: string | number): Promise<TrelloCard> { | |
| async moveCard(boardId: string | undefined, cardId: string, listId: string, pos?: number | 'top' | 'bottom'): Promise<TrelloCard> { |
delorenj
left a comment
delorenj
left a comment
There was a problem hiding this comment.
LGTM. Tested locally: build clean, 85/85 vitest tests pass on the branch. Schema correctly uses union of string|number for compatibility with MCP SDK's ZodEffects constraint. Merging.
2 participants
Adds an optional pos parameter to move_card and update_card_details, allowing callers to specify card position ("top", "bottom", or a numeric value) within a list.
The parameter is passed directly to the Trello PUT /cards/{id} API. Validation uses z.union([z.string(), z.number()]) — a stricter enum+positive-number schema was attempted but is incompatible with the MCP SDK's ZodEffects type constraints. Invalid values are still rejected by the Trello API.
Note: PR #52 covers the same feature for move_card only — this PR extends it to update_card_details as well.