fix(mcp): Fix update_task signature and MCP instructions#421
Merged
Conversation
Resolves #420 - Tasks being duplicated instead of updated Changes: 1. Fixed update_task function signature to use individual optional parameters - Changed from TypedDict to explicit parameters (title, status, etc.) - Consistent with update_project and update_document patterns - Builds update_fields dict internally from provided parameters 2. Updated MCP instructions with correct function names - Replaced non-existent manage_task with actual functions - Added complete function signatures for all tools - Improved workflow documentation with concrete examples This fixes the issue where AI agents were confused by: - Wrong function names in instructions (manage_task vs update_task) - Inconsistent parameter patterns across update functions - TypedDict magic that wasn't clearly documented 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughRefactors update_task to accept explicit optional fields and builds the update payload internally with validation for empty updates. Removes the TypedDict alias. Updates tests to the new signature and validates request JSON. Modifies MCP_INSTRUCTIONS text only; no runtime logic changes elsewhere. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Tools as task_tools.update_task
participant API as HTTP API /api/tasks/{task_id}
Client->>Tools: update_task(task_id, [title|description|status|...])
Tools->>Tools: Build update_fields from provided kwargs
alt No fields provided
Tools-->>Client: validation_error ("No fields to update")
else At least one field
Tools->>API: PUT /api/tasks/{task_id} with JSON(update_fields)
API-->>Tools: 200/4xx response
Tools-->>Client: Result or error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
- Fixed test_update_task_status to use individual parameters - Added test_update_task_no_fields for validation testing - All MCP tests passing (44 tests)
- Remove trailing whitespace - Consistent formatting in instruction text
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
python/src/mcp_server/features/tasks/task_tools.py (4)
10-10: Import Literal for stronger type-safety on status.Prepare for a Literal-typed status parameter in update_task.
-from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Literal
305-313: Signature change LGTM; consider constraining status to allowed literals.Using Literal reduces typos and improves mypy checks while remaining backward-compatible.
- status: Optional[str] = None, + status: Optional[Literal["todo", "doing", "review", "done"]] = None,
339-357: Type the payload and simplify construction of update_fields.Explicit typing helps mypy; the dict-comprehension reduces repetition and mistakes when new fields are added.
- # Build update_fields dict from provided parameters - update_fields = {} - if title is not None: - update_fields["title"] = title - if description is not None: - update_fields["description"] = description - if status is not None: - update_fields["status"] = status - if assignee is not None: - update_fields["assignee"] = assignee - if task_order is not None: - update_fields["task_order"] = task_order - if feature is not None: - update_fields["feature"] = feature - if sources is not None: - update_fields["sources"] = sources - if code_examples is not None: - update_fields["code_examples"] = code_examples + # Build update_fields dict from provided parameters + update_fields: Dict[str, Any] = { + k: v + for k, v in { + "title": title, + "description": description, + "status": status, + "assignee": assignee, + "task_order": task_order, + "feature": feature, + "sources": sources, + "code_examples": code_examples, + }.items() + if v is not None + }
365-379: Consider accepting 204 No Content for robustness.Some APIs return 204 for successful updates. Handling 200 and 204 avoids false negatives and reduces coupling to response shape.
- response = await client.put( - urljoin(api_url, f"/api/tasks/{task_id}"), json=update_fields - ) - - if response.status_code == 200: - result = response.json() - return json.dumps({ - "success": True, - "task": result.get("task"), - "message": result.get("message", "Task updated successfully"), - }) + response = await client.put( + urljoin(api_url, f"/api/tasks/{task_id}"), json=update_fields + ) + + if response.status_code in (200, 204): + if response.status_code == 204 or not response.content: + return json.dumps({ + "success": True, + "task": None, + "message": "Task updated successfully", + }) + result = response.json() + return json.dumps({ + "success": True, + "task": result.get("task"), + "message": result.get("message", "Task updated successfully"), + })python/tests/mcp_server/features/tasks/test_task_tools.py (1)
185-204: Great coverage for no-fields guard; optionally assert no HTTP call is made.You can also assert that PUT is never invoked in this path to harden the contract.
@pytest.mark.asyncio async def test_update_task_no_fields(mock_mcp, mock_context): """Test updating task with no fields returns validation error.""" register_task_tools(mock_mcp) # Get the update_task function update_task = mock_mcp._tools.get("update_task") assert update_task is not None, "update_task tool not registered" - # Call update_task with no optional fields - result = await update_task(mock_context, task_id="task-123") + # Call update_task with no optional fields + with patch("src.mcp_server.features.tasks.task_tools.httpx.AsyncClient") as mock_client: + mock_async_client = AsyncMock() + mock_client.return_value.__aenter__.return_value = mock_async_client + result = await update_task(mock_context, task_id="task-123") result_data = json.loads(result) assert result_data["success"] is False assert "error" in result_data assert isinstance(result_data["error"], dict) assert result_data["error"]["type"] == "validation_error" assert "No fields to update" in result_data["error"]["message"] + + # Ensure we never attempted an HTTP call + mock_async_client.put.assert_not_called()python/src/mcp_server/mcp_server.py (1)
216-221: Inline API snippet for update_task: list all supported fields for parity.Minor nit: documenting all optional params avoids confusion and aligns with task_tools.update_task.
- `update_task(task_id, title=None, status=None, assignee=None, ...)` + - `update_task(task_id, title=None, description=None, status=None, assignee=None, task_order=None, feature=None, sources=None, code_examples=None)`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
python/src/mcp_server/features/tasks/task_tools.py(2 hunks)python/src/mcp_server/mcp_server.py(1 hunks)python/tests/mcp_server/features/tasks/test_task_tools.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
python/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
python/**/*.py: Target Python 3.12 with a 120-character line length
Use Ruff for linting and Mypy for type checking before commit
Files:
python/src/mcp_server/mcp_server.pypython/src/mcp_server/features/tasks/task_tools.pypython/tests/mcp_server/features/tasks/test_task_tools.py
{python/**/*.py,archon-ui-main/src/**/*.{ts,tsx,js,jsx}}
📄 CodeRabbit inference engine (CLAUDE.md)
{python/**/*.py,archon-ui-main/src/**/*.{ts,tsx,js,jsx}}: Remove dead code immediately; do not keep legacy/unused functions
Avoid comments that reference change history (e.g., LEGACY, CHANGED, REMOVED); keep comments focused on current functionality
Files:
python/src/mcp_server/mcp_server.pypython/src/mcp_server/features/tasks/task_tools.pypython/tests/mcp_server/features/tasks/test_task_tools.py
python/tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Place backend tests under python/tests/
Files:
python/tests/mcp_server/features/tasks/test_task_tools.py
🧠 Learnings (1)
📚 Learning: 2025-08-21T11:22:33.541Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-21T11:22:33.541Z
Learning: Applies to python/src/mcp/** : Keep MCP server implementation under python/src/mcp/
Applied to files:
python/src/mcp_server/mcp_server.py
🧬 Code graph analysis (2)
python/src/mcp_server/features/tasks/task_tools.py (2)
python/src/mcp_server/utils/timeout_config.py (1)
get_default_timeout(13-31)python/src/mcp_server/utils/error_handling.py (2)
MCPErrorFormatter(16-148)format_error(20-57)
python/tests/mcp_server/features/tasks/test_task_tools.py (2)
python/tests/mcp_server/features/documents/test_document_tools.py (2)
mock_context(31-33)mock_mcp(13-27)python/src/mcp_server/features/tasks/task_tools.py (1)
update_task(302-386)
🔇 Additional comments (7)
python/src/mcp_server/features/tasks/task_tools.py (1)
358-364: Good: early validation prevents accidental no-op updates.This avoids confusing “successful” calls that don’t change anything and provides clear guidance to callers.
python/tests/mcp_server/features/tasks/test_task_tools.py (1)
171-172: LGTM: verifies payload fields for update_task.Asserting the exact JSON sent (status, assignee) catches regressions in request construction.
Also applies to: 178-183
python/src/mcp_server/mcp_server.py (5)
195-199: Docs: Clear rule to use MCP tools for task management.This addresses the root cause from #420 (wrong tool names) by steering clients to the correct surface.
206-214: Task Management Cycle reads well and matches exported tools.The examples use get_task and update_task consistently with the implemented tool names.
245-249: Status flow guidance is consistent and actionable.Matches the allowed status values used in task_tools.py.
239-243: Research tool availability verifiedThe functions perform_rag_query, search_code_examples, and get_available_sources are all implemented and decorated with @mcp.tool() in python/src/mcp_server/modules/rag_module.py. No further action is needed.
250-256: All Version Management Tools Properly RegisteredI’ve confirmed that the four tools—create_version, list_versions, get_version, and restore_version—are defined in
version_tools.pywith@mcp.tool()and thatregister_version_toolsis imported and invoked inmcp_server.py, with corresponding tests exercising each registration. No further changes are needed here.
Collaborator
Author
|
resolves: #405 |
This was referenced Sep 12, 2025
coleam00
pushed a commit
that referenced
this pull request
Apr 7, 2026
…e cleanup (#601) * feat(cli): add archon complete <branch> command for worktree lifecycle cleanup (#421) After merging a PR, users had to run three separate commands to clean up the worktree, local branch, and remote branch. This adds a single top-level command that handles the full lifecycle in one step. Changes: - Add findActiveByBranchName() to isolation-environments.ts for DB lookup by branch name - Export removeEnvironment and RemoveEnvironmentOptions from @archon/core index - Add isolationCompleteCommand() to packages/cli/src/commands/isolation.ts with explicit uncommitted-changes check and --force flag support - Add 'complete' case to cli.ts with routing, help text, and import - Add isolation.test.ts with 6 tests covering all edge cases - Split CLI test batches in package.json to avoid mock.module() pollution Fixes #421 * fix(cli): address review findings for archon complete command - Remove unused removeEnvironment/RemoveEnvironmentOptions re-exports from @archon/core index (YAGNI — caller already uses deep path import) - Add ORDER BY e.created_at DESC to findActiveByBranchName for deterministic results when multiple active environments share a branch name - Wrap findActiveByBranchName in per-branch try/catch so DB errors are attributed to the specific branch and the summary still prints - Update inline comment to describe all silent-return conditions in removeEnvironment, not just the uncommitted-changes path - Update findActiveByBranchName docstring to document the JOIN and extra codebase_default_cwd field in the return type - Update file-level docstring in isolation.ts to include complete command - Document archon complete command in CLAUDE.md, cli-user-guide.md, getting-started.md, and cli-developer-guide.md - Remove unnecessary String() coercions in summary output line
Tyone88
pushed a commit
to Tyone88/Archon
that referenced
this pull request
Apr 16, 2026
…e cleanup (coleam00#601) * feat(cli): add archon complete <branch> command for worktree lifecycle cleanup (coleam00#421) After merging a PR, users had to run three separate commands to clean up the worktree, local branch, and remote branch. This adds a single top-level command that handles the full lifecycle in one step. Changes: - Add findActiveByBranchName() to isolation-environments.ts for DB lookup by branch name - Export removeEnvironment and RemoveEnvironmentOptions from @archon/core index - Add isolationCompleteCommand() to packages/cli/src/commands/isolation.ts with explicit uncommitted-changes check and --force flag support - Add 'complete' case to cli.ts with routing, help text, and import - Add isolation.test.ts with 6 tests covering all edge cases - Split CLI test batches in package.json to avoid mock.module() pollution Fixes coleam00#421 * fix(cli): address review findings for archon complete command - Remove unused removeEnvironment/RemoveEnvironmentOptions re-exports from @archon/core index (YAGNI — caller already uses deep path import) - Add ORDER BY e.created_at DESC to findActiveByBranchName for deterministic results when multiple active environments share a branch name - Wrap findActiveByBranchName in per-branch try/catch so DB errors are attributed to the specific branch and the summary still prints - Update inline comment to describe all silent-return conditions in removeEnvironment, not just the uncommitted-changes path - Update findActiveByBranchName docstring to document the JOIN and extra codebase_default_cwd field in the return type - Update file-level docstring in isolation.ts to include complete command - Document archon complete command in CLAUDE.md, cli-user-guide.md, getting-started.md, and cli-developer-guide.md - Remove unnecessary String() coercions in summary output line
joaobmonteiro
pushed a commit
to joaobmonteiro/Archon
that referenced
this pull request
Apr 26, 2026
…e cleanup (coleam00#601) * feat(cli): add archon complete <branch> command for worktree lifecycle cleanup (coleam00#421) After merging a PR, users had to run three separate commands to clean up the worktree, local branch, and remote branch. This adds a single top-level command that handles the full lifecycle in one step. Changes: - Add findActiveByBranchName() to isolation-environments.ts for DB lookup by branch name - Export removeEnvironment and RemoveEnvironmentOptions from @archon/core index - Add isolationCompleteCommand() to packages/cli/src/commands/isolation.ts with explicit uncommitted-changes check and --force flag support - Add 'complete' case to cli.ts with routing, help text, and import - Add isolation.test.ts with 6 tests covering all edge cases - Split CLI test batches in package.json to avoid mock.module() pollution Fixes coleam00#421 * fix(cli): address review findings for archon complete command - Remove unused removeEnvironment/RemoveEnvironmentOptions re-exports from @archon/core index (YAGNI — caller already uses deep path import) - Add ORDER BY e.created_at DESC to findActiveByBranchName for deterministic results when multiple active environments share a branch name - Wrap findActiveByBranchName in per-branch try/catch so DB errors are attributed to the specific branch and the summary still prints - Update inline comment to describe all silent-return conditions in removeEnvironment, not just the uncommitted-changes path - Update findActiveByBranchName docstring to document the JOIN and extra codebase_default_cwd field in the return type - Update file-level docstring in isolation.ts to include complete command - Document archon complete command in CLAUDE.md, cli-user-guide.md, getting-started.md, and cli-developer-guide.md - Remove unnecessary String() coercions in summary output line
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
Fixes issue #420 where updating tasks creates new tasks instead of modifying existing ones. The root cause was incorrect MCP instructions and inconsistent function signatures.
Changes Made
update_taskfunction signature to use individual optional parameters instead of TypedDictmanage_taskwith actual functions)Type of Change
Affected Services
Testing
Test Evidence
Checklist
Breaking Changes
None - The changes maintain backward compatibility by using optional parameters.
Additional Notes
This PR resolves two related issues:
manage_taskandmanage_projectfunctions that don't existupdate_taskused TypedDict whileupdate_projectandupdate_documentused individual parametersThe fix ensures all update functions follow the same pattern and AI agents have clear, accurate instructions for using the MCP tools.
Fixes #420
Summary by CodeRabbit
Refactor
Documentation
Tests