- Lower MaxInlineDownloadBytes from 10 MiB → 1 MiB - download_*_attachment always returns browser_download_url; files over cap return metadata + URL only (fall-through to curl with auth token) - Promote Open Question #2 (private-repo auth on browser_download_url) to load-bearing Part-1 verification step, with fallback path documented - Update acceptance criteria to cover both under-cap and over-cap cases - Close beads/forgejo-mcp-zhi; unblock forgejo-mcp-ydg
17 KiB
Issue & Comment Attachments — Implementation Plan
Issue: #106
Branch: feature/issue-attachments (suggested)
Summary
Add full CRUD MCP tools for attachments on issues and issue comments, so an agent reading an issue through forgejo-mcp can discover, retrieve, upload, rename, and delete attachments. This closes the gap reported in #106, where a PDF uploaded via the web UI was invisible to the MCP client.
The Forgejo REST API exposes issue and comment attachments via dedicated endpoints that GET /issues/{index} does not embed. forgejo-sdk/v3@v3.0.0 ships attachment methods for releases only — there is no SDK coverage for issue or comment attachments.
Use Case
An agent working through the MCP server reads an issue the user created in the web UI. The user uploaded a PDF attachment that contains information the issue depends on. The agent must see that the attachment exists, retrieve its bytes, and reason over the content. The agent must also be able to upload, rename, and delete attachments on both issues and issue comments.
Current State
operation/issue/issue.goregisters 14 issue tools; none touch attachments.pkg/forgejo/forgejo.goexposes a singleton SDK client built fromflag.URL+flag.Token; no raw-HTTP helper.forgejo-sdk/v3@v3.0.0/attachment.godefinesAttachment+ release-attachment methods (ListReleaseAttachments,GetReleaseAttachment,CreateReleaseAttachment,EditReleaseAttachment,DeleteReleaseAttachment). No issue or comment equivalents.github.com/mark3labs/mcp-go@v0.44.0supports binary payloads viamcp.BlobResourceContentswrapped inmcp.NewEmbeddedResource.
Key Design Decision: SDK Gap
The SDK does not cover issue or comment attachments. Three options were considered:
- Upstream-first (blocked-feature pattern) — contribute to forgejo-sdk, wait for merge, then wire MCP tools. Consistent with
wiki-support.md/projects-support.md. Ship time gated on upstream review. - Raw HTTP inside forgejo-mcp — bypass the SDK, call the Forgejo API directly using stdlib
net/httpwith the existingflag.URLandflag.Token. Ships immediately; introduces a second HTTP code path in this repo. - Upstream contribution + temporary
replacedirective — contribute upstream and use a fork viago.mod replaceuntil the upstream PR merges.
Decision: Option 2 (raw HTTP). Rationale: the attachment endpoints are stable Gitea/Forgejo API routes, the raw-HTTP surface is small and contained behind a single helper package, and shipping time beats consistency here. A future refactor can migrate to SDK methods without changing the MCP tool surface.
Risk: raw HTTP bypasses SDK versioning. Mitigation: one helper (pkg/forgejo/rawhttp.go) owns all HTTP construction; every attachment tool goes through it; helper is unit-tested with httptest.
Target State
12 New MCP Tools
Issue-scoped (6):
| Tool | HTTP | Endpoint | Returns |
|---|---|---|---|
list_issue_attachments |
GET | /repos/{owner}/{repo}/issues/{index}/assets |
[]Attachment JSON |
get_issue_attachment |
GET | /repos/{owner}/{repo}/issues/{index}/assets/{attachment_id} |
Attachment JSON (metadata) |
download_issue_attachment |
GET | {browser_download_url} |
MCP EmbeddedResource (BlobResourceContents) |
create_issue_attachment |
POST | /repos/{owner}/{repo}/issues/{index}/assets (multipart) |
Attachment JSON |
edit_issue_attachment |
PATCH | /repos/{owner}/{repo}/issues/{index}/assets/{attachment_id} |
Attachment JSON |
delete_issue_attachment |
DELETE | /repos/{owner}/{repo}/issues/{index}/assets/{attachment_id} |
"Delete attachment success" |
Comment-scoped (6):
| Tool | HTTP | Endpoint | Returns |
|---|---|---|---|
list_comment_attachments |
GET | /repos/{owner}/{repo}/issues/comments/{comment_id}/assets |
[]Attachment JSON |
get_comment_attachment |
GET | /repos/{owner}/{repo}/issues/comments/{comment_id}/assets/{attachment_id} |
Attachment JSON |
download_comment_attachment |
GET | {browser_download_url} |
MCP EmbeddedResource |
create_comment_attachment |
POST | /repos/{owner}/{repo}/issues/comments/{comment_id}/assets (multipart) |
Attachment JSON |
edit_comment_attachment |
PATCH | /repos/{owner}/{repo}/issues/comments/{comment_id}/assets/{attachment_id} |
Attachment JSON |
delete_comment_attachment |
DELETE | /repos/{owner}/{repo}/issues/comments/{comment_id}/assets/{attachment_id} |
"Delete attachment success" |
Tool Parameter Schemas
Shared parameters (added to pkg/params/params.go):
AttachmentID = "Attachment ID"
AttachmentName = "New name for the attachment"
AttachmentContent = "Base64-encoded file bytes to upload"
AttachmentFilename = "Filename to associate with the uploaded attachment (e.g. \"requirements.pdf\")"
AttachmentMIME = "MIME type hint for uploaded file (optional; inferred from filename if omitted)"
The MCP tool accepts base64 bytes only — deliberately no file-path parameter, because the MCP server is often invoked in a context where the agent's filesystem is not the same as the Forgejo server's. A future CLI convenience wrapper may accept --file path and encode before dispatch; that is out of scope for the tool definitions themselves.
Parameter matrix:
| Tool | owner |
repo |
index / comment_id |
attachment_id |
name |
content |
filename |
mime_type |
|---|---|---|---|---|---|---|---|---|
list_*_attachments |
✓ | ✓ | ✓ | |||||
get_*_attachment |
✓ | ✓ | ✓ | ✓ | ||||
download_*_attachment |
✓ | ✓ | ✓ | ✓ | ||||
create_*_attachment |
✓ | ✓ | ✓ | ✓ | ✓ | opt | ||
edit_*_attachment |
✓ | ✓ | ✓ | ✓ | ✓ | |||
delete_*_attachment |
✓ | ✓ | ✓ | ✓ |
Architecture
main.go
└─ cmd/cmd.go
└─ operation/operation.go
├─ operation/issue/ (existing)
└─ operation/attachment/ ← NEW domain package
└─ attachment.go (12 tools + handlers)
pkg/forgejo/
├─ forgejo.go (existing SDK client singleton)
└─ rawhttp.go ← NEW raw-HTTP helper
pkg/forgejo/rawhttp.go
Single place that owns raw HTTP access to the Forgejo API. All attachment tools call into this helper; no net/http imports anywhere in operation/attachment/.
Signature sketch:
package forgejo
// DoJSON performs an authenticated JSON request. Encodes body as JSON (nil ok),
// decodes 2xx response into out (nil ok for no-content). Non-2xx → error with
// status code + sanitized body snippet.
func DoJSON(ctx context.Context, method, relPath string, body, out any) (*http.Response, error)
// DoMultipart performs an authenticated multipart POST with a single file part.
// fieldName is the form field (e.g. "attachment"); filename is sent in the part header;
// r supplies the bytes.
func DoMultipart(ctx context.Context, method, relPath, fieldName, filename, mimeType string, r io.Reader, out any) (*http.Response, error)
// DoRaw fetches bytes from a URL (absolute or relative to the configured base URL),
// adding the configured auth header. Returns body bytes + content type.
// Used by download_*_attachment to fetch from browser_download_url.
func DoRaw(ctx context.Context, absoluteOrRelativeURL string) (body []byte, contentType string, err error)
flag.URL+flag.Token+flag.UserAgentfeed the request (same as SDK client).- 401/403 →
ErrUnauthorizedwrapper so callers can distinguish auth failure. - 404 on
GET …/assets→ empty slice, not error (list semantics). - 404 on all other endpoints → error.
operation/attachment/attachment.go
Each handler is thin: parse args → call rawhttp.DoX → to.TextResult or build EmbeddedResource.
Download handlers are the only interesting ones:
func DownloadIssueAttachmentFn(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) {
// 1. GET .../assets/{aid} → Attachment metadata (always returned, includes browser_download_url)
// 2. If Size > MaxInlineDownloadBytes: return metadata only.
// Caller is expected to fetch {browser_download_url} directly via curl/Bash with the same auth token.
// 3. Otherwise: DoRaw(browser_download_url) → base64 → BlobResourceContents,
// return alongside the metadata so the caller still sees browser_download_url.
// 4. Wrap bytes in mcp.NewEmbeddedResource and return with metadata.
}
Constant:
const MaxInlineDownloadBytes = 1 * 1024 * 1024 // 1 MiB
Rationale for the 1 MiB cap and the always-include-URL contract: discussion on issue #106 (comments 13701827, 13703888). For files over the cap, the LLM is expected to fall through to a direct authenticated fetch of browser_download_url; this avoids both context-bloat from large base64 payloads and the tmpfile-hygiene / cross-container permission problems that a save_to_path parameter would introduce.
Registration
operation/operation.go gains:
import "codeberg.org/goern/forgejo-mcp/v2/operation/attachment"
// …
attachment.RegisterTool(s)
Implementation Steps
Part 1 — Verify API contract
- Confirm endpoint shapes against a live Codeberg issue with a PDF attached:
GET /api/v1/repos/{o}/{r}/issues/{index}/assetsreturns[]Attachment.POSTmultipart field name isattachment(matches release-assets convention in SDK).PATCHbody is{"name":"..."}(matches SDK'sEditAttachmentOptions).
- Verify private-repo auth on
browser_download_url(blocks Part 3 — see Open Question #2):- Upload a PDF to a private test repo.
curl -H "Authorization: token $TOKEN" $browser_download_url— must return 200 with the file bytes.- If it returns 401/403/redirect-to-login, the implementation must rewrite responses to substitute the API asset-content path (
GET /repos/{o}/{r}/issues/{index}/assets/{aid}withAccept: application/octet-stream) for any field exposed to the client. Document the chosen URL form in the response schema.
- Record actual response JSON for one attachment in the
Open Questionssection if any field differs from the SDK'sAttachmentstruct.
Part 2 — pkg/forgejo/rawhttp.go
- Implement
DoJSON,DoMultipart,DoRaw. - Unit tests with
httptest.NewServer:- Auth header propagation (
Authorization: token {flag.Token}— match SDK'sSetToken). - User-Agent header propagation.
- Multipart boundary correctness (round-trip via
mime/multipart.NewReader). - 2xx decode, 4xx error mapping, 404-on-list →
io.EOF-style sentinel the caller can treat as empty. - 1 MiB cap enforced by
DoRaw(test reads response withio.LimitReader).
- Auth header propagation (
- All tests live in
pkg/forgejo/rawhttp_test.go; no live-API calls.
Part 3 — operation/attachment/attachment.go
-
Define 12 tools with
mcp.NewTooland aRegisterTool(s *server.MCPServer)function matching the pattern inoperation/issue/issue.go. -
Implement handlers. Shape follows:
func ListIssueAttachmentsFn(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { owner, _ := req.GetArguments()["owner"].(string) repo, _ := req.GetArguments()["repo"].(string) index, _ := to.Float64(req.GetArguments()["index"]) var out []forgejo_sdk.Attachment path := fmt.Sprintf("/repos/%s/%s/issues/%d/assets", owner, repo, int64(index)) if _, err := forgejo.DoJSON(ctx, http.MethodGet, path, nil, &out); err != nil { return to.ErrorResult(fmt.Errorf("list issue attachments err: %v", err)) } return to.TextResult(out) } -
Reuse the SDK's
Attachmentstruct (forgejo_sdk.Attachment) as the DTO — the release and issue/comment attachment JSON shapes are identical in Forgejo.
Part 4 — Wire-up
- Add
attachment.RegisterTool(s)inoperation/operation.go. - Add parameter descriptions to
pkg/params/params.go. - Add tool row to
README.mdtool table.
Part 5 — Demos (Showboat format)
Two files in demos/:
demos/issue-attachments.md— lifecycle walkthrough:- Create a scratch issue (
create_issue). - Upload a small PDF (
create_issue_attachment). - List (
list_issue_attachments), get metadata (get_issue_attachment). - Download & verify bytes (
download_issue_attachment). - Rename (
edit_issue_attachment). - Delete (
delete_issue_attachment). - Close the scratch issue.
- Create a scratch issue (
demos/comment-attachments.md— same lifecycle against a comment on a scratch issue.
Both use ./forgejo-mcp --cli <tool> invocations, match the formatting of demos/issue-labels.md, and carry a valid Showboat header (<!-- showboat-id: … -->).
Part 6 — Tests
- Unit:
pkg/forgejo/rawhttp_test.go(see Part 2). - Handler smoke tests:
operation/attachment/attachment_test.gowith a mock HTTP server injected via a test hook onrawhttp(simplest: make the base URL configurable at request time, then point tests athttptest.NewServer). - No live-API tests in CI (matches repo posture).
Deliverables
pkg/forgejo/rawhttp.go+ tests.operation/attachment/attachment.go+ tests.- Registration wired in
operation/operation.go. - Parameter descriptions added to
pkg/params/params.go. README.mdtool table updated.demos/issue-attachments.md.demos/comment-attachments.md.- CHANGELOG entry under next version.
Acceptance Criteria
./forgejo-mcp --cli list_issue_attachments --owner goern --repo forgejo-mcp --index 106against a Codeberg issue with a PDF returns the attachment metadata../forgejo-mcp --cli download_issue_attachment …for a PDF < 1 MiB returns anEmbeddedResourcewhoseBlobdecodes to the exact bytes of the uploaded file (sha256match), and the response also carriesbrowser_download_urlin the metadata. 2a../forgejo-mcp --cli download_issue_attachment …for a file > 1 MiB returns metadata +browser_download_urland noBlob, and the URL is fetchable withAuthorization: token {flag.Token}(sha256 of fetched bytes matches uploaded file).- Round-trip:
create_issue_attachment→edit_issue_attachment(rename) →list_issue_attachmentsshows the new name →delete_issue_attachment→list_issue_attachmentsno longer shows it. - Same round-trip works against
*_comment_attachmenttools on an issue comment. list_*_attachmentson an entity with zero attachments returns[], not an error.make buildpasses;go test ./...passes.- Both demo files run cleanly end-to-end against a live Forgejo (manual verification).
Open Questions
Inline size cap: 10 MiB is a guess.Resolved: cap is 1 MiB (see issue #106 discussion). Configurability via a--max-inline-download-bytesflag deferred to a follow-up issue if a real use case appears.- Authorization for
browser_download_url(load-bearing — must verify in Part 1): private-repo attachments are served behind session auth on the web path. The "fall through to curl for files over the cap" pattern depends onAuthorization: token …working on the/attachments/{uuid}route. Verification step in Part 1: upload a PDF to a private test repo, confirm the URL returned inbrowser_download_urlreturns 200 with the token attached. If it does not, the implementation must rewrite the response to point at the API asset-content path instead (GET /repos/.../assets/{id}withAccept: application/octet-stream), which is API-authenticated. - Multipart field name for comment attachments: release assets use
attachment; need to confirm issue/comment endpoints accept the same. Likely yes, verify in Part 1. - Exposing
updated_aton create: the Forgejo API documents an optionalupdated_atfield in the multipart form. YAGNI for v1 — omit unless a caller needs it.
Risk Assessment
- Low risk: all 12 tools are additive; no existing tool changes behavior.
- Medium risk: raw HTTP bypasses SDK validation. If Forgejo renames a field or changes auth semantics, attachment tools break silently while other tools keep working. Mitigated by (a) reusing
forgejo_sdk.Attachmentas the DTO so struct drift trips compile errors as soon as the SDK updates, (b) demo files serve as live-integration canaries. - Low risk: binary
EmbeddedResourcesupport is already used elsewhere in the mcp-go ecosystem; the 1 MiB cap bounds context-bloat blast radius, and over-cap responses degrade to a metadata-only payload that points atbrowser_download_url.
Follow-ups (not in this spec)
- Migrate raw-HTTP attachment calls to SDK methods once upstream adds them (track as separate beads issue; this spec is the migration target).
- Configurable inline-download cap.
- Support uploading from a URL instead of base64 bytes (saves client-side encoding for large files).