From 777c134ace5be4b6f8597d870335b769a4f7c997 Mon Sep 17 00:00:00 2001 From: yan Date: Mon, 6 Jul 2026 21:50:32 +0800 Subject: [PATCH 1/2] Add edit_comment action to modify_reviews for task state transitions Wraps POST /api/v11/comments/{id}/edit so agents can update a comment's body and flip its taskState (open -> addressed -> verified), which was previously unreachable via MCP (add_comment only accepts open|comment). - models: EDIT_COMMENT action, TaskState addressed/verified, per-action task_state validation (add_comment still restricted to open|comment), edit_comment requires comment_id and at least one of body/task_state - services: edit_comment() posting only provided fields - handlers: dispatch branch (review_id not required; the endpoint is comment-scoped) - tools: expose action + extended task_state literal, doc updates Co-Authored-By: Claude Fable 5 --- README.md | 1 + p4mcp/handlers/review_handlers.py | 9 +++++++ p4mcp/models/review_models.py | 18 ++++++++++++-- p4mcp/services/review_services.py | 41 ++++++++++++++++++++++++++++++- p4mcp/tools/review_tools.py | 13 ++++++---- 5 files changed, 74 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 1c3336a..e48c538 100755 --- a/README.md +++ b/README.md @@ -1140,6 +1140,7 @@ The MCP server checks properties in this order. Each property is resolved indepe - `delete_participants` - Remove participants from a review - `add_comment` - Add a comment to a review - `reply_comment` - Reply to an existing comment + - `edit_comment` - Edit a comment's body and/or task state (open/addressed/verified; author only) - `append_change` - Add a changelist to an existing review - `replace_with_change` - Replace review content with a changelist - `join` - Join a review as a participant diff --git a/p4mcp/handlers/review_handlers.py b/p4mcp/handlers/review_handlers.py index 27cf16a..7482391 100644 --- a/p4mcp/handlers/review_handlers.py +++ b/p4mcp/handlers/review_handlers.py @@ -163,6 +163,15 @@ def require(attr, label=None): params.body ) + elif action == "edit_comment": + require("comment_id", "comment_id") + result = await self.review_services.edit_comment( + params.comment_id, + getattr(params, "body", None), + getattr(params, "task_state", None), + getattr(params, "notify", None) + ) + elif action == "append_change": require("review_id") require("change_id", "change_id") diff --git a/p4mcp/models/review_models.py b/p4mcp/models/review_models.py index 525c798..40236e4 100644 --- a/p4mcp/models/review_models.py +++ b/p4mcp/models/review_models.py @@ -140,6 +140,7 @@ class ReviewModifyAction(str, Enum): APPEND_PARTICIPANTS = "append_participants" ADD_COMMENT = "add_comment" REPLY_COMMENT = "reply_comment" + EDIT_COMMENT = "edit_comment" APPEND_CHANGE = "append_change" REPLACE_WITH_CHANGE = "replace_with_change" JOIN = "join" @@ -162,6 +163,8 @@ class FixStatus(str, Enum): class TaskState(str, Enum): OPEN = "open" COMMENT = "comment" + ADDRESSED = "addressed" + VERIFIED = "verified" class NotifyMode(str, Enum): IMMEDIATE = "immediate" @@ -393,7 +396,8 @@ class ModifyReviewsParams(BaseParams): ) task_state: Optional[TaskState] = Field( default=None, - description="Task state (optional for add_comment)", + description="Task state (optional for add_comment: open|comment; " + "edit_comment additionally accepts addressed|verified)", examples=["open"] ) notify: Optional[NotifyMode] = Field( @@ -442,7 +446,7 @@ def need(field: Any, label: Optional[str] = None): raise ValueError(f"{label or field} is required for action: {a}") # Actions requiring review_id - if a not in [ReviewModifyAction.CREATE, ReviewModifyAction.ARCHIVE_INACTIVE] and a != ReviewModifyAction.CREATE: + if a not in [ReviewModifyAction.CREATE, ReviewModifyAction.ARCHIVE_INACTIVE, ReviewModifyAction.EDIT_COMMENT] and a != ReviewModifyAction.CREATE: if a not in [ReviewModifyAction.ARCHIVE_INACTIVE] and not self.review_id: raise ValueError(f"review_id is required for action: {a}") @@ -464,6 +468,16 @@ def need(field: Any, label: Optional[str] = None): elif a == ReviewModifyAction.ADD_COMMENT: need("review_id") need("body", "body") + if self.task_state and self.task_state not in [TaskState.OPEN, TaskState.COMMENT]: + raise ValueError( + "task_state must be 'open' or 'comment' for add_comment; " + "'addressed'/'verified' are only reachable via edit_comment" + ) + + elif a == ReviewModifyAction.EDIT_COMMENT: + need("comment_id", "comment_id") + if not self.body and not self.task_state: + raise ValueError("At least one of body or task_state is required for edit_comment action") elif a == ReviewModifyAction.REPLY_COMMENT: need("review_id") diff --git a/p4mcp/services/review_services.py b/p4mcp/services/review_services.py index 337b0f0..ae6e6fc 100644 --- a/p4mcp/services/review_services.py +++ b/p4mcp/services/review_services.py @@ -553,7 +553,46 @@ async def reply_to_comment( except Exception as e: logger.error(f"Failed to reply to comment '{comment_id}' in review '{review_id}': {e}") return {"status": "error", "message": str(e)} - + + async def edit_comment( + self, + comment_id: int, + body: Optional[str] = None, + task_state: Optional[str] = None, + notify: Optional[str] = None, + ) -> Dict[str, Any]: + """POST /api/v11/comments/{id}/edit - Edit a comment body and/or its task state + + Only fields provided are updated. Swarm only allows the comment's author + to edit it. task_state accepts "comment"|"open"|"addressed"|"verified"; + some transitions require an intermediate step (open -> verified must go + through addressed). + + Args: + comment_id = 1234 + body = "Updated comment text." + task_state = "addressed" + notify = "delayed"|"immediate" + """ + try: + auth = await self._get_auth() + api_base = await self._get_api_base() + url = f"{api_base}/comments/{comment_id}/edit" + + payload = {} + if body is not None: + payload["body"] = body + if task_state: + payload["taskState"] = task_state + if notify: + payload["notify"] = notify + + r = requests.post(url, auth=auth, json=payload, verify=self.verify_ssl) + return {"status": "success", "message": self._handle_response(r)} + except Exception as e: + logger.error(f"Failed to edit comment '{comment_id}': {e}") + return {"status": "error", "message": str(e)} + async def append_change_to_review( self, review_id: int, diff --git a/p4mcp/tools/review_tools.py b/p4mcp/tools/review_tools.py index 74806ea..0d1294a 100644 --- a/p4mcp/tools/review_tools.py +++ b/p4mcp/tools/review_tools.py @@ -128,7 +128,7 @@ async def modify_reviews( action: Annotated[Literal[ "create", "refresh_projects", "vote", "transition", "append_participants", "add_comment", "reply_comment", - "append_change", "replace_with_change", "join", + "edit_comment", "append_change", "replace_with_change", "join", "archive_inactive", "mark_comment_read", "mark_comment_unread", "mark_all_comments_read", "mark_all_comments_unread", "update_author", "update_description", @@ -252,12 +252,14 @@ async def modify_reviews( )] = None, body: Annotated[Optional[str], Field( default=None, - description="Comment body (required for add_comment, reply_comment)", + description="Comment body (required for add_comment, reply_comment; optional for edit_comment)", examples=["Looks good."], )] = None, - task_state: Annotated[Optional[Literal["open", "comment"]], Field( + task_state: Annotated[Optional[Literal["open", "comment", "addressed", "verified"]], Field( default=None, - description="Task state", + description="Task state. add_comment accepts only 'open'|'comment'; " + "edit_comment additionally accepts 'addressed'|'verified' (open -> verified " + "requires an intermediate 'addressed' step; only the comment author can edit)", )] = None, notify: Annotated[Optional[Literal["immediate", "delayed"]], Field( default=None, @@ -265,7 +267,8 @@ async def modify_reviews( )] = None, comment_id: Annotated[Optional[int], Field( default=None, - description="Parent comment ID (reply_comment, mark_comment_read/unread)", + description="Comment ID (target of edit_comment, parent for reply_comment, " + "mark_comment_read/unread)", examples=[987], )] = None, not_updated_since: Annotated[Optional[str], Field( From 0b64192b601af734cb54194d9873f06252b81118 Mon Sep 17 00:00:00 2001 From: yan Date: Tue, 7 Jul 2026 10:55:42 +0800 Subject: [PATCH 2/2] Align task state transition docs with live Swarm behavior End-to-end testing against a production Swarm (API v11) showed the server accepts any taskState transition (including open -> verified directly), so describe open -> addressed -> verified as the documented convention rather than an enforced constraint, and note the 403 on editing another user's comment. Co-Authored-By: Claude Fable 5 --- p4mcp/services/review_services.py | 8 +++++--- p4mcp/tools/review_tools.py | 5 +++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/p4mcp/services/review_services.py b/p4mcp/services/review_services.py index ae6e6fc..dfadd90 100644 --- a/p4mcp/services/review_services.py +++ b/p4mcp/services/review_services.py @@ -564,9 +564,11 @@ async def edit_comment( """POST /api/v11/comments/{id}/edit - Edit a comment body and/or its task state Only fields provided are updated. Swarm only allows the comment's author - to edit it. task_state accepts "comment"|"open"|"addressed"|"verified"; - some transitions require an intermediate step (open -> verified must go - through addressed). + to edit it (403 otherwise). task_state accepts + "comment"|"open"|"addressed"|"verified". Swarm's docs describe the flow + open -> addressed -> verified, but live testing against Swarm (API v11) + showed the server does not enforce the ordering; treat it as the + recommended convention rather than a hard constraint. Args: comment_id = 1234 diff --git a/p4mcp/tools/review_tools.py b/p4mcp/tools/review_tools.py index 0d1294a..4915962 100644 --- a/p4mcp/tools/review_tools.py +++ b/p4mcp/tools/review_tools.py @@ -258,8 +258,9 @@ async def modify_reviews( task_state: Annotated[Optional[Literal["open", "comment", "addressed", "verified"]], Field( default=None, description="Task state. add_comment accepts only 'open'|'comment'; " - "edit_comment additionally accepts 'addressed'|'verified' (open -> verified " - "requires an intermediate 'addressed' step; only the comment author can edit)", + "edit_comment additionally accepts 'addressed'|'verified'. The documented " + "flow is open -> addressed -> verified, though Swarm does not necessarily " + "enforce the ordering server-side; only the comment author can edit", )] = None, notify: Annotated[Optional[Literal["immediate", "delayed"]], Field( default=None,