From d6323e9adac7aaf308b270d3ee5b94f0e3fccef3 Mon Sep 17 00:00:00 2001 From: Akhil Vamshi Konam Date: Mon, 21 Sep 2026 11:54:23 +0530 Subject: [PATCH 01/12] feat: add work item management actions to initiative tool (#223) --- CLAUDE.md | 2 +- README.md | 2 +- plane_mcp/tools/README.md | 4 +- plane_mcp/tools/initiative.py | 44 +++++++++++++++++++++- pyproject.toml | 2 +- tests/tools/test_dispatch.py | 71 +++++++++++++++++++++++++++++++++++ 6 files changed, 119 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5adfdd70..4feab874 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -81,7 +81,7 @@ Coercion runs before validation so an argument is judged by the value it repairs ### Tools (`tools/`) -One action-dispatch tool per Plane resource: **30 tools, 204 actions, ~67k chars advertised**. `tools/__init__.py` re-exports `register_tools`, so `server.py` and `__main__.py` see a single entry point. +One action-dispatch tool per Plane resource: **30 tools, 206 actions, ~67k chars advertised**. `tools/__init__.py` re-exports `register_tools`, so `server.py` and `__main__.py` see a single entry point. One module per resource, each exporting `NAME`, `ACTIONS`, `LEGACY` and `register(mcp)`. `ACTIONS` is the single source of truth: the tool description and its `ToolAnnotations` are generated from it, and the conformance suite asserts they agree with the function signature. See `tools/README.md` for the full convention. diff --git a/README.md b/README.md index 6bc61b3e..d6ca26ca 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ work items, cycles, modules, releases, customers and more. Built on [FastMCP](https://github.com/jlowin/fastmcp) and the official [`plane-sdk`](https://pypi.org/project/plane-sdk/). -- **30 tools**, one per Plane resource, covering 204 operations +- **30 tools**, one per Plane resource, covering 206 operations - **Local or remote** — stdio, streamable HTTP, SSE - **OAuth or API key** authentication diff --git a/plane_mcp/tools/README.md b/plane_mcp/tools/README.md index 4fb71fcd..1bb6708e 100644 --- a/plane_mcp/tools/README.md +++ b/plane_mcp/tools/README.md @@ -1,6 +1,6 @@ # The tool surface -**30 tools**, one per Plane resource, each taking an `action` parameter that selects the operation. 204 actions in total. +**30 tools**, one per Plane resource, each taking an `action` parameter that selects the operation. 206 actions in total. ```python workitem(action="create", project_id=..., name="Fix login") @@ -97,7 +97,7 @@ Where the workspace owns a resource outright, both directions of wrong-scope wri | `customer_request` | `list` · `retrieve` · `create` · `update` · `delete` | | `cycle` | `list` · `retrieve` · `create` · `update` · `delete` · `list_workitems` · `manage_workitems` · `transfer_workitems` · `complete` · `archive` · `unarchive` | | `get_pql_reference` | *(no action parameter)* | -| `initiative` | `list` · `retrieve` · `create` · `update` · `delete` · `list_projects` · `add_projects` · `remove_projects` | +| `initiative` | `list` · `retrieve` · `create` · `update` · `delete` · `list_projects` · `add_projects` · `remove_projects` · `list_workitems` · `manage_workitems` | | `intake` | `list` · `retrieve` · `create` · `update` · `delete` | | `label` | `list` · `retrieve` · `create` · `update` · `delete` | | `member` | `me` · `list_workspace` · `list_project` · `list_roles` · `retrieve_role` | diff --git a/plane_mcp/tools/initiative.py b/plane_mcp/tools/initiative.py index 20ff114c..579ae6bf 100644 --- a/plane_mcp/tools/initiative.py +++ b/plane_mcp/tools/initiative.py @@ -19,6 +19,7 @@ UpdateInitiative, ) from plane.models.projects import PaginatedProjectResponse +from plane.models.work_items import PaginatedWorkItemResponse from plane_mcp.client import get_plane_client_context from plane_mcp.toolkit import ( @@ -27,6 +28,7 @@ build_description, coerce_list, envelope, + missing, one_of, opt, page_params, @@ -49,6 +51,9 @@ "Linking projects to an initiative requires the native initiatives feature; " "there is no work-item equivalent. Enable it in workspace settings." ) +_WORKITEMS_NEED_NATIVE = ( + "Linking work items to an initiative requires the native initiatives feature. Enable it in workspace settings." +) ACTIONS = ( Action("list", (), note="returns every initiative; this endpoint does not paginate", read=True), @@ -69,11 +74,20 @@ note="returns nothing, read back with list_projects", destructive=True, ), + Action("list_workitems", ("initiative_id",), ("cursor", "per_page"), read=True), + Action( + "manage_workitems", + ("initiative_id",), + ("add_ids", "remove_ids"), + note="pass at least one of add_ids or remove_ids; each takes one id or several, " + "and removals apply first; returns nothing, read back with list_workitems", + ), ) FOOTER = ( f"state is one of: {', '.join(STATES)}. Dates are ISO 8601 (YYYY-MM-DD). " - "lead is a member id. project_ids takes project UUIDs." + "lead is a member id. project_ids takes project UUIDs, add_ids and remove_ids " + "work item UUIDs. A work item of any type can be linked to an initiative." ) LEGACY = { @@ -114,6 +128,8 @@ def initiative( "list_projects", "add_projects", "remove_projects", + "list_workitems", + "manage_workitems", ], initiative_id: str = "", name: str = "", @@ -123,6 +139,8 @@ def initiative( state: str = "", lead: str = "", project_ids: str = "", + add_ids: str = "", + remove_ids: str = "", cursor: str = "", per_page: int = 0, ) -> Initiative | list[Initiative] | dict[str, Any] | str | None: @@ -139,6 +157,8 @@ def initiative( if action in ("list_projects", "add_projects", "remove_projects"): _require_native(client, workspace_slug, _PROJECTS_NEED_NATIVE) + elif action in ("list_workitems", "manage_workitems"): + _require_native(client, workspace_slug, _WORKITEMS_NEED_NATIVE) else: _require_native(client, workspace_slug, _WORK_ITEM_FALLBACK) @@ -183,6 +203,28 @@ def initiative( client.initiatives.delete(workspace_slug=workspace_slug, initiative_id=initiative_id) return None + if action in ("list_workitems", "manage_workitems"): + work_items = client.initiatives.work_items + + if action == "list_workitems": + linked_workitems: PaginatedWorkItemResponse = work_items.list( + workspace_slug=workspace_slug, + initiative_id=initiative_id, + params=page_params(cursor, per_page), + ) + return envelope(linked_workitems) + + add = coerce_list(add_ids) + remove = coerce_list(remove_ids) + if not add and not remove: + return missing(action, "add_ids or remove_ids") + # Removals first, so one call can swap an id out for another. + if remove: + work_items.remove(workspace_slug=workspace_slug, initiative_id=initiative_id, work_item_ids=remove) + if add: + work_items.add(workspace_slug=workspace_slug, initiative_id=initiative_id, work_item_ids=add) + return None + projects = client.initiatives.projects if action == "list_projects": diff --git a/pyproject.toml b/pyproject.toml index 74db33bc..cc983256 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -13,7 +13,7 @@ keywords = ["mcp", "plane", "fastmcp", "ai", "automation"] dependencies = [ "fastmcp==3.2.0", - "plane-sdk==0.2.23", + "plane-sdk==0.3.1", "py-key-value-aio[redis]>=0.4.4,<0.5.0", "mcp==1.28.1", "PyJWT>=2.13.0", diff --git a/tests/tools/test_dispatch.py b/tests/tools/test_dispatch.py index 93af3333..165957f3 100644 --- a/tests/tools/test_dispatch.py +++ b/tests/tools/test_dispatch.py @@ -48,6 +48,7 @@ ("cycle", "manage_workitems"): {"add_ids": "id-1"}, ("module", "manage_workitems"): {"add_ids": "id-1"}, ("milestone", "manage_workitems"): {"add_ids": "id-1"}, + ("initiative", "manage_workitems"): {"add_ids": "id-1"}, ("workitem", "manage_assignee"): {"add_user_id": "id-1"}, ("workitem", "manage_label"): {"add_label_id": "id-1"}, ("workitem", "count"): {"pql": "state__group = 'started'"}, @@ -178,6 +179,7 @@ def test_a_zero_that_means_something_reaches_the_sdk(tool_name, args, field, reg ("customer", "manage_workitems"), ("initiative", "add_projects"), ("initiative", "remove_projects"), + ("initiative", "manage_workitems"), ("milestone", "manage_workitems"), ("module", "manage_workitems"), ("release", "manage_workitems"), @@ -305,6 +307,75 @@ def test_archiving_a_work_item_confirms_what_it_did(archive, registered, spy): assert spy.recorder.only().method == f"work_items.{verb}" +# Initiatives roll up work items as well as projects. The two child collections +# reach different SDK sub-resources, and `manage_workitems` is the only action on +# this tool that issues two calls, so the order between them is pinned here. + + +class _WorkItemPage: + """One page of work items, carrying the full pagination envelope.""" + + results = [{"id": "w-1"}] + total_count = 1 + count = 1 + next_cursor = "c-2" + prev_cursor = "c-0" + next_page_results = True + prev_page_results = False + + +def _initiative_calls(spy): + """The SDK calls a dispatch made, minus the workspace feature probe.""" + return [call for call in spy.recorder.calls if call.method != "workspaces.get_features"] + + +def test_listing_initiative_workitems_pages_through_its_own_sub_resource(registered, spy): + spy.__dict__["default"] = _WorkItemPage() + + result = registered["initiative"].fn(action="list_workitems", initiative_id="i-1", cursor="c-1", per_page=5) + + call = _initiative_calls(spy)[-1] + assert call.method == "initiatives.work_items.list", ( + "work items must reach the work_items sub-resource, not projects or the deprecated epics" + ) + assert call.kwargs["initiative_id"] == "i-1" + assert call.kwargs["params"] == {"cursor": "c-1", "per_page": 5} + assert result["next_cursor"] == "c-2", "a paged action must hand back the cursor it accepted" + + +def test_managing_initiative_workitems_removes_before_it_adds(registered, spy): + """Removals first, so one call can swap an id out for another without the add + being undone by the remove that follows it.""" + registered["initiative"].fn(action="manage_workitems", initiative_id="i-1", add_ids="w-new", remove_ids="w-old") + + calls = _initiative_calls(spy) + assert [call.method for call in calls] == [ + "initiatives.work_items.remove", + "initiatives.work_items.add", + ] + assert calls[0].kwargs["work_item_ids"] == ["w-old"] + assert calls[1].kwargs["work_item_ids"] == ["w-new"] + + +@pytest.mark.parametrize( + ("supplied", "expected"), + [("w-1,w-2", ["w-1", "w-2"]), ('["w-1", "w-2"]', ["w-1", "w-2"]), ("w-1", ["w-1"])], + ids=["comma", "json", "single"], +) +def test_initiative_workitem_ids_take_one_id_or_several(supplied, expected, registered, spy): + registered["initiative"].fn(action="manage_workitems", initiative_id="i-1", add_ids=supplied) + + assert _initiative_calls(spy)[-1].kwargs["work_item_ids"] == expected + + +def test_managing_initiative_workitems_with_neither_side_asks_for_one(registered, spy): + """A call that names no ids would otherwise reach Plane and change nothing.""" + result = registered["initiative"].fn(action="manage_workitems", initiative_id="i-1") + + assert result == "Error: action 'manage_workitems' requires: add_ids or remove_ids." + assert not _initiative_calls(spy), "the refusal must replace the call, not follow it" + + def test_no_description_warns_about_a_failure_the_caller_cannot_avoid(resource_modules, registered): """Naming a failure mode in a description buys a pre-flight probe on every run. From 60a3777a4ec86937d56ec8dd47dd58e50475af8d Mon Sep 17 00:00:00 2001 From: Akhil Vamshi Konam Date: Mon, 21 Sep 2026 11:56:57 +0530 Subject: [PATCH 02/12] [PAI-1927] fix: stop a padded schema default from refusing the call (#222) * fix: refine action handling and default parameter behavior across * refactor: remove outdated comments regarding default parameter behavior in register functions --- plane_mcp/tools/page.py | 9 +++--- plane_mcp/tools/project_estimate.py | 4 +-- plane_mcp/tools/workitem.py | 7 +++-- tests/test_argument_validation.py | 49 +++++++++++++++++++++++++++++ tests/tools/test_dispatch.py | 18 +++++++++++ 5 files changed, 78 insertions(+), 9 deletions(-) diff --git a/plane_mcp/tools/page.py b/plane_mcp/tools/page.py index e4f3581a..a90eaf92 100644 --- a/plane_mcp/tools/page.py +++ b/plane_mcp/tools/page.py @@ -130,7 +130,7 @@ def page( access: int | None = None, color: str = "", is_locked: bool | None = None, - archive: bool = True, + archive: bool | None = None, external_source: str = "", external_id: str = "", cursor: str = "", @@ -160,14 +160,15 @@ def page( if action == "archive": if not page_id: return missing(action, "page_id") + archiving = archive is not False if project_id: - mover = client.pages.archive_project_page if archive else client.pages.unarchive_project_page + mover = client.pages.archive_project_page if archiving else client.pages.unarchive_project_page mover(workspace_slug=workspace_slug, project_id=project_id, page_id=page_id) else: - mover = client.pages.archive_workspace_page if archive else client.pages.unarchive_workspace_page + mover = client.pages.archive_workspace_page if archiving else client.pages.unarchive_workspace_page mover(workspace_slug=workspace_slug, page_id=page_id) # Plane answers nothing, and delete depends on this having happened. - return {"page_id": page_id, "archived": archive} + return {"page_id": page_id, "archived": archiving} if action in ("update", "delete"): if not page_id: diff --git a/plane_mcp/tools/project_estimate.py b/plane_mcp/tools/project_estimate.py index a2b5ca9d..997c85fa 100644 --- a/plane_mcp/tools/project_estimate.py +++ b/plane_mcp/tools/project_estimate.py @@ -98,7 +98,7 @@ def project_estimate( value: str = "", # Estimate point keys start at 0, so key cannot use the 0 sentinel. key: int | None = None, - last_used: bool = True, + last_used: bool | None = None, external_source: str = "", external_id: str = "", ) -> Estimate | EstimatePoint | list[EstimatePoint] | Project | str | None: @@ -124,7 +124,7 @@ def project_estimate( name=name, type=opt(type), description=opt(description), - last_used=last_used, + last_used=last_used is not False, external_id=opt(external_id), external_source=opt(external_source), ), diff --git a/plane_mcp/tools/workitem.py b/plane_mcp/tools/workitem.py index e5c53b8f..a113b473 100644 --- a/plane_mcp/tools/workitem.py +++ b/plane_mcp/tools/workitem.py @@ -232,7 +232,7 @@ def workitem( # noqa: PLR0911, PLR0912 - one branch per action is the point per_page: int = 0, # Tri-state: False publishes a draft, unset leaves the flag alone. is_draft: bool | None = None, - archive: bool = True, + archive: bool | None = None, ) -> WorkItem | WorkItemDetail | WorkItemSearch | dict[str, Any] | list[Any] | str | None: client, workspace_slug = get_plane_client_context() @@ -376,9 +376,10 @@ def write_payload() -> dict[str, Any]: return None if action == "archive": - operation = client.work_items.archive if archive else client.work_items.unarchive + archiving = archive is not False + operation = client.work_items.archive if archiving else client.work_items.unarchive operation(workspace_slug=workspace_slug, project_id=project_id, work_item_id=workitem_id) - return {"workitem_id": workitem_id, "archived": archive} + return {"workitem_id": workitem_id, "archived": archiving} # manage_assignee / manage_label: read the current set, mutate it, write it back. add, remove, field = ( diff --git a/tests/test_argument_validation.py b/tests/test_argument_validation.py index 36e36b23..bf9bbd1d 100644 --- a/tests/test_argument_validation.py +++ b/tests/test_argument_validation.py @@ -31,6 +31,19 @@ def rejection(): return ValidateActionArguments().rejection +@pytest.fixture(scope="module") +def listed_tools(): + """Tool name -> the input schema the client actually sees.""" + from fastmcp import FastMCP + + from plane_mcp.tools import register_tools + + mcp = FastMCP("test") + register_tools(mcp, legacy_names=False) + tools = asyncio.new_event_loop().run_until_complete(mcp.list_tools()) + return {tool.name: tool.parameters for tool in tools} + + # --- the rule ---------------------------------------------------------------- @@ -129,6 +142,42 @@ def test_every_action_of_every_resource_accepts_its_own_declaration(rejection): assert rejection(tool, arguments) is None, f"{tool} {action} rejected its own parameters" +# --- a default the client echoes back is not a choice ------------------------ + + +def test_a_call_padded_with_the_schema_defaults_is_never_refused(rejection, listed_tools): + """Reported by a customer: `workitem.archive` shipped with `default: true`, so a + client that fills every parameter from the schema sent it on `list`, `count`, + `search` and `retrieve_by_identifier` alike -- and each refused the call. Every + action of every tool must survive being padded out with its own advertised + defaults, because that padding chose nothing.""" + for name, schema in listed_tools.items(): + padding = { + param: prop["default"] + for param, prop in (schema.get("properties") or {}).items() + if param != "action" and "default" in prop + } + for action in action_arguments()[name]: + arguments = {"action": action, **padding} if "action" in schema["properties"] else dict(padding) + assert rejection(name, arguments) is None, f"{name}.{action} refused its own defaults" + + +def test_no_advertised_parameter_defaults_to_a_value_only_one_action_takes(listed_tools): + """The stray-argument rule reads a falsy value as "not chosen". A parameter whose + schema default is truthy is therefore indistinguishable from a deliberate one, so + it must be acceptable to every action of its tool.""" + accepted = action_arguments() + for name, schema in listed_tools.items(): + for param, prop in (schema.get("properties") or {}).items(): + if param == "action" or not prop.get("default"): + continue + refused = [a for a, takes in accepted[name].items() if param not in takes] + assert not refused, ( + f"{name}.{param} defaults to {prop['default']!r} but {', '.join(refused)} " + "would refuse it as a stray argument" + ) + + # --- the middleware, through a real server ----------------------------------- diff --git a/tests/tools/test_dispatch.py b/tests/tools/test_dispatch.py index 165957f3..1a26bb9b 100644 --- a/tests/tools/test_dispatch.py +++ b/tests/tools/test_dispatch.py @@ -307,6 +307,24 @@ def test_archiving_a_work_item_confirms_what_it_did(archive, registered, spy): assert spy.recorder.only().method == f"work_items.{verb}" +@pytest.mark.parametrize( + ("tool", "arguments", "method"), + [ + ("workitem", {"project_id": "p", "workitem_id": "w"}, "work_items.archive"), + ("page", {"project_id": "p", "page_id": "g"}, "pages.archive_project_page"), + ], + ids=["workitem", "page"], +) +def test_archive_omitted_still_archives(tool, arguments, method, registered, spy): + """`archive` used to default to `True` in the signature, which put `default: true` + in the advertised schema -- so a client that pads a call with the schema defaults + sent `archive` on every other action and each refused it as a stray argument. The + default is now unset, and unset has to keep meaning archive.""" + registered[tool].fn(action="archive", **arguments) + + assert spy.recorder.only().method == method + + # Initiatives roll up work items as well as projects. The two child collections # reach different SDK sub-resources, and `manage_workitems` is the only action on # this tool that issues two calls, so the order between them is pinned here. From e8495857e20a3144d4dd23c3377082c1a62ada6c Mon Sep 17 00:00:00 2001 From: Akhil Vamshi Konam Date: Mon, 21 Sep 2026 12:34:55 +0530 Subject: [PATCH 03/12] feat: introduce mentions module for user mentions in comments (#224) --- CLAUDE.md | 1 + plane_mcp/toolkit/README.md | 1 + plane_mcp/toolkit/__init__.py | 17 +- plane_mcp/toolkit/mentions.py | 180 +++++++++++++++++++ plane_mcp/tools/workitem_comment.py | 100 +++++++---- tests/tools/test_mentions.py | 256 ++++++++++++++++++++++++++++ 6 files changed, 523 insertions(+), 32 deletions(-) create mode 100644 plane_mcp/toolkit/mentions.py create mode 100644 tests/tools/test_mentions.py diff --git a/CLAUDE.md b/CLAUDE.md index 4feab874..655ddcd6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -103,6 +103,7 @@ Shared building blocks for the tool surface, split by *when* they act: |---|---|---| | `spec.py` | declaration | `Action`, `build_description`, `build_annotations` | | `runtime.py` | call | `missing`, `needs`, `require`, `one_of`, `opt`, `coerce_list`, `page_params`, `as_params`, `ids_of` | +| `mentions.py` | call | `render_mentions`, `tokenize_mentions`, `mention_user_ids`, `project_mention_error` | | `paging.py` | response | `envelope`, `dump_results`, `pql_failure`, `workitem_page` | | `governance.py` | policy | `workspace_owns_resource`, `GOVERNED_BY`, `workspace_owns`, `migration_in_progress`, `plan_gated` | | `transforms.py` | listing | `StripOutputSchemas` | diff --git a/plane_mcp/toolkit/README.md b/plane_mcp/toolkit/README.md index 805064b0..bf710da2 100644 --- a/plane_mcp/toolkit/README.md +++ b/plane_mcp/toolkit/README.md @@ -6,6 +6,7 @@ Shared building blocks for the tool surface, split by *when* they act. All re-ex |---|---|---| | `spec.py` | declaration | `Action`, `build_description`, `build_annotations` | | `runtime.py` | call | `missing`, `needs`, `require`, `one_of`, `opt`, `coerce_list`, `page_params`, `as_params`, `ids_of` | +| `mentions.py` | call | `render_mentions`, `tokenize_mentions`, `mention_user_ids`, `project_mention_error` | | `paging.py` | response | `envelope`, `dump_results`, `pql_failure`, `workitem_page` | | `governance.py` | policy | `workspace_owns_resource`, `workspace_owns`, `scoped`, `plan_gated` | | `transforms.py` | listing | `StripOutputSchemas` | diff --git a/plane_mcp/toolkit/__init__.py b/plane_mcp/toolkit/__init__.py index 0276da6a..297df2c0 100644 --- a/plane_mcp/toolkit/__init__.py +++ b/plane_mcp/toolkit/__init__.py @@ -9,10 +9,11 @@ Nothing here knows which catalogue is calling it, so a resource package can be renamed without these having to move with it. -The four modules split by *when* they act: +The modules split by *when* they act: spec declaration time -- describe a resource's actions runtime call time -- validate parameters, shape SDK payloads + mentions call time -- render `@[uuid]` into Plane's mention markup, and back paging response time -- Plane's pagination envelope and PQL failures governance refusal time -- recognise a workspace-owned resource, read its flag transforms listing time -- reshape the advertised catalogue @@ -33,6 +34,14 @@ workspace_owns, workspace_owns_resource, ) +from plane_mcp.toolkit.mentions import ( + MENTION_TOKEN, + mention_user_ids, + project_mention_error, + render_mentions, + tokenize_mentions, + unmentionable, +) from plane_mcp.toolkit.paging import dump_results, envelope, pql_failure, workitem_page from plane_mcp.toolkit.runtime import ( as_params, @@ -55,6 +64,7 @@ from plane_mcp.toolkit.transforms import StripOutputSchemas __all__ = [ + "MENTION_TOKEN", "WORK_ITEM_TYPES", "Action", "StripOutputSchemas", @@ -66,6 +76,7 @@ "dump_results", "envelope", "ids_of", + "mention_user_ids", "migration_in_progress", "missing", "needs", @@ -74,11 +85,15 @@ "rich_text", "page_params", "plan_gated", + "project_mention_error", "plan_required", "project_owns", "pql_failure", + "render_mentions", "require", "scoped", + "tokenize_mentions", + "unmentionable", "workspace_owns", "workspace_owns_resource", "workitem_page", diff --git a/plane_mcp/toolkit/mentions.py b/plane_mcp/toolkit/mentions.py new file mode 100644 index 00000000..e617de55 --- /dev/null +++ b/plane_mcp/toolkit/mentions.py @@ -0,0 +1,180 @@ +"""User mentions in Plane rich text. + +Plane shows a mention as a chip, and tells the person they were mentioned, for one +exact element:: + + +""" + +from __future__ import annotations + +import re +import uuid +from html.parser import HTMLParser +from typing import Any + +MENTION_TOKEN = "@[]" + +ELEMENT = "mention-component" +USER_MENTION = "user_mention" + +_UUID = r"[0-9a-fA-F]{8}-(?:[0-9a-fA-F]{4}-){3}[0-9a-fA-F]{12}" +_TOKEN = re.compile(rf"@\[\s*({_UUID})\s*\]") + + +def _canonical(value: Any) -> str | None: + """`value` as a canonical lowercase UUID, or None when it is not one.""" + try: + return str(uuid.UUID(str(value).strip())) + except (AttributeError, TypeError, ValueError): + return None + + +TEXT = object() +"""Marks a run of character data -- the only place a `@[uuid]` token can be written.""" + + +class _Scan(HTMLParser): + """A document as byte spans: mention elements, and the text between all markup. """ + + def __init__(self, html: str) -> None: + super().__init__(convert_charrefs=False) + self._html = html + self._line_starts = [0] + for line in html.splitlines(keepends=True): + self._line_starts.append(self._line_starts[-1] + len(line)) + self.spans: list[tuple[int, int, Any]] = [] + self.feed(html) + self.close() + + def _at(self) -> int: + line, column = self.getpos() + return self._line_starts[line - 1] + column + + def _record(self, tag: str, attrs: list[tuple[str, str | None]]) -> None: + if tag != ELEMENT: + return + start = self._at() + text = self.get_starttag_text() or "" + self.spans.append((start, start + len(text), {k.lower(): (v or "") for k, v in attrs})) + + handle_starttag = _record + handle_startendtag = _record + + def handle_data(self, data: str) -> None: + start = self._at() + self.spans.append((start, start + len(data), TEXT)) + + def handle_endtag(self, tag: str) -> None: + if tag != ELEMENT: + return + element = next((i for i in reversed(range(len(self.spans))) if self.spans[i][2] is not TEXT), None) + if element is None: + return + start, end, attrs = self.spans[element] + close = self._at() + if self._html[end:close].strip(): + return + self.spans[element] = (start, self._html.index(">", close) + 1, attrs) + del self.spans[element + 1 :] + + +def _mentioned_user(attrs: dict[str, str]) -> str | None: + """The user this element addresses, or None if it addresses something else.""" + if attrs.get("entity_name") != USER_MENTION: + return None + return _canonical(attrs.get("entity_identifier")) + + +def _element(user_id: str) -> str: + return ( + f'' + ) + + +def _segments(html: str): + """`html` as `(raw, kind)` pieces, in order, covering every byte exactly once. + + `kind` is `TEXT` for character data, a dict of attributes for a mention element, + and None for markup neither of those -- which is passed through verbatim. + """ + cursor = 0 + for start, end, kind in _Scan(html).spans: + if start > cursor: + yield html[cursor:start], None + yield html[start:end], kind + cursor = end + if cursor < len(html): + yield html[cursor:], None + + +def mention_user_ids(html: str | None) -> list[str]: + """Every user id `html` addresses, by token or by element, first occurrence first.""" + if not html: + return [] + found: dict[str, None] = {} + for raw, kind in _segments(html): + if kind is TEXT: + found.update(dict.fromkeys(filter(None, (_canonical(m[1]) for m in _TOKEN.finditer(raw))))) + elif isinstance(kind, dict) and (user_id := _mentioned_user(kind)): + found.setdefault(user_id, None) + return list(found) + + +def unnamed_mentions(html: str | None) -> bool: + """True if `html` holds a mention element with no `entity_name`.""" + return bool(html) and any(isinstance(kind, dict) and not kind.get("entity_name") for _, kind in _segments(html)) + + +def render_mentions(html: str) -> str: + """Turn every `@[uuid]` token into a mention element, leaving all else alone.""" + if not html: + return html + out = [] + for raw, kind in _segments(html): + if kind is TEXT: + out.append(_TOKEN.sub(lambda m: _element(_canonical(m[1]) or ""), raw)) + elif isinstance(kind, dict) and (user_id := _mentioned_user(kind)): + out.append(_element(user_id)) + else: + out.append(raw) + return "".join(out) + + +def tokenize_mentions(html: str | None) -> str | None: + """The reverse of `render_mentions`: user mention elements become `@[uuid]` tokens.""" + if not html: + return html + return "".join( + f"@[{user_id}]" if isinstance(kind, dict) and (user_id := _mentioned_user(kind)) else raw + for raw, kind in _segments(html) + ) + + +def unmentionable(html: str | None, may_be_mentioned: set[str]) -> list[str]: + """Ids `html` addresses that are not in `may_be_mentioned`, in the order written.""" + return [user_id for user_id in mention_user_ids(html) if user_id not in may_be_mentioned] + + +def project_mention_error(client: Any, workspace_slug: str, project_id: str, html: str | None) -> str | None: + """Why `html` cannot be posted to this project, or None when it can. """ + if unnamed_mentions(html): + return ( + f"Error: a <{ELEMENT}> here has no entity_name, so Plane would show a chip and tell " + f"nobody. Nothing was written. Write {MENTION_TOKEN} instead and the element is " + "generated for you." + ) + if not mention_user_ids(html): + return None + members = client.projects.get_members(workspace_slug=workspace_slug, project_id=project_id) + # A bot is a legitimate target, as it is in Plane's own mention picker. + active = {member.id for member in members if member.is_active is not False} + if unknown := unmentionable(html, active): + return ( + f"Error: {', '.join(unknown)} cannot be mentioned -- not an active member of this " + "project, so Plane would show the chip and tell nobody. Nothing was written. Resolve " + f"the person with `member list_project`, then re-send with {MENTION_TOKEN} for the id " + "it gives, or drop the mention." + ) + return None diff --git a/plane_mcp/tools/workitem_comment.py b/plane_mcp/tools/workitem_comment.py index baf3831a..1337ec23 100644 --- a/plane_mcp/tools/workitem_comment.py +++ b/plane_mcp/tools/workitem_comment.py @@ -1,14 +1,26 @@ -"""Comments on a work item.""" +"""Comments on a work item, including the people they mention.""" from __future__ import annotations -from typing import Literal +from typing import Any, Literal from fastmcp import FastMCP from plane.models.work_items import CreateWorkItemComment, UpdateWorkItemComment, WorkItemComment from plane_mcp.client import get_plane_client_context -from plane_mcp.toolkit import Action, build_annotations, build_description, missing, needs, opt, page_params +from plane_mcp.toolkit import ( + MENTION_TOKEN, + Action, + build_annotations, + build_description, + missing, + needs, + opt, + page_params, + project_mention_error, + render_mentions, + tokenize_mentions, +) NAME = "workitem_comment" TITLE = "Work item comments" @@ -29,7 +41,13 @@ Action("delete", ("project_id", "workitem_id", "comment_id"), destructive=True), ) -FOOTER = "comment_html is HTML, e.g. '

Looks good.

'. access is INTERNAL or EXTERNAL." +FOOTER = ( + "comment_html is HTML, e.g. '

Looks good.

'. access is INTERNAL or EXTERNAL.\n" + f"To mention someone, write {MENTION_TOKEN} inline -- '

{MENTION_TOKEN} can you review?

' -- " + "and the chip and the notification are generated for you. A bare @name is ordinary text " + "that notifies nobody. The id must belong to a member of the work item's project; " + "`member list_project` resolves a name to one. Mentions read back in the same form." +) LEGACY = { "list_work_item_comments": "list", @@ -40,6 +58,13 @@ } +def _legible(comment: Any) -> Any: + """A stored comment with its mention tags rewritten to `@[uuid]` tokens.""" + if html := getattr(comment, "comment_html", None): + comment.comment_html = tokenize_mentions(html) + return comment + + def register(mcp: FastMCP) -> None: @mcp.tool( name=NAME, @@ -64,51 +89,64 @@ def workitem_comment( return error if action == "list": - return client.work_items.comments.list( + page = client.work_items.comments.list( workspace_slug=workspace_slug, project_id=project_id, work_item_id=workitem_id, params=page_params(cursor, per_page), ) + for comment in page.results or []: + _legible(comment) + return page if action == "create": if not comment_html: return missing(action, "comment_html") - return client.work_items.comments.create( - workspace_slug=workspace_slug, - project_id=project_id, - work_item_id=workitem_id, - data=CreateWorkItemComment( - comment_html=comment_html, - access=opt(access), - external_source=opt(external_source), - external_id=opt(external_id), - ), + if error := project_mention_error(client, workspace_slug, project_id, comment_html): + return error + return _legible( + client.work_items.comments.create( + workspace_slug=workspace_slug, + project_id=project_id, + work_item_id=workitem_id, + data=CreateWorkItemComment( + comment_html=render_mentions(comment_html), + access=opt(access), + external_source=opt(external_source), + external_id=opt(external_id), + ), + ) ) if not comment_id: return missing(action, "comment_id") if action == "retrieve": - return client.work_items.comments.retrieve( - workspace_slug=workspace_slug, - project_id=project_id, - work_item_id=workitem_id, - comment_id=comment_id, + return _legible( + client.work_items.comments.retrieve( + workspace_slug=workspace_slug, + project_id=project_id, + work_item_id=workitem_id, + comment_id=comment_id, + ) ) if action == "update": - return client.work_items.comments.update( - workspace_slug=workspace_slug, - project_id=project_id, - work_item_id=workitem_id, - comment_id=comment_id, - data=UpdateWorkItemComment( - comment_html=opt(comment_html), - access=opt(access), - external_source=opt(external_source), - external_id=opt(external_id), - ), + if error := project_mention_error(client, workspace_slug, project_id, comment_html): + return error + return _legible( + client.work_items.comments.update( + workspace_slug=workspace_slug, + project_id=project_id, + work_item_id=workitem_id, + comment_id=comment_id, + data=UpdateWorkItemComment( + comment_html=opt(render_mentions(comment_html)), + access=opt(access), + external_source=opt(external_source), + external_id=opt(external_id), + ), + ) ) client.work_items.comments.delete( diff --git a/tests/tools/test_mentions.py b/tests/tools/test_mentions.py new file mode 100644 index 00000000..198611dc --- /dev/null +++ b/tests/tools/test_mentions.py @@ -0,0 +1,256 @@ +"""User mentions in comments: the `@[uuid]` contract and what it renders to.""" + +from __future__ import annotations + +import re + +import pytest +from plane.models.projects import ProjectMember +from plane.models.work_items import WorkItemComment + +from plane_mcp.toolkit.mentions import ( + mention_user_ids, + render_mentions, + tokenize_mentions, + unnamed_mentions, +) + +ALICE = "c23bd125-3548-44d0-a1fd-3d960b128646" +BOB = "7daf780b-df5d-4826-9365-71cf28bce5a1" +PROJECT = "a7cbd4a0-e9cb-46cc-a142-3a8582cd554c" + +# The whole element, in the order Plane's own matcher needs its attributes to exist. +TAG = re.compile( + r'' +) + + +# --- the markup -------------------------------------------------------------- + + +def test_a_token_becomes_the_whole_element(): + """All three attributes, every time: Plane matches on the tag *and* on + entity_name, so a tag missing one of them notifies nobody.""" + rendered = render_mentions(f"

Hey @[{ALICE}] please review

") + + ((node_id, user_id),) = TAG.findall(rendered) + assert user_id == ALICE + assert node_id != ALICE, "the node id is its own uuid, not the user's" + assert rendered.startswith("

Hey ") and rendered.endswith(" please review

") + + +def test_an_uppercase_or_padded_id_is_normalised(): + assert f'entity_identifier="{ALICE}"' in render_mentions(f"

@[ {ALICE.upper()} ]

") + + +def test_a_user_mention_missing_only_its_id_is_completed(): + """`id` is what the chip needs; the notification matcher does not read it. It can + be filled in without guessing, because entity_name already says who is meant.""" + rendered = render_mentions( + f'

hi

' + ) + + assert len(TAG.findall(rendered)) == 1 + assert rendered.endswith(" hi

") + + +def test_an_element_with_no_entity_name_is_reported_not_guessed(): + """There are six mention kinds. Assuming the missing one meant `user_mention` + could address a stranger, so it is refused instead.""" + html = f'

' + + assert unnamed_mentions(html) is True + assert mention_user_ids(html) == [], "it addresses nobody until it says what it is" + assert render_mentions(html) == html, "and it is not silently rewritten" + + +def test_an_issue_mention_is_left_alone(): + html = '

' + + assert render_mentions(html) == html + assert mention_user_ids(html) == [] + + +@pytest.mark.parametrize( + "html", + ["

@john, take a look

", "

mail user@example.com

", "

a@[b]

"], + ids=["at-name", "email", "not-a-uuid"], +) +def test_text_that_only_looks_like_a_mention_is_left_as_text(html): + assert render_mentions(html) == html + assert mention_user_ids(html) == [] + + +def test_a_mention_round_trips_through_a_read(): + """An update re-sends what a retrieve handed back; the mention has to survive it.""" + written = f"

@[{ALICE}] and @[{BOB}]

" + stored = render_mentions(written) + + assert tokenize_mentions(stored) == written + assert len(TAG.findall(render_mentions(tokenize_mentions(stored)))) == 2 + + +def test_every_mentioned_id_is_reported_once_in_the_order_written(): + html = f"

@[{ALICE}] {render_mentions(f'@[{ALICE}]')} @[{BOB}]

" + + assert mention_user_ids(html) == [ALICE, BOB] + + +# --- markup a regex would get wrong ------------------------------------------- + + +def test_the_other_five_mention_kinds_pass_through_untouched(): + """`TSearchEntities` has six kinds and only one addresses a person.""" + for kind in ("issue_mention", "project_mention", "cycle_mention", "module_mention", "page_mention"): + html = f'

' + assert render_mentions(html) == html, kind + assert tokenize_mentions(html) == html, kind + assert mention_user_ids(html) == [], kind + + +@pytest.mark.parametrize( + "element", + [ + f'', + f"", + f'', + f'', + ], + ids=["reordered", "single-quoted", "newlines-and-self-closing", "uppercase-tag"], +) +def test_the_element_is_recognised_however_it_is_spelled(element): + """Attribute order, quoting and whitespace are the caller's business, not ours -- + which is why this reads through html.parser rather than a pattern.""" + found = mention_user_ids(f"

{element}

") + + assert found == [ALICE] or "USER_MENTION" in element, element + + +def test_replacing_an_element_leaves_every_other_byte_alone(): + """Only the element's own span is rewritten, so surrounding markup -- including + markup a pattern might have swallowed -- survives exactly.""" + around = '
<mention-component>
' + html = f"{around}

@[{ALICE}]

" + + rendered = render_mentions(html) + + assert rendered.startswith(around) + assert "<mention-component>" in rendered, "an escaped example is text, not an element" + assert len(TAG.findall(rendered)) == 1 + + +def test_a_token_inside_an_attribute_is_prose_about_mentions_not_one(): + """Rendering it there would put a quote inside a quote and break the tag.""" + html = f'

how to

' + + assert mention_user_ids(html) == [] + assert render_mentions(html) == html + + +def test_a_stray_closing_tag_is_not_absorbed(): + html = f"

@[{ALICE}]

" + + assert render_mentions(html).endswith("

") + + +# --- through the tool -------------------------------------------------------- + + +def _members(*user_ids, active=True): + return [ProjectMember(id=user_id, is_active=active) for user_id in user_ids] + + +def _posted(spy): + """The comment_html that reached Plane.""" + return spy.recorder.calls[-1].kwargs["data"].comment_html + + +def test_creating_a_comment_sends_the_element_not_the_token(registered, spy): + spy.returns["projects.get_members"] = _members(ALICE) + + registered["workitem_comment"].fn( + action="create", project_id=PROJECT, workitem_id="w", comment_html=f"

@[{ALICE}] look

" + ) + + assert len(TAG.findall(_posted(spy))) == 1 + assert "@[" not in _posted(spy), "the token is ours, not Plane's" + + +def test_updating_a_comment_renders_its_mentions_too(registered, spy): + spy.returns["projects.get_members"] = _members(ALICE) + + registered["workitem_comment"].fn( + action="update", project_id=PROJECT, workitem_id="w", comment_id="c", comment_html=f"

@[{ALICE}]

" + ) + + assert len(TAG.findall(_posted(spy))) == 1 + + +@pytest.mark.parametrize("action", ["create", "update"], ids=["create", "update"]) +def test_a_mention_of_a_non_member_is_refused_before_anything_is_written(action, registered, spy): + """Plane accepts the mention and notifies nobody, so the comment would post + looking addressed to someone who was never told.""" + spy.returns["projects.get_members"] = _members(ALICE) + + result = registered["workitem_comment"].fn( + action=action, + project_id=PROJECT, + workitem_id="w", + comment_id="c", + comment_html=f"

@[{ALICE}] and @[{BOB}]

", + ) + + assert isinstance(result, str) and BOB in result + assert ALICE not in result, "only the id that cannot be mentioned is named" + assert spy.recorder.methods == ["projects.get_members"], "nothing was written" + + +def test_a_deactivated_member_cannot_be_mentioned(registered, spy): + spy.returns["projects.get_members"] = _members(ALICE, active=False) + + result = registered["workitem_comment"].fn( + action="create", project_id=PROJECT, workitem_id="w", comment_html=f"

@[{ALICE}]

" + ) + + assert isinstance(result, str) and ALICE in result + + +def test_a_comment_with_no_mention_never_asks_who_the_members_are(registered, spy): + """The check costs a request, so only a comment that mentions somebody pays it.""" + registered["workitem_comment"].fn( + action="create", project_id=PROJECT, workitem_id="w", comment_html="

Looks good.

" + ) + + assert spy.recorder.methods == ["work_items.comments.create"] + + +@pytest.mark.parametrize( + ("action", "arguments", "method"), + [ + ("retrieve", {"comment_id": "c"}, "work_items.comments.retrieve"), + ("create", {"comment_html": "

hi

"}, "work_items.comments.create"), + ], + ids=["retrieve", "create"], +) +def test_a_comment_reads_back_in_the_form_it_is_written(action, arguments, method, registered, spy): + spy.returns[method] = WorkItemComment(id="c", comment_html=render_mentions(f"

@[{ALICE}]

")) + + result = registered["workitem_comment"].fn(action=action, project_id=PROJECT, workitem_id="w", **arguments) + + assert result.comment_html == f"

@[{ALICE}]

" + + +def test_listing_comments_tokenizes_every_row(registered, spy): + class _Page: + results = [ + WorkItemComment(id="c1", comment_html=render_mentions(f"

@[{ALICE}]

")), + WorkItemComment(id="c2", comment_html="

plain

"), + ] + next_cursor = "c-2" + + spy.returns["work_items.comments.list"] = _Page() + + page = registered["workitem_comment"].fn(action="list", project_id=PROJECT, workitem_id="w") + + assert [comment.comment_html for comment in page.results] == [f"

@[{ALICE}]

", "

plain

"] From cb85bfc0b9151af7f65abad62b2c91f210251e85 Mon Sep 17 00:00:00 2001 From: Akhil Vamshi Konam Date: Mon, 21 Sep 2026 12:42:48 +0530 Subject: [PATCH 04/12] feat: enhance workitem_link tool to support title for links and update error handling (#225) --- plane_mcp/toolkit/mentions.py | 4 ++-- plane_mcp/tools/workitem_link.py | 27 ++++++++++++++------- tests/tools/test_dispatch.py | 40 ++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/plane_mcp/toolkit/mentions.py b/plane_mcp/toolkit/mentions.py index e617de55..af7fb98c 100644 --- a/plane_mcp/toolkit/mentions.py +++ b/plane_mcp/toolkit/mentions.py @@ -35,7 +35,7 @@ def _canonical(value: Any) -> str | None: class _Scan(HTMLParser): - """A document as byte spans: mention elements, and the text between all markup. """ + """A document as byte spans: mention elements, and the text between all markup.""" def __init__(self, html: str) -> None: super().__init__(convert_charrefs=False) @@ -158,7 +158,7 @@ def unmentionable(html: str | None, may_be_mentioned: set[str]) -> list[str]: def project_mention_error(client: Any, workspace_slug: str, project_id: str, html: str | None) -> str | None: - """Why `html` cannot be posted to this project, or None when it can. """ + """Why `html` cannot be posted to this project, or None when it can.""" if unnamed_mentions(html): return ( f"Error: a <{ELEMENT}> here has no entity_name, so Plane would show a chip and tell " diff --git a/plane_mcp/tools/workitem_link.py b/plane_mcp/tools/workitem_link.py index 9745f3df..112d26e9 100644 --- a/plane_mcp/tools/workitem_link.py +++ b/plane_mcp/tools/workitem_link.py @@ -8,7 +8,7 @@ from plane.models.work_items import CreateWorkItemLink, UpdateWorkItemLink, WorkItemLink from plane_mcp.client import get_plane_client_context -from plane_mcp.toolkit import Action, build_annotations, build_description, missing, needs, page_params +from plane_mcp.toolkit import Action, build_annotations, build_description, missing, needs, opt, page_params NAME = "workitem_link" TITLE = "Work item links" @@ -16,11 +16,21 @@ ACTIONS = ( Action("list", ("project_id", "workitem_id"), ("cursor", "per_page"), read=True), Action("retrieve", ("project_id", "workitem_id", "link_id"), read=True), - Action("create", ("project_id", "workitem_id", "url")), - Action("update", ("project_id", "workitem_id", "link_id", "url")), + Action("create", ("project_id", "workitem_id", "url"), ("title",)), + Action( + "update", + ("project_id", "workitem_id", "link_id"), + ("url", "title"), + note="pass at least one of url or title; only the fields you pass are changed", + ), Action("delete", ("project_id", "workitem_id", "link_id"), destructive=True), ) +FOOTER = ( + "url must be http:// or https://. title is the text Plane shows in place of the URL, " + "e.g. title='Design spec' for a long Figma link; without one the URL itself is shown." +) + LEGACY = { "list_work_item_links": "list", "retrieve_work_item_link": "retrieve", @@ -33,7 +43,7 @@ def register(mcp: FastMCP) -> None: @mcp.tool( name=NAME, - description=build_description("External links attached to a work item.", ACTIONS), + description=build_description("External links attached to a work item.", ACTIONS, FOOTER), annotations=build_annotations(TITLE, ACTIONS), ) def workitem_link( @@ -42,6 +52,7 @@ def workitem_link( workitem_id: str = "", link_id: str = "", url: str = "", + title: str = "", cursor: str = "", per_page: int = 0, ) -> WorkItemLink | list[WorkItemLink] | str | None: @@ -65,7 +76,7 @@ def workitem_link( workspace_slug=workspace_slug, project_id=project_id, work_item_id=workitem_id, - data=CreateWorkItemLink(url=url), + data=CreateWorkItemLink(url=url, title=opt(title)), ) if not link_id: @@ -80,14 +91,14 @@ def workitem_link( ) if action == "update": - if not url: - return missing(action, "url") + if not url and not title: + return missing(action, "url or title") return client.work_items.links.update( workspace_slug=workspace_slug, project_id=project_id, work_item_id=workitem_id, link_id=link_id, - data=UpdateWorkItemLink(url=url), + data=UpdateWorkItemLink(url=opt(url), title=opt(title)), ) client.work_items.links.delete( diff --git a/tests/tools/test_dispatch.py b/tests/tools/test_dispatch.py index 1a26bb9b..9fb25dc5 100644 --- a/tests/tools/test_dispatch.py +++ b/tests/tools/test_dispatch.py @@ -45,6 +45,7 @@ ("state", "create"): {"group": "started"}, ("template", "update"): {"name": "Renamed"}, ("collection", "update"): {"name": "Renamed"}, + ("workitem_link", "update"): {"title": "Renamed"}, ("cycle", "manage_workitems"): {"add_ids": "id-1"}, ("module", "manage_workitems"): {"add_ids": "id-1"}, ("milestone", "manage_workitems"): {"add_ids": "id-1"}, @@ -394,6 +395,45 @@ def test_managing_initiative_workitems_with_neither_side_asks_for_one(registered assert not _initiative_calls(spy), "the refusal must replace the call, not follow it" +# A link's title is the text Plane shows instead of the raw URL. The SDK and the API +# both took it; only this surface dropped it, so a link could never be labelled. + + +def _link_sent(spy): + return spy.recorder.only().kwargs["data"] + + +def test_a_link_is_created_with_the_title_it_was_given(registered, spy): + registered["workitem_link"].fn( + action="create", project_id="p", workitem_id="w", url="https://example.com/spec", title="Design spec" + ) + + sent = _link_sent(spy) + assert (sent.url, sent.title) == ("https://example.com/spec", "Design spec") + + +def test_a_link_created_without_a_title_sends_none(registered, spy): + """Omitted means Plane shows the URL; an empty string would be a title of nothing.""" + registered["workitem_link"].fn(action="create", project_id="p", workitem_id="w", url="https://example.com") + + assert "title" not in _link_sent(spy).model_dump(exclude_none=True) + + +def test_a_link_can_be_renamed_without_resending_its_url(registered, spy): + """Plane patches links partially, so a rename touches the title alone.""" + registered["workitem_link"].fn(action="update", project_id="p", workitem_id="w", link_id="l", title="Design spec") + + assert _link_sent(spy).model_dump(exclude_none=True) == {"title": "Design spec"} + + +def test_a_link_update_with_neither_field_is_refused_before_plane_sees_it(registered, spy): + """It would be an empty patch that reports success and changes nothing.""" + result = registered["workitem_link"].fn(action="update", project_id="p", workitem_id="w", link_id="l") + + assert result == "Error: action 'update' requires: url or title." + assert not spy.recorder.calls + + def test_no_description_warns_about_a_failure_the_caller_cannot_avoid(resource_modules, registered): """Naming a failure mode in a description buys a pre-flight probe on every run. From 5dbb3812f37b3d5eb331ae17ca8cd78081cc4b76 Mon Sep 17 00:00:00 2001 From: Akhil Vamshi Konam Date: Mon, 21 Sep 2026 13:08:31 +0530 Subject: [PATCH 05/12] feat: update workitem tool to handle null values for start and target dates (#226) --- plane_mcp/tools/workitem.py | 12 ++++++--- tests/tools/test_dispatch.py | 50 ++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/plane_mcp/tools/workitem.py b/plane_mcp/tools/workitem.py index a113b473..7458f57a 100644 --- a/plane_mcp/tools/workitem.py +++ b/plane_mcp/tools/workitem.py @@ -147,6 +147,8 @@ "resource first if you only have a name.\n" "description_stripped is plain text and is wrapped into HTML on save; description_html wins " "if both are given.\n" + "To clear a field on update: assignees=[] or labels=[], and start_date=null or " + "target_date=null. A field you leave out is not changed.\n" "fields is a sparse fieldset: use `project`, not project_id, and `description_html`, not " "description.\n" f"count group_by and sub_group_by accept: {', '.join(GROUP_BY_VALUES)}. These are grouping " @@ -213,8 +215,8 @@ def workitem( # noqa: PLR0911, PLR0912 - one branch per action is the point description_html: str = "", description_stripped: str = "", priority: str = "", - start_date: str = "", - target_date: str = "", + start_date: str | None = "", + target_date: str | None = "", sort_order: float = 0, parent: str = "", state: str = "", @@ -253,7 +255,8 @@ def retrieve_params() -> RetrieveQueryParams: ) def write_payload() -> dict[str, Any]: - return { + """The fields the caller supplied, and no others.""" + payload = { "name": opt(name), "assignees": coerce_list(assignees), "labels": coerce_list(labels), @@ -271,6 +274,9 @@ def write_payload() -> dict[str, Any]: "state": opt(state), "estimate_point": opt(estimate_point), } + supplied = {field: value for field, value in payload.items() if value is not None} + dates = {"start_date": start_date, "target_date": target_date} + return supplied | {field: None for field, value in dates.items() if value is None} if action in ("list", "list_archived"): if action == "list_archived" and not project_id: diff --git a/tests/tools/test_dispatch.py b/tests/tools/test_dispatch.py index 9fb25dc5..680fd103 100644 --- a/tests/tools/test_dispatch.py +++ b/tests/tools/test_dispatch.py @@ -433,6 +433,56 @@ def test_a_link_update_with_neither_field_is_refused_before_plane_sees_it(regist assert result == "Error: action 'update' requires: url or title." assert not spy.recorder.calls +def _update_sent(spy): + return spy.recorder.only().kwargs["data"].model_dump(exclude_unset=True) + + +@pytest.mark.parametrize("field", ["start_date", "target_date"]) +def test_a_date_set_to_null_is_cleared(field, registered, spy): + registered["workitem"].fn(action="update", project_id="p", workitem_id="w", **{field: None}) + + assert _update_sent(spy) == {field: None} + + +def test_an_update_sends_only_what_it_was_given(registered, spy): + """The regression this guards: every field used to reach the SDK, None when not + given, and only `exclude_none` kept that from wiping them. Correct the SDK + without this and a rename clears every date and the priority.""" + registered["workitem"].fn(action="update", project_id="p", workitem_id="w", name="Renamed") + + assert _update_sent(spy) == {"name": "Renamed"} + + +def test_clearing_one_date_leaves_the_other_alone(registered, spy): + registered["workitem"].fn( + action="update", project_id="p", workitem_id="w", start_date="2026-01-01", target_date=None + ) + + assert _update_sent(spy) == {"start_date": "2026-01-01", "target_date": None} + + +def test_a_date_left_at_its_default_is_not_sent(registered, spy): + """A client padding from the schema sends "" -- that chose nothing, so it clears nothing.""" + registered["workitem"].fn(action="update", project_id="p", workitem_id="w", name="x", target_date="") + + assert "target_date" not in _update_sent(spy) + + +@pytest.mark.parametrize( + ("sent", "arrives"), + [("", ""), (None, None), ("2026-01-01", "2026-01-01")], + ids=["padded-default", "json-null", "date"], +) +def test_argument_repair_keeps_the_default_distinct_from_null(sent, arrives, registered): + """The repair middleware turns "" into None for a field that is not a string. + A date accepts strings, so "" must reach the tool as "" -- or a padded default + would arrive as null and clear the date.""" + from plane_mcp.coercion import coerce_arguments + + repaired, _ = coerce_arguments({"target_date": sent}, registered["workitem"].parameters) + + assert repaired["target_date"] == arrives + def test_no_description_warns_about_a_failure_the_caller_cannot_avoid(resource_modules, registered): """Naming a failure mode in a description buys a pre-flight probe on every run. From 8e7842a4209d84a3a2cc9a53eaadb3fbc4a04711 Mon Sep 17 00:00:00 2001 From: Akhil Vamshi Konam Date: Mon, 21 Sep 2026 13:16:55 +0530 Subject: [PATCH 06/12] feat: update tools and client to support workspace retrieval and enhance action handling (#227) --- CLAUDE.md | 2 +- README.md | 2 +- plane_mcp/client.py | 25 +++++++++-- plane_mcp/tools/README.md | 4 +- plane_mcp/tools/workspace.py | 21 +++++++--- tests/test_client.py | 80 ++++++++++++++++++++++++++++++++++++ tests/tools/test_dispatch.py | 14 +++++++ 7 files changed, 135 insertions(+), 13 deletions(-) create mode 100644 tests/test_client.py diff --git a/CLAUDE.md b/CLAUDE.md index 655ddcd6..86766fa4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -81,7 +81,7 @@ Coercion runs before validation so an argument is judged by the value it repairs ### Tools (`tools/`) -One action-dispatch tool per Plane resource: **30 tools, 206 actions, ~67k chars advertised**. `tools/__init__.py` re-exports `register_tools`, so `server.py` and `__main__.py` see a single entry point. +One action-dispatch tool per Plane resource: **30 tools, 207 actions, ~67k chars advertised**. `tools/__init__.py` re-exports `register_tools`, so `server.py` and `__main__.py` see a single entry point. One module per resource, each exporting `NAME`, `ACTIONS`, `LEGACY` and `register(mcp)`. `ACTIONS` is the single source of truth: the tool description and its `ToolAnnotations` are generated from it, and the conformance suite asserts they agree with the function signature. See `tools/README.md` for the full convention. diff --git a/README.md b/README.md index d6ca26ca..9f4982a1 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ work items, cycles, modules, releases, customers and more. Built on [FastMCP](https://github.com/jlowin/fastmcp) and the official [`plane-sdk`](https://pypi.org/project/plane-sdk/). -- **30 tools**, one per Plane resource, covering 206 operations +- **30 tools**, one per Plane resource, covering 207 operations - **Local or remote** — stdio, streamable HTTP, SSE - **OAuth or API key** authentication diff --git a/plane_mcp/client.py b/plane_mcp/client.py index dd8c6b62..fe2db240 100644 --- a/plane_mcp/client.py +++ b/plane_mcp/client.py @@ -1,7 +1,7 @@ """Plane client initialization for MCP server.""" import os -from typing import NamedTuple +from typing import Any, NamedTuple from fastmcp.server.auth.auth import AccessToken from fastmcp.server.dependencies import get_access_token @@ -18,6 +18,26 @@ class PlaneClientContext(NamedTuple): workspace_slug: str +def _workspace_slug(token: AccessToken | None) -> str: + """The slug a connection is bound to: its token's claim, else the environment.""" + if token: + return token.claims.get("workspace_slug", "") + return os.getenv("PLANE_WORKSPACE_SLUG", "") + + +def current_workspace() -> dict[str, Any]: + """The workspace this connection is bound to, as far as its credentials say.""" + token = get_access_token() + claims = token.claims if token else {} + detail = claims.get("workspace") or {} + return { + "slug": _workspace_slug(token), + "id": detail.get("id"), + "name": detail.get("name"), + "connected_via": claims.get("auth_method", "environment"), + } + + def get_plane_client_context() -> PlaneClientContext: """ Initialize and return a PlaneClient instance with workspace context. @@ -38,18 +58,17 @@ def get_plane_client_context() -> PlaneClientContext: ConfigurationError: If access token is not available or workspace slug is missing """ base_url = os.getenv("PLANE_INTERNAL_BASE_URL") or os.getenv("PLANE_BASE_URL", "https://api.plane.so") - workspace_slug = os.getenv("PLANE_WORKSPACE_SLUG", "") api_key = os.getenv("PLANE_API_KEY", "") access_token = None # Get access token from the OAuth provider (which handles all auth methods) stored_access_token: AccessToken | None = get_access_token() + workspace_slug = _workspace_slug(stored_access_token) if stored_access_token: # Determine authentication method to use appropriate PlaneClient constructor auth_method = stored_access_token.claims.get("auth_method", "oauth") token = stored_access_token.token - workspace_slug = stored_access_token.claims.get("workspace_slug", "") # For API key auth methods, use api_key parameter; for OAuth, use access_token if auth_method in ("api_key_env", "api_key_header"): diff --git a/plane_mcp/tools/README.md b/plane_mcp/tools/README.md index 1bb6708e..f574f6c6 100644 --- a/plane_mcp/tools/README.md +++ b/plane_mcp/tools/README.md @@ -1,6 +1,6 @@ # The tool surface -**30 tools**, one per Plane resource, each taking an `action` parameter that selects the operation. 206 actions in total. +**30 tools**, one per Plane resource, each taking an `action` parameter that selects the operation. 207 actions in total. ```python workitem(action="create", project_id=..., name="Fix login") @@ -120,7 +120,7 @@ Where the workspace owns a resource outright, both directions of wrong-scope wri | `workitem_property` | `list` · `retrieve` · `create` · `update` · `delete` · `manage_type_properties` · `list_options` · `retrieve_option` · `create_option` · `update_option` · `delete_option` · `get_value` · `set_value` · `delete_value` | | `workitem_relation` | `list` · `create` · `delete` · `list_definitions` · `create_definition` · `update_definition` · `delete_definition` | | `workitem_type` | `list` · `retrieve` · `resolve` · `create` · `update` · `delete` · `import_to_project` | -| `workspace` | `get_features` · `update_features` | +| `workspace` | `retrieve` · `get_features` · `update_features` | Every tool's own description lists its actions with their required and optional parameters; that description is generated from `ACTIONS` and is the authoritative diff --git a/plane_mcp/tools/workspace.py b/plane_mcp/tools/workspace.py index c3cd0449..cc12339a 100644 --- a/plane_mcp/tools/workspace.py +++ b/plane_mcp/tools/workspace.py @@ -1,19 +1,25 @@ -"""Workspace-level feature flags.""" +"""The connected workspace, and its feature flags.""" from __future__ import annotations -from typing import Literal +from typing import Any, Literal from fastmcp import FastMCP from plane.models.workspaces import WorkspaceFeature -from plane_mcp.client import get_plane_client_context +from plane_mcp.client import current_workspace, get_plane_client_context from plane_mcp.toolkit import Action, build_annotations, build_description NAME = "workspace" TITLE = "Workspace settings" ACTIONS = ( + Action( + "retrieve", + note="the workspace this connection is bound to: slug, id, name and how it connected. " + "id and name are known only on an OAuth connection and are null otherwise", + read=True, + ), Action("get_features", note="feature flags for the current workspace", read=True), Action( "update_features", @@ -33,11 +39,11 @@ def register(mcp: FastMCP) -> None: @mcp.tool( name=NAME, - description=build_description("Workspace-level feature flags.", ACTIONS, FOOTER), + description=build_description("The connected workspace, and its feature flags.", ACTIONS, FOOTER), annotations=build_annotations(TITLE, ACTIONS), ) def workspace( - action: Literal["get_features", "update_features"], + action: Literal["retrieve", "get_features", "update_features"], # Tri-state throughout: False disables a feature, unset leaves it alone. project_grouping: bool | None = None, initiatives: bool | None = None, @@ -45,7 +51,10 @@ def workspace( customers: bool | None = None, wiki: bool | None = None, pi: bool | None = None, - ) -> WorkspaceFeature: + ) -> WorkspaceFeature | dict[str, Any]: + if action == "retrieve": + return current_workspace() + client, workspace_slug = get_plane_client_context() if action == "get_features": diff --git a/tests/test_client.py b/tests/test_client.py new file mode 100644 index 00000000..9a16f84d --- /dev/null +++ b/tests/test_client.py @@ -0,0 +1,80 @@ +"""Which workspace a connection is bound to, and that every surface agrees on it. + +A caller on a hosted client has no other way to learn the binding: the slug is +resolved from credentials it never sees. `current_workspace` reports it, and the +guarantee worth testing is that it reports the slug the tools actually act on -- +a report that could disagree with them would be worse than none. +""" + +from __future__ import annotations + +import time + +import pytest +from fastmcp.server.auth.auth import AccessToken + +import plane_mcp.client as client_module +from plane_mcp.client import current_workspace, get_plane_client_context + +WORKSPACE = {"id": "ws-1", "name": "Acme Inc", "slug": "acme", "logo_url": None} + + +def _token(**claims) -> AccessToken: + return AccessToken( + token="t", client_id="c", scopes=["read", "write"], expires_at=int(time.time()) + 3600, claims=claims + ) + + +# The claims each provider really issues: plane_oauth_provider.py and +# plane_header_auth_provider.py. stdio has no provider, so no token at all. +CONNECTIONS = { + "oauth": _token(auth_method="oauth", workspace_slug="acme", workspace=WORKSPACE), + "header": _token(auth_method="api_key_header", workspace_slug="acme"), + "stdio": None, +} + + +@pytest.fixture +def connected(monkeypatch): + """Bind the process to one connection, as the transport would.""" + monkeypatch.setenv("PLANE_WORKSPACE_SLUG", "acme") + monkeypatch.setenv("PLANE_API_KEY", "k") + + def bind(kind: str) -> None: + monkeypatch.setattr(client_module, "get_access_token", lambda: CONNECTIONS[kind]) + + return bind + + +def test_an_oauth_connection_names_its_workspace(connected): + """The installation the token was granted for carries the id and name, so they + cost nothing to report.""" + connected("oauth") + + assert current_workspace() == {"slug": "acme", "id": "ws-1", "name": "Acme Inc", "connected_via": "oauth"} + + +@pytest.mark.parametrize(("kind", "via"), [("header", "api_key_header"), ("stdio", "environment")]) +def test_a_key_connection_knows_only_the_slug_its_caller_set(kind, via, connected): + """No v1 endpoint describes a workspace, so id and name are left null rather + than guessed or fetched from somewhere that does not exist.""" + connected(kind) + + assert current_workspace() == {"slug": "acme", "id": None, "name": None, "connected_via": via} + + +@pytest.mark.parametrize("kind", sorted(CONNECTIONS)) +def test_the_reported_slug_is_the_one_every_tool_acts_on(kind, connected): + connected(kind) + + assert current_workspace()["slug"] == get_plane_client_context().workspace_slug + + +def test_a_token_without_a_slug_does_not_borrow_the_environments(connected, monkeypatch): + """A token's claim is authoritative even when empty. Falling back to the + environment would report -- and act on -- a workspace the token was never + granted.""" + monkeypatch.setattr(client_module, "get_access_token", lambda: _token(auth_method="oauth")) + + assert current_workspace()["slug"] == "" + assert get_plane_client_context().workspace_slug == "" diff --git a/tests/tools/test_dispatch.py b/tests/tools/test_dispatch.py index 680fd103..75b856ef 100644 --- a/tests/tools/test_dispatch.py +++ b/tests/tools/test_dispatch.py @@ -68,6 +68,7 @@ # Actions whose guard clause legitimately answers without calling the SDK. NO_CALL_EXPECTED: set[tuple[str, str]] = { ("get_pql_reference", "read"), # returns static reference text + ("workspace", "retrieve"), # answered from the connection's own credentials } # Actions that need populated remote state or an outbound HTTP fetch to get past @@ -433,6 +434,7 @@ def test_a_link_update_with_neither_field_is_refused_before_plane_sees_it(regist assert result == "Error: action 'update' requires: url or title." assert not spy.recorder.calls + def _update_sent(spy): return spy.recorder.only().kwargs["data"].model_dump(exclude_unset=True) @@ -484,6 +486,18 @@ def test_argument_repair_keeps_the_default_distinct_from_null(sent, arrives, reg assert repaired["target_date"] == arrives +def test_retrieving_the_workspace_asks_plane_nothing(registered, spy, monkeypatch): + """The binding is in the connection's own credentials, so a request would be waste. + `current_workspace` itself is covered in tests/test_client.py.""" + from plane_mcp.tools import workspace + + bound = {"slug": "acme", "id": "ws-1", "name": "Acme Inc", "connected_via": "oauth"} + monkeypatch.setattr(workspace, "current_workspace", lambda: bound) + + assert registered["workspace"].fn(action="retrieve") == bound + assert not spy.recorder.calls + + def test_no_description_warns_about_a_failure_the_caller_cannot_avoid(resource_modules, registered): """Naming a failure mode in a description buys a pre-flight probe on every run. From 6de1481c3b9d2600cf1c42b8fa9ef5eabb1756b5 Mon Sep 17 00:00:00 2001 From: Akhil Vamshi Konam Date: Mon, 21 Sep 2026 13:35:41 +0530 Subject: [PATCH 07/12] feat: enhance workitem_property tool to support rich text properties and improve error handling for value submissions (#228) --- plane_mcp/tools/workitem_property.py | 50 +++++++++++++----- tests/tools/test_dispatch.py | 1 + tests/tools/test_workitem_property.py | 73 +++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 14 deletions(-) diff --git a/plane_mcp/tools/workitem_property.py b/plane_mcp/tools/workitem_property.py index 19c98d66..6915564f 100644 --- a/plane_mcp/tools/workitem_property.py +++ b/plane_mcp/tools/workitem_property.py @@ -25,6 +25,7 @@ PropertySettings, PropertyType, RelationType, + RichTextValue, TextAttributeSettings, UpdateWorkItemProperty, UpdateWorkItemPropertyOption, @@ -130,9 +131,10 @@ Action("get_value", ("project_id", "workitem_id", "property_id"), read=True), Action( "set_value", - ("project_id", "workitem_id", "property_id", "value"), - ("external_source", "external_id"), - note="upsert; for a multi-value property this replaces every existing value", + ("project_id", "workitem_id", "property_id"), + ("value", "description_html", "external_source", "external_id"), + note="upsert; pass value, or description_html for a rich text property; for a " + "multi-value property this replaces every existing value", ), Action("delete_value", ("project_id", "workitem_id", "property_id"), destructive=True), ) @@ -156,7 +158,10 @@ "YYYY-MM-DD HH:MM:SS string; DECIMAL as a number; BOOLEAN as true or false; OPTION and " "RELATION as an option or record id string, or an array of them when the property is " 'multi-value. Send the value\'s own type, not a stringified form: "007" stays the text 007, ' - "whereas 7 is the number." + "whereas 7 is the number. " + "A rich text property is a RELATION with relation_type RICH_TEXT. Write it with " + "description_html (HTML, e.g. '

Notes

') instead of value; get_value returns the " + "content in value_detail.description_html, and its value is only that content's storage id." ) LEGACY = { @@ -241,6 +246,11 @@ def _options(options: str) -> list[CreateWorkItemPropertyOption] | None: raise ValueError(f"{OPTIONS_SHAPE}; one entry is unusable ({exc})") from exc +def _wants_rich_text(exc: HttpError) -> bool: + """Plane's refusal of a rich text property sent `value` instead of description_html.""" + return exc.status_code == 400 and "Rich text value must be an object" in str(exc.response) + + def _absent(exc: HttpError) -> bool: """Whether an error means "nothing here", as opposed to "the call failed". @@ -354,6 +364,7 @@ def workitem_property( options: str = "", display_format: str = "", value: str | bool | int | float | list[str] = "", + description_html: str = "", attach_ids: str = "", detach_ids: str = "", is_required: bool | None = None, @@ -403,16 +414,27 @@ def workitem_property( if action == "set_value": # Compared against "" rather than tested for falsiness: `False` # and `0` are values a BOOLEAN or DECIMAL property can hold. - if value == "": - return missing(action, "value") - return values.create( - **target, - data=CreateWorkItemPropertyValue( - value=value, - external_id=opt(external_id), - external_source=opt(external_source), - ), - ) + if value != "" and description_html: + return "Error: pass value or description_html, not both." + if value == "" and not description_html: + return missing(action, "value or description_html") + # Plane takes a rich text value as an object, not a bare HTML string. + try: + return values.create( + **target, + data=CreateWorkItemPropertyValue( + value=RichTextValue(description_html=description_html) if description_html else value, + external_id=opt(external_id), + external_source=opt(external_source), + ), + ) + except HttpError as exc: + if description_html or not _wants_rich_text(exc): + raise + return ( + "Error: this is a rich text property -- send its content as description_html, " + "e.g. description_html='

Notes

', not as value." + ) values.delete(**target) return None diff --git a/tests/tools/test_dispatch.py b/tests/tools/test_dispatch.py index 75b856ef..cd63b942 100644 --- a/tests/tools/test_dispatch.py +++ b/tests/tools/test_dispatch.py @@ -56,6 +56,7 @@ ("intake", "update"): {"status": 1}, ("workitem_relation", "create"): {"relation_type": "blocked_by"}, ("workitem_property", "manage_type_properties"): {"attach_ids": "id-1"}, + ("workitem_property", "set_value"): {"value": "text"}, ("project_estimate", "create_points"): {"points": '[{"value": "1", "key": 0}]'}, ("customer", "delete"): {"customer_id": "id-1"}, ("customer", "manage_workitems"): {"link_ids": "id-1"}, diff --git a/tests/tools/test_workitem_property.py b/tests/tools/test_workitem_property.py index 03b84c91..be3987ac 100644 --- a/tests/tools/test_workitem_property.py +++ b/tests/tools/test_workitem_property.py @@ -120,3 +120,76 @@ def test_list_still_falls_back_when_a_scope_is_genuinely_empty(registered, spy): result = registered["workitem_property"].fn(action="list", project_id="proj-1", workitem_type_id="type-1") assert result == [] + +RICH_TEXT_REFUSAL = HttpError("HTTP 400", 400, {"value": ["Rich text value must be an object"]}) + + +def _rich_text(registered, **arguments): + return registered["workitem_property"].fn( + action="set_value", project_id="p", workitem_id="w", property_id="pr", **arguments + ) + + +def test_a_rich_text_property_is_created_as_a_relation(registered, spy): + registered["workitem_property"].fn( + action="create", project_id="p", display_name="Notes", property_type="RELATION", relation_type="RICH_TEXT" + ) + + data = spy.recorder.calls[-1].kwargs["data"] + assert (data.property_type.value, data.relation_type.value) == ("RELATION", "RICH_TEXT") + + +def test_description_html_is_sent_as_the_object_plane_requires(registered, spy): + _rich_text(registered, description_html="

Notes

") + + sent = spy.recorder.only().kwargs["data"].model_dump(exclude_none=True) + assert sent == {"value": {"description_html": "

Notes

"}} + + +def test_value_and_description_html_together_are_refused(registered, spy): + result = _rich_text(registered, value="x", description_html="

x

") + + assert result == "Error: pass value or description_html, not both." + assert not spy.recorder.calls + + +def test_a_value_call_with_neither_names_both(registered, spy): + result = _rich_text(registered) + + assert result == "Error: action 'set_value' requires: value or description_html." + + +def test_html_sent_as_value_is_turned_toward_description_html(registered, spy): + """Plane's "must be an object" invites a nested object in value, which this schema + refuses -- so the refusal names the parameter that works instead.""" + spy.returns["work_item_properties.values.create"] = RICH_TEXT_REFUSAL + + result = _rich_text(registered, value="

Notes

") + + assert isinstance(result, str) and "description_html" in result + + +def test_an_unrelated_refusal_is_not_mistaken_for_rich_text(registered, spy): + spy.returns["work_item_properties.values.create"] = HttpError("HTTP 400", 400, {"value": ["Not a valid option"]}) + + with pytest.raises(HttpError): + _rich_text(registered, value="opt-x") + + +def test_a_rich_text_value_reads_back_with_its_html(registered, spy): + """For rich text, `value` is the stored content's id; the HTML rides beside it.""" + from plane.models.work_item_properties import WorkItemPropertyValueDetail + + spy.returns["work_item_properties.values.retrieve"] = WorkItemPropertyValueDetail.model_validate( + { + "id": "v", + "property_id": "pr", + "issue_id": "w", + "value": "desc-1", + "value_detail": {"id": "desc-1", "description_html": "

Notes

", "description_stripped": "Notes"}, + } + ) + + result = registered["workitem_property"].fn(action="get_value", project_id="p", workitem_id="w", property_id="pr") + + assert result.value_detail.description_html == "

Notes

" From c62602922f69d40e021c33a2dcabcf9653c63d92 Mon Sep 17 00:00:00 2001 From: Akhil Vamshi Konam Date: Mon, 21 Sep 2026 13:42:22 +0530 Subject: [PATCH 08/12] feat: refactor customer_property tool to use enums for property (#229) --- plane_mcp/tools/customer_property.py | 13 ++++---- tests/tools/test_customer_property.py | 48 +++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 7 deletions(-) create mode 100644 tests/tools/test_customer_property.py diff --git a/plane_mcp/tools/customer_property.py b/plane_mcp/tools/customer_property.py index aa250834..ad20c715 100644 --- a/plane_mcp/tools/customer_property.py +++ b/plane_mcp/tools/customer_property.py @@ -11,11 +11,10 @@ CustomerProperty, PaginatedCustomerPropertyResponse, PropertySettings, - PropertyType, - RelationType, SetCustomerPropertyValues, UpdateCustomerProperty, ) +from plane.models.enums import CustomerPropertyType, CustomerRelationType from plane.models.work_item_property_configurations import DateAttributeSettings, TextAttributeSettings from plane_mcp.client import get_plane_client_context @@ -34,8 +33,8 @@ NAME = "customer_property" TITLE = "Customer properties" -PROPERTY_TYPES = tuple(e.value for e in PropertyType) -RELATION_TYPES = tuple(e.value for e in RelationType) +PROPERTY_TYPES = tuple(e.value for e in CustomerPropertyType) +RELATION_TYPES = tuple(e.value for e in CustomerRelationType) TEXT_FORMATS = get_args(TextAttributeSettings.model_fields["display_format"].annotation) DATE_FORMATS = get_args(DateAttributeSettings.model_fields["display_format"].annotation) @@ -171,8 +170,8 @@ def customer_property( # The API requires `name` but stores a slug of display_name. name=display_name, display_name=display_name, - property_type=PropertyType(property_type), - relation_type=RelationType(relation_type) if relation_type else None, + property_type=CustomerPropertyType(property_type), + relation_type=CustomerRelationType(relation_type) if relation_type else None, description=opt(description), is_required=is_required, default_value=coerce_list(default_value, split=False), @@ -217,7 +216,7 @@ def customer_property( property_id=property_id, data=UpdateCustomerProperty( display_name=opt(display_name), - relation_type=RelationType(relation_type) if relation_type else None, + relation_type=CustomerRelationType(relation_type) if relation_type else None, description=opt(description), is_required=is_required, default_value=coerce_list(default_value, split=False), diff --git a/tests/tools/test_customer_property.py b/tests/tools/test_customer_property.py new file mode 100644 index 00000000..62ce6e10 --- /dev/null +++ b/tests/tools/test_customer_property.py @@ -0,0 +1,48 @@ +"""Customer properties take Plane's customer vocabulary, not the work item one.""" + +from __future__ import annotations + +import pytest +from plane.models.enums import CustomerPropertyType, CustomerRelationType + +from plane_mcp.tools.customer_property import PROPERTY_TYPES, RELATION_TYPES + + +def _create(registered, **arguments): + return registered["customer_property"].fn(action="create", display_name="Tier", **arguments) + + +def test_the_advertised_vocabulary_is_the_customer_one(registered): + description = registered["customer_property"].description + + assert set(PROPERTY_TYPES) == {member.value for member in CustomerPropertyType} + assert set(RELATION_TYPES) == {member.value for member in CustomerRelationType} + for work_item_only in ("FORMULA", "CASCADING", "RELEASE", "RICH_TEXT"): + assert work_item_only not in description, f"{work_item_only} is advertised on customers" + + +@pytest.mark.parametrize("property_type", ["FORMULA", "CASCADING"]) +def test_a_work_item_only_type_is_refused_before_plane_sees_it(property_type, registered, spy): + result = _create(registered, property_type=property_type) + + assert result.startswith("Error: property_type must be one of:") + assert not spy.recorder.calls + + +@pytest.mark.parametrize("action", ["create", "update"]) +@pytest.mark.parametrize("relation_type", ["RELEASE", "RICH_TEXT"]) +def test_a_work_item_only_relation_is_refused_before_plane_sees_it(action, relation_type, registered, spy): + result = registered["customer_property"].fn( + action=action, display_name="Tier", property_id="prop-1", property_type="RELATION", relation_type=relation_type + ) + + assert result.startswith("Error: relation_type must be one of:") + assert not spy.recorder.calls + + +def test_a_customer_type_reaches_the_sdk_in_the_customer_enum(registered, spy): + _create(registered, property_type="RELATION", relation_type="USER") + + data = spy.recorder.only().kwargs["data"] + assert data.property_type is CustomerPropertyType.RELATION + assert data.relation_type is CustomerRelationType.USER From e9421de772c6b6f6d522aa080dd5798b53b42c59 Mon Sep 17 00:00:00 2001 From: akhil-vamshi-konam Date: Mon, 21 Sep 2026 21:44:54 +0530 Subject: [PATCH 09/12] feat: enhance client authentication reporting by standardizing connection method retrieval --- plane_mcp/client.py | 14 ++++++++++---- tests/test_client.py | 12 ++++++++++++ tests/tools/test_workitem_property.py | 1 + 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/plane_mcp/client.py b/plane_mcp/client.py index fe2db240..32ee7407 100644 --- a/plane_mcp/client.py +++ b/plane_mcp/client.py @@ -25,16 +25,22 @@ def _workspace_slug(token: AccessToken | None) -> str: return os.getenv("PLANE_WORKSPACE_SLUG", "") +def _connected_via(token: AccessToken | None) -> str: + """How a connection authenticated: its token's claim, else the environment.""" + if token: + return token.claims.get("auth_method", "oauth") + return "environment" + + def current_workspace() -> dict[str, Any]: """The workspace this connection is bound to, as far as its credentials say.""" token = get_access_token() - claims = token.claims if token else {} - detail = claims.get("workspace") or {} + detail = (token.claims.get("workspace") if token else None) or {} return { "slug": _workspace_slug(token), "id": detail.get("id"), "name": detail.get("name"), - "connected_via": claims.get("auth_method", "environment"), + "connected_via": _connected_via(token), } @@ -67,7 +73,7 @@ def get_plane_client_context() -> PlaneClientContext: workspace_slug = _workspace_slug(stored_access_token) if stored_access_token: # Determine authentication method to use appropriate PlaneClient constructor - auth_method = stored_access_token.claims.get("auth_method", "oauth") + auth_method = _connected_via(stored_access_token) token = stored_access_token.token # For API key auth methods, use api_key parameter; for OAuth, use access_token diff --git a/tests/test_client.py b/tests/test_client.py index 9a16f84d..24dbffcc 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -70,6 +70,18 @@ def test_the_reported_slug_is_the_one_every_tool_acts_on(kind, connected): assert current_workspace()["slug"] == get_plane_client_context().workspace_slug +def test_a_token_that_names_no_method_is_reported_as_the_one_it_is_built_for(connected, monkeypatch): + """`auth_method` had two defaults: the client treated a missing claim as OAuth, + and the report called the same connection "environment". Whatever the default, + both have to read it the same way, or the report describes a connection the + tools are not using. A token is never the environment: that is stdio, with none. + """ + monkeypatch.setattr(client_module, "get_access_token", lambda: _token(workspace_slug="acme")) + + assert current_workspace()["connected_via"] == "oauth" + assert get_plane_client_context().client.config.access_token, "built for OAuth, so it carries the token" + + def test_a_token_without_a_slug_does_not_borrow_the_environments(connected, monkeypatch): """A token's claim is authoritative even when empty. Falling back to the environment would report -- and act on -- a workspace the token was never diff --git a/tests/tools/test_workitem_property.py b/tests/tools/test_workitem_property.py index be3987ac..f288467e 100644 --- a/tests/tools/test_workitem_property.py +++ b/tests/tools/test_workitem_property.py @@ -121,6 +121,7 @@ def test_list_still_falls_back_when_a_scope_is_genuinely_empty(registered, spy): assert result == [] + RICH_TEXT_REFUSAL = HttpError("HTTP 400", 400, {"value": ["Rich text value must be an object"]}) From 306241a3a682161587be7c3dbd6353d7a137e2d7 Mon Sep 17 00:00:00 2001 From: Akhil Vamshi Konam Date: Tue, 22 Sep 2026 09:11:04 +0530 Subject: [PATCH 10/12] feat: update OAuth redirect allowlist and enhance security tests (#232) --- plane_mcp/server.py | 2 + tests/test_oauth_security.py | 17 ++---- tests/test_redirect_allowlist.py | 92 ++++++++++++++++++++++++++++++++ tests/test_stateless_http.py | 13 +---- 4 files changed, 100 insertions(+), 24 deletions(-) create mode 100644 tests/test_redirect_allowlist.py diff --git a/plane_mcp/server.py b/plane_mcp/server.py index 2ccc2cc4..5bb7f5df 100644 --- a/plane_mcp/server.py +++ b/plane_mcp/server.py @@ -33,6 +33,8 @@ # ChatGPT connectors — per-connector callback + legacy redirect "https://chatgpt.com/connector/oauth/*", "https://chatgpt.com/connector_platform_oauth_redirect", + # Gemini custom apps (Spark) — the path carries a per-user, per-connector id + "https://oauth-redirect.googleusercontent.com/r/*", ] diff --git a/tests/test_oauth_security.py b/tests/test_oauth_security.py index 57a5fcfa..aa3e976d 100644 --- a/tests/test_oauth_security.py +++ b/tests/test_oauth_security.py @@ -21,20 +21,11 @@ from starlette.testclient import TestClient from plane_mcp.auth import PlaneOAuthProvider +from plane_mcp.server import get_allowed_client_redirect_uris -# Exact allowed patterns from plane_mcp/server.py -ALLOWED_REDIRECT_URI_PATTERNS = [ - "http://localhost:*", - "http://localhost:*/*", - "http://127.0.0.1:*", - "http://127.0.0.1:*/*", - "cursor://anysphere.cursor-mcp/oauth/*", - "https://www.cursor.com/*", - "https://vscode.dev/redirect", - "https://insiders.vscode.dev/redirect", - "https://antigravity.google/oauth-callback", - "https://claude.ai/*", -] +# The allowlist the server actually ships. Copying it here let it drift, and a +# stale copy silently stops testing the patterns in production. +ALLOWED_REDIRECT_URI_PATTERNS = get_allowed_client_redirect_uris() @pytest.fixture(scope="module") diff --git a/tests/test_redirect_allowlist.py b/tests/test_redirect_allowlist.py new file mode 100644 index 00000000..c5c8ae5a --- /dev/null +++ b/tests/test_redirect_allowlist.py @@ -0,0 +1,92 @@ +"""The OAuth redirect allowlist, checked against the real callback URIs. + +Every entry in DEFAULT_ALLOWED_REDIRECT_URIS exists because one MCP client +could not connect without it, and the pattern only works if it matches the URI +that client actually sends. These tests pin both halves: the real URI is +accepted, and the lookalikes an attacker would try are not. + +Validation runs through fastmcp's `validate_redirect_uri` — the same function +the OAuth proxy calls, so a change in its matching semantics fails here rather +than in production. +""" + +import pytest +from fastmcp.server.auth.redirect_validation import validate_redirect_uri + +from plane_mcp.server import DEFAULT_ALLOWED_REDIRECT_URIS, get_allowed_client_redirect_uris + + +def allowed(uri: str, patterns: list[str] | None = None) -> bool: + return validate_redirect_uri(uri, patterns or DEFAULT_ALLOWED_REDIRECT_URIS) + + +# A real callback URI per supported client. The Gemini and ChatGPT paths carry a +# per-user connector id, so the URI below is a sample of that shape. +CLIENT_REDIRECT_URIS = { + "localhost-dynamic-port": "http://localhost:54321/callback", + "loopback-ip": "http://127.0.0.1:8080/oauth/callback", + "cursor-scheme": "cursor://anysphere.cursor-mcp/oauth/callback", + "cursor-web": "https://www.cursor.com/api/auth/mcp/callback", + "vscode": "https://vscode.dev/redirect", + "vscode-insiders": "https://insiders.vscode.dev/redirect", + "antigravity": "https://antigravity.google/oauth-callback", + "claude-ai": "https://claude.ai/api/mcp/auth_callback", + "chatgpt-connector": "https://chatgpt.com/connector/oauth/callback", + "chatgpt-legacy": "https://chatgpt.com/connector_platform_oauth_redirect", + "gemini-spark": ( + "https://oauth-redirect.googleusercontent.com/r/user_bound_custom-mcp-116109532806053202916-mcp_plane_so" + ), +} + + +@pytest.mark.parametrize("uri", CLIENT_REDIRECT_URIS.values(), ids=CLIENT_REDIRECT_URIS.keys()) +def test_supported_client_callback_is_allowed(uri: str) -> None: + assert allowed(uri), f"{uri} is rejected — that client cannot complete OAuth" + + +# Each one is a near-miss on a pattern above: the same string a phishing client +# would register hoping the allowlist matches on a substring. +LOOKALIKE_URIS = { + "unrelated-domain": "https://attacker.com/steal", + "gemini-host-suffix": "https://oauth-redirect.googleusercontent.com.evil.com/r/user_bound_custom-mcp-1", + "gemini-host-prefix": "https://evil-oauth-redirect.googleusercontent.com/r/user_bound_custom-mcp-1", + "gemini-userinfo": "https://oauth-redirect.googleusercontent.com@evil.com/r/user_bound_custom-mcp-1", + "gemini-parent-domain": "https://googleusercontent.com/r/user_bound_custom-mcp-1", + "gemini-scheme-downgrade": "http://oauth-redirect.googleusercontent.com/r/user_bound_custom-mcp-1", + "gemini-other-path": "https://oauth-redirect.googleusercontent.com/oauth2/callback", + "claude-host-suffix": "https://claude.ai.evil.com/api/mcp/auth_callback", + "chatgpt-path-suffix": "https://chatgpt.com/connector_platform_oauth_redirect_evil", + "cursor-unregistered-scheme": "myapp://callback", + "javascript-scheme": "javascript:alert(document.cookie)", +} + + +@pytest.mark.parametrize("uri", LOOKALIKE_URIS.values(), ids=LOOKALIKE_URIS.keys()) +def test_lookalike_callback_is_refused(uri: str) -> None: + assert not allowed(uri), f"VULNERABILITY: {uri} matches an allowed pattern" + + +class TestRuntimeExtras: + """PLANE_OAUTH_ALLOWED_REDIRECT_URIS onboards a client without a release.""" + + def test_extras_are_appended_after_the_defaults(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv( + "PLANE_OAUTH_ALLOWED_REDIRECT_URIS", " https://new-client.example.com/cb , https://claude.ai/* " + ) + + patterns = get_allowed_client_redirect_uris() + + assert patterns[: len(DEFAULT_ALLOWED_REDIRECT_URIS)] == DEFAULT_ALLOWED_REDIRECT_URIS + assert patterns[len(DEFAULT_ALLOWED_REDIRECT_URIS) :] == ["https://new-client.example.com/cb"] + assert allowed("https://new-client.example.com/cb", patterns) + + def test_the_defaults_stand_alone_without_the_env_var(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("PLANE_OAUTH_ALLOWED_REDIRECT_URIS", raising=False) + + assert get_allowed_client_redirect_uris() == DEFAULT_ALLOWED_REDIRECT_URIS + + def test_the_returned_list_is_a_copy(self) -> None: + """A caller mutating the result must not edit the shipped allowlist.""" + get_allowed_client_redirect_uris().append("https://attacker.com/*") + + assert not allowed("https://attacker.com/steal") diff --git a/tests/test_stateless_http.py b/tests/test_stateless_http.py index 9e1639f2..50cea664 100644 --- a/tests/test_stateless_http.py +++ b/tests/test_stateless_http.py @@ -11,18 +11,9 @@ from starlette.testclient import TestClient from plane_mcp.auth import PlaneHeaderAuthProvider, PlaneOAuthProvider +from plane_mcp.server import get_allowed_client_redirect_uris -ALLOWED_REDIRECT_URI_PATTERNS = [ - "http://localhost:*", - "http://localhost:*/*", - "http://127.0.0.1:*", - "http://127.0.0.1:*/*", - "cursor://*", - "vscode://*", - "vscode-insiders://*", - "windsurf://*", - "claude://*", -] +ALLOWED_REDIRECT_URI_PATTERNS = get_allowed_client_redirect_uris() @pytest.fixture() From 9c15c917a8b1b736b4506be3f8663bb786a62d87 Mon Sep 17 00:00:00 2001 From: akhil-vamshi-konam Date: Tue, 22 Sep 2026 09:16:19 +0530 Subject: [PATCH 11/12] chore: bump version to 0.3.3 in pyproject.toml --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index cc983256..0e8872e2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "plane-mcp-server" -version = "0.3.2" +version = "0.3.3" description = "A Model Context Protocol server for Plane integration" readme = "README.md" requires-python = ">=3.10" From 03d0a09f0212dcb8c62a550c2b99dbfc32aa775f Mon Sep 17 00:00:00 2001 From: akhil-vamshi-konam Date: Tue, 22 Sep 2026 14:18:49 +0530 Subject: [PATCH 12/12] feat: add grok.com to OAuth redirect allowlist --- plane_mcp/server.py | 1 + tests/test_redirect_allowlist.py | 3 +++ 2 files changed, 4 insertions(+) diff --git a/plane_mcp/server.py b/plane_mcp/server.py index 5bb7f5df..a7886d65 100644 --- a/plane_mcp/server.py +++ b/plane_mcp/server.py @@ -35,6 +35,7 @@ "https://chatgpt.com/connector_platform_oauth_redirect", # Gemini custom apps (Spark) — the path carries a per-user, per-connector id "https://oauth-redirect.googleusercontent.com/r/*", + "https://grok.com/connectors-oauth-exchange-code/", ] diff --git a/tests/test_redirect_allowlist.py b/tests/test_redirect_allowlist.py index c5c8ae5a..b6e5633b 100644 --- a/tests/test_redirect_allowlist.py +++ b/tests/test_redirect_allowlist.py @@ -36,6 +36,7 @@ def allowed(uri: str, patterns: list[str] | None = None) -> bool: "gemini-spark": ( "https://oauth-redirect.googleusercontent.com/r/user_bound_custom-mcp-116109532806053202916-mcp_plane_so" ), + "grok-web": "https://grok.com/connectors-oauth-exchange-code/", } @@ -54,6 +55,8 @@ def test_supported_client_callback_is_allowed(uri: str) -> None: "gemini-parent-domain": "https://googleusercontent.com/r/user_bound_custom-mcp-1", "gemini-scheme-downgrade": "http://oauth-redirect.googleusercontent.com/r/user_bound_custom-mcp-1", "gemini-other-path": "https://oauth-redirect.googleusercontent.com/oauth2/callback", + "grok-host-suffix": "https://grok.com.evil.com/connectors-oauth-exchange-code/", + "grok-other-path": "https://grok.com/anything-else", "claude-host-suffix": "https://claude.ai.evil.com/api/mcp/auth_callback", "chatgpt-path-suffix": "https://chatgpt.com/connector_platform_oauth_redirect_evil", "cursor-unregistered-scheme": "myapp://callback",