feat: add DM and file attachment MCP tools #8

Merged

forgejo_admin merged 1 commit from 7-add-dm-and-file-attachment-tools-to-grou into main 2026-03-28 19:30:56 +00:00

Conversation 2 Commits 1 Files changed 6 +360

forgejo_admin commented 2026-03-28 19:26:23 +00:00

Owner

Copy link

Summary

Add four new MCP tools wrapping the groupme-sdk DirectMessagesMixin and FilesMixin, bringing the total tool count from 8 to 12. Depends on groupme-sdk#8 merging first (SDK already installed from feature branch for development).

Changes

• src/groupme_mcp/tools/direct_messages.py -- new module with send_dm and list_dm_chats tools
• src/groupme_mcp/tools/files.py -- new module with upload_file and get_upload_status tools (group resolved by name)
• src/groupme_mcp/tools/__init__.py -- register new tool modules in register_all_tools()
• tests/conftest.py -- patch get_client and _resolve_group in new modules
• tests/test_direct_messages.py -- 8 tests covering send_dm and list_dm_chats
• tests/test_files.py -- 8 tests covering upload_file and get_upload_status

Review Checklist

• ruff format clean
• ruff check clean
• 54 tests pass (16 new + 38 existing)
• Server imports cleanly with all 12 tools registered
• Follows existing tool module patterns (Annotated fields, _error_response, _ok)
• Merge groupme-sdk#8 first (SDK dependency)

Test Plan

• 54 tests pass (16 new + 38 existing)
• Server imports cleanly with all 12 tools registered
• ruff format and ruff check clean

Related Notes

• None

Related

Closes #7

• Dependency: forgejo_admin/groupme-sdk#8 (DirectMessagesMixin + FilesMixin)

## Summary Add four new MCP tools wrapping the groupme-sdk DirectMessagesMixin and FilesMixin, bringing the total tool count from 8 to 12. Depends on groupme-sdk#8 merging first (SDK already installed from feature branch for development). ## Changes - `src/groupme_mcp/tools/direct_messages.py` -- new module with `send_dm` and `list_dm_chats` tools - `src/groupme_mcp/tools/files.py` -- new module with `upload_file` and `get_upload_status` tools (group resolved by name) - `src/groupme_mcp/tools/__init__.py` -- register new tool modules in `register_all_tools()` - `tests/conftest.py` -- patch `get_client` and `_resolve_group` in new modules - `tests/test_direct_messages.py` -- 8 tests covering send_dm and list_dm_chats - `tests/test_files.py` -- 8 tests covering upload_file and get_upload_status ## Review Checklist - [x] ruff format clean - [x] ruff check clean - [x] 54 tests pass (16 new + 38 existing) - [x] Server imports cleanly with all 12 tools registered - [x] Follows existing tool module patterns (Annotated fields, _error_response, _ok) - [ ] Merge groupme-sdk#8 first (SDK dependency) ## Test Plan - 54 tests pass (16 new + 38 existing) - Server imports cleanly with all 12 tools registered - `ruff format` and `ruff check` clean ## Related Notes - None ## Related Closes #7 - Dependency: forgejo_admin/groupme-sdk#8 (DirectMessagesMixin + FilesMixin)

forgejo_admin added 1 commit 2026-03-28 19:26:23 +00:00

feat: add DM and file attachment MCP tools ... 112bf7a891

Add four new tools wrapping the groupme-sdk DirectMessagesMixin and FilesMixin:
- send_dm: send direct messages to users by recipient_id
- list_dm_chats: list DM conversations with pagination
- upload_file: upload files to group storage (resolved by name)
- get_upload_status: poll file upload job status

Includes 16 new tests covering success, error, and edge cases.
Depends on groupme-sdk#7 (DirectMessagesMixin + FilesMixin).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented 2026-03-28 19:27:11 +00:00

Author

Owner

Copy link

QA Review -- PR #8

Pattern Compliance

• Module docstrings follow existing convention ("X tools -- verb phrase")
• Imports from ..server match existing pattern (_error_response, _ok, get_client, mcp, _resolve_group)
• @mcp.tool() decorator on all four functions
• Annotated[..., Field(description=...)] on all parameters
• try/except Exception with _error_response(exc) for all tools
• Return type is str (JSON) on all tools
• _resolve_group used for group-scoped tools (files.py), not used for user-scoped tools (direct_messages.py) -- correct

Tool Registration

• __init__.py registers both new modules in alphabetical order
• Server starts with all 12 tools visible

Test Coverage

• conftest patches get_client in both new modules
• conftest patches _resolve_group in files_mod (the only new module that uses it)
• 8 tests for direct_messages (3 send_dm + 5 list_dm_chats)
• 8 tests for files (5 upload_file + 3 get_upload_status)
• Tests cover: happy path, HTTP error, generic error, pagination, edge cases

SDK Alignment

