feat: implement MCP server wrapping groupme-sdk #2
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "1-feat-implement-mcp-server-wrapping-group"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
MCP server wrapping groupme-sdk so AI agents can manage GroupMe groups, members, and messages from conversations. Follows pal-e-docs-mcp patterns exactly: FastMCP decorators, hatchling build, uv for dependency management, Forgejo PyPI for groupme-sdk.
Changes
pyproject.toml-- project config with groupme-sdk from Forgejo PyPI, hatchling build, ruff + pytest configsrc/groupme_mcp/server.py-- FastMCP app, GroupMeClient lifecycle, _ok/_error_response helperssrc/groupme_mcp/__main__.py-- entry point forpython -m groupme_mcpsrc/groupme_mcp/tools/__init__.py-- tool registration via module importssrc/groupme_mcp/tools/groups.py-- create_group, list_groups, get_groupsrc/groupme_mcp/tools/members.py-- add_member, remove_member, list_memberssrc/groupme_mcp/tools/messages.py-- send_messagetests/conftest.py-- mock_client fixture patching get_client in all tool modulestests/test_groups.py-- 8 tests covering group tools + error handlingtests/test_members.py-- 8 tests covering member tools + error handlingtests/test_messages.py-- 3 tests covering send_message + error handling.gitignore-- standard Python ignoresTest Plan
pytest tests/-- 20 tests, all passingruff format+ruff check-- cleanReview Checklist
pytest tests/-- 20 passed)ruff format+ruff check)Related
plan-wkqforgejo_admin/basketball-api#157Review
Verdict: Approved -- clean implementation, no blockers.
What looks good
get_client()lazy singleton matchesget_sdk()pattern from reference_error_responsecorrectly handlesGroupMeHTTPError(with status_code + response_body),GroupMeError, and generic exceptionsconftest.pypatchesget_clientin each tool module's namespace (same monkeypatch pattern as pal-e-docs-mcp)pyproject.tomlmatches reference structure (hatchling, uv.sources, forgejo index, ruff config).gitignorecovers pycache, venv, egg-info, build artifactsNo issues found
PR #2 Review
DOMAIN REVIEW
Tech stack: Python 3.12+ / FastMCP / groupme-sdk / hatchling / uv / pytest / ruff
This is an MCP server wrapping groupme-sdk, following the established pal-e-docs-mcp pattern. I verified against the reference implementation at
/home/ldraney/pal-e-docs-mcp/.Pattern fidelity (pal-e-docs-mcp): Near-perfect match.
pyproject.tomlstructure: identical (hatchling build, uv sources, Forgejo PyPI index, ruff config, pytest config)server.pylifecycle: same singleton pattern (_client/get_client()), same_ok()/_error_response()helpers, sameregister_all_tools()import-at-module-level patterntools/__init__.pyregistration: identical pattern withnoqa: F401on lazy importsfrom ..server import _error_response, _ok, get_client, mcpimport line, sameAnnotated[..., Field(...)]parameter typing, same try/except-return-string patternPython/PEP compliance:
from __future__ import annotations: used consistentlyError handling architecture:
_error_response()inserver.pyhas a three-tier dispatch:GroupMeHTTPError(includes status_code + optional response_body),GroupMeError(detail only), genericException(detail only). TheGroupMeErrorand genericExceptionbranches produce identical output -- this is intentional (matches pal-e-docs-mcp pattern where SDK errors and generic errors both fall through to the same format).SDK integration:
GroupMeClient()constructor readsGROUPME_ACCESS_TOKENfrom env (verified in groupme-sdk source at/home/ldraney/groupme-sdk/src/groupme_sdk/client.py:33). The MCP server'sget_client()delegates this correctly.GroupMeNotFoundErroris a subclass ofGroupMeHTTPErrorwith hardcodedstatus_code=404(verified in/home/ldraney/groupme-sdk/src/groupme_sdk/exceptions.py:19-23). The_error_responsehandler catches it via theisinstance(exc, GroupMeHTTPError)branch. Correct.Test quality:
monkeypatch.setattron each tool module'sget_clientreference. This is the correct approach since each module importsget_clientinto its own namespace.GroupMeNotFoundErrorcorrectly verifiesstatus_code == 404in the response.BLOCKERS
None.
All BLOCKER criteria pass:
Annotated[..., Field(...)]with descriptions. Theadd_membertool correctly delegates the "at least one of phone/email/user_id" validation to the SDK (which raisesValueError), and this is testedGROUPME_ACCESS_TOKENis read from environment, never hardcodedget_client()function, no duplicated auth logicNITS
Missing
README.mdupdate --pyproject.tomldeclaresreadme = "README.md"and the existing README on main is a 2-line stub. For a publishable package, the README should document installation, configuration (GROUPME_ACCESS_TOKEN), available tools, and usage with Claude/MCP. Not a blocker for an internal tool, but worth a follow-up._clientglobal reset -- The singleton_clientinserver.pyhas noreset_client()function. If the token changes or a test needs to swap clients, there is no clean way to reset. The reference implementation (pal-e-docs-mcp) has the same limitation, so this is a known pattern gap, not a regression.add_membermissing server-side validation -- The tool relies entirely on the SDK to validate that at least one ofphone_number,email, oruser_idis provided. This is fine (SDK raisesValueError, which is caught and returned as an error), but a@mcp.tool()docstring note that the SDK enforces this constraint would aid agent comprehension.tests/__init__.pymissing -- No__init__.pyin the tests directory. Not required by modern pytest, but present in some project conventions.SOP COMPLIANCE
1-feat-implement-mcp-server-wrapping-group(matches issue #1)plan-wkqreferenced in Related sectionforgejo_admin/basketball-api#157feat: implement MCP server wrapping groupme-sdk)PROCESS OBSERVATIONS
membership_audittool correctly deferred (depends on basketball-api DB tables). This is good discipline -- ship what works, defer what has external dependencies.VERDICT: APPROVED