feat: add list_messages tool for reading group message history

forgejo_admin commented

2026-03-26 07:07:44 +00:00

Owner

Summary

Adds read-only list_messages MCP tool for retrieving GroupMe message history by group name
Supports pagination via before_id and configurable limit (1-100, default 20)
Returns sender nickname, text, timestamps, and attachment URLs
Bumps groupme-sdk dependency to >=0.2.0 (ships the underlying list_messages SDK method)

Changes

pyproject.toml -- bump groupme-sdk dependency from >=0.1.0 to >=0.2.0
uv.lock -- updated lockfile reflecting groupme-sdk 0.2.0
src/groupme_mcp/tools/messages.py -- add list_messages tool with name resolution, limit clamping, attachment formatting
tests/test_messages.py -- 10 new tests covering default behavior, limit clamping, pagination, attachments, name resolution, and error handling

Test Plan

Tests pass locally (38 total -- 28 existing + 10 new)
ruff check and ruff format pass
Resolves group names consistently with other tools (uses shared _resolve_group)
Returns 20 recent messages by default
Pagination via before_id works
Limit clamped to 1-100 range

Review Checklist

Passed automated review-fix loop
No secrets committed
No unnecessary file changes
Commit messages are descriptive

Closes #5
Depends on groupme-sdk 0.2.0 (groupme-sdk PR #6)

## Summary - Adds read-only `list_messages` MCP tool for retrieving GroupMe message history by group name - Supports pagination via `before_id` and configurable `limit` (1-100, default 20) - Returns sender nickname, text, timestamps, and attachment URLs - Bumps groupme-sdk dependency to >=0.2.0 (ships the underlying `list_messages` SDK method) ## Changes - `pyproject.toml` -- bump groupme-sdk dependency from >=0.1.0 to >=0.2.0 - `uv.lock` -- updated lockfile reflecting groupme-sdk 0.2.0 - `src/groupme_mcp/tools/messages.py` -- add `list_messages` tool with name resolution, limit clamping, attachment formatting - `tests/test_messages.py` -- 10 new tests covering default behavior, limit clamping, pagination, attachments, name resolution, and error handling ## Test Plan - [x] Tests pass locally (38 total -- 28 existing + 10 new) - [x] ruff check and ruff format pass - [x] Resolves group names consistently with other tools (uses shared `_resolve_group`) - [x] Returns 20 recent messages by default - [x] Pagination via before_id works - [x] Limit clamped to 1-100 range ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related - Closes #5 - Depends on groupme-sdk 0.2.0 (groupme-sdk PR #6)

forgejo_admin added 1 commit

2026-03-26 07:07:44 +00:00

feat: add list_messages tool for reading group message history 86733e3345

Adds a read-only MCP tool that retrieves GroupMe message history by
group name. Supports pagination via before_id and configurable limit
(1-100, default 20). Returns sender nickname, text, timestamps, and
attachment URLs.

Also bumps groupme-sdk dependency to >=0.2.0 which ships the
underlying list_messages SDK method.

Closes #5

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

forgejo_admin commented

2026-03-26 07:08:23 +00:00

Author

Owner

Review: LGTM

Reviewed all 4 changed files (pyproject.toml, uv.lock, messages.py, test_messages.py).

Tool implementation follows existing patterns (resolve_group, get_client, _ok, _error_response)
Limit clamping handles both under (min 1) and over (max 100)
Attachments included only when present -- clean output
10 new tests cover: default behavior, limit clamping (both edges), pagination, attachments (present and absent), name resolution, HTTP errors, generic errors
38 total tests pass
ruff check and format pass
No secrets, no unrelated changes

Ready for merge.

**Review: LGTM** Reviewed all 4 changed files (pyproject.toml, uv.lock, messages.py, test_messages.py). - Tool implementation follows existing patterns (resolve_group, get_client, _ok, _error_response) - Limit clamping handles both under (min 1) and over (max 100) - Attachments included only when present -- clean output - 10 new tests cover: default behavior, limit clamping (both edges), pagination, attachments (present and absent), name resolution, HTTP errors, generic errors - 38 total tests pass - ruff check and format pass - No secrets, no unrelated changes Ready for merge.

forgejo_admin commented

2026-03-26 07:11:44 +00:00

Author

Owner

PR #6 Review

DOMAIN REVIEW

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

This PR adds a list_messages MCP tool to src/groupme_mcp/tools/messages.py and bumps groupme-sdk from >=0.1.0 to >=0.2.0. It also adds a uv.lock file for reproducible builds.

Code quality assessment:

Follows existing patterns precisely. The new list_messages function mirrors the structure of send_message (same module), get_group (groups module), and all other tools: _resolve_group for name resolution, get_client() for SDK access, _ok() / _error_response() for response formatting. Consistent and clean.
Type hints and docstrings. Full PEP 484 type hints via Annotated[..., Field(...)] on all parameters. Docstring follows PEP 257 and clearly describes behavior, return format, and pagination.
Input validation. limit is clamped to [1, 100] via max(1, min(limit, 100)). This is correct and tested. before_id is str | None with a sensible default. group_name is validated by _resolve_group which raises ValueError on no-match or ambiguity.
Error handling. Broad except Exception catches everything and delegates to _error_response(), which distinguishes GroupMeHTTPError (with status code) from generic errors. This matches the existing pattern across all tools.
Attachment handling. Clean conditional inclusion -- attachments key is omitted from output when the list is empty, included when present. This is a good UX choice for MCP consumers.
Module docstring updated. Changed from "send messages" to "send and read messages" -- accurate.
No DRY violations. The tool uses shared helpers (_resolve_group, get_client, _ok, _error_response) consistently. No duplicated auth/security logic.
No hardcoded values. Default limit of 20 and max of 100 match the issue acceptance criteria and are reasonable API defaults, not magic numbers.

Test coverage assessment:

10 new tests in TestListMessages covering:

Happy path: message retrieval with full field assertions (id, sender, text, created_at, group info)
SDK argument verification: confirms group_id, limit, and before_id are passed correctly
Limit clamping: both upper bound (200 -> 100) and lower bound (0 -> 1)
Pagination: before_id pass-through
Attachment inclusion and omission (conditional key behavior)
Group name resolution verification
Error paths: GroupMeHTTPError (HTTP 429) and generic RuntimeError

This is thorough. All acceptance criteria from issue #5 are covered by tests.

Dependency change: groupme-sdk>=0.2.0 is required for the list_messages SDK method. The uv.lock addition ensures reproducible installs. Both are appropriate.

BLOCKERS

None.

New functionality has comprehensive test coverage (10 tests).
No unvalidated user input -- group_name resolves through _resolve_group, limit is clamped, before_id is optional string.
No secrets or credentials in code. Token is read from environment in GroupMeClient() constructor.
No DRY violations in auth/security paths.

NITS

Negative limit values: The clamping max(1, min(limit, 100)) handles zero and negatives correctly (they all clamp to 1), but there is no test for a negative value like limit=-5. Consider adding one for completeness. Non-blocking since the logic is mathematically sound.
PR body says "ten new test cases" but the issue acceptance criteria listed specific test requirements. The mapping is clear on inspection but explicitly stating which acceptance criterion each test satisfies would strengthen traceability. Non-blocking.

SOP COMPLIANCE

Branch named after issue number (5-add-list-messages-tool maps to issue #5)
PR body has Summary, Changes, Test Plan, Related sections
Related section references issue #5 ("Closes issue #5")
Related section does not reference a plan slug (no plan exists for this work -- acceptable for a standalone feature issue)
Tests exist (10 new, 38 total passing per Test Plan)
No secrets, .env files, or credentials committed
No unnecessary file changes -- only pyproject.toml, uv.lock, messages.py, test_messages.py modified
Scope is tight: only list_messages added, no unrelated changes

PROCESS OBSERVATIONS

Deployment frequency: Clean single-feature PR with minimal diff surface. Easy to review, easy to revert. Good for deployment frequency.
Change failure risk: Low. Read-only tool with no side effects. Well-tested. Follows established patterns.
Documentation: Module docstring updated. Tool docstring is clear. No external docs appear to need updating for an MCP tool addition.
SDK dependency coordination: PR correctly depends on groupme-sdk 0.2.0 which provides the list_messages method. The dependency chain is clean.

VERDICT: APPROVED

## PR #6 Review ### DOMAIN REVIEW **Tech stack:** Python 3.12 / FastMCP / Pydantic / groupme-sdk / pytest This PR adds a `list_messages` MCP tool to `src/groupme_mcp/tools/messages.py` and bumps `groupme-sdk` from `>=0.1.0` to `>=0.2.0`. It also adds a `uv.lock` file for reproducible builds. **Code quality assessment:** 1. **Follows existing patterns precisely.** The new `list_messages` function mirrors the structure of `send_message` (same module), `get_group` (groups module), and all other tools: `_resolve_group` for name resolution, `get_client()` for SDK access, `_ok()` / `_error_response()` for response formatting. Consistent and clean. 2. **Type hints and docstrings.** Full PEP 484 type hints via `Annotated[..., Field(...)]` on all parameters. Docstring follows PEP 257 and clearly describes behavior, return format, and pagination. 3. **Input validation.** `limit` is clamped to `[1, 100]` via `max(1, min(limit, 100))`. This is correct and tested. `before_id` is `str | None` with a sensible default. `group_name` is validated by `_resolve_group` which raises `ValueError` on no-match or ambiguity. 4. **Error handling.** Broad `except Exception` catches everything and delegates to `_error_response()`, which distinguishes `GroupMeHTTPError` (with status code) from generic errors. This matches the existing pattern across all tools. 5. **Attachment handling.** Clean conditional inclusion -- `attachments` key is omitted from output when the list is empty, included when present. This is a good UX choice for MCP consumers. 6. **Module docstring updated.** Changed from "send messages" to "send and read messages" -- accurate. 7. **No DRY violations.** The tool uses shared helpers (`_resolve_group`, `get_client`, `_ok`, `_error_response`) consistently. No duplicated auth/security logic. 8. **No hardcoded values.** Default limit of 20 and max of 100 match the issue acceptance criteria and are reasonable API defaults, not magic numbers. **Test coverage assessment:** 10 new tests in `TestListMessages` covering: - Happy path: message retrieval with full field assertions (id, sender, text, created_at, group info) - SDK argument verification: confirms `group_id`, `limit`, and `before_id` are passed correctly - Limit clamping: both upper bound (200 -> 100) and lower bound (0 -> 1) - Pagination: `before_id` pass-through - Attachment inclusion and omission (conditional key behavior) - Group name resolution verification - Error paths: `GroupMeHTTPError` (HTTP 429) and generic `RuntimeError` This is thorough. All acceptance criteria from issue #5 are covered by tests. **Dependency change:** `groupme-sdk>=0.2.0` is required for the `list_messages` SDK method. The `uv.lock` addition ensures reproducible installs. Both are appropriate. ### BLOCKERS None. - New functionality has comprehensive test coverage (10 tests). - No unvalidated user input -- `group_name` resolves through `_resolve_group`, `limit` is clamped, `before_id` is optional string. - No secrets or credentials in code. Token is read from environment in `GroupMeClient()` constructor. - No DRY violations in auth/security paths. ### NITS 1. **Negative limit values:** The clamping `max(1, min(limit, 100))` handles zero and negatives correctly (they all clamp to 1), but there is no test for a negative value like `limit=-5`. Consider adding one for completeness. Non-blocking since the logic is mathematically sound. 2. **PR body says "ten new test cases" but the issue acceptance criteria listed specific test requirements.** The mapping is clear on inspection but explicitly stating which acceptance criterion each test satisfies would strengthen traceability. Non-blocking. ### SOP COMPLIANCE - [x] Branch named after issue number (`5-add-list-messages-tool` maps to issue #5) - [x] PR body has Summary, Changes, Test Plan, Related sections - [x] Related section references issue #5 ("Closes issue #5") - [ ] Related section does not reference a plan slug (no plan exists for this work -- acceptable for a standalone feature issue) - [x] Tests exist (10 new, 38 total passing per Test Plan) - [x] No secrets, .env files, or credentials committed - [x] No unnecessary file changes -- only `pyproject.toml`, `uv.lock`, `messages.py`, `test_messages.py` modified - [x] Scope is tight: only `list_messages` added, no unrelated changes ### PROCESS OBSERVATIONS - **Deployment frequency:** Clean single-feature PR with minimal diff surface. Easy to review, easy to revert. Good for deployment frequency. - **Change failure risk:** Low. Read-only tool with no side effects. Well-tested. Follows established patterns. - **Documentation:** Module docstring updated. Tool docstring is clear. No external docs appear to need updating for an MCP tool addition. - **SDK dependency coordination:** PR correctly depends on groupme-sdk 0.2.0 which provides the `list_messages` method. The dependency chain is clean. ### VERDICT: APPROVED