pal-e-docs

Review: Forgejo MCP PR review tool

review-360-2026-03-27 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — discovered from claude-custom#160
  • [x] Repo — forgejo_admin/forgejo-mcp
  • [x] User Story — clear As/I want/So that
  • [x] Context — explains dogfooding gap with concrete incident
  • [x] File Targets — present but INCORRECT (see below)
  • [x] Acceptance Criteria — 4 items, clear and testable
  • [x] Test Expectations — 3 unit tests + run command
  • [x] Constraints — pattern-following, uv, Forgejo PyPI
  • [x] Checklist — standard 3 items
  • [x] Related — project + incident reference

Traceability

  • [x] story:pm-scope — PM scope management user story
  • [x] arch:mcp-tools — MCP tooling architecture component
  • [x] Forgejo issue — forgejo_admin/forgejo-mcp#15, open

File Targets

  • [ ] src/forgejo_mcp/issues.py — ISSUE: This file does not exist. All MCP tools live in src/forgejo_mcp/tools/workflows.py. The issue incorrectly states the target path.
  • [x] forgejo-sdk SDK method — verified: issue_edit_issue exists at forgejo-sdk/src/forgejo_sdk/issue.py:278 with params: owner, repo, index, title, body, assignee, assignees, due_date, milestone, ref, state, unset_due_date, updated_at.
Corrected file target: src/forgejo_mcp/tools/workflows.py — add update_issue tool following the create_issue pattern (lines 26-45).
Test file target: tests/test_label_comment_repo.py (or new test file) — follow the integration test pattern using @requires_forgejo decorator, direct tool function import, JSON response parsing.

Repo Placement

Correct — issue is filed on forgejo_admin/forgejo-mcp, which is where the tool wrapper lives. SDK is untouched.
WARNING: Duplicate board item detected. Board item #365 (claude-custom#162, title "Forgejo MCP: add update_issue tool", labels: type:feature,arch:forgejo-mcp,story:pm-scope,scope:unplanned) appears to be a duplicate filed on the wrong repo (claude-custom instead of forgejo-mcp). Recommend closing #365 as duplicate of #360.

Dependencies

No blocking dependencies. The SDK method already exists. forgejo-sdk is installed via Forgejo PyPI (uv dependency). No other board items block or are blocked by this work.

Acceptance Criteria

All 4 criteria are agent-verifiable:
  • [x] Tool availability — verifiable by checking MCP tool registration
  • [x] Parameter validation (at least one of title/body required) — verifiable via unit test
  • [x] Return shape (number + URL) — verifiable via JSON assertion
  • [x] Pattern conformance — verifiable by code inspection
Note: Test expectations say "Unit test" but the repo uses integration tests against a live Forgejo instance (see conftest.py @requires_forgejo decorator). The issue should say "Integration test" to match the actual test pattern.

Blast Radius

Low. This is a pure addition — no existing tools are modified. The SDK method is already used in test cleanup code (tests/test_label_comment_repo.py lines 51, 91, 153 call issue_edit_issue for state="closed"). No downstream consumers affected.
No similar gap found in other MCP servers (woodpecker-mcp, etc.) — this is Forgejo-specific.

Recommendation

Two items must be fixed before READY:
  • Fix file target path: Change src/forgejo_mcp/issues.py to src/forgejo_mcp/tools/workflows.py in the issue body.
  • Fix test type label: Change "Unit test" to "Integration test" in Test Expectations to match repo conventions.
Optional cleanup:
  • Close duplicate board item #365 (claude-custom#162) as duplicate of #360.