• send_dm(recipient_id, text) matches SDK DirectMessagesMixin.send_dm signature
• list_chats(page, per_page) matches SDK DirectMessagesMixin.list_chats signature
• upload_file(group_id, file_path, content_type) matches SDK FilesMixin.upload_file signature
• get_upload_status(group_id, job_id) matches SDK FilesMixin.get_upload_status signature

Nits

• None found. Code is clean, consistent with existing patterns, and well-tested.

Dependency Note

• SDK dependency on groupme-sdk#8 is documented in PR body and review checklist. Merge order: SDK first, then this PR.

VERDICT: APPROVED

## QA Review -- PR #8 ### Pattern Compliance - [x] Module docstrings follow existing convention ("X tools -- verb phrase") - [x] Imports from `..server` match existing pattern (`_error_response`, `_ok`, `get_client`, `mcp`, `_resolve_group`) - [x] `@mcp.tool()` decorator on all four functions - [x] `Annotated[..., Field(description=...)]` on all parameters - [x] `try/except Exception` with `_error_response(exc)` for all tools - [x] Return type is `str` (JSON) on all tools - [x] `_resolve_group` used for group-scoped tools (files.py), not used for user-scoped tools (direct_messages.py) -- correct ### Tool Registration - [x] `__init__.py` registers both new modules in alphabetical order - [x] Server starts with all 12 tools visible ### Test Coverage - [x] conftest patches `get_client` in both new modules - [x] conftest patches `_resolve_group` in `files_mod` (the only new module that uses it) - [x] 8 tests for direct_messages (3 send_dm + 5 list_dm_chats) - [x] 8 tests for files (5 upload_file + 3 get_upload_status) - [x] Tests cover: happy path, HTTP error, generic error, pagination, edge cases ### SDK Alignment - [x] `send_dm(recipient_id, text)` matches SDK `DirectMessagesMixin.send_dm` signature - [x] `list_chats(page, per_page)` matches SDK `DirectMessagesMixin.list_chats` signature - [x] `upload_file(group_id, file_path, content_type)` matches SDK `FilesMixin.upload_file` signature - [x] `get_upload_status(group_id, job_id)` matches SDK `FilesMixin.get_upload_status` signature ### Nits - None found. Code is clean, consistent with existing patterns, and well-tested. ### Dependency Note - SDK dependency on `groupme-sdk#8` is documented in PR body and review checklist. Merge order: SDK first, then this PR. **VERDICT: APPROVED**

forgejo_admin commented 2026-03-28 19:29:31 +00:00

Author

Owner

Copy link

PR #8 Review

DOMAIN REVIEW

Tech stack: Python 3.12+ / FastMCP / groupme-sdk / pytest / Pydantic Field annotations

Pattern compliance: Both new tool modules (direct_messages.py, files.py) follow the established pattern from groups.py, members.py, and messages.py exactly:

• Same from ..server import _error_response, _ok, get_client, mcp (plus _resolve_group where needed)
• Same @mcp.tool() decorator with Annotated[..., Field(description=...)] parameter style
• Same try/except Exception -> _error_response(exc) error handling
• Same _ok(...) response wrapping
• direct_messages.py correctly imports only get_client (no _resolve_group) since DMs are user-scoped, not group-scoped
• files.py correctly imports _resolve_group since file operations are group-scoped

Registration: __init__.py registers both new modules alphabetically in the existing import list. noqa: F401 comments present.

conftest patching: get_client patched in both new modules. _resolve_group patched in files_mod only (not dm_mod), which is correct since direct_messages.py does not use _resolve_group.

Test coverage: 16 new tests across 2 files (8 each):

• test_direct_messages.py: happy path, HTTP error, generic error for send_dm; formatted output, pagination params, empty list, HTTP error, generic error for list_dm_chats
• test_files.py: happy path, content type passthrough, file not found, HTTP error, generic error for upload_file; happy path, HTTP error, generic error for get_upload_status

Coverage is thorough -- happy path, edge cases, and error handling all represented.

PEP compliance: from __future__ import annotations present in all modules. Type hints on all parameters via Annotated[...]. Docstrings on all public functions (PEP 257). str | None union syntax (PEP 604). Clean.

Security: No hardcoded credentials. file_path in upload_file is an MCP tool parameter consumed by agents with local filesystem access -- path traversal is not a concern in this context. The SDK handles file I/O and raises FileNotFoundError which is properly caught.

BLOCKERS

None.

NITS

1. No pagination clamping on list_dm_chats: list_messages clamps limit to [1, 100] via max(1, min(limit, 100)), but list_dm_chats passes page/per_page directly to the SDK without bounds checking. Not a blocker since the GroupMe API handles invalid values server-side, but it is a pattern inconsistency. Consider adding per_page = max(1, min(per_page, 100)) for consistency.

2. Branch name truncated: Branch 7-add-dm-and-file-attachment-tools-to-grou is cut off (should be groupme-mcp or similar). Cosmetic only -- does not affect functionality.

