Repository navigation
feat: add update_list tool - #62
Conversation
Summary of ChangesHello @McFunshine, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant enhancement by adding an Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5004062cd0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .string() | ||
| .optional() | ||
| .describe('ID of the Trello board (uses default if not provided)'), | ||
| listId: z.string().describe('ID of the Trello list to update'), |
There was a problem hiding this comment.
Remove or honor unused boardId parameter
The tool schema advertises an optional boardId, but the handler never destructures or forwards it (see the handler signature around line 347). As a result, callers who pass boardId expecting it to affect the update (per the description) will see no effect, which is a silent mismatch between API contract and behavior. Consider removing boardId from the schema/README or wiring it into the request semantics.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Removed boardId from the input schema and updated the description. Fixed in 8b337c3.
There was a problem hiding this comment.
Code Review
This pull request introduces a new update_list tool, which is a valuable addition for managing Trello lists programmatically. The implementation is solid, but I've identified a few areas for improvement to enhance clarity and conciseness. My main feedback revolves around removing an unused boardId parameter from the new tool's definition and documentation to prevent user confusion. Additionally, I've suggested simplifying the tool's handler logic for better readability and to reduce code duplication. Overall, these are good changes that expand the server's capabilities.
| description: "Update a list's name, position, or closed state on a specific board", | ||
| inputSchema: { | ||
| boardId: z | ||
| .string() | ||
| .optional() | ||
| .describe('ID of the Trello board (uses default if not provided)'), | ||
| listId: z.string().describe('ID of the Trello list to update'), |
There was a problem hiding this comment.
The boardId parameter is defined in the schema but is not used by the Trello API for updating a list (PUT /1/lists/{id}), nor is it used in the tool's implementation. The listId is sufficient to uniquely identify the list. Including boardId can be confusing for users. I've removed it and adjusted the description to be more accurate, matching the one in the README.
description: "Update a list's name, position, or other properties",
inputSchema: {
listId: z.string().describe('ID of the Trello list to update'),There was a problem hiding this comment.
Removed boardId from the schema and changed description to "Update a list's name, position, or other properties". Fixed in 8b337c3.
| const params: { | ||
| name?: string; | ||
| pos?: number | 'top' | 'bottom'; | ||
| closed?: boolean; | ||
| subscribed?: boolean; | ||
| idBoard?: string; | ||
| } = {}; | ||
| if (name !== undefined) params.name = name; | ||
| if (pos !== undefined) params.pos = pos; | ||
| if (closed !== undefined) params.closed = closed; | ||
| if (subscribed !== undefined) params.subscribed = subscribed; | ||
| if (idBoard !== undefined) params.idBoard = idBoard; | ||
| const list = await this.trelloClient.updateList(listId, params); |
There was a problem hiding this comment.
This block can be significantly simplified. The explicit type definition for params is a duplicate of the one in trello-client.ts, and the conditional checks for undefined are verbose. You can create the parameters object inline and pass it directly to updateList. The underlying axios client will automatically omit properties with undefined values from the JSON payload.
const list = await this.trelloClient.updateList(listId, { name, pos, closed, subscribed, idBoard });There was a problem hiding this comment.
Replaced the explicit params object and undefined checks with a single inline call: this.trelloClient.updateList(listId, { name, pos, closed, subscribed, idBoard }). Fixed in 8b337c3.
|
Useful tool —
Once rebased + scope decision noted, ready to merge. |
8b337c3 to
7507cc2
Compare
Adds an update_list tool that exposes PUT /1/lists/{id} via the MCP server.
Changes
Use case
Enables programmatic list reordering, renaming, archiving, and moving between boards — essential for workflows like GTD tickler systems where lists need to be repositioned regularly.
Testing