feat(attachment): upload from a server-side file path #16

Merged

Phil merged 1 commit from feat/attachment-file-path into main

2026-09-03 13:50:59 +00:00

Phil commented

2026-09-03 13:50:40 +00:00

Owner

Why

create_issue_attachment / create_comment_attachment only accepted content as a base64 string, which has to travel through the model's context window. A 68 KiB zip is ~91k base64 characters (~86k tokens) — no tool call carries that, and Read/Bash cap far below it. Anything beyond a few KiB was effectively un-uploadable from an agent.

The download side already solved the mirror problem (download_*_attachment streams files over the inline cap to disk and returns only the path, operation/attachment/attachment.go downloadResultFor). This PR adds the counterpart for upload.

What

New optional file_path (absolute path on the MCP server's host) on both create tools; content becomes optional. Exactly one of the two must be given, otherwise a clear error.
filename defaults to the basename of file_path; mime_type is inferred from the extension when omitted (the param description always claimed this — now it is true for both modes).
One shared helper (operation/attachment/upload.go: resolveUploadSource / uploadAttachment) replaces the duplicated decode+multipart code in both handlers.
Size guard before buffering: the helper asks the instance for GET /settings/attachment (max_size, MiB) and rejects oversized files naming the real limit; if the endpoint is unavailable, a documented 50 MiB fallback (FallbackMaxUploadBytes) applies.
Error cases: missing file, directory, non-regular file, unreadable, relative path, both inputs given.
Docs: README.md, demos/issue-attachments.md, docs/plans/issue-attachments.md, operation/params/params.go, extension/manifest.json.

Tests

operation/attachment/upload_test.go: helper unit tests (input selection, defaults, overrides, MIME inference, every error case, instance limit vs fallback) plus handler-level tests for issue and comment scope asserting the multipart part carries the file bytes, basename and application/zip.
Existing TestCreateIssueAttachmentFn_DecodesBase64AndUsesMultipart adjusted to pick the POST record instead of assuming order (the settings preflight now precedes it).
test/e2e/attachments.sh: new step 2b uploads via file_path, checks name defaulted to basename, size, and sha256 of the downloaded bytes; plus a rejection case for content+file_path together.
go vet ./... clean, go test ./... green for every package except operation/actions — TestDispatchWorkflowFn_LegacyFallbackOn204 fails identically on pristine main (b98a5e8) and this PR does not touch that package.

Output bounding

Response shape is unchanged (a single Attachment JSON); no data-dependent size introduced.

Note on transports

file_path assumes the MCP server shares a filesystem with the agent (stdio, or a local proxy such as mcp-hub). Over remote SSE/HTTP the base64 mode remains the only option — documented in the demo.

🤖 Generated with Claude Code

## Why `create_issue_attachment` / `create_comment_attachment` only accepted `content` as a base64 string, which has to travel through the model's context window. A 68 KiB zip is ~91k base64 characters (~86k tokens) — no tool call carries that, and `Read`/`Bash` cap far below it. Anything beyond a few KiB was effectively un-uploadable from an agent. The download side already solved the mirror problem (`download_*_attachment` streams files over the inline cap to disk and returns only the `path`, `operation/attachment/attachment.go` `downloadResultFor`). This PR adds the counterpart for upload. ## What - New optional `file_path` (absolute path on the MCP server's host) on both create tools; `content` becomes optional. Exactly one of the two must be given, otherwise a clear error. - `filename` defaults to the basename of `file_path`; `mime_type` is inferred from the extension when omitted (the param description always claimed this — now it is true for both modes). - One shared helper (`operation/attachment/upload.go`: `resolveUploadSource` / `uploadAttachment`) replaces the duplicated decode+multipart code in both handlers. - Size guard before buffering: the helper asks the instance for `GET /settings/attachment` (`max_size`, MiB) and rejects oversized files naming the real limit; if the endpoint is unavailable, a documented 50 MiB fallback (`FallbackMaxUploadBytes`) applies. - Error cases: missing file, directory, non-regular file, unreadable, relative path, both inputs given. - Docs: `README.md`, `demos/issue-attachments.md`, `docs/plans/issue-attachments.md`, `operation/params/params.go`, `extension/manifest.json`. ## Tests - `operation/attachment/upload_test.go`: helper unit tests (input selection, defaults, overrides, MIME inference, every error case, instance limit vs fallback) plus handler-level tests for issue and comment scope asserting the multipart part carries the file bytes, basename and `application/zip`. - Existing `TestCreateIssueAttachmentFn_DecodesBase64AndUsesMultipart` adjusted to pick the POST record instead of assuming order (the settings preflight now precedes it). - `test/e2e/attachments.sh`: new step 2b uploads via `file_path`, checks name defaulted to basename, size, and sha256 of the downloaded bytes; plus a rejection case for `content`+`file_path` together. - `go vet ./...` clean, `go test ./...` green for every package except `operation/actions` — `TestDispatchWorkflowFn_LegacyFallbackOn204` fails identically on pristine `main` (b98a5e8) and this PR does not touch that package. ## Output bounding Response shape is unchanged (a single `Attachment` JSON); no data-dependent size introduced. ## Note on transports `file_path` assumes the MCP server shares a filesystem with the agent (stdio, or a local proxy such as mcp-hub). Over remote SSE/HTTP the base64 mode remains the only option — documented in the demo. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Phil added 1 commit

2026-09-03 13:50:40 +00:00

feat(attachment): upload from a server-side file path f116bbbea9

create_issue_attachment and create_comment_attachment only accepted
`content`, a base64 string that has to travel through the model's
context window. A 68 KiB zip becomes ~91k base64 characters (~86k
tokens): no tool call carries that, and Read/Bash cap far below it.
Anything beyond a few KiB was effectively un-uploadable from an agent.

The download side already solved the mirror problem — files over the
inline cap are streamed to disk by the MCP server and only the path is
returned. This adds the counterpart for upload: an optional `file_path`
(absolute, regular file) that the server opens itself. Exactly one of
`content` / `file_path` must be given; `filename` defaults to the
basename, `mime_type` is inferred from the extension when omitted
(the parameter description always claimed this; it is now true).

Both create tools share one helper instead of duplicating the decode +
multipart code. Before buffering, the helper asks the instance for its
attachment limit (GET /settings/attachment, max_size in MiB) so an
oversized file is rejected with the real limit named; if the endpoint
is unavailable a documented 50 MiB fallback applies.

Missing / unreadable / directory paths and content+file_path together
each fail with a clear message. Unit tests cover the helper and both
handlers; the e2e script gains a file_path round-trip with sha256
check and a rejection case. Docs, params and the extension manifest
follow.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Phil merged commit 20e5aaba92 into main

2026-09-03 13:50:59 +00:00

Phil referenced this pull request from a commit

2026-09-03 13:51:00 +00:00

Merge pull request 'feat(attachment): upload from a server-side file path' (#16) from feat/attachment-file-path into main

Phil referenced this pull request

2026-09-03 13:54:52 +00:00

Fix failing TestDispatchWorkflowFn_LegacyFallbackOn204 on main #17