fix: return success from module work item add tool#125
Conversation
📝 WalkthroughWalkthroughThe PR fixes ChangesModule Tool Success Response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plane_mcp/tools/modules.py (1)
236-236: 💤 Low valueConsider capturing and logging the SDK response for observability.
The SDK's return value (likely a list of association records based on the PR context) is currently discarded. While returning
{"success": True}aligns with the fix for issue#124, capturing the SDK response would improve debugging and observability.♻️ Optional: Capture SDK response for logging
client, workspace_slug = get_plane_client_context() - client.modules.add_work_items( + _response = client.modules.add_work_items( workspace_slug=workspace_slug, project_id=project_id, module_id=module_id, issue_ids=work_item_ids, ) + # Could log _response here for debugging if needed return {"success": True}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plane_mcp/tools/modules.py` at line 236, The SDK call whose result is currently discarded before the final return should be assigned to a variable (e.g., sdk_response or associations_result), logged for observability (using the module's logger at debug/info level), and included or summarized in the returned payload instead of simply returning {"success": True}; update the code around the SDK invocation and the final return (the place that currently just returns {"success": True}) to capture the SDK response, call logger.debug/ info with that variable, and return a JSON object that contains success plus a concise representation of the SDK response.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plane_mcp/tools/modules.py`:
- Line 3: The local TypedDict AddWorkItemsToModuleResult must be removed and the
function add_work_items_to_module should return either None (preferred, matching
cycles.py and milestones.py) or a plane-sdk Pydantic model from plane.models;
update the function that calls client.modules.add_work_items() (which is a
side-effect operation) to return None and adjust its type hints and any callers
accordingly, or if you need structured output, replace the TypedDict with an
appropriate plane.models Pydantic class and return an instance of that model
instead of the custom TypedDict.
---
Nitpick comments:
In `@plane_mcp/tools/modules.py`:
- Line 236: The SDK call whose result is currently discarded before the final
return should be assigned to a variable (e.g., sdk_response or
associations_result), logged for observability (using the module's logger at
debug/info level), and included or summarized in the returned payload instead of
simply returning {"success": True}; update the code around the SDK invocation
and the final return (the place that currently just returns {"success": True})
to capture the SDK response, call logger.debug/ info with that variable, and
return a JSON object that contains success plus a concise representation of the
SDK response.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74ce87d3-17da-4dfa-aa27-dc1ac9da5b5d
📒 Files selected for processing (5)
plane_mcp/tools/modules.pytests/test_integration.pytests/test_modules.pytests/test_oauth_security.pytests/test_stateless_http.py
💤 Files with no reviewable changes (2)
- tests/test_oauth_security.py
- tests/test_stateless_http.py
| """Module-related tools for Plane MCP Server.""" | ||
|
|
||
| from typing import Any, get_args | ||
| from typing import Any, TypedDict, get_args |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check plane-sdk for the add_work_items return type and related Pydantic models
# Search for the add_work_items method definition
rg -n -A 10 'def add_work_items' --type py
# Search for Pydantic models related to module-issue associations
rg -n 'class.*Module.*Issue|class.*Issue.*Module' --type py
# Check imports and type hints in the modules client
ast-grep --pattern 'def add_work_items($$$) -> $RET_TYPE:
$$$'Repository: makeplane/plane-mcp-server
Length of output: 3515
🏁 Script executed:
# Read the modules.py file to see the TypedDict definition and implementation
cat -n plane_mcp/tools/modules.py | sed -n '1,40p'Repository: makeplane/plane-mcp-server
Length of output: 1430
🏁 Script executed:
# Read the return statement area
cat -n plane_mcp/tools/modules.py | sed -n '213,240p'Repository: makeplane/plane-mcp-server
Length of output: 1086
🏁 Script executed:
# Search for TypedDict definition in the file
rg -n 'class AddWorkItemsToModuleResult' -A 5 plane_mcp/tools/modules.pyRepository: makeplane/plane-mcp-server
Length of output: 230
🏁 Script executed:
# Check imports at the top of modules.py to see what's available from plane-sdk
cat -n plane_mcp/tools/modules.py | head -30Repository: makeplane/plane-mcp-server
Length of output: 1036
🏁 Script executed:
# Search for plane-sdk Pydantic models related to modules or issues
find . -path ./node_modules -prune -o -type f -name "*.py" -exec grep -l "class.*Module.*Pydantic\|class.*BaseModel" {} \; | head -20Repository: makeplane/plane-mcp-server
Length of output: 111
🏁 Script executed:
# Look for what client.modules.add_work_items actually returns by checking the call site
rg -n 'client\.modules\.add_work_items' -B 2 -A 5 plane_mcp/tools/modules.pyRepository: makeplane/plane-mcp-server
Length of output: 371
🏁 Script executed:
# Check if PaginatedModuleWorkItemResponse or similar could be used
rg -n 'PaginatedModuleWorkItemResponse' -B 2 -A 10 --type py | head -50Repository: makeplane/plane-mcp-server
Length of output: 1643
🏁 Script executed:
# Search in plane-sdk imports to see what models are available for module operations
python3 << 'EOF'
import sys
# Check what pydantic models are imported from plane.models.modules
print("Models imported from plane.models.modules:")
models = ["CreateModule", "Module", "PaginatedArchivedModuleResponse",
"PaginatedModuleResponse", "PaginatedModuleWorkItemResponse", "UpdateModule"]
for m in models:
print(f" - {m}")
EOFRepository: makeplane/plane-mcp-server
Length of output: 258
🏁 Script executed:
# Check if there's a ModuleWorkItem or similar model for the returned items
rg -n 'class.*Module.*Item|class.*ModuleIssue|ModuleWorkItem' --type py | grep -v test | head -20Repository: makeplane/plane-mcp-server
Length of output: 252
🏁 Script executed:
# Verify what the actual plane-sdk client.modules.add_work_items method signature is
# by looking at any type hints or documentation in the codebase
rg -n 'modules.add_work_items' -B 3 -A 8 --type pyRepository: makeplane/plane-mcp-server
Length of output: 1439
🏁 Script executed:
# Check other tools in the codebase to see how they handle similar situations
rg -n 'def add_work_items_to' -A 15 plane_mcp/tools/*.pyRepository: makeplane/plane-mcp-server
Length of output: 3074
Replace custom TypedDict return type with a plane-sdk Pydantic model or None.
The coding guideline requires "Tool functions must return Pydantic models from plane-sdk", but AddWorkItemsToModuleResult is a locally-defined TypedDict (line 20). The underlying client.modules.add_work_items() call does not return data—it's a side-effect operation. Either return None (consistent with similar methods in cycles.py and milestones.py), or if structured output is needed, use an appropriate Pydantic model from the plane.models package rather than a custom TypedDict.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plane_mcp/tools/modules.py` at line 3, The local TypedDict
AddWorkItemsToModuleResult must be removed and the function
add_work_items_to_module should return either None (preferred, matching
cycles.py and milestones.py) or a plane-sdk Pydantic model from plane.models;
update the function that calls client.modules.add_work_items() (which is a
side-effect operation) to return None and adjust its type hints and any callers
accordingly, or if you need structured output, replace the TypedDict with an
appropriate plane.models Pydantic class and return an instance of that model
instead of the custom TypedDict.
1 participant
Description
Fixes
add_work_items_to_modulereturningUnexpected response typeeven after the Plane API successfully adds the work item to the module.The module work-item endpoint returns a JSON array of association records. Since the MCP tool was typed as returning
None, that successful SDK response could still conflict with MCP response handling.This PR changes
add_work_items_to_moduleto return a structured success response after the SDK call completes, and adds a regression test covering the SDK returning a list.It also fixes a few existing Ruff issues in tests so
ruff check plane_mcp testspasses.Type of Change
Screenshots and Media (if applicable)
Not applicable.
Test Scenarios
Not run: live integration tests, because they require Plane credentials and a running MCP endpoint.
References
Fixes #124.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests
Chores