feat(gmail): make inline attachment size limit configurable - #947
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: f2fd7be807
ℹ️ 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".
| Output OutputPathFlag `embed:""` | ||
| Name string `name:"name" help:"Filename (used when --out is empty or points to a directory)"` | ||
| Inline bool `name:"inline" help:"Also return the attachment content base64-encoded (contentBase64) in the response; attachments over the inline size limit fall back to the file path with an explanatory reason"` | ||
| InlineMaxBytes int `name:"inline-max-bytes" default:"3145728" help:"Maximum attachment size --inline embeds (bytes)" env:"GOG_GMAIL_INLINE_MAX_BYTES"` |
There was a problem hiding this comment.
When --inline-max-bytes or GOG_GMAIL_INLINE_MAX_BYTES is negative, every attachment exceeds the limit, so the command succeeds without contentBase64 and reports a nonsensical negative size limit. Validate this value before authentication or downloading so an invalid flag or environment setting produces a usage error instead of silently changing the response.
Useful? React with 👍 / 👎.
| Output OutputPathFlag `embed:""` | ||
| Name string `name:"name" help:"Filename (used when --out is empty or points to a directory)"` | ||
| Inline bool `name:"inline" help:"Also return the attachment content base64-encoded (contentBase64) in the response; attachments over the inline size limit fall back to the file path with an explanatory reason"` | ||
| InlineMaxBytes int `name:"inline-max-bytes" default:"3145728" help:"Maximum attachment size --inline embeds (bytes)" env:"GOG_GMAIL_INLINE_MAX_BYTES"` |
There was a problem hiding this comment.
The generated docs/commands/gog-gmail-attachment.md flag table and the usage in docs/spec.md still omit --inline-max-bytes, leaving the repository's published command reference out of sync with the CLI and making this user-facing option undiscoverable there. Run the command-reference generator and update the usage specification as part of this change.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Codex review: needs maintainer review before merge. Reviewed August 2, 2026, 5:30 AM ET / 09:30 UTC. ClawSweeper reviewWhat this changesAdds Merge readinessThis active PR remains necessary: current Priority: P2 Review scores
Verification
How this fits togetherThe Gmail attachment command downloads an attachment to a local path and, when requested, adds a base64 copy to its structured output. This PR makes the size boundary for that optional inline payload configurable without changing ordinary attachment downloads. flowchart LR
A[Gmail attachment request] --> B[Validate inline byte limit]
B --> C[Dry run or Gmail authentication]
C --> D[Fetch attachment]
D --> E[Write attachment file]
D --> F[Inline output requested]
F --> G[Base64 payload or fallback reason]
E --> H[Structured CLI response]
G --> H
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land the focused configuration support with the unchanged 3 MiB default, negative-value rejection, dry-run disclosure, documented command surface, and existing fallback-to-path behavior. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR adds an opt-in configuration capability rather than fixing a reported current-main failure. Its no-credential dry-run path is directly demonstrated by the maintainer-provided built-binary proof. Is this the best way to solve the issue? Yes. Applying a validated limit to the established inline fallback path is the narrowest maintainable solution: it preserves existing output by default and avoids a parallel attachment-delivery mode. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 455caec23623. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (23 earlier review cycles; latest 8 shown)
|
clawsweeper
Bot
added
merge-risk: 🚨 availability
f2fd7be to
6f39a0d
Compare
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Maintainer follow-up: I fixed the dry-run observability gap on the contributor branch in 94a4431.
Proof:
My recommendation is LAND. The default remains byte-for-byte compatible, the configurable decision is entirely local after the normal Gmail attachment fetch, and the real binary now makes the new boundary inspectable without credentials. |
clawsweeper
Bot
added
proof: sufficient
2 participants
Summary
gmail attachment --inlineembeds the attachment content as base64 (contentBase64) in the response, but the size it will embed is capped at a hardcoded 3 MiB. This makes that cap configurable while keeping the current default, so nothing changes unless you ask for it.--inline-max-bytesongmail attachment(default3145728, i.e. the current 3 MiB).GOG_GMAIL_INLINE_MAX_BYTES.reason.Motivation
Automation that consumes attachments only through
--inline— for example a confined agent that cannot read gog's0600output files directly — needs to pull attachments larger than 3 MiB without a separate file read. A fixed 3 MiB cap forces those callers to give up on anything bigger.User-facing changes
--inline-max-bytes <bytes>ongmail attachment.GOG_GMAIL_INLINE_MAX_BYTES.Testing
go test ./internal/cmd/— the oversized-fallback test now drives the boundary crossing through the flag (--inline-max-bytes) rather than a hardcoded constant.make lint— 0 issues.