Review: Add DM and file attachment tools to GroupMe MCP

review-586-2026-03-28 Doc

review needs-refinement

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- discovered during girls roster session 2026-03-28
  • [x] Repo -- forgejo_admin/groupme-mcp
  • [x] User Story -- present (but story key mismatch, see Traceability)
  • [x] Context -- thorough, includes API discoveries and workarounds
  • [x] File Targets -- 4 modify/create targets + exclusion list
  • [x] Acceptance Criteria -- 5 criteria
  • [x] Test Expectations -- unit + integration + run command
  • [x] Constraints -- pattern, env var, docstring guidance
  • [x] Checklist -- standard 3-item
  • [x] Related -- project + board references

Traceability

  • [ ] story:WS-S6 label -- MISMATCH. WS-S6 is "As an admin, I want to assign players to teams via draft board." The issue's actual user story is "As an agent, I want to send DMs and file attachments via MCP tools" which maps to story:GM-5 ("As platform operator, I want to create and manage GroupMe groups via MCP tools") from project-groupme-westside.
  • [x] arch:groupme label -- valid. GroupMe is a documented architecture component in project-groupme-westside.
  • [x] Forgejo issue -- forgejo_admin/groupme-mcp#7, state: open

File Targets

  • [x] src/groupme_mcp/tools/direct_messages.py -- NEW FILE. Parent directory exists (src/groupme_mcp/tools/). Pattern matches existing tool modules (groups.py, members.py, messages.py).
  • [x] src/groupme_mcp/tools/files.py -- NEW FILE. Same directory, consistent naming.
  • [x] src/groupme_mcp/tools/messages.py -- VERIFIED EXISTS. Modify to add attachments param to send_message.
  • [x] src/groupme_mcp/server.py -- VERIFIED EXISTS. Register new tools. Uses FastMCP pattern with auto-discovery.
  • [ ] groupme-sdk -- ISSUE: The MCP wraps groupme-sdk (GroupMeClient inherits GroupsMixin, MembersMixin, MessagesMixin). The SDK has NO DM or file methods. Either the SDK needs new mixins (DirectMessagesMixin, FilesMixin) first, or the MCP tools must break the established pattern and make raw HTTP calls. This dependency is not documented in the issue.

Repo Placement

Issue is filed on forgejo_admin/groupme-mcp which is correct for the MCP tool layer. However, the work also requires changes to forgejo_admin/groupme-sdk (add DM and file upload methods). The SDK is a separate repo with its own CI pipeline (publishes to Forgejo PyPI on push to main). A separate Forgejo issue on groupme-sdk is needed, OR the issue must explicitly scope raw HTTP calls in the MCP (breaking the SDK delegation pattern).

Dependencies

  • [ ] groupme-sdk DM/file methods -- UNDOCUMENTED. SDK needs DirectMessagesMixin and FilesMixin before MCP can wrap them (unless raw HTTP is chosen). No existing board item covers this.
  • [x] Board item #303 (groupme-sdk initial build) -- done.
  • [x] Board item #305 (groupme-mcp initial build) -- done. This ticket extends that work.
  • [x] No blocking items in in_progress column.

Acceptance Criteria

5 acceptance criteria are clear and testable. Each maps to a specific API call with expected return data. The error handling criterion (422 link restriction) is well-specified thanks to Known Behaviors documentation in project-groupme-westside. An agent could verify all 5 after implementation. Note: "integration test with real GroupMe API (manual)" is not automatable -- CI would only run unit tests. This is acceptable but should be explicit in the issue.

Blast Radius

  • groupme-sdk: If SDK gets new mixins, the SDK's test suite and CI pipeline are affected. The SDK auto-publishes to Forgejo PyPI on push to main -- MCP's pyproject.toml would need a version bump after SDK ships.
  • basketball-api: Also consumes groupme-sdk. New mixins are additive only -- no breakage risk, but basketball-api gets new methods for free on next dependency update.
  • No other consumers of groupme-mcp or groupme-sdk identified. Rollback is straightforward (revert MCP commit, no data migrations).

Decomposition Assessment

Three-thing limit: 4 file targets in groupme-mcp + undocumented SDK dependency in a separate repo = 2 repos, 5+ files total. Exceeds 3-file/2-repo threshold.
Five-minute rule: An agent would need to (1) understand SDK patterns, (2) understand MCP patterns, (3) implement 2 new tool modules + modify 2 existing files, (4) write tests for each. Estimated well over 5 minutes.
Independent subtasks: SDK work and MCP work are sequential (MCP depends on SDK), not parallelizable.
NEEDS DECOMPOSITION into at least 2 tickets:
  • groupme-sdk: Add DirectMessagesMixin + FilesMixin (separate Forgejo issue on groupme-sdk repo)
  • groupme-mcp: Add DM and file tools wrapping new SDK methods (this issue, after SDK ships)
Alternatively, if raw HTTP in MCP is acceptable (no SDK changes), this becomes a single-repo 4-file change that might fit in one agent pass -- but that's an architectural decision requiring human input.

Recommendation

  • [LABEL] Fix story label: Change story:WS-S6 to story:GM-5 on the board item. WS-S6 is the draft board story; GM-5 is the MCP tooling story from project-groupme-westside.
  • [SCOPE] Decide SDK strategy: Either (a) create a separate groupme-sdk issue for DM/file mixins and make this ticket depend on it, or (b) explicitly scope this ticket to make raw HTTP calls in the MCP tools (breaking the SDK delegation pattern). This is an architectural decision.
  • [BODY] Add SDK dependency note: If option (a), add to Context: "Depends on groupme-sdk adding DirectMessagesMixin and FilesMixin." Add to Constraints: "Blocked by groupme-sdk#{N}." If option (b), add to Constraints: "Use raw urllib calls in MCP tools (no SDK wrapper for these endpoints)."
  • [DECOMPOSE] If option (a): split into 2 tickets. Ticket 1: groupme-sdk DM + file methods (new Forgejo issue). Ticket 2: groupme-mcp tools (this issue, blocked by ticket 1). Each ticket fits within the 3-file / 5-minute limit independently.