feat(gmail): add --include-attachments to messages search - #962
Open
ronny-rentner wants to merge 4 commits into
Open
feat(gmail): add --include-attachments to messages search#962ronny-rentner wants to merge 4 commits into
ronny-rentner wants to merge 4 commits into
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
Contributor
|
Codex review: needs real behavior proof before merge. Reviewed August 8, 2026, 1:26 AM ET / 05:26 UTC. ClawSweeper reviewWhat this changesThe PR adds Merge readiness⛔ Blocked until real behavior proof is added - 9 items remain Keep open: this is a useful opt-in Gmail feature, but attachment-only search currently retrieves full message payloads, including inline body data, and its text output truncates the attachment IDs needed for downloads. Priority: P2 Review scores
Verification
How this fits togetherGmail message search first lists matching messages, then fetches detail for each result to produce summaries, optional bodies, and attachments. The new path controls what Gmail data is requested and then emitted to JSON or tabular output. flowchart LR
Query[Search query] --> List[Matching message IDs]
List --> Detail[Message detail request]
Detail --> Decision{Requested detail}
Decision --> Attachments[Attachment metadata collection]
Attachments --> Output[JSON or table output]
Decision --> Bodies[Message body data]
Decision needed
Why: The current implementation cannot both request the entire payload and truthfully promise that bodies are not transferred; choosing that privacy boundary is maintainer policy, not a mechanical review choice. Before merge
Findings
Agent review detailsSecurityNeeds attention: The new attachment-only path weakens data minimization by requesting full Gmail payloads containing body data. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Preserve the attachment-only privacy guarantee with a Gmail-supported metadata-only retrieval strategy, retain full download-capable IDs in text output, and demonstrate the corrected live request/result with redacted evidence. Do we have a high-confidence way to reproduce the issue? Yes, from source: enable the new flag for a message with inline body content and the request selects the entire Gmail payload; the added HTTP test also supplies body data while checking only that it is not printed. Is this the best way to solve the issue? No: suppressing body output is not equivalent to avoiding body retrieval, and shortening an opaque identifier makes text results unusable for the existing download command. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8fe3e7995d0b. LabelsLabel justifications:
EvidenceSecurity concerns:
What I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (9 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
--include-attachmentstogmail messages search. It lists each message's attachment metadata (filename, size, mimeType, attachmentId) next to the subject, without fetching message bodies.Today the only way to see a message's attachments in search results is
--include-body, which also decodes and returns 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.format=fullwith a fields mask that selects only the attachment part metadata —payload(headers,parts(filename,mimeType,body(size,attachmentId)))— so no body/data is transferred, just the attachment list.Motivation
Lets a caller enumerate what a message carries — and grab the attachmentIds it needs to download — without pulling message bodies.
User-facing changes
--include-attachmentsongmail messages search.GOG_GMAIL_INCLUDE_ATTACHMENTS(mirrors the existingGOG_GMAIL_INLINE_MAX_BYTESpattern).attachmentsarray in the JSON output, only when the flag is set.Testing
TestExecute_GmailMessagesSearch_IncludeAttachments: the flag lists attachments and does not include the body; the default search lists neither.make fmt/make lintclean; command docs regenerated (make docs-commands).