Let an assistant read the files on an issue, not just post them - #284
Conversation
… them attach_file has existed since the start, so an assistant could upload a report and link it. It could not see a single file anybody else attached. get_issue returned no attachments, no tool listed them, and none returned their contents, so a thread saying "see the attached spec" was unreadable: the assistant could not tell the file was there, let alone open it. list_issue_attachments returns everything hanging off an issue and off every comment on it. read_attachment returns the contents, text-like types as text and anything else base64, truncated with a flag rather than silently cut. get_issue now reports the list the way it already reports labels and relations. Reads are governed by the issue the file hangs off, through the same assertCan and team check comments already use, so a file is exactly as readable as the issue it belongs to and no more. The storage driver gains get(), which the S3 implementation already had the pieces for. attach_file was also undocumented. All three are in docs/mcp.md now.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 39 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
… about docs Two gaps found by using it rather than reading it. attach_file writes to four parent types and the reads only covered two, so an assistant could post a file onto a doc or a project and never open it again. read_attachment now resolves all four, each through that type's own gate: an issue file by the issue, a comment file by its issue, a doc file by loadReadableDoc, a project file by getProject. list_attachments lists one parent of any of the four. list_issue_attachments stays, because it is the only one that spans an issue and its comments. The inbox resolved an issue for issue and comment notifications and nothing at all for the others. A mention inside a doc comment carries entityType doc_comment, so it arrived with issue: null and no other context, and an assistant following the protocol would call get_issue on it and fail. In production that produced an answer about something else entirely, posted on a doc that had asked for a code review. Each row now resolves its doc the same way it resolves its issue.
…nt's files Both from review. Truncating at a byte offset can land inside a multi-byte character, so a UTF-8 text file cut mid-character came back with a replacement character and text that did not match the file. read_attachment now trims back to the last whole character before decoding, so a truncated read is always a genuine prefix. commentIssueId resolved soft-deleted comments, so a caller holding an attachment id could still read a file whose comment had been deleted, while list_issue_attachments and get_issue correctly hid it. The lookup now excludes deleted comments, and the two agree. Both are mutation-verified: restoring the naive decode fails the truncation test at every cut position, and removing the deletedAt filter fails the deletion test.
attach_filehas existed since the start, so an assistant could upload a report and link it. It could not see a single file anybody else attached.get_issuereturned no attachments, no tool listed them, and none returned their contents. A thread saying "see the attached spec" was unreadable: the assistant could not tell the file was there, let alone open it.Found while checking what the Orbit integration could actually do for yodu's agents. I attached a generated markdown file through
attach_file(worked), then calledget_issueon the same issue and got nothing back about it.What changes
list_issue_attachmentsreturns everything hanging off an issue and off every comment on it, oldest first.read_attachmentreturns the contents. Text-like types come back as text, anything else base64. A large file is truncated withtruncated: truerather than silently cut, default 256KB.get_issuenow reports the attachment list, the way it already reports labels and relations, so an assistant learns a file exists without having to ask.StorageDriver.get()is new. The S3 implementation already importedGetObjectCommandfor presigning, so this is the same command with the body read.Authorization
Reads are governed by the issue the file hangs off, not by the file. Both tools resolve a comment attachment back to its issue, then go through the same
assertCan(principal, 'issue:read')and team check thatlistCommentsalready uses. A file is exactly as readable as the issue it belongs to and no more.Two tests cover that boundary: an attachment in another workspace comes back
not_found, and a workspace member who is not on the issue's team is refused withforbidden: You are not a member of that team.Testing
Nine cases in
packages/mcp-server/tests/tools/issue-attachments.test.ts: listing across issue and comment parents, an empty issue,get_issuereporting the list, text read, comment-attachment read, binary as base64, truncation, and the two authorization boundaries.mcp-server 186 pass, core 760 pass, repo typecheck clean, comment policy and byte check clean. The one remaining repo lint warning is pre-existing in
packages/db/tests/check-source-bytes.test.tsand untouched here.Also
attach_filewas never documented. All three tools and a short section on how files work are now indocs/mcp.md.Greptile Summary
The PR adds authorized discovery and reading of files attached to issues, comments, documents, and projects, while also enriching issue and notification responses.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Reviews (3): Last reviewed commit: "fix(mcp): keep truncated text valid, and..." | Re-trigger Greptile