feat(gmail): add +reply, +reply-all, and +forward helpers#105
feat(gmail): add +reply, +reply-all, and +forward helpers#105HeroSizy wants to merge 20 commits intogoogleworkspace:mainfrom
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
🦋 Changeset detectedLatest commit: 52f2971 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
How does this compare to the request in #88? |
jpoehnelt
left a comment
There was a problem hiding this comment.
Code duplication between reply.rs and forward.rs
Missing encode_path_segment() on the message-id in URL construction
a883182 to
e6f388e
Compare
This PR is a direct implementation of the feature request in #88. It adds
One thing not yet covered: attachment forwarding. The current |
84ebbfe to
3ec8332
Compare
Thanks for the review! Both points are fixed now — Sorry about the initial state — I had an AI agent push before I'd properly reviewed. Everything's been cleaned up since. A couple of questions:
|
|
Nice work addressing the prior feedback — dedup and
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #105 +/- ##
==========================================
+ Coverage 55.19% 56.73% +1.54%
==========================================
Files 38 40 +2
Lines 13166 14127 +961
==========================================
+ Hits 7267 8015 +748
- Misses 5899 6112 +213 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3ec8332 to
a9302c6
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces +reply, +reply-all, and +forward helper commands for the Gmail CLI, which is a great feature. The implementation is mostly solid, with good test coverage and documentation. However, I've identified a few areas for improvement concerning code structure and correctness. Specifically, there's a bug in handling repeated email headers, and some refactoring is needed to resolve circular dependencies and improve module organization. Addressing these points will enhance the maintainability and robustness of the new helpers.
src/helpers/gmail/forward.rs
Outdated
| .await | ||
| .map_err(|e| GwsError::Auth(format!("Gmail auth failed: {e}")))?; | ||
| let client = crate::client::build_client()?; | ||
| let orig = super::reply::fetch_message_metadata(&client, &t, &config.message_id).await?; |
There was a problem hiding this comment.
The forward module should not depend on the reply module for core functionalities like fetching message metadata. The fetch_message_metadata function and the OriginalMessage struct are general utilities used by both replying and forwarding. They should be moved to a more central location, like src/helpers/gmail/mod.rs, to improve code organization and reduce coupling between helpers.
There was a problem hiding this comment.
moved OriginalMessage and fetch_message_metadata into gmail/mod.rs so reply and forward share the same helper.
d0679f8 to
2b37938
Compare
|
can you rebase this and/or push another commit to trigger the github actions |
Add first-class reply and forward support to the Gmail helpers, addressing the gap described in googleworkspace#88. These commands handle the complex RFC 2822 threading mechanics (In-Reply-To, References, threadId) that agents and CLI users struggle with today. New commands: - +reply: reply to a message with automatic threading - +reply-all: reply to all recipients with --remove/--cc support - +forward: forward a message with quoted original content
- Use crate::validate::encode_path_segment() on message_id in fetch_message_metadata URL construction per AGENTS.md rules - Update auth::get_token calls to pass None for the new account parameter added on main
- Add send_raw_email() to mod.rs: shared encode→json→auth→execute pattern for sending raw RFC 2822 messages via users.messages.send - Simplify handle_reply: delegate send logic to send_raw_email - Simplify handle_forward: delegate send logic to send_raw_email Addresses code duplication feedback from PR review.
The handlers read matches.get_flag("dry-run") but the flag was missing
from the clap command definitions, so it always returned false. Now
dry-run works for +reply, +reply-all, and +forward.
Same class of bug fixed for +reply/+reply-all/+forward — the handler
reads matches.get_flag("dry-run") but the arg was not registered.
- Prefer Reply-To over From when selecting reply recipients, fixing incorrect routing for mailing lists and support systems - Use exact email address comparison instead of substring matching for --remove filtering and sender deduplication, preventing unintended recipient removal (e.g. ann@ no longer drops joann@)
- extract_email: malformed input (no closing bracket), empty string, whitespace-only - build_reply_all_recipients: display-name sender exclusion, --remove with display name, extra --cc, CC becomes None when all filtered, case-insensitive sender exclusion
Corrects how `build_reply_all_recipients` handles multi-address `Reply-To` headers. Previously, only the first address from `Reply-To` was used for deduplication, leading to potential redundancy by including those addresses in the `Cc` field. The updated logic now parses all addresses in `Reply-To`, ensuring they are fully moved to the `To` field and properly excluded from `Cc`.
reply.rs and forward.rs were missing the copyright header that all other source files in the repo include.
parse_reply_args used get_one("remove") which panics when called
from +reply (which does not register --remove). Switch to
try_get_one to safely return None for unregistered args.
Skip auth and message fetch when --dry-run is set by using placeholder OriginalMessage data. This lets users preview the request structure without needing credentials.
Replace naive comma-split with split_mailbox_list that respects quoted strings, so display names containing commas like "Doe, John" <john@example.com> are handled correctly in reply-all recipient parsing, deduplication, and --remove filtering.
split_mailbox_list toggled quote state on every `"` without accounting for backslash-escaped quotes (`\"`), causing display names like `"Doe \"JD, Sr\""` to split incorrectly at interior commas. Track `prev_backslash` so `\"` inside quoted strings is treated as a literal quote character rather than a delimiter toggle. Double backslashes (`\\`) are handled correctly as well.
- Use reqwest .query() for metadata params per AGENTS.md convention - Add MIME-Version and Content-Type headers to raw messages - Add --from flag to +reply, +reply-all, +forward for send-as/alias - Narrow ReplyConfig/ForwardConfig visibility to pub(super) - Refactor create_reply_raw_message args into ReplyEnvelope struct
- Exclude authenticated user's own email from reply-all CC by fetching user profile via Gmail API - Use format=full to extract full plain-text body instead of truncated snippet for quoting and forwarding - Deduplicate CC addresses using a HashSet - Reuse auth token from message fetch in send_raw_email to eliminate double auth round-trip - Propagate auth errors in send_raw_email instead of silently falling back to unauthenticated requests - Use consistent CRLF line endings in quoted and forwarded message bodies per RFC 2822
8eba0b0 to
b5f2988
Compare
@jpoehnelt Done, rebased |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces +reply, +reply-all, and +forward helper commands for Gmail, which is a great enhancement. The implementation is thorough, with robust handling of email headers, recipient logic, and comprehensive tests. I've found one critical issue related to parsing email headers that could lead to silently dropping recipients if the Gmail API returns multiple headers for To, Cc, or Reply-To. My review includes a suggestion to fix this.
| "Reply-To" => parsed.reply_to = value.to_string(), | ||
| "To" => parsed.to = value.to_string(), | ||
| "Cc" => parsed.cc = value.to_string(), |
There was a problem hiding this comment.
The current implementation for parsing headers overwrites values for To, Cc, and Reply-To if they appear multiple times in the API response. While the Gmail API typically combines these into a single comma-separated header, the specification allows for multiple headers. If the API were to return multiple headers for these fields, recipients would be silently dropped.
To make this more robust, you should concatenate values for these address-list headers with a comma.
"Reply-To" => {
if !parsed.reply_to.is_empty() { parsed.reply_to.push_str(", "); }
parsed.reply_to.push_str(value);
},
"To" => {
if !parsed.to.is_empty() { parsed.to.push_str(", "); }
parsed.to.push_str(value);
},
"Cc" => {
if !parsed.cc.is_empty() { parsed.cc.push_str(", "); }
parsed.cc.push_str(value);
},
Description
Closes #88 — adds first-class reply and forward support to the Gmail CLI helpers.
+reply— Reply to a message by ID. Automatically fetches the original message, setsIn-Reply-To,References, andthreadIdheaders, quotes the original, and sends viausers.messages.send.+reply-all— Same as+replybut addresses all original To/CC recipients. Supports--removeto drop recipients and--ccto add new ones.+forward— Forward a message to new recipients with a standard forwarded-message block (From, Date, Subject, To, Cc). Supports optional--bodyfor a note above the forwarded content.All three commands support
--dry-runfor safe previewing (works without auth credentials).Address handling details:
Reply-TooverFromwhen selecting reply recipients (mailing lists, support systems)Reply-Toheaders (e.g.,list@example.com, owner@example.com) and deduplicates CC against the full set"Doe, John" <john@example.com>) are handled correctly--removefiltering and sender exclusion — e.g., removingann@example.comdoes not affectjoann@example.comKnown limitation:
+forwardcurrently forwards the message text/snippet only. Full MIME forwarding with attachments will be addressed in a follow-up PR (ref #88).New files
src/helpers/gmail/reply.rs+replyand+reply-alllogic, message metadata fetching, RFC 2822 header constructionsrc/helpers/gmail/forward.rs+forwardlogic, forwarded message formattingModified files
src/helpers/gmail/mod.rs+reply/+reply-all/+forwardsubcommands, add handler dispatchDry Run Output:
+reply:+reply-all:+forward:Checklist:
AGENTS.mdguidelines (no generatedgoogle-*crates).cargo fmt --allto format the code perfectly.cargo clippy -- -D warningsand resolved all warnings.pnpx changeset) to document my changes.