3. SDK dependency ordering: PR body notes "Merge groupme-sdk#8 first" as a dependency with an unchecked box. Confirm the SDK PR is merged and the package published to Forgejo PyPI before merging this PR.

SOP COMPLIANCE

• Branch named after issue (7-add-dm-and-file-attachment-tools-to-grou -> issue #7)
• PR body follows template (Summary, Changes, Test Plan, Related)
• Related references parent issue (Closes #7)
• No secrets committed
• No scope creep -- all 6 files directly support the 4 new tools
• Tests exist (16 new, 54 total claimed)

PROCESS OBSERVATIONS

• Clean additive PR: 360 additions, 0 deletions, 6 files. Low change failure risk.
• SDK dependency (groupme-sdk#8) must be resolved before merge. Cross-repo dependency is documented in the PR body -- good practice.
• Test-to-code ratio is healthy: 222 lines of tests for 131 lines of implementation.

VERDICT: APPROVED

## PR #8 Review ### DOMAIN REVIEW **Tech stack**: Python 3.12+ / FastMCP / groupme-sdk / pytest / Pydantic Field annotations **Pattern compliance**: Both new tool modules (`direct_messages.py`, `files.py`) follow the established pattern from `groups.py`, `members.py`, and `messages.py` exactly: - Same `from ..server import _error_response, _ok, get_client, mcp` (plus `_resolve_group` where needed) - Same `@mcp.tool()` decorator with `Annotated[..., Field(description=...)]` parameter style - Same `try/except Exception -> _error_response(exc)` error handling - Same `_ok(...)` response wrapping - `direct_messages.py` correctly imports only `get_client` (no `_resolve_group`) since DMs are user-scoped, not group-scoped - `files.py` correctly imports `_resolve_group` since file operations are group-scoped **Registration**: `__init__.py` registers both new modules alphabetically in the existing import list. `noqa: F401` comments present. **conftest patching**: `get_client` patched in both new modules. `_resolve_group` patched in `files_mod` only (not `dm_mod`), which is correct since `direct_messages.py` does not use `_resolve_group`. **Test coverage**: 16 new tests across 2 files (8 each): - `test_direct_messages.py`: happy path, HTTP error, generic error for `send_dm`; formatted output, pagination params, empty list, HTTP error, generic error for `list_dm_chats` - `test_files.py`: happy path, content type passthrough, file not found, HTTP error, generic error for `upload_file`; happy path, HTTP error, generic error for `get_upload_status` Coverage is thorough -- happy path, edge cases, and error handling all represented. **PEP compliance**: `from __future__ import annotations` present in all modules. Type hints on all parameters via `Annotated[...]`. Docstrings on all public functions (PEP 257). `str | None` union syntax (PEP 604). Clean. **Security**: No hardcoded credentials. `file_path` in `upload_file` is an MCP tool parameter consumed by agents with local filesystem access -- path traversal is not a concern in this context. The SDK handles file I/O and raises `FileNotFoundError` which is properly caught. ### BLOCKERS None. ### NITS 1. **No pagination clamping on `list_dm_chats`**: `list_messages` clamps `limit` to `[1, 100]` via `max(1, min(limit, 100))`, but `list_dm_chats` passes `page`/`per_page` directly to the SDK without bounds checking. Not a blocker since the GroupMe API handles invalid values server-side, but it is a pattern inconsistency. Consider adding `per_page = max(1, min(per_page, 100))` for consistency. 2. **Branch name truncated**: Branch `7-add-dm-and-file-attachment-tools-to-grou` is cut off (should be `groupme-mcp` or similar). Cosmetic only -- does not affect functionality. 3. **SDK dependency ordering**: PR body notes "Merge groupme-sdk#8 first" as a dependency with an unchecked box. Confirm the SDK PR is merged and the package published to Forgejo PyPI before merging this PR. ### SOP COMPLIANCE - [x] Branch named after issue (`7-add-dm-and-file-attachment-tools-to-grou` -> issue #7) - [x] PR body follows template (Summary, Changes, Test Plan, Related) - [x] Related references parent issue (`Closes #7`) - [x] No secrets committed - [x] No scope creep -- all 6 files directly support the 4 new tools - [x] Tests exist (16 new, 54 total claimed) ### PROCESS OBSERVATIONS - Clean additive PR: 360 additions, 0 deletions, 6 files. Low change failure risk. - SDK dependency (`groupme-sdk#8`) must be resolved before merge. Cross-repo dependency is documented in the PR body -- good practice. - Test-to-code ratio is healthy: 222 lines of tests for 131 lines of implementation. ### VERDICT: APPROVED

forgejo_admin merged commit 8b5078dc7f into main 2026-03-28 19:30:56 +00:00

forgejo_admin deleted branch 7-add-dm-and-file-attachment-tools-to-grou 2026-03-28 19:30:56 +00:00

forgejo_admin referenced this pull request from a commit 2026-03-28 19:30:57 +00:00

feat: add DM and file attachment MCP tools (#8)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

forgejo_admin/groupme-mcp!8

No description provided.