feat(gmail): add --include-attachments to messages search - #962
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54c58df304
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| items, err := fetchMessageDetails(ctx, svc, messages, idToName, loc, c.IncludeBody, c.BodyFormat) | ||
| items, err := fetchMessageDetails(ctx, svc, messages, idToName, loc, c.IncludeBody, c.BodyFormat, c.IncludeAttachments) |
There was a problem hiding this comment.
When --include-attachments is used with the default table output or --plain, this path fetches and populates messageItem.Attachments, but the later call to gmailMessageColumns(c.IncludeBody, c.Full) never reads that field. The flag therefore performs the additional full-format API requests while producing exactly the same stdout as a search without the flag. Pass the option into the presentation layer and render the metadata, or explicitly restrict and document the flag as JSON-only.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Codex review: needs maintainer review before merge. Reviewed August 8, 2026, 11:16 PM ET / August 9, 2026, 03:16 UTC. ClawSweeper reviewWhat this changesThe PR adds an opt-in Gmail message-search flag and environment variable that return attachment metadata in table and JSON output without rendering decoded message bodies. Merge readinessKeep open: current main does not implement this opt-in Gmail search capability, and the final patch has resolved the prior table-output safety finding. The supplied redacted live terminal output is sufficient real-behavior proof; no actionable correctness or security finding remains. Priority: P2 Review scores
Verification
How this fits togetherGmail search lists message IDs, optionally fetches each message’s details, then formats results as a table or JSON. This option switches the detail fetch to include attachment metadata and passes it to the existing output surfaces. flowchart LR
A[Gmail search query] --> B[Message ID list]
B --> C[Optional full detail fetch]
C --> D[Attachment collection]
D --> E[Table or JSON output]
Before merge
Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Land the narrow opt-in option after the pending checks finish, retaining metadata-only fetches and unchanged output for existing searches. Do we have a high-confidence way to reproduce the issue? Not applicable as a defect report; the PR includes redacted after-fix terminal output from a live Gmail account for both table and JSON behavior. Is this the best way to solve the issue? Yes. It reuses the existing attachment output shape, preserves default search behavior, and confines the new fetch behavior to an explicit option. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 3fecbc007a5b. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (13 earlier review cycles; latest 8 shown)
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
…t/deep parts in attachment mask
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
…meType/id in text search output
clawsweeper
Bot
added
the
merge-risk: 🚨 security-boundary
…n the search text column
…stom attachment field mask
…h text column (filename, mimeType, size only)
|
On the "render MIME type and identifier in text output" finding: mimeType is now in the 1. The full id doesn't fit a table column. Gmail's 2. A short id isn't feasible either. gogcli already uses a short (first-8-char) form of the and an id captured in one call still downloaded fine in a separate later call. Because the id changes on every call, its short form changes on every search too — it would be neither stable nor a match for a later download's filename, so it conveys nothing a caller can act on. So the column shows |
clawsweeper
Bot
added
proof: sufficient
clawsweeper
Bot
removed
the
merge-risk: 🚨 security-boundary
|
Landed in Maintainer fix before landing:
Proof:
Thanks @ronny-rentner for the careful iteration and live evidence. |
2 participants
Summary
Adds
--include-attachmentstogmail messages search. With it, each result carries its attachment metadata — filename, size, mimeType (and theattachmentIdin JSON) — so you can see what a message holds without switching to--include-body.Today the only way to see a message's attachments in search results is
--include-body, which also decodes and renders the full body.--include-attachmentsgives just the attachment list.--include-attachments(envGOG_GMAIL_INCLUDE_ATTACHMENTS).attachmentOutputshape (the same one--include-bodyalready produces), so the JSON is purely additive — no existing field changes.ATTACHMENTScolumn (filename (mimeType, size)); JSON gains theattachmentsarray (with theattachmentId).format=full— the same complete fetchgmail get/thread/draftsalready use — so attachments at any MIME nesting depth are listed. The body data is transferred but never rendered.Motivation
Lets a caller enumerate what a message carries — and, from the JSON, get the
attachmentIdto download — without switching to--include-body.User-facing changes
--include-attachmentsongmail messages search.GOG_GMAIL_INCLUDE_ATTACHMENTS(mirrors the existingGOG_GMAIL_INLINE_MAX_BYTESpattern).ATTACHMENTScolumn in text output and an additiveattachmentsarray in JSON.Testing
TestExecute_GmailMessagesSearch_IncludeAttachments: lists an attachment nested several MIME levels down (proving the full fetch), does not render the body, and asserts the fetch usesformat=fullwith no capping parts mask; the default search lists neither.make ciclean (fmt / lint / deadcode / tests / docs).Verification
Redacted terminal output from the built binary against a live Gmail account (ids masked,
attachmentIdtruncated; sizes, mimeType, sender, subject verbatim).Text output — the
ATTACHMENTScolumn:JSON — same metadata plus the per-fetch
attachmentId: